docs(security): correct revocation latency (activation vs open session) + S18/S19 knobs

This commit is contained in:
Joseph Doherty
2026-08-20 10:04:03 -04:00
parent dfab7f614d
commit a9b592aeb8
5 changed files with 129 additions and 21 deletions
+28 -4
View File
@@ -814,7 +814,12 @@ Two things to know before relying on any of this:
item is queued `BadUserAccessDenied` and then **detached from its `MonitoredNode2` fan-out — NOT set item is queued `BadUserAccessDenied` and then **detached from its `MonitoredNode2` fan-out — NOT set
`MonitoringMode.Disabled`**: disabling discards the item's queued notifications, so the client is `MonitoringMode.Disabled`**: disabling discards the item's queued notifications, so the client is
silenced without ever being told why (caught only by the over-the-wire test). Detaching is also silenced without ever being told why (caught only by the over-the-wire test). Detaching is also
reversible, so restoring the grant resumes the feed with no client action. reversible, so restoring the grant resumes the feed with no client action. ⚠️ **`Changed` is no longer
the only trigger (arch-review 03/S19):** `AclRevalidationTimer`, armed by `OtOpcUaServerHostedService`
at `Security:Acl:RevalidationIntervalSeconds` (default 60 s, `0` disables, inert while
`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.
- **Anonymous is a normal grantable principal**, not a bypass. `OpcUa:AllowAnonymous` (default `true`, - **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 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 reserved principal `anonymous` and gets roles from `NodeAcl` rows naming it, arriving at the gates as
@@ -826,9 +831,28 @@ Two things to know before relying on any of this:
deliberately *not* in the artifact — and `DeploymentArtifact.ParseDataPlaneAuthority` decodes them deliberately *not* in the artifact — and `DeploymentArtifact.ParseDataPlaneAuthority` decodes them
node-side alongside the cluster's `NodeAcl` rows. `OpcUaPublishActor` installs both on node-side alongside the cluster's `NodeAcl` rows. `OpcUaPublishActor` installs both on
`IDataPlaneAuthoritySource` on every apply, **before** its `plan.IsEmpty` early-return, so a deployment `IDataPlaneAuthoritySource` on every apply, **before** its `plan.IsEmpty` early-return, so a deployment
that only revokes a key still lands. A node with a ConfigDb keeps `ConfigDbApiKeyStore` (per-request that only revokes a key still lands. A node with a ConfigDb keeps `ConfigDbApiKeyStore` (per-activation
read); a driver-only node gets `ArtifactApiKeyStore`. ⚠️ **The consequence is asymmetric revocation: read, wrapped — see below); a driver-only node gets `ArtifactApiKeyStore`. ⚠️ **The asymmetry is
immediate on a ConfigDb node, next-deployment on a driver-only one.** ACTIVATION-scoped, not session-scoped — this line used to say "immediate on a ConfigDb node" and that was
wrong.** Clearing `Enabled` in the ConfigDb stops the **next session activation** on an admin/fused node
(bounded by `Security:ApiKeys:StoreCacheSeconds`, default 5 s) and takes until the next deployment on a
driver-only one — but an **already-open** session re-checks its key against the in-memory *artifact
snapshot* (`DataPlaneAuthorizer.Evaluate` → `snapshot.FindUsableKey`), never against `IApiKeyStore`, so on
**both** node kinds an `Enabled` revocation reaches an open session only at the next deployment.
**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.
- ⚠️ **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
store — and only it, never `ArtifactApiKeyStore`, which is two in-memory dictionary probes — with
`Security:ApiKeys:StoreTimeoutMs` (2000; exceeding it **denies**, via the authenticator's existing
catch-all) plus a `StoreCacheSeconds` (5) read-through cache. Faults and timeouts are never cached, a
cached hit whose `ExpiresUtc` has passed is re-read rather than served, and a timeout logs a distinct
Warning + `otopcua.opcua.apikey_store.bounded{outcome=timeout}`. Both knobs at `0` restore the old
behaviour exactly.
- ⚠️ **The auth generation is a node-local COUNTER driven by the `RevisionHash`, not derived from its - ⚠️ **The auth generation is a node-local COUNTER driven by the `RevisionHash`, not derived from its
bits.** `PermissionTrieCache` treats the highest `GenerationId` as current, so a generation taken from bits.** `PermissionTrieCache` treats the highest `GenerationId` as current, so a generation taken from
hash bytes would install a *lower* number about half the time and the cache would go on serving the hash bytes would install a *lower* number about half the time and the cache would go on serving the
@@ -730,3 +730,22 @@ for the orchestrator.
from the integration test project to `ZB.MOM.WW.OtOpcUa.Host` (the timer lives there). Without it the from the integration test project to `ZB.MOM.WW.OtOpcUa.Host` (the timer lives there). Without it the
test would have called the node manager's pass directly and could not have falsified T8. test would have called the node manager's pass directly and could not have falsified T8.
- OpcUaServer.IntegrationTests: 20 green (was 18). - OpcUaServer.IntegrationTests: 20 green (was 18).
### T10 — docs
- `docs/security.md` store table: the "Revocation takes effect" column is now **two** columns — new
activation vs already-open session — with an explicit ⚠️ paragraph saying the old single column was
wrong and why (an open session re-checks the artifact snapshot, never `IApiKeyStore`), and that expiry
is the exception on both node kinds.
- `docs/security.md` new subsection **"Bounding the key store"** — the lock hazard, both knobs, what a
timeout looks like in the log, the counter to watch, why a timeout denies, and why faults are never
cached.
- `docs/security.md` revocation bullets: split into the **deployment axis** and the **clock axis**; the
live-subscription bullet now names **two triggers**, the timer's key and default, the silenced-not-closed
semantics with re-activation recovery, and the event-monitored-item residual.
- `CLAUDE.md`: the "asymmetric revocation" sentence corrected to activation-scoped (naming the old wording
as wrong), a new bullet for the S18 bound, and the stage-6 "Revocation reaches live subscriptions" bullet
extended with the second trigger and the pass-local-memo constraint.
- **Beyond the plan's file list:** `docs/Configuration.md` gained rows for all three new keys. It carries
the field-by-field tables for `Security:ApiKeys` and `Security:Acl`, so shipping the knobs without them
would have been the exact doc drift this plan's cross-cutting requirement is about.
@@ -76,7 +76,7 @@
{ {
"id": "T10", "id": "T10",
"subject": "Docs: docs/security.md store table + revocation bullets, and the CLAUDE.md 'asymmetric revocation' sentence (activation vs already-open session)", "subject": "Docs: docs/security.md store table + revocation bullets, and the CLAUDE.md 'asymmetric revocation' sentence (activation vs already-open session)",
"status": "pending", "status": "done",
"blockedBy": [ "blockedBy": [
"T5", "T5",
"T8" "T8"
+3
View File
@@ -79,12 +79,15 @@ The checked-in `appsettings*.json` files are deliberately thin: they carry only
|---|---|---|---| |---|---|---|---|
| `Username` | string | `apikey` | The fixed username a key-bearing client sends in its OPC UA UserName token. Carries no information — the key identifies the principal — but is checked rather than ignored so a typo is an explicit denial. | | `Username` | string | `apikey` | The fixed username a key-bearing client sends in its OPC UA UserName token. Carries no information — the key identifies the principal — but is checked rather than ignored so a typo is an explicit denial. |
| `Pepper` | string | `""` | Server-side pepper for the stored key hashes. **Supply via `Security__ApiKeys__Pepper` or `${secret:}` — never commit it.** No default on purpose: an empty pepper still hashes "successfully", so `ApiKeySecret.ComputeHash` throws on one rather than producing a deployment with none of the protection it believes it has. Rotating it invalidates every existing key. | | `Pepper` | string | `""` | Server-side pepper for the stored key hashes. **Supply via `Security__ApiKeys__Pepper` or `${secret:}` — never commit it.** No default on purpose: an empty pepper still hashes "successfully", so `ApiKeySecret.ComputeHash` throws on one rather than producing a deployment with none of the protection it believes it has. Rotating it invalidates every existing key. |
| `StoreTimeoutMs` | int | `2000` | Hard per-call deadline on the API-key store lookup that runs inline on session activation (arch-review 03/S18). Exceeding it **denies** the activation, logs a distinct Warning and meters `otopcua.opcua.apikey_store.bounded{outcome=timeout}`. `≤ 0` disables the deadline — do not, on a node with a ConfigDb: the SDK holds a `SessionManager`-wide lock across that lookup, so an unanswered query stalls **every** session activation on the server. Applies only to `ConfigDbApiKeyStore`; the in-memory artifact store on a driver-only node is not wrapped. |
| `StoreCacheSeconds` | int | `5` | TTL of the read-through cache in front of that store, so a burst of activations costs one round trip. `0` disables it (a store read per activation). Bounds how long an `Enabled` revocation authored in the ConfigDb takes to reach a **new** activation; it has no effect on already-open sessions (those re-check the artifact snapshot) and cannot delay an `ExpiresUtc` — a cached hit that has since expired is re-read rather than served. |
**`Security:Acl`** key fields (`AclOptions`): **`Security:Acl`** key fields (`AclOptions`):
| Key | Type | Default | Meaning | | Key | Type | Default | Meaning |
|---|---|---|---| |---|---|---|---|
| `Enabled` | bool | `false` | `true` makes scoped `NodeAcl` grants decide **write**, **alarm-command** and **HistoryRead** authorization, *replacing* the server-wide `WriteOperate` / `AlarmAck` role checks. `false` keeps those role checks and leaves HistoryRead ungated — byte-for-byte the pre-stage-5 behaviour. Read, Browse and Subscribe are ungated either way. | | `Enabled` | bool | `false` | `true` makes scoped `NodeAcl` grants decide **write**, **alarm-command** and **HistoryRead** authorization, *replacing* the server-wide `WriteOperate` / `AlarmAck` role checks. `false` keeps those role checks and leaves HistoryRead ungated — byte-for-byte the pre-stage-5 behaviour. Read, Browse and Subscribe are ungated either way. |
| `RevalidationIntervalSeconds` | int | `60` | How often live monitored items are re-authorized against the clock, in addition to on every deployment (arch-review 03/S19). A key's `ExpiresUtc` passes with the clock, so without this an expired key kept feeding a live subscription until a deployment happened to land — indefinitely on a fleet deployed once and left running. `0` disables the timer (deploy-triggered only). **Inert while `Enabled` is false**, since the pass early-returns when enforcement is off. An affected session is silenced with `BadUserAccessDenied`, **not closed**, and recovers on re-activation. |
> **Flipping `Security:Acl:Enabled` is a config change, not a redeploy** — both paths are wired in both modes (the node builds its permission trie on every deploy regardless), the same dark-switch discipline as [`MeshTransport`](#meshtransport), [`ConfigSource`](#configsource) and [`Telemetry`](#telemetry). It defaults off because the first deployment carrying the code would otherwise start denying every client that has no key, **including ScadaBridge's Data Connection Layer, which consumes this address space from another repo.** Enforcement fails closed on every unresolvable case, so a wiring or config mistake presents as *everything denied* — check the `OpcUaPublish: data-plane authority generation N installed` line for a non-zero grant count before debugging an individual rule. See [`security.md` § Node ACL enforcement](security.md#node-acl-enforcement-securityaclenabled). > **Flipping `Security:Acl:Enabled` is a config change, not a redeploy** — both paths are wired in both modes (the node builds its permission trie on every deploy regardless), the same dark-switch discipline as [`MeshTransport`](#meshtransport), [`ConfigSource`](#configsource) and [`Telemetry`](#telemetry). It defaults off because the first deployment carrying the code would otherwise start denying every client that has no key, **including ScadaBridge's Data Connection Layer, which consumes this address space from another repo.** Enforcement fails closed on every unresolvable case, so a wiring or config mistake presents as *everything denied* — check the `OpcUaPublish: data-plane authority generation N installed` line for a non-zero grant count before debugging an individual rule. See [`security.md` § Node ACL enforcement](security.md#node-acl-enforcement-securityaclenabled).
+78 -16
View File
@@ -177,16 +177,59 @@ When no authenticator is supplied, `OpcUaApplicationHost` falls back to `NullOpc
the Host wires the real `ApiKeyOpcUaUserAuthenticator` as a singleton in `Program.cs`. Which the Host wires the real `ApiKeyOpcUaUserAuthenticator` as a singleton in `Program.cs`. Which
**`IApiKeyStore`** backs it depends on the node's role: **`IApiKeyStore`** backs it depends on the node's role:
| Node | Store | Revocation takes effect | | Node | Store | An `Enabled = 0` revocation reaches a **new activation** | …and an **already-open session** |
|---|---|---| |---|---|---|---|
| `admin` (including fused `admin,driver`) | `ConfigDbApiKeyStore` — reads the config database per request | On the **next request** | | `admin` (including fused `admin,driver`) | `ConfigDbApiKeyStore` — reads the config database per activation, wrapped in `BoundedApiKeyStore` | On the **next activation**, bounded by `Security:ApiKeys:StoreCacheSeconds` (default 5 s) | On the **next deployment** |
| `driver`-only | `ArtifactApiKeyStore` — reads the keys carried by the deployment artifact, held in memory | On the **next deployment** | | `driver`-only | `ArtifactApiKeyStore` — reads the keys carried by the deployment artifact, held in memory | On the **next deployment** | On the **next deployment** |
| neither role | `NullOpcUaUserAuthenticator` — denies every UserName token | n/a | | neither role | `NullOpcUaUserAuthenticator` — denies every UserName token | n/a | n/a |
⚠️ **The last column is the correction, and it used to be missing.** This table once said a ConfigDb node
revokes "on the next request", which is true only of a **new session activation**. An operation on a
session that is *already open* does not re-read `IApiKeyStore` at all — `DataPlaneAuthorizer.Evaluate`
re-checks the key against the in-memory **artifact snapshot** (`DataPlaneAuthority.FindUsableKey`), which
`OpcUaPublishActor` installs on every apply. So clearing `Enabled` in the ConfigDb stops the *next*
connect on an admin node and nothing else until a deployment lands, on **both** node kinds.
**Expiry is the exception, and it is immediate on both.** `FindUsableKey` compares `ExpiresUtc` against
the authorizer's clock on every call, so an `ExpiresUtc` that has already passed denies at once on an
open session — and since arch-review 03/S19 it reaches a live **subscription** too, on a timer (see
[Data-Plane Authorization](#data-plane-authorization)).
The deny-all is registered **explicitly**, not left absent: `OtOpcUaServerHostedService` takes The deny-all is registered **explicitly**, not left absent: `OtOpcUaServerHostedService` takes
`IOpcUaUserAuthenticator` as a required constructor parameter, so an unregistered seam fails DI and the `IOpcUaUserAuthenticator` as a required constructor parameter, so an unregistered seam fails DI and the
node does not boot at all. node does not boot at all.
### Bounding the key store (`StoreTimeoutMs` / `StoreCacheSeconds`)
The SDK raises impersonation **synchronously, holding a `SessionManager`-wide event lock**, so the key
lookup does not stall one activation — it serializes **every** session activation on the server,
Anonymous and X.509 included. On a driver-only node that is harmless (the artifact store is two
dictionary probes). On an admin or fused node it is two EF round trips to central SQL, so a ConfigDb that
is *slow or hung* rather than *refusing* could hold that lock indefinitely. `BoundedApiKeyStore`
(`src/Server/ZB.MOM.WW.OtOpcUa.Host/OpcUa/BoundedApiKeyStore.cs`) wraps the ConfigDb store — and only it —
with two `Security:ApiKeys` knobs:
| Key | Default | Meaning |
|---|---|---|
| `StoreTimeoutMs` | `2000` | Hard per-call deadline on the store. Exceeding it **denies** the activation. `≤ 0` disables the deadline (the pre-R3-05 unbounded behaviour — do not do this on an admin node). |
| `StoreCacheSeconds` | `5` | TTL of a read-through cache in front of the store, so a burst of activations costs one round trip. `0` disables it (a read per activation). |
- **A timeout denies; it never admits.** The deadline surfaces as an `OperationCanceledException` into
`ApiKeyOpcUaUserAuthenticator`'s existing catch-all, which returns the same opaque denial as a bad
credential. There is no second denial path to get wrong, and the decorator can only ever return what
the inner store returned or throw.
- **What a timeout looks like:** a distinct Warning — *"ApiKeyStore: the key store did not answer within
N ms — denying this activation. Every OPC UA session activation on this server is serialized behind
this call; check ConfigDb health"* — plus
`otopcua.opcua.apikey_store.bounded{outcome=timeout}`. It is deliberately **not** the ordinary
*"no enabled, unexpired key with id X"* line, which would send an operator hunting for a key problem
that does not exist.
- **What to watch:** `otopcua.opcua.apikey_store.bounded{outcome=cache_hit}`. A busy fused central with no
`cache_hit` stream means `StoreCacheSeconds` is 0.
- **Faults and timeouts are never cached** — caching a deny would extend an outage past the outage. Only
a completed lookup (a hit or an authoritative miss) is stored, and a cached hit whose `ExpiresUtc` has
since passed is re-read rather than served, so the TTL can never resurrect an expired key.
### LDAP bind flow (`OtOpcUaLdapAuthService`) ### LDAP bind flow (`OtOpcUaLdapAuthService`)
LDAP is configured under the `Security:Ldap` section (bound to `LdapOptions`, `src/Server/ZB.MOM.WW.OtOpcUa.Security/Ldap/LdapOptions.cs`, `SectionName = "Security:Ldap"`). The app authenticator is `OtOpcUaLdapAuthService` — a thin wrapper around the shared `ZB.MOM.WW.Auth.Ldap` directory client that adds two app-only concerns the shared library deliberately does not model: the `Enabled` master switch and `DevStubMode`. Since #520 it serves the **Admin UI cookie login (`/auth/login`) only** — `LdapOpcUaUserAuthenticator` is deleted and the OPC UA UserName path authenticates API keys instead (see [API-key identity](#api-key-identity-opc-ua-data-plane)). LDAP is configured under the `Security:Ldap` section (bound to `LdapOptions`, `src/Server/ZB.MOM.WW.OtOpcUa.Security/Ldap/LdapOptions.cs`, `SectionName = "Security:Ldap"`). The app authenticator is `OtOpcUaLdapAuthService` — a thin wrapper around the shared `ZB.MOM.WW.Auth.Ldap` directory client that adds two app-only concerns the shared library deliberately does not model: the `Enabled` master switch and `DevStubMode`. Since #520 it serves the **Admin UI cookie login (`/auth/login`) only** — `LdapOpcUaUserAuthenticator` is deleted and the OPC UA UserName path authenticates API keys instead (see [API-key identity](#api-key-identity-opc-ua-data-plane)).
@@ -411,17 +454,36 @@ Behaviours worth knowing before enabling it:
- **A deploy takes effect on sessions that are already open.** The gate stamps each decision with the - **A deploy takes effect on sessions that are already open.** The gate stamps each decision with the
current authority generation rather than the one the session activated under, so an ACL edit lands current authority generation rather than the one the session activated under, so an ACL edit lands
without bouncing clients — in both directions, widening and narrowing. without bouncing clients — in both directions, widening and narrowing.
- **Revocation also reaches an open session.** Every decision re-checks the authenticating key against - **Revocation also reaches an open session — but read the two axes separately.** Every decision
the current snapshot, so a key disabled or expired by a later deployment stops working immediately re-checks the authenticating key against the current snapshot, so an open session is re-authorized on
rather than at the client's next reconnect. On a driver-only node the artifact is the only revocation every operation. What that check can *see* differs by axis:
channel there is, so without this a revoked credential could keep its authority indefinitely. - **The deployment axis** (a key's `Enabled` cleared, a grant edited or removed). The snapshot only
- **Revocation reaches a live subscription too** (#520 stage 6). Every other operation re-asks on each changes when an artifact is applied, so this lands **on the next deployment**, on both node kinds —
request; a subscription is authorized once and then streams, so it needed a push. A deployment including on a fused admin node, whose ConfigDb store is consulted at *activation* only. On a
re-authorizes every live monitored item: one whose grant has gone is queued a `BadUserAccessDenied` driver-only node the artifact is the only revocation channel there is.
and then detached from its node's value fan-out, and a later deployment that restores the grant - **The clock axis** (a key's `ExpiresUtc` passing). `FindUsableKey` compares it against the
re-attaches it. This matters more than it sounds — subscriptions are how machine consumers actually authorizer's clock on every call, so this lands **immediately**, with no deployment.
read this server, so without it revoking a grant would close the operations nobody uses and leave the - **Revocation reaches a live subscription too** (#520 stage 6, extended by arch-review 03/S19). Every
feed running. other operation re-asks on each request; a subscription is authorized once and then streams, so it
needed a push. A denied item is queued a `BadUserAccessDenied` and then detached from its node's value
fan-out; restoring its authority re-attaches it. There are **two triggers**:
- a **deployment**, which re-authorizes every live monitored item (the deployment axis);
- the host's **low-frequency timer**, `Security:Acl:RevalidationIntervalSeconds` (default 60 s, `0`
disables it, inert while `Security:Acl:Enabled` is false). Without it a key that expired by the
clock kept streaming until a deployment happened to land — **indefinitely** on a fleet that has been
deployed once and left running, which is exactly the shape a production fleet settles into.
⚠️ **An expired-key session is silenced, not disconnected.** Its items go `BadUserAccessDenied` and the
session stays open, deliberately: a clock-driven denial must look to a client exactly like a
deploy-driven one, OPC UA has no honest "expire this session" verb (deleting it server-side gives the
client a transport failure rather than an authorization status), and every other operation on that
session already denies. **A client that re-activates with a renewed key recovers on the next tick with
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.
- **An ancestor of a granted node is browsable**, or no narrow grant would be reachable: a client - **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 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 what this necessarily discloses — the *names* of the folders on the path to a granted node are visible