fix(opcua): credential lockout counts per credential identity, ignores transient denials (#844, #845)
#844: a success cleared the whole per-client count, so a client holding a valid key could make four guesses at another key id, sign in with its own, and repeat forever. Failures are now charged to the (client, credential identity) pair -- IOpcUaUserAuthenticator.DescribeCredential, key:<keyId> for API keys -- and to the client as a whole. A success clears only its own identity's count; the per-identity threshold locks that identity; a new per-client cap (OpcUa:CredentialLockout:MaxFailedAttemptsPerClient, default 2 x MaxFailedAttempts) that a success never resets bounds spraying across key ids and locks every credential. Slots are bounded on both counts; a waiter whose identity has room is not held behind one whose identity has none. #845: (1) store timeouts, miss-budget refusals, store/authenticator exceptions, an unset pepper and cancellation are reported Transient (OpcUaUserAuthResult.Unavailable) and released uncounted, so a ConfigDb stall can no longer lock every keyed client out; the client still sees the same opaque denial. (2) a full 4096-entry table defers its next prune until the earliest entry could age out instead of rescanning under the lock per failure. (3) ApplicationUri deliberately NOT added to the key: on a None channel it is a free string, so it would let an attacker reset the count per guess -- documented. (4) wire tests: distinct client certificates on a Sign endpoint are locked out separately; the ApplicationConfigPath branch keeps the SDK lockout off (falsified by removing the loaded-branch disable).
This commit is contained in:
+64
-27
@@ -331,37 +331,74 @@ The host now sets `MaxFailedAuthenticationAttempts = 0` on every configuration i
|
||||
(`OpcUaApplicationHost.DisableSdkAuthenticationLockout`) and enforces a narrower replacement,
|
||||
`FailedCredentialLockout`, in its identity-registry adapter:
|
||||
|
||||
- **UserName activations only.** A failed one counts; at `MaxFailedAttempts` (5) within
|
||||
`FailureWindowSeconds` (60) of each other, that client's **UserName** activations are refused
|
||||
`BadUserAccessDenied` for `LockoutSeconds` (300) without the key store being consulted, and a Warning is
|
||||
logged once. A refusal during the lockout is not counted (it never extends itself); an **expired lockout
|
||||
always starts a fresh count**, whatever `FailureWindowSeconds` is; a successful UserName activation clears the
|
||||
count. **Anonymous activations never consult it**, from any client. The first refusal of each lockout episode
|
||||
is logged at Information, the rest at Debug.
|
||||
- **Parallel activations wait — they are never refused for parallelism.** A client gets at most
|
||||
`MaxFailedAttempts − recordedFailures` verification **slots**; a UserName activation beyond that **queues** (FIFO)
|
||||
until one frees, so no more than `MaxFailedAttempts` unverified guesses are ever in progress-or-failed per window,
|
||||
while a legitimate consumer opening many keyed sessions at once (e.g. a ScadaBridge site node behind one NAT IP on a
|
||||
`None` endpoint) just waits milliseconds and is admitted. A waiter is refused only when the key actually **locks
|
||||
out** (`BadUserAccessDenied`, without reaching the authenticator), or after `SlotWaitSeconds` (default 10)
|
||||
with `BadTimeout` — **not** counted as a credential failure. A cancelled activation leaves the queue and frees
|
||||
any slot it held.
|
||||
- **UserName activations only.** **Anonymous activations never consult it**, from any client.
|
||||
- **Two counts per client (#844).** Each *definite* failure is charged to the (client, **credential identity**)
|
||||
pair and to the client as a whole. The credential identity is `IOpcUaUserAuthenticator.DescribeCredential` —
|
||||
for API keys `key:<keyId>` (the public lookup index, never the secret), for anything that is not a well-formed key
|
||||
`user:<username>`.
|
||||
- **Per credential:** at `MaxFailedAttempts` (5) failures of one identity, each within `FailureWindowSeconds` (60) of
|
||||
the previous, that client is refused **that identity** `BadUserAccessDenied` for `LockoutSeconds` (300), without
|
||||
the key store being consulted. A success with **that same identity** clears its count.
|
||||
- **Per client:** at `MaxFailedAttemptsPerClient` failures across **every** identity (default `0` ⇒
|
||||
`2 × MaxFailedAttempts` = 10), the client is refused **every** UserName activation for `LockoutSeconds`.
|
||||
⚠️ **A success never resets this count** — it only ages out through `FailureWindowSeconds` or an expired lockout.
|
||||
- Why both: before #844 one count per client was cleared by *any* success, so a client holding a valid key could make
|
||||
four guesses at another key id, sign in with its own key, and repeat forever. The per-credential count alone would
|
||||
fix that but would let a client spray four guesses at each of unboundedly many key ids; the client-wide cap that a
|
||||
success cannot reset bounds the spray (≤ 10 unverified guesses per window chain, then 5 minutes locked), while a
|
||||
legitimate client with one stale/revoked key and one good key loses only the stale one.
|
||||
- Timing per count: a refusal during a lockout is not counted (it never extends itself); an **expired lockout always
|
||||
starts a fresh count**, whatever `FailureWindowSeconds` is (an expired client-wide lockout clears every identity's
|
||||
count too). A Warning is logged when a count locks (naming the scope and identity); the first refusal of each
|
||||
lockout episode is logged at Information, the rest at Debug. Identities per client are bounded at 64; further ones
|
||||
share one overflow count (conservative — never laxer).
|
||||
- **Only a definite bad credential counts (#845).** A wrong secret, an unknown/disabled/expired key id, a malformed
|
||||
credential or the wrong username counts. A denial that says nothing about the credential does **not**: a key-store
|
||||
timeout (`Security:ApiKeys:StoreTimeoutMs`), a miss-budget refusal, any other store exception, an unset pepper, an
|
||||
authenticator that throws, or a cancelled activation. The authenticator reports those with
|
||||
`OpcUaUserAuthResult.Unavailable(...)` (`Transient = true`) and the host releases the slot uncounted — so a ConfigDb
|
||||
stall can no longer lock every keyed client out for 5 minutes. The client sees the same opaque
|
||||
`BadIdentityTokenRejected` either way (no oracle).
|
||||
- **Parallel activations wait — they are never refused for parallelism.** An activation gets a verification
|
||||
**slot** only while `identityFailures + identityInFlight < MaxFailedAttempts` **and**
|
||||
`clientFailures + clientInFlight < MaxFailedAttemptsPerClient`; beyond that it **queues** until one frees (a queued
|
||||
activation whose identity has room is not held behind one whose identity has none). So no more unverified guesses
|
||||
than either threshold are ever in progress-or-failed per window, while a legitimate consumer opening many keyed
|
||||
sessions at once (e.g. a ScadaBridge site node behind one NAT IP on a `None` endpoint) just waits milliseconds and is
|
||||
admitted. A waiter is refused only when what it needs actually **locks out** (`BadUserAccessDenied`, without reaching
|
||||
the authenticator), or after `SlotWaitSeconds` (default 10) with `BadTimeout` — **not** counted as a failure. A
|
||||
cancelled activation leaves the queue and frees any slot it held.
|
||||
- **Keyed on certificate thumbprint *and* peer address together.** That separates clients only when they present
|
||||
a certificate. ⚠️ On a `None` endpoint clients commonly present none, so the key is **effectively the IP alone**:
|
||||
NAT neighbours there **do** share one count (for UserName activations only — never for anonymous ones). When the
|
||||
transport reports no peer address at all, every certificate-less client shares the single key `-@-`. (Observed on
|
||||
docker-dev: every host client arrives as `-@192.168.97.1`.)
|
||||
a certificate — on a `Sign`/`SignAndEncrypt` endpoint the certificate is proven, and two clients behind one address
|
||||
with different certificates are locked out separately (pinned over the wire). ⚠️ On a `None` endpoint clients
|
||||
commonly present none, so the key is **effectively the IP alone**: NAT neighbours there **do** share the client-wide
|
||||
count (for UserName activations only — never for anonymous ones; the per-credential count is shared only by
|
||||
neighbours presenting the same key id). When the transport reports no peer address at all, every certificate-less
|
||||
client shares the single key `-@-`. (Observed on docker-dev: every host client arrives as `-@192.168.97.1`.)
|
||||
- **The client `ApplicationUri` is deliberately NOT part of the key (#845, decided).** It would separate NAT
|
||||
neighbours on a `None` endpoint, but there it is a free, unauthenticated string in `CreateSession` (and the
|
||||
presented certificate is unproven), so an attacker would reset both counts simply by sending a new `ApplicationUri`
|
||||
with every guess — turning the lockout off for exactly the client it exists to stop. Keying on it is safe only
|
||||
where the certificate is proven, and there the thumbprint already separates clients. Operators who need NAT
|
||||
neighbours isolated should enable a signed endpoint (each client then presents its own certificate), not widen the key.
|
||||
- **Validated at host start:** with `MaxFailedAttempts > 0`, `LockoutSeconds`, `FailureWindowSeconds` and
|
||||
`SlotWaitSeconds` must all be `> 0` (`OpcUaApplicationHostOptionsValidator`); `MaxFailedAttempts <= 0` disables
|
||||
the lockout.
|
||||
- Every denial counts, including a key-store timeout or miss-budget refusal (as the SDK's did) — a ConfigDb
|
||||
outage can therefore lock a keyed client out for up to `LockoutSeconds` after recovery.
|
||||
- Bounded at 4096 tracked clients (expired entries are pruned; a client arriving at a full table is not
|
||||
tracked). API-key secrets are not guessable, so this is defence in depth, not the brute-force control.
|
||||
`SlotWaitSeconds` must all be `> 0`, and `MaxFailedAttemptsPerClient` must be `0` or `>= MaxFailedAttempts`
|
||||
(`OpcUaApplicationHostOptionsValidator`); `MaxFailedAttempts <= 0` disables the lockout.
|
||||
- **Bounded at 4096 tracked clients.** Entries with nothing left to remember are pruned when the table fills; a client
|
||||
arriving at a still-full table is not tracked. When a prune frees nothing, the next one is deferred until the earliest
|
||||
entry could have aged out (#845) — a flood of new client keys at a full table used to rescan all 4096 entries under the
|
||||
global lock on every failure. A release that frees an entry re-arms pruning immediately. API-key secrets are not
|
||||
guessable, so this is defence in depth, not the brute-force control.
|
||||
- **The SDK lockout is off on both configuration paths** — the configuration the host builds and one loaded from
|
||||
`OpcUa:ApplicationConfigPath` (whose own `MaxFailedAuthenticationAttempts` is overwritten with `0`; pinned over the
|
||||
wire with a file carrying `2`).
|
||||
|
||||
Pinned over the wire by `CredentialLockoutWireTests` (OpcUaServer.IntegrationTests — anonymous admitted after
|
||||
six failures; a valid key refused while locked out; `MaxFailedAttempts = 0` admits after seven failures,
|
||||
proving the SDK lockout is off) and unit-tested by `FailedCredentialLockoutTests`.
|
||||
twelve failures; a valid key refused while client-wide locked out; own-key successes do not launder guesses at another
|
||||
key id; transient denials never lock; distinct client certificates on a `Sign` endpoint are locked out separately; the
|
||||
`ApplicationConfigPath` branch keeps the SDK lockout off; `MaxFailedAttempts = 0` admits after seven failures,
|
||||
proving the SDK lockout is off) and unit-tested by `FailedCredentialLockoutTests`, `OpcUaApplicationHostImpersonationTests`
|
||||
and `ApiKeyAuthenticationTests` (Host.Tests).
|
||||
|
||||
### LDAP bind flow (`OtOpcUaLdapAuthService`)
|
||||
|
||||
|
||||
Reference in New Issue
Block a user