Bug 1257821 - Remove the authenticated `aesgcm128` content coding scheme. r=mt,marco draft
authorKit Cambridge <kcambridge@mozilla.com>
Tue, 22 Mar 2016 19:20:36 -0700
changeset 346336 6759d56f526d09d4f845ba1d6a2f63fac62cb3bc
parent 346335 7614fc1c6285d44b94388f45e7dafb79d8314d89
child 517419 f164cc6aec8e9472d2d1f6f4484b17918a32602b
push id14346
push userkcambridge@mozilla.com
push dateThu, 31 Mar 2016 21:31:31 +0000
reviewersmt, marco
bugs1257821
milestone48.0a1
Bug 1257821 - Remove the authenticated `aesgcm128` content coding scheme. r=mt,marco MozReview-Commit-ID: 5pX2GHJ2lNz
dom/push/PushCrypto.jsm
dom/push/PushRecord.jsm
dom/push/PushService.jsm
dom/push/test/xpcshell/test_crypto.js
dom/push/test/xpcshell/test_notification_data.js
--- a/dom/push/PushCrypto.jsm
+++ b/dom/push/PushCrypto.jsm
@@ -55,51 +55,46 @@ function getEncryptionParams(encryptFiel
   return p.split(';').reduce(parseHeaderFieldParams, {});
 }
 
 this.getCryptoParams = function(headers) {
   if (!headers) {
     return null;
   }
 
-  var requiresAuthenticationSecret = true;
   var keymap;
   var padSize;
   if (headers.encoding == AESGCM_ENCODING) {
     // aesgcm uses the Crypto-Key header, 2 bytes for the pad length, and an
     // authentication secret.
+    // https://tools.ietf.org/html/draft-ietf-httpbis-encryption-encoding-01
     keymap = getEncryptionKeyParams(headers.crypto_key);
     padSize = 2;
   } else if (headers.encoding == AESGCM128_ENCODING) {
-    // aesgcm128 uses Crypto-Key or Encryption-Key, and 1 byte for the pad
-    // length.
-    keymap = getEncryptionKeyParams(headers.crypto_key);
+    // aesgcm128 uses Encryption-Key, 1 byte for the pad length, and no secret.
+    // https://tools.ietf.org/html/draft-thomson-http-encryption-02
+    keymap = getEncryptionKeyParams(headers.encryption_key);
     padSize = 1;
-    if (!keymap) {
-      // Encryption-Key header indicates unauthenticated encryption.
-      requiresAuthenticationSecret = false;
-      keymap = getEncryptionKeyParams(headers.encryption_key);
-    }
   }
   if (!keymap) {
     return null;
   }
 
   var enc = getEncryptionParams(headers.encryption);
   if (!enc) {
     return null;
   }
   var dh = keymap[enc.keyid || DEFAULT_KEYID];
   var salt = enc.salt;
   var rs = (enc.rs)? parseInt(enc.rs, 10) : 4096;
 
   if (!dh || !salt || isNaN(rs) || (rs <= padSize)) {
     return null;
   }
-  return {dh, salt, rs, auth: requiresAuthenticationSecret, padSize};
+  return {dh, salt, rs, padSize};
 }
 
 var parseHeaderFieldParams = (m, v) => {
   var i = v.indexOf('=');
   if (i >= 0) {
     // A quoted string with internal quotes is invalid for all the possible
     // values of this header field.
     m[v.substring(0, i).trim()] = v.substring(i + 1).trim()
@@ -254,49 +249,41 @@ this.PushCrypto = {
     .then(r => concatArray(r));
   },
 
   _deriveKeyAndNonce(padSize, ikm, salt, receiverKey, senderKey,
                      authenticationSecret) {
     var kdfPromise;
     var context;
     var encryptInfo;
-    // The authenticationSecret, when present, is mixed with the ikm using HKDF.
-    // This is its primary purpose.  However, since the authentication secret
-    // was added at the same time that the info string was changed, we also use
-    // its presence to change how the final info string is calculated:
+    // The size of the padding determines which key derivation we use.
     //
-    // 1. When there is no authenticationSecret, the context string is simply
-    // "Content-Encoding: <blah>". This corresponds to old, deprecated versions
-    // of the content encoding.  This should eventually be removed: bug 1230038.
+    // 1. If the pad size is 1, we assume "aesgcm128". This scheme ignores the
+    // authenticationSecret, and uses "Content-Encoding: <blah>" for the
+    // context string. It should eventually be removed: bug 1230038.
     //
-    // 2. When there is an authenticationSecret, the context string is:
+    // 2. If the pad size is 2, we assume "aesgcm", and mix the
+    // authenticationSecret with the ikm using HKDF. The context string is:
     // "Content-Encoding: <blah>\0P-256\0" then the length and value of both the
     // receiver key and sender key.
-    if (authenticationSecret) {
+    if (padSize == 2) {
       // Since we are using an authentication secret, we need to run an extra
       // round of HKDF with the authentication secret as salt.
       var authKdf = new hkdf(authenticationSecret, ikm);
       kdfPromise = authKdf.extract(AUTH_INFO, 32)
         .then(ikm2 => new hkdf(salt, ikm2));
 
-      // We also use the presence of the authentication secret to indicate that
-      // we have extra context to add to the info parameter.
+      // aesgcm requires extra context for the info parameter.
       context = concatArray([
         new Uint8Array([0]), P256DH_INFO,
         this._encodeLength(receiverKey), receiverKey,
         this._encodeLength(senderKey), senderKey
       ]);
-      // Finally, we use the pad size to infer the content encoding.
-      encryptInfo = padSize == 2 ? AESGCM_ENCRYPT_INFO :
-                                   AESGCM128_ENCRYPT_INFO;
+      encryptInfo = AESGCM_ENCRYPT_INFO;
     } else {
-      if (padSize == 2) {
-        throw new Error("aesgcm encoding requires an authentication secret");
-      }
       kdfPromise = Promise.resolve(new hkdf(salt, ikm));
       context = new Uint8Array(0);
       encryptInfo = AESGCM128_ENCRYPT_INFO;
     }
     return kdfPromise.then(kdf => Promise.all([
       kdf.extract(concatArray([encryptInfo, context]), 16)
         .then(gcmBits => crypto.subtle.importKey('raw', gcmBits, 'AES-GCM', false,
                                                  ['decrypt'])),
--- a/dom/push/PushRecord.jsm
+++ b/dom/push/PushRecord.jsm
@@ -221,16 +221,21 @@ PushRecord.prototype = {
   matchesOriginAttributes(pattern) {
     if (this.systemRecord) {
       return false;
     }
     return ChromeUtils.originAttributesMatchPattern(
       this.principal.originAttributes, pattern);
   },
 
+  hasAuthenticationSecret() {
+    return !!this.authenticationSecret &&
+           this.authenticationSecret.byteLength == 16;
+  },
+
   toSubscription() {
     return {
       endpoint: this.pushEndpoint,
       lastPush: this.lastPush,
       pushCount: this.pushCount,
       p256dhKey: this.p256dhPublicKey,
       authenticationSecret: this.authenticationSecret,
       quota: this.quotaApplies() ? this.quota : -1,
--- a/dom/push/PushService.jsm
+++ b/dom/push/PushService.jsm
@@ -711,17 +711,17 @@ this.PushService = {
 
   notifySubscriptionChanges: function(records) {
     records.forEach(record => {
       this._notifySubscriptionChangeObservers(record);
     });
   },
 
   ensureCrypto: function(record) {
-    if (record.authenticationSecret &&
+    if (record.hasAuthenticationSecret() &&
         record.p256dhPublicKey &&
         record.p256dhPrivateKey) {
       return Promise.resolve(record);
     }
 
     let keygen = Promise.resolve([]);
     if (!record.p256dhPublicKey || !record.p256dhPrivateKey) {
       keygen = PushCrypto.generateKeys();
@@ -730,17 +730,17 @@ this.PushService = {
     // is only going to happen on db upgrade from version 4 to higher.
     return keygen
       .then(([pubKey, privKey]) => {
         return this.updateRecordAndNotifyApp(record.keyID, record => {
           if (!record.p256dhPublicKey || !record.p256dhPrivateKey) {
             record.p256dhPublicKey = pubKey;
             record.p256dhPrivateKey = privKey;
           }
-          if (!record.authenticationSecret) {
+          if (!record.hasAuthenticationSecret()) {
             record.authenticationSecret = PushCrypto.generateAuthenticationSecret();
           }
           return record;
         });
       }, error => {
         return this.dropRegistrationAndNotifyApp(record.keyID).then(
           () => Promise.reject(error));
       });
@@ -857,17 +857,17 @@ this.PushService = {
     }
     return PushCrypto.decodeMsg(
       data,
       record.p256dhPrivateKey,
       record.p256dhPublicKey,
       cryptoParams.dh,
       cryptoParams.salt,
       cryptoParams.rs,
-      cryptoParams.auth ? record.authenticationSecret : null,
+      record.authenticationSecret,
       cryptoParams.padSize
     ).then(message => this._notifyApp(record, messageID, message), error => {
       let message = gDOMBundle.formatStringFromName(
         "PushMessageDecryptionFailure", [record.scope, String(error)], 2);
       gPushNotifier.notifyError(record.scope, record.principal, message,
                                 Ci.nsIScriptError.errorFlag);
       return Ci.nsIPushErrorReporter.ACK_DECRYPTION_ERROR;
     });
--- a/dom/push/test/xpcshell/test_crypto.js
+++ b/dom/push/test/xpcshell/test_crypto.js
@@ -19,92 +19,82 @@ add_task(function* test_crypto_getCrypto
       encoding: 'aesgcm',
       crypto_key: 'keyid=p256dh;dh=Iy1Je2Kv11A,p256ecdsa=o2M8QfiEKuI',
       encryption: 'keyid=p256dh;salt=upk1yFkp1xI',
     },
     params: {
       dh: 'Iy1Je2Kv11A',
       salt: 'upk1yFkp1xI',
       rs: 4096,
-      auth: true,
       padSize: 2,
     },
   }, {
     desc: 'aesgcm with quoted key param',
     headers: {
       encoding: 'aesgcm',
       crypto_key: 'dh="byfHbUffc-k"',
       encryption: 'salt=C11AvAsp6Gc',
     },
     params: {
       dh: 'byfHbUffc-k',
       salt: 'C11AvAsp6Gc',
       rs: 4096,
-      auth: true,
       padSize: 2,
     },
   }, {
     desc: 'aesgcm with Crypto-Key and rs = 24',
     headers: {
       encoding: 'aesgcm',
       crypto_key: 'dh="ybuT4VDz-Bg"',
       encryption: 'salt=H7U7wcIoIKs; rs=24',
     },
     params: {
       dh: 'ybuT4VDz-Bg',
       salt: 'H7U7wcIoIKs',
       rs: 24,
-      auth: true,
       padSize: 2,
     },
   }, {
     desc: 'aesgcm128 with Encryption-Key and rs = 2',
     headers: {
       encoding: 'aesgcm128',
       encryption_key: 'keyid=legacy; dh=LqrDQuVl9lY',
       encryption: 'keyid=legacy; salt=YngI8B7YapM; rs=2',
     },
     params: {
       dh: 'LqrDQuVl9lY',
       salt: 'YngI8B7YapM',
       rs: 2,
-      auth: false,
       padSize: 1,
     },
   }, {
     desc: 'aesgcm128 with Encryption-Key',
     headers: {
       encoding: 'aesgcm128',
       encryption_key: 'keyid=v2; dh=VA6wmY1IpiE',
       encryption: 'keyid=v2; salt=khtpyXhpDKM',
     },
     params: {
       dh: 'VA6wmY1IpiE',
       salt: 'khtpyXhpDKM',
       rs: 4096,
-      auth: false,
       padSize: 1,
     }
-  }, {
+  },
+
+  // These headers should be rejected.
+  {
     desc: 'aesgcm128 with Crypto-Key',
     headers: {
       encoding: 'aesgcm128',
       crypto_key: 'keyid=v2; dh=VA6wmY1IpiE',
       encryption: 'keyid=v2; salt=F0Im7RtGgNY',
     },
-    params: {
-      dh: 'VA6wmY1IpiE',
-      salt: 'F0Im7RtGgNY',
-      rs: 4096,
-      auth: true,
-      padSize: 1,
-    },
+    params: null,
   },
-
-  // These headers should be rejected.
   {
     desc: 'Invalid encoding',
     headers: {
       encoding: 'nonexistent',
     },
     params: null,
   }, {
     desc: 'Invalid record size',
@@ -135,23 +125,23 @@ add_task(function* test_crypto_getCrypto
   for (let test of testData) {
     let params = getCryptoParams(test.headers);
     deepEqual(params, test.params, test.desc);
   }
 });
 
 add_task(function* test_crypto_decodeMsg() {
   let privateKey = {
-    "crv": "P-256",
-    "d": "4h23G_KkXC9TvBSK2v0Q7ImpS2YAuRd8hQyN0rFAwBg",
-    "ext": true,
-    "key_ops": ["deriveBits"],
-    "kty":"EC",
-    "x":"sd85ZCbEG6dEkGMCmDyGBIt454Qy-Yo-1xhbaT2Jlk4",
-    "y":"vr3cKpQ-Sp1kpZ9HipNjUCwSA55yy0uM8N9byE8dmLs",
+    crv: 'P-256',
+    d: '4h23G_KkXC9TvBSK2v0Q7ImpS2YAuRd8hQyN0rFAwBg',
+    ext: true,
+    key_ops: ['deriveBits'],
+    kty: 'EC',
+    x: 'sd85ZCbEG6dEkGMCmDyGBIt454Qy-Yo-1xhbaT2Jlk4',
+    y: 'vr3cKpQ-Sp1kpZ9HipNjUCwSA55yy0uM8N9byE8dmLs',
   };
   let publicKey = base64UrlDecode('BLHfOWQmxBunRJBjApg8hgSLeOeEMvmKPtcYW2k9iZZOvr3cKpQ-Sp1kpZ9HipNjUCwSA55yy0uM8N9byE8dmLs');
 
   let expectedSuccesses = [{
     desc: 'padSize = 2, rs = 24, pad = 0',
     result: 'Some message',
     data: 'Oo34w2F9VVnTMFfKtdx48AZWQ9Li9M6DauWJVgXU',
     senderPublicKey: 'BCHFVrflyxibGLlgztLwKelsRZp4gqX3tNfAKFaxAcBhpvYeN1yIUMrxaDKiLh4LNKPtj0BOXGdr-IQ-QP82Wjo',
@@ -180,37 +170,36 @@ add_task(function* test_crypto_decodeMsg
     desc: 'padSize = 2, rs = 3, pad = 0',
     result: 'Small record size',
     data: 'oY4e5eDatDVt2fpQylxbPJM-3vrfhDasfPc8Q1PWt4tPfMVbz_sDNL_cvr0DXXkdFzS1lxsJsj550USx4MMl01ihjImXCjrw9R5xFgFrCAqJD3GwXA1vzS4T5yvGVbUp3SndMDdT1OCcEofTn7VC6xZ-zP8rzSQfDCBBxmPU7OISzr8Z4HyzFCGJeBfqiZ7yUfNlKF1x5UaZ4X6iU_TXx5KlQy_toV1dXZ2eEAMHJUcSdArvB6zRpFdEIxdcHcJyo1BIYgAYTDdAIy__IJVCPY_b2CE5W_6ohlYKB7xDyH8giNuWWXAgBozUfScLUVjPC38yJTpAUi6w6pXgXUWffende5FreQpnMFL1L4G-38wsI_-ISIOzdO8QIrXHxmtc1S5xzYu8bMqSgCinvCEwdeGFCmighRjj8t1zRWo0D14rHbQLPR_b1P5SvEeJTtS9Nm3iibM',
     senderPublicKey: 'BCg6ZIGuE2ZNm2ti6Arf4CDVD_8--aLXAGLYhpghwjl1xxVjTLLpb7zihuEOGGbyt8Qj0_fYHBP4ObxwJNl56bk',
     salt: '5LIDBXbvkBvvb7ZdD-T4PQ',
     rs: 3,
     authSecret: 'g2rWVHUCpUxgcL9Tz7vyeQ',
     padSize: 2,
-  }, {
-    desc: 'padSize = 1, rs = 4096, auth secret, pad = 8',
-    result: 'aesgcm128 with auth secret',
-    data: 'h0FmyldY8aT5EQ6CJrbfRn_IdDvytoLeHb9_q5CjtdFRfgDRknxLmOzavLaVG4oOiS0r',
-    senderPublicKey: 'BCXHk7O8CE-9AOp6xx7g7c-NCaNpns1PyyHpdcmDaijLbT6IdGq0ezGatBwtFc34BBfscFxdk4Tjksa2Mx5rRCM',
-    salt: 'aGBpoKklLtrLcAUCcCr7JQ',
-    rs: 4096,
-    authSecret: 'Sxb6u0gJIhGEogyLawjmCw',
-    padSize: 1,
   }];
   for (let test of expectedSuccesses) {
     let authSecret = test.authSecret ? base64UrlDecode(test.authSecret) : null;
     let result = yield PushCrypto.decodeMsg(base64UrlDecode(test.data),
                                             privateKey, publicKey,
                                             test.senderPublicKey, test.salt,
                                             test.rs, authSecret, test.padSize);
     let decoder = new TextDecoder('utf-8');
     equal(decoder.decode(new Uint8Array(result)), test.result, test.desc);
   }
 
   let expectedFailures = [{
+    desc: 'padSize = 1, rs = 4096, auth secret, pad = 8',
+    data: 'h0FmyldY8aT5EQ6CJrbfRn_IdDvytoLeHb9_q5CjtdFRfgDRknxLmOzavLaVG4oOiS0r',
+    senderPublicKey: 'BCXHk7O8CE-9AOp6xx7g7c-NCaNpns1PyyHpdcmDaijLbT6IdGq0ezGatBwtFc34BBfscFxdk4Tjksa2Mx5rRCM',
+    salt: 'aGBpoKklLtrLcAUCcCr7JQ',
+    rs: 4096,
+    authSecret: 'Sxb6u0gJIhGEogyLawjmCw',
+    padSize: 1,
+  }, {
     desc: 'Missing padding',
     data: 'anvsHj7oBQTPMhv7XSJEsvyMS4-8EtbC7HgFZsKaTg',
     senderPublicKey: 'BMSqfc3ohqw2DDgu3nsMESagYGWubswQPGxrW1bAbYKD18dIHQBUmD3ul_lu7MyQiT5gNdzn5JTXQvCcpf-oZE4',
     salt: 'Czx2i18rar8XWOXAVDnUuw',
     rs: 4096,
     padSize: 1,
   }, {
     desc: 'padSize > rs',
--- a/dom/push/test/xpcshell/test_notification_data.js
+++ b/dom/push/test/xpcshell/test_notification_data.js
@@ -166,82 +166,63 @@ add_task(function* test_notification_ack
         data: 'LKru3ZzxBZuAxYtsaCfaj_fehkrIvqbVd1iSwnwAUgnL-cTeDD-83blxHXTq7r0z9ydTdMtC3UjAcWi8LMnfY-BFzi0qJAjGYIikDA',
       },
       ackCode: 100,
       receive: {
         scope: 'https://example.com/page/3',
         data: 'Some message'
       }
     },
-    // This message uses the newer, authenticated form based on the crypto-key
-    // header field.  No padding or record size changes.
-    {
-      channelID: 'subscription1',
-      version: 'v4',
-      send: {
-        headers: {
-          crypto_key: 'keyid=v4;dh="BHqG01j7rOfp12BEDzxWXxlCaU4cdOx2DZAwCt3QuzEsnXN9lCna9QmZCkVpXsx7sAlaEmtl_VfF1lHlFS7XWcA"',
-          encryption: 'keyid="v4";salt="X5-iy5rzhm4naNmMHdSYJw"',
-          encoding: 'aesgcm128',
-        },
-        data: '7YlxyNlZsNX4UNknHxzTqFrcrzz58W95uXBa0iY',
-      },
-      ackCode: 100,
-      receive: {
-        scope: 'https://example.com/page/1',
-        data: 'Some message'
-      }
-    },
-    // A message encoded with `aesgcm` (2 bytes of padding).
+    // A message encoded with `aesgcm` (2 bytes of padding, authenticated).
     {
       channelID: 'subscription1',
       version: 'v5',
       send: {
         headers: {
-          crypto_key: 'dh="BMh_vsnqu79ZZkMTYkxl4gWDLdPSGE72Lr4w2hksSFW398xCMJszjzdblAWXyhSwakRNEU_GopAm4UGzyMVR83w"',
-          encryption: 'salt="C14Wb7rQTlXzrgcPHtaUzw"',
+          crypto_key: 'keyid=v4;dh="BMh_vsnqu79ZZkMTYkxl4gWDLdPSGE72Lr4w2hksSFW398xCMJszjzdblAWXyhSwakRNEU_GopAm4UGzyMVR83w"',
+          encryption: 'keyid="v4";salt="C14Wb7rQTlXzrgcPHtaUzw"',
           encoding: 'aesgcm',
         },
         data: 'pus4kUaBWzraH34M-d_oN8e0LPpF_X6acx695AMXovDe',
       },
       ackCode: 100,
       receive: {
         scope: 'https://example.com/page/1',
         data: 'Another message'
       }
     },
     // A message with 17 bytes of padding and rs of 24
     {
       channelID: 'subscription2',
       version: 'v5',
       send: {
         headers: {
-          crypto_key: 'keyid="v5"; dh="BJhyKIH5P30YUKn1bolj_LMnael1-KZT_aGXgD2CRspBfv9gcUhVAmpxToZrw7QQEKl9K83b3zcqNY6G_dFhEsI"',
-          encryption: 'keyid=v5;salt="bLmqCy550eK1Ao41tD7orA";rs=24',
-          encoding: 'aesgcm128',
+          crypto_key: 'keyid="v5"; dh="BOp-DpyR9eLY5Ci11_loIFqeHzWfc_0evJmq7N8NKzgp60UAMMM06XIi2VZp2_TSdw1omk7E19SyeCCwRp76E-U"',
+          encryption: 'keyid=v5;salt="TvjOou1TqJOQY_ZsOYV3Ww";rs=24',
+          encoding: 'aesgcm',
         },
-        data: 'SQDlDg1ftLkM_ruZlmyB2bk9L78HYtkcbA-y4-uAxwL-G4KtOA-J-A_rJ007Vi6NUkQe9K4kSZeIBrIUpmGv',
+        data: 'rG9WYQ2ZwUgfj_tMlZ0vcIaNpBN05FW-9RUBZAM-UUZf0_9eGpuENBpUDAw3mFmd2XJpmvPvAtLVs54l3rGwg1o',
       },
       ackCode: 100,
       receive: {
         scope: 'https://example.com/page/2',
         data: 'Some message'
       }
     },
     // A message without key identifiers.
     {
       channelID: 'subscription3',
       version: 'v6',
       send: {
         headers: {
-          crypto_key: 'dh="BEgnDmVw9Gcn1fWA5t53Jtpsgfewk_pzsjSc_PBPpPmROWGQA2v8ESrSsQgosNXx0o-uMMhi9tDAUeks3380kd8"',
-          encryption: 'salt=T9DM8bNxuMHRVTn4LzkJDQ',
-          encoding: 'aesgcm128',
+          crypto_key: 'dh="BEEjwWbF5jZKCgW0kmUWgG-wNcRvaa9_3zZElHAF8przHwd4cp5_kQsc-IMNZcVA0iUix31jxuMOytU-5DwWtyQ"',
+          encryption: 'salt=aAQcr2khAksgNspPiFEqiQ',
+          encoding: 'aesgcm',
         },
-        data: '7KUCi0dBBJbWmsYTqEqhFrgTv4ZOo_BmQRQ_2kY',
+        data: 'pEYgefdI-7L46CYn5dR9TIy2AXGxe07zxclbhstY',
       },
       ackCode: 100,
       receive: {
         scope: 'https://example.com/page/3',
         data: 'Some message'
       }
     },
     // A malformed encrypted message.