diff --git a/archreview/plans/R3-05-acl-hot-path-expiry-plan.md b/archreview/plans/R3-05-acl-hot-path-expiry-plan.md index d917512f..89f6ddf0 100644 --- a/archreview/plans/R3-05-acl-hot-path-expiry-plan.md +++ b/archreview/plans/R3-05-acl-hot-path-expiry-plan.md @@ -910,3 +910,91 @@ Gate rows deleted (`NodeAcl` row, the `expiry-probe` key, nothing else had been recreated **without** the override (`Security__Acl__Enabled` absent from both env lists, i.e. default `false`), parity `b344565cad3d` ×6 re-asserted, readiness back, final deploy `1c666be3…` Sealed, anonymous read of `HR100` answers a value again. Rig left UP on the master image with `DisableLogin=true`. + +## S18 hard-deadline follow-up (2026-08-20) + +The gate's S18 finding is **fixed on `fix/r3-05-s18-hard-deadline`** (branched off master `9242630c`), plus the +compose recipe defect the same leg exposed. Not yet live re-gated — the paused-SQL leg has to be re-run. + +### What was wrong + +`BoundedApiKeyStore.BoundAsync` bounded the inner store with a +`CancellationTokenSource(TimeSpan, TimeProvider)` linked into the call. **That bounds the call only if the +callee chooses to observe the token**, and `Microsoft.Data.SqlClient` does not while a connect or login is +in flight on a black-holed socket. So the deadline never returned control: the gate's keyed activation ran +**42.9 s** against a 2000 ms bound, the *"did not answer within 2000 ms"* Warning was emitted only at +unpause, and because the whole thing sits inside the SDK's synchronous `SessionManager`-lock-holding +impersonation callback, an unrelated already-open subscription stalled **57 s** behind it. The T3 TTL cache +cannot help: a miss is exactly what the outage produces. + +### Red before green — the observation, not just the claim + +Every pre-existing deadline test drives `GatedApiKeyStore`, whose waits take the token +(`Task.Delay(Timeout.Infinite, ct)`, `gate.WaitAsync(ct)`). A cooperative `CancelAfter` unblocks those, so +**all 15 of them passed against the defective build** — the suite could not see the gate's failure at all. +The new fake, `UncooperativeApiKeyStore`, ignores `ct` entirely and completes only when the test releases +it, modelling SqlClient. Against the unmodified `BoundedApiKeyStore` the three new tests fail — with a +test-side `WaitAsync(2 s)`, so the red is a **failure, not a hang**: + +``` +Failed! - Failed: 3, Passed: 15, Skipped: 0, Total: 18 + An_uncancellable_store_is_abandoned_at_the_deadline [FAIL] + An_abandoned_lookup_that_completes_later_is_not_cached [FAIL] + An_abandoned_lookup_that_faults_later_is_observed_and_swallowed [FAIL] +Shouldly.ShouldAssertException : … should throw System.OperationCanceledException but threw System.TimeoutException +``` + +`TimeoutException` rather than `OperationCanceledException` **is** the finding, restated as an assertion: +with a 200 ms bound the decorator had still not returned 2 s later. + +### The fix + +`BoundAsync` now races the inner call against `Task.Delay(timeout, _time, …)` — the injected clock, so the +deadline stays on the same `TimeProvider` as the authorizer and the revalidation timer — with +`Task.WhenAny`: + +- **Inner wins** ⇒ stop the delay timer, dispose, `await inner` (faults and a genuine *caller* cancellation + propagate exactly as before). +- **Delay wins** ⇒ cancel the linked token anyway (a *cooperative* inner store should still stop), hand the + abandoned task to `ObserveAbandoned`, meter `outcome=timeout`, log the (now honest) Warning, and throw + `OperationCanceledException` — the same exception the authenticator's catch-all already maps to the + standard opaque denial. **Fail-closed, unchanged: nothing here can admit.** +- `ObserveAbandoned` attaches a continuation that disposes the CTS, **touches `finished.Exception`** (the + observation itself, so a late SqlClient fault can never become an `UnobservedTaskException`), and logs at + Debug. Debug rather than Warning because the timeout is already reported at Warning — a ConfigDb outage + should not produce two lines per activation for one event. +- **An abandoned answer is never cached.** It was taken before the deadline and would be served after it, + so a key revoked or expired in the meantime could be admitted from a lookup nobody was still waiting for. + `Store(...)` is simply unreachable on the timeout path; the test asserts it by call count (a cached late + answer would make the next lookup a hit and leave `Calls` at 1). +- `StoreTimeoutMs ≤ 0` still short-circuits to the unbounded call, byte-for-byte — the documented rollback. + +Files: `src/Server/ZB.MOM.WW.OtOpcUa.Host/OpcUa/BoundedApiKeyStore.cs`, +`tests/Server/ZB.MOM.WW.OtOpcUa.Host.Tests/OpcUa/BoundedApiKeyStoreTests.cs`, `docs/security.md` +§"Bounding the key store". Host.Tests **137/137 green** (was 134; +3). + +### Recipe defect fixed: `OTOPCUA_ACL_ENABLED` never reached the centrals + +The gate's own correction note, actioned. `Security__Acl__Enabled: "${OTOPCUA_ACL_ENABLED:-false}"` was +wired to **site-a-1 / site-a-2 only**, so +`OTOPCUA_ACL_ENABLED=true docker compose up -d central-1 central-2` was a silent no-op — on the one node +kind that carries `BoundedApiKeyStore` at all (only a fused admin+driver node wraps `ConfigDbApiKeyStore`). +The key is now on **central-1 and central-2** too, same env var, **same default-`false`**, so today's rig +behaviour is unchanged and the plan's step 1 does what it says. site-b stays hardcoded OFF as the A/B +control. + +### Live re-gate — still owed + +Re-run **step 8's paused-SQL leg only** (setup steps 2-4 unchanged, and step 1 now needs no scratch +override). Expected, versus what the gate saw: + +| Observation | Gate (defective) | Expected now | +|---|---|---| +| Keyed `read` connect with SQL paused | 42.9 s, client `BadRequestTimeout` | denies in ≈ `StoreTimeoutMs` (2 s) | +| *"did not answer within 2000 ms"* Warning | at unpause, ~40 s late | at the deadline, while SQL is still paused | +| Already-open subscription on the same node | 57 s gap | no gap beyond the ~2 s of the activation | +| `apikey_store_bounded{outcome="timeout"}` | 1, ~40 s late | 1, promptly | +| Falsification control (pause with **no** activation) | streamed clean | must still stream clean | + +⚠️ The Debug abandonment line is not visible at the rig's default log level — it is not part of the gate's +pass criteria; the Warning and the restored subscription are. diff --git a/docker-dev/docker-compose.yml b/docker-dev/docker-compose.yml index 5ea2f7bf..e1d5542f 100644 --- a/docker-dev/docker-compose.yml +++ b/docker-dev/docker-compose.yml @@ -403,6 +403,13 @@ services: Security__Ldap__ServiceAccountPassword: "serviceaccount123" Security__DeployApiKey: "docker-dev-deploy-key" Security__Auth__DisableLogin: "${OTOPCUA_DISABLE_LOGIN:-true}" + # #520 stage 6 — node ACL enforcement, the dark switch. Default OFF, exactly as in production. + # The centrals read the SAME env override as the site-a pair, so an ACL live gate can enable it + # where BoundedApiKeyStore actually lives (only a fused admin+driver node wraps ConfigDbApiKeyStore): + # OTOPCUA_ACL_ENABLED=true docker compose -f docker-dev/docker-compose.yml up -d central-1 central-2 + # Added 2026-08-20: the R3-05 gate found this key wired only to site-a-1/2, so that command was a + # silent no-op on the centrals and the gate needed a scratch compose override to proceed. + Security__Acl__Enabled: "${OTOPCUA_ACL_ENABLED:-false}" # Map the shared-GLAuth data-plane groups (ReadOnly/WriteOperate/...) to data-plane role # strings the OPC UA session ACL gates read. The DB LdapGroupRoleMapping is AdminRole-typed # (AdminUI roles); this free-form GroupToRole baseline is the path for data-plane roles. @@ -529,6 +536,10 @@ services: Security__Ldap__ServiceAccountPassword: "serviceaccount123" Security__DeployApiKey: "docker-dev-deploy-key" Security__Auth__DisableLogin: "${OTOPCUA_DISABLE_LOGIN:-true}" + # Node ACL enforcement, the dark switch — same env override and same default-OFF as central-1. + # Both centrals must carry it: `up -d central-1 central-2` recreates both, and a pair split across + # two enforcement postures is not a rig anyone can reason about. + Security__Acl__Enabled: "${OTOPCUA_ACL_ENABLED:-false}" # Data-plane group→role baseline (see central-1). Security__Ldap__GroupToRole__ReadOnly: "ReadOnly" Security__Ldap__GroupToRole__WriteOperate: "WriteOperate"