1270778 - Hello can no longer decrypt/encrypt room data in FxA accounts "DOMException [DataError: "Data provided to an operation does not meet requirements"]

Status: RESOLVED FIXED

(Hello (Loop) :: Client, defect, P1)

Description

[Mark Banner (:standard8)]

STR:

1) Start Firefox on a fresh profile.
2) Open up Hello, create a room, try and enter it

=> Everything OK.

3) Exit the room, go back to Hello and "Sign in or sign up"
4) Log in with an FxA account (we've been using pre-existing ones with Hello rooms, though I think it'll be reproducible without).
5) Attempt to create a room

=> Fails, error on console:

"Data provided to an operation does not meet requirements"

Similar messages happen for attempting to open a room.

I've used mozregression to track this down to bug 1256488.

Comment 1

[Mark Banner (:standard8)]

The code we're using fails here:

http://hg.mozilla.org/mozilla-central/file/e5a10bc7dac4/browser/extensions/loop/chrome/content/shared/js/crypto.js#l107

  var ALGORITHM = "AES-GCM";
  // We use JSON web key formats for the generated keys.
  // https://tools.ietf.org/html/draft-ietf-jose-json-web-key-41
  var KEY_FORMAT = "jwk";
...
      // First import the key to a format we can use.
      rootObject.crypto.subtle.importKey(KEY_FORMAT,
        { k: key, kty: KEY_TYPE },
        ALGORITHM,
        // If the key is extractable.
        true,
        // What we're using it for.
        ["encrypt"]
      ).then(function(cipherText) {
        ...


The `key` we're supplying here is obtained from FxA and temporarily stored in prefs:

http://hg.mozilla.org/mozilla-central/file/e5a10bc7dac4/browser/extensions/loop/chrome/content/modules/MozLoopService.jsm#l1279

  _fxAOAuthComplete: function(deferred, result, keys) {
    if (keys.kBr) {
      Services.prefs.setCharPref("loop.key.fxa", keys.kBr.k);
    }
    gFxAOAuthClientPromise = null;
    // Note: The state was already verified in FxAccountsOAuthClient.
    deferred.resolve(result);
  },

That callback is called from:

http://hg.mozilla.org/mozilla-central/file/e5a10bc7dac4/services/fxaccounts/FxAccountsOAuthClient.jsm#l223

Hence, Hello isn't doing any translation here, but something definitely seems messed up.

Comment 2

[Lina Butler [:lina]]

Ah, yes. Web Crypto now uses RFC 7515, which disallows padded Base64...but it looks like the FxA "kBr" key has padding. It might make sense to either work around this in crypto.js via `k: key.replace(/=/g, '')`, or in _fxAOAuthComplete.

Comment 3

[GitHub Autolander Bot]

Attachment: [loop] kitcambridge:bug/1270778 > mozilla:master

Comment 4

[Mark Banner (:standard8)]

Comment on attachment 8749592 [details] [review]
[loop] kitcambridge:bug/1270778 > mozilla:master

Thanks for the PR! r=Standard8

Comment 5

[Mark Banner (:standard8)]

Landed on master:

https://github.com/mozilla/loop/commit/dd22f9221b61f8cfdbe97c0639b253388038688a

We'll get this merged across to central in the next release.

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

(In reply to Kit Cambridge [:kitcambridge] from comment #2)
> Ah, yes. Web Crypto now uses RFC 7515, which disallows padded Base64...
Did Web Crypto allow padded base64 before? If so, why was the backward incompatible change made?
Or is this only about our implementation and that Web Crypto spec and other impls always requiring non-padded base64?

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

nm, I saw your comments in the other bug. Does any other browser engine than Gecko and Blink support Web Crypto? If so, how do they behave?

Comment 8

[Lina Butler [:lina]]

Looks like `crypto.webkitSubtle.importKey` doesn't support JWK in Safari 9.1. I'll test Edge on my Windows box next week.

Comment 9

[Lina Butler [:lina]]

(In reply to Kit Cambridge [:kitcambridge] from comment #8)
> Looks like `crypto.webkitSubtle.importKey` doesn't support JWK in Safari
> 9.1.

More specifically, it only supports BufferSource, not JsonWebKey.

Comment 10

[Marcia Knous [:marcia]]

Tracking - Hello Mark can you clarify which loop version will this end up in? And are you going to push it to 48 and 49? Thanks.

Comment 11

[Mark Banner (:standard8)]

Bug 1273671 has landed this on mozilla-central for 49. There's an approval request for 48.

Comment 13

[Sylvestre Ledru [:Sylvestre]]

I approved the uplift in bug 1273671, stop tracking this one