fix(ext/crypto) - exportKey JWK for AES/HMAC must use base64url (deno…

…land#13264) Co-authored-by: Divy Srivastava <[email protected]>
DjDeveloperr · Jan 5, 2022 · c4a0a43 · c4a0a43
1 parent 80bf282
commit c4a0a43
Show file tree

Hide file tree

Showing 3 changed files with 160 additions and 36 deletions.
diff --git a/cli/tests/unit/webcrypto_test.ts b/cli/tests/unit/webcrypto_test.ts
@@ -1388,8 +1388,6 @@ Deno.test(async function testImportEcSpkiPkcs8() {
     for (
       const hash of [/*"SHA-1", */ "SHA-256" /*"SHA-384", "SHA-512"*/]
     ) {
-      console.log(hash);
-
       const signatureECDSA = await subtle.sign(
         { name: "ECDSA", hash },
         privateKeyECDSA,
@@ -1420,27 +1418,118 @@ Deno.test(async function testImportEcSpkiPkcs8() {
   }
 });
 
-Deno.test(async function testBase64Forgiving() {
-  const keyData = `{
-    "kty": "oct",
-    "k": "xxx",
-    "alg": "HS512",
-    "key_ops": ["sign", "verify"],
-    "ext": true
-  }`;
-
+async function roundTripSecretJwk(
+  jwk: JsonWebKey,
+  algId: AlgorithmIdentifier | HmacImportParams,
+  ops: KeyUsage[],
+  validateKeys: (
+    key: CryptoKey,
+    originalJwk: JsonWebKey,
+    exportedJwk: JsonWebKey,
+  ) => void,
+) {
   const key = await crypto.subtle.importKey(
     "jwk",
-    JSON.parse(keyData),
-    { name: "HMAC", hash: "SHA-512" },
+    jwk,
+    algId,
     true,
-    ["sign", "verify"],
+    ops,
   );
 
   assert(key instanceof CryptoKey);
   assertEquals(key.type, "secret");
-  assertEquals((key.algorithm as HmacKeyAlgorithm).length, 16);
 
   const exportedKey = await crypto.subtle.exportKey("jwk", key);
-  assertEquals(exportedKey.k, "xxw");
+
+  validateKeys(key, jwk, exportedKey);
+}
+
+Deno.test(async function testSecretJwkBase64Url() {
+  // Test 16bits with "overflow" in 3rd pos of 'quartet', no padding
+  const keyData = `{
+      "kty": "oct",
+      "k": "xxx",
+      "alg": "HS512",
+      "key_ops": ["sign", "verify"],
+      "ext": true
+    }`;
+
+  await roundTripSecretJwk(
+    JSON.parse(keyData),
+    { name: "HMAC", hash: "SHA-512" },
+    ["sign", "verify"],
+    (key, _orig, exp) => {
+      assertEquals((key.algorithm as HmacKeyAlgorithm).length, 16);
+
+      assertEquals(exp.k, "xxw");
+    },
+  );
+
+  // HMAC 128bits with base64url characters (-_)
+  await roundTripSecretJwk(
+    {
+      kty: "oct",
+      k: "HnZXRyDKn-_G5Fx4JWR1YA",
+      alg: "HS256",
+      "key_ops": ["sign", "verify"],
+      ext: true,
+    },
+    { name: "HMAC", hash: "SHA-256" },
+    ["sign", "verify"],
+    (key, orig, exp) => {
+      assertEquals((key.algorithm as HmacKeyAlgorithm).length, 128);
+
+      assertEquals(orig.k, exp.k);
+    },
+  );
+
+  // HMAC 104bits/(12+1) bytes with base64url characters (-_), padding and overflow in 2rd pos of "quartet"
+  await roundTripSecretJwk(
+    {
+      kty: "oct",
+      k: "a-_AlFa-2-OmEGa_-z==",
+      alg: "HS384",
+      "key_ops": ["sign", "verify"],
+      ext: true,
+    },
+    { name: "HMAC", hash: "SHA-384" },
+    ["sign", "verify"],
+    (key, _orig, exp) => {
+      assertEquals((key.algorithm as HmacKeyAlgorithm).length, 104);
+
+      assertEquals("a-_AlFa-2-OmEGa_-w", exp.k);
+    },
+  );
+
+  // AES-CBC 128bits with base64url characters (-_) no padding
+  await roundTripSecretJwk(
+    {
+      kty: "oct",
+      k: "_u3K_gEjRWf-7cr-ASNFZw",
+      alg: "A128CBC",
+      "key_ops": ["encrypt", "decrypt"],
+      ext: true,
+    },
+    { name: "AES-CBC" },
+    ["encrypt", "decrypt"],
+    (_key, orig, exp) => {
+      assertEquals(orig.k, exp.k);
+    },
+  );
+
+  // AES-CBC 128bits of '1' with padding chars
+  await roundTripSecretJwk(
+    {
+      kty: "oct",
+      k: "_____________________w==",
+      alg: "A128CBC",
+      "key_ops": ["encrypt", "decrypt"],
+      ext: true,
+    },
+    { name: "AES-CBC" },
+    ["encrypt", "decrypt"],
+    (_key, _orig, exp) => {
+      assertEquals(exp.k, "_____________________w");
+    },
+  );
 });
diff --git a/ext/crypto/00_crypto.js b/ext/crypto/00_crypto.js
@@ -12,7 +12,6 @@
   const core = window.Deno.core;
   const webidl = window.__bootstrap.webidl;
   const { DOMException } = window.__bootstrap.domException;
-  const { btoa } = window.__bootstrap.base64;
 
   const {
     ArrayBuffer,
@@ -25,8 +24,6 @@
     Int32Array,
     Int8Array,
     ObjectAssign,
-    StringFromCharCode,
-    StringPrototypeReplace,
     StringPrototypeToLowerCase,
     StringPrototypeToUpperCase,
     Symbol,
@@ -175,15 +172,6 @@
     },
   };
 
-  function unpaddedBase64(bytes) {
-    let binaryString = "";
-    for (let i = 0; i < bytes.length; i++) {
-      binaryString += StringFromCharCode(bytes[i]);
-    }
-    const base64String = btoa(binaryString);
-    return StringPrototypeReplace(base64String, /=/g, "");
-  }
-
   // See https://www.w3.org/TR/WebCryptoAPI/#dfn-normalize-an-algorithm
   // 18.4.4
   function normalizeAlgorithm(algorithm, op) {
@@ -1836,16 +1824,18 @@
         return data.buffer;
       }
       case "jwk": {
-        // 1-3.
+        // 1-2.
         const jwk = {
           kty: "oct",
-          // 5.
-          ext: key[_extractable],
-          // 6.
-          "key_ops": key.usages,
-          k: unpaddedBase64(innerKey.data),
         };
 
+        // 3.
+        const data = core.opSync("op_crypto_export_key", {
+          format: "jwksecret",
+          algorithm: "AES",
+        }, innerKey);
+        ObjectAssign(jwk, data);
+
         // 4.
         const algorithm = key[_algorithm];
         switch (algorithm.length) {
@@ -1865,6 +1855,12 @@
             );
         }
 
+        // 5.
+        jwk.key_ops = key.usages;
+
+        // 6.
+        jwk.ext = key[_extractable];
+
         // 7.
         return jwk;
       }
@@ -3092,11 +3088,18 @@
         return bits.buffer;
       }
       case "jwk": {
-        // 1-3.
+        // 1-2.
         const jwk = {
           kty: "oct",
-          k: unpaddedBase64(innerKey.data),
         };
+
+        // 3.
+        const data = core.opSync("op_crypto_export_key", {
+          format: "jwksecret",
+          algorithm: key[_algorithm].name,
+        }, innerKey);
+        jwk.k = data.k;
+
         // 4.
         const algorithm = key[_algorithm];
         // 5.

diff --git a/ext/crypto/export_key.rs b/ext/crypto/export_key.rs
@@ -26,6 +26,7 @@ pub enum ExportKeyFormat {
   Spki,
   JwkPublic,
   JwkPrivate,
+  JwkSecret,
 }
 
 #[derive(Deserialize)]
@@ -37,13 +38,20 @@ pub enum ExportKeyAlgorithm {
   RsaPss {},
   #[serde(rename = "RSA-OAEP")]
   RsaOaep {},
+  #[serde(rename = "AES")]
+  Aes {},
+  #[serde(rename = "HMAC")]
+  Hmac {},
 }
 
 #[derive(Serialize)]
 #[serde(untagged)]
 pub enum ExportKeyResult {
   Pkcs8(ZeroCopyBuf),
   Spki(ZeroCopyBuf),
+  JwkSecret {
+    k: String,
+  },
   JwkPublicRsa {
     n: String,
     e: String,
@@ -69,13 +77,20 @@ pub fn op_crypto_export_key(
     ExportKeyAlgorithm::RsassaPkcs1v15 {}
     | ExportKeyAlgorithm::RsaPss {}
     | ExportKeyAlgorithm::RsaOaep {} => export_key_rsa(opts.format, key_data),
+    ExportKeyAlgorithm::Aes {} | ExportKeyAlgorithm::Hmac {} => {
+      export_key_symmetric(opts.format, key_data)
+    }
   }
 }
 
 fn uint_to_b64(bytes: UIntBytes) -> String {
   base64::encode_config(bytes.as_bytes(), base64::URL_SAFE_NO_PAD)
 }
 
+fn bytes_to_b64(bytes: &[u8]) -> String {
+  base64::encode_config(bytes, base64::URL_SAFE_NO_PAD)
+}
+
 fn export_key_rsa(
   format: ExportKeyFormat,
   key_data: RawKeyData,
@@ -166,5 +181,22 @@ fn export_key_rsa(
         qi: uint_to_b64(private_key.coefficient),
       })
     }
+    _ => Err(unsupported_format()),
+  }
+}
+
+fn export_key_symmetric(
+  format: ExportKeyFormat,
+  key_data: RawKeyData,
+) -> Result<ExportKeyResult, deno_core::anyhow::Error> {
+  match format {
+    ExportKeyFormat::JwkSecret => {
+      let bytes = key_data.as_secret_key()?;
+
+      Ok(ExportKeyResult::JwkSecret {
+        k: bytes_to_b64(bytes),
+      })
+    }
+    _ => Err(unsupported_format()),
   }
 }