1188716 - Lockscreen can be bypassed with swiping while the device is booting (having the SIM pin lock facilitates it)

Status: RESOLVED FIXED

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

Description

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Attachment: lock screen bug.mov

1. turn Sim pin on
2. set up lockscreen
3. reboot the device
4. while the device is booting swipe where the lockscreen swipe should be located

Expected: lockscreen appears
Actual: lockscreen is by passed and sim pin is requested.

Serial: 351dd0f2 (State: device)
Build ID               20150724001207
Gaia Revision          9dba58d18006e921546cec62c76074ce81e16518
Gaia Date              2015-07-23 12:36:57
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/41e10c6740be
Gecko Version          34.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150724.035246
Firmware Date          Fri Jul 24 03:52:57 EDT 2015
Bootloader             L1TC000118D0

Attached video shows issue.  And yes, lockscreen was turned on.

Comment 1

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Affected are master, 2.2, 2.1, 2.0 for the latest builds.  1.3 is not affected.

Comment 2

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

I thought this bug is related to bug 1173284; decided to branch it out separately to avoid the confusion.

Comment 3

[Greg Weng [:snowmantw][:gweng][:λ]]

Thanks Naoki. I'll check this after landing the patch of Bug 1173284.

Comment 4

[Greg Weng [:snowmantw][:gweng][:λ]]

I've tested this bug and I think there is a more simple solution for these branches: since it's unlike Bug 1173284, at the moment users see the slider, the value usually has been read. So we could delay the slider to initialize itself until the value get read, which means the slider would be disabled until the value is read, thus no by-passing bugs anymore.

In Bug 1173284 we can't do that since user will have 3 seconds couldn't do anything even though the screen already shows the panel, but in this case there is no such long pending time.

Comment 5

[Greg Weng [:snowmantw][:gweng][:λ]]

Attachment: Patch for v2.2

Tim: as I said, this is the quick and effective solution. If you think it's good, I will start to write tests and set approval (for other branches, too).

Comment 6

[Greg Weng [:snowmantw][:gweng][:λ]]

Comment on attachment 8640403 [details] [review]
Patch for v2.2

It works well and passed all test on CI. And I also added a new unit test. So set the review flag.

Comment 7

[Greg Weng [:snowmantw][:gweng][:λ]]

Comment on attachment 8640403 [details] [review]
Patch for v2.2

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): old regression; traceback to v1.3
[User impact] if declined: Secure flaw in occasional cases (see how QA reproduce it)
[Testing completed]: CI result:
https://treeherder.mozilla.org/#/jobs?repo=gaia&revision=ed424195d3c14bf7d0d48b9c74d00645df51166b

[Risk to taking this patch] (and alternatives if risky): No
[String changes made]: No

---

By the way, I don't know if this needs to be uplifted to 2.1 and 2.0. But if so, the solution remains the same. And there is no approval-gaia flag for 2.1 and 2.0 for this patch, so I don't know how to do that. Another issue is I don't know if uplift to old branches with a security needs a sec-approval or any other unusual requirements, so I ask here.

Comment 8

[Christiane Ruetten [INACTIVE]]

SIM PIN unlock happens after pass code unlock, so what's your theory of the mechanism that makes SIM PIN request affect unlocking? All I can think of is increasing system load during bootstrapping, enlarging the race window.

Marking as blocking bug 1189314.

Comment 9

[Greg Weng [:snowmantw][:gweng][:λ]]

(In reply to Christiane Ruetten [:cr] from comment #8)
> SIM PIN unlock happens after pass code unlock, so what's your theory of the
> mechanism that makes SIM PIN request affect unlocking? All I can think of is
> increasing system load during bootstrapping, enlarging the race window.
> 
> Marking as blocking bug 1189314.

I can easily reproduce it without a SIM card. But I agree that facilitates reproducing it.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

v2.2: https://github.com/mozilla-b2g/gaia/commit/f8b119ac30e97df991c97682ac4d4f9ca22e1793

Comment 11

[Greg Weng [:snowmantw][:gweng][:λ]]

Paul: I will try to get approval for v2.1, but I don't know what's about v2.0? Shall I uplift the patch for it?

Comment 12

[Greg Weng [:snowmantw][:gweng][:λ]]

Attachment: Patch for v2.1

Comment 13

[Greg Weng [:snowmantw][:gweng][:λ]]

I've found there is no approval flag for 2.1, too. I now don't know how to uplift that...

Comment 14

[Greg Weng [:snowmantw][:gweng][:λ]]

Attachment: Patch v2.0

Comment 15

[Greg Weng [:snowmantw][:gweng][:λ]]

Change the title to make it matches the symptom.

Comment 16

[Greg Weng [:snowmantw][:gweng][:λ]]

I've test patches on v2.1 and v2.0. And they could guarantee that

Respect passcode timeout after rebooting: Yes
Respect timeout after setting it and before rebooting: Yes
With passcode, no bypassing while booting: Yes
Without passcode, no bypassing while booting: Yes
Secure app works without bypassing passcode: Yes
Unlocking to ordinary app works after passcode expired: Yes

Of course QA and sec-team need to verify them But if they don't cause any test failures I think they indeed fix the bug.

Comment 17

[Ryan VanderMeulen [:RyanVM]]

We still have CI coverage for the 2.1s branch, so we can merge the 2.1 PR and then merge 2.1->2.1s to get test results (under the assumption that the two branches are close enough to trust the results). However, we have no infra for testing 2.0 at this point, so we'd be flying blind if we land the patch on 2.0/2.0m.

Could we get additional QA help verifying these fixes (and probably basic smoketests still passing) if need-be? Note that we don't run Flame-KK builds on 2.1s, so we'd probably need to resort to manually building & flashing after the fix lands.

Comment 18

[Ryan VanderMeulen [:RyanVM]]

Greg also confirmed on IRC that this bug only pertains to fixing the older branches. Bug 1173284 was sufficient to fix master.

Comment 19

[Mahendra Potharaju [:mahe]]

Naoki, 

Please assign a QA resource to test this patch. Will request Ryan to uplift this to 2.0.

Thanks

Comment 20

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Peter, Kevin, here is the patch for 2.0/2.1/2.2 uplift.  Can you please have someone test these?

Comment 21

[Christiane Ruetten [INACTIVE]]

Vance,

 in the attachments you find pull requests for backported patches for the lockscreen issue (main bug 1173284), but they may not be suitable for partners. Ask :gweng for a diff patch that's distributable to them. Feel free to send the following preliminary advisory along:

-----8<----------------
(Mozilla partner confidential, not for publication)

Mozilla has identified and fixed a vulnerability in Firefox OS that allows unauthorized access to devices locked with a pass code. If the user swipes right to unlock the device immediately when the lock screen appears after reboot, the pass code query would be skipped. Usually, this time window is too brief to be exploitable. However, it can be artificially elongated by stressing the event queue with premature, highly-frequent swiping, resulting in higher system load during boot-up. The time window is also elongated, for example, when the device is booking into an encrypted wireless network.

The technical reason for this behavior is a race condition in LockScreen which would rely on unsafe defaults until the actual user settings have been read back asynchronously. Our patch adds a safeguard to LockScreen that would not allow unlock attempts until the settings have been read back.

Comment 22

[Christiane Ruetten [INACTIVE]]

I'm adding [patch-b2g-main*+] tags so we can keep track of patches for partners and community editions when they can't land in our repos.

Comment 23

[Ryan VanderMeulen [:RyanVM]]

v2.1: https://github.com/mozilla-b2g/gaia/commit/f688e0039e315df27f3d9112dabbdfef26934b89

Comment 24

[Pi Wei Cheng [:piwei] (inactive)]

The patch has already landed on v2.2 and v2.1. However on PVT website we stopped getting new builds on v2.1 for Flame since last month, so I tested the patch for v2.0 and v2.1 on Flame, as well as tested latest build on v2.2 Flame.

Confirmed that on all tested branches the original bug has been fixed. 

Also tested the scenarios listed on comment 16 as best as I can since those descriptions are short and seem ambiguous.

Further tested the lockscreen particularly in the area of bypassing passcode and was unsuccessful.

Nothing else seemed broken by this patch. All tests look good.

Comment 25

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Do we know what prevents this bug from closing? What's the action item here?

Comment 26

[Christiane Ruetten [INACTIVE]]

AFAICS there hasn't been a decision on the 2.0 backport from product and dev, yet. From our view it's a "nice to have". Once this bug is set to wontfix, fixed or unaffected, this bug can be closed. If you prefer we can close this bug and block tracking bug 1189314 with a dedicated 2.0 backport bug.

Comment 27

[Andrew McCreight [:mccr8]]

It looks like this was fixed in the trunk versions of b2g, so I'm going to mark this fixed. I'll leave the affected flags for the older versions of B2G.

Comment 28

[Liz Henry (:lizzard) (relman/hg->git project)]

Christiane, can you go ahead and file that dedicated 2.0 backport bug? Thanks.

Comment 29

[Christiane Ruetten [INACTIVE]]

The tracking bug is bug 1189314. The decision was to not provide a backport for 2.0. Setting to wontfix.