From fe056cc464a15794b517002810f10510070250a3 Mon Sep 17 00:00:00 2001 From: Ned Wolpert Date: Wed, 5 Aug 2026 08:01:05 -0700 Subject: [PATCH] fix(core): case-fold the username before consulting the ceremony rate limiter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit UserLookup implementations resolve usernames case-insensitively — DynamoDbUserLookup lower-cases the identity key — but rateLimitedBucket passed the raw request string to CeremonyRateLimiter.tryAcquireForUsername. So "alice", "Alice", and "ALICE" drew on three independent per-username budgets against one account; an 8-character username yields 256 of them, turning a 10/min allowance into ~2560/min. The per-IP bucket does not compensate. It caps a single source at 30/min, but the per-username bucket exists precisely for the distributed case — many sources, one victim — and that is the case the split defeated. Folded at the call site rather than inside InMemoryCeremonyRateLimiter so every implementation inherits the fix, including a host's shared Redis limiter, which had the same bug and no way to know it. The SPI now documents that the argument arrives folded and must be used as given. Case-folding is the safe direction even where a backend is still case-sensitive: the worst case is two genuinely distinct accounts sharing one throttle bucket, which is stricter, not weaker. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 12 ++++++++++++ .../DefaultPasskeyAuthenticationService.java | 10 +++++++++- .../pkauth/spi/CeremonyRateLimiter.java | 9 ++++++++- ...tPasskeyAuthenticationServiceRateLimitTest.java | 14 ++++++++++++++ 4 files changed, 43 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 99bdbbc..5848247 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,6 +39,18 @@ finding from that review. clean **400** — covered by an adapter-level test asserting the refusal reaches the client as a 400 and not a 500. `StartAuthenticationRequest` still accepts a `null` username (the usernameless / discoverable-credential flow). +- **The per-username ceremony rate limit can no longer be bypassed by varying + case.** `UserLookup` implementations resolve usernames case-insensitively + (`DynamoDbUserLookup` lower-cases the identity key), but the ceremony service + passed the raw request string to `CeremonyRateLimiter.tryAcquireForUsername` — + so `alice`, `Alice`, and `ALICE` drew on three independent budgets against one + account, and an 8-character username yielded 256 of them. The per-IP bucket did + not compensate: the per-username bucket exists for the distributed case, which + is exactly the case the split defeated. The service now case-folds + (`toLowerCase(Locale.ROOT)`) before consulting the limiter, so **every** + `CeremonyRateLimiter` — including a host's shared Redis implementation — + inherits the fix. The SPI documents that the argument arrives folded and MUST + be used as given. ## [2.2.0] — 2026-06-27 diff --git a/pk-auth-core/src/main/java/com/codeheadsystems/pkauth/internal/DefaultPasskeyAuthenticationService.java b/pk-auth-core/src/main/java/com/codeheadsystems/pkauth/internal/DefaultPasskeyAuthenticationService.java index 219e687..cae68a1 100644 --- a/pk-auth-core/src/main/java/com/codeheadsystems/pkauth/internal/DefaultPasskeyAuthenticationService.java +++ b/pk-auth-core/src/main/java/com/codeheadsystems/pkauth/internal/DefaultPasskeyAuthenticationService.java @@ -63,6 +63,7 @@ import java.util.EnumSet; import java.util.HexFormat; import java.util.List; +import java.util.Locale; import java.util.Objects; import java.util.Optional; import java.util.Set; @@ -903,7 +904,14 @@ private AssertionResult outcomeAssertion(AssertionResult result, long start) { LOG.info("{} rate-limited ip-bucket clientIp={}", phase, clientIp); return "ip"; } - if (username != null && !rateLimiter.tryAcquireForUsername(username)) { + // Case-fold before bucketing. UserLookup implementations resolve usernames case-insensitively + // (DynamoDbUserLookup lower-cases the identity key), so keying the bucket on the raw request + // string gave "alice", "Alice", and "ALICE" three independent budgets against one account — an + // 8-character username yields 256 of them. The per-IP bucket does not compensate: the + // per-username bucket exists precisely for the distributed case, which is the case the split + // defeated. Folding here rather than inside the limiter means every CeremonyRateLimiter + // implementation — including a host's shared Redis one — inherits the fix. + if (username != null && !rateLimiter.tryAcquireForUsername(username.toLowerCase(Locale.ROOT))) { LOG.info("{} rate-limited username-bucket username={}", phase, username); return "username"; } diff --git a/pk-auth-core/src/main/java/com/codeheadsystems/pkauth/spi/CeremonyRateLimiter.java b/pk-auth-core/src/main/java/com/codeheadsystems/pkauth/spi/CeremonyRateLimiter.java index d46fce3..762d1bc 100644 --- a/pk-auth-core/src/main/java/com/codeheadsystems/pkauth/spi/CeremonyRateLimiter.java +++ b/pk-auth-core/src/main/java/com/codeheadsystems/pkauth/spi/CeremonyRateLimiter.java @@ -60,7 +60,14 @@ public interface CeremonyRateLimiter { * budget. Only called by the ceremony service for {@code start*} requests that carry a username; * {@code finish*} requests have no username on the wire and consult only the per-IP bucket. * - * @param username the username supplied on the start ceremony request; never {@code null} + *

The value arrives already case-folded ({@code toLowerCase(Locale.ROOT)}), + * because {@link UserLookup} implementations resolve usernames case-insensitively. Since 2.3.0 + * the ceremony service folds it before calling, so implementations MUST use the argument as given + * and MUST NOT re-derive a bucket key from a differently-cased source — case variants of one + * username would otherwise get independent budgets and multiply the effective limit against a + * single account. + * + * @param username the case-folded username from the start ceremony request; never {@code null} * @return {@code true} when allowed; {@code false} when the per-username budget for the current * window is exhausted * @since 0.9.1 diff --git a/pk-auth-core/src/test/java/com/codeheadsystems/pkauth/internal/DefaultPasskeyAuthenticationServiceRateLimitTest.java b/pk-auth-core/src/test/java/com/codeheadsystems/pkauth/internal/DefaultPasskeyAuthenticationServiceRateLimitTest.java index e139e44..3336688 100644 --- a/pk-auth-core/src/test/java/com/codeheadsystems/pkauth/internal/DefaultPasskeyAuthenticationServiceRateLimitTest.java +++ b/pk-auth-core/src/test/java/com/codeheadsystems/pkauth/internal/DefaultPasskeyAuthenticationServiceRateLimitTest.java @@ -199,6 +199,20 @@ void allowedCallsConsultLimiterAndProceed() { verify(challengeStore).put(any(), any(), any()); } + @Test + void usernameBucketKeyIsCaseFoldedSoVariantsShareOneBudget() { + // Regression guard: UserLookup resolves usernames case-insensitively, so case variants must + // not each get their own per-username budget. Keying on the raw request string let a + // distributed attacker multiply the 10/min allowance against one account by 2^len. + service.startRegistration( + new StartRegistrationRequest("Alice", "Alice", null, null), "1.1.1.1"); + service.startRegistration( + new StartRegistrationRequest("ALICE", "Alice", null, null), "1.1.1.1"); + service.startAuthentication(new StartAuthenticationRequest("aLiCe", null), "1.1.1.1"); + + assertThat(limiter.usernameCalls).containsExactly("alice", "alice", "alice"); + } + private static FinishRegistrationRequest stubFinishRegistration() { return new FinishRegistrationRequest( CHALLENGE_ID,