src: update ECDASSigPointer implementation in ncrypto

PR-URL: #56526 Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
nodejs · Jan 14, 2025 · 6e23885 · 6e23885
1 parent 6879ab9
commit 6e23885
Show file tree

Hide file tree

Showing 4 changed files with 110 additions and 19 deletions.
diff --git 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<const unsigned char>& 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<unsigned char> ECDSASigPointer::encode() const {
+  if (!sig_)
+    return {
+        .data = nullptr,
+        .len = 0,
+    };
+  Buffer<unsigned char> 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
@@ -197,7 +197,6 @@ using DeleteFnPtr = typename FunctionDeleter<T, function>::Pointer;
 
 using BignumCtxPointer = DeleteFnPtr<BN_CTX, BN_CTX_free>;
 using BignumGenCallbackPointer = DeleteFnPtr<BN_GENCB, BN_GENCB_free>;
-using ECDSASigPointer = DeleteFnPtr<ECDSA_SIG, ECDSA_SIG_free>;
 using ECGroupPointer = DeleteFnPtr<EC_GROUP, EC_GROUP_free>;
 using ECKeyPointer = DeleteFnPtr<EC_KEY, EC_KEY_free>;
 using ECPointPointer = DeleteFnPtr<EC_POINT, EC_POINT_free>;
@@ -821,6 +820,38 @@ class X509Pointer final {
   DeleteFnPtr<X509, X509_free> 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<const unsigned char>& buffer);
+
+  inline const BIGNUM* r() const { return pr_; }
+  inline const BIGNUM* s() const { return ps_; }
+
+  bool setParams(BignumPointer&& r, BignumPointer&& s);
+
+  Buffer<unsigned char> encode() const;
+
+ private:
+  DeleteFnPtr<ECDSA_SIG, ECDSA_SIG_free> 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
@@ -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<const unsigned char> 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
@@ -1,9 +1,9 @@
+#include <ncrypto.h>
 #include "crypto/crypto_bio.h"
 #include "gtest/gtest.h"
 #include "node_options.h"
 #include "node_test_fixture.h"
 #include "openssl/err.h"
-#include <ncrypto.h>
 
 using v8::Local;
 using v8::String;