crypto: fix memory leak, reduce heap allocations, clean up by bnoordhuis · Pull Request #14122 · nodejs/node

The cipher kind doesn't change over the lifetime of the cipher so make
it const.

PR-URL: #14122
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>

Don't allocate + copy + free; allocate and fill in place, then hand off
the pointer to Buffer::New().

Avoids unnecessary heap allocations in the following methods:

- crypto.CryptoStream#getSession()
- tls.TLSSocket#getSession()

PR-URL: #14122
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>

Add a test that ensures the second call to .digest() returns an empty
HMAC, like it did before.  No comment on whether that is the right
behavior or not.

PR-URL: #14122
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>

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.

PR-URL: #14122
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>

PR-URL: #14122
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>

PR-URL: #14122
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>

PR-URL: #14122
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>

PR-URL: #14122
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>

This also renames a misnamed variable `error_` to `success_`.

PR-URL: #14122
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>

PR-URL: #14122
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>

PR-URL: #14122
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>

Fix a memory leak in dh.setPublicKey() and dh.setPrivateKey() where the
old keys weren't freed.

Fixes: #8377
PR-URL: #14122
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>

Don't allocate + copy + free; allocate and fill in place, then hand off
the pointer to Buffer::New().

Avoids unnecessary heap allocations in the following methods:

- crypto.Cipher#final()
- crypto.Cipher#update()
- crypto.Cipheriv#final()
- crypto.Cipheriv#update()
- crypto.Decipher#final()
- crypto.Decipher#update()
- crypto.Decipheriv#final()
- crypto.Decipheriv#update()
- crypto.privateDecrypt()
- crypto.privateEncrypt()
- crypto.publicDecrypt()
- crypto.publicEncrypt()

PR-URL: #14122
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>

PR-URL: #14122
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>

Put the 8 kB initial buffer on the stack first and don't copy it to the
heap until its exact size is known (which is normally much smaller.)

PR-URL: #14122
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>

Fix a memory leak by removing the heap allocation altogether.

Fixes: #13917
PR-URL: #14122
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>

The EVP_CIPHER can be reconstructed from the EVP_CIPHER_CTX instance,
no need to store it separately.

This brought to light the somewhat dubious practice of accessing the
EVP_CIPHER after the EVP_CIPHER_CTX instance had been destroyed.

It's mostly harmless due to the static nature of built-in EVP_CIPHER
instances but it segfaults when the cipher is provided by an ENGINE
and the ENGINE is unloaded because its reference count drops to zero.

PR-URL: #14122
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>

PR-URL: #14122
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>

The cipher kind doesn't change over the lifetime of the cipher so make
it const.

PR-URL: #14122
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>

Don't allocate + copy + free; allocate and fill in place, then hand off
the pointer to Buffer::New().

Avoids unnecessary heap allocations in the following methods:

- crypto.CryptoStream#getSession()
- tls.TLSSocket#getSession()

PR-URL: #14122
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>

Add a test that ensures the second call to .digest() returns an empty
HMAC, like it did before.  No comment on whether that is the right
behavior or not.

PR-URL: #14122
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>

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.

PR-URL: #14122
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>