fix(server): resolve Medium code-review finding (Server-011)

Advertise UserName token policy on any non-None security profile when
Ldap.Enabled; emit a startup LogWarning when Ldap.Enabled=true but
SecurityProfile=None so the misconfiguration is surfaced before clients
connect rather than silently producing no credential path.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
Joseph Doherty
2026-05-22 11:01:43 -04:00
parent 6075254f38
commit a00f0338b5
2 changed files with 18 additions and 4 deletions

View File

@@ -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 |

View File

@@ -156,6 +156,15 @@ public sealed class OpcUaApplicationHost : IAsyncDisposable
/// </summary>
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)
{