Skip to content
New issue

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

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

Already on GitHub? # to your account

crypto.getCiphers() includes unsupported ciphers #41857

Closed
mscdex opened this issue Feb 5, 2022 · 5 comments
Closed

crypto.getCiphers() includes unsupported ciphers #41857

mscdex opened this issue Feb 5, 2022 · 5 comments
Labels
crypto Issues and PRs related to the crypto subsystem.

Comments

@mscdex
Copy link
Contributor

mscdex commented Feb 5, 2022

Version

v17.x, master

Platform

n/a

Subsystem

crypto

What steps will reproduce the bug?

$ node -e 'console.log(crypto.getCiphers().includes("rc4")); crypto.createCipheriv("rc4", Buffer.alloc(16), Buffer.alloc(0))'

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

Yes.

What is the expected behavior?

false
node:internal/crypto/cipher:116
    this[kHandle].initiv(cipher, credential, iv, authTagLength);
                  ^

Error: error:0308010C:digital envelope routines::unsupported
    at Cipheriv.createCipherBase (node:internal/crypto/cipher:116:19)
    at Cipheriv.createCipherWithIV (node:internal/crypto/cipher:135:3)
    at new Cipheriv (node:internal/crypto/cipher:243:3)
    at Object.createCipheriv (node:crypto:138:10)
    at [eval]:1:58
    at Script.runInThisContext (node:vm:129:12)
    at Object.runInThisContext (node:vm:305:38)
    at node:internal/process/execution:75:19
    at [eval]-wrapper:6:22
    at evalScript (node:internal/process/execution:74:60) {
  library: 'digital envelope routines',
  reason: 'unsupported',
  code: 'ERR_OSSL_EVP_UNSUPPORTED'
}

What do you see instead?

true
node:internal/crypto/cipher:116
    this[kHandle].initiv(cipher, credential, iv, authTagLength);
                  ^

Error: error:0308010C:digital envelope routines::unsupported
    at Cipheriv.createCipherBase (node:internal/crypto/cipher:116:19)
    at Cipheriv.createCipherWithIV (node:internal/crypto/cipher:135:3)
    at new Cipheriv (node:internal/crypto/cipher:243:3)
    at Object.createCipheriv (node:crypto:138:10)
    at [eval]:1:58
    at Script.runInThisContext (node:vm:129:12)
    at Object.runInThisContext (node:vm:305:38)
    at node:internal/process/execution:75:19
    at [eval]-wrapper:6:22
    at evalScript (node:internal/process/execution:74:60) {
  library: 'digital envelope routines',
  reason: 'unsupported',
  code: 'ERR_OSSL_EVP_UNSUPPORTED'
}

Additional information

Node.js shouldn't fib about its supported ciphers.

Whatever change is made for crypto.getCiphers() should probably also be made for the other crypto.get*() methods for consistency.

@Mesteery Mesteery added the crypto Issues and PRs related to the crypto subsystem. label Feb 5, 2022
@mscdex
Copy link
Contributor Author

mscdex commented Feb 5, 2022

FWIW I think calling the various _fetch() (and _free()) OpenSSL APIs for each algorithm name seems to do the trick, however just making that change causes existing tests (e.g. test/parallel/test-crypto-authenticated.js and a couple others) to fail (for supported algorithms like "aes-128-gcm") for unknown reasons with "unsupported" errors from OpenSSL.

@Trott
Copy link
Member

Trott commented Feb 5, 2022

@nodejs/crypto

@mscdex
Copy link
Contributor Author

mscdex commented Feb 5, 2022

I figured out what was happening, I have a potential solution incoming....

mscdex added a commit to mscdex/io.js that referenced this issue Feb 5, 2022
mscdex added a commit to mscdex/io.js that referenced this issue Feb 5, 2022
mscdex added a commit to mscdex/io.js that referenced this issue Feb 5, 2022
mscdex added a commit to mscdex/io.js that referenced this issue Feb 5, 2022
mscdex added a commit to mscdex/io.js that referenced this issue Feb 6, 2022
mscdex added a commit to mscdex/io.js that referenced this issue Feb 6, 2022
@panva panva closed this as completed in cde35ea Feb 8, 2022
bengl pushed a commit to bengl/node that referenced this issue Feb 21, 2022
Fixes: nodejs#41857

PR-URL: nodejs#41864
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
bengl pushed a commit to bengl/node that referenced this issue Feb 21, 2022
Fixes: nodejs#41857

PR-URL: nodejs#41864
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
danielleadams pushed a commit to danielleadams/node that referenced this issue Apr 21, 2022
Fixes: nodejs#41857

PR-URL: nodejs#41864
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
@noscripter
Copy link

noscripter commented May 6, 2022

I'm now using v18.1.0 and still have the same issue.

			return new BulkUpdateDecorator(require("crypto").createHash(algorithm));

image

Use v10.24.1 is OK.

@mscdex
Copy link
Contributor Author

mscdex commented May 6, 2022

@noscripter You'll have to provide more details. I cannot duplicate the issue of crypto.getCiphers()/crypto.getHashes() returning unsupported ciphers/hashes in node v18.x.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
crypto Issues and PRs related to the crypto subsystem.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants