From 6e23885d18dfb20caee11e80fdb8a879cd9b818c Mon Sep 17 00:00:00 2001 From: James M Snell Date: Wed, 8 Jan 2025 08:37:45 -0800 Subject: [PATCH] src: update ECDASSigPointer implementation in ncrypto PR-URL: https://github.com/nodejs/node/pull/56526 Reviewed-By: Yagiz Nizipli Reviewed-By: Antoine du Hamel --- deps/ncrypto/ncrypto.cc | 64 +++++++++++++++++++++++++++++ deps/ncrypto/ncrypto.h | 33 ++++++++++++++- src/crypto/crypto_sig.cc | 30 ++++++-------- test/cctest/test_node_crypto_env.cc | 2 +- 4 files changed, 110 insertions(+), 19 deletions(-) diff --git a/deps/ncrypto/ncrypto.cc b/deps/ncrypto/ncrypto.cc index 02baca9d3be05d..728970d2aa03ca 100644 --- a/deps/ncrypto/ncrypto.cc +++ b/deps/ncrypto/ncrypto.cc @@ -2613,4 +2613,68 @@ bool CipherCtxPointer::getAeadTag(size_t len, unsigned char* out) { return EVP_CIPHER_CTX_ctrl(ctx_.get(), EVP_CTRL_AEAD_GET_TAG, len, out); } +// ============================================================================ + +ECDSASigPointer::ECDSASigPointer() : sig_(nullptr) {} +ECDSASigPointer::ECDSASigPointer(ECDSA_SIG* sig) : sig_(sig) { + if (sig_) { + ECDSA_SIG_get0(sig_.get(), &pr_, &ps_); + } +} +ECDSASigPointer::ECDSASigPointer(ECDSASigPointer&& other) noexcept + : sig_(other.release()) { + if (sig_) { + ECDSA_SIG_get0(sig_.get(), &pr_, &ps_); + } +} + +ECDSASigPointer& ECDSASigPointer::operator=(ECDSASigPointer&& other) noexcept { + sig_.reset(other.release()); + if (sig_) { + ECDSA_SIG_get0(sig_.get(), &pr_, &ps_); + } + return *this; +} + +ECDSASigPointer::~ECDSASigPointer() { + reset(); +} + +void ECDSASigPointer::reset(ECDSA_SIG* sig) { + sig_.reset(); + pr_ = nullptr; + ps_ = nullptr; +} + +ECDSA_SIG* ECDSASigPointer::release() { + pr_ = nullptr; + ps_ = nullptr; + return sig_.release(); +} + +ECDSASigPointer ECDSASigPointer::New() { + return ECDSASigPointer(ECDSA_SIG_new()); +} + +ECDSASigPointer ECDSASigPointer::Parse(const Buffer& sig) { + const unsigned char* ptr = sig.data; + return ECDSASigPointer(d2i_ECDSA_SIG(nullptr, &ptr, sig.len)); +} + +bool ECDSASigPointer::setParams(BignumPointer&& r, BignumPointer&& s) { + if (!sig_) return false; + return ECDSA_SIG_set0(sig_.get(), r.release(), s.release()); +} + +Buffer ECDSASigPointer::encode() const { + if (!sig_) + return { + .data = nullptr, + .len = 0, + }; + Buffer buf; + buf.len = i2d_ECDSA_SIG(sig_.get(), &buf.data); + return buf; +} + } // namespace ncrypto diff --git a/deps/ncrypto/ncrypto.h b/deps/ncrypto/ncrypto.h index 4a9f1878b5451f..8dc5e968521a42 100644 --- a/deps/ncrypto/ncrypto.h +++ b/deps/ncrypto/ncrypto.h @@ -197,7 +197,6 @@ using DeleteFnPtr = typename FunctionDeleter::Pointer; using BignumCtxPointer = DeleteFnPtr; using BignumGenCallbackPointer = DeleteFnPtr; -using ECDSASigPointer = DeleteFnPtr; using ECGroupPointer = DeleteFnPtr; using ECKeyPointer = DeleteFnPtr; using ECPointPointer = DeleteFnPtr; @@ -821,6 +820,38 @@ class X509Pointer final { DeleteFnPtr cert_; }; +class ECDSASigPointer final { + public: + explicit ECDSASigPointer(); + explicit ECDSASigPointer(ECDSA_SIG* sig); + ECDSASigPointer(ECDSASigPointer&& other) noexcept; + ECDSASigPointer& operator=(ECDSASigPointer&& other) noexcept; + NCRYPTO_DISALLOW_COPY(ECDSASigPointer) + ~ECDSASigPointer(); + + inline bool operator==(std::nullptr_t) noexcept { return sig_ == nullptr; } + inline operator bool() const { return sig_ != nullptr; } + inline ECDSA_SIG* get() const { return sig_.get(); } + inline operator ECDSA_SIG*() const { return sig_.get(); } + void reset(ECDSA_SIG* sig = nullptr); + ECDSA_SIG* release(); + + static ECDSASigPointer New(); + static ECDSASigPointer Parse(const Buffer& buffer); + + inline const BIGNUM* r() const { return pr_; } + inline const BIGNUM* s() const { return ps_; } + + bool setParams(BignumPointer&& r, BignumPointer&& s); + + Buffer encode() const; + + private: + DeleteFnPtr sig_; + const BIGNUM* pr_ = nullptr; + const BIGNUM* ps_ = nullptr; +}; + #ifndef OPENSSL_NO_ENGINE class EnginePointer final { public: diff --git a/src/crypto/crypto_sig.cc b/src/crypto/crypto_sig.cc index 0b73f0281217b8..9bfda29010c68d 100644 --- a/src/crypto/crypto_sig.cc +++ b/src/crypto/crypto_sig.cc @@ -150,16 +150,16 @@ bool ExtractP1363( unsigned char* out, size_t len, size_t n) { - ECDSASigPointer asn1_sig(d2i_ECDSA_SIG(nullptr, &sig_data, len)); + ncrypto::Buffer sig_buffer{ + .data = sig_data, + .len = len, + }; + auto asn1_sig = ECDSASigPointer::Parse(sig_buffer); if (!asn1_sig) return false; - const BIGNUM* pr; - const BIGNUM* ps; - ECDSA_SIG_get0(asn1_sig.get(), &pr, &ps); - - return BignumPointer::EncodePaddedInto(pr, out, n) > 0 && - BignumPointer::EncodePaddedInto(ps, out + n, n) > 0; + return BignumPointer::EncodePaddedInto(asn1_sig.r(), out, n) > 0 && + BignumPointer::EncodePaddedInto(asn1_sig.s(), out + n, n) > 0; } // Returns the maximum size of each of the integers (r, s) of the DSA signature. @@ -213,23 +213,19 @@ ByteSource ConvertSignatureToDER(const EVPKeyPointer& pkey, ByteSource&& out) { if (out.size() != 2 * n) return ByteSource(); - ECDSASigPointer asn1_sig(ECDSA_SIG_new()); + auto asn1_sig = ECDSASigPointer::New(); CHECK(asn1_sig); BignumPointer r(sig_data, n); CHECK(r); BignumPointer s(sig_data + n, n); CHECK(s); - CHECK_EQ(1, ECDSA_SIG_set0(asn1_sig.get(), r.release(), s.release())); - - unsigned char* data = nullptr; - int len = i2d_ECDSA_SIG(asn1_sig.get(), &data); - - if (len <= 0) - return ByteSource(); + CHECK(asn1_sig.setParams(std::move(r), std::move(s))); - CHECK_NOT_NULL(data); + auto buf = asn1_sig.encode(); + if (buf.len <= 0) return ByteSource(); - return ByteSource::Allocated(data, len); + CHECK_NOT_NULL(buf.data); + return ByteSource::Allocated(buf); } void CheckThrow(Environment* env, SignBase::Error error) { diff --git a/test/cctest/test_node_crypto_env.cc b/test/cctest/test_node_crypto_env.cc index 79c2e78a71cbb6..77aab8f4182d4f 100644 --- a/test/cctest/test_node_crypto_env.cc +++ b/test/cctest/test_node_crypto_env.cc @@ -1,9 +1,9 @@ +#include #include "crypto/crypto_bio.h" #include "gtest/gtest.h" #include "node_options.h" #include "node_test_fixture.h" #include "openssl/err.h" -#include using v8::Local; using v8::String;