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 000000000..a06fb8e86 Binary files /dev/null and b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.IntegrationTests/Docker/nat/opc-plc-nat-server.der differ 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