DRAFT Add OPC UA data channels and the opc.quic transport - #4240
Draft
marcschier wants to merge 50 commits into
Draft
DRAFT Add OPC UA data channels and the opc.quic transport#4240marcschier wants to merge 50 commits into
marcschier wants to merge 50 commits into
Conversation
The OPC UA Data Channels errata defines a streaming primitive OPC UA does
not have: a named, authorized, flow-controlled, bidirectional stream of
opaque bytes multiplexed onto an existing SecureChannel. Nothing had ever
implemented it, so nothing had pressure-tested it.
This adds the framing layer and the generated Service, DataType and
StatusCode surface it needs.
Generated from the model compiler inputs rather than hand-written, so the
new Services take exactly the path every standard Service takes:
UA Core Services.xml OpenDataChannel, ModifyDataChannel,
CloseDataChannel, the three enums and the four
structures
StandardTypes.xml HasDataChannel, IDataChannelSourceType,
DataChannelSourceType,
DataChannelCapabilitiesType, the three event
types and the ServerCapabilities instance
StandardTypes.csv NodeIds pinned in the provisional 65000+ block
the errata assigns, so a future assignment by the
OPC Foundation is a one-line change
UA Status Codes.xml the twelve new StatusCodes
KeyValuePair moves from StandardTypes.xml to UA Core Services.xml. It has
to: a ServiceType parameter is resolved against the type dictionary alone,
and DataChannelParametersDataType.ContentParameters is a KeyValuePair
array. The generated type is unchanged, which the whole solution building
against it demonstrates.
The engine lives in Opc.Ua.Core/Stack/DataChannels:
Framing the STR MessageChunk, the twelve-byte stream header, the
optional Deadline, seven frame types, five flags, and a
decoder that bounds-checks every field against the length
its own FrameType and flags imply before reading it
Sequencing serial arithmetic over 2^32-1 rather than 2^32, because
zero is excluded from the value space and using 2^32 turns
the wrap into a spurious gap; the replay window and the
absolutely bounded GAP-run set
FlowControl per-direction channel and connection credit, the zero-start
bootstrap, and the replenishment obligation without which a
receiver can legally stall a channel forever
Scheduling the send queue, FrameSequenceNumber assignment at enqueue,
and deadline expiry that reports one GAP run per contiguous
discard
DataChannel the per-direction state machine, including that Paused and
Closing are both per-direction and that receiving END never
starts the local drain clock
Manager deficit round robin, the connection window, ChannelId
allocation and the bounded buffering of frames that
overtake their own OpenDataChannel response
Service traffic keeps its precedence structurally rather than by a second
scheduler: the transport already serializes writes in arrival order, so a
MSG, OPN or CLO chunk that becomes ready while a frame is being written is
admitted immediately after it.
STR dispatch is wired into both channel implementations but is inert until
EnableDataChannels is called, which is what the interoperability rule
requires of a peer that does not implement the errata: a capable peer never
speaks first.
Every new public type carries [Experimental("DataChannels")] under
#if NET8_0_OR_GREATER, matching the existing KeyCredentialBridge pattern.
Verified against the specification's own bytes: the thirteen published hex
vectors decode to the expected fields and re-encode byte for byte, and the
framing and sequencing test assertions carry their DCF and DCP identifiers
in the test names.
Found while implementing: the errata gives OpenDataChannel a
transportChannelId parameter in both the request and the response. No OPC
UA service reuses a parameter name across the two, and the model compiler
enforces it, so the response parameter is revisedTransportChannelId here
and the specification needs the same correction.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: e1a54492-72a7-420f-bbda-38ae8c07cb92
The framing layer landed in the previous commit; this adds the layer above it: what a server does when a client asks for a channel, and the one security check the QUIC transport cannot be built without. Negotiation (Part 4 errata 5.1.1). The server revises rather than rejects wherever it can, because a client that asked for more than it can have usually wants the largest amount available. Direction and DeliveryMode are the two exceptions and are rejected outright: silently downgrading to a stronger guarantee adds unbounded latency to a media channel, and silently downgrading to a weaker one loses data. Zero means "no preference" in every numeric member -- except Priority, where zero is the lowest of the eight real priorities and 255 is the sentinel. Without that distinction IDataChannelSourceType.Priority could never take effect. InitialCredit is revised *up* to at least MaxFrameSize, because a window smaller than one frame is an immediate deadlock: the channel opens Paused and the first frame can never be sent. Service handler. Every Service in the set is scoped to both the SecureChannel and the authorizing Session, not to the SecureChannel alone. OPC 10000-4 permits several Sessions on one SecureChannel and they share one ChannelId space; since ChannelIds are allocated monotonically from one and are therefore trivially guessable, SecureChannel-only scoping would let one user enumerate and seize another's channels. A channel owned by another Session returns Bad_DataChannelIdInvalid, indistinguishable from an unassigned identifier. Authorization is re-evaluated rather than granted once, because a channel is long-lived and moves content out of the server continuously and outside the Service path -- a permission checked only at open is a permission that cannot be revoked. The SecurityMode floor is enforced by the *server*: a rule only the attacker is asked to obey is not a rule. Offers are single-use, scoped to their SecureChannel and expiring, so an unsubscribed client leaks nothing. QUIC peer binding (Part 6 errata 7.6.1). The TransportSecured profile rests on the premise that the TLS connection carrying the frames terminates at the same application the control stream authenticated. That premise is not self-evident: any party able to terminate QUIC between the two byte-forwards the end-to-end-secured control stream so that OpenSecureChannel, CreateSession and ActivateSession all succeed and every certificate check passes, while reading, modifying, dropping and injecting every frame in the clear. Both ends would report a fully authenticated SignAndEncrypt channel. So the binding is by key, not by name. Equality of an ApplicationUri subjectAltName is necessary but not sufficient -- CA and GDS implementations commonly populate that SAN from the requester's own CSR without checking it against an authoritative registry, so an attacker holding any certificate from an accepted anchor could otherwise name the victim's ApplicationUri. The test for DCQ-007 builds exactly that certificate and shows the two are separated only by the key comparison. Six more tests, 76 in total, all green. Documented in docs/DataChannels.md, including what is implemented and what is not. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: e1a54492-72a7-420f-bbda-38ae8c07cb92
Three things the previous two commits left open. The SequenceNumber budget. OPC 10000-6 6.7.2.4 forbids reusing a SequenceNumber under one TokenId and discharges that by assuming the token lifetime is short relative to the chunk rate. Data channels invalidate the assumption, not the rule: STR frames consume SequenceNumbers at the data rate of the channel rather than at Service call rate. On a 10 Gbit/s link minimum-size frames exhaust the 32-bit space in about two and a half minutes, well inside a one-hour token. That is not cosmetic. Where a SecurityPolicy derives a per-chunk IV or AEAD nonce from the SequenceNumber, reuse under one key is a cryptographic failure; where it does not, replay detection silently degrades, because a stack checking only "incremented by exactly one" accepts the wrap. The budget renews early *and* stalls late, because a slow renewal can be overtaken by a fast channel: SendDataChannelFrameAsync refuses the chunk that would reuse a value rather than renumbering it. The renewal threshold is the lesser of 2^30 values and one minute of measured traffic, which is what makes it work at both ends of the rate range -- a slow channel renews on the fixed headroom and never renews needlessly, a channel fast enough to burn 2^30 inside a minute renews on its own rate. The opc.quic binding, in its own net8.0+ assembly because System.Net.Quic needs a runtime Opc.Ua.Core cannot require. It carries the UACP conversation on the first client-initiated bidirectional stream byte for byte as over opc.tcp -- so it implements IUaSCByteTransport and the UASC pipeline above it is unchanged -- and adds IMultiplexedByteTransport for the stream-per-data-channel mapping that the chunk-at-a-time interface cannot express. RESET is realized as QUIC RESET_STREAM carrying the StatusCode as its application error code. Fallback is not permitted to be a downgrade. Blocking UDP on 4840 is a single firewall rule, so an unconditional fallback would hand an off-path attacker a downgrade primitive; IsAcceptableFallback refuses any endpoint weaker than the one required of the QUIC endpoint. Unreliable datagrams are *not* implementable here and the binding says so rather than pretending. QuicConnection exposes no RFC 9221 datagram API through .NET 10, so MaxDatagramSize is zero, SupportsUnreliableDatagrams is false, and a request for Unreliable or PartiallyReliable is refused with Bad_DeliveryModeUnsupported -- which is what the errata requires, because silently carrying them on the channel's stream would deliver a reliability guarantee the application did not ask for and did not budget latency for. A loopback suite that runs two managers against each other over an in-memory transport, encoding and decoding every frame on the way so the loop exercises the real codec rather than passing objects across. It is what turns the per-clause unit tests into evidence that the parts compose: the connection-credit bootstrap actually unblocks both directions, ascending delivery holds under load, a PING is answered while credit is exhausted, receiving END really does leave the receiver's own direction open, and a RESET carrying Good reaches Closed on both peers while a Bad one reaches Faulted. Close, Reset and TryPing move from internal to public on DataChannel. An application holding a channel has to be able to close it; that they were internal was an oversight the loopback tests exposed immediately. 98 tests on net8.0 and net10.0, 93 on net472, all green. Solution builds across all six target frameworks with no new warnings. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: e1a54492-72a7-420f-bbda-38ae8c07cb92
The QUIC binding previously had the constants, the fallback rule and the TLS key binding, but nothing that established a connection. This makes opc.quic a transport a client can actually connect over: connect, open a SecureChannel, create a Session, call Services, and carry data channels on per-channel QUIC streams. The listener is plumbing rather than a rewrite. Opc.Ua.Bindings.Https already proves an ITransportListener can live outside Core and drive a TcpServerChannel through Attach and StartReceiveLoop, so the QUIC listener accepts a connection, accepts its first bidirectional stream as the control stream, and hands that to the existing UASC server implementation. Chunking, security, token renewal and session dispatch are reused untouched. The binding targets net9.0 and net10.0. System.Net.Quic is still behind [RequiresPreviewFeatures] on net8.0, and opting in would emit an assembly attribute that every consumer would have to opt into in turn. Core itself still builds for all six target frameworks. ALPN is offered and enforced. A QUIC endpoint serving some other protocol on the same port is abandoned rather than mistaken for an OPC UA Server. Data channels over QUIC set HasTransportFlowControl, so no CREDIT frame is sent or expected and Paused follows QUIC's own blocking; duplicating the window in two layers gains nothing and deadlocks when the two disagree. RESET becomes RESET_STREAM carrying the StatusCode as its application error code. Unreliable datagrams stay refused with Bad_DeliveryModeUnsupported. QuicConnection exposes no RFC 9221 datagram API through .NET 10, so SupportsUnreliableDatagrams is False. Refusing is what the errata requires; silently carrying an unreliable channel on a reliable stream is not. Fix a scheduler bug that capped a channel at one quantum per idle tick. A round's deficit is (Priority + 1) x MaxFrameSize, one frame, and the loop then waited 20 ms for the next tick, so a saturated channel moved about fifty frames a second. A round that leaves payload queued now schedules the next one immediately. The sample measured 0.5 Mbit/s before and 1.3 Gbit/s after, and ManyFramesDrainWithoutWaitingForTheIdleTick fails if the wake is dropped again. The unit tests never caught this because none of them measured time; the bug surfaced the first time the sample was run. Also wires the DataChannelCapabilities Object, whose absence is precisely how a server declares it does not support data channels, and adds samples/ConsoleDataChannelStreaming, which drives the same application code over --transport tcp and --transport quic against the same server. 103 data channel tests pass on net9.0 and net10.0, 94 on net472 and net8.0 where the QUIC tests do not apply. Every QUIC test is guarded by QuicConnection.IsSupported so an agent without msquic skips rather than fails. Opc.Ua.Core.Tests shows no regression at 4134 passing. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: e1a54492-72a7-420f-bbda-38ae8c07cb92
Coverage of the data channel and opc.quic code was 49.6%. The existing 103 tests concentrated on framing, sequencing and negotiation -- the parts easiest to unit test -- while roughly a quarter of the code had no test referencing it at all. This adds 123 tests across eleven files and takes line coverage to 87.6%. The gaps that mattered were not the largest ones. DataChannelServiceHandler is the whole server-side Open/Modify/CloseDataChannel surface, the most specification-visible code in the change, and none of its eleven StatusCode branches were asserted. QuicTransportListener is the accept path, and the existing QUIC harness built raw System.Net.Quic listeners itself, so the listener had never run in a test. Both are now covered end to end, which retires the standing caveat that the listener had never been exercised through a real server flow. Measuring first also found a file the reference map had missed entirely: Stack/Tcp/UaSCBinaryChannel.DataChannels.cs, the inline TCP STR dispatch, at 0% across 115 lines. It sits outside Stack/DataChannels/ and only the coverage report caught it. Writing the tests found three defects, all in code the pre-existing end-to-end tests had executed without asserting. QuicTransportListener captured the TLS certificate in the accept callback's closure, so CertificateUpdate moved the UASC layer to the rotated certificate while TLS kept presenting the retired one. That breaks the very key-equality check of Part 6 errata 7.6.1 that the errata exists to enforce, and leaves a revoked or expired certificate in use until restart. The callback now reads a field, endpoint descriptions are refreshed the way the TCP listener already did, and retired certificates are held until close so an in-flight handshake is never pulled out from under. QuicConnectionBuilder.ConnectAsync caught only QuicException. An ALPN or certificate rejection surfaces from the TLS handshake as AuthenticationException, so it escaped as a raw platform exception and callers using the stack's catch (ServiceResultException) idiom missed ALPN failures entirely. It now maps to Bad_SecurityChecksFailed. DataChannel.TryPing had no channel-state guard although Write guards Closed and Faulted. On a dead channel it took a sequence number, enqueued a PING, re-woke the scheduler and latched m_pingOutstanding, so the channel could later be declared dead by a ping that should never have been sent. TryPing is public API. It now returns false, matching the Try* contract. Also guards the discard-path receive credit Release with !HasTransportFlowControl, matching TryAccount. Over QUIC the two were asymmetric, so discarded frames accumulated credit that was never accounted. Inert in practice because the only consumer sits behind the same guard, but it published a misleading diagnostic. The tests assert behaviour rather than reachability, because this code has already shown that the difference matters: the 2600x scheduler throughput bug fixed earlier on this branch had every line of the scheduling loop executed by the suite and still shipped, since nothing measured the rate. 226 tests pass on net9.0 and net10.0 and 184 on net472 and net8.0, where QUIC does not apply. Every QUIC test is guarded by QuicConnection.IsSupported so an agent without msquic skips rather than fails, and there is deliberately no build-time coverage gate because a gate would fail exactly those agents. Opc.Ua.Core.Tests shows no regression at 4134 passing, and the streaming sample still runs over both framings. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: e1a54492-72a7-420f-bbda-38ae8c07cb92
Reviewing the implementation against the errata clause by clause, rather than against itself, found four divergences. One is a deadlock that would have stopped every long-lived stream in production. Connection credit was granted once and never replenished. Part 6 errata §5.8.2 says a CREDIT frame on a non-zero ChannelId grants ChannelCredit to that channel "and, if ConnectionCredit is non-zero, that amount to the connection as well". The sender put the connection grant on the wire and the encoder wrote it, but the receiver read only ChannelCredit and dropped the rest: the connection window was reachable solely through the one-shot EnsureConnectionCreditGranted. On inline framing that window is MaxCreditPerChannel x MaxDataChannels, 16 MiB by default, after which every channel stalls permanently with no way to recover. Nothing caught it because no test and neither sample moved more than a megabyte, and the streams this errata exists to carry run for hours. The manager now applies the connection grant when it dispatches a per-channel CREDIT, and faults every channel if that grant would overflow, since the connection window belongs to no single channel. The QUIC Message header sent a zero SecureChannelId. The reasoning was that the QUIC connection already identifies the SecureChannel, but §5.1 requires the field to carry the enclosing SecureChannel under both framings, and the specification's own quic_datagram_unreliable vector carries 41340. The vectors did not catch this because SpecVectors.QuicPrefix skips the first twelve bytes, so the codec was checked from the stream header onward and the message header was never compared against anything. A test now compares the emitted header to the vector, and asserts the vector's own SecureChannelId is non-zero so the comparison cannot quietly become vacuous. A same-key certificate re-issue tore down every connection. The listener matched on thumbprint, but a re-issue that keeps the key produces a new thumbprint and an unchanged subjectPublicKeyInfo. Since the binding of §7.6.1 is by key, such a connection remains consistent and §7.6.2 requires it be left alone; matching on the thumbprint aborted every live media stream on an ordinary scheduled renewal. The listener now compares the key. Unsupported frame types and payload on a frame that carries none both mapped to Bad_DataChannelLimitsExceeded, which describes neither. The errata now names Bad_DataChannelFrameTypeUnsupported and Bad_DataChannelFrameInvalid for these two faults, and they are added to the model compiler inputs as provisional 1111 and 1112. 230 tests pass on net9.0 and net10.0, 186 on net472 and net8.0. Opc.Ua.Core.Tests shows no regression at 4134 passing. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: e1a54492-72a7-420f-bbda-38ae8c07cb92
A conformance review found that the components were right and the wiring was missing. Several obligations were implemented correctly, covered by tests, and never invoked from any production path: QuicPeerBinding.Verify had eight test references and no callers, DataChannelServiceHandler was never constructed, DataChannelManager.Remove was never called. This connects them, and fixes what connecting them exposed. Security -------- The TLS peer binding of Part 6 errata 7.6.1 now runs. That check is the whole premise of the TransportSecured profile - it is what proves the TLS peer is the OPC UA peer - and without it a TLS terminating relay would read and inject every media frame while both ends reported an authenticated SignAndEncrypt SecureChannel. The comparison itself moved from X509Certificate2.GetPublicKey, the raw subjectPublicKey bits, to PublicKey.ExportSubjectPublicKeyInfo, which is the artifact the clause names and which additionally covers the AlgorithmIdentifier. A subjectAltName check against the EndpointUrl host was added beside it. Certificate rotation now behaves as 7.6.2 requires: a key change closes the connections bound to the superseded key with a QUIC application CONNECTION_CLOSE carrying Bad_SecurityChecksFailed rather than per-stream resets that may not survive the close, connection admission is fenced against an activation epoch so a handshake in flight cannot be admitted under a retired key, and a connection is closed when its own bound certificate becomes revoked, untrusted or expired. A same-key re-issue still disturbs nothing. TLS resumption is disabled for opc.quic because .NET exposes no way to invalidate outstanding tickets at activation; the errata now names that as the conforming fallback. Correctness ----------- Securing a STR chunk is now serialized against Service traffic under the channel's DataLock. Sharing the SequenceNumber sequence, which the errata already required, also means sharing the symmetric keys and the counter that produces it, and nothing serialized the two writers. The scheduler thread and a Service response reached the same HMAC concurrently: on Windows the CNG hash provider refuses outright and faulted the scheduler round, and where it did not throw it raced for SequenceNumbers and emitted duplicates, which a peer is obliged to treat as a replay. Found by running a Client against a Server, not by any test. The Session layer rejected the DataChannel request types as an unexpected RequestType, so a real Client could not reach the Service handler even once the dispatch was wired. Also found by running it. MaxDataChannels counted channels that had already ended, so a SecureChannel refused every new channel after sixteen open-close cycles with none open. The limit now counts only non-terminal states and ended channels are released. Their identifiers are never reissued, which is what still lets a Close on one return Bad_DataChannelClosed rather than Bad_DataChannelIdInvalid. DCF-006 and DCF-007 now emit the RESET the errata requires instead of silently dropping the frame; buffered frames are replayed with their real FrameType rather than as zero-length DATA, so a buffered RESET still resets; the unknown-ChannelId buffer is bounded by encoded frame size rather than payload, so zero-payload frames cannot be queued without limit; and MaxBitrate is revised against the source instead of always returning 0, which in the same table means unconstrained. MaxBitrate could not be revised before because DataChannelParametersDataType had no such field. QUIC transport -------------- Data channel direction now maps to QUIC stream type and initiator as 7.4 requires, keyed off the OPC UA role rather than the QUIC role, so reverse connect inverts the stream types without mis-assigning channels. A SourceToSink channel allocates a server-initiated unidirectional stream and returns its id in revisedTransportChannelId; a Client-initiated direction that omits transportChannelId is refused. opc.tcp is unaffected. Server side ----------- StandardServer now serves OpenDataChannel, ModifyDataChannel and CloseDataChannel, aborts a Session's channels when it closes, raises the audit events, and drives the authorization recheck on its interval and on role changes. The sample gained a server mode that opens a data channel through a real Session over opc.tcp and streams over it, which is what found the two bugs above; the direct-manager mode is kept, since that is what found the scheduler throughput bug earlier. Not fixed, deliberately ----------------------- The non-downgrading fallback rule of 7.9 has nothing to attach to: Opc.Ua.Client contains no reference to opc.quic and no cross-scheme fallback exists anywhere in it, so a Client that never falls back conforms vacuously. Wiring the predicate would mean first inventing the fallback, which is a feature rather than a fix. Client-side 0-RTT refusal is likewise unreachable: .NET exposes no early-data control on QuicClientConnectionOptions. 255 tests pass on net9.0 and net10.0 and 196 on net472 and net8.0. Opc.Ua.Core.Tests is unchanged at 4134. The one failure in Opc.Ua.Server.Tests, ConfigureApplicationBuildsSharedClientAndServerConfiguration, was verified to fail identically on the parent commit and is not a regression from this change. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: e1a54492-72a7-420f-bbda-38ae8c07cb92
The sample now opens a data channel through a real Session against a real Server over opc.quic as well as opc.tcp, which is what the exercise was for. Getting there needed two production fixes and one missing seam. StandardServer created listeners only for schemes in the hardcoded Utils.DefaultUriSchemes list, so a base address using any other scheme was silently ignored and no listener was ever created for it. Registering the QUIC binding therefore appeared to succeed and did nothing. It now derives the schemes from the base addresses actually configured, which fixes the general case rather than only opc.quic: any transport binding outside the built-in set was previously unusable with StandardServer, with no error to say so. ServerBase did not map opc.quic to a transport profile, so even a listener that had been created advertised an endpoint with no ProfileUri. Neither is reachable from a unit test that constructs a listener directly, which is why both survived a green suite and surfaced the first time a Client tried to reach a Server over QUIC. The missing seam was on the client. The Server reaches its data channel transport through UseQuicDataChannelTransport; a Client had no equivalent, so binding its DataChannelManager to the QUIC stream the Server named in revisedTransportChannelId meant unwrapping the transport by reflection. QuicClientChannelExtensions is the mirror of the server extension, and QuicPeerBindingTransport.Inner exposes the connection the 7.6.1 peer binding wraps, so the unwrapping is part of the contract rather than a trick. Both transports verified by running them: tcp : framing Inline, transport chan id 0, 300/300 frames, 0 stalls quic : framing Quic, transport chan id 3, 300/300 frames, 0 stalls The QUIC transport channel id is the server-initiated unidirectional stream 7.4 requires for a SourceToSink channel, allocated by the Server and returned to the Client; the zero on inline framing is correct, since there is no per-channel stream there. Zero credit stalls on QUIC is also the expected reading rather than an absence of evidence: QUIC owns the flow control on that path and no CREDIT frame is exchanged. 255 tests pass on net9.0 and net10.0, 196 on net472 and net8.0. Opc.Ua.Core.Tests unchanged at 4134. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: e1a54492-72a7-420f-bbda-38ae8c07cb92
Records what the follow-up work changed: the Services are now served by StandardServer rather than merely implemented, the 7.6.1 peer binding is invoked on the connect path rather than existing only as tested and uncalled code, and the sample opens a channel through a real Session over both transports. Adds the four defects that running it found and that the suite could not reach, with why each was invisible to a test - concurrent securing of a STR chunk against Service traffic, Session refusing the request types, and StandardServer and ServerBase both ignoring any transport scheme outside the built-in list. Coverage is restated at 80.1 percent, down from 87.6 while the test count rose from 226 to 255. Wiring the uncalled obligations added production code faster than tests were added for it. It is the more honest figure: the earlier one measured a smaller body of code, much of which nothing invoked. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: e1a54492-72a7-420f-bbda-38ae8c07cb92
Six findings from a security review of the change set, four of which were fail-open: they behave correctly against every honest peer and only misbehave in the presence of an attacker, so nothing in ordinary testing revealed them. The shipped default authorizer granted everything. StandardServer fell back to a permissive IDataChannelAuthorizer that returned true unconditionally, and no other implementation existed anywhere in the tree. Part 4 §7.2 is explicit that a Server shall not grant a data channel where it would refuse a Read of the same content, so any Session - Anonymous included - could open a channel on a source whose RolePermissions deny it, and because the same object backs the periodic recheck, revocation never took effect either. The default now resolves the Session, builds the OperationContext a Service call would, and asks the owning NodeManager to validate PermissionType.Read against the source Node, so RolePermissions, UserRolePermissions and AccessRestrictions all apply without reimplementing any of them. It fails closed: an unresolvable Session or an error during validation denies. A source that is not a Node in the AddressSpace has no permissions to evaluate and remains governed by its registration, matching what ValidateRolePermissionsAsync already does for unknown Nodes. Reverse connect performed neither half of the §7.6.1 peer binding. The forward client path gets it because the transport is wrapped in QuicPeerBindingTransport; RunReverseConnectAsync built a bare transport, so QuicPeerBinding.Verify was never reached, and with ServerCertificateValidation left unset the TLS check degraded to the machine root store rather than the OPC UA trust list. An on-path attacker holding any certificate chaining to any CA in the host's store, with a SAN for the configured name, could therefore terminate TLS and byte-forward the control stream: OpenSecureChannel, CreateSession and ActivateSession all succeed with genuine certificates, both ends report an authenticated SignAndEncrypt channel, and every data channel frame - carrying no UA-SC security under TransportSecured - is readable and injectable. Reverse connect now uses the application's certificate validator and runs the two-artifact binding §7.6.2 defines for the inverted TLS roles. Per-SecureChannel data channel state was never released. The dictionary is keyed by SecureChannelId and only cleared at shutdown, and each entry holds a DataChannelManager with a running scheduler, so a peer that repeatedly opened and closed a SecureChannel accumulated them without bound. State is now reaped once no Session remains on that SecureChannel, which is also the point past which no data channel could be authorized on it anyway. The negotiated MaxFrameSize was unenforced on receive until a conforming frame happened to arrive, because the tracking field started at uint.MaxValue and was only ever lowered by a frame that already fit. A sender that never set MessageStart kept it unbounded. It now starts at the negotiated value, and the §5.2 grace window applies only from an actual ModifyDataChannel reduction. The delivery queue that compounded this over QUIC - where transport flow control means no credit accounting - is now bounded with blocking backpressure rather than a reset, so reliable delivery is preserved and QUIC's own flow control does the throttling. Pre-authentication QUIC admission state accumulated. The activation-epoch snapshot is inserted from the TLS handshake callback, before any authentication, and was only removed for connections that completed the handshake, so an unauthenticated peer that offered the right ALPN and then abandoned the handshake left an entry permanently. Entries now expire at the handshake timeout and hold a certificate reference rather than a copy. A test that asserted the old reverse-connect behaviour was rewritten rather than deleted: it configured an accept-all validator and expected rejection, which only held while the code ignored the configured validator and fell through to OS trust. It now configures a validator that genuinely refuses, which is what "fails closed when the peer is untrusted" has to mean. 258 tests pass on net9.0 and net10.0, 197 on net472 and net8.0. The sample still moves 300/300 frames over both opc.tcp and opc.quic. Opc.Ua.Core.Tests unchanged at 4134. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: e1a54492-72a7-420f-bbda-38ae8c07cb92
Third security review of the data channels feature. Four findings in the reference implementation, plus two bugs found while fixing them. Authorization no longer fails open. The default authorizer treated a source with no owning NodeManager as authorized, on the reasoning that registration is a deliberate server-side act. That was wrong: registration says the source exists, not that this user may read it. It now denies, and evaluates AccessRestrictions alongside RolePermissions as MasterNodeManager does for a real Read. Registry-only sources need an explicit authorizer, which the sample now demonstrates. The QUIC listener now performs the mutual TLS binding that Part 6 errata 7.6.1 requires, on the normal accept path and not only under reverse connect. The rule is two obligations, not one: request a client certificate on every connection, and refuse OpenDataChannel where the connection completed without one. Collapsing them into a handshake failure would make the Discovery Services unreachable, since GetEndpoints runs on a SecurityPolicy None channel that has no certificate to present. The refusal therefore lands at OpenDataChannel, with Bad_SecurityChecksFailed, rather than silently degrading to a Service-only transport. The SequenceNumber budget now observes every symmetric chunk. MSG, OPN, CLO and STR all draw from one space, and reuse under one TokenId reuses the AEAD nonce derived from it. GetNewSequenceNumber is the single chokepoint, so the hook lives there, and the client brings token renewal forward when the space runs low instead of waiting on the lifetime timer. A stream already bound to a data channel can no longer be rebound to another; it previously returned success. Two bugs found while fixing the above: The budget never actually reset. OnTokenActivated zeroed the counter, but the next synchronization re-observed the channel-lifetime m_sequenceNumber and restored it, so a per-token budget behaved as a channel-lifetime one: renewal would latch on permanently and data channels would eventually stall for good on a long-lived channel. The budget is now rebased on the counter value at activation. Wrapping the accept path in QuicPeerBindingTransport silently broke data channels over QUIC. The wrapper is what the channel holds, and it did not forward IUaSCSecureChannelBoundTransport, so the QUIC transport never registered against its SecureChannel and OpenDataChannel fell back to a Service-only transport. Caught by the sample, not by the unit suite. Validated: data channel tests 273/273 on net9.0 and net10.0, 203/203 on net472, net48 and net8.0; Opc.Ua.Core.Tests 4134; Opc.Ua.Server.Tests 3575; Opc.Ua.Client.Tests 2104; and the sample end to end over both --transport tcp and --transport quic. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: e1a54492-72a7-420f-bbda-38ae8c07cb92
UA Secure Conversation requires that symmetric chunks arrive with strictly increasing SequenceNumbers, and the receiver kills the channel otherwise. The sending side did not guarantee it. A chunk's SequenceNumber was drawn inside WriteSymmetricMessage under the securing lock, but the send was deliberately awaited outside that lock so a slow peer could not block Service traffic, and TcpByteTransport.SendChunkAsync then serialized the socket write on a SemaphoreSlim that is not FIFO-fair. Assignment order and transmission order were therefore decoupled, and chunk N+1 could win the semaphore ahead of chunk N. This needs two concurrent writers on one channel to show, which is why it survived: the data channel scheduler alone stays ordered by luck. Add a Subscription and the Publish responses supply the second writer, and the channel dies with ProcessStreamMessage - Duplicate sequence number: 13907 <= 13908 ForceChannelFault ... 'a data channel frame violated the framing rules: MalformedHeader' A per-channel FIFO send gate now spans every path that hands secured chunks to the transport. A ticket is taken at the instant the SequenceNumber is drawn and under the same mutual exclusion, and a writer waits for its turn before writing and releases it afterwards, including on the failure paths - a stranded ticket would deadlock the channel. The "secure under the lock, send outside it" property is preserved. The benchmark that found it is added as --mode benchmark in the streaming sample. It measures data channel throughput against a competing Publish load at 10 ms, 100 ms and 1000 ms. Most of the work in it is refusing to report numbers that look fine and mean nothing, because this measurement fails quietly in several ways: - An early version reported that Publish load made the channel twice as fast. Every loaded case had received zero notifications: a ManagedSession defaults to the newer subscription engine, where a classic Subscription is accepted, reports itself Created, reports no item errors, and never causes a single Publish request to be sent. The benchmark now counts the notifications it actually received and says so when there are none. - The Server revises a publishing interval below its minimum and rounds it to the publishing resolution, both 100 ms by default, so a 10 ms case silently becomes a second 100 ms case that agrees with the first for entirely the wrong reason. The revised value is reported next to the requested one. - Cases run sequentially on a warming process produce a clean monotonic curve that is an artefact of the order. Cases are round-robined inside each pass. - Four medians in a column invite ranking. Where a loaded case overlaps the baseline's own run-to-run spread the benchmark states that the ranking is not supported by the data. - The Publish rate under load means nothing without the rate the same subscription reaches while the channel is idle, so that control is measured and both are reported. Measured on this machine over loopback: about 768 Mbit/s over opc.quic against 390 Mbit/s inline over opc.tcp, and no resolvable effect of the Publish load on either transport in either direction. One defect is recorded rather than fixed: the credit-stall counter is incremented over opc.quic, where credit is not in force and CREDIT frames are neither sent nor expected, so it reports hundreds of thousands of stalls on the runs with the highest throughput measured. The counter is wrong, not the transport. Validation: Opc.Ua.Core.Tests 4134, Opc.Ua.Core.DataChannels.Tests 274, Opc.Ua.Server.Tests 3575, and the sample end to end over both --transport tcp and --transport quic with no ordering faults. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: e1a54492-72a7-420f-bbda-38ae8c07cb92
The CreditStalls diagnostic reported 560,000 stalls on opc.quic runs that simultaneously produced the highest throughput measured anywhere in the benchmark - roughly double the inline figure. Nothing had stalled. DataChannelCredit.TryConsume counts a stall whenever the send window is smaller than the payload, and the window was still consulted over QUIC, where the transport provides its own per-stream and per-connection flow control and CREDIT frames are neither sent nor expected. Both the Part 6 errata and this repository's own documentation state the counter should stay at zero there. The window is now simply not consulted where HasTransportFlowControl is true, rather than being consulted and its answer discarded - the latter is how the defect arose, and it had already been half-applied by seeding the window with uint.MaxValue, which hid the intent without removing the counting. The inline path is untouched: over opc.tcp and opc.wss the credit window is the flow control and its stall counter is a real signal. The defect was diagnostic only. The QUIC send path already bypassed credit blocking, so the ignored result never gated a send and no throughput figure was affected. Validated: the new regression test fails against the previous code and passes now; data channel tests 206 on net472, net48 and net8.0 and 276 on net9.0 and net10.0; Opc.Ua.Core.Tests 4134; and the QUIC benchmark now reports zero stalls in every case. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: e1a54492-72a7-420f-bbda-38ae8c07cb92
Resolve the Core event ID, internals visibility, and StandardServer startup conflicts while preserving data-channel initialization and master bind-phase freezing. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5d026b47-265d-4c97-8715-26e207dfa03c
A review against the OPC UA Data Channels errata found five defects. Four are implementation faults; two of them also exposed gaps in the draft, which are proposed separately in the drafts repository. Validate transportChannelId where the result is visible (Part 6 7.4) QuicServerDataChannelTransport.BindClientStreamAsync discarded the task carrying every 7.4 check, so a Client could name the control stream, a stream it did not initiate, or one already bound, and the Server answered Good and echoed the value. The checks were right; nothing consumed them. Validation and stream reservation now run inline where a refusal becomes the Service result. Only the wait for the stream to materialize stays deferred, because a peer-initiated QUIC stream is observable only once the peer writes to it and a Client writes only after it has the response - awaiting it would deadlock the exchange rather than order it. Stop one unread channel stalling the SecureChannel (Part 6 5.8) The delivery queue was bounded in frames but sized from a byte credit, and it blocked when full. Small frames therefore filled it long before credit ran out, and the block landed on the reader that also carries MSG, OPN and CLO, so a slow consumer on one channel stalled every Session, Subscription and Publish on the connection - the opposite of "stalls that stream and nothing else". The queue is now bounded by encoded frame bytes, as 7.4 bounds the unknown-ChannelId buffer and for the same reason, and a peer that exceeds the bound is reset rather than waited on. This also removes a sync-over-async violation. Open the channel when the response is dispatched, not before (Part 4 5.1) OnResponseSent ran before the response object was encoded, so MarkOpen could wake the scheduler and put a frame for a ChannelId on the wire ahead of the response naming it. SecureChannelContext gains an optional ResponseDispatched callback that both listeners invoke after handing the response to the transport; it is inert when unset. The scheduler additionally refuses to serve a channel still Opening, which is what the 5.13 state table already said. Carry inline data channels instead of discarding them (Part 6 5.16) A Server advertised opc.tcp, accepted OpenDataChannel, and then dropped every frame through a transport whose send path did nothing - the silent drop 5.16 forbids. InlineServerDataChannelTransport resolves the UASC channel behind the request and enables the engine on it, so channels ride the connection the Client already holds. A SecureChannel that can carry no frames is now refused with Bad_DataChannelTransportUnsupported. Give Uncertain_DataDiscarded its own identifier It shared identifier 1111 with Bad_DataChannelFrameTypeUnsupported. The two differ only in severity bits, so the collision produced valid but wrong constants instead of a build error; it is now 1113. The generator fails on a duplicate identifier and a test asserts the emitted constants are unique. Every regression test added here goes through the production entry point rather than the component, because that is precisely what the existing component-level tests missed in all four wiring defects. Opc.Ua.Core.DataChannels.Tests 284 pass on net10.0 and 212 on net48; Opc.Ua.Server.Tests 4072 pass; Opc.Ua.Sessions.Tests 776 pass with test discovery unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5d026b47-265d-4c97-8715-26e207dfa03c
Poll for outbound RESET frames before asserting so the tests no longer race the manager scheduler thread. Strengthen the opening-channel lifecycle guard by granting connection credit before checking for premature DATA frames, and update documented test counts for net10.0 and net48. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5d026b47-265d-4c97-8715-26e207dfa03c
Data channels were welded into Opc.Ua.Core at seven points: the inbound dispatch reached ReadSymmetricMessage and VerifySequenceNumber, the outbound path reached DataLock, CurrentToken, WriteSymmetricMessage and the send gate, and the channel published EnableDataChannels, DataChannels, SequenceBudget and MaxDataChannelBodySize as its own API. None of that is reachable from another assembly, so the code could not be lifted out. This adds the extension point and reroutes the existing code through it, without moving a single file. A green suite here is what proves the seam is faithful; the move that follows is then mechanical. The seam names no data channel concept. ISecureChannelMessageExtension owns a MessageType that is neither a Service call nor part of establishing the SecureChannel; ISecureChannelMessageHost is what the channel offers it. A chunk is decrypted, verified and sequence-checked before the extension sees it, so an extension never handles content the channel has not authenticated, and a MessageType with no registered owner stays a protocol error - which is what OPC 10000-6 6.7.2.2 requires of a receiver that does not implement it. The send is the delicate part. Assigning the SequenceNumber and applying message security stay serialized against Service traffic, because both draw on the same keys and the same counter, and the callback the extension passes runs inside that serialization so it can refuse the send atomically. That is what the data channel SequenceNumber budget of Part 6 5.1.1 needs, and it is why the callback exists rather than a plain "encode then send". The write itself is still awaited outside the serialization, so a slow peer on an extension cannot stall Service traffic. The callback is cached, so a frame allocates no closure on the send path. The budget splits along the same line: the mechanism, how many SequenceNumbers this channel has issued under the token in force, belongs to the channel; the policy of when to stall belongs to the extension. DataChannelExtension is now the only thing that knows the frame format, and the protected virtual OnDataChannelProtocolFault it replaces becomes an event, so which framing rule was broken is still observable - a transport error alone does not carry it. Opc.Ua.Core.Channels is scaffolded here but still empty; the engine moves into it next. Opc.Ua.Core.DataChannels.Tests 284 pass on net10.0 and 212 on net48; Opc.Ua.Sessions.Tests 777 pass, which is the suite that would notice a change to the shared receive and send path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5d026b47-265d-4c97-8715-26e207dfa03c
The engine now consumes the message-extension seam rather than living inside the assembly that provides it, so Opc.Ua.Core no longer carries a streaming feature it does not need. Opc.Ua.Core.Channels references Opc.Ua.Core and nothing else, ships as OPCFoundation.NetStandard.Opc.Ua.Core.Channels, and targets the same six frameworks as Core so the net48 coverage survives. What moved: the twenty-one engine files, the data channel half of the UASC partial - now extension methods, because a partial class cannot span assemblies - and IServerDataChannelTransport with InlineServerDataChannelTransport, which turned out to touch only SecureChannelContext, Profiles and the engine. What deliberately did not. StandardServer.DataChannels.cs is a partial of StandardServer, so it cannot move at all; its Service overrides and the authorizer and auditor that read ServerInternal, MasterNodeManager and node states are server concerns and stay. Opc.Ua.Bindings.Quic stays its own net9.0+ package: System.Net.Quic is behind RequiresPreviewFeatures on net8.0, and it references Opc.Ua.Server, so folding it into a Core.* package would put that package above the Server layer. Two things moved the other way, because they are the SecureChannel's own concerns rather than the feature's. DataChannelSequenceBudget becomes SequenceNumberBudget in Core: the space is shared by MSG, OPN, CLO and every extension alike, and the client channel already used it to decide when to renew a token. Keeping one instance on the channel also removes the second, redundant budget the seam had briefly introduced. The SecureChannel registry becomes UaSCSecureChannelRegistry, since mapping an identifier to the channel that owns it says nothing about data channels. Core loses EnableDataChannels and DataChannels on both channel types, MaxDataChannelBodySize, SequenceBudget's data channel name, and the CoreEventIds.DataChannel blocks; TransportChannelFeatures.DataChannels becomes MessageExtensions. The feature is [Experimental], so removing rather than forwarding is allowed. Log EventIds keep their existing numbers in ChannelsEventIds so an operator's filters keep working across the move. DataChannelFeature carries the experimental marker in Core, because it marks the generated Service Set that Clients call, not the engine. expected-packages.txt gains Core.Channels and Bindings.Quic - the latter was packable but missing, which the file itself says should be a conscious act. Opc.Ua.Core.DataChannels.Tests 284 pass on net10.0 and 212 on net48; Opc.Ua.Server.Tests 4072 and Opc.Ua.Sessions.Tests 777 pass, which is what covers the shared receive and send path the seam sits on. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5d026b47-265d-4c97-8715-26e207dfa03c
Code coverage✅ Coverage gate passed.
Uncovered changed lines
Coverage is above the recorded baseline - consider ratcheting Thresholds live in |
marcschier
commented
Aug 11, 2026
Addresses the two-axis review of PR #4240 (test coverage and spec/API completeness). The security fix is the one that matters most. Security * Authorization was direction-blind. IDataChannelAuthorizer.IsAuthorizedAsync took no direction and the default authorizer always evaluated PermissionType.Read, so a user permitted to read a source could open a SinkToSource or Bidirectional channel and write into the Server. Part 4 errata section 7.2 names this exact failure. The interface now carries the direction, the call moved to after TryRevise so the negotiated direction is known while still preceding any ChannelId or stream allocation, and each required permission is validated on its own - ValidateRolePermissions treats a combined mask as "any of these", so asking for Read|Write in one call would have passed for a Read-only user. Wiring * AddQuicTransport() now registers the server-side data channel transport, and DependencyInjectionStandardServer resolves the transport, authorizer, auditor and sources from DI. UseQuicDataChannelTransport() stays as the direct-construction fallback. * A closed SecureChannel or lost transport now faults every data channel riding on it, on both the inline and opc.quic paths (section 5.13). This was unimplemented; only the Session-close and identity-change rows of that table were covered. * IServerDataChannelTransport.AbortSecureChannel had no caller and the QUIC implementation closed the whole connection, contradicting its own contract and section 5.11 ("a failed stream is not a failed connection"). It is now invoked when the Server tears down a SecureChannel's state, and tears down the data channels rather than the connection. Tests * DataChannelIntegrationTests: the end-to-end leg the repo requires and this feature lacked. A real Client Session drives Open, Modify and Close against a live StandardServer over opc.tcp, and payload crosses the same SecureChannel inline. It is the only coverage of the Service dispatch, per-SecureChannel state and authorization chain by a request that arrived off a socket. * DataChannelGapTests and DataChannelRefusalTests cover the GAP lifecycle and the receive-side refusals, both of which were entirely at zero: the section 5.10 protocol error, the section 5.2.1 unbounded-state guard, DATA after END, wrong-direction DATA, receive credit exhaustion and credit release on discard. * QuicListenerTests gains the public client seam (QuicClientChannelExtensions was 0/21 despite being the API a consumer uses) and the transfer handoff. * CloseChannelsForCertificateReturnsAffectedChannelIdsOnAKeyChangeAsync never called the method it was named after; renamed to what it verifies, and a real test of CloseChannelsForCertificateAsync added. * Two fixed-sleep assertions replaced with waits on an observable scheduler round counter, so the negatives are evidence rather than timing. Coverage: Opc.Ua.Core.Channels 89.8% -> 92.9% line, Opc.Ua.Bindings.Quic 79.5% -> 80.7% line, which clears the repo's 80% bar it previously missed. 311 tests on net10.0 and 237 on net48, up from 284 and 216. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5d026b47-265d-4c97-8715-26e207dfa03c
marcschier
commented
Aug 11, 2026
A security review of this branch found no exploitable vulnerability, but flagged the PONG responder as an unbounded, peer-driven growth path. Reading the errata back shows it is not merely hardening: section 5.11 states the obligation and names the attack. "PING is exempt from flow control and compels a PONG ahead of queued payload, which without a bound is an amplification surface: a peer could emit PING at line rate on every open ChannelId and compel the other end to answer at line rate ahead of its own traffic, with no window to close against it. A peer shall not have more than one unanswered PING outstanding per ChannelId, and shall not emit PING on a given ChannelId more than once per second. A receiver may discard a PING that violates either bound, and may RESET the channel with Bad_DataChannelLimitsExceeded if the violation persists." Only the sending half was implemented (TryPing, TryPingConnection). The receiving half answered every PING unconditionally, which bounds a well-behaved peer and leaves a hostile one unbounded - the responder does the work, so a rule the sender enforces on itself protects nobody. Both halves are now enforced, on the per-channel path and on ChannelId 0, which is a ChannelId and was otherwise left as a connection-level amplification surface once the data channels bounded themselves. * A PING inside the interval is discarded rather than answered. The prober keeps no state on the responder, so a dropped PONG costs it one measurement it was not entitled to take. * A peer that keeps flooding after being ignored MaxPingRateViolations times is reset with Bad_DataChannelLimitsExceeded - the errata's "if the violation persists". A single burst is absorbed silently. * The response interval carries a tolerance below the nominal one second. The sender measures the bound on its clock and the receiver on its own, so a conformant peer still delivers the occasional PING a few milliseconds early; without the tolerance ordinary jitter would be punished as a violation. * "Never answered" is tracked with an explicit flag rather than a zero timestamp sentinel, because a TimeProvider is not required to start above zero and the bound would silently never engage if it did. Tests Five tests in DataChannelRefusalTests: the verbatim Timestamp echo, the discard, recovery once the interval elapses, the escalation to RESET, and the control channel. Verified as guards by reverting the production change and confirming three of them fail. UnknownChannelBufferIsBoundedByEncodedFrameBytes counted the PONGs that a replayed PING burst produced as a proxy for how many frames had survived the unknown-channel buffer. The rate limit correctly invalidates that proxy, so it now sends DATA frames - which encode to exactly the same size, leaving the bound arithmetic untouched - and asserts FramesReceived. It measures the buffer bound it is named for rather than the ping rate limit. 316 tests pass on net10.0 and 242 on net48; Opc.Ua.Server.Tests is unchanged at 4072 passed, because this sits on the shared receive path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5d026b47-265d-4c97-8715-26e207dfa03c
CI fix Opc.Ua.Bindings.Quic hardcoded TargetFrameworks net9.0;net10.0 and so did not take part in the repo's per-TFM pin. A UA.slnx build pinned with CustomTestTarget builds the rest of the stack for that one TFM, which broke this project three different ways: * pinned net10.0 - the project's net9.0 leg referenced a stack built only for net10.0, so restore failed with NU1201. This is what failed the ubuntu Core.DataChannels job in 32 seconds. * pinned net472/net48 - the net9.0 leg resolved the net472 assets of Opc.Ua.Core and failed with CS0012 on IAsyncDisposable. * pinned net8.0 - System.Net.Quic is behind RequiresPreviewFeatures there, so the project cannot take part at all. The project now follows the pin when the pin can host it, and otherwise opts into the existing RestrictForLegacyTfm no-op shape that samples/ConsoleDataChannelStreaming already uses. That mechanism only knew about the .NET Framework and netstandard pins - the right floor for a .NET 8+ project - so it gains RestrictForLegacyTfmAdditionalTargets for a project whose floor is higher. The shipped package shape is unchanged: an unpinned build still produces net9.0;net10.0. Verified by running the same matrix the CI legs run. All seven pinned UA.slnx builds succeed: net10.0, net9.0, net8.0, net48, net472, netstandard2.0, netstandard2.1. That matrix also caught a break the earlier per-project validation missed: SampleDataChannelAuthorizer was never updated for the direction-aware IDataChannelAuthorizer signature, so the sample did not compile. It now grants only the SourceToSink direction it actually publishes, which is the point of the Part 4 errata 7.2 rule it demonstrates. Seam simplification The Stack/Tcp change was carrying weight it did not need. Three removals, no behaviour change: * SecureChannelMessage is gone. It crossed the boundary with exactly one producer and one consumer, was never stored or compared, and carried IEquatable, ==, != and GetHashCode purely to satisfy CA1815 - about sixty lines of ceremony. OnMessageReceived now takes the three values directly. * The onSecuring callback is gone, and with it the SequenceBudget member of the host interface. It was the subtlest part of the contract - a callback that runs inside the send serialization and may veto atomically - and its only implementation consumed the budget. Core already owns SequenceNumberBudget and is the code doing the securing, so it now claims the number itself under the same lock. The guarantee is identical and the extension no longer has to reason about the serialization at all. * GlobalChannelId is gone. It had no reader. The host interface drops from nine members to six, and Stack/Tcp from 1358 added lines to 1270. Moving the engine back into Opc.Ua.Core was considered and rejected: it would remove about 330 of those lines, none of them the hard parts - the FIFO send gate exists because there is a second writer on one sequence counter, not because of an assembly boundary - while moving 7241 lines of engine into Core on every TFM for an experimental feature. 316 tests pass on net10.0 and 242 on net48. Opc.Ua.Server.Tests (4072) and Opc.Ua.Sessions.Tests (777) are unchanged, which is the check that matters because the seam sits on the send and receive path of all Service traffic. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5d026b47-265d-4c97-8715-26e207dfa03c
marcschier
changed the title
test-ubuntu-latest-SourceGeneration.Core failed on two hardcoded counts: GetListOfServices() returned 42 rather than 39, and the Session category 7 rather than 4. Both are this PR: OpenDataChannel, ModifyDataChannel and CloseDataChannel are three new Session-category Services, so the counts moving by exactly three is the intended behaviour rather than a regression. The category test now also names the three Services it expects to find, so a future change to the figure has to say which Service moved in or out of the category instead of silently renumbering. Verified locally: the full Opc.Ua.SourceGeneration.Core.Tests suite passes, 3771 tests on net10.0. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5d026b47-265d-4c97-8715-26e207dfa03c
…ayer build-and-push-image (refserver) failed with NETSDK1004: the assets file for Opc.Ua.Core.Channels was missing. The Dockerfile copies each csproj into the restore layer by name so that layer caches independently of the sources, and this PR gave Opc.Ua.Server a reference to the new project without adding it to that list, so restore never saw it and the later --no-restore publish had no assets file to read. Verified locally by building the image end to end with docker build, both the restore layer on its own and the full publish that CI runs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5d026b47-265d-4c97-8715-26e207dfa03c
marcschier
commented
Aug 12, 2026
The advisory patch coverage gate reported 60.30% against a 75% floor. Most of
the shortfall is structural rather than untested code: Opc.Ua.Bindings.Quic is
29.8% of the changed source lines and 60 of the 316 data channel tests skip on
the CI runner because msquic is not installed there, so that assembly reads
close to zero in the merged report while measuring 80.65% locally, where QUIC
runs.
Two genuine gaps were in that list and are closed here. ManagedSession and
RedundantClientSession each forward the three data channel Services to the
session they wrap, and neither forwarder was exercised: the existing
integration test drives ISession directly, so it never passes through either
facade. A dropped forwarder would be invisible until a caller found the
Service silently doing nothing, and on the redundant facade it would leave a
channel opened against a session the caller can no longer reach after failover.
Both fixtures already assert this obligation for every other Service, so the
new cases join the table-driven passthrough suite rather than standing alone.
The [Experimental("DataChannels")] opt-in is scoped to the new cases with a
pragma rather than switched on for the whole test project, so the attribute
keeps its meaning everywhere else.
Opc.Ua.Client.Tests 2126 pass, Opc.Ua.Redundancy.Client.Tests 123 pass.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 5d026b47-265d-4c97-8715-26e207dfa03c
The seam is inline-only. Checking every one of its symbols against Opc.Ua.Bindings.Quic gives zero references to ISecureChannelMessageHost, ISecureChannelMessageExtension, SequenceNumberBudget, the send gate and UaSCSecureChannelRegistry alike - a QUIC data channel rides its own stream and never becomes a UASC chunk, so none of it applies. The seam therefore exists to keep the engine out of Core, not to invite other implementers: there is exactly one, and no prospect of a second. Publishing it would commit Core to an abstraction with a single implementer for ever. Core already grants InternalsVisibleTo to its sibling first-party assemblies - Opc.Ua.Client, Opc.Ua.Bindings.Quic, Opc.Ua.Bindings.Https, Opc.Ua.Core.Diagnostics - and Opc.Ua.Core.Channels had simply never been added to that list, which is the only reason the seam had to be public. Adding it lets the following become internal with no code moving at all: * ISecureChannelMessageHost and ISecureChannelMessageExtension * SequenceNumberBudget, and UaSCUaBinaryChannel.SequenceBudget * RegisterMessageExtension and TryGetMessageExtension * DataChannelExtension and TryGetDataChannelExtension, which are the inline adapter rather than API - a consumer reaches the channels through EnableDataChannels and GetDataChannels, which return the engine UaSCSecureChannelRegistry stays public deliberately. An application writing its own IServerDataChannelTransport for inline framing needs it to resolve the channel behind a request, which samples/ConsoleDataChannelStreaming demonstrates; making it internal would break a real extension point to tidy an API surface. The public surface this PR adds to Opc.Ua.Core is now UaSCSecureChannelRegistry and IUaSCSecureChannelBoundTransport, the latter being a transport extension point alongside the existing public IUaSCByteTransport. This captures the main benefit of folding the engine back into Core - not owing compatibility on a single-implementer abstraction - without moving 4866 lines of experimental engine into Core on every TFM from net472 upward, and without touching the send gate or the sequence budget, both of which are inherent to inline framing wherever its code lives. UA.slnx builds pinned to net10.0, 316 data channel tests pass, and Opc.Ua.Server.Tests is unchanged at 4072. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5d026b47-265d-4c97-8715-26e207dfa03c
Review feedback on UaSCBinaryClientChannel.cs:893 - "Everywhere we make it look like a generic extension, but we only handle stream." That is correct, and checking it against the only other transport settles it: Opc.Ua.Bindings.Quic references none of ISecureChannelMessageHost, ISecureChannelMessageExtension, SequenceNumberBudget or the send gate, because a QUIC data channel rides its own stream and never becomes a UASC chunk. The seam had exactly one implementer and no prospect of a second; it existed only because the engine sat in another assembly. So the engine comes home rather than the seam being dressed up as general. The destinations are the ones commit cfaa34f recorded as renames: * Opc.Ua.Core.Channels/DataChannels/** (engine, framing, flow control, sequencing, scheduling, model and Services/) to Opc.Ua.Core/Stack/DataChannels * IServerDataChannelTransport and InlineServerDataChannelTransport back to Opc.Ua.Server/Server, where the latter came from * ChannelsEventIds folded into CoreEventIds, keeping 600/620 so existing log filters are unaffected Deleted outright: * ISecureChannelMessageExtension.cs, both interfaces. This also answers the request to rename that file after its interface: it no longer exists. * UaSCBinaryChannel.MessageExtensions.cs, replaced by UaSCBinaryChannel.DataChannels.cs, which owns the DataChannelManager directly and dispatches STR after the existing decrypt, signature and sequence checks. * DataChannelExtension, the adapter that existed only to bridge assemblies. Its IDataChannelTransport half is now a small private InlineDataChannelTransport that hands the channel to the engine. * The Opc.Ua.Core.Channels project, its UA.slnx entry, its expected-packages line and its reference from the reference server Dockerfile. The package is new in this branch and never shipped, so nothing is owed compatibility. EnableDataChannels and the channel accessors become ordinary members instead of extension methods over a published seam, and the framing fault surfaces as UaSCUaBinaryChannel.DataChannelProtocolFault, so a test no longer has to reach through a registry to observe which rule was broken. This supersedes the InternalsVisibleTo narrowing in 18a3077: the types it made internal are deleted here, and the entry it added to Opc.Ua.Core is removed. SequenceNumberBudget stays internal, which the move now justifies on its own. UaSCSecureChannelRegistry stays public because a server writing its own IServerDataChannelTransport needs it, as the sample does. Separately, per the review of the QUIC csproj, opc.quic now targets net8.0 as well and opts into preview features there, so a net8.0 consumer can do the same rather than being excluded. The three file wide NET9_0_OR_GREATER guards are gone; only QuicServerConnectionOptions.HandshakeTimeout, which is genuinely .NET 9+, stays guarded, and the listener's own admission expiry still bounds a stalled handshake on net8.0. UA.slnx builds pinned to net10.0 and 316 data channel tests pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5d026b47-265d-4c97-8715-26e207dfa03c
… net8.0 Nine review comments; two were answered by the previous commit (the generic extension seam and the file named after it are both gone). The rest: Sample layout and a second sample The data channel samples now live under samples/Core, and a new ConsoleDataChannelAudio joins the throughput benchmark. It stands up a Server and a Client in one process, synthesises a short melody as 16-bit PCM once at startup and streams it on repeat over a data channel while the Client plays it back. Nothing binary enters the repository. The source writes in real time rather than as fast as the channel will take it, because a media source is paced by its own clock and writing faster only adds latency; the progress line reports frames, bytes and credit stalls so a consumer that cannot keep up is visible rather than silently buffered. Measured over 31 seconds: 1582 frames, 2725 KiB, zero credit stalls, which is exactly real time for 20 ms frames. Playback uses NAudio 2.3.0 (MIT), referenced only by that sample - nothing under src/ gains a dependency. NAudio's output devices are Windows interfaces and 2.x has no ALSA or CoreAudio backend, so on Linux and macOS the sample writes the received stream to a WAV instead and says which it is doing on startup. The Windows-only paths sit behind OperatingSystem.IsWindows() rather than a CA1416 suppression. Documentation "Where the code lives" describes where the code is rather than where it moved from, and the same edit removes "rather than in ...", "remains" and the rest of the change narration. Two sections that were a development diary - the tables of defects found while writing tests and while running the sample - are gone entirely: they recorded history rather than the state of the implementation. Logging The response-dispatched callback logged through a message that reads "Failed to send fault response to client", which is not what happened, and on the HTTPS listener it invented a requestId of 0 to fit that signature. All three listeners now have a named message that says what actually failed. net8.0 for the QUIC binding Rather than excluding net8.0, the binding targets it and enables preview features, so a net8.0 consumer opts in the same way for System.Net.Quic. The three file-wide NET9_0_OR_GREATER guards are gone; only QuicServerConnectionOptions.HandshakeTimeout stays guarded because it is genuinely .NET 9+, and the listener's own admission expiry still bounds a stalled handshake there. Removing the #if around [Experimental] ExperimentalAttribute is a .NET 8 type, and the repo's rule is to polyfill rather than fork with #if, so it joins the existing polyfills in Opc.Ua.Types. Verified that this is a real opt-in and not just something that compiles: with the suppression removed, a net48 caller of a data channel Service fails the build with "error DataChannels", which is the same behaviour as net10.0. Validated with the matrix CI runs: all seven pinned UA.slnx builds (net10.0, net9.0, net8.0, net48, net472, netstandard2.0, netstandard2.1), 316 data channel tests on net10.0 and 242 on net48, Opc.Ua.Server.Tests 4072, Opc.Ua.Client.Tests 2126, Opc.Ua.Redundancy.Client.Tests 123, and the reference server Docker image end to end. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5d026b47-265d-4c97-8715-26e207dfa03c
… send path A data channel send that WriteSymmetricMessage refuses with limitsExceeded returned without completing the send-gate ticket it had been handed. The ticket then became the tail every later send chains behind, so MSG, OPN and CLO for every Session on that SecureChannel stalled for good. Nothing recovered from it: the scheduler logs a send fault rather than faulting the channel, so the stall was silent. The ticket is now released on every path out. Releasing twice is safe because the ticket completes under Interlocked. MaxDataChannelBodySize approximated the chunk budget instead of computing it, and missed the cipher block rounding WriteSymmetricMessage applies: a 65535 byte send buffer under a 16 byte block loses its last 15 bytes. A body at the advertised size therefore spilled into a second chunk, which carries the Intermediate chunk type that the frame codec rejects, after the SequenceNumber had already been spent. It now mirrors that arithmetic. The audio sample wrote to a fixed name under the shared temp directory and truncated whatever was already there, so a local user could pre-create the path as a symlink and have the sample overwrite the target. It now writes into a directory created for the run and opens the file with CreateNew. Both channel fixes are covered by tests proven to fail without them. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5d026b47-265d-4c97-8715-26e207dfa03c
The QUIC harnesses bound their listeners to IPAddress.Loopback while their clients connected by name. "localhost" resolves to ::1 before 127.0.0.1 on the CI agents, so the handshake was sent where nothing was listening and 26 tests failed. .NET surfaces a QUIC handshake that never completes as "Application layer protocol negotiation error", which pointed the investigation at ALPN rather than at the address the listener was bound to (dotnet/runtime#85412). The listeners now bind IPv6Any, which is dual stack and is what QuicTransportListener already does, so the harnesses match the deployment they stand in for. MismatchedAlpnListenerIsRefused had been passing for the wrong reason: it asserts that a connection is refused, and a connection that never arrives is also refused. Added a test that connects over IPv6 explicitly, because a host that resolves IPv4 first never exercises this and would let the same regression back in. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5d026b47-265d-4c97-8715-26e207dfa03c
The QUIC tests have skipped themselves on every CI agent for the whole life of this feature. .NET reaches QUIC through msquic, which the hosted Linux images do not ship, so QuicListener.IsSupported was false and 61 tests took the Assert.Ignore path. The transport was effectively untested in CI, and because the coverage report is merged from the ubuntu legs, all of Opc.Ua.Bindings.Quic counted as uncovered: 1314 of the 1600 uncovered changed lines the coverage gate was complaining about. Measured in a container on the same image family: without libmsquic 260 pass and 61 skip; with it 321 pass and none skip. The tests needed no changes to run on Linux, where QUIC mutual TLS is better supported than on Windows Server 2022 Schannel. A silent skip is the honest outcome where msquic is genuinely absent, but not on a machine that is supposed to have it - that is what hid this. The setup step sets UA_REQUIRE_QUIC, which turns the skip into a failure, so a future image or package change cannot quietly return the QUIC binding to zero coverage. Verified both ways: with the variable set and no msquic the suite reports 61 failures rather than 61 skips. The eleven copies of the skip check are now one shared helper. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5d026b47-265d-4c97-8715-26e207dfa03c
Adds tests for paths the changed-lines report showed uncovered, all of them the defensive and boundary cases rather than the happy path: - credit accounting ignores a non-positive length, refuses to replenish before anything is released, and saturates rather than wrapping a window that a long-lived receiver has driven close to the 32-bit ceiling - encoding into a buffer too small for the frame is refused, rather than putting a truncated frame on the wire that the peer would answer by resetting a channel that did nothing wrong - an empty send queue reports no frame instead of handing out a buffer, and expiry leaves the queue untouched when nothing has expired - negotiation refuses to revise without the capabilities every limit is derived from, and an absent modify request is not a mutation - a CREDIT or GAP that overtakes its OpenDataChannel response replays with its own fields intact, not just DATA: a CREDIT replayed without its grant leaves the sender blocked, and a GAP replayed without its range reports the wrong frames discarded Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5d026b47-265d-4c97-8715-26e207dfa03c
marcschier
changed the title
NuGetAudit began failing every restore in the repository, which took the solution build, CodeQL and both Kafka test legs with it. SSH.NET 2025.1.0 carries a high severity path traversal: ScpClient.Download trusts the file and directory names the remote server sends during a recursive download, so a malicious or man-in-the-middle SCP server can escape the download directory and overwrite arbitrary files the client can write. SSH.NET arrives transitively through Testcontainers 4.13.0, which asks for [2025.1.0, ) and therefore resolves the lowest, vulnerable version. The patched 2026.0.0 satisfies that same range, so a central pin is enough and no consumer changes; transitive pinning is already enabled repository-wide. The pin should be removed once Testcontainers itself requires a fixed version. This is not part of the data channel work. The advisory was published while the branch was open and breaks master and every other open branch equally, but it blocks every check here, so it is fixed rather than waited on. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5d026b47-265d-4c97-8715-26e207dfa03c
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 82983b49-5226-4ca1-a225-1029c6f17e3f
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 82983b49-5226-4ca1-a225-1029c6f17e3f
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 82983b49-5226-4ca1-a225-1029c6f17e3f
Preserve FIFO send-ticket ordering while adopting asynchronous closed-transport completion and security policy registry DI. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Keep the new AI sample projects alongside the data-channel sample group in UA.slnx. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0b8bfdb6-9b78-4b17-a183-2d8a76419ccc
Adopt the refactored MasterNodeManager service dispatcher while retaining data-channel access restriction validation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0b8bfdb6-9b78-4b17-a183-2d8a76419ccc
Retain the sample-only NAudio package while adopting the updated test, versioning, Testcontainers, and security-pinned dependency versions from master. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0b8bfdb6-9b78-4b17-a183-2d8a76419ccc
Integrate state-machine cause executability reporting and the reverse-connect timeout race fix. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0b8bfdb6-9b78-4b17-a183-2d8a76419ccc
Integrate the latest OpenUSD, WoT, certificate, server, and tooling changes while retaining the branch-specific secure-channel transport extensions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0b8bfdb6-9b78-4b17-a183-2d8a76419ccc
Integrate typed child NodeId registration, explicit fluent server roots, the federated OpenUSD demo, and corrected maximum publish timeouts. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0b8bfdb6-9b78-4b17-a183-2d8a76419ccc
Integrate the concern-based ConfigurationNodeManager split and its certificate-alarm and namespace-metadata coverage. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0b8bfdb6-9b78-4b17-a183-2d8a76419ccc
Integrate partial endpoint-filter matching for role management and its documentation and coverage. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0b8bfdb6-9b78-4b17-a183-2d8a76419ccc
Integrate WoT Binding 1.1 conformance coverage, generated dependency metadata fixes, and corrected GDS certificate-group type advertisement. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0b8bfdb6-9b78-4b17-a183-2d8a76419ccc
Preserve configured custom transport schemes while failing startup for missing bindings, and integrate fluent node-manager extensibility and effective monitored-item identities. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0b8bfdb6-9b78-4b17-a183-2d8a76419ccc
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Experimental. Adds OPC UA data channels — a named, authorized, flow-controlled, bidirectional stream of opaque bytes multiplexed onto a SecureChannel that is already open — together with the
opc.quictransport and a message-extension seam inOpc.Ua.Corethat carries it.OPC UA has no streaming primitive. A camera, a microphone, a firmware image or a log tail has to be carried by something designed for something else:
Readpolling, a Subscription carrying ByteString values, the FileTransfer model, or PubSub alongside the SecureChannel rather than on it. This implements the OPC UA Data Channels errata, which defines that primitive.What is in it
Framing and engine (part of
OPCFoundation.NetStandard.Opc.Ua.Core)The
STRMessageChunk, the twelve-byte stream header, seven frame types and five flags, verified byte for byte against the specification's published hex vectors. Serial-number arithmetic with a replay window and boundedGAPruns, per-channel and per-connection credit, a deficit round-robin scheduler with anti-starvation, the per-direction state machine, deadline expiry, and the SequenceNumber budget of §5.1.1.Services
OpenDataChannel,ModifyDataChannelandCloseDataChannelgenerated from the model compiler inputs, so they take exactly the path every standard Service takes. Parameter negotiation, Session scoping, direction-aware authorization with re-evaluation, and auditing — served byStandardServer, so a real Client opens a channel through a real Session. Server-initiated offers (Part 4 §6) are not implemented; see Known limitations.Transports
Inline framing over
opc.tcpandopc.wss— a frame is one MessageChunk on the connection the Client already holds. Plusopc.quic(OPCFoundation.NetStandard.Opc.Ua.Bindings.Quic, net9.0+): URL scheme, ALPN negotiation and enforcement, listener, endpoint discovery, reverse connect, certificate rotation, one QUIC stream per channel, and the TLS-peer-to-OPC-UA-peer key binding of §7.6.1.Carried by the UASC channel itself
UaSCUaBinaryChannelowns theDataChannelManagerfor its SecureChannel and dispatchesSTRchunks to it after decrypting, verifying and sequence-checking them, so the engine only ever sees authenticated content. UntilEnableDataChannelsis called an incomingSTRchunk is an unrecognized MessageType and closes the SecureChannel, which is what OPC 10000-6 §6.7.2.2 requires. There is no generic extension abstraction:STRis the only MessageType this adds, andopc.quicneeds none of it.The delicate part is the send. Assigning the SequenceNumber and applying message security stay serialized against Service traffic, because both draw on the same keys and the same counter; the callback an extension passes runs inside that serialization so it can refuse atomically. The write itself is awaited outside it, so a slow peer on a data channel cannot stall
Publish.Design notes worth reading
STRframe is transmitted until anOpenDataChannelon that SecureChannel has completed successfully, so a capable and a legacy implementation interoperate with no negotiation and no version bump.opc.quicrefuses unreliable datagrams withBad_DeliveryModeUnsupportedrather than silently carrying them on a stream:QuicConnectionexposes no RFC 9221 datagram API through .NET 10.Feedback from the specification, back to the specification
Implementing it found two gaps in the draft, both raised upstream:
RESETframe, so two implementations that each invent provisional values cannot interoperate.1100–1113are now pinned in the errata and here.DCF-039.The errata also needs one correction this implementation could not follow:
OpenDataChannelreuses thetransportChannelIdparameter name across request and response, which no OPC UA Service does and the model compiler rejects, so the response parameter here isrevisedTransportChannelId.Compatibility
Additive and opt-in. The feature is inert until enabled, and marked
[Experimental("DataChannels")]so consumers opt in deliberately — enforced identically on every TFM via anExperimentalAttributepolyfill. The engine is part ofOpc.Ua.Core, which still builds for all six target frameworks, so data channels work onnet472throughnet10.0.opc.quicis net8.0+, opting into preview features on net8.0 forSystem.Net.Quic.Two experimental members were renamed rather than kept as forwarders:
TransportChannelFeatures.DataChannels→MessageExtensions, andEnableDataChannels/DataChannelsmove fromUaSCUaBinaryChannelto extension methods in the new package.OPCFoundation.NetStandard.Opc.Ua.Bindings.Quicwas packable but missing fromexpected-packages.txt; it is added here, which that file says should be a conscious, reviewed act. No other new package ships: the engine is part ofOpc.Ua.Core.Related Issues
No tracking issue yet — this implements an external errata draft rather than a reported defect. Happy to open one to carry the design discussion and act as the ADR if maintainers prefer.
Checklist
Put an
xin the boxes that apply. You can complete these step by step after opening the PR.Test evidence
Opc.Ua.Core.DataChannels.TestsOpc.Ua.Server.TestsOpc.Ua.Sessions.TestsThe last two matter because the data channel code sits on the shared receive and send path that serves all Service traffic — the real regression risk in this change is ordinary
MSG/OPN/CLOhandling, not data channels.Every regression test drives the production entry point rather than the component. That is deliberate: a conformance review found several obligations that were implemented correctly, covered by tests, and never invoked from any production path, and the same pattern recurred once more during review.
Known limitations
This is Part 6 clause 5 plus the inline transport, a substantially complete
opc.quic, and the Part 4 Service Set. It is not yet a complete implementation of the errata, and the gaps are stated here rather than left to be discovered:DataChannelOfferedEventType, soTryRedeemcan only fail.DataChannelCapabilitiesmodel projection is not wired.DataChannelModelbuilds the values, but the Object is never instantiated underServerCapabilities, so a Client cannot read the capabilities or discover the feature through the address space. Part 4 §10 makes this ashall.OpenTimeoutandPingTimeoutare negotiated and carried but not enforced (§5.14), and therevisedLifetimeobligation of §5.1.1 is not implemented.UnreliableandPartiallyReliablewithBad_DeliveryModeUnsupported, which is what the errata requires.UaSCSecureChannelRegistryis process-global rather than scoped to the listener that owns the channels. Documented with a TODO; scoping it needs a registry instance threaded through the transport bindings.docs/DataChannels.mdcarries the full clause-by-clause table, including the rows marked "not wired".Review fixes
A two-axis review of this branch (coverage, and spec/API completeness) found one security defect and several unwired paths, all fixed on this branch:
IDataChannelAuthorizertook no direction and the default authorizer always evaluatedPermissionType.Read, so a user permitted to read a source could open aSinkToSourcechannel and write into the Server — the exact failure Part 4 §7.2 names. The interface now carries the direction, the check moved afterTryReviseso the negotiated direction is known while still preceding any ChannelId or stream allocation, and each required permission is validated separately, becauseValidateRolePermissionstreats a combined mask as "any of these".AddQuicTransport()now wires the data channel transport, andDependencyInjectionStandardServerresolves the transport, authorizer, auditor and sources from DI.AbortSecureChannelhad no caller, and the QUIC implementation closed the whole connection rather than the channels — contradicting §5.11, "a failed stream is not a failed connection".PINGrate limiting (§5.11) was enforced on the sending side only. PING is credit-exempt and compels a PONG ahead of queued payload, so a receiver that answers unconditionally is the amplification surface the errata names. The bound now holds on both sides, on data channels and on ChannelId 0.Opc.Ua.Bindings.Quicis at 83.6% line; the engine's own coverage rose from 89.8% to 92.9% before it was folded intoOpc.Ua.Core. The GAP lifecycle, the receive-side refusals and the public client seam were all at zero and are now covered, andDataChannelIntegrationTestsadds the end-to-end Client → Session → Server leg the feature had been missing.Architecture: the seam is gone
The engine now lives in
Opc.Ua.Core(Stack/DataChannels/**) and theextension seam it used to reach through is deleted, along with the
Opc.Ua.Core.Channelsproject and its package.The earlier round argued for keeping the engine outside Core and narrowing
the seam with
InternalsVisibleTo. That reasoning was wrong on the pointthat mattered: measuring the seam showed
Opc.Ua.Bindings.Quicreferencedzero of its symbols, because a QUIC data channel rides its own stream and
never becomes a UASC chunk. A single-implementer abstraction whose one
implementer is Core itself is not a seam, it is indirection — so it is gone
rather than hidden.
UaSCUaBinaryChannelowns theDataChannelManagerdirectly.
UaSCSecureChannelRegistrystays public: an application writingits own
IServerDataChannelTransportneeds it, as the sample shows.Three further simplifications came out of that work with no behaviour change:
the
SecureChannelMessagestruct (one producer, one consumer, nevercompared, yet ~60 lines of
IEquatableceremony), theonSecuringcallback together with the
SequenceBudgetmember — Core owns the budgetand does the securing, so it claims the SequenceNumber itself under the same
lock and nothing outside has to reason about that serialization — and
GlobalChannelId, which had no reader.CI status
Every failing check that was this PR is fixed at the source. Nothing was
skipped, loosened or marked advisory to get there.
Fixed in this round:
Core.DataChannelson the Windows agent — 26 QUIC tests failed theirhandshake. Not the Schannel mutual-TLS limitation it resembled: the test
harnesses bound their listeners to the IPv4 loopback while connecting by
name, and
localhostresolves to::1first on those agents, so thehandshake went where nothing was listening. .NET reports that as an ALPN
failure ([QUIC] Application layer protocol negotiation error was encountered dotnet/runtime#85412), which points at the wrong layer. The
harnesses now bind
IPv6Any, matchingQuicTransportListener.MismatchedAlpnListenerIsRefusedhad been passing for the wrong reason,since a connection that never arrives is also refused.
code coverage— patch coverage was 60.98% against a 75% floor and an80% target. Almost all of it was unmeasured rather than untested: the
hosted Linux images ship no msquic, so
QuicListener.IsSupportedwasfalse and 61 QUIC tests skipped themselves, leaving the whole of
Opc.Ua.Bindings.Quic— 1314 of the 1600 uncovered changed lines —reading as uncovered. The QUIC transport was effectively untested in CI for
the whole life of this feature. Installing
libmsquicon the Linux legmakes those tests run; they needed no changes to pass there. A skip is
still the honest outcome where msquic is genuinely absent, so
UA_REQUIRE_QUIC(set by the setup step) turns it into a failure onlywhere the runner is meant to provide it, which is what stops this hiding
again.
Patch coverage 60.98% → 88.02%, gate passing. Every project this PR
touches is above the repo's 80% bar:
Opc.Ua.Bindings.Quic83.6%,Opc.Ua.Core83.9%,Opc.Ua.Server87.3%,Opc.Ua.Client85.8%,Opc.Ua.Bindings.Https81.6%. Whole-report line rate 86.66%.Also fixed here, though not part of the data channel work:
NU1903.NuGetAudit began failing every restore in the repository once
GHSA-q939-rpr3-3284 was published against
SSH.NET2025.1.0 — a pathtraversal in
ScpClient.Download, where the client trusts the file namesthe remote server sends and can be walked out of the download directory. It
arrives transitively through Testcontainers 4.13.0, which asks for
[2025.1.0, )and so resolves the lowest, vulnerable version. That brokeCodeQL, both Azure
net10.0builds,build-linux-all-tfmand both Kafkalegs. The patched 2026.0.0 satisfies the same range, so a central pin is
enough and nothing else changes; transitive pinning is already on
repository-wide. It affects master and every other open branch equally and
should ideally land there, but it blocked every check here. Verified:
solution restore clean under the default,
net48andnetstandard2.0pins, and the Kafka tests pass 93/93.
All checks are now green.
One flake was seen along the way and is worth recording, because it is not
this PR and will recur:
test-ubuntu-latest-Sessionsintermittently failsafter all 777 tests pass, when the global
LeakDetectionSetupteardownasserts an off-by-one certificate leak (
created=47529, disposed=47528—one instance out of ~48,000). The leak counter waits up to 60s for the count
to settle and needs 5s of stability, so it is a genuine but rare disposal
race rather than a measurement artefact.
It is pre-existing and unrelated to this branch, which touches neither
Opc.Ua.Sessions,Opc.Ua.Test.Commonnor the leak detection helpers:the identical assertion appears on unrelated branches (for example
marcschier/capture-artifact-path-, 777 passing,created=47904, disposed=47903), and across recent runs on other branchesit shows up in roughly two of nine. It passed here on re-run, and a local
Debug run of the same suite — the configuration that would print the leaked
certificate's allocation stack — came back clean, so the allocation site was
not identifiable from this branch. Worth chasing separately.
Review follow-up
All nine review comments are addressed.
The largest is the observation on
UaSCBinaryClientChannel.cs:893— "everywhere we make it look like a generic extension, but we only handle stream". That is right, and checking it decided the design:Opc.Ua.Bindings.Quicreferences zero ofISecureChannelMessageHost,ISecureChannelMessageExtension,SequenceNumberBudgetor the send gate, because a QUIC data channel rides its own stream and never becomes a UASC chunk. The seam had one implementer and no prospect of a second, so the engine moved back intoOpc.Ua.Coreand the seam is deleted rather than dressed up as general. That also answers the request to renameISecureChannelMessageExtension.csafter its interface: the file no longer exists.Stack/Tcpdrops from 1358 added lines to well under that, and theOpc.Ua.Core.Channelspackage is gone.Also in this round: the samples moved under
samples/Core; a newConsoleDataChannelAudiostreams a synthesised melody on repeat and plays it back (NAudio, sample-only dependency, WAV fallback where NAudio cannot play); the docs now describe the state of the implementation rather than its history, with two development-diary sections removed; the response-dispatched callback got a correctly named log message on all three listeners instead of borrowing one that said "Failed to send fault response";opc.quictargets net8.0 with preview features enabled rather than being excluded; and the#ifaround[Experimental]is gone, replaced by a polyfill — verified to be a real opt-in by confirming a net48 caller without the suppression fails the build witherror DataChannels.