Closed Bug 1270778 Opened 6 years ago Closed 6 years ago

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

Categories

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

defect

Tracking

(firefox47 unaffected, firefox48- fixed, firefox49+ fixed)

RESOLVED FIXED
Tracking Status
firefox47 --- unaffected
firefox48 - fixed
firefox49 + fixed

People

(Reporter: standard8, Assigned: lina)

References

Details

(Keywords: regression)

Attachments

(1 file)

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.
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.
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.
Component: DOM → Client
Product: Core → Hello (Loop)
Version: Trunk → unspecified
Assignee: nobody → kcambridge
Rank: 10
Priority: -- → P1
Comment on attachment 8749592 [details] [review]
[loop] kitcambridge:bug/1270778 > mozilla:master

Thanks for the PR! r=Standard8
Attachment #8749592 - Flags: review+
Landed on master:

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

We'll get this merged across to central in the next release.
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
(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?
Flags: needinfo?(kcambridge)
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?
Flags: needinfo?(kcambridge)
Looks like `crypto.webkitSubtle.importKey` doesn't support JWK in Safari 9.1. I'll test Edge on my Windows box next week.
(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.
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.
Flags: needinfo?(standard8)
Bug 1273671 has landed this on mozilla-central for 49. There's an approval request for 48.
Flags: needinfo?(standard8)
Duplicate of this bug: 1272993
I approved the uplift in bug 1273671, stop tracking this one
You need to log in before you can comment on or make changes to this bug.