Page MenuHomePhabricator

Put a hard limit on password login attempts from the same remote address
ClosedPublic

Authored by epriestley on Jan 18 2019, 2:02 PM.

Details

Summary

Ref T13222. Currently, if a remote address fails a few login attempts (5) in a short period of time (15 minutes) we require a CAPTCHA for each additional attempt.

This relies on:

  • Administrators configuring ReCAPTCHA, which they may just not bother with.
  • Administrators being comfortable with Google running arbitrary trusted Javascript, which they may not be comfortable with.
  • ReCAPTCHA actually being effective, which seems likely true for unsophisticated attackers but perhaps less true for more sophisticated attackers (see https://github.com/ecthros/uncaptcha2, for example).

(For unsophisticated attackers and researchers, "Rumola" has been the standard CAPTCHA bypass tool for some time. This is an extension that pays humans to solve CAPTCHAs for you. This is not practical at "brute force a strong password" scale. Google appears to have removed it from the Chrome store. The "submit the captcha back to Google's APIs" trick probably isn't practical at brute-force-scale either, but it's easier to imagine weaponizing that than weaponizing human solvers.)

Add a hard gate behind the CAPTHCA wall so that we fail into a secure state if there's no CAPTCHA or the attacker can defeat CAPTCHAs at a very low cost.

The big downside to this is that an attacker who controls your remote address (e.g., is behind the same NAT device you're behind on corpnet) can lock you out of your account. However:

  • That should be a lot of access (although maybe this isn't that high of a barrier in many cases, since compromising a "smart fridge" or "smart water glass" or whatever might be good enough).
  • You can still do "Forgot password?" and login via email link, although this may not be obvious.
Test Plan
  • Logged in normally.
  • Failed many many login attempts, got hard gated.

Diff Detail

Repository
rP Phabricator
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

epriestley created this revision.Jan 18 2019, 2:02 PM
epriestley requested review of this revision.Jan 18 2019, 2:04 PM
epriestley updated this revision to Diff 47734.Jan 18 2019, 2:09 PM
  • Provide a hint that the way out for legitimate users is to do a password reset.
amckinley accepted this revision.Jan 19 2019, 12:55 AM
amckinley added inline comments.
src/applications/auth/provider/PhabricatorPasswordAuthProvider.php
267

"require a they"

This revision is now accepted and ready to land.Jan 19 2019, 12:55 AM
epriestley updated this revision to Diff 47744.Jan 19 2019, 3:48 AM
  • Correct typo.
This revision was automatically updated to reflect the committed changes.