From 2b945672e9e87ccc46f17db2a21784d4de2dfe33 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Wed, 7 Oct 2026 23:10:35 -0400 Subject: [PATCH] fix(acl): revalidate event/alarm subscriptions on both triggers; key-usability check on role gates (#562, #563) #562: direct event (alarm) monitored items are now tracked in a second map (_eventItems, fed by the SubscribeToEvents override) and re-authorized by RevalidateMonitoredItems on both triggers (DataPlaneAuthorityState.Changed and AclRevalidationTimer). A denied item is detached via the SDK's own protected SubscribeToEvents(unsubscribe: true) - never MonitoringMode.Disabled - and re-attached on restore; a client delete of a stood-down item is absorbed. Over-the-wire tests: revoke -> stop (positive control on a still-granted notifier) -> restore -> resume; clock-driven expiry via the real timer with a control session; delete-while-stood-down is not resurrected; unexpired negative control. Server-object (all-events) subscriptions are documented as outside the ACL (observed over the wire). #563: DataPlaneAuthorizer.AuthorizeKeyUsable runs the FindUsableKey probe on the WriteOperate and AlarmAck role gates regardless of Security:Acl:Enabled, so an expired or deployment-disabled key loses its activation-time roles on its open session. Anonymous (no key id) is unaffected; reads/subscriptions stay ungated with the flag off. No new flag. Docs: docs/security.md and the CLAUDE.md API-key/ACL section. Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe --- CLAUDE.md | 23 +- docs/security.md | 48 +- .../OtOpcUaNodeManager.cs | 192 +++++- .../Security/DataPlaneAuthorizer.cs | 38 ++ .../ZB.MOM.WW.OtOpcUa.OpcUaServer.csproj | 3 + .../AclEventSubscriptionEnforcementTests.cs | 549 ++++++++++++++++++ .../NodeManagerKeyUsabilityGateTests.cs | 297 ++++++++++ 7 files changed, 1141 insertions(+), 9 deletions(-) create mode 100644 tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/AclEventSubscriptionEnforcementTests.cs create mode 100644 tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerKeyUsabilityGateTests.cs diff --git a/CLAUDE.md b/CLAUDE.md index 65a02ec7..d9bb3c4a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -859,6 +859,15 @@ Two things to know before relying on any of this: browse, subscribe and HistoryRead are ungated for any admitted session including Anonymous**. With the flag **on** (`DataPlaneAuthorizer`), scoped grants decide **every** data-plane operation: write and alarm commands and HistoryRead (stage 5), read and browse and subscription creation (stage 6). + - **The two role gates re-check the session's KEY on every call, flag off or on (Gitea #563).** Roles + are resolved once at activation, so an expired / deployment-disabled key used to keep `WriteOperate` / + `AlarmAck` on its open session until the client disconnected. `DataPlaneAuthorizer.AuthorizeKeyUsable` + runs the same `FindUsableKey` probe against the same snapshot + clock as the ACL path (one dictionary + lookup, no I/O) and denies `BadUserAccessDenied` before the role check. **Only those two gates** — + reads/subscriptions stay ungated with the flag off, by design. Anonymous carries no key and is + unaffected. No flag: a dead key keeping write authority is a defect, not a rollout choice. ⚠️ On a + fused node a key minted after the last deploy authenticates (ConfigDb) but cannot write/ack until a + deployment carries it — the flag-on path always behaved this way. - ⚠️ **The ACL decision REPLACES the role check — the two are never chained.** Chained, a grant would have to confer the server-wide role *as well as* pass the scope check, and only `Cluster`-scoped grants confer a role, so every narrow grant would stay inert and the flag would appear to do @@ -910,6 +919,15 @@ Two things to know before relying on any of this: `Security:Acl:Enabled` is false), runs the same pass on the clock — because `ExpiresUtc` passes with the clock, not with a deployment. The pass's verdict memo is **pass-local and must stay that way**; hoisting it across ticks would serve an expiry that landed between two ticks the earlier tick's allow. + **Event/alarm items ride both triggers too (Gitea #562)** — a second map (`_eventItems`), fed by the + `SubscribeToEvents` override, holds every *direct* notifier subscription; a denied one is detached via + the SDK's own protected `SubscribeToEvents(…, unsubscribe: true)` (what a client delete does) and + re-attached with `unsubscribe: false` on restore — again never `Disabled`. Unlike a data item it is + **not told** (no value slot for a status; the feed goes quiet), and a client delete of a stood-down + item is absorbed by the override (the SDK would otherwise remove an item that is not attached). + ⚠️ **Server-object (`i=2253`) event subscriptions are outside the ACL entirely** — fanned out by + `SubscribeToAllEvents` + the core node manager's Server notifier, which our creation gate never sees; + live-observed: a grantless anonymous session receives every alarm through one. Open residual. - **Anonymous is a normal grantable principal**, not a bypass. `OpcUa:AllowAnonymous` (default `true`, preserving today's behaviour) controls *admission*; when admitted the session authenticates as the reserved principal `anonymous` and gets roles from `NodeAcl` rows naming it, arriving at the gates as @@ -932,8 +950,9 @@ Two things to know before relying on any of this: **Expiry is the exception**: `FindUsableKey` compares `ExpiresUtc` against the authorizer's clock on every call, so it is immediate everywhere — and since arch-review 03/S19 it reaches live subscriptions too, on the host's `Security:Acl:RevalidationIntervalSeconds` timer (default 60 s; the session is **silenced with - `BadUserAccessDenied`, not closed**, and recovers on re-activation). ⚠️ Event/alarm monitored items are - revalidated by neither trigger — pre-existing, and a named residual. + `BadUserAccessDenied`, not closed**, and recovers on re-activation). Direct event/alarm subscriptions + are revalidated on both triggers as well (#562); with the flag **off**, expiry and revocation reach the + two role gates (#563) but not reads or subscriptions. - ⚠️ **The activation lookup is BOUNDED (arch-review 03/S18).** The SDK raises impersonation synchronously under a `SessionManager`-wide lock, so a ConfigDb that is *slow* rather than *refusing* stalled **every** session activation on the server, Anonymous and X.509 included. `BoundedApiKeyStore` wraps the ConfigDb diff --git a/docs/security.md b/docs/security.md index cb9f8147..56abd2ab 100644 --- a/docs/security.md +++ b/docs/security.md @@ -406,6 +406,27 @@ Both gates fail closed — a session with no identity or no matching role gets the constants in `OpcUaDataPlaneRoles`, derived from the principal's cluster-scoped `NodeAcl` grants (see [Role grant source](#role-grant-source-data-plane)). +**Both gates also re-check the session's API key on every call, flag off or on** (Gitea #563). A +session's roles are resolved once, at activation, so before this an open session kept `WriteOperate` / +`AlarmAck` after its key's `ExpiresUtc` passed — or after a deployment disabled or removed the key — +until the client chose to disconnect, which on a machine-to-machine data plane can be never. Now each +write and each Part 9 method call first asks `DataPlaneAuthorizer.AuthorizeKeyUsable`: the same +`FindUsableKey` probe, against the same deployed snapshot and the same clock, that the ACL path makes +(one dictionary lookup, no I/O). An unusable key gets `BadUserAccessDenied` (logged as +`AclGate: denied … key … is no longer enabled or has expired`), checked *before* the "no gateway ⇒ +`BadNotWritable`" availability answer. Things to know: + +- **Only these two gates.** Read, Browse, Subscribe and HistoryRead stay ungated with the flag off — + that is the flag-off posture, and an expired key's session can still read and keep its subscriptions. + Turn `Security:Acl:Enabled` on to have expiry reach those too. +- **Anonymous is unaffected.** It carries no key id, so there is nothing to expire; its roles stay what + activation resolved from the `anonymous` principal's grants. +- **No flag.** An expired or disabled key keeping write authority is a defect, not a rollout choice. +- ⚠️ **On a fused admin+driver node a key minted after the last deployment authenticates (the ConfigDb + store is read at activation) but its writes and acks are refused until a deployment carries it** — the + check reads the deployed snapshot, which is also what the flag-on path has always done. Deploy after + minting a key. + **What has no authorization check at all:** - **Read, Browse, TranslateBrowsePaths** — no override, no handler. Any admitted session, including @@ -496,9 +517,30 @@ Behaviours worth knowing before enabling it: no reconnect** — closing the session instead would force a full reconnect-and-resubscribe on every machine consumer in the fleet at once when a shared key expired. - ⚠️ **Event/alarm monitored items are revalidated by neither trigger.** The live-item map is fed only - from data-change items, so an alarm subscription is gated at creation and never again. Pre-existing — - the deployment trigger has the same hole — and tracked as a residual of arch-review 03/S19. + **Event/alarm monitored items are revalidated by both triggers too** (Gitea #562). A direct event + subscription — one on an equipment or device folder's event notifier — is tracked in a second map + alongside the data-change items and re-asked the creation gate's question (`Subscribe` on the + notifier's scope, for the identity of the session that created it). A denied item is **detached** from + the notifier's event fan-out through the SDK's own unsubscribe — exactly what the client's delete would + do — never set `MonitoringMode.Disabled`, so restoring its authority re-attaches it and alarms resume + with no client action. Events raised while it was stood down are not replayed. Two differences from a + data-change item: + - **The client is not told.** An event item has no value slot to carry `BadUserAccessDenied`, and a + fabricated "access denied" event would land in every alarm consumer's stream, so the feed simply goes + quiet. The operator-facing signal is the `AclGate: RevalidateEventSubscriptions withheld N of M` log line. + - **A client may delete a stood-down item** normally; the delete is absorbed (the item is already off + the fan-out) and a later restoring deployment does not resurrect it. + + ⚠️ **Server-object (all-events) event subscriptions are outside the ACL entirely — at creation and at + revalidation.** A subscription to `i=2253` (the Server object) is fanned out by the SDK through + `SubscribeToAllEvents` and through the Server object's own notifier, which the core node manager owns — + our node manager's creation gate never sees it, and an equipment folder with no direct subscriber + forwards its events to the Server object regardless of what our node manager attaches. So with the flag + on, a session subscribed to the Server object still receives every alarm in the address space (observed + over the wire 2026-10-07: an anonymous session holding no grant at all received a native alarm through a + Server-object event item). Scope + alarm consumers by subscribing to the equipment/device folders, and treat Server-object event access as + ungated until this is closed. - **An ancestor of a granted node is browsable**, or no narrow grant would be reachable: a client arrives at a deep node by browsing its ancestors, and the trie otherwise inherits downward only. Note what this necessarily discloses — the *names* of the folders on the path to a granted node are visible diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs index fa9c4007..c09d4061 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs @@ -348,6 +348,7 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 { Volatile.Read(ref _dataPlaneAuthority).Changed -= OnAuthorityChanged; _liveItems.Clear(); + _eventItems.Clear(); } base.Dispose(disposing); @@ -357,7 +358,9 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 /// The node-scoped ACL gate (#520 stage 5), or null when none is wired. Consulted by the /// write, alarm-command and HistoryRead seams only while /// is true; otherwise each keeps its - /// server-wide role check and behaviour is identical to before this stage. + /// server-wide role check and behaviour is identical to before this stage — with one exception: the + /// two role gates consult in either + /// mode, so an expired or revoked key loses its activation-time roles (Gitea #563). /// /// /// Null means "not wired" and is treated as not-enabled, not as deny-all. That looks like the wrong @@ -1595,6 +1598,12 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 identity, AlarmOperationFor(operation), condition.NodeId, RealmOf(condition.NodeId)); } + // Gitea #563: the activation-time AlarmAck role is honoured only while the key that earned it is + // still usable — an expired or deployment-disabled key loses it on its open session. + var keyVeto = _authorizer?.AuthorizeKeyUsable( + identity, AlarmOperationFor(operation), condition.NodeId); + if (keyVeto is not null) return keyVeto; + // Fail closed: no role / no identity ⇒ veto. Returning a bad ServiceResult aborts the SDK's // state change and surfaces the status to the client; we never route or mutate. return identity is null @@ -1670,10 +1679,14 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 // #520 stage 5. With ACLs enforced the scoped decision REPLACES the server-wide WriteOperate role // check — see AclOptions for why chaining them would leave every narrow grant inert. The // gateway-wired check applies either way: it is an availability fact, not an authorization one. + // With the flag off, Gitea #563: the activation-time WriteOperate role is honoured only while the + // session's key is still usable — checked first, so an expired key reads as denied rather than as + // "writes unavailable" on a node with no gateway. var gate = AclEnforced ? _authorizer!.Authorize(identity, OpcUaOperation.WriteOperate, node.NodeId, RealmOf(node.NodeId)) ?? (gatewayWired ? null : new ServiceResult(StatusCodes.BadNotWritable, "writes unavailable")) - : EvaluateEquipmentWriteGate(identity, gatewayWired); + : _authorizer?.AuthorizeKeyUsable(identity, OpcUaOperation.WriteOperate, node.NodeId) + ?? EvaluateEquipmentWriteGate(identity, gatewayWired); if (gate is not null) return gate; // Item A (synchronous structural fail-fast): reject a structurally-invalid write INLINE — return Bad @@ -3222,7 +3235,58 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 return StatusCodes.BadUserAccessDenied; } - return base.SubscribeToEvents(context, sourceId, subscriptionId, monitoredItem, unsubscribe); + if (unsubscribe) + { + // Gitea #562: an item RevalidateMonitoredItems stood down is ALREADY off its notifier's fan-out. + // Handing its delete to the SDK would make it remove an item that is not there — at best a + // spurious BadNodeIdUnknown on the client's delete, at worst a NullReferenceException inside + // MonitoredNode2.Remove when the node's event list has gone null. Checked and dropped under the + // same Lock the revalidation pass re-attaches under, so the two cannot interleave into + // re-attaching an item the client has just deleted. + lock (Lock) + { + if (_eventItems.TryRemove(monitoredItem, out var stoodDown) && stoodDown.Suppressed) + return ServiceResult.Good; + } + + return base.SubscribeToEvents(context, sourceId, subscriptionId, monitoredItem, unsubscribe); + } + + var result = base.SubscribeToEvents(context, sourceId, subscriptionId, monitoredItem, unsubscribe); + TrackLiveEventItem(sourceId, monitoredItem); + return result; + } + + /// + /// Starts (or refreshes) tracking a direct event subscription for revalidation (Gitea #562). + /// + /// The manager handle the item subscribed through. + /// The event item. + /// + /// + /// Only items the SDK actually attached are tracked — a subscribe the SDK refused (a source that + /// is not an event notifier, an unknown node) has nothing to stand down. The SDK reports an item + /// that was already attached (a ModifyMonitoredItems re-subscribe) as a non-Good status + /// while having re-attached it anyway, so attachment is read off the fan-out list itself rather + /// than inferred from the status the subscribe returned. + /// + /// + /// A re-subscribe of an item that was stood down is the client re-asking, and it has just passed + /// the creation gate above, so the entry is replaced with a live one. + /// + /// + private void TrackLiveEventItem(object sourceId, IEventMonitoredItem monitoredItem) + { + if (IsHandleInNamespace(sourceId) is not { } handle || !OwnsNamespace(handle.NodeId)) return; + + lock (Lock) + { + if (MonitoredNodes.TryGetValue(handle.NodeId, out var monitored) + && monitored.EventMonitoredItems?.Contains(monitoredItem) == true) + { + _eventItems[monitoredItem] = new LiveEventItem(handle); + } + } } /// @@ -3293,6 +3357,54 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 public bool Suppressed { get; set; } } + /// + /// Live event monitored items subscribed directly to a notifier this manager owns (Gitea #562), + /// the event-side twin of . + /// + /// + /// + /// A second map because the SDK keeps the two kinds apart: a data-change item reaches this + /// manager through , an event item only through + /// , and + /// they hang off different lists of . Before #562 nothing tracked event + /// items at all, so an alarm feed authorized at creation survived every later revocation. + /// + /// + /// Maintained by that override's subscribe/unsubscribe arms — the SDK routes every create, modify + /// and delete of a direct event item through it, subscription teardown included. Mutated under + /// Lock, so the membership check the revalidation pass makes before re-attaching an item + /// cannot race a client deleting it. + /// + /// + /// ⚠️ Server-object (all-events) subscriptions are not in this map, because they are not + /// authorized by this manager in the first place: the SDK fans them out through + /// SubscribeToAllEvents and through the Server object's own notifier, which the core node + /// manager owns. See docs/security.md § Data-Plane Authorization. + /// + /// + private readonly ConcurrentDictionary _eventItems = new(); + + /// + /// 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 + /// invisible to that client, so only the server side can show it. + /// + internal int AttachedEventItemCount + { + get + { + lock (Lock) return MonitoredNodes.Values.Sum(n => n.EventMonitoredItems?.Count ?? 0); + } + } + + /// A tracked event item: the notifier it subscribed to, and whether we have stood it down. + /// The notifier's handle; its is what the item is attached to. + private sealed record LiveEventItem(NodeHandle Handle) + { + /// True while the item has been detached from its notifier's event fan-out for want of a grant. + public bool Suppressed { get; set; } + } + /// /// Re-authorizes every live monitored item against the current generation, standing down the ones /// whose grant has gone. @@ -3346,7 +3458,7 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 /// public void RevalidateMonitoredItems() { - if (!AclEnforced || _liveItems.IsEmpty) return; + if (!AclEnforced || (_liveItems.IsEmpty && _eventItems.IsEmpty)) return; var now = DateTime.UtcNow; var stoodDown = 0; @@ -3391,11 +3503,83 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 stoodDown++; } + var (eventsStoodDown, eventsRestored) = RevalidateEventItems(verdicts); + if (stoodDown > 0 || restored > 0) { _authorizer!.LogFiltered( "RevalidateSubscriptions", principal: null, stoodDown, _liveItems.Count); } + + if (eventsStoodDown > 0 || eventsRestored > 0) + { + _authorizer!.LogFiltered( + "RevalidateEventSubscriptions", principal: null, eventsStoodDown, _eventItems.Count); + } + } + + /// + /// The event-item half of (Gitea #562): re-authorizes every + /// tracked direct event subscription against its notifier, standing down the denied ones and + /// re-attaching the restored ones. + /// + /// The pass-local memo shared with the data-change half. + /// How many items were stood down and how many restored. + /// + /// + /// The verdict is the same question the creation gate in + /// + /// asks — Subscribe on the notifier's scope, for the identity of the session that created + /// the item — so an event feed is held to exactly the authority it was admitted under, re-asked on + /// both triggers (a deployment, and the host's AclRevalidationTimer). + /// + /// + /// ⚠️ Detached through the SDK's own unsubscribe, never . + /// Same reason as the data-change half: a disabled item can only be re-enabled by the client, and + /// a machine consumer will never try, so a revocation reversed by the next deployment would leave + /// the feed dead. The stand-down is the SDK's protected SubscribeToEvents(…, unsubscribe: + /// true) — precisely what the client's own delete does to the notifier's + /// fan-out — and the restore is the same call with + /// unsubscribe: false, so neither leaves the fan-out in a shape a client could not have + /// produced. The item itself stays in the client's subscription untouched. + /// + /// + /// The client is not told. Unlike a data-change item, an event item has no value slot to + /// carry a BadUserAccessDenied: its notifications are event field lists, and inventing an + /// event to say "access denied" would put a fabricated event in front of every alarm consumer. The + /// feed goes quiet, and the operator-facing signal is the summary line this pass logs. + /// + /// + /// Membership is re-checked under Lock immediately before each attach/detach: the walk is + /// over a concurrent snapshot, and re-attaching an item the client deleted mid-pass would leave it + /// on the notifier's fan-out with no owner and no way to remove it. + /// + /// + private (int StoodDown, int Restored) RevalidateEventItems( + Dictionary<(string Session, AddressSpaceRealm Realm, string Node), bool> verdicts) + { + var stoodDown = 0; + var restored = 0; + + foreach (var (item, tracked) in _eventItems) + { + var permitted = IsSubscriptionStillPermitted( + item.EffectiveIdentity as RoleCarryingUserIdentity, tracked.Handle.NodeId, verdicts); + if (permitted != tracked.Suppressed) continue; + + lock (Lock) + { + if (!_eventItems.TryGetValue(item, out var current) || !ReferenceEquals(current, tracked)) continue; + + SubscribeToEvents(SystemContext, tracked.Handle.Node, item, unsubscribe: !permitted); + tracked.Suppressed = !permitted; + } + + if (permitted) restored++; + else stoodDown++; + } + + return (stoodDown, restored); } /// 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 127e5778..890e87b5 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/Security/DataPlaneAuthorizer.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/Security/DataPlaneAuthorizer.cs @@ -201,6 +201,44 @@ public sealed class DataPlaneAuthorizer AddressSpaceRealm realm) => Evaluate(identity, operation, nodeId, realm, cached: false); + /// + /// Whether the API key behind is still usable — consulted by the two + /// server-wide role gates (WriteOperate, AlarmAck) regardless of + /// (Gitea #563). + /// + /// + /// + /// With the flag off a session's roles are resolved once, at activation, so before this check a + /// key that expired by the clock — or that a deployment disabled or removed — kept + /// WriteOperate / AlarmAck on its open session until the client disconnected. With + /// the flag on, already makes the identical check first thing, so the role + /// gates call this only on the flag-off path and the two postures agree on key usability. + /// + /// + /// It is the same probe against the same + /// snapshot and the same clock as : one dictionary lookup, no I/O, so it is + /// safe on the SDK's synchronous write and method-call threads. A session carrying no key id — + /// Anonymous, or a legacy identity — has nothing that can expire and always passes; its roles stay + /// exactly what activation resolved. A key absent from the snapshot denies, as it does with the flag + /// on: on a driver-only node the artifact is the only key source there is, so "absent" means + /// "removed by a deployment". + /// + /// + /// The role-carrying identity off the SDK context, or null. + /// The operation being attempted, for the denial log. + /// The target node, for the denial log. + /// null to proceed to the role check; otherwise the veto to return to the SDK. + public ServiceResult? AuthorizeKeyUsable( + RoleCarryingUserIdentity? identity, OpcUaOperation operation, NodeId nodeId) + { + if (identity?.ApiKeyId is not { Length: > 0 } keyId) return null; + + var snapshot = _authority.Current; + return snapshot.FindUsableKey(keyId, _time.GetUtcNow().UtcDateTime) is null + ? DenyRevokedKey(operation, nodeId, identity.Principal, keyId, snapshot.GenerationId, cached: false) + : null; + } + /// /// The decision itself. Ordered so the cached seams reach their answer having allocated nothing: /// every check that must run per call comes first, the memoized verdict is consulted next, and the diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/ZB.MOM.WW.OtOpcUa.OpcUaServer.csproj b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/ZB.MOM.WW.OtOpcUa.OpcUaServer.csproj index 72b787cc..52dc5e0d 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/ZB.MOM.WW.OtOpcUa.OpcUaServer.csproj +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/ZB.MOM.WW.OtOpcUa.OpcUaServer.csproj @@ -32,6 +32,9 @@ + + diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/AclEventSubscriptionEnforcementTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/AclEventSubscriptionEnforcementTests.cs new file mode 100644 index 00000000..4016780c --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/AclEventSubscriptionEnforcementTests.cs @@ -0,0 +1,549 @@ +using System.Net; +using System.Net.Sockets; +using System.Text; +using Microsoft.Extensions.Logging.Abstractions; +using Opc.Ua; +using Opc.Ua.Client; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Commons.OpcUa; +using ZB.MOM.WW.OtOpcUa.Configuration.Entities; +using ZB.MOM.WW.OtOpcUa.Configuration.Enums; +using ZB.MOM.WW.OtOpcUa.Core.Authorization; +using ZB.MOM.WW.OtOpcUa.Host.OpcUa; +using ZB.MOM.WW.OtOpcUa.OpcUaServer.Security; + +namespace ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests; + +/// +/// Gitea #562 over the wire — an alarm (event) subscription is re-authorized by both revalidation +/// triggers, exactly as a data-change subscription is: a deployment that removes its grant, and the host's +/// clock-driven once the session's key expires. +/// +/// +/// +/// Before #562 an event monitored item was gated at creation and never again, so an alarm feed +/// survived the revocation of the authority it was admitted under until the client disconnected. +/// +/// +/// Over the wire for the reason AclSubscriptionEnforcementTests gives: whether an event item +/// can recover the identity of the session that created it, and whether detaching it from the SDK's +/// event fan-out really stops delivery (and re-attaching really resumes it), are assumptions about SDK +/// behaviour that fail silently if wrong. +/// +/// +/// Every absence assertion is paired with a positive control on the same event: one condition is +/// wired to two equipment notifiers (or watched by two sessions), so a single transition must reach the +/// still-authorized item before the denied one's silence counts for anything. +/// +/// +public sealed class AclEventSubscriptionEnforcementTests +{ + private const string Cluster = "SITE-A"; + private const string RawDeviceFolder = "Plant/Modbus/dev1"; + private const string RawAlarmPath = "Plant/Modbus/dev1/temp_hi"; + private const string EqA = "EqA"; + private const string EqB = "EqB"; + private const string KeyPrincipal = "expiry-probe"; + private const string KeyId = "k-event-expiry"; + + // [0]=EventId, [1]=Message — the message is each transition's discriminator. + private const int MessageFieldIndex = 1; + + private static readonly DateTime Start = new(2026, 10, 7, 12, 0, 0, DateTimeKind.Utc); + + /// + /// The deployment axis, end to end: revoke → the alarm feed stops (while the still-granted control + /// item receives the same event) → restore → the feed resumes with no client action. + /// + /// + /// The restore leg is the reason the stand-down is a detach rather than + /// MonitoringMode.Disabled: a disabled item can only be re-enabled by the client, and a machine + /// consumer never will. + /// + [Fact] + public async Task Revoking_then_restoring_a_grant_stops_then_resumes_a_live_alarm_subscription() + { + var ct = TestContext.Current.CancellationToken; + var rig = await Rig.StartAsync("AclEventRevoke", ct); + try + { + var state = rig.Enforce(EquipmentGrant(EqA), EquipmentGrant(EqB)); + rig.Materialise(); + + using var session = await rig.ConnectAsync(anonymous: true, ct); + var collector = new EventCollector(); + var subscription = await CreateSubscriptionAsync(session, collector, ct); + var onA = AddEventItem(subscription, rig.UnsNodeId(session, EqA)); + var onB = AddEventItem(subscription, rig.UnsNodeId(session, EqB)); + await subscription.ApplyChangesAsync(ct); + onA.Status.Error.ShouldBeNull(); + onB.Status.Error.ShouldBeNull(); + + // Baseline: both items receive the transition. + rig.Transition("t1", active: true); + await WaitUntilAsync(() => collector.Has(onA, "t1") && collector.Has(onB, "t1")); + + // The next deployment carries only EqB's grant. + state.Apply(Cluster, "rev-2", [], [EquipmentGrant(EqB)], ScopePaths()); + rig.AttachedEventItems.ShouldBe(1); + + rig.Transition("t2", active: false); + // Positive control: the same event reached the still-granted item, so it was fired and published… + await WaitUntilAsync(() => collector.Has(onB, "t2")); + // …and settle past a publishing interval so a late copy for EqA would have arrived too. + await Task.Delay(TimeSpan.FromMilliseconds(700), ct); + collector.Has(onA, "t2").ShouldBeFalse("the revoked item must not receive events after the deployment"); + + // A deployment that restores the grant brings the feed back on its own. + state.Apply(Cluster, "rev-3", [], [EquipmentGrant(EqA), EquipmentGrant(EqB)], ScopePaths()); + rig.AttachedEventItems.ShouldBe(2); + + rig.Transition("t3", active: true); + await WaitUntilAsync(() => collector.Has(onA, "t3") && collector.Has(onB, "t3")); + collector.Has(onA, "t2").ShouldBeFalse("nothing withheld while stood down is replayed on restore"); + + await subscription.DeleteAsync(true, ct); + } + finally { await rig.DisposeAsync(); } + } + + /// + /// A client may delete an item while it is stood down: the delete succeeds, and a later deployment + /// that would have restored it does not resurrect it onto the notifier's fan-out. + /// + /// + /// The SDK's own unsubscribe would try to remove an item that is no longer attached, so the node + /// manager short-circuits it. Asserted over the wire because the failure — an exception inside the + /// SDK's delete path, or a ghost item still receiving events — is only observable there. + /// + [Fact] + public async Task A_stood_down_alarm_item_can_be_deleted_and_is_not_resurrected() + { + var ct = TestContext.Current.CancellationToken; + var rig = await Rig.StartAsync("AclEventDelete", ct); + try + { + var state = rig.Enforce(EquipmentGrant(EqA), EquipmentGrant(EqB)); + rig.Materialise(); + + using var session = await rig.ConnectAsync(anonymous: true, ct); + var collector = new EventCollector(); + var subscription = await CreateSubscriptionAsync(session, collector, ct); + var onA = AddEventItem(subscription, rig.UnsNodeId(session, EqA)); + var onB = AddEventItem(subscription, rig.UnsNodeId(session, EqB)); + await subscription.ApplyChangesAsync(ct); + + rig.Transition("t1", active: true); + await WaitUntilAsync(() => collector.Has(onA, "t1") && collector.Has(onB, "t1")); + + rig.AttachedEventItems.ShouldBe(2); + state.Apply(Cluster, "rev-2", [], [EquipmentGrant(EqB)], ScopePaths()); + rig.AttachedEventItems.ShouldBe(1); + + subscription.RemoveItem(onA); + await subscription.ApplyChangesAsync(ct); + onA.Status.Error.ShouldBeNull(); + + // A restoring deployment must not re-attach the deleted item. The client could never see that + // happen — it has deleted the item — so the server-side fan-out is what is asserted. + state.Apply(Cluster, "rev-3", [], [EquipmentGrant(EqA), EquipmentGrant(EqB)], ScopePaths()); + rig.AttachedEventItems.ShouldBe(1, "the deleted item must not be resurrected onto EqA's fan-out"); + + rig.Transition("t2", active: false); + await WaitUntilAsync(() => collector.Has(onB, "t2")); + await Task.Delay(TimeSpan.FromMilliseconds(700), ct); + collector.Has(onA, "t2").ShouldBeFalse(); + + await subscription.DeleteAsync(true, ct); + } + finally { await rig.DisposeAsync(); } + } + + /// + /// The clock axis: the key behind a session's alarm subscription expires, one revalidation tick runs + /// through the host's own , and that session stops receiving alarms + /// — with no deployment anywhere in the run — while an independent session on the same notifier keeps + /// receiving the same event. + /// + [Fact] + public async Task An_expired_key_stops_a_live_alarm_subscription_with_no_deploy() + { + var ct = TestContext.Current.CancellationToken; + var rig = await Rig.StartAsync("AclEventExpiry", ct); + try + { + var key = new ApiKey + { + ApiKeyId = KeyId, + Name = KeyId, + SecretHash = "hash", + Principal = KeyPrincipal, + Enabled = true, + CreatedUtc = Start.AddDays(-1), + ExpiresUtc = Start.AddMinutes(10), + }; + var state = rig.Enforce( + [key], ClusterGrant(KeyPrincipal, "acl-key"), ClusterGrant(OpcUaPrincipals.Anonymous, "acl-anon")); + rig.Materialise(); + + using var keySession = await rig.ConnectAsync(anonymous: false, ct); + using var controlSession = await rig.ConnectAsync(anonymous: true, ct); + + var keyEvents = new EventCollector(); + var keySubscription = await CreateSubscriptionAsync(keySession, keyEvents, ct); + var keyItem = AddEventItem(keySubscription, rig.UnsNodeId(keySession, EqA)); + await keySubscription.ApplyChangesAsync(ct); + keyItem.Status.Error.ShouldBeNull(); + + var controlEvents = new EventCollector(); + var controlSubscription = await CreateSubscriptionAsync(controlSession, controlEvents, ct); + var controlItem = AddEventItem(controlSubscription, rig.UnsNodeId(controlSession, EqA)); + await controlSubscription.ApplyChangesAsync(ct); + + rig.Transition("t1", active: true); + await WaitUntilAsync(() => keyEvents.Has(keyItem, "t1") && controlEvents.Has(controlItem, "t1")); + + // Only the clock moves. + rig.Advance(TimeSpan.FromMinutes(20)); + rig.TickRevalidation(); + + rig.Transition("t2", active: false); + await WaitUntilAsync(() => controlEvents.Has(controlItem, "t2")); + await Task.Delay(TimeSpan.FromMilliseconds(700), ct); + keyEvents.Has(keyItem, "t2").ShouldBeFalse("the expired key's alarm feed must stop on the tick"); + state.Current.GenerationId.ShouldBe(1); + + await controlSubscription.DeleteAsync(true, ct); + await keySubscription.DeleteAsync(true, ct); + } + finally { await rig.DisposeAsync(); } + } + + /// + /// The negative control for the clock axis: the same run with the clock advanced but not past + /// ExpiresUtc keeps the alarm feed live across a tick. + /// + /// + /// Without it the test above also passes on a build whose tick stands down every event item it sees, + /// which would silence every alarm consumer in the fleet once a minute. + /// + [Fact] + public async Task An_unexpired_key_keeps_its_alarm_subscription_across_a_tick() + { + var ct = TestContext.Current.CancellationToken; + var rig = await Rig.StartAsync("AclEventLive", ct); + try + { + var key = new ApiKey + { + ApiKeyId = KeyId, + Name = KeyId, + SecretHash = "hash", + Principal = KeyPrincipal, + Enabled = true, + CreatedUtc = Start.AddDays(-1), + ExpiresUtc = Start.AddHours(4), + }; + rig.Enforce([key], ClusterGrant(KeyPrincipal, "acl-key")); + rig.Materialise(); + + using var session = await rig.ConnectAsync(anonymous: false, ct); + var events = new EventCollector(); + var subscription = await CreateSubscriptionAsync(session, events, ct); + var item = AddEventItem(subscription, rig.UnsNodeId(session, EqA)); + await subscription.ApplyChangesAsync(ct); + + rig.Transition("t1", active: true); + await WaitUntilAsync(() => events.Has(item, "t1")); + + rig.Advance(TimeSpan.FromMinutes(20)); + rig.TickRevalidation(); + + rig.Transition("t2", active: false); + await WaitUntilAsync(() => events.Has(item, "t2")); + + await subscription.DeleteAsync(true, ct); + } + finally { await rig.DisposeAsync(); } + } + + // ───────────────────────────── helpers ───────────────────────────── + + private static NodeAcl EquipmentGrant(string equipment) => new() + { + NodeAclId = $"acl-{equipment}", + ClusterId = Cluster, + Principal = OpcUaPrincipals.Anonymous, + ScopeKind = NodeAclScopeKind.Equipment, + ScopeId = $"eq-{equipment}", + PermissionFlags = NodePermissions.Read | NodePermissions.Browse | NodePermissions.Subscribe, + }; + + private static NodeAcl ClusterGrant(string principal, string id) => new() + { + NodeAclId = id, + ClusterId = Cluster, + Principal = principal, + ScopeKind = NodeAclScopeKind.Cluster, + PermissionFlags = NodePermissions.Read | NodePermissions.Browse | NodePermissions.Subscribe, + }; + + private static IReadOnlyDictionary> ScopePaths() => + new Dictionary> + { + [new AclScopeRef(NodeAclScopeKind.Equipment, $"eq-{EqA}")] = [new NodeAclPath([NodeAclRealms.Uns, EqA])], + [new AclScopeRef(NodeAclScopeKind.Equipment, $"eq-{EqB}")] = [new NodeAclPath([NodeAclRealms.Uns, EqB])], + }; + + private static async Task CreateSubscriptionAsync( + ISession session, EventCollector collector, CancellationToken ct) + { + var subscription = new Subscription(session.DefaultSubscription) { PublishingInterval = 100 }; + subscription.FastEventCallback = collector.OnEvents; + session.AddSubscription(subscription); + await subscription.CreateAsync(ct); + return subscription; + } + + private static MonitoredItem AddEventItem(Subscription subscription, NodeId source) + { + var filter = new EventFilter(); + filter.AddSelectClause(ObjectTypes.BaseEventType, BrowseNames.EventId); + filter.AddSelectClause(ObjectTypes.BaseEventType, BrowseNames.Message); + + var item = new MonitoredItem(subscription.DefaultItem) + { + StartNodeId = source, + AttributeId = Attributes.EventNotifier, + Filter = filter, + QueueSize = 100, + SamplingInterval = 0, + }; + subscription.AddItem(item); + return item; + } + + private static async Task WaitUntilAsync(Func condition) + { + var deadline = DateTime.UtcNow + TimeSpan.FromSeconds(10); + while (DateTime.UtcNow < deadline) + { + if (condition()) return; + await Task.Delay(50); + } + condition().ShouldBeTrue("condition not met within timeout"); + } + + /// Records which transition messages reached which monitored item. + private sealed class EventCollector + { + private readonly object _gate = new(); + private readonly HashSet<(uint Handle, string Message)> _seen = []; + + public void OnEvents(Subscription subscription, EventNotificationList notification, IList stringTable) + { + lock (_gate) + { + foreach (var e in notification.Events) + { + if (e.EventFields.Count > MessageFieldIndex + && e.EventFields[MessageFieldIndex].Value is LocalizedText { Text: { } text }) + { + _seen.Add((e.ClientHandle, text)); + } + } + } + } + + public bool Has(MonitoredItem item, string message) + { + lock (_gate) return _seen.Contains((item.ClientHandle, message)); + } + } + + /// A booted server with ACL enforcement, one native alarm wired to two equipment notifiers. + private sealed class Rig : IAsyncDisposable + { + private readonly string _pkiRoot; + private readonly FakeClock _clock = new(Start); + private OpcUaApplicationHost _host = null!; + private OtOpcUaSdkServer _server = null!; + private SdkAddressSpaceSink _sink = null!; + private AclRevalidationTimer? _timer; + private int _port; + + private Rig(string pkiRoot) => _pkiRoot = pkiRoot; + + private OtOpcUaNodeManager Nm => _server.NodeManager!; + + public static async Task StartAsync(string name, CancellationToken ct) + { + var rig = new Rig(Path.Combine(Path.GetTempPath(), $"otopcua-{name}-{Guid.NewGuid():N}")); + rig._port = AllocateFreePort(); + rig._server = new OtOpcUaSdkServer(); + rig._host = new OpcUaApplicationHost( + new OpcUaApplicationHostOptions + { + ApplicationName = $"OtOpcUa.{name}", + ApplicationUri = $"urn:OtOpcUa.{name}.{Guid.NewGuid():N}", + OpcUaPort = rig._port, + PublicHostname = "127.0.0.1", + PkiStoreRoot = rig._pkiRoot, + EnabledSecurityProfiles = [OpcUaSecurityProfile.None], + AutoAcceptUntrustedClientCertificates = true, + }, + NullLogger.Instance, + new KeyPresentingAuthenticator()); + + await rig._host.StartAsync(rig._server, ct); + rig._sink = new SdkAddressSpaceSink(rig.Nm); + return rig; + } + + /// Switches enforcement on with the given grants and no keys. + public DataPlaneAuthorityState Enforce(params NodeAcl[] grants) => Enforce([], grants); + + /// + /// Switches enforcement on, wiring the authorizer, the authority source (the deployment trigger) + /// and the host's revalidation timer (the clock trigger) over one hand-advanced clock. + /// + public DataPlaneAuthorityState Enforce(ApiKey[] keys, params NodeAcl[] grants) + { + var state = new DataPlaneAuthorityState(); + state.Apply(Cluster, "rev-1", keys, grants, ScopePaths()); + + Nm.DataPlaneAuthority = state; + Nm.Authorizer = new DataPlaneAuthorizer( + state, new TriePermissionEvaluator(state.Tries), enabled: true, NullLogger.Instance, _clock); + + // A long interval: the tick is driven by hand through the same component the host arms. + _timer = new AclRevalidationTimer( + () => _server.NodeManager?.RevalidateMonitoredItems(), + TimeSpan.FromHours(1), + TimeProvider.System, + NullLogger.Instance); + return state; + } + + /// One native condition on a raw device, wired as an event source of two equipment folders. + public void Materialise() + { + _sink.EnsureFolder(RawDeviceFolder, parentNodeId: null, displayName: "dev1", realm: AddressSpaceRealm.Raw); + _sink.MaterialiseAlarmCondition( + RawAlarmPath, RawDeviceFolder, "temp_hi", "OffNormalAlarm", 700, AddressSpaceRealm.Raw, isNative: true); + _sink.EnsureFolder(EqA, parentNodeId: null, displayName: "A", realm: AddressSpaceRealm.Uns); + _sink.EnsureFolder(EqB, parentNodeId: null, displayName: "B", realm: AddressSpaceRealm.Uns); + _sink.WireAlarmNotifiers(RawAlarmPath, AddressSpaceRealm.Raw, [EqA, EqB], AddressSpaceRealm.Uns); + } + + /// Fires one genuine condition transition, carrying . + public void Transition(string message, bool active) => + _sink.WriteAlarmCondition( + RawAlarmPath, + new AlarmConditionSnapshot( + Active: active, Acknowledged: !active, Confirmed: true, Enabled: true, + Shelving: AlarmShelvingKind.Unshelved, Severity: 700, Message: message), + DateTime.UtcNow, + AddressSpaceRealm.Raw); + + public NodeId UnsNodeId(ISession session, string id) + { + var ns = (ushort)session.NamespaceUris.GetIndex(V3NodeIds.UnsNamespaceUri); + ns.ShouldBeGreaterThan((ushort)0); + return new NodeId(id, ns); + } + + /// Event items attached to the fan-out of the manager's own notifiers. + public int AttachedEventItems => Nm.AttachedEventItemCount; + + public void Advance(TimeSpan by) => _clock.Advance(by); + + public void TickRevalidation() => _timer!.Tick(); + + public async Task ConnectAsync(bool anonymous, CancellationToken ct) + { + var appConfig = new ApplicationConfiguration + { + ApplicationName = "OtOpcUa.AclEventClient", + ApplicationUri = $"urn:OtOpcUa.AclEventClient.{Guid.NewGuid():N}", + ApplicationType = ApplicationType.Client, + SecurityConfiguration = TestClientSecurity.Build(TestClientSecurity.AllocatePkiRoot()), + ClientConfiguration = new ClientConfiguration { DefaultSessionTimeout = 60_000 }, + }; + await appConfig.ValidateAsync(ApplicationType.Client, ct); + appConfig.CertificateValidator.CertificateValidation += (_, e) => e.Accept = true; + + var endpoint = await CoreClientUtils.SelectEndpointAsync( + appConfig, $"opc.tcp://127.0.0.1:{_port}/OtOpcUa", false, DefaultTelemetry.Create(_ => { }), ct); + + var identity = anonymous + ? new UserIdentity(new AnonymousIdentityToken()) + : new UserIdentity(new UserNameIdentityToken + { + UserName = "apikey", + // 1.5.378 carries the UserName token password as raw bytes; the host decodes UTF-8. + DecryptedPassword = Encoding.UTF8.GetBytes($"otua_{KeyId}_secret"), + }); + + return await new DefaultSessionFactory(DefaultTelemetry.Create(_ => { })).CreateAsync( + appConfig, + new ConfiguredEndpoint(null, endpoint, EndpointConfiguration.Create(appConfig)), + updateBeforeConnect: false, + checkDomain: false, + sessionName: "AclEventSubscriptionEnforcementTests", + sessionTimeout: 60_000, + identity: identity, + preferredLocales: null, + ct: ct); + } + + public async ValueTask DisposeAsync() + { + _timer?.Dispose(); + await _host.DisposeAsync(); + if (Directory.Exists(_pkiRoot)) + { + try { Directory.Delete(_pkiRoot, recursive: true); } + catch (IOException) { /* best effort */ } + } + } + + private static int AllocateFreePort() + { + var listener = new TcpListener(IPAddress.Loopback, 0); + listener.Start(); + var port = ((IPEndPoint)listener.LocalEndpoint).Port; + listener.Stop(); + return port; + } + + /// + /// Admits the fixed apikey username as carrying + /// ; anonymous falls through to the interface default (the + /// anonymous principal). Credential verification itself is covered in the Host suite. + /// + private sealed class KeyPresentingAuthenticator : IOpcUaUserAuthenticator + { + /// + public Task AuthenticateUserNameAsync( + string username, string password, CancellationToken ct) => + Task.FromResult( + string.Equals(username, "apikey", StringComparison.Ordinal) + && password.StartsWith($"otua_{KeyId}_", StringComparison.Ordinal) + ? OpcUaUserAuthResult.AllowKey(KeyPrincipal, [], KeyId) + : OpcUaUserAuthResult.Deny("Invalid API key.")); + } + + /// A hand-advanced clock; the authorizer reads it on every decision. + private sealed class FakeClock(DateTime utcNow) : TimeProvider + { + private DateTimeOffset _now = new(utcNow, TimeSpan.Zero); + + /// + public override DateTimeOffset GetUtcNow() => _now; + + public void Advance(TimeSpan by) => _now = _now.Add(by); + } + } +} diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerKeyUsabilityGateTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerKeyUsabilityGateTests.cs new file mode 100644 index 00000000..64addb48 --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerKeyUsabilityGateTests.cs @@ -0,0 +1,297 @@ +using Microsoft.Extensions.Logging.Abstractions; +using Opc.Ua; +using Opc.Ua.Server; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Commons.OpcUa; +using ZB.MOM.WW.OtOpcUa.Configuration.Entities; +using ZB.MOM.WW.OtOpcUa.Configuration.Enums; +using ZB.MOM.WW.OtOpcUa.Core.Authorization; +using ZB.MOM.WW.OtOpcUa.OpcUaServer.Security; + +namespace ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests; + +/// +/// Gitea #563 (arch-review 03/S19 residual R2) — with Security:Acl:Enabled off, the two +/// server-wide role gates (WriteOperate on Value writes, AlarmAck on the Part 9 methods) +/// re-check that the session's API key is still usable on every call. +/// +/// +/// +/// A session's roles are resolved once, at activation, from the key's grants. Before #563 nothing +/// with the flag off ever looked at the key again, so a key whose ExpiresUtc passed — or that +/// a deployment disabled — kept WriteOperate / AlarmAck until the client chose to +/// disconnect, which on a machine-to-machine data plane can be never. +/// +/// +/// Every test here is an allow-then-deny on the same session: the first call proves the +/// session genuinely holds the role, so the later denial can only have come from the key check. A +/// deny-only test would pass against a gate that denied everything. +/// +/// +public sealed class NodeManagerKeyUsabilityGateTests : IDisposable +{ + private const string Cluster = "SITE-A"; + private const string Principal = "scadabridge"; + private const string KeyId = "k-563"; + + private static readonly DateTime Start = new(2026, 10, 7, 12, 0, 0, DateTimeKind.Utc); + + private static CancellationToken Ct => TestContext.Current.CancellationToken; + + private readonly string _pkiRoot = Path.Combine( + Path.GetTempPath(), $"otopcua-keygate-{Guid.NewGuid():N}"); + + /// + /// THE #563 test for the write gate: ACL off, a key-carrying session holding WriteOperate + /// writes; only the clock moves past ExpiresUtc; the next write is refused and never reaches + /// the device. + /// + [Fact] + public async Task An_expired_key_loses_its_write_role_on_the_clock_with_acl_off() + { + var (host, server) = await BootAsync(); + try + { + var nm = server.NodeManager!; + var gateway = new RecordingWriteGateway(); + nm.NodeWriteGateway = gateway; + var clock = new FakeClock(Start); + var (state, authorizer) = Wire(nm, clock, Key(expiresUtc: Start.AddMinutes(10))); + authorizer.Enabled.ShouldBeFalse(); + var nodeId = Writable(nm, "Plant/Modbus/dev1/Setpoint"); + var identity = KeyIdentity(OpcUaDataPlaneRoles.WriteOperate); + + Write(nm, identity, nodeId, 1f)[0].StatusCode.Code.ShouldBe(StatusCodes.Good); + + clock.Advance(TimeSpan.FromMinutes(20)); + + Write(nm, identity, nodeId, 2f)[0].StatusCode.Code.ShouldBe(StatusCodes.BadUserAccessDenied); + gateway.Writes.Count.ShouldBe(1); + state.Current.GenerationId.ShouldBe(1); // no deployment moved anything — only the clock did + } + finally { await host.DisposeAsync(); } + } + + /// + /// The deployment axis of the same gate: a deployment that disables the key strips the write role + /// from the session that is already open. + /// + [Fact] + public async Task A_key_disabled_by_a_deployment_loses_its_write_role_with_acl_off() + { + var (host, server) = await BootAsync(); + try + { + var nm = server.NodeManager!; + var gateway = new RecordingWriteGateway(); + nm.NodeWriteGateway = gateway; + var (state, _) = Wire(nm, new FakeClock(Start), Key(expiresUtc: null)); + var nodeId = Writable(nm, "Plant/Modbus/dev1/Setpoint"); + var identity = KeyIdentity(OpcUaDataPlaneRoles.WriteOperate); + + Write(nm, identity, nodeId, 1f)[0].StatusCode.Code.ShouldBe(StatusCodes.Good); + + var disabled = Key(expiresUtc: null); + disabled.Enabled = false; + state.Apply(Cluster, "rev-2", [disabled], [], NoScopes); + + Write(nm, identity, nodeId, 2f)[0].StatusCode.Code.ShouldBe(StatusCodes.BadUserAccessDenied); + gateway.Writes.Count.ShouldBe(1); + } + finally { await host.DisposeAsync(); } + } + + /// + /// Anonymous carries no key, so nothing can expire: an anonymous session granted + /// WriteOperate keeps writing however far the clock moves, against a snapshot holding no keys + /// at all. Pins that the new check keys on the presence of a key id, not on "is this session in the + /// key table". + /// + [Fact] + public async Task An_anonymous_session_is_unaffected_by_the_key_check() + { + var (host, server) = await BootAsync(); + try + { + var nm = server.NodeManager!; + var gateway = new RecordingWriteGateway(); + nm.NodeWriteGateway = gateway; + var clock = new FakeClock(Start); + Wire(nm, clock); + var nodeId = Writable(nm, "Plant/Modbus/dev1/Setpoint"); + var anonymous = new RoleCarryingUserIdentity( + new AnonymousIdentityToken(), [OpcUaDataPlaneRoles.WriteOperate], + OpcUaPrincipals.Anonymous, apiKeyId: null); + + Write(nm, anonymous, nodeId, 1f)[0].StatusCode.Code.ShouldBe(StatusCodes.Good); + clock.Advance(TimeSpan.FromDays(400)); + Write(nm, anonymous, nodeId, 2f)[0].StatusCode.Code.ShouldBe(StatusCodes.Good); + + gateway.Writes.Count.ShouldBe(2); + } + finally { await host.DisposeAsync(); } + } + + /// + /// The alarm half: ACL off, a key-carrying session holding AlarmAck acknowledges; the key + /// expires by the clock; the next acknowledge is refused and routes nothing. + /// + [Fact] + public async Task An_expired_key_loses_its_alarm_ack_role_on_the_clock_with_acl_off() + { + var (host, server) = await BootAsync(); + try + { + var nm = server.NodeManager!; + var routed = new List(); + nm.AlarmCommandRouter = routed.Add; + var clock = new FakeClock(Start); + Wire(nm, clock, Key(expiresUtc: Start.AddMinutes(10))); + + nm.EnsureFolder("eq-1", parentNodeId: null, displayName: "Equipment 1", realm: AddressSpaceRealm.Uns); + nm.MaterialiseAlarmCondition( + "alm-1", "eq-1", "HighTemp", "OffNormalAlarm", severity: 700, realm: AddressSpaceRealm.Uns); + var condition = nm.TryGetAlarmCondition("alm-1")!; + var ctx = new ServerSystemContext(server.CurrentInstance) + { + UserIdentity = KeyIdentity(OpcUaDataPlaneRoles.AlarmAck), + }; + + condition.OnAcknowledge!(ctx, condition, Guid.NewGuid().ToByteArray(), new LocalizedText("1")) + .ShouldBe(ServiceResult.Good); + + clock.Advance(TimeSpan.FromMinutes(20)); + + condition.OnAcknowledge!(ctx, condition, Guid.NewGuid().ToByteArray(), new LocalizedText("2")) + .StatusCode.Code.ShouldBe(StatusCodes.BadUserAccessDenied); + routed.Count.ShouldBe(1); + } + finally { await host.DisposeAsync(); } + } + + // ───────────────────────────── helpers ───────────────────────────── + + private static readonly IReadOnlyDictionary> NoScopes = + new Dictionary>(); + + private static ApiKey Key(DateTime? expiresUtc) => new() + { + ApiKeyId = KeyId, + Name = KeyId, + SecretHash = "hash", + Principal = Principal, + Enabled = true, + CreatedUtc = Start.AddDays(-1), + ExpiresUtc = expiresUtc, + }; + + /// A key-carrying session whose activation-time roles are . + private static RoleCarryingUserIdentity KeyIdentity(params string[] roles) => + new(new UserNameIdentityToken { UserName = "apikey" }, roles, Principal, KeyId); + + /// + /// Wires a production-shaped, flag-OFF authorizer over one applied snapshot. The node manager's + /// authority source and the authorizer's are the same instance, as the host wires them. + /// + private static (DataPlaneAuthorityState State, DataPlaneAuthorizer Authorizer) Wire( + OtOpcUaNodeManager nm, TimeProvider clock, params ApiKey[] keys) + { + var state = new DataPlaneAuthorityState(); + state.Apply(Cluster, "rev-1", keys, [], NoScopes); + var authorizer = new DataPlaneAuthorizer( + state, new TriePermissionEvaluator(state.Tries), enabled: false, NullLogger.Instance, clock); + nm.DataPlaneAuthority = state; + nm.Authorizer = authorizer; + return (state, authorizer); + } + + private static NodeId Writable(OtOpcUaNodeManager nm, string rawPath) + { + nm.EnsureVariable(rawPath, parentFolderNodeId: null, displayName: rawPath, dataType: "Float", + writable: true, historianTagname: null, realm: AddressSpaceRealm.Raw); + return nm.TryGetVariable(rawPath, AddressSpaceRealm.Raw)!.NodeId; + } + + private static List Write( + OtOpcUaNodeManager nm, IUserIdentity identity, NodeId nodeId, object value) + { + var context = new OperationContext( + new RequestHeader(), secureChannelContext: null, RequestType.Write, identity); + var writes = new List + { + new() + { + NodeId = nodeId, + AttributeId = Attributes.Value, + Value = new DataValue(new Variant(value)) { StatusCode = StatusCodes.Good }, + }, + }; + var errors = Enumerable.Repeat(null!, writes.Count).ToList(); + nm.Write(context, writes, errors); + return errors; + } + + /// Records what actually reached the device. + private sealed class RecordingWriteGateway : IOpcUaNodeWriteGateway + { + public List Writes { get; } = []; + + public Task WriteAsync( + string nodeId, object? value, AddressSpaceRealm realm, CancellationToken ct) + { + lock (Writes) Writes.Add(nodeId); + return Task.FromResult(new NodeWriteOutcome(true, null)); + } + } + + /// A hand-advanced clock; the authorizer reads it on every decision. + private sealed class FakeClock(DateTime utcNow) : TimeProvider + { + private DateTimeOffset _now = new(utcNow, TimeSpan.Zero); + + /// + public override DateTimeOffset GetUtcNow() => _now; + + /// Moves the clock forward. + /// How far forward. + public void Advance(TimeSpan by) => _now = _now.Add(by); + } + + private async Task<(OpcUaApplicationHost Host, OtOpcUaSdkServer Server)> BootAsync() + { + var host = new OpcUaApplicationHost( + new OpcUaApplicationHostOptions + { + ApplicationName = "OtOpcUa.KeyGateTest", + ApplicationUri = $"urn:OtOpcUa.KeyGateTest:{Guid.NewGuid():N}", + OpcUaPort = AllocateFreePort(), + PublicHostname = "localhost", + PkiStoreRoot = _pkiRoot, + }, + NullLogger.Instance); + + var server = new OtOpcUaSdkServer(); + await host.StartAsync(server, Ct); + return (host, server); + } + + private static int AllocateFreePort() + { + using var listener = new System.Net.Sockets.TcpListener(System.Net.IPAddress.Loopback, 0); + listener.Start(); + var port = ((System.Net.IPEndPoint)listener.LocalEndpoint).Port; + listener.Stop(); + return port; + } + + /// Cleans up the PKI root directory. + public void Dispose() + { + if (Directory.Exists(_pkiRoot)) + { + try { Directory.Delete(_pkiRoot, recursive: true); } + catch (IOException) { /* best effort */ } + } + } +}