search

nodejs / node

Node.js JavaScript runtime ✨🐢🚀✨
https://nodejs.org
Other
108k stars 29.8k forks source link

The signature and verification of SM2 return incorrect results in crypto.createSign and crypto.createVerify #53761

Open xicilion opened 4 months ago

xicilion commented 4 months ago

Version

v22.0.0

Platform

Darwin air.lan 23.5.0 Darwin Kernel Version 23.5.0: Wed May  1 20:19:05 PDT 2024; root:xnu-10063.121.3~5/RELEASE_ARM64_T8112 arm64

Subsystem

crypto

What steps will reproduce the bug?

When using crypto.createSign to sign SM2, I got an incorrect signature. The same issue occurs with crypto.createVerify.

const crypto = require('crypto');

const data = "AABB";

var keys = {
    privateKey: crypto.createPrivateKey(`-----BEGIN PRIVATE KEY-----
MIGHAgEAMBMGByqGSM49AgEGCCqBHM9VAYItBG0wawIBAQQgbjCNHopgvyGVfLaP
PamI9E9lf6jXT+xm1Pns1t/xQTihRANCAATV+I7HUGF2gC+miVl3JfjpoZaU2hrZ
QqHwKUNtIDE/uxxWNLBbYKaiLOWrbYA8skrWQWl3RkbXW4ZI28afRw9g
-----END PRIVATE KEY-----
`),
    publicKey: crypto.createPublicKey(`-----BEGIN PUBLIC KEY-----
MFkwEwYHKoZIzj0CAQYIKoEcz1UBgi0DQgAE1fiOx1BhdoAvpolZdyX46aGWlNoa
2UKh8ClDbSAxP7scVjSwW2Cmoizlq22APLJK1kFpd0ZG11uGSNvGn0cPYA==
-----END PUBLIC KEY-----`)
};

var sig = crypto.createSign('sm3').update(data).sign(keys.privateKey);
// var sig = crypto.sign('sm3', data, keys.privateKey);
var ok = crypto.verify('sm3', data, keys.publicKey, sig);
console.log("verify: ", ok);

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

always.

What is the expected behavior? Why is that the expected behavior?

verify: true

What do you see instead?

verify: false

Additional information

No response

RedYetiDev commented 4 months ago

(+ repro-exists)

I've made a few changes to the code for better logging, and here is my output:

const crypto = require('crypto');

const data = "AABB";

// Define the private and public keys
const privateKeyPEM = `-----BEGIN PRIVATE KEY-----
MIGHAgEAMBMGByqGSM49AgEGCCqBHM9VAYItBG0wawIBAQQgbjCNHopgvyGVfLaP
PamI9E9lf6jXT+xm1Pns1t/xQTihRANCAATV+I7HUGF2gC+miVl3JfjpoZaU2hrZ
QqHwKUNtIDE/uxxWNLBbYKaiLOWrbYA8skrWQWl3RkbXW4ZI28afRw9g
-----END PRIVATE KEY-----
`;

const publicKeyPEM = `-----BEGIN PUBLIC KEY-----
MFkwEwYHKoZIzj0CAQYIKoEcz1UBgi0DQgAE1fiOx1BhdoAvpolZdyX46aGWlNoa
2UKh8ClDbSAxP7scVjSwW2Cmoizlq22APLJK1kFpd0ZG11uGSNvGn0cPYA==
-----END PUBLIC KEY-----
`;

// Create the private and public key objects
const privateKey = crypto.createPrivateKey(privateKeyPEM);
const publicKey = crypto.createPublicKey(publicKeyPEM);

// Log the private and public keys
console.log("Private Key:");
console.log(privateKey.export({ type: 'pkcs8', format: 'pem' }));

console.log("Public Key:");
console.log(publicKey.export({ type: 'spki', format: 'pem' }));

// Create the signature using the private key
const sign = crypto.createSign('sm3');
sign.update(data);
const signature = sign.sign(privateKey);

// Log the signature
console.log("Signature:");
console.log(signature.toString('hex'));

// Verify the signature using the public key via crypto.createVerify
const verify = crypto.createVerify('sm3');
verify.update(data);
const isVerified = verify.verify(publicKey, signature);

// Log the verification result
console.log("Signature verified (via crypto.createVerify):", isVerified);

// Verify the signature using the public key via crypto.verify
const verify2 = crypto.verify('sm3', data, publicKey, signature);
console.log("Signature verified (via crypto.verify):", verify2);
Private Key:
-----BEGIN PRIVATE KEY-----
MIGHAgEAMBMGByqGSM49AgEGCCqBHM9VAYItBG0wawIBAQQgbjCNHopgvyGVfLaP
PamI9E9lf6jXT+xm1Pns1t/xQTihRANCAATV+I7HUGF2gC+miVl3JfjpoZaU2hrZ
QqHwKUNtIDE/uxxWNLBbYKaiLOWrbYA8skrWQWl3RkbXW4ZI28afRw9g
-----END PRIVATE KEY-----

Public Key:
-----BEGIN PUBLIC KEY-----
MFkwEwYHKoZIzj0CAQYIKoEcz1UBgi0DQgAE1fiOx1BhdoAvpolZdyX46aGWlNoa
2UKh8ClDbSAxP7scVjSwW2Cmoizlq22APLJK1kFpd0ZG11uGSNvGn0cPYA==
-----END PUBLIC KEY-----

Signature:
3044022056cc3567aa4842c4b1c5f9de75059dbaf63efab5aa34097e5a8547419aa3175e02206deb865046f12b25f111922da6858c7e97e475b656604679bcd4a0b05d8c76f1
Signature verified (via crypto.createVerify): true
Signature verified (via crypto.verify): false

This seems to only affect crypto.verify, CC @nodejs/crypto

RedYetiDev commented 4 months ago

That being said, I believe the issue title is misleading. crypto.createVerify appears to be fine, it's crypto.verify that is affected, right?

tniessen commented 4 months ago

Related: https://github.com/nodejs/node/pull/37066

xicilion commented 4 months ago

My test results are as follows:

  1. The signature result of cresteSign can be verified using createVerify;
  2. The signature result of crypto.sign can be verified using crypto.verify;
  3. crypto.sign/crypto.verify and cresteSign/createVerify cannot verify each other;
  4. openssl command line can verify with crypto.sign/crypto.verify.

So the results of crypto.createSign and crypto.createVerify on SM2 are both incorrect.

RedYetiDev commented 4 months ago

crypto.sign/crypto.verify and cresteSign/createVerify cannot verify each other;

Can you provide example code with createVerify? I'm unable to reproduce that part.

xicilion commented 4 months ago 
const crypto = require('crypto');

const data = "AABB";

var keys = {
    privateKey: crypto.createPrivateKey(`-----BEGIN PRIVATE KEY-----
MIGHAgEAMBMGByqGSM49AgEGCCqBHM9VAYItBG0wawIBAQQgbjCNHopgvyGVfLaP
PamI9E9lf6jXT+xm1Pns1t/xQTihRANCAATV+I7HUGF2gC+miVl3JfjpoZaU2hrZ
QqHwKUNtIDE/uxxWNLBbYKaiLOWrbYA8skrWQWl3RkbXW4ZI28afRw9g
-----END PRIVATE KEY-----
`),
    publicKey: crypto.createPublicKey(`-----BEGIN PUBLIC KEY-----
MFkwEwYHKoZIzj0CAQYIKoEcz1UBgi0DQgAE1fiOx1BhdoAvpolZdyX46aGWlNoa
2UKh8ClDbSAxP7scVjSwW2Cmoizlq22APLJK1kFpd0ZG11uGSNvGn0cPYA==
-----END PUBLIC KEY-----`)
};

// var sig = crypto.createSign('sm3').update(data).sign(keys.privateKey);
var sig = crypto.sign('sm3', data, keys.privateKey);
var ok = crypto.createVerify('sm3').update(data).verify(keys.publicKey, sig);
console.log("verify: ", ok);
RedYetiDev commented 4 months ago

Thanks! I understand now. My reproduction used createSign, which worked, this is weird indeed. @tniessen confirmed-bug?

RedYetiDev commented 4 months ago 
const crypto = require('crypto');

// Data to be signed and verified
const data = "AABB";

// Define the private and public keys
const privateKeyPEM = `
-----BEGIN PRIVATE KEY-----
MIGHAgEAMBMGByqGSM49AgEGCCqBHM9VAYItBG0wawIBAQQgbjCNHopgvyGVfLaP
PamI9E9lf6jXT+xm1Pns1t/xQTihRANCAATV+I7HUGF2gC+miVl3JfjpoZaU2hrZ
QqHwKUNtIDE/uxxWNLBbYKaiLOWrbYA8skrWQWl3RkbXW4ZI28afRw9g
-----END PRIVATE KEY-----
`;

const publicKeyPEM = `
-----BEGIN PUBLIC KEY-----
MFkwEwYHKoZIzj0CAQYIKoEcz1UBgi0DQgAE1fiOx1BhdoAvpolZdyX46aGWlNoa
2UKh8ClDbSAxP7scVjSwW2Cmoizlq22APLJK1kFpd0ZG11uGSNvGn0cPYA==
-----END PUBLIC KEY-----
`;

// Function to create private key object
function createPrivateKey(privateKeyPEM) {
    return crypto.createPrivateKey(privateKeyPEM);
}

// Function to create public key object
function createPublicKey(publicKeyPEM) {
    return crypto.createPublicKey(publicKeyPEM);
}

// Array of signer functions
const SIGNERS = [
    // Via crypto.createSign
    (data, privateKey) => {
        const sign = crypto.createSign('sm3');
        sign.update(data);
        return sign.sign(privateKey);
    },
    // Via crypto.sign
    (data, privateKey) => {
        return crypto.sign('sm3', Buffer.from(data), { key: privateKey });
    }
];

// Array of verifier functions
const VERIFIERS = [
    // Via crypto.createVerify
    (data, signature, publicKey) => {
        const verify = crypto.createVerify('sm3');
        verify.update(data);
        return verify.verify(publicKey, signature);
    },
    // Via crypto.verify
    (data, signature, publicKey) => {
        return crypto.verify('sm3', Buffer.from(data), publicKey, signature);
    }
];

// Function to sign data using all signers and verify using all verifiers
function signAndVerifyAll(data, privateKey, publicKey) {
    const results = SIGNERS.flatMap(signer => {
        const signature = signer(data, privateKey);
        return VERIFIERS.map(verifier => {
            return {
                "Sign Method": SIGNERS.indexOf(signer) === 0 ? 'crypto.createSign' : 'crypto.sign',
                "Verify Method": VERIFIERS.indexOf(verifier) === 0 ? 'crypto.createVerify' : 'crypto.verify',
                "Signature": signature.toString('hex'),
                "Result": verifier(data, signature, publicKey)
            };
        });
    });
    return results;
}

// Create private and public key objects
const privateKey = createPrivateKey(privateKeyPEM);
const publicKey = createPublicKey(publicKeyPEM);

// Sign and verify data using all methods
const results = signAndVerifyAll(data, privateKey, publicKey);

// Output results in a table
console.table(results);
┌─────────┬─────────────────────┬───────────────────────┬──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┬────────┐
│ (index) │ Sign Method         │ Verify Method         │ Signature                                                                                                                                        │ Result │
├─────────┼─────────────────────┼───────────────────────┼──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┼────────┤
│ 0       │ 'crypto.createSign' │ 'crypto.createVerify' │ '3045022046fa9c7b15e1ff55aa7f5dfcc2092614e25b2b321c82e0e1c09b03d21bd48fc9022100e45cddee885a1c075691f00b577c617ff584a89c65a19896600d5569d752d9f4' │ true   │
│ 1       │ 'crypto.createSign' │ 'crypto.verify'       │ '3045022046fa9c7b15e1ff55aa7f5dfcc2092614e25b2b321c82e0e1c09b03d21bd48fc9022100e45cddee885a1c075691f00b577c617ff584a89c65a19896600d5569d752d9f4' │ false  │
│ 2       │ 'crypto.sign'       │ 'crypto.createVerify' │ '3045022100cf800e13a0ead14be0ec9d614257fb6f409187642404dccb3a4374ace9fc162602206bf6820b8902ae9ce4cd10708f188fddd5bc2a41db2c47c003ca8265a00396ae' │ false  │
│ 3       │ 'crypto.sign'       │ 'crypto.verify'       │ '3045022100cf800e13a0ead14be0ec9d614257fb6f409187642404dccb3a4374ace9fc162602206bf6820b8902ae9ce4cd10708f188fddd5bc2a41db2c47c003ca8265a00396ae' │ true   │
└─────────┴─────────────────────┴───────────────────────┴──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┴────────┘

I've generated the table above to demonstrate which methods are incompatible.

RedYetiDev commented 4 months ago

openssl command line can verify with crypto.sign/crypto.verify.

So crypto.createXXX seems to have the wrong algorithm, IIUC

xicilion commented 4 months ago

crypto.sign and crypto.verify are correct.

RedYetiDev commented 4 months ago

Thanks for the excellent info! When the @nodejs/crypto team gets a chance to look at this issue, they will be able to help even more!