fix(security): amortize LoginThrottle.Prune off the failed-bind hot path (plan R2-07 T11)
This commit is contained in:
@@ -41,6 +41,18 @@ public sealed class LoginThrottle
|
|||||||
/// </summary>
|
/// </summary>
|
||||||
public const int MaxTrackedKeys = 10_000;
|
public const int MaxTrackedKeys = 10_000;
|
||||||
|
|
||||||
|
/// <summary>Amortization cadence: a full prune sweep runs every Nth failure write
|
||||||
|
/// (plus immediately whenever the map exceeds <see cref="MaxTrackedKeys"/>), so a
|
||||||
|
/// spray does not pay an O(n) scan — and a possible full sort — on every failed
|
||||||
|
/// request (arch-review R2 N8). Lockout SEMANTICS are unaffected: IsLockedOut reads
|
||||||
|
/// window/lockout expiry directly and never depends on pruning.</summary>
|
||||||
|
internal const int PruneEveryNFailures = 64;
|
||||||
|
|
||||||
|
/// <summary>Diagnostics/test accessor: number of currently tracked keys.</summary>
|
||||||
|
internal int TrackedKeyCount => _entries.Count;
|
||||||
|
|
||||||
|
private int _failureWrites;
|
||||||
|
|
||||||
private readonly TimeProvider _clock;
|
private readonly TimeProvider _clock;
|
||||||
private readonly IOptions<SecurityOptions> _options;
|
private readonly IOptions<SecurityOptions> _options;
|
||||||
|
|
||||||
@@ -118,7 +130,13 @@ public sealed class LoginThrottle
|
|||||||
return Arm(existing with { Count = existing.Count + 1 }, opts, now, lockout);
|
return Arm(existing with { Count = existing.Count + 1 }, opts, now, lockout);
|
||||||
});
|
});
|
||||||
|
|
||||||
Prune(now);
|
// Amortized prune (R2 N8): every Nth failure, or immediately when over the hard
|
||||||
|
// cap — the cap is the memory-safety invariant and must never wait for the cadence.
|
||||||
|
if (Interlocked.Increment(ref _failureWrites) % PruneEveryNFailures == 0
|
||||||
|
|| _entries.Count > MaxTrackedKeys)
|
||||||
|
{
|
||||||
|
Prune(now);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/// <summary>
|
/// <summary>
|
||||||
|
|||||||
@@ -25,6 +25,12 @@ public class LoginThrottleTests
|
|||||||
private static LoginThrottle Create(TimeProvider clock, SecurityOptions? options = null) =>
|
private static LoginThrottle Create(TimeProvider clock, SecurityOptions? options = null) =>
|
||||||
new(clock, Options.Create(options ?? new SecurityOptions()));
|
new(clock, Options.Create(options ?? new SecurityOptions()));
|
||||||
|
|
||||||
|
private static (LoginThrottle throttle, TestTimeProvider clock) Create()
|
||||||
|
{
|
||||||
|
var clock = new TestTimeProvider(Start);
|
||||||
|
return (Create(clock), clock);
|
||||||
|
}
|
||||||
|
|
||||||
[Fact]
|
[Fact]
|
||||||
public void FiveFailuresInWindow_LocksOut_ThenWindowExpiryUnlocks()
|
public void FiveFailuresInWindow_LocksOut_ThenWindowExpiryUnlocks()
|
||||||
{
|
{
|
||||||
@@ -110,4 +116,29 @@ public class LoginThrottleTests
|
|||||||
|
|
||||||
Assert.False(throttle.IsLockedOut("alice", "10.0.0.1"));
|
Assert.False(throttle.IsLockedOut("alice", "10.0.0.1"));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// ── arch-review R2 N8: amortized prune ─────────────────────────────────────
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void KeySpray_MapStaysBounded_AtTheHardCap()
|
||||||
|
{
|
||||||
|
var (throttle, _) = Create(); // existing helper: throttle over a fake clock
|
||||||
|
// Spray far past the cap: the over-cap eviction must run on EVERY write once
|
||||||
|
// the cap is exceeded, regardless of the amortized cadence (arch-review R2 N8).
|
||||||
|
for (var i = 0; i < LoginThrottle.MaxTrackedKeys + 500; i++)
|
||||||
|
throttle.RecordFailure($"user{i}", "10.0.0.1");
|
||||||
|
Assert.True(throttle.TrackedKeyCount <= LoginThrottle.MaxTrackedKeys);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void ExpiredEntries_AreEventuallySwept_OnTheAmortizedCadence()
|
||||||
|
{
|
||||||
|
var (throttle, clock) = Create();
|
||||||
|
throttle.RecordFailure("stale", "10.0.0.1");
|
||||||
|
clock.Advance(TimeSpan.FromMinutes(10)); // window fully expired
|
||||||
|
for (var i = 0; i < LoginThrottle.PruneEveryNFailures; i++)
|
||||||
|
throttle.RecordFailure($"fresh{i}", "10.0.0.1"); // cadence tick fires within these
|
||||||
|
Assert.False(throttle.IsLockedOut("stale", "10.0.0.1"));
|
||||||
|
Assert.True(throttle.TrackedKeyCount <= LoginThrottle.PruneEveryNFailures); // "stale" swept
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user