From a65215684c0eb81ce532c8618bb9228043f19e67 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Sun, 19 Apr 2026 01:44:07 -0400 Subject: [PATCH] Phase 3 PR 70 -- Apply SecurityPolicy explicitly + expand to standard OPC UA policy list. Before this PR SecurityPolicy was a string field that got ignored -- the driver only passed useSecurity=SecurityMode!=None to SelectEndpointAsync, so an operator asking for Basic256Sha256 on a server that also advertised Basic128Rsa15 could silently end up on the weaker cipher (the SDK's SelectEndpoint returns whichever matching endpoint the server listed first). PR 70 makes policy matching explicit. SecurityPolicy is now an OpcUaSecurityPolicy enum covering the six standard policies documented in OPC UA 1.04: None, Basic128Rsa15 (deprecated, brownfield interop only), Basic256 (deprecated), Basic256Sha256 (recommended baseline), Aes128_Sha256_RsaOaep, Aes256_Sha256_RsaPss. Each maps through MapSecurityPolicy to the SecurityPolicies URI constant the SDK uses for endpoint matching. New SelectMatchingEndpointAsync replaces CoreClientUtils.SelectEndpointAsync. Flow: opens a DiscoveryClient via the non-obsolete DiscoveryClient.CreateAsync(ApplicationConfiguration, Uri, DiagnosticsMasks, ct) path, calls GetEndpointsAsync to enumerate every endpoint the server advertises, filters client-side by policy URI AND mode. When no endpoint matches, throws InvalidOperationException with the full list of what the server DID advertise formatted as 'Policy/Mode' pairs so the operator sees exactly what to fix in their config without a Wireshark trace. Fail-loud behaviour intentional -- a silent fall-through to weaker crypto is worse than a clear config error. MapSecurityPolicy is internal-visible to tests via InternalsVisibleTo from PR 66. Unit tests (OpcUaClientSecurityPolicyTests, 5 facts): MapSecurityPolicy_returns_known_non_empty_uri_for_every_enum_value theory covers all 6 policies; URI contains the enum name for non-None so operators can grep logs back to the config value; MapSecurityPolicy_None_matches_SDK_None_URI, MapSecurityPolicy_Basic256Sha256_matches_SDK_URI, MapSecurityPolicy_Aes256_Sha256_RsaPss_matches_SDK_URI all cross-check against the SDK's SecurityPolicies.* constants to catch a future enum-vs-URI drift; Every_enum_value_has_a_mapping walks Enum.GetValues to ensure adding a new case doesn't silently fall through the switch. Scaffold test updated to assert SecurityPolicy default = None (was previously unchecked). 23/23 OpcUaClient.Tests pass (13 prior + 5 scaffold + 5 new policy). dotnet build clean. Note on DiscoveryClient: the synchronous DiscoveryClient.Create(...) overloads are all [Obsolete] in SDK 1.5.378; must use DiscoveryClient.CreateAsync. GetEndpointsAsync(null, ct) returns EndpointDescriptionCollection directly (not a wrapper). --- .../OpcUaClientDriver.cs | 73 ++++++++++++++++--- .../OpcUaClientDriverOptions.cs | 39 +++++++++- .../OpcUaClientDriverScaffoldTests.cs | 1 + .../OpcUaClientSecurityPolicyTests.cs | 54 ++++++++++++++ 4 files changed, 156 insertions(+), 11 deletions(-) create mode 100644 tests/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests/OpcUaClientSecurityPolicyTests.cs diff --git a/src/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDriver.cs b/src/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDriver.cs index 9baef88..3b8ed40 100644 --- a/src/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDriver.cs +++ b/src/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDriver.cs @@ -74,15 +74,9 @@ public sealed class OpcUaClientDriver(OpcUaClientDriverOptions options, string d // requested security policy/mode so the driver doesn't have to hand-validate. // UseSecurity=false when SecurityMode=None shortcuts around cert validation // entirely and is the typical dev-bench configuration. - var useSecurity = _options.SecurityMode != OpcUaSecurityMode.None; - // The non-obsolete SelectEndpointAsync overloads all require an ITelemetryContext - // parameter. Passing null is valid — the SDK falls through to its built-in default - // trace sink. Plumbing a telemetry context through every driver surface is out of - // scope; the driver emits its own logs via the health surface anyway. - var selected = await CoreClientUtils.SelectEndpointAsync( - appConfig, _options.EndpointUrl, useSecurity, - telemetry: null!, - ct: cancellationToken).ConfigureAwait(false); + var selected = await SelectMatchingEndpointAsync( + appConfig, _options.EndpointUrl, _options.SecurityPolicy, _options.SecurityMode, + cancellationToken).ConfigureAwait(false); var endpointConfig = EndpointConfiguration.Create(appConfig); endpointConfig.OperationTimeout = (int)_options.Timeout.TotalMilliseconds; var endpoint = new ConfiguredEndpoint(null, selected, endpointConfig); @@ -231,6 +225,67 @@ public sealed class OpcUaClientDriver(OpcUaClientDriverOptions options, string d return config; } + /// + /// Select the remote endpoint matching both the requested + /// and . The SDK's CoreClientUtils.SelectEndpointAsync + /// only honours a boolean "use security" flag; we need policy-aware matching so an + /// operator asking for Basic256Sha256 against a server that also offers + /// Basic128Rsa15 doesn't silently end up on the weaker cipher. + /// + private static async Task SelectMatchingEndpointAsync( + ApplicationConfiguration appConfig, + string endpointUrl, + OpcUaSecurityPolicy policy, + OpcUaSecurityMode mode, + CancellationToken ct) + { + // GetEndpoints returns everything the server advertises; policy + mode filter is + // applied client-side so the selection is explicit and fails loudly if the operator + // asks for a combination the server doesn't publish. DiscoveryClient.CreateAsync + // is the non-obsolete path in SDK 1.5.378; the synchronous Create(..) variants are + // all deprecated. + using var client = await DiscoveryClient.CreateAsync( + appConfig, new Uri(endpointUrl), Opc.Ua.DiagnosticsMasks.None, ct).ConfigureAwait(false); + var all = await client.GetEndpointsAsync(null, ct).ConfigureAwait(false); + + var wantedPolicyUri = MapSecurityPolicy(policy); + var wantedMode = mode switch + { + OpcUaSecurityMode.None => MessageSecurityMode.None, + OpcUaSecurityMode.Sign => MessageSecurityMode.Sign, + OpcUaSecurityMode.SignAndEncrypt => MessageSecurityMode.SignAndEncrypt, + _ => throw new ArgumentOutOfRangeException(nameof(mode)), + }; + + var match = all.FirstOrDefault(e => + e.SecurityPolicyUri == wantedPolicyUri && e.SecurityMode == wantedMode); + + if (match is null) + { + var advertised = string.Join(", ", all + .Select(e => $"{ShortPolicyName(e.SecurityPolicyUri)}/{e.SecurityMode}")); + throw new InvalidOperationException( + $"No endpoint at '{endpointUrl}' matches SecurityPolicy={policy} + SecurityMode={mode}. " + + $"Server advertises: {advertised}"); + } + return match; + } + + /// Convert a driver to the OPC UA policy URI. + internal static string MapSecurityPolicy(OpcUaSecurityPolicy policy) => policy switch + { + OpcUaSecurityPolicy.None => SecurityPolicies.None, + OpcUaSecurityPolicy.Basic128Rsa15 => SecurityPolicies.Basic128Rsa15, + OpcUaSecurityPolicy.Basic256 => SecurityPolicies.Basic256, + OpcUaSecurityPolicy.Basic256Sha256 => SecurityPolicies.Basic256Sha256, + OpcUaSecurityPolicy.Aes128_Sha256_RsaOaep => SecurityPolicies.Aes128_Sha256_RsaOaep, + OpcUaSecurityPolicy.Aes256_Sha256_RsaPss => SecurityPolicies.Aes256_Sha256_RsaPss, + _ => throw new ArgumentOutOfRangeException(nameof(policy), policy, null), + }; + + private static string ShortPolicyName(string policyUri) => + policyUri?.Substring(policyUri.LastIndexOf('#') + 1) ?? "(null)"; + public async Task ReinitializeAsync(string driverConfigJson, CancellationToken cancellationToken) { await ShutdownAsync(cancellationToken).ConfigureAwait(false); diff --git a/src/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDriverOptions.cs b/src/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDriverOptions.cs index 87629eb..a620bae 100644 --- a/src/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDriverOptions.cs +++ b/src/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDriverOptions.cs @@ -16,8 +16,16 @@ public sealed class OpcUaClientDriverOptions /// Remote OPC UA endpoint URL, e.g. opc.tcp://plc.internal:4840. public string EndpointUrl { get; init; } = "opc.tcp://localhost:4840"; - /// Security policy. One of None, Basic256Sha256, Aes128_Sha256_RsaOaep. - public string SecurityPolicy { get; init; } = "None"; + /// + /// Security policy to require when selecting an endpoint. Either a + /// enum constant or a free-form string (for + /// forward-compatibility with future OPC UA policies not yet in the enum). + /// Matched against EndpointDescription.SecurityPolicyUri suffix — the driver + /// connects to the first endpoint whose policy name matches AND whose mode matches + /// . When set to + /// the driver picks any unsecured endpoint regardless of policy string. + /// + public OpcUaSecurityPolicy SecurityPolicy { get; init; } = OpcUaSecurityPolicy.None; /// Security mode. public OpcUaSecurityMode SecurityMode { get; init; } = OpcUaSecurityMode.None; @@ -96,6 +104,33 @@ public enum OpcUaSecurityMode SignAndEncrypt, } +/// +/// OPC UA security policies recognized by the driver. Maps to the standard +/// http://opcfoundation.org/UA/SecurityPolicy# URI suffixes the SDK uses for +/// endpoint matching. +/// +/// +/// and are deprecated per OPC UA +/// spec v1.04 — they remain in the enum only for brownfield interop with older servers. +/// Prefer , , or +/// for new deployments. +/// +public enum OpcUaSecurityPolicy +{ + /// No security. Unsigned, unencrypted wire. + None, + /// Deprecated (OPC UA 1.04). Retained for legacy server interop. + Basic128Rsa15, + /// Deprecated (OPC UA 1.04). Retained for legacy server interop. + Basic256, + /// Recommended baseline for current deployments. + Basic256Sha256, + /// Current OPC UA policy; AES-128 + SHA-256 + RSA-OAEP. + Aes128_Sha256_RsaOaep, + /// Current OPC UA policy; AES-256 + SHA-256 + RSA-PSS. + Aes256_Sha256_RsaPss, +} + /// User authentication type sent to the remote server. public enum OpcUaAuthType { diff --git a/tests/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests/OpcUaClientDriverScaffoldTests.cs b/tests/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests/OpcUaClientDriverScaffoldTests.cs index c145cd5..02911af 100644 --- a/tests/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests/OpcUaClientDriverScaffoldTests.cs +++ b/tests/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests/OpcUaClientDriverScaffoldTests.cs @@ -18,6 +18,7 @@ public sealed class OpcUaClientDriverScaffoldTests var opts = new OpcUaClientDriverOptions(); opts.EndpointUrl.ShouldBe("opc.tcp://localhost:4840", "4840 is the IANA-assigned OPC UA port"); opts.SecurityMode.ShouldBe(OpcUaSecurityMode.None); + opts.SecurityPolicy.ShouldBe(OpcUaSecurityPolicy.None); opts.AuthType.ShouldBe(OpcUaAuthType.Anonymous); opts.AutoAcceptCertificates.ShouldBeFalse("production default must reject untrusted server certs"); } diff --git a/tests/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests/OpcUaClientSecurityPolicyTests.cs b/tests/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests/OpcUaClientSecurityPolicyTests.cs new file mode 100644 index 0000000..c616a68 --- /dev/null +++ b/tests/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests/OpcUaClientSecurityPolicyTests.cs @@ -0,0 +1,54 @@ +using Opc.Ua; +using Shouldly; +using Xunit; + +namespace ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests; + +[Trait("Category", "Unit")] +public sealed class OpcUaClientSecurityPolicyTests +{ + [Theory] + [InlineData(OpcUaSecurityPolicy.None)] + [InlineData(OpcUaSecurityPolicy.Basic128Rsa15)] + [InlineData(OpcUaSecurityPolicy.Basic256)] + [InlineData(OpcUaSecurityPolicy.Basic256Sha256)] + [InlineData(OpcUaSecurityPolicy.Aes128_Sha256_RsaOaep)] + [InlineData(OpcUaSecurityPolicy.Aes256_Sha256_RsaPss)] + public void MapSecurityPolicy_returns_known_non_empty_uri_for_every_enum_value(OpcUaSecurityPolicy policy) + { + var uri = OpcUaClientDriver.MapSecurityPolicy(policy); + uri.ShouldNotBeNullOrEmpty(); + // Each URI should end in the enum name (for the non-None policies) so a driver + // operator reading logs can correlate the URI back to the config value. + if (policy != OpcUaSecurityPolicy.None) + uri.ShouldContain(policy.ToString()); + } + + [Fact] + public void MapSecurityPolicy_None_matches_SDK_None_URI() + { + OpcUaClientDriver.MapSecurityPolicy(OpcUaSecurityPolicy.None) + .ShouldBe(SecurityPolicies.None); + } + + [Fact] + public void MapSecurityPolicy_Basic256Sha256_matches_SDK_URI() + { + OpcUaClientDriver.MapSecurityPolicy(OpcUaSecurityPolicy.Basic256Sha256) + .ShouldBe(SecurityPolicies.Basic256Sha256); + } + + [Fact] + public void MapSecurityPolicy_Aes256_Sha256_RsaPss_matches_SDK_URI() + { + OpcUaClientDriver.MapSecurityPolicy(OpcUaSecurityPolicy.Aes256_Sha256_RsaPss) + .ShouldBe(SecurityPolicies.Aes256_Sha256_RsaPss); + } + + [Fact] + public void Every_enum_value_has_a_mapping() + { + foreach (OpcUaSecurityPolicy p in Enum.GetValues()) + Should.NotThrow(() => OpcUaClientDriver.MapSecurityPolicy(p)); + } +}