FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

crypto: fix crypto.verify callback invocation with a private keyobject by panva · Pull Request #37795 · nodejs/node · GitHub

/ node Public

crypto: fix crypto.verify callback invocation with a private keyobject - #37795

Closed
panva wants to merge 1 commit into
nodejs:masterfrom
panva:cb-verify-fix
Closed

crypto: fix crypto.verify callback invocation with a private keyobject#37795
panva wants to merge 1 commit into
nodejs:masterfrom
panva:cb-verify-fix

Conversation

panva commented Mar 18, 2021

Copy link
Copy Markdown
Member

fixes #37794

nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. crypto Issues and PRs related to the crypto subsystem. needs-ci PRs that need a full CI run. labels Mar 18, 2021

panva commented Mar 18, 2021
edited
Loading

Copy link
Copy Markdown
Member Author

Before

const crypto = require('crypto');
const assert = require('assert');

const data = Buffer.from('hello');
const { privateKey } = crypto.generateKeyPairSync('ed25519');
const signature = crypto.sign(null, data, privateKey);

assert(crypto.verify(null, data, privateKey, signature)); // OK

crypto.verify(null, data, privateKey, signature, (err, verified) => { // 💥
  assert(!err);
  assert(verified);
});
// node[49326]: ../src/crypto/crypto_sig.cc:850:static bool node::crypto::SignTraits::DeriveBits(node::Environment *, const node::crypto::SignConfiguration &, node::crypto::ByteSource *): Assertion `(params.key->GetKeyType()) == (kKeyTypePublic)' failed.

After

const crypto = require('crypto');
const assert = require('assert');

const data = Buffer.from('hello');
const { privateKey } = crypto.generateKeyPairSync('ed25519');
const signature = crypto.sign(null, data, privateKey);

assert(crypto.verify(null, data, privateKey, signature)); // OK

crypto.verify(null, data, privateKey, signature, (err, verified) => {
  assert(!err);
  assert(verified); // OK
});

Copy link
Copy Markdown
Collaborator

panva requested a review from jasnell March 18, 2021 11:37

panva commented Mar 18, 2021

Copy link
Copy Markdown
Member Author

cc @nodejs/crypto

panva requested a review from tniessen March 18, 2021 12:14

panva commented Mar 18, 2021

Copy link
Copy Markdown
Member Author

@jasnell should we fast-track and do v15.12.1?

jasnell commented Mar 18, 2021

Copy link
Copy Markdown
Member

Yeah i think so

panva added the fast-track PRs that do not need to wait for 48 hours to land. label Mar 18, 2021

panva commented Mar 18, 2021

Copy link
Copy Markdown
Member Author

👍 to fast-track

Comment thread src/crypto/crypto_sig.cc Outdated

Copy link
Copy Markdown
Collaborator

panva commented Mar 18, 2021

Copy link
Copy Markdown
Member Author

Landed in 5e6386e

panva closed this Mar 18, 2021
panva added a commit that referenced this pull request Mar 18, 2021
fixes #37794

PR-URL: #37795
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

panva commented Mar 18, 2021
edited
Loading

Copy link
Copy Markdown
Member Author

@nodejs/releasers This fixes a possible crash introduced in a new optional argument added to a stable API with yesterday's 15.12.0 release (#37500). We're proposing a quick 15.12.1 patch release.

panva deleted the cb-verify-fix branch March 18, 2021 19:14
ruyadorno pushed a commit that referenced this pull request Mar 20, 2021
fixes #37794

PR-URL: #37795
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
ruyadorno mentioned this pull request Mar 30, 2021
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. crypto Issues and PRs related to the crypto subsystem. fast-track PRs that do not need to wait for 48 hours to land. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

crypto: verify with callback crashes when private key is used

6 participants


Back | FazBrowse Home | New Git URL