diff --git a/src/config/config.js b/src/config/config.js index b006a3f4..b142d342 100644 --- a/src/config/config.js +++ b/src/config/config.js @@ -255,6 +255,13 @@ export default { * @property {Array} knownNotations */ knownNotations: [], + /** + * If true, a salt notation is used to randomize EdDSA signatures generated by v4 and v5 keys (v6 signatures are always non-deterministic, by design). + * This protects EdDSA signatures from potentially leaking the secret key in case of faults (i.e. bitflips) which, in principle, could occur + * during the signing computation. + * NOTE: the notation is interoperable, but will reveal that the signature has been generated using OpenPGP.js, which may not be desirable in some cases. + */ + nonDeterministicEdDSASignaturesViaNotation: true, /** * Whether to use the the noble-curves library for curves (other than Curve25519) that are not supported by the available native crypto API. * When false, certain standard curves will not be supported (depending on the platform). diff --git a/src/key/helper.js b/src/key/helper.js index 9ca90bf4..97a68c26 100644 --- a/src/key/helper.js +++ b/src/key/helper.js @@ -219,7 +219,7 @@ export async function createSignaturePacket(dataToSign, privateKey, signingKeyPa signaturePacket.publicKeyAlgorithm = signingKeyPacket.algorithm; signaturePacket.hashAlgorithm = await getPreferredHashAlgo(privateKey, signingKeyPacket, date, userID, config); signaturePacket.rawNotations = notations; - await signaturePacket.sign(signingKeyPacket, dataToSign, date, detached); + await signaturePacket.sign(signingKeyPacket, dataToSign, date, detached, config); return signaturePacket; } diff --git a/src/packet/signature.js b/src/packet/signature.js index b650ca99..674f3ad8 100644 --- a/src/packet/signature.js +++ b/src/packet/signature.js @@ -26,6 +26,12 @@ import defaultConfig from '../config'; // Symbol to store cryptographic validity of the signature, to avoid recomputing multiple times on verification. const verified = Symbol('verified'); +// A salt notation is used to randomize EdDSA signatures, as they are known to be vulnerable to fault attacks +// which can lead to secret key extraction if two signatures over the same data can be collected (see https://github.com/jedisct1/libsodium/issues/170). +// v6 signatures do not need to rely on this notation, as they already include a separate, built-in salt. +const SALT_NOTATION_NAME = 'salt@notations.openpgpjs.org'; +const publicKeyAlgorithmsToAlwaysSalt = new Set([enums.publicKey.eddsaLegacy, enums.publicKey.ed25519, enums.publicKey.ed448]); + // GPG puts the Issuer and Signature subpackets in the unhashed area. // Tampering with those invalidates the signature, so we still trust them and parse them. // All other unhashed subpackets are ignored. @@ -195,7 +201,7 @@ class SignaturePacket { * @throws {Error} if signing failed * @async */ - async sign(key, data, date = new Date(), detached = false) { + async sign(key, data, date = new Date(), detached = false, config) { this.version = key.version; this.created = util.normalizeDate(date); @@ -205,6 +211,31 @@ class SignaturePacket { const arr = [new Uint8Array([this.version, this.signatureType, this.publicKeyAlgorithm, this.hashAlgorithm])]; + // add randomness to the signature + if (this.version === 6) { + const saltLength = saltLengthForHash(this.hashAlgorithm); + if (this.salt === null) { + this.salt = crypto.random.getRandomBytes(saltLength); + } else if (saltLength !== this.salt.length) { + throw new Error('Provided salt does not have the required length'); + } + } else if (config.nonDeterministicEdDSASignaturesViaNotation && publicKeyAlgorithmsToAlwaysSalt.has(this.publicKeyAlgorithm)) { + const saltNotations = this.rawNotations.filter(({ name }) => (name === SALT_NOTATION_NAME)); + // since re-signing the same object is not supported, it's not expected to have multiple salt notations, + // but we guard against it as a sanity check + if (saltNotations.length === 0) { + const saltValue = crypto.random.getRandomBytes(saltLengthForHash(this.hashAlgorithm)); + this.rawNotations.push({ + name: SALT_NOTATION_NAME, + value: saltValue, + humanReadable: false, + critical: false + }); + } else { + throw new Error('Unexpected existing salt notation'); + } + } + // Add hashed subpackets arr.push(this.writeHashedSubPackets()); @@ -215,14 +246,6 @@ class SignaturePacket { this.signatureData = util.concat(arr); - if (this.version === 6) { - const saltLength = saltLengthForHash(this.hashAlgorithm); - if (this.salt === null) { - this.salt = crypto.random.getRandomBytes(saltLength); - } else if (saltLength !== this.salt.length) { - throw new Error('Provided salt does not have the required length'); - } - } const toHash = this.toHash(this.signatureType, data, detached); const hash = await this.hash(this.signatureType, data, toHash, detached); diff --git a/test/general/packet.js b/test/general/packet.js index b40e2ba5..ad34ca31 100644 --- a/test/general/packet.js +++ b/test/general/packet.js @@ -1432,7 +1432,7 @@ V+HOQJQxXJkVRYa3QrFUehiMzTeqqMdgC6ZqJy7+ signature.publicKeyAlgorithm = openpgp.enums.publicKey.rsaSign; signature.signatureType = openpgp.enums.signature.text; - return signature.sign(key, literal).then(async () => { + return signature.sign(key, literal, undefined, undefined, openpgp.config).then(async () => { signed.push(literal); signed.push(signature); diff --git a/test/general/signature.js b/test/general/signature.js index 86b01476..ccc0b64e 100644 --- a/test/general/signature.js +++ b/test/general/signature.js @@ -1420,6 +1420,68 @@ DAAKCRDyMVUMT0fjjlnQAQDFHUs6TIcxrNTtEZFjUFm1M0PJ1Dng/cDW4xN80fsn expect(notations[1].critical).to.be.false; }); + it('EdDSA v4 signatures are randomized via salt notation (`config.nonDeterministicEdDSASignaturesViaNotation`)', async function() { + const v4SigningKey = await openpgp.readKey({ + armoredKey: `-----BEGIN PGP PRIVATE KEY BLOCK----- + +xVgEX8+jfBYJKwYBBAHaRw8BAQdA9GbdDjprR0sWf0R5a5IpulUauc0FsmzJ +mOYCfoowt8EAAP9UwaqC0LWWQ5RlX7mps3728vFa/If1KBVwAjk7Uqhi2BKL +zQ90ZXN0MiA8YkBhLmNvbT7CjAQQFgoAHQUCX8+jfAQLCQcIAxUICgQWAgEA +AhkBAhsDAh4BACEJEG464aV2od77FiEEIcg441MtKnyJnPDRbjrhpXah3vuR +gQD+Il6Gw2oIok4/ANyDDLBYZtKqRrMv4NcfF9DHYuAFcP4BAPhFOffyP3qU +AEZb7QPrWdLfhn8/FeSFZxJvnmupQ9sDx10EX8+jfBIKKwYBBAGXVQEFAQEH +QOSzo9cX1U2esGFClprOt0QWXNJ97228R5tKFxo6/0NoAwEIBwAA/0n4sq2i +N6/jE+6rVO4o/7LW0xahxpV1tTA6qv1Op9TwFIDCeAQYFggACQUCX8+jfAIb +DAAhCRBuOuGldqHe+xYhBCHIOONTLSp8iZzw0W464aV2od773XcA/jlmX8/c +1/zIotEkyMZB4mI+GAg3FQ6bIACFBH1sz0MzAP9Snri0P4FRZ8D5THRCJoUm +GBgpBmrf6IVv484jBswGDA== +=8rBO +-----END PGP PRIVATE KEY BLOCK-----` + }); + + const date = new Date('Tue, 25 Dec 2023 00:00:00 GMT'); + const text = 'test'; + const armoredRandomSignature1 = await openpgp.sign({ + message: await openpgp.createMessage({ text }), + signingKeys: v4SigningKey, + date, + detached: true + }); + const armoredRandomSignature2 = await openpgp.sign({ + message: await openpgp.createMessage({ text }), + signingKeys: v4SigningKey, + date, + detached: true + }); + const randomSignature1 = await openpgp.readSignature({ armoredSignature: armoredRandomSignature1 }); + const randomSignature2 = await openpgp.readSignature({ armoredSignature: armoredRandomSignature2 }); + expect(randomSignature1.packets[0].signedHashValue).to.not.deep.equal(randomSignature2.packets[0].signedHashValue); + + // ensure the signatures are verifiable, as a sanity check + const verification1 = await openpgp.verify({ message: await openpgp.createMessage({ text }), signature: randomSignature1, verificationKeys: v4SigningKey, expectSigned: true }); + expect(verification1.data).to.equal(text); + + const armoredDeterministicSignature1 = await openpgp.sign({ + message: await openpgp.createMessage({ text }), + signingKeys: v4SigningKey, + date, + detached: true, + config: { nonDeterministicEdDSASignaturesViaNotation: false } + }); + const armoredDeterministicSignature2 = await openpgp.sign({ + message: await openpgp.createMessage({ text }), + signingKeys: v4SigningKey, + date, + detached: true, + config: { nonDeterministicEdDSASignaturesViaNotation: false } + }); + const deterministicSignature1 = await openpgp.readSignature({ armoredSignature: armoredDeterministicSignature1 }); + const deterministicSignature2 = await openpgp.readSignature({ armoredSignature: armoredDeterministicSignature2 }); + expect(deterministicSignature1.packets[0].signedHashValue).to.deep.equal(deterministicSignature2.packets[0].signedHashValue); + const verification2 = await openpgp.verify({ message: await openpgp.createMessage({ text }), signature: deterministicSignature1, verificationKeys: v4SigningKey, expectSigned: true }); + expect(verification2.data).to.equal(text); + }); + it('Verify v6 cleartext signed message with openpgp.verify', async function() { // test vector from https://www.ietf.org/archive/id/draft-ietf-openpgp-crypto-refresh-08.html#name-sample-v6-certificate-trans const cleartextMessage = `-----BEGIN PGP SIGNED MESSAGE----- diff --git a/test/security/subkey_trust.js b/test/security/subkey_trust.js index e8aee45b..5d7f5fbe 100644 --- a/test/security/subkey_trust.js +++ b/test/security/subkey_trust.js @@ -50,7 +50,7 @@ export default () => it('Does not trust subkeys without Primary Key Binding Sign fakeBindingSignature.publicKeyAlgorithm = attackerPrivKey.keyPacket.algorithm; fakeBindingSignature.hashAlgorithm = enums.hash.sha256; fakeBindingSignature.keyFlags = [enums.keyFlags.signData]; - await fakeBindingSignature.sign(attackerPrivKey.keyPacket, dataToSign); + await fakeBindingSignature.sign(attackerPrivKey.keyPacket, dataToSign, undefined, undefined, openpgp.config); const newList = new PacketList(); newList.push( pktPubAttacker[0], // attacker private key