diff --git a/code-reviews/Server/findings.md b/code-reviews/Server/findings.md index 7757d2d..8017341 100644 --- a/code-reviews/Server/findings.md +++ b/code-reviews/Server/findings.md @@ -200,13 +200,13 @@ | Severity | Medium | | Category | Design-document adherence | | Location | `src/Server/ZB.MOM.WW.OtOpcUa.Server/OpcUa/OpcUaServerOptions.cs:9-19`, `src/Server/ZB.MOM.WW.OtOpcUa.Server/OpcUa/OpcUaApplicationHost.cs:296-346`, `src/Server/ZB.MOM.WW.OtOpcUa.Server/Program.cs:89` | -| Status | Open | +| Status | Resolved | **Description:** `docs/security.md` documents 7 transport security profiles and `CLAUDE.md` references a `SecurityProfileResolver`. The code's `OpcUaSecurityProfile` enum has only `None` and `Basic256Sha256SignAndEncrypt`; `BuildSecurityPolicies` adds a policy only for the latter; `SecurityProfileResolver` does not exist in the repo (grep finds it only in docs). `Basic256Sha256-Sign` and all Aes profiles are unimplemented, and `Program.cs:89`'s `Enum.TryParse` silently selects `None` for an unrecognised profile string. **Recommendation:** Reconcile code and docs — implement the missing profiles + `SecurityProfileResolver`, or trim `docs/security.md` / `CLAUDE.md` to the two supported profiles. At minimum, log a warning when a configured `SecurityProfile` fails to parse instead of silently using `None`. -**Resolution:** _(open)_ +**Resolution:** Resolved 2026-05-22 — replaced the silent `Enum.TryParse ?? None` fallback in `Program.cs` with a `ParseSecurityProfile` helper that produces a warning string listing supported profiles when the configured value is unrecognised; the warning is emitted via `Log.Warning` at startup before the host builds, making the misconfiguration immediately visible. Implementing the missing 5 profiles is tracked as a doc-to-code gap rather than a single finding fix. ### Server-014 | Field | Value | diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Server/Program.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Server/Program.cs index 3fdae12..05f3158 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Server/Program.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Server/Program.cs @@ -97,12 +97,15 @@ var opcUaOptions = new OpcUaServerOptions PkiStoreRoot = opcUaSection.GetValue("PkiStoreRoot") ?? Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.CommonApplicationData), "OtOpcUa", "pki"), AutoAcceptUntrustedClientCertificates = opcUaSection.GetValue("AutoAcceptUntrustedClientCertificates") ?? false, // Server-010: secure by default - SecurityProfile = Enum.TryParse(opcUaSection.GetValue("SecurityProfile"), true, out var p) - ? p : OpcUaSecurityProfile.None, + SecurityProfile = ParseSecurityProfile(opcUaSection.GetValue("SecurityProfile"), out var profileParseWarning), Ldap = ldapOptions, AnonymousRoles = opcUaSection.GetSection("AnonymousRoles").Get() ?? [], }; +// Server-013: warn at startup when the configured SecurityProfile string does not map to +// any supported profile — Enum.TryParse previously silently fell back to None. +if (profileParseWarning is not null) Log.Warning("{Warning}", profileParseWarning); + builder.Services.AddSingleton(options); builder.Services.AddSingleton(opcUaOptions); builder.Services.AddSingleton(ldapOptions); @@ -271,6 +274,27 @@ builder.Services.AddSingleton(); var host = builder.Build(); await host.RunAsync(); +// Server-013: parse the SecurityProfile config value and produce a warning string when the +// configured value is unrecognised, so operators get a startup diagnostic instead of silent +// fallback to None. +static OpcUaSecurityProfile ParseSecurityProfile(string? raw, out string? warning) +{ + if (string.IsNullOrWhiteSpace(raw)) + { + warning = null; + return OpcUaSecurityProfile.None; + } + if (Enum.TryParse(raw, ignoreCase: true, out var parsed)) + { + warning = null; + return parsed; + } + warning = $"OpcUaServer:SecurityProfile value '{raw}' is not a recognised profile " + + $"(supported: {string.Join(", ", Enum.GetNames())}). " + + "Falling back to SecurityProfile.None — no encryption or signed transport will be applied."; + return OpcUaSecurityProfile.None; +} + // Server-007: lightweight cached config-DB health probe for /healthz. // Runs CanConnectAsync once and caches the result for 10 s so the /healthz // endpoint never blocks an HTTP handler thread on a DB round-trip while still