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

Verification with dsaEncoding ieee-p1363 fails. #31866

Closed
wdenbakker opened this issue Feb 19, 2020 · 3 comments
Closed

Verification with dsaEncoding ieee-p1363 fails. #31866

wdenbakker opened this issue Feb 19, 2020 · 3 comments
Labels
confirmed-bug Issues with confirmed bugs. crypto Issues and PRs related to the crypto subsystem.

Comments

@wdenbakker
Copy link

When you verify a 'ieee-p1363' encoded signature it fails, even if the signature is correct.

  • Version: v13.8.0
  • Platform: 64-bit Windows 10
  • Subsystem: crypto

What steps will reproduce the bug?

const crypto = require('crypto');
const key = crypto.generateKeyPairSync('ec', { namedCurve: 'P-256' });

//ieee-p1363 signature, which seems to be the correct.
const signatureP1363 = crypto.createSign('SHA256').update('abc').sign({ key: key.privateKey, dsaEncoding: 'ieee-p1363' });
//ieee-p1363 verification, which fails.
console.log(crypto.createVerify('SHA256').update('abc').verify({ key: key.publicKey, dsaEncoding: 'ieee-p1363' }, signatureP1363));

//Compared to der signature and verification, which work as expected:
const signatureDER = crypto.createSign('SHA256').update('abc').sign({ key: key.privateKey, dsaEncoding: 'der' });
console.log(crypto.createVerify('SHA256').update('abc').verify({ key: key.publicKey, dsaEncoding: 'der' }, signatureDER));

Additional information

The problem seems to be the verification algorithm, not the signing algorithm. You can test that the generated signature is correct in chrome using:

//key.publicKey.export({ format: 'der', type: 'spki' }).toString('base64');
const base64key = 'MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAE/V0xKgeZJeIFra+gshXB6OpM5IKuhHwcBkpu5ZdMZZM62x+GahHJdrll+Q3aihYNfakkzf7W65dIdDAhLImu0w==';
//signatureP1363.toString('base64');
const base64sig = '+aocUpmRHRSxfpCJpwCCuQoFagatOlsFganmXiqtztFo9iBHqE6z7A7KQcMs1k9VASt3cgtkJqyPKAY4OTyJ8A==';
//Verify the signature
const uint8key = Uint8Array.from(atob(base64key), (c) => c.charCodeAt(0));
const uint8sig = Uint8Array.from(atob(base64sig), (c) => c.charCodeAt(0));
const uint8data = Uint8Array.from('abc', (c) => c.charCodeAt(0));
const params = { name: 'ECDSA', hash: 'SHA-256', namedCurve: 'P-256' };
const cryptokey = await window.crypto.subtle.importKey('spki', uint8key, params, false, ['verify']);
await window.crypto.subtle.verify(params, cryptokey, uint8sig, uint8data);
@targos
Copy link
Member

targos commented Feb 19, 2020

@nodejs/crypto

@tniessen
Copy link
Member

tniessen commented Feb 19, 2020

Interesting.

EVP_PKEY_verify returns -1, which indicates that something other than the verification fails.

d2i_ECDSA_SIG returns NULL, indicating that the DER is invalid.

Seems that createVerify always fails, but verify always succeeds.

tniessen added a commit to tniessen/node that referenced this issue Feb 20, 2020
@tniessen tniessen added confirmed-bug Issues with confirmed bugs. crypto Issues and PRs related to the crypto subsystem. labels Feb 20, 2020
@tniessen
Copy link
Member

Turns out I am just stupid and it's an easy fix: #31876

As a workaround, you can use crypto.verify instead, which handles ieee-p1363 correctly.

codebytere pushed a commit that referenced this issue Feb 27, 2020
Fixes: #31866

PR-URL: #31876
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
codebytere pushed a commit that referenced this issue Mar 15, 2020
Fixes: #31866

PR-URL: #31876
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
codebytere pushed a commit that referenced this issue Mar 17, 2020
Fixes: #31866

PR-URL: #31876
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
codebytere pushed a commit that referenced this issue Mar 30, 2020
Fixes: #31866

PR-URL: #31876
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
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
confirmed-bug Issues with confirmed bugs. crypto Issues and PRs related to the crypto subsystem.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants