Closed Bug 888911 Opened 11 years ago Closed 11 years ago

Implement an increasing lockout delay to prevent passcode guessing.

Categories

(Firefox OS Graveyard :: Gaia::System::Lockscreen, defect)

All
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(b2g-v1.2 fixed)

RESOLVED FIXED
Tracking Status
b2g-v1.2 --- fixed

People

(Reporter: pauljt, Assigned: pauljt)

References

()

Details

(Whiteboard: [perf-reviewed])

Attachments

(1 file, 1 obsolete file)

+++ This bug was initially created as a clone of Bug #877541 +++

Currently we only support 4 digit pass-codes. It's feasible that someone with a lot of time could manually bruteforce the lockscreen code. This attack would be more feasible if/when we support external input devices. Introducing a simple increasing timeout would mitigate this risk. (and of course reseting the delay back to the minimum after a correct attempt).

That is in:
https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/lockscreen.js

Instead of a constant kPassCodeErrorTimeout as 500 milliseconds, after 5 failed attempts, start adding 1000 millis for each failed attempt, up to a maximum (say 5 minutes, need to be careful people cant effectively lock you out of your device by repeatedly failing login). Android apparently uses 30s delay every 5 attempts, but there have been attacks against this with HID devices. e.g. http://forums.hak5.org/index.php?/topic/28165-payload-android-brute-force-4-digit-pin/. 

We should just fix 877541 but this provides some mitigation in the mean time. 

Given how simple this change is, I am suggesting this for leo.

Thoughts?
Nice to have but too late in the 1.1 cycle to do a new feature request, hence pushing this to our next version request, koi ?
blocking-b2g: leo? → koi?
This patch implements a lockout delay doubling the timeout for every failed attempt beyond the 5th one.
Attachment #784951 - Flags: review?(timdream)
Comment on attachment 784951 [details] [diff] [review]
Patch adding expontential lockout delay

Make sense!
Attachment #784951 - Flags: review?(timdream) → review+
master: https://github.com/mozilla-b2g/gaia/commit/28a93e5eebada09920ab940252454326fc321c8f
Assignee: nobody → ptheriault
Status: NEW → RESOLVED
Closed: 11 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
And backed out for Travis CI failures.
https://github.com/mozilla-b2g/gaia/commit/a14994b80a5bd2e179c9f6e1ce08af459450bf35

And damn you for making me learn how to back things out from Git! :)
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
I suppose a link to the failing test run would have helped too.
https://travis-ci.org/mozilla-b2g/gaia/builds/10276652
Well, this has been a learning experience (that'll teach me to try to interpret test runs outside of tbpl). The bustage was pre-existing, so this is good to re-land. Sorry for the churn, I'm a total git n00b.
That's a relief -  I was really struggling to correlate that link above to my patch.

Do I need to do something to my patch to reland it? 

Also feel free to let me know if I could/should have done something different here - I too am a total git... er.. 'apprentice' :)
Whiteboard: [perf-reviewed]
Not sure if this is waiting on anything else, tagging with checkin-needed just not visible enough.
Keywords: checkin-needed
Need a new PR please.
Attached file Same Patch, new PR
Attachment #784951 - Attachment is obsolete: true
Thanks, sorry again for the mix-up.

Master: https://github.com/mozilla-b2g/gaia/commit/062f125a49ea7c528e3c1b50adee4abd533cb09a
Status: REOPENED → RESOLVED
Closed: 11 years ago11 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Shipped.
blocking-b2g: koi? → ---
Attachment mime type: text/plain → text/x-github-pull-request
This feature was just brought to UX's attention as a ni? UX flag on the report of this feature as a bug, in bug #927482 (where I will copy this comment for coverage). I've reviewed it with Francis Djabri, a member of the UX team and the product owner for System Front-end, Peter Dolanjski.

In future, module owners should consult with UX for something this user facing, for review before landing. Everything doesn't need a spec, but UX input should be given for something like this.

UX is a fan of this feature for the purpose of mitigating brute force attacks.  BlackBerry used to do this by forcing the user to type "blackberry" after x incorrect password attempts.  This would break up the entry to help discourage such attacks.

But the implementation here needs some usability improvements.  The key missing piece is informing the user why there is a delay.  When a delay is introduced, we need to tell the user "incorrect password, try re-entering in x seconds" (potentially with a count down) or something to that effect. 

If we can add some UX for this, we suggest also considering how we might present a prompt that will let the user know that their device will be wiped after x number of failed attempts.  In a stolen device scenario this is the only way to stop brute force attacks completely.  This feature is already in the Systems Front-end backlog. It's not top priority, but they may deliver it at some point.
Stephany, sure thing - apologies for not getting UX involved, will do next time. I did actually consider putting a message in the patch, but this was an urgent risk that needed fixing and so I wanted to change as little as possible to incresae landing chances. Note we _already_ had a lockout delay, I just changed the duration to increase. I suppose if we are locking out for longer then maybe the message is more important but it seems small in the scheme of security UI issues (I can think of many security UI issues in Gaia that are much higher priority than this, though of course, my priorities are security, which I expect differs from UX priorities :) 

A couple points on your reply:
- I strongly advise against the 'type in "blackberry"' style control. This is just a pain in the butt for regular users and a not a hurdle at all for attackers (the main threat here is automated style bruteforcing attacks such as http://news.cnet.com/8301-17938_105-57595281-1/3d-printed-button-bashing-robot-guesses-phone-pins/)
- Furthermore, bruteforcing is already dissuaded - the delay increase exponentially, which means someone trying to brute-force the phone needs to wait exponentially long. So if they want to say, lock the phone out for 30 minutes, they need to wait 15 +7.5 +3.25 + 1.75.... before they can do this. So this approach dissuades both password guessing and DoSing the phone through lockout
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: