From a224fd639198ea54fb3222464a33453dfd96c000 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Wed, 11 Nov 2026 01:28:58 -0500 Subject: [PATCH] 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)); + } + } +}