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

fix(opcua): drop the reflective folder-event verdict invalidation; deny the SDK channel under enforcement (#878)
v2-ci / unit-tests (push) Waiting to run
Details

Browse Source 
The SDK memoizes INodeManager3.ValidateEventRolePermissions per (item, event type,
SourceNode). #826 kept it fresh by reflecting into the private
MonitoredNode2.InvalidatePermissionCaches (plus a detach/re-attach fallback). Since #855
condition events never take that channel, and the search for #878 found nothing
production-raised that does except a condition's SDK method audits, which reach it only
with auditing on, SignAndEncrypt and a SecurityAdmin session (nothing grants that role).

Under enforcement the channel verdict is now a flat deny for items on our nodes, so no
cached verdict depends on a grant, and InvalidateFolderEventVerdicts, the reflection,
the fallback, its counter and the guard test are removed. Method audits still reach
Server-object items, decided per event.

Tests: FolderEventVerdictInvalidationTests -> FolderEventVerdictFreshnessTests; new
revoked-grant test (positive control: a still-granted session) and method-audit test
(SecurityAdmin mapped for the test; positive controls: Server-object item, flag off);
the plain-event test runs each mode on a fresh server. Mutation-checked.

Claude-Session: https://claude.ai/code/session_01GT67ACmpMR7b9LGRPLijMi
This commit is contained in:
Joseph Doherty
2026-11-12 11:38:11 -05:00
parent 3a312ffdd1
commit fa811feb3d
5 changed files with 414 additions and 394 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs
+49 -130
View File
@@ -43,8 +43,9 @@ namespace ZB.MOM.WW.OtOpcUa.OpcUaServer;
/// </summary>
// INodeManager3 is RE-IMPLEMENTED (Gitea #826): the SDK's per-folder event delivery (MonitoredNode2) asks
// INodeManager3.ValidateEventRolePermissions for every event and item, and the base implementation is not virtual, so
// re-implementing the interface is the only way to put the per-event ACL decision on that path. Every other member
// maps to the inherited implementation exactly as before.
// re-implementing the interface is the only way to put an ACL decision on that path. Since #855 condition events no
// longer take it, and since #878 the decision there is a flat deny under enforcement (the SDK memoizes it, so it must
// not depend on grants). Every other member maps to the inherited implementation exactly as before.
public sealed class OtOpcUaNodeManager : CustomNodeManager2, INodeManager3
{
/// <summary>v3 dual-namespace: both the Raw (device tree) and UNS (equipment tree) namespaces are
@@ -632,7 +633,6 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2, INodeManager3
// walking its own dead item map on each deploy.
previous.Changed -= OnAuthorityChanged;
next.Changed += OnAuthorityChanged;
InvalidateFolderEventVerdicts();
}
}
@@ -713,10 +713,7 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2, INodeManager3
// there is no second wiring that could silently not run (an unattached resolver splits opaque ids,
// which only ever DENIES narrow grants, so a miss would read as working enforcement).
if (value is not null) value.UnsSubjectResolver = ResolveUnsAclSubject;
var wasEnforced = AclEnforced;
_authorizer = value;
// Across an on/off transition the cached verdicts were decided under the other mode.
InvalidateFolderEventVerdicts(force: wasEnforced || AclEnforced);
}
}
@@ -1513,7 +1510,7 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2, INodeManager3
/// <c>MonitoredNode2</c> whose delivery task drains a BOUNDED channel (4096, <c>FullMode = Wait</c>), and
/// the synchronous <c>ReportEvent</c> BLOCKS when that channel is full. Condition events are reported
/// under <c>Lock</c>, and the delivery task takes <c>Lock</c> on every per-event verdict-cache miss (the
/// SDK's own <c>GetNodeMetadata</c> — flag off too — and #826's <see cref="IsNotifierEventVisibleTo"/>). So
/// SDK's own <c>GetNodeMetadata</c> — flag off too — and, then, #826's per-event check). So
/// a burst of more than 4096 events under one hold of <c>Lock</c>, starting at a miss, deadlocked the
/// node manager (<c>FolderEventFloodWireTests</c> stalled at exactly 4097 events before this).
/// </para>
@@ -1839,9 +1836,9 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2, INodeManager3
/// <param name="e">The condition's snapshot.</param>
/// <remarks>
/// <para>
/// Mirrors the SDK's <c>MonitoredNode2.ProcessEventForItemAsync</c> — its audit guard, then the verdict through
/// <see cref="INodeManager3.ValidateEventRolePermissions"/> (the SDK's type + SourceNode <c>ReceiveEvents</c>
/// check and, under enforcement, #826's live-notifier + grant check), then <c>QueueEvent</c> — but per event:
/// Mirrors the SDK's <c>MonitoredNode2.ProcessEventForItemAsync</c> — its audit guard, then the SDK's type +
/// SourceNode <c>ReceiveEvents</c> check (the base <c>ValidateEventRolePermissions</c>), then, under
/// enforcement, #826's live-notifier + grant check, then <c>QueueEvent</c> — but per event:
/// nothing is memoised, so a deploy, revocation or reference removal decides the very next event. The item's
/// gate and refresh cut apply exactly as on the Server object (<see cref="QueueConditionEventToItem"/>).
/// </para>
@@ -4710,10 +4707,6 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2, INodeManager3
/// <inheritdoc cref="IOpcUaAddressSpaceSink.CompleteAddressSpaceApply"/>
public void CompleteAddressSpaceApply()
{
// Gitea #826: the apply moved conditions, wiring and folders, and the SDK memoizes each folder item's per-event
// verdict per (item, event type, condition) until told otherwise — forget them before the restored reports
// below go out, so the next event on every folder item is decided against the address space as applied.
InvalidateFolderEventVerdicts();
int reported = 0, discarded;
lock (Lock)
{
@@ -6417,10 +6410,10 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2, INodeManager3
/// <summary>
/// The per-event decision for an event item on one of this manager's own notifiers (an equipment folder, a raw
/// device folder) — the SDK fan-out path the Server-object filtering (#586) does not reach (Gitea #826).
/// device folder) — the folder twin of the Server-object filtering (#586), Gitea #826.
/// </summary>
/// <param name="item">The event item; its manager handle names the notifier it is attached to.</param>
/// <param name="e">The event as the SDK delivers it (a condition snapshot, or a cloned audit event).</param>
/// <param name="e">The event (a condition snapshot or the condition itself).</param>
/// <returns><see langword="true"/> when the item's session may receive it through that notifier.</returns>
/// <remarks>
/// <para>
@@ -6432,24 +6425,18 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2, INodeManager3
/// as it is from the Server object.
/// </para>
/// <para>
/// An event that names no live condition of this manager is withheld (fail closed): only condition events
/// and the condition's own method audits reach these notifiers.
/// An event that names no live condition of this manager is withheld (fail closed).
/// </para>
/// <para>
/// ⚠️ <b>Decided on the live instance, never on the event's own (review of #826).</b> The SDK caches the
/// verdict per <c>(item, event type, SourceNode)</c> — no instance identity — so a verdict must be a
/// function of the condition's NodeId and the current state alone. Denying an event because it came from a
/// REPLACED instance (a surgical remove + re-materialise under the same NodeId, the old instance's last
/// event still in the SDK's queue) cached a deny the new instance then inherited until the next
/// invalidation. A late event from a replaced instance is therefore delivered exactly when the live one's
/// would be.
/// <b>Decided on the live instance, never on the event's own (review of #826).</b> A late event from a
/// REPLACED instance (a surgical remove + re-materialise under the same NodeId) is delivered exactly when the
/// live one's would be. (When the SDK still cached this verdict, denying the replaced instance's event left a
/// deny the new instance inherited; nothing caches it now — see #855 and #878.)
/// </para>
/// <para>
/// Runs per event, uncached, from the per-item folder delivery (#855) — on the reporter's thread under
/// <c>Lock</c>, or off it for a deferred event (before the item gate is taken) — and, for the events that still
/// travel the SDK channel (a condition's method audits), on the SDK's per-node delivery task, where the
/// inherited role-permission check already takes <c>Lock</c> (via <c>GetNodeMetadata</c>); the scope
/// resolution here takes it the same way.
/// Asked per event, uncached, by ConditionRefresh on a folder item (<see cref="RefreshDirectItems"/>); live
/// delivery (<see cref="DeliverToNotifierItems"/>) resolves the same three conditions once per hop. Never on
/// the SDK's channel: that verdict is the flat deny of <see cref="INodeManager3.ValidateEventRolePermissions"/>.
/// </para>
/// </remarks>
private bool IsNotifierEventVisibleTo(IEventMonitoredItem item, IFilterTarget e)
@@ -6506,121 +6493,57 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2, INodeManager3
}
/// <summary>
/// Validates whether an event item may receive an event: the SDK's role-permission check, and — with enforcement
/// on, for an item on one of this manager's own nodes — <see cref="IsNotifierEventVisibleTo"/> (Gitea #826).
/// Validates whether an event item may receive an event through the SDK's own per-notifier channel: the SDK's
/// role-permission check, and — with enforcement on, for an item on one of this manager's own nodes — a flat
/// deny (Gitea #826, #878).
/// </summary>
/// <param name="monitoredItem">The event item.</param>
/// <param name="filterTarget">The event.</param>
/// <returns>Good to deliver; otherwise the veto.</returns>
/// <remarks>
/// <para>
/// Condition events to an item attached to one of this manager's notifiers are decided here per event and per
/// item by <see cref="DeliverToNotifierItems"/> (#855, uncached); the SDK's <c>MonitoredNode2</c> still asks it
/// for what travels its channel (a condition's method audit events). The Server-object path (#586) and
/// ConditionRefresh decide the same question themselves, so a folder's subscribers stop receiving a moved
/// alarm (or a native alarm whose reference was dropped) the moment the generation says so, not when the
/// rebuild lands.
/// <b>Only the SDK channel asks this.</b> In SDK 2.0.1 the sole caller of the re-implemented
/// <c>INodeManager3.ValidateEventRolePermissions</c> is <c>MonitoredNode2.GetOrAddEventPermissionAsync</c>
/// (through <c>AsyncNodeManagerAdapter</c>); the base <c>ConditionRefresh</c> uses the non-virtual base
/// method, and this manager's own deliveries — <see cref="DeliverToNotifierItems"/> (#855), the Server-object
/// path (#586), <see cref="RefreshDirectItems"/> — decide per event themselves, uncached. Condition events at
/// our notifiers never travel the channel (#855). What still can: a condition's SDK-raised Part 9 method
/// audit events (with auditing on, over SignAndEncrypt, to a session the SDK's type check admits — only
/// <c>SecurityAdmin</c>), and any other event a <c>NodeState.ReportEvent</c> on one of our folders would
/// raise (none is raised).
/// </para>
/// <para>
/// ⚠️ On the SDK channel path the SDK MEMOIZES the verdict per <c>(item, event type, SourceNode)</c> until the monitored node's
/// permission generation moves, so <see cref="InvalidateFolderEventVerdicts"/> must run whenever the inputs
/// change: a new authority generation or authorizer, the revalidation tick, and the end of every apply. A
/// condition's SourceNode is the condition itself, so a verdict is never shared between two conditions.
/// ⚠️ <b>The SDK MEMOIZES this verdict</b> per <c>(item, event type, SourceNode)</c> until the monitored
/// node's permission generation moves (an item is added or removed, a rebind, a namespace default-permission
/// change) — not when a deployment, a grant or an alarm's notifier wiring changes. A grant-dependent verdict
/// here would therefore go stale, and keeping it fresh needed the SDK's PRIVATE
/// <c>MonitoredNode2.InvalidatePermissionCaches</c> by reflection (#826). So under enforcement the verdict is
/// a constant — every such event is withheld from items on our nodes — and nothing has to be invalidated:
/// a cached deny can never hide an event a later grant would admit, because no grant admits one, and no
/// allow is ever cached to survive a revoked grant. A condition's method audits still reach Server-object
/// items, decided per event (<see cref="OnConditionReportEvent"/>). Withholding them from folder items is
/// stricter than Part 3 (type + SourceNode <c>ReceiveEvents</c>), which it permits.
/// </para>
/// <para>
/// <b>Residual, by construction unreachable today:</b> a verdict the SDK cached while enforcement was OFF
/// survives the switch to ON. The switch happens once, when the host wires the authorizer just after the
/// server starts listening, and the only cacheable allow is a method audit to a <c>SecurityAdmin</c> session:
/// that needs <c>ServerConfiguration.AuditingEnabled</c> (never set here; reachable only through an
/// <c>OpcUa:ApplicationConfigPath</c> XML file) AND a role the SDK's default <c>RoleManager</c> maps to no
/// identity, which this server neither configures nor lets a session add (the RoleSet methods themselves
/// require <c>SecurityAdmin</c>). The evidence and the removed reflection are recorded on Gitea #878.
/// </para>
/// </remarks>
ServiceResult INodeManager3.ValidateEventRolePermissions(IEventMonitoredItem monitoredItem, IFilterTarget filterTarget)
{
var result = ValidateEventRolePermissions(monitoredItem, filterTarget);
if (ServiceResult.IsBad(result) || !AclEnforced || !OwnsNamespace(monitoredItem.NodeId)) return result;
return IsNotifierEventVisibleTo(monitoredItem, filterTarget) ? result : FolderEventDenied;
return FolderEventDenied;
}
/// <summary>The verdict a withheld folder event carries back to the SDK (never reaches the client).</summary>
private static readonly ServiceResult FolderEventDenied = new(StatusCodes.BadUserAccessDenied);
/// <summary>
/// Drops every per-event verdict the SDK has memoized for event items on this manager's notifiers (Gitea #826),
/// so the next event on each is decided afresh by <see cref="IsNotifierEventVisibleTo"/>.
/// </summary>
/// <param name="force">Run even while enforcement is off — the authorizer setter's on/off transition, where the
/// cached verdicts were decided under the other mode.</param>
/// <remarks>
/// <para>
/// <b>Node-wide, session-agnostic.</b> Each notifier's cache is cleared whole through the SDK's own private
/// <c>MonitoredNode2.InvalidatePermissionCaches</c> (what the SDK runs when namespace default permissions
/// change, and on <c>Rebind</c>): it advances the node's permission generation — a verdict computed
/// concurrently under the old one is recomputed, never stored — and clears the cache in one O(cache) pass.
/// The public <c>InvalidatePermissionCacheForSession</c> is NOT sufficient (review of #826): it reaches only
/// items whose <c>Session</c> is set, and an ORPHANED subscription's items (session closed without deleting
/// its subscriptions) have none — they kept a stale allow through a deploy, queued the moved alarm's events
/// and delivered them on <c>TransferSubscriptions</c>. It also rescanned the whole cache once per item
/// (items² × conditions per folder).
/// </para>
/// <para>
/// The private method is resolved once by reflection against the pinned SDK and pinned by
/// <c>FolderEventVerdictInvalidationGuardTests</c>. Should a future SDK drop it, this falls back to the
/// supported route: the per-session drop for items with a session, and a detach + re-attach of each
/// session-less item on its node (<c>Remove</c> drops that item's entries), which can miss an event racing
/// the swap — the reason it is only the fallback.
/// </para>
/// <para>
/// Skipped while enforcement is off (<paramref name="force"/> aside): the verdicts are then the SDK's own
/// role check, which nothing here changes. The node list is copied under <c>Lock</c>; the invalidation
/// takes no lock of ours.
/// </para>
/// </remarks>
private void InvalidateFolderEventVerdicts(bool force = false)
{
if (_root is null || (!force && !AclEnforced)) return;
List<MonitoredNode2> nodes;
lock (Lock) nodes = MonitoredNodes.Values.Where(n => n.EventMonitoredItems is { IsEmpty: false }).ToList();
foreach (var node in nodes)
{
Interlocked.Increment(ref _folderEventVerdictInvalidations);
if (InvalidateNodePermissionCaches is { } invalidate)
{
invalidate(node);
continue;
}
var sessions = new HashSet<NodeId>();
var sessionless = new List<IEventMonitoredItem>();
foreach (var item in node.EventMonitoredItems.Values)
{
if (item is null) continue;
if (item.Session?.Id is { IsNull: false } id) sessions.Add(id);
else sessionless.Add(item);
}
foreach (var session in sessions) node.InvalidatePermissionCacheForSession(session);
foreach (var item in sessionless)
{
node.Remove(item);
node.Add(item);
}
}
}
/// <summary>Notifier caches <see cref="InvalidateFolderEventVerdicts"/> has invalidated since start. Test
/// observability (it must not run while enforcement is off).</summary>
internal long FolderEventVerdictInvalidations => Interlocked.Read(ref _folderEventVerdictInvalidations);
private long _folderEventVerdictInvalidations;
/// <summary>
/// The SDK's private node-wide cache invalidation (<c>MonitoredNode2.InvalidatePermissionCaches</c>), or null
/// when the SDK no longer has it — see <see cref="InvalidateFolderEventVerdicts"/>.
/// </summary>
internal static readonly Action<MonitoredNode2>? InvalidateNodePermissionCaches =
typeof(MonitoredNode2).GetMethod(
"InvalidatePermissionCaches",
System.Reflection.BindingFlags.Instance | System.Reflection.BindingFlags.NonPublic,
Type.EmptyTypes) is { ReturnType: var returns } method && returns == typeof(void)
? method.CreateDelegate<Action<MonitoredNode2>>()
: null;
/// <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>
@@ -7311,10 +7234,6 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2, INodeManager3
public void RevalidateMonitoredItems()
{
if (!AclEnforced) return;
// Gitea #826: both triggers (a deploy, the clock) also reach the per-event decision on folder event items, whose
// verdicts the SDK memoizes — a deploy that moves an alarm must not keep serving its old folder's allow.
InvalidateFolderEventVerdicts();
if (_liveItems.IsEmpty && _eventItems.IsEmpty) return;
var now = UtcNow;