fix(security): resolve the event-subscribe gate's source through IsHandleInNamespace
SubscribeToEvents' sourceId is the manager HANDLE, not a NodeState — the same object-typed trap as ContinuationPoint.NodeToBrowse, found the same way. The `sourceId as NodeState` cast compiled, always yielded null, and silently disabled the gate: an ungranted session could subscribe to an equipment's event notifier and receive every alarm it raises, including the condition fields, while every value on it was refused. Now uses the SDK's own IsHandleInNamespace, which cannot be wrong about the type. The new test passes a null monitored item deliberately: the gate must deny without touching the item, which also makes the test a discriminator rather than a restatement — against the broken cast the call falls through to the base implementation, which does dereference it, so the test fails loudly. Claude-Session: https://claude.ai/code/session_015p7wGqy3YpZNCpDzTpGMKo
This commit is contained in:
@@ -2877,9 +2877,15 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2
|
||||
IEventMonitoredItem monitoredItem,
|
||||
bool unsubscribe)
|
||||
{
|
||||
// ⚠️ sourceId is the MANAGER HANDLE, not a NodeState — the same `object`-typed trap as
|
||||
// ContinuationPoint.NodeToBrowse. Casting it to NodeState compiles, always yields null, and
|
||||
// silently disables this gate; IsHandleInNamespace is the SDK's own resolver and cannot be
|
||||
// wrong about it.
|
||||
//
|
||||
// Only creation is gated. Refusing an UNSUBSCRIBE would strand the item on a node the session can
|
||||
// no longer reach — the client could never detach it, and the server would keep delivering.
|
||||
if (AclEnforced && !unsubscribe && sourceId is NodeState source
|
||||
if (AclEnforced && !unsubscribe
|
||||
&& IsHandleInNamespace(sourceId) is { } source
|
||||
&& !Permits(IdentityOf(context), OpcUaOperation.CreateMonitoredItems, source.NodeId))
|
||||
{
|
||||
return StatusCodes.BadUserAccessDenied;
|
||||
|
||||
@@ -229,6 +229,45 @@ public sealed class NodeManagerStage6EnforcementTests : IDisposable
|
||||
}
|
||||
|
||||
|
||||
/// <summary>
|
||||
/// Event subscriptions are gated, and the gate resolves its source through the SDK's own
|
||||
/// <c>IsHandleInNamespace</c> rather than a cast.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// <para>
|
||||
/// ⚠️ This exists because the first version of that gate read <c>sourceId as NodeState</c>.
|
||||
/// <c>sourceId</c> is the manager <b>handle</b>, so the cast compiled, always yielded null, and
|
||||
/// silently disabled the gate — the same <c>object</c>-typed trap as
|
||||
/// <c>ContinuationPoint.NodeToBrowse</c>. Nothing about the code looked wrong.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// The monitored item is deliberately <see langword="null"/>: a denial must be reached without
|
||||
/// the gate touching the item at all. That also makes the test a discriminator rather than a
|
||||
/// restatement — against the broken cast the gate falls through to the base implementation,
|
||||
/// which does dereference the item, so the test fails loudly instead of passing.
|
||||
/// </para>
|
||||
/// </remarks>
|
||||
[Fact]
|
||||
public async Task Event_subscriptions_are_gated_on_the_notifier()
|
||||
{
|
||||
var (host, server) = await BootAsync();
|
||||
try
|
||||
{
|
||||
var nm = server.NodeManager!;
|
||||
nm.Authorizer = Authorizer(enabled: true, GrantOn(NodePermissions.Read));
|
||||
var nodeId = Variable(nm, "Plant/Modbus/dev1/Speed");
|
||||
|
||||
var context = new OperationContext(
|
||||
new RequestHeader(), secureChannelContext: null, RequestType.CreateMonitoredItems, Identity());
|
||||
|
||||
var result = nm.SubscribeToEvents(
|
||||
context, nm.GetManagerHandle(nodeId), subscriptionId: 1, monitoredItem: null!, unsubscribe: false);
|
||||
|
||||
result.StatusCode.Code.ShouldBe(StatusCodes.BadUserAccessDenied);
|
||||
}
|
||||
finally { await host.DisposeAsync(); }
|
||||
}
|
||||
|
||||
// ───────────────────────────── helpers ─────────────────────────────
|
||||
|
||||
private static NodeId Variable(OtOpcUaNodeManager nm, string rawPath)
|
||||
|
||||
Reference in New Issue
Block a user