diff --git a/sessions/src/main/kotlin/com/lightningkite/lightningserver/sessions/proofs/BackupCodeEndpoints.kt b/sessions/src/main/kotlin/com/lightningkite/lightningserver/sessions/proofs/BackupCodeEndpoints.kt index dd02b5a74..45a4471a8 100644 --- a/sessions/src/main/kotlin/com/lightningkite/lightningserver/sessions/proofs/BackupCodeEndpoints.kt +++ b/sessions/src/main/kotlin/com/lightningkite/lightningserver/sessions/proofs/BackupCodeEndpoints.kt @@ -170,14 +170,19 @@ public class BackupCodeEndpoints( ), successCode = HttpStatus.OK, implementation = { input: IdentificationAndPassword -> - cache().constrainAttemptRate( - cacheKey = "backup-code-count-${input.property}-${input.value}" - ) { - val subject = input.type + val subject = input.type - val handler = serverRuntime.server.principalTypes.values.find { it.name == subject } - ?: throw IllegalArgumentException("No subject $subject recognized") + val handler = serverRuntime.server.principalTypes.values.find { it.name == subject } + ?: throw IllegalArgumentException("No subject $subject recognized") + // Normalize BEFORE building the rate-limit key: the key must be derived from the canonical + // identifier so that case/whitespace variants of the same account share one bucket. Keying on + // the raw value would let an attacker dodge the limiter (and its exponential backoff) simply + // by varying case or whitespace. + val normalizedValue = handler.normalizePropertyValue(input.property, input.value) + cache().constrainAttemptRate( + cacheKey = "backup-code-count-${input.property}-${normalizedValue}" + ) { val subjectId = handler.fetchUserIdString(input.property, input.value) ?: throw BadRequestException("Invalid Backup Code") diff --git a/sessions/src/main/kotlin/com/lightningkite/lightningserver/sessions/proofs/PasswordProofEndpoints.kt b/sessions/src/main/kotlin/com/lightningkite/lightningserver/sessions/proofs/PasswordProofEndpoints.kt index 5a97a91cc..107f7a826 100644 --- a/sessions/src/main/kotlin/com/lightningkite/lightningserver/sessions/proofs/PasswordProofEndpoints.kt +++ b/sessions/src/main/kotlin/com/lightningkite/lightningserver/sessions/proofs/PasswordProofEndpoints.kt @@ -165,11 +165,15 @@ public class PasswordProofEndpoints( successCode = HttpStatus.OK, implementation = { input: IdentificationAndPassword -> val now = now() - cache().constrainAttemptRate("password-${input.property}-${input.value}") { - val subject = input.type - val handler = serverRuntime.server.principalTypes.values.find { it.name == subject } - ?: throw IllegalArgumentException("No subject $subject recognized") - val normalizedValue = handler.normalizePropertyValue(input.property, input.value) + val subject = input.type + val handler = serverRuntime.server.principalTypes.values.find { it.name == subject } + ?: throw IllegalArgumentException("No subject $subject recognized") + // Normalize BEFORE building the rate-limit key: the key must be derived from the canonical + // identifier so that case/whitespace variants (e.g. "Bob@x.com" vs "bob@x.com ") of the same + // account share one bucket. Keying on the raw value would let an attacker dodge the limiter + // (and its exponential backoff) simply by varying case or whitespace. + val normalizedValue = handler.normalizePropertyValue(input.property, input.value) + cache().constrainAttemptRate("password-${input.property}-${normalizedValue}") { val subjectId = handler.fetchUserIdString(input.property, normalizedValue) ?: throw BadRequestException("User ID and code do not match") diff --git a/sessions/src/main/kotlin/com/lightningkite/lightningserver/sessions/proofs/TimeBasedOTPProofEndpoints.kt b/sessions/src/main/kotlin/com/lightningkite/lightningserver/sessions/proofs/TimeBasedOTPProofEndpoints.kt index f9771ca8c..a756b4843 100644 --- a/sessions/src/main/kotlin/com/lightningkite/lightningserver/sessions/proofs/TimeBasedOTPProofEndpoints.kt +++ b/sessions/src/main/kotlin/com/lightningkite/lightningserver/sessions/proofs/TimeBasedOTPProofEndpoints.kt @@ -153,13 +153,17 @@ public class TimeBasedOTPProofEndpoints( implementation = { input: IdentificationAndPassword -> val now = now() val gracePeriod = now().minus(5.seconds) + val subject = input.type + val handler = serverRuntime.server.principalTypes[subject] + ?: throw IllegalArgumentException("No subject $subject recognized") + // Normalize BEFORE building the rate-limit key: the key must be derived from the canonical + // identifier so that case/whitespace variants of the same account share one bucket. Keying on + // the raw value would let an attacker dodge the limiter (and its exponential backoff) simply + // by varying case or whitespace. + val normalizedValue = handler.normalizePropertyValue(input.property, input.value) cache().constrainAttemptRate( - cacheKey = "totp-count-${input.property}-${input.value}" + cacheKey = "totp-count-${input.property}-${normalizedValue}" ) { - val subject = input.type - val handler = serverRuntime.server.principalTypes[subject] - ?: throw IllegalArgumentException("No subject $subject recognized") - val normalizedValue = handler.normalizePropertyValue(input.property, input.value) val subjectId = handler.fetchUserIdString(input.property, normalizedValue) ?: throw BadRequestException("User ID and code do not match") diff --git a/sessions/src/test/kotlin/com/lightningkite/lightningserver/sessions/proofs/BackupCodeEndpointsTest.kt b/sessions/src/test/kotlin/com/lightningkite/lightningserver/sessions/proofs/BackupCodeEndpointsTest.kt index e8375c244..9a151149d 100644 --- a/sessions/src/test/kotlin/com/lightningkite/lightningserver/sessions/proofs/BackupCodeEndpointsTest.kt +++ b/sessions/src/test/kotlin/com/lightningkite/lightningserver/sessions/proofs/BackupCodeEndpointsTest.kt @@ -590,4 +590,73 @@ class BackupCodeEndpointsTest { } } } + + /** + * Security regression: the rate-limit key must be built from the NORMALIZED identifier, so that + * case/whitespace variants of one account share a single bucket. If the key were derived from the + * raw value, an attacker could dodge the limiter (and its exponential backoff) by varying case. + */ + @Test + fun `rate limiter shares one bucket across case variants of the same identifier`() = runBlocking { + TestUser.users.clear() + val userId = Uuid.random() + val user = TestUser(userId, "test@example.com") + TestUser.users[userId] = user + + object : ServerBuilder() { + val database = setting("database", Database.Settings("ram")) + val cache = setting("cache", Cache.Settings("ram")) + + init { + register(TestUser) + } + + val backupCodes = path.path("auth").path("backup") include BackupCodeEndpoints( + database = database, + cache = cache, + proofSigner = RuntimeDeferred.Cached { testBasis.signer("proof") }, + proofExpiration = 1.hours + ) + }.let { server -> + server.test({}) { + server.backupCodes.modelInfo.table().insert( + listOf( + BackupCodeSecret( + code = "validcode", + subjectId = TestUser.idString(userId), + subjectType = TestUser.name + ) + ) + ) + + // Five distinct case variants that all normalize to "test@example.com". The default limit + // is 5 attempts; five failing attempts across these variants must fill ONE shared bucket. + val emailVariants = listOf( + "Test@example.com", + "tEst@example.com", + "teSt@example.com", + "tesT@example.com", + "TEST@example.com", + ) + for (variant in emailVariants) { + assertFailsWith { + server.backupCodes.prove.test( + null, IdentificationAndPassword("TestUser", "email", variant, "wrongcode") + ) + } + } + + // A sixth attempt with yet another distinct variant must be blocked by the shared limiter. + val blocked = assertFailsWith { + server.backupCodes.prove.test( + null, IdentificationAndPassword("TestUser", "email", "TesT@example.com", "wrongcode") + ) + } + assertTrue( + blocked.message.contains("Too many attempts"), + "Expected the shared rate limiter to block, but got: ${blocked.message}" + ) + } + } + } } diff --git a/sessions/src/test/kotlin/com/lightningkite/lightningserver/sessions/proofs/PasswordProofEndpointsTest.kt b/sessions/src/test/kotlin/com/lightningkite/lightningserver/sessions/proofs/PasswordProofEndpointsTest.kt index b00dace4d..2ddfd52c2 100644 --- a/sessions/src/test/kotlin/com/lightningkite/lightningserver/sessions/proofs/PasswordProofEndpointsTest.kt +++ b/sessions/src/test/kotlin/com/lightningkite/lightningserver/sessions/proofs/PasswordProofEndpointsTest.kt @@ -433,4 +433,67 @@ class PasswordProofEndpointsTest { } } } + + /** + * Security regression: the rate-limit key must be built from the NORMALIZED identifier, so that + * case/whitespace variants of one account share a single bucket. If the key were derived from the + * raw value, an attacker could dodge the limiter (and its exponential backoff) by varying case. + */ + @Test + fun `rate limiter shares one bucket across case variants of the same identifier`() = runBlocking { + TestUser.users.clear() + val userId = Uuid.random() + val user = TestUser(userId, "test@example.com") + TestUser.users[userId] = user + + object : ServerBuilder() { + val database = setting("database", Database.Settings("ram")) + val cache = setting("cache", Cache.Settings("ram")) + + init { + register(TestUser) + } + + val passwordProof = path.path("auth").path("password") include PasswordProofEndpoints( + database = database, + cache = cache, + proofSigner = RuntimeDeferred.Cached { testBasis.signer("proof") }, + proofExpiration = 1.hours + ) + }.let { server -> + server.test({}) { + server.passwordProof.establish(TestUser, userId, EstablishPassword("correctPassword")) + + // Five distinct case variants that all normalize to "test@example.com". The default limit + // is 5 attempts; five failing attempts across these variants must fill ONE shared bucket. + val emailVariants = listOf( + "Test@example.com", + "tEst@example.com", + "teSt@example.com", + "tesT@example.com", + "TEST@example.com", + ) + for (variant in emailVariants) { + assertFailsWith { + server.passwordProof.prove.test( + null, IdentificationAndPassword("TestUser", "email", variant, "wrongPassword") + ) + } + } + + // A sixth attempt with yet another distinct variant must be blocked by the shared limiter. + // Under the old raw-value key this variant would be an untouched bucket and fail only with + // the ordinary "does not match" error. + val blocked = assertFailsWith { + server.passwordProof.prove.test( + null, IdentificationAndPassword("TestUser", "email", "TesT@example.com", "wrongPassword") + ) + } + assertTrue( + blocked.message.contains("Too many attempts"), + "Expected the shared rate limiter to block, but got: ${blocked.message}" + ) + } + } + } } diff --git a/sessions/src/test/kotlin/com/lightningkite/lightningserver/sessions/proofs/TimeBasedOTPProofEndpointsTest.kt b/sessions/src/test/kotlin/com/lightningkite/lightningserver/sessions/proofs/TimeBasedOTPProofEndpointsTest.kt index c0c3d7f85..afc09ab54 100644 --- a/sessions/src/test/kotlin/com/lightningkite/lightningserver/sessions/proofs/TimeBasedOTPProofEndpointsTest.kt +++ b/sessions/src/test/kotlin/com/lightningkite/lightningserver/sessions/proofs/TimeBasedOTPProofEndpointsTest.kt @@ -526,4 +526,81 @@ class TimeBasedOTPProofEndpointsTest { } } } + + /** + * Security regression: the rate-limit key must be built from the NORMALIZED identifier, so that + * case/whitespace variants of one account share a single bucket. If the key were derived from the + * raw value, an attacker could dodge the limiter (and its exponential backoff) by varying case. + */ + @Test + fun `rate limiter shares one bucket across case variants of the same identifier`() = runBlocking { + TestUser.users.clear() + val userId = Uuid.random() + val user = TestUser(userId, "test@example.com") + TestUser.users[userId] = user + + object : ServerBuilder() { + val database = setting("database", Database.Settings("ram")) + val cache = setting("cache", Cache.Settings("ram")) + + init { + register(TestUser) + } + + val totpEndpoints = path.path("auth").path("totp") include TimeBasedOTPProofEndpoints( + database = database, + cache = cache, + proofSigner = RuntimeDeferred.Cached { testBasis.signer("proof") }, + proofExpiration = 1.hours, + config = testConfig + ) + }.let { server -> + server.test({}) { + server.totpEndpoints.modelInfo.table().insert( + listOf( + TotpSecret( + subjectId = TestUser.idString(userId), + subjectType = TestUser.name, + secretBase32 = testSecretBase32, + label = "test", + issuer = "TestApp", + period = 30.seconds, + digits = 6, + algorithm = TotpHashAlgorithm.SHA1, + establishedAt = Clock.System.now(), + lastUsedAt = Clock.System.now() + ) + ) + ) + + // Five distinct case variants that all normalize to "test@example.com". The default limit + // is 5 attempts; five failing attempts across these variants must fill ONE shared bucket. + val emailVariants = listOf( + "Test@example.com", + "tEst@example.com", + "teSt@example.com", + "tesT@example.com", + "TEST@example.com", + ) + for (variant in emailVariants) { + assertFailsWith { + server.totpEndpoints.prove.test( + null, IdentificationAndPassword("TestUser", "email", variant, "000000") + ) + } + } + + // A sixth attempt with yet another distinct variant must be blocked by the shared limiter. + val blocked = assertFailsWith { + server.totpEndpoints.prove.test( + null, IdentificationAndPassword("TestUser", "email", "TesT@example.com", "000000") + ) + } + assertTrue( + blocked.message.contains("Too many attempts"), + "Expected the shared rate limiter to block, but got: ${blocked.message}" + ) + } + } + } }