diff --git a/docs/Authentication.md b/docs/Authentication.md index bdae026..eb37cb7 100644 --- a/docs/Authentication.md +++ b/docs/Authentication.md @@ -138,7 +138,9 @@ would otherwise let any peer throttle a key it does not hold — and it joins th partition only when the presented token is validly shaped (`mxgw__`, key id at most 64 characters), with at most 32 key-id partitions per address before the overflow collapses onto that address's fallback -partition. An over-limit state admits one probe per +partition. That peer-layer collapse does not exempt a validly shaped key id from +the cross-peer aggregate: its failures still feed and consult the key-id aggregate, +whose own cardinality is bounded by the configured LRU cap. An over-limit state admits one probe per `ApiKeyFailureProbeIntervalSeconds` through to the real verifier and refuses everything else with `ResourceExhausted` before the store read, so a legitimate holder presenting the correct secret always reaches the constant-time compare and diff --git a/src/ZB.MOM.WW.MxGateway.Server/Security/Authorization/ApiKeyFailureLimiter.cs b/src/ZB.MOM.WW.MxGateway.Server/Security/Authorization/ApiKeyFailureLimiter.cs index 42118c8..6269f66 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/Security/Authorization/ApiKeyFailureLimiter.cs +++ b/src/ZB.MOM.WW.MxGateway.Server/Security/Authorization/ApiKeyFailureLimiter.cs @@ -28,8 +28,11 @@ namespace ZB.MOM.WW.MxGateway.Server.Security.Authorization; /// /// The tracked partition set is a bounded LRU (). /// Because only a validly shaped token mints a key-id partition and each transport peer may mint at -/// most of them (the overflow collapses onto that address's -/// fallback partition), the map bounds memory and resists a junk-token flush (SEC-32): +/// most of them (the peer-layer overflow collapses onto that +/// address's fallback partition), the map bounds memory and resists a junk-token flush +/// (SEC-32). A validly shaped presented key id still feeds its cross-peer aggregate when its peer +/// layer uses the fallback, so exhausting the per-peer cap cannot bypass distributed-spray +/// protection; the aggregate map remains bounded by the same capped eviction policy: /// eviction prefers fully expired windows, never removes an over-limit partition below a transient /// overshoot ceiling of twice the cap, and only then falls back to least-recently-active. Successful /// peers are removed on reset, so in steady state the map holds only partitions with recent failures @@ -155,9 +158,9 @@ public sealed class ApiKeyFailureLimiter WindowState? peerState = _partitions.TryGetValue(partitionKey, out WindowState? tracked) ? tracked : null; WindowState? aggregateState = null; - if (effectiveKeyId is not null + if (partition.KeyId is not null && _aggregateLimit > 0 - && _aggregates.TryGetValue(effectiveKeyId, out WindowState? aggregate)) + && _aggregates.TryGetValue(partition.KeyId, out WindowState? aggregate)) { aggregateState = aggregate; } @@ -222,11 +225,13 @@ public sealed class ApiKeyFailureLimiter (Peer: peer, KeyId: effectiveKeyId)); RecordInto(state, now, _limit); - // Only a key id that earned its own partition feeds the aggregate: an id squeezed out by the - // per-peer cap is spray, and letting it through would make aggregate cardinality unbounded. - if (effectiveKeyId is not null && _aggregateLimit > 0) + // The aggregate follows the validly shaped presented key id independently of the peer + // layer's storage choice. Otherwise an attacker could exhaust each peer's dedicated-key cap + // and route a victim id through the fallback to bypass cross-peer throttling. Aggregate + // cardinality is still bounded by EvictIfOverCapacity below. + if (partition.KeyId is not null && _aggregateLimit > 0) { - WindowState aggregate = _aggregates.GetOrAdd(effectiveKeyId, static _ => new WindowState(null, null)); + WindowState aggregate = _aggregates.GetOrAdd(partition.KeyId, static _ => new WindowState(null, null)); RecordInto(aggregate, now, _aggregateLimit); } @@ -261,7 +266,8 @@ public sealed class ApiKeyFailureLimiter // contributed by other key ids — and by junk-shaped tokens — from the same address, so // removing it would let one successful authentication wipe an in-progress spray. The shared // bucket decays by window expiry instead; the caller still recovers through probe admission. - if (string.Equals(effectiveKeyId, partition.KeyId, StringComparison.Ordinal)) + if (effectiveKeyId is not null + && string.Equals(effectiveKeyId, partition.KeyId, StringComparison.Ordinal)) { RemovePartition(partitionKey); } diff --git a/src/ZB.MOM.WW.MxGateway.Tests/Security/Authorization/ApiKeyFailureLimiterTests.cs b/src/ZB.MOM.WW.MxGateway.Tests/Security/Authorization/ApiKeyFailureLimiterTests.cs index cdcdef1..2e7e3e6 100644 --- a/src/ZB.MOM.WW.MxGateway.Tests/Security/Authorization/ApiKeyFailureLimiterTests.cs +++ b/src/ZB.MOM.WW.MxGateway.Tests/Security/Authorization/ApiKeyFailureLimiterTests.cs @@ -101,6 +101,37 @@ public sealed class ApiKeyFailureLimiterTests limiter.Check(new ApiKeyThrottlePartition("ipv4:10.0.9.9:1", "other-key"))); } + /// + /// SEC-38: a valid key id still feeds and consults its cross-peer aggregate when each + /// attacking peer has exhausted its 32 dedicated key-id partitions and therefore records + /// the peer layer through the fallback bucket. + /// + [Fact] + public void AggregateLayer_TripsForValidKeyIdCollapsedToPeerFallback() + { + ManualTimeProvider clock = new(DateTimeOffset.UnixEpoch); + ApiKeyFailureLimiter limiter = CreateLimiter( + clock, + limit: 100, + aggregateLimit: 3, + maxPartitions: 4096); + + for (int peerIndex = 0; peerIndex < 3; peerIndex++) + { + string peer = $"ipv4:10.0.0.{peerIndex + 1}:1"; + for (int keyIndex = 0; keyIndex < ApiKeyFailureLimiter.MaxKeyIdPartitionsPerPeer; keyIndex++) + { + limiter.RecordFailure(new ApiKeyThrottlePartition(peer, $"prefill-{peerIndex}-{keyIndex}")); + } + + limiter.RecordFailure(new ApiKeyThrottlePartition(peer, "victim")); + } + + Assert.Equal( + ApiKeyThrottleDecision.ThrottledByAggregate, + limiter.Check(new ApiKeyThrottlePartition("ipv4:10.0.9.9:1", "victim"))); + } + /// /// A throttled partition is a valve, not a wall: exactly one request per probe interval is /// admitted to the real verifier, so the holder of the correct secret can always get through. @@ -340,6 +371,29 @@ public sealed class ApiKeyFailureLimiterTests Assert.Equal(ApiKeyThrottleDecision.ThrottledByPeer, limiter.Check(overCap)); } + /// + /// SEC-39: a request with no effective key id does not own the peer's shared fallback + /// partition. Resetting after such a request must leave other fallback failures tracked. + /// + [Fact] + public void Reset_WithNullEffectiveKeyId_DoesNotClearSharedFallbackPartition() + { + ManualTimeProvider clock = new(DateTimeOffset.UnixEpoch); + ApiKeyFailureLimiter limiter = CreateLimiter(clock, limit: 3, aggregateLimit: 100); + ApiKeyThrottlePartition fallback = new("ipv4:10.0.0.1:1", KeyId: null); + + RecordFailures(limiter, fallback, 3); + Assert.Equal( + ApiKeyThrottleDecision.ThrottledByPeer, + limiter.Check(fallback, out ApiKeyFailureLimiter.PartitionResolution resolution)); + Assert.Null(resolution.EffectiveKeyId); + + limiter.Reset(fallback, resolution); + + Assert.True(limiter.IsTracked(fallback)); + Assert.Equal(ApiKeyThrottleDecision.ThrottledByPeer, limiter.Check(fallback)); + } + /// /// SEC-32: a spray of unique junk partitions (junk tokens resolve to the sender's fallback /// partition, one per address) must not evict a partition that is currently throttled — the