From a83bf1fa274baa8ab9c0e98881ebcc0549d9ed07 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 12 Nov 2026 11:38:41 -0500 Subject: [PATCH] fix(opcua): a configured OpcUa:EnabledSecurityProfiles list replaces the default, so None can be dropped (#879) ConfigurationBinder appends configured collection items to an existing list, so the pre-filled [None, Sign, SignAndEncrypt] default made "drop None" a no-op: every deployment that listed only the secure profiles still advertised an unsigned/unencrypted None endpoint. EnabledSecurityProfiles is now null by default; ResolveEnabledSecurityProfiles() applies the unchanged three-profile default only when nothing is configured. A set-but-empty list fails startup validation and BuildSecurityPolicies no longer falls back to a None endpoint on empty input (it throws). Sweep of all options classes found no other non-empty pre-filled collection default. Tests: options binding (indexed, JSON, env forms, default, empty, bad name), a booted-server wire test (no None endpoint, None connection refused with BadSecurityPolicyRejected, secure connection works; default case is the control). Claude-Session: https://claude.ai/code/session_01GT67ACmpMR7b9LGRPLijMi --- CLAUDE.md | 2 +- docs/Configuration.md | 2 +- docs/OpcUaServer.md | 2 +- docs/security.md | 4 +- .../BindRepro/Program.cs | 2 +- .../opcua-windows-service/README.md | 2 +- .../OpcUaApplicationHostOptionsValidator.cs | 8 +- .../OpcUaApplicationHost.cs | 59 +++++-- .../OpcUaSecurityProfilesBindingTests.cs | 156 ++++++++++++++++++ .../SecurityProfilesReplaceWireTests.cs | 153 +++++++++++++++++ .../OpcUaApplicationHostSecurityTests.cs | 13 +- 11 files changed, 372 insertions(+), 31 deletions(-) create mode 100644 tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/OpcUaSecurityProfilesBindingTests.cs create mode 100644 tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/SecurityProfilesReplaceWireTests.cs diff --git a/CLAUDE.md b/CLAUDE.md index 201d68593..4a1cec3bb 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -348,7 +348,7 @@ See `docs/v2/dev-environment.md` for the full inventory and rationale. ## Transport Security -The server supports configurable OPC UA transport security via the `OpcUa:EnabledSecurityProfiles` list in `appsettings.json`. Phase 1 profiles (the `OpcUaSecurityProfile` enum members): `None` (default), `Basic256Sha256Sign`, `Basic256Sha256SignAndEncrypt`. Security policies are built from the enabled profiles by `BuildSecurityPolicies` at startup. The server certificate is always created even for `None`-only deployments because `UserName` token encryption depends on it. See `docs/security.md` for the full guide. +The server supports configurable OPC UA transport security via the `OpcUa:EnabledSecurityProfiles` list in `appsettings.json`. Phase 1 profiles (the `OpcUaSecurityProfile` enum members): `None`, `Basic256Sha256Sign`, `Basic256Sha256SignAndEncrypt`. The default (key absent) is all three; a configured list REPLACES it (lmxopcua#879 — the property is null by default and `ResolveEnabledSecurityProfiles()` applies the default, because `ConfigurationBinder` appends to a pre-filled list); a set-but-empty list fails validation. Security policies are built from the enabled profiles by `BuildSecurityPolicies` at startup. The server certificate is always created even for `None`-only deployments because `UserName` token encryption depends on it. See `docs/security.md` for the full guide. ## Library Preferences diff --git a/docs/Configuration.md b/docs/Configuration.md index c6e49f3ea..d80c4ca15 100644 --- a/docs/Configuration.md +++ b/docs/Configuration.md @@ -71,7 +71,7 @@ Until #678.6 the overlay never loaded on a node carrying a `cluster-*` role (the | `PublicHostname` | string | `0.0.0.0` | Hostname/IP advertised in endpoint descriptions. Required. | | `ApplicationConfigPath` | string? | `null` | Optional path to an application config XML; loaded instead of building from defaults. | | `PkiStoreRoot` | string | `pki` | Root of the PKI hierarchy (`own`/`issuer`/`trusted`/`rejected` substores created under it). Required. See [`security.md`](security.md). | -| `EnabledSecurityProfiles` | list of `OpcUaSecurityProfile` | `[None, Basic256Sha256Sign, Basic256Sha256SignAndEncrypt]` | Transport-security profiles, one endpoint per entry. Must contain ≥1. Profile detail in [`security.md`](security.md). | +| `EnabledSecurityProfiles` | list of `OpcUaSecurityProfile` | _(absent)_ = `[None, Basic256Sha256Sign, Basic256Sha256SignAndEncrypt]` | Transport-security profiles, one endpoint per entry. **A configured list replaces the default** (#879); the default applies only when the key is absent. A set-but-empty list fails startup validation. Profile detail in [`security.md`](security.md). | | `AutoAcceptUntrustedClientCertificates` | bool | `false` | Auto-trust unknown client certs on first connect (dev convenience). Not validated. See [`security.md`](security.md). | | `PeerApplicationUris` | list of string | `[]` (empty) | Partner node `ApplicationUri`s published in `Server.ServerArray` for redundancy discovery. See [`Redundancy.md`](Redundancy.md). | | `DiscoveryServerUrl` | string? | `null` | OPC UA discovery server (LDS/GDS) to register with via `RegisterServer2`, e.g. `opc.tcp://lds-host:4840`. **Unset = no registration** (#876 — the SDK default registered with `opc.tcp://localhost:4840`, i.e. the server itself, and logged `BadServiceUnsupported` forever). Must be an absolute `opc.tcp://` URL when set (validated). Ignored when `ApplicationConfigPath` is set (the XML's own `RegistrationEndpoint`/`MaxRegistrationInterval` apply). | diff --git a/docs/OpcUaServer.md b/docs/OpcUaServer.md index 29d17e817..f6872d862 100644 --- a/docs/OpcUaServer.md +++ b/docs/OpcUaServer.md @@ -49,7 +49,7 @@ Environmental knobs that aren't per-tenant — bind address, port, PKI store roo ## Transport -The server binds a TCP endpoint at `opc.tcp://{PublicHostname}:{OpcUaPort}/OtOpcUa` (defaults `0.0.0.0:4840`). The `ApplicationConfiguration` is built programmatically in `OpcUaApplicationHost.BuildConfigurationAsync` — there are no UA XML files unless `ApplicationConfigPath` is set. Security profiles are listed in `OpcUaApplicationHostOptions.EnabledSecurityProfiles`; by default all three baseline profiles are exposed (`None`, `Basic256Sha256` + Sign, `Basic256Sha256` + SignAndEncrypt) and the SDK publishes one endpoint descriptor per profile. Production deployments typically drop `None`. User token policies (`Anonymous`, `UserName`) are always attached; the `UserName` policy is SDK-encrypted with the server certificate so it works on `None` endpoints too. See `docs/security.md` for hardening. +The server binds a TCP endpoint at `opc.tcp://{PublicHostname}:{OpcUaPort}/OtOpcUa` (defaults `0.0.0.0:4840`). The `ApplicationConfiguration` is built programmatically in `OpcUaApplicationHost.BuildConfigurationAsync` — there are no UA XML files unless `ApplicationConfigPath` is set. Security profiles are listed in `OpcUaApplicationHostOptions.EnabledSecurityProfiles`; by default (key absent) all three baseline profiles are exposed (`None`, `Basic256Sha256` + Sign, `Basic256Sha256` + SignAndEncrypt) and the SDK publishes one endpoint descriptor per profile. A configured list replaces that default (lmxopcua#879). Production deployments typically drop `None`. User token policies (`Anonymous`, `UserName`) are always attached; the `UserName` policy is SDK-encrypted with the server certificate so it works on `None` endpoints too. See `docs/security.md` for hardening. ## Session impersonation diff --git a/docs/security.md b/docs/security.md index 3502370b5..e32ce01dd 100644 --- a/docs/security.md +++ b/docs/security.md @@ -88,7 +88,7 @@ The profiles are the members of the `OpcUaSecurityProfile` enum (`src/Server/ZB. `BuildSecurityPolicies` (`OpcUaApplicationHost.cs`) maps each configured profile to an SDK `ServerSecurityPolicy`. The server exposes a separate endpoint per configured profile and clients select the one they prefer at session open. The enum's XML doc notes that Aes128/Aes256 variants can be added later by extending the enum + `BuildSecurityPolicies` — the wiring is profile-agnostic — but they are **not implemented today**. There is no `SecurityProfileResolver` class. -> **Config value form.** The enum binds by member name, so a profile string with hyphens (e.g. `Basic256Sha256-Sign`) does **not** bind — use the exact enum-member spelling above. If `EnabledSecurityProfiles` is empty, the server falls back to a single `None` endpoint (logged, very visible) so it still has a listening endpoint. +> **Config value form.** The enum binds by member name, so a profile string with hyphens (e.g. `Basic256Sha256-Sign`) does **not** bind — use the exact enum-member spelling above. An empty profile set is never accepted: startup validation rejects a set-but-empty list and `BuildSecurityPolicies` throws on empty input — the server no longer falls back to a `None` endpoint (lmxopcua#879). ### Configuration @@ -108,7 +108,7 @@ Transport security is configured in the `OpcUa` section of the Host process's bo } ``` -`EnabledSecurityProfiles` is a **list** — the server publishes one endpoint per entry. The default (when the key is omitted) is all three baseline profiles (`None`, `Basic256Sha256Sign`, `Basic256Sha256SignAndEncrypt`); production deployments typically drop `None`. The list must contain at least one entry (`OpcUaApplicationHostOptionsValidator` enforces `MinCount(…, 1)`). +`EnabledSecurityProfiles` is a **list** — the server publishes one endpoint per entry. **A configured list replaces the default; it is never merged with it** (lmxopcua#879 — before the fix the binder appended your entries to a pre-filled `[None, Sign, SignAndEncrypt]`, so `None` could not be dropped; any deployment that "dropped `None`" on an older build still advertised it). The default applies only when the key is absent (or an empty `[]`, which binds to nothing): all three baseline profiles (`None`, `Basic256Sha256Sign`, `Basic256Sha256SignAndEncrypt`). A list that is set but empty (reachable only from code) is a configuration error — `OpcUaApplicationHostOptionsValidator` fails startup — and never means "None only" or "everything". An unrecognised profile name fails the bind rather than being ignored. The same holds for the environment-variable form (`OpcUa__EnabledSecurityProfiles__0=…`). Verify with a `GetEndpoints` probe after changing it. The server certificate is auto-generated on first start if none exists in `PkiStoreRoot/own/`. Always generated even for `None`-only deployments because UserName token encryption depends on it. diff --git a/scripts/validation/opcua-windows-service/BindRepro/Program.cs b/scripts/validation/opcua-windows-service/BindRepro/Program.cs index 0e70aa9f1..470ce87f5 100644 --- a/scripts/validation/opcua-windows-service/BindRepro/Program.cs +++ b/scripts/validation/opcua-windows-service/BindRepro/Program.cs @@ -8,4 +8,4 @@ var config = new ConfigurationBuilder().AddInMemoryCollection(new Dictionary — that interface does not derive from - // IReadOnlyCollection, so it can't bind to MinCount's IReadOnlyCollection parameter - // directly. ToList() bridges to the shared primitive while preserving the count (and message). - builder.MinCount(o.EnabledSecurityProfiles?.ToList(), 1, "OpcUa:EnabledSecurityProfiles"); + // IReadOnlyCollection, so ToList() bridges to MinCount's parameter (count + message preserved). + if (o.EnabledSecurityProfiles is not null) + builder.MinCount(o.EnabledSecurityProfiles.ToList(), 1, "OpcUa:EnabledSecurityProfiles"); // #876: discovery registration is opt-in; when configured, the URL must be an absolute opc.tcp URI and the // interval positive (0 would silently disable the registration that was asked for). diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OpcUaApplicationHost.cs b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OpcUaApplicationHost.cs index 7d78429c4..893462bad 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OpcUaApplicationHost.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OpcUaApplicationHost.cs @@ -79,18 +79,54 @@ public sealed class OpcUaApplicationHostOptions public bool AllowAnonymous { get; set; } = true; /// - /// Transport-security profiles exposed by the server. The SDK publishes one endpoint - /// descriptor per profile and clients choose at session open. Default = all three - /// baseline profiles (None + Basic256Sha256 in both modes); production deployments - /// typically drop None. + /// Transport-security profiles exposed by the server (OpcUa:EnabledSecurityProfiles); the SDK + /// publishes one endpoint descriptor per profile and clients choose at session open. /// - public IList EnabledSecurityProfiles { get; set; } = new List + /// + /// + /// Replace semantics (lmxopcua#879). This is by default and the + /// three-profile default (: None + + /// Basic256Sha256Sign + Basic256Sha256SignAndEncrypt) is applied by + /// only when nothing was configured. It must NOT be + /// pre-filled: ConfigurationBinder appends configured collection items to an existing list + /// instead of replacing it, so a pre-filled default made it impossible to drop None. + /// + /// + /// Empty list. A key that is absent, or an array with no items ([] binds to nothing), + /// means "not configured" and yields the default. A list that is non-null but empty (only + /// reachable from code, or an env value the binder materialises as an empty list) is a + /// configuration error: OpcUaApplicationHostOptionsValidator fails startup and + /// throws. It never falls back to None. + /// + /// + public IList? EnabledSecurityProfiles { get; set; } + + /// + /// The profiles applied when is not configured — all three + /// baseline profiles, unchanged from before lmxopcua#879. Production deployments configure the list + /// explicitly (without None) to drop the unsecured endpoint. + /// + public static IReadOnlyList DefaultSecurityProfiles { get; } = new[] { OpcUaSecurityProfile.None, OpcUaSecurityProfile.Basic256Sha256Sign, OpcUaSecurityProfile.Basic256Sha256SignAndEncrypt, }; + /// + /// The profiles the server actually publishes: the configured list when one was given, otherwise + /// . + /// + /// The list was set but is empty. + public IReadOnlyList ResolveEnabledSecurityProfiles() + { + if (EnabledSecurityProfiles is null) return DefaultSecurityProfiles; + if (EnabledSecurityProfiles.Count == 0) + throw new InvalidOperationException( + "OpcUa:EnabledSecurityProfiles is set but empty; list at least one profile or omit the key to use the default."); + return EnabledSecurityProfiles.ToList(); + } + /// /// When true, unknown client certificates are auto-added to the trusted store on first /// connection. Convenient for dev; should be false in production (operators promote via @@ -635,7 +671,7 @@ public sealed class OpcUaApplicationHost : IAsyncDisposable DisableSdkAuthenticationLockout(serverConfig); ApplyDiscoveryRegistration(serverConfig, _options); - serverConfig.SecurityPolicies = BuildSecurityPolicies(_options.EnabledSecurityProfiles).ToArray(); + serverConfig.SecurityPolicies = BuildSecurityPolicies(_options.ResolveEnabledSecurityProfiles()).ToArray(); serverConfig.UserTokenPolicies = BuildUserTokenPolicies(_options.AllowAnonymous).ToArray(); var config = new ApplicationConfiguration(_telemetry) @@ -779,14 +815,11 @@ public sealed class OpcUaApplicationHost : IAsyncDisposable _ => throw new InvalidOperationException($"Unknown OpcUaSecurityProfile: {profile}"), }; } + // lmxopcua#879: an empty profile list used to fall back to a single None endpoint. A misconfiguration must + // never silently widen the transport to unsecured, so it is an error (the host validator rejects it first). if (!any) - { - yield return new ServerSecurityPolicy - { - SecurityMode = MessageSecurityMode.None, - SecurityPolicyUri = SecurityPolicies.None, - }; - } + throw new InvalidOperationException( + "No OPC UA security profile is enabled; OpcUa:EnabledSecurityProfiles must list at least one."); } /// diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/OpcUaSecurityProfilesBindingTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/OpcUaSecurityProfilesBindingTests.cs new file mode 100644 index 000000000..a8f6a0b46 --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/OpcUaSecurityProfilesBindingTests.cs @@ -0,0 +1,156 @@ +using System.Text; +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Options; +using Shouldly; +using Xunit; +using ZB.MOM.WW.Configuration; +using ZB.MOM.WW.OtOpcUa.Host.Configuration; +using ZB.MOM.WW.OtOpcUa.OpcUaServer; + +namespace ZB.MOM.WW.OtOpcUa.Host.IntegrationTests; + +/// +/// lmxopcua#879 — ConfigurationBinder appends configured collection items to a pre-filled list, so +/// OpcUa:EnabledSecurityProfiles could never drop None. These tests pin "the configured list REPLACES +/// the default" through the same AddValidatedOptions wiring Program.cs uses, for the indexed +/// (env-var / in-memory) form and the JSON array form, plus the unchanged default and the empty-list rule. +/// +public sealed class OpcUaSecurityProfilesBindingTests +{ + private static readonly OpcUaSecurityProfile[] AllThree = + [ + OpcUaSecurityProfile.None, + OpcUaSecurityProfile.Basic256Sha256Sign, + OpcUaSecurityProfile.Basic256Sha256SignAndEncrypt, + ]; + + private static IOptions Resolve(IConfiguration configuration, out ServiceProvider provider) + { + var services = new ServiceCollection(); + services.AddSingleton(configuration); + services.AddValidatedOptions(configuration, "OpcUa"); + provider = services.BuildServiceProvider(); + return provider.GetRequiredService>(); + } + + /// The #879 repro: only the secure profiles configured gives exactly those, no None. + [Fact] + public void Configured_secure_profiles_replace_the_default_and_drop_None() + { + var configuration = new ConfigurationBuilder().AddInMemoryCollection(new Dictionary + { + ["OpcUa:EnabledSecurityProfiles:0"] = "Basic256Sha256SignAndEncrypt", + ["OpcUa:EnabledSecurityProfiles:1"] = "Basic256Sha256Sign", + }).Build(); + + var options = Resolve(configuration, out var provider); + using var sp = provider; + + options.Value.ResolveEnabledSecurityProfiles().ShouldBe( + [OpcUaSecurityProfile.Basic256Sha256SignAndEncrypt, OpcUaSecurityProfile.Basic256Sha256Sign]); + options.Value.ResolveEnabledSecurityProfiles().ShouldNotContain(OpcUaSecurityProfile.None); + } + + /// The JSON array form (appsettings.json, as docs/security.md shows) behaves the same as the indexed form. + [Fact] + public void Json_array_form_replaces_the_default_too() + { + var json = """{ "OpcUa": { "EnabledSecurityProfiles": [ "Basic256Sha256Sign", "Basic256Sha256SignAndEncrypt" ] } }"""; + var configuration = new ConfigurationBuilder() + .AddJsonStream(new MemoryStream(Encoding.UTF8.GetBytes(json))).Build(); + + var options = Resolve(configuration, out var provider); + using var sp = provider; + + options.Value.ResolveEnabledSecurityProfiles().ShouldBe( + [OpcUaSecurityProfile.Basic256Sha256Sign, OpcUaSecurityProfile.Basic256Sha256SignAndEncrypt]); + } + + /// The environment-variable form (OpcUa__EnabledSecurityProfiles__0) used by the Windows service gate. + [Fact] + public void Environment_variable_form_replaces_the_default() + { + const string prefix = "OTOPCUA_879_TEST_"; + Environment.SetEnvironmentVariable(prefix + "OpcUa__EnabledSecurityProfiles__0", "Basic256Sha256SignAndEncrypt"); + try + { + var configuration = new ConfigurationBuilder().AddEnvironmentVariables(prefix).Build(); + var options = Resolve(configuration, out var provider); + using var sp = provider; + + options.Value.ResolveEnabledSecurityProfiles().ShouldBe([OpcUaSecurityProfile.Basic256Sha256SignAndEncrypt]); + } + finally { Environment.SetEnvironmentVariable(prefix + "OpcUa__EnabledSecurityProfiles__0", null); } + } + + /// Configuring None explicitly alongside the secure profiles still works (an operator may want it). + [Fact] + public void Explicit_None_is_honoured_when_listed() + { + var configuration = new ConfigurationBuilder().AddInMemoryCollection(new Dictionary + { + ["OpcUa:EnabledSecurityProfiles:0"] = "None", + }).Build(); + + var options = Resolve(configuration, out var provider); + using var sp = provider; + + options.Value.ResolveEnabledSecurityProfiles().ShouldBe([OpcUaSecurityProfile.None]); + } + + /// Nothing configured: the three-profile default is applied, unchanged from before the fix. + [Fact] + public void Nothing_configured_keeps_the_three_profile_default() + { + var configuration = new ConfigurationBuilder().AddInMemoryCollection(new Dictionary + { + ["OpcUa:OpcUaPort"] = "4841", // a section exists, the list key does not + }).Build(); + + var options = Resolve(configuration, out var provider); + using var sp = provider; + + options.Value.ResolveEnabledSecurityProfiles().ShouldBe(AllThree); + new OpcUaApplicationHostOptions().ResolveEnabledSecurityProfiles().ShouldBe(AllThree); + } + + /// An empty JSON array binds to nothing, i.e. "not configured": the default, never "None only". + [Fact] + public void Empty_json_array_is_treated_as_not_configured_and_yields_the_default() + { + var json = """{ "OpcUa": { "EnabledSecurityProfiles": [] } }"""; + var configuration = new ConfigurationBuilder() + .AddJsonStream(new MemoryStream(Encoding.UTF8.GetBytes(json))).Build(); + + var options = Resolve(configuration, out var provider); + using var sp = provider; + + options.Value.ResolveEnabledSecurityProfiles().ShouldBe(AllThree); + } + + /// A list that is set but empty fails startup validation and cannot be resolved — it never means "None". + [Fact] + public void An_explicitly_empty_list_fails_validation_and_resolution() + { + var empty = new OpcUaApplicationHostOptions { EnabledSecurityProfiles = new List() }; + + new OpcUaApplicationHostOptionsValidator().Validate(null, empty).Failed.ShouldBeTrue(); + Should.Throw(() => empty.ResolveEnabledSecurityProfiles()); + } + + /// An unparseable profile name fails startup rather than being ignored (which would otherwise leave the default). + [Fact] + public void An_unknown_profile_name_fails_options_resolution() + { + var configuration = new ConfigurationBuilder().AddInMemoryCollection(new Dictionary + { + ["OpcUa:EnabledSecurityProfiles:0"] = "Basic256Sha256-SignAndEncrypt", + }).Build(); + + var options = Resolve(configuration, out var provider); + using var sp = provider; + + Should.Throw(() => _ = options.Value.ResolveEnabledSecurityProfiles()); + } +} diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/SecurityProfilesReplaceWireTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/SecurityProfilesReplaceWireTests.cs new file mode 100644 index 000000000..876b1e890 --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/SecurityProfilesReplaceWireTests.cs @@ -0,0 +1,153 @@ +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.Logging.Abstractions; +using Opc.Ua; +using Opc.Ua.Client; +using Opc.Ua.Configuration; +using Shouldly; +using Xunit; + +namespace ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests; + +/// +/// lmxopcua#879 over the wire. The options come from a real ConfigurationBinder bind (the path the Host +/// uses), so a regression to a pre-filled list shows up as an advertised None endpoint that admits a +/// session. The default case is the positive control: the same probes must find and use None there. +/// +public sealed class SecurityProfilesReplaceWireTests +{ + private static CancellationToken Ct => TestContext.Current.CancellationToken; + + [Fact] + public async Task Configuring_only_the_secure_profiles_removes_the_None_endpoint_and_refuses_a_None_connection() + { + await using var rig = await Rig.StartAsync(new Dictionary + { + ["OpcUa:EnabledSecurityProfiles:0"] = "Basic256Sha256Sign", + ["OpcUa:EnabledSecurityProfiles:1"] = "Basic256Sha256SignAndEncrypt", + }, Ct); + + var modes = (await rig.GetEndpointModesAsync(Ct)).Distinct().Order().ToArray(); + modes.ShouldBe([MessageSecurityMode.Sign, MessageSecurityMode.SignAndEncrypt]); + + // Positive control: the secure endpoint really is usable, so the refusal below is about None, not about the rig. + using (var secure = await rig.ConnectAsync(MessageSecurityMode.SignAndEncrypt, Ct)) + secure.Connected.ShouldBeTrue(); + + var refusal = await Should.ThrowAsync(() => rig.ConnectAsync(MessageSecurityMode.None, Ct)); + refusal.StatusCode.ShouldBe(StatusCodes.BadSecurityPolicyRejected); + } + + [Fact] + public async Task Nothing_configured_still_advertises_all_three_and_a_None_connection_is_admitted() + { + await using var rig = await Rig.StartAsync(new Dictionary(), Ct); + + (await rig.GetEndpointModesAsync(Ct)).Distinct().Order().ToArray() + .ShouldBe([MessageSecurityMode.None, MessageSecurityMode.Sign, MessageSecurityMode.SignAndEncrypt]); + using var none = await rig.ConnectAsync(MessageSecurityMode.None, Ct); + none.Connected.ShouldBeTrue(); + } + + private sealed class Rig : IAsyncDisposable + { + private readonly string _pki = TestClientSecurity.AllocatePkiRoot(); + private readonly string _clientPki = TestClientSecurity.AllocatePkiRoot(); + private OpcUaApplicationHost _host = null!; + private int _port; + + public static async Task StartAsync(Dictionary values, CancellationToken ct) + { + var rig = new Rig(); + var section = new ConfigurationBuilder().AddInMemoryCollection(values).Build(); + try + { + (rig._host, _) = await OpcUaHostStart.StartAsync(port => + { + var options = new OpcUaApplicationHostOptions + { + ApplicationName = "OtOpcUa.Sec879", + ApplicationUri = $"urn:OtOpcUa.Sec879:{Guid.NewGuid():N}", + PublicHostname = "127.0.0.1", + PkiStoreRoot = rig._pki, + AutoAcceptUntrustedClientCertificates = true, + }; + section.GetSection("OpcUa").Bind(options); // the real binder, exactly what AddValidatedOptions does + options.OpcUaPort = port; + rig._port = port; + return new OpcUaApplicationHost(options, NullLogger.Instance); + }, () => new OtOpcUaSdkServer(), ct); + return rig; + } + catch { await rig.DisposeAsync(); throw; } + } + + private string Url => $"opc.tcp://127.0.0.1:{_port}/OtOpcUa"; + + private (ApplicationConfiguration Config, ApplicationInstance App)? _client; + + private async Task<(ApplicationConfiguration Config, ApplicationInstance App)> ClientAsync(CancellationToken ct) + { + if (_client is { } cached) return cached; + var security = TestClientSecurity.Build(_clientPki); + security.ApplicationCertificate!.SubjectName = "CN=OtOpcUa.Sec879Client"; + var telemetry = DefaultTelemetry.Create(_ => { }); + var config = new ApplicationConfiguration(telemetry) + { + ApplicationName = "OtOpcUa.Sec879Client", + ApplicationUri = $"urn:OtOpcUa.Sec879Client.{Guid.NewGuid():N}", + ApplicationType = ApplicationType.Client, + SecurityConfiguration = security, + ClientConfiguration = new ClientConfiguration { DefaultSessionTimeout = 60_000 }, + }; + await config.ValidateAsync(ApplicationType.Client, ct); + var app = new ApplicationInstance(telemetry) { ApplicationName = config.ApplicationName, ApplicationType = ApplicationType.Client }; + app.ApplicationConfiguration = config; + await app.CheckApplicationInstanceCertificatesAsync(silent: true, lifeTimeInMonths: null, ct); + _client = (config, app); + return (config, app); + } + + public async Task GetEndpointModesAsync(CancellationToken ct) + { + var (config, _) = await ClientAsync(ct); + using var discovery = await DiscoveryClient.CreateAsync(config, new Uri(Url), DiagnosticsMasks.None, ct); + var endpoints = await discovery.GetEndpointsAsync(default, ct); + return endpoints.ToArray()!.Select(e => e.SecurityMode).ToArray(); + } + + /// + /// Opens an anonymous session on the endpoint of the requested mode. For None the endpoint + /// description is synthesised rather than discovered, so a server that no longer advertises it is + /// probed with exactly what a downgrading client would send. + /// + public async Task ConnectAsync(MessageSecurityMode mode, CancellationToken ct) + { + var (config, _) = await ClientAsync(ct); + var telemetry = DefaultTelemetry.Create(_ => { }); + // Always start from a discovered secure endpoint (its token policies are what the server really sent); + // None is a clone of it with the mode and policy downgraded, i.e. what a downgrading client sends. + var description = await CoreClientUtils.SelectEndpointAsync(config, Url, true, telemetry, ct) + ?? throw new InvalidOperationException("No secure endpoint."); + if (mode == MessageSecurityMode.None) + { + description = (EndpointDescription)description.Clone(); + description.SecurityMode = MessageSecurityMode.None; + description.SecurityPolicyUri = SecurityPolicies.None; + description.ServerCertificate = default; + } + else description.SecurityMode.ShouldBe(mode); + return await new DefaultSessionFactory(telemetry).CreateAsync( + config, new ConfiguredEndpoint(null, description, EndpointConfiguration.Create(config)), + updateBeforeConnect: false, checkDomain: false, sessionName: "Sec879", sessionTimeout: 60_000, + identity: new UserIdentity(new AnonymousIdentityToken()), preferredLocales: default, ct: ct); + } + + public async ValueTask DisposeAsync() + { + if (_client is { } client) await client.App.DisposeAsync(); + if (_host is not null) await _host.DisposeAsync(); + foreach (var dir in new[] { _pki, _clientPki }) + if (Directory.Exists(dir)) Directory.Delete(dir, recursive: true); + } + } +} diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/OpcUaApplicationHostSecurityTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/OpcUaApplicationHostSecurityTests.cs index 55be1cb09..943689fe3 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/OpcUaApplicationHostSecurityTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/OpcUaApplicationHostSecurityTests.cs @@ -61,16 +61,13 @@ public sealed class OpcUaApplicationHostSecurityTests : IDisposable } /// - /// Verifies that BuildSecurityPolicies falls back to None when given empty input. + /// lmxopcua#879: an empty profile list is an error, never a silent fallback to an unsecured None endpoint. /// [Fact] - public void BuildSecurityPolicies_empty_input_falls_back_to_none() + public void BuildSecurityPolicies_empty_input_throws_instead_of_falling_back_to_none() { - var policies = OpcUaApplicationHost.BuildSecurityPolicies(Array.Empty()).ToList(); - - policies.Count.ShouldBe(1); - policies[0].SecurityMode.ShouldBe(MessageSecurityMode.None); - policies[0].SecurityPolicyUri.ShouldBe(SecurityPolicies.None); + Should.Throw( + () => OpcUaApplicationHost.BuildSecurityPolicies(Array.Empty()).ToList()); } /// @@ -118,7 +115,7 @@ public sealed class OpcUaApplicationHostSecurityTests : IDisposable OpcUaPort = port, PublicHostname = "localhost", PkiStoreRoot = _pkiRoot, - EnabledSecurityProfiles = + EnabledSecurityProfiles = new List { OpcUaSecurityProfile.None, OpcUaSecurityProfile.Basic256Sha256Sign,