Skip to content
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.

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

crypto.generateKeyPair generates error ERR_OSSL_CRYPTO_MALLOC_FAILURE in node 17.2.0 #41428

Closed
Raynos opened this issue Jan 7, 2022 · 9 comments · Fixed by #42319
Closed
Labels
crypto Issues and PRs related to the crypto subsystem. openssl Issues and PRs related to the OpenSSL dependency.

Comments

@Raynos
Copy link
Contributor

Raynos commented Jan 7, 2022

Version

17.2.0

Platform

Linux raynos-Precision-5530 5.4.0-91-generic #102-Ubuntu SMP Fri Nov 5 16:31:28 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux

Subsystem

crypto

What steps will reproduce the bug?

Run the following script in node 17.2.0 and node 16.

const crypto = require('crypto')
crypto.generateKeyPair('rsa', {
        modulusLength: 4096,
        publicKeyEncoding: {
          type: 'spki',
          format: 'pem'
        },
        privateKeyEncoding: {
          type: 'pkcs8',
          format: 'pem',
          cipher: 'aes-256-cbc',
          passphrase: ''
        }
      }, (err, publicKey, privateKey) => {
        console.log('err', err)

        // cb(null, { publicKey, privateKey })
      })

Node 16 has no error, node 17 has an err

The err is

[Error: error:078C0100:common libcrypto routines::malloc failure] {
  opensslErrorStack: [ 'error:078C0100:common libcrypto routines::malloc failure' ],
  library: 'common libcrypto routines',
  reason: 'malloc failure',
  code: 'ERR_OSSL_CRYPTO_MALLOC_FAILURE'
}

How often does it reproduce? Is there a required condition?

every time on linux and mac.

What is the expected behavior?

I expected node 17 to not have a regression

What do you see instead?

I see an error.

[Error: error:078C0100:common libcrypto routines::malloc failure] {
  opensslErrorStack: [ 'error:078C0100:common libcrypto routines::malloc failure' ],
  library: 'common libcrypto routines',
  reason: 'malloc failure',
  code: 'ERR_OSSL_CRYPTO_MALLOC_FAILURE'
}

Additional information

No response

@Mesteery Mesteery added crypto Issues and PRs related to the crypto subsystem. openssl Issues and PRs related to the OpenSSL dependency. labels Jan 7, 2022
@panva
Copy link
Member

panva commented Jan 7, 2022

Not sure if it's intended or not, but the error only occurs when the private key encryption passphrase is empty, when one is provided this works as expected, likewise when no encryption is requested it's fine.

cc @nodejs/crypto

@Raynos
Copy link
Contributor Author

Raynos commented Jan 7, 2022

I'm using this in my test suite, setting passphrase to a string like 'hello' does indeed allow me to run my test suite on both node 16 and node 17.

@RaisinTen
Copy link
Contributor

I think this is a bug in OpenSSL, so I sent a report with my findings: openssl/openssl#17506

@RaisinTen
Copy link
Contributor

Also, cc @tniessen since you had worked on #35914

@RaisinTen
Copy link
Contributor

My fix landed in the OpenSSL repo: openssl/openssl#17507! Will cherry-pick it to https://github.com/quictls/openssl tomorrow.

@mhdawson
Copy link
Member

@RaisinTen was this cherry picked over?

@RaisinTen
Copy link
Contributor

@mhdawson I had sent the cherry-pick PR to the quictls repo - quictls/openssl#75 but I'm yet to get a review from the maintainers.

@mhdawson
Copy link
Member

mhdawson commented Mar 1, 2022

Thanks for the update, was just wondering if it was progressing.,

RaisinTen added a commit to RaisinTen/node that referenced this issue Mar 13, 2022
openssl/openssl@59ccb72:

```
commit 59ccb72cd5cec3b4e312853621e12a68dacdbc7e
Author: Darshan Sen <[email protected]>
Date:   Fri Jan 14 16:22:41 2022 +0530

    Fix invalid malloc failures in PEM_write_bio_PKCS8PrivateKey()

    When `PEM_write_bio_PKCS8PrivateKey()` was passed an empty passphrase
    string, `OPENSSL_memdup()` was incorrectly getting used for 0 bytes size
    allocation, which resulted in malloc failures.

    Fixes: openssl/openssl#17506

    Signed-off-by: Darshan Sen <[email protected]>

    Reviewed-by: Bernd Edlinger <[email protected]>
    Reviewed-by: Paul Dale <[email protected]>
    Reviewed-by: Tomas Mraz <[email protected]>
    (Merged from openssl/openssl#17507)
```

openssl/openssl@1d28ada:

```
commit 1d28ada1c39997c10fe5392f4235bbd2bc44b40f
Author: Darshan Sen <[email protected]>
Date:   Sat Jan 22 17:56:05 2022 +0530

    Allow empty passphrase in PEM_write_bio_PKCS8PrivateKey_nid()

    Signed-off-by: Darshan Sen <[email protected]>

    Reviewed-by: Bernd Edlinger <[email protected]>
    Reviewed-by: Paul Dale <[email protected]>
    Reviewed-by: Tomas Mraz <[email protected]>
    (Merged from openssl/openssl#17507)
```

Refs: openssl/openssl#17507
Fixes: nodejs#41428
Signed-off-by: Darshan Sen <[email protected]>
RaisinTen added a commit to RaisinTen/node that referenced this issue Mar 13, 2022
@RaisinTen
Copy link
Contributor

RaisinTen commented Mar 13, 2022

Sent a cherry-pick pr to Node.js anyways because my change has already landed in the openssl repo - #42319

RaisinTen added a commit to RaisinTen/node that referenced this issue Mar 17, 2022
nodejs-github-bot pushed a commit that referenced this issue Mar 19, 2022
Refs: openssl/openssl#17507
Refs: #41428
Signed-off-by: Darshan Sen <[email protected]>

PR-URL: #42319
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
bengl pushed a commit that referenced this issue Mar 21, 2022
Refs: openssl/openssl#17507
Refs: #41428
Signed-off-by: Darshan Sen <[email protected]>

PR-URL: #42319
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
danielleadams pushed a commit to danielleadams/node that referenced this issue Apr 21, 2022
Refs: openssl/openssl#17507
Refs: nodejs#41428
Signed-off-by: Darshan Sen <[email protected]>

PR-URL: nodejs#42319
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
danielleadams pushed a commit that referenced this issue Apr 24, 2022
Refs: openssl/openssl#17507
Refs: #41428
Signed-off-by: Darshan Sen <[email protected]>

PR-URL: #42319
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
danielleadams pushed a commit that referenced this issue Apr 24, 2022
Refs: openssl/openssl#17507
Refs: #41428
Signed-off-by: Darshan Sen <[email protected]>

PR-URL: #42319
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
danielleadams pushed a commit that referenced this issue Apr 24, 2022
Refs: openssl/openssl#17507
Refs: #41428
Signed-off-by: Darshan Sen <[email protected]>

PR-URL: #42319
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
xtx1130 pushed a commit to xtx1130/node that referenced this issue Apr 25, 2022
Refs: openssl/openssl#17507
Refs: nodejs#41428
Signed-off-by: Darshan Sen <[email protected]>

PR-URL: nodejs#42319
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
crypto Issues and PRs related to the crypto subsystem. openssl Issues and PRs related to the OpenSSL dependency.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants