diff --git a/code-reviews/Server/findings.md b/code-reviews/Server/findings.md index 34a0209..7757d2d 100644 --- a/code-reviews/Server/findings.md +++ b/code-reviews/Server/findings.md @@ -172,13 +172,13 @@ | Severity | Medium | | Category | Security | | Location | `src/Server/ZB.MOM.WW.OtOpcUa.Server/OpcUa/OpcUaApplicationHost.cs:322-346` | -| Status | Open | +| Status | Resolved | **Description:** `BuildUserTokenPolicies` advertises a `UserName` token policy only when `SecurityProfile == Basic256Sha256SignAndEncrypt && Ldap.Enabled`. With the default `SecurityProfile = None` and `Ldap.Enabled = true`, the LDAP authenticator is wired but no UserName policy is advertised — clients cannot present credentials; the only path in is Anonymous. The operator's intent is silently not honoured, with no diagnostic. **Recommendation:** Validate config at startup and warn/fail when `Ldap.Enabled = true` but no UserName policy is advertised. Allow UserName tokens on any non-None profile (they are stack-encrypted regardless, per `docs/security.md`). -**Resolution:** _(open)_ +**Resolution:** Resolved 2026-05-22 — `BuildUserTokenPolicies` now advertises a `UserName` token policy whenever `Ldap.Enabled && SecurityProfile != None` (previously required `== Basic256Sha256SignAndEncrypt`); `StartAsync` logs a `LogWarning` at startup when `Ldap.Enabled = true` but `SecurityProfile = None`, surfacing the misconfiguration before clients connect. ### Server-012 | Field | Value | diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Server/OpcUa/OpcUaApplicationHost.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Server/OpcUa/OpcUaApplicationHost.cs index fcef9c2..6c59657 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Server/OpcUa/OpcUaApplicationHost.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Server/OpcUa/OpcUaApplicationHost.cs @@ -156,6 +156,15 @@ public sealed class OpcUaApplicationHost : IAsyncDisposable /// public async Task StartAsync(CancellationToken ct) { + // Server-011: warn when LDAP is enabled but the security profile would prevent clients + // from presenting UserName tokens (None profile with UserName policy omitted means the + // operator intent is silently not honoured). + if (_options.Ldap.Enabled && _options.SecurityProfile == OpcUaSecurityProfile.None) + _logger.LogWarning( + "LDAP authentication is enabled but SecurityProfile is None — no UserName token " + + "policy will be advertised and clients cannot present credentials. " + + "Set SecurityProfile to Basic256Sha256SignAndEncrypt to enable LDAP login."); + _application = new ApplicationInstance { ApplicationName = _options.ApplicationName, @@ -339,8 +348,13 @@ public sealed class OpcUaApplicationHost : IAsyncDisposable }, }; - if (_options.SecurityProfile == OpcUaSecurityProfile.Basic256Sha256SignAndEncrypt - && _options.Ldap.Enabled) + // Server-011: advertise UserName tokens whenever LDAP is enabled and a non-None + // security profile is active. Previously gated on Basic256Sha256SignAndEncrypt only, + // so Ldap.Enabled with SecurityProfile = None silently produced no UserName policy + // and clients could not present credentials. Adding future non-None profiles here + // means operators don't have to remember to flip a second knob when adding a new + // profile. + if (_options.Ldap.Enabled && _options.SecurityProfile != OpcUaSecurityProfile.None) { tokens.Add(new UserTokenPolicy(UserTokenType.UserName) {