Fix ECDH key import when strict_crypto_checks is enabled

With the `strict_crypto_checks` flag enabled, imported ECDH keys were required to have empty usages, but for private keys the usage set should be allowed to include deriveBits and deriveKey. Since the compatibility date for the flag has not yet passed, this is unlikely to have had a significant effect.
cloudflare · Jun 22, 2023 · 84c2b96 · 84c2b96
1 parent 4515869
commit 84c2b96
Show file tree

Hide file tree

Showing 2 changed files with 13 additions and 8 deletions.
diff --git a/src/workerd/api/crypto-impl-asymmetric.c++ b/src/workerd/api/crypto-impl-asymmetric.c++
@@ -360,7 +360,7 @@ enum class UsageFamily {
   EncryptDecrypt,
 };
 
-ImportAsymmetricResult importAsymmetric(kj::StringPtr format,
+ImportAsymmetricResult importAsymmetric(jsg::Lock& js, kj::StringPtr format,
     SubtleCrypto::ImportKeyData keyData, kj::StringPtr normalizedName, bool extractable,
     kj::ArrayPtr<const kj::String> keyUsages,
     kj::FunctionParam<kj::Own<EVP_PKEY>(SubtleCrypto::JsonWebKey)> readJwk,
@@ -388,9 +388,14 @@ ImportAsymmetricResult importAsymmetric(kj::StringPtr format,
     } else {
       // Public key.
       keyType = "public";
+      auto strictCrypto = FeatureFlags::get(js).getStrictCrypto();
+      // restrict key usages to public key usages. In the case of ECDH, usages must be empty, but
+      // if the strict crypto compat flag is not enabled allow the same usages as with private ECDH
+      // keys, i.e. derivationKeyMask().
       usages =
           CryptoKeyUsageSet::validate(normalizedName, CryptoKeyUsageSet::Context::importPublic,
                                       keyUsages, allowedUsages & (normalizedName == "ECDH" ?
+                                      strictCrypto ? CryptoKeyUsageSet():
                                       CryptoKeyUsageSet::derivationKeyMask() :
                                       CryptoKeyUsageSet::publicKeyMask()));
     }
@@ -1118,7 +1123,7 @@ kj::Own<CryptoKey::Impl> CryptoKey::Impl::importRsa(
   auto [normalizedHashName, hashEvpMd] = lookupDigestAlgorithm(hash);
 
   auto [evpPkey, keyType, usages] = importAsymmetric(
-      kj::mv(format), kj::mv(keyData), normalizedName, extractable, keyUsages,
+      js, kj::mv(format), kj::mv(keyData), normalizedName, extractable, keyUsages,
       // Verbose lambda capture needed because: https://bugs.llvm.org/show_bug.cgi?id=35984
       [hashEvpMd = hashEvpMd, &algorithm](SubtleCrypto::JsonWebKey keyDataJwk) -> kj::Own<EVP_PKEY> {
     JSG_REQUIRE(keyDataJwk.kty == "RSA", DOMDataError,
@@ -1219,7 +1224,7 @@ kj::Own<CryptoKey::Impl> CryptoKey::Impl::importRsaRaw(
   // data. Importing raw keys is currently not supported for this algorithm.
   CryptoKeyUsageSet allowedUsages = CryptoKeyUsageSet::sign() | CryptoKeyUsageSet::verify();
   auto [evpPkey, keyType, usages] = importAsymmetric(
-      kj::mv(format), kj::mv(keyData), normalizedName, extractable, keyUsages,
+      js, kj::mv(format), kj::mv(keyData), normalizedName, extractable, keyUsages,
       // Verbose lambda capture needed because: https://bugs.llvm.org/show_bug.cgi?id=35984
       [](SubtleCrypto::JsonWebKey keyDataJwk) -> kj::Own<EVP_PKEY> {
     JSG_REQUIRE(keyDataJwk.kty == "RSA", DOMDataError,
@@ -1854,7 +1859,7 @@ kj::Own<CryptoKey::Impl> CryptoKey::Impl::importEcdsa(
   auto [evpPkey, keyType, usages] = [&, curveId = curveId] {
     if (format != "raw") {
       return importAsymmetric(
-          format, kj::mv(keyData), normalizedName, extractable, keyUsages,
+          js, format, kj::mv(keyData), normalizedName, extractable, keyUsages,
           // Verbose lambda capture needed because: https://bugs.llvm.org/show_bug.cgi?id=35984
           [curveId = curveId](SubtleCrypto::JsonWebKey keyDataJwk) -> kj::Own<EVP_PKEY> {
         return ellipticJwkReader(curveId, kj::mv(keyDataJwk));
@@ -1910,11 +1915,11 @@ kj::Own<CryptoKey::Impl> CryptoKey::Impl::importEcdh(
 
     if (format != "raw") {
       return importAsymmetric(
-          format, kj::mv(keyData), normalizedName, extractable, keyUsages,
+          js, format, kj::mv(keyData), normalizedName, extractable, keyUsages,
           // Verbose lambda capture needed because: https://bugs.llvm.org/show_bug.cgi?id=35984
           [curveId = curveId](SubtleCrypto::JsonWebKey keyDataJwk) -> kj::Own<EVP_PKEY> {
         return ellipticJwkReader(curveId, kj::mv(keyDataJwk));
-      }, usageSet);
+      }, CryptoKeyUsageSet::derivationKeyMask());
     } else {
       // The usage set is required to be empty for public ECDH keys, including raw keys.
       return importEllipticRaw(kj::mv(keyData), curveId, normalizedName, keyUsages, usageSet);
@@ -2290,7 +2295,7 @@ kj::Own<CryptoKey::Impl> CryptoKey::Impl::importEddsa(
     auto nid = normalizedName == "X25519" ? NID_X25519 : NID_ED25519;
     if (format != "raw") {
       return importAsymmetric(
-          format, kj::mv(keyData), normalizedName, extractable, keyUsages,
+          js, format, kj::mv(keyData), normalizedName, extractable, keyUsages,
           [nid](SubtleCrypto::JsonWebKey keyDataJwk) -> kj::Own<EVP_PKEY> {
         return ellipticJwkReader(nid, kj::mv(keyDataJwk));
       }, normalizedName == "X25519" ? CryptoKeyUsageSet::derivationKeyMask() :

diff --git a/src/workerd/io/compatibility-date.capnp b/src/workerd/io/compatibility-date.capnp
@@ -329,5 +329,5 @@ struct CompatibilityFlags @0x8f8c1b68151b6cef {
       $compatDisableFlag("no_strict_crypto_checks")
       $compatEnableDate("2023-08-01");
   # Perform additional error checking in the Web Crypto API to conform with the specification as
-  # well as reject key parameters that may be unsafe based on the modulus or key length.
+  # well as reject key parameters that may be unsafe based on key length or public exponent.
 }