Rate-limit password-based login attempts #3013
Conversation
Deploying matrix-authentication-service-docs with
|
| Latest commit: |
3b89cf8
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://d64cc012.matrix-authentication-service-docs.pages.dev |
| Branch Preview URL: | https://quenting-ratelimits.matrix-authentication-service-docs.pages.dev |
| @@ -650,6 +670,57 @@ mod tests { | |||
| assert_eq!(body, old_body); | |||
| } | |||
|
|
|||
| /// Test that password logins are rate limited. | |||
| #[sqlx::test(migrator = "mas_storage_pg::MIGRATOR")] | |||
There was a problem hiding this comment.
I had no idea this macro was a thing.
There was a problem hiding this comment.
It's nice, it creates a temporary database and run the migrator on it, so each test gets an isolated database
| use nonzero_ext::nonzero; | ||
| use ulid::Ulid; | ||
|
|
||
| const PASSWORD_CHECK_FOR_REQUESTER_QUOTA: Quota = Quota::per_minute(nonzero!(3u32)); |
There was a problem hiding this comment.
Not really sure either way but 3/min seems a bit harsh, just thinking about e.g. the company's shared office which has a shared IPv4.
There was a problem hiding this comment.
I'll do config and tweak those numbers in another PR
| impl Default for LimiterInner { | ||
| fn default() -> Self { | ||
| Self { | ||
| password_check_for_requester: RateLimiter::keyed(PASSWORD_CHECK_FOR_REQUESTER_QUOTA), |
There was a problem hiding this comment.
can't remember for sure but I think you want to call retain_recent on these ratelimiters occasionally to perform housekeeping.
There was a problem hiding this comment.
Good call, I did that in 5b1c9b4
|
@reivilibre if you could glimpse at 5b1c9b4 and retick, that would be great |
5b1c9b4 to
110c695
Compare
110c695 to
3b89cf8
Compare
This is a first step for #2541.
It sets up a
Limiterutility, which will hold all the rate-limiting logic.As a first candidate, I rate-limited password-based logins.
There are two levels of rate-limiting:
I'm hesitant about the latter, but this is to try avoid distributed password-guessing attacks.
You'd need about 600 IPs to effectively lock out a user, which should be fine?