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

fix(acl): close the #808 review findings — reference variables, owner folder chain, NUL keys, unknown ids (#808)

Browse Source 
Review of af5c9744:
1. UNS reference variables (s=Area/Line/Equip/Eff) of a moved or renamed
   equipment stayed readable/writable under the OLD names' grants: they now
   resolve as ReferenceVariable and are authorized at their split path only
   while it equals their live equipment folder's placement (live chain ==
   generation chain) + one name; otherwise denied (Read and Write).
2. TryPlaceScriptedCondition now also requires the owner folder's live chain
   to match the generation (AuthorizeScriptedCondition gains ownerFolderChain),
   so a moved equipment's alarm/children/commands follow its folder.
3. Identifiers containing NUL are denied before the decision cache, keeping
   client ids disjoint from the NUL-prefixed resolved-scope keys (a client
   could otherwise poison a real folder's cached verdict for its key shard).
4. An id no live node holds is Unknown: placed at [\0unknown] so only
   Cluster / Namespace-uns grants decide it (a removed opaque id can no longer
   split onto a same-named narrow grant; narrow grantees get no NodeDeleted for
   removed UNS folders/vtags/conditions — documented); 4-segment
   reference-shaped ids keep their split.
5. A wire test drives placements from DeploymentArtifact.ParseDataPlaneAuthority.

Baseline updates: #797 owner tests (unit + wire) and Sdk2RepeatedAlarmAuthority
now install folder scopes; Sdk2HistoryAuthority's UNS alias used the RAW path
as its UNS id (never minted in production) and now carries EqA/HistoryAlias.
Each new guard was falsified (reverting it fails its test).
This commit is contained in:
Joseph Doherty
2026-11-10 23:44:11 -05:00
parent af5c9744b7
commit e39a125069
11 changed files with 557 additions and 64 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs
+60 -20
View File
@@ -2304,6 +2304,7 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2
// authority. Never a fallback to splitting the opaque id.
var native = false;
string? owner = null;
List<string>? ownerChain = null;
lock (Lock)
{
var key = MapKey(condition.NodeId);
@@ -2311,12 +2312,11 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2
{
native = true;
}
else if (_scriptedAlarmOwners.TryGetValue(key, out var binding)
&& ReferenceEquals(binding.Alarm, condition)
&& _alarmConditions.TryGetValue(key, out var live)
&& ReferenceEquals(live, condition))
else
{
owner = binding.EquipmentId;
owner = BoundScriptedOwner(key, condition);
// Gitea #808 review: the owner's folder must also sit where the generation deploys it.
ownerChain = BoundScriptedOwnerChain(key, condition);
}
}
@@ -2324,7 +2324,7 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2
? _authorizer!.Authorize(
identity, AlarmOperationFor(operation), condition.NodeId, RealmOf(condition.NodeId))
: _authorizer!.AuthorizeScriptedCondition(
identity, AlarmOperationFor(operation), condition.NodeId, owner);
identity, AlarmOperationFor(operation), condition.NodeId, owner, ownerChain);
}
// The activation-time AlarmAck role is honoured only while the key that earned it is
@@ -4224,7 +4224,10 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2
/// </para>
/// <list type="bullet">
/// <item><b>UNS reference variables</b> — <c>s=Area/Line/Equipment/EffectiveName</c>, NAMES: the id IS the
/// ACL path (<see cref="UnsAclSubjectKind.Path"/>), so a <c>Tag</c> grant keeps matching it.</item>
/// ACL path, so a <c>Tag</c> grant keeps matching it — but only while it equals its live equipment folder's
/// placement + one name (<see cref="UnsAclSubjectKind.ReferenceVariable"/>, #808 review).</item>
/// <item><b>Anything no live node holds</b> (removed, never deployed, probed):
/// <see cref="UnsAclSubjectKind.Unknown"/> — decided only by a realm-wide grant.</item>
/// <item><b>Hierarchy folders and what they own</b> — Area / Line / Equipment folders under their opaque
/// entity ids (<c>s=UA-…</c>/<c>s=UL-…</c>/<c>s=EQ-…</c>), and an equipment's virtual tags and their
/// sub-folders (<c>s=EQ-…/Name</c>): <see cref="UnsAclSubjectKind.Folder"/>, with the live id ancestry.</item>
@@ -4263,21 +4266,18 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2
if (node is null)
{
// Not a folder or variable: a scripted condition (s=<id>) or one of its instance-scoped children
// (s=<id>/<browse path>, #629). Anything else is an unknown id and keeps the split.
// (s=<id>/<browse path>, #629). Anything else is held by NO live node — removed, never deployed, or a
// probe — and is Unknown: only a realm-wide grant decides it (review of #808: splitting a removed opaque
// id let it match a narrow grant on an unrelated entity of the same name).
var conditionId = slash < 0 ? identifier : identifier[..slash];
var conditionKey = MapKey(AddressSpaceRealm.Uns, conditionId);
if (!_alarmConditions.TryGetValue(conditionKey, out var condition)) return UnsAclSubject.Path;
if (!_alarmConditions.TryGetValue(conditionKey, out var condition)) return UnsAclSubject.Unknown;
lock (Lock)
{
if (_nativeAlarmNodeIds.Contains(conditionKey)) return UnsAclSubject.Path;
var owner = _scriptedAlarmOwners.TryGetValue(conditionKey, out var binding)
&& ReferenceEquals(binding.Alarm, condition)
&& _alarmConditions.TryGetValue(conditionKey, out var live)
&& ReferenceEquals(live, condition)
? binding.EquipmentId
: null;
return UnsAclSubject.ScriptedCondition(conditionId, owner);
return UnsAclSubject.ScriptedCondition(
conditionId, BoundScriptedOwner(conditionKey, condition), BoundScriptedOwnerChain(conditionKey, condition));
}
}
@@ -4292,11 +4292,30 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2
break;
}
if (owning is null || !firstSegment.SequenceEqual(owning.NodeId.IdentifierAsString)) return UnsAclSubject.Path;
// No live hierarchy folder above it: a shape only test rigs mint (a UNS node hung straight under the root).
if (owning is null) return UnsAclSubject.Path;
// Its live ancestry, root-first: every live hierarchy folder up to the first non-UNS-folder (the root).
var chain = LiveHierarchyChain(owning);
// A node whose first id segment is NOT its equipment's id is a UNS reference variable
// (s=Area/Line/Equipment/EffectiveName): authorized at its split path, but only while that path is where the
// current generation places the equipment it hangs under (review of #808 — a moved or renamed equipment's
// variables keep their OLD-name ids until re-pointed, and the old names' grants must not keep them).
return firstSegment.SequenceEqual(owning.NodeId.IdentifierAsString)
? UnsAclSubject.Folder(chain, isFolderItself: ReferenceEquals(node, owning))
: UnsAclSubject.ReferenceVariable(chain);
}
/// <summary>
/// The live ancestry of <paramref name="folder"/>, root-first and ending at it: every live hierarchy folder (a
/// slash-free UNS folder id) up to the first node that is not one (the shared root).
/// </summary>
/// <param name="folder">A live hierarchy folder.</param>
/// <returns>The id chain.</returns>
private List<string> LiveHierarchyChain(NodeState folder)
{
var chain = new List<string>(3);
current = owning;
NodeState? current = folder;
for (var depth = 0; current is not null && depth < MaxUnsAncestry; depth++, current = ParentOf(current))
{
if (LiveUnsFolderId(current) is not { } id || id.Contains('/')) break;
@@ -4304,9 +4323,30 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2
}
chain.Reverse();
return UnsAclSubject.Folder(chain, isFolderItself: ReferenceEquals(node, owning));
return chain;
}
/// <summary>The <c>EquipmentId</c> the exact live <paramref name="condition"/> is bound to, or null. Call under <c>Lock</c>.</summary>
private string? BoundScriptedOwner(RealmKey key, ConditionState condition) =>
_scriptedAlarmOwners.TryGetValue(key, out var binding)
&& ReferenceEquals(binding.Alarm, condition)
&& _alarmConditions.TryGetValue(key, out var live)
&& ReferenceEquals(live, condition)
? binding.EquipmentId
: null;
/// <summary>
/// The live folder-id ancestry of the equipment folder a BOUND scripted condition hangs under (the #808 folder
/// rule applied to the #797 owner), or null when unbound or no longer under that folder. Call under <c>Lock</c>.
/// </summary>
private List<string>? BoundScriptedOwnerChain(RealmKey key, ConditionState condition) =>
BoundScriptedOwner(key, condition) is { } owner
&& ParentOf(condition) is FolderState parent
&& LiveUnsFolderId(parent) is { } parentId
&& string.Equals(parentId, owner, StringComparison.Ordinal)
? LiveHierarchyChain(parent)
: null;
/// <summary>The node's parent in the instance tree, or null for a type/root node.</summary>
/// <param name="node">The node.</param>
/// <returns>Its parent.</returns>
src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/Security/DataPlaneAuthorizer.cs
+89 -7
View File
@@ -147,6 +147,17 @@ public sealed class DataPlaneAuthorizer
/// </summary>
internal const string OwnedNodeSegment = "\u0000owned";
/// <summary>
/// The single segment, directly under the UNS realm, that an identifier no live node holds is placed at
/// (Gitea #808 review): only a realm-wide grant reaches it, and — unlike the realm root itself — it carries no
/// Browse transit, so a narrow grantee is never handed a removed opaque id (a NodeDeleted model change, a
/// probe) and a removed id can never match a narrow grant on an unrelated entity that shares its name.
/// </summary>
internal const string UnknownNodeSegment = "\u0000unknown";
/// <summary>The depth of a UNS reference-variable id: <c>Area/Line/Equipment/EffectiveName</c>.</summary>
private const int ReferenceVariableSegments = 4;
/// <summary>
/// Authorizes <paramref name="operation"/> on <paramref name="nodeId"/> through the decision cache —
/// the entry point for Read, Browse and subscription creation, which ask the same question many
@@ -299,12 +310,15 @@ public sealed class DataPlaneAuthorizer
/// <param name="conditionNodeId">The condition the method targets (for the denial log only — never parsed).</param>
/// <param name="ownerEquipmentId">The <c>EquipmentId</c> the live condition is bound to, or null when the
/// node manager holds no binding for this exact instance.</param>
/// <param name="ownerFolderChain">The LIVE folder-id ancestry of the equipment folder the condition hangs under
/// (root-first, ending at <paramref name="ownerEquipmentId"/>), or null when unbound (Gitea #808 review).</param>
/// <returns><c>null</c> to proceed; otherwise the veto to return to the SDK.</returns>
public ServiceResult? AuthorizeScriptedCondition(
RoleCarryingUserIdentity? identity,
OpcUaOperation operation,
NodeId conditionNodeId,
string? ownerEquipmentId)
string? ownerEquipmentId,
IReadOnlyList<string>? ownerFolderChain)
{
if (identity is null)
return Deny(operation, conditionNodeId, principal: null, "the session carries no role-carrying identity");
@@ -325,8 +339,11 @@ public sealed class DataPlaneAuthorizer
return DenyRevokedKey(operation, conditionNodeId, identity.Principal, keyId, snapshot.GenerationId, cached: false);
conditionNodeId.TryGetValue(out string alarmId);
if (!TryPlaceScriptedCondition(snapshot, alarmId, ownerEquipmentId, out var segments, out var reason))
if (!TryPlaceScriptedCondition(
snapshot, alarmId, ownerEquipmentId, ownerFolderChain, out var segments, out var reason))
{
return Deny(operation, conditionNodeId, identity.Principal, reason);
}
var scope = new NodeScope
{
@@ -348,6 +365,7 @@ public sealed class DataPlaneAuthorizer
/// <param name="snapshot">The generation in force.</param>
/// <param name="alarmId">The condition's <c>ScriptedAlarmId</c>.</param>
/// <param name="ownerEquipmentId">The live owner binding, or null when unbound.</param>
/// <param name="ownerFolderChain">The live id ancestry of the owner's folder, or null when unbound.</param>
/// <param name="segments">The owning equipment's path below the UNS realm root.</param>
/// <param name="reason">Why placement failed (the caller denies).</param>
/// <returns><see langword="true"/> when the condition is placed.</returns>
@@ -355,6 +373,7 @@ public sealed class DataPlaneAuthorizer
DataPlaneAuthoritySnapshot snapshot,
string? alarmId,
string? ownerEquipmentId,
IReadOnlyList<string>? ownerFolderChain,
out IReadOnlyList<string> segments,
out string reason)
{
@@ -383,21 +402,41 @@ public sealed class DataPlaneAuthorizer
return false;
}
// The owner's FOLDER must also sit where this generation deploys it (the folder rule, #808 review): an
// equipment moved to another line keeps its id, so the owner checks above pass while its folder — and this
// alarm under it — still hangs under the old line. Without this the new line's grant could read and command
// the alarm while the folder above it is denied.
if (ownerFolderChain is null
|| !snapshot.TryGetUnsFolderSegments(ownerEquipmentId, ownerFolderChain, out var folderSegments)
|| !folderSegments.SequenceEqual(segments, StringComparer.Ordinal))
{
segments = [];
reason = $"owner equipment {ownerEquipmentId}'s folder is not placed as generation {snapshot.GenerationId} deploys it (moved, or not yet rebuilt)";
return false;
}
reason = string.Empty;
return true;
}
/// <summary>
/// Turns a non-<see cref="UnsAclSubjectKind.Path"/> UNS subject into the segments it is authorized at in
/// <paramref name="snapshot"/> (Gitea #808). Every gap fails — the caller denies; there is no fallback to
/// splitting the identifier, to the realm root, or to the cluster.
/// <paramref name="snapshot"/> (Gitea #808). For a folder, a node a folder owns, a reference variable and a
/// scripted condition every gap fails — the caller denies, with no fallback to splitting the identifier, to
/// the realm root or to the cluster. The one deliberate exception is <see cref="UnsAclSubjectKind.Unknown"/>
/// (no live node holds the id): it is placed at <see cref="UnknownNodeSegment"/> directly under the realm, so
/// only a realm-wide (<c>Cluster</c> / <c>Namespace uns</c>) grant decides it — never a narrow grant that
/// happens to share a name with a removed opaque id — except a reference-variable-shaped id (exactly four
/// segments), which keeps its name-path split.
/// </summary>
/// <param name="identifier">The node's <c>s=</c> identifier (split only for the shapes that are paths).</param>
/// <param name="snapshot">The generation in force.</param>
/// <param name="subject">The live facts the node manager reported.</param>
/// <param name="segments">The resolved path below the UNS realm root.</param>
/// <param name="reason">Why placement failed.</param>
/// <returns><see langword="true"/> when the subject is placed.</returns>
private static bool TryPlaceUnsSubject(
string identifier,
DataPlaneAuthoritySnapshot snapshot,
in UnsAclSubject subject,
out IReadOnlyList<string> segments,
@@ -407,6 +446,36 @@ public sealed class DataPlaneAuthorizer
bool owned;
switch (subject.Kind)
{
case UnsAclSubjectKind.Unknown:
{
var split = identifier.Split('/', StringSplitOptions.RemoveEmptyEntries);
segments = split.Length == ReferenceVariableSegments ? split : [UnknownNodeSegment];
reason = string.Empty;
return true;
}
case UnsAclSubjectKind.ReferenceVariable:
{
// A reference variable's id IS its name path, so a Tag grant keeps matching it — but only while that
// path is where THIS generation puts its equipment (plus one effective-name segment). An equipment
// moved to another line, or an Area/Line/Equipment renamed, leaves the variable's id on the OLD names
// until the address space re-points it; the old names' grants must not keep reading / writing it.
var split = identifier.Split('/', StringSplitOptions.RemoveEmptyEntries);
if (subject.FolderChain is not { Count: > 0 } equipmentChain
|| !snapshot.TryGetUnsFolderSegments(equipmentChain[^1], equipmentChain, out var equipment)
|| split.Length != equipment.Count + 1
|| !split.Take(equipment.Count).SequenceEqual(equipment, StringComparer.Ordinal))
{
segments = [];
reason = $"the UNS reference variable's path does not match its equipment {subject.FolderChain?.LastOrDefault() ?? "(none)"}'s placement in generation {snapshot.GenerationId} (moved, renamed, or not yet re-pointed)";
return false;
}
segments = split;
reason = string.Empty;
return true;
}
case UnsAclSubjectKind.Folder:
if (subject.FolderChain is not { Count: > 0 } chain
|| !snapshot.TryGetUnsFolderSegments(chain[^1], chain, out owner))
@@ -420,7 +489,8 @@ public sealed class DataPlaneAuthorizer
break;
case UnsAclSubjectKind.ScriptedCondition:
if (!TryPlaceScriptedCondition(snapshot, subject.AlarmId, subject.OwnerEquipmentId, out owner, out reason))
if (!TryPlaceScriptedCondition(
snapshot, subject.AlarmId, subject.OwnerEquipmentId, subject.FolderChain, out owner, out reason))
{
segments = [];
return false;
@@ -444,7 +514,10 @@ public sealed class DataPlaneAuthorizer
/// The decision-cache key for a node authorized at a RESOLVED scope rather than at its split identifier. It
/// names the scope, not the node, because the cached grant walk depends only on the scope — and keying on the
/// node would let a verdict outlive a re-bind or re-parent the resolver sees but the generation does not. The
/// leading NUL keeps it disjoint from every real <c>s=</c> identifier.
/// leading NUL keeps it disjoint from every <c>s=</c> identifier — which <c>Evaluate</c> ENFORCES by denying any
/// identifier that contains a NUL before the cache is consulted (review of #808: otherwise a client reading
/// the nonexistent <c>s="\0Area1\0Line1\0Pump"</c> would memoize a deny under the real folder's key for its
/// whole key shard).
/// </summary>
/// <param name="segments">The resolved path below the UNS realm root.</param>
/// <returns>The cache key.</returns>
@@ -515,6 +588,15 @@ public sealed class DataPlaneAuthorizer
"the node id carries no string identifier, so it addresses no ACL scope", cached);
}
// No node this server mints carries a NUL, and the resolved-scope cache keys are NUL-prefixed: refusing NUL
// here keeps the two keyspaces disjoint, so a client-chosen identifier can never collide with (and poison)
// a resolved scope's memoized verdict. Denied uncached, before the lookup.
if (identifier.Contains('\u0000'))
{
return Deny(operation, nodeId, identity.Principal,
"the node id carries a NUL, which no node in this address space does", cached);
}
var sessionKey = identity.ApiKeyId ?? identity.Principal;
// Gitea #808: a UNS folder minted under an opaque entity id, a node it owns, and a scripted condition (or
@@ -528,7 +610,7 @@ public sealed class DataPlaneAuthorizer
var subject = resolve(identifier);
if (subject.Kind != UnsAclSubjectKind.Path)
{
if (!TryPlaceUnsSubject(snapshot, subject, out var placed, out var reason))
if (!TryPlaceUnsSubject(identifier, snapshot, subject, out var placed, out var reason))
return Deny(operation, nodeId, identity.Principal, reason, cached);
resolvedSegments = placed;
src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/Security/UnsAclSubject.cs
+43 -5
View File
@@ -3,9 +3,27 @@ namespace ZB.MOM.WW.OtOpcUa.OpcUaServer.Security;
/// <summary>How a UNS-namespace node is to be placed in the ACL scope tree (Gitea #808).</summary>
public enum UnsAclSubjectKind
{
/// <summary>The node's <c>s=</c> identifier IS its ACL path (a UNS reference variable, or an unknown id).</summary>
/// <summary>
/// The node's <c>s=</c> identifier IS its ACL path and nothing live vouches for it — a UNS node with no live
/// hierarchy-folder ancestor (only test rigs mint those). Split as before.
/// </summary>
Path,
/// <summary>
/// No live node holds the identifier (removed, never deployed, or a client probing). Only a realm-wide grant
/// may decide it — except a reference-variable-shaped id (exactly four segments), which keeps its split so a
/// removed reference variable's NodeDeleted still reaches the grantees of that name path.
/// </summary>
Unknown,
/// <summary>
/// A UNS reference variable (<c>s=Area/Line/Equipment/EffectiveName</c>) hanging under a live equipment folder:
/// authorized at its split path, but only while that path equals the equipment's placement in the current
/// generation plus one effective-name segment (a moved or renamed equipment's variables are denied until the
/// address space re-points them).
/// </summary>
ReferenceVariable,
/// <summary>
/// A UNS Area / Line / Equipment folder minted under its opaque entity id, or a node owned by one
/// (a virtual tag <c>EQ-…/Name</c> or its sub-folder): placed at the folder's path in the current generation.
@@ -42,12 +60,18 @@ public readonly record struct UnsAclSubject
/// <summary>The identifier is the ACL path — the pre-#808 behaviour.</summary>
public static readonly UnsAclSubject Path = new() { Kind = UnsAclSubjectKind.Path };
/// <summary>No live node holds the identifier.</summary>
public static readonly UnsAclSubject Unknown = new() { Kind = UnsAclSubjectKind.Unknown };
/// <summary>Which placement rule applies.</summary>
public UnsAclSubjectKind Kind { get; init; }
/// <summary>
/// For <see cref="UnsAclSubjectKind.Folder"/>: the owning folder's LIVE id ancestry, root-first, ending at
/// the owning folder itself (e.g. <c>[UA-…, UL-…, EQ-…]</c>).
/// The owning folder's LIVE id ancestry, root-first, ending at the owning folder itself
/// (e.g. <c>[UA-…, UL-…, EQ-…]</c>): for <see cref="UnsAclSubjectKind.Folder"/> and
/// <see cref="UnsAclSubjectKind.ReferenceVariable"/> the folder at or above the node; for
/// <see cref="UnsAclSubjectKind.ScriptedCondition"/> the folder the bound condition hangs under (null when
/// unbound).
/// </summary>
public IReadOnlyList<string>? FolderChain { get; init; }
@@ -76,7 +100,21 @@ public readonly record struct UnsAclSubject
/// <summary>A scripted condition (or one of its children) and its live owner binding.</summary>
/// <param name="alarmId">The condition's <c>ScriptedAlarmId</c>.</param>
/// <param name="ownerEquipmentId">The live owner binding, or null when unbound.</param>
/// <param name="ownerFolderChain">The live id ancestry of the folder the condition hangs under, or null.</param>
/// <returns>The subject.</returns>
public static UnsAclSubject ScriptedCondition(string alarmId, string? ownerEquipmentId) =>
new() { Kind = UnsAclSubjectKind.ScriptedCondition, AlarmId = alarmId, OwnerEquipmentId = ownerEquipmentId };
public static UnsAclSubject ScriptedCondition(
string alarmId, string? ownerEquipmentId, IReadOnlyList<string>? ownerFolderChain) =>
new()
{
Kind = UnsAclSubjectKind.ScriptedCondition,
AlarmId = alarmId,
OwnerEquipmentId = ownerEquipmentId,
FolderChain = ownerFolderChain,
};
/// <summary>A UNS reference variable under a live equipment folder.</summary>
/// <param name="equipmentChain">Root-first live ancestry ending at the variable's equipment folder.</param>
/// <returns>The subject.</returns>
public static UnsAclSubject ReferenceVariable(IReadOnlyList<string> equipmentChain) =>
new() { Kind = UnsAclSubjectKind.ReferenceVariable, FolderChain = equipmentChain };
}