Skip to content

Conversation

@panva
Copy link
Member

fixes#37794

@nodejs-github-botnodejs-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
Copy link
MemberAuthor

panva commented Mar 18, 2021

Before

constcrypto=require('crypto');constassert=require('assert');constdata=Buffer.from('hello');const{ privateKey }=crypto.generateKeyPairSync('ed25519');constsignature=crypto.sign(null,data,privateKey);assert(crypto.verify(null,data,privateKey,signature));// OKcrypto.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

constcrypto=require('crypto');constassert=require('assert');constdata=Buffer.from('hello');const{ privateKey }=crypto.generateKeyPairSync('ed25519');constsignature=crypto.sign(null,data,privateKey);assert(crypto.verify(null,data,privateKey,signature));// OKcrypto.verify(null,data,privateKey,signature,(err,verified)=>{assert(!err);assert(verified);// OK});

@nodejs-github-bot
Copy link
Collaborator

@panvapanva requested a review from jasnellMarch 18, 2021 11:37
@panva
Copy link
MemberAuthor

cc @nodejs/crypto

@panvapanva requested a review from tniessenMarch 18, 2021 12:14
@panva
Copy link
MemberAuthor

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

@jasnell
Copy link
Member

Yeah i think so

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

👍 to fast-track

@nodejs-github-bot
Copy link
Collaborator

@panva
Copy link
MemberAuthor

Landed in 5e6386e

@panvapanva closed this Mar 18, 2021
panva added a commit that referenced this pull request Mar 18, 2021
@panva
Copy link
MemberAuthor

panva commented Mar 18, 2021

@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.

@panvapanva deleted the cb-verify-fix branch March 18, 2021 19:14
ruyadorno pushed a commit that referenced this pull request Mar 20, 2021
@ruyadornoruyadorno mentioned this pull request Mar 30, 2021
Sign up for freeto 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++.cryptoIssues and PRs related to the crypto subsystem.fast-trackPRs that do not need to wait for 48 hours to land.needs-ciPRs 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

@panva@nodejs-github-bot@jasnell@cjihrig@RaisinTen@targos