From 8e8408a9ca68def9851bcc7539bdcc3111c6dc5a Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Wed, 11 Nov 2026 01:32:38 -0500 Subject: [PATCH 1/4] fix(opcuaclient): keep the configured IPv6 zone index through the endpoint rewrite (#819) .NET's Uri parses a zone ([fe80::1%eth0]) but drops it from Host, Authority, AbsoluteUri and ToString(); only DnsSafeHost/IdnHost carry it. The shared OpcUaEndpointUrlRewriter rebuilt the URL through UriBuilder, so the zone was lost and link-local endpoints were unroutable. The SDK dials new Uri(EndpointUrl).IdnHost (TcpByteTransport -> DnsEndPoint), which keeps it, so splicing the zone back into the string is enough. Kept verbatim (%eth0, %4, or RFC 6874 %25eth0) so the session dials exactly what GetEndpoints reached. Applies to the driver, the AdminUI browser and the Client CLI/UI (all share the rewriter). --- .../OpcUaEndpointUrlRewriter.cs | 29 ++++++++++++++- .../EndpointSelectorRewriteParityTests.cs | 6 ++- .../OpcUaClientEndpointRewriteTests.cs | 37 +++++++++++++++++++ 3 files changed, 69 insertions(+), 3 deletions(-) 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 index c15217a60..68f98169d 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Contracts/OpcUaEndpointUrlRewriter.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Contracts/OpcUaEndpointUrlRewriter.cs @@ -46,6 +46,33 @@ public static class OpcUaEndpointUrlRewriter return advertisedUrl; var builder = new UriBuilder(advertised) { Host = configured.DnsSafeHost, Port = port }; // AbsoluteUri, not ToString(): ToString() un-escapes, turning an advertised "/x%20y" into "/x y" (#812). - return builder.Uri.AbsoluteUri; + return RestoreZoneIndex(builder.Uri, configured); + } + + /// + /// Gitea #819 — keep an IPv6 zone index ([fe80::1%eth0]) from the configured host. .NET's + /// parses the zone (only / carry it) but drops it from + /// , , and ToString(), so + /// any re-serialised URL loses it. The SDK dials new Uri(EndpointUrl).IdnHost, which DOES carry the zone + /// — so the zone is spliced back into the string, verbatim as the operator wrote it (%eth0, %4, or + /// an RFC 6874 %25eth0: not normalised, so the session dials exactly what GetEndpoints just reached). + /// Without it a link-local endpoint is unroutable: the session would dial a zone-less fe80:: address. + /// + /// The rewritten URL (configured host, zone already lost). + /// The operator-configured URL. + /// 's absolute URI with the configured zone index restored. + private static string RestoreZoneIndex(Uri rebuilt, Uri configured) + { + var absolute = rebuilt.AbsoluteUri; + if (configured.HostNameType != UriHostNameType.IPv6) return absolute; + var zoned = configured.DnsSafeHost; // "fe80::1%eth0" — the only accessor that keeps it + if (zoned.IndexOf('%') < 0) return absolute; + + var bracketed = rebuilt.Host; // "[fe80::1]" + var authorityStart = absolute.IndexOf("://", StringComparison.Ordinal); + if (authorityStart < 0) return absolute; + var hostAt = absolute.IndexOf(bracketed, authorityStart + 3, StringComparison.OrdinalIgnoreCase); + if (hostAt < 0) return absolute; + return absolute[..hostAt] + "[" + zoned + "]" + absolute[(hostAt + bracketed.Length)..]; } } diff --git a/tests/Client/ZB.MOM.WW.OtOpcUa.Client.Shared.Tests/Adapters/EndpointSelectorRewriteParityTests.cs b/tests/Client/ZB.MOM.WW.OtOpcUa.Client.Shared.Tests/Adapters/EndpointSelectorRewriteParityTests.cs index 84d861b46..4ba60048a 100644 --- a/tests/Client/ZB.MOM.WW.OtOpcUa.Client.Shared.Tests/Adapters/EndpointSelectorRewriteParityTests.cs +++ b/tests/Client/ZB.MOM.WW.OtOpcUa.Client.Shared.Tests/Adapters/EndpointSelectorRewriteParityTests.cs @@ -42,8 +42,10 @@ public sealed class EndpointSelectorRewriteParityTests [InlineData("opc.tcp://plc-a:4840/Server", "opc.tcp://PLC-A:4840/", "opc.tcp://plc-a:4840/Server")] // A real rewrite keeps the advertised path's percent-escaping intact. [InlineData("opc.tcp://a:4840/x%20y?q=1", "opc.tcp://b:1/", "opc.tcp://b:1/x%20y?q=1")] - // An IPv6 zone index on the configured host is dropped (UriBuilder behaviour, same in both copies). - [InlineData("opc.tcp://internal-host:4840/OtOpcUa", "opc.tcp://[fe80::1%25eth0]:4841", "opc.tcp://[fe80::1]:4841/OtOpcUa")] + // #819: an IPv6 zone index on the configured host is KEPT, verbatim (it used to be dropped — UriBuilder behaviour, + // preserved deliberately through #812 — leaving link-local endpoints unroutable). + [InlineData("opc.tcp://internal-host:4840/OtOpcUa", "opc.tcp://[fe80::1%25eth0]:4841", "opc.tcp://[fe80::1%25eth0]:4841/OtOpcUa")] + [InlineData("opc.tcp://internal-host:4840/OtOpcUa", "opc.tcp://[fe80::1%eth0]:4841", "opc.tcp://[fe80::1%eth0]:4841/OtOpcUa")] public void SelectBest_rewrites_exactly_as_before(string advertised, string configured, string expected) { Select(advertised, configured).ShouldBe(expected); 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 53e9b7a9c..2d27d1f27 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 @@ -94,6 +94,43 @@ public sealed class OpcUaClientEndpointRewriteTests .ShouldBe("opc.tcp://[fd00::1]:54840/OtOpcUa"); } + /// + /// Gitea #819 — a configured IPv6 zone index survives the rewrite verbatim. .NET's drops it from + /// every re-serialisation (Host, Authority, AbsoluteUri, ToString), so before #819 a link-local endpoint came out + /// zone-less and unroutable. + /// + [Theory] + [InlineData("opc.tcp://[fe80::1%eth0]:4841", "opc.tcp://[fe80::1%eth0]:4841/OtOpcUa")] + [InlineData("opc.tcp://[fe80::1%4]:4841/", "opc.tcp://[fe80::1%4]:4841/OtOpcUa")] + // RFC 6874's encoded delimiter is kept as written, not normalised: the session dials what GetEndpoints reached. + [InlineData("opc.tcp://[fe80::1%25eth0]:4841", "opc.tcp://[fe80::1%25eth0]:4841/OtOpcUa")] + // Port-less configured URL: the advertised port is kept, the zone still is. + [InlineData("opc.tcp://[fe80::1%eth0]", "opc.tcp://[fe80::1%eth0]:4840/OtOpcUa")] + public void An_IPv6_zone_index_on_the_configured_host_is_preserved(string configured, string expected) + { + OpcUaEndpointUrlRewriter.Rewrite("opc.tcp://internal-host:4840/OtOpcUa", configured).ShouldBe(expected); + } + + [Fact] + public void A_zone_index_reaches_the_host_the_SDK_dials() + { + // The SDK's TCP transport connects to new Uri(EndpointDescription.EndpointUrl).IdnHost (ConfiguredEndpoint + // re-parses the string), so the zone must survive into that accessor — and the advertised path with it. + var rewritten = OpcUaEndpointUrlRewriter.Rewrite( + "opc.tcp://[fe80::9]:4840/Ua", "opc.tcp://[fe80::1%eth0]:4841"); + var endpoint = new ConfiguredEndpoint(null, new EndpointDescription { EndpointUrl = rewritten }, EndpointConfiguration.Create()); + endpoint.EndpointUrl!.IdnHost.ShouldBe("fe80::1%eth0"); + endpoint.EndpointUrl.Port.ShouldBe(4841); + endpoint.EndpointUrl.AbsolutePath.ShouldBe("/Ua"); + } + + [Fact] + public void A_zone_matching_no_op_returns_the_advertised_string_verbatim() + { + OpcUaEndpointUrlRewriter.Rewrite("opc.tcp://[fe80::1%eth0]:4840/Ua", "opc.tcp://[fe80::1%eth0]:4840/") + .ShouldBe("opc.tcp://[fe80::1%eth0]:4840/Ua"); + } + [Fact] public void An_IPv6_advertised_host_is_replaced_by_a_configured_name() { From bde6cce2fc6bcc8d0da6e55708c62e1e4fb57920 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Wed, 11 Nov 2026 01:32:38 -0500 Subject: [PATCH 2/4] test(infra): plain opc-plc fixture healthcheck uses bash /dev/tcp (#817) The opc-plc image ships no netstat, so the old check left otopcua-opc-plc permanently unhealthy. Same probe as the NAT fixture (27302314). Redeployed to /opt/otopcua-opcuaclient on the shared docker host; reports healthy. --- .../Docker/docker-compose.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.IntegrationTests/Docker/docker-compose.yml b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.IntegrationTests/Docker/docker-compose.yml index 18849ce10..8b8f83c16 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.IntegrationTests/Docker/docker-compose.yml +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.IntegrationTests/Docker/docker-compose.yml @@ -38,7 +38,9 @@ services: # opc-plc doesn't expose an HTTP health endpoint by default; use a TCP # probe via a shell the base image ships with. The fixture does its own # TCP probe but healthcheck surfaces status in `docker ps` for humans. - test: ["CMD-SHELL", "netstat -an | grep -q ':50000.*LISTEN' || exit 1"] + # #817: bash's /dev/tcp, NOT netstat — the image ships no netstat, so the old + # `netstat -an | grep` check left the container permanently "unhealthy" (same fix as nat/, 27302314). + test: ["CMD", "bash", "-c", "exec 3<>/dev/tcp/127.0.0.1/50000"] interval: 5s timeout: 2s retries: 10 From ae91e6b7f75b910b12e13f99bb34ea63fe6959b6 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Wed, 11 Nov 2026 01:33:00 -0500 Subject: [PATCH 3/4] feat(opcuaclient): opt-in server-certificate host-name check; NAT fixture keeps its PKI (#816, #818) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #816 finding: the SDK checks that the server certificate names the host only when a session is created with checkDomain:true. The driver, the AdminUI browser and Client.Shared all called the 8-argument DefaultSessionFactory.CreateAsync, which forwards checkDomain:false — so no host-name check ever ran (why a trusted NAT cert for *.invalid browsed fine over an IP in #813). When enabled the SDK (Session.OpenAsync -> ValidateDomains -> FindDomain) compares ConfiguredEndpoint.EndpointUrl.IdnHost — the CONFIGURED host, because OpcUaEndpointSelection/EndpointSelector rewrote the advertised one — against the cert's DNS/IP SANs; secured sessions only, and the auto-accept handler accepts a mismatch. Policy: certificateValidation.validateServerHostName, tri-state. Unset = OpcUaCertificateValidationOptions.DefaultValidateServerHostName = false, the pre-#816 behaviour, so nothing changes on upgrade; true = validate the configured host; false = explicit NAT/port-publish opt-out. Threaded into the driver, the AdminUI browser (same JSON) and the AdminUI form (a select that keeps "unset" unset), Client.Shared ConnectionSettings.ValidateServerHostName and the CLI --validate-host-name. A driver refusal names the opt-out. #818: the NAT fixture's /app/pki is on the opc-plc-nat-pki volume (verified: same thumbprint across --force-recreate), its PUBLIC cert is committed as Docker/nat/opc-plc-nat-server.der, and NatPublishedEndpointBrowseTests run every secure browse on a throwaway PKI (internal browser seam): refusal with nothing trusted, positive with the cert trusted (unset/false host check), and a BadCertificateHostNameInvalid refusal with the check on. The trusted cert is OPCUA_NAT_SERVER_CERT or the committed one (skip on thumbprint mismatch); OPCUA_NAT_SERVER_CERT_TRUSTED is retired. CI exports its throwaway NAT container's public cert. ServerHostNameValidationTests pin the driver against the plain opc-plc. MIN_EXECUTED not raised (needs a hosted run): +6 in v2-integration (440 -> 446), documented in scripts/ci/README.md. --- .claude/rules/drivers.md | 7 + .github/workflows/v2-integration.yml | 2 + docs/Client.CLI.md | 1 + docs/drivers/OpcUaClient.md | 77 +++++++- .../2026-05-28-driver-browsers-design.md | 5 +- infra/README.md | 9 +- scripts/ci/README.md | 14 +- scripts/ci/integration-fixtures.sh | 9 + .../CommandBase.cs | 10 +- .../Adapters/DefaultSessionFactory.cs | 6 +- .../Adapters/ISessionFactory.cs | 4 + .../Models/ConnectionSettings.cs | 9 + .../OpcUaClientService.cs | 3 +- .../OpcUaClientDriverBrowser.cs | 24 ++- ....OtOpcUa.Driver.OpcUaClient.Browser.csproj | 1 + .../OpcUaClientDriverOptions.cs | 29 +++ .../OpcUaClientDriver.cs | 29 ++- ...B.MOM.WW.OtOpcUa.Driver.OpcUaClient.csproj | 2 + .../Drivers/Forms/OpcUaClientDriverForm.razor | 23 +++ .../CommandBaseTests.cs | 16 ++ .../Fakes/FakeSessionFactory.cs | 6 +- .../OpcUaClientServiceTests.cs | 19 ++ .../NatPublishedEndpointBrowseTests.cs | 176 +++++++++++++++--- ...pcUaClient.Browser.IntegrationTests.csproj | 7 + .../Docker/README.md | 6 +- .../Docker/nat/docker-compose.yml | 23 ++- .../Docker/nat/opc-plc-nat-server.der | Bin 0 -> 1043 bytes .../ServerHostNameValidationTests.cs | 108 +++++++++++ .../OpcUaClientServerHostNameOptionTests.cs | 57 ++++++ ...aClientDriverPageFormSerializationTests.cs | 25 +++ 30 files changed, 648 insertions(+), 59 deletions(-) create mode 100644 tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.IntegrationTests/Docker/nat/opc-plc-nat-server.der create mode 100644 tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.IntegrationTests/ServerHostNameValidationTests.cs create mode 100644 tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests/OpcUaClientServerHostNameOptionTests.cs diff --git a/.claude/rules/drivers.md b/.claude/rules/drivers.md index 1c2dfd3e1..ecdbfcca6 100644 --- a/.claude/rules/drivers.md +++ b/.claude/rules/drivers.md @@ -288,6 +288,13 @@ host logger factory (wired in `DriverFactoryBootstrap`; the FOCAS and TwinCAT fa AFTER configured ones; more than one discovered server without `discoveryServerUri` fails the connect. Session events are the OTel counter `otopcua.driver.opcuaclient.events`. `HistoryAggregateType` carries the full Part 13 set; an aggregate the gateway can't serve is `BadAggregateNotSupported` per node. +- **OpcUaClient server host-name check (#816):** the SDK checks the server certificate's host only with + `checkDomain: true`; its 8-arg `DefaultSessionFactory.CreateAsync` hard-codes `false`, so driver, AdminUI browser and + Client.Shared never checked before #816. Now `certificateValidation.validateServerHostName` (tri-state, unset = off = + unchanged; AdminUI keeps it unset) / CLI `--validate-host-name`. It checks the **configured** host (the rewritten + `ConfiguredEndpoint.EndpointUrl`), secured sessions only, and auto-accept accepts a mismatch. Any new session-create + call site must pass the flag — the overload without it silently disables the check. The rewriter keeps an IPv6 + zone index verbatim (#819; .NET `Uri` drops it from every re-serialisation). `docs/drivers/OpcUaClient.md`. - **MTConnect (#627/#628):** TIME_SERIES reads as a variable-length `double[]` (Float64 only); a count disagreeing with `sampleCount` is `BadDecodingError`. One MTConnect driver = one agent client = at most one `/raw` Device (`MTConnectMultipleDevices` deploy error; `RawTreeService.CreateDeviceAsync` refuses a second). diff --git a/.github/workflows/v2-integration.yml b/.github/workflows/v2-integration.yml index 48d4b1bd8..aa0af29eb 100644 --- a/.github/workflows/v2-integration.yml +++ b/.github/workflows/v2-integration.yml @@ -74,6 +74,8 @@ jobs: - name: assert not-all-skipped if: always() env: + # #816/#818 added 6 executing OpcUaClient tests; raise to 446 once a hosted run confirms them + # (scripts/ci/README.md §"Fixture-backed integration"). MIN_EXECUTED: "440" run: bash scripts/ci/assert-not-all-skipped.sh artifacts/trx/*.trx - name: fixture logs on failure diff --git a/docs/Client.CLI.md b/docs/Client.CLI.md index e603776ff..6cb9eaeec 100644 --- a/docs/Client.CLI.md +++ b/docs/Client.CLI.md @@ -39,6 +39,7 @@ All commands accept these options: | `-P` / `--password` | Password for `UserName` token authentication | | `-S` / `--security` | Transport security mode: `none`, `sign`, `encrypt`, `signandencrypt` (default: `none`) | | `-F` / `--failover-urls` | Comma-separated failover endpoint URLs for redundancy | +| `--validate-host-name` | Require the server certificate to list the `--url` host (#816). Secured connections only; off by default (the CLI's long-standing behaviour). The CLI auto-accepts untrusted certificates, so this checks a misconfiguration, not trust. Leave off behind NAT / when connecting by an IP the certificate omits | | `--verbose` | Enable debug-level Serilog console logging (default: warning) | **`--security none` does not fall back.** Endpoint selection picks the server's `None`-policy endpoint and diff --git a/docs/drivers/OpcUaClient.md b/docs/drivers/OpcUaClient.md index 8b4ad31b5..9ed288728 100644 --- a/docs/drivers/OpcUaClient.md +++ b/docs/drivers/OpcUaClient.md @@ -108,7 +108,14 @@ The rule, for `opc.tcp` (and any scheme, as long as advertised and configured sc a real rewrite keeps the advertised path's percent-escaping (`AbsoluteUri`, #812); - a **different scheme** (an `https` endpoint next to an `opc.tcp` one) is a different transport and is left alone; - an unusable advertised URL (null / blank / unparseable) falls back to the configured one in the driver and the - browser; the Client CLI/UI instead reports it (it validates before calling the rewriter). + browser; the Client CLI/UI instead reports it (it validates before calling the rewriter); +- an **IPv6 zone index** on the configured host (`opc.tcp://[fe80::1%eth0]:4840`) is **kept, verbatim** (#819). + .NET's `Uri` parses the zone but drops it from `Host`/`Authority`/`AbsoluteUri`/`ToString()` — only + `DnsSafeHost`/`IdnHost` carry it — so the rewriter splices it back into the string. The SDK dials + `new Uri(EndpointUrl).IdnHost` (`TcpByteTransport` → `DnsEndPoint`), which keeps it, so a link-local endpoint now + routes; before #819 the session dialled a zone-less `fe80::` address. The zone text is not normalised: write the + form your OS resolves (`%eth0` / `%4`). .NET does **not** decode RFC 6874's `%25eth0` (its `DnsSafeHost` is + `fe80::1%25eth0`), so that form is passed through as written — GetEndpoints and the session see the same string. **What the rewrite does NOT touch.** It only chooses the network address. The **security policy and mode** are still selected from the server's advertisement exactly as before — the driver and browser filter on the configured @@ -118,7 +125,51 @@ user-token policies** are the advertised ones. **Certificate validation is uncha validates the certificate of the server actually reached, against the same trust store (the driver's PKI, the browse picker's separate `adminui-browse-pki`, the CLI's own) with the same `AutoAcceptCertificates` semantics. The NAT tests below pin that a secure browse through the NAT is refused on an untrusted certificate and succeeds once it -is trusted. +is trusted. The optional **host-name** check (#816, next section) is the one place the rewrite matters to validation: +it runs against the rewritten URL, i.e. the **configured** host. + +### Server certificate host name (#816) + +**Finding (SDK 2.0.1).** The SDK only checks that the server certificate names the host you connected to when the +session is created with `checkDomain: true`. All three surfaces — the driver (`OpenSessionOnEndpointAsync`), the +AdminUI browser (`OpcUaClientDriverBrowser.OpenAsync`) and Client.Shared (`DefaultSessionFactory`) — called the +8-argument `DefaultSessionFactory.CreateAsync(config, endpoint, updateBeforeConnect, sessionName, …)`, which +forwards `checkDomain: false`. So **no host-name check ever ran**; trust was the trusted-peer / issuer store alone. +That is why, in #813, a trusted NAT certificate for `otopcua-opc-plc-nat-private.invalid` browsed fine over an IP. + +Where the check lives when enabled: `Session.OpenAsync` → after `CertificateManager.ValidateAsync` accepts the chain, +`if (checkDomain) CertificateValidationExtensions.ValidateDomains(validator, serverCert, session.ConfiguredEndpoint)` +→ `CertificateValidationCore.ValidateDomains` → `CertificateValidationHelpers.FindDomain`, which compares +`ConfiguredEndpoint.EndpointUrl.IdnHost` with the certificate's DNS / IP SANs (IP literals normalised; a `localhost` +/ `127.0.0.1` / `::1` URL is compared as the machine's own host name + FQDN). A miss is `BadCertificateHostNameInvalid` +("The domain '…' is not listed in the server certificate."), passed to the certificate manager's `AcceptError` hook +first. Consequences: + +- **It checks the CONFIGURED host.** `ConfiguredEndpoint.EndpointUrl` is the URL `OpcUaEndpointSelection` / + `EndpointSelector` rewrote, so the advertised host the server named itself never takes part. +- **Secured sessions only** — the block runs only when the channel (or the user-token policy) needs the server + certificate; a `None` session validates no certificate at all. +- **`autoAcceptCertificates` accepts a mismatch too** (the driver's auto-accept `AcceptError` handler returns `true` + for everything but revocation). The Client CLI always auto-accepts. +- Test Connect (`OpcUaClientDriverProbe`) only calls GetEndpoints on an unsecured discovery channel — no session, so no + certificate or host-name check. + +**Policy: `certificateValidation.validateServerHostName`** (driver JSON; AdminUI driver form "Server host-name check"; +the browser reads the same JSON): + +| Value | Meaning | +|---|---| +| absent / `null` (**default**) | `OpcUaCertificateValidationOptions.DefaultValidateServerHostName` = **off** — exactly the pre-#816 behaviour, so no existing deployment changes on upgrade. The AdminUI form keeps it absent ("Default (off)") rather than writing a concrete `false`. | +| `true` | Validate: the certificate must list the configured endpoint host. The driver's connect failure then appends the remedy (fix the certificate, connect by a name it lists, or opt out). | +| `false` | Explicit opt-out — the NAT / container port-publish / connect-by-an-IP-the-cert-omits case. | + +When to turn it on: it earns its keep when trust comes from a **CA** (a trusted issuer vouches for every certificate +it signed, so only the host check ties the certificate to *this* server). With the server's own self-signed +certificate pinned in the trusted-peer store it adds little — the pin already identifies the server. Client +CLI: `--validate-host-name` (`ConnectionSettings.ValidateServerHostName`, default `false`). Pinned by +`ServerHostNameValidationTests` (driver vs. the plain opc-plc, whose certificate names only its container id) and +`NatPublishedEndpointBrowseTests` (browser vs. the NAT opc-plc): unset/`false` connect, `true` is refused naming the +configured host. **Running the NAT tests.** `NatPublishedEndpointBrowseTests` (`tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Browser.IntegrationTests`, `Fixture=opc-plc-nat`) run against @@ -133,13 +184,20 @@ OPCUA_NAT_ENDPOINT=opc.tcp://10.100.0.35:50010 dotnet test \ tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Browser.IntegrationTests --filter Fixture=opc-plc-nat ``` -The secure case asserts a certificate **refusal** by default. To make it positive, trust the fixture's server -certificate in the browse PKI and set `OPCUA_NAT_SERVER_CERT_TRUSTED=1` — the certificate is regenerated whenever -the container is recreated, so fetch the current one: -`ssh dohertj2@10.100.0.35 'docker exec otopcua-opc-plc-nat sh -c "cat /app/pki/own/certs/*.der"' > "/OtOpcUa/adminui-browse-pki/trusted/certs/opc-plc-nat.der"` -(`~/Library/Application Support/...` on macOS, `~/.local/share/...` on Linux), and delete it again afterwards. CI -(`v2-integration.yml`) starts its own throwaway NAT copy (`scripts/ci/integration-fixtures.sh`); `v2-e2e.yml` points at -the shared one. Neither trusts the certificate, so CI runs the refusal form. +Every secure browse there runs on a **throwaway browse PKI** (an internal PKI-root seam on the browser), never your +`adminui-browse-pki`. The refusal case trusts nothing and always runs. The positive case (unset / `false` host check) +and the host-name refusal (`true`) trust the fixture's **public** certificate (#818), from: + +1. `OPCUA_NAT_SERVER_CERT` — a DER or PEM file; must be the certificate the server presents, or the test **fails**; else +2. the committed `tests/.../Driver.OpcUaClient.IntegrationTests/Docker/nat/opc-plc-nat-server.der` — the shared-host + stack's certificate, stable because its PKI lives on the `opc-plc-nat-pki` volume. If it is not the certificate + the server presents (another NAT fixture, or the stack's certificate was rolled) these tests **skip**. + +The private key never leaves the volume. The committed certificate expires **2027-11-10**; to roll it, see the comment +at the top of `Docker/nat/docker-compose.yml`. CI (`v2-integration.yml`) starts its own throwaway NAT copy +(`scripts/ci/integration-fixtures.sh`), copies that container's public certificate out with `docker exec` and exports +`OPCUA_NAT_SERVER_CERT`, so it runs the positive cases too; `v2-e2e.yml` points at the shared stack and uses the +committed certificate. ### Session lifetime @@ -379,6 +437,7 @@ also refuses them explicitly — defence in depth (#277), not a behaviour change |---|---|---| | `rejectUnknownRevocationStatus` | `false` | Reject a server certificate whose revocation status cannot be determined (no CRL for its issuer, or a stale one). Leave `false` for brownfield sites without CRL infrastructure. **Only effective with `autoAcceptCertificates` off** — the auto-accept handler accepts every suppressible error, including revocation-unknown. | | `issuerStorePath` | `null` (= `%LocalAppData%/OtOpcUa/pki/issuers`) | Directory used as the trusted-issuer store. The SDK reads issuer CRLs (`*.crl`) from its `crl/` sub-folder, so this is how an operator points the driver at a managed CRL set. | +| `validateServerHostName` | `null` (= off) | #816: require the server certificate to name the **configured** endpoint host (`BadCertificateHostNameInvalid` otherwise). Secured sessions only; accepted anyway under `autoAcceptCertificates`. `false` = explicit NAT opt-out. See "Server certificate host name" above. | ### Subscription tuning (#273) diff --git a/docs/plans/2026-05-28-driver-browsers-design.md b/docs/plans/2026-05-28-driver-browsers-design.md index 39f3a3d4e..41c34ed75 100644 --- a/docs/plans/2026-05-28-driver-browsers-design.md +++ b/docs/plans/2026-05-28-driver-browsers-design.md @@ -280,8 +280,9 @@ Razor picker body BrowserSessionService IDriverBrowser (`ssh dohertj2@10.100.0.35 'cd /opt/otopcua-opcuaclient-nat && docker compose up -d'`, compose in `tests/.../Driver.OpcUaClient.IntegrationTests/Docker/nat/`), and CI starts a throwaway copy. Run: `OPCUA_NAT_ENDPOINT=opc.tcp://10.100.0.35:50010 dotnet test tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Browser.IntegrationTests --filter Fixture=opc-plc-nat`. - The secure case asserts a certificate refusal unless the fixture cert is trusted in the browse PKI and - `OPCUA_NAT_SERVER_CERT_TRUSTED=1` (see `docs/drivers/OpcUaClient.md`). + The secure cases run on a throwaway browse PKI: a refusal with nothing trusted, and (#818) positive / #816 + host-name cases that trust the fixture's public certificate (`OPCUA_NAT_SERVER_CERT`, or the committed + `Docker/nat/opc-plc-nat-server.der`) — see `docs/drivers/OpcUaClient.md`. (`OPCUA_NAT_SERVER_CERT_TRUSTED` is retired.) - No Galaxy integration suite in v1 (requires wonder-app-vd03; deferred) ### Specific regression tests diff --git a/infra/README.md b/infra/README.md index d04ea3993..8ee322e17 100644 --- a/infra/README.md +++ b/infra/README.md @@ -100,9 +100,12 @@ when it isn't running. Bring one up, `dotnet test`, tear down. Repo compose live > `NatPublishedEndpointBrowseTests` (Browser.IntegrationTests, `Fixture=opc-plc-nat`) use it, and each asserts that > premise before testing the shared `OpcUaEndpointUrlRewriter` (configured host:port win, advertised path kept): > `OPCUA_NAT_ENDPOINT=opc.tcp://10.100.0.35:50010 dotnet test tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Browser.IntegrationTests --filter Fixture=opc-plc-nat`. -> A separate stack so it never touches `otopcua-opc-plc`; it carries the `project: lmxopcua` label. The secure case -> turns positive with `OPCUA_NAT_SERVER_CERT_TRUSTED=1` once the container's cert is in the browse PKI -> (`docs/drivers/OpcUaClient.md` §"Endpoint address"). CI runs its own throwaway copy (`otopcua-ci-opcplc-nat`, §10). +> A separate stack so it never touches `otopcua-opc-plc`; it carries the `project: lmxopcua` label. Its PKI is on the +> `opc-plc-nat-pki` volume (#818), so its certificate survives a recreate; the PUBLIC half is committed as +> `Docker/nat/opc-plc-nat-server.der` and the positive secure / #816 host-name cases trust it in a throwaway browse PKI +> (they skip if the server presents a different one; `OPCUA_NAT_SERVER_CERT=` overrides). `docker compose down -v` +> rolls the certificate — refresh the committed copy then (`docs/drivers/OpcUaClient.md` §"Endpoint address"). CI runs +> its own throwaway copy (`otopcua-ci-opcplc-nat`, §10) and exports its certificate as `OPCUA_NAT_SERVER_CERT`. > **MTConnect is a TWO-service stack.** The `mtconnect/agent` image ships no simulator, so a lone > Agent answers `/probe` and reports every observation `UNAVAILABLE` forever. The `adapter` service diff --git a/scripts/ci/README.md b/scripts/ci/README.md index 4cd24ccc4..a1393437b 100644 --- a/scripts/ci/README.md +++ b/scripts/ci/README.md @@ -75,9 +75,19 @@ Neither touches `v2-ci.yml`; each has its own concurrency group (`lmxopcua-v2-in Local run: 468 executed / 8 skipped (Oracle x2, the blackhole `docker pause` gate, one QuestDB stress test, 4 others), about 4.5 min of test time, QuestDB the longest (~3.3 min). Not covered (no pullable image or needs credentials): Modbus/S7/AB/FOCAS/TwinCAT sims, Mqtt, MTConnect, Oracle, Host real-infra mode. + **#816/#818 added 6 executing tests here, floor NOT yet raised:** `ServerHostNameValidationTests` (3, vs the plain + `otopcua-ci-opcplc`) and the NAT positive-secure / host-name cases (3, which run because `integration-fixtures.sh` + now exports the NAT container's public certificate as `OPCUA_NAT_SERVER_CERT`). Locally (2026-11-11, shared host) + the two OpcUaClient projects went 13 → 19 executed with 0 skipped, and `assert-not-all-skipped.sh` counted them. + Once a hosted run's TRX shows `Driver.OpcUaClient.trx` executed=8 and `Driver.OpcUaClient.Browser.trx` executed=11, + raise `MIN_EXECUTED` in `v2-integration.yml` **440 → 446**. If the NAT three show as skipped there, the certificate + hand-off (`docker exec … /app/pki/own/certs/*.der`) did not work on the runner — fix that, do not lower the bump. - **`v2-e2e.yml`** (manual + weekly cron only; `timeout-minutes: 90`). `probe-e2e-fixtures.sh` TCP-probes 10.100.0.35 ports 5020/1102/50000; if any is closed every later step is skipped and the run is green with a notice. Otherwise runs the Modbus, S7, OpcUaClient and OpcUaClient.Browser integration projects against the - shared fleet with `MIN_EXECUTED=35` (`OPCUA_NAT_ENDPOINT` points at the shared NAT stack on :50010; its 3 tests - skip if it is down, so it is not a probe-required port) (local: 42 executed / 38 skipped; Modbus skips the profiles that are not + shared fleet with `MIN_EXECUTED=35` (`OPCUA_NAT_ENDPOINT` points at the shared NAT stack on :50010; its 6 tests + skip if it is down, so it is not a probe-required port — and its 3 certificate-trusting ones also skip if the stack's + certificate no longer matches the committed `Docker/nat/opc-plc-nat-server.der`, so do not count them in the floor; + #816's 3 `ServerHostNameValidationTests` need only the probe-required :50000, so this floor may go **35 → 38** after + a hosted run shows them executed) (local: 42 executed / 38 skipped; Modbus skips the profiles that are not up). No test carries `Category=E2E` today, so the zero-match guard is this executed floor. diff --git a/scripts/ci/integration-fixtures.sh b/scripts/ci/integration-fixtures.sh index 05a203603..8da099e3b 100755 --- a/scripts/ci/integration-fixtures.sh +++ b/scripts/ci/integration-fixtures.sh @@ -80,6 +80,14 @@ up() { wait_for "questdb" 120 curl -fsS "http://$(fixture_host):$QDB_HTTP_PORT/exec?query=select%201" wait_for "opc-plc" 120 bash -c "exec 3<>/dev/tcp/$(fixture_host)/$OPC_PORT" wait_for "opc-plc (NAT)" 120 bash -c "exec 3<>/dev/tcp/$(fixture_host)/$OPC_NAT_PORT" + # #818: hand the NAT server's PUBLIC certificate to NatPublishedEndpointBrowseTests (OPCUA_NAT_SERVER_CERT) so the + # positive secure / host-name cases run here too. Copied out of the container — the trust channel is docker itself, + # not the wire. The throwaway container makes a fresh certificate per run, so the committed shared-host copy cannot + # be used; the private key never leaves the container. + local nat_cert="${RUNNER_TEMP:-${TMPDIR:-/tmp}}/otopcua-ci-fixtures/opc-plc-nat-server.der" + mkdir -p "$(dirname "$nat_cert")" + wait_for "opc-plc (NAT) certificate" 60 bash -c \ + "docker exec otopcua-ci-opcplc-nat bash -c 'cat /app/pki/own/certs/*.der' > '$nat_cert' && [ -s '$nat_cert' ]" local h; h="$(fixture_host)" local -a exports=( @@ -92,6 +100,7 @@ up() { "HISTORIAN_QUESTDB_TEST_CONNECTION=Host=$h;Port=$QDB_PORT;Username=admin;Password=quest;Database=qdb;ServerCompatibilityMode=NoTypeLoading" "OPCUA_SIM_ENDPOINT=opc.tcp://$h:$OPC_PORT" "OPCUA_NAT_ENDPOINT=opc.tcp://$h:$OPC_NAT_PORT" + "OPCUA_NAT_SERVER_CERT=$nat_cert" ) for e in "${exports[@]}"; do echo "export \"$e\"" diff --git a/src/Client/ZB.MOM.WW.OtOpcUa.Client.CLI/CommandBase.cs b/src/Client/ZB.MOM.WW.OtOpcUa.Client.CLI/CommandBase.cs index 86bddc6e9..ed95be7c6 100644 --- a/src/Client/ZB.MOM.WW.OtOpcUa.Client.CLI/CommandBase.cs +++ b/src/Client/ZB.MOM.WW.OtOpcUa.Client.CLI/CommandBase.cs @@ -57,6 +57,13 @@ public abstract class CommandBase : ICommand [CommandOption("failover-urls", 'F', Description = "Comma-separated failover endpoint URLs for redundancy")] public string? FailoverUrls { get; init; } + /// + /// Gets a value indicating whether the server certificate must name the host given in (#816). + /// + [CommandOption("validate-host-name", + Description = "Require the server certificate to list the --url host (secured connections only; off by default — leave off behind NAT)")] + public bool ValidateHostName { get; init; } + /// /// Gets a value indicating whether verbose Serilog output should be enabled for troubleshooting. /// @@ -88,7 +95,8 @@ public abstract class CommandBase : ICommand Username = Username, Password = Password, SecurityMode = securityMode, - AutoAcceptCertificates = true + AutoAcceptCertificates = true, + ValidateServerHostName = ValidateHostName }; return settings; diff --git a/src/Client/ZB.MOM.WW.OtOpcUa.Client.Shared/Adapters/DefaultSessionFactory.cs b/src/Client/ZB.MOM.WW.OtOpcUa.Client.Shared/Adapters/DefaultSessionFactory.cs index 0619ec1e0..367f2680a 100644 --- a/src/Client/ZB.MOM.WW.OtOpcUa.Client.Shared/Adapters/DefaultSessionFactory.cs +++ b/src/Client/ZB.MOM.WW.OtOpcUa.Client.Shared/Adapters/DefaultSessionFactory.cs @@ -18,6 +18,7 @@ internal sealed class DefaultSessionFactory : ISessionFactory string sessionName, uint sessionTimeoutMs, UserIdentity identity, + bool validateServerHostName, CancellationToken ct) { var endpointConfig = EndpointConfiguration.Create(config); @@ -26,7 +27,10 @@ internal sealed class DefaultSessionFactory : ISessionFactory var created = await new Opc.Ua.Client.DefaultSessionFactory(ClientTelemetry.Context).CreateAsync( config, configuredEndpoint, - false, + false, // updateBeforeConnect + // #816: the overload without checkDomain hard-codes false. The endpoint URL here is the operator's host + // (EndpointSelector rewrote the advertised one), so that is the host the certificate is checked against. + validateServerHostName, // checkDomain sessionName, sessionTimeoutMs, identity, diff --git a/src/Client/ZB.MOM.WW.OtOpcUa.Client.Shared/Adapters/ISessionFactory.cs b/src/Client/ZB.MOM.WW.OtOpcUa.Client.Shared/Adapters/ISessionFactory.cs index 0593be846..e3149db8f 100644 --- a/src/Client/ZB.MOM.WW.OtOpcUa.Client.Shared/Adapters/ISessionFactory.cs +++ b/src/Client/ZB.MOM.WW.OtOpcUa.Client.Shared/Adapters/ISessionFactory.cs @@ -15,6 +15,9 @@ internal interface ISessionFactory /// The session name. /// Session timeout in milliseconds. /// The user identity. + /// + /// The SDK's checkDomain: require the server certificate to name the endpoint URL's host (#816). + /// /// Cancellation token. /// A session adapter wrapping the created session. Task CreateSessionAsync( @@ -23,5 +26,6 @@ internal interface ISessionFactory string sessionName, uint sessionTimeoutMs, UserIdentity identity, + bool validateServerHostName, CancellationToken ct = default); } \ No newline at end of file diff --git a/src/Client/ZB.MOM.WW.OtOpcUa.Client.Shared/Models/ConnectionSettings.cs b/src/Client/ZB.MOM.WW.OtOpcUa.Client.Shared/Models/ConnectionSettings.cs index d9cf64882..952bda84b 100644 --- a/src/Client/ZB.MOM.WW.OtOpcUa.Client.Shared/Models/ConnectionSettings.cs +++ b/src/Client/ZB.MOM.WW.OtOpcUa.Client.Shared/Models/ConnectionSettings.cs @@ -40,6 +40,15 @@ public sealed class ConnectionSettings /// public bool AutoAcceptCertificates { get; set; } = true; + /// + /// Gitea #816 — require the server certificate to list the host of the URL being connected (the operator's + /// host: discovery keeps it in place of the server-advertised one) among its subject-alternative names; + /// a mismatch fails the connect with BadCertificateHostNameInvalid. Defaults to false, the + /// behaviour before #816 (the SDK overload used never checked). Secured connections only. Leave it off behind + /// NAT / a port publish / when connecting by an IP the certificate does not name. + /// + public bool ValidateServerHostName { get; set; } + /// /// Path to the certificate store. Defaults to ; the /// consuming application configuration factory resolves the canonical path via diff --git a/src/Client/ZB.MOM.WW.OtOpcUa.Client.Shared/OpcUaClientService.cs b/src/Client/ZB.MOM.WW.OtOpcUa.Client.Shared/OpcUaClientService.cs index 513fb30fe..012a3c261 100644 --- a/src/Client/ZB.MOM.WW.OtOpcUa.Client.Shared/OpcUaClientService.cs +++ b/src/Client/ZB.MOM.WW.OtOpcUa.Client.Shared/OpcUaClientService.cs @@ -777,6 +777,7 @@ public sealed class OpcUaClientService : IOpcUaClientService SecurityMode = settings.SecurityMode, SessionTimeoutSeconds = settings.SessionTimeoutSeconds, AutoAcceptCertificates = settings.AutoAcceptCertificates, + ValidateServerHostName = settings.ValidateServerHostName, CertificateStorePath = settings.CertificateStorePath, Username = settings.Username, Password = settings.Password @@ -794,7 +795,7 @@ public sealed class OpcUaClientService : IOpcUaClientService var sessionTimeoutMs = (uint)(settings.SessionTimeoutSeconds * 1000); return await _sessionFactory.CreateSessionAsync(config, endpoint, "OtOpcUaClient", sessionTimeoutMs, identity, - ct); + settings.ValidateServerHostName, ct); } catch { 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 a815a79ae..f80b054bf 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 @@ -35,6 +35,7 @@ public sealed class OpcUaClientDriverBrowser : IDriverBrowser private readonly ILogger _logger; private readonly ISecretResolver? _secretResolver; + private readonly string? _pkiRootOverride; /// Creates a new browser. Logger defaults to NullLogger when not supplied. /// Optional logger; defaults to . @@ -49,6 +50,18 @@ public sealed class OpcUaClientDriverBrowser : IDriverBrowser _secretResolver = secretResolver; } + /// + /// Test seam (Gitea #818): a browser whose PKI lives under instead of the shared + /// %LocalAppData%/OtOpcUa/adminui-browse-pki, so an integration test can trust a fixture's public + /// certificate in a throwaway store without touching the operator's browse trust list. + /// + /// Directory holding the own/trusted/issuers/rejected stores. + internal OpcUaClientDriverBrowser(string pkiRoot) : this() + { + ArgumentException.ThrowIfNullOrWhiteSpace(pkiRoot); + _pkiRootOverride = pkiRoot; + } + /// // Sourced from the shared DriverTypeNames set rather than the hand-written literal // this line used to carry. The value is unchanged; the constant is what @@ -82,7 +95,7 @@ public sealed class OpcUaClientDriverBrowser : IDriverBrowser "driver's PKI store instead.", endpoint); - var app = await BuildBrowseAppConfigurationAsync(cancellationToken).ConfigureAwait(false); + var app = await BuildBrowseAppConfigurationAsync(_pkiRootOverride, cancellationToken).ConfigureAwait(false); ownedApplication = app; var appConfig = app.ApplicationConfiguration!; @@ -101,6 +114,9 @@ public sealed class OpcUaClientDriverBrowser : IDriverBrowser appConfig, configuredEndpoint, updateBeforeConnect: false, + // Gitea #816: the same host-name policy as the deployed driver (the SDK's overload without this + // argument hard-codes false), checked against the configured host the endpoint was rewritten to. + checkDomain: opts.CertificateValidation.EffectiveValidateServerHostName, sessionName: "OtOpcUa AdminUI Browse", (uint)opts.SessionTimeout.TotalMilliseconds, identity, @@ -143,8 +159,10 @@ public sealed class OpcUaClientDriverBrowser : IDriverBrowser /// the runtime driver (%LocalAppData%/OtOpcUa/adminui-browse-pki/). Browse /// trust decisions don't leak into the deployed driver's cert store. /// + /// Test-only PKI root; null = the shared browse PKI. /// Cancellation token for the configuration build. - private static async Task BuildBrowseAppConfigurationAsync(CancellationToken ct) + private static async Task BuildBrowseAppConfigurationAsync( + string? pkiRootOverride, CancellationToken ct) { // The ITelemetryContext overload is the non-obsolete ctor; an empty builder keeps SDK // internals off any logging provider. @@ -154,7 +172,7 @@ public sealed class OpcUaClientDriverBrowser : IDriverBrowser ApplicationType = ApplicationType.Client, }; - var pkiRoot = Path.Combine( + var pkiRoot = pkiRootOverride ?? Path.Combine( Environment.GetFolderPath(Environment.SpecialFolder.LocalApplicationData), "OtOpcUa", "adminui-browse-pki"); diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Browser/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Browser.csproj b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Browser/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Browser.csproj index 94fd7e3bc..6e73ee159 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Browser/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Browser.csproj +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Browser/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Browser.csproj @@ -19,5 +19,6 @@ + diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Contracts/OpcUaClientDriverOptions.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Contracts/OpcUaClientDriverOptions.cs index e08683626..ea9824cc0 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Contracts/OpcUaClientDriverOptions.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Contracts/OpcUaClientDriverOptions.cs @@ -231,6 +231,35 @@ public sealed record OpcUaCertificateValidationOptions /// operator-managed CRL set. null/empty (default) = %LocalAppData%/OtOpcUa/pki/issuers. /// public string? IssuerStorePath { get; init; } + + /// + /// The value an unset resolves to. false keeps the behaviour every + /// deployment had before Gitea #816 (the SDK never checked the host name for this driver family). + /// + public const bool DefaultValidateServerHostName = false; + + /// + /// Gitea #816 — whether the server certificate must list the host the operator CONFIGURED (the endpoint URL's + /// host, which the shared keeps in place of the server-advertised one) + /// among its DNS / IP subject-alternative names. A mismatch refuses the session with + /// BadCertificateHostNameInvalid. + /// + /// null (default) = (off). + /// true = validate. Worth it when trust comes from a CA (a trusted issuer vouches for every + /// certificate it signed, so only the host check ties the certificate to THIS server); adds little when the + /// server's own self-signed certificate is pinned in the trusted-peer store. + /// false = explicit opt-out — the NAT / port-publish / IP-addressed case, where the server's + /// certificate names its private host and the reachable address is something else. + /// + /// Only applies to secured sessions (a None channel validates no server certificate at all), and + /// accepts a host-name mismatch like any other + /// certificate error. + /// + public bool? ValidateServerHostName { get; init; } + + /// with the default applied. + [System.Text.Json.Serialization.JsonIgnore] + public bool EffectiveValidateServerHostName => ValidateServerHostName ?? DefaultValidateServerHostName; } /// OPC UA message security mode. 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 4d1820985..c39baf985 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDriver.cs @@ -581,7 +581,7 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested) { throw; } catch (Exception ex) { - attemptErrors.Add($"{url} -> {ex.GetType().Name}: {ex.Message}"); + attemptErrors.Add($"{url} -> {ex.GetType().Name}: {ex.Message}{HostNameRefusalHint(ex)}"); } } @@ -726,6 +726,14 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit private void WriteLifecycleHealth(long generation, Func next) => UpdateHealth(generation, next); + /// + /// Test seam (Gitea #816): when set, the PKI stores live under this directory instead of the shared + /// %LocalAppData%/OtOpcUa/pki, so an integration test can trust a fixture's certificate in a throwaway + /// store with off (auto-accept also accepts a + /// host-name mismatch, which would hide the check under test). Set before InitializeAsync. + /// + internal string? PkiRootOverride { get; set; } + /// /// Build a minimal in-memory . Certificates live /// under the OS user profile — on Windows that's %LocalAppData%\OtOpcUa\pki @@ -747,7 +755,7 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit }; certValidation.Application = app; - var pkiRoot = Path.Combine( + var pkiRoot = PkiRootOverride ?? Path.Combine( Environment.GetFolderPath(Environment.SpecialFolder.LocalApplicationData), "OtOpcUa", "pki"); @@ -1051,10 +1059,14 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit endpointConfig.OperationTimeout = (int)_options.Timeout.TotalMilliseconds; var endpoint = new ConfiguredEndpoint(null, selected, endpointConfig); + // Gitea #816: the SDK's 8-argument CreateAsync hard-codes checkDomain:false, so before #816 the server + // certificate's host name was never checked. The check (when enabled) runs against endpoint.EndpointUrl — + // the CONFIGURED host, because SelectMatching rewrote the advertised one. var session = await new DefaultSessionFactory(_sdkTelemetry).CreateAsync( appConfig, endpoint, false, // updateBeforeConnect + _options.CertificateValidation.EffectiveValidateServerHostName, // checkDomain (#816) _options.SessionName, (uint)_options.SessionTimeout.TotalMilliseconds, identity, @@ -1085,6 +1097,19 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit return OpcUaEndpointSelection.SelectMatching(all.ToList(), endpointUrl, policy, mode); } + /// + /// Gitea #816 — the operator-facing remedy appended to a BadCertificateHostNameInvalid connect failure (only + /// reachable with certificateValidation.validateServerHostName on). The SDK's own text names the host it + /// checked but not the knob. + /// + /// The per-endpoint connect failure. + /// The hint, or an empty string for any other failure. + internal static string HostNameRefusalHint(Exception ex) => + ex is ServiceResultException sre && sre.StatusCode == StatusCodes.BadCertificateHostNameInvalid + ? " (the server certificate does not name the configured endpoint host; add that host to the certificate, " + + "connect by a name it lists, or — behind NAT / a port publish — set certificateValidation.validateServerHostName=false)" + : string.Empty; + /// 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). diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.csproj b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.csproj index a93a9d070..1b656d9dd 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.csproj +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.csproj @@ -28,6 +28,8 @@ + + diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/OpcUaClientDriverForm.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/OpcUaClientDriverForm.razor index 2b0f68094..d080f2dc9 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/OpcUaClientDriverForm.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/OpcUaClientDriverForm.razor @@ -192,6 +192,15 @@
Revoked certificates are always rejected.
+
+ + + + + + +
Secured sessions only: the server certificate must name the endpoint host you configured.
+
@@ -349,6 +358,8 @@ public int SubscriptionPriority { get; set; } public bool RejectUnknownRevocationStatus { get; set; } public string? IssuerStorePath { get; set; } + // #816 tri-state: "" = unset (the driver default) so the form never re-emits a concrete default; "true"/"false". + public string ValidateServerHostName { get; set; } = ""; public string? DiscoveryUrl { get; set; } public string? DiscoveryServerUri { get; set; } @@ -381,6 +392,12 @@ SubscriptionPriority = r.SubscriptionPriority, RejectUnknownRevocationStatus = r.CertificateValidation.RejectUnknownRevocationStatus, IssuerStorePath = r.CertificateValidation.IssuerStorePath, + ValidateServerHostName = r.CertificateValidation.ValidateServerHostName switch + { + true => "true", + false => "false", + null => "", + }, DiscoveryUrl = r.DiscoveryUrl, DiscoveryServerUri = r.DiscoveryServerUri, }; @@ -417,6 +434,12 @@ { RejectUnknownRevocationStatus = RejectUnknownRevocationStatus, IssuerStorePath = string.IsNullOrWhiteSpace(IssuerStorePath) ? null : IssuerStorePath, + ValidateServerHostName = ValidateServerHostName switch + { + "true" => true, + "false" => false, + _ => null, + }, }, DiscoveryUrl = string.IsNullOrWhiteSpace(DiscoveryUrl) ? null : DiscoveryUrl, DiscoveryServerUri = string.IsNullOrWhiteSpace(DiscoveryServerUri) ? null : DiscoveryServerUri, diff --git a/tests/Client/ZB.MOM.WW.OtOpcUa.Client.CLI.Tests/CommandBaseTests.cs b/tests/Client/ZB.MOM.WW.OtOpcUa.Client.CLI.Tests/CommandBaseTests.cs index d88f7a76e..bff5d7066 100644 --- a/tests/Client/ZB.MOM.WW.OtOpcUa.Client.CLI.Tests/CommandBaseTests.cs +++ b/tests/Client/ZB.MOM.WW.OtOpcUa.Client.CLI.Tests/CommandBaseTests.cs @@ -38,6 +38,22 @@ public class CommandBaseTests settings.AutoAcceptCertificates.ShouldBeTrue(); } + /// Gitea #816: --validate-host-name maps to the server host-name check, which is off without it. + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task ValidateHostNameOption_MapsToConnectionSettings(bool flag) + { + var fakeService = new FakeOpcUaClientService(); + var factory = new FakeOpcUaClientServiceFactory(fakeService); + var command = new ConnectCommand(factory) { Url = "opc.tcp://localhost:4840", ValidateHostName = flag }; + + using var console = TestConsoleHelper.CreateConsole(); + await command.ExecuteAsync(console); + + fakeService.LastConnectionSettings!.ValidateServerHostName.ShouldBe(flag); + } + /// Verifies that encrypt option maps to SignAndEncrypt. [Fact] public async Task SecurityOption_Encrypt_MapsToSignAndEncrypt() diff --git a/tests/Client/ZB.MOM.WW.OtOpcUa.Client.Shared.Tests/Fakes/FakeSessionFactory.cs b/tests/Client/ZB.MOM.WW.OtOpcUa.Client.Shared.Tests/Fakes/FakeSessionFactory.cs index 521b6fbb8..427f70f7e 100644 --- a/tests/Client/ZB.MOM.WW.OtOpcUa.Client.Shared.Tests/Fakes/FakeSessionFactory.cs +++ b/tests/Client/ZB.MOM.WW.OtOpcUa.Client.Shared.Tests/Fakes/FakeSessionFactory.cs @@ -14,6 +14,8 @@ internal sealed class FakeSessionFactory : ISessionFactory public bool ThrowOnCreate { get; set; } /// Gets the endpoint URL from the last CreateSessionAsync call. public string? LastEndpointUrl { get; private set; } + /// Gets the host-name-validation flag from the last CreateSessionAsync call (#816). + public bool? LastValidateServerHostName { get; private set; } /// /// Optional gate that, when set, blocks until completed. @@ -33,14 +35,16 @@ internal sealed class FakeSessionFactory : ISessionFactory /// The session name. /// The session timeout in milliseconds. /// The user identity. + /// The SDK checkDomain flag (#816). /// Cancellation token. /// A task containing the session adapter. public async Task CreateSessionAsync( ApplicationConfiguration config, EndpointDescription endpoint, string sessionName, - uint sessionTimeoutMs, UserIdentity identity, CancellationToken ct) + uint sessionTimeoutMs, UserIdentity identity, bool validateServerHostName, CancellationToken ct) { CreateCallCount++; LastEndpointUrl = endpoint.EndpointUrl ?? string.Empty; + LastValidateServerHostName = validateServerHostName; // One-shot: the gate holds only the call that observed it, so a later call (e.g. a user's Connect while a // failover is held) runs freely. diff --git a/tests/Client/ZB.MOM.WW.OtOpcUa.Client.Shared.Tests/OpcUaClientServiceTests.cs b/tests/Client/ZB.MOM.WW.OtOpcUa.Client.Shared.Tests/OpcUaClientServiceTests.cs index 8c74f90c1..104f20fbf 100644 --- a/tests/Client/ZB.MOM.WW.OtOpcUa.Client.Shared.Tests/OpcUaClientServiceTests.cs +++ b/tests/Client/ZB.MOM.WW.OtOpcUa.Client.Shared.Tests/OpcUaClientServiceTests.cs @@ -55,6 +55,25 @@ public class OpcUaClientServiceTests : IDisposable _service.CurrentConnectionInfo.ShouldBe(info); } + /// + /// Gitea #816: the host-name check reaches the session factory (the SDK's checkDomain) — off by default, the + /// pre-#816 behaviour — and survives the per-endpoint settings copy the connect path makes. + /// + [Theory] + [InlineData(null)] + [InlineData(true)] + [InlineData(false)] + public async Task ConnectAsync_threads_ValidateServerHostName_to_the_session(bool? validate) + { + var settings = ValidSettings(); + if (validate is { } v) settings.ValidateServerHostName = v; + + await _service.ConnectAsync(settings, TestContext.Current.CancellationToken); + + _sessionFactory.LastValidateServerHostName.ShouldBe(validate ?? false); + _configFactory.LastSettings!.ValidateServerHostName.ShouldBe(validate ?? false); + } + /// /// Verifies that invalid connection settings fail validation before any OPC UA session is created. /// 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 index 39b59408f..5e66a574a 100644 --- 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 @@ -1,5 +1,6 @@ using System.Net; using System.Net.Sockets; +using System.Security.Cryptography.X509Certificates; using Opc.Ua; using Opc.Ua.Client; using Shouldly; @@ -20,27 +21,54 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Browser.IntegrationTests; /// # or locally / in CI (scripts/ci/integration-fixtures.sh starts the same shape as otopcua-ci-opcplc-nat): /// 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 --ph=otopcua-806-nat-parity.invalid --autoaccept --ut -/// OPCUA_NAT_ENDPOINT=opc.tcp://127.0.0.1:32781 dotnet test --filter Fixture=opc-plc-nat +/// docker exec otopcua-806-nat bash -c 'cat /app/pki/own/certs/*.der' > /tmp/nat.der # public cert only +/// OPCUA_NAT_ENDPOINT=opc.tcp://127.0.0.1:32781 OPCUA_NAT_SERVER_CERT=/tmp/nat.der 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. +/// Secure cases (#813/#816/#818). Every browse here runs on a THROWAWAY browse PKI (the browser's +/// internal PKI-root seam), never the operator's adminui-browse-pki. The refusal case trusts nothing. The +/// positive and host-name cases trust the fixture's PUBLIC certificate — from OPCUA_NAT_SERVER_CERT (a DER or +/// PEM file; what CI exports after copying it out of its throwaway container) or else the committed +/// Docker/nat/opc-plc-nat-server.der (the shared-host stack's certificate, stable since #818 put its PKI on a +/// volume) — and SKIP when the committed certificate is not the one the server presents, so they can never pass +/// against a different server. /// [Trait("Category", "Integration"), Trait("Fixture", "opc-plc-nat")] -public sealed class NatPublishedEndpointBrowseTests +public sealed class NatPublishedEndpointBrowseTests : IDisposable { private const string EnvVar = "OPCUA_NAT_ENDPOINT"; + private const string CertEnvVar = "OPCUA_NAT_SERVER_CERT"; + private const string CommittedCertFile = "opc-plc-nat-server.der"; + + private readonly string _pkiRoot = Path.Combine(Path.GetTempPath(), "otopcua-nat-pki-" + Guid.NewGuid().ToString("N")); 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" - } - """; + /// + public void Dispose() + { + try { Directory.Delete(_pkiRoot, recursive: true); } catch { /* best-effort temp cleanup */ } + } + + /// A browser on this test's throwaway PKI. + private OpcUaClientDriverBrowser Browser() => new(_pkiRoot); + + private static string Json(string endpoint, string policy = "None", string mode = "None", bool? validateHostName = null) + { + var certValidation = validateHostName is { } v + ? $$""","CertificateValidation":{"ValidateServerHostName":{{(v ? "true" : "false")}}}""" + : ""; + return $$""" + { + "EndpointUrl":"{{endpoint}}", + "SecurityPolicy":"{{policy}}","SecurityMode":"{{mode}}","AuthType":"Anonymous", + "SessionTimeout":"00:01:00","Timeout":"00:00:10","PerEndpointConnectTimeout":"00:00:10"{{certValidation}} + } + """; + } private static string RequireFixture() { @@ -60,8 +88,11 @@ public sealed class NatPublishedEndpointBrowseTests 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) + /// + /// The premise: the server advertises a host:port other than the configured one, and that host is unresolvable. + /// Returns the DER certificate the server presents on its secure endpoint. + /// + private static async Task AssertAdvertisesUnreachableAddressAsync(string endpoint, CancellationToken ct) { var appConfig = new ApplicationConfiguration(DefaultTelemetry.Create(_ => { })) { @@ -84,6 +115,61 @@ public sealed class NatPublishedEndpointBrowseTests await Should.ThrowAsync(() => Dns.GetHostAddressesAsync(advertised.Host, ct), $"advertised host '{advertised.Host}' must be unresolvable from here, or this test proves nothing"); } + + return endpoints.First(e => e.SecurityPolicyUri == SecurityPolicies.Basic256Sha256).ServerCertificate.ToArray(); + } + + /// + /// Trust the fixture's public certificate in this test's throwaway PKI, or SKIP when no certificate for THIS + /// server is available (see the class remarks). An explicit OPCUA_NAT_SERVER_CERT that does not match + /// FAILS: it names the certificate the operator meant, so a mismatch is a fixture problem worth seeing. + /// + /// The certificate the server presents (from its GetEndpoints answer). + private void TrustFixtureCertificate(byte[] presentedDer) + { + using var presented = X509CertificateLoader.LoadCertificate(presentedDer); + X509Certificate2 trusted; + if (Environment.GetEnvironmentVariable(CertEnvVar) is { Length: > 0 } path) + { + trusted = LoadPublicCertificate(path); + trusted.Thumbprint.ShouldBe(presented.Thumbprint, + $"{CertEnvVar}={path} is not the certificate the NAT fixture presents"); + } + else + { + var committed = Path.Combine(AppContext.BaseDirectory, CommittedCertFile); + if (!File.Exists(committed)) + Assert.Skip($"{CommittedCertFile} is not deployed next to the tests and {CertEnvVar} is unset."); + trusted = LoadPublicCertificate(committed); + if (trusted.Thumbprint != presented.Thumbprint) + Assert.Skip($"the committed {CommittedCertFile} ({trusted.Thumbprint}) is not this fixture's certificate " + + $"({presented.Thumbprint}, expires {presented.NotAfter:yyyy-MM-dd}); set {CertEnvVar} or " + + "refresh the committed copy (see Docker/nat/docker-compose.yml)."); + } + + using (trusted) + { + trusted.HasPrivateKey.ShouldBeFalse("only a PUBLIC certificate is ever trusted from a file"); + var certs = Path.Combine(_pkiRoot, "trusted", "certs"); + Directory.CreateDirectory(certs); + File.WriteAllBytes(Path.Combine(certs, "opc-plc-nat.der"), trusted.RawData); + } + } + + private static X509Certificate2 LoadPublicCertificate(string path) + { + var bytes = File.ReadAllBytes(path); + var text = System.Text.Encoding.ASCII.GetString(bytes); + return text.Contains("-----BEGIN CERTIFICATE-----", StringComparison.Ordinal) + ? X509Certificate2.CreateFromPem(text) + : X509CertificateLoader.LoadCertificate(bytes); + } + + private static StatusCode? FindStatus(Exception? ex) + { + for (; ex is not null; ex = ex.InnerException) + if (ex is ServiceResultException sre) return sre.StatusCode; + return null; } [Fact] @@ -93,7 +179,7 @@ public sealed class NatPublishedEndpointBrowseTests var ct = TestContext.Current.CancellationToken; await AssertAdvertisesUnreachableAddressAsync(endpoint, ct); - await using var session = await new OpcUaClientDriverBrowser().OpenAsync(Json(endpoint), ct); + await using var session = await Browser().OpenAsync(Json(endpoint), ct); var roots = await session.RootAsync(ct); roots.Count.ShouldBeGreaterThan(0); @@ -111,7 +197,7 @@ public sealed class NatPublishedEndpointBrowseTests 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)); + await Should.ThrowAsync(() => Browser().OpenAsync(Json("opc.tcp://127.0.0.1:1"), ct)); sw.Elapsed.ShouldBeLessThan(TimeSpan.FromSeconds(30)); } @@ -122,29 +208,59 @@ public sealed class NatPublishedEndpointBrowseTests 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; - } - + // Basic256Sha256/SignAndEncrypt through the published port, on a browse PKI that trusts NOTHING (the browse + // never auto-accepts): 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 ex = await Should.ThrowAsync(async () => { - await using var session = await new OpcUaClientDriverBrowser().OpenAsync(json, ct); + await using var session = await Browser().OpenAsync(Json(endpoint, "Basic256Sha256", "SignAndEncrypt"), 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( + var status = FindStatus(ex); + status.ShouldNotBeNull($"expected a certificate refusal, got {ex.GetType().Name}: {ex.Message}"); + status.Value.ShouldBeOneOf( StatusCodes.BadCertificateUntrusted, StatusCodes.BadSecurityChecksFailed, StatusCodes.BadCertificateChainIncomplete); } + + /// + /// #818 — the positive secure case, runnable in CI. #816 — with the host-name check unset (the default) or + /// explicitly off, a trusted certificate is accepted although it names *-nat-private.invalid and the + /// browse dialled the published address: the NAT opt-out. + /// + [Theory] + [InlineData(null)] + [InlineData(false)] + public async Task Secure_browse_with_the_fixture_certificate_trusted_succeeds_without_a_host_name_check(bool? validate) + { + var endpoint = RequireFixture(); + var ct = TestContext.Current.CancellationToken; + TrustFixtureCertificate(await AssertAdvertisesUnreachableAddressAsync(endpoint, ct)); + + await using var session = await Browser().OpenAsync( + Json(endpoint, "Basic256Sha256", "SignAndEncrypt", validate), ct); + (await session.RootAsync(ct)).Count.ShouldBeGreaterThan(0); + } + + /// + /// #816 — with the host-name check on, the SAME trusted certificate is refused, and the host checked is the one + /// the operator configured (the published address), not the server-advertised private name the certificate lists. + /// + [Fact] + public async Task Host_name_validation_checks_the_configured_host_and_refuses_the_NAT_certificate() + { + var endpoint = RequireFixture(); + var ct = TestContext.Current.CancellationToken; + TrustFixtureCertificate(await AssertAdvertisesUnreachableAddressAsync(endpoint, ct)); + + var ex = await Should.ThrowAsync(async () => + { + await using var session = await Browser().OpenAsync( + Json(endpoint, "Basic256Sha256", "SignAndEncrypt", validateHostName: true), ct); + }); + TestContext.Current.TestOutputHelper?.WriteLine($"secure open refused: {ex.GetType().Name}: {ex.Message}"); + FindStatus(ex).ShouldBe(StatusCodes.BadCertificateHostNameInvalid, $"expected a host-name refusal, got {ex}"); + ex.ToString().ShouldContain($"'{new Uri(endpoint).IdnHost}'"); + } } diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Browser.IntegrationTests/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Browser.IntegrationTests.csproj b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Browser.IntegrationTests/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Browser.IntegrationTests.csproj index dee7cb7d4..19f51816a 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Browser.IntegrationTests/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Browser.IntegrationTests.csproj +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Browser.IntegrationTests/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Browser.IntegrationTests.csproj @@ -24,4 +24,11 @@ + + + + + diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.IntegrationTests/Docker/README.md b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.IntegrationTests/Docker/README.md index 19817102d..816049ce7 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.IntegrationTests/Docker/README.md +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.IntegrationTests/Docker/README.md @@ -10,7 +10,8 @@ here since opc-plc ships pre-containerized. | File | Purpose | |---|---| | [`docker-compose.yml`](docker-compose.yml) | Service definition for `otopcua-opc-plc` — image pin, port map, command flags. | -| [`nat/docker-compose.yml`](nat/docker-compose.yml) | `otopcua-opc-plc-nat` — a SEPARATE stack (`/opt/otopcua-opcuaclient-nat` on the shared host): opc-plc advertising a private `.invalid` host on internal port 50000, published on 50010. For `NatPublishedEndpointBrowseTests` (`OPCUA_NAT_ENDPOINT=opc.tcp://10.100.0.35:50010`, #806/#813); see the compose file's header. | +| [`nat/docker-compose.yml`](nat/docker-compose.yml) | `otopcua-opc-plc-nat` — a SEPARATE stack (`/opt/otopcua-opcuaclient-nat` on the shared host): opc-plc advertising a private `.invalid` host on internal port 50000, published on 50010. For `NatPublishedEndpointBrowseTests` (`OPCUA_NAT_ENDPOINT=opc.tcp://10.100.0.35:50010`, #806/#813); see the compose file's header. Its PKI is on a volume (#818). | +| [`nat/opc-plc-nat-server.der`](nat/opc-plc-nat-server.der) | The NAT stack's **public** server certificate (no key), trusted by the positive secure / host-name NAT tests in a throwaway browse PKI. Refresh it when the stack's certificate is rolled (`docker compose down -v`); expires 2027-11-10. | | (this file) | How to run it. | ## Install @@ -34,7 +35,8 @@ docker compose up ``` First run pulls the image (~250 MB). Startup takes ~5-10 seconds; the -healthcheck in the compose file surfaces ready state in `docker ps`. +healthcheck in the compose file surfaces ready state in `docker ps` (a bash `/dev/tcp` probe — +the image has no `netstat`, which kept the old check permanently "unhealthy", #817). To run detached (CI pattern): diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.IntegrationTests/Docker/nat/docker-compose.yml b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.IntegrationTests/Docker/nat/docker-compose.yml index ccb40f66b..32f2956d5 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.IntegrationTests/Docker/nat/docker-compose.yml +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.IntegrationTests/Docker/nat/docker-compose.yml @@ -20,6 +20,16 @@ # ssh dohertj2@10.100.0.35 'cd /opt/otopcua-opcuaclient-nat && docker compose up -d' # OPCUA_NAT_ENDPOINT=opc.tcp://10.100.0.35:50010 dotnet test \ # tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Browser.IntegrationTests --filter Fixture=opc-plc-nat +# +# #818: the server's PKI (/app/pki — own cert + private key, and the client certs --aa accepted) lives in the +# named volume `opc-plc-nat-pki`, so recreating the container keeps the SAME server certificate. Its PUBLIC half is +# committed next to this file (opc-plc-nat-server.der) and the secure tests trust it in a throwaway browse PKI. The +# private key never leaves the volume. To roll the certificate deliberately (e.g. at expiry, 2027-11-10): +# ssh dohertj2@10.100.0.35 'cd /opt/otopcua-opcuaclient-nat && docker compose down -v && docker compose up -d' +# ssh dohertj2@10.100.0.35 'docker exec otopcua-opc-plc-nat bash -c "cat /app/pki/own/certs/*.der"' \ +# > tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.IntegrationTests/Docker/nat/opc-plc-nat-server.der +# (until the committed copy is refreshed the positive secure tests SKIP on a thumbprint mismatch — they never pass +# against a certificate other than the committed one). services: opc-plc-nat: image: mcr.microsoft.com/iotedge/opc-plc:2.14.10 @@ -40,11 +50,20 @@ services: # --aa: accept the browser's self-signed client cert. Trust in the OTHER direction (the browser trusting # this server's cert) is deliberately not automatic — that is what the secure test pins. - "--aa" + volumes: + - opc-plc-nat-pki:/app/pki + # The image runs as uid 1654 ("app") and ships no /app/pki, so Docker creates the volume mount point root-owned + # and opc-plc could not write its certificate there. A throwaway test simulator: run it as root rather than add a + # chown init container. + user: "0" healthcheck: - # The image ships no netstat (the sibling ../docker-compose.yml healthcheck is permanently "unhealthy" for that - # reason); bash's /dev/tcp is there. + # The image ships no netstat (the sibling ../docker-compose.yml healthcheck was permanently "unhealthy" for that + # reason until #817 switched it to this probe too); bash's /dev/tcp is there. test: ["CMD", "bash", "-c", "exec 3<>/dev/tcp/127.0.0.1/50000"] interval: 5s timeout: 2s retries: 10 start_period: 10s + +volumes: + opc-plc-nat-pki: diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.IntegrationTests/Docker/nat/opc-plc-nat-server.der b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.IntegrationTests/Docker/nat/opc-plc-nat-server.der new file mode 100644 index 0000000000000000000000000000000000000000..a06fb8e86ba0fd5fb9ebabe7550b2ad49fa58d7c GIT binary patch literal 1043 zcmXqLV&OMvV*0*-nTe5!i7WVQ(x=8LR^lmH2E1&XT5TR}-+37sxmg(uv<$@!MA?`_ zS-6Dx`~#d7-11BFQW8rt^YaY(fog#g%))H`1<3(9$p&)byhdh*hK2?Z5C!C#BXLPG ztBFww*(r>y49rc8{0s(7j9g4jjEoHDcJEGL-zhs~Zs1?Fzehuw;yo^3h)6oB-}UPn zFZ&mxD;eusm4l-kPg+WE<1-3Ar?ULrmslOE8!x6E3jOx~y!7(=(3@V&ZNc&XqJ=^m zTFRfro;)*8DB)*Ov7vS3?LSv@qT zzmd80P*2bC*0x2L^Dj(aE_`~sZKMCQ>SH0SQeMJ+T1)0tXl^)Gb?ZL=>@PBRlQcGb zX1nsFE#j=^;_Lq|*Ekv-VG;;;@O=AY^X)^gKYo^+bdrggk%4h>6Qe0GL<|jhfDs`p z%*gnkg~@=yKo-Q~V-aH!*)T7FS=;=>8QIj9=UHa0N2eMF-7ts-Nh`C27z8z7Rj^QR zi4G}o)r6dU_&|p9gIvPG%*6D?Kmo*81@XlUMA$gA*%(<_*_j#PEGC0ukP1PT9D}Sj zwbG(ID@bCp$}h<;NG?s(1(Lc2ImxUqtVj9muHELDYA&|Uc8clL8|UMls?W}J zNbcBUxj%0D^v~_}4|6+w1TJRzUY8fy+NO3$QCw51^vTEf_0o2dz70G4?7mKNUO2;G z{m0@)X{Cl&`~scJ-IbqiT(P$3j=_}JP$A*&vVRse6^$Fj9GK_bme;dwJ=xkGuzit# zjZ~}4)2RP;zy8>77X2>$@vmRMK*zO{>nhqFIA$Ji5%{xc;*t=vFPm~&J{N^q`uxyU zUv%5EGe~}3mWY3jo{?np+eQ_sPXQ~HBzt~O-O~Ojf0}ZELjUQz$@;gfCVxBU`$Qsq VnNZKQRW>c60_m0CALMYC0RXN#i^%{0 literal 0 HcmV?d00001 diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.IntegrationTests/ServerHostNameValidationTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.IntegrationTests/ServerHostNameValidationTests.cs new file mode 100644 index 000000000..1687d7d06 --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.IntegrationTests/ServerHostNameValidationTests.cs @@ -0,0 +1,108 @@ +using Opc.Ua; +using Shouldly; +using Xunit; + +namespace ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.IntegrationTests; + +/// +/// Gitea #816 — does the driver check the server certificate's host name? Against the plain opc-plc fixture, whose +/// certificate names only its container hostname (e.g. DNS:1fc1de3fcb21) while the driver dials the docker +/// host's address, so a host check MUST fail if it runs at all. +/// Before #816 the driver called the SDK's 8-argument DefaultSessionFactory.CreateAsync, which hard-codes +/// checkDomain: false: the mismatch was never noticed. Now certificateValidation.validateServerHostName +/// decides; unset keeps the old behaviour. +/// Each test trusts the server's certificate in a THROWAWAY PKI (the driver's PkiRootOverride) with +/// auto-accept OFF — auto-accept also accepts a host-name mismatch and would hide the check. The certificate is +/// taken from the server's own GetEndpoints answer: what is under test is the host check, not how trust was +/// established. +/// +[Collection(OpcPlcCollection.Name)] +[Trait("Category", "Integration")] +[Trait("Simulator", "opc-plc")] +public sealed class ServerHostNameValidationTests(OpcPlcFixture sim) : IDisposable +{ + private readonly string _pkiRoot = Path.Combine(Path.GetTempPath(), "otopcua-816-" + Guid.NewGuid().ToString("N")); + + /// + public void Dispose() + { + try { Directory.Delete(_pkiRoot, recursive: true); } catch { /* best-effort temp cleanup */ } + } + + private OpcUaClientDriverOptions SecureOptions(bool? validateHostName) => OpcPlcProfile.BuildOptions(sim.EndpointUrl) with + { + SecurityPolicy = OpcUaSecurityPolicy.Basic256Sha256, + SecurityMode = OpcUaSecurityMode.SignAndEncrypt, + AutoAcceptCertificates = false, + CertificateValidation = new OpcUaCertificateValidationOptions { ValidateServerHostName = validateHostName }, + }; + + /// Trust the fixture's server certificate (as advertised on its secure endpoint) in the throwaway PKI. + private async Task TrustServerCertificateAsync(CancellationToken ct) + { + var appConfig = new ApplicationConfiguration(DefaultTelemetry.Create(_ => { })) + { + ApplicationName = "OtOpcUa #816 trust", + ApplicationType = ApplicationType.Client, + SecurityConfiguration = new SecurityConfiguration(), + TransportQuotas = new TransportQuotas(), + ClientConfiguration = new ClientConfiguration(), + DisableHiResClock = true, + }; + using var discovery = await DiscoveryClient.CreateAsync(appConfig, new Uri(sim.EndpointUrl), DiagnosticsMasks.None, ct); + var endpoints = await discovery.GetEndpointsAsync(default, ct); + var secure = endpoints.ToList().First(e => e.SecurityPolicyUri == SecurityPolicies.Basic256Sha256); + var der = secure.ServerCertificate.ToArray(); + der.ShouldNotBeEmpty("the secure endpoint must carry the server certificate"); + + var certs = Path.Combine(_pkiRoot, "trusted", "certs"); + Directory.CreateDirectory(certs); + await File.WriteAllBytesAsync(Path.Combine(certs, "opc-plc.der"), der, ct); + } + + private async Task InitializeAsync(bool? validateHostName, string id, CancellationToken ct) + { + var drv = new OpcUaClientDriver(SecureOptions(validateHostName), driverInstanceId: id) { PkiRootOverride = _pkiRoot }; + try + { + await drv.InitializeAsync("{}", ct); + return drv; + } + catch + { + await drv.DisposeAsync(); + throw; + } + } + + [Theory] + [InlineData(null)] // unset = the pre-#816 behaviour every existing deployment has + [InlineData(false)] // the explicit NAT / IP-addressed opt-out + public async Task Without_host_name_validation_a_trusted_certificate_for_another_host_connects(bool? validate) + { + if (sim.SkipReason is not null) Assert.Skip(sim.SkipReason); + var ct = TestContext.Current.CancellationToken; + await TrustServerCertificateAsync(ct); + + await using var drv = await InitializeAsync(validate, $"opcua-816-off-{validate}", ct); + var snapshots = await drv.ReadAsync([OpcPlcProfile.StepUpRef], ct); + snapshots[0].StatusCode.ShouldBe(0u, "the secured session must read once the server certificate is trusted"); + } + + [Fact] + public async Task With_host_name_validation_the_certificate_is_checked_against_the_configured_host() + { + if (sim.SkipReason is not null) Assert.Skip(sim.SkipReason); + var ct = TestContext.Current.CancellationToken; + await TrustServerCertificateAsync(ct); + + var ex = await Should.ThrowAsync(() => InitializeAsync(true, "opcua-816-on", ct)); + TestContext.Current.TestOutputHelper?.WriteLine($"refused: {ex}"); + // The driver's failover aggregate keeps per-endpoint failures as text. The SDK message names the host it + // checked: the CONFIGURED one (the endpoint URL was rewritten to it), not whatever the server advertised. + var configuredHost = new Uri(sim.EndpointUrl).IdnHost; + var text = ex.ToString(); + text.ShouldContain($"The domain '{configuredHost}' is not listed in the server certificate"); + text.ShouldContain("validateServerHostName=false", Case.Sensitive, "the refusal must name the opt-out"); + } +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests/OpcUaClientServerHostNameOptionTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests/OpcUaClientServerHostNameOptionTests.cs new file mode 100644 index 000000000..a4c27f1b7 --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests/OpcUaClientServerHostNameOptionTests.cs @@ -0,0 +1,57 @@ +using Opc.Ua; +using Shouldly; +using Xunit; + +namespace ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests; + +/// +/// Gitea #816 — certificateValidation.validateServerHostName. The wire behaviour (the SDK's checkDomain against +/// the CONFIGURED host) is pinned against real servers by ServerHostNameValidationTests (driver, plain opc-plc) +/// and NatPublishedEndpointBrowseTests (browser, NAT opc-plc); these pin the option contract. +/// +[Trait("Category", "Unit")] +public sealed class OpcUaClientServerHostNameOptionTests +{ + [Fact] + public void Unset_resolves_to_off_so_existing_deployments_are_unchanged() + { + var o = new OpcUaClientDriverOptions(); + o.CertificateValidation.ValidateServerHostName.ShouldBeNull(); + o.CertificateValidation.EffectiveValidateServerHostName.ShouldBeFalse(); + OpcUaCertificateValidationOptions.DefaultValidateServerHostName.ShouldBeFalse(); + + OpcUaClientDriverFactoryExtensions.ParseOptions("d", """{"endpointUrl":"opc.tcp://x:1"}""") + .CertificateValidation.EffectiveValidateServerHostName.ShouldBeFalse(); + } + + [Theory] + [InlineData("true", true)] + [InlineData("false", false)] + public void Factory_json_binds_the_explicit_choice(string json, bool expected) + { + var o = OpcUaClientDriverFactoryExtensions.ParseOptions("d", + $$$"""{"endpointUrl":"opc.tcp://x:1","certificateValidation":{"validateServerHostName":{{{json}}}}}"""); + + o.CertificateValidation.ValidateServerHostName.ShouldBe(expected); + o.CertificateValidation.EffectiveValidateServerHostName.ShouldBe(expected); + } + + [Fact] + public void The_effective_value_is_not_serialised_back() + { + var json = System.Text.Json.JsonSerializer.Serialize(new OpcUaCertificateValidationOptions()); + json.ShouldNotContain("Effective"); + } + + [Fact] + public void A_host_name_refusal_names_the_opt_out() + { + var hint = OpcUaClientDriver.HostNameRefusalHint(new ServiceResultException( + StatusCodes.BadCertificateHostNameInvalid, "The domain '10.1.2.3' is not listed in the server certificate.")); + hint.ShouldContain("validateServerHostName=false"); + + OpcUaClientDriver.HostNameRefusalHint(new ServiceResultException(StatusCodes.BadCertificateUntrusted)) + .ShouldBeEmpty(); + OpcUaClientDriver.HostNameRefusalHint(new InvalidOperationException("x")).ShouldBeEmpty(); + } +} diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/OpcUaClientDriverPageFormSerializationTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/OpcUaClientDriverPageFormSerializationTests.cs index eede6ea95..9940ba49f 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/OpcUaClientDriverPageFormSerializationTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/OpcUaClientDriverPageFormSerializationTests.cs @@ -180,6 +180,31 @@ public sealed class OpcUaClientDriverPageFormSerializationTests def.DiscoveryUrl.ShouldBeNull(); } + /// + /// #816: the server host-name check is tri-state. Unset must stay unset through the form (so a later change of + /// the driver default reaches configs nobody chose for), and an explicit opt-in / NAT opt-out must survive. + /// + [Theory] + [InlineData(null)] + [InlineData(true)] + [InlineData(false)] + public void FormModel_RoundTrip_PreservesHostNameValidationTriState(bool? value) + { + var formOpts = new JsonSerializerOptions(_opts) { Converters = { new JsonStringEnumConverter() } }; + var original = new OpcUaClientDriverOptions + { + CertificateValidation = new OpcUaCertificateValidationOptions { ValidateServerHostName = value }, + }; + + var back = OpcUaClientDriverForm.OpcUaClientFormModel.FromRecord(original).ToRecord([]); + back.CertificateValidation.ValidateServerHostName.ShouldBe(value); + + var reparsed = JsonSerializer.Deserialize( + JsonSerializer.Serialize(back, formOpts), formOpts)!; + reparsed.CertificateValidation.ValidateServerHostName.ShouldBe(value); + reparsed.CertificateValidation.EffectiveValidateServerHostName.ShouldBe(value ?? false); + } + /// /// #648: stored DriverConfig JSON from before targetNamespaceKind/unsMappingTable were removed /// must still load into the form (the form's options use UnmappedMemberHandling.Skip) and the legacy From 9aff90f884565f6f2a6c744cc9d5cc5ef0a51ee6 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Wed, 11 Nov 2026 01:53:01 -0500 Subject: [PATCH 4/4] fix(opcuaclient): never pass checkDomain for a None endpoint; document both SDK check sites (#816 review) SDK 2.0.1 checks the server host name in TWO places: Session.OpenAsync (after the chain is trusted) and DefaultSessionFactory.CreateChannelAsync, against the certificate in the GetEndpoints reply and before any security-mode test. opc-plc sends a certificate on its None endpoint, so `connect -u opc.tcp://10.100.0.35:50000 --validate-host-name` was refused on an unauthenticated channel, where a host check proves nothing. All three call sites now pass checkDomain only when the selected endpoint's SecurityMode != None: OpcUaEndpointSelection.CheckDomainFor for the driver (OpenSessionOnEndpointAsync, so the failover sweep too) and the AdminUI browser; the same expression in Client.Shared ConnectToEndpointAsync (shared by failover). New live tests: driver None + flag on vs :50000 reads; browser None + flag on vs the NAT fixture browses; the secure + flag refusals still hold. Docs: name both check sites; a zone-qualified IPv6 host never matches (FindDomain compares the zone too), so link-local needs validateServerHostName=false; the match is loose (first label alone, subject DC= parts count, no wildcards), which caveats the CA recommendation. --- .claude/rules/drivers.md | 7 ++- docs/Client.CLI.md | 2 +- docs/drivers/OpcUaClient.md | 43 ++++++++++++++----- .../CommandBase.cs | 2 +- .../Models/ConnectionSettings.cs | 6 ++- .../OpcUaClientService.cs | 6 ++- .../OpcUaClientDriverBrowser.cs | 6 ++- .../OpcUaClientDriverOptions.cs | 9 ++-- .../OpcUaEndpointSelection.cs | 17 ++++++++ .../OpcUaClientDriver.cs | 6 ++- .../Drivers/Forms/OpcUaClientDriverForm.razor | 2 +- .../OpcUaClientServiceTests.cs | 21 ++++++--- .../NatPublishedEndpointBrowseTests.cs | 15 +++++++ .../ServerHostNameValidationTests.cs | 21 +++++++++ .../OpcUaClientServerHostNameOptionTests.cs | 15 +++++++ 15 files changed, 145 insertions(+), 33 deletions(-) diff --git a/.claude/rules/drivers.md b/.claude/rules/drivers.md index ecdbfcca6..5de204ad0 100644 --- a/.claude/rules/drivers.md +++ b/.claude/rules/drivers.md @@ -292,8 +292,11 @@ host logger factory (wired in `DriverFactoryBootstrap`; the FOCAS and TwinCAT fa `checkDomain: true`; its 8-arg `DefaultSessionFactory.CreateAsync` hard-codes `false`, so driver, AdminUI browser and Client.Shared never checked before #816. Now `certificateValidation.validateServerHostName` (tri-state, unset = off = unchanged; AdminUI keeps it unset) / CLI `--validate-host-name`. It checks the **configured** host (the rewritten - `ConfiguredEndpoint.EndpointUrl`), secured sessions only, and auto-accept accepts a mismatch. Any new session-create - call site must pass the flag — the overload without it silently disables the check. The rewriter keeps an IPv6 + `ConfiguredEndpoint.EndpointUrl`) and auto-accept accepts a mismatch. ⚠️ The SDK checks in TWO places — + `DefaultSessionFactory.CreateChannelAsync` (GetEndpoints cert, BEFORE any security-mode test, so it fired on `None`) + and `Session.OpenAsync` — so pass `checkDomain` only for a non-`None` endpoint (`OpcUaEndpointSelection.CheckDomainFor`). + Any new session-create call site must do that — the overload without the argument silently disables the check. + Zone-qualified IPv6 hosts never match (zone compared); the match is loose (first label alone counts). The rewriter keeps an IPv6 zone index verbatim (#819; .NET `Uri` drops it from every re-serialisation). `docs/drivers/OpcUaClient.md`. - **MTConnect (#627/#628):** TIME_SERIES reads as a variable-length `double[]` (Float64 only); a count disagreeing with `sampleCount` is `BadDecodingError`. One MTConnect driver = one agent client = at most one `/raw` Device diff --git a/docs/Client.CLI.md b/docs/Client.CLI.md index 6cb9eaeec..5f71be86b 100644 --- a/docs/Client.CLI.md +++ b/docs/Client.CLI.md @@ -39,7 +39,7 @@ All commands accept these options: | `-P` / `--password` | Password for `UserName` token authentication | | `-S` / `--security` | Transport security mode: `none`, `sign`, `encrypt`, `signandencrypt` (default: `none`) | | `-F` / `--failover-urls` | Comma-separated failover endpoint URLs for redundancy | -| `--validate-host-name` | Require the server certificate to list the `--url` host (#816). Secured connections only; off by default (the CLI's long-standing behaviour). The CLI auto-accepts untrusted certificates, so this checks a misconfiguration, not trust. Leave off behind NAT / when connecting by an IP the certificate omits | +| `--validate-host-name` | Require the server certificate to list the `--url` host (#816). Only with `-S sign`/`encrypt` — ignored for `-S none` (the SDK would otherwise check the GetEndpoints certificate even on an unsecured channel); off by default (the CLI's long-standing behaviour). The CLI auto-accepts untrusted certificates, so this checks a misconfiguration, not trust. Leave off behind NAT / when connecting by an IP the certificate omits / for a zone-qualified IPv6 host | | `--verbose` | Enable debug-level Serilog console logging (default: warning) | **`--security none` does not fall back.** Endpoint selection picks the server's `None`-policy endpoint and diff --git a/docs/drivers/OpcUaClient.md b/docs/drivers/OpcUaClient.md index 9ed288728..fa4c18ccc 100644 --- a/docs/drivers/OpcUaClient.md +++ b/docs/drivers/OpcUaClient.md @@ -137,18 +137,37 @@ AdminUI browser (`OpcUaClientDriverBrowser.OpenAsync`) and Client.Shared (`Defau forwards `checkDomain: false`. So **no host-name check ever ran**; trust was the trusted-peer / issuer store alone. That is why, in #813, a trusted NAT certificate for `otopcua-opc-plc-nat-private.invalid` browsed fine over an IP. -Where the check lives when enabled: `Session.OpenAsync` → after `CertificateManager.ValidateAsync` accepts the chain, -`if (checkDomain) CertificateValidationExtensions.ValidateDomains(validator, serverCert, session.ConfiguredEndpoint)` -→ `CertificateValidationCore.ValidateDomains` → `CertificateValidationHelpers.FindDomain`, which compares -`ConfiguredEndpoint.EndpointUrl.IdnHost` with the certificate's DNS / IP SANs (IP literals normalised; a `localhost` -/ `127.0.0.1` / `::1` URL is compared as the machine's own host name + FQDN). A miss is `BadCertificateHostNameInvalid` -("The domain '…' is not listed in the server certificate."), passed to the certificate manager's `AcceptError` hook -first. Consequences: +Where the check lives when enabled — **two** places in SDK 2.0.1, both calling +`CertificateValidationExtensions.ValidateDomains` → `CertificateValidationCore.ValidateDomains` → +`CertificateValidationHelpers.FindDomain`: + +1. `DefaultSessionFactory.CreateChannelAsync` — before the channel opens, against the certificate in the + **GetEndpoints reply** (`endpoint.Description.ServerCertificate`), whenever that reply carries one and **before any + security-mode test**. opc-plc sends its certificate on its `None` endpoint too, so `checkDomain: true` refused a + `None` connect (found in review, reproduced against `:50000`). +2. `Session.OpenAsync` — after `CertificateManager.ValidateAsync` accepts the chain the server returned in + CreateSession (only when the channel or the user-token policy needs the server certificate). + +`FindDomain` compares `ConfiguredEndpoint.EndpointUrl.IdnHost` with the names `X509Utils.GetDomainsFromCertificate` +returns (DNS / IP SANs, and the subject's `DC=` parts); IP literals are normalised, and a `localhost` / `127.0.0.1` / +`::1` URL is compared as the machine's own host name + FQDN. A miss is `BadCertificateHostNameInvalid` ("The domain +'…' is not listed in the server certificate."), passed to the certificate manager's `AcceptError` hook first. +Consequences: - **It checks the CONFIGURED host.** `ConfiguredEndpoint.EndpointUrl` is the URL `OpcUaEndpointSelection` / `EndpointSelector` rewrote, so the advertised host the server named itself never takes part. -- **Secured sessions only** — the block runs only when the channel (or the user-token policy) needs the server - certificate; a `None` session validates no certificate at all. +- **Sign / SignAndEncrypt endpoints only — enforced here, not by the SDK.** Because of site 1, all three call sites + pass `checkDomain` only when the selected endpoint's `SecurityMode != None` (`OpcUaEndpointSelection.CheckDomainFor` + for the driver and the browser, the same expression in Client.Shared's `ConnectToEndpointAsync`; both cover the + failover sweep, which goes through the same per-endpoint open). A host-name check on an unauthenticated channel + proves nothing. +- **It is a loose match.** For a DNS host the SDK also accepts a match on the **first label alone**: + `plc1.site-a.corp` passes a certificate whose only SAN is `PLC1` (case-insensitive), and so would + `plc1.site-b.corp`. There is no wildcard support (`*.site-a.corp` matches nothing), and the subject's `DC=` + components count as names. +- **A zone-qualified IPv6 host never passes.** `FindDomain` compares the host *with* its zone (`fe80::1%eth0`; macOS + normalises `%en0` to `%14`) against the certificate's zone-less IP SANs, so a link-local endpoint (#819) needs + `validateServerHostName: false`. The repo does not fork the SDK check to strip the zone. - **`autoAcceptCertificates` accepts a mismatch too** (the driver's auto-accept `AcceptError` handler returns `true` for everything but revocation). The Client CLI always auto-accepts. - Test Connect (`OpcUaClientDriverProbe`) only calls GetEndpoints on an unsecured discovery channel — no session, so no @@ -164,7 +183,9 @@ the browser reads the same JSON): | `false` | Explicit opt-out — the NAT / container port-publish / connect-by-an-IP-the-cert-omits case. | When to turn it on: it earns its keep when trust comes from a **CA** (a trusted issuer vouches for every certificate -it signed, so only the host check ties the certificate to *this* server). With the server's own self-signed +it signed, so only the host check ties the certificate to *this* server) — within the limits above: a +first-label-only match means a certificate issued as `PLC1` for one site is accepted for `plc1` at every site, so give +certificates fully-qualified, site-unique names. With the server's own self-signed certificate pinned in the trusted-peer store it adds little — the pin already identifies the server. Client CLI: `--validate-host-name` (`ConnectionSettings.ValidateServerHostName`, default `false`). Pinned by `ServerHostNameValidationTests` (driver vs. the plain opc-plc, whose certificate names only its container id) and @@ -437,7 +458,7 @@ also refuses them explicitly — defence in depth (#277), not a behaviour change |---|---|---| | `rejectUnknownRevocationStatus` | `false` | Reject a server certificate whose revocation status cannot be determined (no CRL for its issuer, or a stale one). Leave `false` for brownfield sites without CRL infrastructure. **Only effective with `autoAcceptCertificates` off** — the auto-accept handler accepts every suppressible error, including revocation-unknown. | | `issuerStorePath` | `null` (= `%LocalAppData%/OtOpcUa/pki/issuers`) | Directory used as the trusted-issuer store. The SDK reads issuer CRLs (`*.crl`) from its `crl/` sub-folder, so this is how an operator points the driver at a managed CRL set. | -| `validateServerHostName` | `null` (= off) | #816: require the server certificate to name the **configured** endpoint host (`BadCertificateHostNameInvalid` otherwise). Secured sessions only; accepted anyway under `autoAcceptCertificates`. `false` = explicit NAT opt-out. See "Server certificate host name" above. | +| `validateServerHostName` | `null` (= off) | #816: require the server certificate to name the **configured** endpoint host (`BadCertificateHostNameInvalid` otherwise). Sign / SignAndEncrypt endpoints only (never passed for `None`); accepted anyway under `autoAcceptCertificates`; loose (first-label) match; never matches a zone-qualified IPv6 host. `false` = explicit NAT / link-local opt-out. See "Server certificate host name" above. | ### Subscription tuning (#273) diff --git a/src/Client/ZB.MOM.WW.OtOpcUa.Client.CLI/CommandBase.cs b/src/Client/ZB.MOM.WW.OtOpcUa.Client.CLI/CommandBase.cs index ed95be7c6..8bb02aca5 100644 --- a/src/Client/ZB.MOM.WW.OtOpcUa.Client.CLI/CommandBase.cs +++ b/src/Client/ZB.MOM.WW.OtOpcUa.Client.CLI/CommandBase.cs @@ -61,7 +61,7 @@ public abstract class CommandBase : ICommand /// Gets a value indicating whether the server certificate must name the host given in (#816). /// [CommandOption("validate-host-name", - Description = "Require the server certificate to list the --url host (secured connections only; off by default — leave off behind NAT)")] + Description = "Require the server certificate to list the --url host (sign/encrypt only, never with --security none; off by default — leave off behind NAT)")] public bool ValidateHostName { get; init; } /// diff --git a/src/Client/ZB.MOM.WW.OtOpcUa.Client.Shared/Models/ConnectionSettings.cs b/src/Client/ZB.MOM.WW.OtOpcUa.Client.Shared/Models/ConnectionSettings.cs index 952bda84b..233242dd9 100644 --- a/src/Client/ZB.MOM.WW.OtOpcUa.Client.Shared/Models/ConnectionSettings.cs +++ b/src/Client/ZB.MOM.WW.OtOpcUa.Client.Shared/Models/ConnectionSettings.cs @@ -44,8 +44,10 @@ public sealed class ConnectionSettings /// Gitea #816 — require the server certificate to list the host of the URL being connected (the operator's /// host: discovery keeps it in place of the server-advertised one) among its subject-alternative names; /// a mismatch fails the connect with BadCertificateHostNameInvalid. Defaults to false, the - /// behaviour before #816 (the SDK overload used never checked). Secured connections only. Leave it off behind - /// NAT / a port publish / when connecting by an IP the certificate does not name. + /// behaviour before #816 (the SDK overload used never checked). Applied to Sign / SignAndEncrypt endpoints only + /// — never passed for , where SDK 2.0.1 would check the GetEndpoints + /// certificate at channel creation anyway. Leave it off behind NAT / a port publish / when connecting by an IP + /// the certificate does not name / for a zone-qualified IPv6 host (the SDK compares the zone too). /// public bool ValidateServerHostName { get; set; } diff --git a/src/Client/ZB.MOM.WW.OtOpcUa.Client.Shared/OpcUaClientService.cs b/src/Client/ZB.MOM.WW.OtOpcUa.Client.Shared/OpcUaClientService.cs index 012a3c261..8ea477560 100644 --- a/src/Client/ZB.MOM.WW.OtOpcUa.Client.Shared/OpcUaClientService.cs +++ b/src/Client/ZB.MOM.WW.OtOpcUa.Client.Shared/OpcUaClientService.cs @@ -794,8 +794,12 @@ public sealed class OpcUaClientService : IOpcUaClientService : new UserIdentity(); var sessionTimeoutMs = (uint)(settings.SessionTimeoutSeconds * 1000); + // #816: never on a None endpoint — SDK 2.0.1 also checks the host at channel creation against the + // GetEndpoints certificate, before any security-mode test, and on an unauthenticated channel that proves + // nothing (same rule as OpcUaEndpointSelection.CheckDomainFor for the driver and the AdminUI browser). + var checkDomain = settings.ValidateServerHostName && endpoint.SecurityMode != MessageSecurityMode.None; return await _sessionFactory.CreateSessionAsync(config, endpoint, "OtOpcUaClient", sessionTimeoutMs, identity, - settings.ValidateServerHostName, ct); + checkDomain, ct); } catch { 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 f80b054bf..cb318298c 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 @@ -115,8 +115,10 @@ public sealed class OpcUaClientDriverBrowser : IDriverBrowser configuredEndpoint, updateBeforeConnect: false, // Gitea #816: the same host-name policy as the deployed driver (the SDK's overload without this - // argument hard-codes false), checked against the configured host the endpoint was rewritten to. - checkDomain: opts.CertificateValidation.EffectiveValidateServerHostName, + // argument hard-codes false), checked against the configured host the endpoint was rewritten to — + // and, like the driver, never on a None endpoint. + checkDomain: OpcUaEndpointSelection.CheckDomainFor( + endpointDesc, opts.CertificateValidation.EffectiveValidateServerHostName), sessionName: "OtOpcUa AdminUI Browse", (uint)opts.SessionTimeout.TotalMilliseconds, identity, diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Contracts/OpcUaClientDriverOptions.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Contracts/OpcUaClientDriverOptions.cs index ea9824cc0..df2c4ca34 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Contracts/OpcUaClientDriverOptions.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Contracts/OpcUaClientDriverOptions.cs @@ -251,9 +251,12 @@ public sealed record OpcUaCertificateValidationOptions /// false = explicit opt-out — the NAT / port-publish / IP-addressed case, where the server's /// certificate names its private host and the reachable address is something else. /// - /// Only applies to secured sessions (a None channel validates no server certificate at all), and - /// accepts a host-name mismatch like any other - /// certificate error. + /// Only applies to Sign / SignAndEncrypt endpoints: the driver and browser never pass the SDK's + /// checkDomain for a None endpoint, because SDK 2.0.1 would otherwise check at channel creation + /// against the GetEndpoints certificate even there. + /// accepts a host-name mismatch like any other certificate error. A zone-qualified IPv6 host + /// ([fe80::1%eth0]) never matches (the SDK compares the host WITH its zone against the certificate's + /// zone-less IP SANs), so a link-local endpoint needs false. ///
public bool? ValidateServerHostName { get; init; } 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 index f29898288..8b2d2fe70 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Session/OpcUaEndpointSelection.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Session/OpcUaEndpointSelection.cs @@ -46,6 +46,23 @@ public static class OpcUaEndpointSelection return match; } + /// + /// Gitea #816 — the checkDomain to hand the SDK for : the operator's choice, but + /// never on a endpoint. SDK 2.0.1 checks the host in TWO places — in + /// DefaultSessionFactory.CreateChannelAsync, against the certificate in the GetEndpoints reply, BEFORE any + /// security-mode test (opc-plc sends one even on its None endpoint), and in Session.OpenAsync once the + /// chain is trusted — so passing true on a None session refused it on a certificate the channel never + /// uses. A host-name check on an unauthenticated channel proves nothing. + /// + /// The endpoint the session will open on. + /// The configured (effective) host-name policy. + /// The checkDomain argument. + public static bool CheckDomainFor(EndpointDescription selected, bool validateServerHostName) + { + ArgumentNullException.ThrowIfNull(selected); + return validateServerHostName && selected.SecurityMode != MessageSecurityMode.None; + } + /// Convert a driver to the OPC UA policy URI. /// The driver security policy to map. /// The corresponding OPC UA security policy URI. 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 c39baf985..acba83198 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDriver.cs @@ -1061,12 +1061,14 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit // Gitea #816: the SDK's 8-argument CreateAsync hard-codes checkDomain:false, so before #816 the server // certificate's host name was never checked. The check (when enabled) runs against endpoint.EndpointUrl — - // the CONFIGURED host, because SelectMatching rewrote the advertised one. + // the CONFIGURED host, because SelectMatching rewrote the advertised one — and never on a None endpoint + // (CheckDomainFor; the SDK would otherwise check at channel creation even there). var session = await new DefaultSessionFactory(_sdkTelemetry).CreateAsync( appConfig, endpoint, false, // updateBeforeConnect - _options.CertificateValidation.EffectiveValidateServerHostName, // checkDomain (#816) + OpcUaEndpointSelection.CheckDomainFor( // checkDomain (#816) + selected, _options.CertificateValidation.EffectiveValidateServerHostName), _options.SessionName, (uint)_options.SessionTimeout.TotalMilliseconds, identity, diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/OpcUaClientDriverForm.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/OpcUaClientDriverForm.razor index d080f2dc9..115cb5eb7 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/OpcUaClientDriverForm.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/OpcUaClientDriverForm.razor @@ -199,7 +199,7 @@ -
Secured sessions only: the server certificate must name the endpoint host you configured.
+
Sign / SignAndEncrypt only (never on None): the server certificate must name the endpoint host you configured. Off for link-local IPv6.
diff --git a/tests/Client/ZB.MOM.WW.OtOpcUa.Client.Shared.Tests/OpcUaClientServiceTests.cs b/tests/Client/ZB.MOM.WW.OtOpcUa.Client.Shared.Tests/OpcUaClientServiceTests.cs index 104f20fbf..c021b3966 100644 --- a/tests/Client/ZB.MOM.WW.OtOpcUa.Client.Shared.Tests/OpcUaClientServiceTests.cs +++ b/tests/Client/ZB.MOM.WW.OtOpcUa.Client.Shared.Tests/OpcUaClientServiceTests.cs @@ -56,21 +56,28 @@ public class OpcUaClientServiceTests : IDisposable } /// - /// Gitea #816: the host-name check reaches the session factory (the SDK's checkDomain) — off by default, the - /// pre-#816 behaviour — and survives the per-endpoint settings copy the connect path makes. + /// Gitea #816: the host-name check reaches the session factory (the SDK's checkDomain) on a SECURED endpoint — off + /// by default, the pre-#816 behaviour — and survives the per-endpoint settings copy the connect path makes. On a + /// None endpoint it is never passed: SDK 2.0.1 would check the GetEndpoints certificate at channel creation even + /// there (review of #816, reproduced against opc-plc). /// [Theory] - [InlineData(null)] - [InlineData(true)] - [InlineData(false)] - public async Task ConnectAsync_threads_ValidateServerHostName_to_the_session(bool? validate) + [InlineData(SecurityMode.Sign, null, false)] + [InlineData(SecurityMode.Sign, true, true)] + [InlineData(SecurityMode.SignAndEncrypt, true, true)] + [InlineData(SecurityMode.Sign, false, false)] + [InlineData(SecurityMode.None, true, false)] + [InlineData(SecurityMode.None, null, false)] + public async Task ConnectAsync_threads_ValidateServerHostName_to_secured_sessions_only( + SecurityMode mode, bool? validate, bool expectedCheckDomain) { var settings = ValidSettings(); + settings.SecurityMode = mode; if (validate is { } v) settings.ValidateServerHostName = v; await _service.ConnectAsync(settings, TestContext.Current.CancellationToken); - _sessionFactory.LastValidateServerHostName.ShouldBe(validate ?? false); + _sessionFactory.LastValidateServerHostName.ShouldBe(expectedCheckDomain); _configFactory.LastSettings!.ValidateServerHostName.ShouldBe(validate ?? false); } 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 index 5e66a574a..dcb736d38 100644 --- 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 @@ -190,6 +190,21 @@ public sealed class NatPublishedEndpointBrowseTests : IDisposable children.Count.ShouldBeGreaterThan(0); } + /// + /// Review of #816: with the host-name check ON, an unsecured browse still opens although the NAT certificate (sent + /// in the GetEndpoints reply even for the None endpoint) names another host — the check never runs on None. + /// + [Fact] + public async Task Host_name_validation_does_not_apply_to_a_None_browse() + { + var endpoint = RequireFixture(); + var ct = TestContext.Current.CancellationToken; + await AssertAdvertisesUnreachableAddressAsync(endpoint, ct); + + await using var session = await Browser().OpenAsync(Json(endpoint, validateHostName: true), ct); + (await session.RootAsync(ct)).Count.ShouldBeGreaterThan(0); + } + [Fact] public async Task Unreachable_configured_endpoint_still_fails_cleanly() { diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.IntegrationTests/ServerHostNameValidationTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.IntegrationTests/ServerHostNameValidationTests.cs index 1687d7d06..6c84ec382 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.IntegrationTests/ServerHostNameValidationTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.IntegrationTests/ServerHostNameValidationTests.cs @@ -89,6 +89,27 @@ public sealed class ServerHostNameValidationTests(OpcPlcFixture sim) : IDisposab snapshots[0].StatusCode.ShouldBe(0u, "the secured session must read once the server certificate is trusted"); } + /// + /// Review of #816 (reproduced live): SDK 2.0.1 also checks the host at channel creation, against the certificate + /// in the GetEndpoints reply — which opc-plc sends on its None endpoint too — before any security-mode test. The + /// driver must not pass checkDomain for a None session, or the flag refuses an unsecured connect. + /// + [Fact] + public async Task With_host_name_validation_on_a_None_session_still_connects() + { + if (sim.SkipReason is not null) Assert.Skip(sim.SkipReason); + var ct = TestContext.Current.CancellationToken; + var options = OpcPlcProfile.BuildOptions(sim.EndpointUrl) with + { + AutoAcceptCertificates = false, + CertificateValidation = new OpcUaCertificateValidationOptions { ValidateServerHostName = true }, + }; + + await using var drv = new OpcUaClientDriver(options, driverInstanceId: "opcua-816-none") { PkiRootOverride = _pkiRoot }; + await drv.InitializeAsync("{}", ct); + (await drv.ReadAsync([OpcPlcProfile.StepUpRef], ct))[0].StatusCode.ShouldBe(0u); + } + [Fact] public async Task With_host_name_validation_the_certificate_is_checked_against_the_configured_host() { diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests/OpcUaClientServerHostNameOptionTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests/OpcUaClientServerHostNameOptionTests.cs index a4c27f1b7..8208ab004 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests/OpcUaClientServerHostNameOptionTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests/OpcUaClientServerHostNameOptionTests.cs @@ -43,6 +43,21 @@ public sealed class OpcUaClientServerHostNameOptionTests json.ShouldNotContain("Effective"); } + /// + /// Review of #816: SDK 2.0.1 also checks the host in DefaultSessionFactory.CreateChannelAsync, against the + /// GetEndpoints certificate and before any security-mode test, so checkDomain is never passed for a None endpoint. + /// + [Theory] + [InlineData(MessageSecurityMode.None, true, false)] + [InlineData(MessageSecurityMode.None, false, false)] + [InlineData(MessageSecurityMode.Sign, true, true)] + [InlineData(MessageSecurityMode.SignAndEncrypt, true, true)] + [InlineData(MessageSecurityMode.SignAndEncrypt, false, false)] + public void CheckDomain_is_passed_only_for_secured_endpoints(MessageSecurityMode mode, bool validate, bool expected) + { + OpcUaEndpointSelection.CheckDomainFor(new EndpointDescription { SecurityMode = mode }, validate).ShouldBe(expected); + } + [Fact] public void A_host_name_refusal_names_the_opt_out() {