From 63d0f7085371f8a1c60020afd29699142ad40370 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Fri, 24 Jan 2025 12:53:42 -0800 Subject: [PATCH 1/7] src: clean up some obsolete crypto methods Signed-off-by: James M Snell --- src/crypto/crypto_common.cc | 22 +++------------------- src/crypto/crypto_common.h | 9 --------- src/crypto/crypto_tls.cc | 8 +++----- src/quic/tlscontext.cc | 2 +- 4 files changed, 7 insertions(+), 34 deletions(-) diff --git a/src/crypto/crypto_common.cc b/src/crypto/crypto_common.cc index 591509e735b943..59a681385a2857 100644 --- a/src/crypto/crypto_common.cc +++ b/src/crypto/crypto_common.cc @@ -53,32 +53,16 @@ SSLSessionPointer GetTLSSession(const unsigned char* buf, size_t length) { return SSLSessionPointer(d2i_SSL_SESSION(nullptr, &buf, length)); } -long VerifyPeerCertificate( // NOLINT(runtime/int) - const SSLPointer& ssl, - long def) { // NOLINT(runtime/int) - return ssl.verifyPeerCertificate().value_or(def); -} - -bool UseSNIContext( - const SSLPointer& ssl, BaseObjectPtr context) { - return ssl.setSniContext(context->ctx()); -} - -bool SetGroups(SecureContext* sc, const char* groups) { - return sc->ctx().setGroups(groups); -} - MaybeLocal GetValidationErrorReason(Environment* env, int err) { auto reason = X509Pointer::ErrorReason(err).value_or(""); if (reason == "") return Undefined(env->isolate()); - return OneByteString(env->isolate(), reason.data(), reason.length()); + return OneByteString(env->isolate(), reason); } MaybeLocal GetValidationErrorCode(Environment* env, int err) { - if (err == 0) - return Undefined(env->isolate()); + if (err == 0) return Undefined(env->isolate()); auto error = X509Pointer::ErrorCode(err); - return OneByteString(env->isolate(), error.data(), error.length()); + return OneByteString(env->isolate(), error); } MaybeLocal GetCert(Environment* env, const SSLPointer& ssl) { diff --git a/src/crypto/crypto_common.h b/src/crypto/crypto_common.h index dba1cc14b1c486..e9dd8407767d6f 100644 --- a/src/crypto/crypto_common.h +++ b/src/crypto/crypto_common.h @@ -17,15 +17,6 @@ namespace crypto { ncrypto::SSLSessionPointer GetTLSSession(const unsigned char* buf, size_t length); -long VerifyPeerCertificate( // NOLINT(runtime/int) - const ncrypto::SSLPointer& ssl, - long def = X509_V_ERR_UNSPECIFIED); // NOLINT(runtime/int) - -bool UseSNIContext(const ncrypto::SSLPointer& ssl, - BaseObjectPtr context); - -bool SetGroups(SecureContext* sc, const char* groups); - v8::MaybeLocal GetValidationErrorReason(Environment* env, int err); v8::MaybeLocal GetValidationErrorCode(Environment* env, int err); diff --git a/src/crypto/crypto_tls.cc b/src/crypto/crypto_tls.cc index 7104ee19a6dd79..374193577d2801 100644 --- a/src/crypto/crypto_tls.cc +++ b/src/crypto/crypto_tls.cc @@ -1612,7 +1612,7 @@ void TLSWrap::CertCbDone(const FunctionCallbackInfo& args) { // Store the SNI context for later use. w->sni_context_ = BaseObjectPtr(sc); - if (UseSNIContext(w->ssl_, w->sni_context_) && !w->SetCACerts(sc)) { + if (w->ssl_.setSniContext(w->sni_context_->ctx()) && !w->SetCACerts(sc)) { // Not clear why sometimes we throw error, and sometimes we call // onerror(). Both cause .destroy(), but onerror does a bit more. unsigned long err = ERR_get_error(); // NOLINT(runtime/int) @@ -1675,8 +1675,7 @@ void TLSWrap::SetKeyCert(const FunctionCallbackInfo& args) { if (cons->HasInstance(ctx)) { SecureContext* sc = Unwrap(ctx.As()); CHECK_NOT_NULL(sc); - if (!UseSNIContext(w->ssl_, BaseObjectPtr(sc)) || - !w->SetCACerts(sc)) { + if (!w->ssl_.setSniContext(sc->ctx()) || !w->SetCACerts(sc)) { unsigned long err = ERR_get_error(); // NOLINT(runtime/int) return ThrowCryptoError(env, err, "SetKeyCert"); } @@ -1851,8 +1850,7 @@ void TLSWrap::VerifyError(const FunctionCallbackInfo& args) { // peer certificate is questionable but it's compatible with what was // here before. long x509_verify_error = // NOLINT(runtime/int) - VerifyPeerCertificate( - w->ssl_, + w->ssl_.verifyPeerCertificate().value_or( X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT); if (x509_verify_error == X509_V_OK) diff --git a/src/quic/tlscontext.cc b/src/quic/tlscontext.cc index c8f88b34561eb2..bff1c1edea2186 100644 --- a/src/quic/tlscontext.cc +++ b/src/quic/tlscontext.cc @@ -589,7 +589,7 @@ SSLPointer TLSSession::Initialize( std::optional TLSSession::VerifyPeerIdentity(Environment* env) { - int err = crypto::VerifyPeerCertificate(ssl_); + int err = ssl_.verifyPeerCertificate().value_or(X509_V_ERR_UNSPECIFIED); if (err == X509_V_OK) return std::nullopt; Local reason; Local code; From 0e4a696792604c6ed24d060ec8a28f8d8b8b6cbd Mon Sep 17 00:00:00 2001 From: James M Snell Date: Fri, 24 Jan 2025 13:25:38 -0800 Subject: [PATCH 2/7] src: eliminate some unnecessary boilerplate/utility in crypto common Signed-off-by: James M Snell --- deps/ncrypto/ncrypto.cc | 18 +++++ deps/ncrypto/ncrypto.h | 4 ++ src/crypto/crypto_common.cc | 130 ++++++++++++------------------------ src/crypto/crypto_common.h | 5 -- src/quic/tlscontext.cc | 9 ++- 5 files changed, 73 insertions(+), 93 deletions(-) diff --git a/deps/ncrypto/ncrypto.cc b/deps/ncrypto/ncrypto.cc index be3ef98d763366..2ff0213564e16a 100644 --- a/deps/ncrypto/ncrypto.cc +++ b/deps/ncrypto/ncrypto.cc @@ -2585,6 +2585,24 @@ EVPKeyPointer SSLPointer::getPeerTempKey() const { return EVPKeyPointer(raw_key); } +std::optional SSLPointer::getCipherName() const { + auto cipher = getCipher(); + if (cipher == nullptr) return std::nullopt; + return SSL_CIPHER_get_name(cipher); +} + +std::optional SSLPointer::getCipherStandardName() const { + auto cipher = getCipher(); + if (cipher == nullptr) return std::nullopt; + return SSL_CIPHER_standard_name(cipher); +} + +std::optional SSLPointer::getCipherVersion() const { + auto cipher = getCipher(); + if (cipher == nullptr) return std::nullopt; + return SSL_CIPHER_get_version(cipher); +} + SSLCtxPointer::SSLCtxPointer(SSL_CTX* ctx) : ctx_(ctx) {} SSLCtxPointer::SSLCtxPointer(SSLCtxPointer&& other) noexcept diff --git a/deps/ncrypto/ncrypto.h b/deps/ncrypto/ncrypto.h index 75ac9fd8d705aa..e2c023b32a7202 100644 --- a/deps/ncrypto/ncrypto.h +++ b/deps/ncrypto/ncrypto.h @@ -914,6 +914,10 @@ class SSLPointer final { const SSL_CIPHER* getCipher() const; bool isServer() const; + std::optional getCipherName() const; + std::optional getCipherStandardName() const; + std::optional getCipherVersion() const; + std::optional verifyPeerCertificate() const; void getCiphers(std::function cb) const; diff --git a/src/crypto/crypto_common.cc b/src/crypto/crypto_common.cc index 59a681385a2857..e81bda7ad36f2b 100644 --- a/src/crypto/crypto_common.cc +++ b/src/crypto/crypto_common.cc @@ -43,7 +43,6 @@ using v8::Integer; using v8::Local; using v8::MaybeLocal; using v8::Object; -using v8::String; using v8::Undefined; using v8::Value; @@ -73,35 +72,6 @@ MaybeLocal GetCert(Environment* env, const SSLPointer& ssl) { } namespace { -template -bool Set( - Local context, - Local target, - Local name, - MaybeLocal maybe_value) { - Local value; - if (!maybe_value.ToLocal(&value)) - return false; - - // Undefined is ignored, but still considered successful - if (value->IsUndefined()) - return true; - - return !target->Set(context, name, value).IsNothing(); -} - -template -MaybeLocal GetCipherValue(Environment* env, const SSL_CIPHER* cipher) { - if (cipher == nullptr) - return Undefined(env->isolate()); - - return OneByteString(env->isolate(), getstr(cipher)); -} - -constexpr auto GetCipherName = GetCipherValue; -constexpr auto GetCipherStandardName = GetCipherValue; -constexpr auto GetCipherVersion = GetCipherValue; - StackOfX509 CloneSSLCerts(X509Pointer&& cert, const STACK_OF(X509)* const ssl_certs) { StackOfX509 peer_certs(sk_X509_new(nullptr)); @@ -132,11 +102,11 @@ MaybeLocal AddIssuerChainToObject(X509Pointer* cert, Local ca_info; if (!X509Certificate::toObject(env, ca).ToLocal(&ca_info)) return {}; CHECK(ca_info->IsObject()); - - if (!Set(env->context(), - object, - env->issuercert_string(), - ca_info.As())) { + if (object + ->Set(env->context(), + env->issuercert_string(), + ca_info.As()) + .IsNothing()) { return {}; } object = ca_info.As(); @@ -168,10 +138,10 @@ MaybeLocal GetLastIssuedCert( CHECK(ca_info->IsObject()); - if (!Set(env->context(), - issuer_chain, - env->issuercert_string(), - ca_info.As())) { + if (issuer_chain + ->Set( + env->context(), env->issuercert_string(), ca_info.As()) + .IsNothing()) { return {}; } issuer_chain = ca_info.As(); @@ -188,41 +158,30 @@ MaybeLocal GetLastIssuedCert( return MaybeLocal(issuer_chain); } -} // namespace - -MaybeLocal GetCurrentCipherName(Environment* env, - const SSLPointer& ssl) { - return GetCipherName(env, ssl.getCipher()); -} - -MaybeLocal GetCurrentCipherVersion(Environment* env, - const SSLPointer& ssl) { - return GetCipherVersion(env, ssl.getCipher()); -} - -template (*Get)(Environment* env, const SSL_CIPHER* cipher)> -MaybeLocal GetCurrentCipherValue(Environment* env, - const SSLPointer& ssl) { - return Get(env, ssl.getCipher()); +Local maybeString(Environment* env, + std::optional value) { + if (!value.has_value()) return Undefined(env->isolate()); + return OneByteString(env->isolate(), value.value()); } +} // namespace MaybeLocal GetCipherInfo(Environment* env, const SSLPointer& ssl) { if (ssl.getCipher() == nullptr) return MaybeLocal(); EscapableHandleScope scope(env->isolate()); Local info = Object::New(env->isolate()); - if (!Set(env->context(), - info, - env->name_string(), - GetCurrentCipherValue(env, ssl)) || - !Set(env->context(), - info, - env->standard_name_string(), - GetCurrentCipherValue(env, ssl)) || - !Set(env->context(), - info, - env->version_string(), - GetCurrentCipherValue(env, ssl))) { + if (info->Set(env->context(), + env->name_string(), + maybeString(env, ssl.getCipherName())) + .IsNothing() || + info->Set(env->context(), + env->standard_name_string(), + maybeString(env, ssl.getCipherStandardName())) + .IsNothing() || + info->Set(env->context(), + env->version_string(), + maybeString(env, ssl.getCipherVersion())) + .IsNothing()) { return MaybeLocal(); } @@ -242,11 +201,12 @@ MaybeLocal GetEphemeralKey(Environment* env, const SSLPointer& ssl) { int kid = key.id(); switch (kid) { case EVP_PKEY_DH: - if (!Set(context, info, env->type_string(), env->dh_string()) || - !Set(context, - info, - env->size_string(), - Integer::New(env->isolate(), key.bits()))) { + if (info->Set(context, env->type_string(), env->dh_string()) + .IsNothing() || + info->Set(context, + env->size_string(), + Integer::New(env->isolate(), key.bits())) + .IsNothing()) { return MaybeLocal(); } break; @@ -261,16 +221,16 @@ MaybeLocal GetEphemeralKey(Environment* env, const SSLPointer& ssl) { } else { curve_name = OBJ_nid2sn(kid); } - if (!Set( - context, info, env->type_string(), env->ecdh_string()) || - !Set(context, - info, - env->name_string(), - OneByteString(env->isolate(), curve_name)) || - !Set(context, - info, - env->size_string(), - Integer::New(env->isolate(), key.bits()))) { + if (info->Set(context, env->type_string(), env->ecdh_string()) + .IsNothing() || + info->Set(context, + env->name_string(), + OneByteString(env->isolate(), curve_name)) + .IsNothing() || + info->Set(context, + env->size_string(), + Integer::New(env->isolate(), key.bits())) + .IsNothing()) { return MaybeLocal(); } } @@ -362,10 +322,8 @@ MaybeLocal GetPeerCert( // Last certificate should be self-signed. if (cert.view().isIssuedBy(cert.view()) && - !Set(env->context(), - issuer_chain, - env->issuercert_string(), - issuer_chain)) { + issuer_chain->Set(env->context(), env->issuercert_string(), issuer_chain) + .IsNothing()) { return {}; } diff --git a/src/crypto/crypto_common.h b/src/crypto/crypto_common.h index e9dd8407767d6f..8bd561b9e1b57e 100644 --- a/src/crypto/crypto_common.h +++ b/src/crypto/crypto_common.h @@ -42,11 +42,6 @@ v8::MaybeLocal ECPointToBuffer( point_conversion_form_t form, const char** error); -v8::MaybeLocal GetCurrentCipherName(Environment* env, - const ncrypto::SSLPointer& ssl); -v8::MaybeLocal GetCurrentCipherVersion( - Environment* env, const ncrypto::SSLPointer& ssl); - } // namespace crypto } // namespace node diff --git a/src/quic/tlscontext.cc b/src/quic/tlscontext.cc index bff1c1edea2186..1190570bf67a4e 100644 --- a/src/quic/tlscontext.cc +++ b/src/quic/tlscontext.cc @@ -34,6 +34,7 @@ using v8::Maybe; using v8::MaybeLocal; using v8::Nothing; using v8::Object; +using v8::Undefined; using v8::Value; namespace quic { @@ -619,11 +620,15 @@ MaybeLocal TLSSession::ephemeral_key(Environment* env) const { } MaybeLocal TLSSession::cipher_name(Environment* env) const { - return crypto::GetCurrentCipherName(env, ssl_); + auto name = ssl_.getCipherName(); + if (!name.has_value()) return Undefined(env->isolate()); + return OneByteString(env->isolate(), name.value()); } MaybeLocal TLSSession::cipher_version(Environment* env) const { - return crypto::GetCurrentCipherVersion(env, ssl_); + auto version = ssl_.getCipherVersion(); + if (!version.has_value()) return Undefined(env->isolate()); + return OneByteString(env->isolate(), version.value()); } const std::string_view TLSSession::servername() const { From 0c16a101840ad36d94afc906fb5a35b6afe99d0e Mon Sep 17 00:00:00 2001 From: James M Snell Date: Fri, 24 Jan 2025 16:01:40 -0800 Subject: [PATCH 3/7] src: convert more of aes to ncrypto Signed-off-by: James M Snell --- src/crypto/crypto_aes.cc | 78 ++++++++++++++++++++++++---------------- src/crypto/crypto_aes.h | 12 +++---- 2 files changed, 52 insertions(+), 38 deletions(-) diff --git a/src/crypto/crypto_aes.cc b/src/crypto/crypto_aes.cc index ce32b578f9b24a..3f4e3e6672ae91 100644 --- a/src/crypto/crypto_aes.cc +++ b/src/crypto/crypto_aes.cc @@ -19,6 +19,7 @@ namespace node { using ncrypto::BignumPointer; using ncrypto::Cipher; using ncrypto::CipherCtxPointer; +using ncrypto::DataPointer; using v8::FunctionCallbackInfo; using v8::Just; using v8::JustVoid; @@ -31,6 +32,9 @@ using v8::Value; namespace crypto { namespace { +constexpr size_t kAesBlockSize = 16; +constexpr const char* kDefaultWrapIV = "\xa6\xa6\xa6\xa6\xa6\xa6\xa6\xa6"; + // Implements general AES encryption and decryption for CBC // The key_data must be a secret key. // On success, this function sets out to a new ByteSource @@ -113,15 +117,17 @@ WebCryptoCipherStatus AES_Cipher(Environment* env, return WebCryptoCipherStatus::FAILED; } - ByteSource::Builder buf(buf_len); + auto buf = DataPointer::Alloc(buf_len); + auto ptr = static_cast(buf.get()); // In some outdated version of OpenSSL (e.g. // ubi81_sharedlibs_openssl111fips_x64) may be used in sharedlib mode, the - // logic will be failed when input size is zero. The newly OpenSSL has fixed + // logic will be failed when input size is zero. The newer OpenSSL has fixed // it up. But we still have to regard zero as special in Node.js code to // prevent old OpenSSL failure. // - // Refs: https://github.com/openssl/openssl/commit/420cb707b880e4fb649094241371701013eeb15f + // Refs: + // https://github.com/openssl/openssl/commit/420cb707b880e4fb649094241371701013eeb15f // Refs: https://github.com/nodejs/node/pull/38913#issuecomment-866505244 buffer = { .data = in.data(), @@ -129,14 +135,14 @@ WebCryptoCipherStatus AES_Cipher(Environment* env, }; if (in.empty()) { out_len = 0; - } else if (!ctx.update(buffer, buf.data(), &out_len)) { + } else if (!ctx.update(buffer, ptr, &out_len)) { return WebCryptoCipherStatus::FAILED; } total += out_len; CHECK_LE(out_len, buf_len); out_len = ctx.getBlockSize(); - if (!ctx.update({}, buf.data() + total, &out_len, true)) { + if (!ctx.update({}, ptr + total, &out_len, true)) { return WebCryptoCipherStatus::FAILED; } total += out_len; @@ -144,14 +150,20 @@ WebCryptoCipherStatus AES_Cipher(Environment* env, // If using AES_GCM, grab the generated auth tag and append // it to the end of the ciphertext. if (cipher_mode == kWebCryptoCipherEncrypt && mode == EVP_CIPH_GCM_MODE) { - if (!ctx.getAeadTag(tag_len, buf.data() + total)) { + if (!ctx.getAeadTag(tag_len, ptr + total)) { return WebCryptoCipherStatus::FAILED; } total += tag_len; } + if (total == 0) { + *out = ByteSource::Allocated(nullptr, 0); + return WebCryptoCipherStatus::OK; + } + // It's possible that we haven't used the full allocated space. Size down. - *out = std::move(buf).release(total); + buf = buf.resize(total); + *out = ByteSource::Allocated(buf.release()); return WebCryptoCipherStatus::OK; } @@ -213,11 +225,10 @@ WebCryptoCipherStatus AES_CTR_Cipher2(const KeyObjectData& key_data, if (!ctx) { return WebCryptoCipherStatus::FAILED; } - const bool encrypt = cipher_mode == kWebCryptoCipherEncrypt; if (!ctx.init( params.cipher, - encrypt, + cipher_mode == kWebCryptoCipherEncrypt, reinterpret_cast(key_data.GetSymmetricKey()), counter)) { // Cipher init failed @@ -230,19 +241,14 @@ WebCryptoCipherStatus AES_CTR_Cipher2(const KeyObjectData& key_data, .data = in.data(), .len = in.size(), }; - if (!ctx.update(buffer, out, &out_len)) { + if (!ctx.update(buffer, out, &out_len) || + !ctx.update({}, out + out_len, &final_len, true)) { return WebCryptoCipherStatus::FAILED; } - if (!ctx.update({}, out + out_len, &final_len, true)) { - return WebCryptoCipherStatus::FAILED; - } - - out_len += final_len; - if (static_cast(out_len) != in.size()) - return WebCryptoCipherStatus::FAILED; - - return WebCryptoCipherStatus::OK; + return static_cast(out_len + final_len) != in.size() + ? WebCryptoCipherStatus::FAILED + : WebCryptoCipherStatus::OK; } WebCryptoCipherStatus AES_CTR_Cipher(Environment* env, @@ -258,8 +264,9 @@ WebCryptoCipherStatus AES_CTR_Cipher(Environment* env, auto num_output = BignumPointer::New(); - if (!num_output.setWord(CeilDiv(in.size(), kAesBlockSize))) + if (!num_output.setWord(CeilDiv(in.size(), kAesBlockSize))) { return WebCryptoCipherStatus::FAILED; + } // Just like in chromium's implementation, if the counter will // be incremented more than there are counter values, we fail. @@ -272,7 +279,7 @@ WebCryptoCipherStatus AES_CTR_Cipher(Environment* env, } // Output size is identical to the input size. - ByteSource::Builder buf(in.size()); + auto buf = DataPointer::Alloc(in.size()); // Also just like in chromium's implementation, if we can process // the input without wrapping the counter, we'll do it as a single @@ -283,8 +290,10 @@ WebCryptoCipherStatus AES_CTR_Cipher(Environment* env, params, in, params.iv.data(), - buf.data()); - if (status == WebCryptoCipherStatus::OK) *out = std::move(buf).release(); + static_cast(buf.get())); + if (status == WebCryptoCipherStatus::OK) { + *out = ByteSource::Allocated(buf.release()); + } return status; } @@ -297,14 +306,16 @@ WebCryptoCipherStatus AES_CTR_Cipher(Environment* env, params, ByteSource::Foreign(in.data(), input_size_part1), params.iv.data(), - buf.data()); + static_cast(buf.get())); - if (status != WebCryptoCipherStatus::OK) + if (status != WebCryptoCipherStatus::OK) { return status; + } // Wrap the counter around to zero std::vector new_counter_block = BlockWithZeroedCounter(params); + auto ptr = static_cast(buf.get()) + input_size_part1; // Encrypt the second part... status = AES_CTR_Cipher2(key_data, @@ -313,9 +324,11 @@ WebCryptoCipherStatus AES_CTR_Cipher(Environment* env, ByteSource::Foreign(in.data() + input_size_part1, in.size() - input_size_part1), new_counter_block.data(), - buf.data() + input_size_part1); + ptr); - if (status == WebCryptoCipherStatus::OK) *out = std::move(buf).release(); + if (status == WebCryptoCipherStatus::OK) { + *out = ByteSource::Allocated(buf.release()); + } return status; } @@ -456,7 +469,7 @@ Maybe AESCipherTraits::AdditionalConfig( int cipher_nid; #define V(name, _, nid) \ - case kKeyVariantAES_##name: { \ + case AESKeyVariant::name: { \ cipher_nid = nid; \ break; \ } @@ -507,7 +520,7 @@ WebCryptoCipherStatus AESCipherTraits::DoCipher(Environment* env, const ByteSource& in, ByteSource* out) { #define V(name, fn, _) \ - case kKeyVariantAES_##name: \ + case AESKeyVariant::name: \ return fn(env, key_data, cipher_mode, params, in, out); switch (params.variant) { VARIANTS(V) @@ -520,8 +533,13 @@ WebCryptoCipherStatus AESCipherTraits::DoCipher(Environment* env, void AES::Initialize(Environment* env, Local target) { AESCryptoJob::Initialize(env, target); -#define V(name, _, __) NODE_DEFINE_CONSTANT(target, kKeyVariantAES_##name); +#define V(name, _, __) \ + constexpr static auto kKeyVariantAES_##name = \ + static_cast(AESKeyVariant::name); \ + NODE_DEFINE_CONSTANT(target, kKeyVariantAES_##name); + VARIANTS(V) + #undef V } diff --git a/src/crypto/crypto_aes.h b/src/crypto/crypto_aes.h index 7bcfa36afdace4..b5c371e5a6e7ad 100644 --- a/src/crypto/crypto_aes.h +++ b/src/crypto/crypto_aes.h @@ -9,11 +9,8 @@ #include "env.h" #include "v8.h" -namespace node { -namespace crypto { -constexpr size_t kAesBlockSize = 16; +namespace node::crypto { constexpr unsigned kNoAuthTagLength = static_cast(-1); -constexpr const char* kDefaultWrapIV = "\xa6\xa6\xa6\xa6\xa6\xa6\xa6\xa6"; #define VARIANTS(V) \ V(CTR_128, AES_CTR_Cipher, NID_aes_128_ctr) \ @@ -29,8 +26,8 @@ constexpr const char* kDefaultWrapIV = "\xa6\xa6\xa6\xa6\xa6\xa6\xa6\xa6"; V(KW_192, AES_Cipher, NID_id_aes192_wrap) \ V(KW_256, AES_Cipher, NID_id_aes256_wrap) -enum AESKeyVariant { -#define V(name, _, __) kKeyVariantAES_##name, +enum class AESKeyVariant { +#define V(name, _, __) name, VARIANTS(V) #undef V }; @@ -81,8 +78,7 @@ namespace AES { void Initialize(Environment* env, v8::Local target); void RegisterExternalReferences(ExternalReferenceRegistry* registry); } // namespace AES -} // namespace crypto -} // namespace node +} // namespace node::crypto #endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS #endif // SRC_CRYPTO_CRYPTO_AES_H_ From a2ec037ac9903c0ac8cdeb85534a9d65bc51377b Mon Sep 17 00:00:00 2001 From: James M Snell Date: Sat, 25 Jan 2025 08:40:05 -0800 Subject: [PATCH 4/7] src: drop ByteSource::Builder in favor of ncrypto::DataPointer Signed-off-by: James M Snell --- deps/ncrypto/ncrypto.h | 7 +++- src/crypto/README.md | 4 --- src/crypto/crypto_ec.cc | 33 ++++++++++++------- src/crypto/crypto_keygen.cc | 8 +++-- src/crypto/crypto_random.cc | 7 ++-- src/crypto/crypto_sig.cc | 8 ++--- src/crypto/crypto_util.cc | 22 +++++++------ src/crypto/crypto_util.h | 65 +++++-------------------------------- 8 files changed, 62 insertions(+), 92 deletions(-) diff --git a/deps/ncrypto/ncrypto.h b/deps/ncrypto/ncrypto.h index e2c023b32a7202..0d1d466c3a0c14 100644 --- a/deps/ncrypto/ncrypto.h +++ b/deps/ncrypto/ncrypto.h @@ -384,7 +384,12 @@ class DataPointer final { inline bool operator==(std::nullptr_t) noexcept { return data_ == nullptr; } inline operator bool() const { return data_ != nullptr; } - inline void* get() const noexcept { return data_; } + + template + inline T* get() const noexcept { + return static_cast(data_); + } + inline size_t size() const noexcept { return len_; } void reset(void* data = nullptr, size_t len = 0); void reset(const Buffer& buffer); diff --git a/src/crypto/README.md b/src/crypto/README.md index 53f64ba22d8261..884869b725d54f 100644 --- a/src/crypto/README.md +++ b/src/crypto/README.md @@ -106,10 +106,6 @@ an `ArrayBuffer` (`v8::BackingStore`), or allocated data. * If allocated data is used, then it must have been allocated using OpenSSL's allocator. It will be freed automatically when the `ByteSource` is destroyed. -The `ByteSource::Builder` class can be used to allocate writable memory that can -then be released as a `ByteSource`, making it read-only, or freed by destroying -the `ByteSource::Builder` without releasing it as a `ByteSource`. - ### `ArrayBufferOrViewContents` The `ArrayBufferOrViewContents` class is a helper utility that abstracts diff --git a/src/crypto/crypto_ec.cc b/src/crypto/crypto_ec.cc index 98f1e1312769ca..d453d79a07341e 100644 --- a/src/crypto/crypto_ec.cc +++ b/src/crypto/crypto_ec.cc @@ -483,15 +483,16 @@ bool ECDHBitsTraits::DeriveBits(Environment* env, const auto pub = ECKeyPointer::GetPublicKey(public_key); int field_size = EC_GROUP_get_degree(group); len = (field_size + 7) / 8; - ByteSource::Builder buf(len); + auto buf = DataPointer::Alloc(len); CHECK_NOT_NULL(pub); CHECK_NOT_NULL(private_key); - if (ECDH_compute_key(buf.data(), len, pub, private_key, nullptr) <= + if (ECDH_compute_key( + static_cast(buf.get()), len, pub, private_key, nullptr) <= 0) { return false; } - *out = std::move(buf).release(); + *out = ByteSource::Allocated(buf.release()); } } @@ -605,14 +606,19 @@ WebCryptoKeyExportStatus EC_Raw_Export(const KeyObjectData& key_data, size_t len = EC_POINT_point2oct(group, point, form, nullptr, 0, nullptr); if (len == 0) return WebCryptoKeyExportStatus::FAILED; - ByteSource::Builder data(len); - size_t check_len = EC_POINT_point2oct( - group, point, form, data.data(), len, nullptr); + auto data = DataPointer::Alloc(len); + size_t check_len = + EC_POINT_point2oct(group, + point, + form, + static_cast(data.get()), + len, + nullptr); if (check_len == 0) return WebCryptoKeyExportStatus::FAILED; CHECK_EQ(len, check_len); - *out = std::move(data).release(); + *out = ByteSource::Allocated(data.release()); } return WebCryptoKeyExportStatus::OK; @@ -660,15 +666,20 @@ WebCryptoKeyExportStatus ECKeyExportTraits::DoExport( const size_t need = EC_POINT_point2oct(group, point, form, nullptr, 0, nullptr); if (need == 0) return WebCryptoKeyExportStatus::FAILED; - ByteSource::Builder data(need); - const size_t have = EC_POINT_point2oct( - group, point, form, data.data(), need, nullptr); + auto data = DataPointer::Alloc(need); + const size_t have = + EC_POINT_point2oct(group, + point, + form, + static_cast(data.get()), + need, + nullptr); if (have == 0) return WebCryptoKeyExportStatus::FAILED; auto ec = ECKeyPointer::New(group); CHECK(ec); auto uncompressed = ECPointPointer::New(group); ncrypto::Buffer buffer{ - .data = data.data(), + .data = static_cast(data.get()), .len = data.size(), }; CHECK(uncompressed.setFromBuffer(buffer, group)); diff --git a/src/crypto/crypto_keygen.cc b/src/crypto/crypto_keygen.cc index 7c3a85e9f8a24d..34f1e726d6c010 100644 --- a/src/crypto/crypto_keygen.cc +++ b/src/crypto/crypto_keygen.cc @@ -12,6 +12,7 @@ namespace node { +using ncrypto::DataPointer; using ncrypto::EVPKeyCtxPointer; using v8::FunctionCallbackInfo; using v8::Int32; @@ -70,10 +71,11 @@ Maybe SecretKeyGenTraits::AdditionalConfig( KeyGenJobStatus SecretKeyGenTraits::DoKeyGen(Environment* env, SecretKeyGenConfig* params) { - ByteSource::Builder bytes(params->length); - if (!ncrypto::CSPRNG(bytes.data(), params->length)) + auto bytes = DataPointer::Alloc(params->length); + if (!ncrypto::CSPRNG(static_cast(bytes.get()), + params->length)) return KeyGenJobStatus::FAILED; - params->out = std::move(bytes).release(); + params->out = ByteSource::Allocated(bytes.release()); return KeyGenJobStatus::OK; } diff --git a/src/crypto/crypto_random.cc b/src/crypto/crypto_random.cc index 4e9ff906c06571..423e37431099ea 100644 --- a/src/crypto/crypto_random.cc +++ b/src/crypto/crypto_random.cc @@ -13,6 +13,7 @@ namespace node { using ncrypto::BignumPointer; using ncrypto::ClearErrorOnReturn; +using ncrypto::DataPointer; using v8::ArrayBuffer; using v8::Boolean; using v8::FunctionCallbackInfo; @@ -196,9 +197,9 @@ bool CheckPrimeTraits::DeriveBits( int ret = params.candidate.isPrime(params.checks, getPrimeCheckCallback(env)); if (ret < 0) [[unlikely]] return false; - ByteSource::Builder buf(1); - buf.data()[0] = ret; - *out = std::move(buf).release(); + auto buf = DataPointer::Alloc(1); + static_cast(buf.get())[0] = ret; + *out = ByteSource::Allocated(buf.release()); return true; } diff --git a/src/crypto/crypto_sig.cc b/src/crypto/crypto_sig.cc index 175a8e92ef437f..a28f12237d0883 100644 --- a/src/crypto/crypto_sig.cc +++ b/src/crypto/crypto_sig.cc @@ -700,12 +700,12 @@ bool SignTraits::DeriveBits( break; } case SignConfiguration::Mode::Verify: { - ByteSource::Builder buf(1); - buf.data()[0] = 0; + auto buf = DataPointer::Alloc(1); + static_cast(buf.get())[0] = 0; if (context.verify(params.data, params.signature)) { - buf.data()[0] = 1; + static_cast(buf.get())[0] = 1; } - *out = std::move(buf).release(); + *out = ByteSource::Allocated(buf.release()); } } diff --git a/src/crypto/crypto_util.cc b/src/crypto/crypto_util.cc index 61f8cbf6703b50..ead997ce78e226 100644 --- a/src/crypto/crypto_util.cc +++ b/src/crypto/crypto_util.cc @@ -29,6 +29,7 @@ namespace node { using ncrypto::BignumPointer; using ncrypto::BIOPointer; using ncrypto::CryptoErrorList; +using ncrypto::DataPointer; using ncrypto::EnginePointer; using ncrypto::EVPKeyCtxPointer; using v8::ArrayBuffer; @@ -363,9 +364,9 @@ MaybeLocal ByteSource::ToBuffer(Environment* env) { ByteSource ByteSource::FromBIO(const BIOPointer& bio) { CHECK(bio); BUF_MEM* bptr = bio; - ByteSource::Builder out(bptr->length); - memcpy(out.data(), bptr->data, bptr->length); - return std::move(out).release(); + auto out = DataPointer::Alloc(bptr->length); + memcpy(out.get(), bptr->data, bptr->length); + return ByteSource::Allocated(out.release()); } ByteSource ByteSource::FromEncodedString(Environment* env, @@ -375,10 +376,10 @@ ByteSource ByteSource::FromEncodedString(Environment* env, ByteSource out; if (StringBytes::Size(env->isolate(), key, enc).To(&length) && length > 0) { - ByteSource::Builder buf(length); - size_t actual = - StringBytes::Write(env->isolate(), buf.data(), length, key, enc); - out = std::move(buf).release(actual); + auto buf = DataPointer::Alloc(length); + size_t actual = StringBytes::Write( + env->isolate(), static_cast(buf.get()), length, key, enc); + out = ByteSource::Allocated(buf.resize(actual).release()); } return out; @@ -395,11 +396,12 @@ ByteSource ByteSource::FromString(Environment* env, Local str, CHECK(str->IsString()); size_t size = str->Utf8Length(env->isolate()); size_t alloc_size = ntc ? size + 1 : size; - ByteSource::Builder out(alloc_size); + auto out = DataPointer::Alloc(alloc_size); int opts = String::NO_OPTIONS; if (!ntc) opts |= String::NO_NULL_TERMINATION; - str->WriteUtf8(env->isolate(), out.data(), alloc_size, nullptr, opts); - return std::move(out).release(); + str->WriteUtf8( + env->isolate(), static_cast(out.get()), alloc_size, nullptr, opts); + return ByteSource::Allocated(out.release()); } ByteSource ByteSource::FromBuffer(Local buffer, bool ntc) { diff --git a/src/crypto/crypto_util.h b/src/crypto/crypto_util.h index e3c9a82c17b382..659387378188be 100644 --- a/src/crypto/crypto_util.h +++ b/src/crypto/crypto_util.h @@ -166,53 +166,6 @@ T* MallocOpenSSL(size_t count) { // contents are zeroed. class ByteSource final { public: - class Builder { - public: - // Allocates memory using OpenSSL's memory allocator. - explicit Builder(size_t size) - : data_(MallocOpenSSL(size)), size_(size) {} - - Builder(Builder&& other) = delete; - Builder& operator=(Builder&& other) = delete; - Builder(const Builder&) = delete; - Builder& operator=(const Builder&) = delete; - - ~Builder() { OPENSSL_clear_free(data_, size_); } - - // Returns the underlying non-const pointer. - template - T* data() { - return reinterpret_cast(data_); - } - - // Returns the (allocated) size in bytes. - size_t size() const { return size_; } - - // Returns if (allocated) size is zero. - bool empty() const { return size_ == 0; } - - // Finalizes the Builder and returns a read-only view that is optionally - // truncated. - ByteSource release(std::optional resize = std::nullopt) && { - if (resize) { - CHECK_LE(*resize, size_); - if (*resize == 0) { - OPENSSL_clear_free(data_, size_); - data_ = nullptr; - } - size_ = *resize; - } - ByteSource out = ByteSource::Allocated(data_, size_); - data_ = nullptr; - size_ = 0; - return out; - } - - private: - void* data_; - size_t size_; - }; - ByteSource() = default; ByteSource(ByteSource&& other) noexcept; ~ByteSource(); @@ -651,18 +604,18 @@ class ArrayBufferOrViewContents final { } inline ByteSource ToCopy() const { - if (empty()) return ByteSource(); - ByteSource::Builder buf(size()); - memcpy(buf.data(), data(), size()); - return std::move(buf).release(); + if (empty()) return {}; + auto buf = ncrypto::DataPointer::Alloc(size()); + memcpy(buf.get(), data(), size()); + return ByteSource::Allocated(buf.release()); } inline ByteSource ToNullTerminatedCopy() const { - if (empty()) return ByteSource(); - ByteSource::Builder buf(size() + 1); - memcpy(buf.data(), data(), size()); - buf.data()[size()] = 0; - return std::move(buf).release(size()); + if (empty()) return {}; + auto buf = ncrypto::DataPointer::Alloc(size() + 1); + memcpy(buf.get(), data(), size()); + static_cast(buf.get())[size()] = 0; + return ByteSource::Allocated(buf.release()); } inline ncrypto::DataPointer ToDataPointer() const { From 5843486948ec52290add75ee7f2c62c36dde619e Mon Sep 17 00:00:00 2001 From: James M Snell Date: Sat, 25 Jan 2025 13:52:02 -0800 Subject: [PATCH 5/7] src: move more x509 to ncrypto Signed-off-by: James M Snell --- deps/ncrypto/ncrypto.cc | 71 ++++++++++++++++++++++++++++++++++++ deps/ncrypto/ncrypto.h | 39 ++++++++++++++++++++ src/crypto/crypto_x509.cc | 75 +++++++-------------------------------- 3 files changed, 123 insertions(+), 62 deletions(-) diff --git a/deps/ncrypto/ncrypto.cc b/deps/ncrypto/ncrypto.cc index 2ff0213564e16a..70182313ab431e 100644 --- a/deps/ncrypto/ncrypto.cc +++ b/deps/ncrypto/ncrypto.cc @@ -915,6 +915,18 @@ BIOPointer X509View::toDER() const { return bio; } +const X509Name X509View::getSubjectName() const { + ClearErrorOnReturn clearErrorOnReturn; + if (cert_ == nullptr) return {}; + return X509Name(X509_get_subject_name(cert_)); +} + +const X509Name X509View::getIssuerName() const { + ClearErrorOnReturn clearErrorOnReturn; + if (cert_ == nullptr) return {}; + return X509Name(X509_get_issuer_name(cert_)); +} + BIOPointer X509View::getSubject() const { ClearErrorOnReturn clearErrorOnReturn; if (cert_ == nullptr) return {}; @@ -3831,4 +3843,63 @@ DataPointer hashDigest(const Buffer& buf, return data.resize(result_size); } +// ============================================================================ + +X509Name::X509Name() : name_(nullptr), total_(0) {} + +X509Name::X509Name(const X509_NAME* name) + : name_(name), total_(X509_NAME_entry_count(name)) {} + +X509Name::Iterator::Iterator(const X509Name& name, int pos) + : name_(name), loc_(pos) {} + +X509Name::Iterator& X509Name::Iterator::operator++() { + ++loc_; + return *this; +} + +X509Name::Iterator::operator bool() const { + return loc_ < name_.total_; +} + +bool X509Name::Iterator::operator==(const Iterator& other) const { + return loc_ == other.loc_; +} + +bool X509Name::Iterator::operator!=(const Iterator& other) const { + return loc_ != other.loc_; +} + +std::pair X509Name::Iterator::operator*() const { + if (loc_ == name_.total_) return {{}, {}}; + + X509_NAME_ENTRY* entry = X509_NAME_get_entry(name_, loc_); + if (entry == nullptr) [[unlikely]] + return {{}, {}}; + + ASN1_OBJECT* name = X509_NAME_ENTRY_get_object(entry); + ASN1_STRING* value = X509_NAME_ENTRY_get_data(entry); + + if (name == nullptr || value == nullptr) [[unlikely]] { + return {{}, {}}; + } + + int nid = OBJ_obj2nid(name); + std::string name_str; + if (nid != NID_undef) { + name_str = std::string(OBJ_nid2sn(nid)); + } else { + char buf[80]; + OBJ_obj2txt(buf, sizeof(buf), name, 0); + name_str = std::string(buf); + } + + unsigned char* value_str; + int value_str_size = ASN1_STRING_to_UTF8(&value_str, value); + + return { + std::move(name_str), + std::string(reinterpret_cast(value_str), value_str_size)}; +} + } // namespace ncrypto diff --git a/deps/ncrypto/ncrypto.h b/deps/ncrypto/ncrypto.h index 0d1d466c3a0c14..3340c137e52669 100644 --- a/deps/ncrypto/ncrypto.h +++ b/deps/ncrypto/ncrypto.h @@ -934,6 +934,43 @@ class SSLPointer final { DeleteFnPtr ssl_; }; +class X509Name final { + public: + X509Name(); + explicit X509Name(const X509_NAME* name); + NCRYPTO_DISALLOW_COPY_AND_MOVE(X509Name) + + inline operator const X509_NAME*() const { return name_; } + inline operator bool() const { return name_ != nullptr; } + inline const X509_NAME* get() const { return name_; } + inline size_t size() const { return total_; } + + class Iterator final { + public: + Iterator(const X509Name& name, int pos); + Iterator(const Iterator& other) = default; + Iterator(Iterator&& other) = default; + Iterator& operator=(const Iterator& other) = delete; + Iterator& operator=(Iterator&& other) = delete; + Iterator& operator++(); + operator bool() const; + bool operator==(const Iterator& other) const; + bool operator!=(const Iterator& other) const; + std::pair operator*() const; + + private: + const X509Name& name_; + int loc_; + }; + + inline Iterator begin() const { return Iterator(*this, 0); } + inline Iterator end() const { return Iterator(*this, total_); } + + private: + const X509_NAME* name_; + int total_; +}; + class X509View final { public: static X509View From(const SSLPointer& ssl); @@ -955,6 +992,8 @@ class X509View final { BIOPointer toPEM() const; BIOPointer toDER() const; + const X509Name getSubjectName() const; + const X509Name getIssuerName() const; BIOPointer getSubject() const; BIOPointer getSubjectAltName() const; BIOPointer getIssuer() const; diff --git a/src/crypto/crypto_x509.cc b/src/crypto/crypto_x509.cc index 782eced277518d..b974667a4cacb9 100644 --- a/src/crypto/crypto_x509.cc +++ b/src/crypto/crypto_x509.cc @@ -21,6 +21,7 @@ using ncrypto::ClearErrorOnReturn; using ncrypto::DataPointer; using ncrypto::ECKeyPointer; using ncrypto::SSLPointer; +using ncrypto::X509Name; using ncrypto::X509Pointer; using ncrypto::X509View; using v8::Array; @@ -92,6 +93,11 @@ void Fingerprint(const FunctionCallbackInfo& args) { } } +MaybeLocal ToV8Value(Environment* env, std::string_view val) { + return String::NewFromUtf8( + env->isolate(), val.data(), NewStringType::kNormal, val.size()); +} + MaybeLocal ToV8Value(Local context, BIOPointer&& bio) { if (!bio) [[unlikely]] return {}; @@ -106,51 +112,6 @@ MaybeLocal ToV8Value(Local context, BIOPointer&& bio) { return ret; } -MaybeLocal ToV8Value(Local context, const ASN1_OBJECT* obj) { - // If OpenSSL knows the type, use the short name of the type as the key, and - // the numeric representation of the type's OID otherwise. - int nid = OBJ_obj2nid(obj); - char buf[80]; - const char* str; - if (nid != NID_undef) { - str = OBJ_nid2sn(nid); - CHECK_NOT_NULL(str); - } else { - OBJ_obj2txt(buf, sizeof(buf), obj, true); - str = buf; - } - - Local result; - if (!String::NewFromUtf8(context->GetIsolate(), str).ToLocal(&result)) { - return {}; - } - return result; -} - -MaybeLocal ToV8Value(Local context, const ASN1_STRING* str) { - // The previous implementation used X509_NAME_print_ex, which escapes some - // characters in the value. The old implementation did not decode/unescape - // values correctly though, leading to ambiguous and incorrect - // representations. The new implementation only converts to Unicode and does - // not escape anything. - unsigned char* value_str; - int value_str_size = ASN1_STRING_to_UTF8(&value_str, str); - if (value_str_size < 0) [[unlikely]] { - return Undefined(context->GetIsolate()); - } - DataPointer free_value_str(value_str, value_str_size); - - Local result; - if (!String::NewFromUtf8(context->GetIsolate(), - reinterpret_cast(value_str), - NewStringType::kNormal, - value_str_size) - .ToLocal(&result)) { - return {}; - } - return result; -} - MaybeLocal ToV8Value(Local context, const BIOPointer& bio) { if (!bio) [[unlikely]] return {}; @@ -594,14 +555,9 @@ bool Set(Environment* env, // Convert an X509_NAME* into a JavaScript object. // Each entry of the name is converted into a property of the object. // The property value may be a single string or an array of strings. -template static MaybeLocal GetX509NameObject(Environment* env, - const X509View& cert) { - X509_NAME* name = get_name(cert.get()); - CHECK_NOT_NULL(name); - - int cnt = X509_NAME_entry_count(name); - CHECK_GE(cnt, 0); + const X509Name& name) { + if (!name) return {}; Local v8_name; Local v8_value; @@ -610,14 +566,9 @@ static MaybeLocal GetX509NameObject(Environment* env, Object::New(env->isolate(), Null(env->isolate()), nullptr, nullptr, 0); if (result.IsEmpty()) return {}; - for (int i = 0; i < cnt; i++) { - X509_NAME_ENTRY* entry = X509_NAME_get_entry(name, i); - CHECK_NOT_NULL(entry); - - if (!ToV8Value(env->context(), X509_NAME_ENTRY_get_object(entry)) - .ToLocal(&v8_name) || - !ToV8Value(env->context(), X509_NAME_ENTRY_get_data(entry)) - .ToLocal(&v8_value)) { + for (auto i : name) { + if (!ToV8Value(env, i.first).ToLocal(&v8_name) || + !ToV8Value(env, i.second).ToLocal(&v8_value)) { return {}; } @@ -727,11 +678,11 @@ MaybeLocal X509ToObject(Environment* env, const X509View& cert) { if (!Set(env, info, env->subject_string(), - GetX509NameObject(env, cert)) || + GetX509NameObject(env, cert.getSubjectName())) || !Set(env, info, env->issuer_string(), - GetX509NameObject(env, cert)) || + GetX509NameObject(env, cert.getIssuerName())) || !Set(env, info, env->subjectaltname_string(), From a69e2512c3455a11a28ec758c22693b48fbbef77 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Sun, 26 Jan 2025 15:08:02 -0800 Subject: [PATCH 6/7] src: cleanup more of crypto/ncrypto --- deps/ncrypto/ncrypto.cc | 43 +++++- deps/ncrypto/ncrypto.h | 29 +++- src/crypto/crypto_cipher.cc | 124 +++++++++-------- src/crypto/crypto_dh.cc | 8 +- src/crypto/crypto_dh.h | 6 +- src/crypto/crypto_dsa.cc | 60 +++----- src/crypto/crypto_dsa.h | 12 +- src/crypto/crypto_ec.cc | 55 ++++---- src/crypto/crypto_ec.h | 18 +-- src/crypto/crypto_keygen.cc | 10 +- src/crypto/crypto_keygen.h | 23 +-- src/crypto/crypto_keys.cc | 269 +++++++++++++++++------------------- src/crypto/crypto_keys.h | 14 +- src/crypto/crypto_rsa.cc | 34 ++--- src/crypto/crypto_rsa.h | 12 +- 15 files changed, 366 insertions(+), 351 deletions(-) diff --git a/deps/ncrypto/ncrypto.cc b/deps/ncrypto/ncrypto.cc index 70182313ab431e..82da0d34cba4b6 100644 --- a/deps/ncrypto/ncrypto.cc +++ b/deps/ncrypto/ncrypto.cc @@ -2402,6 +2402,15 @@ EVPKeyPointer::operator Rsa() const { return Rsa(rsa); } +EVPKeyPointer::operator Dsa() const { + int type = id(); + if (type != EVP_PKEY_DSA) return {}; + + OSSL3_CONST DSA* dsa = EVP_PKEY_get0_DSA(get()); + if (dsa == nullptr) return {}; + return Dsa(dsa); +} + bool EVPKeyPointer::validateDsaParameters() const { if (!pkey_) return false; /* Validate DSA2 parameters from FIPS 186-4 */ @@ -2660,8 +2669,8 @@ bool SSLCtxPointer::setGroups(const char* groups) { // ============================================================================ -const Cipher Cipher::FromName(const char* name) { - return Cipher(EVP_get_cipherbyname(name)); +const Cipher Cipher::FromName(std::string_view name) { + return Cipher(EVP_get_cipherbyname(name.data())); } const Cipher Cipher::FromNid(int nid) { @@ -3902,4 +3911,34 @@ std::pair X509Name::Iterator::operator*() const { std::string(reinterpret_cast(value_str), value_str_size)}; } +// ============================================================================ + +Dsa::Dsa() : dsa_(nullptr) {} + +Dsa::Dsa(OSSL3_CONST DSA* dsa) : dsa_(dsa) {} + +const BIGNUM* Dsa::getP() const { + if (dsa_ == nullptr) return nullptr; + const BIGNUM* p; + DSA_get0_pqg(dsa_, &p, nullptr, nullptr); + return p; +} + +const BIGNUM* Dsa::getQ() const { + if (dsa_ == nullptr) return nullptr; + const BIGNUM* q; + DSA_get0_pqg(dsa_, nullptr, &q, nullptr); + return q; +} + +size_t Dsa::getModulusLength() const { + if (dsa_ == nullptr) return 0; + return BignumPointer::GetBitCount(getP()); +} + +size_t Dsa::getDivisorLength() const { + if (dsa_ == nullptr) return 0; + return BignumPointer::GetBitCount(getQ()); +} + } // namespace ncrypto diff --git a/deps/ncrypto/ncrypto.h b/deps/ncrypto/ncrypto.h index 3340c137e52669..31d999d3e7e235 100644 --- a/deps/ncrypto/ncrypto.h +++ b/deps/ncrypto/ncrypto.h @@ -221,6 +221,7 @@ class ECDSASigPointer; class ECGroupPointer; class ECPointPointer; class ECKeyPointer; +class Dsa; class Rsa; class Ec; @@ -267,7 +268,7 @@ class Cipher final { bool isSupportedAuthenticatedMode() const; - static const Cipher FromName(const char* name); + static const Cipher FromName(std::string_view name); static const Cipher FromNid(int nid); static const Cipher FromCtx(const CipherCtxPointer& ctx); @@ -292,10 +293,35 @@ class Cipher final { const CipherParams& params, const Buffer in); + static constexpr bool IsValidGCMTagLength(unsigned int tag_len) { + return tag_len == 4 || tag_len == 8 || (tag_len >= 12 && tag_len <= 16); + } + private: const EVP_CIPHER* cipher_ = nullptr; }; +// ============================================================================ +// DSA + +class Dsa final { + public: + Dsa(); + Dsa(OSSL3_CONST DSA* dsa); + NCRYPTO_DISALLOW_COPY_AND_MOVE(Dsa) + + inline operator bool() const { return dsa_ != nullptr; } + inline operator OSSL3_CONST DSA*() const { return dsa_; } + + const BIGNUM* getP() const; + const BIGNUM* getQ() const; + size_t getModulusLength() const; + size_t getDivisorLength() const; + + private: + OSSL3_CONST DSA* dsa_; +}; + // ============================================================================ // RSA @@ -767,6 +793,7 @@ class EVPKeyPointer final { std::optional getBytesOfRS() const; int getDefaultSignPadding() const; operator Rsa() const; + operator Dsa() const; bool isRsaVariant() const; bool isOneShotVariant() const; diff --git a/src/crypto/crypto_cipher.cc b/src/crypto/crypto_cipher.cc index dca59f16723ef8..85996f605a9c52 100644 --- a/src/crypto/crypto_cipher.cc +++ b/src/crypto/crypto_cipher.cc @@ -35,10 +35,6 @@ using v8::Value; namespace crypto { namespace { -bool IsValidGCMTagLength(unsigned int tag_len) { - return tag_len == 4 || tag_len == 8 || (tag_len >= 12 && tag_len <= 16); -} - // Collects and returns information on the given cipher void GetCipherInfo(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); @@ -50,7 +46,7 @@ void GetCipherInfo(const FunctionCallbackInfo& args) { const auto cipher = ([&] { if (args[1]->IsString()) { Utf8Value name(env->isolate(), args[1]); - return Cipher::FromName(*name); + return Cipher::FromName(name.ToStringView()); } else { int nid = args[1].As()->Value(); return Cipher::FromNid(nid); @@ -166,16 +162,19 @@ void GetCipherInfo(const FunctionCallbackInfo& args) { } // namespace void CipherBase::GetSSLCiphers(const FunctionCallbackInfo& args) { + MarkPopErrorOnReturn mark_pop_error_on_return; Environment* env = Environment::GetCurrent(args); auto ctx = SSLCtxPointer::New(); if (!ctx) { - return ThrowCryptoError(env, ERR_get_error(), "SSL_CTX_new"); + return ThrowCryptoError( + env, mark_pop_error_on_return.peekError(), "SSL_CTX_new"); } auto ssl = SSLPointer::New(ctx); if (!ssl) { - return ThrowCryptoError(env, ERR_get_error(), "SSL_new"); + return ThrowCryptoError( + env, mark_pop_error_on_return.peekError(), "SSL_new"); } LocalVector arr(env->isolate()); @@ -309,6 +308,7 @@ void CipherBase::CommonInit(const char* cipher_type, const unsigned char* iv, int iv_len, unsigned int auth_tag_len) { + MarkPopErrorOnReturn mark_pop_error_on_return; CHECK(!ctx_); ctx_ = CipherCtxPointer::New(); CHECK(ctx_); @@ -319,14 +319,16 @@ void CipherBase::CommonInit(const char* cipher_type, const bool encrypt = (kind_ == kCipher); if (!ctx_.init(cipher, encrypt)) { - return ThrowCryptoError(env(), ERR_get_error(), + return ThrowCryptoError(env(), + mark_pop_error_on_return.peekError(), "Failed to initialize cipher"); } if (cipher.isSupportedAuthenticatedMode()) { CHECK_GE(iv_len, 0); - if (!InitAuthenticated(cipher_type, iv_len, auth_tag_len)) + if (!InitAuthenticated(cipher_type, iv_len, auth_tag_len)) { return; + } } if (!ctx_.setKeyLength(key_len)) { @@ -335,7 +337,8 @@ void CipherBase::CommonInit(const char* cipher_type, } if (!ctx_.init(Cipher(), encrypt, key, iv)) { - return ThrowCryptoError(env(), ERR_get_error(), + return ThrowCryptoError(env(), + mark_pop_error_on_return.peekError(), "Failed to initialize cipher"); } } @@ -422,8 +425,9 @@ void CipherBase::InitIv(const char* cipher_type, const bool has_iv = iv_buf.size() > 0; // Throw if no IV was passed and the cipher requires an IV - if (!has_iv && expected_iv_len != 0) + if (!has_iv && expected_iv_len != 0) { return THROW_ERR_CRYPTO_INVALID_IV(env()); + } // Throw if an IV was passed which does not match the cipher's fixed IV length // static_cast for the iv_buf.size() is safe because we've verified @@ -438,8 +442,9 @@ void CipherBase::InitIv(const char* cipher_type, // Check for invalid IV lengths, since OpenSSL does not under some // conditions: // https://www.openssl.org/news/secadv/20190306.txt. - if (iv_buf.size() > 12) + if (iv_buf.size() > 12) { return THROW_ERR_CRYPTO_INVALID_IV(env()); + } } CommonInit( @@ -504,7 +509,7 @@ bool CipherBase::InitAuthenticated( const int mode = ctx_.getMode(); if (mode == EVP_CIPH_GCM_MODE) { if (auth_tag_len != kNoAuthTagLength) { - if (!IsValidGCMTagLength(auth_tag_len)) { + if (!Cipher::IsValidGCMTagLength(auth_tag_len)) { THROW_ERR_CRYPTO_INVALID_AUTH_TAG( env(), "Invalid authentication tag length: %u", @@ -595,9 +600,11 @@ void CipherBase::GetAuthTag(const FunctionCallbackInfo& args) { return; } - args.GetReturnValue().Set( - Buffer::Copy(env, cipher->auth_tag_, cipher->auth_tag_len_) - .FromMaybe(Local())); + Local ret; + if (Buffer::Copy(env, cipher->auth_tag_, cipher->auth_tag_len_) + .ToLocal(&ret)) { + args.GetReturnValue().Set(ret); + } } void CipherBase::SetAuthTag(const FunctionCallbackInfo& args) { @@ -624,7 +631,7 @@ void CipherBase::SetAuthTag(const FunctionCallbackInfo& args) { // Restrict GCM tag lengths according to NIST 800-38d, page 9. is_valid = (cipher->auth_tag_len_ == kNoAuthTagLength || cipher->auth_tag_len_ == tag_len) && - IsValidGCMTagLength(tag_len); + Cipher::IsValidGCMTagLength(tag_len); } else { // At this point, the tag length is already known and must match the // length of the given authentication tag. @@ -693,12 +700,12 @@ bool CipherBase::SetAAD( return false; } - if (!CheckCCMMessageLength(plaintext_len)) + if (!CheckCCMMessageLength(plaintext_len)) { return false; + } - if (kind_ == kDecipher) { - if (!MaybePassAuthTagToOpenSSL()) - return false; + if (kind_ == kDecipher && !MaybePassAuthTagToOpenSSL()) { + return false; } ncrypto::Buffer buffer{ @@ -738,19 +745,20 @@ CipherBase::UpdateResult CipherBase::Update( const char* data, size_t len, std::unique_ptr* out) { - if (!ctx_ || len > INT_MAX) - return kErrorState; + if (!ctx_ || len > INT_MAX) return kErrorState; MarkPopErrorOnReturn mark_pop_error_on_return; const int mode = ctx_.getMode(); - if (mode == EVP_CIPH_CCM_MODE && !CheckCCMMessageLength(len)) + if (mode == EVP_CIPH_CCM_MODE && !CheckCCMMessageLength(len)) { return kErrorMessageSize; + } // Pass the authentication tag to OpenSSL if possible. This will only happen // once, usually on the first update. - if (kind_ == kDecipher && IsAuthenticatedMode()) + if (kind_ == kDecipher && IsAuthenticatedMode()) { CHECK(MaybePassAuthTagToOpenSSL()); + } const int block_size = ctx_.getBlockSize(); CHECK_GT(block_size, 0); @@ -800,34 +808,38 @@ CipherBase::UpdateResult CipherBase::Update( } void CipherBase::Update(const FunctionCallbackInfo& args) { - Decode(args, [](CipherBase* cipher, - const FunctionCallbackInfo& args, - const char* data, size_t size) { - std::unique_ptr out; - Environment* env = Environment::GetCurrent(args); - - if (size > INT_MAX) [[unlikely]] { - return THROW_ERR_OUT_OF_RANGE(env, "data is too long"); - } - UpdateResult r = cipher->Update(data, size, &out); - - if (r != kSuccess) { - if (r == kErrorState) { - ThrowCryptoError(env, ERR_get_error(), - "Trying to add data in unsupported state"); - } - return; - } + Decode( + args, + [](CipherBase* cipher, + const FunctionCallbackInfo& args, + const char* data, + size_t size) { + MarkPopErrorOnReturn mark_pop_error_on_return; + std::unique_ptr out; + Environment* env = Environment::GetCurrent(args); + + if (size > INT_MAX) [[unlikely]] { + return THROW_ERR_OUT_OF_RANGE(env, "data is too long"); + } + UpdateResult r = cipher->Update(data, size, &out); + + if (r != kSuccess) { + if (r == kErrorState) { + ThrowCryptoError(env, + mark_pop_error_on_return.peekError(), + "Trying to add data in unsupported state"); + } + return; + } - Local ab = ArrayBuffer::New(env->isolate(), std::move(out)); - args.GetReturnValue().Set( - Buffer::New(env, ab, 0, ab->ByteLength()).FromMaybe(Local())); - }); + auto ab = ArrayBuffer::New(env->isolate(), std::move(out)); + args.GetReturnValue().Set(Buffer::New(env, ab, 0, ab->ByteLength()) + .FromMaybe(Local())); + }); } bool CipherBase::SetAutoPadding(bool auto_padding) { - if (!ctx_) - return false; + if (!ctx_) return false; MarkPopErrorOnReturn mark_pop_error_on_return; return ctx_.setPadding(auto_padding); } @@ -841,8 +853,7 @@ void CipherBase::SetAutoPadding(const FunctionCallbackInfo& args) { } bool CipherBase::Final(std::unique_ptr* out) { - if (!ctx_) - return false; + if (!ctx_) return false; const int mode = ctx_.getMode(); @@ -906,11 +917,13 @@ bool CipherBase::Final(std::unique_ptr* out) { void CipherBase::Final(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); + MarkPopErrorOnReturn mark_pop_error_on_return; CipherBase* cipher; ASSIGN_OR_RETURN_UNWRAP(&cipher, args.This()); - if (cipher->ctx_ == nullptr) + if (cipher->ctx_ == nullptr) { return THROW_ERR_CRYPTO_INVALID_STATE(env); + } std::unique_ptr out; @@ -923,10 +936,10 @@ void CipherBase::Final(const FunctionCallbackInfo& args) { ? "Unsupported state or unable to authenticate data" : "Unsupported state"; - return ThrowCryptoError(env, ERR_get_error(), msg); + return ThrowCryptoError(env, mark_pop_error_on_return.peekError(), msg); } - Local ab = ArrayBuffer::New(env->isolate(), std::move(out)); + auto ab = ArrayBuffer::New(env->isolate(), std::move(out)); args.GetReturnValue().Set( Buffer::New(env, ab, 0, ab->ByteLength()).FromMaybe(Local())); } @@ -974,8 +987,7 @@ void PublicKeyCipher::Cipher(const FunctionCallbackInfo& args) { auto data = KeyObjectData::GetPublicOrPrivateKeyFromJs(args, &offset); if (!data) return; const auto& pkey = data.GetAsymmetricKey(); - if (!pkey) - return; + if (!pkey) return; ArrayBufferOrViewContents buf(args[offset]); if (!buf.CheckSizeInt32()) [[unlikely]] { diff --git a/src/crypto/crypto_dh.cc b/src/crypto/crypto_dh.cc index 0e25a937e175fa..3cfcc42a989f55 100644 --- a/src/crypto/crypto_dh.cc +++ b/src/crypto/crypto_dh.cc @@ -524,11 +524,11 @@ bool DHBitsTraits::DeriveBits( return true; } -Maybe GetDhKeyDetail(Environment* env, - const KeyObjectData& key, - Local target) { +bool GetDhKeyDetail(Environment* env, + const KeyObjectData& key, + Local target) { CHECK_EQ(key.GetAsymmetricKey().id(), EVP_PKEY_DH); - return JustVoid(); + return true; } void DiffieHellman::Initialize(Environment* env, Local target) { diff --git a/src/crypto/crypto_dh.h b/src/crypto/crypto_dh.h index 2af668035bbb0a..c18729ad114a3f 100644 --- a/src/crypto/crypto_dh.h +++ b/src/crypto/crypto_dh.h @@ -115,9 +115,9 @@ struct DHBitsTraits final { using DHBitsJob = DeriveBitsJob; -v8::Maybe GetDhKeyDetail(Environment* env, - const KeyObjectData& key, - v8::Local target); +bool GetDhKeyDetail(Environment* env, + const KeyObjectData& key, + v8::Local target); } // namespace crypto } // namespace node diff --git a/src/crypto/crypto_dsa.cc b/src/crypto/crypto_dsa.cc index cac05aa55f8dd2..05f370ce70ea13 100644 --- a/src/crypto/crypto_dsa.cc +++ b/src/crypto/crypto_dsa.cc @@ -14,14 +14,13 @@ namespace node { -using ncrypto::BignumPointer; +using ncrypto::Dsa; using ncrypto::EVPKeyCtxPointer; using v8::FunctionCallbackInfo; using v8::Int32; using v8::JustVoid; using v8::Local; using v8::Maybe; -using v8::Nothing; using v8::Number; using v8::Object; using v8::Uint32; @@ -106,41 +105,28 @@ WebCryptoKeyExportStatus DSAKeyExportTraits::DoExport( } } -Maybe GetDsaKeyDetail(Environment* env, - const KeyObjectData& key, - Local target) { - const BIGNUM* p; // Modulus length - const BIGNUM* q; // Divisor length - - Mutex::ScopedLock lock(key.mutex()); - const auto& m_pkey = key.GetAsymmetricKey(); - int type = m_pkey.id(); - CHECK(type == EVP_PKEY_DSA); - - const DSA* dsa = EVP_PKEY_get0_DSA(m_pkey.get()); - CHECK_NOT_NULL(dsa); - - DSA_get0_pqg(dsa, &p, &q, nullptr); - - size_t modulus_length = BignumPointer::GetBitCount(p); - size_t divisor_length = BignumPointer::GetBitCount(q); - - if (target - ->Set( - env->context(), - env->modulus_length_string(), - Number::New(env->isolate(), static_cast(modulus_length))) - .IsNothing() || - target - ->Set( - env->context(), - env->divisor_length_string(), - Number::New(env->isolate(), static_cast(divisor_length))) - .IsNothing()) { - return Nothing(); - } - - return JustVoid(); +bool GetDsaKeyDetail(Environment* env, + const KeyObjectData& key, + Local target) { + if (!key) return false; + Dsa dsa = key.GetAsymmetricKey(); + if (!dsa) return false; + + size_t modulus_length = dsa.getModulusLength(); + size_t divisor_length = dsa.getDivisorLength(); + + return target + ->Set(env->context(), + env->modulus_length_string(), + Number::New(env->isolate(), + static_cast(modulus_length))) + .IsJust() && + target + ->Set(env->context(), + env->divisor_length_string(), + Number::New(env->isolate(), + static_cast(divisor_length))) + .IsJust(); } namespace DSAAlg { diff --git a/src/crypto/crypto_dsa.h b/src/crypto/crypto_dsa.h index c31aa9786558c2..d761aad2ad02c7 100644 --- a/src/crypto/crypto_dsa.h +++ b/src/crypto/crypto_dsa.h @@ -10,8 +10,7 @@ #include "memory_tracker.h" #include "v8.h" -namespace node { -namespace crypto { +namespace node::crypto { struct DsaKeyPairParams final : public MemoryRetainer { unsigned int modulus_bits; int divisor_bits; @@ -60,16 +59,15 @@ struct DSAKeyExportTraits final { using DSAKeyExportJob = KeyExportJob; -v8::Maybe GetDsaKeyDetail(Environment* env, - const KeyObjectData& key, - v8::Local target); +bool GetDsaKeyDetail(Environment* env, + const KeyObjectData& key, + v8::Local target); namespace DSAAlg { void Initialize(Environment* env, v8::Local target); void RegisterExternalReferences(ExternalReferenceRegistry* registry); } // namespace DSAAlg -} // namespace crypto -} // namespace node +} // namespace node::crypto #endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS #endif // SRC_CRYPTO_CRYPTO_DSA_H_ diff --git a/src/crypto/crypto_ec.cc b/src/crypto/crypto_ec.cc index d453d79a07341e..3753adaee80385 100644 --- a/src/crypto/crypto_ec.cc +++ b/src/crypto/crypto_ec.cc @@ -697,9 +697,9 @@ WebCryptoKeyExportStatus ECKeyExportTraits::DoExport( } } -Maybe ExportJWKEcKey(Environment* env, - const KeyObjectData& key, - Local target) { +bool ExportJWKEcKey(Environment* env, + const KeyObjectData& key, + Local target) { Mutex::ScopedLock lock(key.mutex()); const auto& m_pkey = key.GetAsymmetricKey(); CHECK_EQ(m_pkey.id(), EVP_PKEY_EC); @@ -720,14 +720,14 @@ Maybe ExportJWKEcKey(Environment* env, if (!EC_POINT_get_affine_coordinates(group, pub, x.get(), y.get(), nullptr)) { ThrowCryptoError(env, ERR_get_error(), "Failed to get elliptic-curve point coordinates"); - return Nothing(); + return false; } if (target->Set( env->context(), env->jwk_kty_string(), env->jwk_ec_string()).IsNothing()) { - return Nothing(); + return false; } if (SetEncodedValue( @@ -742,7 +742,7 @@ Maybe ExportJWKEcKey(Environment* env, env->jwk_y_string(), y.get(), degree_bytes).IsNothing()) { - return Nothing(); + return false; } Local crv_name; @@ -763,27 +763,28 @@ Maybe ExportJWKEcKey(Environment* env, default: { THROW_ERR_CRYPTO_JWK_UNSUPPORTED_CURVE( env, "Unsupported JWK EC curve: %s.", OBJ_nid2sn(nid)); - return Nothing(); + return false; } } if (target->Set( env->context(), env->jwk_crv_string(), crv_name).IsNothing()) { - return Nothing(); + return false; } if (key.GetKeyType() == kKeyTypePrivate) { auto pvt = ECKeyPointer::GetPrivateKey(ec); - return SetEncodedValue(env, target, env->jwk_d_string(), pvt, degree_bytes); + return SetEncodedValue(env, target, env->jwk_d_string(), pvt, degree_bytes) + .IsJust(); } - return JustVoid(); + return true; } -Maybe ExportJWKEdKey(Environment* env, - const KeyObjectData& key, - Local target) { +bool ExportJWKEdKey(Environment* env, + const KeyObjectData& key, + Local target) { Mutex::ScopedLock lock(key.mutex()); const auto& pkey = key.GetAsymmetricKey(); @@ -820,7 +821,8 @@ Maybe ExportJWKEdKey(Environment* env, return true; }; - if (target + return !( + target ->Set(env->context(), env->jwk_crv_string(), OneByteString(env->isolate(), curve)) @@ -829,11 +831,7 @@ Maybe ExportJWKEdKey(Environment* env, !trySetKey(env, pkey.rawPrivateKey(), target, env->jwk_d_string())) || !trySetKey(env, pkey.rawPublicKey(), target, env->jwk_x_string()) || target->Set(env->context(), env->jwk_kty_string(), env->jwk_okp_string()) - .IsNothing()) { - return Nothing(); - } - - return JustVoid(); + .IsNothing()); } KeyObjectData ImportJWKEcKey(Environment* env, @@ -897,9 +895,9 @@ KeyObjectData ImportJWKEcKey(Environment* env, return KeyObjectData::CreateAsymmetric(type, std::move(pkey)); } -Maybe GetEcKeyDetail(Environment* env, - const KeyObjectData& key, - Local target) { +bool GetEcKeyDetail(Environment* env, + const KeyObjectData& key, + Local target) { Mutex::ScopedLock lock(key.mutex()); const auto& m_pkey = key.GetAsymmetricKey(); CHECK_EQ(m_pkey.id(), EVP_PKEY_EC); @@ -910,14 +908,11 @@ Maybe GetEcKeyDetail(Environment* env, const auto group = ECKeyPointer::GetGroup(ec); int nid = EC_GROUP_get_curve_name(group); - if (target - ->Set(env->context(), - env->named_curve_string(), - OneByteString(env->isolate(), OBJ_nid2sn(nid))) - .IsNothing()) { - return Nothing(); - } - return JustVoid(); + return target + ->Set(env->context(), + env->named_curve_string(), + OneByteString(env->isolate(), OBJ_nid2sn(nid))) + .IsJust(); } // WebCrypto requires a different format for ECDSA signatures than diff --git a/src/crypto/crypto_ec.h b/src/crypto/crypto_ec.h index 7aeeebbe303eda..308f0b6d0440ab 100644 --- a/src/crypto/crypto_ec.h +++ b/src/crypto/crypto_ec.h @@ -141,22 +141,22 @@ struct ECKeyExportTraits final { using ECKeyExportJob = KeyExportJob; -v8::Maybe ExportJWKEcKey(Environment* env, - const KeyObjectData& key, - v8::Local target); +bool ExportJWKEcKey(Environment* env, + const KeyObjectData& key, + v8::Local target); -v8::Maybe ExportJWKEdKey(Environment* env, - const KeyObjectData& key, - v8::Local target); +bool ExportJWKEdKey(Environment* env, + const KeyObjectData& key, + v8::Local target); KeyObjectData ImportJWKEcKey(Environment* env, v8::Local jwk, const v8::FunctionCallbackInfo& args, unsigned int offset); -v8::Maybe GetEcKeyDetail(Environment* env, - const KeyObjectData& key, - v8::Local target); +bool GetEcKeyDetail(Environment* env, + const KeyObjectData& key, + v8::Local target); } // namespace crypto } // namespace node diff --git a/src/crypto/crypto_keygen.cc b/src/crypto/crypto_keygen.cc index 34f1e726d6c010..597c8bab781fc0 100644 --- a/src/crypto/crypto_keygen.cc +++ b/src/crypto/crypto_keygen.cc @@ -73,8 +73,9 @@ KeyGenJobStatus SecretKeyGenTraits::DoKeyGen(Environment* env, SecretKeyGenConfig* params) { auto bytes = DataPointer::Alloc(params->length); if (!ncrypto::CSPRNG(static_cast(bytes.get()), - params->length)) + params->length)) { return KeyGenJobStatus::FAILED; + } params->out = ByteSource::Allocated(bytes.release()); return KeyGenJobStatus::OK; } @@ -82,11 +83,7 @@ KeyGenJobStatus SecretKeyGenTraits::DoKeyGen(Environment* env, MaybeLocal SecretKeyGenTraits::EncodeKey(Environment* env, SecretKeyGenConfig* params) { auto data = KeyObjectData::CreateSecret(std::move(params->out)); - Local ret; - if (!KeyObjectHandle::Create(env, data).ToLocal(&ret)) { - return MaybeLocal(); - } - return ret; + return KeyObjectHandle::Create(env, data).FromMaybe(Local()); } namespace Keygen { @@ -99,7 +96,6 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) { NidKeyPairGenJob::RegisterExternalReferences(registry); SecretKeyGenJob::RegisterExternalReferences(registry); } - } // namespace Keygen } // namespace crypto } // namespace node diff --git a/src/crypto/crypto_keygen.h b/src/crypto/crypto_keygen.h index 6fdac8333b6ac6..e43d8cb0475ff2 100644 --- a/src/crypto/crypto_keygen.h +++ b/src/crypto/crypto_keygen.h @@ -11,8 +11,7 @@ #include "memory_tracker.h" #include "v8.h" -namespace node { -namespace crypto { +namespace node::crypto { namespace Keygen { void Initialize(Environment* env, v8::Local target); void RegisterExternalReferences(ExternalReferenceRegistry* registry); @@ -184,13 +183,11 @@ struct KeyPairGenTraits final { static v8::MaybeLocal EncodeKey(Environment* env, AdditionalParameters* params) { v8::Local keys[2]; - if (params->key - .ToEncodedPublicKey(env, params->public_key_encoding, &keys[0]) - .IsNothing() || - params->key - .ToEncodedPrivateKey(env, params->private_key_encoding, &keys[1]) - .IsNothing()) { - return v8::MaybeLocal(); + if (!params->key.ToEncodedPublicKey( + env, params->public_key_encoding, &keys[0]) || + !params->key.ToEncodedPrivateKey( + env, params->private_key_encoding, &keys[1])) { + return {}; } return v8::Array::New(env->isolate(), keys, arraysize(keys)); } @@ -233,11 +230,6 @@ struct KeyPairGenConfig final : public MemoryRetainer { AlgorithmParams params; KeyPairGenConfig() = default; - ~KeyPairGenConfig() { - if (key) { - Mutex::ScopedLock priv_lock(key.mutex()); - } - } explicit KeyPairGenConfig(KeyPairGenConfig&& other) noexcept : public_key_encoding(other.public_key_encoding), @@ -291,8 +283,7 @@ struct NidKeyPairGenTraits final { using NidKeyPairGenJob = KeyGenJob>; using SecretKeyGenJob = KeyGenJob; -} // namespace crypto -} // namespace node +} // namespace node::crypto #endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS #endif // SRC_CRYPTO_CRYPTO_KEYGEN_H_ diff --git a/src/crypto/crypto_keys.cc b/src/crypto/crypto_keys.cc index 2c55828facc35b..2ec1589c41621a 100644 --- a/src/crypto/crypto_keys.cc +++ b/src/crypto/crypto_keys.cc @@ -32,7 +32,6 @@ using v8::FunctionTemplate; using v8::Int32; using v8::Isolate; using v8::Just; -using v8::JustVoid; using v8::Local; using v8::Maybe; using v8::MaybeLocal; @@ -82,10 +81,11 @@ Maybe GetKeyFormatAndTypeFromJs( return Just(config); } -MaybeLocal BIOToStringOrBuffer( +MaybeLocal ToV8Value( Environment* env, const BIOPointer& bio, const EVPKeyPointer::AsymmetricKeyEncodingConfig& config) { + if (!bio) return {}; BUF_MEM* bptr = bio; if (config.format == EVPKeyPointer::PKFormatType::PEM) { // PEM is an ASCII format, so we will return it as a string. @@ -103,11 +103,9 @@ MaybeLocal WritePrivateKey( Environment* env, const EVPKeyPointer& pkey, const EVPKeyPointer::PrivateKeyEncodingConfig& config) { - CHECK(pkey); + if (!pkey) return {}; auto res = pkey.writePrivateKey(config); - if (res) { - return BIOToStringOrBuffer(env, std::move(res.value), config); - } + if (res) return ToV8Value(env, std::move(res.value), config); ThrowCryptoError( env, res.openssl_error.value_or(0), "Failed to encode private key"); @@ -118,47 +116,37 @@ MaybeLocal WritePublicKey( Environment* env, const EVPKeyPointer& pkey, const EVPKeyPointer::PublicKeyEncodingConfig& config) { - CHECK(pkey); + if (!pkey) return {}; auto res = pkey.writePublicKey(config); - if (res) { - return BIOToStringOrBuffer(env, res.value, config); - } + if (res) return ToV8Value(env, res.value, config); ThrowCryptoError( env, res.openssl_error.value_or(0), "Failed to encode public key"); return MaybeLocal(); } -Maybe ExportJWKSecretKey(Environment* env, - const KeyObjectData& key, - Local target) { +bool ExportJWKSecretKey(Environment* env, + const KeyObjectData& key, + Local target) { CHECK_EQ(key.GetKeyType(), kKeyTypeSecret); Local error; Local raw; - MaybeLocal key_data = StringBytes::Encode(env->isolate(), - key.GetSymmetricKey(), - key.GetSymmetricKeySize(), - BASE64URL, - &error); - if (!key_data.ToLocal(&raw)) { - CHECK(!error.IsEmpty()); + if (!StringBytes::Encode(env->isolate(), + key.GetSymmetricKey(), + key.GetSymmetricKeySize(), + BASE64URL, + &error) + .ToLocal(&raw)) { + DCHECK(!error.IsEmpty()); env->isolate()->ThrowException(error); - return Nothing(); + return false; } - if (target->Set( - env->context(), - env->jwk_kty_string(), - env->jwk_oct_string()).IsNothing() || - target->Set( - env->context(), - env->jwk_k_string(), - raw).IsNothing()) { - return Nothing(); - } - - return JustVoid(); + return target + ->Set(env->context(), env->jwk_kty_string(), env->jwk_oct_string()) + .IsJust() && + target->Set(env->context(), env->jwk_k_string(), raw).IsJust(); } KeyObjectData ImportJWKSecretKey(Environment* env, Local jwk) { @@ -174,10 +162,10 @@ KeyObjectData ImportJWKSecretKey(Environment* env, Local jwk) { ByteSource::FromEncodedString(env, key.As())); } -Maybe ExportJWKAsymmetricKey(Environment* env, - const KeyObjectData& key, - Local target, - bool handleRsaPss) { +bool ExportJWKAsymmetricKey(Environment* env, + const KeyObjectData& key, + Local target, + bool handleRsaPss) { switch (key.GetAsymmetricKey().id()) { case EVP_PKEY_RSA_PSS: { if (handleRsaPss) return ExportJWKRsaKey(env, key, target); @@ -197,7 +185,7 @@ Maybe ExportJWKAsymmetricKey(Environment* env, return ExportJWKEdKey(env, key, target); } THROW_ERR_CRYPTO_JWK_UNSUPPORTED_KEY_TYPE(env); - return Nothing(); + return false; } KeyObjectData ImportJWKAsymmetricKey(Environment* env, @@ -216,35 +204,39 @@ KeyObjectData ImportJWKAsymmetricKey(Environment* env, return {}; } -Maybe GetSecretKeyDetail(Environment* env, - const KeyObjectData& key, - Local target) { +bool GetSecretKeyDetail(Environment* env, + const KeyObjectData& key, + Local target) { // For the secret key detail, all we care about is the length, // converted to bits. - size_t length = key.GetSymmetricKeySize() * CHAR_BIT; - if (target - ->Set(env->context(), - env->length_string(), - Number::New(env->isolate(), static_cast(length))) - .IsNothing()) { - return Nothing(); - } - return JustVoid(); + return target + ->Set(env->context(), + env->length_string(), + Number::New( + env->isolate(), + static_cast(key.GetSymmetricKeySize() * CHAR_BIT))) + .IsJust(); } -Maybe GetAsymmetricKeyDetail(Environment* env, - const KeyObjectData& key, - Local target) { +bool GetAsymmetricKeyDetail(Environment* env, + const KeyObjectData& key, + Local target) { + if (!key) { + THROW_ERR_CRYPTO_OPERATION_FAILED(env); + return false; + } switch (key.GetAsymmetricKey().id()) { case EVP_PKEY_RSA: // Fall through + case EVP_PKEY_RSA2: + // Fall through case EVP_PKEY_RSA_PSS: return GetRsaKeyDetail(env, key, target); case EVP_PKEY_DSA: return GetDsaKeyDetail(env, key, target); case EVP_PKEY_EC: return GetEcKeyDetail(env, key, target); case EVP_PKEY_DH: return GetDhKeyDetail(env, key, target); } THROW_ERR_CRYPTO_INVALID_KEYTYPE(env); - return Nothing(); + return false; } KeyObjectData TryParsePrivateKey( @@ -266,30 +258,34 @@ KeyObjectData TryParsePrivateKey( return {}; } -// This maps true to JustVoid and false to Nothing(). -static inline Maybe NothingIfFalse(bool b) { - return b ? JustVoid() : Nothing(); +bool ExportJWKInner(Environment* env, + const KeyObjectData& key, + Local result, + bool handleRsaPss) { + return key.GetKeyType() == kKeyTypeSecret + ? ExportJWKSecretKey(env, key, result.As()) + : ExportJWKAsymmetricKey( + env, key, result.As(), handleRsaPss); } -Maybe ExportJWKInner(Environment* env, - const KeyObjectData& key, - Local result, - bool handleRsaPss) { - switch (key.GetKeyType()) { - case kKeyTypeSecret: - return ExportJWKSecretKey(env, key, result.As()); - case kKeyTypePublic: - // Fall through - case kKeyTypePrivate: - return ExportJWKAsymmetricKey( - env, key, result.As(), handleRsaPss); - default: - UNREACHABLE(); +int GetOKPCurveFromName(const char* name) { + int nid; + if (strcmp(name, "Ed25519") == 0) { + nid = EVP_PKEY_ED25519; + } else if (strcmp(name, "Ed448") == 0) { + nid = EVP_PKEY_ED448; + } else if (strcmp(name, "X25519") == 0) { + nid = EVP_PKEY_X25519; + } else if (strcmp(name, "X448") == 0) { + nid = EVP_PKEY_X448; + } else { + nid = NID_undef; } + return nid; } } // namespace -Maybe KeyObjectData::ToEncodedPublicKey( +bool KeyObjectData::ToEncodedPublicKey( Environment* env, const EVPKeyPointer::PublicKeyEncodingConfig& config, Local* out) { @@ -297,36 +293,33 @@ Maybe KeyObjectData::ToEncodedPublicKey( if (config.output_key_object) { // Note that this has the downside of containing sensitive data of the // private key. - return NothingIfFalse( - KeyObjectHandle::Create(env, addRefWithType(KeyType::kKeyTypePublic)) - .ToLocal(out)); + return KeyObjectHandle::Create(env, addRefWithType(KeyType::kKeyTypePublic)) + .ToLocal(out); } else if (config.format == EVPKeyPointer::PKFormatType::JWK) { *out = Object::New(env->isolate()); return ExportJWKInner( env, addRefWithType(KeyType::kKeyTypePublic), *out, false); } - return NothingIfFalse( - WritePublicKey(env, GetAsymmetricKey(), config).ToLocal(out)); + return WritePublicKey(env, GetAsymmetricKey(), config).ToLocal(out); } -Maybe KeyObjectData::ToEncodedPrivateKey( +bool KeyObjectData::ToEncodedPrivateKey( Environment* env, const EVPKeyPointer::PrivateKeyEncodingConfig& config, Local* out) { CHECK(key_type_ != KeyType::kKeyTypeSecret); if (config.output_key_object) { - return NothingIfFalse( - KeyObjectHandle::Create(env, addRefWithType(KeyType::kKeyTypePrivate)) - .ToLocal(out)); + return KeyObjectHandle::Create(env, + addRefWithType(KeyType::kKeyTypePrivate)) + .ToLocal(out); } else if (config.format == EVPKeyPointer::PKFormatType::JWK) { *out = Object::New(env->isolate()); return ExportJWKInner( env, addRefWithType(KeyType::kKeyTypePrivate), *out, false); } - return NothingIfFalse( - WritePrivateKey(env, GetAsymmetricKey(), config).ToLocal(out)); + return WritePrivateKey(env, GetAsymmetricKey(), config).ToLocal(out); } Maybe @@ -502,6 +495,7 @@ KeyObjectData KeyObjectData::GetParsedKey(KeyType type, EVPKeyPointer&& pkey, ParseKeyResult ret, const char* default_msg) { + MarkPopErrorOnReturn mark_pop_error_on_return; switch (ret) { case ParseKeyResult::kParseKeyOk: { return CreateAsymmetric(type, std::move(pkey)); @@ -512,7 +506,7 @@ KeyObjectData KeyObjectData::GetParsedKey(KeyType type, return {}; } default: { - ThrowCryptoError(env, ERR_get_error(), default_msg); + ThrowCryptoError(env, mark_pop_error_on_return.peekError(), default_msg); return {}; } } @@ -593,12 +587,12 @@ size_t KeyObjectData::GetSymmetricKeySize() const { } bool KeyObjectHandle::HasInstance(Environment* env, Local value) { - Local t = env->crypto_key_object_handle_constructor(); + auto t = env->crypto_key_object_handle_constructor(); return !t.IsEmpty() && t->HasInstance(value); } Local KeyObjectHandle::Initialize(Environment* env) { - Local templ = env->crypto_key_object_handle_constructor(); + auto templ = env->crypto_key_object_handle_constructor(); if (templ.IsEmpty()) { Isolate* isolate = env->isolate(); templ = NewFunctionTemplate(isolate, New); @@ -646,8 +640,9 @@ MaybeLocal KeyObjectHandle::Create(Environment* env, Local obj; Local ctor = KeyObjectHandle::Initialize(env); CHECK(!env->crypto_key_object_handle_constructor().IsEmpty()); - if (!ctor->NewInstance(env->context(), 0, nullptr).ToLocal(&obj)) - return MaybeLocal(); + if (!ctor->NewInstance(env->context(), 0, nullptr).ToLocal(&obj)) { + return {}; + } KeyObjectHandle* key = Unwrap(obj); CHECK_NOT_NULL(key); @@ -783,22 +778,6 @@ void KeyObjectHandle::InitECRaw(const FunctionCallbackInfo& args) { args.GetReturnValue().Set(true); } -int GetOKPCurveFromName(const char* name) { - int nid; - if (strcmp(name, "Ed25519") == 0) { - nid = EVP_PKEY_ED25519; - } else if (strcmp(name, "Ed448") == 0) { - nid = EVP_PKEY_ED448; - } else if (strcmp(name, "X25519") == 0) { - nid = EVP_PKEY_X25519; - } else if (strcmp(name, "X448") == 0) { - nid = EVP_PKEY_X448; - } else { - nid = NID_undef; - } - return nid; -} - void KeyObjectHandle::InitEDRaw(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); KeyObjectHandle* key; @@ -867,6 +846,7 @@ void KeyObjectHandle::Equals(const FunctionCallbackInfo& args) { break; } case kKeyTypePublic: + // Fall through case kKeyTypePrivate: { EVP_PKEY* pkey = key.GetAsymmetricKey().get(); EVP_PKEY* pkey2 = key2.GetAsymmetricKey().get(); @@ -898,22 +878,16 @@ void KeyObjectHandle::GetKeyDetail(const FunctionCallbackInfo& args) { const auto& data = key->Data(); - switch (data.GetKeyType()) { - case kKeyTypeSecret: - if (GetSecretKeyDetail(env, data, args[0].As()).IsNothing()) - return; - break; - case kKeyTypePublic: - // Fall through - case kKeyTypePrivate: - if (GetAsymmetricKeyDetail(env, data, args[0].As()).IsNothing()) - return; - break; - default: - UNREACHABLE(); + if (data.GetKeyType() == kKeyTypeSecret) { + if (GetSecretKeyDetail(env, data, args[0].As())) [[likely]] { + args.GetReturnValue().Set(args[0]); + } + return; } - args.GetReturnValue().Set(args[0]); + if (GetAsymmetricKeyDetail(env, data, args[0].As())) [[likely]] { + args.GetReturnValue().Set(args[0]); + } } Local KeyObjectHandle::GetAsymmetricKeyType() const { @@ -957,11 +931,8 @@ bool KeyObjectHandle::CheckEcKeyData() const { CHECK(ctx); CHECK_EQ(key.id(), EVP_PKEY_EC); - if (data_.GetKeyType() == kKeyTypePrivate) { - return ctx.privateCheck(); - } - - return ctx.publicCheck(); + return data_.GetKeyType() == kKeyTypePrivate ? ctx.privateCheck() + : ctx.publicCheck(); } void KeyObjectHandle::CheckEcKeyData(const FunctionCallbackInfo& args) { @@ -984,12 +955,17 @@ void KeyObjectHandle::Export(const FunctionCallbackInfo& args) { ASSIGN_OR_RETURN_UNWRAP(&key, args.This()); KeyType type = key->Data().GetKeyType(); + unsigned int offset = 0; - MaybeLocal result; + Local result; if (type == kKeyTypeSecret) { - result = key->ExportSecretKey(); - } else if (type == kKeyTypePublic) { - unsigned int offset = 0; + if (key->ExportSecretKey().ToLocal(&result)) [[likely]] { + args.GetReturnValue().Set(result); + } + return; + } + + if (type == kKeyTypePublic) { EVPKeyPointer::PublicKeyEncodingConfig config; if (!KeyObjectData::GetPublicKeyEncodingFromJs( args, &offset, kKeyContextExport) @@ -997,28 +973,29 @@ void KeyObjectHandle::Export(const FunctionCallbackInfo& args) { return; } CHECK_EQ(offset, static_cast(args.Length())); - result = key->ExportPublicKey(config); - } else { - CHECK_EQ(type, kKeyTypePrivate); - unsigned int offset = 0; - EVPKeyPointer::PrivateKeyEncodingConfig config; - if (!KeyObjectData::GetPrivateKeyEncodingFromJs( - args, &offset, kKeyContextExport) - .To(&config)) { - return; + if (key->ExportPublicKey(config).ToLocal(&result)) [[likely]] { + args.GetReturnValue().Set(result); } - CHECK_EQ(offset, static_cast(args.Length())); - result = key->ExportPrivateKey(config); + return; } - if (!result.IsEmpty()) - args.GetReturnValue().Set(result.FromMaybe(Local())); + CHECK_EQ(type, kKeyTypePrivate); + EVPKeyPointer::PrivateKeyEncodingConfig config; + if (!KeyObjectData::GetPrivateKeyEncodingFromJs( + args, &offset, kKeyContextExport) + .To(&config)) { + return; + } + CHECK_EQ(offset, static_cast(args.Length())); + if (key->ExportPrivateKey(config).ToLocal(&result)) [[likely]] { + args.GetReturnValue().Set(result); + } } MaybeLocal KeyObjectHandle::ExportSecretKey() const { - const char* buf = data_.GetSymmetricKey(); - unsigned int len = data_.GetSymmetricKeySize(); - return Buffer::Copy(env(), buf, len).FromMaybe(Local()); + return Buffer::Copy( + env(), data_.GetSymmetricKey(), data_.GetSymmetricKeySize()) + .FromMaybe(Local()); } MaybeLocal KeyObjectHandle::ExportPublicKey( @@ -1040,9 +1017,9 @@ void KeyObjectHandle::ExportJWK( CHECK(args[0]->IsObject()); CHECK(args[1]->IsBoolean()); - ExportJWKInner(env, key->Data(), args[0], args[1]->IsTrue()); - - args.GetReturnValue().Set(args[0]); + if (ExportJWKInner(env, key->Data(), args[0], args[1]->IsTrue())) { + args.GetReturnValue().Set(args[0]); + } } void NativeKeyObject::Initialize(Environment* env, Local target) { diff --git a/src/crypto/crypto_keys.h b/src/crypto/crypto_keys.h index e4f3e03e59bf48..794e2699c8163f 100644 --- a/src/crypto/crypto_keys.h +++ b/src/crypto/crypto_keys.h @@ -16,9 +16,7 @@ #include #include -namespace node { -namespace crypto { - +namespace node::crypto { enum KeyType { kKeyTypeSecret, kKeyTypePublic, @@ -85,12 +83,12 @@ class KeyObjectData final : public MemoryRetainer { unsigned int* offset, KeyEncodingContext context); - v8::Maybe ToEncodedPublicKey( + bool ToEncodedPublicKey( Environment* env, const ncrypto::EVPKeyPointer::PublicKeyEncodingConfig& config, v8::Local* out); - v8::Maybe ToEncodedPrivateKey( + bool ToEncodedPrivateKey( Environment* env, const ncrypto::EVPKeyPointer::PrivateKeyEncodingConfig& config, v8::Local* out); @@ -378,15 +376,11 @@ WebCryptoKeyExportStatus PKEY_SPKI_Export(const KeyObjectData& key_data, WebCryptoKeyExportStatus PKEY_PKCS8_Export(const KeyObjectData& key_data, ByteSource* out); -int GetOKPCurveFromName(const char* name); - namespace Keys { void Initialize(Environment* env, v8::Local target); void RegisterExternalReferences(ExternalReferenceRegistry* registry); } // namespace Keys - -} // namespace crypto -} // namespace node +} // namespace node::crypto #endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS #endif // SRC_CRYPTO_CRYPTO_KEYS_H_ diff --git a/src/crypto/crypto_rsa.cc b/src/crypto/crypto_rsa.cc index f0e0f9fd5f94a1..7742cf204df6d8 100644 --- a/src/crypto/crypto_rsa.cc +++ b/src/crypto/crypto_rsa.cc @@ -318,9 +318,9 @@ WebCryptoCipherStatus RSACipherTraits::DoCipher(Environment* env, return WebCryptoCipherStatus::FAILED; } -Maybe ExportJWKRsaKey(Environment* env, - const KeyObjectData& key, - Local target) { +bool ExportJWKRsaKey(Environment* env, + const KeyObjectData& key, + Local target) { Mutex::ScopedLock lock(key.mutex()); const auto& m_pkey = key.GetAsymmetricKey(); @@ -328,7 +328,7 @@ Maybe ExportJWKRsaKey(Environment* env, if (!rsa || target->Set(env->context(), env->jwk_kty_string(), env->jwk_rsa_string()) .IsNothing()) { - return Nothing(); + return false; } auto pub_key = rsa.getPublicKey(); @@ -337,7 +337,7 @@ Maybe ExportJWKRsaKey(Environment* env, .IsNothing() || SetEncodedValue(env, target, env->jwk_e_string(), pub_key.e) .IsNothing()) { - return Nothing(); + return false; } if (key.GetKeyType() == kKeyTypePrivate) { @@ -354,11 +354,11 @@ Maybe ExportJWKRsaKey(Environment* env, .IsNothing() || SetEncodedValue(env, target, env->jwk_qi_string(), pvt_key.qi) .IsNothing()) { - return Nothing(); + return false; } } - return JustVoid(); + return true; } KeyObjectData ImportJWKRsaKey(Environment* env, @@ -441,16 +441,16 @@ KeyObjectData ImportJWKRsaKey(Environment* env, return KeyObjectData::CreateAsymmetric(type, std::move(pkey)); } -Maybe GetRsaKeyDetail(Environment* env, - const KeyObjectData& key, - Local target) { +bool GetRsaKeyDetail(Environment* env, + const KeyObjectData& key, + Local target) { Mutex::ScopedLock lock(key.mutex()); const auto& m_pkey = key.GetAsymmetricKey(); // TODO(tniessen): Remove the "else" branch once we drop support for OpenSSL // versions older than 1.1.1e via FIPS / dynamic linking. const ncrypto::Rsa rsa = m_pkey; - if (!rsa) return Nothing(); + if (!rsa) return false; auto pub_key = rsa.getPublicKey(); @@ -461,7 +461,7 @@ Maybe GetRsaKeyDetail(Environment* env, env->isolate(), static_cast(BignumPointer::GetBitCount(pub_key.n)))) .IsNothing()) { - return Nothing(); + return false; } auto public_exponent = ArrayBuffer::NewBackingStore( @@ -479,7 +479,7 @@ Maybe GetRsaKeyDetail(Environment* env, env->public_exponent_string(), ArrayBuffer::New(env->isolate(), std::move(public_exponent))) .IsNothing()) { - return Nothing(); + return false; } if (m_pkey.id() == EVP_PKEY_RSA_PSS) { @@ -500,7 +500,7 @@ Maybe GetRsaKeyDetail(Environment* env, env->hash_algorithm_string(), OneByteString(env->isolate(), params.digest)) .IsNothing()) { - return Nothing(); + return false; } // If, for some reason, the MGF is not MGF1, then the MGF1 hash function @@ -512,7 +512,7 @@ Maybe GetRsaKeyDetail(Environment* env, env->mgf1_hash_algorithm_string(), OneByteString(env->isolate(), digest)) .IsNothing()) { - return Nothing(); + return false; } } @@ -521,12 +521,12 @@ Maybe GetRsaKeyDetail(Environment* env, env->salt_length_string(), Integer::New(env->isolate(), params.salt_length)) .IsNothing()) { - return Nothing(); + return false; } } } - return JustVoid(); + return true; } namespace RSAAlg { diff --git a/src/crypto/crypto_rsa.h b/src/crypto/crypto_rsa.h index 6fc48da1aea2cd..db5ba492faa398 100644 --- a/src/crypto/crypto_rsa.h +++ b/src/crypto/crypto_rsa.h @@ -112,18 +112,18 @@ struct RSACipherTraits final { using RSACipherJob = CipherJob; -v8::Maybe ExportJWKRsaKey(Environment* env, - const KeyObjectData& key, - v8::Local target); +bool ExportJWKRsaKey(Environment* env, + const KeyObjectData& key, + v8::Local target); KeyObjectData ImportJWKRsaKey(Environment* env, v8::Local jwk, const v8::FunctionCallbackInfo& args, unsigned int offset); -v8::Maybe GetRsaKeyDetail(Environment* env, - const KeyObjectData& key, - v8::Local target); +bool GetRsaKeyDetail(Environment* env, + const KeyObjectData& key, + v8::Local target); namespace RSAAlg { void Initialize(Environment* env, v8::Local target); From d5c54ff99c2daef5ee57a86443230bbc843d9046 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Tue, 28 Jan 2025 15:59:52 -0800 Subject: [PATCH 7/7] src, deps: port electron's boringssl workarounds --- deps/ncrypto/ncrypto.cc | 100 +++++++++++++++++++++++++++++++++++++--- deps/ncrypto/ncrypto.h | 16 +++++++ 2 files changed, 110 insertions(+), 6 deletions(-) diff --git a/deps/ncrypto/ncrypto.cc b/deps/ncrypto/ncrypto.cc index 82da0d34cba4b6..0a0da0263676d9 100644 --- a/deps/ncrypto/ncrypto.cc +++ b/deps/ncrypto/ncrypto.cc @@ -1,4 +1,5 @@ #include "ncrypto.h" +#include #include #include #include @@ -99,7 +100,15 @@ std::optional CryptoErrorList::pop_front() { // ============================================================================ DataPointer DataPointer::Alloc(size_t len) { +#ifdef OPENSSL_IS_BORINGSSL + // Boringssl does not implement OPENSSL_zalloc + auto ptr = OPENSSL_malloc(len); + if (ptr == nullptr) return {}; + memset(ptr, 0, len); + return DataPointer(ptr, len); +#else return DataPointer(OPENSSL_zalloc(len), len); +#endif } DataPointer DataPointer::Copy(const Buffer& buffer) { @@ -218,7 +227,12 @@ BignumPointer BignumPointer::New() { } BignumPointer BignumPointer::NewSecure() { +#ifdef OPENSSL_IS_BORINGSSL + // Boringssl does not implement BN_secure_new. + return New(); +#else return BignumPointer(BN_secure_new()); +#endif } BignumPointer& BignumPointer::operator=(BignumPointer&& other) noexcept { @@ -492,6 +506,7 @@ constexpr int days_from_epoch(int y, unsigned m, unsigned d) { return era * 146097 + static_cast(doe) - 719468; } +#ifndef OPENSSL_IS_BORINGSSL // tm must be in UTC // using time_t causes problems on 32-bit systems and windows x64. int64_t PortableTimeGM(struct tm* t) { @@ -512,6 +527,7 @@ int64_t PortableTimeGM(struct tm* t) { t->tm_min) + t->tm_sec; } +#endif // ============================================================================ // SPKAC @@ -822,7 +838,7 @@ bool SafeX509SubjectAltNamePrint(const BIOPointer& out, X509_EXTENSION* ext) { bool ok = true; - for (int i = 0; i < sk_GENERAL_NAME_num(names); i++) { + for (OPENSSL_SIZE_T i = 0; i < sk_GENERAL_NAME_num(names); i++) { GENERAL_NAME* gen = sk_GENERAL_NAME_value(names, i); if (i != 0) BIO_write(out.get(), ", ", 2); @@ -846,7 +862,7 @@ bool SafeX509InfoAccessPrint(const BIOPointer& out, X509_EXTENSION* ext) { bool ok = true; - for (int i = 0; i < sk_ACCESS_DESCRIPTION_num(descs); i++) { + for (OPENSSL_SIZE_T i = 0; i < sk_ACCESS_DESCRIPTION_num(descs); i++) { ACCESS_DESCRIPTION* desc = sk_ACCESS_DESCRIPTION_value(descs, i); if (i != 0) BIO_write(out.get(), "\n", 1); @@ -999,15 +1015,31 @@ BIOPointer X509View::getValidTo() const { } int64_t X509View::getValidToTime() const { +#ifdef OPENSSL_IS_BORINGSSL + // Boringssl does not implement ASN1_TIME_to_tm in a public way, + // and only recently added ASN1_TIME_to_posix. Some boringssl + // users on older version may still need to patch around this + // or use a different implementation. + int64_t tp; + ASN1_TIME_to_posix(X509_get0_notAfter(cert_), &tp); + return tp; +#else struct tm tp; ASN1_TIME_to_tm(X509_get0_notAfter(cert_), &tp); return PortableTimeGM(&tp); +#endif } int64_t X509View::getValidFromTime() const { +#ifdef OPENSSL_IS_BORINGSSL + int64_t tp; + ASN1_TIME_to_posix(X509_get0_notBefore(cert_), &tp); + return tp; +#else struct tm tp; ASN1_TIME_to_tm(X509_get0_notBefore(cert_), &tp); return PortableTimeGM(&tp); +#endif } DataPointer X509View::getSerialNumber() const { @@ -1324,7 +1356,12 @@ BIOPointer BIOPointer::NewMem() { } BIOPointer BIOPointer::NewSecMem() { +#ifdef OPENSSL_IS_BORINGSSL + // Boringssl does not implement the BIO_s_secmem API. + return BIOPointer(BIO_new(BIO_s_mem())); +#else return BIOPointer(BIO_new(BIO_s_secmem())); +#endif } BIOPointer BIOPointer::New(const BIO_METHOD* method) { @@ -1394,8 +1431,11 @@ BignumPointer DHPointer::FindGroup(const std::string_view name, #define V(n, p) \ if (EqualNoCase(name, n)) return BignumPointer(p(nullptr)); if (option != FindGroupOption::NO_SMALL_PRIMES) { +#ifndef OPENSSL_IS_BORINGSSL + // Boringssl does not support the 768 and 1024 small primes V("modp1", BN_get_rfc2409_prime_768); V("modp2", BN_get_rfc2409_prime_1024); +#endif V("modp5", BN_get_rfc3526_prime_1536); } V("modp14", BN_get_rfc3526_prime_2048); @@ -1467,15 +1507,22 @@ DHPointer::CheckResult DHPointer::check() { DHPointer::CheckPublicKeyResult DHPointer::checkPublicKey( const BignumPointer& pub_key) { ClearErrorOnReturn clearErrorOnReturn; - if (!pub_key || !dh_) return DHPointer::CheckPublicKeyResult::CHECK_FAILED; + if (!pub_key || !dh_) { + return DHPointer::CheckPublicKeyResult::CHECK_FAILED; + } int codes = 0; - if (DH_check_pub_key(dh_.get(), pub_key.get(), &codes) != 1) + if (DH_check_pub_key(dh_.get(), pub_key.get(), &codes) != 1) { return DHPointer::CheckPublicKeyResult::CHECK_FAILED; + } +#ifndef OPENSSL_IS_BORINGSSL + // Boringssl does not define DH_CHECK_PUBKEY_TOO_SMALL or TOO_LARGE if (codes & DH_CHECK_PUBKEY_TOO_SMALL) { return DHPointer::CheckPublicKeyResult::TOO_SMALL; - } else if (codes & DH_CHECK_PUBKEY_TOO_SMALL) { + } else if (codes & DH_CHECK_PUBKEY_TOO_LARGE) { return DHPointer::CheckPublicKeyResult::TOO_LARGE; - } else if (codes != 0) { + } +#endif + if (codes != 0) { return DHPointer::CheckPublicKeyResult::INVALID; } return CheckPublicKeyResult::NONE; @@ -2530,6 +2577,7 @@ std::optional SSLPointer::verifyPeerCertificate() const { const std::string_view SSLPointer::getClientHelloAlpn() const { if (ssl_ == nullptr) return {}; +#ifndef OPENSSL_IS_BORINGSSL const unsigned char* buf; size_t len; size_t rem; @@ -2546,10 +2594,15 @@ const std::string_view SSLPointer::getClientHelloAlpn() const { len = (buf[0] << 8) | buf[1]; if (len + 2 != rem) return {}; return reinterpret_cast(buf + 3); +#else + // Boringssl doesn't have a public API for this. + return {}; +#endif } const std::string_view SSLPointer::getClientHelloServerName() const { if (ssl_ == nullptr) return {}; +#ifndef OPENSSL_IS_BORINGSSL const unsigned char* buf; size_t len; size_t rem; @@ -2569,6 +2622,10 @@ const std::string_view SSLPointer::getClientHelloServerName() const { len = (*(buf + 3) << 8) | *(buf + 4); if (len + 2 > rem) return {}; return reinterpret_cast(buf + 5); +#else + // Boringssl doesn't have a public API for this. + return {}; +#endif } std::optional SSLPointer::GetServerName( @@ -2602,7 +2659,11 @@ bool SSLPointer::isServer() const { EVPKeyPointer SSLPointer::getPeerTempKey() const { if (!ssl_) return {}; EVP_PKEY* raw_key = nullptr; +#ifndef OPENSSL_IS_BORINGSSL if (!SSL_get_peer_tmp_key(get(), &raw_key)) return {}; +#else + if (!SSL_get_server_tmp_key(get(), &raw_key)) return {}; +#endif return EVPKeyPointer(raw_key); } @@ -3167,9 +3228,15 @@ int EVPKeyCtxPointer::initForSign() { } bool EVPKeyCtxPointer::setDhParameters(int prime_size, uint32_t generator) { +#ifndef OPENSSL_IS_BORINGSSL if (!ctx_) return false; return EVP_PKEY_CTX_set_dh_paramgen_prime_len(ctx_.get(), prime_size) == 1 && EVP_PKEY_CTX_set_dh_paramgen_generator(ctx_.get(), generator) == 1; +#else + // TODO(jasnell): Boringssl appears not to support this operation. + // Is there an alternative approach that Boringssl does support? + return false; +#endif } bool EVPKeyCtxPointer::setDsaParameters(uint32_t bits, @@ -3249,6 +3316,7 @@ bool EVPKeyCtxPointer::setRsaPssSaltlen(int salt_len) { } bool EVPKeyCtxPointer::setRsaImplicitRejection() { +#ifndef OPENSSL_IS_BORINGSSL if (!ctx_) return false; return EVP_PKEY_CTX_ctrl_str( ctx_.get(), "rsa_pkcs1_implicit_rejection", "1") > 0; @@ -3259,6 +3327,11 @@ bool EVPKeyCtxPointer::setRsaImplicitRejection() { // of how it is set. The call to set the value // will not affect what is used since a different context is // used in the call if the option is supported +#else + // TODO(jasnell): Boringssl appears not to support this operation. + // Is there an alternative approach that Boringssl does support? + return true; +#endif } bool EVPKeyCtxPointer::setRsaOaepLabel(DataPointer&& data) { @@ -3311,16 +3384,31 @@ EVPKeyPointer EVPKeyCtxPointer::paramgen() const { bool EVPKeyCtxPointer::publicCheck() const { if (!ctx_) return false; +#ifndef OPENSSL_IS_BORINGSSL + return EVP_PKEY_public_check(ctx_.get()) == 1; #if OPENSSL_VERSION_MAJOR >= 3 return EVP_PKEY_public_check_quick(ctx_.get()) == 1; #else return EVP_PKEY_public_check(ctx_.get()) == 1; #endif +#else // OPENSSL_IS_BORINGSSL + // Boringssl appears not to support this operation. + // TODO(jasnell): Is there an alternative approach that Boringssl does + // support? + return true; +#endif } bool EVPKeyCtxPointer::privateCheck() const { if (!ctx_) return false; +#ifndef OPENSSL_IS_BORINGSSL return EVP_PKEY_check(ctx_.get()) == 1; +#else + // Boringssl appears not to support this operation. + // TODO(jasnell): Is there an alternative approach that Boringssl does + // support? + return true; +#endif } bool EVPKeyCtxPointer::verify(const Buffer& sig, diff --git a/deps/ncrypto/ncrypto.h b/deps/ncrypto/ncrypto.h index 31d999d3e7e235..f73c84edce20bf 100644 --- a/deps/ncrypto/ncrypto.h +++ b/deps/ncrypto/ncrypto.h @@ -40,6 +40,14 @@ #define NCRYPTO_MUST_USE_RESULT #endif +#ifdef OPENSSL_IS_BORINGSSL +// Boringssl has opted to use size_t for some size related +// APIs while Openssl is still using ints +using OPENSSL_SIZE_T = size_t; +#else +using OPENSSL_SIZE_T = int; +#endif + namespace ncrypto { // ============================================================================ @@ -845,17 +853,25 @@ class DHPointer final { UNABLE_TO_CHECK_GENERATOR = DH_UNABLE_TO_CHECK_GENERATOR, NOT_SUITABLE_GENERATOR = DH_NOT_SUITABLE_GENERATOR, Q_NOT_PRIME = DH_CHECK_Q_NOT_PRIME, +#ifndef OPENSSL_IS_BORINGSSL + // Boringssl does not define the DH_CHECK_INVALID_[Q or J]_VALUE INVALID_Q = DH_CHECK_INVALID_Q_VALUE, INVALID_J = DH_CHECK_INVALID_J_VALUE, +#endif CHECK_FAILED = 512, }; CheckResult check(); enum class CheckPublicKeyResult { NONE, +#ifndef OPENSSL_IS_BORINGSSL + // Boringssl does not define DH_R_CHECK_PUBKEY_TOO_SMALL or TOO_LARGE TOO_SMALL = DH_R_CHECK_PUBKEY_TOO_SMALL, TOO_LARGE = DH_R_CHECK_PUBKEY_TOO_LARGE, INVALID = DH_R_CHECK_PUBKEY_INVALID, +#else + INVALID = DH_R_INVALID_PUBKEY, +#endif CHECK_FAILED = 512, }; // Check to see if the given public key is suitable for this DH instance.