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

Narrow UNS grants cannot browse/read/subscribe UNS folders or receive scripted-alarm Server-object events (opaque folder ids split as paths) #808

New Issue
Closed
opened 2026-11-10 21:23:49 -05:00 by dohertj2 · 2 comments
dohertj2 commented 2026-11-10 21:23:49 -05:00
Owner
Copy Link
Copy Source

Found while fixing #797 (by code reading; not live-gated).

AddressSpaceApplier.MaterialiseHierarchy mints UNS Area/Line/Equipment folders under their opaque entity ids (UnsAreaId / UnsLineId / EquipmentId, e.g. EQ-xxxxxxxxxxxx), while AclScopePathResolver plants UnsArea/UnsLine/Equipment grants at NAME paths [uns, Area, Line, Equipment]. DataPlaneAuthorizer.Evaluate/BuildScope splits the NodeId identifier, so with Security:Acl:Enabled:

  • Read / Browse / CreateMonitoredItems on a UNS folder (s=EQ-...) evaluates [uns, EQ-...] and never matches an Equipment/Line/Area grant (only Cluster / Namespace(uns)). Browse transit does not help either: [uns, EQ-...] is not an ancestor of [uns, Area, Line, Equipment].
  • Scripted condition nodes and their children (s=scripted-id/...) are likewise unreachable for Read/Browse/Subscribe by a narrow grant.
  • OtOpcUaNodeManager.IsConditionVisibleTo (Server-object i=2253 delivery, #586) checks the condition's ancestor folder NodeIds, so a session holding only an Equipment grant receives NO scripted-alarm events through the Server object in production (the ServerEventRig fixture hides this: its folder id EqA coincides with its scope path [uns, EqA]).

#797 fixed only the Part 9 command gate for scripted conditions, via an explicit owner binding (live condition -> EquipmentId -> generation's EquipmentScopePaths). The same EquipmentId -> path lookup (DataPlaneAuthoritySnapshot.EquipmentScopePaths, extended to areas/lines) is the likely fix for folder ids and for scripted-condition event visibility. Tests must use production-shaped opaque folder ids, not the rig's synthetic EqA.

Found while fixing #797 (by code reading; not live-gated). AddressSpaceApplier.MaterialiseHierarchy mints UNS Area/Line/Equipment folders under their opaque entity ids (UnsAreaId / UnsLineId / EquipmentId, e.g. EQ-xxxxxxxxxxxx), while AclScopePathResolver plants UnsArea/UnsLine/Equipment grants at NAME paths [uns, Area, Line, Equipment]. DataPlaneAuthorizer.Evaluate/BuildScope splits the NodeId identifier, so with Security:Acl:Enabled: - Read / Browse / CreateMonitoredItems on a UNS folder (s=EQ-...) evaluates [uns, EQ-...] and never matches an Equipment/Line/Area grant (only Cluster / Namespace(uns)). Browse transit does not help either: [uns, EQ-...] is not an ancestor of [uns, Area, Line, Equipment]. - Scripted condition nodes and their children (s=scripted-id/...) are likewise unreachable for Read/Browse/Subscribe by a narrow grant. - OtOpcUaNodeManager.IsConditionVisibleTo (Server-object i=2253 delivery, #586) checks the condition's ancestor folder NodeIds, so a session holding only an Equipment grant receives NO scripted-alarm events through the Server object in production (the ServerEventRig fixture hides this: its folder id EqA coincides with its scope path [uns, EqA]). #797 fixed only the Part 9 command gate for scripted conditions, via an explicit owner binding (live condition -> EquipmentId -> generation's EquipmentScopePaths). The same EquipmentId -> path lookup (DataPlaneAuthoritySnapshot.EquipmentScopePaths, extended to areas/lines) is the likely fix for folder ids and for scripted-condition event visibility. Tests must use production-shaped opaque folder ids, not the rig's synthetic EqA.
dohertj2 added the type:bugpriority:p2 labels 2026-11-10 21:23:49 -05:00
dohertj2 commented 2026-11-10 21:33:25 -05:00
Author
Owner
Copy Link
Copy Source

Confirmed by over-the-wire probe during the #797 review (commit 3d1b86b4, production-shaped UA-/UL-/EQ- opaque folders, condition s=scripted-eqa-hi, session with only an Equipment grant on its equipment):

  • Server-object event subscription: received nothing (a Cluster-grant observer did see the event).
  • Read ns=3;s=scripted-eqa-hi/EventId → BadUserAccessDenied.
  • Read the session's own equipment folder EQ-… → BadUserAccessDenied.

Consequence: #797's command-gate fix is necessary but not sufficient end-to-end. Acknowledge/Confirm/AddComment need the current EventId (a fabricated one returns BadEventIdUnknown), so a narrow principal still cannot ack its own scripted alarms in production. Shelve/Unshelve/Enable/Disable (no EventId) do work. The #797 wire tests obtain the EventId through a Cluster-grant observer session, so they do not cover this path. Also: the comment near OtOpcUaNodeManager.cs ~5270 ('an Equipment grant covers the UNS folder a native condition is wired into') is false in production for the same reason. And an Area whose name equals a scripted alarm id still gains Read/Subscribe on that condition (commands are now closed).

Confirmed by over-the-wire probe during the #797 review (commit 3d1b86b4, production-shaped UA-/UL-/EQ- opaque folders, condition s=scripted-eqa-hi, session with only an Equipment grant on its equipment): - Server-object event subscription: received nothing (a Cluster-grant observer did see the event). - Read ns=3;s=scripted-eqa-hi/EventId → BadUserAccessDenied. - Read the session's own equipment folder EQ-… → BadUserAccessDenied. Consequence: #797's command-gate fix is necessary but not sufficient end-to-end. Acknowledge/Confirm/AddComment need the current EventId (a fabricated one returns BadEventIdUnknown), so a narrow principal still cannot ack its own scripted alarms in production. Shelve/Unshelve/Enable/Disable (no EventId) do work. The #797 wire tests obtain the EventId through a Cluster-grant observer session, so they do not cover this path. Also: the comment near OtOpcUaNodeManager.cs ~5270 ('an Equipment grant covers the UNS folder a native condition is wired into') is false in production for the same reason. And an Area whose name equals a scripted alarm id still gains Read/Subscribe on that condition (commands are now closed).
dohertj2 commented 2026-11-11 00:49:01 -05:00
Author
Owner
Copy Link
Copy Source

Fixed on master (merge b89f4580; commits af5c9744 + e39a1250, pushed in 5bbe1e3c).

The authority generation now carries UnsFolderScopes (folder id → name path + folder-id chain, built by DeploymentArtifact.BuildUnsFolderScopes with the same resolver that places grants). OtOpcUaNodeManager.ResolveUnsAclSubject reports each UNS node's live placement, and DataPlaneAuthorizer.Evaluate places it against the generation before the decision cache. Covers UNS folders, virtual tags (s=EQ-…/Name, which were broken too), scripted conditions + children, reference variables, HistoryRead, folder event items, i=2253 delivery, Call and model-change filtering. Fail-closed: live ancestry must match the generation (a moved equipment/alarm is denied under old and new line grants until the rebuild); NUL ids rejected before the cache; unknown/removed opaque ids are decided only by realm-wide grants. Independent review found 4 gaps in the first commit, all fixed in e39a1250.

Tests: new UnsFolderScopeWireTests (opaque UA-/UL-/EQ- ids, incl. one artifact-driven test), each guard shown to be load-bearing. OpcUaServer.Tests 1141/1142 (1 skip), IntegrationTests 218/222 (4 skip), Runtime.Tests 1007/1010, Core.Tests 362.

Live gate on docker-dev (b89f4580, Security:Acl on): PASS on all steps. A key with only an Equipment grant on eqa browsed only its own subtree, read its folder/alarm/vt/reference (sibling eqb: BadUserAccessDenied), received only eqa's alarm on i=2253 (sibling's fired first as a control, confirmed by a Cluster key), and acknowledged/confirmed with the received EventId (/alerts showed the ack by the narrow key); acking the sibling was denied. Moving eqa to a new line: the old-line grant lost access, the new-line grant gained it.

ScadaBridge impact: no NodeId/wire change; with ACLs on, Area/Line/Equipment-scoped keys can now browse/read/subscribe/HistoryRead their subtree and ack/confirm/comment scripted and native alarms — a Cluster grant is no longer needed to ack. Narrow grantees no longer receive NodeDeleted model-change events for removed UNS folders/VTs/conditions.

Residuals: #826 (per-folder event items during the stale window).

Fixed on master (merge b89f4580; commits af5c9744 + e39a1250, pushed in 5bbe1e3c). The authority generation now carries `UnsFolderScopes` (folder id → name path + folder-id chain, built by `DeploymentArtifact.BuildUnsFolderScopes` with the same resolver that places grants). `OtOpcUaNodeManager.ResolveUnsAclSubject` reports each UNS node's live placement, and `DataPlaneAuthorizer.Evaluate` places it against the generation before the decision cache. Covers UNS folders, virtual tags (`s=EQ-…/Name`, which were broken too), scripted conditions + children, reference variables, HistoryRead, folder event items, i=2253 delivery, Call and model-change filtering. Fail-closed: live ancestry must match the generation (a moved equipment/alarm is denied under old and new line grants until the rebuild); NUL ids rejected before the cache; unknown/removed opaque ids are decided only by realm-wide grants. Independent review found 4 gaps in the first commit, all fixed in e39a1250. Tests: new `UnsFolderScopeWireTests` (opaque UA-/UL-/EQ- ids, incl. one artifact-driven test), each guard shown to be load-bearing. OpcUaServer.Tests 1141/1142 (1 skip), IntegrationTests 218/222 (4 skip), Runtime.Tests 1007/1010, Core.Tests 362. **Live gate on docker-dev (b89f4580, Security:Acl on): PASS on all steps.** A key with only an Equipment grant on `eqa` browsed only its own subtree, read its folder/alarm/vt/reference (sibling `eqb`: BadUserAccessDenied), received only `eqa`'s alarm on i=2253 (sibling's fired first as a control, confirmed by a Cluster key), and acknowledged/confirmed with the received EventId (`/alerts` showed the ack by the narrow key); acking the sibling was denied. Moving `eqa` to a new line: the old-line grant lost access, the new-line grant gained it. **ScadaBridge impact:** no NodeId/wire change; with ACLs on, Area/Line/Equipment-scoped keys can now browse/read/subscribe/HistoryRead their subtree and ack/confirm/comment scripted and native alarms — a Cluster grant is no longer needed to ack. Narrow grantees no longer receive NodeDeleted model-change events for removed UNS folders/VTs/conditions. Residuals: #826 (per-folder event items during the stale window).
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
auto-managed
queue management
 blocked
Blocked by dependency or external hold
 cross-driver
queue management
 cross-repo
Needs coordinated change in another family repo (wire contract etc.)
 driver/abcip
queue management
 driver/ablegacy
queue management
 driver/focas
queue management
 driver/mqtt
MQTT / Sparkplug B broker-subscribe driver
 driver/opcuaclient
queue management
 driver/s7
queue management
 driver/twincat
queue management
 migrated
Migrated from a markdown backlog (2026-11 standardization)
 phase/1
queue management
 phase/2
queue management
 phase/3
queue management
 phase/4
queue management
 phase/5
queue management
 phase/6
queue management
 priority:p0
Must-have / blocking
 priority:p1
High priority
 priority:p2
Normal priority
 priority:p3
Low priority / nice to have
 queue/blocked
queue management
 queue/done
queue management
 queue/failed
queue management
 queue/in-progress
queue management
 queue/queued
queue management
 type:bug
Defect in shipped behaviour
 type:debt
Technical debt / hygiene
 type:deploy-gate
Rollout/deploy/migration step not yet done on an environment
 type:docs
Documentation work
 type:feature
Feature implementation
 type:followup
Follow-up deferred from a plan, PR or gate
 type:infra
Infrastructure and scaffolding
 type:review-finding
Unfixed finding from an architecture/code review
 type:test
Test work
No Label priority:p2 type:bug
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
dohertj2 (dohertj2)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: dohertj2/lmxopcua#808
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?