fix(#11090): remove password from rate limiter lookup keys#11091
Open
YASHSHARMAOFFICIALLY wants to merge 1 commit into
Open
fix(#11090): remove password from rate limiter lookup keys#11091YASHSHARMAOFFICIALLY wants to merge 1 commit into
YASHSHARMAOFFICIALLY wants to merge 1 commit into
Conversation
The rate limiter included the raw password as a rate limit key. This caused all login attempts sharing the same password to share a single rate limit bucket, enabling cross-account lockout regardless of username or IP address. Rate limiting by IP and username is sufficient to prevent brute-force attacks against individual accounts.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #11090
Related to #10705
Context
The rate limiter in
api/src/services/rate-limit.jsuses multiple keys per request: IP, username, and password. When any single key exhausts its 10-attempt budget within 10 seconds, all subsequent requests matching that key are blocked.Because the raw password is used as a key, all accounts sharing the same password share a rate limit bucket. An attacker can lock out every account with a given password by sending 10 failed login requests with any username and that password. This is especially relevant in CHT deployments where administrators provision devices for community health workers and password patterns may overlap.
The combination of IP + username is sufficient to prevent brute-force attacks against individual accounts.
Changes
api/src/services/rate-limit.jsreq.body.passwordandbasicAuth.passwordfromgetKeys().api/tests/mocha/services/rate-limit.spec.jsisLimitedtests: adjusted call counts and removed password key assertions.consumetests: adjusted call counts and removed password key assertions.Test plan