1091005 - [FxOS] Loop/Hello uses SJCL when it could use WebCrypto instead

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Loop, defect)

Description

[Frederik Braun [:freddy]]

It is safer to use built-in WebAPIs for cryptography than those implemented in JavaScript. One argument would be side-channels for cryptography.

Here's a list of how things can be replaced:
* sjlc.misc.hmac with WebCrypto HMAC
* sjlc.codec.* and sjlc.bitArray* with the Encoding API, i.e. https://developer.mozilla.org/en-US/docs/Web/API/Encoding_API


Feel free to reach out to me if you have questions about the specifics. No need to block release (end of the week?) for this, but surely something to take into account for future versions.


AFAIU the only usage of SJCL is in token.js (e.g.: https://github.com/mozilla-b2g/firefoxos-loop-client/blob/f83a3c5aa90c0f78a8047c7efadace01ad01325b/app/libs/token.js)

Comment 1

[Frederik Braun [:freddy]]

It seems the default CCs for Loop are currently not doing triage. Maria, can you take a look at this bug?

Comment 2

[Maria Angeles Oteo (:oteo)]

Sure, first setting ni to Antonio (who is the expert one in these issues) and can provide more detail

Comment 3

[Antonio Manuel Amaya Calvo (:amac)]

Well, we have two problems with this (or maybe three):

* HKDF [1] doesn't seem to be implemented in Firefox's Webcrypto currently. Once that's implemented, this should be direct.
* My initial implementation of this using HMAC.sign doesn't work, unsurprisingly
* And  dom.webcrypto.enabled is  falsein B2G32 (and this has to run in 2.0).

The first two problems might be solvable (by more work) but sadly the third one isn't. So at this point I would say this is a wont fix (at least not for this version).

[1] https://dvcs.w3.org/hg/webcrypto-api/raw-file/tip/spec/Overview.html#hkdf-ctr

Comment 4

[Antonio Manuel Amaya Calvo (:amac)]

Just out of curiosity, I finished my initial version of this. It's available at:

https://github.com/AntonioMA/firefoxos-loop-client/blob/webcrypto-version/app/libs/token_wc.js

in case you want to take a look, Frederik.

I found something that I believe is a bug on webcrypto (an empty array as raw key gives an exception when according to FIPS-198-1, Section 4, step 3 it should expand it to an array of B 0's. In any case, this version gives the same results as the SJCL based one.

But as I said on my previous comments, I don't think we're going to land this on Loop for the time being, since it has to work on 2.0 and there Webcrypto is disabled.

Comment 5

[Frederik Braun [:freddy]]

I wasn't aware of the limitation to make it work on 2.0 - sorry about the overhead I caused.
Code looks good, but I'd give it a more detailed review if this has any chance of landing.

Comment 6

[Antonio Manuel Amaya Calvo (:amac)]

Attachment: Detect and use Webcrypto if available

Note that after fixing this we should also fix hawk.js, which has CryptoJS embedded!

Comment 7

[Frederik Braun [:freddy]]

Comment on attachment 8537189 [details] [diff] [review]
Detect and use Webcrypto if available

Giving f+, since I'm formally not entitled to r+ code that I'm neither peer or module owner of :)

Comment 8

[Cristian Rodriguez (:crdlc)]

Comment on attachment 8537189 [details] [diff] [review]
Detect and use Webcrypto if available

Good job as usual. Left some comments but nothing blocking. Thanks Antonio

Comment 9

[Antonio Manuel Amaya Calvo (:amac)]

Comment on attachment 8537189 [details] [diff] [review]
Detect and use Webcrypto if available

I'm re-requesting review because I added a new unit test (to check the signUp function since I rewrote it again).

Comment 10

[Cristian Rodriguez (:crdlc)]

Comment on attachment 8537189 [details] [diff] [review]
Detect and use Webcrypto if available

LGTM, thanks Antonio

Comment 11

[Cristian Rodriguez (:crdlc)]

in master:

https://github.com/mozilla-b2g/firefoxos-loop-client/commit/23110d0016d4199e78630a70178f6e8010d941e9