From 378962eeb1f56ee46901137c4aa1df5f1e1f638f Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Tue, 10 Nov 2026 21:00:04 -0500 Subject: [PATCH] fix(opcuaclient-browser): keep configured host/port behind NAT (#806) The AdminUI OPC UA browse picker connected to the EndpointUrl the server advertised in GetEndpoints verbatim, so behind NAT / a container port publish it dialled the server's private hostname + internal port and failed DNS where the runtime driver (which rewrites per #676) connected. - Move the rewrite to the shared OpcUaEndpointUrlRewriter (.Contracts, pure Uri logic, no SDK dependency) and the policy+mode selection to OpcUaEndpointSelection (.Session leaf). Driver and browser both select through it, so the browse connects exactly where the driver does. - Policy/mode filter, server certificate and strict browse PKI (no auto-accept) are unchanged. - Unit tests: rewrite (host/port differ, same, IPv6 both sides, path kept, port-less, scheme mismatch untouched), browser selection seam. - Live NAT tests (Fixture=opc-plc-nat, OPCUA_NAT_ENDPOINT): browse via a published port with a premise check that the advertised host:port differs and does not resolve, nsu= round-trip, unreachable endpoint fails cleanly, SignAndEncrypt reaches the server and is refused BadCertificateUntrusted. Verified red without the rewrite. Refs #806 --- .../OpcUaClientDriverBrowser.cs | 61 +++----- .../OpcUaEndpointUrlRewriter.cs | 43 ++++++ .../OpcUaEndpointSelection.cs | 79 ++++++++++ .../OpcUaClientDriver.cs | 85 ++-------- .../NatPublishedEndpointBrowseTests.cs | 146 ++++++++++++++++++ ...pcUaClientBrowserEndpointSelectionTests.cs | 108 +++++++++++++ .../OpcUaClientEndpointRewriteTests.cs | 82 +++++++++- 7 files changed, 486 insertions(+), 118 deletions(-) create mode 100644 src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Contracts/OpcUaEndpointUrlRewriter.cs create mode 100644 src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Session/OpcUaEndpointSelection.cs create mode 100644 tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Browser.IntegrationTests/NatPublishedEndpointBrowseTests.cs create mode 100644 tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Browser.Tests/OpcUaClientBrowserEndpointSelectionTests.cs diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Browser/OpcUaClientDriverBrowser.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Browser/OpcUaClientDriverBrowser.cs index a1be26450..a815a79ae 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Browser/OpcUaClientDriverBrowser.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Browser/OpcUaClientDriverBrowser.cs @@ -223,7 +223,10 @@ public sealed class OpcUaClientDriverBrowser : IDriverBrowser return OpcUaUserIdentityFactory.Build(resolved); } - /// Select the endpoint matching the requested SecurityPolicy + SecurityMode pair. + /// + /// Discover the server's endpoints and select the one matching the requested SecurityPolicy + SecurityMode pair, + /// through . + /// /// Application configuration used by the discovery client. /// Remote endpoint URL to query. /// Required security policy. @@ -236,45 +239,27 @@ public sealed class OpcUaClientDriverBrowser : IDriverBrowser using var client = await DiscoveryClient.CreateAsync( appConfig, new Uri(url), DiagnosticsMasks.None, ct).ConfigureAwait(false); var all = await client.GetEndpointsAsync(default, ct).ConfigureAwait(false); - - var wantedPolicy = MapPolicy(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.ToList().FirstOrDefault(e => - e.SecurityPolicyUri == wantedPolicy && e.SecurityMode == wantedMode); - if (match is null) - { - var advertised = string.Join(", ", all.ToList().Select(e => - $"{ShortName(e.SecurityPolicyUri)}/{e.SecurityMode}")); - throw new InvalidOperationException( - $"No endpoint at '{url}' matches SecurityPolicy={policy} + SecurityMode={mode}. " + - $"Server advertises: {advertised}"); - } - return match; + return SelectBrowseEndpoint(all.ToList(), url, policy, mode); } - /// Convert the driver options enum to the OPC UA policy URI. - /// The driver security policy to map. - private static string MapPolicy(OpcUaSecurityPolicy p) => p 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(p)), - }; - - /// Render an OPC UA security-policy URI as its short suffix for diag messages. - /// Full policy URI to shorten. - private static string ShortName(string? uri) => - uri?.Substring(uri.LastIndexOf('#') + 1) ?? "(null)"; + /// + /// Gitea #806 — the browse session's endpoint choice, separated from the network so it is testable. It is the + /// runtime driver's selection (): the policy + mode filter + /// is unchanged, and the matched endpoint's URL keeps the operator-configured host AND port with the advertised + /// path (). The browse used to connect to the advertised URL verbatim, so + /// behind NAT / a container port publish it dialled the server's private hostname and internal port and failed + /// DNS where the deployed driver connected fine. Certificate validation is not touched: the browse PKI still + /// refuses an untrusted server certificate. + /// + /// Everything GetEndpoints returned. + /// The URL the form configured (the one GetEndpoints was sent to). + /// Required security policy. + /// Required message security mode. + /// The endpoint to open the browse session on. + internal static EndpointDescription SelectBrowseEndpoint( + IEnumerable advertised, string configuredUrl, + OpcUaSecurityPolicy policy, OpcUaSecurityMode mode) => + OpcUaEndpointSelection.SelectMatching(advertised, configuredUrl, policy, mode); } internal static class BrowseTelemetry diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Contracts/OpcUaEndpointUrlRewriter.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Contracts/OpcUaEndpointUrlRewriter.cs new file mode 100644 index 000000000..003474469 --- /dev/null +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Contracts/OpcUaEndpointUrlRewriter.cs @@ -0,0 +1,43 @@ +namespace ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient; + +/// +/// The ONE implementation of "connect where the operator pointed us, not where the server says it lives" for the +/// OpcUaClient driver family. A server reports its GetEndpoints results with ITS view of its own address; behind +/// NAT, a port map, a container port publish or a proxy that address is unreachable from here (or bypasses the +/// proxy). Shared by the runtime driver (Gitea #676) and the AdminUI live-browse picker (Gitea #806, which used +/// the advertised URL unchanged and failed DNS on a published-port fixture the driver handled). +/// +/// +/// Pure string/ logic, deliberately in .Contracts: it is not session-typed, so it must not +/// pull the OPC UA SDK in (pinned by ContractsIsDependencyLightTests). It only chooses the network +/// address; the security policy, mode and server certificate of the selected endpoint are untouched, so the +/// secure-channel certificate check still validates the server actually reached. +/// +public static class OpcUaEndpointUrlRewriter +{ + /// + /// The address to open the session on. When the advertised URL's scheme matches the configured one, the + /// configured host AND port (when it names one) replace the advertised ones and the advertised path is kept — + /// the SDK's own CoreClientUtils.SelectEndpoint rule. A different scheme is a different transport and is + /// left alone; an unparseable advertised URL falls back to the configured one. + /// + /// The EndpointUrl the server returned from GetEndpoints. + /// The URL the operator configured (the one GetEndpoints was sent to). + /// The URL to connect the session to. + public static string Rewrite(string? advertisedUrl, string configuredUrl) + { + if (!Uri.TryCreate(configuredUrl, UriKind.Absolute, out var configured)) return advertisedUrl ?? configuredUrl; + if (string.IsNullOrWhiteSpace(advertisedUrl) + || !Uri.TryCreate(advertisedUrl, UriKind.Absolute, out var advertised)) + return configuredUrl; + if (!string.Equals(advertised.Scheme, configured.Scheme, StringComparison.OrdinalIgnoreCase)) return advertisedUrl; + // A port-less configured URL (Uri.Port == -1 for opc.tcp) keeps the advertised port — copying -1 would drop it + // and the SDK would fall back to 4840 (review of 3b88056f; same fix as the client's EndpointSelector). + var builder = new UriBuilder(advertised) + { + Host = configured.DnsSafeHost, + Port = configured.Port > 0 ? configured.Port : advertised.Port, + }; + return builder.Uri.ToString(); + } +} diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Session/OpcUaEndpointSelection.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Session/OpcUaEndpointSelection.cs new file mode 100644 index 000000000..f29898288 --- /dev/null +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Session/OpcUaEndpointSelection.cs @@ -0,0 +1,79 @@ +using Opc.Ua; + +namespace ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient; + +/// +/// Policy-aware endpoint selection shared by the runtime driver and the AdminUI browse picker, so the browser +/// connects exactly where (and how) the deployed driver will. The SDK's CoreClientUtils.SelectEndpointAsync +/// only honours a boolean "use security" flag; an operator asking for Basic256Sha256 against a server that +/// also offers Basic128Rsa15 must not silently end up on the weaker cipher, so the policy + mode filter is +/// explicit and fails loudly. The selected endpoint's URL is then normalised by +/// (Gitea #676 driver, #806 browser). +/// +public static class OpcUaEndpointSelection +{ + /// + /// Pick the first advertised endpoint whose SecurityPolicyUri AND SecurityMode match, and point its + /// EndpointUrl at the configured host/port (advertised path kept). Security policy, mode, server + /// certificate and user-token policies are taken from the server's advertisement unchanged. + /// + /// Everything GetEndpoints returned. + /// The URL GetEndpoints was sent to — the operator-configured, reachable address. + /// Required security policy. + /// Required message security mode. + /// The matching endpoint, its URL rewritten. + /// No advertised endpoint matches the policy + mode pair. + public static EndpointDescription SelectMatching( + IEnumerable advertised, string configuredUrl, + OpcUaSecurityPolicy policy, OpcUaSecurityMode mode) + { + ArgumentNullException.ThrowIfNull(advertised); + var all = advertised.ToList(); + var wantedPolicyUri = MapSecurityPolicy(policy); + var wantedMode = ToMessageSecurityMode(mode); + + var match = all.FirstOrDefault(e => e.SecurityPolicyUri == wantedPolicyUri && e.SecurityMode == wantedMode); + if (match is null) + { + var list = string.Join(", ", all.Select(e => $"{ShortPolicyName(e.SecurityPolicyUri)}/{e.SecurityMode}")); + throw new InvalidOperationException( + $"No endpoint at '{configuredUrl}' matches SecurityPolicy={policy} + SecurityMode={mode}. " + + $"Server advertises: {list}"); + } + + // Connect where the operator pointed us, not where the server says it lives. + match.EndpointUrl = OpcUaEndpointUrlRewriter.Rewrite(match.EndpointUrl, configuredUrl); + return match; + } + + /// Convert a driver to the OPC UA policy URI. + /// The driver security policy to map. + /// The corresponding OPC UA security policy URI. + public 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), + }; + + /// Convert a driver to the SDK message security mode. + /// The driver security mode to map. + /// The corresponding . + public static MessageSecurityMode ToMessageSecurityMode(OpcUaSecurityMode mode) => mode switch + { + OpcUaSecurityMode.None => MessageSecurityMode.None, + OpcUaSecurityMode.Sign => MessageSecurityMode.Sign, + OpcUaSecurityMode.SignAndEncrypt => MessageSecurityMode.SignAndEncrypt, + _ => throw new ArgumentOutOfRangeException(nameof(mode)), + }; + + /// Render an OPC UA security-policy URI as its short suffix for diagnostic messages. + /// Full policy URI to shorten. + /// The suffix after the last #, or (null). + public static string ShortPolicyName(string? policyUri) => + policyUri?.Substring(policyUri.LastIndexOf('#') + 1) ?? "(null)"; +} diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDriver.cs index 411ed295a..4d1820985 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDriver.cs @@ -978,13 +978,8 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit && !string.IsNullOrWhiteSpace(e.EndpointUrl)) .Select(e => e.EndpointUrl!); - private static MessageSecurityMode ToMessageSecurityMode(OpcUaSecurityMode mode) => mode switch - { - OpcUaSecurityMode.None => MessageSecurityMode.None, - OpcUaSecurityMode.Sign => MessageSecurityMode.Sign, - OpcUaSecurityMode.SignAndEncrypt => MessageSecurityMode.SignAndEncrypt, - _ => throw new ArgumentOutOfRangeException(nameof(mode)), - }; + private static MessageSecurityMode ToMessageSecurityMode(OpcUaSecurityMode mode) => + OpcUaEndpointSelection.ToMessageSecurityMode(mode); /// /// Range checks for the subscription-tuning and discovery options. Throws at @@ -1071,11 +1066,9 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit } /// - /// 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. + /// Select the remote endpoint matching both the requested and , + /// with its URL pointed at the configured host/port. The filter + rewrite live in the shared + /// so the AdminUI browse picker selects identically (#806). /// private static async Task SelectMatchingEndpointAsync( ApplicationConfiguration appConfig, @@ -1084,61 +1077,20 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit 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. Use the SDK's asynchronous - // discovery factory and pass the caller's cancellation token. + // 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. using var client = await DiscoveryClient.CreateAsync( appConfig, new Uri(endpointUrl), Opc.Ua.DiagnosticsMasks.None, ct).ConfigureAwait(false); var all = await client.GetEndpointsAsync(default, ct).ConfigureAwait(false); - - var wantedPolicyUri = MapSecurityPolicy(policy); - var wantedMode = ToMessageSecurityMode(mode); - - var match = all.ToList().FirstOrDefault(e => - e.SecurityPolicyUri == wantedPolicyUri && e.SecurityMode == wantedMode); - - if (match is null) - { - var advertised = string.Join(", ", all.ToList() - .Select(e => $"{ShortPolicyName(e.SecurityPolicyUri)}/{e.SecurityMode}")); - throw new InvalidOperationException( - $"No endpoint at '{endpointUrl}' matches SecurityPolicy={policy} + SecurityMode={mode}. " + - $"Server advertises: {advertised}"); - } - // connect where the operator pointed us, not where the server says it lives. - match.EndpointUrl = RewriteEndpointUrl(match.EndpointUrl, endpointUrl); - return match; + return OpcUaEndpointSelection.SelectMatching(all.ToList(), endpointUrl, policy, mode); } - /// - /// the address to open the session on. A server reports its endpoints with ITS view of its own - /// address; behind NAT, a port map, a container port publish or a proxy that address is unreachable from here - /// (or bypasses the proxy). When the advertised URL's scheme matches the configured one, the configured host - /// AND port (when it names one) replace the advertised ones and the advertised path is kept — the SDK's own - /// CoreClientUtils.SelectEndpoint rule, which this driver's policy-aware selection bypassed. A different - /// scheme is a different transport and is left alone; an unparseable advertised URL falls back to the - /// configured one. - /// + /// Delegates to the shared (Gitea #676 / #806). /// The EndpointUrl the server returned from GetEndpoints. /// The URL the driver was configured with (the one GetEndpoints was sent to). /// The URL to connect the session to. - internal static string RewriteEndpointUrl(string? advertisedUrl, string configuredUrl) - { - if (!Uri.TryCreate(configuredUrl, UriKind.Absolute, out var configured)) return advertisedUrl ?? configuredUrl; - if (string.IsNullOrWhiteSpace(advertisedUrl) - || !Uri.TryCreate(advertisedUrl, UriKind.Absolute, out var advertised)) - return configuredUrl; - if (!string.Equals(advertised.Scheme, configured.Scheme, StringComparison.OrdinalIgnoreCase)) return advertisedUrl; - // A port-less configured URL (Uri.Port == -1 for opc.tcp) keeps the advertised port — copying -1 would drop it - // and the SDK would fall back to 4840 (review of 3b88056f; same fix as the client's EndpointSelector). - var builder = new UriBuilder(advertised) - { - Host = configured.DnsSafeHost, - Port = configured.Port > 0 ? configured.Port : advertised.Port, - }; - return builder.Uri.ToString(); - } + internal static string RewriteEndpointUrl(string? advertisedUrl, string configuredUrl) => + OpcUaEndpointUrlRewriter.Rewrite(advertisedUrl, configuredUrl); /// /// Build a carrying a client user-authentication @@ -1156,19 +1108,8 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit /// Convert a driver to the OPC UA policy URI. /// The driver security policy to map. /// The corresponding OPC UA security 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)"; + internal static string MapSecurityPolicy(OpcUaSecurityPolicy policy) => + OpcUaEndpointSelection.MapSecurityPolicy(policy); /// public async Task ReinitializeAsync(string driverConfigJson, CancellationToken cancellationToken) diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Browser.IntegrationTests/NatPublishedEndpointBrowseTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Browser.IntegrationTests/NatPublishedEndpointBrowseTests.cs new file mode 100644 index 000000000..459aff0cd --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Browser.IntegrationTests/NatPublishedEndpointBrowseTests.cs @@ -0,0 +1,146 @@ +using System.Net; +using System.Net.Sockets; +using Opc.Ua; +using Opc.Ua.Client; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Commons.Browsing; +using ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Browser; + +namespace ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Browser.IntegrationTests; + +/// +/// Gitea #806 — real NAT / port-publish browse. The fixture is an opc-plc container whose hostname is private and +/// whose internal port differs from the published one, so its GetEndpoints advertise an address this machine +/// cannot reach: +/// +/// docker run -d --rm --name otopcua-806-nat --hostname otopcua-806-nat-parity -p 32781:50000 \ +/// mcr.microsoft.com/iotedge/opc-plc:2.14.10 --pn=50000 --autoaccept --ut +/// OPCUA_NAT_ENDPOINT=opc.tcp://127.0.0.1:32781 dotnet test --filter Fixture=opc-plc-nat +/// +/// Skipped unless OPCUA_NAT_ENDPOINT is set and accepts TCP. Every test first proves the premise (the +/// advertised URL is NOT the configured host:port, and its host does not resolve here), so a fixture that +/// advertises a reachable address cannot make these pass vacuously. Before the fix the positive cases failed with +/// SocketException: nodename nor servname provided, or not known. +/// +[Trait("Category", "Integration"), Trait("Fixture", "opc-plc-nat")] +public sealed class NatPublishedEndpointBrowseTests +{ + private const string EnvVar = "OPCUA_NAT_ENDPOINT"; + + private static string? Endpoint => Environment.GetEnvironmentVariable(EnvVar) is { Length: > 0 } e ? e : null; + + private static string Json(string endpoint, string policy = "None", string mode = "None") => $$""" + { + "EndpointUrl":"{{endpoint}}", + "SecurityPolicy":"{{policy}}","SecurityMode":"{{mode}}","AuthType":"Anonymous", + "SessionTimeout":"00:01:00","Timeout":"00:00:10","PerEndpointConnectTimeout":"00:00:10" + } + """; + + private static string RequireFixture() + { + var endpoint = Endpoint; + if (endpoint is null) Assert.Skip($"{EnvVar} not set — no NAT/port-published opc-plc fixture."); + var uri = new Uri(endpoint); + try + { + using var tcp = new TcpClient(); + if (!tcp.ConnectAsync(uri.Host, uri.Port).Wait(TimeSpan.FromSeconds(2)) || !tcp.Connected) + Assert.Skip($"{EnvVar}={endpoint} did not accept TCP within 2s."); + } + catch (Exception ex) when (ex is not Xunit.Sdk.SkipException) + { + Assert.Skip($"{EnvVar}={endpoint} unreachable: {ex.GetBaseException().Message}"); + } + return endpoint; + } + + /// The premise: the server advertises a host:port other than the configured one, and that host is unresolvable. + private static async Task AssertAdvertisesUnreachableAddressAsync(string endpoint, CancellationToken ct) + { + var appConfig = new ApplicationConfiguration(DefaultTelemetry.Create(_ => { })) + { + ApplicationName = "OtOpcUa #806 premise", + ApplicationType = ApplicationType.Client, + SecurityConfiguration = new SecurityConfiguration(), + TransportQuotas = new TransportQuotas(), + ClientConfiguration = new ClientConfiguration(), + DisableHiResClock = true, + }; + using var discovery = await DiscoveryClient.CreateAsync(appConfig, new Uri(endpoint), DiagnosticsMasks.None, ct); + var endpoints = (await discovery.GetEndpointsAsync(default, ct)).ToList(); + endpoints.ShouldNotBeEmpty(); + + var configured = new Uri(endpoint); + foreach (var advertised in endpoints.Select(e => new Uri(e.EndpointUrl!))) + { + (advertised.Host, advertised.Port).ShouldNotBe((configured.Host, configured.Port), + "fixture must advertise a different host:port, or this test proves nothing"); + await Should.ThrowAsync(() => Dns.GetHostAddressesAsync(advertised.Host, ct), + $"advertised host '{advertised.Host}' must be unresolvable from here, or this test proves nothing"); + } + } + + [Fact] + public async Task Browse_through_a_published_port_succeeds_and_namespaced_ids_round_trip() + { + var endpoint = RequireFixture(); + var ct = TestContext.Current.CancellationToken; + await AssertAdvertisesUnreachableAddressAsync(endpoint, ct); + + await using var session = await new OpcUaClientDriverBrowser().OpenAsync(Json(endpoint), ct); + var roots = await session.RootAsync(ct); + roots.Count.ShouldBeGreaterThan(0); + + // opc-plc's own nodes live in a non-zero namespace; the browse encodes them namespace-URI-stable (nsu=). + var folder = roots.FirstOrDefault(n => n.Kind == BrowseNodeKind.Folder && n.NodeId.StartsWith("nsu=")); + folder.ShouldNotBeNull("expected an opc-plc folder in its own (nsu=) namespace under Objects"); + var children = await session.ExpandAsync(folder.NodeId, ct); + children.Count.ShouldBeGreaterThan(0); + } + + [Fact] + public async Task Unreachable_configured_endpoint_still_fails_cleanly() + { + RequireFixture(); + var ct = TestContext.Current.CancellationToken; + // A closed loopback port: the rewrite must not invent a reachable address; the open fails promptly. + var sw = System.Diagnostics.Stopwatch.StartNew(); + await Should.ThrowAsync(() => new OpcUaClientDriverBrowser().OpenAsync(Json("opc.tcp://127.0.0.1:1"), ct)); + sw.Elapsed.ShouldBeLessThan(TimeSpan.FromSeconds(30)); + } + + [Fact] + public async Task Secure_browse_reaches_the_server_and_keeps_strict_certificate_validation() + { + var endpoint = RequireFixture(); + var ct = TestContext.Current.CancellationToken; + await AssertAdvertisesUnreachableAddressAsync(endpoint, ct); + + // Basic256Sha256/SignAndEncrypt through the published port. The browse PKI never auto-accepts, so unless the + // operator trusted this fixture's certificate in the browse PKI (then set OPCUA_NAT_SERVER_CERT_TRUSTED=1) the + // open must be refused on the certificate — which proves the session reached the SERVER (no DNS failure on the + // advertised host) and that the rewrite did not relax trust. + var trusted = Environment.GetEnvironmentVariable("OPCUA_NAT_SERVER_CERT_TRUSTED") == "1"; + var json = Json(endpoint, "Basic256Sha256", "SignAndEncrypt"); + if (trusted) + { + await using var session = await new OpcUaClientDriverBrowser().OpenAsync(json, ct); + (await session.RootAsync(ct)).Count.ShouldBeGreaterThan(0); + return; + } + + var ex = await Should.ThrowAsync(async () => + { + await using var session = await new OpcUaClientDriverBrowser().OpenAsync(json, ct); + }); + TestContext.Current.TestOutputHelper?.WriteLine($"secure open refused: {ex.GetType().Name}: {ex.Message}"); + ex.ShouldNotBeOfType($"must not fail on DNS for the advertised host: {ex}"); + var sre = ex as ServiceResultException ?? ex.InnerException as ServiceResultException; + sre.ShouldNotBeNull($"expected a certificate refusal, got {ex.GetType().Name}: {ex.Message}"); + sre.StatusCode.ShouldBeOneOf( + StatusCodes.BadCertificateUntrusted, StatusCodes.BadSecurityChecksFailed, + StatusCodes.BadCertificateChainIncomplete); + } +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Browser.Tests/OpcUaClientBrowserEndpointSelectionTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Browser.Tests/OpcUaClientBrowserEndpointSelectionTests.cs new file mode 100644 index 000000000..9f167b493 --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Browser.Tests/OpcUaClientBrowserEndpointSelectionTests.cs @@ -0,0 +1,108 @@ +using System.Security.Cryptography; +using System.Security.Cryptography.X509Certificates; +using Opc.Ua; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Browser; + +namespace ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Browser.Tests; + +/// +/// Gitea #806 — the AdminUI browse picker used the endpoint URL the server ADVERTISED in GetEndpoints verbatim, so +/// behind NAT / a container port publish it dialled the server's private hostname + internal port and failed DNS +/// (opc.tcp://otopcua-sdk1-browser-nat-parity:50000/) where the runtime driver, which rewrites to the +/// configured host/port, connected. The browser now selects through the driver's shared +/// ; these tests pin the browse seam's behaviour, including that +/// the security choice is not altered by the rewrite. +/// +[Trait("Category", "Unit")] +public sealed class OpcUaClientBrowserEndpointSelectionTests +{ + private static EndpointDescription Ep(string url, MessageSecurityMode mode, string policyUri, byte[]? cert = null) => + new() + { + EndpointUrl = url, SecurityMode = mode, SecurityPolicyUri = policyUri, + ServerCertificate = cert is null ? default : new ByteString(cert), + }; + + [Fact] + public void Advertised_private_host_and_port_are_replaced_by_the_configured_ones() + { + var advertised = new[] { Ep("opc.tcp://otopcua-sdk1-browser-nat-parity:50000/", MessageSecurityMode.None, SecurityPolicies.None) }; + + var picked = OpcUaClientDriverBrowser.SelectBrowseEndpoint( + advertised, "opc.tcp://127.0.0.1:32772", OpcUaSecurityPolicy.None, OpcUaSecurityMode.None); + + picked.EndpointUrl.ShouldBe("opc.tcp://127.0.0.1:32772/"); + } + + [Fact] + public void The_advertised_path_is_kept() + { + var advertised = new[] { Ep("opc.tcp://internal:4840/Quickstarts/ReferenceServer", MessageSecurityMode.None, SecurityPolicies.None) }; + + OpcUaClientDriverBrowser.SelectBrowseEndpoint( + advertised, "opc.tcp://gateway.example:14840/", OpcUaSecurityPolicy.None, OpcUaSecurityMode.None) + .EndpointUrl.ShouldBe("opc.tcp://gateway.example:14840/Quickstarts/ReferenceServer"); + } + + [Fact] + public void The_browser_and_the_driver_choose_the_same_url() + { + const string advertisedUrl = "opc.tcp://internal-host:4840/OtOpcUa"; + const string configured = "opc.tcp://[fd00::1]:54840"; + var picked = OpcUaClientDriverBrowser.SelectBrowseEndpoint( + [Ep(advertisedUrl, MessageSecurityMode.None, SecurityPolicies.None)], configured, + OpcUaSecurityPolicy.None, OpcUaSecurityMode.None); + + picked.EndpointUrl.ShouldBe(OpcUaEndpointUrlRewriter.Rewrite(advertisedUrl, configured)); + picked.EndpointUrl.ShouldBe("opc.tcp://[fd00::1]:54840/OtOpcUa"); + } + + [Fact] + public void Security_selection_and_server_certificate_are_unchanged_by_the_rewrite() + { + using var rsa = RSA.Create(2048); + using var cert = new CertificateRequest("CN=srv", rsa, HashAlgorithmName.SHA256, RSASignaturePadding.Pkcs1) + .CreateSelfSigned(DateTimeOffset.UtcNow.AddDays(-1), DateTimeOffset.UtcNow.AddDays(1)); + var der = cert.RawData; + var advertised = new[] + { + Ep("opc.tcp://internal:50000/", MessageSecurityMode.None, SecurityPolicies.None), + Ep("opc.tcp://internal:50000/", MessageSecurityMode.Sign, SecurityPolicies.Basic256Sha256, der), + Ep("opc.tcp://internal:50000/", MessageSecurityMode.SignAndEncrypt, SecurityPolicies.Basic128Rsa15, der), + Ep("opc.tcp://internal:50000/", MessageSecurityMode.SignAndEncrypt, SecurityPolicies.Basic256Sha256, der), + }; + + var picked = OpcUaClientDriverBrowser.SelectBrowseEndpoint( + advertised, "opc.tcp://10.100.0.35:32772", + OpcUaSecurityPolicy.Basic256Sha256, OpcUaSecurityMode.SignAndEncrypt); + + picked.ShouldBeSameAs(advertised[3]); + picked.SecurityPolicyUri.ShouldBe(SecurityPolicies.Basic256Sha256); + picked.SecurityMode.ShouldBe(MessageSecurityMode.SignAndEncrypt); + picked.ServerCertificate.ToArray().ShouldBe(der); + picked.EndpointUrl.ShouldBe("opc.tcp://10.100.0.35:32772/"); + } + + [Fact] + public void An_unadvertised_policy_mode_pair_still_fails_loudly() + { + var advertised = new[] { Ep("opc.tcp://internal:50000/", MessageSecurityMode.None, SecurityPolicies.None) }; + + var ex = Should.Throw(() => OpcUaClientDriverBrowser.SelectBrowseEndpoint( + advertised, "opc.tcp://127.0.0.1:32772", OpcUaSecurityPolicy.Basic256Sha256, OpcUaSecurityMode.SignAndEncrypt)); + ex.Message.ShouldContain("Basic256Sha256"); + ex.Message.ShouldContain("None/None"); + } + + [Fact] + public void A_different_transport_scheme_is_left_alone() + { + var advertised = new[] { Ep("https://internal:443/ua", MessageSecurityMode.None, SecurityPolicies.None) }; + + OpcUaClientDriverBrowser.SelectBrowseEndpoint( + advertised, "opc.tcp://127.0.0.1:32772", OpcUaSecurityPolicy.None, OpcUaSecurityMode.None) + .EndpointUrl.ShouldBe("https://internal:443/ua"); + } +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests/OpcUaClientEndpointRewriteTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests/OpcUaClientEndpointRewriteTests.cs index 6e9ce0e8f..d6ceebb53 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests/OpcUaClientEndpointRewriteTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests/OpcUaClientEndpointRewriteTests.cs @@ -1,3 +1,4 @@ +using Opc.Ua; using Shouldly; using Xunit; @@ -7,22 +8,33 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests; /// Gitea #676 — the driver connects to the endpoint URL the server ADVERTISES in GetEndpoints. A server behind NAT, /// a port map or a proxy advertises an address the client cannot reach (or one that bypasses the proxy), so the /// driver must rewrite host AND port to the configured URL when the scheme matches — what the SDK's own -/// CoreClientUtils.SelectEndpoint does. +/// CoreClientUtils.SelectEndpoint does. Gitea #806 moved the rule to the shared +/// so the AdminUI browse picker applies the identical rule; these tests pin +/// the shared implementation and that the driver delegates to it. /// public sealed class OpcUaClientEndpointRewriteTests { [Fact] public void Host_and_port_of_the_advertised_url_are_replaced_by_the_configured_ones() { - OpcUaClientDriver.RewriteEndpointUrl("opc.tcp://internal-host:4840/OtOpcUa", "opc.tcp://10.1.2.3:54840/OtOpcUa") + OpcUaEndpointUrlRewriter.Rewrite("opc.tcp://internal-host:4840/OtOpcUa", "opc.tcp://10.1.2.3:54840/OtOpcUa") .ShouldBe("opc.tcp://10.1.2.3:54840/OtOpcUa"); } + [Fact] + public void The_806_docker_port_publish_shape_is_rewritten() + { + // The exact #806 fixture: opc-plc in a container advertises its container hostname + internal port; the + // operator reaches it through a published host port. + OpcUaEndpointUrlRewriter.Rewrite("opc.tcp://otopcua-sdk1-browser-nat-parity:50000/", "opc.tcp://127.0.0.1:32772") + .ShouldBe("opc.tcp://127.0.0.1:32772/"); + } + [Fact] public void The_advertised_path_is_kept() { // The path is the server's to choose (several endpoints may share a host:port); only the address is ours. - OpcUaClientDriver.RewriteEndpointUrl("opc.tcp://plc-a:4840/Server/Ua", "opc.tcp://gateway:6000/") + OpcUaEndpointUrlRewriter.Rewrite("opc.tcp://plc-a:4840/Server/Ua", "opc.tcp://gateway:6000/") .ShouldBe("opc.tcp://gateway:6000/Server/Ua"); } @@ -30,31 +42,52 @@ public sealed class OpcUaClientEndpointRewriteTests public void Only_the_port_differs_still_rewrites() { // The in-process stall proxy shape: same host, the configured port is the proxy's. - OpcUaClientDriver.RewriteEndpointUrl("opc.tcp://127.0.0.1:4840/OtOpcUa", "opc.tcp://127.0.0.1:61000/OtOpcUa") + OpcUaEndpointUrlRewriter.Rewrite("opc.tcp://127.0.0.1:4840/OtOpcUa", "opc.tcp://127.0.0.1:61000/OtOpcUa") .ShouldBe("opc.tcp://127.0.0.1:61000/OtOpcUa"); } + [Fact] + public void Same_host_and_port_is_unchanged() + { + OpcUaEndpointUrlRewriter.Rewrite("opc.tcp://plc-a:4840/Server", "opc.tcp://plc-a:4840/Server") + .ShouldBe("opc.tcp://plc-a:4840/Server"); + } + + [Fact] + public void Host_matching_is_case_insensitive_and_keeps_the_configured_spelling() + { + OpcUaEndpointUrlRewriter.Rewrite("opc.tcp://PLC-A:4840/Server", "OPC.TCP://plc-a:4840/") + .ShouldBe("opc.tcp://plc-a:4840/Server"); + } + [Fact] public void A_configured_url_without_a_port_keeps_the_advertised_port() { // Review of 3b88056f: Uri.Port is -1 for a port-less opc.tcp URL; copying it would drop the advertised port and // the SDK would fall back to 4840. - OpcUaClientDriver.RewriteEndpointUrl("opc.tcp://internal-host:62541/Server", "opc.tcp://gateway/Server") + OpcUaEndpointUrlRewriter.Rewrite("opc.tcp://internal-host:62541/Server", "opc.tcp://gateway/Server") .ShouldBe("opc.tcp://gateway:62541/Server"); } [Fact] public void An_IPv6_configured_host_is_bracketed() { - OpcUaClientDriver.RewriteEndpointUrl("opc.tcp://internal-host:4840/OtOpcUa", "opc.tcp://[fd00::1]:54840/OtOpcUa") + OpcUaEndpointUrlRewriter.Rewrite("opc.tcp://internal-host:4840/OtOpcUa", "opc.tcp://[fd00::1]:54840/OtOpcUa") .ShouldBe("opc.tcp://[fd00::1]:54840/OtOpcUa"); } + [Fact] + public void An_IPv6_advertised_host_is_replaced_by_a_configured_name() + { + OpcUaEndpointUrlRewriter.Rewrite("opc.tcp://[fd00::7]:4840/Ua", "opc.tcp://gateway:6000") + .ShouldBe("opc.tcp://gateway:6000/Ua"); + } + [Fact] public void A_different_scheme_is_left_alone() { // An https endpoint advertised next to an opc.tcp one is a different transport, not a NAT artefact. - OpcUaClientDriver.RewriteEndpointUrl("https://srv:443/ua", "opc.tcp://srv:4840/ua") + OpcUaEndpointUrlRewriter.Rewrite("https://srv:443/ua", "opc.tcp://srv:4840/ua") .ShouldBe("https://srv:443/ua"); } @@ -64,6 +97,39 @@ public sealed class OpcUaClientEndpointRewriteTests [InlineData("not a url")] public void An_unusable_advertised_url_falls_back_to_the_configured_one(string? advertised) { - OpcUaClientDriver.RewriteEndpointUrl(advertised, "opc.tcp://srv:4840/ua").ShouldBe("opc.tcp://srv:4840/ua"); + OpcUaEndpointUrlRewriter.Rewrite(advertised, "opc.tcp://srv:4840/ua").ShouldBe("opc.tcp://srv:4840/ua"); + } + + [Fact] + public void The_driver_delegates_to_the_shared_rewriter() + { + OpcUaClientDriver.RewriteEndpointUrl("opc.tcp://internal-host:4840/OtOpcUa", "opc.tcp://10.1.2.3:54840") + .ShouldBe(OpcUaEndpointUrlRewriter.Rewrite("opc.tcp://internal-host:4840/OtOpcUa", "opc.tcp://10.1.2.3:54840")); + } + + [Fact] + public void Shared_selection_filters_policy_and_mode_then_rewrites_the_match() + { + var advertised = new[] + { + new EndpointDescription + { + EndpointUrl = "opc.tcp://internal:50000/", SecurityMode = MessageSecurityMode.None, + SecurityPolicyUri = SecurityPolicies.None, + }, + new EndpointDescription + { + EndpointUrl = "opc.tcp://internal:50000/", SecurityMode = MessageSecurityMode.SignAndEncrypt, + SecurityPolicyUri = SecurityPolicies.Basic256Sha256, + }, + }; + + var picked = OpcUaEndpointSelection.SelectMatching( + advertised, "opc.tcp://127.0.0.1:32772", OpcUaSecurityPolicy.Basic256Sha256, OpcUaSecurityMode.SignAndEncrypt); + + picked.ShouldBeSameAs(advertised[1]); + picked.SecurityPolicyUri.ShouldBe(SecurityPolicies.Basic256Sha256); + picked.SecurityMode.ShouldBe(MessageSecurityMode.SignAndEncrypt); + picked.EndpointUrl.ShouldBe("opc.tcp://127.0.0.1:32772/"); } }