dohertj2/lmxopcua
1
0
Fork 0
Code Issues 5 Pull Requests Actions Packages Projects Releases Wiki Activity

fix(security): env: credential references follow the per-instance rule (#849)

Browse Source 
env:<PREFIX><INSTANCE>_... is the per-instance convention for every prefix of a
driver's scope (OTOPCUA_GALAXY_, GALAXY_MXGW_, OTOPCUA_OPCUACLIENT_,
OTOPCUA_HEIDENHAIN_), INSTANCE = a driver id or name upper-cased with every other
character as '_'. A variable whose part after the prefix equals or starts with
(at a '_' boundary) ANOTHER same-type instance's id or name is refused with
CredentialReferenceOtherInstance at save, deploy, Test Connect and browse - the
same DriverCredentialPolicy.DescribeOtherInstanceProblems the secret: rule uses
(now over CollectInstanceReferences / ReferenceNames). Another-instance matches
win over the owner's, as for secrets. Shared variables that name no instance
(GALAXY_MXGW_API_KEY) keep working. HasInstanceSegments now also gates on env:,
so an env:-only browse needs the ConfigDb (fail-closed without it).

Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
Joseph Doherty
2026-11-11 06:24:47 -05:00
parent d1b6cad299
commit 4a6229e4d2
7 changed files with 284 additions and 52 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/DriverCredentialPolicy.cs
+111 -45
View File
@@ -211,8 +211,8 @@ public static class DriverCredentialPolicy
/// <summary>
/// The instance-identifying segment of an in-scope <c>secret:</c> reference: the first path segment after the
/// matched namespace (<c>secret:galaxy/&lt;X&gt;/apikey</c> ⇒ <c>X</c>; <c>secret:otopcua/galaxy/&lt;X&gt;</c> ⇒
/// <c>X</c>). Null for anything else — <c>env:</c> references carry no instance convention, and literals and
/// out-of-scope references are the #815 rule's business.
/// <c>X</c>). Null for anything else — <c>env:</c> references use <see cref="EnvironmentInstancePart"/>, and literals
/// and out-of-scope references are the #815 rule's business.
/// </summary>
/// <param name="value">The authored field value.</param>
/// <param name="scope">The driver's scope.</param>
@@ -232,24 +232,81 @@ public static class DriverCredentialPolicy
}
/// <summary>
/// True when the config carries at least one in-scope <c>secret:</c> reference — i.e. the cross-instance rule
/// (<see cref="DescribeOtherInstanceProblems"/>) has something to check, so a seam must load the instance list.
/// Lets seams skip the ConfigDb read otherwise.
/// lmxopcua#849: the part of an in-scope <c>env:</c> reference after the matched (longest) prefix —
/// <c>env:OTOPCUA_GALAXY_&lt;REST&gt;</c> ⇒ <c>REST</c>. The per-instance convention is
/// <c>env:&lt;PREFIX&gt;&lt;INSTANCE&gt;_…</c>, INSTANCE being a driver id or name in
/// <see cref="NormalizeEnvironmentIdentifier"/> form; the part NAMES an instance when it equals that form or starts
/// with it followed by <c>_</c> (see <see cref="ReferenceNames"/>). Null for anything but an in-scope <c>env:</c>.
/// </summary>
/// <param name="value">The authored field value.</param>
/// <param name="scope">The driver's scope.</param>
/// <returns>The remainder, or null.</returns>
public static string? EnvironmentInstancePart(string? value, CredentialScope scope)
{
ArgumentNullException.ThrowIfNull(scope);
if (CredentialReferences.Parse(value, scope, out var name) != CredentialReferenceKind.Environment) return null;
var prefix = scope.EnvVariablePrefixes
.Where(p => name.StartsWith(p, StringComparison.Ordinal))
.OrderByDescending(p => p.Length)
.FirstOrDefault();
return prefix is null ? null : name[prefix.Length..];
}
/// <summary>
/// lmxopcua#849: a driver id or name in environment-variable form — ASCII letters upper-cased, digits kept, every
/// other character <c>_</c> (<c>DRV-1a2b</c> ⇒ <c>DRV_1A2B</c>, <c>Line 1</c> ⇒ <c>LINE_1</c>). Empty when the
/// identifier has no letter or digit (such an identifier names no variable).
/// </summary>
/// <param name="identifier">The id or name.</param>
/// <returns>The normalised form.</returns>
public static string NormalizeEnvironmentIdentifier(string? identifier)
{
if (string.IsNullOrEmpty(identifier) || !identifier.Any(char.IsAsciiLetterOrDigit)) return "";
return string.Create(identifier.Length, identifier, static (span, id) =>
{
for (var i = 0; i < id.Length; i++)
span[i] = char.IsAsciiLetterOrDigit(id[i]) ? char.ToUpperInvariant(id[i]) : '_';
});
}
/// <summary>
/// Whether an instance reference (<see cref="CollectInstanceReferences"/>) names the driver id or name
/// <paramref name="identifier"/>: a <c>secret:</c> segment equal to it (case-insensitive), or an <c>env:</c> part
/// equal to its <see cref="NormalizeEnvironmentIdentifier"/> form or starting with that form plus <c>_</c>.
/// </summary>
/// <param name="reference">The reference.</param>
/// <param name="identifier">A driver id or name.</param>
/// <returns>Whether it names it.</returns>
public static bool ReferenceNames(InstanceReference reference, string? identifier)
{
if (string.IsNullOrEmpty(identifier)) return false;
if (reference.Kind == CredentialReferenceKind.Secret)
return string.Equals(reference.Part, identifier, StringComparison.OrdinalIgnoreCase);
var normalized = NormalizeEnvironmentIdentifier(identifier);
return normalized.Length > 0
&& (string.Equals(reference.Part, normalized, StringComparison.Ordinal)
|| reference.Part.StartsWith(normalized + "_", StringComparison.Ordinal));
}
/// <summary>
/// True when the config carries at least one in-scope <c>secret:</c> or <c>env:</c> reference — i.e. the
/// cross-instance rule (<see cref="DescribeOtherInstanceProblems"/>) has something to check, so a seam must load
/// the instance list. Lets seams skip the ConfigDb read otherwise.
/// </summary>
/// <param name="driverType">The owning driver's type.</param>
/// <param name="json">The config JSON.</param>
/// <returns>Whether any instance segment is present.</returns>
/// <returns>Whether any instance reference is present.</returns>
public static bool HasInstanceSegments(string? driverType, string? json) =>
CollectSegments(driverType, json).Count > 0;
CollectInstanceReferences(driverType, json).Count > 0;
/// <summary>
/// lmxopcua#835: inside one namespace, a reference whose instance-identifying segment (see
/// <see cref="InstanceSegment"/>) names a DIFFERENT existing instance of the same driver type — by its
/// <see cref="DriverInstanceIdentity.DriverInstanceId"/> or its <see cref="DriverInstanceIdentity.Name"/>,
/// case-insensitively — is refused: otherwise a Designer could create (or Test Connect / browse) a driver that
/// points at their own endpoint with <c>secret:galaxy/&lt;other-instance&gt;/apikey</c> and capture that
/// instance's key. Shared names that are no other instance's id or name (<c>secret:galaxy/gw/apikey</c>,
/// <c>secret:galaxy/apikey</c>) stay valid, as does a reference naming the instance itself.
/// lmxopcua#835/#849: inside one namespace, a reference that names (see <see cref="ReferenceNames"/>) a DIFFERENT
/// existing instance of the same driver type — by its <see cref="DriverInstanceIdentity.DriverInstanceId"/> or its
/// <see cref="DriverInstanceIdentity.Name"/> — is refused: otherwise a Designer could create (or Test Connect /
/// browse) a driver that points at their own endpoint with <c>secret:galaxy/&lt;other-instance&gt;/apikey</c> (or
/// <c>env:OTOPCUA_GALAXY_&lt;OTHER_INSTANCE&gt;_KEY</c>) and capture that instance's key. Shared names that name no
/// other instance (<c>secret:galaxy/gw/apikey</c>, <c>env:GALAXY_MXGW_API_KEY</c>) stay valid, as does a reference
/// naming the instance itself.
/// </summary>
/// <remarks>
/// A match against ANOTHER instance wins over a match against the owner — names are not unique, and renaming
@@ -267,24 +324,26 @@ public static class DriverCredentialPolicy
ArgumentNullException.ThrowIfNull(instances);
var problems = new List<string>();
if (!TryGet(driverType, out var rules)) return problems;
var segments = CollectSegments(driverType, json);
if (segments.Count == 0) return problems;
var references = CollectInstanceReferences(driverType, json);
if (references.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)
foreach (var reference in references)
{
var other = others.FirstOrDefault(o =>
string.Equals(o.DriverInstanceId, segment, StringComparison.OrdinalIgnoreCase)
|| string.Equals(o.Name, segment, StringComparison.OrdinalIgnoreCase));
var other = others.FirstOrDefault(o => ReferenceNames(reference, o.DriverInstanceId) || ReferenceNames(reference, o.Name));
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]}<this-driver-id>/…) or a shared name that is no other "
+ "instance's id or name (lmxopcua#835).");
problems.Add(reference.Kind == CredentialReferenceKind.Secret
? $"{reference.FieldPath} names a secret under '{reference.Part}/', 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]}<this-driver-id>/…) or a shared name that is no other "
+ "instance's id or name (lmxopcua#835)."
: $"{reference.FieldPath} names an environment variable of another {rules.Scope.Label} driver instance "
+ $"('{other.Name}', {other.DriverInstanceId}) — a driver may reference only its own variables "
+ $"(env:{rules.Scope.EnvVariablePrefixes[0]}<THIS_DRIVER_ID>_…, the id upper-cased with every other "
+ "character as '_') or a shared name that starts with no other instance's id or name (lmxopcua#849).");
}
return problems;
@@ -397,29 +456,36 @@ public static class DriverCredentialPolicy
using (doc) use(doc.RootElement);
}
private static List<(string FieldPath, string Segment)> CollectSegments(string? driverType, string? json)
/// <summary>
/// Every in-scope instance reference of a config (lmxopcua#835 <c>secret:</c> segments + #849 <c>env:</c> parts), at
/// any depth. Empty for an unreadable config or a type without rules.
/// </summary>
/// <param name="driverType">The owning driver's type.</param>
/// <param name="json">The config JSON.</param>
/// <returns>The references, in document order.</returns>
public static IReadOnlyList<InstanceReference> CollectInstanceReferences(string? driverType, string? json)
{
var found = new List<(string, string)>();
var found = new List<InstanceReference>();
if (!TryGet(driverType, out var rules) || string.IsNullOrWhiteSpace(json)) return found;
JsonDocument doc;
try
// Unreadable JSON is reported by DescribeProblems.
WithDocument(json, root => Visit(root, "", rules, 0, (fieldPath, value, field, _) =>
{
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));
});
}
if (field is null || value.ValueKind != JsonValueKind.String) return;
var text = value.GetString();
if (InstanceSegment(text, rules.Scope) is { Length: > 0 } segment)
found.Add(new(fieldPath, CredentialReferenceKind.Secret, segment));
else if (EnvironmentInstancePart(text, rules.Scope) is { Length: > 0 } part)
found.Add(new(fieldPath, CredentialReferenceKind.Environment, part));
}));
return found;
}
}
/// <summary>The instance-identifying part of one in-scope credential reference (lmxopcua#835 / #849).</summary>
/// <param name="FieldPath">Where it sits in the config (never the value).</param>
/// <param name="Kind"><see cref="CredentialReferenceKind.Secret"/> or <see cref="CredentialReferenceKind.Environment"/>.</param>
/// <param name="Part">
/// The <c>secret:</c> instance segment (<see cref="DriverCredentialPolicy.InstanceSegment"/>) or the <c>env:</c> remainder
/// after the prefix (<see cref="DriverCredentialPolicy.EnvironmentInstancePart"/>).
/// </param>
public readonly record struct InstanceReference(string FieldPath, CredentialReferenceKind Kind, string Part);
src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Browsing/BrowserSessionService.cs
+3 -3
View File
@@ -26,7 +26,7 @@ namespace ZB.MOM.WW.OtOpcUa.AdminUI.Browsing;
/// <param name="authenticationStateProvider">Supplies the calling circuit's user.</param>
/// <param name="dbFactory">
/// The config DB, for the cross-instance credential rule (lmxopcua#835). Null (test graphs) ⇒ a config carrying an
/// in-scope <c>secret:</c> reference cannot be checked and is refused (fail-closed).
/// in-scope <c>secret:</c>/<c>env:</c> reference cannot be checked and is refused (fail-closed).
/// </param>
/// <param name="audit">The credential-use audit sink (#835 review I-1); null ⇒ the structured log line only.</param>
public sealed class BrowserSessionService(
@@ -73,7 +73,7 @@ public sealed class BrowserSessionService(
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: "
return new(false, "Browse refused: the driver-instance list needed to check this config's secret:/env: "
+ "references is unavailable (lmxopcua#835, fail-closed).", Guid.Empty);
try
{
@@ -88,7 +88,7 @@ public sealed class BrowserSessionService(
// Review L-2: a ConfigDb fault must not become a circuit error — refuse (fail-closed) with a message.
logger.LogWarning(ex, "Browse credential check failed for driverType={DriverType}", driverType);
return new(false, "Browse refused: the driver-instance list could not be read to check this config's "
+ "secret: references (lmxopcua#835, fail-closed). Try again.", Guid.Empty);
+ "secret:/env: references (lmxopcua#835/#849, fail-closed). Try again.", Guid.Empty);
}
}
src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/AdminOperations/AdminOperationsActor.cs
+2 -2
View File
@@ -611,9 +611,9 @@ public sealed class AdminOperationsActor : ReceiveActor
var sw = System.Diagnostics.Stopwatch.StartNew();
try
{
// lmxopcua#835: a reference naming ANOTHER driver instance's secret (secret:galaxy/<other>/…) is refused
// lmxopcua#835/#849: a reference naming ANOTHER driver instance's secret (secret:galaxy/<other>/…, env:OTOPCUA_GALAXY_<OTHER>_…) 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).
// carries an in-scope secret:/env: reference at all).
if (DriverCredentialPolicy.HasInstanceSegments(msg.DriverType, msg.ConfigJson))
{
await using var db = await _dbFactory.CreateDbContextAsync(cts.Token);