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

Case-sensitive algorithm argument in crypto.createVerify() #5031

Closed
jhermsmeier opened this issue Feb 1, 2016 · 5 comments
Closed

Case-sensitive algorithm argument in crypto.createVerify() #5031

jhermsmeier opened this issue Feb 1, 2016 · 5 comments
Labels
crypto Issues and PRs related to the crypto subsystem. doc Issues and PRs related to the documentations. good first issue Issues that are suitable for first-time contributors.

Comments

@jhermsmeier
Copy link

Contrary to what can be seen & read in the documentation,

const verify = crypto.createVerify('rsa-sha256');

crypto.createVerify() throws if the algorithm argument is lowercase:

crypto.js:295
  this._handle.init(algorithm);
               ^

Error: Unknown message digest
    at Error (native)
    at new Verify (crypto.js:295:16)
    at Object.Verify (crypto.js:292:12)

It works perfectly fine if the algorithm is uppercased. Is this intended behavior (and the documentation just needs updating) or is this indeed a minor bug?

@r-52 r-52 added the crypto Issues and PRs related to the crypto subsystem. label Feb 1, 2016
@bnoordhuis
Copy link
Member

Strictly speaking it's the documentation that is wrong. The official openssl name is either RSA-SHA256 or sha256WithRSAEncryption and is case-sensitive.

We could add a crypto.getHashes().filter(s => RegExp(name, 'i').test(s)) step for convenience but I'd worry it introduce problems down the line.

@jhermsmeier
Copy link
Author

Ah, interesting. But wouldn't just uppercasing the passed in algorithm argument fix the issue without causing problems anywhere else?

@bnoordhuis
Copy link
Member

Unfortunately not. If you uppercase e.g. sha256WithRSAEncryption, openssl won't recognize it.

@Fishrock123 Fishrock123 added doc Issues and PRs related to the documentations. good first issue Issues that are suitable for first-time contributors. labels Feb 2, 2016
@raineroviir
Copy link
Contributor

I've created a pull request which corrects this issue in the documentation
#5044

@jhermsmeier
Copy link
Author

Cool, thanks!

rvagg pushed a commit that referenced this issue Feb 8, 2016
Fixes: #5031
PR-URL: #5044
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this issue Feb 22, 2016
Fixes: #5031
PR-URL: #5044
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this issue Feb 22, 2016
Fixes: #5031
PR-URL: #5044
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this issue Mar 2, 2016
Fixes: #5031
PR-URL: #5044
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
scovetta pushed a commit to scovetta/node that referenced this issue Apr 2, 2016
Fixes: nodejs#5031
PR-URL: nodejs#5044
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
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. doc Issues and PRs related to the documentations. good first issue Issues that are suitable for first-time contributors.
Projects
None yet
Development

No branches or pull requests

5 participants