Merge branch 'fix/acl-anchor-browse-transit' — ACL browse transit reaches the shared root folder (narrow Browse grants were unreachable by navigation; r3 T7 live-gate finding)
v2-ci / build (push) Successful in 6m24s
v2-ci / unit-tests (push) Failing after 17m36s

This commit is contained in:
Joseph Doherty
2026-08-18 10:02:18 -04:00
9 changed files with 348 additions and 11 deletions
+7 -1
View File
@@ -801,7 +801,13 @@ Two things to know before relying on any of this:
grant deep in the tree is reached by browsing the nodes above it, and the trie inherits downward
only, so `PermissionTrieNode.BrowseTransitPrincipals` marks the path. Planting a real `Browse` grant
on each ancestor instead would be collected by the additive downward walk and leak `Browse` onto
every sibling subtree beneath that ancestor.
every sibling subtree beneath that ancestor. ⚠️ **The topmost ancestor is the single shared root
folder** (`s=OtOpcUa`, raw namespace) that BOTH realm trees hang under — its identifier is a display
name, not a path, so `DataPlaneAuthorizer.BuildScope` maps it onto the trie's **zero-segment
realm-root scope** (`V3NodeIds.SharedRootFolderId`, realm-qualified: in the UNS namespace that
identifier is an ordinary Area), and its Browse considers transit in **either** realm because it
fronts both trees. Before this every narrow Browse grant was unreachable by navigation — the Browse
gate denies the whole call when its source node is denied, so clients were stopped at the front door.
- **Revocation reaches live subscriptions** (stage 6): every other operation re-asks per request, but a
subscription is authorized once and then streams — and streaming is how machine consumers actually
read this server. `DataPlaneAuthorityState.Changed` fires `RevalidateMonitoredItems`. ⚠️ A denied
@@ -818,6 +818,17 @@ round-2 register. **None of these blocks the branch.**
Client.CLI's own default recursive browse. Discovered by the T7 live gate. **RECOMMEND filing a
Gitea issue before anyone enables `Security:Acl:Enabled` in production.** Likely fix: mark the
realm-root container browse-transit whenever any grant resolves beneath it.
- **Closed by `89630ee4` + `ff9d617a` (branch `fix/acl-anchor-browse-transit`, 2026-08-18)** — no
Gitea issue needed; fixed directly. The actual mechanism was one step off the guess above: the
trie's zero-segment realm-root branch already existed and was correct — nothing ever *produced*
that scope, because the shared anchor's identifier (`s=OtOpcUa`, now `V3NodeIds.SharedRootFolderId`)
split into a one-segment scope no grant path contains. `DataPlaneAuthorizer.BuildScope` now maps
the anchor (realm-qualified: Raw namespace only) onto the zero-segment scope, and `Evaluate` adds
a Browse-only second walk against the zero-segment **Uns** scope — the single anchor fronts both
trees, so a UNS-only grant holder must also get through. 11 new tests (3 trie pins, 7 authorizer
incl. falsification of the OR-leg and of the realm qualification, 1 e2e browse-succeeds-and-still-
filters through a booted server). Full high-risk chain: spec ✅, code CHANGES REQUESTED (stale
"two namespace root folders" XML remark) → fixed → delta re-APPROVED.
- **`ns=0` standard nodes are ungated — the boot warning overstates.** With the gate on, both
the key and anonymous read `i=2256` and browse `i=2253` successfully: those are served by the
SDK's `CoreNodeManager` and never reach `DataPlaneAuthorizer`. Defensible (standard server
+14 -3
View File
@@ -427,12 +427,23 @@ Behaviours worth knowing before enabling it:
what this necessarily discloses — the *names* of the folders on the path to a granted node are visible
to whoever holds that grant. It is conferred only by grants that actually carry `Browse`, and it
confers nothing but `Browse`.
- **…including the shared root folder every browse starts at.** Both realm trees hang under a single
anchor folder (`s=OtOpcUa`, minted in the **raw** namespace by `CreateAddressSpace`; top-level raw
folders and top-level UNS areas alike parent under it). Its identifier is a display name rather than
a path, so splitting it produced a segment no grant path can contain, and the Browse gate refuses the
*whole* call when its source node is denied — every narrow `Browse` grant was therefore unreachable
by navigation, denied at the front door. The gate maps that one identifier onto the trie's
zero-segment **realm-root** scope, and because the anchor fronts both trees its browse considers
transit in **either** realm: a principal whose only grant lives in the UNS tree still enters through
this raw-namespace node. The mapping is realm-qualified — in the UNS namespace `s=OtOpcUa` is an
ordinary Area an operator happened to name that way and is evaluated as the path it is.
- **A denied node reports `BadUserAccessDenied` rather than `BadNodeIdUnknown`**, because the gate runs
before node resolution. That is the information-hiding direction — error codes can no longer be used
to enumerate the address space — but it does mean "access denied" is not evidence the node exists.
- **The two namespace root folders remain visible** to any admitted session. They are referenced from
the ns=0 `Objects` folder, whose browse the SDK's own node manager serves; a session with no grants
sees the two roots and is refused on entry.
- **The shared root folder remains *listed*** for any admitted session. It is referenced from the ns=0
`Objects` folder, whose browse the SDK's own node manager serves, so a session with no grants sees the
anchor in that listing — and is refused the moment it tries to browse *into* it (the transit above is
what a granted session enters on). There is one anchor, not one per namespace.
#### Cost
@@ -28,6 +28,16 @@ public static class V3NodeIds
/// <summary>The UNS namespace URI (equipment-oriented subtree).</summary>
public const string UnsNamespaceUri = "https://zb.com/otopcua/uns";
/// <summary>
/// The s= identifier of the single shared root folder both realm trees hang under
/// (minted in the RAW namespace by the node manager's CreateAddressSpace; top-level
/// Raw folders AND top-level UNS areas alike parent under it). The ACL gate needs this
/// identity to map the anchor onto the trie's zero-segment realm-root scope — the
/// identifier is a display name, never a RawPath/UNS-path segment, so without the
/// mapping no grant path can ever match it.
/// </summary>
public const string SharedRootFolderId = "OtOpcUa";
/// <summary>The namespace URI for a realm.</summary>
/// <param name="realm">The address-space realm.</param>
/// <returns>The realm's namespace URI.</returns>
@@ -2919,9 +2919,13 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2
/// that grant, because navigation requires them.
/// </para>
/// <para>
/// ⚠️ The two namespace root folders are referenced from the ns=0 <c>Objects</c> folder, whose
/// browse is served by the SDK's own node manager and is not gated here. A session with no
/// grants at all therefore still sees that the two roots exist, and is refused on entry.
/// ⚠️ There is ONE shared root folder (<c>V3NodeIds.SharedRootFolderId</c>, minted in the raw
/// namespace by <see cref="CreateAddressSpace"/>) fronting BOTH realm trees — not one root per
/// namespace. It is referenced from the ns=0 <c>Objects</c> folder, whose browse is served by
/// the SDK's own node manager and is not gated here, so a session with no grants at all still
/// sees that the root exists — and is refused on entry. A session holding any Browse grant in
/// either realm enters it via the zero-segment realm-root scope
/// <c>Security.DataPlaneAuthorizer.BuildScope</c> maps the anchor to.
/// </para>
/// </remarks>
public override void Browse(
@@ -4219,11 +4223,15 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2
base.CreateAddressSpace(externalReferences);
// Create one root folder under Objects/ for every variable we mint to hang under.
// Its identifier is V3NodeIds.SharedRootFolderId rather than a literal because the ACL gate
// keys the trie's realm-root scope off exactly this identity — see
// DataPlaneAuthorizer.IsSharedRootFolder. Both realm trees parent here (ResolveParentFolder),
// so the anchor is the one node every browse of this server passes through.
_root = new FolderState(null)
{
NodeId = new NodeId("OtOpcUa", NamespaceIndex),
BrowseName = new QualifiedName("OtOpcUa", NamespaceIndex),
DisplayName = "OtOpcUa",
NodeId = new NodeId(V3NodeIds.SharedRootFolderId, NamespaceIndex),
BrowseName = new QualifiedName(V3NodeIds.SharedRootFolderId, NamespaceIndex),
DisplayName = V3NodeIds.SharedRootFolderId,
EventNotifier = EventNotifiers.None,
TypeDefinitionId = ObjectTypeIds.FolderType,
};
@@ -271,6 +271,23 @@ public sealed class DataPlaneAuthorizer
var scope = BuildScope(identifier, realm, snapshot.ClusterId);
var allowed = Walk(snapshot, identity, operation, scope, now);
// The single shared anchor fronts BOTH realm trees, so its browse must consider transit in
// either: a principal whose only Browse grant lives in the UNS tree still enters through this
// node. Browse only — transit confers navigation, never read/write/subscribe, matching the
// evaluator's own rule.
if (!allowed && operation == OpcUaOperation.Browse && IsSharedRootFolder(identifier, realm))
{
allowed = Walk(
snapshot, identity, operation,
new NodeScope
{
ClusterId = snapshot.ClusterId!,
Kind = NodeHierarchyKind.Uns,
Segments = [],
},
now);
}
if (cached) Decisions.Set(snapshot.GenerationId, sessionKey, operation, realm, identifier, allowed);
return allowed
@@ -337,6 +354,26 @@ public sealed class DataPlaneAuthorizer
/// string" guard this used to carry moved up into <c>Evaluate</c>, where it has to run before
/// the cache lookup anyway — the identifier is the cache key.
/// </para>
/// <para>
/// <b>The one identifier that is not a path is the shared root folder</b>
/// (<see cref="V3NodeIds.SharedRootFolderId"/>), which both realm trees hang under. Its
/// identifier is its display identity, so splitting it yields a one-segment scope containing a
/// name no grant path can ever hold — which denied the anchor for every principal short of a
/// cluster grant, and the node manager refuses a whole Browse whose SOURCE is denied. It maps
/// instead onto the zero-segment scope: the realm root, which is exactly the question
/// <see cref="PermissionTrie.HasBrowseTransit"/> was written to answer ("is this an ancestor of
/// anything granted in this realm?") and which no production path had ever asked it.
/// </para>
/// <para>
/// Keying that on the identifier introduces no new ambiguity: a genuine top-level raw folder
/// literally named <c>OtOpcUa</c> would already mint the identical NodeId as the anchor in the
/// served address space itself (<c>EnsureFolder</c> takes the RawPath as the id), so the
/// collision — if an operator ever authors one — predates this mapping and lives in the address
/// space rather than in the gate. One narrowing side effect in that degenerate case: a grant
/// scoped to the colliding folder used to leak onto the anchor's own Read/Write (the old
/// one-segment scope descended into it); the zero-segment scope no longer does, which is the
/// fail-closed direction.
/// </para>
/// </remarks>
/// <param name="identifier">The node's non-empty <c>s=</c> identifier.</param>
/// <param name="realm">The namespace the node was resolved in.</param>
@@ -346,9 +383,27 @@ public sealed class DataPlaneAuthorizer
{
ClusterId = clusterId,
Kind = realm == AddressSpaceRealm.Raw ? NodeHierarchyKind.Raw : NodeHierarchyKind.Uns,
Segments = identifier.Split('/', StringSplitOptions.RemoveEmptyEntries),
Segments = IsSharedRootFolder(identifier, realm)
? []
: identifier.Split('/', StringSplitOptions.RemoveEmptyEntries),
};
/// <summary>
/// Whether this node is the single shared root folder both realm trees hang under.
/// </summary>
/// <remarks>
/// <b>Realm-qualified on purpose.</b> The anchor is minted in the RAW namespace; in the UNS
/// namespace <c>s=OtOpcUa</c> is an ordinary Area an operator happened to name that way, and giving
/// it anchor semantics would collapse a real UNS path onto the realm root — silently widening every
/// browse of it.
/// </remarks>
/// <param name="identifier">The node's <c>s=</c> identifier.</param>
/// <param name="realm">The namespace the node was resolved in.</param>
/// <returns><see langword="true"/> when the node is the shared anchor folder.</returns>
private static bool IsSharedRootFolder(string identifier, AddressSpaceRealm realm) =>
realm == AddressSpaceRealm.Raw
&& string.Equals(identifier, V3NodeIds.SharedRootFolderId, StringComparison.Ordinal);
/// <summary>Denies with a constant reason — nothing to build, so nothing to defer.</summary>
/// <param name="operation">The operation being attempted.</param>
/// <param name="nodeId">The target node.</param>
@@ -148,6 +148,54 @@ public sealed class PermissionTrieBrowseTransitTests
evaluator.Authorize(session, OpcUaOperation.Browse, RawScope("Plant", "Siemens")).IsAllowed.ShouldBeFalse();
}
// ───────────────── the realm root (zero-segment scope) ─────────────────
/// <summary>
/// A zero-segment scope addresses the realm root itself, which is an ancestor of everything granted
/// in that realm — so a Browse grant anywhere in the raw tree makes the raw realm root transitable.
/// </summary>
/// <remarks>
/// The branch is as old as <see cref="PermissionTrie.HasBrowseTransit"/> but had no production
/// caller until the ACL gate learned to map the address space's single shared root folder
/// (<c>s=OtOpcUa</c>, raw namespace) onto it. Until then every narrow Browse grant was unreachable:
/// the anchor's identifier is a display name, never a path segment, so splitting it produced a
/// one-segment scope no grant path could contain and the front door denied the whole browse.
/// </remarks>
[Fact]
public void The_realm_root_is_transit_for_any_browse_grant_in_that_realm()
{
var trie = TrieGranting(NodePermissions.Browse, "Plant", "Modbus", "dev1");
trie.HasBrowseTransit(RawScope(), [Principal]).ShouldBeTrue();
}
/// <summary>
/// Realm roots are separate nodes: a raw-realm grant does not make the UNS realm root transitable.
/// Without this the anchor's second-realm OR-leg in the gate would be indistinguishable from an
/// unconditional allow.
/// </summary>
[Fact]
public void The_other_realms_root_is_not_transit()
{
var trie = TrieGranting(NodePermissions.Browse, "Plant", "Modbus", "dev1");
trie.HasBrowseTransit(
new NodeScope { ClusterId = Cluster, Kind = NodeHierarchyKind.Uns, Segments = [] },
[Principal]).ShouldBeFalse();
}
/// <summary>
/// Only a grant carrying the Browse bit marks the realm root — the same rule that governs every
/// other ancestor, applied at depth zero.
/// </summary>
[Fact]
public void A_grant_without_the_browse_bit_does_not_mark_the_realm_root()
{
var trie = TrieGranting(NodePermissions.Read, "Plant", "Modbus", "dev1");
trie.HasBrowseTransit(RawScope(), [Principal]).ShouldBeFalse();
}
// ───────────────────────────── helpers ─────────────────────────────
private static NodeScope RawScope(params string[] segments) =>
@@ -391,8 +391,156 @@ public sealed class DataPlaneAuthorizerTests
.Enabled.ShouldBe(enabled);
}
// ───────────────────────────── the shared root folder ─────────────────────────────
/// <summary>
/// <b>The load-bearing allow.</b> Both realm trees hang under one anchor folder
/// (<c>s=OtOpcUa</c>, raw namespace), and the node manager's Browse gate refuses the WHOLE browse
/// when its source node is denied — so a session whose only grant is a narrow Browse deep in the raw
/// tree could not get past the front door at all. The anchor's identifier is a display name rather
/// than a path, so the ordinary split produced a one-segment scope (<c>["OtOpcUa"]</c>) that no
/// grant path can ever contain.
/// </summary>
[Fact]
public void The_shared_root_folder_is_browsable_on_a_narrow_raw_browse_grant()
{
var gate = Gate(
ScopedGrant(NodePermissions.Browse, NodeAclScopeKind.Tag, "tag-1"),
Scopes(("tag-1", NodeAclScopeKind.Tag, ["raw", "Plant", "Modbus", "dev1", "Speed"])));
gate.AuthorizeCached(Identity(), OpcUaOperation.Browse, Anchor(), AddressSpaceRealm.Raw)
.ShouldBeNull();
}
/// <summary>
/// The anchor fronts BOTH trees, so a principal whose only Browse grant lives in the UNS tree still
/// enters through this raw-namespace node — the gate must consider transit in either realm.
/// </summary>
/// <remarks>
/// This is the test that falsifies the second walk: delete the UNS OR-leg from
/// <c>DataPlaneAuthorizer.Evaluate</c> and only this fact goes red, because the first walk asks the
/// raw realm root, which a purely-UNS grant never marks.
/// </remarks>
[Fact]
public void The_shared_root_folder_is_browsable_on_a_uns_only_browse_grant()
{
var gate = Gate(
ScopedGrant(NodePermissions.Browse, NodeAclScopeKind.Equipment, "eq-1"),
Scopes(("eq-1", NodeAclScopeKind.Equipment, ["uns", "Area", "Line", "Eq"])));
gate.AuthorizeCached(Identity(), OpcUaOperation.Browse, Anchor(), AddressSpaceRealm.Raw)
.ShouldBeNull();
}
/// <summary>
/// The anchor is not a bypass: a principal holding no grant at all still cannot browse it, so the
/// mapping opens the front door only for a session that has somewhere to go.
/// </summary>
[Fact]
public void The_shared_root_folder_is_denied_without_any_grant()
{
var state = new DataPlaneAuthorityState();
state.Apply(Cluster, "rev-1", [Key(enabled: true)], [], Scopes());
var gate = new DataPlaneAuthorizer(
state, new TriePermissionEvaluator(state.Tries), enabled: true, NullLogger.Instance);
Veto(gate.AuthorizeCached(Identity(), OpcUaOperation.Browse, Anchor(), AddressSpaceRealm.Raw));
}
/// <summary>
/// Transit is marked only by grants that actually carry <c>Browse</c>. A deep Read-only grant leaves
/// the anchor closed — the node is readable by NodeId, not discoverable by navigating.
/// </summary>
[Fact]
public void The_shared_root_folder_is_denied_when_the_deep_grant_lacks_browse()
{
var gate = Gate(
ScopedGrant(NodePermissions.Read, NodeAclScopeKind.Tag, "tag-1"),
Scopes(("tag-1", NodeAclScopeKind.Tag, ["raw", "Plant", "Modbus", "dev1", "Speed"])));
Veto(gate.AuthorizeCached(Identity(), OpcUaOperation.Browse, Anchor(), AddressSpaceRealm.Raw));
}
/// <summary>
/// Transit confers navigation and nothing else, at the anchor exactly as at every other ancestor: a
/// deep Browse grant does not make the root folder readable.
/// </summary>
[Fact]
public void The_shared_root_folder_is_not_readable_on_transit_alone()
{
var gate = Gate(
ScopedGrant(NodePermissions.Browse, NodeAclScopeKind.Tag, "tag-1"),
Scopes(("tag-1", NodeAclScopeKind.Tag, ["raw", "Plant", "Modbus", "dev1", "Speed"])));
gate.AuthorizeCached(Identity(), OpcUaOperation.Browse, Anchor(), AddressSpaceRealm.Raw).ShouldBeNull();
Veto(gate.AuthorizeCached(Identity(), OpcUaOperation.Read, Anchor(), AddressSpaceRealm.Raw));
}
/// <summary>
/// Guard order is unchanged by the anchor branch: the key's usability is re-checked before any walk
/// runs, so a revoked key is refused the anchor even though its grant would confer transit.
/// </summary>
[Fact]
public void A_revoked_key_is_denied_the_shared_root_folder()
{
var state = new DataPlaneAuthorityState();
var grant = ScopedGrant(NodePermissions.Browse, NodeAclScopeKind.Tag, "tag-1");
var scopes = Scopes(("tag-1", NodeAclScopeKind.Tag, ["raw", "Plant", "Modbus", "dev1", "Speed"]));
var gate = new DataPlaneAuthorizer(
state, new TriePermissionEvaluator(state.Tries), enabled: true, NullLogger.Instance);
state.Apply(Cluster, "rev-1", [Key(enabled: true)], [grant], scopes);
gate.AuthorizeCached(Identity(), OpcUaOperation.Browse, Anchor(), AddressSpaceRealm.Raw).ShouldBeNull();
state.Apply(Cluster, "rev-2", [Key(enabled: false)], [grant], scopes);
Veto(gate.AuthorizeCached(Identity(), OpcUaOperation.Browse, Anchor(), AddressSpaceRealm.Raw));
}
/// <summary>
/// Anchor semantics are realm-qualified. In the UNS namespace <c>s=OtOpcUa</c> is an ordinary Area
/// that an operator happened to name after the root folder, and it must be evaluated as the path it
/// is — a Browse grant in an unrelated UNS subtree must not open it.
/// </summary>
/// <remarks>
/// The discriminating grant is a <b>UNS</b> one deliberately. Un-qualifying
/// <c>IsSharedRootFolder</c> maps this node onto the zero-segment UNS scope, which the unrelated
/// grant's own transit marks — so the Area silently becomes browsable for a principal granted
/// nothing anywhere near it. A raw-only grant would have been refused either way and would have
/// proven nothing.
/// </remarks>
[Fact]
public void A_uns_area_named_like_the_anchor_gets_no_anchor_semantics()
{
var unsArea = new NodeId(V3NodeIds.SharedRootFolderId, 3);
var gate = Gate(
ScopedGrant(NodePermissions.Browse, NodeAclScopeKind.Equipment, "eq-1"),
Scopes(("eq-1", NodeAclScopeKind.Equipment, ["uns", "Area", "Line", "Eq"])));
Veto(gate.AuthorizeCached(Identity(), OpcUaOperation.Browse, unsArea, AddressSpaceRealm.Uns));
// Non-vacuity, twice over: the grant's OWN ancestor Area is browsable on transit, and the same
// identifier in the RAW namespace — the real anchor — is browsable on the same grant.
gate.AuthorizeCached(Identity(), OpcUaOperation.Browse, new NodeId("Area", 3), AddressSpaceRealm.Uns)
.ShouldBeNull();
gate.AuthorizeCached(Identity(), OpcUaOperation.Browse, Anchor(), AddressSpaceRealm.Raw)
.ShouldBeNull();
}
// ───────────────────────────── helpers ─────────────────────────────
/// <summary>The single shared root folder both realm trees hang under, in the raw namespace.</summary>
private static NodeId Anchor() => new(V3NodeIds.SharedRootFolderId, 2);
private static NodeAcl ScopedGrant(NodePermissions flags, NodeAclScopeKind kind, string scopeId) => new()
{
NodeAclId = $"acl-{scopeId}",
ClusterId = Cluster,
Principal = Principal,
ScopeKind = kind,
ScopeId = scopeId,
PermissionFlags = flags,
};
private static void Veto(ServiceResult? result)
{
result.ShouldNotBeNull();
@@ -348,6 +348,46 @@ public sealed class NodeManagerStage6EnforcementTests : IDisposable
finally { await host.DisposeAsync(); }
}
/// <summary>
/// End to end: a session holding one narrow Browse grant can browse the shared root folder both
/// realm trees hang under — the node every client's first browse lands on — and the reference
/// filter still withholds the children it was not granted.
/// </summary>
/// <remarks>
/// <para>
/// The anchor is minted by <c>CreateAddressSpace</c> as <c>s=OtOpcUa</c> in the raw namespace,
/// and <c>ResolveParentFolder</c> parents every top-level folder and every top-level UNS area
/// under it. The Browse override refuses the WHOLE call when its SOURCE node is denied, so
/// until the gate mapped this identifier onto the trie's realm-root scope, every grant narrower
/// than the cluster was unreachable by navigation — this call threw
/// <c>BadUserAccessDenied</c> outright rather than returning a filtered page.
/// </para>
/// <para>
/// The second half is the non-vacuity: "browse succeeded" alone would also be satisfied by a
/// gate that had stopped filtering, so the denied sibling must still be absent from the page.
/// </para>
/// </remarks>
[Fact]
public async Task The_shared_root_folder_is_browsable_on_a_narrow_grant_and_still_filters()
{
var (host, server) = await BootAsync();
try
{
var nm = server.NodeManager!;
var granted = Variable(nm, "GrantedTag");
var denied = Variable(nm, "DeniedTag");
nm.Authorizer = TagBrowseAuthorizer("GrantedTag");
var anchor = new NodeId(V3NodeIds.SharedRootFolderId, granted.NamespaceIndex);
var references = Browse(nm, Identity(), anchor);
references.Select(r => r.NodeId).ShouldContain((ExpandedNodeId)granted);
references.Select(r => r.NodeId).ShouldNotContain((ExpandedNodeId)denied);
}
finally { await host.DisposeAsync(); }
}
/// <summary>
/// <see cref="OtOpcUaNodeManager.TranslateBrowsePath"/> compacts its <c>targetIds</c> with the exact
/// same forward two-pointer algorithm as <see cref="Browse_filtering_preserves_survivor_order"/>