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

fix(acl): filter Server-object (i=2253) event subscriptions per event through the data-plane ACL (#586)

Browse Source 
Server-object (all-events) items bypassed the ACL entirely: the SDK attached them to every root
notifier's fan-out via SubscribeToAllEvents, and a root notifier with no direct subscriber forwarded
to the core Server object, which this node manager never sees.

- SubscribeToAllEvents override records the item and attaches it to nothing.
- Each condition's own OnReportEvent (fires once per event) queues the event to each recorded item
  whose session holds Subscribe on the condition (RawPath) or any notifier folder it reports through
  -- the per-folder path's question; otherwise mirrors MonitoredNode2.OnReportEvent.
- The root-notifier forward to the core Server object (OnReportEvent override) is dropped.
- ConditionRefresh on an i=2253 item is filtered the same way (once per condition).
- Revalidation is implicit: every event re-asks the authorizer (per-generation cache, key expiry and
  revocation re-checked per call). Inert with Security:Acl:Enabled=false.

Over-the-wire tests: no-grant vs granted (positive control), created-before-deploy, revoke/restore by
deployment, key expiry with no tick, refresh, and flag-off. Falsified: disabling the filter, or
restoring the forward, turns the enforcement tests red.

Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe
This commit is contained in:
Joseph Doherty
2026-10-07 23:37:47 -04:00
parent 35152fcaf7
commit 75e559f6d8
5 changed files with 655 additions and 30 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs
+258 -9
View File
@@ -349,6 +349,7 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2
Volatile.Read(ref _dataPlaneAuthority).Changed -= OnAuthorityChanged;
_liveItems.Clear();
_eventItems.Clear();
_allEventsItems.Clear();
}
base.Dispose(disposing);
@@ -999,9 +1000,10 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2
try
{
// InstanceStateSnapshot is the IFilterTarget — a frozen copy of the condition's fields at fire
// time. ReportEvent walks inverse notifier references up to the root-notifier folder (promoted
// in MaterialiseAlarmCondition), whose OnReportEvent hands off to Server.ReportEvent → the
// event reaches subscribed monitored items.
// time. ReportEvent walks inverse notifier references up to the root-notifier folders (promoted
// in MaterialiseAlarmCondition), whose fan-outs deliver it to direct notifier subscriptions; the
// condition's own OnReportEvent hands the same snapshot to the Server-object (all-events)
// subscriptions, per item, through the ACL (Gitea #586 — see OnConditionReportEvent).
var snapshot = new InstanceStateSnapshot();
snapshot.Initialize(SystemContext, alarm);
alarm.ReportEvent(SystemContext, snapshot);
@@ -1345,10 +1347,16 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2
return HandleAlarmCommand(context, condition, enabling ? "Enable" : "Disable", comment: null, unshelveAt: null);
};
// Gitea #586: the condition itself delivers its events to Server-object (all-events)
// subscriptions, item by item through the ACL. A condition is never an event notifier, so no
// MonitoredNode2 ever claims this single-slot delegate — every event the condition reports, ours
// or the SDK's own method-driven state changes, passes through it exactly once.
alarm.OnReportEvent = OnConditionReportEvent;
parent.AddChild(alarm);
// Promote the equipment folder to an event notifier + register it as a root notifier so
// T16's ReportEvent has a notifier path up to the Server object. Guard so repeated
// T16's ReportEvent reaches the folder's subscribers. Guard so repeated
// materialise under the same folder doesn't double-add the root notifier.
EnsureFolderIsEventNotifier(parent);
@@ -2070,7 +2078,8 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2
/// <summary>Promote <paramref name="folder"/> to <see cref="EventNotifiers.SubscribeToEvents"/> and
/// register it as a root notifier (idempotent — guarded by <see cref="_notifierFolders"/>) so the
/// alarm condition has a notifier path to the Server object for T16's event propagation.</summary>
/// folder is a subscribable event source linked under the Server object (Server-object delivery itself is
/// the condition's own — see OnConditionReportEvent, Gitea #586).</summary>
/// <remarks>
/// Phase C: when a real historian is wired at promotion time (the source is NOT the
/// <see cref="NullHistorianDataSource"/>), the folder ALSO gets the
@@ -3376,14 +3385,254 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2
/// cannot race a client deleting it.
/// </para>
/// <para>
/// ⚠️ <b>Server-object (all-events) subscriptions are not in this map</b>, because they are not
/// authorized by this manager in the first place: the SDK fans them out through
/// <c>SubscribeToAllEvents</c> and through the Server object's own notifier, which the core node
/// manager owns. See <c>docs/security.md</c> § Data-Plane Authorization.
/// <b>Server-object (all-events) subscriptions are not in this map.</b> They are not attached to
/// any notifier of ours at all; they are held in <see cref="_allEventsItems"/> and decided per
/// event at delivery (Gitea #586), so there is nothing for this map's stand-down to detach.
/// </para>
/// </remarks>
private readonly ConcurrentDictionary<IEventMonitoredItem, LiveEventItem> _eventItems = new();
/// <summary>
/// Live Server-object (<c>i=2253</c>) event monitored items — every item the SDK handed this manager
/// through <see cref="SubscribeToAllEvents"/> (Gitea #586).
/// </summary>
/// <remarks>
/// A set (the value is unused). Maintained by <see cref="SubscribeToAllEvents"/>'s subscribe and
/// unsubscribe arms, which the SDK's <c>MasterNodeManager</c> drives for every create, modify,
/// transfer, restore and delete of an all-events item, subscription teardown included.
/// </remarks>
private readonly ConcurrentDictionary<IEventMonitoredItem, byte> _allEventsItems = new();
/// <inheritdoc />
/// <remarks>
/// <para>
/// <b>Gitea #586.</b> The SDK's base implementation attaches a Server-object item to the event
/// fan-out of every one of this manager's root notifiers, so every alarm in the server reached it
/// with no ACL decision at all — and, for an item created before a root notifier existed (the
/// SDK's <c>AddRootNotifier</c> calls its subscribe with <c>unsubscribe: true</c>), through the
/// root notifier's forward to <c>Server.ReportEvent</c> and the core node manager's Server object,
/// which this manager never sees. The Server object is where every condition lands exactly once,
/// so it was the one feed carrying every alarm, ungated.
/// </para>
/// <para>
/// This override attaches the item to nothing. It records it, and each condition hands its events
/// to the recorded items itself (<see cref="OnConditionReportEvent"/>), deciding every item against
/// the authorizer at delivery time. With no attachment there is no fan-out to keep in step with a
/// rebuild, and no stand-down to reverse: revalidation is implicit, because every event re-asks.
/// </para>
/// <para>
/// Creation is never refused: an all-events item is legitimate for a session granted part of the
/// address space, so what is withheld is each event the session may not see, not the item.
/// </para>
/// </remarks>
public override ServiceResult SubscribeToAllEvents(
OperationContext context, uint subscriptionId, IEventMonitoredItem monitoredItem, bool unsubscribe)
{
if (unsubscribe) _allEventsItems.TryRemove(monitoredItem, out _);
else _allEventsItems[monitoredItem] = 0;
return ServiceResult.Good;
}
/// <inheritdoc />
/// <remarks>
/// <para>
/// The base is the delegate <c>AddRootNotifier</c> installs on a root notifier: it forwards every
/// event reaching that folder to <c>Server.ReportEvent</c>, where the core node manager's Server
/// object queues it to every all-events item with no ACL decision — the bypass Gitea #586 closes.
/// Server-object items now receive conditions from the condition itself
/// (<see cref="OnConditionReportEvent"/>), so the forward could only ever deliver an ungated
/// duplicate. Dropped.
/// </para>
/// <para>
/// Only condition events bubble through this manager's notifiers; the audit and model-change
/// events it raises go to <c>Server.ReportEvent</c> directly and are untouched by this.
/// </para>
/// </remarks>
protected override void OnReportEvent(ISystemContext context, NodeState node, IFilterTarget e)
{
}
/// <summary>
/// Delivers one condition event to the Server-object (all-events) subscriptions this manager holds,
/// item by item, through the data-plane ACL (Gitea #586).
/// </summary>
/// <param name="context">The reporting context; a session-bearing one restricts delivery to that session.</param>
/// <param name="node">The condition that reported.</param>
/// <param name="e">The event — the condition's snapshot.</param>
/// <remarks>
/// <para>
/// Installed as each condition's <c>OnReportEvent</c> in <see cref="MaterialiseAlarmCondition"/>,
/// so it sees every event the condition reports exactly once, whichever notifiers it then bubbles
/// through — the one-copy-at-the-Server-object behaviour the multi-notifier wiring relies on.
/// </para>
/// <para>
/// Everything except the ACL decision mirrors the SDK's <c>MonitoredNode2.OnReportEvent</c>, which
/// is what delivered these events before: the SDK's own role-permission check, and the rule that an
/// event reported under a session's context reaches only that session's items.
/// </para>
/// <para>
/// Inert while <c>Security:Acl:Enabled</c> is false — every item receives every condition, as it
/// always has.
/// </para>
/// </remarks>
private void OnConditionReportEvent(ISystemContext context, NodeState node, IFilterTarget e)
{
if (_allEventsItems.IsEmpty) return;
var scopes = AclEnforced ? ConditionVisibilityScopes(node) : null;
var reportingSession = (context as ISessionSystemContext)?.SessionId?.Identifier;
foreach (var item in _allEventsItems.Keys)
{
if (scopes is not null && !IsConditionVisibleTo(item, scopes)) continue;
if (ServiceResult.IsBad(ValidateEventRolePermissions(item, e))) continue;
if (reportingSession is not null
&& item.Session?.Id?.Identifier is { } own
&& !own.Equals(reportingSession))
{
continue;
}
item.QueueEvent(e);
}
}
/// <summary>
/// The nodes whose grant admits a condition to a session: the condition itself, every root-notifier
/// folder its events bubble up through, and every equipment folder wired as its extra notifier.
/// </summary>
/// <param name="condition">The condition.</param>
/// <returns>The candidate scopes; <c>Subscribe</c> on any one of them admits the condition.</returns>
/// <remarks>
/// <para>
/// The folders are exactly the per-folder path's question. A session that may subscribe to a
/// notifier the condition reports through already receives the condition there, so the Server
/// object withholding it would protect nothing; a session granted on none of them receives it
/// nowhere. The folders must be asked as well as the condition because the trie inherits downward
/// only within a realm: an <c>Equipment</c> grant covers the UNS folder a native condition is wired
/// into, but not the condition, which lives at its RawPath in the raw realm.
/// </para>
/// <para>
/// The condition itself (<c>ConditionId</c> = RawPath for a native alarm) admits a grant scoped to
/// the alarm's own raw tag — the tag's grant covers its alarm wherever the alarm can be named.
/// </para>
/// </remarks>
private List<NodeId> ConditionVisibilityScopes(NodeState condition)
{
var scopes = new List<NodeId> { condition.NodeId };
lock (Lock)
{
for (var ancestor = (condition as BaseInstanceState)?.Parent;
ancestor is not null;
ancestor = (ancestor as BaseInstanceState)?.Parent)
{
if (_notifierFolders.TryGetValue(ancestor.NodeId, out var notifier) && ReferenceEquals(notifier, ancestor))
scopes.Add(ancestor.NodeId);
}
if (_alarmNotifierWiring.TryGetValue(MapKey(condition.NodeId), out var wiring)
&& ReferenceEquals(wiring.Alarm, condition))
{
scopes.AddRange(wiring.Folders.Select(f => f.NodeId));
}
}
return scopes;
}
/// <summary>Whether <paramref name="item"/>'s session may see a condition with the given scopes.</summary>
/// <param name="item">The Server-object event item.</param>
/// <param name="scopes">From <see cref="ConditionVisibilityScopes"/>.</param>
/// <returns><see langword="true"/> when the session holds <c>Subscribe</c> on any scope.</returns>
/// <remarks>
/// The identity is the one the item was created under — the thread delivering the event has no
/// session of its own — and the operation is the one the per-folder creation gate asks.
/// <see cref="Security.DataPlaneAuthorizer.IsAllowedCached"/> re-checks key revocation and expiry on
/// every call and keys its decision cache on the current generation, which is what makes revalidation
/// implicit here: a deployment or an expiry takes effect on the next event, with no pass to run.
/// </remarks>
private bool IsConditionVisibleTo(IEventMonitoredItem item, List<NodeId> scopes)
{
var identity = item.EffectiveIdentity as RoleCarryingUserIdentity;
foreach (var scope in scopes)
{
if (_authorizer!.IsAllowedCached(identity, OpcUaOperation.CreateMonitoredItems, scope, RealmOf(scope)))
return true;
}
return false;
}
/// <inheritdoc />
/// <remarks>
/// <para>
/// A refresh on a Server-object item is the other way it learns about conditions — it is how a
/// client connecting after a transition gets the current alarm picture — and the SDK base replays
/// every condition under every root notifier to it after only its own role-permission check. With
/// enforcement on, those items are refreshed here instead, through the same
/// <see cref="IsConditionVisibleTo"/> decision live delivery uses (Gitea #586), and each condition
/// is replayed once rather than once per notifier it is wired to.
/// </para>
/// <para>
/// Items on one of this manager's own notifiers keep the base behaviour: a stood-down item is off
/// its notifier's fan-out, which the base already treats as "not refreshed".
/// </para>
/// </remarks>
public override ServiceResult ConditionRefresh(OperationContext context, IList<IEventMonitoredItem> monitoredItems)
{
if (!AclEnforced) return base.ConditionRefresh(context, monitoredItems);
var direct = new List<IEventMonitoredItem>();
var serverItems = new List<IEventMonitoredItem>();
foreach (var item in monitoredItems)
{
if (item is null) continue;
(item.NodeId == ObjectIds.Server ? serverItems : direct).Add(item);
}
if (direct.Count > 0) base.ConditionRefresh(context, direct);
if (serverItems.Count == 0) return ServiceResult.Good;
var systemContext = SystemContext.Copy(context);
List<NodeState> notifiers;
lock (Lock) notifiers = RootNotifiers?.ToList() ?? [];
var events = new List<IFilterTarget>();
foreach (var notifier in notifiers) notifier.ConditionRefresh(systemContext, events, true);
// One snapshot per condition, with its scopes resolved once.
var conditions = new List<(IFilterTarget Event, List<NodeId> Scopes)>();
var seen = new HashSet<NodeState>(System.Collections.Generic.ReferenceEqualityComparer.Instance);
foreach (var e in events)
{
if (ConditionOf(e) is not { } condition || !seen.Add(condition)) continue;
conditions.Add((e, ConditionVisibilityScopes(condition)));
}
foreach (var item in serverItems)
{
foreach (var (e, scopes) in conditions)
{
if (!IsConditionVisibleTo(item, scopes)) continue;
if (ServiceResult.IsBad(ValidateEventRolePermissions(item, e))) continue;
item.QueueEvent(e);
}
}
return ServiceResult.Good;
}
/// <summary>The condition a refresh event describes, or null when it names none (withheld: fail closed).</summary>
/// <param name="e">A refresh event.</param>
/// <returns>The condition node.</returns>
private static NodeState? ConditionOf(IFilterTarget e) => e switch
{
InstanceStateSnapshot { Handle: NodeState condition } => condition,
NodeState condition => condition,
_ => null,
};
/// <summary>
/// How many event items are attached to the event fan-out of nodes this manager owns. Test
/// observability for Gitea #562: an item resurrected onto a notifier after its client deleted it is