diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Commons/Messages/Admin/TestDriverConnect.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Commons/Messages/Admin/TestDriverConnect.cs index dafdaf471..f2fb79151 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Commons/Messages/Admin/TestDriverConnect.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Commons/Messages/Admin/TestDriverConnect.cs @@ -9,11 +9,17 @@ namespace ZB.MOM.WW.OtOpcUa.Commons.Messages.Admin; /// Driver config as JSON (same shape as DriverInstance.DriverConfig). /// Per-probe timeout; server clamps to [1, 60]. /// Round-trip correlation token. +/// +/// The driver instance the config belongs to (lmxopcua#835 — its own secret:galaxy/<id>/… references are +/// allowed, another instance's are refused). Null = an instance not yet created: a reference naming ANY existing +/// instance is refused, so omitting it can only make the check stricter. +/// public sealed record TestDriverConnect( string DriverType, string ConfigJson, int TimeoutSeconds, - Guid CorrelationId); + Guid CorrelationId, + string? DriverInstanceId = null); /// Reply for . /// True iff the probe succeeded. diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Validation/CredentialReferenceDeployChecks.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Validation/CredentialReferenceDeployChecks.cs index 6b62bf087..ad04b1103 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Validation/CredentialReferenceDeployChecks.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Validation/CredentialReferenceDeployChecks.cs @@ -30,21 +30,51 @@ public static class CredentialReferenceDeployChecks + "field at it. A driver deployed before this upgrade keeps resolving the old reference (warn-only, deprecated) " + "only until this deploy succeeds. See docs/security.md §\"Credential references are namespaced per driver\"."; + /// + /// lmxopcua#835: a credential reference whose instance segment (secret:galaxy/<X>/…) names ANOTHER + /// existing driver instance of the same type, by id or name. + /// + public const string OtherInstanceCode = "CredentialReferenceOtherInstance"; + + /// Appended to every error. + public const string OtherInstanceHint = + " Store this driver's own credential under its own driver id (or a shared name that is no other driver's id or " + + "name) and point the field at it. A driver deployed before this upgrade keeps resolving the old reference until " + + "this deploy succeeds. See docs/security.md §\"Per-instance credential references\"."; + /// Runs the gate. /// The draft. /// The errors. public static IReadOnlyList Errors(DraftSnapshot draft) { var errors = new List(); + var instances = draft.DriverInstances + .Select(d => new DriverInstanceIdentity(d.DriverInstanceId, d.Name, d.DriverType)) + .ToList(); foreach (var driver in draft.DriverInstances) { - if (string.Equals(driver.DriverType, DriverTypeNames.Heidenhain, StringComparison.OrdinalIgnoreCase)) continue; if (!DriverCredentialPolicy.TryGet(driver.DriverType, out _)) continue; + var devices = draft.Devices + .Where(d => string.Equals(d.DriverInstanceId, driver.DriverInstanceId, StringComparison.Ordinal)) + .ToList(); + + // lmxopcua#835 — applies to Heidenhain too (HeidenhainDeployChecks reports only the namespace rule). + foreach (var problem in DriverCredentialPolicy.DescribeOtherInstanceProblems( + driver.DriverType, driver.DriverConfig, driver.DriverInstanceId, instances)) + errors.Add(new(OtherInstanceCode, $"{driver.DriverType} driver '{driver.Name}': {problem}{OtherInstanceHint}", driver.DriverInstanceId)); + foreach (var device in devices) + { + foreach (var problem in DriverCredentialPolicy.DescribeOtherInstanceProblems( + driver.DriverType, device.DeviceConfig, driver.DriverInstanceId, instances)) + errors.Add(new(OtherInstanceCode, $"{driver.DriverType} device '{device.Name}': {problem}{OtherInstanceHint}", device.DeviceId)); + } + + if (string.Equals(driver.DriverType, DriverTypeNames.Heidenhain, StringComparison.OrdinalIgnoreCase)) continue; foreach (var problem in DriverCredentialPolicy.DescribeProblems(driver.DriverType, driver.DriverConfig)) errors.Add(new(Code, $"{driver.DriverType} driver '{driver.Name}': {problem}{MigrationHint}", driver.DriverInstanceId)); - foreach (var device in draft.Devices.Where(d => string.Equals(d.DriverInstanceId, driver.DriverInstanceId, StringComparison.Ordinal))) + foreach (var device in devices) { foreach (var problem in DriverCredentialPolicy.DescribeProblems(driver.DriverType, device.DeviceConfig)) errors.Add(new(Code, $"{driver.DriverType} device '{device.Name}': {problem}{MigrationHint}", device.DeviceId)); diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Validation/DriverInstanceIdentities.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Validation/DriverInstanceIdentities.cs new file mode 100644 index 000000000..f3fac585d --- /dev/null +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Validation/DriverInstanceIdentities.cs @@ -0,0 +1,48 @@ +using Microsoft.EntityFrameworkCore; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; + +namespace ZB.MOM.WW.OtOpcUa.Configuration.Validation; + +/// +/// Loads the driver-instance identities the cross-instance credential rule (lmxopcua#835, +/// ) matches against — the ONE query the AdminUI save +/// path, live browse and Test Connect share (the deploy gate reads the same rows from its draft snapshot). +/// +public static class DriverInstanceIdentities +{ + /// Every stored driver instance of , fleet-wide. + /// The config DB. + /// The driver type (compared case-insensitively, client-side). + /// Cancellation. + /// The identities. + public static async Task> LoadAsync( + OtOpcUaConfigDbContext db, string driverType, CancellationToken ct) + { + ArgumentNullException.ThrowIfNull(db); + var rows = await db.DriverInstances.AsNoTracking() + .Select(d => new { d.DriverInstanceId, d.Name, d.DriverType }) + .ToListAsync(ct).ConfigureAwait(false); + return rows + .Where(r => string.Equals(r.DriverType, driverType, StringComparison.OrdinalIgnoreCase)) + .Select(r => new DriverInstanceIdentity(r.DriverInstanceId, r.Name, r.DriverType)) + .ToList(); + } + + /// + /// The cross-instance problems of — reading the ConfigDb only when the config carries + /// an in-scope secret: reference at all. + /// + /// The config DB. + /// The owning driver's type. + /// The config about to be stored / probed / browsed. + /// The owning driver; null when it is being created. + /// Cancellation. + /// The problems. + public static async Task> DescribeOtherInstanceProblemsAsync( + OtOpcUaConfigDbContext db, string? driverType, string? configJson, string? ownDriverInstanceId, CancellationToken ct) + { + if (driverType is null || !DriverCredentialPolicy.HasInstanceSegments(driverType, configJson)) return []; + var instances = await LoadAsync(db, driverType, ct).ConfigureAwait(false); + return DriverCredentialPolicy.DescribeOtherInstanceProblems(driverType, configJson, ownDriverInstanceId, instances); + } +} diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/CredentialFilePaths.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/CredentialFilePaths.cs new file mode 100644 index 000000000..1dbad06a6 --- /dev/null +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/CredentialFilePaths.cs @@ -0,0 +1,121 @@ +namespace ZB.MOM.WW.OtOpcUa.Core.Abstractions; + +/// +/// A driver-config field naming a node-local file that holds key material (lmxopcua#836 — OpcUaClient +/// UserCertificatePath). The node loads the file and USES its private key against the author-chosen endpoint, +/// so an unrestricted path lets a Designer make the node sign a hostile server's nonce with any PFX the service +/// account can read. The field is therefore confined to one node-configured directory. +/// +/// The JSON property name (matched case-insensitively, as the driver binds it). +/// How the operator configures the directory (for messages). +public sealed record CredentialPathField(string Name, string DirectoryDescription); + +/// +/// The one rule for values (lmxopcua#836): authored as a RELATIVE path inside a +/// node-configured directory, checked syntactically at every admin seam (save, deploy gate, Test Connect, browse) and +/// canonicalised — including symbolic links / junctions — at load time by . +/// +/// +/// The admin seams cannot see the driver node's directory, so the authored form carries no directory at all: an +/// absolute, rooted, drive-qualified or UNC path is refused there, as is any . / .. / empty segment or a +/// character outside [A-Za-z0-9._-]. Resolution re-checks the syntax (fail-closed), joins it to the node's +/// directory, and requires both the lexical result and the real (link-resolved) result to stay inside the real +/// directory. +/// +public static class CredentialFilePaths +{ + private const int MaxLength = 260; + + /// True when is blank or an acceptable relative path. + /// The authored value. + /// Whether it may be stored / deployed. + public static bool IsAcceptable(string? value) => string.IsNullOrEmpty(value) || IsAcceptableRelative(value); + + /// True when is a non-empty relative path of plain segments. + /// The authored value. + /// Whether it is a relative path the resolver will accept. + public static bool IsAcceptableRelative(string? value) + { + if (string.IsNullOrEmpty(value) || value.Length > MaxLength) return false; + if (value[0] is '/' or '\\') return false; + var segments = value.Split('/', '\\'); + foreach (var seg in segments) + { + if (seg.Length == 0 || seg is "." or "..") return false; + if (!seg.All(c => char.IsAsciiLetterOrDigit(c) || c is '-' or '_' or '.')) return false; + } + + return !Path.IsPathRooted(value); + } + + /// The one wording of the rule — names the field and the directory setting, never the value. + /// The field path. + /// How the directory is configured. + /// The message. + public static string RuleMessage(string field, string directoryDescription) => + $"{field} must be blank or a relative path (e.g. 'plant1/user.pfx') inside the node's certificate directory " + + $"({directoryDescription}) — an absolute, drive or UNC path, a '.'/'..' segment, or a character outside " + + "[A-Za-z0-9._-] is refused (lmxopcua#836)."; + + /// + /// Resolves an authored relative path inside , fail-closed: throws + /// when the value is not an acceptable relative path, or when the + /// result — lexically or after resolving every symbolic link / junction on the way — leaves the directory. + /// + /// The authored relative path. + /// The node's configured directory. + /// Field label for messages. + /// How the directory is configured (for messages). + /// The full path to load (the lexical path inside the directory). + public static string ResolveWithin(string? value, string directory, string field, string directoryDescription) + { + ArgumentException.ThrowIfNullOrWhiteSpace(directory); + if (!IsAcceptableRelative(value)) + throw new CredentialReferenceException(RuleMessage(field, directoryDescription) + " (fail-closed)"); + + var root = Path.TrimEndingDirectorySeparator(Path.GetFullPath(directory)); + var full = Path.GetFullPath(Path.Combine(root, value!)); + if (!IsUnder(full, root)) + throw new CredentialReferenceException($"{field} resolves outside the certificate directory (fail-closed)."); + + var realRoot = RealPath(root); + var realFull = RealPath(full); + if (!IsUnder(realFull, realRoot)) + throw new CredentialReferenceException( + $"{field} resolves (through a symbolic link or junction) outside the certificate directory (fail-closed)."); + return full; + } + + private static StringComparison PathComparison => + OperatingSystem.IsWindows() || OperatingSystem.IsMacOS() ? StringComparison.OrdinalIgnoreCase : StringComparison.Ordinal; + + private static bool IsUnder(string path, string root) + { + var prefix = root.EndsWith(Path.DirectorySeparatorChar) ? root : root + Path.DirectorySeparatorChar; + return path.StartsWith(prefix, PathComparison); + } + + /// + /// The path with every existing symbolic link / junction component replaced by its final target, walked from the + /// root (a link's target may itself sit under another link). Components that do not exist are kept verbatim. + /// + private static string RealPath(string fullPath) + { + var root = Path.GetPathRoot(fullPath) ?? ""; + var current = root; + var rest = fullPath[root.Length..].Split( + [Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar], StringSplitOptions.RemoveEmptyEntries); + for (var i = 0; i < rest.Length; i++) + { + current = Path.Combine(current, rest[i]); + FileSystemInfo info = Directory.Exists(current) ? new DirectoryInfo(current) : new FileInfo(current); + if (!info.Exists || info.LinkTarget is null) continue; + var target = info.ResolveLinkTarget(returnFinalTarget: true); + if (target is null) continue; + // Re-walk the target itself: its own parents may be links too. + current = RealPath(Path.GetFullPath(target.FullName)); + } + + return current; + } +} diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/DriverCredentialPolicy.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/DriverCredentialPolicy.cs index 2b3eeb745..694a1d38d 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/DriverCredentialPolicy.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/DriverCredentialPolicy.cs @@ -11,7 +11,19 @@ public sealed record CredentialField(string Name, CredentialLiteralPolicy Litera /// 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); +/// Fields naming a node-local key-material file, confined to a node directory (lmxopcua#836). +public sealed record DriverCredentialRules( + string DriverType, CredentialScope Scope, IReadOnlyList Fields, IReadOnlyList? PathFields = null) +{ + /// The path fields (never null). + public IReadOnlyList PathFieldList => PathFields ?? []; +} + +/// The identity of one driver instance, as the cross-instance rule (lmxopcua#835) matches it. +/// The instance id. +/// The instance's display name. +/// The instance's driver type. +public sealed record DriverInstanceIdentity(string DriverInstanceId, string Name, string DriverType); /// /// The per-driver credential namespaces (lmxopcua#815) — the single table the AdminUI save path, the deploy gate, @@ -39,6 +51,14 @@ public static class DriverCredentialPolicy /// OPC UA Client user / certificate passwords: secret:opcuaclient/… / env:OTOPCUA_OPCUACLIENT_…. public static readonly CredentialScope OpcUaClientScope = new("OpcUaClient", ["opcuaclient/"], ["OTOPCUA_OPCUACLIENT_"]); + /// The node config key of the OpcUaClient user-certificate directory (lmxopcua#836). + public const string OpcUaClientUserCertificateDirectoryKey = "Drivers:OpcUaClient:UserCertificateDirectory"; + + /// OpcUaClient UserCertificatePath: a relative path inside the node's user-certificate directory (#836). + public static readonly CredentialPathField OpcUaClientUserCertificatePathField = new( + "UserCertificatePath", + OpcUaClientUserCertificateDirectoryKey + ", default /OtOpcUa/pki/user-certificates"); + /// Galaxy Gateway.ApiKeySecretRef (literal accepted for back-compat, with a warning at resolve). public static readonly CredentialField GalaxyApiKeyField = new("ApiKeySecretRef", CredentialLiteralPolicy.Allowed); @@ -59,7 +79,7 @@ public static class DriverCredentialPolicy [ new("Password", CredentialLiteralPolicy.Allowed), new("UserCertificatePassword", CredentialLiteralPolicy.Allowed), - ]), + ], [OpcUaClientUserCertificatePathField]), }; /// The rules for a driver type, if it resolves credential references. @@ -110,7 +130,35 @@ public static class DriverCredentialPolicy return problems; } - private static void Scan(JsonElement element, string path, DriverCredentialRules rules, List problems, int depth) + private static void Scan(JsonElement element, string path, DriverCredentialRules rules, List problems, int depth) => + Visit(element, path, rules, depth, (fieldPath, value, field, pathField) => + { + if (field is not null) + { + var ok = value.ValueKind switch + { + JsonValueKind.Null => true, + JsonValueKind.String => CredentialReferences.IsAcceptable(value.GetString(), rules.Scope, field.Literals), + _ => false, + }; + if (!ok) problems.Add(CredentialReferences.RuleMessage(fieldPath, rules.Scope, field.Literals)); + } + else if (pathField is not null) + { + var ok = value.ValueKind switch + { + JsonValueKind.Null => true, + JsonValueKind.String => CredentialFilePaths.IsAcceptable(value.GetString()), + _ => false, + }; + if (!ok) problems.Add(CredentialFilePaths.RuleMessage(fieldPath, pathField.DirectoryDescription)); + } + }); + + /// Walks every credential / path field of at any depth, keys matched case-insensitively. + private static void Visit( + JsonElement element, string path, DriverCredentialRules rules, int depth, + Action onField) { if (depth > 32) return; if (element.ValueKind == JsonValueKind.Array) @@ -118,7 +166,7 @@ public static class DriverCredentialPolicy var i = 0; foreach (var item in element.EnumerateArray()) { - Scan(item, $"{path}[{i}]", rules, problems, depth + 1); + Visit(item, $"{path}[{i}]", rules, depth + 1, onField); i++; } @@ -130,19 +178,141 @@ public static class DriverCredentialPolicy { 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) + var pathField = field is null + ? rules.PathFieldList.FirstOrDefault(f => string.Equals(f.Name, p.Name, StringComparison.OrdinalIgnoreCase)) + : null; + if (field is null && pathField is null) { - Scan(p.Value, fieldPath, rules, problems, depth + 1); + Visit(p.Value, fieldPath, rules, depth + 1, onField); 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)); + onField(fieldPath, p.Value, field, pathField); } } + + // --------------------------------------------------------------------------------------------------------------- + // lmxopcua#835 — cross-instance references inside one namespace + // --------------------------------------------------------------------------------------------------------------- + + /// + /// The instance-identifying segment of an in-scope secret: reference: the first path segment after the + /// matched namespace (secret:galaxy/<X>/apikey ⇒ X; secret:otopcua/galaxy/<X> ⇒ + /// X). Null for anything else — env: references carry no instance convention, and literals and + /// out-of-scope references are the #815 rule's business. + /// + /// The authored field value. + /// The driver's scope. + /// The segment, or null. + public static string? InstanceSegment(string? value, CredentialScope scope) + { + ArgumentNullException.ThrowIfNull(scope); + if (CredentialReferences.Parse(value, scope, out var name) != CredentialReferenceKind.Secret) return null; + var ns = scope.SecretNamespaces + .Where(n => name.StartsWith(n, StringComparison.Ordinal)) + .OrderByDescending(n => n.Length) + .FirstOrDefault(); + if (ns is null) return null; + var rest = name[ns.Length..]; + var slash = rest.IndexOf('/'); + return slash < 0 ? rest : rest[..slash]; + } + + /// + /// True when the config carries at least one in-scope secret: reference — i.e. the cross-instance rule + /// () has something to check, so a seam must load the instance list. + /// Lets seams skip the ConfigDb read otherwise. + /// + /// The owning driver's type. + /// The config JSON. + /// Whether any instance segment is present. + public static bool HasInstanceSegments(string? driverType, string? json) => + CollectSegments(driverType, json).Count > 0; + + /// + /// lmxopcua#835: inside one namespace, a reference whose instance-identifying segment (see + /// ) names a DIFFERENT existing instance of the same driver type — by its + /// or its , + /// case-insensitively — is refused: otherwise a Designer could create (or Test Connect / browse) a driver that + /// points at their own endpoint with secret:galaxy/<other-instance>/apikey and capture that + /// instance's key. Shared names that are no other instance's id or name (secret:galaxy/gw/apikey, + /// secret:galaxy/apikey) stay valid, as does a reference naming the instance itself. + /// + /// + /// A match against ANOTHER instance wins over a match against the owner — names are not unique, and renaming + /// one's own driver after a victim's must not unlock the victim's secret. + /// is null for a driver that does not exist yet (every listed instance is then "another"). + /// + /// The owning driver's type. + /// The DriverConfig / DeviceConfig / merged config JSON. + /// The driver the config belongs to; null when it is being created. + /// The known driver instances (any type; the owner may be included). + /// The problems; empty when acceptable, unreadable, or the type has no rules. + public static IReadOnlyList DescribeOtherInstanceProblems( + string? driverType, string? json, string? ownDriverInstanceId, IEnumerable instances) + { + ArgumentNullException.ThrowIfNull(instances); + var problems = new List(); + if (!TryGet(driverType, out var rules)) return problems; + var segments = CollectSegments(driverType, json); + if (segments.Count == 0) return problems; + + var others = instances + .Where(i => string.Equals(i.DriverType, rules.DriverType, StringComparison.OrdinalIgnoreCase) + && !string.Equals(i.DriverInstanceId, ownDriverInstanceId, StringComparison.Ordinal)) + .ToList(); + foreach (var (fieldPath, segment) in segments) + { + var other = others.FirstOrDefault(o => + string.Equals(o.DriverInstanceId, segment, StringComparison.OrdinalIgnoreCase) + || string.Equals(o.Name, segment, StringComparison.OrdinalIgnoreCase)); + if (other is null) continue; + problems.Add( + $"{fieldPath} names a secret under '{segment}/', which is another {rules.Scope.Label} driver instance " + + $"('{other.Name}', {other.DriverInstanceId}) — a driver may reference only its own secrets " + + $"(secret:{rules.Scope.SecretNamespaces[0]}/…) or a shared name that is no other " + + "instance's id or name (lmxopcua#835)."); + } + + return problems; + } + + /// + /// Both rules at once — (namespace, literals, path fields) followed + /// by — as every seam that knows the owner and the instance list applies them. + /// + /// The owning driver's type. + /// The config JSON. + /// The owning driver; null when it is being created. + /// The known driver instances. + /// The problems. + public static IReadOnlyList DescribeProblems( + string? driverType, string? json, string? ownDriverInstanceId, IEnumerable instances) => + [.. DescribeProblems(driverType, json), .. DescribeOtherInstanceProblems(driverType, json, ownDriverInstanceId, instances)]; + + private static List<(string FieldPath, string Segment)> CollectSegments(string? driverType, string? json) + { + var found = new List<(string, string)>(); + if (!TryGet(driverType, out var rules) || string.IsNullOrWhiteSpace(json)) return found; + JsonDocument doc; + try + { + doc = JsonDocument.Parse(json, new JsonDocumentOptions { CommentHandling = JsonCommentHandling.Skip, AllowTrailingCommas = true }); + } + catch (JsonException) + { + return found; // DescribeProblems reports unreadable JSON. + } + + using (doc) + { + Visit(doc.RootElement, "", rules, 0, (fieldPath, value, field, _) => + { + if (field is null || value.ValueKind != JsonValueKind.String) return; + if (InstanceSegment(value.GetString(), rules.Scope) is { Length: > 0 } segment) found.Add((fieldPath, segment)); + }); + } + + return found; + } } diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/LegacyCredentialReferenceTracker.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/LegacyCredentialReferenceTracker.cs index 4138e552f..0551d4644 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/LegacyCredentialReferenceTracker.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/LegacyCredentialReferenceTracker.cs @@ -61,22 +61,44 @@ public sealed class LegacyCredentialReferenceTracker( public void Report(string field) { var forms = $"secret:{scope.SecretNamespaces[0]} or env:{scope.EnvVariablePrefixes[0]}"; + Record( + field, + $"out-of-namespace ref still resolved (#815 warn-only, deprecated); next deploy refuses it — use {forms}", + $"{scope.Label} driver '{driverInstanceId}': {field} is a credential reference outside the {scope.Label} " + + $"namespace. It is still resolved for this already-deployed driver (lmxopcua#815 warn-only " + + $"compatibility — deprecated, a later release fails closed) but the deploy gate refuses it: migrate " + + $"it to {forms} before the next deploy. See docs/security.md §\"Credential references are namespaced per driver\"."); + } + + /// + /// Records that the key-material file field (lmxopcua#836, e.g. OpcUaClient + /// UserCertificatePath) was loaded from a path outside the node's certificate directory through the same + /// warn-only window (warns, rate-limited). Never carries the path. + /// + /// The field label. + /// How the operator configures the directory. + public void ReportPath(string field, string directoryDescription) => + Record( + field, + $"path outside the certificate directory still loaded (#836 warn-only, deprecated); next deploy refuses it — " + + $"move the file under {directoryDescription} and author it as a relative path", + $"{scope.Label} driver '{driverInstanceId}': {field} is not a relative path inside the node's certificate " + + $"directory ({directoryDescription}). It is still loaded for this already-deployed driver (lmxopcua#836 " + + "warn-only compatibility — deprecated, a later release fails closed) but the deploy gate refuses it: move " + + "the file into that directory and set the field to its relative path before the next deploy. See " + + "docs/security.md §\"Key-material file paths are confined to a node directory\"."); + + private void Record(string field, string diagnostic, string warning) + { var now = (clock ?? (() => DateTime.UtcNow))(); bool log; lock (_gate) { - _diagnostics[DiagnosticsKeyPrefix + field] = - $"out-of-namespace ref still resolved (#815 warn-only, deprecated); next deploy refuses it — use {forms}"; + _diagnostics[DiagnosticsKeyPrefix + field] = diagnostic; log = !_lastWarned.TryGetValue(field, out var last) || now - last >= WarnInterval; if (log) _lastWarned[field] = now; } - if (log) - { - warn($"{scope.Label} driver '{driverInstanceId}': {field} is a credential reference outside the {scope.Label} " - + $"namespace. It is still resolved for this already-deployed driver (lmxopcua#815 warn-only " - + $"compatibility — deprecated, a later release fails closed) but the deploy gate refuses it: migrate " - + $"it to {forms} before the next deploy. See docs/security.md §\"Credential references are namespaced per driver\"."); - } + if (log) warn(warning); } } diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Browser/OpcUaClientDriverBrowser.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Browser/OpcUaClientDriverBrowser.cs index cb318298c..fffd22558 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Browser/OpcUaClientDriverBrowser.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Browser/OpcUaClientDriverBrowser.cs @@ -234,12 +234,17 @@ public sealed class OpcUaClientDriverBrowser : IDriverBrowser /// Driver options carrying the form's auth fields. /// The secret store, or null when none is available. /// Cancellation token for the secret resolution. + /// + /// Test seam: the user-certificate directory (lmxopcua#836); null ⇒ the node's configured one. The browse never + /// passes a legacy tracker, so a pre-#836 absolute UserCertificatePath is refused here (hard-fail). + /// /// The identity the browse session activates with. internal static async Task BuildBrowseUserIdentityAsync( - OpcUaClientDriverOptions opts, ISecretResolver? secretResolver, CancellationToken ct) + OpcUaClientDriverOptions opts, ISecretResolver? secretResolver, CancellationToken ct, string? userCertificateDirectory = null) { ct.ThrowIfCancellationRequested(); - var resolved = await OpcUaUserIdentityFactory.ResolveSecretRefsAsync(opts, secretResolver, ct).ConfigureAwait(false); + var resolved = await OpcUaUserIdentityFactory.ResolveSecretRefsAsync( + opts, secretResolver, ct, userCertificateDirectory: userCertificateDirectory).ConfigureAwait(false); return OpcUaUserIdentityFactory.Build(resolved); } diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Session/OpcUaUserCertificatePaths.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Session/OpcUaUserCertificatePaths.cs new file mode 100644 index 000000000..2787b5a60 --- /dev/null +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Session/OpcUaUserCertificatePaths.cs @@ -0,0 +1,57 @@ +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; + +namespace ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient; + +/// +/// The node's OPC UA client user-certificate directory (lmxopcua#836). UserCertificatePath is authored as a +/// path RELATIVE to it and resolved by (lexical + link-resolved +/// containment), so a Designer can no longer make the node present an arbitrary PFX the service account can read. +/// +/// +/// Configured once per process from +/// (Drivers:OpcUaClient:UserCertificateDirectory, env Drivers__OpcUaClient__UserCertificateDirectory) +/// by the host at startup; unset ⇒ , beside the driver's PKI stores. Both the runtime +/// driver and the AdminUI browser read it, so admin and driver nodes must hold the same relative layout. +/// +public static class OpcUaUserCertificatePaths +{ + private static volatile string? _configured; + + /// The default: <LocalAppData>/OtOpcUa/pki/user-certificates. + public static string DefaultDirectory => Path.Combine( + Environment.GetFolderPath(Environment.SpecialFolder.LocalApplicationData), "OtOpcUa", "pki", "user-certificates"); + + /// The directory in force (configured, else ). + public static string CurrentDirectory => _configured ?? DefaultDirectory; + + /// Sets the process-wide directory (blank ⇒ the default). Called by the host at startup. + /// The configured directory. + public static void Configure(string? directory) => + _configured = string.IsNullOrWhiteSpace(directory) ? null : Path.GetFullPath(directory); + + /// + /// Resolves an authored UserCertificatePath to the file to load, fail-closed + /// (). A fully-qualified (pre-#836) path is refused unless + /// is given (the RUNNING driver only — deprecated warn-only window), in which case it is + /// reported and returned verbatim, exactly as before #836. Anything else that is not an acceptable relative path is + /// always refused. + /// + /// The authored path. + /// The directory; null ⇒ . + /// The running driver's tracker; null ⇒ hard-fail. + /// The path to load. + public static string Resolve(string value, string? directory = null, LegacyCredentialReferenceTracker? legacy = null) + { + var field = DriverCredentialPolicy.OpcUaClientUserCertificatePathField; + // Only a fully-qualified path is a pre-#836 value worth keeping alive: a relative one resolved against the + // service's working directory, which was never a meaningful configuration. + if (legacy is not null && !CredentialFilePaths.IsAcceptableRelative(value) && Path.IsPathFullyQualified(value)) + { + legacy.ReportPath(field.Name, field.DirectoryDescription); + return value; + } + + return CredentialFilePaths.ResolveWithin( + value, directory ?? CurrentDirectory, $"OpcUaClientDriverOptions.{field.Name}", field.DirectoryDescription); + } +} 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 e3e111f90..a043e99ec 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 @@ -81,6 +81,12 @@ public static class OpcUaUserIdentityFactory if (string.IsNullOrWhiteSpace(options.UserCertificatePath)) throw new InvalidOperationException( "OpcUaAuthType.Certificate requires OpcUaClientDriverOptions.UserCertificatePath to be set."); + // lmxopcua#836: an authored (relative) path is only ever loaded after ResolveSecretRefsAsync resolved it inside the + // node's user-certificate directory — never against the process's working directory. + if (!Path.IsPathFullyQualified(options.UserCertificatePath)) + throw new InvalidOperationException( + "OpcUaClientDriverOptions.UserCertificatePath has not been resolved against the node's user-certificate " + + "directory (ResolveSecretRefsAsync) — refusing to load a relative path (fail-closed)."); if (!File.Exists(options.UserCertificatePath)) throw new FileNotFoundException( $"User certificate not found at '{options.UserCertificatePath}'.", @@ -120,13 +126,26 @@ public static class OpcUaUserIdentityFactory /// out-of-namespace value keeps the old semantics (a case-insensitive secret: resolves, anything else is /// a literal) with a rate-limited warning. The AdminUI browser passes null (hard-fail). /// + /// + /// lmxopcua#836: the directory a identity's + /// is resolved inside (relative path, lexical + + /// link-resolved containment); null ⇒ . The returned + /// options carry the resolved full path. An absolute (pre-#836) path is refused unless + /// is given, in which case it is loaded as before with a warn-only report. + /// /// An options copy whose credential fields carry resolved plaintext. public static async Task ResolveSecretRefsAsync( OpcUaClientDriverOptions options, ISecretResolver? resolver, CancellationToken ct, - Func? environment = null, LegacyCredentialReferenceTracker? legacy = null) + Func? environment = null, LegacyCredentialReferenceTracker? legacy = null, + string? userCertificateDirectory = null) { ArgumentNullException.ThrowIfNull(options); + // #836 first: a refused certificate path fails before any secret/env read. + var certPath = options.AuthType == OpcUaAuthType.Certificate && !string.IsNullOrWhiteSpace(options.UserCertificatePath) + ? OpcUaUserCertificatePaths.Resolve(options.UserCertificatePath, userCertificateDirectory, legacy) + : options.UserCertificatePath; + var password = await ResolveFieldAsync(options.Password, nameof(options.Password), resolver, ct, environment, legacy) .ConfigureAwait(false); var certPassword = await ResolveFieldAsync( @@ -134,12 +153,13 @@ public static class OpcUaUserIdentityFactory .ConfigureAwait(false); if (ReferenceEquals(password, options.Password) - && ReferenceEquals(certPassword, options.UserCertificatePassword)) + && ReferenceEquals(certPassword, options.UserCertificatePassword) + && string.Equals(certPath, options.UserCertificatePath, StringComparison.Ordinal)) { return options; } - return options with { Password = password, UserCertificatePassword = certPassword }; + return options with { Password = password, UserCertificatePassword = certPassword, UserCertificatePath = certPath }; } private static async Task ResolveFieldAsync( 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 f3861c768..38104536c 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Browsing/BrowserSessionService.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Browsing/BrowserSessionService.cs @@ -1,7 +1,10 @@ using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Components.Authorization; +using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.Logging; using ZB.MOM.WW.OtOpcUa.Commons.Browsing; +using ZB.MOM.WW.OtOpcUa.Configuration; +using ZB.MOM.WW.OtOpcUa.Configuration.Validation; using ZB.MOM.WW.OtOpcUa.Core.Abstractions; using ZB.MOM.WW.OtOpcUa.Security.Auth; @@ -20,13 +23,18 @@ namespace ZB.MOM.WW.OtOpcUa.AdminUI.Browsing; /// The Discover-backed fallback browser. /// Policy evaluator for . /// Supplies the calling circuit's user. +/// +/// The config DB, for the cross-instance credential rule (lmxopcua#835). Null (test graphs) ⇒ a config carrying an +/// in-scope secret: reference cannot be checked and is refused (fail-closed). +/// public sealed class BrowserSessionService( IEnumerable browsers, BrowseSessionRegistry registry, ILogger logger, IUniversalDriverBrowser universalBrowser, IAuthorizationService authorizationService, - AuthenticationStateProvider authenticationStateProvider) : IBrowserSessionService + AuthenticationStateProvider authenticationStateProvider, + IDbContextFactory? dbFactory = null) : IBrowserSessionService { /// Upper bound on a single root/expand/attributes call. public static readonly TimeSpan PerCallTimeout = TimeSpan.FromSeconds(20); @@ -35,7 +43,11 @@ public sealed class BrowserSessionService( browsers.ToDictionary(b => b.DriverType, StringComparer.OrdinalIgnoreCase); /// - public async Task OpenAsync(string driverType, string configJson, CancellationToken ct) + public Task OpenAsync(string driverType, string configJson, CancellationToken ct) => + OpenAsync(driverType, configJson, null, ct); + + /// + public async Task OpenAsync(string driverType, string configJson, string? driverInstanceId, 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 @@ -44,6 +56,19 @@ public sealed class BrowserSessionService( if (credentialProblems.Count > 0) return new(false, "Browse refused: " + string.Join(" ", credentialProblems), Guid.Empty); + // lmxopcua#835: nor may an in-namespace reference name ANOTHER driver instance's secret. + if (DriverCredentialPolicy.HasInstanceSegments(driverType, configJson)) + { + if (dbFactory is null) + return new(false, "Browse refused: the driver-instance list needed to check this config's secret: " + + "references is unavailable (lmxopcua#835, fail-closed).", Guid.Empty); + await using var db = await dbFactory.CreateDbContextAsync(ct).ConfigureAwait(false); + var instanceProblems = await DriverInstanceIdentities + .DescribeOtherInstanceProblemsAsync(db, driverType, configJson, driverInstanceId, ct).ConfigureAwait(false); + if (instanceProblems.Count > 0) + return new(false, "Browse refused: " + string.Join(" ", instanceProblems), 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/Browsing/IBrowserSessionService.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Browsing/IBrowserSessionService.cs index 0a70f8880..27ac9755d 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Browsing/IBrowserSessionService.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Browsing/IBrowserSessionService.cs @@ -27,6 +27,19 @@ public interface IBrowserSessionService /// The result of opening the session, including the registry token on success. Task OpenAsync(string driverType, string configJson, CancellationToken ct); + /// + /// As , naming the driver instance the config belongs to + /// (lmxopcua#835: its own secret:<ns>/<id>/… references are allowed, another instance's are refused; + /// null ⇒ a not-yet-created instance, which is stricter). The default forwards without the id (test doubles). + /// + /// The driver type to open a browse session against. + /// The driver configuration as JSON. + /// The owning driver instance, or null. + /// Cancellation token for the operation. + /// The result of opening the session, including the registry token on success. + Task OpenAsync(string driverType, string configJson, string? driverInstanceId, CancellationToken ct) => + OpenAsync(driverType, configJson, ct); + /// Returns the root nodes of an open session. Throws /// if the token is unknown. /// Registry handle for the open browse session. diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Clients/AdminProbeService.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Clients/AdminProbeService.cs index 7db8de619..93fef2ed6 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Clients/AdminProbeService.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Clients/AdminProbeService.cs @@ -25,15 +25,17 @@ public sealed class AdminProbeService /// Driver config as JSON (same shape as DriverInstance.DriverConfig). /// Per-probe timeout; actor clamps to [1, 60]. /// Optional cancellation token from the caller. + /// The owning driver instance (lmxopcua#835); null ⇒ a not-yet-created one. /// The probe result, or a timeout failure if no reply arrives in time. public async Task TestAsync( string driverType, string configJson, int timeoutSeconds, - CancellationToken ct = default) + CancellationToken ct = default, + string? driverInstanceId = null) { var correlationId = Guid.NewGuid(); - var msg = new TestDriverConnect(driverType, configJson, timeoutSeconds, correlationId); + var msg = new TestDriverConnect(driverType, configJson, timeoutSeconds, correlationId, driverInstanceId); // 65s outer guard — the actor's CTS clamps to 60s; if the Ask never returns we still want // a deterministic failure surface for the UI. diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/DeviceModal.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/DeviceModal.razor index fb380aaf8..75d26663e 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/DeviceModal.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/DeviceModal.razor @@ -59,7 +59,7 @@ }
- +
@if (DriverConfigSaveGate.Validate(_driverType, _deviceConfigJson) is { } gateError) diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/DriverTestConnectButton.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/DriverTestConnectButton.razor index 356b8ef7f..df8890a5a 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/DriverTestConnectButton.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/DriverTestConnectButton.razor @@ -60,6 +60,12 @@ ///
[Parameter] public int TimeoutSeconds { get; set; } = TestConnectBudget.DefaultSeconds; + /// + /// The driver instance the config belongs to (lmxopcua#835: its own secret:<ns>/<id>/… references are + /// allowed, another instance's are refused). Null ⇒ treated as a not-yet-created instance (stricter). + /// + [Parameter] public string? DriverInstanceId { get; set; } + private const int ClearAfterMs = 30_000; private bool _busy; @@ -83,7 +89,7 @@ var json = GetConfigJson() ?? "{}"; var budget = TestConnectBudget.Resolve(json, TimeoutSeconds, DriverType); _lastBudgetSeconds = budget; - _result = await Probe.TestAsync(DriverType, json, budget); + _result = await Probe.TestAsync(DriverType, json, budget, driverInstanceId: DriverInstanceId); } catch (Exception ex) { 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 bc27c6c1b..f7b6dae71 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 @@ -146,7 +146,8 @@
+ placeholder="plant1/user.pfx" /> +
Relative path inside the node's user-certificate directory (Drivers:OpcUaClient:UserCertificateDirectory, default <LocalAppData>/OtOpcUa/pki/user-certificates). Absolute paths and .. are refused (#836).
diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Raw/RawBrowseModal.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Raw/RawBrowseModal.razor index 8ced0c544..964bc392b 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Raw/RawBrowseModal.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Raw/RawBrowseModal.razor @@ -463,7 +463,12 @@ StateHasChanged(); try { - var result = await BrowserService.OpenAsync(_driverType, _mergedConfig, default); + // lmxopcua#835: name the owning driver so its own secret://… references are told apart from + // another instance's (an unresolvable owner is treated as a new driver — stricter). + var ownerDriverId = string.IsNullOrEmpty(_effectiveDeviceId) + ? null + : await Svc.LoadDeviceDriverInstanceIdAsync(_effectiveDeviceId); + var result = await BrowserService.OpenAsync(_driverType, _mergedConfig, ownerDriverId, default); // Host disposed, modal closed, or modal re-opened mid-open: close the session nobody will (R04 L4). if (BrowseOpenGuard.CloseIfOrphaned(result, _disposed || IsStale(generation), BrowserService)) { return; } if (result.Ok) diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Deployments/CredentialReferencePreflight.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Deployments/CredentialReferencePreflight.cs index b553bce25..f584da4ee 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Deployments/CredentialReferencePreflight.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Deployments/CredentialReferencePreflight.cs @@ -77,22 +77,26 @@ public sealed class CredentialReferencePreflight(IDbContextFactory(); var driverList = drivers.ToList(); var byId = driverList.ToDictionary(d => d.DriverInstanceId, StringComparer.Ordinal); + // lmxopcua#835: the cross-instance rule is checked against the instance set of the same source. + var draftInstances = driverList.Select(d => new DriverInstanceIdentity(d.DriverInstanceId, d.Name, d.DriverType)).ToList(); foreach (var d in driverList) { - foreach (var p in DriverCredentialPolicy.DescribeProblems(d.DriverType, d.DriverConfig)) + foreach (var p in DriverCredentialPolicy.DescribeProblems(d.DriverType, d.DriverConfig, d.DriverInstanceId, draftInstances)) findings.Add(new(CredentialFindingSource.Draft, d.DriverType, d.Name, d.DriverInstanceId, null, p)); } foreach (var dev in devices) { if (!byId.TryGetValue(dev.DriverInstanceId, out var owner)) continue; - foreach (var p in DriverCredentialPolicy.DescribeProblems(owner.DriverType, dev.DeviceConfig)) + foreach (var p in DriverCredentialPolicy.DescribeProblems(owner.DriverType, dev.DeviceConfig, owner.DriverInstanceId, draftInstances)) findings.Add(new(CredentialFindingSource.Draft, owner.DriverType, owner.Name, owner.DriverInstanceId, dev.Name, p)); } - foreach (var s in deployed) + var deployedList = deployed.ToList(); + var deployedInstances = deployedList.Select(s => new DriverInstanceIdentity(s.DriverInstanceId, s.Name, s.DriverType)).ToList(); + foreach (var s in deployedList) { - foreach (var p in DriverCredentialPolicy.DescribeProblems(s.DriverType, s.DriverConfig)) + foreach (var p in DriverCredentialPolicy.DescribeProblems(s.DriverType, s.DriverConfig, s.DriverInstanceId, deployedInstances)) findings.Add(new(CredentialFindingSource.Deployed, s.DriverType, s.Name, s.DriverInstanceId, null, p)); } 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 9b62b3a16..6b69c6a97 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/DriverConfigSaveGate.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/DriverConfigSaveGate.cs @@ -1,3 +1,5 @@ +using ZB.MOM.WW.OtOpcUa.Configuration; +using ZB.MOM.WW.OtOpcUa.Configuration.Validation; using ZB.MOM.WW.OtOpcUa.Core.Abstractions; namespace ZB.MOM.WW.OtOpcUa.AdminUI.Uns; @@ -26,4 +28,24 @@ public static class DriverConfigSaveGate var problems = DriverCredentialPolicy.DescribeProblems(driverType, configJson); return problems.Count == 0 ? null : string.Join(" ", problems); } + + /// + /// The authority check runs on every driver/device create + update: + /// plus the cross-instance rule (lmxopcua#835 — a secret:galaxy/<X>/… reference may not name ANOTHER + /// existing instance of the same driver type), which needs the stored instance list. + /// + /// The config DB (read only when the config carries an in-scope secret: reference). + /// The owning driver's type. + /// The DriverConfig or DeviceConfig JSON about to be stored. + /// The owning driver; null when it is being created. + /// Cancellation. + /// The refusal (never containing a credential value), or null. + public static async Task ValidateAsync( + OtOpcUaConfigDbContext db, string? driverType, string? configJson, string? ownDriverInstanceId, CancellationToken ct) + { + if (Validate(driverType, configJson) is { } refusal) return refusal; + var problems = await DriverInstanceIdentities + .DescribeOtherInstanceProblemsAsync(db, driverType, configJson, ownDriverInstanceId, ct).ConfigureAwait(false); + return problems.Count == 0 ? null : string.Join(" ", problems); + } } diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/IRawTreeService.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/IRawTreeService.cs index b3f1ef046..3e8013a28 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/IRawTreeService.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/IRawTreeService.cs @@ -394,4 +394,15 @@ public interface IRawTreeService /// A token to cancel the load. /// The parent driver's type + the merged config JSON, or null when the device/driver cannot be resolved. Task<(string DriverType, string MergedConfigJson)?> LoadMergedProbeConfigAsync(string deviceId, CancellationToken ct = default); + + /// + /// The driver instance owning , or null when it cannot be resolved — handed to the live + /// browse so the cross-instance credential rule (lmxopcua#835) can tell the driver's own secrets from another + /// instance's. The default (test doubles) returns null, which is the stricter "not-yet-created" treatment. + /// + /// The device. + /// A token to cancel the load. + /// The owning driver instance id, or null. + Task LoadDeviceDriverInstanceIdAsync(string deviceId, CancellationToken ct = default) => + Task.FromResult(null); } diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/RawTreeService.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/RawTreeService.cs index 6b5a9064d..e1fa723b8 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/RawTreeService.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/RawTreeService.cs @@ -488,6 +488,10 @@ public sealed class RawTreeService(IDbContextFactory dbF await using var db = await dbFactory.CreateDbContextAsync(ct); + // lmxopcua#835: a new driver owns no secrets yet — a reference naming any existing instance is refused. + if (await DriverConfigSaveGate.ValidateAsync(db, driverType, driverConfigJson, null, ct) is { } instanceRefusal) + return new UnsMutationResult(false, instanceRefusal); + if (!await db.ServerClusters.AnyAsync(c => c.ClusterId == clusterId, ct)) return new UnsMutationResult(false, $"Cluster '{clusterId}' not found."); @@ -621,7 +625,7 @@ public sealed class RawTreeService(IDbContextFactory dbF var owner = await db.DriverInstances.FirstOrDefaultAsync(d => d.DriverInstanceId == driverInstanceId, ct); if (owner is null) return new UnsMutationResult(false, $"Driver '{driverInstanceId}' not found."); - if (DriverConfigSaveGate.Validate(owner.DriverType, deviceConfigJson) is { } configRefusal) + if (await DriverConfigSaveGate.ValidateAsync(db, owner.DriverType, deviceConfigJson, owner.DriverInstanceId, ct) is { } configRefusal) return new UnsMutationResult(false, configRefusal); // MTConnect owns one agent client per driver — mirror the deploy gate @@ -1116,7 +1120,7 @@ public sealed class RawTreeService(IDbContextFactory dbF var entity = await db.DriverInstances.FirstOrDefaultAsync(d => d.DriverInstanceId == driverInstanceId, ct); if (entity is null) return new UnsMutationResult(false, "Row no longer exists."); - if (DriverConfigSaveGate.Validate(entity.DriverType, driverConfigJson) is { } configRefusal) + if (await DriverConfigSaveGate.ValidateAsync(db, entity.DriverType, driverConfigJson, entity.DriverInstanceId, ct) is { } configRefusal) return new UnsMutationResult(false, configRefusal); // Sibling uniqueness in the same scope RenameDriverAsync uses (cluster + folder). @@ -1178,7 +1182,8 @@ public sealed class RawTreeService(IDbContextFactory dbF var entity = await db.Devices.FirstOrDefaultAsync(dev => dev.DeviceId == deviceId, ct); if (entity is null) return new UnsMutationResult(false, "Row no longer exists."); - if (DriverConfigSaveGate.Validate(await ResolveDriverTypeAsync(db, deviceId, ct), deviceConfigJson) is { } configRefusal) + if (await DriverConfigSaveGate.ValidateAsync( + db, await ResolveDriverTypeAsync(db, deviceId, ct), deviceConfigJson, entity.DriverInstanceId, ct) is { } configRefusal) return new UnsMutationResult(false, configRefusal); if (await db.Devices.AnyAsync( @@ -1201,6 +1206,16 @@ public sealed class RawTreeService(IDbContextFactory dbF } } + /// + public async Task LoadDeviceDriverInstanceIdAsync(string deviceId, CancellationToken ct = default) + { + await using var db = await dbFactory.CreateDbContextAsync(ct); + return await db.Devices.AsNoTracking() + .Where(dev => dev.DeviceId == deviceId) + .Select(dev => dev.DriverInstanceId) + .FirstOrDefaultAsync(ct); + } + /// public async Task<(string DriverType, string MergedConfigJson)?> LoadMergedProbeConfigAsync( string deviceId, CancellationToken ct = default) 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 0f8ed08f8..d72e40614 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/AdminOperations/AdminOperationsActor.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/AdminOperations/AdminOperationsActor.cs @@ -611,6 +611,19 @@ public sealed class AdminOperationsActor : ReceiveActor var sw = System.Diagnostics.Stopwatch.StartNew(); try { + // lmxopcua#835: a reference naming ANOTHER driver instance's secret (secret:galaxy//…) is refused + // before the probe resolves it — off the mailbox, since it reads the ConfigDb (only when the config + // carries an in-scope secret: reference at all). + if (DriverCredentialPolicy.HasInstanceSegments(msg.DriverType, msg.ConfigJson)) + { + await using var db = await _dbFactory.CreateDbContextAsync(cts.Token); + var instanceProblems = await DriverInstanceIdentities.DescribeOtherInstanceProblemsAsync( + db, msg.DriverType, msg.ConfigJson, msg.DriverInstanceId, cts.Token); + if (instanceProblems.Count > 0) + return new TestDriverConnectResult( + false, "Test Connect refused: " + string.Join(" ", instanceProblems), null, msg.CorrelationId); + } + var result = await probe.ProbeAsync(msg.ConfigJson, timeout, cts.Token); sw.Stop(); return new TestDriverConnectResult( diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Host/Program.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Host/Program.cs index ae81785d6..132ead6f6 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Host/Program.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Host/Program.cs @@ -194,6 +194,10 @@ builder.Services.AddValidatedOptions(builder. // Test Connect probe (admin role, which must probe the stack the driver would run). A bad value fails host // start here rather than silently falling back. builder.Services.AddAbCipDriverDefaults(builder.Configuration); +// lmxopcua#836: the directory OpcUaClient UserCertificatePath (a relative path) is confined to. Unconditional for the +// same reason — the driver (driver role) and the AdminUI live browse (admin role) both load user certificates. +ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.OpcUaUserCertificatePaths.Configure( + builder.Configuration[ZB.MOM.WW.OtOpcUa.Core.Abstractions.DriverCredentialPolicy.OpcUaClientUserCertificateDirectoryKey]); // lmxopcua#765: refresh/staleness of the cached API-key → Galaxy ack identity set (driver nodes consume it). builder.Services.AddGalaxyAckIdentityCache(builder.Configuration); diff --git a/tests/Core/ZB.MOM.WW.OtOpcUa.Configuration.Tests/DraftValidatorCredentialInstanceAndPathTests.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Configuration.Tests/DraftValidatorCredentialInstanceAndPathTests.cs new file mode 100644 index 000000000..db32e2045 --- /dev/null +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Configuration.Tests/DraftValidatorCredentialInstanceAndPathTests.cs @@ -0,0 +1,91 @@ +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; + +/// +/// The deploy gate for lmxopcua#835 (a credential reference naming ANOTHER driver instance's secret) and #836 (the +/// OpcUaClient user-certificate path confined to the node's certificate directory). +/// +[Trait("Category", "Unit")] +public sealed class DraftValidatorCredentialInstanceAndPathTests +{ + private static IReadOnlyList Validate(params DriverInstance[] drivers) => + Validate(drivers, []); + + private static IReadOnlyList Validate(DriverInstance[] drivers, Device[] devices) => + DraftValidator.Validate(new DraftSnapshot + { + GenerationId = 1, + ClusterId = "c", + DriverInstances = drivers, + Devices = devices, + Tags = [], + }).Where(e => e.Code is CredentialReferenceDeployChecks.Code or CredentialReferenceDeployChecks.OtherInstanceCode).ToList(); + + private static DriverInstance Drv(string id, string name, string type, string config) => + new() { DriverInstanceId = id, ClusterId = "c", Name = name, DriverType = type, DriverConfig = config }; + + [Fact] + public void A_galaxy_driver_naming_another_instances_key_is_refused() + { + var e = Validate( + Drv("DRV-victim", "Victim", "GalaxyMxGateway", """{"Gateway":{"ApiKeySecretRef":"secret:galaxy/DRV-victim/apikey"}}"""), + Drv("DRV-evil", "Evil", "GalaxyMxGateway", """{"Gateway":{"Endpoint":"http://evil:5001","ApiKeySecretRef":"secret:galaxy/DRV-victim/apikey"}}""")) + .ShouldHaveSingleItem(); + e.Code.ShouldBe(CredentialReferenceDeployChecks.OtherInstanceCode); + e.Context.ShouldBe("DRV-evil"); + e.Message.ShouldContain("lmxopcua#835"); + } + + [Fact] + public void A_device_config_naming_another_instance_by_name_is_refused() => + Validate( + [ + Drv("DRV-a", "Victim", "OpcUaClient", "{}"), + Drv("DRV-b", "Evil", "OpcUaClient", "{}"), + ], + [new Device { DeviceId = "dev-b", DriverInstanceId = "DRV-b", Name = "d", DeviceConfig = """{"Password":"secret:opcuaclient/victim/pw"}""" }]) + .ShouldHaveSingleItem().Context.ShouldBe("dev-b"); + + [Fact] + public void Shared_and_own_keys_pass() => + Validate( + Drv("DRV-a", "A", "GalaxyMxGateway", """{"Gateway":{"ApiKeySecretRef":"secret:galaxy/DRV-a/apikey"}}"""), + Drv("DRV-b", "B", "GalaxyMxGateway", """{"Gateway":{"ApiKeySecretRef":"secret:galaxy/gw/apikey"}}"""), + Drv("DRV-c", "C", "GalaxyMxGateway", """{"Gateway":{"ApiKeySecretRef":"secret:galaxy/gw/apikey"}}""")) + .ShouldBeEmpty(); + + [Fact] + public void Heidenhain_cross_instance_is_reported_by_this_gate() => + DraftValidator.Validate(new DraftSnapshot + { + GenerationId = 1, ClusterId = "c", + DriverInstances = + [ + Drv("H1", "line1", "Heidenhain", """{"Access":{"InspectPassword":"secret:heidenhain/line1/inspect"}}"""), + Drv("H2", "line2", "Heidenhain", """{"Access":{"InspectPassword":"secret:heidenhain/line1/inspect"}}"""), + ], + Devices = [], Tags = [], + }) + .Where(e => e.Code == CredentialReferenceDeployChecks.OtherInstanceCode) + .ShouldHaveSingleItem().Context.ShouldBe("H2"); + + [Theory] + [InlineData("""{"AuthType":"Certificate","UserCertificatePath":"C:\\Windows\\System32\\config\\systemprofile\\host.pfx"}""")] + [InlineData("""{"UserCertificatePath":"/etc/ssl/private/host.pfx"}""")] + [InlineData("""{"UserCertificatePath":"plant1/../../host.pfx"}""")] + public void An_opcuaclient_user_certificate_path_outside_the_directory_is_refused(string config) + { + var e = Validate(Drv("DRV-o", "o", "OpcUaClient", config)).ShouldHaveSingleItem(); + e.Code.ShouldBe(CredentialReferenceDeployChecks.Code); + e.Message.ShouldContain("lmxopcua#836"); + e.Message.ShouldNotContain("host.pfx"); + } + + [Fact] + public void A_relative_user_certificate_path_passes() => + Validate(Drv("DRV-o", "o", "OpcUaClient", """{"AuthType":"Certificate","UserCertificatePath":"plant1/user.pfx"}""")).ShouldBeEmpty(); +} diff --git a/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests/CredentialFilePathsTests.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests/CredentialFilePathsTests.cs new file mode 100644 index 000000000..b2a4d0856 --- /dev/null +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests/CredentialFilePathsTests.cs @@ -0,0 +1,166 @@ +using Shouldly; +using Xunit; + +namespace ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests; + +/// +/// lmxopcua#836: a key-material file field (OpcUaClient UserCertificatePath) is a relative path confined to a +/// node directory — syntactically at the admin seams, lexically + through links at load time. +/// +[Trait("Category", "Unit")] +public sealed class CredentialFilePathsTests +{ + [Theory] + [InlineData("user.pfx")] + [InlineData("plant1/user.pfx")] + [InlineData("plant1\\user.pfx")] + [InlineData("a/b-c_d.e/f.pfx")] + public void Plain_relative_paths_are_acceptable(string path) => CredentialFilePaths.IsAcceptableRelative(path).ShouldBeTrue(); + + [Theory] + [InlineData("/etc/ssl/private/host.pfx")] + [InlineData("\\\\server\\share\\x.pfx")] + [InlineData("\\x.pfx")] + [InlineData("C:\\certs\\user.pfx")] + [InlineData("C:user.pfx")] + [InlineData("../user.pfx")] + [InlineData("plant1/../../user.pfx")] + [InlineData("./user.pfx")] + [InlineData("plant1//user.pfx")] + [InlineData("plant1/")] + [InlineData("~/user.pfx")] + [InlineData("user pfx")] + [InlineData("secret:opcuaclient/x")] + public void Absolute_traversal_and_odd_paths_are_refused(string path) + { + CredentialFilePaths.IsAcceptableRelative(path).ShouldBeFalse(); + CredentialFilePaths.IsAcceptable(path).ShouldBeFalse(); + } + + [Fact] + public void Blank_is_acceptable_but_not_a_relative_path() + { + CredentialFilePaths.IsAcceptable(null).ShouldBeTrue(); + CredentialFilePaths.IsAcceptable("").ShouldBeTrue(); + CredentialFilePaths.IsAcceptableRelative("").ShouldBeFalse(); + } + + [Theory] + [InlineData("""{"UserCertificatePath":"/etc/ssl/private/host.pfx"}""", 1)] + [InlineData("""{"userCertificatePath":"..\\..\\Windows\\x.pfx"}""", 1)] + [InlineData("""{"Devices":[{"UserCertificatePath":"C:\\x.pfx"}]}""", 1)] + [InlineData("""{"UserCertificatePath":42}""", 1)] + [InlineData("""{"UserCertificatePath":"plant1/user.pfx"}""", 0)] + [InlineData("""{"UserCertificatePath":null}""", 0)] + public void The_driver_policy_scans_the_OpcUaClient_path_field(string config, int expected) + { + var problems = DriverCredentialPolicy.DescribeProblems(DriverTypeNames.OpcUaClient, config); + problems.Count.ShouldBe(expected); + if (expected > 0) + { + problems[0].ShouldContain("lmxopcua#836"); + problems[0].ShouldContain(DriverCredentialPolicy.OpcUaClientUserCertificateDirectoryKey); + problems[0].ShouldNotContain("host.pfx"); + } + } + + [Fact] + public void Other_drivers_do_not_scan_the_path_field() => + DriverCredentialPolicy.DescribeProblems(DriverTypeNames.Galaxy, """{"UserCertificatePath":"/etc/x.pfx"}""").ShouldBeEmpty(); + + [Fact] + public void ResolveWithin_returns_the_path_inside_the_directory() + { + using var dir = new TempDir(); + var resolved = CredentialFilePaths.ResolveWithin("plant1/user.pfx", dir.Path, "F", "D"); + resolved.ShouldBe(Path.Combine(Path.GetFullPath(dir.Path), "plant1", "user.pfx")); + } + + [Theory] + [InlineData("../escape.pfx")] + [InlineData("/etc/passwd")] + public void ResolveWithin_refuses_a_bad_value_fail_closed(string value) + { + using var dir = new TempDir(); + Should.Throw(() => CredentialFilePaths.ResolveWithin(value, dir.Path, "F", "D")) + .Message.ShouldContain("fail-closed"); + } + + [Fact] + public void ResolveWithin_refuses_a_file_symlink_that_escapes_the_directory() + { + using var dir = new TempDir(); + using var outside = new TempDir(); + var target = Path.Combine(outside.Path, "victim.pfx"); + File.WriteAllText(target, "x"); + var link = Path.Combine(dir.Path, "user.pfx"); + if (!TryCreateLink(() => File.CreateSymbolicLink(link, target))) return; + + Should.Throw(() => CredentialFilePaths.ResolveWithin("user.pfx", dir.Path, "F", "D")) + .Message.ShouldContain("symbolic link"); + } + + [Fact] + public void ResolveWithin_refuses_a_directory_symlink_that_escapes_the_directory() + { + using var dir = new TempDir(); + using var outside = new TempDir(); + File.WriteAllText(Path.Combine(outside.Path, "victim.pfx"), "x"); + if (!TryCreateLink(() => Directory.CreateSymbolicLink(Path.Combine(dir.Path, "plant1"), outside.Path))) return; + + Should.Throw(() => CredentialFilePaths.ResolveWithin("plant1/victim.pfx", dir.Path, "F", "D")); + } + + [Fact] + public void ResolveWithin_accepts_a_symlink_that_stays_inside_the_directory() + { + using var dir = new TempDir(); + Directory.CreateDirectory(Path.Combine(dir.Path, "real")); + File.WriteAllText(Path.Combine(dir.Path, "real", "user.pfx"), "x"); + if (!TryCreateLink(() => File.CreateSymbolicLink(Path.Combine(dir.Path, "alias.pfx"), Path.Combine(dir.Path, "real", "user.pfx")))) return; + + CredentialFilePaths.ResolveWithin("alias.pfx", dir.Path, "F", "D").ShouldEndWith("alias.pfx"); + } + + [Fact] + public void ResolveWithin_accepts_a_directory_that_is_itself_reached_through_a_link() + { + // e.g. macOS /var -> /private/var: the configured directory's own parents may be links. + using var outside = new TempDir(); + using var holder = new TempDir(); + var linkedDir = Path.Combine(holder.Path, "certs"); + if (!TryCreateLink(() => Directory.CreateSymbolicLink(linkedDir, outside.Path))) return; + File.WriteAllText(Path.Combine(outside.Path, "user.pfx"), "x"); + + CredentialFilePaths.ResolveWithin("user.pfx", linkedDir, "F", "D").ShouldEndWith("user.pfx"); + } + + private static bool TryCreateLink(Action create) + { + try + { + create(); + return true; + } + catch (Exception e) when (e is UnauthorizedAccessException or IOException) + { + return false; // Windows without the symlink privilege: the lexical tests still cover the rule. + } + } + + private sealed class TempDir : IDisposable + { + public TempDir() + { + Path = System.IO.Path.Combine(System.IO.Path.GetTempPath(), "otopcua-836-" + Guid.NewGuid().ToString("N")); + Directory.CreateDirectory(Path); + } + + public string Path { get; } + + public void Dispose() + { + try { Directory.Delete(Path, recursive: true); } catch (IOException) { } catch (UnauthorizedAccessException) { } + } + } +} diff --git a/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests/CredentialInstanceScopeTests.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests/CredentialInstanceScopeTests.cs new file mode 100644 index 000000000..22e032bca --- /dev/null +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests/CredentialInstanceScopeTests.cs @@ -0,0 +1,116 @@ +using Shouldly; +using Xunit; + +namespace ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests; + +/// +/// lmxopcua#835: inside one namespace, a credential reference whose instance segment +/// (secret:galaxy/<X>/…) names ANOTHER existing instance of the same driver type is refused. +/// +[Trait("Category", "Unit")] +public sealed class CredentialInstanceScopeTests +{ + private static readonly DriverInstanceIdentity[] Instances = + [ + new("DRV-aaa", "PlantA", DriverTypeNames.Galaxy), + new("DRV-bbb", "PlantB", DriverTypeNames.Galaxy), + new("DRV-ccc", "UpstreamC", DriverTypeNames.OpcUaClient), + new("DRV-ddd", "UpstreamD", DriverTypeNames.OpcUaClient), + ]; + + private static string Galaxy(string apiKeyRef) => + "{\"Gateway\":{\"Endpoint\":\"http://evil:5001\",\"ApiKeySecretRef\":" + System.Text.Json.JsonSerializer.Serialize(apiKeyRef) + "}}"; + + [Theory] + [InlineData("secret:galaxy/DRV-bbb/apikey")] + [InlineData("secret:galaxy/PlantB/apikey")] + [InlineData("secret:galaxy/plantb/apikey")] // case-insensitive + [InlineData("secret:otopcua/galaxy/PlantB/apikey")] // the second namespace + [InlineData("${secret:galaxy/PlantB/apikey}")] // the token spelling + [InlineData("secret:galaxy/PlantB")] // the segment alone + public void A_reference_naming_another_instance_is_refused(string reference) + { + var problem = DriverCredentialPolicy + .DescribeOtherInstanceProblems(DriverTypeNames.Galaxy, Galaxy(reference), "DRV-aaa", Instances) + .ShouldHaveSingleItem(); + problem.ShouldContain("Gateway.ApiKeySecretRef"); + problem.ShouldContain("lmxopcua#835"); + problem.ShouldContain("DRV-bbb"); + } + + [Theory] + [InlineData("secret:galaxy/DRV-aaa/apikey")] // its own id + [InlineData("secret:galaxy/PlantA/apikey")] // its own name + [InlineData("secret:galaxy/gw/apikey")] // a shared name no instance uses + [InlineData("secret:galaxy/apikey")] + [InlineData("secret:galaxy/UpstreamC/apikey")] // another TYPE's instance — not this namespace's owner + [InlineData("env:OTOPCUA_GALAXY_PLANTB_KEY")] // env carries no instance convention + [InlineData("dev:k")] + [InlineData("")] + public void Own_shared_and_non_secret_references_pass(string reference) => + DriverCredentialPolicy.DescribeOtherInstanceProblems(DriverTypeNames.Galaxy, Galaxy(reference), "DRV-aaa", Instances) + .ShouldBeEmpty(); + + [Fact] + public void A_driver_being_created_may_name_no_existing_instance() => + DriverCredentialPolicy.DescribeOtherInstanceProblems(DriverTypeNames.Galaxy, Galaxy("secret:galaxy/PlantA/apikey"), null, Instances) + .ShouldHaveSingleItem(); + + [Fact] + public void Renaming_ones_own_driver_after_the_victim_does_not_unlock_the_victims_secret() + { + // Attacker DRV-zzz renamed itself "PlantB" (names are unique per folder, not fleet-wide). + var withTwin = Instances.Append(new DriverInstanceIdentity("DRV-zzz", "PlantB", DriverTypeNames.Galaxy)); + DriverCredentialPolicy.DescribeOtherInstanceProblems(DriverTypeNames.Galaxy, Galaxy("secret:galaxy/PlantB/apikey"), "DRV-zzz", withTwin) + .ShouldHaveSingleItem(); + } + + [Fact] + public void The_ack_password_and_device_level_fields_are_checked_too() + { + DriverCredentialPolicy.DescribeOtherInstanceProblems( + DriverTypeNames.Galaxy, """{"AlarmAck":{"FallbackPasswordSecretRef":"secret:otopcua/galaxy/DRV-bbb/ack"}}""", "DRV-aaa", Instances) + .ShouldHaveSingleItem().ShouldContain("AlarmAck.FallbackPasswordSecretRef"); + DriverCredentialPolicy.DescribeOtherInstanceProblems( + DriverTypeNames.Galaxy, """{"Devices":[{"Gateway":{"ApiKeySecretRef":"secret:galaxy/PlantB/k"}}]}""", "DRV-aaa", Instances) + .ShouldHaveSingleItem().ShouldContain("Devices[0].Gateway.ApiKeySecretRef"); + } + + [Theory] + [InlineData("""{"Password":"secret:opcuaclient/UpstreamD/pw"}""", 1)] + [InlineData("""{"UserCertificatePassword":"secret:opcuaclient/DRV-ddd/pfx"}""", 1)] + [InlineData("""{"Password":"secret:opcuaclient/UpstreamC/pw"}""", 0)] + [InlineData("""{"Password":"secret:opcuaclient/shared/pw"}""", 0)] + public void OpcUaClient_has_the_same_rule(string config, int expected) => + DriverCredentialPolicy.DescribeOtherInstanceProblems(DriverTypeNames.OpcUaClient, config, "DRV-ccc", Instances) + .Count.ShouldBe(expected); + + [Fact] + public void Heidenhain_has_the_same_rule() + { + IEnumerable hh = [new("H1", "line1", DriverTypeNames.Heidenhain), new("H2", "line2", DriverTypeNames.Heidenhain)]; + DriverCredentialPolicy.DescribeOtherInstanceProblems( + DriverTypeNames.Heidenhain, """{"Access":{"InspectPassword":"secret:heidenhain/line2/inspect"}}""", "H1", hh) + .ShouldHaveSingleItem(); + } + + [Fact] + public void HasInstanceSegments_gates_the_ConfigDb_read() + { + DriverCredentialPolicy.HasInstanceSegments(DriverTypeNames.Galaxy, Galaxy("secret:galaxy/x/apikey")).ShouldBeTrue(); + DriverCredentialPolicy.HasInstanceSegments(DriverTypeNames.Galaxy, Galaxy("env:OTOPCUA_GALAXY_X")).ShouldBeFalse(); + DriverCredentialPolicy.HasInstanceSegments(DriverTypeNames.Galaxy, Galaxy("env:ConnectionStrings__ConfigDb")).ShouldBeFalse(); + DriverCredentialPolicy.HasInstanceSegments(DriverTypeNames.Modbus, """{"Password":"secret:galaxy/x"}""").ShouldBeFalse(); + DriverCredentialPolicy.HasInstanceSegments(DriverTypeNames.Galaxy, "not json").ShouldBeFalse(); + } + + [Fact] + public void The_combined_overload_reports_both_rules() + { + var config = """{"Gateway":{"ApiKeySecretRef":"secret:galaxy/PlantB/k"},"AlarmAck":{"FallbackPasswordSecretRef":"env:ConnectionStrings__ConfigDb"}}"""; + var problems = DriverCredentialPolicy.DescribeProblems(DriverTypeNames.Galaxy, config, "DRV-aaa", Instances); + problems.Count.ShouldBe(2); + problems.ShouldContain(p => p.Contains("lmxopcua#815")); + problems.ShouldContain(p => p.Contains("lmxopcua#835")); + } +} 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 aedd293c2..b7f38384f 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 @@ -45,7 +45,10 @@ public sealed class OpcUaClientBrowserCertificateAuthTests ex.Message.ShouldNotContain("does not support"); } - /// A valid PFX whose password is a secret: ref resolves through the injected store. + /// + /// A valid PFX (a relative path inside the user-certificate directory — lmxopcua#836) whose password is a + /// secret: ref resolves through the injected store. + /// [Fact] public async Task A_certificate_identity_is_built_with_its_secret_password_resolved() { @@ -55,12 +58,13 @@ public sealed class OpcUaClientBrowserCertificateAuthTests var opts = new OpcUaClientDriverOptions { AuthType = OpcUaAuthType.Certificate, - UserCertificatePath = path, + UserCertificatePath = Path.GetFileName(path), UserCertificatePassword = "secret:opcuaclient/browse-cert", }; var identity = await OpcUaClientDriverBrowser.BuildBrowseUserIdentityAsync( - opts, new OneSecret("opcuaclient/browse-cert", "s3cret-pw"), TestContext.Current.CancellationToken); + opts, new OneSecret("opcuaclient/browse-cert", "s3cret-pw"), TestContext.Current.CancellationToken, + userCertificateDirectory: Path.GetDirectoryName(path)); identity.TokenType.ShouldBe(UserTokenType.Certificate); } @@ -70,6 +74,34 @@ public sealed class OpcUaClientBrowserCertificateAuthTests } } + /// + /// lmxopcua#836: the browse is a hard-fail path — an absolute UserCertificatePath (any PFX the service account + /// can read) is refused before it is loaded or its password resolved, even when the file exists. + /// + [Fact] + public async Task The_browse_refuses_an_absolute_user_certificate_path() + { + var path = WritePfx("s3cret-pw"); + try + { + var opts = new OpcUaClientDriverOptions + { + AuthType = OpcUaAuthType.Certificate, + UserCertificatePath = path, + UserCertificatePassword = "s3cret-pw", + }; + + var ex = await Should.ThrowAsync(() => + OpcUaClientDriverBrowser.BuildBrowseUserIdentityAsync( + opts, null, TestContext.Current.CancellationToken, userCertificateDirectory: Path.GetDirectoryName(path))); + ex.Message.ShouldContain("lmxopcua#836"); + } + finally + { + try { File.Delete(path); } catch { /* best-effort */ } + } + } + /// With no secret store, a secret: ref fails closed instead of being sent as the literal. [Fact] public async Task A_secret_ref_with_no_secret_store_fails_closed() diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests/OpcUaClientUserCertificatePathTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests/OpcUaClientUserCertificatePathTests.cs new file mode 100644 index 000000000..a8a2010f6 --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests/OpcUaClientUserCertificatePathTests.cs @@ -0,0 +1,150 @@ +using System.Security.Cryptography; +using System.Security.Cryptography.X509Certificates; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; + +namespace ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests; + +/// +/// lmxopcua#836: the user-certificate identity loads only a PFX inside the node's user-certificate directory — the +/// authored UserCertificatePath is relative, resolved (and link-checked) by ResolveSecretRefsAsync, which +/// both the running driver and the AdminUI browser call before building the identity. +/// +[Trait("Category", "Unit")] +public sealed class OpcUaClientUserCertificatePathTests : IDisposable +{ + private readonly string _dir = Path.Combine(Path.GetTempPath(), "otopcua-836-ucert-" + Guid.NewGuid().ToString("N")); + private readonly string _outside = Path.Combine(Path.GetTempPath(), "otopcua-836-outside-" + Guid.NewGuid().ToString("N")); + + public OpcUaClientUserCertificatePathTests() + { + Directory.CreateDirectory(Path.Combine(_dir, "plant1")); + Directory.CreateDirectory(_outside); + } + + public void Dispose() + { + foreach (var d in new[] { _dir, _outside }) + try { Directory.Delete(d, recursive: true); } catch (IOException) { } catch (UnauthorizedAccessException) { } + } + + private static void WritePfx(string path) + { + using var rsa = RSA.Create(2048); + var req = new CertificateRequest("CN=Issue836", rsa, HashAlgorithmName.SHA256, RSASignaturePadding.Pkcs1); + using var cert = req.CreateSelfSigned(DateTimeOffset.UtcNow.AddMinutes(-5), DateTimeOffset.UtcNow.AddHours(1)); + File.WriteAllBytes(path, cert.Export(X509ContentType.Pfx, "pw")); + } + + private static OpcUaClientDriverOptions Cert(string path) => + new() { AuthType = OpcUaAuthType.Certificate, UserCertificatePath = path, UserCertificatePassword = "pw" }; + + [Fact] + public async Task A_relative_path_inside_the_directory_resolves_and_loads() + { + WritePfx(Path.Combine(_dir, "plant1", "user.pfx")); + + var resolved = await OpcUaUserIdentityFactory.ResolveSecretRefsAsync( + Cert("plant1/user.pfx"), null, TestContext.Current.CancellationToken, userCertificateDirectory: _dir); + + resolved.UserCertificatePath.ShouldBe(Path.Combine(Path.GetFullPath(_dir), "plant1", "user.pfx")); + OpcUaUserIdentityFactory.Build(resolved).TokenType.ShouldBe(Opc.Ua.UserTokenType.Certificate); + } + + [Theory] + [InlineData("../outside.pfx")] + [InlineData("plant1/../../outside.pfx")] + public async Task Traversal_is_refused_before_any_load(string path) => + await Should.ThrowAsync(() => OpcUaUserIdentityFactory.ResolveSecretRefsAsync( + Cert(path), null, TestContext.Current.CancellationToken, userCertificateDirectory: _dir)); + + [Fact] + public async Task An_absolute_path_is_refused_on_the_hard_fail_path_even_inside_the_directory() + { + var abs = Path.Combine(_dir, "plant1", "user.pfx"); + WritePfx(abs); + var ex = await Should.ThrowAsync(() => OpcUaUserIdentityFactory.ResolveSecretRefsAsync( + Cert(abs), null, TestContext.Current.CancellationToken, userCertificateDirectory: _dir)); + ex.Message.ShouldContain("lmxopcua#836"); + ex.Message.ShouldNotContain(abs); + } + + [Fact] + public async Task A_symlink_escaping_the_directory_is_refused() + { + var victim = Path.Combine(_outside, "victim.pfx"); + WritePfx(victim); + try + { + File.CreateSymbolicLink(Path.Combine(_dir, "user.pfx"), victim); + } + catch (Exception e) when (e is UnauthorizedAccessException or IOException) + { + return; // no symlink privilege (Windows): covered lexically above + } + + await Should.ThrowAsync(() => OpcUaUserIdentityFactory.ResolveSecretRefsAsync( + Cert("user.pfx"), null, TestContext.Current.CancellationToken, userCertificateDirectory: _dir)); + } + + [Fact] + public async Task The_running_driver_keeps_loading_a_legacy_absolute_path_and_warns() + { + var abs = Path.Combine(_outside, "legacy.pfx"); + WritePfx(abs); + var warnings = new List(); + var tracker = new LegacyCredentialReferenceTracker(DriverCredentialPolicy.OpcUaClientScope, "ouc", warnings.Add); + + var resolved = await OpcUaUserIdentityFactory.ResolveSecretRefsAsync( + Cert(abs), null, TestContext.Current.CancellationToken, legacy: tracker, userCertificateDirectory: _dir); + + resolved.UserCertificatePath.ShouldBe(abs); + OpcUaUserIdentityFactory.Build(resolved).TokenType.ShouldBe(Opc.Ua.UserTokenType.Certificate); + warnings.ShouldHaveSingleItem().ShouldContain("lmxopcua#836"); + warnings[0].ShouldNotContain(abs); + tracker.Diagnostics.ShouldContainKey(LegacyCredentialReferenceTracker.DiagnosticsKeyPrefix + "UserCertificatePath"); + } + + [Fact] + public async Task The_legacy_tracker_does_not_relax_a_relative_path_that_escapes() + { + var tracker = new LegacyCredentialReferenceTracker(DriverCredentialPolicy.OpcUaClientScope, "ouc", _ => { }); + await Should.ThrowAsync(() => OpcUaUserIdentityFactory.ResolveSecretRefsAsync( + Cert("../x.pfx"), null, TestContext.Current.CancellationToken, legacy: tracker, userCertificateDirectory: _dir)); + } + + [Fact] + public async Task A_non_certificate_auth_type_leaves_the_path_alone() + { + var options = new OpcUaClientDriverOptions { AuthType = OpcUaAuthType.Anonymous, UserCertificatePath = "/etc/stale.pfx" }; + (await OpcUaUserIdentityFactory.ResolveSecretRefsAsync(options, null, TestContext.Current.CancellationToken, userCertificateDirectory: _dir)) + .ShouldBeSameAs(options); + } + + [Fact] + public void LoadCertificate_refuses_an_unresolved_relative_path() + { + Should.Throw(() => OpcUaUserIdentityFactory.LoadCertificate(Cert("plant1/user.pfx"))) + .Message.ShouldContain("not been resolved"); + } + + [Fact] + public async Task The_path_is_checked_before_any_secret_is_read() + { + var reads = 0; + var options = Cert("/etc/x.pfx") with { UserCertificatePassword = "secret:opcuaclient/pfx-pw" }; + await Should.ThrowAsync(() => OpcUaUserIdentityFactory.ResolveSecretRefsAsync( + options, new CountingResolver(() => reads++), TestContext.Current.CancellationToken, userCertificateDirectory: _dir)); + reads.ShouldBe(0); + } + + private sealed class CountingResolver(Action onRead) : ZB.MOM.WW.Secrets.Abstractions.ISecretResolver + { + public Task GetAsync(ZB.MOM.WW.Secrets.Abstractions.SecretName name, CancellationToken ct) + { + onRead(); + return Task.FromResult("pw"); + } + } +} 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 index c22c5ce14..a5459573e 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Browsing/BrowserSessionCredentialScopeTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Browsing/BrowserSessionCredentialScopeTests.cs @@ -42,7 +42,8 @@ public sealed class BrowserSessionCredentialScopeTests { var bespoke = new RecordingBrowser(DriverTypeNames.Galaxy); var svc = new BrowserSessionService( - [bespoke], new BrowseSessionRegistry(), NullLogger.Instance, new RecordingUniversal(), null!, null!); + [bespoke], new BrowseSessionRegistry(), NullLogger.Instance, new RecordingUniversal(), null!, null!, + SeededFactory()); await svc.OpenAsync(DriverTypeNames.Galaxy, """{"Gateway":{"ApiKeySecretRef":"secret:galaxy/main/apikey"}}""", TestContext.Current.CancellationToken); @@ -50,6 +51,94 @@ public sealed class BrowserSessionCredentialScopeTests bespoke.Opens.ShouldBe(1); } + /// lmxopcua#835: a reference naming ANOTHER instance's secret never opens a browser. + [Theory] + [InlineData("DRV-evil")] + [InlineData(null)] + public async Task A_reference_naming_another_instance_never_opens_a_browser(string? ownerId) + { + var bespoke = new RecordingBrowser(DriverTypeNames.Galaxy); + var universal = new RecordingUniversal(); + var svc = new BrowserSessionService( + [bespoke], new BrowseSessionRegistry(), NullLogger.Instance, universal, null!, null!, SeededFactory()); + + var result = await svc.OpenAsync(DriverTypeNames.Galaxy, + """{"Gateway":{"Endpoint":"http://evil:5001","ApiKeySecretRef":"secret:galaxy/DRV-victim/apikey"}}""", ownerId, + TestContext.Current.CancellationToken); + + result.Ok.ShouldBeFalse(); + result.Message.ShouldNotBeNull().ShouldContain("lmxopcua#835"); + bespoke.Opens.ShouldBe(0); + universal.Opens.ShouldBe(0); + } + + /// Positive control: the owning driver's own id-scoped secret reaches the browser. + [Fact] + public async Task The_owners_own_secret_reaches_the_browser() + { + var bespoke = new RecordingBrowser(DriverTypeNames.Galaxy); + var svc = new BrowserSessionService( + [bespoke], new BrowseSessionRegistry(), NullLogger.Instance, new RecordingUniversal(), null!, null!, + SeededFactory()); + + await svc.OpenAsync(DriverTypeNames.Galaxy, """{"Gateway":{"ApiKeySecretRef":"secret:galaxy/Victim/apikey"}}""", "DRV-victim", + TestContext.Current.CancellationToken); + + bespoke.Opens.ShouldBe(1); + } + + /// Without the instance list the cross-instance rule cannot be checked, so the browse fails closed. + [Fact] + public async Task Without_a_config_db_an_in_scope_secret_reference_fails_closed() + { + var bespoke = new RecordingBrowser(DriverTypeNames.Galaxy); + var svc = new BrowserSessionService( + [bespoke], new BrowseSessionRegistry(), NullLogger.Instance, new RecordingUniversal(), null!, null!); + + var result = await svc.OpenAsync(DriverTypeNames.Galaxy, """{"Gateway":{"ApiKeySecretRef":"secret:galaxy/main/apikey"}}""", + TestContext.Current.CancellationToken); + + result.Ok.ShouldBeFalse(); + result.Message.ShouldNotBeNull().ShouldContain("fail-closed"); + bespoke.Opens.ShouldBe(0); + } + + /// lmxopcua#836: an absolute OpcUaClient user-certificate path never opens a browser. + [Fact] + public async Task An_absolute_user_certificate_path_never_opens_a_browser() + { + var bespoke = new RecordingBrowser(DriverTypeNames.OpcUaClient); + var svc = new BrowserSessionService( + [bespoke], new BrowseSessionRegistry(), NullLogger.Instance, new RecordingUniversal(), null!, null!); + + var result = await svc.OpenAsync(DriverTypeNames.OpcUaClient, + """{"EndpointUrl":"opc.tcp://evil:4840","AuthType":"Certificate","UserCertificatePath":"C:\\ProgramData\\x\\host.pfx"}""", + TestContext.Current.CancellationToken); + + result.Ok.ShouldBeFalse(); + result.Message.ShouldNotBeNull().ShouldContain("lmxopcua#836"); + bespoke.Opens.ShouldBe(0); + } + + private static Microsoft.EntityFrameworkCore.IDbContextFactory SeededFactory() + { + var name = $"browse-835-{Guid.NewGuid():N}"; + using (var db = Uns.RawTreeTestDb.CreateNamed(name)) + { + db.DriverInstances.Add(new ZB.MOM.WW.OtOpcUa.Configuration.Entities.DriverInstance + { + DriverInstanceId = "DRV-victim", ClusterId = "c", Name = "Victim", DriverType = DriverTypeNames.Galaxy, DriverConfig = "{}", + }); + db.DriverInstances.Add(new ZB.MOM.WW.OtOpcUa.Configuration.Entities.DriverInstance + { + DriverInstanceId = "DRV-evil", ClusterId = "c", Name = "Evil", DriverType = DriverTypeNames.Galaxy, DriverConfig = "{}", + }); + db.SaveChanges(); + } + + return Uns.RawTreeTestDb.Factory(name); + } + private sealed class RecordingBrowser(string driverType) : IDriverBrowser { public int Opens { get; private set; } diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Deployments/CredentialReferencePreflightInstanceAndPathTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Deployments/CredentialReferencePreflightInstanceAndPathTests.cs new file mode 100644 index 000000000..d47573f2c --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Deployments/CredentialReferencePreflightInstanceAndPathTests.cs @@ -0,0 +1,55 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.AdminUI.Deployments; +using ZB.MOM.WW.OtOpcUa.Configuration.Entities; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; +using ZB.MOM.WW.OtOpcUa.Runtime.Drivers; + +namespace ZB.MOM.WW.OtOpcUa.AdminUI.Tests.Deployments; + +/// +/// The upgrade preflight / /raw banner lists the configs the deploy gate will now refuse for lmxopcua#835 +/// (another instance's secret) and #836 (absolute user-certificate path) — Draft and Deployed alike. +/// +[Trait("Category", "Unit")] +public sealed class CredentialReferencePreflightInstanceAndPathTests +{ + private static DriverInstance Drv(string id, string name, string type, string config) => + new() { DriverInstanceId = id, ClusterId = "c", Name = name, DriverType = type, DriverConfig = config }; + + [Fact] + public void Draft_and_deployed_cross_instance_references_are_listed() + { + var drivers = new[] + { + Drv("DRV-a", "A", DriverTypeNames.Galaxy, """{"Gateway":{"ApiKeySecretRef":"secret:galaxy/DRV-a/apikey"}}"""), + Drv("DRV-b", "B", DriverTypeNames.Galaxy, """{"Gateway":{"ApiKeySecretRef":"secret:galaxy/DRV-a/apikey"}}"""), + }; + var devices = new[] { new Device { DeviceId = "dev-b", DriverInstanceId = "DRV-b", Name = "gw", DeviceConfig = """{"AlarmAck":{"FallbackPasswordSecretRef":"secret:otopcua/galaxy/A/ack"}}""" } }; + var deployed = new[] + { + new DriverInstanceSpec(Guid.NewGuid(), "DRV-a", "A", DriverTypeNames.Galaxy, true, "{}"), + new DriverInstanceSpec(Guid.NewGuid(), "DRV-b", "B", DriverTypeNames.Galaxy, true, """{"Gateway":{"ApiKeySecretRef":"secret:galaxy/A/apikey"}}"""), + }; + + var findings = CredentialReferencePreflight.Scan(drivers, devices, deployed); + + findings.Count.ShouldBe(3); + findings.ShouldAllBe(f => f.DriverInstanceId == "DRV-b" && f.Problem.Contains("lmxopcua#835")); + findings.Count(f => f.Source == CredentialFindingSource.Draft).ShouldBe(2); + findings.Single(f => f.Source == CredentialFindingSource.Deployed).DriverName.ShouldBe("B"); + } + + [Fact] + public void An_absolute_user_certificate_path_is_listed_without_the_path() + { + var findings = CredentialReferencePreflight.Scan( + [Drv("o", "Upstream", DriverTypeNames.OpcUaClient, """{"AuthType":"Certificate","UserCertificatePath":"C:\\certs\\plant.pfx"}""")], + [], + []); + + var f = findings.ShouldHaveSingleItem(); + f.Problem.ShouldContain("lmxopcua#836"); + f.Problem.ShouldNotContain("plant.pfx"); + } +} diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/CredentialInstanceAndPathSavePathTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/CredentialInstanceAndPathSavePathTests.cs new file mode 100644 index 000000000..d0eb48d90 --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/CredentialInstanceAndPathSavePathTests.cs @@ -0,0 +1,130 @@ +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#835 (a credential reference naming ANOTHER driver instance's secret) and #836 (the OpcUaClient +/// user-certificate path confined to a node directory) at the SAVE path (RawTreeService): nothing reaches the +/// config DB, through driver and device create + update. +/// +[Trait("Category", "Unit")] +public sealed class CredentialInstanceAndPathSavePathTests +{ + private static CancellationToken Ct => TestContext.Current.CancellationToken; + + private static string Galaxy(string reference) => + """{"Gateway":{"Endpoint":"http://evil:5001","ApiKeySecretRef":"REF"}}""".Replace("REF", reference); + + private static async Task<(RawTreeService Svc, string Db, string VictimId)> WithVictimAsync() + { + var name = RawTreeTestDb.SeedFresh(); + var svc = new RawTreeService(RawTreeTestDb.Factory(name)); + var victim = await svc.CreateDriverAsync(RawTreeTestDb.ClusterId, null, "Victim", DriverTypeNames.Galaxy, + Galaxy("secret:galaxy/victim-own/apikey"), Ct); + victim.Ok.ShouldBeTrue(victim.Error); + return (svc, name, victim.CreatedId!); + } + + [Fact] + public async Task CreateDriver_refuses_a_reference_naming_another_instance() + { + var (svc, name, victimId) = await WithVictimAsync(); + + var byName = await svc.CreateDriverAsync(RawTreeTestDb.ClusterId, null, "evil1", DriverTypeNames.Galaxy, + Galaxy("secret:galaxy/Victim/apikey"), Ct); + var byId = await svc.CreateDriverAsync(RawTreeTestDb.ClusterId, null, "evil2", DriverTypeNames.Galaxy, + Galaxy($"secret:galaxy/{victimId}/apikey"), Ct); + + byName.Ok.ShouldBeFalse(); + byName.Error!.ShouldContain("lmxopcua#835"); + byId.Ok.ShouldBeFalse(); + using var db = RawTreeTestDb.CreateNamed(name); + db.DriverInstances.ShouldNotContain(d => d.Name == "evil1" || d.Name == "evil2"); + } + + [Fact] + public async Task Update_and_device_paths_refuse_and_keep_the_stored_config() + { + var (svc, name, victimId) = await WithVictimAsync(); + var evil = await svc.CreateDriverAsync(RawTreeTestDb.ClusterId, null, "evil", DriverTypeNames.Galaxy, + Galaxy("secret:galaxy/shared/apikey"), Ct); + evil.Ok.ShouldBeTrue(evil.Error); + + byte[] rv; + string deviceId; + byte[] deviceRv; + using (var db = RawTreeTestDb.CreateNamed(name)) + { + rv = db.DriverInstances.Single(d => d.DriverInstanceId == evil.CreatedId).RowVersion; + var device = db.Devices.Single(d => d.DriverInstanceId == evil.CreatedId); + deviceId = device.DeviceId; + deviceRv = device.RowVersion; + } + + var stolen = Galaxy($"secret:galaxy/{victimId}/apikey"); + (await svc.UpdateDriverAsync(evil.CreatedId!, "evil", stolen, null, rv, Ct)).Error.ShouldNotBeNull().ShouldContain("lmxopcua#835"); + (await svc.UpdateDeviceAsync(deviceId, "dev", stolen, true, deviceRv, Ct)).Ok.ShouldBeFalse(); + (await svc.CreateDeviceAsync(evil.CreatedId!, "dev2", stolen, Ct)).Ok.ShouldBeFalse(); + + using var check = RawTreeTestDb.CreateNamed(name); + check.DriverInstances.Single(d => d.DriverInstanceId == evil.CreatedId).DriverConfig.ShouldContain("secret:galaxy/shared/apikey"); + check.Devices.Where(d => d.DriverInstanceId == evil.CreatedId).ShouldAllBe(d => !d.DeviceConfig.Contains(victimId)); + } + + [Fact] + public async Task A_driver_may_save_its_own_id_scoped_secret_on_update_and_on_its_devices() + { + var (svc, name, victimId) = await WithVictimAsync(); + byte[] rv; + string deviceId; + byte[] deviceRv; + using (var db = RawTreeTestDb.CreateNamed(name)) + { + rv = db.DriverInstances.Single(d => d.DriverInstanceId == victimId).RowVersion; + var device = db.Devices.Single(d => d.DriverInstanceId == victimId); + deviceId = device.DeviceId; + deviceRv = device.RowVersion; + } + + var own = Galaxy($"secret:galaxy/{victimId}/apikey"); + (await svc.UpdateDriverAsync(victimId, "Victim", own, null, rv, Ct)).Ok.ShouldBeTrue(); + (await svc.UpdateDeviceAsync(deviceId, "dev", Galaxy("secret:galaxy/Victim/apikey"), true, deviceRv, Ct)).Ok.ShouldBeTrue(); + } + + [Theory] + [InlineData("""{"AuthType":"Certificate","UserCertificatePath":"C:\\Windows\\System32\\config\\systemprofile\\host.pfx"}""")] + [InlineData("""{"AuthType":"Certificate","UserCertificatePath":"/etc/ssl/private/host.pfx"}""")] + [InlineData("""{"AuthType":"Certificate","UserCertificatePath":"../host.pfx"}""")] + public async Task An_opcuaclient_user_certificate_path_outside_the_directory_is_never_stored(string config) + { + var name = RawTreeTestDb.SeedFresh(); + var svc = new RawTreeService(RawTreeTestDb.Factory(name)); + var result = await svc.CreateDriverAsync(RawTreeTestDb.ClusterId, null, "ouc836", DriverTypeNames.OpcUaClient, config, Ct); + result.Ok.ShouldBeFalse(); + result.Error!.ShouldContain("lmxopcua#836"); + DriverConfigSaveGate.Validate(DriverTypeNames.OpcUaClient, config).ShouldNotBeNull(); // the modals' live check too + using var db = RawTreeTestDb.CreateNamed(name); + db.DriverInstances.ShouldNotContain(d => d.Name == "ouc836"); + } + + [Fact] + public async Task A_relative_user_certificate_path_saves() + { + var name = RawTreeTestDb.SeedFresh(); + var svc = new RawTreeService(RawTreeTestDb.Factory(name)); + (await svc.CreateDriverAsync(RawTreeTestDb.ClusterId, null, "ouc-ok", DriverTypeNames.OpcUaClient, + """{"AuthType":"Certificate","UserCertificatePath":"plant1/user.pfx"}""", Ct)).Ok.ShouldBeTrue(); + } + + [Fact] + public async Task LoadDeviceDriverInstanceId_names_the_owning_driver() + { + var name = RawTreeTestDb.SeedFresh(); + var svc = new RawTreeService(RawTreeTestDb.Factory(name)); + (await svc.LoadDeviceDriverInstanceIdAsync(RawTreeTestDb.FolderedDeviceId, Ct)).ShouldBe(RawTreeTestDb.FolderedDriverId); + (await svc.LoadDeviceDriverInstanceIdAsync("DEV-nope", Ct)).ShouldBeNull(); + } +} diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/AdminOperationsActorCredentialScopeTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/AdminOperationsActorCredentialScopeTests.cs index 86780d72d..b179aa1d3 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/AdminOperationsActorCredentialScopeTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests/AdminOperationsActorCredentialScopeTests.cs @@ -50,6 +50,83 @@ public sealed class AdminOperationsActorCredentialScopeTests : ControlPlaneActor probe.Calls.ShouldBe(1); } + /// + /// lmxopcua#835: a reference naming ANOTHER instance's secret never reaches the probe, whether the caller names + /// its own driver or not (a null owner — a not-yet-created driver — is the stricter case). + /// + [Theory] + [InlineData("DRV-evil")] + [InlineData(null)] + public void A_reference_naming_another_instances_secret_never_reaches_the_probe(string? ownerId) + { + var dbName = Guid.NewGuid().ToString("N"); + var factory = NewInMemoryDbFactory(dbName); + SeedGalaxyDrivers(factory); + var probe = new RecordingProbe(DriverTypeNames.Galaxy); + var actor = Sys.ActorOf(AdminOperationsActor.Props(factory, CreateTestProbe("coord").Ref, new IDriverProbe[] { probe })); + + actor.Tell(new TestDriverConnect( + DriverTypeNames.Galaxy, """{"Gateway":{"Endpoint":"http://attacker:5001","ApiKeySecretRef":"secret:galaxy/Victim/apikey"}}""", + 5, Guid.NewGuid(), ownerId)); + + var reply = ExpectMsg(PresenceBudget, cancellationToken: TestContext.Current.CancellationToken); + reply.Ok.ShouldBeFalse(); + reply.Message.ShouldNotBeNull().ShouldContain("Test Connect refused"); + reply.Message.ShouldContain("lmxopcua#835"); + probe.Calls.ShouldBe(0); + } + + /// Positive control: the owner's own secret (and a shared one) does reach the probe. + [Theory] + [InlineData("secret:galaxy/DRV-victim/apikey")] + [InlineData("secret:galaxy/shared-gw/apikey")] + public void The_owners_own_or_a_shared_secret_reaches_the_probe(string reference) + { + var factory = NewInMemoryDbFactory(); + SeedGalaxyDrivers(factory); + var probe = new RecordingProbe(DriverTypeNames.Galaxy); + var actor = Sys.ActorOf(AdminOperationsActor.Props(factory, CreateTestProbe("coord").Ref, new IDriverProbe[] { probe })); + + actor.Tell(new TestDriverConnect( + DriverTypeNames.Galaxy, """{"Gateway":{"Endpoint":"http://gw:5001","ApiKeySecretRef":"REF"}}""".Replace("REF", reference), + 5, Guid.NewGuid(), "DRV-victim")); + + ExpectMsg(PresenceBudget, cancellationToken: TestContext.Current.CancellationToken).Ok.ShouldBeTrue(); + probe.Calls.ShouldBe(1); + } + + /// lmxopcua#836: an absolute OpcUaClient user-certificate path never reaches the probe. + [Fact] + public void An_absolute_user_certificate_path_never_reaches_the_probe() + { + var probe = new RecordingProbe(DriverTypeNames.OpcUaClient); + var actor = Sys.ActorOf(AdminOperationsActor.Props(NewInMemoryDbFactory(), CreateTestProbe("coord").Ref, new IDriverProbe[] { probe })); + + actor.Tell(new TestDriverConnect( + DriverTypeNames.OpcUaClient, + """{"EndpointUrl":"opc.tcp://attacker:4840","AuthType":"Certificate","UserCertificatePath":"/etc/ssl/private/host.pfx"}""", + 5, Guid.NewGuid())); + + var reply = ExpectMsg(PresenceBudget, cancellationToken: TestContext.Current.CancellationToken); + reply.Ok.ShouldBeFalse(); + reply.Message.ShouldNotBeNull().ShouldContain("lmxopcua#836"); + probe.Calls.ShouldBe(0); + } + + private static void SeedGalaxyDrivers(Microsoft.EntityFrameworkCore.IDbContextFactory factory) + { + using var db = factory.CreateDbContext(); + db.DriverInstances.Add(new ZB.MOM.WW.OtOpcUa.Configuration.Entities.DriverInstance + { + DriverInstanceId = "DRV-victim", ClusterId = "c", Name = "Victim", DriverType = DriverTypeNames.Galaxy, DriverConfig = "{}", + }); + db.DriverInstances.Add(new ZB.MOM.WW.OtOpcUa.Configuration.Entities.DriverInstance + { + DriverInstanceId = "DRV-evil", ClusterId = "c", Name = "Evil", DriverType = DriverTypeNames.Galaxy, DriverConfig = "{}", + }); + db.SaveChanges(); + } + private sealed class RecordingProbe(string driverType) : IDriverProbe { private int _calls;