From 13563086c2bb46846199d34a2cd4c8ce460cc4c9 Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Wed, 29 Jun 2022 14:53:57 +0200 Subject: [PATCH] fix: re-enable HKDF crypto functionality (#34767) * fix: re-enable HKDF crypto functionality * chore: update patches Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com> --- .../fix_crypto_tests_to_run_with_bssl.patch | 13 +++++ ...ingssl_and_openssl_incompatibilities.patch | 47 +++---------------- .../node/support_v8_sandboxed_pointers.patch | 10 ++-- script/node-disabled-tests.json | 2 - 4 files changed, 24 insertions(+), 48 deletions(-) diff --git a/patches/node/fix_crypto_tests_to_run_with_bssl.patch b/patches/node/fix_crypto_tests_to_run_with_bssl.patch index 878160f610ebf..052e1e10ffaec 100644 --- a/patches/node/fix_crypto_tests_to_run_with_bssl.patch +++ b/patches/node/fix_crypto_tests_to_run_with_bssl.patch @@ -537,6 +537,19 @@ index af2146982c7a3bf7bd7527f44e4b17a3b605026e..f6b91f675cfea367c608892dee078b56 // Non-XOF hash functions should accept valid outputLength options as well. assert.strictEqual(crypto.createHash('sha224', { outputLength: 28 }) +diff --git a/test/parallel/test-crypto-hkdf.js b/test/parallel/test-crypto-hkdf.js +index 16744201a935dcd25af4e0f446701b08fe08dd64..e7ef0b78a19fb755456d038fc676eedb2f71ff07 100644 +--- a/test/parallel/test-crypto-hkdf.js ++++ b/test/parallel/test-crypto-hkdf.js +@@ -117,8 +117,6 @@ const algorithms = [ + ['sha256', 'secret', 'salt', 'info', 10], + ['sha512', 'secret', 'salt', '', 15], + ]; +-if (!common.hasOpenSSL3) +- algorithms.push(['whirlpool', 'secret', '', 'info', 20]); + + algorithms.forEach(([ hash, secret, salt, info, length ]) => { + { diff --git a/test/parallel/test-crypto-padding.js b/test/parallel/test-crypto-padding.js index f1f14b472997e76bb4100edb1c6cf4fc24d1074d..5057e3f9bc5bb78aceffa5e79530f8ceed84e6f7 100644 --- a/test/parallel/test-crypto-padding.js diff --git a/patches/node/fix_handle_boringssl_and_openssl_incompatibilities.patch b/patches/node/fix_handle_boringssl_and_openssl_incompatibilities.patch index 5f0d8ae50dab9..1e6d8e14b488b 100644 --- a/patches/node/fix_handle_boringssl_and_openssl_incompatibilities.patch +++ b/patches/node/fix_handle_boringssl_and_openssl_incompatibilities.patch @@ -188,28 +188,6 @@ index c7894baf00ee9ce4684f4c752f1c7c9b98163741..655895dbff8b88daa53c7b40a5feca42 if (EVP_PKEY_paramgen(param_ctx.get(), &raw_params) <= 0) return EVPKeyCtxPointer(); -diff --git a/src/crypto/crypto_hkdf.cc b/src/crypto/crypto_hkdf.cc -index 0aa96ada47abe4b66fb616c665101278bbe0afb6..1e9a4863c5faea5f6b275483ca16f3a6e8dac25b 100644 ---- a/src/crypto/crypto_hkdf.cc -+++ b/src/crypto/crypto_hkdf.cc -@@ -101,6 +101,7 @@ bool HKDFTraits::DeriveBits( - Environment* env, - const HKDFConfig& params, - ByteSource* out) { -+#ifndef OPENSSL_IS_BORINGSSL - EVPKeyCtxPointer ctx = - EVPKeyCtxPointer(EVP_PKEY_CTX_new_id(EVP_PKEY_HKDF, nullptr)); - if (!ctx || -@@ -132,6 +133,9 @@ bool HKDFTraits::DeriveBits( - - *out = std::move(buf); - return true; -+#else -+ return false; -+#endif - } - - void HKDFConfig::MemoryInfo(MemoryTracker* tracker) const { diff --git a/src/crypto/crypto_random.cc b/src/crypto/crypto_random.cc index fc88deb460314c2620d842ec30141bcd13109d60..c097ccfcffb1158317ba09e7c4beb725ccbab74f 100644 --- a/src/crypto/crypto_random.cc @@ -244,10 +222,10 @@ index ae4550e9fde8120c35409e495d5b763a95546509..188a7efe76df2a1aa2eb2746f4d74836 if (target diff --git a/src/crypto/crypto_util.cc b/src/crypto/crypto_util.cc -index e1ef170a9f17634d218492a2ce888c3a4365e097..8dffad89c80e0906780d1b26ba9a65ba1e76ce0a 100644 +index e1ef170a9f17634d218492a2ce888c3a4365e097..f55e292fbbc75448b15dc9be0327ad2dedef49e0 100644 --- a/src/crypto/crypto_util.cc +++ b/src/crypto/crypto_util.cc -@@ -508,24 +508,14 @@ Maybe Decorate(Environment* env, Local obj, +@@ -508,24 +508,15 @@ Maybe Decorate(Environment* env, Local obj, V(BIO) \ V(PKCS7) \ V(X509V3) \ @@ -269,10 +247,11 @@ index e1ef170a9f17634d218492a2ce888c3a4365e097..8dffad89c80e0906780d1b26ba9a65ba - V(ASYNC) \ - V(KDF) \ - V(SM2) \ ++ V(HKDF) \ V(USER) \ #define V(name) case ERR_LIB_##name: lib = #name "_"; break; -@@ -684,7 +674,7 @@ void SecureBuffer(const FunctionCallbackInfo& args) { +@@ -684,7 +675,7 @@ void SecureBuffer(const FunctionCallbackInfo& args) { CHECK(args[0]->IsUint32()); Environment* env = Environment::GetCurrent(args); uint32_t len = args[0].As()->Value(); @@ -281,7 +260,7 @@ index e1ef170a9f17634d218492a2ce888c3a4365e097..8dffad89c80e0906780d1b26ba9a65ba if (data == nullptr) { // There's no memory available for the allocation. // Return nothing. -@@ -696,7 +686,7 @@ void SecureBuffer(const FunctionCallbackInfo& args) { +@@ -696,7 +687,7 @@ void SecureBuffer(const FunctionCallbackInfo& args) { data, len, [](void* data, size_t len, void* deleter_data) { @@ -290,7 +269,7 @@ index e1ef170a9f17634d218492a2ce888c3a4365e097..8dffad89c80e0906780d1b26ba9a65ba }, data); Local buffer = ArrayBuffer::New(env->isolate(), store); -@@ -704,10 +694,12 @@ void SecureBuffer(const FunctionCallbackInfo& args) { +@@ -704,10 +695,12 @@ void SecureBuffer(const FunctionCallbackInfo& args) { } void SecureHeapUsed(const FunctionCallbackInfo& args) { @@ -303,20 +282,6 @@ index e1ef170a9f17634d218492a2ce888c3a4365e097..8dffad89c80e0906780d1b26ba9a65ba } } // namespace -diff --git a/src/crypto/crypto_util.h b/src/crypto/crypto_util.h -index c431159e6f77f8c86844bcadb86012b056d03372..0ce3a8f219a2952f660ff72a6ce36ee109add649 100644 ---- a/src/crypto/crypto_util.h -+++ b/src/crypto/crypto_util.h -@@ -16,7 +16,9 @@ - #include - #include - #include -+#ifndef OPENSSL_IS_BORINGSSL - #include -+#endif - #include - #include - #include diff --git a/src/node_metadata.h b/src/node_metadata.h index 4486d5af2c1622c7c8f44401dc3ebb986d8e3c2e..db1769f1b3f1617ed8dbbea57b5e324183b42be2 100644 --- a/src/node_metadata.h diff --git a/patches/node/support_v8_sandboxed_pointers.patch b/patches/node/support_v8_sandboxed_pointers.patch index a07e9f2560115..08a61216abb4a 100644 --- a/patches/node/support_v8_sandboxed_pointers.patch +++ b/patches/node/support_v8_sandboxed_pointers.patch @@ -118,7 +118,7 @@ index 2abf5994405e8da2a04d1b23b75ccd3658398474..024d612a04d83583b397549589d994e3 DebuggingArrayBufferAllocator::~DebuggingArrayBufferAllocator() { diff --git a/src/crypto/crypto_util.cc b/src/crypto/crypto_util.cc -index 8dffad89c80e0906780d1b26ba9a65ba1e76ce0a..45bc99ce75248794e95b2dcb0101c28152e2bfd0 100644 +index f55e292fbbc75448b15dc9be0327ad2dedef49e0..7719574859637aecc98f8a4b00ba6ebca8280631 100644 --- a/src/crypto/crypto_util.cc +++ b/src/crypto/crypto_util.cc @@ -318,10 +318,35 @@ ByteSource& ByteSource::operator=(ByteSource&& other) noexcept { @@ -171,7 +171,7 @@ index 8dffad89c80e0906780d1b26ba9a65ba1e76ce0a..45bc99ce75248794e95b2dcb0101c281 return ArrayBuffer::New(env->isolate(), std::move(store)); } -@@ -665,6 +691,16 @@ CryptoJobMode GetCryptoJobMode(v8::Local args) { +@@ -666,6 +692,16 @@ CryptoJobMode GetCryptoJobMode(v8::Local args) { } namespace { @@ -188,7 +188,7 @@ index 8dffad89c80e0906780d1b26ba9a65ba1e76ce0a..45bc99ce75248794e95b2dcb0101c281 // SecureBuffer uses openssl to allocate a Uint8Array using // OPENSSL_secure_malloc. Because we do not yet actually // make use of secure heap, this has the same semantics as -@@ -692,6 +728,7 @@ void SecureBuffer(const FunctionCallbackInfo& args) { +@@ -693,6 +729,7 @@ void SecureBuffer(const FunctionCallbackInfo& args) { Local buffer = ArrayBuffer::New(env->isolate(), store); args.GetReturnValue().Set(Uint8Array::New(buffer, 0, len)); } @@ -197,10 +197,10 @@ index 8dffad89c80e0906780d1b26ba9a65ba1e76ce0a..45bc99ce75248794e95b2dcb0101c281 void SecureHeapUsed(const FunctionCallbackInfo& args) { #ifndef OPENSSL_IS_BORINGSSL diff --git a/src/crypto/crypto_util.h b/src/crypto/crypto_util.h -index 0ce3a8f219a2952f660ff72a6ce36ee109add649..06e9eb72e4ea60db4c63d08b24b80a1e6c4f3eaf 100644 +index c431159e6f77f8c86844bcadb86012b056d03372..9f57ac58d826cb0aae422ddca54e2136618c4bfe 100644 --- a/src/crypto/crypto_util.h +++ b/src/crypto/crypto_util.h -@@ -257,7 +257,7 @@ class ByteSource { +@@ -255,7 +255,7 @@ class ByteSource { // Creates a v8::BackingStore that takes over responsibility for // any allocated data. The ByteSource will be reset with size = 0 // after being called. diff --git a/script/node-disabled-tests.json b/script/node-disabled-tests.json index fe69ab29c7e22..8870c8fe8492e 100644 --- a/script/node-disabled-tests.json +++ b/script/node-disabled-tests.json @@ -15,7 +15,6 @@ "parallel/test-crypto-ecb", "parallel/test-crypto-engine", "parallel/test-crypto-fips", - "parallel/test-crypto-hkdf.js", "parallel/test-crypto-keygen", "parallel/test-crypto-keygen-deprecation", "parallel/test-crypto-key-objects", @@ -104,7 +103,6 @@ "parallel/test-trace-events-vm", "parallel/test-trace-events-worker-metadata", "parallel/test-v8-untrusted-code-mitigations", - "parallel/test-webcrypto-derivebits-hkdf", "parallel/test-webcrypto-derivebits-node-dh", "parallel/test-webcrypto-ed25519-ed448", "parallel/test-webcrypto-encrypt-decrypt",