fix(SEC-38,SEC-39): preserve aggregate and fallback throttles

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_<keyId>_<secret>`, 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

src/ZB.MOM.WW.MxGateway.Server/Security/Authorization/ApiKeyFailureLimiter.cs

@@ -28,8 +28,11 @@ namespace ZB.MOM.WW.MxGateway.Server.Security.Authorization;
 /// <para>
 /// The tracked partition set is a bounded LRU (<see cref="SecurityOptions.ApiKeyFailureTrackedPeers"/>).
 /// Because only a validly shaped token mints a key-id partition and each transport peer may mint at
-/// most <see cref="MaxKeyIdPartitionsPerPeer"/> of them (the overflow collapses onto that address's
-/// fallback partition), the map bounds memory <em>and</em> resists a junk-token flush (SEC-32):
+/// most <see cref="MaxKeyIdPartitionsPerPeer"/> of them (the peer-layer overflow collapses onto that
+/// address's fallback partition), the map bounds memory <em>and</em> 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);
         }

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")));
     }
 
+    /// <summary>
+    ///     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.
+    /// </summary>
+    [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")));
+    }
+
     /// <summary>
     /// 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));
     }
 
+    /// <summary>
+    ///     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.
+    /// </summary>
+    [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));
+    }
+
     /// <summary>
     /// 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