Bug 1652609 - When generating a JWE, include `kid` of the target key if known. r=m_and_m,lina:m_and_m,:lina

Differential Revision: https://phabricator.services.mozilla.com/D83426

services/crypto/modules/jwcrypto.jsm

@@ -63,8 +63,6 @@ class JWCrypto {
    * from https://tools.ietf.org/html/rfc7516. The only supported content encryption
    * algorithm is enc="A256GCM" [1] and the only supported key encryption algorithm
    * is alg="ECDH-ES" [2].
-   * The IV is generated randomly: if you are using long-lived keys you might be
-   * exposing yourself to a birthday attack. Please consult your nearest cryptographer.
    *
    * @param {Object} key Peer Public JWK.
    * @param {ArrayBuffer} data
@@ -76,9 +74,18 @@ class JWCrypto {
    */
   async generateJWE(key, data) {
     // Generate an ephemeral key to use just for this encryption.
+    // The public component gets embedded in the JWE header.
     const epk = await crypto.subtle.generateKey(ECDH_PARAMS, true, [
       "deriveKey",
     ]);
+    const ownPublicJWK = await crypto.subtle.exportKey("jwk", epk.publicKey);
+    // Remove properties added by our WebCrypto implementation but that aren't typically
+    // used with JWE in the wild. This saves space in the resulting JWE, and makes it easier
+    // to re-import the resulting JWK.
+    delete ownPublicJWK.key_ops;
+    delete ownPublicJWK.ext;
+    let header = { alg: "ECDH-ES", enc: "A256GCM", epk: ownPublicJWK };
+    // Import the peer's public key.
     const peerPublicKey = await crypto.subtle.importKey(
       "jwk",
       key,
@@ -86,20 +93,20 @@ class JWCrypto {
       false,
       ["deriveKey"]
     );
-    return this._generateJWE(epk, peerPublicKey, data);
-  }
-
-  async _generateJWE(epk, peerPublicKey, data) {
-    let iv = crypto.getRandomValues(new Uint8Array(AES_GCM_IV_SIZE));
-    const ownPublicJWK = await crypto.subtle.exportKey("jwk", epk.publicKey);
-    delete ownPublicJWK.key_ops;
+    if (key.hasOwnProperty("kid")) {
+      header.kid = key.kid;
+    }
     // Do ECDH agreement to get the content encryption key.
     const contentKey = await deriveECDHSharedAESKey(
       epk.privateKey,
       peerPublicKey,
       ["encrypt"]
     );
-    let header = { alg: "ECDH-ES", enc: "A256GCM", epk: ownPublicJWK };
+    // Encrypt with AES-GCM using the generated key.
+    // Note that the IV is generated randomly, which *in general* is not safe to do with AES-GCM because
+    // it's too short to guarantee uniqueness. But we know that the AES-GCM key itself is unique and will
+    // only be used for this single encryption, making a random IV safe to use for this particular use-case.
+    let iv = crypto.getRandomValues(new Uint8Array(AES_GCM_IV_SIZE));
     // Yes, additionalData is the byte representation of the base64 representation of the stringified header.
     const additionalData = UTF8_ENCODER.encode(
       ChromeUtils.base64URLEncode(UTF8_ENCODER.encode(JSON.stringify(header)), {
@@ -116,10 +123,11 @@ class JWCrypto {
       contentKey,
       data
     );
+    // JWE needs the authentication tag as a separate string.
     const tagIdx = encrypted.byteLength - ((AES_TAG_LEN + 7) >> 3);
     let ciphertext = encrypted.slice(0, tagIdx);
     let tag = encrypted.slice(tagIdx);
-    // JWE serialization.
+    // JWE serialization in compact format.
     header = UTF8_ENCODER.encode(JSON.stringify(header));
     header = ChromeUtils.base64URLEncode(header, { pad: false });
     tag = ChromeUtils.base64URLEncode(tag, { pad: false });

services/crypto/tests/unit/test_jwcrypto.js

@@ -43,8 +43,8 @@ const generateKeyPair = promisify(jwcrypto.generateKeyPair);
 const generateAssertion = promisify(jwcrypto.generateAssertion);
 
 add_task(async function test_jwe_roundtrip_ecdh_es_encryption() {
-  const data = crypto.getRandomValues(new Uint8Array(123));
-  const localEpk = await crypto.subtle.generateKey(
+  const plaintext = crypto.getRandomValues(new Uint8Array(123));
+  const remoteKey = await crypto.subtle.generateKey(
     {
       name: "ECDH",
       namedCurve: "P-256",
@@ -52,7 +52,16 @@ add_task(async function test_jwe_roundtrip_ecdh_es_encryption() {
     true,
     ["deriveKey"]
   );
-  const remoteEpk = await crypto.subtle.generateKey(
+  const remoteJWK = await crypto.subtle.exportKey("jwk", remoteKey.publicKey);
+  delete remoteJWK.key_ops;
+  const jwe = await jwcrypto.generateJWE(remoteJWK, plaintext);
+  const decrypted = await jwcrypto.decryptJWE(jwe, remoteKey.privateKey);
+  Assert.deepEqual(plaintext, decrypted);
+});
+
+add_task(async function test_jwe_header_includes_key_id() {
+  const plaintext = crypto.getRandomValues(new Uint8Array(123));
+  const remoteKey = await crypto.subtle.generateKey(
     {
       name: "ECDH",
       namedCurve: "P-256",
@@ -60,9 +69,17 @@ add_task(async function test_jwe_roundtrip_ecdh_es_encryption() {
     true,
     ["deriveKey"]
   );
-  const jwe = await jwcrypto._generateJWE(localEpk, remoteEpk.publicKey, data);
-  const decryptedJWE = await jwcrypto.decryptJWE(jwe, remoteEpk.privateKey);
-  Assert.deepEqual(data, decryptedJWE);
+  const remoteJWK = await crypto.subtle.exportKey("jwk", remoteKey.publicKey);
+  delete remoteJWK.key_ops;
+  remoteJWK.kid = "key identifier";
+  const jwe = await jwcrypto.generateJWE(remoteJWK, plaintext);
+  let [header /* other items deliberately ignored */] = jwe.split(".");
+  header = JSON.parse(
+    new TextDecoder().decode(
+      ChromeUtils.base64URLDecode(header, { padding: "reject" })
+    )
+  );
+  Assert.equal(header.kid, "key identifier");
 });
 
 add_task(async function test_sanity() {