fix(opcua): parallel UserName activations wait for a slot instead of being refused (#833)

A legitimate consumer opening several keyed sessions at once from one key
(e.g. a ScadaBridge site node behind one NAT IP on a None endpoint) must
never see BadUserAccessDenied for parallelism. BeginAsync now hands out at
most MaxFailedAttempts - recordedFailures verification slots per key;
further activations queue FIFO and are woken as slots free. A waiter is
refused only when the key locks out (never reaching the authenticator) or,
uncounted, after OpcUa:CredentialLockout:SlotWaitSeconds (default 10,
BadTimeout). Cancelled waiters are dequeued; pruning accounts for waiters.
The security property is unchanged: no more than MaxFailedAttempts
unverified guesses in progress-or-failed per window.
This commit is contained in:
Joseph Doherty
2026-11-11 02:43:38 -05:00
parent 61b9adeb9c
commit 2314a53842
9 changed files with 397 additions and 159 deletions
+4 -2
View File
@@ -240,8 +240,10 @@ Two things to know before relying on any of this:
keyed cert-thumbprint-else-peer-IP. `OpcUaApplicationHost.DisableSdkAuthenticationLockout` zeroes
`MaxFailedAuthenticationAttempts` on built AND loaded configs; `FailedCredentialLockout` (`OpcUa:CredentialLockout`,
5 / 60 s / 300 s) counts and refuses UserName activations only, keyed thumbprint+peer together. Do not re-enable the
SDK setting. The check RESERVES an in-flight slot (`TryBegin`/`Complete`/`Abandon`) — a check and a separate
record let a parallel burst through; recorded + in-flight ≥ max ⇒ refused. An expired lockout resets the count. On
SDK setting. The check takes a verification SLOT (`BeginAsync`/`Complete`/`Abandon`) — a check and a
separate record let a parallel burst through. At most `max − failures` verify at once; the rest WAIT (never refused
for parallelism — ScadaBridge opens many keyed sessions from one IP), refused only on lockout or, uncounted, after
`SlotWaitSeconds`. An expired lockout resets the count. On
`None` endpoints the key is effectively IP-only (NAT neighbours share it, UserName only). `docs/security.md`
§"Failed-credential lockout".
- **Keys reach a driver-only node inside the deployment artifact** (stage 4). `ConfigComposer` snapshots
+1
View File
@@ -76,6 +76,7 @@ Until #678.6 the overlay never loaded on a node carrying a `cluster-*` role (the
| `PeerApplicationUris` | list of string | `[]` (empty) | Partner node `ApplicationUri`s published in `Server.ServerArray` for redundancy discovery. See [`Redundancy.md`](Redundancy.md). |
| `CredentialLockout:MaxFailedAttempts` | int | `5` | Failed UserName (API-key) activations from one client before its UserName activations are refused `BadUserAccessDenied` (#833). `≤ 0` disables. **Never refuses Anonymous.** See [`security.md`](security.md#failed-credential-lockout-opcuacredentiallockout). |
| `CredentialLockout:FailureWindowSeconds` | int | `60` | A failure more than this long after the client's previous one starts a fresh count. Must be `> 0` while the lockout is enabled (validated). |
| `CredentialLockout:SlotWaitSeconds` | int | `10` | How long a UserName activation waits for one of its client's verification slots (at most `MaxFailedAttempts − failures` verify at once; the rest queue) before that one attempt fails `BadTimeout`, uncounted. Must be `> 0` while enabled (validated). |
| `CredentialLockout:LockoutSeconds` | int | `300` | How long a locked-out client's UserName activations are refused; an expired lockout starts a fresh count. Must be `> 0` while the lockout is enabled (validated). |
> **Transport security profiles** (the values in `EnabledSecurityProfiles` — `None`, `Basic256Sha256Sign`, `Basic256Sha256SignAndEncrypt`) and the PKI trust flow are documented in full in [`security.md`](security.md). This page does not duplicate them.
+11 -8
View File
@@ -338,19 +338,22 @@ The host now sets `MaxFailedAuthenticationAttempts = 0` on every configuration i
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 are held to the threshold.** The check reserves an in-flight slot under the same lock,
and a client whose recorded failures plus in-progress UserName activations already reach
`MaxFailedAttempts` is refused (`BadUserAccessDenied`, "too many authentication attempts in progress"), so a
burst cannot slip past the check before its first failure is recorded. ⚠️ The same rule caps a **legitimate**
client at `MaxFailedAttempts` concurrent UserName activations (the check is milliseconds, so this only bites a
burst of simultaneous session opens from one key).
- **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.
- **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`.)
- **Validated at host start:** with `MaxFailedAttempts > 0`, both `LockoutSeconds` and `FailureWindowSeconds` must be
`> 0` (`OpcUaApplicationHostOptionsValidator`); `MaxFailedAttempts <= 0` disables the lockout.
- **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
@@ -34,8 +34,9 @@ public sealed class OpcUaApplicationHostOptionsValidator : OptionsValidatorBase<
// nothing, a zero FailureWindowSeconds would never accumulate. MaxFailedAttempts <= 0 disables it.
var lockout = o.CredentialLockout;
builder.RequireThat(lockout is null || lockout.MaxFailedAttempts <= 0
|| (lockout.LockoutSeconds > 0 && lockout.FailureWindowSeconds > 0),
"OpcUa:CredentialLockout:LockoutSeconds and OpcUa:CredentialLockout:FailureWindowSeconds must both be "
|| (lockout.LockoutSeconds > 0 && lockout.FailureWindowSeconds > 0 && lockout.SlotWaitSeconds > 0),
"OpcUa:CredentialLockout:LockoutSeconds, OpcUa:CredentialLockout:FailureWindowSeconds and "
+ "OpcUa:CredentialLockout:SlotWaitSeconds must all be "
+ "greater than 0 while OpcUa:CredentialLockout:MaxFailedAttempts is greater than 0 (set "
+ "MaxFailedAttempts to 0 to disable the lockout)");
}
@@ -275,31 +275,35 @@ public sealed class OpcUaApplicationHost : IAsyncDisposable
var lockoutKey = tokenType == UserTokenType.UserName
? FailedCredentialLockout.KeyFor(context.ChannelCertificate?.Thumbprint, peer)
: null;
if (lockoutKey is not null
&& !host._credentialLockout.TryBegin(lockoutKey, out var refusal, out var remaining, out var firstRefusal))
if (lockoutKey is not null)
{
// Logged once per lockout episode at Information; every further refusal at Debug, so a client hammering
// a lockout cannot flood the log (#833 review).
var level = firstRefusal ? LogLevel.Information : LogLevel.Debug;
if (refusal == CredentialLockoutRefusal.LockedOut)
// Waits (bounded) for one of this client's verification slots — parallel activations queue, they are
// never refused for parallelism. Refused only when locked out, or (uncounted) when no slot freed in time.
var decision = await host._credentialLockout.BeginAsync(lockoutKey, ct).ConfigureAwait(false);
if (decision.Verdict == CredentialLockoutVerdict.LockedOut)
{
var seconds = (long)Math.Ceiling(remaining.TotalSeconds);
host._logger.Log(level,
// Logged once per lockout episode at Information; every further refusal at Debug, so a client
// hammering a lockout cannot flood the log (#833 review).
var seconds = (long)Math.Ceiling(decision.Remaining.TotalSeconds);
host._logger.Log(decision.FirstRefusal ? LogLevel.Information : LogLevel.Debug,
"OpcUaApplicationHost: UserName activation refused — client {Client} is locked out for {Seconds}s more",
lockoutKey, seconds);
return AuthenticationResult.Reject(ServiceResult.Create(StatusCodes.BadUserAccessDenied,
$"Too many failed authentication attempts. Try again in {seconds} seconds."));
}
host._logger.Log(level,
"OpcUaApplicationHost: UserName activation refused — client {Client} already has as many failed plus "
+ "in-progress attempts as OpcUa:CredentialLockout allows",
lockoutKey);
return AuthenticationResult.Reject(ServiceResult.Create(StatusCodes.BadUserAccessDenied,
"Too many authentication attempts in progress. Try again shortly."));
if (decision.Verdict == CredentialLockoutVerdict.SlotWaitTimedOut)
{
host._logger.LogWarning(
"OpcUaApplicationHost: UserName activation for client {Client} timed out waiting for a "
+ "verification slot (OpcUa:CredentialLockout:SlotWaitSeconds); not counted as a failure",
lockoutKey);
return AuthenticationResult.Reject(ServiceResult.Create(StatusCodes.BadTimeout,
"Timed out waiting to verify the credential. Try again."));
}
}
// The slot TryBegin reserved is released on every path — completed with the outcome, or abandoned
// The slot BeginAsync granted is released on every path — completed with the outcome, or abandoned
// (uncounted) when the activation was cancelled or threw.
var completed = false;
try
@@ -7,7 +7,7 @@ namespace ZB.MOM.WW.OtOpcUa.OpcUaServer.Security;
/// <remarks>
/// The defaults are the OPC Foundation SDK 2.0.1 <c>SessionManager</c> lockout this replaces: 5 failures,
/// each inside 60 s of the previous one, lock the client out for 5 minutes. When enabled
/// (<see cref="MaxFailedAttempts"/> &gt; 0) both durations must be positive — enforced at host start by
/// (<see cref="MaxFailedAttempts"/> &gt; 0) every duration must be positive — enforced at host start by
/// <c>OpcUaApplicationHostOptionsValidator</c>.
/// </remarks>
public sealed class OpcUaCredentialLockoutOptions
@@ -20,24 +20,37 @@ public sealed class OpcUaCredentialLockoutOptions
/// <summary>How long a locked-out client's UserName activations are refused, in seconds.</summary>
public int LockoutSeconds { get; set; } = 300;
}
/// <summary>Why <see cref="FailedCredentialLockout.TryBegin"/> refused an activation.</summary>
public enum CredentialLockoutRefusal
{
/// <summary>Not refused.</summary>
None,
/// <summary>The client is locked out.</summary>
LockedOut,
/// <summary>
/// The client's recorded failures plus its activations still being checked already reach the threshold, so
/// one more concurrent attempt could exceed it before any of them has been recorded.
/// How long a UserName activation waits for one of its client's verification slots before that one attempt
/// is failed (<c>BadTimeout</c>, NOT counted as a credential failure). Only reached when a client has more
/// verifications in progress than <see cref="MaxFailedAttempts"/> minus its recorded failures; a verification
/// is milliseconds, so in practice this bounds a pathological store stall. Kept well under the SDK's
/// operation timeout so the client gets this answer rather than a transport timeout.
/// </summary>
TooManyInFlight,
public int SlotWaitSeconds { get; set; } = 10;
}
/// <summary>The outcome of <see cref="FailedCredentialLockout.BeginAsync"/>.</summary>
public enum CredentialLockoutVerdict
{
/// <summary>A verification slot is held — authenticate, then call <see cref="FailedCredentialLockout.Complete"/>.</summary>
Proceed,
/// <summary>The client is locked out; refuse without authenticating.</summary>
LockedOut,
/// <summary>No slot freed within the wait; fail this one attempt, uncounted.</summary>
SlotWaitTimedOut,
}
/// <summary>What <see cref="FailedCredentialLockout.BeginAsync"/> decided.</summary>
/// <param name="Verdict">Proceed, locked out, or timed out waiting.</param>
/// <param name="Remaining">Time left on the lockout when <see cref="CredentialLockoutVerdict.LockedOut"/>.</param>
/// <param name="FirstRefusal">True for the first refusal of a lockout episode (log it at Information, the rest at Debug).</param>
public readonly record struct CredentialLockoutDecision(
CredentialLockoutVerdict Verdict, TimeSpan Remaining, bool FirstRefusal);
/// <summary>
/// Brute-force lockout for UserName (API-key) activations, scoped so it can never refuse an
/// <b>anonymous</b> activation (lmxopcua#833).
@@ -69,16 +82,22 @@ public enum CredentialLockoutRefusal
/// window); a successful UserName activation clears the client's count.
/// </para>
/// <para>
/// <b>Concurrency.</b> The authenticate call is awaited between the check and the outcome, so a check and a
/// separate record would let a burst of parallel activations all pass before the first failure lands.
/// <see cref="TryBegin"/> therefore <b>reserves</b> an in-flight slot under the same lock as the check, and
/// refuses while recorded failures + in-flight attempts already reach the threshold;
/// <see cref="Complete"/> releases the slot and records the outcome.
/// <b>Concurrency — parallelism waits, it is never refused.</b> The authenticate call is awaited between the
/// check and the outcome, so a check and a separate record would let a parallel burst of guesses all pass
/// before the first failure lands. <see cref="BeginAsync"/> therefore hands out at most
/// <c>MaxFailedAttempts − recordedFailures</c> verification <b>slots</b> per key; an activation beyond that
/// <b>waits</b> (FIFO) for a slot, and <see cref="Complete"/> / <see cref="Abandon"/> release it and wake the
/// next waiter. So no more than <c>MaxFailedAttempts</c> unverified guesses are ever in progress-or-failed
/// per window, while a legitimate client opening many keyed sessions at once (a ScadaBridge site node behind
/// one NAT IP) just queues for milliseconds. Waiters are refused only when the key actually locks out (they
/// never reach the authenticator) or after <see cref="OpcUaCredentialLockoutOptions.SlotWaitSeconds"/>
/// (uncounted); a cancelled waiter is removed from the queue.
/// </para>
/// <para>
/// Memory is bounded at <see cref="MaxTrackedClients"/>: expired, idle entries are pruned when it fills,
/// and a client that still finds it full is not tracked (it can only evade by rotating certificates or
/// addresses, which it could do against the SDK lockout too; API-key secrets are not guessable).
/// Memory is bounded at <see cref="MaxTrackedClients"/>: entries with no failures, slots or waiters are
/// pruned when it fills, and a client that still finds it full is not tracked (it can only evade by
/// rotating certificates or addresses, which it could do against the SDK lockout too; API-key secrets are
/// not guessable).
/// </para>
/// </remarks>
public sealed class FailedCredentialLockout
@@ -111,85 +130,95 @@ public sealed class FailedCredentialLockout
$"{(string.IsNullOrEmpty(certificateThumbprint) ? "-" : certificateThumbprint)}@{peer?.ToString() ?? "-"}";
/// <summary>
/// Checks <paramref name="key"/> and, when it may proceed, reserves an in-flight slot that the caller MUST
/// release with <see cref="Complete"/> (only when this returns <see langword="true"/>).
/// Acquires a verification slot for <paramref name="key"/>, waiting for one when the client already has as
/// many in progress as it may. On <see cref="CredentialLockoutVerdict.Proceed"/> the caller MUST release the
/// slot with <see cref="Complete"/> or <see cref="Abandon"/>.
/// </summary>
/// <param name="key">The client key from <see cref="KeyFor"/>.</param>
/// <param name="refusal">Why it was refused, when this returns false.</param>
/// <param name="remaining">The time left on the lockout when refused <see cref="CredentialLockoutRefusal.LockedOut"/>.</param>
/// <param name="firstRefusal">
/// True for the first refusal of this lockout episode (so the caller logs it once at Information and the
/// rest at Debug).
/// </param>
/// <returns><see langword="true"/> when the activation may be authenticated.</returns>
public bool TryBegin(string key, out CredentialLockoutRefusal refusal, out TimeSpan remaining, out bool firstRefusal)
/// <param name="ct">Cancels the wait; a cancelled waiter is dequeued and the exception propagates.</param>
/// <returns>The decision.</returns>
public async ValueTask<CredentialLockoutDecision> BeginAsync(string key, CancellationToken ct = default)
{
refusal = CredentialLockoutRefusal.None;
remaining = TimeSpan.Zero;
firstRefusal = false;
if (!Enabled) return true;
var now = _time.GetUtcNow();
if (!Enabled) return new(CredentialLockoutVerdict.Proceed, TimeSpan.Zero, false);
Waiter waiter;
Entry entry;
lock (_gate)
{
if (!_entries.TryGetValue(key, out var entry))
var now = _time.GetUtcNow();
if (!_entries.TryGetValue(key, out var existing))
{
if (_entries.Count >= MaxTrackedClients) Prune(now);
if (_entries.Count >= MaxTrackedClients) return true; // untracked: nothing to reserve
entry = new Entry();
_entries[key] = entry;
// Untracked when still full: it gets no slot accounting, as before (see remarks).
if (_entries.Count >= MaxTrackedClients) return new(CredentialLockoutVerdict.Proceed, TimeSpan.Zero, false);
existing = new Entry();
_entries[key] = existing;
}
entry = existing;
Refresh(entry, now);
if (entry.LockedUntil > now)
if (TryLockedOut(entry, now, out var refused)) return refused;
if (entry.Waiters.Count == 0 && HasFreeSlot(entry))
{
refusal = CredentialLockoutRefusal.LockedOut;
remaining = entry.LockedUntil - now;
firstRefusal = !entry.RefusalLogged;
entry.RefusalLogged = true;
return false;
entry.InFlight++;
return new(CredentialLockoutVerdict.Proceed, TimeSpan.Zero, false);
}
if (entry.Failures + entry.InFlight >= _options.MaxFailedAttempts)
waiter = new Waiter();
entry.Waiters.AddLast(waiter.Node = new LinkedListNode<Waiter>(waiter));
}
var wait = TimeSpan.FromSeconds(Math.Max(1, _options.SlotWaitSeconds));
try
{
return await waiter.Completion.Task.WaitAsync(wait, _time, ct).ConfigureAwait(false);
}
catch (Exception ex) when (ex is TimeoutException or OperationCanceledException)
{
lock (_gate)
{
refusal = CredentialLockoutRefusal.TooManyInFlight;
firstRefusal = !entry.RefusalLogged;
entry.RefusalLogged = true;
return false;
if (waiter.Completion.Task.IsCompletedSuccessfully)
{
// Decided in the same instant the wait gave up. A timeout takes the decision as made; a
// cancellation hands a granted slot back (waking the next waiter) so nothing leaks.
var decided = waiter.Completion.Task.Result;
if (ex is not OperationCanceledException) return decided;
if (decided.Verdict == CredentialLockoutVerdict.Proceed) ReleaseCore(key, entry, outcome: null);
}
else
{
entry.Waiters.Remove(waiter.Node!);
waiter.Completion.TrySetCanceled();
RemoveIfIdle(key, entry);
}
}
entry.InFlight++;
return true;
if (ex is OperationCanceledException) throw;
return new(CredentialLockoutVerdict.SlotWaitTimedOut, TimeSpan.Zero, false);
}
}
/// <summary>Releases the slot <see cref="TryBegin"/> reserved and records the activation's outcome.</summary>
/// <summary>Releases a slot from <see cref="BeginAsync"/> and records the activation's outcome.</summary>
/// <param name="key">The client key.</param>
/// <param name="succeeded">Whether the activation was admitted.</param>
/// <returns><see langword="true"/> when this failure locked the client out.</returns>
public bool Complete(string key, bool succeeded)
{
if (!Enabled) return false;
var now = _time.GetUtcNow();
lock (_gate)
{
if (!_entries.TryGetValue(key, out var entry)) return false; // was untracked at TryBegin
if (entry.InFlight > 0) entry.InFlight--;
return succeeded ? RecordSuccessCore(key, entry) : RecordFailureCore(entry, now);
return _entries.TryGetValue(key, out var entry) && ReleaseCore(key, entry, succeeded);
}
}
/// <summary>Releases a reserved slot without recording an outcome (the activation was cancelled or threw).</summary>
/// <summary>Releases a slot without recording an outcome (the activation was cancelled or threw).</summary>
/// <param name="key">The client key.</param>
public void Abandon(string key)
{
if (!Enabled) return;
lock (_gate)
{
if (_entries.TryGetValue(key, out var entry) && entry.InFlight > 0)
{
entry.InFlight--;
if (IsIdle(entry, _time.GetUtcNow())) _entries.Remove(key);
}
if (_entries.TryGetValue(key, out var entry)) ReleaseCore(key, entry, outcome: null);
}
}
@@ -212,7 +241,7 @@ public sealed class FailedCredentialLockout
}
}
/// <summary>Records one failed UserName activation with no reservation (test seam / direct callers).</summary>
/// <summary>Records one failed UserName activation with no slot (test seam / direct callers).</summary>
/// <param name="key">The client key.</param>
/// <returns><see langword="true"/> when this failure locked the client out.</returns>
public bool RecordFailure(string key)
@@ -228,17 +257,22 @@ public sealed class FailedCredentialLockout
entry = new Entry();
_entries[key] = entry;
}
return RecordFailureCore(entry, now);
var locked = RecordFailureCore(entry, now);
Pump(entry, now);
return locked;
}
}
/// <summary>Clears a client's count after a successful UserName activation (no reservation).</summary>
/// <summary>Clears a client's count after a successful UserName activation (no slot).</summary>
/// <param name="key">The client key.</param>
public void RecordSuccess(string key)
{
lock (_gate)
{
if (_entries.TryGetValue(key, out var entry)) RecordSuccessCore(key, entry);
if (!_entries.TryGetValue(key, out var entry)) return;
ClearCount(entry);
Pump(entry, _time.GetUtcNow());
RemoveIfIdle(key, entry);
}
}
@@ -248,16 +282,65 @@ public sealed class FailedCredentialLockout
get { lock (_gate) return _entries.Count; }
}
private bool RecordSuccessCore(string key, Entry entry)
/// <summary>How many activations are waiting for a slot on <paramref name="key"/> (test observation).</summary>
internal int WaitingCount(string key)
{
if (entry.InFlight == 0) _entries.Remove(key);
else
lock (_gate) return _entries.TryGetValue(key, out var e) ? e.Waiters.Count : 0;
}
private bool HasFreeSlot(Entry entry) => entry.Failures + entry.InFlight < _options.MaxFailedAttempts;
private bool TryLockedOut(Entry entry, DateTimeOffset now, out CredentialLockoutDecision refused)
{
refused = default;
if (entry.LockedUntil <= now) return false;
refused = new(CredentialLockoutVerdict.LockedOut, entry.LockedUntil - now, !entry.RefusalLogged);
entry.RefusalLogged = true;
return true;
}
/// <summary>Releases one slot, records <paramref name="outcome"/> (null = uncounted) and wakes waiters.</summary>
private bool ReleaseCore(string key, Entry entry, bool? outcome)
{
var now = _time.GetUtcNow();
if (entry.InFlight > 0) entry.InFlight--;
var lockedNow = false;
if (outcome == true) ClearCount(entry);
else if (outcome == false) lockedNow = RecordFailureCore(entry, now);
Pump(entry, now);
RemoveIfIdle(key, entry);
return lockedNow;
}
/// <summary>
/// Hands freed slots to waiters in order — or, when the key is locked out, refuses every waiter (they never
/// reach the authenticator). Called under the gate after anything that can free a slot or lock the key.
/// </summary>
private void Pump(Entry entry, DateTimeOffset now)
{
Refresh(entry, now);
while (entry.Waiters.First is { } first)
{
entry.Failures = 0;
entry.LockedUntil = default;
entry.RefusalLogged = false;
if (TryLockedOut(entry, now, out var refused))
{
entry.Waiters.RemoveFirst();
first.Value.Completion.TrySetResult(refused);
continue;
}
if (!HasFreeSlot(entry)) return;
entry.Waiters.RemoveFirst();
entry.InFlight++;
if (!first.Value.Completion.TrySetResult(new(CredentialLockoutVerdict.Proceed, TimeSpan.Zero, false)))
entry.InFlight--; // a waiter that already gave up never takes the slot
}
return false;
}
private static void ClearCount(Entry entry)
{
entry.Failures = 0;
entry.LockedUntil = default;
entry.RefusalLogged = false;
}
private bool RecordFailureCore(Entry entry, DateTimeOffset now)
@@ -281,9 +364,7 @@ public sealed class FailedCredentialLockout
{
if (entry.LockedUntil != default && entry.LockedUntil <= now)
{
entry.Failures = 0;
entry.LockedUntil = default;
entry.RefusalLogged = false;
ClearCount(entry);
}
else if (entry.LockedUntil == default && entry.Failures > 0
&& now - entry.LastFailure > TimeSpan.FromSeconds(Math.Max(0, _options.FailureWindowSeconds)))
@@ -293,19 +374,34 @@ public sealed class FailedCredentialLockout
}
}
private void RemoveIfIdle(string key, Entry entry)
{
if (IsIdle(entry, _time.GetUtcNow())) _entries.Remove(key);
}
private void Prune(DateTimeOffset now)
{
foreach (var key in _entries.Where(e => IsIdle(e.Value, now)).Select(e => e.Key).ToList())
_entries.Remove(key);
}
/// <summary>Idle = nothing to remember: no failures, no lockout, no slot held and nobody waiting.</summary>
private bool IsIdle(Entry entry, DateTimeOffset now)
{
Refresh(entry, now);
return entry.InFlight == 0 && entry.Failures == 0 && entry.LockedUntil == default;
return entry.InFlight == 0 && entry.Waiters.Count == 0 && entry.Failures == 0 && entry.LockedUntil == default;
}
/// <summary>One client's failure count, in-flight reservations and lockout. Mutated only under the gate.</summary>
/// <summary>One activation waiting for a slot.</summary>
private sealed class Waiter
{
public readonly TaskCompletionSource<CredentialLockoutDecision> Completion =
new(TaskCreationOptions.RunContinuationsAsynchronously);
public LinkedListNode<Waiter>? Node;
}
/// <summary>One client's failure count, slots, waiters and lockout. Mutated only under the gate.</summary>
private sealed class Entry
{
public int Failures;
@@ -313,5 +409,6 @@ public sealed class FailedCredentialLockout
public DateTimeOffset LastFailure;
public DateTimeOffset LockedUntil;
public bool RefusalLogged;
public readonly LinkedList<Waiter> Waiters = new();
}
}
@@ -79,4 +79,17 @@ public sealed class OpcUaApplicationHostOptionsValidatorTests
result.Succeeded.ShouldBe(valid);
if (!valid) result.Failures!.ShouldContain(f => f.Contains("OpcUa:CredentialLockout:LockoutSeconds"));
}
/// <summary>#833 review: the slot wait must be positive while the lockout is enabled.</summary>
[Fact]
public void Credential_lockout_slot_wait_must_be_positive_while_enabled()
{
var result = Sut.Validate(null, new OpcUaApplicationHostOptions
{
CredentialLockout = new ZB.MOM.WW.OtOpcUa.OpcUaServer.Security.OpcUaCredentialLockoutOptions { SlotWaitSeconds = 0 },
});
result.Failed.ShouldBeTrue();
result.Failures!.ShouldContain(f => f.Contains("OpcUa:CredentialLockout:SlotWaitSeconds"));
}
}
@@ -59,6 +59,29 @@ public sealed class CredentialLockoutWireTests
auth.UserNameCalls.ShouldBe(callsBefore, "a locked-out client's credential is refused without being checked");
}
/// <summary>
/// #833 review — one client opening more keyed sessions at once than MaxFailedAttempts (a ScadaBridge site
/// node behind one NAT IP) is never refused for parallelism: the extra activations wait for a slot.
/// </summary>
[Fact]
public async Task Parallel_valid_activations_beyond_the_threshold_all_succeed()
{
var ct = TestContext.Current.CancellationToken;
var auth = new CountingAuthenticator();
await using var rig = await Rig.StartAsync(auth, new OpcUaCredentialLockoutOptions { MaxFailedAttempts = 2 }, ct);
var sessions = await Task.WhenAll(Enumerable.Range(0, 8).Select(_ => rig.ConnectAsync(GoodKey, ct)));
try
{
sessions.ShouldAllBe(s => s.Connected);
auth.UserNameCalls.ShouldBe(8);
}
finally
{
foreach (var s in sessions) s.Dispose();
}
}
[Fact]
public async Task A_disabled_lockout_admits_a_valid_key_after_any_number_of_failures()
{
@@ -102,52 +102,166 @@ public sealed class FailedCredentialLockoutTests
lockout.TrackedCount.ShouldBe(1, "expired entries are pruned to make room");
}
private static CancellationToken Ct => TestContext.Current.CancellationToken;
/// <summary>
/// #833 review — the authenticate call is awaited between the check and the outcome. A burst of parallel
/// activations that all check before any failure is recorded must still be held to the threshold: the check
/// reserves an in-flight slot, so only MaxFailedAttempts may be in progress at once.
/// #833 review — a legitimate client opening many keyed sessions at once (a ScadaBridge site node behind one
/// NAT IP) must never be refused for parallelism: at most MaxFailedAttempts verify concurrently, the rest
/// WAIT for a slot, and all 20 succeed.
/// </summary>
[Fact]
public async Task A_parallel_burst_is_held_to_the_threshold_before_any_failure_is_recorded()
public async Task An_all_valid_parallel_burst_queues_for_slots_and_every_activation_succeeds()
{
var lockout = new FailedCredentialLockout(new OpcUaCredentialLockoutOptions { MaxFailedAttempts = 3 }, new ManualClock());
const int burst = 20;
using var allChecked = new CountdownEvent(burst);
var release = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
var admitted = 0;
var lockout = new FailedCredentialLockout(new OpcUaCredentialLockoutOptions { MaxFailedAttempts = 5 }, new ManualClock());
var concurrent = 0;
var maxConcurrent = 0;
var proceeded = 0;
var attempts = Enumerable.Range(0, burst).Select(attempt => Task.Run(async () =>
await Task.WhenAll(Enumerable.Range(0, 20).Select(attempt => Task.Run(async () =>
{
var ok = lockout.TryBegin(Client, out _, out _, out _);
if (ok) Interlocked.Increment(ref admitted);
allChecked.Signal();
await release.Task; // every attempt checks before any outcome lands — the race window
if (ok) lockout.Complete(Client, succeeded: false);
}, TestContext.Current.CancellationToken)).ToArray();
var decision = await lockout.BeginAsync(Client, Ct);
decision.Verdict.ShouldBe(CredentialLockoutVerdict.Proceed);
Interlocked.Increment(ref proceeded);
var now = Interlocked.Increment(ref concurrent);
InterlockedMax(ref maxConcurrent, now);
await Task.Delay(20, Ct); // the authenticate call
Interlocked.Decrement(ref concurrent);
lockout.Complete(Client, succeeded: true);
}, Ct)));
allChecked.Wait(TimeSpan.FromSeconds(10), TestContext.Current.CancellationToken).ShouldBeTrue();
proceeded.ShouldBe(20);
maxConcurrent.ShouldBe(5, "exactly MaxFailedAttempts verify at once — no more, and the cap is actually reached");
lockout.TrackedCount.ShouldBe(0, "nothing left behind once the burst drains");
}
/// <summary>
/// #833 review — the security half: a parallel burst of guesses that all arrive before any failure is
/// recorded still reaches the authenticator only MaxFailedAttempts times; once those fail, every waiter is
/// refused as locked out without being authenticated.
/// </summary>
[Fact]
public async Task An_all_bad_parallel_burst_authenticates_only_max_attempts_and_refuses_the_waiters_as_locked_out()
{
var lockout = new FailedCredentialLockout(new OpcUaCredentialLockoutOptions { MaxFailedAttempts = 5 }, new ManualClock());
var release = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
var authenticated = 0;
var lockedOut = 0;
var attempts = Enumerable.Range(0, 20).Select(attempt => Task.Run(async () =>
{
var decision = await lockout.BeginAsync(Client, Ct);
if (decision.Verdict == CredentialLockoutVerdict.LockedOut)
{
Interlocked.Increment(ref lockedOut);
return;
}
decision.Verdict.ShouldBe(CredentialLockoutVerdict.Proceed);
Interlocked.Increment(ref authenticated);
await release.Task; // hold the slots until all 15 others are queued behind them
lockout.Complete(Client, succeeded: false);
}, Ct)).ToArray();
await WaitUntilAsync(() => lockout.WaitingCount(Client) == 15);
authenticated.ShouldBe(5);
release.SetResult();
await Task.WhenAll(attempts);
admitted.ShouldBe(3, "only MaxFailedAttempts activations may be checked at once");
lockout.IsLockedOut(Client, out _).ShouldBeTrue("the three failures lock the client");
authenticated.ShouldBe(5, "no waiter may reach the authenticator once the key locks out");
lockedOut.ShouldBe(15);
lockout.IsLockedOut(Client, out _).ShouldBeTrue();
lockout.WaitingCount(Client).ShouldBe(0);
}
/// <summary>After a recorded failure the client has one slot fewer: capacity is MaxFailedAttempts − failures.</summary>
[Fact]
public async Task Recorded_failures_shrink_the_number_of_concurrent_verifications()
{
var lockout = new FailedCredentialLockout(new OpcUaCredentialLockoutOptions { MaxFailedAttempts = 3 }, new ManualClock());
lockout.RecordFailure(Client);
lockout.RecordFailure(Client);
(await lockout.BeginAsync(Client, Ct)).Verdict.ShouldBe(CredentialLockoutVerdict.Proceed);
var second = lockout.BeginAsync(Client, Ct).AsTask();
await WaitUntilAsync(() => lockout.WaitingCount(Client) == 1);
second.IsCompleted.ShouldBeFalse("one failure short of the threshold leaves exactly one slot");
lockout.Complete(Client, succeeded: true); // a success clears the count and hands the slot on
(await second).Verdict.ShouldBe(CredentialLockoutVerdict.Proceed);
lockout.Complete(Client, succeeded: true);
lockout.TrackedCount.ShouldBe(0);
}
/// <summary>A cancelled waiter is dequeued and leaves nothing behind (no leaked slot, entry prunable).</summary>
[Fact]
public async Task A_cancelled_waiter_is_removed_and_leaks_nothing()
{
var lockout = new FailedCredentialLockout(new OpcUaCredentialLockoutOptions { MaxFailedAttempts = 1 }, new ManualClock());
(await lockout.BeginAsync(Client, Ct)).Verdict.ShouldBe(CredentialLockoutVerdict.Proceed);
using var cts = CancellationTokenSource.CreateLinkedTokenSource(Ct);
var waiting = lockout.BeginAsync(Client, cts.Token).AsTask();
await WaitUntilAsync(() => lockout.WaitingCount(Client) == 1);
cts.Cancel();
await Should.ThrowAsync<OperationCanceledException>(waiting);
lockout.WaitingCount(Client).ShouldBe(0);
lockout.Complete(Client, succeeded: true);
lockout.TrackedCount.ShouldBe(0, "the held slot and the cancelled waiter are both gone");
(await lockout.BeginAsync(Client, Ct)).Verdict.ShouldBe(CredentialLockoutVerdict.Proceed);
lockout.Abandon(Client);
lockout.TrackedCount.ShouldBe(0);
}
/// <summary>A waiter that never gets a slot times out, uncounted — the client is not one failure closer.</summary>
[Fact]
public async Task A_slot_wait_timeout_fails_that_attempt_without_counting_it()
{
var lockout = new FailedCredentialLockout(
new OpcUaCredentialLockoutOptions { MaxFailedAttempts = 1, SlotWaitSeconds = 1 }, new ManualClock());
(await lockout.BeginAsync(Client, Ct)).Verdict.ShouldBe(CredentialLockoutVerdict.Proceed);
(await lockout.BeginAsync(Client, Ct)).Verdict.ShouldBe(CredentialLockoutVerdict.SlotWaitTimedOut);
lockout.WaitingCount(Client).ShouldBe(0);
lockout.Complete(Client, succeeded: true);
lockout.TrackedCount.ShouldBe(0, "the timed-out attempt recorded no failure");
}
[Fact]
public void In_flight_attempts_are_refused_as_TooManyInFlight_and_released_slots_free_up()
public async Task Only_the_first_refusal_of_a_lockout_episode_is_flagged_for_Information_logging()
{
var lockout = new FailedCredentialLockout(new OpcUaCredentialLockoutOptions { MaxFailedAttempts = 2 }, new ManualClock());
lockout.TryBegin(Client, out _, out _, out _).ShouldBeTrue();
lockout.TryBegin(Client, out _, out _, out _).ShouldBeTrue();
lockout.TryBegin(Client, out var refusal, out _, out var first).ShouldBeFalse();
refusal.ShouldBe(CredentialLockoutRefusal.TooManyInFlight);
first.ShouldBeTrue();
var clock = new ManualClock();
var lockout = new FailedCredentialLockout(new OpcUaCredentialLockoutOptions { MaxFailedAttempts = 1, LockoutSeconds = 10 }, clock);
lockout.RecordFailure(Client).ShouldBeTrue();
lockout.Abandon(Client); // a cancelled activation frees its slot, uncounted
lockout.Complete(Client, succeeded: true); // a success clears the count
lockout.TryBegin(Client, out _, out _, out _).ShouldBeTrue();
lockout.Complete(Client, succeeded: true);
lockout.TrackedCount.ShouldBe(0);
var r1 = await lockout.BeginAsync(Client, Ct);
r1.Verdict.ShouldBe(CredentialLockoutVerdict.LockedOut);
r1.FirstRefusal.ShouldBeTrue();
(await lockout.BeginAsync(Client, Ct)).FirstRefusal.ShouldBeFalse();
clock.Advance(TimeSpan.FromSeconds(11));
(await lockout.BeginAsync(Client, Ct)).Verdict.ShouldBe(CredentialLockoutVerdict.Proceed);
lockout.Complete(Client, succeeded: false).ShouldBeTrue("a new episode");
(await lockout.BeginAsync(Client, Ct)).FirstRefusal.ShouldBeTrue("each lockout episode logs its first refusal once");
}
private static void InterlockedMax(ref int target, int value)
{
int seen;
while ((seen = Volatile.Read(ref target)) < value && Interlocked.CompareExchange(ref target, value, seen) != seen)
{
}
}
private static async Task WaitUntilAsync(Func<bool> condition)
{
var deadline = DateTime.UtcNow + TimeSpan.FromSeconds(10);
while (!condition())
{
DateTime.UtcNow.ShouldBeLessThan(deadline, "condition not reached in time");
await Task.Delay(5, Ct);
}
}
/// <summary>
@@ -170,26 +284,6 @@ public sealed class FailedCredentialLockoutTests
lockout.RecordFailure(Client).ShouldBeTrue();
}
[Fact]
public void Only_the_first_refusal_of_a_lockout_episode_is_flagged_for_Information_logging()
{
var clock = new ManualClock();
var lockout = new FailedCredentialLockout(new OpcUaCredentialLockoutOptions { MaxFailedAttempts = 1, LockoutSeconds = 10 }, clock);
lockout.RecordFailure(Client).ShouldBeTrue();
lockout.TryBegin(Client, out var r1, out _, out var first1).ShouldBeFalse();
r1.ShouldBe(CredentialLockoutRefusal.LockedOut);
first1.ShouldBeTrue();
lockout.TryBegin(Client, out _, out _, out var first2).ShouldBeFalse();
first2.ShouldBeFalse();
clock.Advance(TimeSpan.FromSeconds(11));
lockout.TryBegin(Client, out _, out _, out _).ShouldBeTrue();
lockout.Complete(Client, succeeded: false).ShouldBeTrue("a new episode");
lockout.TryBegin(Client, out _, out _, out var first3).ShouldBeFalse();
first3.ShouldBeTrue("each lockout episode logs its first refusal once");
}
[Fact]
public void The_host_switches_the_sdk_lockout_off()
{