968567 - Expose the NSS implementation of PBKDF2 HMAC SHA256 from bug 974162 to chrome JS for use by FxAccounts

Status: RESOLVED INCOMPLETE

(Core :: Security: PSM, defect)

Description

[Jed Parsons (use needinfo, please) [:jedp, :jparsons]]

Needed for Firefox Accounts on b2g and desktop.

The implementation in services/crypto/modules/utils.js (pbkdf2Generate) is too slow for our production needs.  (1000 rounds on my 2.3GHz laptop takes 3.5 seconds.)

We will need a way to call this from javascript.  We will also need a way to return the computed DK to the caller.

Comment 1

[Chris Karlof [:ckarlof]]

:bsmith, it turns out, it's ass slow the way we're doing it now. We'd like please.

Comment 2

[Richard Newman [:rnewman]]

Hooray traction on Bug 554827!

Comment 3

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

(In reply to Richard Newman [:rnewman] from comment #2)
> Hooray traction on Bug 554827!

Yes, in a couple of weeks I will have time to work on this.

If anybody wants to steal this from me, I believe that fixing bug 554827 is the starting point, and that it shouldn't be unintuitive. Steal away!

Comment 4

[Chris Karlof [:ckarlof]]

If there's a path that would get this done by Feb 14, that would be fantastic.

Comment 6

[Sam Penrose]

Hey Brian -- quick status ping. 13 days ago you said you'd have time to work on this in two weeks. I imagine you haven't started, which is fine, but FYI our code freeze date is March 17th, in 19 working days. Does getting this bug landed in that time frame look doable? Thanks, Sam

Comment 7

[Richard Barnes [:rbarnes]]

I'm confused as to why this is not just an instance of WebCrypto (Bug 865789).  It doesn't seem like we should implement one API just for this bug, only to have it replaced by the full API.  ETA for WebCrypto is still TBD, but could be as soon as March/April.

Comment 8

[Jed Parsons (use needinfo, please) [:jedp, :jparsons]]

That is likely the right thing to do for the long run.  But for now, a performant pbkdf2 hmac sha256 is required for Firefox Accounts usability on FirefoxOS 1.4, which must be code-complete in about 3.5 weeks.

Comment 9

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

(In reply to Richard Barnes [:rbarnes] from comment #7)
> I'm confused as to why this is not just an instance of WebCrypto (Bug
> 865789).  It doesn't seem like we should implement one API just for this
> bug, only to have it replaced by the full API.  ETA for WebCrypto is still
> TBD, but could be as soon as March/April.

Richard, that would be the best way of doing it, except that this bug has to be fixed in Firefox 29, and we're not going to uplift WebCrypto to Firefox 29. The reason it is required for Firefox 29 is because we have a requirement that the Firefox Accounts feature be in Firefox 29. In that sense, this is not a normal bug.

Comment 10

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

(In reply to Sam Penrose from comment #6)
> Hey Brian -- quick status ping. 13 days ago you said you'd have time to work
> on this in two weeks. I imagine you haven't started, which is fine, but FYI
> our code freeze date is March 17th, in 19 working days. Does getting this
> bug landed in that time frame look doable? Thanks, Sam

Hi Sam, as you can see in the status whiteboard, ETA for my part of this is end of February. I believe I will fix my part of this bug (getting the password into NSS, and getting the stretched password/key out of NSS) before then. However, I am not planning to implement the DOM parts of this bug. Please keep that in mind when you are tracking this. It may be a good idea for us to split this bug into two parts: the part I'm working on, and the DOM part that somebody else will do. In fact, I'm going to split this bug that way right now.

Comment 11

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Sam, please find somebody who knows how to do DOM work to take this part of the bug. The NSS-related parts that I'm doing are now bug 974162.

Comment 12

[Sam Penrose]

Will do. Thanks very much Brian.

Comment 13

[(no longer active)]

Can someone please explain what is requested here?  Comment 0 kind of scares me...

Comment 14

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Ehsan, I wrote "web content" but I think that is bad wording.

Jed, do you need the PKBDF2 function exposed to *web* content, or to *xul* content?

Ehsan, this is what your concern is, right?

Comment 15

[(no longer active)]

(In reply to Brian Smith (:briansmith, was :bsmith; NEEDINFO? for response) from comment #14)
> Ehsan, this is what your concern is, right?

Exposing this to *web* content.  Exposing it to chrome only code is fine.

Comment 16

[Richard Barnes [:rbarnes]]

(In reply to :Ehsan Akhgari (needinfo? me!) (slow responsiveness, emailapocalypse) from comment #15)
> (In reply to Brian Smith (:briansmith, was :bsmith; NEEDINFO? for response)
> from comment #14)
> > Ehsan, this is what your concern is, right?
> 
> Exposing this to *web* content.  Exposing it to chrome only code is fine.

I had the same concern.  I found jedp on IRC, and he assured me that the need is only for chrome.

Comment 17

[(no longer active)]

(In reply to comment #16)
> (In reply to :Ehsan Akhgari (needinfo? me!) (slow responsiveness,
> emailapocalypse) from comment #15)
> > (In reply to Brian Smith (:briansmith, was :bsmith; NEEDINFO? for response)
> > from comment #14)
> > > Ehsan, this is what your concern is, right?
> > 
> > Exposing this to *web* content.  Exposing it to chrome only code is fine.
> 
> I had the same concern.  I found jedp on IRC, and he assured me that the need
> is only for chrome.

Thanks a lot, and sorry for the false alarm!  Please carry on.

Comment 18

[Masatoshi Kimura [:emk]]

Changing the title to remove confusions.

Comment 19

[Jed Parsons (use needinfo, please) [:jedp, :jparsons]]

(In reply to Brian Smith (:briansmith, was :bsmith; NEEDINFO? for response) from comment #14)
> Ehsan, I wrote "web content" but I think that is bad wording.
> 
> Jed, do you need the PKBDF2 function exposed to *web* content, or to *xul*
> content?
> 
> Ehsan, this is what your concern is, right?

This is correct.  We would be calling this function from privileged chrome JS code, not content JS.  Sorry about the confusion.

Specifically, we want to replace services/crypto/modules/utils.js's pbkdf2Generate().

It would be ideal if the function signature of the replacement were similar.  I.e., it would allow the caller to specify P, S, c, dkLen, and optionally the hmac alg and hmac length.

Comment 21

[Jason Smith [:jsmith]]

Backlog - FxAccounts doesn't hit the QC feature list & DSDS feature list, so this doesn't block.

Comment 22

[Sam Penrose]

Per Jason's flag edit, we do not need this by the 28th. We will let Brian know when we are blocked by this one.

Comment 23

[Sylvestre Ledru [:Sylvestre]]

Brian, I guess you won't ask for an uplift for this bug, right? If not, I am going to untrack it for 29. Thanks

Comment 24

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

(In reply to Sylvestre Ledru [:sylvestre] from comment #23)
> Brian, I guess you won't ask for an uplift for this bug, right? If not, I am
> going to untrack it for 29. Thanks

Right.

Comment 25

[Jed Parsons (use needinfo, please) [:jedp, :jparsons]]

Will meet with :rbarnes and :spenrose tomorrow to talk about how to go forward

Comment 26

[Richard Barnes [:rbarnes]]

Talked with :spenrose this afternoon.  I will check with Wan-Teh on the status of Bug 554827, and make that closes.  I'm glad to be a reviewer for this bug.

Comment 27

[Sam Penrose]

Jed, Q/A does not currently feel this performance issue is a priority. Are you comfortable with my closing this WONTFIX?

Comment 28

[Richard Newman [:rnewman]]

Suspect that INCOMPLETE is a better resolution, if any. WONTFIX kinda implies that we don't want it, which is the opposite of truth, no? :)

Comment 29

[Sam Penrose]

(In reply to Richard Newman [:rnewman] from comment #28)
> Suspect that INCOMPLETE is a better resolution, if any. WONTFIX kinda
> implies that we don't want it, which is the opposite of truth, no? :)

Richard: a fair question, to which I believe the answer is more "no" than "yes". I believe strongly that performance is meaningless outside the context of systems. I believe strongly that some form of whole-system acceptance testing should determine whether we have a performance issue. Currently I believe this responsibility falls generally under Q/A. Jed saw problems months ago on his MBP. I feel strongly that, given anecdotal evidence that we do not have a problem on device today, we should not give the initial case any special weight at all. Conversely, if we do feel there is a performance problem on device (which to my knowledge we do not), we should look at it with fresh eyes, and not assume that it is related to the issue he singles out here. I don't know or much care which resolution status all of that maps to; I care very much that we not invest engineering effort into bugs just because they made sense in another context months ago.

Comment 30

[Edwin Wong [:edwong]]

rpappa or njpark, could you get wall clock time on Buri/Flame to submit username:password for signup/signin till the next screen appears.  These numbers should drive the decision if performance is within out goals for FxOS.  From my own experience signing in, I'm guessing we're at ~2-3s right now on a Buri.

Comment 31

[Richard Barnes [:rbarnes]]

In case it's helpful: Bug 998804 just landed, with WebCrypto support for SHA-256.  It's currently only exposed to content, but if you could figure out how to expose it to chrome JS, it might let you at least optimize the inner loop of PBKDF2.

Comment 32

[No-Jun Park [:njpark]]

On Buri:
From the username prompt to the next screen: 3.19 seconds
From the password prompt to the next screen: 3.95 seconds

On Flame: (on WiFi)
From the username prompt to the next screen: 2.55 seconds
From the password prompt to the next screen: 1.93 seconds

Comment 33

[Edwin Wong [:edwong]]

:njpark - thanks so much! I agree with the QA team, that current performance is within our goals. Ideally, product owners should sign off, but I'm OK marking this leave it open/incomplete/defer