Page MenuHomePhabricator

BotPasswords doesn't throttle login attempts
Closed, ResolvedPublic

Description

In looking at T165803: Login API shouldn't lock you out in case you make too many correct logins I realized that BotPasswords doesn't do any login throttling.

The only caller is ApiLogin, and all code paths there currently will also call AuthManager, which by default will do throttling. But that throttling only occurs after the BotPassword login is attempted, so an attacker could ignore the throttling messages to keep attempting to attack a BotPassword.

Event Timeline

Anomie created this task.May 20 2017, 9:11 AM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMay 20 2017, 9:11 AM

dpatrick triaged this task as Medium priority.May 30 2017, 9:12 PM
Bawolff added a subscriber: Bawolff.Jun 5 2017, 9:15 PM

Reviewed. Looks good.

The fact that the normal throttle is still totally independent of the botpassword throttle (That is, you can be unthrottled for a botpassword login but throttled for a normal login) is a little bit weird, but I think its fine.

I deployed this patch at 21:30 June 5.

Anomie added a comment.Jun 6 2017, 1:29 AM

The fact that the normal throttle is still totally independent of the botpassword throttle (That is, you can be unthrottled for a botpassword login but throttled for a normal login) is a little bit weird, but I think its fine.

Well, the other way around would have either meant each login attempt would increment the throttle twice or we'd have to introduce some deeply weird communication in there to tell AuthManager not to increment the throttle after all.

Reedy added a subscriber: Reedy.Nov 2 2017, 11:59 PM

This comment was removed by Reedy.
Reedy added a comment.Nov 11 2017, 1:19 AM

Reedy closed this task as Resolved.Nov 13 2017, 4:18 PM

Closes as resolved (due to patches existing, and backports having been made), for ease of tracking in parent bugs

Reedy changed the visibility from "Custom Policy" to "Public (No Login Required)".Nov 15 2017, 12:01 AM
Reedy removed a project: Patch-For-Review.

Change 391418 had a related patch set uploaded (by Reedy; owner: Anomie):
[mediawiki/core@REL1_30] SECURITY: Add throttling for BotPasswords authentication attempts

https://gerrit.wikimedia.org/r/391418

Change 391418 merged by Reedy:
[mediawiki/core@REL1_30] SECURITY: Add throttling for BotPasswords authentication attempts

https://gerrit.wikimedia.org/r/391418

Change 391434 had a related patch set uploaded (by Ejegg; owner: Anomie):
[mediawiki/core@fundraising/REL1_27] SECURITY: Add throttling for BotPasswords authentication attempts

https://gerrit.wikimedia.org/r/391434

Change 391434 merged by Ejegg:
[mediawiki/core@fundraising/REL1_27] SECURITY: Add throttling for BotPasswords authentication attempts

https://gerrit.wikimedia.org/r/391434

Change 391448 had a related patch set uploaded (by Reedy; owner: Anomie):
[mediawiki/core@master] SECURITY: Add throttling for BotPasswords authentication attempts

https://gerrit.wikimedia.org/r/391448

Change 391448 merged by jenkins-bot:
[mediawiki/core@master] SECURITY: Add throttling for BotPasswords authentication attempts

https://gerrit.wikimedia.org/r/391448

Change 391709 had a related patch set uploaded (by Chad; owner: Anomie):
[mediawiki/core@wmf/1.31.0-wmf.8] SECURITY: Add throttling for BotPasswords authentication attempts

https://gerrit.wikimedia.org/r/391709

Change 391709 merged by jenkins-bot:
[mediawiki/core@wmf/1.31.0-wmf.8] SECURITY: Add throttling for BotPasswords authentication attempts

https://gerrit.wikimedia.org/r/391709