Per-folder event ACL (#826): residual risks to watch #838
Assignees
dohertj2 (dohertj2)
Clear assignees
dohertj2 (dohertj2)
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: dohertj2/lmxopcua#838
Reference in New Issue
Block a user
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?
From #826 and its review.
MonitoredNode2.InvalidatePermissionCachesby reflection (a guard test fails if an SDK upgrade removes it). The reflection-free fallback detaches/re-attaches session-less items and can lose an event raised during that window. Re-check on every SDK bump; ask upstream for a public API.Progress (Pushed to master in
d1b6cad2.): the event-queue overflow risk was REAL — >4096 condition events under one node-manager Lock hold froze the node manager (predates #826). Fixed in91e28e87+7482a888: overflow defers per condition (newest kept; caps 1024/condition, 65,536 total; liveness re-checked so a removed condition's queued events are never delivered; metersotopcua.opcua.condition_events.deferred|discarded; Warning on entering deferral). SDK 2.0.1 decompiled: notifier list is snapshotted, disposed MonitoredNode2 completes its channel. Part 3 conclusion (docs/security.md): non-condition events are authorised by event type + source node; the folder permission only governs attaching. Keeping open for the reflection item. ConditionRefresh ordering: #854.Item 2 done (
4bb9c453, follow-on284358a2), Pushed to master ine857135e.EventQueueOverflowAclWireTestsfloods >4096 condition events at a folder and a Server-object subscription with ACL enforcement on and a non-publishing API-key client, while another session reads underLock: no deadlock, reads stay responsive, held-back queue capped at 2×(1024+1), overflow follows the SDK rule (oldest dropped, oneEventQueueOverflowEventTypeahead of survivors), and no denied event leaks — including a grant revoked while 6000 events are held back. No defect found. Item 3: SDK 2.0.1ValidateEventReceivePermissionschecks ReceiveEvents on the EventType then the SourceNode (Part 3 §8.55); our extra folder decision is stricter than the spec, which it permits, and our audit/model-change events reach Server-object subscribers via their own filter (#588) — no change. Item 1 (private-SDK reflection) stays open as a watch item.Wrap-up of the three residual risks, checked against master at
e5a03e3c(not the issue text).MonitoredNode2.InvalidatePermissionCachesby reflection; reflection-free fallback can lose an eventOtOpcUaNodeManager.cs:6548(InvalidateNodePermissionCaches), used byInvalidateFolderEventVerdicts(~6502).FolderEventVerdictInvalidationTests.The_sdks_node_wide_permission_cache_invalidation_is_still_resolvablefails on an SDK bump that drops it, so the re-check is automatic. Could not be upgraded to (b): I tried to exercise the fallback route (and the orphan test, with invalidation disabled outright, still passes), because since #855 condition events are decided per item uncached and no producible event reaches a folder item through the SDK's cached verdict (method audits are denied by the SDK's type-level ReceiveEvents check). Split out as #878 with the exact action.Lock; 4096-slot SDK queue overflow/back-pressure unexercisedMaxDeferredEventsPerCondition = 1024,MaxDeferredEventsTotal = 65_536,OtOpcUaNodeManager.cs:1415/1422). Pinned byEventQueueOverflowAclWireTests(both the single-hold and per-projection shapes, plusA_grant_revoked_while_overflowed_events_are_deferred_stops_them_all) andFolderEventFloodWireTests.docs/security.md("Which permissions an event on a folder follows"): the SDK checks ReceiveEvents on event type + SourceNode; ours is stricter. Code:IsNotifierEventVisibleTowithholds any event naming no live condition (OtOpcUaNodeManager.cs:6389). That fail-closed line had no test; addedA_non_condition_event_on_a_folder_is_withheld_under_enforcement_and_delivered_without_itine5a03e3c(positive controls: same event delivered flag-off, condition event delivered flag-on). Mutation-checked: changing the line toreturn truemakes it fail; restored.Closing: items 2 and 3 are done; item 1 is a passive watch item (the guard test is the tripwire; upstream public-API request is optional, nothing actionable on our side) and the new finding about its test coverage has its own tracker, #878.
Citation correction for the disposition comment above (master
e5a03e3c): the reflection field is atOtOpcUaNodeManager.cs:6547(not 6548), and its use inInvalidateFolderEventVerdictsis at:6513(not ~6502). The other citations stand.