From 84c2b969d840bc99c597e2daa0a06b297c71a36a Mon Sep 17 00:00:00 2001 From: Felix Hanau Date: Wed, 21 Jun 2023 20:53:57 -0400 Subject: [PATCH] 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. --- src/workerd/api/crypto-impl-asymmetric.c++ | 19 ++++++++++++------- src/workerd/io/compatibility-date.capnp | 2 +- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/src/workerd/api/crypto-impl-asymmetric.c++ b/src/workerd/api/crypto-impl-asymmetric.c++ index 0665a46c9d7c..ae9c8ee6e7c2 100644 --- 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 keyUsages, kj::FunctionParam(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::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 { JSG_REQUIRE(keyDataJwk.kty == "RSA", DOMDataError, @@ -1219,7 +1224,7 @@ kj::Own 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 { JSG_REQUIRE(keyDataJwk.kty == "RSA", DOMDataError, @@ -1854,7 +1859,7 @@ kj::Own 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 { return ellipticJwkReader(curveId, kj::mv(keyDataJwk)); @@ -1910,11 +1915,11 @@ kj::Own 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 { 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::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 { 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 index 97f337ebe2ae..34fd7531bfc8 100644 --- 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. }