Skip to content

Commit

Permalink
crypto: reduce memory usage of SignFinal
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
tniessen committed Oct 12, 2018
1 parent bcbb937 commit 4b4b71b
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 24 deletions.
42 changes: 20 additions & 22 deletions src/node_crypto.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3517,36 +3517,38 @@ void Sign::SignUpdate(const FunctionCallbackInfo<Value>& args) {
sign->CheckThrow(err);
}

static int Node_SignFinal(EVPMDPointer&& mdctx, unsigned char* md,
unsigned int* sig_len,
const EVPKeyPointer& pkey, int padding,
int pss_salt_len) {
static MallocedBuffer<unsigned char> Node_SignFinal(EVPMDPointer&& mdctx,
const EVPKeyPointer& pkey,
int padding,
int pss_salt_len) {
unsigned char m[EVP_MAX_MD_SIZE];
unsigned int m_len;

*sig_len = 0;
if (!EVP_DigestFinal_ex(mdctx.get(), m, &m_len))
return 0;
return MallocedBuffer<unsigned char>();

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<unsigned char> 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;
return 1;
EVP_PKEY_sign(pkctx.get(), sig.data, &sig_len, m, m_len) > 0) {
return MallocedBuffer<unsigned char>(sig.release(), sig_len);
}
return 0;

return MallocedBuffer<unsigned char>();
}

SignBase::Error Sign::SignFinal(const char* key_pem,
int key_pem_len,
const char* passphrase,
unsigned char* sig,
unsigned int* sig_len,
MallocedBuffer<unsigned char>* buffer,
int padding,
int salt_len) {
if (!mdctx_)
Expand Down Expand Up @@ -3591,10 +3593,8 @@ 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))
return kSignOk;
else
return kSignPrivateKey;
*buffer = Node_SignFinal(std::move(mdctx), pkey, padding, salt_len);
return buffer->is_empty() ? kSignPrivateKey : kSignOk;
}


Expand All @@ -3618,22 +3618,20 @@ void Sign::SignFinal(const FunctionCallbackInfo<Value>& args) {
int salt_len = args[3].As<Int32>()->Value();

ClearErrorOnReturn clear_error_on_return;
unsigned char md_value[8192];
unsigned int md_len = sizeof(md_value);
MallocedBuffer<unsigned char> sig;

Error err = sign->SignFinal(
buf,
buf_len,
len >= 2 && !args[1]->IsNull() ? *passphrase : nullptr,
md_value,
&md_len,
&sig,
padding,
salt_len);
if (err != kSignOk)
return sign->CheckThrow(err);

Local<Object> rc =
Buffer::Copy(env, reinterpret_cast<char*>(md_value), md_len)
Buffer::New(env, reinterpret_cast<char*>(sig.data), sig.size)
.ToLocalChecked();
args.GetReturnValue().Set(rc);
}
Expand Down
3 changes: 1 addition & 2 deletions src/node_crypto.h
Original file line number Diff line number Diff line change
Expand Up @@ -521,8 +521,7 @@ class Sign : public SignBase {
Error SignFinal(const char* key_pem,
int key_pem_len,
const char* passphrase,
unsigned char* sig,
unsigned int* sig_len,
MallocedBuffer<unsigned char>* sig,
int padding,
int saltlen);

Expand Down

0 comments on commit 4b4b71b

Please sign in to comment.