feat(security): enforce node ACLs on write, alarms and HistoryRead (#520 stage 5)
`Security:Acl:Enabled` (default false) makes scoped `NodeAcl` grants decide data-plane authorization at the three seams that have one, and gives HistoryRead a check it never had. Read, Browse and Subscribe stay ungated — stage 6. The ACL decision REPLACES the server-wide role check rather than chaining with it. Chained, a grant would have to confer the role as well as pass the scope check, and only Cluster-scoped grants confer a role — so every narrow grant would have stayed inert and the flag would have looked inert too. The load-bearing test is an ALLOW: a session carrying no roles at all writing successfully on the strength of its grant. A deny-only test passes against a build where the ACL is never consulted, because the role gate denies too. Found en route: the principal never reached the session. The authenticator passed it as OpcUaUserAuthResult.DisplayName, but RoleCarryingUserIdentity derives DisplayName from the presented token — for a key-bearing client that is the static username `apikey`, identical fleet-wide. So the trie's matching key was dropped at the identity boundary, and the audit ClientUserId on a rejected write recorded `apikey` for every consuming system. Principal and ApiKeyId are now carried explicitly. Beyond the stage as sketched: - Revocation reaches an OPEN session. Every decision re-checks the authenticating key against the current snapshot; without it a key revoked by a deployment kept its authority until the client chose to reconnect, which for a machine-to-machine consumer can be indefinitely. - UserAuthorizationState is built per CALL, not per session. The per-session shape exists so an LDAP session can re-resolve membership that changed out of band; nothing can change out of band under API keys, so the generation stamp is the whole invalidation signal and a per-call record picks it up for free. IsStale/NeedsRefresh therefore cannot fire here and are documented as vestigial. - The trie is built inside DataPlaneAuthorityState.Apply, under the same generation as the snapshot, rather than by a second wiring at the publish actor. An uninstalled trie denies everything — indistinguishable from working enforcement. Deleting the install turns 13 of the 22 new tests red. - The deploy log reports what the BUILD dropped instead of re-deriving it. The old count agreed with the builder on one of its four drop reasons; a rule with a blank ScopeId, a retired scope kind, or a path not starting at a realm vanished with the log reading zero. Both IApiKeyStore implementations widened to the full grant set. The narrowing was redundant — ApiKeyInterimRoleMapper applies the same filter itself — and became a trap once scope was enforced. Pinned by asserting the roles are unchanged, with the two grants carrying different flags so it cannot pass by accident. Falsified by mutation: enforcement never engaging, the write seam ignoring the ACL, the realm dropped from the scope, the revocation check removed, and the trie never installed each turn the intended tests red and no others. Full solution suite green. Claude-Session: https://claude.ai/code/session_015p7wGqy3YpZNCpDzTpGMKo
This commit is contained in:
@@ -0,0 +1,45 @@
|
||||
namespace ZB.MOM.WW.OtOpcUa.Core.Authorization;
|
||||
|
||||
/// <summary>
|
||||
/// <c>Security:Acl</c> — the switch that turns per-node ACL evaluation into the authorization
|
||||
/// decision on the OPC UA data plane (#520, stage 5).
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// <para>
|
||||
/// <b>Default OFF, and that is the whole point.</b> This is the dark-switch discipline
|
||||
/// <c>MeshTransport</c>, <c>ConfigSource</c> and <c>Telemetry</c> all shipped under: both sides
|
||||
/// are wired in both modes, so flipping it is a config change rather than a redeploy. Without it,
|
||||
/// the first deployment carrying this code would start denying every client that has no key —
|
||||
/// including <b>ScadaBridge's Data Connection Layer, which consumes this address space from
|
||||
/// another repo</b> and has never presented one.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// <b>What <see cref="Enabled"/> actually swaps.</b> With it off, the two long-standing server-wide
|
||||
/// role gates (<c>WriteOperate</c> on Value writes, <c>AlarmAck</c> on the Part 9 methods) remain
|
||||
/// the only authorization in the server and behave exactly as they always have; HistoryRead stays
|
||||
/// ungated. With it on, those role checks are <b>replaced</b> — not supplemented — by a scoped ACL
|
||||
/// decision, and HistoryRead gains one.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// ⚠️ <b>Replaced, not supplemented, is load-bearing.</b> Running both in series would mean a grant
|
||||
/// had to confer the server-wide role as well as pass the scope check, and only
|
||||
/// <c>Cluster</c>-scoped grants confer a role (see <c>ApiKeyInterimRoleMapper</c>) — so every
|
||||
/// narrow grant would still be inert and the flag would appear to do nothing. Narrow grants
|
||||
/// working is the entire reason this stage exists.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// Read, Browse and CreateMonitoredItems are still ungated in either mode — that is stage 6, and
|
||||
/// the AdminUI banners stay up until it lands.
|
||||
/// </para>
|
||||
/// </remarks>
|
||||
public sealed class AclOptions
|
||||
{
|
||||
/// <summary>The configuration section name.</summary>
|
||||
public const string SectionName = "Security:Acl";
|
||||
|
||||
/// <summary>
|
||||
/// When true, node ACL grants decide write, alarm-command and HistoryRead authorization. Default
|
||||
/// false, which preserves the pre-stage-5 role gates byte-for-byte.
|
||||
/// </summary>
|
||||
public bool Enabled { get; set; }
|
||||
}
|
||||
@@ -32,6 +32,7 @@ public sealed class DataPlaneAuthoritySnapshot
|
||||
IReadOnlyList<ApiKey> keys,
|
||||
IReadOnlyList<NodeAcl> grants,
|
||||
IReadOnlyDictionary<AclScopeRef, IReadOnlyList<NodeAclPath>> scopePaths,
|
||||
IReadOnlyList<PermissionTrieBuildDiagnostic> droppedGrants,
|
||||
IReadOnlyDictionary<string, ApiKey> keysById,
|
||||
IReadOnlyDictionary<string, IReadOnlyList<NodeAcl>> grantsByPrincipal)
|
||||
{
|
||||
@@ -41,6 +42,7 @@ public sealed class DataPlaneAuthoritySnapshot
|
||||
Keys = keys;
|
||||
Grants = grants;
|
||||
ScopePaths = scopePaths;
|
||||
DroppedGrants = droppedGrants;
|
||||
_keysById = keysById;
|
||||
_grantsByPrincipal = grantsByPrincipal;
|
||||
}
|
||||
@@ -76,6 +78,18 @@ public sealed class DataPlaneAuthoritySnapshot
|
||||
/// </summary>
|
||||
public IReadOnlyDictionary<AclScopeRef, IReadOnlyList<NodeAclPath>> ScopePaths { get; }
|
||||
|
||||
/// <summary>
|
||||
/// Grants the trie build discarded, with the reason. Each one is an authored rule that will never
|
||||
/// match anything, so this is the only signal an operator gets that a rule they wrote is inert.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// Reported by the build itself rather than re-derived by the caller. An earlier version of the
|
||||
/// install log counted unresolvable scopes independently, which agreed with the builder only for
|
||||
/// the <c>UnresolvedScope</c> reason — it silently missed rows dropped for a missing
|
||||
/// <c>ScopeId</c>, a retired scope kind, or a scope path that does not start at a realm.
|
||||
/// </remarks>
|
||||
public IReadOnlyList<PermissionTrieBuildDiagnostic> DroppedGrants { get; }
|
||||
|
||||
/// <summary>True once a real artifact has been applied.</summary>
|
||||
public bool HasArtifact => GenerationId > 0;
|
||||
|
||||
@@ -112,6 +126,7 @@ public sealed class DataPlaneAuthoritySnapshot
|
||||
/// <param name="keys">The API keys carried by the artifact.</param>
|
||||
/// <param name="grants">The ACL grants for this node's cluster.</param>
|
||||
/// <param name="scopePaths">Where each grant's scope sits in the address space.</param>
|
||||
/// <param name="droppedGrants">Grants the trie build discarded, or empty when no trie was built.</param>
|
||||
/// <returns>The built snapshot.</returns>
|
||||
public static DataPlaneAuthoritySnapshot Create(
|
||||
string? clusterId,
|
||||
@@ -119,7 +134,8 @@ public sealed class DataPlaneAuthoritySnapshot
|
||||
long generationId,
|
||||
IReadOnlyList<ApiKey> keys,
|
||||
IReadOnlyList<NodeAcl> grants,
|
||||
IReadOnlyDictionary<AclScopeRef, IReadOnlyList<NodeAclPath>> scopePaths)
|
||||
IReadOnlyDictionary<AclScopeRef, IReadOnlyList<NodeAclPath>> scopePaths,
|
||||
IReadOnlyList<PermissionTrieBuildDiagnostic>? droppedGrants = null)
|
||||
{
|
||||
ArgumentNullException.ThrowIfNull(revisionHash);
|
||||
ArgumentNullException.ThrowIfNull(keys);
|
||||
@@ -154,7 +170,7 @@ public sealed class DataPlaneAuthoritySnapshot
|
||||
StringComparer.OrdinalIgnoreCase);
|
||||
|
||||
return new DataPlaneAuthoritySnapshot(
|
||||
clusterId, revisionHash, generationId, keys, grants, scopePaths, byId, frozen);
|
||||
clusterId, revisionHash, generationId, keys, grants, scopePaths, droppedGrants ?? [], byId, frozen);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -207,11 +223,35 @@ public interface IDataPlaneAuthoritySink
|
||||
/// Process-singleton holder of <see cref="DataPlaneAuthoritySnapshot"/>. Registered on every
|
||||
/// driver-role node; the artifact-apply path writes it and OPC UA session activation reads it.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// <para>
|
||||
/// <b>It owns the <see cref="PermissionTrieCache"/> rather than being handed one</b> (#520 stage 5).
|
||||
/// The trie is built from the same grants and the same generation as the snapshot, inside the same
|
||||
/// <see cref="Apply"/> call, so the two cannot describe different deployments. Injecting the cache
|
||||
/// would have made "install the snapshot" and "install the trie" two wirings, and this repo has
|
||||
/// been bitten repeatedly by the second one silently not running — a trie that was never installed
|
||||
/// denies everything, which reads as "ACLs are working" until someone checks whether an ALLOW is
|
||||
/// possible at all.
|
||||
/// </para>
|
||||
/// </remarks>
|
||||
public sealed class DataPlaneAuthorityState : IDataPlaneAuthoritySource, IDataPlaneAuthoritySink
|
||||
{
|
||||
/// <summary>
|
||||
/// Generations of the trie retained per cluster. One is enough for correctness now — every
|
||||
/// authorization call stamps itself with the current generation — but an in-flight request that
|
||||
/// read the snapshot just before a deploy landed still resolves against the generation it saw.
|
||||
/// </summary>
|
||||
private const int RetainedTrieGenerations = 3;
|
||||
|
||||
private readonly Lock _gate = new();
|
||||
private DataPlaneAuthoritySnapshot _current = DataPlaneAuthoritySnapshot.Empty;
|
||||
|
||||
/// <summary>
|
||||
/// The permission tries built from the applied artifacts, keyed <c>(cluster, generation)</c>. Read
|
||||
/// by <see cref="TriePermissionEvaluator"/> on the authorization path.
|
||||
/// </summary>
|
||||
public PermissionTrieCache Tries { get; } = new();
|
||||
|
||||
/// <inheritdoc />
|
||||
public DataPlaneAuthoritySnapshot Current => Volatile.Read(ref _current);
|
||||
|
||||
@@ -244,10 +284,49 @@ public sealed class DataPlaneAuthorityState : IDataPlaneAuthoritySource, IDataPl
|
||||
? previous.GenerationId
|
||||
: previous.GenerationId + 1;
|
||||
|
||||
// Build and install the trie BEFORE publishing the snapshot. An authorization call that reads
|
||||
// the new snapshot must find its trie already in the cache; publishing first opens a window in
|
||||
// which a session stamps itself with a generation the cache has not got, and the evaluator
|
||||
// fails closed on exactly that condition. The window would be microseconds and only ever
|
||||
// reachable during a deploy — precisely the kind of defect that never reproduces.
|
||||
var dropped = BuildAndInstallTrie(clusterId, generation, grants, scopePaths);
|
||||
|
||||
var next = DataPlaneAuthoritySnapshot.Create(
|
||||
clusterId, revisionHash, generation, keys, grants, scopePaths);
|
||||
clusterId, revisionHash, generation, keys, grants, scopePaths, dropped);
|
||||
Volatile.Write(ref _current, next);
|
||||
return next;
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Builds the permission trie for a generation and installs it in <see cref="Tries"/>.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// A node with no cluster identity gets no trie. The trie is keyed by cluster and the evaluator
|
||||
/// matches the session's cluster against the node's, so there is no honest key to file it under;
|
||||
/// inventing a sentinel would silently apply one cluster's grants to a node whose cluster is
|
||||
/// unknown. With enforcement enabled such a node denies everything, which is the fail-closed
|
||||
/// direction and is surfaced by the caller's install log.
|
||||
/// </remarks>
|
||||
/// <param name="clusterId">The node's cluster, or null/blank when it has none.</param>
|
||||
/// <param name="generationId">The generation to file the trie under.</param>
|
||||
/// <param name="grants">The ACL grants to plant.</param>
|
||||
/// <param name="scopePaths">Where each grant's scope sits in the address space.</param>
|
||||
/// <returns>The grants the build discarded.</returns>
|
||||
private IReadOnlyList<PermissionTrieBuildDiagnostic> BuildAndInstallTrie(
|
||||
string? clusterId,
|
||||
long generationId,
|
||||
IReadOnlyList<NodeAcl> grants,
|
||||
IReadOnlyDictionary<AclScopeRef, IReadOnlyList<NodeAclPath>> scopePaths)
|
||||
{
|
||||
if (string.IsNullOrWhiteSpace(clusterId)) return [];
|
||||
|
||||
var diagnostics = new List<PermissionTrieBuildDiagnostic>();
|
||||
var trie = PermissionTrieBuilder.Build(
|
||||
clusterId, generationId, grants, scopePaths, diagnostics.Add);
|
||||
|
||||
Tries.Install(trie);
|
||||
Tries.Prune(clusterId, RetainedTrieGenerations);
|
||||
return diagnostics;
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user