Skip to content

crypto: fix memory leak, reduce heap allocations, clean up #14122

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
bnoordhuis wants to merge 18 commits into from
Closed

crypto: fix memory leak, reduce heap allocations, clean up #14122

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
a9d1431
src: remove extra heap allocations in CipherBase
bnoordhuis Jul 10, 2017
df1e3e8
src: remove unneeded const_cast
bnoordhuis Jul 10, 2017
87fc516
src: avoid heap allocation in sign.final()
bnoordhuis Jul 10, 2017
647370a
src: don't heap allocate GCM cipher auth tag
bnoordhuis Jul 10, 2017
f46e980
src: remove superfluous cipher_ data member
bnoordhuis Jul 10, 2017
5ae2cb8
src: remove unused Local
bnoordhuis Jul 10, 2017
dd6f4a9
src: make CipherBase::kind_ const
bnoordhuis Jul 10, 2017
d383042
src: remove extra heap allocation in GetSession()
bnoordhuis Jul 10, 2017
496ef4b
src: avoid heap allocation in hmac.digest()
bnoordhuis Jul 10, 2017
40807c1
src: remove extra heap allocations in DH functions
bnoordhuis Jul 10, 2017
5fd09df
src: refactor PBKDF2Request
bnoordhuis Jul 10, 2017
160b60f
src: make array arg length compile-time checkable
bnoordhuis Jul 10, 2017
3818792
src: avoid heap allocation in crypto.pbkdf2()
bnoordhuis Jul 10, 2017
0ce90a9
src: remove PBKDF2Request::release()
bnoordhuis Jul 10, 2017
1cfd02e
src: simplify PBKDF2Request
bnoordhuis Jul 10, 2017
b8e3a3e
src: guard against double free in randomBytes()
bnoordhuis Jul 10, 2017
083ccd2
src: reduce allocations in exportPublicKey()
bnoordhuis Jul 10, 2017
e4ac1b6
src: fix memory leak in DH key setters
bnoordhuis Jul 10, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
src: remove extra heap allocations in DH functions 
Replace allocate + Encode() + free patterns by calls to Malloc +
the Buffer::New() overload that takes ownership of the pointer.
Avoids unnecessary heap allocations and copying around of data.

DRY the accessor functions for the prime, generator, public key and
private key properties; deletes about 40 lines of quadruplicated code.
  • Loading branch information
@bnoordhuis
bnoordhuis committed Jul 17, 2017
commit 40807c1de24dd79b3fbd7ab7d5cb5cc3a0090a5d
106 changes: 29 additions & 77 deletions src/node_crypto.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4762,99 +4762,49 @@ void DiffieHellman::GenerateKeys(const FunctionCallbackInfo<Value>& args) {
return ThrowCryptoError(env, ERR_get_error(), "Key generation failed");
}

int dataSize = BN_num_bytes(diffieHellman->dh->pub_key);
char* data = new char[dataSize];
BN_bn2bin(diffieHellman->dh->pub_key,
reinterpret_cast<unsigned char*>(data));

args.GetReturnValue().Set(Encode(env->isolate(), data, dataSize, BUFFER));
delete[] data;
size_t size = BN_num_bytes(diffieHellman->dh->pub_key);
char* data = Malloc(size);
BN_bn2bin(diffieHellman->dh->pub_key, reinterpret_cast<unsigned char*>(data));
args.GetReturnValue().Set(Buffer::New(env, data, size).ToLocalChecked());
}


void DiffieHellman::GetPrime(const FunctionCallbackInfo<Value>& args) {
void DiffieHellman::GetField(const FunctionCallbackInfo<Value>& args,
BIGNUM* (DH::*field), const char* err_if_null) {
Environment* env = Environment::GetCurrent(args);

DiffieHellman* diffieHellman;
ASSIGN_OR_RETURN_UNWRAP(&diffieHellman, args.Holder());
DiffieHellman* dh;
ASSIGN_OR_RETURN_UNWRAP(&dh, args.Holder());
if (!dh->initialised_) return env->ThrowError("Not initialized");

if (!diffieHellman->initialised_) {
return ThrowCryptoError(env, ERR_get_error(), "Not initialized");
}
const BIGNUM* num = (dh->dh)->*field;
if (num == nullptr) return env->ThrowError(err_if_null);

int dataSize = BN_num_bytes(diffieHellman->dh->p);
char* data = new char[dataSize];
BN_bn2bin(diffieHellman->dh->p, reinterpret_cast<unsigned char*>(data));
size_t size = BN_num_bytes(num);
char* data = Malloc(size);
BN_bn2bin(num, reinterpret_cast<unsigned char*>(data));
args.GetReturnValue().Set(Buffer::New(env, data, size).ToLocalChecked());
}

args.GetReturnValue().Set(Encode(env->isolate(), data, dataSize, BUFFER));
delete[] data;
void DiffieHellman::GetPrime(const FunctionCallbackInfo<Value>& args) {
GetField(args, &DH::p, "p is null");
}


void DiffieHellman::GetGenerator(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);

DiffieHellman* diffieHellman;
ASSIGN_OR_RETURN_UNWRAP(&diffieHellman, args.Holder());

if (!diffieHellman->initialised_) {
return ThrowCryptoError(env, ERR_get_error(), "Not initialized");
}

int dataSize = BN_num_bytes(diffieHellman->dh->g);
char* data = new char[dataSize];
BN_bn2bin(diffieHellman->dh->g, reinterpret_cast<unsigned char*>(data));

args.GetReturnValue().Set(Encode(env->isolate(), data, dataSize, BUFFER));
delete[] data;
GetField(args, &DH::g, "g is null");
}


void DiffieHellman::GetPublicKey(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);

DiffieHellman* diffieHellman;
ASSIGN_OR_RETURN_UNWRAP(&diffieHellman, args.Holder());

if (!diffieHellman->initialised_) {
return ThrowCryptoError(env, ERR_get_error(), "Not initialized");
}

if (diffieHellman->dh->pub_key == nullptr) {
return env->ThrowError("No public key - did you forget to generate one?");
}

int dataSize = BN_num_bytes(diffieHellman->dh->pub_key);
char* data = new char[dataSize];
BN_bn2bin(diffieHellman->dh->pub_key,
reinterpret_cast<unsigned char*>(data));

args.GetReturnValue().Set(Encode(env->isolate(), data, dataSize, BUFFER));
delete[] data;
GetField(args, &DH::pub_key,
"No public key - did you forget to generate one?");
}


void DiffieHellman::GetPrivateKey(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);

DiffieHellman* diffieHellman;
ASSIGN_OR_RETURN_UNWRAP(&diffieHellman, args.Holder());

if (!diffieHellman->initialised_) {
return ThrowCryptoError(env, ERR_get_error(), "Not initialized");
}

if (diffieHellman->dh->priv_key == nullptr) {
return env->ThrowError("No private key - did you forget to generate one?");
}

int dataSize = BN_num_bytes(diffieHellman->dh->priv_key);
char* data = new char[dataSize];
BN_bn2bin(diffieHellman->dh->priv_key,
reinterpret_cast<unsigned char*>(data));

args.GetReturnValue().Set(Encode(env->isolate(), data, dataSize, BUFFER));
delete[] data;
GetField(args, &DH::priv_key,
"No private key - did you forget to generate one?");
}


Expand Down Expand Up @@ -4882,7 +4832,7 @@ void DiffieHellman::ComputeSecret(const FunctionCallbackInfo<Value>& args) {
}

int dataSize = DH_size(diffieHellman->dh);
char* data = new char[dataSize];
char* data = Malloc(dataSize);

int size = DH_compute_key(reinterpret_cast<unsigned char*>(data),
key,
Expand All @@ -4894,7 +4844,7 @@ void DiffieHellman::ComputeSecret(const FunctionCallbackInfo<Value>& args) {

checked = DH_check_pub_key(diffieHellman->dh, key, &checkResult);
BN_free(key);
delete[] data;
free(data);

if (!checked) {
return ThrowCryptoError(env, ERR_get_error(), "Invalid Key");
Expand All @@ -4909,6 +4859,8 @@ void DiffieHellman::ComputeSecret(const FunctionCallbackInfo<Value>& args) {
} else {
return env->ThrowError("Invalid key");
}

UNREACHABLE();
}

BN_free(key);
Expand All @@ -4924,8 +4876,8 @@ void DiffieHellman::ComputeSecret(const FunctionCallbackInfo<Value>& args) {
memset(data, 0, dataSize - size);
}

args.GetReturnValue().Set(Encode(env->isolate(), data, dataSize, BUFFER));
delete[] data;
auto rc = Buffer::New(env->isolate(), data, dataSize).ToLocalChecked();
args.GetReturnValue().Set(rc);
}


Expand Down
2 changes: 2 additions & 0 deletions src/node_crypto.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -690,6 +690,8 @@ class DiffieHellman : public BaseObject {
}

private:
static void GetField(const v8::FunctionCallbackInfo<v8::Value>& args,
BIGNUM* (DH::*field), const char* err_if_null);
bool VerifyContext();

bool initialised_;
Expand Down