Repository navigation
Secure the UADP DataSetMetaData announcement on secured connections - #4712
Merged
Merged
Conversation
MetaDataPublisher encoded the DataSetMetaData announcement itself and
called IPubSubTransport.SendAsync directly. That bypassed the connection's
security wrapper and its fail-closed check, so a WriterGroup configured for
SignAndEncrypt announced its DataSet layout (field names, types, version)
unsigned and unencrypted. A secured subscriber dropped the announcement
("Dropping unsecured inbound frame ... requiring SignAndEncrypt"), and any
listener could read it. OPC 10000-14 6.2.5.2 applies the WriterGroup's
SecurityMode to the NetworkMessages it publishes, and 7.2.4.6.3 carries the
security of a discovery announcement in its SecurityHeader.
Cause, confirmed on the wire: the documented udp-uadp exchange sent eight
secured DataSetMessages and one DiscoveryResponse (announcement type 2,
DataSetMetaData) with ExtendedFlags1 = 0x81, i.e. without the SecurityHeader
bit.
Fix:
- MetaDataPublisher builds the announcement and hands it to the connection
(PubSubConnection.SendMetaDataAnnouncementAsync), which applies the
connection's message security, or refuses to send when it cannot secure
the message.
- UadpDiscoveryCoder can encode a discovery message with the SecurityHeader
bit and reports the security boundary after the PublisherId (and
DataSetClassId), where UadpDecoder.TryReadOuterPrefix ends the prefix.
UadpEncoder.EncodeWithSecurityBoundary accepts discovery messages.
- PubSubConnection.SendNetworkMessageAsync wraps UADP discovery responses.
Before, a secured connection refused them (metadata and writer
configuration probe responses, and the MQTT discovery topics); now it
sends them secured. The receive path is unchanged: it already unwraps
secured discovery frames and registers authenticated metadata.
Tests:
- MetaDataPublisherTests.UadpPathSecuresTheAnnouncementOfASecuredWriterGroupAsync:
the announcement of a SignAndEncrypt WriterGroup carries a SecurityHeader,
does not contain the DataSet name in cleartext, and unwraps and decodes
with the subscriber's keys. Red before the fix (no SecurityHeader).
- UadpDiscoveryTests.SecuredDiscoveryMessagesSplitWhereTheReceiverExpects:
for requests and responses, with and without a DataSetClassId, the
secured encoding differs from the unsecured one only by the SecurityHeader
flag, and its boundary equals the receiver's prefix length.
Validation: the documented exchange (with the sample KeyNonce fix applied)
now sends the announcement secured, logs no unsecured-frame warning, and the
subscriber logs "Registered inbound metadata for writer 1". docs/PubSub.md
no longer lists the unsecured-frame warning and describes the secured
announcement.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
marcschier
marked this pull request as ready for review
October 9, 2026 11:05
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Secured discovery requests are dropped, while transport-specific discovery announcements still bypass security.
2 open findings
What changed in this PR
Routes UADP metadata announcements through connection security and adds secured discovery encoding.
Changes:
- Adds security boundaries for UADP discovery messages.
- Secures metadata announcements through the connection pipeline.
- Adds regression tests and updates PubSub documentation.
| File | Description |
|---|---|
src/Opc.Ua.PubSub/Application/MetaDataPublisher.cs |
Delegates announcement sending to the connection. |
src/Opc.Ua.PubSub/Application/PubSubApplication.cs |
Updates publisher construction. |
src/Opc.Ua.PubSub/Connections/PubSubConnection.cs |
Wraps discovery responses with UADP security. |
src/Opc.Ua.PubSub/Encoding/Uadp/UadpDiscoveryCoder.cs |
Adds secured discovery encoding boundaries. |
src/Opc.Ua.PubSub/Encoding/Uadp/UadpEncoder.cs |
Supports secured discovery messages. |
tests/Opc.Ua.PubSub.Tests/Application/MetaDataPublisherTests.cs |
Tests encrypted metadata announcements. |
tests/Opc.Ua.PubSub.Tests/Encoding/Uadp/UadpDiscoveryTests.cs |
Tests discovery security boundaries. |
docs/PubSub.md |
Documents secured metadata announcements. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Code coverage✅ Coverage gate passed.
Uncovered changed lines
Coverage is above the recorded baseline - consider ratcheting Thresholds live in |
Review feedback: - A secured connection dropped its own discovery requests: the fail-closed check refused every UadpDiscoveryRequestMessage, so a secured subscriber could not ask a publisher for metadata, writer configuration, or endpoints. Requests are now secured like responses on connections that use the UADP encoding. - The ApplicationInformation, PublisherEndpoints, and PubSubConnection announcements that UDP and Ethernet send to their transport-specific discovery destination were still encoded in plaintext. SendDiscoveryResponseAsync now uses the same encode-and-secure step as SendNetworkMessageAsync (EncodeOutboundAsync), which also refuses to send a message that a secured connection cannot protect. The MQTT last-will status message is unchanged: it is registered with the broker when the connection opens and published later, so it cannot carry a current SecurityHeader. docs/PubSub.md describes the secured discovery and this exception. Tests: - PubSubDiscoveryTests.SecuredSubscriberDiscoversSecuredPublisherWithoutPlaintextFramesAsync connects a secured publisher and a secured subscriber through an in-memory transport that also sends announcements to a separate destination. Metadata, writer configuration, and endpoints are discovered, and no frame without a SecurityHeader is sent. Before the fix the metadata request was dropped. - UadpDiscoveryTests.EncodeWithSecurityBoundaryRejectsAMessageItCannotSecure. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
romanett
approved these changes
Oct 9, 2026
Review feedback: - Secured NetworkMessages are encoded at the security boundary, outside UadpEncoder.EncodeAsync, which is where SentNetworkMessages and SentDataSetMessages are counted. Secured data messages were therefore never counted, and the discovery messages this branch now secures stopped being counted. EncodeAndWrapUadpAsync counts them the same way the encoder counts unsecured messages. - MessageSecurityMode.Sign signs without encrypting, so the DataSet layout of a Sign-only announcement stays readable. docs/PubSub.md, the MetaDataPublisher remarks, and SendMetaDataAnnouncementAsync now describe applying the configured SecurityMode, and limit confidentiality to SignAndEncrypt. Test: MetaDataPublisherTests.SecuredNetworkMessagesAreCountedAsSentAsync (the counter read 0 before the fix). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Securing the discovery announcements (previous commits) made them depend on a current security key. When the key provider has none yet, as before a Security Key Service delivers the first key, or when the key does not fit the policy (the reference sample's 12-byte KeyNonce, fixed in #4710), wrapping the startup announcements threw out of EnableAsync, and StartAsync failed. The aot-ubuntu-latest job failed this way in PubSubAotTests.StartsAndStopsPublisher_UdpUadp. A response to a discovery probe that could not be secured threw into the receive loop and ended it. SendDiscoveryResponseAsync now logs a discovery message that cannot be secured or sent (DiscoveryMessageNotSent, Warning) and drops it, as the UDP transport already does for network errors of discovery announcements. Requests sent through RequestDiscoveryAsync still throw to the caller. Tests (PubSubDiscoveryTests), both red before the fix: - StartupAnnouncementsWithoutACurrentKeyDoNotFailTheStartAsync - ReceiveLoopSurvivesADiscoveryResponseThatCannotBeSecuredAsync Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #4712 +/- ##
=======================================
Coverage 82.84% 82.85%
=======================================
Files 2267 2267
Lines 345943 345941 -2
Branches 60303 60303
=======================================
+ Hits 286609 286620 +11
+ Misses 39238 39230 -8
+ Partials 20096 20091 -5
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
3 of 7 tasks
marcschier
added a commit
that referenced
this pull request
Oct 11, 2026
…#4724) # Description #4712 makes a connection configured for message security send its DataSetMetaData announcement through the connection's security step, so the announcement is never sent unsecured. Its tests cover the UADP announcement. This adds the JSON case. The UADP security wrapper cannot protect a JSON message, so a connection whose WriterGroup uses `SignAndEncrypt` must refuse the JSON metadata announcement rather than publish the DataSet layout in plaintext. `MetaDataPublisherTests.JsonAnnouncementOfASecuredWriterGroupIsRefusedAsync` checks that nothing is published on the metadata topic and that the refusal is counted in `EncryptionErrors`. `PubSubApplication.StartAsync` returns after the initial announcement was sent or refused, so the check needs no wait. The test covers the fail-closed branch of `PubSubConnection.EncodeOutboundAsync` and the refused-send return of `SendNetworkMessageAsync`, two of the three changed lines the patch-coverage report of #4712 listed as uncovered. Test-only change. Review: the test double `RecordingTransport` guards its recorded sends with a private `System.Threading.Lock` and exposes `SendCount` and `GetSendsSnapshot()`, so the tests no longer lock the list itself (b7e0ba4). ### Validation | Command | Result | | --- | --- | | `dotnet test tests/Opc.Ua.PubSub.Tests -f net10.0 --filter "Name~JsonAnnouncementOfASecuredWriterGroupIsRefusedAsync"` with the fail-closed branch of `EncodeOutboundAsync` temporarily removed | Fails: `Expected: no item String containing "/metadata/"`, but the announcement was published on `opcua/json/metadata/p17/7/42` | | `dotnet test tests/Opc.Ua.PubSub.Tests -f net10.0` | 1499 passed, 1 skipped | | `dotnet test tests/Opc.Ua.PubSub.Tests -f net48 --filter FullyQualifiedName~MetaDataPublisherTests` | 10 passed | | On the merged head with the lock change (07142f7): `MetaDataPublisherTests` on net10.0 (3 runs) and net48; the full `Opc.Ua.PubSub.Tests` on net10.0 | 10 passed each; 1498 passed, 2 skipped (the UDP multicast loopback tests, which skip when loopback is not available) | ## Related Issues - Follow-up to #4712. ## Checklist - [ ] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [ ] I have addressed **all** PR feedback received. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: marcschier <11168470+marcschier@users.noreply.github.com>
This was referenced Oct 12, 2026
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
MetaDataPublisherencoded the DataSetMetaData announcement itself and calledIPubSubTransport.SendAsyncdirectly. That bypassed the connection's security wrapper and its fail-closed check. A WriterGroup configured forSignAndEncrypttherefore announced its DataSet layout (field names, types, version) unsigned and unencrypted. A secured subscriber dropped the announcement (Dropping unsecured inbound frame ... requiring SignAndEncrypt, documented as an expected warning in docs/PubSub.md), and any listener could read it. OPC 10000-14 §6.2.5.2 applies the WriterGroup's SecurityMode to the NetworkMessages it publishes, and §7.2.4.6.3 carries the security of a discovery announcement in its SecurityHeader.Confirmed on the wire: the documented
udp-uadpexchange sent secured DataSetMessages plus one DiscoveryResponse (announcement type 2, DataSetMetaData) with ExtendedFlags10x81, meaning the SecurityHeader bit was clear.Review feedback widened the scope to every UADP discovery message of a secured connection, and to the diagnostics of secured messages.
Fix:
MetaDataPublisherbuilds the announcement and hands it to the connection (PubSubConnection.SendMetaDataAnnouncementAsync).PubSubConnection.EncodeOutboundAsync) now serves data messages, discovery requests and responses, and the announcements that UDP and Ethernet send to their transport-specific discovery destination (ApplicationInformation, PublisherEndpoints, PubSubConnection). On a connection configured for message security, it applies the SecurityMode (Sign, orSignAndEncrypt) to UADP messages and refuses to send any message it cannot secure.DiscoveryMessageNotSent, Warning) and dropped. Securing announcements makes them depend on a current key; without this, a key provider with no key yet, for example before a Security Key Service delivers the first key, madeStartAsyncfail, and a probe response that could not be secured ended the receive loop. Requests sent throughRequestDiscoveryAsyncstill throw to the caller.SentNetworkMessagesandSentDataSetMessageslike unsecured ones. Before, they bypassed the encoder that counts them, so secured data messages were never counted.UadpDiscoveryCodercan encode a discovery message with the SecurityHeader bit set, and reports the security boundary after the PublisherId (and DataSetClassId), which is whereUadpDecoder.TryReadOuterPrefixends the prefix.UadpEncoder.EncodeWithSecurityBoundaryaccepts discovery messages.Signauthenticates;SignAndEncryptalso encrypts) and the last-will exception.Tests:
MetaDataPublisherTests.UadpPathSecuresTheAnnouncementOfASecuredWriterGroupAsync: the announcement of a SignAndEncrypt WriterGroup carries a SecurityHeader, does not contain the DataSet name in cleartext, and unwraps and decodes with the subscriber's keys. Before the fix, the announcement had no SecurityHeader.MetaDataPublisherTests.SecuredNetworkMessagesAreCountedAsSentAsync: before the fix,SentNetworkMessagesread 0 on a secured connection.PubSubDiscoveryTests.SecuredSubscriberDiscoversSecuredPublisherWithoutPlaintextFramesAsync: a secured subscriber and a secured publisher, connected through an in-memory transport that also sends announcements to a separate destination, discover metadata, writer configuration, and endpoints, and no frame lacks a SecurityHeader. Before the fix, the metadata request was dropped. With only the announcement change reverted, the PublisherEndpoints response was dropped.PubSubDiscoveryTests.StartupAnnouncementsWithoutACurrentKeyDoNotFailTheStartAsyncandReceiveLoopSurvivesADiscoveryResponseThatCannotBeSecuredAsync: both failed before the fail-soft change.UadpDiscoveryTests.SecuredDiscoveryMessagesSplitWhereTheReceiverExpects: for requests and responses, with and without a DataSetClassId, the secured encoding differs from the unsecured one only by the SecurityHeader flag, and its boundary equals the receiver's prefix length.UadpDiscoveryTests.EncodeWithSecurityBoundaryRejectsAMessageItCannotSecure.Validation
dotnet test tests/Opc.Ua.PubSub.Tests -f net10.0 -p:CustomTestTarget=net10.0dotnet test tests/Opc.Ua.PubSub.Tests -f net48 -p:CustomTestTarget=net48dotnet teston net10.0 forOpc.Ua.PubSub.Udp.Tests,.Eth.Tests,.Mqtt.Tests,.Server.Tests,fuzzing/Opc.Ua.PubSub.Fuzz.Testsdotnet teston net10.0 forOpc.Ua.PubSub.Kafka.Tests,.Adapter.Tests,.Diagnostics.Tests,Opc.Ua.Redundancy.PubSub.Tests(on 9417ccb)tests\Opc.Ua.Aot.Tests\bin\Debug\net10.0\Opc.Ua.Aot.Tests.exe --treenode-filter "/*/*/PubSubAotTests/*"(JIT; the AOT job failed on 94d4391)StartsAndStopsPublisher_UdpUadpdotnet build src/Opc.Ua.PubSub -c Release(all target frameworks)0x91, 268 bytes); 14 DataSets; no unsecured-frame warning; the subscriber logsRegistered inbound metadata for writer 1dotnet build src/Opc.Ua.PubSub -c Releasedotnet test tests/Opc.Ua.PubSub.Tests -f net10.0; the changed fixtures on net48;dotnet test tests/Opc.Ua.PubSub.Udp.Tests -f net10.0Opc.Ua.Aot.Tests.exe --treenode-filter "/*/*/PubSubAotTests/*"(JIT)SamplePublisherDeliversSecuredDataSetToSampleSubscriberAsyncopc.udp://127.0.0.1:4840, then the publisher alone against a UDP listenerRelated Issues
Checklist