fix(security): wrap the ConfigDb key store on admin nodes + registration guard

This commit is contained in:
Joseph Doherty
2026-08-20 09:45:08 -04:00
parent 6048257dfe
commit 39a10a277a
7 changed files with 100 additions and 9 deletions
@@ -67,9 +67,20 @@ public static class DataPlaneAuthRegistration
// NOT from IClusterRoleInfo: that depends on the ActorSystem, and the same options-not-services
// rule is what keeps the redundancy-singleton registration in Program.cs from recursing into
// ActorSystem construction. Null on a node with no cluster-{ClusterId} role ⇒ no filter.
services.AddSingleton<IApiKeyStore>(sp => new ConfigDbApiKeyStore(
sp.GetRequiredService<IDbContextFactory<OtOpcUaConfigDbContext>>(),
ConfigDbApiKeyStore.ResolveClusterId(sp.GetRequiredService<IOptions<AkkaClusterOptions>>().Value)));
//
// arch-review 03/S18 — and it is WRAPPED. The lookup runs inline in the SDK's synchronous
// impersonation callback, under a SessionManager-wide lock, with CancellationToken.None: an
// unanswered ConfigDb query therefore stalls every session activation in the server. The
// decorator adds the deadline that bounds that, plus a short TTL that turns a burst of
// activations into one round trip. It cannot admit anything the inner store did not.
services.AddSingleton<IApiKeyStore>(sp => new BoundedApiKeyStore(
new ConfigDbApiKeyStore(
sp.GetRequiredService<IDbContextFactory<OtOpcUaConfigDbContext>>(),
ConfigDbApiKeyStore.ResolveClusterId(
sp.GetRequiredService<IOptions<AkkaClusterOptions>>().Value)),
sp.GetRequiredService<IOptions<ApiKeyOptions>>(),
sp.GetRequiredService<ILoggerFactory>().CreateLogger<BoundedApiKeyStore>(),
sp.GetRequiredService<TimeProvider>()));
services.AddSingleton<IOpcUaUserAuthenticator, ApiKeyOpcUaUserAuthenticator>();
// Minting surface for the AdminUI. Same node condition as the store above: both need a ConfigDb.
@@ -18,9 +18,18 @@ namespace ZB.MOM.WW.OtOpcUa.Host.OpcUa;
/// <b>This runs inline on session activation.</b> The SDK raises <c>ImpersonateUser</c>
/// synchronously while holding a SessionManager-wide event lock, so a slow authenticator does not
/// stall one request — it serializes every session activation server-wide. That is why
/// verification is a single indexed lookup plus one HMAC, and why there is no directory call,
/// no retry, and no network I/O on this path at all. It is the main practical advantage of keys
/// over the LDAP bind this replaces.
/// verification is a single indexed lookup plus one HMAC, with no directory call and no retry —
/// the main practical advantage of keys over the LDAP bind this replaces.
/// </para>
/// <para>
/// ⚠️ <b>"No network I/O" is TRUE ONLY ON A DRIVER-ONLY NODE</b> (arch-review 03/S18). The claim
/// describes <see cref="ArtifactApiKeyStore"/>, which is two dictionary probes on an in-memory
/// snapshot. On an <b>admin or fused</b> node <see cref="IApiKeyStore"/> is
/// <see cref="ConfigDbApiKeyStore"/>, and a UserName activation costs <b>two</b> EF round trips
/// (the key row, then the principal's <c>NodeAcl</c> rows) — inside this same lock. What bounds
/// that is <see cref="BoundedApiKeyStore"/>, which wraps it in production; the deadline it applies
/// surfaces here as an ordinary throw and is denied by the catch-all below, like any other store
/// failure.
/// </para>
/// <para>
/// <b>Fail closed, and say little.</b> Every rejection returns the same opaque message. The
@@ -81,6 +81,18 @@ public sealed class BoundedApiKeyStore : IApiKeyStore
_time = time;
}
/// <summary>The store being bounded.</summary>
/// <remarks>
/// Exposed so the composition root's wiring can be asserted. A decorator registered in one branch
/// of a role conditional is the "built but never wired" shape, and a guard that cannot see what is
/// inside the wrapper cannot tell a correctly-wired node from one wrapping the wrong store.
/// </remarks>
public IApiKeyStore Inner => _inner;
/// <summary>The clock this bound's deadline and TTL are measured on.</summary>
/// <remarks>Same reason as <see cref="Inner"/>: the shared-clock invariant is only pinnable by reference.</remarks>
public TimeProvider Clock => _time;
/// <inheritdoc />
/// <remarks>
/// Keyed by <paramref name="apiKeyId"/> alone. <paramref name="utcNow"/> is deliberately not part
@@ -14,8 +14,19 @@ namespace ZB.MOM.WW.OtOpcUa.Host.OpcUa;
/// <b>A driver-only node has no ConfigDb since per-cluster mesh Phase 4</b>, so this store cannot be
/// registered there; that node uses <see cref="ArtifactApiKeyStore"/> instead (#520 stage 4).
/// <para>
/// Reading per request is what makes revocation immediate here, which is why this store is
/// preferred wherever a ConfigDb exists — including on a fused admin+driver node.
/// Reading per activation is what makes revocation prompt here, which is why this store is
/// preferred wherever a ConfigDb exists — including on a fused admin+driver node. ⚠️ It reaches a
/// <b>new session activation</b> only: an already-open session re-checks its key against the
/// in-memory artifact snapshot on every operation, not against this store, so clearing
/// <c>Enabled</c> here does not stop an open session until the next deployment. See
/// <c>docs/security.md</c>.
/// </para>
/// <para>
/// ⚠️ <b>In production this store is WRAPPED by <see cref="BoundedApiKeyStore"/></b> (arch-review
/// 03/S18), because these queries run inline in the SDK's impersonation callback under a
/// SessionManager-wide lock. A call may therefore be cancelled by the wrapper's deadline even
/// though the caller passed <see cref="CancellationToken.None"/> — the resulting
/// <see cref="OperationCanceledException"/> is a denial, not a bug.
/// </para>
/// <para>
/// ⚠️ <b>The ConfigDb holds the WHOLE fleet's ACL rows, not just this node's cluster's.</b> That is