crypto: reduce memory usage of SignFinal

The fixed-size buffer on the stack is unnecessary and way too large for most applications. This change removes it and allocates the required memory directly instead of copying into heap later. PR-URL: #23427 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Refael Ackermann <[email protected]>

Author: tniessen

src/node_crypto.cc

@@ -43,6 +43,7 @@
 
 #include<algorithm>
 #include<memory>
+#include<utility>
 #include<vector>
 
 staticconstint X509_NAME_FLAGS = ASN1_STRFLGS_ESC_CTRL
@@ -3530,46 +3531,51 @@ void Sign::SignUpdate(const FunctionCallbackInfo<Value>& args){
  sign->CheckThrow(err);
 }
 
-staticintNode_SignFinal(EVPMDPointer&& mdctx, unsignedchar* md,
-unsignedint* sig_len,
-const EVPKeyPointer& pkey,int padding,
-int pss_salt_len){
+staticMallocedBuffer<unsignedchar> Node_SignFinal(EVPMDPointer&& mdctx,
+const EVPKeyPointer& pkey,
+int padding,
+int pss_salt_len){
 unsignedchar m[EVP_MAX_MD_SIZE];
 unsignedint m_len;
 
- *sig_len = 0;
 if (!EVP_DigestFinal_ex(mdctx.get(), m, &m_len))
-return0;
+return MallocedBuffer<unsignedchar>();
+
+int signed_sig_len = EVP_PKEY_size(pkey.get());
+CHECK_GE(signed_sig_len, 0);
+size_t sig_len = static_cast<size_t>(signed_sig_len);
+ MallocedBuffer<unsignedchar> sig(sig_len);
 
-size_t sltmp = static_cast<size_t>(EVP_PKEY_size(pkey.get()));
  EVPKeyCtxPointer pkctx(EVP_PKEY_CTX_new(pkey.get(), nullptr));
 if (pkctx &&
 EVP_PKEY_sign_init(pkctx.get()) > 0 &&
 ApplyRSAOptions(pkey, pkctx.get(), padding, pss_salt_len) &&
 EVP_PKEY_CTX_set_signature_md(pkctx.get(),
 EVP_MD_CTX_md(mdctx.get())) > 0 &&
-EVP_PKEY_sign(pkctx.get(), md, &sltmp, m, m_len) > 0){
-*sig_len = sltmp;
-return1;
+EVP_PKEY_sign(pkctx.get(), sig.data, &sig_len, m, m_len) > 0){
+sig.Truncate(sig_len);
+returnsig;
  }
-return0;
+
+return MallocedBuffer<unsignedchar>();
 }
 
-SignBase::Error Sign::SignFinal(constchar* key_pem,
-int key_pem_len,
-constchar* passphrase,
-unsignedchar* sig,
-unsignedint* sig_len,
-int padding,
-int salt_len){
+std::pair<SignBase::Error, MallocedBuffer<unsignedchar>> Sign::SignFinal(
+constchar* key_pem,
+int key_pem_len,
+constchar* passphrase,
+int padding,
+int salt_len){
+ MallocedBuffer<unsignedchar> buffer;
+
 if (!mdctx_)
-returnkSignNotInitialised;
+returnstd::make_pair(kSignNotInitialised, std::move(buffer));
 
  EVPMDPointer mdctx = std::move(mdctx_);
 
  BIOPointer bp(BIO_new_mem_buf(const_cast<char*>(key_pem), key_pem_len));
 if (!bp)
-returnkSignPrivateKey;
+returnstd::make_pair(kSignPrivateKey, std::move(buffer));
 
  EVPKeyPointer pkey(PEM_read_bio_PrivateKey(bp.get(),
 nullptr,
@@ -3580,7 +3586,7 @@ SignBase::Error Sign::SignFinal(const char* key_pem,
 // without `pkey` being set to nullptr;
 // cf. the test of `test_bad_rsa_privkey.pem` for an example.
 if (!pkey || 0 != ERR_peek_error())
-returnkSignPrivateKey;
+returnstd::make_pair(kSignPrivateKey, std::move(buffer));
 
 #ifdef NODE_FIPS_MODE
 /* Validate DSA2 parameters from FIPS 186-4 */
@@ -3604,10 +3610,9 @@ SignBase::Error Sign::SignFinal(const char* key_pem,
  }
 #endif// NODE_FIPS_MODE
 
-if (Node_SignFinal(std::move(mdctx), sig, sig_len, pkey, padding, salt_len))
-returnkSignOk;
-else
-returnkSignPrivateKey;
+ buffer = Node_SignFinal(std::move(mdctx), pkey, padding, salt_len);
+ Error error = buffer.is_empty() ? kSignPrivateKey : kSignOk;
+returnstd::make_pair(error, std::move(buffer));
 }
 
 
@@ -3631,22 +3636,22 @@ void Sign::SignFinal(const FunctionCallbackInfo<Value>& args){
 int salt_len = args[3].As<Int32>()->Value();
 
  ClearErrorOnReturn clear_error_on_return;
-unsignedchar md_value[8192];
-unsignedint md_len = sizeof(md_value);
 
- Error err = sign->SignFinal(
+std::pair<Error, MallocedBuffer<unsignedchar>> ret = sign->SignFinal(
  buf,
  buf_len,
  len >= 2 && !args[1]->IsNull() ? *passphrase : nullptr,
- md_value,
- &md_len,
  padding,
  salt_len);
-if (err != kSignOk)
-return sign->CheckThrow(err);
+
+if (std::get<Error>(ret) != kSignOk)
+return sign->CheckThrow(std::get<Error>(ret));
+
+ MallocedBuffer<unsignedchar> sig =
+std::move(std::get<MallocedBuffer<unsignedchar>>(ret));
 
  Local<Object> rc =
-Buffer::Copy(env, reinterpret_cast<char*>(md_value), md_len)
+Buffer::New(env, reinterpret_cast<char*>(sig.release()), sig.size)
  .ToLocalChecked();
  args.GetReturnValue().Set(rc);
 }

src/node_crypto.h

@@ -518,13 +518,12 @@ class Sign : public SignBase{
 public:
 staticvoidInitialize(Environment* env, v8::Local<v8::Object> target);
 
- Error SignFinal(constchar* key_pem,
-int key_pem_len,
-constchar* passphrase,
-unsignedchar* sig,
-unsignedint* sig_len,
-int padding,
-int saltlen);
+ std::pair<Error, MallocedBuffer<unsignedchar>> SignFinal(
+constchar* key_pem,
+int key_pem_len,
+constchar* passphrase,
+int padding,
+int saltlen);
 
 protected:
 staticvoidNew(const v8::FunctionCallbackInfo<v8::Value>& args);

src/util.h

@@ -439,9 +439,14 @@ struct MallocedBuffer{
 return ret;
  }
 
+voidTruncate(size_t new_size){
+CHECK(new_size <= size);
+ size = new_size;
+ }
+
 inlineboolis_empty() const{return data == nullptr}
 
-MallocedBuffer() : data(nullptr){}
+MallocedBuffer() : data(nullptr), size(0){}
 explicitMallocedBuffer(size_t size) : data(Malloc<T>(size)), size(size){}
 MallocedBuffer(T* data, size_t size) : data(data), size(size){}
 MallocedBuffer(MallocedBuffer&& other) : data(other.data), size(other.size){