From ac499cb0bb21d6e9830f8426a60c7c59e11a240f Mon Sep 17 00:00:00 2001 From: James M Snell Date: Sat, 18 Jan 2025 14:31:19 -0800 Subject: [PATCH] src: move more crypto to ncrypto --- deps/ncrypto/ncrypto.cc | 81 +++++++++++++++++++++++++++++++++++++ deps/ncrypto/ncrypto.h | 31 +++++++++++++- src/crypto/crypto_hash.cc | 75 +++++++++++++++++----------------- src/crypto/crypto_hkdf.cc | 4 +- src/crypto/crypto_hmac.cc | 65 ++++++++++++++++------------- src/crypto/crypto_random.cc | 4 +- src/crypto/crypto_util.h | 5 ++- 7 files changed, 192 insertions(+), 73 deletions(-) diff --git a/deps/ncrypto/ncrypto.cc b/deps/ncrypto/ncrypto.cc index dbd6a5f61d34fe..614473b6879064 100644 --- a/deps/ncrypto/ncrypto.cc +++ b/deps/ncrypto/ncrypto.cc @@ -3721,4 +3721,85 @@ bool extractP1363(const Buffer& buf, BignumPointer::EncodePaddedInto(asn1_sig.s(), dest + n, n) > 0; } +// ============================================================================ + +HMACCtxPointer::HMACCtxPointer() : ctx_(nullptr) {} + +HMACCtxPointer::HMACCtxPointer(HMAC_CTX* ctx) : ctx_(ctx) {} + +HMACCtxPointer::HMACCtxPointer(HMACCtxPointer&& other) noexcept + : ctx_(other.release()) {} + +HMACCtxPointer& HMACCtxPointer::operator=(HMACCtxPointer&& other) noexcept { + ctx_.reset(other.release()); + return *this; +} + +HMACCtxPointer::~HMACCtxPointer() { + reset(); +} + +void HMACCtxPointer::reset(HMAC_CTX* ctx) { + ctx_.reset(ctx); +} + +HMAC_CTX* HMACCtxPointer::release() { + return ctx_.release(); +} + +bool HMACCtxPointer::init(const Buffer& buf, const EVP_MD* md) { + if (!ctx_) return false; + return HMAC_Init_ex(ctx_.get(), buf.data, buf.len, md, nullptr) == 1; +} + +bool HMACCtxPointer::update(const Buffer& buf) { + if (!ctx_) return false; + return HMAC_Update(ctx_.get(), + static_cast(buf.data), + buf.len) == 1; +} + +DataPointer HMACCtxPointer::digest() { + auto data = DataPointer::Alloc(EVP_MAX_MD_SIZE); + if (!data) return {}; + Buffer buf = data; + if (!digestInto(&buf)) return {}; + return data.resize(buf.len); +} + +bool HMACCtxPointer::digestInto(Buffer* buf) { + if (!ctx_) return false; + + unsigned int len = buf->len; + if (!HMAC_Final(ctx_.get(), static_cast(buf->data), &len)) { + return false; + } + buf->len = len; + return true; +} + +HMACCtxPointer HMACCtxPointer::New() { + return HMACCtxPointer(HMAC_CTX_new()); +} + +DataPointer hashDigest(const Buffer& buf, + const EVP_MD* md) { + if (md == nullptr) return {}; + size_t md_len = EVP_MD_size(md); + unsigned int result_size; + auto data = DataPointer::Alloc(md_len); + if (!data) return {}; + + if (!EVP_Digest(buf.data, + buf.len, + reinterpret_cast(data.get()), + &result_size, + md, + nullptr)) { + return {}; + } + + return data.resize(result_size); +} + } // namespace ncrypto diff --git a/deps/ncrypto/ncrypto.h b/deps/ncrypto/ncrypto.h index fdb8f09efd1832..3b853c8c17c855 100644 --- a/deps/ncrypto/ncrypto.h +++ b/deps/ncrypto/ncrypto.h @@ -201,7 +201,6 @@ struct FunctionDeleter { template using DeleteFnPtr = typename FunctionDeleter::Pointer; -using HMACCtxPointer = DeleteFnPtr; using PKCS8Pointer = DeleteFnPtr; using RSAPointer = DeleteFnPtr; using SSLSessionPointer = DeleteFnPtr; @@ -239,6 +238,9 @@ struct Buffer { size_t len = 0; }; +DataPointer hashDigest(const Buffer& data, + const EVP_MD* md); + class Cipher final { public: Cipher() = default; @@ -1185,6 +1187,33 @@ class EVPMDCtxPointer final { DeleteFnPtr ctx_; }; +class HMACCtxPointer final { + public: + HMACCtxPointer(); + explicit HMACCtxPointer(HMAC_CTX* ctx); + HMACCtxPointer(HMACCtxPointer&& other) noexcept; + HMACCtxPointer& operator=(HMACCtxPointer&& other) noexcept; + NCRYPTO_DISALLOW_COPY(HMACCtxPointer) + ~HMACCtxPointer(); + + inline bool operator==(std::nullptr_t) noexcept { return ctx_ == nullptr; } + inline operator bool() const { return ctx_ != nullptr; } + inline HMAC_CTX* get() const { return ctx_.get(); } + inline operator HMAC_CTX*() const { return ctx_.get(); } + void reset(HMAC_CTX* ctx = nullptr); + HMAC_CTX* release(); + + bool init(const Buffer& buf, const EVP_MD* md); + bool update(const Buffer& buf); + DataPointer digest(); + bool digestInto(Buffer* buf); + + static HMACCtxPointer New(); + + private: + DeleteFnPtr ctx_; +}; + #ifndef OPENSSL_NO_ENGINE class EnginePointer final { public: diff --git a/src/crypto/crypto_hash.cc b/src/crypto/crypto_hash.cc index 588ab6d9a391d4..851847483327c1 100644 --- a/src/crypto/crypto_hash.cc +++ b/src/crypto/crypto_hash.cc @@ -11,6 +11,7 @@ namespace node { +using ncrypto::DataPointer; using ncrypto::EVPMDCtxPointer; using ncrypto::MarkPopErrorOnReturn; using v8::Context; @@ -220,7 +221,7 @@ void Hash::OneShotDigest(const FunctionCallbackInfo& args) { CHECK(args[5]->IsUint32() || args[5]->IsUndefined()); // outputEncodingId const EVP_MD* md = GetDigestImplementation(env, args[0], args[1], args[2]); - if (md == nullptr) { + if (md == nullptr) [[unlikely]] { Utf8Value method(isolate, args[0]); std::string message = "Digest method " + method.ToString() + " is not supported"; @@ -229,41 +230,36 @@ void Hash::OneShotDigest(const FunctionCallbackInfo& args) { enum encoding output_enc = ParseEncoding(isolate, args[4], args[5], HEX); - int md_len = EVP_MD_size(md); - unsigned int result_size; - ByteSource::Builder output(md_len); - int success; - // On smaller inputs, EVP_Digest() can be slower than the - // deprecated helpers e.g SHA256_XXX. The speedup may not - // be worth using deprecated APIs, however, so we use - // EVP_Digest(), unless there's a better alternative - // in the future. - // https://github.com/openssl/openssl/issues/19612 - if (args[3]->IsString()) { - Utf8Value utf8(isolate, args[3]); - success = EVP_Digest(utf8.out(), - utf8.length(), - output.data(), - &result_size, - md, - nullptr); - } else { + DataPointer output = ([&] { + if (args[3]->IsString()) { + Utf8Value utf8(isolate, args[3]); + ncrypto::Buffer buf{ + .data = reinterpret_cast(utf8.out()), + .len = utf8.length(), + }; + return ncrypto::hashDigest(buf, md); + } + ArrayBufferViewContents input(args[3]); - success = EVP_Digest(input.data(), - input.length(), - output.data(), - &result_size, - md, - nullptr); - } - if (!success) { + ncrypto::Buffer buf{ + .data = reinterpret_cast(input.data()), + .len = input.length(), + }; + return ncrypto::hashDigest(buf, md); + })(); + + if (!output) [[unlikely]] { return ThrowCryptoError(env, ERR_get_error()); } Local error; - MaybeLocal rc = StringBytes::Encode( - env->isolate(), output.data(), md_len, output_enc, &error); - if (rc.IsEmpty()) { + MaybeLocal rc = + StringBytes::Encode(env->isolate(), + static_cast(output.get()), + output.size(), + output_enc, + &error); + if (rc.IsEmpty()) [[unlikely]] { CHECK(!error.IsEmpty()); env->isolate()->ThrowException(error); return; @@ -339,7 +335,7 @@ void Hash::New(const FunctionCallbackInfo& args) { bool Hash::HashInit(const EVP_MD* md, Maybe xof_md_len) { mdctx_ = EVPMDCtxPointer::New(); - if (!mdctx_.digestInit(md)) { + if (!mdctx_.digestInit(md)) [[unlikely]] { mdctx_.reset(); return false; } @@ -348,7 +344,7 @@ bool Hash::HashInit(const EVP_MD* md, Maybe xof_md_len) { if (xof_md_len.IsJust() && xof_md_len.FromJust() != md_len_) { // This is a little hack to cause createHash to fail when an incorrect // hashSize option was passed for a non-XOF hash function. - if (!mdctx_.hasXofFlag()) { + if (!mdctx_.hasXofFlag()) [[unlikely]] { EVPerr(EVP_F_EVP_DIGESTFINALXOF, EVP_R_NOT_XOF_OR_INVALID_LENGTH); mdctx_.reset(); return false; @@ -406,7 +402,7 @@ void Hash::HashDigest(const FunctionCallbackInfo& args) { // so we need to cache it. // See https://github.com/nodejs/node/issues/28245. auto data = hash->mdctx_.digestFinal(len); - if (!data) { + if (!data) [[unlikely]] { return ThrowCryptoError(env, ERR_get_error()); } @@ -416,7 +412,7 @@ void Hash::HashDigest(const FunctionCallbackInfo& args) { Local error; MaybeLocal rc = StringBytes::Encode( env->isolate(), hash->digest_.data(), len, encoding, &error); - if (rc.IsEmpty()) { + if (rc.IsEmpty()) [[unlikely]] { CHECK(!error.IsEmpty()); env->isolate()->ThrowException(error); return; @@ -482,7 +478,7 @@ Maybe HashTraits::AdditionalConfig( static_cast(args[offset + 2] .As()->Value()) / CHAR_BIT; if (params->length != expected) { - if ((EVP_MD_flags(params->digest) & EVP_MD_FLAG_XOF) == 0) { + if ((EVP_MD_flags(params->digest) & EVP_MD_FLAG_XOF) == 0) [[unlikely]] { THROW_ERR_CRYPTO_INVALID_DIGEST(env, "Digest method not supported"); return Nothing(); } @@ -505,7 +501,8 @@ bool HashTraits::DeriveBits( if (params.length > 0) [[likely]] { auto data = ctx.digestFinal(params.length); - if (!data) return false; + if (!data) [[unlikely]] + return false; *out = ByteSource::Allocated(data.release()); } @@ -535,7 +532,7 @@ void InternalVerifyIntegrity(const v8::FunctionCallbackInfo& args) { digest, &digest_size, md_type, - nullptr) != 1) { + nullptr) != 1) [[unlikely]] { return ThrowCryptoError( env, ERR_get_error(), "Digest method not supported"); } @@ -549,7 +546,7 @@ void InternalVerifyIntegrity(const v8::FunctionCallbackInfo& args) { digest_size, BASE64, &error); - if (rc.IsEmpty()) { + if (rc.IsEmpty()) [[unlikely]] { CHECK(!error.IsEmpty()); env->isolate()->ThrowException(error); return; diff --git a/src/crypto/crypto_hkdf.cc b/src/crypto/crypto_hkdf.cc index 2a465c849def44..10bb8e4258bf63 100644 --- a/src/crypto/crypto_hkdf.cc +++ b/src/crypto/crypto_hkdf.cc @@ -55,7 +55,7 @@ Maybe HKDFTraits::AdditionalConfig( Utf8Value hash(env->isolate(), args[offset]); params->digest = ncrypto::getDigestByName(hash.ToStringView()); - if (params->digest == nullptr) { + if (params->digest == nullptr) [[unlikely]] { THROW_ERR_CRYPTO_INVALID_DIGEST(env, "Invalid digest: %s", *hash); return Nothing(); } @@ -88,7 +88,7 @@ Maybe HKDFTraits::AdditionalConfig( // HKDF-Expand computes up to 255 HMAC blocks, each having as many bits as the // output of the hash function. 255 is a hard limit because HKDF appends an // 8-bit counter to each HMAC'd message, starting at 1. - if (!ncrypto::checkHkdfLength(params->digest, params->length)) { + if (!ncrypto::checkHkdfLength(params->digest, params->length)) [[unlikely]] { THROW_ERR_CRYPTO_INVALID_KEYLEN(env); return Nothing(); } diff --git a/src/crypto/crypto_hmac.cc b/src/crypto/crypto_hmac.cc index 7d9ec677ed5d81..56a09e1a2d9b0f 100644 --- a/src/crypto/crypto_hmac.cc +++ b/src/crypto/crypto_hmac.cc @@ -71,14 +71,20 @@ void Hmac::HmacInit(const char* hash_type, const char* key, int key_len) { HandleScope scope(env()->isolate()); const EVP_MD* md = ncrypto::getDigestByName(hash_type); - if (md == nullptr) [[unlikely]] + if (md == nullptr) [[unlikely]] { return THROW_ERR_CRYPTO_INVALID_DIGEST( env(), "Invalid digest: %s", hash_type); + } if (key_len == 0) { key = ""; } - ctx_.reset(HMAC_CTX_new()); - if (!ctx_ || !HMAC_Init_ex(ctx_.get(), key, key_len, md, nullptr)) [[unlikely]] { + + ctx_ = HMACCtxPointer::New(); + ncrypto::Buffer key_buf{ + .data = key, + .len = static_cast(key_len), + }; + if (!ctx_.init(key_buf, md)) [[unlikely]] { ctx_.reset(); return ThrowCryptoError(env(), ERR_get_error()); } @@ -95,9 +101,11 @@ void Hmac::HmacInit(const FunctionCallbackInfo& args) { } bool Hmac::HmacUpdate(const char* data, size_t len) { - return ctx_ && HMAC_Update(ctx_.get(), - reinterpret_cast(data), - len) == 1; + ncrypto::Buffer buf{ + .data = data, + .len = len, + }; + return ctx_.update(buf); } void Hmac::HmacUpdate(const FunctionCallbackInfo& args) { @@ -123,24 +131,27 @@ void Hmac::HmacDigest(const FunctionCallbackInfo& args) { } unsigned char md_value[EVP_MAX_MD_SIZE]; - unsigned int md_len = 0; + ncrypto::Buffer buf{ + .data = md_value, + .len = sizeof(md_value), + }; if (hmac->ctx_) { - bool ok = HMAC_Final(hmac->ctx_.get(), md_value, &md_len); - hmac->ctx_.reset(); - if (!ok) { + if (!hmac->ctx_.digestInto(&buf)) [[unlikely]] { + hmac->ctx_.reset(); return ThrowCryptoError(env, ERR_get_error(), "Failed to finalize HMAC"); } + hmac->ctx_.reset(); } Local error; MaybeLocal rc = StringBytes::Encode(env->isolate(), reinterpret_cast(md_value), - md_len, + buf.len, encoding, &error); - if (rc.IsEmpty()) { + if (rc.IsEmpty()) [[unlikely]] { CHECK(!error.IsEmpty()); env->isolate()->ThrowException(error); return; @@ -225,31 +236,29 @@ bool HmacTraits::DeriveBits( Environment* env, const HmacConfig& params, ByteSource* out) { - HMACCtxPointer ctx(HMAC_CTX_new()); + auto ctx = HMACCtxPointer::New(); - if (!ctx || !HMAC_Init_ex(ctx.get(), - params.key.GetSymmetricKey(), - params.key.GetSymmetricKeySize(), - params.digest, - nullptr)) { + ncrypto::Buffer key_buf{ + .data = params.key.GetSymmetricKey(), + .len = params.key.GetSymmetricKeySize(), + }; + if (!ctx.init(key_buf, params.digest)) [[unlikely]] { return false; } - if (!HMAC_Update( - ctx.get(), - params.data.data(), - params.data.size())) { + ncrypto::Buffer buffer{ + .data = params.data.data(), + .len = params.data.size(), + }; + if (!ctx.update(buffer)) [[unlikely]] { return false; } - ByteSource::Builder buf(EVP_MAX_MD_SIZE); - unsigned int len; - - if (!HMAC_Final(ctx.get(), buf.data(), &len)) { + auto buf = ctx.digest(); + if (!buf) [[unlikely]] return false; - } - *out = std::move(buf).release(len); + *out = ByteSource::Allocated(buf.release()); return true; } diff --git a/src/crypto/crypto_random.cc b/src/crypto/crypto_random.cc index 03c5eb9fc31b18..4e9ff906c06571 100644 --- a/src/crypto/crypto_random.cc +++ b/src/crypto/crypto_random.cc @@ -14,7 +14,6 @@ namespace node { using ncrypto::BignumPointer; using ncrypto::ClearErrorOnReturn; using v8::ArrayBuffer; -using v8::BackingStore; using v8::Boolean; using v8::FunctionCallbackInfo; using v8::Int32; @@ -195,7 +194,8 @@ bool CheckPrimeTraits::DeriveBits( const CheckPrimeConfig& params, ByteSource* out) { int ret = params.candidate.isPrime(params.checks, getPrimeCheckCallback(env)); - if (ret < 0) [[unlikely]] return false; + if (ret < 0) [[unlikely]] + return false; ByteSource::Builder buf(1); buf.data()[0] = ret; *out = std::move(buf).release(); diff --git a/src/crypto/crypto_util.h b/src/crypto/crypto_util.h index 504746e3c87812..e3c9a82c17b382 100644 --- a/src/crypto/crypto_util.h +++ b/src/crypto/crypto_util.h @@ -229,7 +229,10 @@ class ByteSource final { template operator ncrypto::Buffer() const { - return ncrypto::Buffer{.data = data(), .len = size()}; + return ncrypto::Buffer{ + .data = data(), + .len = size(), + }; } inline size_t size() const { return size_; }