Oliver Nelson [:oliverthor] Reporter
	Description • 9 years ago

Description:
test_lockscreen_unlock_to_homescreen_with_passcode.py is failing consistently today, in what appears to be trying to access the locator for lockscreen. This was previously failing inconsistently to bug 1176279 (which also handles 2 other lockscreen python tests).

* http://jenkins1.qa.scl3.mozilla.com/view/UI/job/flame-kk-319.mozilla-central.nightly.ui.functional.non-smoke.1.bitbar/150/HTML_Report
* http://jenkins1.qa.scl3.mozilla.com/view/UI/job/flame-kk.ui.adhoc.bitbar/128/HTML_Report/

Repro Steps:
1) Update phone to 20150708043647
2) Open Settings and enable lockscreen
3) Set passcode to '1337'
4) Lock phone
5) Unlock via code from step 3

Actual:
Test times out once passcode keypad is present on screen

Expected: 
Test submits appropriate passcode and passes to homescreen


Traceback (most recent call last):
File "/var/lib/jenkins/jobs/flame-kk.ui.adhoc.bitbar/workspace/.env/lib/python2.7/site-packages/marionette_client-0.16-py2.7.egg/marionette/marionette_test.py", line 296, in run
testMethod()
File "/var/lib/jenkins/jobs/flame-kk.ui.adhoc.bitbar/workspace/tests/python/gaia-ui-tests/gaiatest/tests/functional/lockscreen/test_lockscreen_unlock_to_homescreen_with_passcode.py", line 29, in test_unlock_to_homescreen_with_passcode
homescreen = lock_screen.unlock_to_homescreen_using_passcode(self._input_passcode)
File "/var/lib/jenkins/jobs/flame-kk.ui.adhoc.bitbar/workspace/tests/python/gaia-ui-tests/gaiatest/apps/lockscreen/app.py", line 89, in unlock_to_homescreen_using_passcode
self.wait_for_lockscreen_not_visible()
File "/var/lib/jenkins/jobs/flame-kk.ui.adhoc.bitbar/workspace/tests/python/gaia-ui-tests/gaiatest/apps/lockscreen/app.py", line 108, in wait_for_lockscreen_not_visible
self.wait_for_element_not_displayed(*self._lockscreen_locator)
File "/var/lib/jenkins/jobs/flame-kk.ui.adhoc.bitbar/workspace/tests/python/gaia-ui-tests/gaiatest/apps/base.py", line 49, in wait_for_element_not_displayed
lambda m: not m.find_element(by, locator).is_displayed())
File "/var/lib/jenkins/jobs/flame-kk.ui.adhoc.bitbar/workspace/.env/lib/python2.7/site-packages/marionette_driver-0.9-py2.7.egg/marionette_driver/wait.py", line 143, in until
cause=last_exc)
TimeoutException: TimeoutException: Timed out after 10.1 seconds


Environmental Variables:
Device firmware (base) 	L1TC100118D0
Device firmware (date) 	08 Jul 2015 12:20:29
Device firmware (incremental) 	eng.cltbld.20150708.082017
Device firmware (release) 	4.4.2
Device identifier 	flame
Device memory 	219772 kB
Device serial 	94f7ce17
Device uptime 	0 days 0 hours 2 minutes 7 seconds
Gaia date 	08 Jul 2015 01:52:21
Gaia revision 	553e4e281d11
Gecko build 	20150708043647
Gecko revision 	9b902b7669ae
Gecko version 	42.0a1


Reproducible manually: No

Repro frequency: 6/6

	No-Jun Park [:njpark]
	Comment 1 • 9 years ago

So while it cannot be reproduced manually, it happens consistenly in gaia ui tests, probably because https://github.com/mozilla-b2g/gaia/blob/e0cdccd2d0cbf987015f56422cf512d819aca252/tests/python/gaia-ui-tests/gaiatest/tests/functional/lockscreen/test_lockscreen_unlock_to_homescreen_with_passcode.py#L16 might have issues.  
ni?ing mozfreddyb for confirmation.

	Frederik Braun [:freddy]
	Comment 2 • 9 years ago

I could not repro locally, but I do not see any problems in the cited piece of code off hand, Sorry.

What can we do?

	Frederik Braun [:freddy]
	Comment 3 • 9 years ago

Do we shutdown or reboot the device in between test?
If yes, this might be related to bug 1181565, which will hopefully be shipped very soon.

	Martijn Wargers (dead) Assignee
	Comment 5 • 9 years ago

This is still an issue, using:
Build ID               20150714010206
Gaia Revision          7676b68b4d32ed13243eeb719188847121bd5611
Gaia Date              2015-07-13 23:56:32
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/0931671a14ef
Gecko Version          42.0a1

Apparently not fixed by bug 1181565.
Frederik, can you take a look why this is still failing? I don't think there is anything wrong with the Gaia UI test.

	Frederik Braun [:freddy]
	Comment 6 • 9 years ago

I am looking into this again, but I can't seem to find a way to step through things across layers with a debugger.

All I have so far is the stack trace from comment 0, but I can not reproduce :-/

	Martijn Wargers (dead) Assignee
	Comment 8 • 9 years ago

Comment on attachment 8635256 [details] [review]
[gaia] mwargers:1181662 > mozilla-b2g:master

Ok, after some chatting on irc, we found out that there was something wrong with the values set in the settings. This makes the test pass.

	Frederik Braun [:freddy]
	Comment 9 • 9 years ago

Comment on attachment 8635256 [details] [review]
[gaia] mwargers:1181662 > mozilla-b2g:master

r+ with nits addressed:

Looks good, but I think we can do better than "magic values here", I was half-joking earlier on IRC, sorry.

I think we should explain some more in the comments.
* ArrayBuffers are represented as objects keys from 0 to n-1.
* The passcode is stored using PBKDF2 with a non-deterministic salt. These values here are the result of a pre-computation of PBKDF2 with the given salt, 1000 iterations of SHA-1 and the passcode "1337".

	Martijn Wargers (dead) Assignee
	Comment 10 • 9 years ago

Thanks, I used that as a comment for explanation.

	Johan Lorenzo [:jlorenzo]
	Comment 11 • 9 years ago

Comment on attachment 8635256 [details] [review]
[gaia] mwargers:1181662 > mozilla-b2g:master

I'm sorry, I'll need a bit more of explanation to understand the patch :s

* What are ArrayBuffers in this context?
* Is, in this particular example, n equal to 20?
* The digest is the result of 1000 pre-computations, isn't it? Is the salt also digested? If so, how was it? If not, what do represent the values associated to [0..7]?

	Martijn Wargers (dead) Assignee
	Comment 12 • 9 years ago

Johan, would you mind if I just check in, what we have now and adjust the comment when Frederik responds.

	Johan Lorenzo [:jlorenzo]
	Comment 13 • 9 years ago

Comment on attachment 8635256 [details] [review]
[gaia] mwargers:1181662 > mozilla-b2g:master

Ok. That sounds fine to me: r+ with taking into account comment 11 and comment 12.

	Martijn Wargers (dead) Assignee
	Comment 14 • 9 years ago

Thanks! 
Merged: https://github.com/mozilla-b2g/gaia/commit/9112cd1606af42725da741a3f3948270ba2228aa

Leaving this open to address the improving of the comment regarding this pull request.

	Frederik Braun [:freddy]
	Comment 15 • 9 years ago

(In reply to Johan Lorenzo [:jlorenzo] (QA) from comment #11)
> Comment on attachment 8635256 [details] [review]
> [gaia] mwargers:1181662 > mozilla-b2g:master
> 
> I'm sorry, I'll need a bit more of explanation to understand the patch :s
> 
> * What are ArrayBuffers in this context?

The code for setting the passcode uses ArrayBuffers. The settings DB does not support this and sees an array buffer of [3,6,9] objects of the format {0: 3, 1: 6, 2: 9} (hence objects with keys from 0 to n-1)


> * Is, in this particular example, n equal to 20?

n is just array.length So 8 for the salt and 20 for the digest.

> * The digest is the result of 1000 pre-computations, isn't it? Is the salt
> also digested? If so, how was it? If not, what do represent the values
> associated to [0..7]?

Yes, PBKDF2 takes the following inputs:

The plain text passcode (here 1337), the amount of iterations (1000), the salt and the name of the internal hash function (our webcrypto imlementation only support SHA-1, but we intend to upgrade once we support SHA-2).

I wanted to avoid implementing a complicated password hashing scheme in Python, that's why we pre-computed it for the test. 

The salt is generated randomly whenever you set a new password. This is to protect against building a dictionary. The salt can be stored in the settings, next to the digest of the pin code. The verification routine needs to know all these settings to make sure it checks using the correct parameters.

This is also to allow us changing the security settings of our passcode storage (e.g. increasing iterations, changing hash algo etc.).


(Thanks for asking all of these questions! Tim wrote about the architecture on his blog <https://timtaubert.de/blog/2015/05/implementing-a-pbkdf2-based-password-storage-scheme-for-firefox-os/>, but *please* continue asking. I realize this whole passcode scheme needs better documentation. I will leave a comment in bug 1100945 to remind me taking care of this, when I come back from PTO)

	Martijn Wargers (dead) Assignee
	Comment 17 • 9 years ago

Comment on attachment 8640587 [details] [review]
[gaia] mwargers:1181662_explanation > mozilla-b2g:master

This is based on comment 15. Is this a correct enough explanation, Freddy? Otherwise, could you perhaps improve it? I don't really understand this stuff, to be honest.

	Frederik Braun [:freddy]
	Comment 18 • 9 years ago

Comment on attachment 8640587 [details] [review]
[gaia] mwargers:1181662_explanation > mozilla-b2g:master

Looks good to me!

	Martijn Wargers (dead) Assignee
	Comment 19 • 9 years ago

Comment on attachment 8640587 [details] [review]
[gaia] mwargers:1181662_explanation > mozilla-b2g:master

You're happy with the explanation too, Johan?

	Martijn Wargers (dead) Assignee
	Comment 20 • 9 years ago

Never mind, I just checked the improved explanation comment in: https://github.com/mozilla-b2g/gaia/commit/a18f1c55ecdc19a8d643177c2948407790833b6a