From 89630ee418141aad717d2381b9cfa61411d11824 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Tue, 18 Aug 2026 09:45:04 -0400 Subject: [PATCH] fix(acl): browse transit reaches the shared root folder (narrow Browse grants were unreachable) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With Security:Acl:Enabled=true, a session holding any grant narrower than Cluster could not browse the served address space at all — confirmed by the live gate. Mechanism. Both realm trees hang under ONE anchor folder minted in CreateAddressSpace: NodeId("OtOpcUa", ). ResolveParentFolder parents every top-level raw folder AND every top-level UNS area under it, and the Browse override throws BadUserAccessDenied for the WHOLE call when its source node is denied. DataPlaneAuthorizer.BuildScope split that identifier like a path, producing {Kind: Raw, Segments: ["OtOpcUa"]} — but trie grant paths are ["raw", ] and "OtOpcUa" is never a path segment, so PermissionTrie.HasBrowseTransit failed at the realm-child lookup and CollectMatches yielded only Cluster-scoped grants. Deny, at the front door. The trie already handled a zero-segment scope as "the realm root, itself an ancestor of everything granted in that realm" (PermissionTrie.cs:101-118) — no production path had ever produced that scope. This wires it up. - V3NodeIds.SharedRootFolderId names the anchor identity; CreateAddressSpace now mints the NodeId/BrowseName/DisplayName from it (behaviour-identical, makes the coupling explicit). - BuildScope maps that identifier (realm-qualified to Raw — in the UNS namespace s=OtOpcUa is an ordinary Area) onto the zero-segment Raw scope. - Evaluate adds a second-realm OR-leg for Browse on the anchor only: it fronts both trees, so a principal whose only Browse grant lives in the UNS tree still enters through this raw node. Browse only; transit confers navigation, never read/write/subscribe. - No trie, evaluator or builder change. No reordering above the decision cache — key revocation is still re-checked before the lookup — and the cache key is unchanged (the combined verdict caches under the anchor's own identifier/realm; a generation advance clears it). Falsifications actually run (implement-then-revert-the-line, plus one genuine red-first): - DataPlaneAuthorizerTests: the 7 new facts were written BEFORE the fix and 4 went red (the 3 deny-only ones passed vacuously, as a deny-only test must). - Deleting the UNS OR-leg reddens exactly one fact, The_shared_root_folder_is_browsable_on_a_uns_only_browse_grant (26/27 pass). - Dropping the realm qualification from IsSharedRootFolder reddens exactly A_uns_area_named_like_the_anchor_gets_no_anchor_semantics. Its first draft did NOT catch this — a raw-only grant is refused either way; the discriminating grant had to be an unrelated UNS one, whose own transit marks the zero-segment UNS scope the un-qualified mapping would land on. - Disabling the BuildScope anchor branch reddens the new end-to-end fact NodeManagerStage6EnforcementTests.The_shared_root_folder_is_browsable_on_a_narrow_grant_and_still_filters (real booted server, real Browse dispatch: BadUserAccessDenied on the whole call), which also pins that the reference filter still withholds the non-granted sibling. - Core PermissionTrieBrowseTransitTests gains 3 characterization facts pinning the previously-dead realm-root branch (transit true for the granting realm's root, false for the other realm's, false without the Browse bit). These were green on arrival — the trie was always correct; nothing called it. Tests: Core 351 -> 354 passed; OpcUaServer 482 -> 490 passed. Build clean. --- .../OpcUa/V3NodeIds.cs | 10 ++ .../OtOpcUaNodeManager.cs | 10 +- .../Security/DataPlaneAuthorizer.cs | 54 ++++++- .../PermissionTrieBrowseTransitTests.cs | 48 ++++++ .../DataPlaneAuthorizerTests.cs | 148 ++++++++++++++++++ .../NodeManagerStage6EnforcementTests.cs | 40 +++++ 6 files changed, 306 insertions(+), 4 deletions(-) diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Commons/OpcUa/V3NodeIds.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Commons/OpcUa/V3NodeIds.cs index f6296c7a..73739604 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Commons/OpcUa/V3NodeIds.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Commons/OpcUa/V3NodeIds.cs @@ -28,6 +28,16 @@ public static class V3NodeIds /// The UNS namespace URI (equipment-oriented subtree). public const string UnsNamespaceUri = "https://zb.com/otopcua/uns"; + /// + /// 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. + /// + public const string SharedRootFolderId = "OtOpcUa"; + /// The namespace URI for a realm. /// The address-space realm. /// The realm's namespace URI. diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs index 35db4d74..9a050d3f 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs @@ -4219,11 +4219,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, }; diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/Security/DataPlaneAuthorizer.cs b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/Security/DataPlaneAuthorizer.cs index 68870287..7206a90e 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/Security/DataPlaneAuthorizer.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/Security/DataPlaneAuthorizer.cs @@ -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,23 @@ public sealed class DataPlaneAuthorizer /// string" guard this used to carry moved up into Evaluate, where it has to run before /// the cache lookup anyway — the identifier is the cache key. /// + /// + /// The one identifier that is not a path is the shared root folder + /// (), 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 + /// was written to answer ("is this an ancestor of + /// anything granted in this realm?") and which no production path had ever asked it. + /// + /// + /// Keying that on the identifier introduces no new ambiguity: a genuine top-level raw folder + /// literally named OtOpcUa would already mint the identical NodeId as the anchor in the + /// served address space itself (EnsureFolder 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. + /// /// /// The node's non-empty s= identifier. /// The namespace the node was resolved in. @@ -346,9 +380,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), }; + /// + /// Whether this node is the single shared root folder both realm trees hang under. + /// + /// + /// Realm-qualified on purpose. The anchor is minted in the RAW namespace; in the UNS + /// namespace s=OtOpcUa 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. + /// + /// The node's s= identifier. + /// The namespace the node was resolved in. + /// when the node is the shared anchor folder. + private static bool IsSharedRootFolder(string identifier, AddressSpaceRealm realm) => + realm == AddressSpaceRealm.Raw + && string.Equals(identifier, V3NodeIds.SharedRootFolderId, StringComparison.Ordinal); + /// Denies with a constant reason — nothing to build, so nothing to defer. /// The operation being attempted. /// The target node. diff --git a/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests/Authorization/PermissionTrieBrowseTransitTests.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests/Authorization/PermissionTrieBrowseTransitTests.cs index f42e5592..09af7fc1 100644 --- a/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests/Authorization/PermissionTrieBrowseTransitTests.cs +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests/Authorization/PermissionTrieBrowseTransitTests.cs @@ -148,6 +148,54 @@ public sealed class PermissionTrieBrowseTransitTests evaluator.Authorize(session, OpcUaOperation.Browse, RawScope("Plant", "Siemens")).IsAllowed.ShouldBeFalse(); } + // ───────────────── the realm root (zero-segment scope) ───────────────── + + /// + /// 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. + /// + /// + /// The branch is as old as but had no production + /// caller until the ACL gate learned to map the address space's single shared root folder + /// (s=OtOpcUa, 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. + /// + [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(); + } + + /// + /// 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. + /// + [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(); + } + + /// + /// Only a grant carrying the Browse bit marks the realm root — the same rule that governs every + /// other ancestor, applied at depth zero. + /// + [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) => diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/DataPlaneAuthorizerTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/DataPlaneAuthorizerTests.cs index 405aa61b..4c288257 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/DataPlaneAuthorizerTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/DataPlaneAuthorizerTests.cs @@ -391,8 +391,156 @@ public sealed class DataPlaneAuthorizerTests .Enabled.ShouldBe(enabled); } + // ───────────────────────────── the shared root folder ───────────────────────────── + + /// + /// The load-bearing allow. Both realm trees hang under one anchor folder + /// (s=OtOpcUa, 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 (["OtOpcUa"]) that no + /// grant path can ever contain. + /// + [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(); + } + + /// + /// 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. + /// + /// + /// This is the test that falsifies the second walk: delete the UNS OR-leg from + /// DataPlaneAuthorizer.Evaluate and only this fact goes red, because the first walk asks the + /// raw realm root, which a purely-UNS grant never marks. + /// + [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(); + } + + /// + /// 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. + /// + [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)); + } + + /// + /// Transit is marked only by grants that actually carry Browse. A deep Read-only grant leaves + /// the anchor closed — the node is readable by NodeId, not discoverable by navigating. + /// + [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)); + } + + /// + /// 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. + /// + [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)); + } + + /// + /// 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. + /// + [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)); + } + + /// + /// Anchor semantics are realm-qualified. In the UNS namespace s=OtOpcUa 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. + /// + /// + /// The discriminating grant is a UNS one deliberately. Un-qualifying + /// IsSharedRootFolder 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. + /// + [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 ───────────────────────────── + /// The single shared root folder both realm trees hang under, in the raw namespace. + 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(); diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerStage6EnforcementTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerStage6EnforcementTests.cs index a0b3c57d..6d74b601 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerStage6EnforcementTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerStage6EnforcementTests.cs @@ -348,6 +348,46 @@ public sealed class NodeManagerStage6EnforcementTests : IDisposable finally { await host.DisposeAsync(); } } + /// + /// 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. + /// + /// + /// + /// The anchor is minted by CreateAddressSpace as s=OtOpcUa in the raw namespace, + /// and ResolveParentFolder 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 + /// BadUserAccessDenied outright rather than returning a filtered page. + /// + /// + /// 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. + /// + /// + [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(); } + } + /// /// compacts its targetIds with the exact /// same forward two-pointer algorithm as