From a224fd639198ea54fb3222464a33453dfd96c000 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Wed, 11 Nov 2026 01:28:58 -0500 Subject: [PATCH 1/3] fix(security): scope operator-authored credential references per driver (#815) A Designer could point a Galaxy / OpcUaClient endpoint at a host they control, set a credential field to any reference the node can resolve (env:ConnectionStrings__ConfigDb, env:ServerHistorian__ApiKey, secret:, Galaxy file:) and click Test Connect / live browse; the node resolved the value and sent it to that host. One shared rule in Core.Abstractions (CredentialReferences + DriverCredentialPolicy) generalises the #803 Heidenhain namespace: - Galaxy: secret:galaxy/... | secret:otopcua/galaxy/... | env:OTOPCUA_GALAXY_... | legacy env:GALAXY_MXGW_...; file: refused; dev:/literal API key unchanged. - OpcUaClient: secret:opcuaclient/... | env:OTOPCUA_OPCUACLIENT_...; literals unchanged. - Heidenhain adopts the shared parser/resolver (namespace unchanged). Enforced at the AdminUI save path (DriverConfigSaveGate), the deploy gate (CredentialReferenceDeployChecks, code CredentialReferenceOutOfScope), Test Connect (AdminOperationsActor, before any probe), live browse (BrowserSessionService, before any browser) and again at resolve time in every driver/browser (fail closed, before any store/env read). Co-Authored-By: Claude --- .../CredentialReferenceDeployChecks.cs | 46 +++ .../Validation/DraftValidator.cs | 1 + .../CredentialReferences.cs | 302 ++++++++++++++++++ .../DriverCredentialPolicy.cs | 148 +++++++++ .../GalaxySecretRef.cs | 117 +++---- .../Runtime/GalaxyAckCredentialResolver.cs | 71 ++-- .../HeidenhainOptionsParser.cs | 109 ++----- .../HeidenhainCredentials.cs | 32 +- .../OpcUaUserIdentityFactory.cs | 54 ++-- .../Browsing/BrowserSessionService.cs | 8 + .../Drivers/Forms/GalaxyDriverForm.razor | 12 +- .../Drivers/Forms/OpcUaClientDriverForm.razor | 2 + .../Uns/DriverConfigSaveGate.cs | 12 +- .../AdminOperations/AdminOperationsActor.cs | 14 + .../DraftValidatorCredentialReferenceTests.cs | 61 ++++ .../CredentialReferencesTests.cs | 134 ++++++++ .../GalaxyDriverApiKeyResolverTests.cs | 111 ++++--- .../Runtime/GalaxyAckCredentialTests.cs | 42 +++ .../OpcUaClientBrowserCertificateAuthTests.cs | 6 +- .../OpcUaClientSecretResolutionTests.cs | 63 +++- .../BrowserSessionCredentialScopeTests.cs | 78 +++++ .../Uns/CredentialReferenceSavePathTests.cs | 77 +++++ ...dminOperationsActorCredentialScopeTests.cs | 66 ++++ 23 files changed, 1252 insertions(+), 314 deletions(-) create mode 100644 src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Validation/CredentialReferenceDeployChecks.cs create mode 100644 src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/CredentialReferences.cs create mode 100644 src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/DriverCredentialPolicy.cs create mode 100644 tests/Core/ZB.MOM.WW.OtOpcUa.Configuration.Tests/DraftValidatorCredentialReferenceTests.cs create mode 100644 tests/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests/CredentialReferencesTests.cs create mode 100644 tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Browsing/BrowserSessionCredentialScopeTests.cs create mode 100644 tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/CredentialReferenceSavePathTests.cs create mode 100644 tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/AdminOperationsActorCredentialScopeTests.cs diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Validation/CredentialReferenceDeployChecks.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Validation/CredentialReferenceDeployChecks.cs new file mode 100644 index 000000000..5badc1b7f --- /dev/null +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Validation/CredentialReferenceDeployChecks.cs @@ -0,0 +1,46 @@ +using ZB.MOM.WW.OtOpcUa.Commons.Types; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; + +namespace ZB.MOM.WW.OtOpcUa.Configuration.Validation; + +/// +/// Deploy gate for credential references (lmxopcua#815): every DriverConfig and DeviceConfig of a driver type +/// listed in must carry its credential fields as blank, an acceptable +/// literal, or a reference inside that driver's namespace (e.g. Galaxy secret:galaxy/… / +/// env:OTOPCUA_GALAXY_…). The same table the AdminUI save path, Test Connect, live browse and every +/// driver's resolve-time check read — so the gate and the runtime cannot disagree. +/// +/// +/// Heidenhain is excluded here only because already reports the same rule +/// (as ) through the shared parser; reporting it twice would +/// duplicate every error. The message names the field and the allowed forms, never the value. +/// +public static class CredentialReferenceDeployChecks +{ + /// A credential field holding a reference outside the driver's namespace (or a refused literal). + public const string Code = "CredentialReferenceOutOfScope"; + + /// Runs the gate. + /// The draft. + /// The errors. + public static IReadOnlyList Errors(DraftSnapshot draft) + { + var errors = new List(); + foreach (var driver in draft.DriverInstances) + { + if (string.Equals(driver.DriverType, DriverTypeNames.Heidenhain, StringComparison.OrdinalIgnoreCase)) continue; + if (!DriverCredentialPolicy.TryGet(driver.DriverType, out _)) continue; + + foreach (var problem in DriverCredentialPolicy.DescribeProblems(driver.DriverType, driver.DriverConfig)) + errors.Add(new(Code, $"{driver.DriverType} driver '{driver.Name}': {problem}", driver.DriverInstanceId)); + + foreach (var device in draft.Devices.Where(d => string.Equals(d.DriverInstanceId, driver.DriverInstanceId, StringComparison.Ordinal))) + { + foreach (var problem in DriverCredentialPolicy.DescribeProblems(driver.DriverType, device.DeviceConfig)) + errors.Add(new(Code, $"{driver.DriverType} device '{device.Name}': {problem}", device.DeviceId)); + } + } + + return errors; + } +} diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Validation/DraftValidator.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Validation/DraftValidator.cs index 5f9ed4f6e..c2880f739 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Validation/DraftValidator.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Validation/DraftValidator.cs @@ -47,6 +47,7 @@ public static class DraftValidator errors.AddRange(SqlTagDeployChecks.WriteBlockErrors(draft)); ValidateMTConnectSingleDevice(draft, errors); errors.AddRange(HeidenhainDeployChecks.Errors(draft)); + errors.AddRange(CredentialReferenceDeployChecks.Errors(draft)); ValidateNodeAclScopes(draft, errors); return errors; } diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/CredentialReferences.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/CredentialReferences.cs new file mode 100644 index 000000000..725e3ab82 --- /dev/null +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/CredentialReferences.cs @@ -0,0 +1,302 @@ +namespace ZB.MOM.WW.OtOpcUa.Core.Abstractions; + +/// +/// The namespace a driver's operator-authored credential references are confined to (lmxopcua#815): the +/// secret-store prefix a secret: reference must start with, and the environment-variable prefixes an +/// env: reference must start with. +/// +/// +/// The scope is the exfiltration guard. A Designer authors the endpoint AND the credential reference, and the +/// node resolves the reference and sends the value to that endpoint (Test Connect, live browse, deploy). An +/// unscoped reference could name any secret or process variable the node can read — +/// env:ConnectionStrings__ConfigDb, env:ServerHistorian__ApiKey, the mesh PSK — and send it in +/// cleartext to a host the Designer controls. Inside its own namespace a reference can only name values that +/// were provisioned to be sent to that driver's kind of endpoint. +/// +/// Driver label used in messages (e.g. Galaxy). +/// +/// The ONLY secret-store prefixes allowed, each ending in / (e.g. galaxy/). The first entry is the +/// documented form. +/// +/// +/// The ONLY environment-variable prefixes allowed (exact, upper-case, e.g. OTOPCUA_GALAXY_); the remainder +/// must be [A-Z0-9_]+. The first entry is the documented form. +/// +public sealed record CredentialScope(string Label, IReadOnlyList SecretNamespaces, IReadOnlyList EnvVariablePrefixes); + +/// Whether a credential field may carry a literal (non-reference) value. +public enum CredentialLiteralPolicy +{ + /// A literal is refused — the field must be blank or a namespaced reference. + Refused, + + /// A literal is accepted and used verbatim (the author already knows it; nothing is resolved). + Allowed, + + /// Only the explicit dev:VALUE literal is accepted (Galaxy's opt-in cleartext marker). + DevPrefixOnly, +} + +/// The kind of a parsed credential field value. +public enum CredentialReferenceKind +{ + /// Null or empty. + Blank, + + /// A secret: (or ${secret:…}) reference inside the scope's namespace. + Secret, + + /// An env: reference inside the scope's prefixes. + Environment, + + /// Not a reference: a literal value (acceptability depends on the field's ). + Literal, + + /// + /// A reference (secret:, ${secret:…}, env:, file:) that is outside the scope or + /// malformed. Never resolved; always refused. + /// + OutOfScope, +} + +/// A credential reference that may not (or cannot) be resolved. The message never carries a resolved value. +/// The message. +public sealed class CredentialReferenceException(string message) : InvalidOperationException(message); + +/// +/// The one parser + resolver of operator-authored credential references (secret: / env:), shared by +/// every driver, probe, browser, the AdminUI save path and the deploy gate (lmxopcua#815, generalising #803). +/// Resolution is fail-closed and re-checks the scope itself, so a reference that skipped every earlier gate is +/// still never looked up. +/// +public static class CredentialReferences +{ + /// Prefix of a secret-store reference. + public const string SecretPrefix = "secret:"; + + /// Prefix of an environment-variable reference. + public const string EnvPrefix = "env:"; + + /// Prefix of a file reference — never allowed in operator-authored config (it reads any node file). + public const string FilePrefix = "file:"; + + /// Galaxy's explicit cleartext marker. + public const string DevPrefix = "dev:"; + + private const string TokenSecretPrefix = "${secret:"; + + /// + /// Parses against . A prefix in another casing + /// (SECRET:x) is — never a literal sent verbatim and + /// never resolved; the prefix, the namespace and the name are matched exactly. + /// + /// The authored field value. + /// The driver's scope. + /// The full secret name (incl. the namespace) or the environment variable; empty otherwise. + /// The kind. + public static CredentialReferenceKind Parse(string? value, CredentialScope scope, out string name) + { + ArgumentNullException.ThrowIfNull(scope); + name = ""; + if (string.IsNullOrEmpty(value)) return CredentialReferenceKind.Blank; + + // A prefix in any casing is a reference (never a literal sent verbatim), but only the exact lower-case + // spelling is ever resolved — 'SECRET:galaxy/x' is refused, not silently honoured. + if (HasPrefixOtherCase(value, TokenSecretPrefix) || HasPrefixOtherCase(value, SecretPrefix) + || HasPrefixOtherCase(value, EnvPrefix)) + return CredentialReferenceKind.OutOfScope; + + string? secret = null; + if (value.StartsWith(TokenSecretPrefix, StringComparison.Ordinal)) + { + if (!value.EndsWith('}')) return CredentialReferenceKind.OutOfScope; + secret = value[TokenSecretPrefix.Length..^1]; + } + else if (value.StartsWith(SecretPrefix, StringComparison.Ordinal)) + { + secret = value[SecretPrefix.Length..]; + } + + if (secret is not null) + { + if (!IsScopedSecretName(secret, scope)) return CredentialReferenceKind.OutOfScope; + name = secret; + return CredentialReferenceKind.Secret; + } + + if (value.StartsWith(EnvPrefix, StringComparison.Ordinal)) + { + var variable = value[EnvPrefix.Length..]; + if (!IsScopedVariable(variable, scope)) return CredentialReferenceKind.OutOfScope; + name = variable; + return CredentialReferenceKind.Environment; + } + + return value.StartsWith(FilePrefix, StringComparison.OrdinalIgnoreCase) + ? CredentialReferenceKind.OutOfScope + : CredentialReferenceKind.Literal; + } + + /// True when may be stored/deployed in a field with the given literal policy. + /// The authored value. + /// The driver's scope. + /// The field's literal policy. + /// Whether it is acceptable. + public static bool IsAcceptable(string? value, CredentialScope scope, CredentialLiteralPolicy literals) => + Parse(value, scope, out _) switch + { + CredentialReferenceKind.Blank or CredentialReferenceKind.Secret or CredentialReferenceKind.Environment => true, + CredentialReferenceKind.Literal => literals switch + { + CredentialLiteralPolicy.Allowed => true, + CredentialLiteralPolicy.DevPrefixOnly => value!.StartsWith(DevPrefix, StringComparison.OrdinalIgnoreCase), + _ => false, + }, + _ => false, + }; + + /// The one wording of the rule — names the allowed forms, never the value. + /// The field. + /// The driver's scope. + /// The field's literal policy. + /// The message. + public static string RuleMessage(string field, CredentialScope scope, CredentialLiteralPolicy literals) + { + ArgumentNullException.ThrowIfNull(scope); + var env = string.Join(" or ", scope.EnvVariablePrefixes.Select(p => $"'env:{p}'")); + var secrets = string.Join(", ", scope.SecretNamespaces.Select(n => $"'secret:{n}'")); + var forms = $"{secrets} or {env}"; + return literals switch + { + CredentialLiteralPolicy.Allowed => + $"{field} must be a literal value, {forms} — a secret:/env:/file: reference outside the {scope.Label} " + + "namespace is refused (lmxopcua#815).", + CredentialLiteralPolicy.DevPrefixOnly => + $"{field} must be blank, {forms} (or 'dev:' on a dev rig) — a literal, a file: reference, or a " + + $"reference outside the {scope.Label} namespace is refused (lmxopcua#815).", + _ => + $"{field} must be blank, {forms} — a literal password, or a reference outside the {scope.Label} " + + "namespace, is refused (lmxopcua#815).", + }; + } + + /// + /// Resolves a field value, fail-closed. Blank returns ""; an acceptable literal returns verbatim; an + /// out-of-scope reference or a refused literal throws BEFORE any + /// store or environment read; an absent secret / unset variable returns . + /// + /// The authored value. + /// The driver's scope. + /// Field label for messages. + /// The field's literal policy. + /// Secret-store lookup (by full name); null ⇒ a secret: reference throws. + /// Environment reader; null ⇒ . + /// Cancellation. + /// The value, or null when the referenced value is absent. + public static async ValueTask ResolveOrNullAsync( + string? value, + CredentialScope scope, + string field, + CredentialLiteralPolicy literals, + Func>? secretLookup, + Func? environment, + CancellationToken ct) + { + var kind = Parse(value, scope, out var name); + if (!IsAcceptable(value, scope, literals)) + throw new CredentialReferenceException(RuleMessage(field, scope, literals) + " (fail-closed)"); + switch (kind) + { + case CredentialReferenceKind.Blank: + return ""; + case CredentialReferenceKind.Literal: + return value; + case CredentialReferenceKind.Environment: + return (environment ?? System.Environment.GetEnvironmentVariable)(name); + case CredentialReferenceKind.Secret: + if (secretLookup is null) + throw new CredentialReferenceException( + $"{field} references secret '{name}' but no secret store is available (fail-closed)."); + return await secretLookup(name, ct).ConfigureAwait(false); + default: + throw new CredentialReferenceException(RuleMessage(field, scope, literals) + " (fail-closed)"); + } + } + + /// + /// As , but an absent secret / unset (or empty) variable throws + /// naming the secret or variable (never a value). + /// + /// The authored value. + /// The driver's scope. + /// Field label for messages. + /// The field's literal policy. + /// Secret-store lookup; null ⇒ a secret: reference throws. + /// Environment reader; null ⇒ the process environment. + /// Cancellation. + /// The value ("" for blank). + public static async ValueTask ResolveAsync( + string? value, + CredentialScope scope, + string field, + CredentialLiteralPolicy literals, + Func>? secretLookup, + Func? environment, + CancellationToken ct) + { + var resolved = await ResolveOrNullAsync(value, scope, field, literals, secretLookup, environment, ct).ConfigureAwait(false); + var kind = Parse(value, scope, out var name); + if (!string.IsNullOrEmpty(resolved) || kind is CredentialReferenceKind.Blank or CredentialReferenceKind.Literal) + return resolved ?? ""; + throw kind == CredentialReferenceKind.Environment + ? new CredentialReferenceException($"{field} environment variable '{name}' is not set (fail-closed).") + : new CredentialReferenceException($"{field} secret '{name}' is absent from the store (fail-closed)."); + } + + /// A print-safe form of a value: the reference itself, or a redaction for a literal / dev value. + /// The authored value. + /// A string safe to log. + public static string Describe(string? value) + { + if (string.IsNullOrEmpty(value)) return ""; + var v = value.Trim(); + if (v.StartsWith(DevPrefix, StringComparison.OrdinalIgnoreCase)) return "dev:"; + return v.StartsWith(SecretPrefix, StringComparison.OrdinalIgnoreCase) + || v.StartsWith(TokenSecretPrefix, StringComparison.OrdinalIgnoreCase) + || v.StartsWith(EnvPrefix, StringComparison.OrdinalIgnoreCase) + || v.StartsWith(FilePrefix, StringComparison.OrdinalIgnoreCase) + ? v + : ""; + } + + private static bool HasPrefixOtherCase(string value, string prefix) => + value.StartsWith(prefix, StringComparison.OrdinalIgnoreCase) && !value.StartsWith(prefix, StringComparison.Ordinal); + + private static bool IsScopedSecretName(string secret, CredentialScope scope) + { + foreach (var ns in scope.SecretNamespaces) + { + if (!secret.StartsWith(ns, StringComparison.Ordinal)) continue; + var rest = secret[ns.Length..]; + if (rest.Length is 0 or > 200) continue; + if (!rest.All(c => char.IsAsciiLetterOrDigit(c) || c is '-' or '_' or '.' or '/')) continue; + if (rest.Split('/').Any(seg => seg.Length == 0 || seg is "." or "..")) continue; + return true; + } + + return false; + } + + private static bool IsScopedVariable(string variable, CredentialScope scope) + { + foreach (var prefix in scope.EnvVariablePrefixes) + { + if (!variable.StartsWith(prefix, StringComparison.Ordinal)) continue; + var rest = variable[prefix.Length..]; + if (rest.Length is > 0 and <= 100 && rest.All(c => char.IsAsciiLetterUpper(c) || char.IsAsciiDigit(c) || c == '_')) + return true; + } + + return false; + } +} diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/DriverCredentialPolicy.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/DriverCredentialPolicy.cs new file mode 100644 index 000000000..2b3eeb745 --- /dev/null +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/DriverCredentialPolicy.cs @@ -0,0 +1,148 @@ +using System.Text.Json; + +namespace ZB.MOM.WW.OtOpcUa.Core.Abstractions; + +/// One credential field of a driver config and whether it may carry a literal. +/// The JSON property name (matched case-insensitively, as every driver binds). +/// The literal policy. +public sealed record CredentialField(string Name, CredentialLiteralPolicy Literals); + +/// The credential scope + fields of one driver type. +/// The value. +/// The namespace its references are confined to. +/// The credential fields its DriverConfig/DeviceConfig may carry. +public sealed record DriverCredentialRules(string DriverType, CredentialScope Scope, IReadOnlyList Fields); + +/// +/// The per-driver credential namespaces (lmxopcua#815) — the single table the AdminUI save path, the deploy gate, +/// Test Connect, live browse and every driver's resolve-time check read. +/// +/// +/// Every driver that RESOLVES a secret:/env: reference from operator-authored config and sends the +/// value to an operator-chosen endpoint is listed here. Drivers that resolve nothing (literal credentials only — +/// MQTT, MTConnect URI user-info) or whose resolved value carries its own endpoint (Sql's +/// Sql__ConnectionStrings__<ref>, already a fixed prefix) are not. +/// +public static class DriverCredentialPolicy +{ + /// Heidenhain LSV2 passwords: secret:heidenhain/… / env:OTOPCUA_HEIDENHAIN_… (#803). + public static readonly CredentialScope HeidenhainScope = new("Heidenhain", ["heidenhain/"], ["OTOPCUA_HEIDENHAIN_"]); + + /// + /// Galaxy gateway API key + fallback ack password: secret:galaxy/… (or secret:otopcua/galaxy/…, the + /// form the ack-fallback docs used) / env:OTOPCUA_GALAXY_…, plus the + /// legacy env:GALAXY_MXGW_… prefix the docker-dev rig and the parity-rig docs already use + /// (GALAXY_MXGW_API_KEY) — a variable provisioned to be sent to an mxaccessgw. + /// + public static readonly CredentialScope GalaxyScope = new("Galaxy", ["galaxy/", "otopcua/galaxy/"], ["OTOPCUA_GALAXY_", "GALAXY_MXGW_"]); + + /// OPC UA Client user / certificate passwords: secret:opcuaclient/… / env:OTOPCUA_OPCUACLIENT_…. + public static readonly CredentialScope OpcUaClientScope = new("OpcUaClient", ["opcuaclient/"], ["OTOPCUA_OPCUACLIENT_"]); + + /// Galaxy Gateway.ApiKeySecretRef (literal accepted for back-compat, with a warning at resolve). + public static readonly CredentialField GalaxyApiKeyField = new("ApiKeySecretRef", CredentialLiteralPolicy.Allowed); + + /// Galaxy AlarmAck.FallbackPasswordSecretRef (reference or explicit dev: only). + public static readonly CredentialField GalaxyAckPasswordField = new("FallbackPasswordSecretRef", CredentialLiteralPolicy.DevPrefixOnly); + + private static readonly IReadOnlyDictionary Rules = + new Dictionary(StringComparer.OrdinalIgnoreCase) + { + [DriverTypeNames.Heidenhain] = new(DriverTypeNames.Heidenhain, HeidenhainScope, + [ + new("InspectPassword", CredentialLiteralPolicy.Refused), + new("DncPassword", CredentialLiteralPolicy.Refused), + new("DataPassword", CredentialLiteralPolicy.Refused), + ]), + [DriverTypeNames.Galaxy] = new(DriverTypeNames.Galaxy, GalaxyScope, [GalaxyApiKeyField, GalaxyAckPasswordField]), + [DriverTypeNames.OpcUaClient] = new(DriverTypeNames.OpcUaClient, OpcUaClientScope, + [ + new("Password", CredentialLiteralPolicy.Allowed), + new("UserCertificatePassword", CredentialLiteralPolicy.Allowed), + ]), + }; + + /// The rules for a driver type, if it resolves credential references. + /// The driver type. + /// The rules. + /// Whether the type has rules. + public static bool TryGet(string? driverType, out DriverCredentialRules rules) + { + if (driverType is not null && Rules.TryGetValue(driverType, out var r)) + { + rules = r; + return true; + } + + rules = null!; + return false; + } + + /// + /// Scans a RAW DriverConfig / DeviceConfig / merged config document for credential fields of + /// whose value is not acceptable — at ANY depth (a field is bound wherever the + /// driver's merge places it), keys matched case-insensitively. Messages name the field path, never the value. + /// Unreadable JSON is reported for a type with rules (it cannot be checked); other types return empty. + /// + /// The owning driver's type. + /// The config JSON. + /// The problems; empty when acceptable or the type has no credential rules. + public static IReadOnlyList DescribeProblems(string? driverType, string? json) + { + var problems = new List(); + if (!TryGet(driverType, out var rules) || string.IsNullOrWhiteSpace(json)) return problems; + JsonDocument doc; + try + { + doc = JsonDocument.Parse(json, new JsonDocumentOptions { CommentHandling = JsonCommentHandling.Skip, AllowTrailingCommas = true }); + } + catch (JsonException) + { + problems.Add($"{rules.Scope.Label} config is not valid JSON, so its credential fields cannot be checked."); + return problems; + } + + using (doc) + { + Scan(doc.RootElement, "", rules, problems, 0); + } + + return problems; + } + + private static void Scan(JsonElement element, string path, DriverCredentialRules rules, List problems, int depth) + { + if (depth > 32) return; + if (element.ValueKind == JsonValueKind.Array) + { + var i = 0; + foreach (var item in element.EnumerateArray()) + { + Scan(item, $"{path}[{i}]", rules, problems, depth + 1); + i++; + } + + return; + } + + if (element.ValueKind != JsonValueKind.Object) return; + foreach (var p in element.EnumerateObject()) + { + var fieldPath = path.Length == 0 ? p.Name : $"{path}.{p.Name}"; + var field = rules.Fields.FirstOrDefault(f => string.Equals(f.Name, p.Name, StringComparison.OrdinalIgnoreCase)); + if (field is null) + { + Scan(p.Value, fieldPath, rules, problems, depth + 1); + continue; + } + + var ok = p.Value.ValueKind switch + { + JsonValueKind.Null => true, + JsonValueKind.String => CredentialReferences.IsAcceptable(p.Value.GetString(), rules.Scope, field.Literals), + _ => false, + }; + if (!ok) problems.Add(CredentialReferences.RuleMessage(fieldPath, rules.Scope, field.Literals)); + } + } +} diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Contracts/GalaxySecretRef.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Contracts/GalaxySecretRef.cs index d7bca47f3..5e685ad6d 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Contracts/GalaxySecretRef.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Contracts/GalaxySecretRef.cs @@ -1,33 +1,24 @@ using Microsoft.Extensions.Logging; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; using ZB.MOM.WW.Secrets.Abstractions; namespace ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Config; /// -/// Resolves Gateway.ApiKeySecretRef to the actual API-key string. Five -/// forms supported, evaluated in order: +/// Resolves Gateway.ApiKeySecretRef to the actual API-key string. The reference is resolved on the node +/// and sent to the operator-authored Gateway.Endpoint, so references are confined to the Galaxy +/// namespace (lmxopcua#815, ) and the check is repeated here, at +/// resolve time, before any store or environment read: /// -/// env:NAME — reads Environment.GetEnvironmentVariable(NAME). -/// Throws when the variable is unset, so a misconfigured deployment fails -/// fast rather than silently sending an empty key. -/// file:PATH — reads UTF-8 text from PATH, trimming -/// whitespace. Lets operators stash the key in an ACL'd file outside the -/// repo (the same pattern as the legacy .local/galaxy-host-secret.txt). -/// dev:KEY — explicit cleartext literal. The dev: prefix -/// is a deliberate opt-in signal (dev box, parity rig) so the resolver -/// doesn't emit a warning; production should never use this arm. -/// secret:NAME — resolves NAME through the shared -/// ZB.MOM.WW.Secrets (the encrypted-at-rest -/// store). Fail-closed: a secret: ref whose secret is absent/tombstoned -/// throws rather than falling through to the literal arm — the production path -/// that retires the cleartext dev:/literal-in-DB model. -/// Anything else — used as the literal API key for back-compat with -/// configs that pre-date this resolver. When a logger is supplied the -/// resolver emits a startup warning so an operator who accidentally -/// committed a cleartext key sees it. +/// secret:galaxy/NAME (or ${secret:galaxy/NAME}) — resolved through the shared +/// ZB.MOM.WW.Secrets ; fail-closed when absent. +/// env:OTOPCUA_GALAXY_NAME / legacy env:GALAXY_MXGW_NAME — the process environment; +/// fail-closed when unset. +/// dev:KEY — explicit cleartext literal (dev box, parity rig); no warning. +/// Any other unprefixed string — the literal key (back-compat), with a warning when a logger is supplied. +/// A secret:/env: reference OUTSIDE the namespace, and every file: reference (it +/// read any file the node could open) — refused with . /// -/// A future PR can swap any of these arms for a different backing store without -/// changing the call site. /// /// /// Lives in the Contracts project so both the runtime GalaxyDriver and the @@ -37,80 +28,50 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Config; public static class GalaxySecretRef { /// - /// Resolves the supplied secret reference. The secret:NAME arm resolves - /// through and is fail-closed (throws when the secret - /// is absent). When the ref falls through to the back-compat literal arm (an - /// unprefixed cleartext API key in DriverConfig JSON) and a - /// is supplied, emits a . - /// The dev: prefix is the explicit opt-in path that doesn't warn. + /// Resolves the supplied API-key reference (see the type remarks). Fail-closed for an out-of-namespace + /// reference, a file: reference, an absent secret and an unset variable. /// /// The secret reference string to resolve. /// The shared secret resolver used by the secret: arm. /// Optional logger for warning on cleartext keys. /// Cancellation token for the async secret: resolution. + /// Environment reader (tests); null ⇒ the process environment. /// The resolved API-key string. public static async Task ResolveApiKeyAsync( string secretRef, ISecretResolver resolver, ILogger? logger = null, - CancellationToken ct = default) + CancellationToken ct = default, + Func? environment = null) { ArgumentException.ThrowIfNullOrEmpty(secretRef); ArgumentNullException.ThrowIfNull(resolver); - if (secretRef.StartsWith("env:", StringComparison.OrdinalIgnoreCase)) + var scope = DriverCredentialPolicy.GalaxyScope; + if (CredentialReferences.Parse(secretRef, scope, out _) == CredentialReferenceKind.Literal) { - var name = secretRef[4..]; - var value = Environment.GetEnvironmentVariable(name); - return !string.IsNullOrEmpty(value) - ? value - : throw new InvalidOperationException( - $"Galaxy.Gateway.ApiKeySecretRef='{secretRef}' resolves to env var '{name}', but it is unset."); - } - - if (secretRef.StartsWith("file:", StringComparison.OrdinalIgnoreCase)) - { - var path = secretRef[5..]; - if (!File.Exists(path)) + if (secretRef.StartsWith(CredentialReferences.DevPrefix, StringComparison.OrdinalIgnoreCase)) { - throw new InvalidOperationException( - $"Galaxy.Gateway.ApiKeySecretRef='{secretRef}' points at '{path}', which doesn't exist."); + // Explicit dev opt-in — no warning, the operator deliberately chose a cleartext literal. + return secretRef[CredentialReferences.DevPrefix.Length..]; } - var contents = File.ReadAllText(path).Trim(); - return !string.IsNullOrEmpty(contents) - ? contents - : throw new InvalidOperationException( - $"Galaxy.Gateway.ApiKeySecretRef='{secretRef}' file '{path}' is empty."); + + // Back-compat literal arm. Warn so an operator who accidentally committed a cleartext key sees it. + logger?.LogWarning( + "Galaxy.Gateway.ApiKeySecretRef is being treated as a literal cleartext API key. " + + "Prefer secret:galaxy/ or env:OTOPCUA_GALAXY_, or the explicit dev:KEY prefix for dev rigs — " + + "a literal key in DriverConfig JSON is stored in cleartext in the central config DB."); + return secretRef; } - if (secretRef.StartsWith("dev:", StringComparison.OrdinalIgnoreCase)) - { - // Explicit dev opt-in — no warning, the operator deliberately chose a - // cleartext literal (dev box, parity rig). - return secretRef[4..]; - } - - if (secretRef.StartsWith("secret:", StringComparison.OrdinalIgnoreCase)) - { - // Production path: resolve the name through the shared encrypted secret store. - // Fail-closed — an absent/tombstoned secret throws rather than falling through - // to the literal arm (which would silently treat the ref string as the key). - var name = secretRef["secret:".Length..]; - var value = await resolver.GetAsync(new SecretName(name), ct).ConfigureAwait(false); - return !string.IsNullOrEmpty(value) - ? value - : throw new InvalidOperationException( - $"Galaxy.Gateway.ApiKeySecretRef='{secretRef}' resolves secret '{name}', but it is absent from the store (fail-closed)."); - } - - // Back-compat literal arm. An unprefixed string is treated as the literal - // API key — but emit a warning so an operator who accidentally committed a - // cleartext key into DriverConfig sees it. Use the dev: prefix to suppress - // this warning when the literal is intentional. - logger?.LogWarning( - "Galaxy.Gateway.ApiKeySecretRef is being treated as a literal cleartext API key. " + - "Prefer env:NAME, file:PATH, or the explicit dev:KEY prefix for dev rigs — " + - "a literal key in DriverConfig JSON is stored in cleartext in the central config DB."); - return secretRef; + // secret:/env: inside the namespace resolve; anything else (out of scope, file:) throws before any read. + return await CredentialReferences.ResolveAsync( + secretRef, + scope, + "Galaxy.Gateway.ApiKeySecretRef", + DriverCredentialPolicy.GalaxyApiKeyField.Literals, + async (name, c) => await resolver.GetAsync(new SecretName(name), c).ConfigureAwait(false), + environment, + ct).ConfigureAwait(false); } } diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy/Runtime/GalaxyAckCredentialResolver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy/Runtime/GalaxyAckCredentialResolver.cs index c20295d3c..77b2ad751 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy/Runtime/GalaxyAckCredentialResolver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy/Runtime/GalaxyAckCredentialResolver.cs @@ -1,4 +1,5 @@ using Microsoft.Extensions.Logging; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; using ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Config; using ZB.MOM.WW.Secrets.Abstractions; @@ -20,15 +21,19 @@ internal sealed class GalaxyAckCredentialResolver : IGalaxyAckCredentialResolver private readonly GalaxyAlarmAckOptions _options; private readonly ISecretResolver _secrets; private readonly ILogger _logger; + private readonly Func? _environment; /// Initializes a new instance of the class. /// The owning driver instance id (for messages). /// The driver's ack-credential options. /// The shared secret resolver. /// Logger; never receives a secret. + /// Environment reader (tests); null ⇒ the process environment. public GalaxyAckCredentialResolver( - string driverInstanceId, GalaxyAlarmAckOptions? options, ISecretResolver secrets, ILogger logger) + string driverInstanceId, GalaxyAlarmAckOptions? options, ISecretResolver secrets, ILogger logger, + Func? environment = null) { + _environment = environment; _driverInstanceId = driverInstanceId; _options = options ?? new GalaxyAlarmAckOptions(); _secrets = secrets ?? throw new ArgumentNullException(nameof(secrets)); @@ -142,56 +147,28 @@ internal sealed class GalaxyAckCredentialResolver : IGalaxyAckCredentialResolver } /// - /// Resolves a password reference. Accepts secret:NAME, ${secret:NAME}, env:NAME, - /// file:PATH and dev:VALUE; returns null when the referenced value is absent; throws - /// for a bare literal (a cleartext password in DriverConfig). + /// Resolves the fallback password reference through the shared lmxopcua#815 resolver: secret:galaxy/NAME + /// (or secret:otopcua/galaxy/NAME, ${secret:…}), env:OTOPCUA_GALAXY_NAME / + /// env:GALAXY_MXGW_NAME, or dev:VALUE; returns null when the referenced value is absent; throws + /// — before any store/environment read — for a bare literal, a + /// file: reference, or a reference outside the Galaxy namespace (the password is sent to the + /// operator-authored gateway endpoint, so it must never be able to name another subsystem's secret). /// internal async Task ResolvePasswordRefAsync(string reference, CancellationToken ct) { - if (reference.StartsWith("${secret:", StringComparison.OrdinalIgnoreCase) && reference.EndsWith('}')) - { - reference = "secret:" + reference["${secret:".Length..^1]; - } - - if (reference.StartsWith("secret:", StringComparison.OrdinalIgnoreCase)) - { - return await _secrets.GetAsync(new SecretName(reference["secret:".Length..]), ct).ConfigureAwait(false); - } - - if (reference.StartsWith("env:", StringComparison.OrdinalIgnoreCase)) - { - return Environment.GetEnvironmentVariable(reference[4..]); - } - - if (reference.StartsWith("file:", StringComparison.OrdinalIgnoreCase)) - { - var path = reference[5..]; - return File.Exists(path) - ? (await File.ReadAllTextAsync(path, ct).ConfigureAwait(false)).TrimEnd('\r', '\n') - : null; - } - - if (reference.StartsWith("dev:", StringComparison.OrdinalIgnoreCase)) - { - return reference[4..]; - } - - throw new FormatException("AlarmAck.FallbackPasswordSecretRef must be a secret:/env:/file:/dev: reference, not a literal."); + var resolved = await CredentialReferences.ResolveOrNullAsync( + reference, + DriverCredentialPolicy.GalaxyScope, + "AlarmAck.FallbackPasswordSecretRef", + DriverCredentialPolicy.GalaxyAckPasswordField.Literals, + async (name, c) => await _secrets.GetAsync(new SecretName(name), c).ConfigureAwait(false), + _environment, + ct).ConfigureAwait(false); + return resolved is not null && reference.StartsWith(CredentialReferences.DevPrefix, StringComparison.OrdinalIgnoreCase) + ? resolved[CredentialReferences.DevPrefix.Length..] + : resolved; } // Safe-to-print form of a reference: the kind + name for secret/env/file, never the dev: value or a literal. - internal static string DescribeRef(string reference) - { - var r = reference.Trim(); - if (r.StartsWith("dev:", StringComparison.OrdinalIgnoreCase)) return "dev:"; - if (r.StartsWith("${secret:", StringComparison.OrdinalIgnoreCase) - || r.StartsWith("secret:", StringComparison.OrdinalIgnoreCase) - || r.StartsWith("env:", StringComparison.OrdinalIgnoreCase) - || r.StartsWith("file:", StringComparison.OrdinalIgnoreCase)) - { - return r; - } - - return ""; - } + internal static string DescribeRef(string reference) => CredentialReferences.Describe(reference); } diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Heidenhain.Contracts/HeidenhainOptionsParser.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Heidenhain.Contracts/HeidenhainOptionsParser.cs index 925b4735c..bd1aa3899 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Heidenhain.Contracts/HeidenhainOptionsParser.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Heidenhain.Contracts/HeidenhainOptionsParser.cs @@ -1,4 +1,5 @@ using System.Text.Json; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; namespace ZB.MOM.WW.OtOpcUa.Driver.Heidenhain; @@ -21,14 +22,14 @@ public static class HeidenhainOptionsParser /// exfiltration guard: the reference is resolved on the node and sent in cleartext in A_LG to a Host /// the author chose, so it must never be able to name another subsystem's secret. /// - public const string SecretNamespace = "heidenhain/"; + public static readonly string SecretNamespace = DriverCredentialPolicy.HeidenhainScope.SecretNamespaces[0]; /// /// The ONLY environment variables a Heidenhain password may name: env:OTOPCUA_HEIDENHAIN_<NAME> /// (exact upper-case prefix; NAME is [A-Z0-9_]+) — never ConnectionStrings__*, /// ServerHistorian__ApiKey or any other process variable. /// - public const string EnvVariablePrefix = "OTOPCUA_HEIDENHAIN_"; + public static readonly string EnvVariablePrefix = DriverCredentialPolicy.HeidenhainScope.EnvVariablePrefixes[0]; /// The password fields a Heidenhain DriverConfig/DeviceConfig may carry. public static readonly IReadOnlyList PasswordFields = ["InspectPassword", "DncPassword", "DataPassword"]; @@ -164,105 +165,41 @@ public static class HeidenhainOptionsParser /// True for blank or a namespaced reference. public static bool TryParseCredentialReference(string? value, out CredentialKind kind, out string name) { - kind = CredentialKind.None; - name = ""; - if (string.IsNullOrEmpty(value)) return true; - if (value.StartsWith(SecretPrefix, StringComparison.Ordinal)) + // The shared lmxopcua#815 parser — one rule for every driver that resolves credential references. + switch (CredentialReferences.Parse(value, DriverCredentialPolicy.HeidenhainScope, out name)) { - var secret = value[SecretPrefix.Length..]; - if (!secret.StartsWith(SecretNamespace, StringComparison.Ordinal)) return false; - var rest = secret[SecretNamespace.Length..]; - if (rest.Length is 0 or > 200) return false; - foreach (var c in rest) - if (!(char.IsAsciiLetterOrDigit(c) || c is '-' or '_' or '.' or '/')) return false; - if (rest.Split('/').Any(seg => seg.Length == 0 || seg is "." or "..")) return false; - kind = CredentialKind.Secret; - name = secret; - return true; - } - - if (value.StartsWith(EnvPrefix, StringComparison.Ordinal)) - { - var variable = value[EnvPrefix.Length..]; - if (!variable.StartsWith(EnvVariablePrefix, StringComparison.Ordinal)) return false; - var rest = variable[EnvVariablePrefix.Length..]; - if (rest.Length is 0 or > 100 || !rest.All(c => char.IsAsciiLetterUpper(c) || char.IsAsciiDigit(c) || c == '_')) + case CredentialReferenceKind.Blank: + kind = CredentialKind.None; + return true; + case CredentialReferenceKind.Secret: + kind = CredentialKind.Secret; + return true; + case CredentialReferenceKind.Environment: + kind = CredentialKind.Environment; + return true; + default: + kind = CredentialKind.None; + name = ""; return false; - kind = CredentialKind.Environment; - name = variable; - return true; } - - return false; } /// /// Scans a RAW DriverConfig or DeviceConfig document (before it is stored) for password fields that are not - /// namespaced references — at the top level (a device / merged config), under Access, and under every - /// Devices[] entry; keys matched case-insensitively, as the parser binds them. The messages name the - /// field, never the value. Unreadable JSON is reported, since it cannot be checked. + /// namespaced references — at any depth (top level, Access, Devices[]…); keys matched + /// case-insensitively, as the parser binds them. The messages name the field, never the value. Unreadable + /// JSON is reported, since it cannot be checked. Delegates to the shared . /// /// The config JSON. /// The problems; empty when every password field is acceptable. - public static IReadOnlyList DescribeCredentialProblems(string? json) - { - var problems = new List(); - if (string.IsNullOrWhiteSpace(json)) return problems; - JsonDocument doc; - try - { - doc = JsonDocument.Parse(json, new JsonDocumentOptions { CommentHandling = JsonCommentHandling.Skip, AllowTrailingCommas = true }); - } - catch (JsonException) - { - problems.Add("Heidenhain config is not valid JSON, so its credential fields cannot be checked."); - return problems; - } - - using (doc) - { - if (doc.RootElement.ValueKind != JsonValueKind.Object) return problems; - ScanObject(doc.RootElement, "", problems); - foreach (var p in doc.RootElement.EnumerateObject()) - { - if (string.Equals(p.Name, "Access", StringComparison.OrdinalIgnoreCase) && p.Value.ValueKind == JsonValueKind.Object) - ScanObject(p.Value, "Access.", problems); - if (string.Equals(p.Name, "Devices", StringComparison.OrdinalIgnoreCase) && p.Value.ValueKind == JsonValueKind.Array) - { - var i = 0; - foreach (var d in p.Value.EnumerateArray()) - { - if (d.ValueKind == JsonValueKind.Object) ScanObject(d, $"Devices[{i}].", problems); - i++; - } - } - } - } - - return problems; - } + public static IReadOnlyList DescribeCredentialProblems(string? json) => + DriverCredentialPolicy.DescribeProblems(DriverTypeNames.Heidenhain, json); /// The one wording of the credential rule (never includes the value). /// The field. /// The message. public static string CredentialRuleMessage(string field) => - $"{field} must be blank, 'secret:{SecretNamespace}' or 'env:{EnvVariablePrefix}' — a literal " - + "password, or a reference outside the Heidenhain namespace, is refused."; - - private static void ScanObject(JsonElement obj, string prefix, List problems) - { - foreach (var p in obj.EnumerateObject()) - { - if (!PasswordFields.Any(f => string.Equals(f, p.Name, StringComparison.OrdinalIgnoreCase))) continue; - var ok = p.Value.ValueKind switch - { - JsonValueKind.Null => true, - JsonValueKind.String => IsCredentialReference(p.Value.GetString()), - _ => false, - }; - if (!ok) problems.Add(CredentialRuleMessage(prefix + p.Name)); - } - } + CredentialReferences.RuleMessage(field, DriverCredentialPolicy.HeidenhainScope, CredentialLiteralPolicy.Refused); private static void CheckRef(List errors, string field, string? value) { diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Heidenhain/HeidenhainCredentials.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Heidenhain/HeidenhainCredentials.cs index f441dfcb1..a83518677 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Heidenhain/HeidenhainCredentials.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Heidenhain/HeidenhainCredentials.cs @@ -1,3 +1,4 @@ +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; using ZB.MOM.WW.Secrets.Abstractions; namespace ZB.MOM.WW.OtOpcUa.Driver.Heidenhain; @@ -31,26 +32,23 @@ internal sealed class HeidenhainCredentials(ISecretResolver? resolver, FuncResolves one reference. + /// Resolves one reference through the shared lmxopcua#815 resolver (scope re-checked before any read). public async ValueTask ResolveReferenceAsync(string? reference, string role, CancellationToken ct) { - if (!HeidenhainOptionsParser.TryParseCredentialReference(reference, out var kind, out var name)) - throw new HeidenhainCredentialException(HeidenhainOptionsParser.CredentialRuleMessage($"{role} password") + " (fail-closed)"); - switch (kind) + try { - case HeidenhainOptionsParser.CredentialKind.None: - return ""; - case HeidenhainOptionsParser.CredentialKind.Environment: - return _environment(name) is { Length: > 0 } value - ? value - : throw new HeidenhainCredentialException($"{role} password environment variable '{name}' is not set (fail-closed)."); - default: - if (resolver is null) - throw new HeidenhainCredentialException($"{role} password references secret '{name}' but no secret store is available (fail-closed)."); - var resolved = await resolver.GetAsync(new SecretName(name), ct).ConfigureAwait(false); - return !string.IsNullOrEmpty(resolved) - ? resolved - : throw new HeidenhainCredentialException($"{role} password secret '{name}' is absent from the store (fail-closed)."); + return await CredentialReferences.ResolveAsync( + reference, + DriverCredentialPolicy.HeidenhainScope, + $"{role} password", + CredentialLiteralPolicy.Refused, + resolver is null ? null : async (name, c) => await resolver.GetAsync(new SecretName(name), c).ConfigureAwait(false), + _environment, + ct).ConfigureAwait(false); + } + catch (CredentialReferenceException ex) + { + throw new HeidenhainCredentialException(ex.Message); } } } diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Session/OpcUaUserIdentityFactory.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Session/OpcUaUserIdentityFactory.cs index 20605be4e..f1433c253 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Session/OpcUaUserIdentityFactory.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Session/OpcUaUserIdentityFactory.cs @@ -1,5 +1,6 @@ using Opc.Ua; using Opc.Ua.Security.Certificates; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; using ZB.MOM.WW.Secrets.Abstractions; namespace ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient; @@ -12,7 +13,6 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient; /// public static class OpcUaUserIdentityFactory { - private const string SecretPrefix = "secret:"; /// /// Build the user-identity token from options whose credential fields are already resolved plaintext @@ -103,25 +103,29 @@ public static class OpcUaUserIdentityFactory } /// - /// Return a copy of with any secret:-prefixed - /// / - /// resolved through . Non-secret / null / empty fields pass through unchanged (the - /// reference-equal original is returned when nothing changed). Fail-closed: an absent secret, or a - /// secret: ref with no resolver available, throws rather than sending the literal as a credential. + /// Return a copy of with a secret:opcuaclient/… or + /// env:OTOPCUA_OPCUACLIENT_… / + /// resolved (lmxopcua#815 namespace). Literal / + /// null / empty fields pass through unchanged (the reference-equal original is returned when nothing changed). + /// Fail-closed: a reference outside the namespace (or file:) throws before any read, and an absent + /// secret, an unset variable, or a secret: ref with no resolver available throws rather than sending + /// the reference string as a credential. /// /// The raw options (credential fields may carry secret: refs). /// The shared secret resolver, or null when none is available. /// Cancellation token. + /// Environment reader (tests); null ⇒ the process environment. /// An options copy whose credential fields carry resolved plaintext. public static async Task ResolveSecretRefsAsync( - OpcUaClientDriverOptions options, ISecretResolver? resolver, CancellationToken ct) + OpcUaClientDriverOptions options, ISecretResolver? resolver, CancellationToken ct, + Func? environment = null) { ArgumentNullException.ThrowIfNull(options); - var password = await ResolveFieldAsync(options.Password, nameof(options.Password), resolver, ct) + var password = await ResolveFieldAsync(options.Password, nameof(options.Password), resolver, ct, environment) .ConfigureAwait(false); var certPassword = await ResolveFieldAsync( - options.UserCertificatePassword, nameof(options.UserCertificatePassword), resolver, ct) + options.UserCertificatePassword, nameof(options.UserCertificatePassword), resolver, ct, environment) .ConfigureAwait(false); if (ReferenceEquals(password, options.Password) @@ -134,24 +138,22 @@ public static class OpcUaUserIdentityFactory } private static async Task ResolveFieldAsync( - string? value, string fieldName, ISecretResolver? resolver, CancellationToken ct) + string? value, string fieldName, ISecretResolver? resolver, CancellationToken ct, + Func? environment) { - if (string.IsNullOrEmpty(value) - || !value.StartsWith(SecretPrefix, StringComparison.OrdinalIgnoreCase)) - { - return value; - } + // A literal (or blank) passes through verbatim, reference-equal — the author already knows it. + var kind = CredentialReferences.Parse(value, DriverCredentialPolicy.OpcUaClientScope, out _); + if (kind is CredentialReferenceKind.Blank or CredentialReferenceKind.Literal) return value; - var name = value[SecretPrefix.Length..]; - if (resolver is null) - throw new InvalidOperationException( - $"OpcUaClientDriverOptions.{fieldName}='{value}' references secret '{name}', but no secret store is " + - "available here (fail-closed)."); - var resolved = await resolver.GetAsync(new SecretName(name), ct).ConfigureAwait(false); - return !string.IsNullOrEmpty(resolved) - ? resolved - : throw new InvalidOperationException( - $"OpcUaClientDriverOptions.{fieldName}='{value}' resolves secret '{name}', but it is " + - "absent from the store (fail-closed)."); + // lmxopcua#815: the value is sent to the operator-authored endpoint, so only secret:opcuaclient/… and + // env:OTOPCUA_OPCUACLIENT_… resolve; anything else (out of scope, file:) throws BEFORE any store/env read. + return await CredentialReferences.ResolveAsync( + value, + DriverCredentialPolicy.OpcUaClientScope, + $"OpcUaClientDriverOptions.{fieldName}", + CredentialLiteralPolicy.Allowed, + resolver is null ? null : async (name, c) => await resolver.GetAsync(new SecretName(name), c).ConfigureAwait(false), + environment, + ct).ConfigureAwait(false); } } diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Browsing/BrowserSessionService.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Browsing/BrowserSessionService.cs index 2121697ff..f3861c768 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Browsing/BrowserSessionService.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Browsing/BrowserSessionService.cs @@ -2,6 +2,7 @@ using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Components.Authorization; using Microsoft.Extensions.Logging; using ZB.MOM.WW.OtOpcUa.Commons.Browsing; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; using ZB.MOM.WW.OtOpcUa.Security.Auth; namespace ZB.MOM.WW.OtOpcUa.AdminUI.Browsing; @@ -36,6 +37,13 @@ public sealed class BrowserSessionService( /// public async Task OpenAsync(string driverType, string configJson, CancellationToken ct) { + // lmxopcua#815: a live browse resolves the config's credential references and sends them to the + // author-chosen endpoint — refuse an out-of-namespace reference before any browser (or the + // Discover-backed universal browser) is opened. Nothing is read from the store or environment here. + var credentialProblems = DriverCredentialPolicy.DescribeProblems(driverType, configJson); + if (credentialProblems.Count > 0) + return new(false, "Browse refused: " + string.Join(" ", credentialProblems), Guid.Empty); + if (!_browsersByType.TryGetValue(driverType, out var browser)) return await OpenUniversalAsync(driverType, configJson, ct).ConfigureAwait(false); try diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/GalaxyDriverForm.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/GalaxyDriverForm.razor index 656cdf383..865fd7339 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/GalaxyDriverForm.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/GalaxyDriverForm.razor @@ -21,8 +21,8 @@
-
Forms: env:NAME, file:PATH, dev:KEY. Cleartext is accepted but triggers a startup warning.
+ placeholder="env:OTOPCUA_GALAXY_API_KEY" /> +
Forms: secret:galaxy/NAME, env:OTOPCUA_GALAXY_NAME (or legacy env:GALAXY_MXGW_NAME), dev:KEY. Cleartext is accepted but triggers a startup warning. References outside the Galaxy namespace and file: are refused (#815).
@@ -107,7 +107,7 @@ -
Forms: secret:NAME (production), env:NAME, file:PATH, dev:VALUE. A bare literal is refused.
+
Forms: secret:galaxy/NAME or secret:otopcua/galaxy/NAME (production), env:OTOPCUA_GALAXY_NAME, dev:VALUE. A bare literal, file:, or a reference outside the Galaxy namespace is refused (#815).
@@ -220,7 +220,7 @@ } private static GalaxyDriverOptions CreateDefaultOptions() => new( - Gateway: new GalaxyGatewayOptions("https://localhost:5001", "env:MX_API_KEY"), + Gateway: new GalaxyGatewayOptions("https://localhost:5001", "env:OTOPCUA_GALAXY_API_KEY"), MxAccess: new GalaxyMxAccessOptions("OtOpcUa"), Repository: new GalaxyRepositoryOptions(), Reconnect: new GalaxyReconnectOptions()); @@ -241,7 +241,7 @@ { // GalaxyGatewayOptions public string GatewayEndpoint { get; set; } = "https://localhost:5001"; - public string GatewayApiKeySecretRef { get; set; } = "env:MX_API_KEY"; + public string GatewayApiKeySecretRef { get; set; } = "env:OTOPCUA_GALAXY_API_KEY"; public bool GatewayUseTls { get; set; } = true; public string? GatewayCaCertificatePath { get; set; } public int GatewayConnectTimeoutSeconds { get; set; } = 10; @@ -273,7 +273,7 @@ public static GalaxyFormModel FromRecord(GalaxyDriverOptions r) { - var gw = r.Gateway ?? new GalaxyGatewayOptions("https://localhost:5001", "env:MX_API_KEY"); + var gw = r.Gateway ?? new GalaxyGatewayOptions("https://localhost:5001", "env:OTOPCUA_GALAXY_API_KEY"); var mx = r.MxAccess ?? new GalaxyMxAccessOptions("OtOpcUa"); var repo = r.Repository ?? new GalaxyRepositoryOptions(); var rc = r.Reconnect ?? new GalaxyReconnectOptions(); 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..bd35b42d2 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 @@ -138,6 +138,7 @@
+
Literal, secret:opcuaclient/NAME or env:OTOPCUA_OPCUACLIENT_NAME. Other references are refused (#815).
} @if (_form.OpcUa.AuthType == OpcUaAuthType.Certificate) @@ -150,6 +151,7 @@
+
Literal, secret:opcuaclient/NAME or env:OTOPCUA_OPCUACLIENT_NAME.
} diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/DriverConfigSaveGate.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/DriverConfigSaveGate.cs index 9b46518e7..9b62b3a16 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/DriverConfigSaveGate.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/DriverConfigSaveGate.cs @@ -1,5 +1,4 @@ using ZB.MOM.WW.OtOpcUa.Core.Abstractions; -using ZB.MOM.WW.OtOpcUa.Driver.Heidenhain; namespace ZB.MOM.WW.OtOpcUa.AdminUI.Uns; @@ -10,9 +9,11 @@ namespace ZB.MOM.WW.OtOpcUa.AdminUI.Uns; /// block Save while the form is invalid. /// /// -/// Today it carries one rule: a Heidenhain password field must be blank or a reference in the Heidenhain -/// namespace () — otherwise a literal password -/// would be persisted in a draft (and replicated) even though the deploy gate would later refuse it. +/// Today it carries one rule, for every driver that resolves credential references +/// ( — Heidenhain, Galaxy, OpcUaClient): a credential field must be blank, +/// an acceptable literal for that field, or a reference inside the driver's namespace (lmxopcua#803/#815) — +/// otherwise an exfiltration-shaped reference (or a Heidenhain literal password) would be persisted in a draft +/// (and replicated) even though the deploy gate and every resolver would later refuse it. /// public static class DriverConfigSaveGate { @@ -22,8 +23,7 @@ public static class DriverConfigSaveGate /// The refusal (never containing a credential value), or null. public static string? Validate(string? driverType, string? configJson) { - if (!string.Equals(driverType, DriverTypeNames.Heidenhain, StringComparison.OrdinalIgnoreCase)) return null; - var problems = HeidenhainOptionsParser.DescribeCredentialProblems(configJson); + var problems = DriverCredentialPolicy.DescribeProblems(driverType, configJson); return problems.Count == 0 ? null : string.Join(" ", problems); } } diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/AdminOperations/AdminOperationsActor.cs b/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/AdminOperations/AdminOperationsActor.cs index d4de355e3..0f8ed08f8 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/AdminOperations/AdminOperationsActor.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/AdminOperations/AdminOperationsActor.cs @@ -585,6 +585,20 @@ public sealed class AdminOperationsActor : ReceiveActor return; } + // lmxopcua#815: refuse a credential reference outside the driver's namespace BEFORE any probe runs — the + // probe would otherwise be the seam that resolves it and sends it to the author-chosen endpoint. Nothing + // is read from the secret store or the environment on this path. + var credentialProblems = DriverCredentialPolicy.DescribeProblems(msg.DriverType, msg.ConfigJson); + if (credentialProblems.Count > 0) + { + replyTo.Tell(new TestDriverConnectResult( + false, + "Test Connect refused: " + string.Join(" ", credentialProblems), + null, + msg.CorrelationId)); + return; + } + Task.Run(() => ProbeAsync(probe, msg)).PipeTo(replyTo); } diff --git a/tests/Core/ZB.MOM.WW.OtOpcUa.Configuration.Tests/DraftValidatorCredentialReferenceTests.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Configuration.Tests/DraftValidatorCredentialReferenceTests.cs new file mode 100644 index 000000000..a682e54aa --- /dev/null +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Configuration.Tests/DraftValidatorCredentialReferenceTests.cs @@ -0,0 +1,61 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Configuration.Entities; +using ZB.MOM.WW.OtOpcUa.Configuration.Validation; + +namespace ZB.MOM.WW.OtOpcUa.Configuration.Tests; + +/// lmxopcua#815: the credential-reference namespace deploy gate (Galaxy, OpcUaClient). +[Trait("Category", "Unit")] +public sealed class DraftValidatorCredentialReferenceTests +{ + private static IReadOnlyList Validate(string type, string driverConfig, string deviceConfig = "{}") => + DraftValidator.Validate(new DraftSnapshot + { + GenerationId = 1, + ClusterId = "c", + DriverInstances = [new DriverInstance { DriverInstanceId = "drv", ClusterId = "c", Name = "drv", DriverType = type, DriverConfig = driverConfig }], + Devices = [new Device { DeviceId = "dev", DriverInstanceId = "drv", Name = "dev1", DeviceConfig = deviceConfig }], + Tags = [], + }).Where(e => e.Code == CredentialReferenceDeployChecks.Code).ToList(); + + [Theory] + [InlineData("GalaxyMxGateway", """{"Gateway":{"Endpoint":"http://evil:5001","ApiKeySecretRef":"env:ConnectionStrings__ConfigDb"}}""")] + [InlineData("GalaxyMxGateway", """{"Gateway":{"ApiKeySecretRef":"env:ServerHistorian__ApiKey"}}""")] + [InlineData("GalaxyMxGateway", """{"Gateway":{"ApiKeySecretRef":"file:C:\\otopcua\\appsettings.json"}}""")] + [InlineData("GalaxyMxGateway", """{"AlarmAck":{"FallbackPasswordSecretRef":"secret:otopcua/apikeys/ab/galaxy-ack-password"}}""")] + [InlineData("OpcUaClient", """{"EndpointUrl":"opc.tcp://evil:4840","Password":"secret:otopcua/historian/api-key"}""")] + [InlineData("OpcUaClient", """{"UserCertificatePassword":"env:ConnectionStrings__ConfigDb"}""")] + public void An_out_of_namespace_reference_is_refused_naming_the_allowed_form(string type, string config) + { + var e = Validate(type, config).ShouldHaveSingleItem(); + e.Context.ShouldBe("drv"); + e.Message.ShouldContain("secret:"); + e.Message.ShouldContain("lmxopcua#815"); + e.Message.ShouldNotContain("ConnectionStrings__ConfigDb"); + } + + [Fact] + public void A_reference_in_a_device_config_is_checked_too() + => Validate("GalaxyMxGateway", "{}", """{"Gateway":{"ApiKeySecretRef":"env:ConnectionStrings__ConfigDb"}}""") + .ShouldHaveSingleItem().Context.ShouldBe("dev"); + + [Theory] + [InlineData("GalaxyMxGateway", """{"Gateway":{"ApiKeySecretRef":"secret:galaxy/main/apikey"},"AlarmAck":{"FallbackPasswordSecretRef":"secret:otopcua/galaxy/ack-fallback"}}""")] + [InlineData("GalaxyMxGateway", """{"Gateway":{"ApiKeySecretRef":"env:GALAXY_MXGW_API_KEY"}}""")] + [InlineData("GalaxyMxGateway", """{"Gateway":{"ApiKeySecretRef":"dev:k"}}""")] + [InlineData("OpcUaClient", """{"Password":"literal","UserCertificatePassword":"env:OTOPCUA_OPCUACLIENT_CERT_PW"}""")] + [InlineData("Modbus", """{"Password":"env:ConnectionStrings__ConfigDb"}""")] + public void Namespaced_literal_and_unrelated_configs_pass(string type, string config) + => Validate(type, config).ShouldBeEmpty(); + + [Fact] + public void Heidenhain_is_reported_once_by_its_own_gate_not_twice() + => DraftValidator.Validate(new DraftSnapshot + { + GenerationId = 1, ClusterId = "c", + DriverInstances = [new DriverInstance { DriverInstanceId = "hh", ClusterId = "c", Name = "hh", DriverType = "Heidenhain", DriverConfig = """{"Access":{"InspectPassword":"env:ConnectionStrings__ConfigDb"}}""" }], + Devices = [], Tags = [], + }) + .Count(e => e.Code is CredentialReferenceDeployChecks.Code or HeidenhainDeployChecks.CredentialCode).ShouldBe(1); +} diff --git a/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests/CredentialReferencesTests.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests/CredentialReferencesTests.cs new file mode 100644 index 000000000..d387ad539 --- /dev/null +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests/CredentialReferencesTests.cs @@ -0,0 +1,134 @@ +using Shouldly; +using Xunit; + +namespace ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests; + +/// lmxopcua#815 — the shared credential-reference parser, resolver and per-driver policy table. +public sealed class CredentialReferencesTests +{ + private static readonly CredentialScope Galaxy = DriverCredentialPolicy.GalaxyScope; + + [Theory] + [InlineData(null, CredentialReferenceKind.Blank)] + [InlineData("", CredentialReferenceKind.Blank)] + [InlineData("secret:galaxy/inst/apikey", CredentialReferenceKind.Secret)] + [InlineData("${secret:galaxy/inst/apikey}", CredentialReferenceKind.Secret)] + [InlineData("secret:otopcua/galaxy/ack-fallback", CredentialReferenceKind.Secret)] + [InlineData("env:OTOPCUA_GALAXY_API_KEY", CredentialReferenceKind.Environment)] + [InlineData("env:GALAXY_MXGW_API_KEY", CredentialReferenceKind.Environment)] + [InlineData("plain-key", CredentialReferenceKind.Literal)] + [InlineData("dev:k", CredentialReferenceKind.Literal)] + [InlineData("secret:otopcua/historian/api-key", CredentialReferenceKind.OutOfScope)] + [InlineData("secret:otopcua/apikeys/ab/galaxy-ack-password", CredentialReferenceKind.OutOfScope)] + [InlineData("secret:OT-MESH-PSK-MAIN", CredentialReferenceKind.OutOfScope)] + [InlineData("secret:galaxy/", CredentialReferenceKind.OutOfScope)] + [InlineData("secret:galaxy/../otopcua/historian/api-key", CredentialReferenceKind.OutOfScope)] + [InlineData("secret:galaxy//x", CredentialReferenceKind.OutOfScope)] + [InlineData("secret:galaxyx/y", CredentialReferenceKind.OutOfScope)] + [InlineData("${secret:galaxy/x", CredentialReferenceKind.OutOfScope)] + [InlineData("SECRET:galaxy/x", CredentialReferenceKind.OutOfScope)] + [InlineData("Env:OTOPCUA_GALAXY_X", CredentialReferenceKind.OutOfScope)] + [InlineData("env:ConnectionStrings__ConfigDb", CredentialReferenceKind.OutOfScope)] + [InlineData("env:ServerHistorian__ApiKey", CredentialReferenceKind.OutOfScope)] + [InlineData("env:OTOPCUA_GALAXY_", CredentialReferenceKind.OutOfScope)] + [InlineData("env:OTOPCUA_GALAXY_lower", CredentialReferenceKind.OutOfScope)] + [InlineData("env:OTOPCUA_HEIDENHAIN_X", CredentialReferenceKind.OutOfScope)] + [InlineData("file:appsettings.json", CredentialReferenceKind.OutOfScope)] + [InlineData("FILE:x", CredentialReferenceKind.OutOfScope)] + public void Parse_classifies_against_the_galaxy_scope(string? value, CredentialReferenceKind expected) => + CredentialReferences.Parse(value, Galaxy, out _).ShouldBe(expected); + + [Fact] + public void Scopes_do_not_overlap_across_drivers() + { + CredentialReferences.Parse("secret:heidenhain/x", Galaxy, out _).ShouldBe(CredentialReferenceKind.OutOfScope); + CredentialReferences.Parse("secret:galaxy/x", DriverCredentialPolicy.OpcUaClientScope, out _) + .ShouldBe(CredentialReferenceKind.OutOfScope); + CredentialReferences.Parse("env:OTOPCUA_GALAXY_X", DriverCredentialPolicy.HeidenhainScope, out _) + .ShouldBe(CredentialReferenceKind.OutOfScope); + } + + [Theory] + [InlineData(CredentialLiteralPolicy.Allowed, "plain", true)] + [InlineData(CredentialLiteralPolicy.Refused, "plain", false)] + [InlineData(CredentialLiteralPolicy.Refused, "dev:x", false)] + [InlineData(CredentialLiteralPolicy.DevPrefixOnly, "dev:x", true)] + [InlineData(CredentialLiteralPolicy.DevPrefixOnly, "plain", false)] + [InlineData(CredentialLiteralPolicy.Allowed, "env:ConnectionStrings__ConfigDb", false)] + public void IsAcceptable_applies_the_literal_policy(CredentialLiteralPolicy policy, string value, bool expected) => + CredentialReferences.IsAcceptable(value, Galaxy, policy).ShouldBe(expected); + + /// The fail-closed core: an out-of-scope reference throws before either source is consulted. + [Theory] + [InlineData("env:ConnectionStrings__ConfigDb")] + [InlineData("secret:otopcua/historian/api-key")] + [InlineData("file:appsettings.json")] + public async Task Resolve_never_reads_for_an_out_of_scope_reference(string value) + { + var reads = 0; + var ex = await Should.ThrowAsync(async () => await CredentialReferences.ResolveAsync( + value, Galaxy, "F", CredentialLiteralPolicy.Allowed, + (_, _) => { reads++; return ValueTask.FromResult("LEAKED"); }, + _ => { reads++; return "LEAKED"; }, + CancellationToken.None)); + reads.ShouldBe(0); + ex.Message.ShouldContain("'secret:galaxy/'"); + ex.Message.ShouldContain("'env:OTOPCUA_GALAXY_'"); + ex.Message.ShouldNotContain("LEAKED"); + } + + [Fact] + public async Task Resolve_reads_scoped_references_and_fails_closed_when_absent() + { + (await CredentialReferences.ResolveAsync("secret:galaxy/a", Galaxy, "F", CredentialLiteralPolicy.Refused, + (n, _) => ValueTask.FromResult(n == "galaxy/a" ? "v" : null), _ => null, CancellationToken.None)) + .ShouldBe("v"); + (await CredentialReferences.ResolveAsync("env:OTOPCUA_GALAXY_A", Galaxy, "F", CredentialLiteralPolicy.Refused, + null, n => n == "OTOPCUA_GALAXY_A" ? "e" : null, CancellationToken.None)).ShouldBe("e"); + (await Should.ThrowAsync(async () => await CredentialReferences.ResolveAsync( + "secret:galaxy/missing", Galaxy, "F", CredentialLiteralPolicy.Refused, + (_, _) => ValueTask.FromResult(null), null, CancellationToken.None))).Message.ShouldContain("absent"); + (await Should.ThrowAsync(async () => await CredentialReferences.ResolveAsync( + "env:OTOPCUA_GALAXY_UNSET", Galaxy, "F", CredentialLiteralPolicy.Refused, + null, _ => null, CancellationToken.None))).Message.ShouldContain("not set"); + (await Should.ThrowAsync(async () => await CredentialReferences.ResolveAsync( + "secret:galaxy/a", Galaxy, "F", CredentialLiteralPolicy.Refused, + null, null, CancellationToken.None))).Message.ShouldContain("no secret store"); + } + + [Fact] + public void DescribeProblems_scans_every_depth_case_insensitively_and_never_echoes_the_value() + { + const string json = """ + { + "gateway": { "endpoint": "http://evil:5001", "apiKeySecretRef": "env:ConnectionStrings__ConfigDb" }, + "Devices": [ { "AlarmAck": { "FallbackPasswordSecretRef": "hunter2" } } ] + } + """; + var problems = DriverCredentialPolicy.DescribeProblems(DriverTypeNames.Galaxy, json); + problems.Count.ShouldBe(2); + problems.ShouldContain(p => p.StartsWith("gateway.apiKeySecretRef", StringComparison.Ordinal)); + problems.ShouldContain(p => p.StartsWith("Devices[0].AlarmAck.FallbackPasswordSecretRef", StringComparison.Ordinal)); + problems.ShouldAllBe(p => !p.Contains("hunter2") && !p.Contains("ConnectionStrings__ConfigDb")); + } + + [Theory] + [InlineData(DriverTypeNames.Galaxy, """{"Gateway":{"ApiKeySecretRef":"secret:galaxy/main/apikey"},"AlarmAck":{"FallbackPasswordSecretRef":"dev:x"}}""")] + [InlineData(DriverTypeNames.Galaxy, """{"Gateway":{"ApiKeySecretRef":"literal-key"}}""")] + [InlineData(DriverTypeNames.OpcUaClient, """{"Password":"plain","UserCertificatePassword":"secret:opcuaclient/plant/cert"}""")] + [InlineData(DriverTypeNames.Heidenhain, """{"Access":{"InspectPassword":"env:OTOPCUA_HEIDENHAIN_INSPECT"}}""")] + [InlineData(DriverTypeNames.Modbus, """{"Password":"env:ConnectionStrings__ConfigDb"}""")] + [InlineData(DriverTypeNames.Galaxy, "")] + public void DescribeProblems_accepts_namespaced_and_unrelated_configs(string driverType, string json) => + DriverCredentialPolicy.DescribeProblems(driverType, json).ShouldBeEmpty(); + + [Theory] + [InlineData(DriverTypeNames.OpcUaClient, """{"Password":"secret:galaxy/main/apikey"}""")] + [InlineData(DriverTypeNames.OpcUaClient, """{"UserCertificatePassword":"env:ServerHistorian__ApiKey"}""")] + [InlineData(DriverTypeNames.Galaxy, """{"Gateway":{"ApiKeySecretRef":"file:C:\\ProgramData\\x.txt"}}""")] + [InlineData(DriverTypeNames.Galaxy, """{"Gateway":{"ApiKeySecretRef":42}}""")] + [InlineData(DriverTypeNames.Heidenhain, """{"InspectPassword":"plain"}""")] + [InlineData(DriverTypeNames.Galaxy, "{not json")] + public void DescribeProblems_refuses_out_of_namespace_configs(string driverType, string json) => + DriverCredentialPolicy.DescribeProblems(driverType, json).ShouldNotBeEmpty(); +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests/GalaxyDriverApiKeyResolverTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests/GalaxyDriverApiKeyResolverTests.cs index 08a6bbd85..c32520531 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests/GalaxyDriverApiKeyResolverTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests/GalaxyDriverApiKeyResolverTests.cs @@ -1,14 +1,15 @@ using Microsoft.Extensions.Logging; using Shouldly; using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; using ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Config; using ZB.MOM.WW.Secrets.Abstractions; namespace ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests; /// -/// Follow-up #2 + G-2a — pins the five resolution forms supported by -/// : env:NAME, file:PATH, +/// Follow-up #2 + G-2a + lmxopcua#815 — pins the resolution forms supported by +/// : env:OTOPCUA_GALAXY_NAME (file: is refused), /// dev:KEY, secret:NAME (via the shared ), and /// the literal-string fallback. The secret: arm is fail-closed — an absent secret /// throws rather than leaking the ref string as the key. (The resolver was extracted from @@ -38,7 +39,7 @@ public sealed class GalaxyDriverApiKeyResolverTests [Fact] public async Task Env_prefix_resolves_to_environment_variable() { - const string name = "OTOPCUA_TEST_GALAXY_API_KEY"; + const string name = "OTOPCUA_GALAXY_TEST_API_KEY"; Environment.SetEnvironmentVariable(name, "key-from-env"); try { @@ -55,41 +56,13 @@ public sealed class GalaxyDriverApiKeyResolverTests [Fact] public async Task Env_prefix_unset_variable_throws_with_descriptive_message() { - const string name = "OTOPCUA_TEST_GALAXY_API_KEY_UNSET"; + const string name = "OTOPCUA_GALAXY_TEST_API_KEY_UNSET"; Environment.SetEnvironmentVariable(name, null); var ex = await Should.ThrowAsync(() => GalaxySecretRef.ResolveApiKeyAsync($"env:{name}", FakeResolver())); ex.Message.ShouldContain(name); - ex.Message.ShouldContain("unset"); - } - - /// Verifies that file: prefix resolves to trimmed file contents. - [Fact] - public async Task File_prefix_resolves_to_trimmed_file_contents() - { - var path = Path.Combine(Path.GetTempPath(), $"galaxy-key-{Guid.NewGuid():N}.txt"); - File.WriteAllText(path, " key-from-file \n"); - try - { - (await GalaxySecretRef.ResolveApiKeyAsync($"file:{path}", FakeResolver(), ct: TestContext.Current.CancellationToken)) - .ShouldBe("key-from-file"); - } - finally - { - File.Delete(path); - } - } - - /// Verifies that file: prefix with missing path throws. - [Fact] - public async Task File_prefix_missing_path_throws() - { - var path = Path.Combine(Path.GetTempPath(), $"does-not-exist-{Guid.NewGuid():N}.txt"); - var ex = await Should.ThrowAsync(() => - GalaxySecretRef.ResolveApiKeyAsync($"file:{path}", FakeResolver())); - ex.Message.ShouldContain(path); - ex.Message.ShouldContain("doesn't exist"); + ex.Message.ShouldContain("not set"); } // ===== G-2a: secret: arm — resolves through the shared ISecretResolver, fail-closed ===== @@ -161,7 +134,7 @@ public sealed class GalaxyDriverApiKeyResolverTests [Fact] public async Task Env_prefix_does_not_emit_literal_warning() { - const string name = "OTOPCUA_TEST_GALAXY_API_KEY_NOWARN"; + const string name = "OTOPCUA_GALAXY_TEST_API_KEY_NOWARN"; Environment.SetEnvironmentVariable(name, "v"); try { @@ -211,21 +184,77 @@ public sealed class GalaxyDriverApiKeyResolverTests : null); } - /// Verifies that file: prefix with empty file throws. - [Fact] - public async Task File_prefix_empty_file_throws() + // ===== lmxopcua#815: references are confined to the Galaxy namespace, checked before any read ===== + + /// + /// An out-of-namespace reference — another subsystem's secret or process variable — throws, and neither the + /// store nor the environment is consulted (the value is never read, so it can never be sent). + /// + [Theory] + [InlineData("env:ConnectionStrings__ConfigDb")] + [InlineData("env:ServerHistorian__ApiKey")] + [InlineData("env:OTOPCUA_GALAXY_lower")] + [InlineData("secret:otopcua/historian/api-key")] + [InlineData("secret:OT-MESH-PSK-MAIN")] + [InlineData("secret:galaxy/../otopcua/historian/api-key")] + [InlineData("${secret:otopcua/apikeys/abc/galaxy-ack-password}")] + [InlineData("SECRET:galaxy/inst1/apikey")] + [InlineData("file:/tmp/any-file")] + [InlineData("file:appsettings.json")] + public async Task Out_of_namespace_reference_is_refused_without_reading_store_or_environment(string reference) { - var path = Path.Combine(Path.GetTempPath(), $"galaxy-key-empty-{Guid.NewGuid():N}.txt"); - File.WriteAllText(path, " \n "); + var store = new CountingResolver(); + var envReads = 0; + var ex = await Should.ThrowAsync(() => GalaxySecretRef.ResolveApiKeyAsync( + reference, store, ct: TestContext.Current.CancellationToken, environment: _ => { envReads++; return "LEAKED"; })); + ex.Message.ShouldContain("secret:galaxy/"); + ex.Message.ShouldContain("env:OTOPCUA_GALAXY_"); + ex.Message.ShouldNotContain("LEAKED"); + store.Calls.ShouldBe(0); + envReads.ShouldBe(0); + } + + /// The namespaced environment forms (incl. the legacy GALAXY_MXGW_ variable) resolve. + [Theory] + [InlineData("env:OTOPCUA_GALAXY_API_KEY", "OTOPCUA_GALAXY_API_KEY")] + [InlineData("env:GALAXY_MXGW_API_KEY", "GALAXY_MXGW_API_KEY")] + public async Task Namespaced_environment_reference_resolves(string reference, string variable) + { + var key = await GalaxySecretRef.ResolveApiKeyAsync( + reference, FakeResolver(), ct: TestContext.Current.CancellationToken, + environment: n => n == variable ? "the-key" : null); + key.ShouldBe("the-key"); + } + + /// A file reference is refused even when the file exists — its contents are never returned. + [Fact] + public async Task File_reference_is_refused_even_when_the_file_exists() + { + var path = Path.Combine(Path.GetTempPath(), $"galaxy-key-{Guid.NewGuid():N}.txt"); + File.WriteAllText(path, "key-from-file"); try { - var ex = await Should.ThrowAsync(() => + var ex = await Should.ThrowAsync(() => GalaxySecretRef.ResolveApiKeyAsync($"file:{path}", FakeResolver())); - ex.Message.ShouldContain("empty"); + ex.Message.ShouldNotContain("key-from-file"); } finally { File.Delete(path); } } + + /// Counts store lookups. + private sealed class CountingResolver : ISecretResolver + { + /// Gets the number of lookups. + public int Calls { get; private set; } + + /// + public Task GetAsync(SecretName name, CancellationToken ct) + { + Calls++; + return Task.FromResult("LEAKED"); + } + } } diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests/Runtime/GalaxyAckCredentialTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests/Runtime/GalaxyAckCredentialTests.cs index ec932f926..4dfd7db3a 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests/Runtime/GalaxyAckCredentialTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests/Runtime/GalaxyAckCredentialTests.cs @@ -300,6 +300,48 @@ public sealed class GalaxyAckCredentialTests .Password.ShouldBe(FallbackPassword); } + /// + /// lmxopcua#815: the fallback password is sent to the operator-authored gateway endpoint, so a reference + /// outside the Galaxy namespace is refused — the store/environment value is never read, never sent. + /// + [Theory] + [InlineData("secret:otopcua/apikeys/0a1b/galaxy-ack-password")] + [InlineData("${secret:otopcua/historian/api-key}")] + [InlineData("env:ConnectionStrings__ConfigDb")] + [InlineData("file:appsettings.json")] + public async Task Resolver_refuses_a_fallback_reference_outside_the_galaxy_namespace(string reference) + { + const string Stolen = "stolen-value"; + var secrets = new DictSecretResolver + { + ["otopcua/apikeys/0a1b/galaxy-ack-password"] = Stolen, + ["otopcua/historian/api-key"] = Stolen, + }; + var envReads = 0; + var logger = new CapturingLogger(); + var resolver = new GalaxyAckCredentialResolver( + "drv", new GalaxyAlarmAckOptions("ack-svc", null, reference), secrets, logger, + _ => { envReads++; return Stolen; }); + + var result = await resolver.ResolveAsync(null, CancellationToken.None); + + result.Credential.ShouldBeNull(); + result.RefusalReason.ShouldNotBeNull().ShouldNotContain(Stolen); + logger.AllText.ShouldNotContain(Stolen); + envReads.ShouldBe(0); + } + + [Fact] + public async Task Resolver_resolves_a_namespaced_environment_fallback() + { + var resolver = new GalaxyAckCredentialResolver( + "drv", new GalaxyAlarmAckOptions("ack-svc", null, "env:OTOPCUA_GALAXY_ACK_PASSWORD"), new DictSecretResolver(), + new CapturingLogger(), n => n == "OTOPCUA_GALAXY_ACK_PASSWORD" ? FallbackPassword : null); + + (await resolver.ResolveAsync(null, CancellationToken.None)).Credential.ShouldNotBeNull() + .Password.ShouldBe(FallbackPassword); + } + [Fact] public async Task Resolver_secret_store_fault_is_a_refusal_not_a_throw() { diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Browser.Tests/OpcUaClientBrowserCertificateAuthTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Browser.Tests/OpcUaClientBrowserCertificateAuthTests.cs index a20c12d65..aedd293c2 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Browser.Tests/OpcUaClientBrowserCertificateAuthTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Browser.Tests/OpcUaClientBrowserCertificateAuthTests.cs @@ -56,11 +56,11 @@ public sealed class OpcUaClientBrowserCertificateAuthTests { AuthType = OpcUaAuthType.Certificate, UserCertificatePath = path, - UserCertificatePassword = "secret:opcua/browse-cert", + UserCertificatePassword = "secret:opcuaclient/browse-cert", }; var identity = await OpcUaClientDriverBrowser.BuildBrowseUserIdentityAsync( - opts, new OneSecret("opcua/browse-cert", "s3cret-pw"), TestContext.Current.CancellationToken); + opts, new OneSecret("opcuaclient/browse-cert", "s3cret-pw"), TestContext.Current.CancellationToken); identity.TokenType.ShouldBe(UserTokenType.Certificate); } @@ -78,7 +78,7 @@ public sealed class OpcUaClientBrowserCertificateAuthTests { AuthType = OpcUaAuthType.Username, Username = "op", - Password = "secret:opcua/pw", + Password = "secret:opcuaclient/pw", }; var ex = await Should.ThrowAsync(() => diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests/OpcUaClientSecretResolutionTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests/OpcUaClientSecretResolutionTests.cs index 642ede3d3..d319072e3 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests/OpcUaClientSecretResolutionTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests/OpcUaClientSecretResolutionTests.cs @@ -1,5 +1,6 @@ using Shouldly; using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; using ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient; using ZB.MOM.WW.Secrets.Abstractions; @@ -16,9 +17,9 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests; /// public sealed class OpcUaClientSecretResolutionTests { - private const string KnownPwName = "opcua/inst/password"; + private const string KnownPwName = "opcuaclient/inst/password"; private const string KnownPwValue = "resolved-password-plaintext"; - private const string KnownCertPwName = "opcua/inst/cert-pw"; + private const string KnownCertPwName = "opcuaclient/inst/cert-pw"; private const string KnownCertPwValue = "resolved-cert-pw-plaintext"; /// A fake resolver: returns known values for two known names, null otherwise. @@ -70,14 +71,68 @@ public sealed class OpcUaClientSecretResolutionTests [Fact] public async Task Unknown_secret_ref_is_fail_closed() { - var options = new OpcUaClientDriverOptions { Password = "secret:opcua/inst/absent" }; + var options = new OpcUaClientDriverOptions { Password = "secret:opcuaclient/inst/absent" }; var ex = await Should.ThrowAsync(() => OpcUaClientSecretResolution.ResolveSecretRefsAsync( options, FakeResolver(), CancellationToken.None)); ex.Message.ShouldContain("Password"); - ex.Message.ShouldContain("opcua/inst/absent"); + ex.Message.ShouldContain("opcuaclient/inst/absent"); + } + + /// + /// lmxopcua#815: the password is sent to the operator-authored endpoint, so a reference outside the + /// OpcUaClient namespace throws BEFORE the store or the environment is read — it is never resolved, never sent. + /// + [Theory] + [InlineData("secret:otopcua/historian/api-key")] + [InlineData("secret:galaxy/inst1/apikey")] + [InlineData("secret:OT-MESH-PSK-MAIN")] + [InlineData("env:ConnectionStrings__ConfigDb")] + [InlineData("env:ServerHistorian__ApiKey")] + [InlineData("Secret:opcuaclient/inst/password")] + [InlineData("file:appsettings.json")] + public async Task Out_of_namespace_reference_is_refused_without_reading(string reference) + { + var store = new CountingResolver(); + var envReads = 0; + foreach (var options in new[] + { + new OpcUaClientDriverOptions { Password = reference }, + new OpcUaClientDriverOptions { UserCertificatePassword = reference }, + }) + { + var ex = await Should.ThrowAsync(() => + OpcUaUserIdentityFactory.ResolveSecretRefsAsync( + options, store, CancellationToken.None, _ => { envReads++; return "LEAKED"; })); + ex.Message.ShouldContain("secret:opcuaclient/"); + ex.Message.ShouldNotContain("LEAKED"); + } + + store.Calls.ShouldBe(0); + envReads.ShouldBe(0); + } + + /// A namespaced environment reference resolves. + [Fact] + public async Task Namespaced_environment_reference_resolves() + { + var resolved = await OpcUaUserIdentityFactory.ResolveSecretRefsAsync( + new OpcUaClientDriverOptions { Password = "env:OTOPCUA_OPCUACLIENT_PLANT_PW" }, FakeResolver(), + CancellationToken.None, n => n == "OTOPCUA_OPCUACLIENT_PLANT_PW" ? "pw" : null); + resolved.Password.ShouldBe("pw"); + } + + private sealed class CountingResolver : ISecretResolver + { + public int Calls { get; private set; } + + public Task GetAsync(SecretName name, CancellationToken ct) + { + Calls++; + return Task.FromResult("LEAKED"); + } } /// A fake resolver: returns known values for two known names, null otherwise. diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Browsing/BrowserSessionCredentialScopeTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Browsing/BrowserSessionCredentialScopeTests.cs new file mode 100644 index 000000000..c22c5ce14 --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Browsing/BrowserSessionCredentialScopeTests.cs @@ -0,0 +1,78 @@ +using Microsoft.Extensions.Logging.Abstractions; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.AdminUI.Browsing; +using ZB.MOM.WW.OtOpcUa.Commons.Browsing; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; + +namespace ZB.MOM.WW.OtOpcUa.AdminUI.Tests.Browsing; + +/// +/// lmxopcua#815: the AdminUI live browse (Galaxy / OpcUaClient bespoke browsers, and the Discover-backed universal +/// browser) resolves the config's credential references and sends them to the author-chosen endpoint — an +/// out-of-namespace reference is refused before any browser is opened. +/// +[Trait("Category", "Unit")] +public sealed class BrowserSessionCredentialScopeTests +{ + [Theory] + [InlineData(DriverTypeNames.Galaxy, """{"Gateway":{"Endpoint":"http://evil:5001","ApiKeySecretRef":"env:ConnectionStrings__ConfigDb"}}""")] + [InlineData(DriverTypeNames.OpcUaClient, """{"EndpointUrl":"opc.tcp://evil:4840","Password":"secret:otopcua/historian/api-key"}""")] + [InlineData(DriverTypeNames.Heidenhain, """{"Access":{"InspectPassword":"secret:galaxy/main/apikey"}}""")] + public async Task Out_of_namespace_reference_never_opens_a_browser(string driverType, string config) + { + var bespoke = new RecordingBrowser(DriverTypeNames.Galaxy); + var bespoke2 = new RecordingBrowser(DriverTypeNames.OpcUaClient); + var universal = new RecordingUniversal(); + var svc = new BrowserSessionService( + [bespoke, bespoke2], new BrowseSessionRegistry(), NullLogger.Instance, universal, null!, null!); + + var result = await svc.OpenAsync(driverType, config, TestContext.Current.CancellationToken); + + result.Ok.ShouldBeFalse(); + result.Message.ShouldNotBeNull().ShouldContain("lmxopcua#815"); + bespoke.Opens.ShouldBe(0); + bespoke2.Opens.ShouldBe(0); + universal.Opens.ShouldBe(0); + } + + /// Positive control: a namespaced reference reaches the browser. + [Fact] + public async Task A_namespaced_reference_reaches_the_browser() + { + var bespoke = new RecordingBrowser(DriverTypeNames.Galaxy); + var svc = new BrowserSessionService( + [bespoke], new BrowseSessionRegistry(), NullLogger.Instance, new RecordingUniversal(), null!, null!); + + await svc.OpenAsync(DriverTypeNames.Galaxy, """{"Gateway":{"ApiKeySecretRef":"secret:galaxy/main/apikey"}}""", + TestContext.Current.CancellationToken); + + bespoke.Opens.ShouldBe(1); + } + + private sealed class RecordingBrowser(string driverType) : IDriverBrowser + { + public int Opens { get; private set; } + + public string DriverType => driverType; + + public Task OpenAsync(string configJson, CancellationToken cancellationToken) + { + Opens++; + throw new InvalidOperationException("opened"); + } + } + + private sealed class RecordingUniversal : IUniversalDriverBrowser + { + public int Opens { get; private set; } + + public bool CanBrowse(string driverType, string configJson) => true; + + public Task OpenAsync(string driverType, string configJson, CancellationToken ct) + { + Opens++; + throw new InvalidOperationException("opened"); + } + } +} diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/CredentialReferenceSavePathTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/CredentialReferenceSavePathTests.cs new file mode 100644 index 000000000..4b79985c4 --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/CredentialReferenceSavePathTests.cs @@ -0,0 +1,77 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.AdminUI.Uns; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; + +namespace ZB.MOM.WW.OtOpcUa.AdminUI.Tests.Uns; + +/// +/// lmxopcua#815 at the SAVE path (RawTreeService): a Galaxy / OpcUaClient credential reference outside the +/// driver's namespace never reaches the config DB, through driver and device create + update. +/// +[Trait("Category", "Unit")] +public sealed class CredentialReferenceSavePathTests +{ + private static CancellationToken Ct => TestContext.Current.CancellationToken; + + [Theory] + [InlineData(DriverTypeNames.Galaxy, """{"Gateway":{"Endpoint":"http://evil:5001","ApiKeySecretRef":"env:ConnectionStrings__ConfigDb"}}""")] + [InlineData(DriverTypeNames.Galaxy, """{"Gateway":{"ApiKeySecretRef":"file:appsettings.json"}}""")] + [InlineData(DriverTypeNames.Galaxy, """{"AlarmAck":{"FallbackPasswordSecretRef":"secret:otopcua/apikeys/ab/galaxy-ack-password"}}""")] + [InlineData(DriverTypeNames.OpcUaClient, """{"EndpointUrl":"opc.tcp://evil:4840","Password":"secret:otopcua/historian/api-key"}""")] + [InlineData(DriverTypeNames.OpcUaClient, """{"UserCertificatePassword":"env:ServerHistorian__ApiKey"}""")] + public async Task CreateDriver_refuses_an_out_of_namespace_reference_and_persists_nothing(string type, string config) + { + var name = RawTreeTestDb.SeedFresh(); + var svc = new RawTreeService(RawTreeTestDb.Factory(name)); + var result = await svc.CreateDriverAsync(RawTreeTestDb.ClusterId, null, "d815", type, config, Ct); + result.Ok.ShouldBeFalse(); + result.Error!.ShouldContain("lmxopcua#815"); + using var db = RawTreeTestDb.CreateNamed(name); + db.DriverInstances.ShouldNotContain(d => d.Name == "d815"); + } + + [Fact] + public async Task Update_paths_refuse_and_keep_the_stored_config() + { + var name = RawTreeTestDb.SeedFresh(); + var svc = new RawTreeService(RawTreeTestDb.Factory(name)); + var created = await svc.CreateDriverAsync(RawTreeTestDb.ClusterId, null, "gx", DriverTypeNames.Galaxy, + """{"Gateway":{"Endpoint":"http://gw:5001","ApiKeySecretRef":"secret:galaxy/gx/apikey"}}""", Ct); + created.Ok.ShouldBeTrue(created.Error); + + byte[] rv; + string deviceId; + byte[] deviceRv; + using (var db = RawTreeTestDb.CreateNamed(name)) + { + rv = db.DriverInstances.Single(d => d.DriverInstanceId == created.CreatedId).RowVersion; + var device = db.Devices.Single(d => d.DriverInstanceId == created.CreatedId); + deviceId = device.DeviceId; + deviceRv = device.RowVersion; + } + + (await svc.UpdateDriverAsync(created.CreatedId!, "gx", + """{"Gateway":{"ApiKeySecretRef":"env:ConnectionStrings__ConfigDb"}}""", null, rv, Ct)).Ok.ShouldBeFalse(); + (await svc.UpdateDeviceAsync(deviceId, "dev", + """{"Gateway":{"ApiKeySecretRef":"env:ConnectionStrings__ConfigDb"}}""", true, deviceRv, Ct)).Ok.ShouldBeFalse(); + (await svc.CreateDeviceAsync(created.CreatedId!, "dev2", + """{"Gateway":{"ApiKeySecretRef":"secret:otopcua/historian/api-key"}}""", Ct)).Ok.ShouldBeFalse(); + + using var check = RawTreeTestDb.CreateNamed(name); + check.DriverInstances.Single(d => d.DriverInstanceId == created.CreatedId).DriverConfig.ShouldContain("secret:galaxy/gx/apikey"); + check.Devices.Where(d => d.DriverInstanceId == created.CreatedId) + .ShouldAllBe(d => !d.DeviceConfig.Contains("ConnectionStrings") && !d.DeviceConfig.Contains("historian")); + } + + [Theory] + [InlineData(DriverTypeNames.Galaxy, """{"Gateway":{"ApiKeySecretRef":"env:GALAXY_MXGW_API_KEY"},"AlarmAck":{"FallbackPasswordSecretRef":"secret:otopcua/galaxy/ack-fallback"}}""")] + [InlineData(DriverTypeNames.OpcUaClient, """{"Password":"literal-password","UserCertificatePassword":"secret:opcuaclient/plant/cert"}""")] + public async Task Namespaced_references_save(string type, string config) + { + var name = RawTreeTestDb.SeedFresh(); + var svc = new RawTreeService(RawTreeTestDb.Factory(name)); + var result = await svc.CreateDriverAsync(RawTreeTestDb.ClusterId, null, "ok815", type, config, Ct); + result.Ok.ShouldBeTrue(result.Error); + } +} diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/AdminOperationsActorCredentialScopeTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/AdminOperationsActorCredentialScopeTests.cs new file mode 100644 index 000000000..0303f9991 --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/AdminOperationsActorCredentialScopeTests.cs @@ -0,0 +1,66 @@ +using Akka.Actor; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Commons.Messages.Admin; +using ZB.MOM.WW.OtOpcUa.ControlPlane.AdminOperations; +using ZB.MOM.WW.OtOpcUa.ControlPlane.Tests.Harness; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; + +namespace ZB.MOM.WW.OtOpcUa.ControlPlane.Tests; + +/// +/// lmxopcua#815: Test Connect with a credential reference outside the driver's namespace is refused BEFORE any +/// probe runs — the probe is the seam that would resolve the reference (store / environment) and send the value +/// to the author-chosen endpoint, so "the probe was never invoked" is "nothing was read". +/// +public sealed class AdminOperationsActorCredentialScopeTests : ControlPlaneActorTestBase +{ + [Theory] + [InlineData(DriverTypeNames.Galaxy, """{"Gateway":{"Endpoint":"http://attacker:5001","ApiKeySecretRef":"env:ConnectionStrings__ConfigDb"}}""")] + [InlineData(DriverTypeNames.Galaxy, """{"Gateway":{"Endpoint":"http://attacker:5001","ApiKeySecretRef":"env:ServerHistorian__ApiKey"}}""")] + [InlineData(DriverTypeNames.Galaxy, """{"Gateway":{"Endpoint":"http://attacker:5001","ApiKeySecretRef":"file:appsettings.json"}}""")] + [InlineData(DriverTypeNames.OpcUaClient, """{"EndpointUrl":"opc.tcp://attacker:4840","Password":"secret:otopcua/historian/api-key"}""")] + [InlineData(DriverTypeNames.Heidenhain, """{"Access":{"InspectPassword":"secret:galaxy/main/apikey"},"Devices":[{"Host":"attacker"}]}""")] + public void Out_of_namespace_reference_never_reaches_the_probe(string driverType, string config) + { + var probe = new RecordingProbe(driverType); + var actor = Sys.ActorOf(AdminOperationsActor.Props(NewInMemoryDbFactory(), CreateTestProbe("coord").Ref, new IDriverProbe[] { probe })); + + actor.Tell(new TestDriverConnect(driverType, config, 5, Guid.NewGuid())); + + var reply = ExpectMsg(PresenceBudget, cancellationToken: TestContext.Current.CancellationToken); + reply.Ok.ShouldBeFalse(); + reply.Message.ShouldNotBeNull().ShouldContain("Test Connect refused"); + reply.Message.ShouldContain("lmxopcua#815"); + probe.Calls.ShouldBe(0); + } + + /// Positive control: a namespaced reference does reach the probe (the refusal is not blanket). + [Fact] + public void A_namespaced_reference_reaches_the_probe() + { + var probe = new RecordingProbe(DriverTypeNames.Galaxy); + var actor = Sys.ActorOf(AdminOperationsActor.Props(NewInMemoryDbFactory(), CreateTestProbe("coord").Ref, new IDriverProbe[] { probe })); + + actor.Tell(new TestDriverConnect( + DriverTypeNames.Galaxy, """{"Gateway":{"Endpoint":"http://gw:5001","ApiKeySecretRef":"secret:galaxy/main/apikey"}}""", 5, Guid.NewGuid())); + + ExpectMsg(PresenceBudget, cancellationToken: TestContext.Current.CancellationToken).Ok.ShouldBeTrue(); + probe.Calls.ShouldBe(1); + } + + private sealed class RecordingProbe(string driverType) : IDriverProbe + { + private int _calls; + + public int Calls => Volatile.Read(ref _calls); + + public string DriverType => driverType; + + public Task ProbeAsync(string configJson, TimeSpan timeout, CancellationToken ct) + { + Interlocked.Increment(ref _calls); + return Task.FromResult(new DriverProbeResult(true, null, TimeSpan.Zero)); + } + } +} From 9742a66a9eabc07e99779c38fe7bb43cd253d635 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Wed, 11 Nov 2026 01:37:25 -0500 Subject: [PATCH 2/3] docs(security): credential-reference namespaces + migration note (#815) docs/security.md gains the canonical table of per-driver namespaces, the five enforcement seams, the out-of-scope cases (appsettings ${secret:}, Sql connectionStringRef) and the migration steps for existing configs. Galaxy, OpcUaClient, Heidenhain, Configuration and driver-specs docs, the Galaxy options XML docs, the drivers rule file and the docker-dev compose comment point at it. docker-dev's env:GALAXY_MXGW_API_KEY stays valid (allowed legacy prefix), so no rig change is needed. Co-Authored-By: Claude --- .claude/rules/drivers.md | 8 +++ docker-dev/docker-compose.yml | 3 +- docs/Configuration.md | 2 +- docs/drivers/Galaxy.md | 8 ++- docs/drivers/Heidenhain.md | 6 +- docs/drivers/OpcUaClient.md | 6 ++ docs/security.md | 61 +++++++++++++++++++ docs/v2/driver-specs.md | 2 +- .../GalaxyDriverOptions.cs | 28 ++++----- 9 files changed, 102 insertions(+), 22 deletions(-) diff --git a/.claude/rules/drivers.md b/.claude/rules/drivers.md index 1c2dfd3e1..ba4bf4094 100644 --- a/.claude/rules/drivers.md +++ b/.claude/rules/drivers.md @@ -319,6 +319,14 @@ host logger factory (wired in `DriverFactoryBootstrap`; the FOCAS and TwinCAT fa `BadConfigurationError` on that role's reads only (`HeidenhainCredentialPersisted`). Hardware-unproven: the in-process simulator + Java golden vectors are the evidence; `Category=LiveIntegration` (`HEIDENHAIN_LSV2_HOST`) is the commissioning gate. `docs/drivers/Heidenhain.md`. +- **Credential references are namespaced per driver (#815):** ONE table (`Core.Abstractions.DriverCredentialPolicy`) + + ONE parser/resolver (`CredentialReferences`) for Galaxy (`secret:galaxy/`·`secret:otopcua/galaxy/`·`env:OTOPCUA_GALAXY_`· + `env:GALAXY_MXGW_`), OpcUaClient (`secret:opcuaclient/`·`env:OTOPCUA_OPCUACLIENT_`) and Heidenhain. Enforced at save + (`DriverConfigSaveGate`), deploy (`CredentialReferenceOutOfScope`), Test Connect (`AdminOperationsActor`, before the + probe), browse (`BrowserSessionService`, before the browser) and at resolve time before any store/env read; `file:` is + refused everywhere. A NEW driver that resolves a `secret:`/`env:` field and sends it to an authored endpoint MUST add a + row there and resolve through `CredentialReferences` — never `ISecretResolver.GetAsync` on a raw authored name. + `docs/security.md` §"Credential references are namespaced per driver". - **Native condition state survives a full rebuild (#641):** `RebuildAddressSpace` captures each NATIVE condition's live node state (client acks included), the same-apply re-materialise restores it, and `CompleteAddressSpaceApply` (called by `OpcUaPublishActor` at the end of every apply) reports ONE event per restored off-normal condition and diff --git a/docker-dev/docker-compose.yml b/docker-dev/docker-compose.yml index 42baee8d3..abfc17977 100644 --- a/docker-dev/docker-compose.yml +++ b/docker-dev/docker-compose.yml @@ -679,7 +679,8 @@ services: Serilog__MinimumLevel__Override__Microsoft.EntityFrameworkCore: "Warning" Serilog__MinimumLevel__Override__Microsoft.AspNetCore: "Warning" # Resolved at runtime by GalaxyDriver.ResolveApiKey when a DriverInstance's - # Gateway.ApiKeySecretRef = "env:GALAXY_MXGW_API_KEY". + # Gateway.ApiKeySecretRef = "env:GALAXY_MXGW_API_KEY" (#815: GALAXY_MXGW_* is an allowed + # Galaxy env prefix alongside OTOPCUA_GALAXY_*; any other variable is refused). GALAXY_MXGW_API_KEY: "${GALAXY_MXGW_API_KEY:-mxgw_otopcua2_GI7-tNozYE6cXGUSgEzL3AHDV7bYcYIHdMwKYgyHdX4}" # Pair-local config cache + REPLICATION ON — the site-a pair is the enablement demo. # site-a-1 is the initiator: it binds the h2c sync listener on 9001 AND dials the peer. diff --git a/docs/Configuration.md b/docs/Configuration.md index b708fd53a..a9f229797 100644 --- a/docs/Configuration.md +++ b/docs/Configuration.md @@ -385,7 +385,7 @@ The Galaxy/MxAccess connection settings are **not an `appsettings` section.** Th | Record | Key fields (default) | Meaning | |---|---|---| -| `GalaxyGatewayOptions` (`Gateway`) | `Endpoint`; `ApiKeySecretRef`; `UseTls` (`true`); `CaCertificatePath` (`null`); `ConnectTimeoutSeconds` (`10`); `DefaultCallTimeoutSeconds` (`30`); `StreamTimeoutSeconds` (`0` = unlimited) | mxaccessgw gateway connection. `ApiKeySecretRef` supports `env:NAME` / `file:PATH` / `dev:KEY` / literal forms (resolved at `InitializeAsync`); prefer `env:`/`file:` in production. Never store a cleartext key. | +| `GalaxyGatewayOptions` (`Gateway`) | `Endpoint`; `ApiKeySecretRef`; `UseTls` (`true`); `CaCertificatePath` (`null`); `ConnectTimeoutSeconds` (`10`); `DefaultCallTimeoutSeconds` (`30`); `StreamTimeoutSeconds` (`0` = unlimited) | mxaccessgw gateway connection. `ApiKeySecretRef` supports `secret:galaxy/` (or `secret:otopcua/galaxy/`) / `env:OTOPCUA_GALAXY_` (or legacy `env:GALAXY_MXGW_`) / `dev:KEY` / literal forms (resolved at `InitializeAsync`); prefer `secret:` in production. Any other `secret:`/`env:` reference and every `file:` reference is refused at save, deploy (`CredentialReferenceOutOfScope`), Test Connect, browse and resolve — #815, see `docs/security.md` §"Credential references are namespaced per driver". Never store a cleartext key. | | `GalaxyMxAccessOptions` (`MxAccess`) | `ClientName`; `PublishingIntervalMs` (`1000`); `WriteUserId` (`0` = anonymous); `EventPumpChannelCapacity` (`50000`) | MXAccess client identity + tuning. `ClientName` **must be unique per OtOpcUa instance** (redundancy pairs enforce this). | | `GalaxyRepositoryOptions` (`Repository`) | `DiscoverPageSize` (`5000`); `WatchDeployEvents` (`true`) | Galaxy Repository browse paging + deploy-event watching. | | `GalaxyReconnectOptions` (`Reconnect`) | `InitialBackoffMs` (`500`); `MaxBackoffMs` (`30000`); `ReplayOnSessionLost` (`true`) | In-driver reconnect-supervisor backoff. | diff --git a/docs/drivers/Galaxy.md b/docs/drivers/Galaxy.md index 3cc617714..39a97d5b6 100644 --- a/docs/drivers/Galaxy.md +++ b/docs/drivers/Galaxy.md @@ -81,7 +81,7 @@ Project root files: `DriverConfig` JSON binds to `Config/GalaxyDriverOptions.cs`. The four sections are: -- **`Gateway`** — endpoint, API key secret ref, TLS knobs, connect/call/stream timeouts. `StreamTimeoutSeconds = 0` keeps the long-lived `StreamEvents` RPC open for the driver's lifetime. +- **`Gateway`** — endpoint, API key secret ref (`secret:galaxy/` · `secret:otopcua/galaxy/` · `env:OTOPCUA_GALAXY_` · legacy `env:GALAXY_MXGW_` · `dev:KEY` · literal; anything else, incl. `file:`, is refused — #815, [`docs/security.md`](../security.md) §"Credential references are namespaced per driver"), TLS knobs, connect/call/stream timeouts. `StreamTimeoutSeconds = 0` keeps the long-lived `StreamEvents` RPC open for the driver's lifetime. - **`MxAccess`** — `ClientName` (must be unique per OtOpcUa instance — redundancy pairs enforce uniqueness at install time), `PublishingIntervalMs` (forwarded as `buffered_update_interval_ms` on subscribe), `WriteUserId` for ArchestrA secured-write, `EventPumpChannelCapacity` (default 50_000 — one second of headroom at 50k tags / 1Hz; tune via the `galaxy.events.dropped` metric). - **`Repository`** — `DiscoverPageSize`, `WatchDeployEvents`. - **`Reconnect`** — `InitialBackoffMs`, `MaxBackoffMs`, `ReplayOnSessionLost` (calls the gateway's `ReplaySubscriptions` RPC after reconnect rather than re-issuing subscribe-bulk for every tag). @@ -103,8 +103,10 @@ Galaxy password, so `GalaxyAckCredentialResolver` picks the account per acknowle not silently downgraded to the fallback. 2. **The driver's fallback account** — `AlarmAck.FallbackUser` / `FallbackDomain` / `FallbackPasswordSecretRef` in `DriverConfig`, used by keys without an identity and by anonymous - sessions. The password ref must be `secret:NAME` / `${secret:NAME}` (production), `env:NAME`, - `file:PATH` or `dev:VALUE`; a bare literal is refused so a password never sits in DriverConfig. + sessions. The password ref must be `secret:galaxy/NAME` / `secret:otopcua/galaxy/NAME` (or the `${secret:…}` + spelling; production), `env:OTOPCUA_GALAXY_NAME` / `env:GALAXY_MXGW_NAME`, or `dev:VALUE`; a bare literal, a + `file:` reference or a reference outside the Galaxy namespace is refused (#815) — the password is sent to the + authored gateway endpoint, so it must not be able to name another subsystem's secret. 3. **Neither** → the driver throws `DriverOperationRefusedException`; **nothing is sent to the gateway**. The refusal is excluded from the resilience breaker (it says nothing about gateway health). How the client learns of it (#760): diff --git a/docs/drivers/Heidenhain.md b/docs/drivers/Heidenhain.md index 978088a90..115bb6b35 100644 --- a/docs/drivers/Heidenhain.md +++ b/docs/drivers/Heidenhain.md @@ -91,7 +91,11 @@ Host at their own machine. Anything outside the namespace is refused by **every* (`HeidenhainCredentials`, fail-closed independently of earlier validation), the AdminUI **save path** (`RawTreeService` create/update driver + device refuse the config server-side via `DriverConfigSaveGate`, and the driver/device modals disable Save while it is invalid — so a literal never reaches the config database), and the -deploy gate `HeidenhainCredentialPersisted`. Messages never echo the value. +deploy gate `HeidenhainCredentialPersisted`. Messages never echo the value. Since #815 the parser and resolver are +the shared `CredentialReferences` / `DriverCredentialPolicy` (`Core.Abstractions`) that Galaxy and OpcUaClient use +too, and Test Connect / live browse also refuse an out-of-namespace reference before any probe or browser runs; a +prefix in another casing (`SECRET:`) is refused, and `${secret:heidenhain/}` is accepted as a spelling of +`secret:heidenhain/`. Resolved **at each login**, never held in options or session state. Fail-closed: an absent secret or unset variable fails the login rather than sending the reference text. An unusable **INSPECT** credential fails the diff --git a/docs/drivers/OpcUaClient.md b/docs/drivers/OpcUaClient.md index 8b4ad31b5..2e2b2b92a 100644 --- a/docs/drivers/OpcUaClient.md +++ b/docs/drivers/OpcUaClient.md @@ -81,6 +81,12 @@ for every field (security policy/mode, auth type, session timeout, keep-alive, browse root, node/depth caps). `ReconnectPeriod` was **deleted** (#676 — unused since #670 removed the SDK reconnect handler, and the AdminUI no longer shows it); stored configs that still carry `reconnectPeriod` keep parsing in the factory, the Test-Connect probe and the browse picker (unknown keys are skipped), and the form drops it on the next save. +**Credentials (#815).** `Password` / `UserCertificatePassword` are a literal, `secret:opcuaclient/` (the node's +secret store) or `env:OTOPCUA_OPCUACLIENT_`. They are resolved on the node and sent to the configured endpoint, +so any other `secret:`/`env:` reference (and `file:`) is refused at save, deploy (`CredentialReferenceOutOfScope`), Test +Connect, browse and again at session open, before anything is read. Migrate an existing `secret:` by re-creating +the secret as `opcuaclient/`. See [`docs/security.md`](../security.md) §"Credential references are namespaced per driver". + `ProbeTimeoutSeconds` (default 15, AdminUI "Probe timeout") caps the Test-Connect probe's deadline — it can shorten the budget the host hands the probe, never extend it (review 2026-10-21 O-4; it used to be authored and consumed by nothing). diff --git a/docs/security.md b/docs/security.md index be8288f35..20e59cfa4 100644 --- a/docs/security.md +++ b/docs/security.md @@ -1144,6 +1144,67 @@ Supporting hygiene, none of which is the control: If a deployment needs the browse surface narrower than "anyone who may author config", the lever is the page policy (and the LDAP group→role mapping behind it), not the browser. +#### Credential references are namespaced per driver (#815) + +The outbound-connect capability above has a sharper edge whenever the config carries a **credential +reference**: a Designer authors both the endpoint *and* the reference, and the node resolves the reference +and sends the value to that endpoint — on Test Connect, on a live browse, and at runtime. Before #815 an +unscoped reference let anyone who may author config exfiltrate any value the admin node could read, with no +deploy: point a Galaxy `Gateway.Endpoint` at their own host, set `ApiKeySecretRef` to +`env:ConnectionStrings__ConfigDb` (or `env:ServerHistorian__ApiKey`, or `file:`), and click Test +Connect / browse; or set an OpcUaClient `Password` to `secret:`. + +Every operator-authored reference is now confined to its driver's namespace — one table, +`DriverCredentialPolicy` (`Core.Abstractions`), one parser/resolver, `CredentialReferences`: + +| Driver | Fields | Allowed references | Literal | +|---|---|---|---| +| `GalaxyMxGateway` | `Gateway.ApiKeySecretRef` | `secret:galaxy/`, `secret:otopcua/galaxy/`, `env:OTOPCUA_GALAXY_`, legacy `env:GALAXY_MXGW_` | allowed (warns), `dev:` | +| `GalaxyMxGateway` | `AlarmAck.FallbackPasswordSecretRef` | same | `dev:` only | +| `OpcUaClient` | `Password`, `UserCertificatePassword` | `secret:opcuaclient/`, `env:OTOPCUA_OPCUACLIENT_` | allowed | +| `Heidenhain` | `InspectPassword`, `DncPassword`, `DataPassword` | `secret:heidenhain/`, `env:OTOPCUA_HEIDENHAIN_` (#803) | refused | + +Rules (all drivers): the prefixes are lower-case and exact (`SECRET:` / `Env:` are refused, never treated +as a literal); `${secret:}` is accepted as a spelling of `secret:`; secret names are +`[A-Za-z0-9._-]` segments with no empty, `.` or `..` segment; the variable suffix is `[A-Z0-9_]+`; **`file:` is +refused everywhere** (Galaxy used to read any file the node could open). A field is matched at any depth of +the DriverConfig / DeviceConfig, case-insensitively, as the drivers bind it. + +Enforced, fail-closed, at every seam — the earlier ones exist for a clear message, the last one is the guarantee: + +1. **AdminUI save** — `DriverConfigSaveGate` (`RawTreeService` driver/device create + update): the config is + never stored. +2. **Deploy gate** — `CredentialReferenceDeployChecks`, code **`CredentialReferenceOutOfScope`** (Heidenhain keeps + `HeidenhainCredentialPersisted`). +3. **Test Connect** — `AdminOperationsActor` refuses before any probe runs (`"Test Connect refused: …"`). +4. **Live browse** — `BrowserSessionService.OpenAsync` refuses before any bespoke or universal browser opens. +5. **Resolve time** — `GalaxySecretRef`, `GalaxyAckCredentialResolver`, `OpcUaUserIdentityFactory` (driver + + browser) and `HeidenhainCredentials` all resolve through `CredentialReferences`, which re-checks the scope + **before** any secret-store or environment read. + +Messages name the field and the allowed forms, never the value. Out of scope by design: `${secret:}` tokens in +`appsettings.json` (host config, not operator-authored), and the `Sql` driver's `connectionStringRef`, which is +already a fixed prefix (`Sql__ConnectionStrings__`) whose value carries its own server — a Designer can +choose *which* provisioned database a ref names, but cannot redirect its credentials to another host. + +**Migration (existing configs).** After the upgrade, a stored config with an out-of-namespace reference fails +its next deploy (`CredentialReferenceOutOfScope`), its Test Connect / browse, and — for a running driver — its +next (re)initialise, which faults the driver with the same message. To migrate: + +- **Galaxy API key** — `env:GALAXY_MXGW_API_KEY` (the docker-dev rig and docs' name) keeps working. Any other + variable: rename it to `OTOPCUA_GALAXY_` on every node that runs the driver *and* the admin nodes + (Test Connect / browse), and set `env:OTOPCUA_GALAXY_`. A store secret already named `galaxy/…` keeps + working; otherwise re-create it as `galaxy//apikey` (`/admin/secrets` or the `secret` CLI). A + `file:` key must move into the store or a namespaced variable. +- **Galaxy fallback ack password** — the documented `secret:otopcua/galaxy/…` name keeps working; `env:` / `file:` + references move as above. +- **OpcUaClient** — re-create `secret:` as `opcuaclient/` (and update the field), or move an + environment value to `OTOPCUA_OPCUACLIENT_`. Literal passwords are unaffected. + +Residual (accepted): within one namespace, a Designer can still point one driver instance at another +instance's secret (e.g. `secret:galaxy//apikey`) — the value then reaches the endpoint the Designer +chose. Per-instance scoping is a possible follow-up. + ### Role grant source Admin reads `LdapGroupRoleMapping` rows from the Config DB (`src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Entities/LdapGroupRoleMapping.cs`) — the same pattern as the data-plane `NodeAcl` but scoped to Admin roles + (optionally) one cluster for multi-site fleets (a system-wide row, `IsSystemWide = true`, stacks additively with cluster-scoped rows). The `RoleGrants.razor` page lets `Administrator`s edit these mappings without leaving the UI. diff --git a/docs/v2/driver-specs.md b/docs/v2/driver-specs.md index 2501b1a8c..59bcc45e4 100644 --- a/docs/v2/driver-specs.md +++ b/docs/v2/driver-specs.md @@ -51,7 +51,7 @@ Per `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy/Config/GalaxyDriverOptions.cs`: { "Gateway": { "Endpoint": "http://localhost:5120", - "ApiKeySecretRef": "secret:galaxy-gw-api-key", + "ApiKeySecretRef": "secret:galaxy/gw/apikey", "UseTls": true, "CaCertificatePath": null, "ConnectTimeoutSeconds": 10, diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Contracts/GalaxyDriverOptions.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Contracts/GalaxyDriverOptions.cs index 56fff75ae..bca223edd 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Contracts/GalaxyDriverOptions.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Contracts/GalaxyDriverOptions.cs @@ -64,9 +64,11 @@ public sealed record GalaxyDriverOptions( /// /// /// -/// must be a reference — secret:NAME (production), -/// env:NAME, file:PATH or dev:VALUE (rigs only). A bare literal is rejected so a -/// Galaxy password can never sit in cleartext in the DriverConfig JSON / config DB. +/// must be a Galaxy-namespaced reference (lmxopcua#815) — +/// secret:galaxy/NAME / secret:otopcua/galaxy/NAME (production), env:OTOPCUA_GALAXY_NAME +/// / env:GALAXY_MXGW_NAME, or dev:VALUE (rigs only). A bare literal, file: and any other +/// reference are rejected so a Galaxy password never sits in cleartext in the config DB and the field can +/// never name another subsystem's secret. /// /// /// Galaxy user the fallback acknowledges as; null/empty = no fallback. @@ -84,20 +86,16 @@ public sealed record GalaxyAlarmAckOptions( /// /// Connection details for the MxAccess gateway. is -/// resolved by at InitializeAsync time. Five forms -/// supported, in priority order: +/// resolved by at InitializeAsync time. References are confined +/// to the Galaxy namespace (lmxopcua#815 — the key is sent to the authored Endpoint): /// -/// env:NAME — read from an environment variable (recommended for -/// production; the central config DB holds only the indirection, not the key). -/// file:PATH — read from an ACL'd file outside the repo. -/// secret:NAME — resolved through the shared ZB.MOM.WW.Secrets +/// secret:galaxy/NAME (or secret:otopcua/galaxy/NAME) — the shared ZB.MOM.WW.Secrets /// encrypted store; fail-closed if absent (the production path). -/// dev:KEY — explicit cleartext opt-in for dev rigs / parity tests; -/// no startup warning. -/// Anything else — treated as a literal cleartext API key for back-compat. -/// The resolver emits a Warning at startup so an operator who accidentally -/// committed a cleartext key sees it; production should migrate to env: -/// or file:. +/// env:OTOPCUA_GALAXY_NAME (or legacy env:GALAXY_MXGW_NAME) — an environment variable. +/// dev:KEY — explicit cleartext opt-in for dev rigs / parity tests; no startup warning. +/// Any other unprefixed string — a literal cleartext API key for back-compat, with a startup +/// Warning. +/// Any other secret:/env: reference, and every file: reference — refused. /// /// // PR 6.5 tuning notes: From 4563940792e5c59b2eb5ac75f26808ef539b91ef Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Wed, 11 Nov 2026 02:07:33 -0500 Subject: [PATCH 3/3] fix(security): warn-only compatibility for deployed drivers + upgrade preflight (#815) User decision: save, Test Connect, live browse and the deploy gate keep refusing out-of-namespace credential references, but an ALREADY-DEPLOYED driver must not fault at the upgrade restart or (Galaxy) at the next gateway reconnect, since ReopenAsync re-resolves Gateway.ApiKeySecretRef. - LegacyCredentialReferenceTracker (Core.Abstractions): the one, deprecated, call-site-chosen compatibility path. Running Galaxy (InitializeAsync, ReopenAsync, ack fallback) and OpcUaClient (session open) pass it and keep the pre-#815 semantics (case-insensitive prefixes, Galaxy file:, OpcUaClient literal pass-through) with a rate-limited Warning (driver/field/allowed form, never the value) and a credential-compat: diagnostics entry (both drivers now implement IDriverDiagnostics). Browsers and probes pass none and stay hard-fail; Heidenhain has none. - Deploy-gate message names the migration. - Upgrade preflight: CredentialReferencePreflightService (admin nodes, once after ApplicationStarted, ConfigDb only, resolves nothing) logs a Warning per Draft row / Deployed-artifact finding; the /raw banner shows them plus Running drivers reporting warn-only use (driver-health diagnostics). - docs/security.md: exact replacement table (incl. the old form default env:MX_API_KEY, file:, un-namespaced secret:, casing changes), the ReopenAsync reconnect behaviour, the warn-only window and its removal. Co-Authored-By: Claude --- .claude/rules/drivers.md | 5 + docs/drivers/Galaxy.md | 2 +- docs/drivers/OpcUaClient.md | 3 +- docs/security.md | 61 +++-- .../CredentialReferenceDeployChecks.cs | 14 +- .../LegacyCredentialReferenceTracker.cs | 82 +++++++ .../GalaxySecretRef.cs | 62 +++++- .../GalaxyDriver.cs | 21 +- .../Runtime/GalaxyAckCredentialResolver.cs | 34 ++- .../OpcUaUserIdentityFactory.cs | 39 +++- .../OpcUaClientDriver.cs | 14 +- .../OpcUaClientSecretResolution.cs | 6 +- .../Components/Pages/Raw/GlobalRaw.razor | 2 + .../Raw/CredentialReferenceBanner.razor | 85 +++++++ .../CredentialReferencePreflight.cs | 209 ++++++++++++++++++ .../EndpointRouteBuilderExtensions.cs | 4 + .../DraftValidatorCredentialReferenceTests.cs | 1 + .../GalaxyDriverBrowserTests.cs | 17 ++ .../GalaxyDriverLegacyCredentialTests.cs | 146 ++++++++++++ .../OpcUaClientSecretResolutionTests.cs | 29 +++ .../CredentialReferencePreflightTests.cs | 177 +++++++++++++++ ...dminOperationsActorCredentialScopeTests.cs | 1 + 22 files changed, 980 insertions(+), 34 deletions(-) create mode 100644 src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/LegacyCredentialReferenceTracker.cs create mode 100644 src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Raw/CredentialReferenceBanner.razor create mode 100644 src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Deployments/CredentialReferencePreflight.cs create mode 100644 tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests/GalaxyDriverLegacyCredentialTests.cs create mode 100644 tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Deployments/CredentialReferencePreflightTests.cs diff --git a/.claude/rules/drivers.md b/.claude/rules/drivers.md index ba4bf4094..9456c6b89 100644 --- a/.claude/rules/drivers.md +++ b/.claude/rules/drivers.md @@ -326,6 +326,11 @@ host logger factory (wired in `DriverFactoryBootstrap`; the FOCAS and TwinCAT fa probe), browse (`BrowserSessionService`, before the browser) and at resolve time before any store/env read; `file:` is refused everywhere. A NEW driver that resolves a `secret:`/`env:` field and sends it to an authored endpoint MUST add a row there and resolve through `CredentialReferences` — never `ISecretResolver.GetAsync` on a raw authored name. + ⚠️ **Deprecated warn-only window:** a RUNNING Galaxy (init, every `ReopenAsync` gateway reconnect, ack fallback) or + OpcUaClient (session open) driver passes its `LegacyCredentialReferenceTracker` and so still resolves a pre-#815 + out-of-namespace ref with the OLD semantics + a rate-limited Warning + a `credential-compat:` diagnostics entry; + browsers/probes pass none (hard-fail), Heidenhain never has one. Chosen by call site — do not add a global flag, and do + not hand the tracker to an admin-node path. Upgrade preflight: `CredentialReferencePreflightService` + the `/raw` banner. `docs/security.md` §"Credential references are namespaced per driver". - **Native condition state survives a full rebuild (#641):** `RebuildAddressSpace` captures each NATIVE condition's live node state (client acks included), the same-apply re-materialise restores it, and `CompleteAddressSpaceApply` diff --git a/docs/drivers/Galaxy.md b/docs/drivers/Galaxy.md index 39a97d5b6..b768aaed8 100644 --- a/docs/drivers/Galaxy.md +++ b/docs/drivers/Galaxy.md @@ -81,7 +81,7 @@ Project root files: `DriverConfig` JSON binds to `Config/GalaxyDriverOptions.cs`. The four sections are: -- **`Gateway`** — endpoint, API key secret ref (`secret:galaxy/` · `secret:otopcua/galaxy/` · `env:OTOPCUA_GALAXY_` · legacy `env:GALAXY_MXGW_` · `dev:KEY` · literal; anything else, incl. `file:`, is refused — #815, [`docs/security.md`](../security.md) §"Credential references are namespaced per driver"), TLS knobs, connect/call/stream timeouts. `StreamTimeoutSeconds = 0` keeps the long-lived `StreamEvents` RPC open for the driver's lifetime. +- **`Gateway`** — endpoint, API key secret ref (`secret:galaxy/` · `secret:otopcua/galaxy/` · `env:OTOPCUA_GALAXY_` · legacy `env:GALAXY_MXGW_` · `dev:KEY` · literal; anything else, incl. `file:`, is refused — #815 (an already-deployed driver keeps resolving a pre-#815 ref on init and every gateway reconnect in deprecated warn-only mode until its next deploy), [`docs/security.md`](../security.md) §"Credential references are namespaced per driver"), TLS knobs, connect/call/stream timeouts. `StreamTimeoutSeconds = 0` keeps the long-lived `StreamEvents` RPC open for the driver's lifetime. - **`MxAccess`** — `ClientName` (must be unique per OtOpcUa instance — redundancy pairs enforce uniqueness at install time), `PublishingIntervalMs` (forwarded as `buffered_update_interval_ms` on subscribe), `WriteUserId` for ArchestrA secured-write, `EventPumpChannelCapacity` (default 50_000 — one second of headroom at 50k tags / 1Hz; tune via the `galaxy.events.dropped` metric). - **`Repository`** — `DiscoverPageSize`, `WatchDeployEvents`. - **`Reconnect`** — `InitialBackoffMs`, `MaxBackoffMs`, `ReplayOnSessionLost` (calls the gateway's `ReplaySubscriptions` RPC after reconnect rather than re-issuing subscribe-bulk for every tag). diff --git a/docs/drivers/OpcUaClient.md b/docs/drivers/OpcUaClient.md index 2e2b2b92a..0e69a6442 100644 --- a/docs/drivers/OpcUaClient.md +++ b/docs/drivers/OpcUaClient.md @@ -84,7 +84,8 @@ factory, the Test-Connect probe and the browse picker (unknown keys are skipped) **Credentials (#815).** `Password` / `UserCertificatePassword` are a literal, `secret:opcuaclient/` (the node's secret store) or `env:OTOPCUA_OPCUACLIENT_`. They are resolved on the node and sent to the configured endpoint, so any other `secret:`/`env:` reference (and `file:`) is refused at save, deploy (`CredentialReferenceOutOfScope`), Test -Connect, browse and again at session open, before anything is read. Migrate an existing `secret:` by re-creating +Connect, browse and (for the AdminUI browser) again before anything is read; an already-deployed driver's session open +keeps the pre-#815 semantics in deprecated warn-only mode until its next deploy. Migrate an existing `secret:` by re-creating the secret as `opcuaclient/`. See [`docs/security.md`](../security.md) §"Credential references are namespaced per driver". `ProbeTimeoutSeconds` (default 15, AdminUI "Probe timeout") caps the Test-Connect probe's deadline — it can shorten diff --git a/docs/security.md b/docs/security.md index 20e59cfa4..e8b491025 100644 --- a/docs/security.md +++ b/docs/security.md @@ -1178,28 +1178,59 @@ Enforced, fail-closed, at every seam — the earlier ones exist for a clear mess `HeidenhainCredentialPersisted`). 3. **Test Connect** — `AdminOperationsActor` refuses before any probe runs (`"Test Connect refused: …"`). 4. **Live browse** — `BrowserSessionService.OpenAsync` refuses before any bespoke or universal browser opens. -5. **Resolve time** — `GalaxySecretRef`, `GalaxyAckCredentialResolver`, `OpcUaUserIdentityFactory` (driver + - browser) and `HeidenhainCredentials` all resolve through `CredentialReferences`, which re-checks the scope - **before** any secret-store or environment read. +5. **Resolve time** — `GalaxySecretRef`, `GalaxyAckCredentialResolver`, `OpcUaUserIdentityFactory` and + `HeidenhainCredentials` all resolve through `CredentialReferences`, which re-checks the scope **before** any + secret-store or environment read. This is hard-fail on every admin-node path (the Galaxy and OpcUaClient + AdminUI browsers, every Test Connect probe) and everywhere for Heidenhain. The one exception is the deprecated + warn-only compatibility path for already-deployed drivers, below. Messages name the field and the allowed forms, never the value. Out of scope by design: `${secret:}` tokens in `appsettings.json` (host config, not operator-authored), and the `Sql` driver's `connectionStringRef`, which is already a fixed prefix (`Sql__ConnectionStrings__`) whose value carries its own server — a Designer can choose *which* provisioned database a ref names, but cannot redirect its credentials to another host. -**Migration (existing configs).** After the upgrade, a stored config with an out-of-namespace reference fails -its next deploy (`CredentialReferenceOutOfScope`), its Test Connect / browse, and — for a running driver — its -next (re)initialise, which faults the driver with the same message. To migrate: +**Migration (existing configs).** A stored config with an out-of-namespace reference is listed by the upgrade +preflight (below), refused by its next deploy (`CredentialReferenceOutOfScope` — the message names the +migration), and refused by Test Connect / live browse. Values most likely to be in real config databases, with +the exact replacement (create the new secret / variable first — on every node that runs the driver **and** on +the admin nodes, which run Test Connect and browse — then edit the field): -- **Galaxy API key** — `env:GALAXY_MXGW_API_KEY` (the docker-dev rig and docs' name) keeps working. Any other - variable: rename it to `OTOPCUA_GALAXY_` on every node that runs the driver *and* the admin nodes - (Test Connect / browse), and set `env:OTOPCUA_GALAXY_`. A store secret already named `galaxy/…` keeps - working; otherwise re-create it as `galaxy//apikey` (`/admin/secrets` or the `secret` CLI). A - `file:` key must move into the store or a namespaced variable. -- **Galaxy fallback ack password** — the documented `secret:otopcua/galaxy/…` name keeps working; `env:` / `file:` - references move as above. -- **OpcUaClient** — re-create `secret:` as `opcuaclient/` (and update the field), or move an - environment value to `OTOPCUA_OPCUACLIENT_`. Literal passwords are unaffected. +| Old value | Why it is refused | Replace with | +|---|---|---| +| Galaxy `env:MX_API_KEY` — the **old AdminUI Galaxy form default** (pre-#815 `GalaxyDriverForm.razor`), so the likeliest value in a real DB | variable outside the namespace | rename the variable to `OTOPCUA_GALAXY_API_KEY` and set `env:OTOPCUA_GALAXY_API_KEY` (or store it and use `secret:galaxy//apikey`) | +| Galaxy `env:` (e.g. `env:MY_API_KEY`) | same | variable `OTOPCUA_GALAXY_` → `env:OTOPCUA_GALAXY_`. `env:GALAXY_MXGW_API_KEY` (docker-dev, parity docs) needs **no** change | +| Galaxy `file:` — `docs/Configuration.md` used to recommend it for production | `file:` is refused everywhere | put the file's contents in the store as `galaxy//apikey` → `secret:galaxy//apikey` (or a variable `OTOPCUA_GALAXY_`) | +| Galaxy `secret:` not under `galaxy/` (e.g. the old `docs/v2/driver-specs.md` example `secret:galaxy-gw-api-key`) | secret outside the namespace | re-create it as `galaxy/` (e.g. `galaxy/gw/apikey`) → `secret:galaxy/gw/apikey`. Names already under `galaxy/` or `otopcua/galaxy/` need no change | +| Galaxy `Env:X`, `SECRET:x`, `File:x`, … (any non-lower-case prefix) — **pre-#815 resolved these case-insensitively** | prefixes are now exact | the lower-case namespaced form from the rows above | +| Galaxy `AlarmAck.FallbackPasswordSecretRef` = `env:` / `file:` / `secret:` outside the namespace | same | as above; the documented `secret:otopcua/galaxy/` needs no change | +| OpcUaClient `Password` / `UserCertificatePassword` = `secret:` not under `opcuaclient/` (e.g. `secret:opcua/plant/pw`) | secret outside the namespace | re-create the secret as `opcuaclient/` → `secret:opcuaclient/` | +| OpcUaClient `SECRET:x` / `Secret:x` — pre-#815 resolved case-insensitively | prefixes are now exact | `secret:opcuaclient/` | +| OpcUaClient **literal** password that happens to start with `env:`, `file:`, `secret:`/`${secret:` in any casing (e.g. a real password `Env:Prod1`) — pre-#815 sent it verbatim | it now parses as a reference | store it: `secret:opcuaclient/` (a literal that does not start with one of those prefixes is unaffected) | + +**Warn-only compatibility for already-deployed drivers (⚠️ deprecated — a later release fails closed).** Refusing +at resolve time would fault running plant drivers at the upgrade restart — and, for Galaxy, at **every gateway +reconnect**: `GalaxyDriver.ReopenAsync` (the `ReconnectSupervisor` callback) rebuilds the client options through +`BuildClientOptionsAsync`, which re-resolves `Gateway.ApiKeySecretRef` each time, so a running driver would fault +at its next reconnect, not at a deploy. So, for one compatibility window, the running-driver call sites — Galaxy +`InitializeAsync` / `ReopenAsync` / the ack-fallback resolver, and OpcUaClient session open — resolve an +out-of-namespace reference with the **old** semantics (Galaxy: case-insensitive `env:`/`file:`/`dev:`/`secret:`, +literal otherwise; OpcUaClient: case-insensitive `secret:`, literal otherwise) and log a Warning naming the +driver, field and allowed form (never the value), at most once per field per hour. The use also rides the +driver's diagnostics bag (`credential-compat:`) to `/hosts` and to the `/raw` credential banner. This is +chosen **by call site**, not by a flag (`LegacyCredentialReferenceTracker`): the AdminUI browsers and every probe +never get the tracker and stay hard-fail, and Heidenhain (new driver, no legacy configs) is hard-fail everywhere. +It cannot be used to exfiltrate: save, Test Connect, browse and the deploy gate all refuse such a reference, so it +can only be the reference an administrator deployed **before** the upgrade, and it lives only until that driver's +next successful deploy (which requires migrating it). The hard-fail release deletes the tracker, the per-driver +`Legacy…` resolve methods and their `legacy` parameters — nothing else changes. + +**Upgrade preflight.** On an admin node, `CredentialReferencePreflightService` runs once, 15 s after +`ApplicationStarted` (never inside an Akka starter, and it resolves nothing — ConfigDb reads only). It scans +every credential-resolving driver's stored `DriverConfig` / `DeviceConfig` rows (**Draft**) and the latest +deployment artifact (**Deployed** — what the running drivers were given) and logs one Warning per finding: +driver, device, field, allowed form, never the value. The same list, plus running drivers that reported a +warn-only resolution (**Running**, from driver-health diagnostics, fleet-wide), is shown as a banner on `/raw`, +with a Re-check button. Residual (accepted): within one namespace, a Designer can still point one driver instance at another instance's secret (e.g. `secret:galaxy//apikey`) — the value then reaches the endpoint the Designer diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Validation/CredentialReferenceDeployChecks.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Validation/CredentialReferenceDeployChecks.cs index 5badc1b7f..6b62bf087 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Validation/CredentialReferenceDeployChecks.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Validation/CredentialReferenceDeployChecks.cs @@ -20,6 +20,16 @@ public static class CredentialReferenceDeployChecks /// A credential field holding a reference outside the driver's namespace (or a refused literal). public const string Code = "CredentialReferenceOutOfScope"; + /// + /// Appended to every error: a config deployed before #815 is still RUNNING in warn-only compatibility mode, and + /// this deploy is where it stops — so the message names the migration, not just the rule. + /// + public const string MigrationHint = + " Migrate it before deploying: create the value under the allowed name (secret store via /admin/secrets or the " + + "secret CLI, or the environment variable on every node that runs this driver AND the admin nodes) and point the " + + "field at it. A driver deployed before this upgrade keeps resolving the old reference (warn-only, deprecated) " + + "only until this deploy succeeds. See docs/security.md §\"Credential references are namespaced per driver\"."; + /// Runs the gate. /// The draft. /// The errors. @@ -32,12 +42,12 @@ public static class CredentialReferenceDeployChecks if (!DriverCredentialPolicy.TryGet(driver.DriverType, out _)) continue; foreach (var problem in DriverCredentialPolicy.DescribeProblems(driver.DriverType, driver.DriverConfig)) - errors.Add(new(Code, $"{driver.DriverType} driver '{driver.Name}': {problem}", driver.DriverInstanceId)); + errors.Add(new(Code, $"{driver.DriverType} driver '{driver.Name}': {problem}{MigrationHint}", driver.DriverInstanceId)); foreach (var device in draft.Devices.Where(d => string.Equals(d.DriverInstanceId, driver.DriverInstanceId, StringComparison.Ordinal))) { foreach (var problem in DriverCredentialPolicy.DescribeProblems(driver.DriverType, device.DeviceConfig)) - errors.Add(new(Code, $"{driver.DriverType} device '{device.Name}': {problem}", device.DeviceId)); + errors.Add(new(Code, $"{driver.DriverType} device '{device.Name}': {problem}{MigrationHint}", device.DeviceId)); } } diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/LegacyCredentialReferenceTracker.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/LegacyCredentialReferenceTracker.cs new file mode 100644 index 000000000..4138e552f --- /dev/null +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/LegacyCredentialReferenceTracker.cs @@ -0,0 +1,82 @@ +namespace ZB.MOM.WW.OtOpcUa.Core.Abstractions; + +/// +/// ⚠️ DEPRECATED compatibility path (lmxopcua#815) — the ONE switch between "enforce" and "warn-only" credential +/// resolution, and it is chosen by CALL SITE, never by a global flag: a deployed, running driver (Galaxy +/// InitializeAsync/ReopenAsync + ack fallback, OpcUaClient session open) passes its instance's tracker +/// and so keeps resolving a pre-#815 out-of-namespace reference with its OLD semantics, warning instead of +/// faulting; every admin-node path (Test Connect probe, AdminUI live browse) and Heidenhain pass +/// and stay hard-fail. +/// +/// +/// +/// Why it is safe to keep for running drivers: the AdminUI save path and the deploy gate refuse every +/// out-of-namespace reference, so a Designer can no longer CREATE one — what remains are configs deployed +/// before the upgrade, whose endpoint and reference an administrator already deployed. Such a config runs +/// only until its next deploy (which the gate refuses until it is migrated). +/// +/// +/// Removing it (the planned hard-fail release): delete this type, the per-driver Legacy… resolve +/// methods it gates and the legacy parameters; every caller then takes the enforce branch it already +/// has. Nothing else changes. +/// +/// +/// Each use is reported through at most once per per field +/// (a Galaxy gateway reconnect re-resolves every time), and is exposed as so the +/// driver's bag carries it to the AdminUI (/hosts and the /raw +/// credential banner). Messages name the driver, field and allowed forms — never the value. +/// +/// +/// The driver's scope (for the allowed forms). +/// The driver instance (for messages). +/// Sink for a rate-limited warning line. +/// Clock (tests); null ⇒ . +public sealed class LegacyCredentialReferenceTracker( + CredentialScope scope, string driverInstanceId, Action warn, Func? clock = null) +{ + /// The key prefix (one entry per field). + public const string DiagnosticsKeyPrefix = "credential-compat:"; + + /// Minimum spacing between two warnings for the same field. + public static readonly TimeSpan WarnInterval = TimeSpan.FromHours(1); + + private readonly object _gate = new(); + private readonly Dictionary _lastWarned = new(StringComparer.Ordinal); + private readonly SortedDictionary _diagnostics = new(StringComparer.Ordinal); + + /// The scope this tracker reports against. + public CredentialScope Scope => scope; + + /// The current legacy uses, keyed credential-compat:<field>. + public IReadOnlyDictionary Diagnostics + { + get + { + lock (_gate) return new Dictionary(_diagnostics, StringComparer.Ordinal); + } + } + + /// Records that was resolved through the legacy path (warns, rate-limited). + /// The field label. + public void Report(string field) + { + var forms = $"secret:{scope.SecretNamespaces[0]} or env:{scope.EnvVariablePrefixes[0]}"; + var now = (clock ?? (() => DateTime.UtcNow))(); + bool log; + lock (_gate) + { + _diagnostics[DiagnosticsKeyPrefix + field] = + $"out-of-namespace ref still resolved (#815 warn-only, deprecated); next deploy refuses it — use {forms}"; + log = !_lastWarned.TryGetValue(field, out var last) || now - last >= WarnInterval; + if (log) _lastWarned[field] = now; + } + + if (log) + { + warn($"{scope.Label} driver '{driverInstanceId}': {field} is a credential reference outside the {scope.Label} " + + $"namespace. It is still resolved for this already-deployed driver (lmxopcua#815 warn-only " + + $"compatibility — deprecated, a later release fails closed) but the deploy gate refuses it: migrate " + + $"it to {forms} before the next deploy. See docs/security.md §\"Credential references are namespaced per driver\"."); + } + } +} diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Contracts/GalaxySecretRef.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Contracts/GalaxySecretRef.cs index 5e685ad6d..7af820c19 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Contracts/GalaxySecretRef.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Contracts/GalaxySecretRef.cs @@ -36,19 +36,32 @@ public static class GalaxySecretRef /// Optional logger for warning on cleartext keys. /// Cancellation token for the async secret: resolution. /// Environment reader (tests); null ⇒ the process environment. + /// + /// ⚠️ Deprecated #815 compatibility: the RUNNING driver passes its tracker, so a pre-#815 out-of-namespace + /// reference is resolved with the old semantics and warned about instead of failing. The AdminUI browser + /// passes null (hard-fail). See . + /// /// The resolved API-key string. public static async Task ResolveApiKeyAsync( string secretRef, ISecretResolver resolver, ILogger? logger = null, CancellationToken ct = default, - Func? environment = null) + Func? environment = null, + LegacyCredentialReferenceTracker? legacy = null) { ArgumentException.ThrowIfNullOrEmpty(secretRef); ArgumentNullException.ThrowIfNull(resolver); var scope = DriverCredentialPolicy.GalaxyScope; - if (CredentialReferences.Parse(secretRef, scope, out _) == CredentialReferenceKind.Literal) + var kind = CredentialReferences.Parse(secretRef, scope, out _); + if (kind == CredentialReferenceKind.OutOfScope && legacy is not null) + { + legacy.Report("Gateway.ApiKeySecretRef"); + return await LegacyResolveApiKeyAsync(secretRef, resolver, logger, ct, environment).ConfigureAwait(false); + } + + if (kind == CredentialReferenceKind.Literal) { if (secretRef.StartsWith(CredentialReferences.DevPrefix, StringComparison.OrdinalIgnoreCase)) { @@ -74,4 +87,49 @@ public static class GalaxySecretRef environment, ct).ConfigureAwait(false); } + + /// + /// ⚠️ DEPRECATED (lmxopcua#815 warn-only compatibility; delete in the hard-fail release): the pre-#815 resolver, + /// verbatim — case-insensitive env: / file: / dev: / secret:, anything else a + /// literal. Reached ONLY for an out-of-namespace reference in an already-deployed, running driver. + /// + private static async Task LegacyResolveApiKeyAsync( + string secretRef, ISecretResolver resolver, ILogger? logger, CancellationToken ct, Func? environment) + { + if (secretRef.StartsWith("env:", StringComparison.OrdinalIgnoreCase)) + { + var name = secretRef[4..]; + var value = (environment ?? Environment.GetEnvironmentVariable)(name); + return !string.IsNullOrEmpty(value) + ? value + : throw new InvalidOperationException( + $"Galaxy.Gateway.ApiKeySecretRef resolves to env var '{name}', but it is unset."); + } + + if (secretRef.StartsWith("file:", StringComparison.OrdinalIgnoreCase)) + { + var path = secretRef[5..]; + if (!File.Exists(path)) + throw new InvalidOperationException($"Galaxy.Gateway.ApiKeySecretRef points at '{path}', which doesn't exist."); + var contents = (await File.ReadAllTextAsync(path, ct).ConfigureAwait(false)).Trim(); + return !string.IsNullOrEmpty(contents) + ? contents + : throw new InvalidOperationException($"Galaxy.Gateway.ApiKeySecretRef file '{path}' is empty."); + } + + if (secretRef.StartsWith("dev:", StringComparison.OrdinalIgnoreCase)) return secretRef[4..]; + + if (secretRef.StartsWith("secret:", StringComparison.OrdinalIgnoreCase)) + { + var name = secretRef["secret:".Length..]; + var value = await resolver.GetAsync(new SecretName(name), ct).ConfigureAwait(false); + return !string.IsNullOrEmpty(value) + ? value + : throw new InvalidOperationException( + $"Galaxy.Gateway.ApiKeySecretRef resolves secret '{name}', but it is absent from the store (fail-closed)."); + } + + logger?.LogWarning("Galaxy.Gateway.ApiKeySecretRef is being treated as a literal cleartext API key."); + return secretRef; + } } diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy/GalaxyDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy/GalaxyDriver.cs index 72e5b4a70..d1be1bb04 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy/GalaxyDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy/GalaxyDriver.cs @@ -38,8 +38,17 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Galaxy; /// /// public sealed class GalaxyDriver - : IDriver, ITagDiscovery, IReadable, IWritable, ISubscribable, IRediscoverable, IHostConnectivityProbe, IAlarmSource, IAlarmAcknowledgePreflight, ISkippedTagReporter, IDisposable, IAsyncDisposable + : IDriver, ITagDiscovery, IReadable, IWritable, ISubscribable, IRediscoverable, IHostConnectivityProbe, IAlarmSource, IAlarmAcknowledgePreflight, ISkippedTagReporter, IDriverDiagnostics, IDisposable, IAsyncDisposable { + // ⚠️ lmxopcua#815 warn-only compatibility (deprecated): this RUNNING driver keeps resolving a pre-#815 + // out-of-namespace credential reference (API key on InitializeAsync AND every gateway ReopenAsync, plus the + // ack fallback password) with the old semantics, warning rate-limited and surfacing it on the diagnostics bag. + // The AdminUI browser and the probe never get a tracker, so they stay hard-fail. + private readonly LegacyCredentialReferenceTracker _legacyCredentials; + + /// + IReadOnlyDictionary IDriverDiagnostics.GetDiagnostics() => _legacyCredentials.Diagnostics; + /// Authored tags this driver could not dial — a TagConfig with no usable /// attributeRef has no MXAccess address to bind. private readonly SkippedTagLedger _skippedTags = new(); @@ -342,6 +351,8 @@ public sealed class GalaxyDriver _options = options ?? throw new ArgumentNullException(nameof(options)); _logger = logger ?? NullLogger.Instance; _secretResolver = secretResolver ?? NullSecretResolver.Instance; + _legacyCredentials = new LegacyCredentialReferenceTracker( + DriverCredentialPolicy.GalaxyScope, _driverInstanceId, message => _logger.LogWarning("{CredentialCompatWarning}", message)); _hierarchySource = hierarchySource; _dataReader = dataReader; _dataWriter = dataWriter; @@ -349,7 +360,7 @@ public sealed class GalaxyDriver _alarmAcknowledger = alarmAcknowledger; _alarmFeed = alarmFeed; _ackCredentialResolver = ackCredentialResolver - ?? new GalaxyAckCredentialResolver(_driverInstanceId, _options.AlarmAck, _secretResolver, _logger); + ?? new GalaxyAckCredentialResolver(_driverInstanceId, _options.AlarmAck, _secretResolver, _logger, legacy: _legacyCredentials); // Build the v3 RawPath ⇄ attributeRef tables from the authored raw tags. Empty when no tags // are authored (live-browse path), leaving every translation an identity no-op. @@ -1098,7 +1109,9 @@ public sealed class GalaxyDriver } } - private async Task BuildClientOptionsAsync( + // Internal: the ONE client-options build InitializeAsync, ReopenAsync (every gateway reconnect) and the + // repository/alarm clients share — the #815 warn-only tests drive it directly to prove a reconnect re-resolves. + internal async Task BuildClientOptionsAsync( GalaxyGatewayOptions gw, CancellationToken cancellationToken) { // Resolve the API-key ref BEFORE the object initializer — the secret: arm is @@ -1108,7 +1121,7 @@ public sealed class GalaxyDriver // (GalaxySecretRef) so the runtime driver and the AdminUI browser share one // implementation; the secret: arm resolves through the shared ISecretResolver. var apiKey = await GalaxySecretRef - .ResolveApiKeyAsync(gw.ApiKeySecretRef, _secretResolver, _logger, cancellationToken) + .ResolveApiKeyAsync(gw.ApiKeySecretRef, _secretResolver, _logger, cancellationToken, legacy: _legacyCredentials) .ConfigureAwait(false); return new MxGatewayClientOptions { diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy/Runtime/GalaxyAckCredentialResolver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy/Runtime/GalaxyAckCredentialResolver.cs index 77b2ad751..6dc58a0a7 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy/Runtime/GalaxyAckCredentialResolver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy/Runtime/GalaxyAckCredentialResolver.cs @@ -22,6 +22,7 @@ internal sealed class GalaxyAckCredentialResolver : IGalaxyAckCredentialResolver private readonly ISecretResolver _secrets; private readonly ILogger _logger; private readonly Func? _environment; + private readonly LegacyCredentialReferenceTracker? _legacy; /// Initializes a new instance of the class. /// The owning driver instance id (for messages). @@ -29,11 +30,13 @@ internal sealed class GalaxyAckCredentialResolver : IGalaxyAckCredentialResolver /// The shared secret resolver. /// Logger; never receives a secret. /// Environment reader (tests); null ⇒ the process environment. + /// ⚠️ #815 warn-only compatibility tracker of the running driver (deprecated); null ⇒ hard-fail. public GalaxyAckCredentialResolver( string driverInstanceId, GalaxyAlarmAckOptions? options, ISecretResolver secrets, ILogger logger, - Func? environment = null) + Func? environment = null, LegacyCredentialReferenceTracker? legacy = null) { _environment = environment; + _legacy = legacy; _driverInstanceId = driverInstanceId; _options = options ?? new GalaxyAlarmAckOptions(); _secrets = secrets ?? throw new ArgumentNullException(nameof(secrets)); @@ -156,6 +159,13 @@ internal sealed class GalaxyAckCredentialResolver : IGalaxyAckCredentialResolver /// internal async Task ResolvePasswordRefAsync(string reference, CancellationToken ct) { + if (_legacy is not null + && CredentialReferences.Parse(reference, DriverCredentialPolicy.GalaxyScope, out _) == CredentialReferenceKind.OutOfScope) + { + _legacy.Report("AlarmAck.FallbackPasswordSecretRef"); + return await LegacyResolvePasswordRefAsync(reference, ct).ConfigureAwait(false); + } + var resolved = await CredentialReferences.ResolveOrNullAsync( reference, DriverCredentialPolicy.GalaxyScope, @@ -169,6 +179,28 @@ internal sealed class GalaxyAckCredentialResolver : IGalaxyAckCredentialResolver : resolved; } + /// + /// ⚠️ DEPRECATED (lmxopcua#815 warn-only compatibility; delete in the hard-fail release): the pre-#815 resolver, + /// verbatim. Reached ONLY for an out-of-namespace reference in an already-deployed, running driver. + /// + private async Task LegacyResolvePasswordRefAsync(string reference, CancellationToken ct) + { + if (reference.StartsWith("${secret:", StringComparison.OrdinalIgnoreCase) && reference.EndsWith('}')) + reference = "secret:" + reference["${secret:".Length..^1]; + if (reference.StartsWith("secret:", StringComparison.OrdinalIgnoreCase)) + return await _secrets.GetAsync(new SecretName(reference["secret:".Length..]), ct).ConfigureAwait(false); + if (reference.StartsWith("env:", StringComparison.OrdinalIgnoreCase)) + return (_environment ?? Environment.GetEnvironmentVariable)(reference[4..]); + if (reference.StartsWith("file:", StringComparison.OrdinalIgnoreCase)) + { + var path = reference[5..]; + return File.Exists(path) ? (await File.ReadAllTextAsync(path, ct).ConfigureAwait(false)).TrimEnd('\r', '\n') : null; + } + + if (reference.StartsWith("dev:", StringComparison.OrdinalIgnoreCase)) return reference[4..]; + throw new FormatException("AlarmAck.FallbackPasswordSecretRef must be a secret:/env:/dev: reference, not a literal."); + } + // Safe-to-print form of a reference: the kind + name for secret/env/file, never the dev: value or a literal. internal static string DescribeRef(string reference) => CredentialReferences.Describe(reference); } diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Session/OpcUaUserIdentityFactory.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Session/OpcUaUserIdentityFactory.cs index f1433c253..e3e111f90 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Session/OpcUaUserIdentityFactory.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Session/OpcUaUserIdentityFactory.cs @@ -115,17 +115,22 @@ public static class OpcUaUserIdentityFactory /// The shared secret resolver, or null when none is available. /// Cancellation token. /// Environment reader (tests); null ⇒ the process environment. + /// + /// ⚠️ Deprecated #815 compatibility: the RUNNING driver's session open passes its tracker, so a pre-#815 + /// out-of-namespace value keeps the old semantics (a case-insensitive secret: resolves, anything else is + /// a literal) with a rate-limited warning. The AdminUI browser passes null (hard-fail). + /// /// An options copy whose credential fields carry resolved plaintext. public static async Task ResolveSecretRefsAsync( OpcUaClientDriverOptions options, ISecretResolver? resolver, CancellationToken ct, - Func? environment = null) + Func? environment = null, LegacyCredentialReferenceTracker? legacy = null) { ArgumentNullException.ThrowIfNull(options); - var password = await ResolveFieldAsync(options.Password, nameof(options.Password), resolver, ct, environment) + var password = await ResolveFieldAsync(options.Password, nameof(options.Password), resolver, ct, environment, legacy) .ConfigureAwait(false); var certPassword = await ResolveFieldAsync( - options.UserCertificatePassword, nameof(options.UserCertificatePassword), resolver, ct, environment) + options.UserCertificatePassword, nameof(options.UserCertificatePassword), resolver, ct, environment, legacy) .ConfigureAwait(false); if (ReferenceEquals(password, options.Password) @@ -139,12 +144,18 @@ public static class OpcUaUserIdentityFactory private static async Task ResolveFieldAsync( string? value, string fieldName, ISecretResolver? resolver, CancellationToken ct, - Func? environment) + Func? environment, LegacyCredentialReferenceTracker? legacy) { // A literal (or blank) passes through verbatim, reference-equal — the author already knows it. var kind = CredentialReferences.Parse(value, DriverCredentialPolicy.OpcUaClientScope, out _); if (kind is CredentialReferenceKind.Blank or CredentialReferenceKind.Literal) return value; + if (kind == CredentialReferenceKind.OutOfScope && legacy is not null) + { + legacy.Report(fieldName); + return await LegacyResolveFieldAsync(value!, fieldName, resolver, ct).ConfigureAwait(false); + } + // lmxopcua#815: the value is sent to the operator-authored endpoint, so only secret:opcuaclient/… and // env:OTOPCUA_OPCUACLIENT_… resolve; anything else (out of scope, file:) throws BEFORE any store/env read. return await CredentialReferences.ResolveAsync( @@ -156,4 +167,24 @@ public static class OpcUaUserIdentityFactory environment, ct).ConfigureAwait(false); } + + /// + /// ⚠️ DEPRECATED (lmxopcua#815 warn-only compatibility; delete in the hard-fail release): the pre-#815 resolver, + /// verbatim — a case-insensitive secret: resolves (fail-closed), anything else passes through as a + /// literal. Reached ONLY for an out-of-namespace value in an already-deployed, running driver. + /// + private static async Task LegacyResolveFieldAsync( + string value, string fieldName, ISecretResolver? resolver, CancellationToken ct) + { + if (!value.StartsWith(CredentialReferences.SecretPrefix, StringComparison.OrdinalIgnoreCase)) return value; + var name = value[CredentialReferences.SecretPrefix.Length..]; + if (resolver is null) + throw new InvalidOperationException( + $"OpcUaClientDriverOptions.{fieldName} references secret '{name}', but no secret store is available here (fail-closed)."); + var resolved = await resolver.GetAsync(new SecretName(name), ct).ConfigureAwait(false); + return !string.IsNullOrEmpty(resolved) + ? resolved + : throw new InvalidOperationException( + $"OpcUaClientDriverOptions.{fieldName} resolves secret '{name}', but it is absent from the store (fail-closed)."); + } } 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..150a131e1 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDriver.cs @@ -29,8 +29,16 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient; /// back on its own recovers in place. /// /// -public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWritable, ISubscribable, IHostConnectivityProbe, IAlarmSource, ISkippedTagReporter, IDriverConnectionLossSource, IDisposable, IAsyncDisposable +public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWritable, ISubscribable, IHostConnectivityProbe, IAlarmSource, ISkippedTagReporter, IDriverConnectionLossSource, IDriverDiagnostics, IDisposable, IAsyncDisposable { + // ⚠️ lmxopcua#815 warn-only compatibility (deprecated): the session open of this RUNNING driver keeps + // resolving a pre-#815 out-of-namespace credential with the old semantics, warning rate-limited and surfacing + // it on the diagnostics bag. The AdminUI browser never gets a tracker, so it stays hard-fail. + private readonly LegacyCredentialReferenceTracker _legacyCredentials; + + /// + IReadOnlyDictionary IDriverDiagnostics.GetDiagnostics() => _legacyCredentials.Diagnostics; + /// Authored tags whose TagConfig carried no usable nodeId, so there is no upstream node /// to dial. This driver's skip path was the quietest of the twelve: NamespaceMap /// drops such an entry with a bare continue and not even a log line. @@ -82,6 +90,8 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit _diag = new OpcUaClientDiagnostics(driverInstanceId); _logger = logger ?? NullLogger.Instance; _secretResolver = secretResolver ?? NullSecretResolver.Instance; + _legacyCredentials = new LegacyCredentialReferenceTracker( + DriverCredentialPolicy.OpcUaClientScope, driverInstanceId, message => _logger.LogWarning("{CredentialCompatWarning}", message)); _sdkTelemetry = logger is null ? SdkTelemetry.Context : SdkTelemetry.ForLogger(logger); PostRecoveryReconcileDelay = () => _options.Timeout + _options.KeepAliveInterval; } @@ -554,7 +564,7 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit // in BuildCertificateIdentity — flow through BuildUserIdentity(resolvedOptions), so a // single resolve covers both paths. var resolvedOptions = await OpcUaClientSecretResolution - .ResolveSecretRefsAsync(_options, _secretResolver, cancellationToken).ConfigureAwait(false); + .ResolveSecretRefsAsync(_options, _secretResolver, cancellationToken, _legacyCredentials).ConfigureAwait(false); var identity = BuildUserIdentity(resolvedOptions); diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientSecretResolution.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientSecretResolution.cs index bda9a35dd..8e5eb290d 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientSecretResolution.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientSecretResolution.cs @@ -31,16 +31,18 @@ internal static class OpcUaClientSecretResolution /// The raw driver options (credential fields may carry secret: refs). /// The shared secret resolver used by the secret: arm. /// Cancellation token for the async secret resolution. + /// ⚠️ #815 warn-only compatibility tracker of the running driver (deprecated); null ⇒ hard-fail. /// An options copy whose credential fields carry resolved plaintext. /// /// A secret: ref names a secret that is absent/tombstoned in the store (fail-closed). /// internal static Task ResolveSecretRefsAsync( - OpcUaClientDriverOptions options, ISecretResolver resolver, CancellationToken ct) + OpcUaClientDriverOptions options, ISecretResolver resolver, CancellationToken ct, + Core.Abstractions.LegacyCredentialReferenceTracker? legacy = null) { ArgumentNullException.ThrowIfNull(options); ArgumentNullException.ThrowIfNull(resolver); // One authority, shared with the AdminUI live browse (Driver.OpcUaClient.Session). - return OpcUaUserIdentityFactory.ResolveSecretRefsAsync(options, resolver, ct); + return OpcUaUserIdentityFactory.ResolveSecretRefsAsync(options, resolver, ct, legacy: legacy); } } diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/Raw/GlobalRaw.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/Raw/GlobalRaw.razor index 7ef8633ac..07fd06670 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/Raw/GlobalRaw.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/Raw/GlobalRaw.razor @@ -23,6 +23,8 @@ Changes apply on the next deployment. + +
Project tree diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Raw/CredentialReferenceBanner.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Raw/CredentialReferenceBanner.razor new file mode 100644 index 000000000..90a3fedf1 --- /dev/null +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Raw/CredentialReferenceBanner.razor @@ -0,0 +1,85 @@ +@using Microsoft.EntityFrameworkCore +@using ZB.MOM.WW.OtOpcUa.AdminUI.Deployments +@using ZB.MOM.WW.OtOpcUa.AdminUI.Hubs +@using ZB.MOM.WW.OtOpcUa.Configuration +@inject CredentialReferencePreflightState State +@inject IDriverStatusSnapshotStore DriverStatus +@inject IServiceProvider Services + +@* lmxopcua#815 upgrade preflight surface: stored / deployed configs whose credential references sit outside their + driver's namespace (the boot scan, re-runnable here) plus running drivers that reported resolving one through the + deprecated warn-only compatibility path (driver-health diagnostics, fleet-wide). Never shows a value. *@ + +@if (_findings.Count > 0 || _error is not null) +{ + +} + +@code { + private const int MaxRows = 25; + private IReadOnlyList _findings = []; + private string? _error; + private bool _busy; + + protected override void OnInitialized() => Compose(State.Current); + + private void Compose(CredentialReferencePreflightState.Snapshot snapshot) + { + _error = snapshot.Error; + _findings = snapshot.Findings + .Concat(CredentialReferencePreflight.RunningFindings(DriverStatus.GetAll())) + .ToList(); + } + + private async Task RecheckAsync() + { + var factory = Services.GetService(typeof(IDbContextFactory)) as IDbContextFactory; + if (factory is null) return; + _busy = true; + try + { + var findings = await new CredentialReferencePreflight(factory).ScanAsync(CancellationToken.None); + var snapshot = new CredentialReferencePreflightState.Snapshot(findings, DateTime.UtcNow, null); + State.Set(snapshot); + Compose(snapshot); + } + catch (Exception) + { + _error = "The credential-reference preflight could not read the config DB."; + } + finally + { + _busy = false; + } + } +} diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Deployments/CredentialReferencePreflight.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Deployments/CredentialReferencePreflight.cs new file mode 100644 index 000000000..a87f361f7 --- /dev/null +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Deployments/CredentialReferencePreflight.cs @@ -0,0 +1,209 @@ +using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Hosting; +using Microsoft.Extensions.Logging; +using ZB.MOM.WW.OtOpcUa.Configuration; +using ZB.MOM.WW.OtOpcUa.Configuration.Entities; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; +using ZB.MOM.WW.OtOpcUa.Runtime.Drivers; + +namespace ZB.MOM.WW.OtOpcUa.AdminUI.Deployments; + +/// Where a credential-reference finding came from. +public enum CredentialFindingSource +{ + /// A stored DriverConfig / DeviceConfig row (what the next deploy will gate). + Draft, + + /// The latest deployment artifact (what the running drivers were given). + Deployed, + + /// A running driver that reported resolving it through the #815 warn-only compatibility path. + Running, +} + +/// One out-of-namespace credential reference (never carries the value). +/// Where it was found. +/// The driver type. +/// The driver's display name (or instance id for a running driver). +/// The driver instance id. +/// The device, when the reference sits in a DeviceConfig. +/// The field and the allowed forms. +public sealed record CredentialReferenceFinding( + CredentialFindingSource Source, string DriverType, string DriverName, string DriverInstanceId, string? DeviceName, string Problem); + +/// +/// Upgrade preflight for lmxopcua#815: lists every stored or deployed DriverConfig / DeviceConfig whose credential +/// fields carry a reference outside the driver's namespace — configs written before the namespaces existed that +/// the deploy gate will now refuse (and that a running driver resolves only in warn-only compatibility mode). +/// Reads the ConfigDb only; resolves nothing, so it never touches the secret store or the environment. +/// +/// The config DB. +public sealed class CredentialReferencePreflight(IDbContextFactory dbFactory) +{ + /// Scans the draft rows and the latest deployment artifact. + /// Cancellation. + /// The findings. + public async Task> ScanAsync(CancellationToken ct) + { + await using var db = await dbFactory.CreateDbContextAsync(ct).ConfigureAwait(false); + var drivers = await db.DriverInstances.AsNoTracking().ToListAsync(ct).ConfigureAwait(false); + var ruled = drivers.Where(d => DriverCredentialPolicy.TryGet(d.DriverType, out _)).ToList(); + var ids = ruled.Select(d => d.DriverInstanceId).ToHashSet(StringComparer.Ordinal); + var devices = ids.Count == 0 + ? [] + : await db.Devices.AsNoTracking().Where(d => ids.Contains(d.DriverInstanceId)).ToListAsync(ct).ConfigureAwait(false); + var latestBlob = await db.Deployments.AsNoTracking() + .Where(d => d.ArtifactBlob.Length > 0) + .OrderByDescending(d => d.CreatedAtUtc) + .Select(d => d.ArtifactBlob) + .FirstOrDefaultAsync(ct).ConfigureAwait(false); + var deployed = latestBlob is null ? [] : DeploymentArtifact.ParseDriverInstances(latestBlob); + return Scan(ruled, devices, deployed); + } + + /// The pure scan (tests drive it directly). + /// Stored driver rows. + /// Stored device rows. + /// The latest artifact's driver specs. + /// The findings. + public static IReadOnlyList Scan( + IEnumerable drivers, IEnumerable devices, IEnumerable deployed) + { + var findings = new List(); + var driverList = drivers.ToList(); + var byId = driverList.ToDictionary(d => d.DriverInstanceId, StringComparer.Ordinal); + foreach (var d in driverList) + { + foreach (var p in DriverCredentialPolicy.DescribeProblems(d.DriverType, d.DriverConfig)) + findings.Add(new(CredentialFindingSource.Draft, d.DriverType, d.Name, d.DriverInstanceId, null, p)); + } + + foreach (var dev in devices) + { + if (!byId.TryGetValue(dev.DriverInstanceId, out var owner)) continue; + foreach (var p in DriverCredentialPolicy.DescribeProblems(owner.DriverType, dev.DeviceConfig)) + findings.Add(new(CredentialFindingSource.Draft, owner.DriverType, owner.Name, owner.DriverInstanceId, dev.Name, p)); + } + + foreach (var s in deployed) + { + foreach (var p in DriverCredentialPolicy.DescribeProblems(s.DriverType, s.DriverConfig)) + findings.Add(new(CredentialFindingSource.Deployed, s.DriverType, s.Name, s.DriverInstanceId, null, p)); + } + + return findings; + } + + /// + /// Running drivers that reported a warn-only legacy resolution on their diagnostics bag + /// (), fleet-wide via the driver-health feed. + /// + /// The driver-health snapshots. + /// The findings. + public static IReadOnlyList RunningFindings(IEnumerable snapshots) => + snapshots + .Where(s => s.Diagnostics is not null) + .SelectMany(s => s.Diagnostics! + .Where(kv => kv.Key.StartsWith(LegacyCredentialReferenceTracker.DiagnosticsKeyPrefix, StringComparison.Ordinal)) + .Select(kv => new CredentialReferenceFinding( + CredentialFindingSource.Running, "", s.DriverInstanceId, s.DriverInstanceId, null, + $"{kv.Key[LegacyCredentialReferenceTracker.DiagnosticsKeyPrefix.Length..]}: {kv.Value}"))) + .ToList(); +} + +/// The last preflight result, shown by the AdminUI /raw credential banner. +public sealed class CredentialReferencePreflightState +{ + private volatile Snapshot _current = new([], null, null); + + /// An immutable result. + /// The findings. + /// When the scan ran (null = not yet). + /// A scan failure, if any. + public sealed record Snapshot(IReadOnlyList Findings, DateTime? ScannedAtUtc, string? Error); + + /// The current result. + public Snapshot Current => _current; + + /// Replaces the result. + /// The new result. + public void Set(Snapshot snapshot) => _current = snapshot; +} + +/// +/// Runs once after host start on an admin node and logs one Warning per +/// finding (driver, device, field, allowed forms — never a value). Scheduled after ApplicationStarted plus a +/// short delay, off every Akka starter; it needs only the ConfigDb, never ISecretResolver. +/// +/// Resolves the ConfigDb factory lazily (absent ⇒ the preflight is skipped). +/// Host lifetime. +/// Where the result is kept for the AdminUI. +/// Logger. +public sealed class CredentialReferencePreflightService( + IServiceProvider services, + IHostApplicationLifetime lifetime, + CredentialReferencePreflightState state, + ILogger logger) : BackgroundService +{ + /// Delay after ApplicationStarted before the scan (tests set it to zero). + public TimeSpan StartDelay { get; init; } = TimeSpan.FromSeconds(15); + + /// + protected override async Task ExecuteAsync(CancellationToken stoppingToken) + { + var started = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + using (lifetime.ApplicationStarted.Register(() => started.TrySetResult())) + using (stoppingToken.Register(() => started.TrySetCanceled(stoppingToken))) + { + try + { + await started.Task.ConfigureAwait(false); + if (StartDelay > TimeSpan.Zero) await Task.Delay(StartDelay, stoppingToken).ConfigureAwait(false); + } + catch (OperationCanceledException) + { + return; + } + } + + await RunOnceAsync(stoppingToken).ConfigureAwait(false); + } + + /// One scan + log pass (internal so tests drive it directly). + /// Cancellation. + /// The findings. + internal async Task> RunOnceAsync(CancellationToken ct) + { + var dbFactory = services.GetService>(); + if (dbFactory is null) + { + logger.LogDebug("Credential-reference preflight skipped: no ConfigDb on this node."); + return []; + } + + try + { + var findings = await new CredentialReferencePreflight(dbFactory).ScanAsync(ct).ConfigureAwait(false); + state.Set(new(findings, DateTime.UtcNow, null)); + foreach (var f in findings) + { + logger.LogWarning( + "Credential-reference preflight (lmxopcua#815): {Source} {DriverType} driver '{DriverName}'{Device}: {Problem} " + + "The deploy gate refuses this; a running driver resolves it only in deprecated warn-only mode. " + + "See docs/security.md \"Credential references are namespaced per driver\".", + f.Source, f.DriverType, f.DriverName, f.DeviceName is null ? "" : $" device '{f.DeviceName}'", f.Problem); + } + + if (findings.Count == 0) + logger.LogInformation("Credential-reference preflight (lmxopcua#815): every stored and deployed reference is namespaced."); + return findings; + } + catch (Exception ex) when (ex is not OperationCanceledException) + { + logger.LogWarning(ex, "Credential-reference preflight (lmxopcua#815) could not read the config DB."); + state.Set(new([], DateTime.UtcNow, "The credential-reference preflight could not read the config DB.")); + return []; + } + } +} diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/EndpointRouteBuilderExtensions.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/EndpointRouteBuilderExtensions.cs index f085754c5..faa797df2 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/EndpointRouteBuilderExtensions.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/EndpointRouteBuilderExtensions.cs @@ -72,6 +72,10 @@ public static class EndpointRouteBuilderExtensions // Browse pipeline — see docs/plans/2026-05-28-driver-browsers-design.md services.AddSingleton(); services.AddHostedService(); + // lmxopcua#815 upgrade preflight: one ConfigDb scan after start (no secret resolution), WARN per + // out-of-namespace credential reference, shown by the /raw credential banner. + services.AddSingleton(); + services.AddHostedService(); services.AddScoped(); services.AddScoped(); services.AddScoped(); diff --git a/tests/Core/ZB.MOM.WW.OtOpcUa.Configuration.Tests/DraftValidatorCredentialReferenceTests.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Configuration.Tests/DraftValidatorCredentialReferenceTests.cs index a682e54aa..68dc92637 100644 --- a/tests/Core/ZB.MOM.WW.OtOpcUa.Configuration.Tests/DraftValidatorCredentialReferenceTests.cs +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Configuration.Tests/DraftValidatorCredentialReferenceTests.cs @@ -32,6 +32,7 @@ public sealed class DraftValidatorCredentialReferenceTests e.Context.ShouldBe("drv"); e.Message.ShouldContain("secret:"); e.Message.ShouldContain("lmxopcua#815"); + e.Message.ShouldContain("Migrate it before deploying"); e.Message.ShouldNotContain("ConnectionStrings__ConfigDb"); } diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Browser.Tests/GalaxyDriverBrowserTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Browser.Tests/GalaxyDriverBrowserTests.cs index 5ba065861..e0deaaa77 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Browser.Tests/GalaxyDriverBrowserTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Browser.Tests/GalaxyDriverBrowserTests.cs @@ -54,6 +54,23 @@ public sealed class GalaxyDriverBrowserTests ex.Message.ShouldContain("ClientName"); } + /// + /// lmxopcua#815: the browser runs on the admin node against an operator-chosen endpoint with no deploy, so it + /// never gets the running driver's warn-only compatibility — a legacy ref (the old form default + /// env:MX_API_KEY, a file: path, another subsystem's secret) is refused before any connect. + /// + [Theory] + [InlineData("env:MX_API_KEY")] + [InlineData("file:appsettings.json")] + [InlineData("secret:otopcua/historian/api-key")] + public async Task OpenAsync_refuses_an_out_of_namespace_api_key_ref_with_no_warn_only_fallback(string reference) + { + var json = "{\"Gateway\":{\"Endpoint\":\"http://127.0.0.1:1\",\"ApiKeySecretRef\":\"" + reference + + "\"},\"MxAccess\":{\"ClientName\":\"X\"},\"Repository\":{},\"Reconnect\":{}}"; + await Should.ThrowAsync( + () => _sut.OpenAsync(json, TestContext.Current.CancellationToken)); + } + /// A JSON literal that deserializes to null must fail fast with a /// "deserialized to null" message — never a downstream NRE. [Fact] diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests/GalaxyDriverLegacyCredentialTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests/GalaxyDriverLegacyCredentialTests.cs new file mode 100644 index 000000000..ce42c5796 --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests/GalaxyDriverLegacyCredentialTests.cs @@ -0,0 +1,146 @@ +using Microsoft.Extensions.Logging; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; +using ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Config; +using ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Runtime; +using ZB.MOM.WW.Secrets.Abstractions; + +namespace ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests; + +/// +/// lmxopcua#815 warn-only compatibility (deprecated): an ALREADY-DEPLOYED Galaxy driver whose config predates the +/// namespaces — e.g. the old AdminUI form default env:MX_API_KEY — keeps resolving it on init and on every +/// gateway reconnect (ReopenAsync → BuildClientOptionsAsync), warning rate-limited and surfacing it +/// on the diagnostics bag; while the admin-node paths (browser) stay hard-fail. +/// +public sealed class GalaxyDriverLegacyCredentialTests +{ + private const string LegacyVariable = "MX_API_KEY_815_TEST"; + + private static GalaxyDriverOptions Opts(string apiKeyRef, string? ackRef = null) => new( + new GalaxyGatewayOptions("https://mxgw.test:5001", apiKeyRef), + new GalaxyMxAccessOptions("Compat815"), + new GalaxyRepositoryOptions(WatchDeployEvents: false), + new GalaxyReconnectOptions()) + { + AlarmAck = ackRef is null ? new GalaxyAlarmAckOptions() : new GalaxyAlarmAckOptions("ack-svc", null, ackRef), + }; + + [Fact] + public async Task A_running_driver_with_a_legacy_env_ref_resolves_on_init_and_every_reconnect_and_warns_once() + { + Environment.SetEnvironmentVariable(LegacyVariable, "the-real-key"); + try + { + var logger = new CaptureLogger(); + var driver = new GalaxyDriver("gx-legacy", Opts($"env:{LegacyVariable}"), new NullResolver(), logger); + var ct = TestContext.Current.CancellationToken; + + // InitializeAsync and each ReopenAsync (gateway reconnect) build the client options through this one method. + (await driver.BuildClientOptionsAsync(Opts($"env:{LegacyVariable}").Gateway, ct)).ApiKey.ShouldBe("the-real-key"); + (await driver.BuildClientOptionsAsync(Opts($"env:{LegacyVariable}").Gateway, ct)).ApiKey.ShouldBe("the-real-key"); + (await driver.BuildClientOptionsAsync(Opts($"env:{LegacyVariable}").Gateway, ct)).ApiKey.ShouldBe("the-real-key"); + + var warnings = logger.Entries.Where(e => e.Level == LogLevel.Warning && e.Message.Contains("lmxopcua#815")).ToList(); + warnings.Count.ShouldBe(1); // rate-limited: one per field per hour, not one per reconnect + warnings[0].Message.ShouldContain("Gateway.ApiKeySecretRef"); + warnings[0].Message.ShouldContain("env:OTOPCUA_GALAXY_"); + warnings[0].Message.ShouldNotContain("the-real-key"); + + var diagnostics = ((IDriverDiagnostics)driver).GetDiagnostics(); + diagnostics.Keys.ShouldContain(LegacyCredentialReferenceTracker.DiagnosticsKeyPrefix + "Gateway.ApiKeySecretRef"); + diagnostics.Values.ShouldAllBe(v => !v.Contains("the-real-key")); + DriverDiagnostics.Bound(diagnostics).Values.ShouldAllBe(v => v.Length <= DriverDiagnostics.MaxValueLength); + } + finally + { + Environment.SetEnvironmentVariable(LegacyVariable, null); + } + } + + [Fact] + public async Task A_running_driver_keeps_the_old_case_insensitive_and_file_semantics() + { + var path = Path.Combine(Path.GetTempPath(), $"gx-815-{Guid.NewGuid():N}.txt"); + File.WriteAllText(path, " key-from-file \n"); + try + { + var driver = new GalaxyDriver("gx-legacy-file", Opts($"file:{path}"), new NullResolver()); + (await driver.BuildClientOptionsAsync(Opts($"file:{path}").Gateway, TestContext.Current.CancellationToken)) + .ApiKey.ShouldBe("key-from-file"); + ((IDriverDiagnostics)driver).GetDiagnostics().ShouldNotBeEmpty(); + } + finally + { + File.Delete(path); + } + } + + [Fact] + public async Task A_namespaced_ref_resolves_without_a_compatibility_warning() + { + var logger = new CaptureLogger(); + var driver = new GalaxyDriver("gx-ok", Opts("dev:k"), new NullResolver(), logger); + (await driver.BuildClientOptionsAsync(Opts("dev:k").Gateway, TestContext.Current.CancellationToken)).ApiKey.ShouldBe("k"); + logger.Entries.ShouldNotContain(e => e.Message.Contains("lmxopcua#815")); + ((IDriverDiagnostics)driver).GetDiagnostics().ShouldBeEmpty(); + } + + /// The same legacy ref through the admin-node browser path (no tracker) is refused before any read. + [Fact] + public async Task The_same_ref_without_a_tracker_is_refused() + { + var reads = 0; + await Should.ThrowAsync(() => GalaxySecretRef.ResolveApiKeyAsync( + "env:MX_API_KEY", new NullResolver(), ct: TestContext.Current.CancellationToken, + environment: _ => { reads++; return "the-real-key"; })); + reads.ShouldBe(0); + } + + [Fact] + public async Task The_running_ack_fallback_keeps_a_legacy_ref_and_warns() + { + var warnings = new List(); + var tracker = new LegacyCredentialReferenceTracker(DriverCredentialPolicy.GalaxyScope, "gx", warnings.Add); + var resolver = new GalaxyAckCredentialResolver( + "gx", new GalaxyAlarmAckOptions("ack-svc", null, "env:LEGACY_ACK_PW"), new NullResolver(), new CaptureLogger(), + n => n == "LEGACY_ACK_PW" ? "pw" : null, tracker); + + (await resolver.ResolveAsync(null, TestContext.Current.CancellationToken)).Credential.ShouldNotBeNull().Password.ShouldBe("pw"); + warnings.ShouldHaveSingleItem().ShouldContain("AlarmAck.FallbackPasswordSecretRef"); + } + + [Fact] + public void The_tracker_re_warns_after_the_interval() + { + var now = new DateTime(2026, 11, 11, 0, 0, 0, DateTimeKind.Utc); + var warnings = new List(); + var tracker = new LegacyCredentialReferenceTracker(DriverCredentialPolicy.GalaxyScope, "gx", warnings.Add, () => now); + tracker.Report("F"); + tracker.Report("F"); + now += LegacyCredentialReferenceTracker.WarnInterval; + tracker.Report("F"); + tracker.Report("G"); + warnings.Count.ShouldBe(3); + } + + private sealed class NullResolver : ISecretResolver + { + public Task GetAsync(SecretName name, CancellationToken ct) => Task.FromResult(null); + } + + private sealed class CaptureLogger : ILogger + { + public List<(LogLevel Level, string Message)> Entries { get; } = new(); + + public IDisposable? BeginScope(TState state) where TState : notnull => null; + + public bool IsEnabled(LogLevel logLevel) => true; + + public void Log(LogLevel logLevel, EventId eventId, TState state, Exception? exception, Func formatter) + { + lock (Entries) Entries.Add((logLevel, formatter(state, exception))); + } + } +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests/OpcUaClientSecretResolutionTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests/OpcUaClientSecretResolutionTests.cs index d319072e3..f52bac365 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests/OpcUaClientSecretResolutionTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests/OpcUaClientSecretResolutionTests.cs @@ -124,6 +124,35 @@ public sealed class OpcUaClientSecretResolutionTests resolved.Password.ShouldBe("pw"); } + /// + /// lmxopcua#815 warn-only compatibility (deprecated): the RUNNING driver's session open keeps the old semantics + /// for an out-of-namespace value — a case-insensitive secret: resolves, anything else is a literal — and + /// warns once; the browser (no tracker) refuses the same value (see the theory above). + /// + [Fact] + public async Task The_running_driver_keeps_old_semantics_for_a_legacy_value_and_warns() + { + var warnings = new List(); + var tracker = new LegacyCredentialReferenceTracker(DriverCredentialPolicy.OpcUaClientScope, "ouc", warnings.Add); + var legacyOptions = new OpcUaClientDriverOptions + { + Password = $"SECRET:{KnownPwName}", // pre-#815: case-insensitive secret: resolved + UserCertificatePassword = "env:NOT_A_REF", // pre-#815: only secret: resolved — a literal password + }; + + for (var i = 0; i < 3; i++) + { + var resolved = await OpcUaClientSecretResolution.ResolveSecretRefsAsync( + legacyOptions, FakeResolver(), CancellationToken.None, tracker); + resolved.Password.ShouldBe(KnownPwValue); + resolved.UserCertificatePassword.ShouldBe("env:NOT_A_REF"); + } + + warnings.Count.ShouldBe(2); // one per field, rate-limited across the three session opens + warnings.ShouldAllBe(w => !w.Contains(KnownPwValue)); + tracker.Diagnostics.Count.ShouldBe(2); + } + private sealed class CountingResolver : ISecretResolver { public int Calls { get; private set; } diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Deployments/CredentialReferencePreflightTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Deployments/CredentialReferencePreflightTests.cs new file mode 100644 index 000000000..dbc05518c --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Deployments/CredentialReferencePreflightTests.cs @@ -0,0 +1,177 @@ +using System.Text; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Hosting; +using Microsoft.Extensions.Logging; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.AdminUI.Deployments; +using ZB.MOM.WW.OtOpcUa.AdminUI.Tests.Uns; +using ZB.MOM.WW.OtOpcUa.Commons.Messages.Drivers; +using ZB.MOM.WW.OtOpcUa.Configuration; +using ZB.MOM.WW.OtOpcUa.Configuration.Entities; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; + +namespace ZB.MOM.WW.OtOpcUa.AdminUI.Tests.Deployments; + +/// +/// lmxopcua#815 upgrade preflight: stored (draft) and deployed configs with out-of-namespace credential references +/// are listed and logged — driver, device, field, allowed form — and never the value; nothing is resolved. +/// +[Trait("Category", "Unit")] +public sealed class CredentialReferencePreflightTests +{ + private static CancellationToken Ct => TestContext.Current.CancellationToken; + + private const string LegacyGalaxy = """{"Gateway":{"Endpoint":"http://gw:5001","ApiKeySecretRef":"env:MX_API_KEY"}}"""; + + private static string SeedLegacy() + { + var name = RawTreeTestDb.SeedFresh(); + using var db = RawTreeTestDb.CreateNamed(name); + // Written straight to the DB, as a pre-#815 build would have — the save gate now refuses it. + db.DriverInstances.Add(new DriverInstance + { + DriverInstanceId = "gx-old", ClusterId = RawTreeTestDb.ClusterId, Name = "Galaxy main", + DriverType = DriverTypeNames.Galaxy, DriverConfig = LegacyGalaxy, + }); + db.Devices.Add(new Device + { + DeviceId = "ouc-dev", DriverInstanceId = "gx-old", Name = "gw1", + DeviceConfig = """{"AlarmAck":{"FallbackPasswordSecretRef":"file:C:\\keys\\ack.txt"}}""", + }); + db.DriverInstances.Add(new DriverInstance + { + DriverInstanceId = "mb", ClusterId = RawTreeTestDb.ClusterId, Name = "modbus", + DriverType = DriverTypeNames.Modbus, DriverConfig = """{"Password":"env:ConnectionStrings__ConfigDb"}""", + }); + var artifact = """ + {"DriverInstances":[{"DriverInstanceId":"ouc","Name":"Upstream","DriverType":"OpcUaClient", + "DriverConfig":"{\"Password\":\"secret:plant/opcua-pw\"}"}]} + """; + db.Deployments.Add(new Deployment { RevisionHash = "abc", CreatedBy = "t", ArtifactBlob = Encoding.UTF8.GetBytes(artifact) }); + db.SaveChanges(); + return name; + } + + [Fact] + public async Task Scan_lists_draft_rows_devices_and_the_deployed_artifact_without_values() + { + var findings = await new CredentialReferencePreflight(RawTreeTestDb.Factory(SeedLegacy())).ScanAsync(Ct); + + findings.ShouldContain(f => f.Source == CredentialFindingSource.Draft && f.DriverName == "Galaxy main" + && f.DeviceName == null && f.Problem.Contains("Gateway.ApiKeySecretRef") + && f.Problem.Contains("env:OTOPCUA_GALAXY_")); + findings.ShouldContain(f => f.Source == CredentialFindingSource.Draft && f.DeviceName == "gw1" + && f.Problem.Contains("AlarmAck.FallbackPasswordSecretRef")); + findings.ShouldContain(f => f.Source == CredentialFindingSource.Deployed && f.DriverName == "Upstream" + && f.Problem.Contains("secret:opcuaclient/")); + findings.ShouldNotContain(f => f.DriverInstanceId == "mb"); // not a credential-resolving driver + findings.ShouldAllBe(f => !f.Problem.Contains("MX_API_KEY") && !f.Problem.Contains("plant/opcua-pw") + && !f.Problem.Contains("ack.txt")); + } + + [Fact] + public async Task A_clean_database_has_no_findings() + => (await new CredentialReferencePreflight(RawTreeTestDb.Factory(RawTreeTestDb.SeedFresh())).ScanAsync(Ct)).ShouldBeEmpty(); + + [Fact] + public async Task The_boot_pass_logs_one_warning_per_finding_and_publishes_the_state() + { + var services = new ServiceCollection() + .AddSingleton(RawTreeTestDb.Factory(SeedLegacy())) + .BuildServiceProvider(); + var state = new CredentialReferencePreflightState(); + var logger = new CaptureLogger(); + var svc = new CredentialReferencePreflightService(services, new FakeLifetime(), state, logger) { StartDelay = TimeSpan.Zero }; + + var findings = await svc.RunOnceAsync(Ct); + + findings.Count.ShouldBe(3); + state.Current.Findings.Count.ShouldBe(3); + state.Current.ScannedAtUtc.ShouldNotBeNull(); + var warnings = logger.Entries.Where(e => e.Level == LogLevel.Warning).ToList(); + warnings.Count.ShouldBe(3); + warnings.ShouldAllBe(w => w.Message.Contains("lmxopcua#815") && !w.Message.Contains("MX_API_KEY")); + warnings.ShouldContain(w => w.Message.Contains("Galaxy main") && w.Message.Contains("Gateway.ApiKeySecretRef")); + } + + [Fact] + public async Task A_node_without_a_config_db_skips_quietly() + { + var svc = new CredentialReferencePreflightService( + new ServiceCollection().BuildServiceProvider(), new FakeLifetime(), new CredentialReferencePreflightState(), new CaptureLogger()); + (await svc.RunOnceAsync(Ct)).ShouldBeEmpty(); + } + + [Fact] + public async Task The_hosted_service_waits_for_ApplicationStarted_before_scanning() + { + var services = new ServiceCollection().AddSingleton(RawTreeTestDb.Factory(SeedLegacy())).BuildServiceProvider(); + var lifetime = new FakeLifetime(); + var state = new CredentialReferencePreflightState(); + var svc = new CredentialReferencePreflightService(services, lifetime, state, new CaptureLogger()) { StartDelay = TimeSpan.Zero }; + + await svc.StartAsync(Ct); + await Task.Delay(100, Ct); + state.Current.ScannedAtUtc.ShouldBeNull(); // nothing before the host has started + + lifetime.Start(); + var deadline = DateTime.UtcNow.AddSeconds(10); + while (state.Current.ScannedAtUtc is null && DateTime.UtcNow < deadline) await Task.Delay(20, Ct); + state.Current.Findings.Count.ShouldBe(3); + await svc.StopAsync(Ct); + } + + [Fact] + public void Running_drivers_reporting_warn_only_resolution_are_surfaced_from_driver_health() + { + var snapshots = new[] + { + new DriverHealthChanged("c", "gx-old", "Healthy", null, null, 0, DateTime.UtcNow, + Diagnostics: new Dictionary + { + [LegacyCredentialReferenceTracker.DiagnosticsKeyPrefix + "Gateway.ApiKeySecretRef"] = "deprecated …", + ["s7.pdu"] = "480", + }), + new DriverHealthChanged("c", "mb", "Healthy", null, null, 0, DateTime.UtcNow), + }; + + var running = CredentialReferencePreflight.RunningFindings(snapshots); + + running.ShouldHaveSingleItem().ShouldSatisfyAllConditions( + f => f.Source.ShouldBe(CredentialFindingSource.Running), + f => f.DriverInstanceId.ShouldBe("gx-old"), + f => f.Problem.ShouldStartWith("Gateway.ApiKeySecretRef")); + } + + private sealed class FakeLifetime : IHostApplicationLifetime + { + private readonly CancellationTokenSource _started = new(); + + public CancellationToken ApplicationStarted => _started.Token; + + public CancellationToken ApplicationStopping => CancellationToken.None; + + public CancellationToken ApplicationStopped => CancellationToken.None; + + public void Start() => _started.Cancel(); + + public void StopApplication() + { + } + } + + private sealed class CaptureLogger : ILogger + { + public List<(LogLevel Level, string Message)> Entries { get; } = new(); + + public IDisposable? BeginScope(TState state) where TState : notnull => null; + + public bool IsEnabled(LogLevel logLevel) => true; + + public void Log(LogLevel logLevel, EventId eventId, TState state, Exception? exception, Func formatter) + { + lock (Entries) Entries.Add((logLevel, formatter(state, exception))); + } + } +} diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/AdminOperationsActorCredentialScopeTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/AdminOperationsActorCredentialScopeTests.cs index 0303f9991..86780d72d 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/AdminOperationsActorCredentialScopeTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/AdminOperationsActorCredentialScopeTests.cs @@ -17,6 +17,7 @@ public sealed class AdminOperationsActorCredentialScopeTests : ControlPlaneActor { [Theory] [InlineData(DriverTypeNames.Galaxy, """{"Gateway":{"Endpoint":"http://attacker:5001","ApiKeySecretRef":"env:ConnectionStrings__ConfigDb"}}""")] + [InlineData(DriverTypeNames.Galaxy, """{"Gateway":{"Endpoint":"http://attacker:5001","ApiKeySecretRef":"env:MX_API_KEY"}}""")] [InlineData(DriverTypeNames.Galaxy, """{"Gateway":{"Endpoint":"http://attacker:5001","ApiKeySecretRef":"env:ServerHistorian__ApiKey"}}""")] [InlineData(DriverTypeNames.Galaxy, """{"Gateway":{"Endpoint":"http://attacker:5001","ApiKeySecretRef":"file:appsettings.json"}}""")] [InlineData(DriverTypeNames.OpcUaClient, """{"EndpointUrl":"opc.tcp://attacker:4840","Password":"secret:otopcua/historian/api-key"}""")]