diff --git a/src/config/config.js b/src/config/config.js index d5c9c14c..866a0c34 100644 --- a/src/config/config.js +++ b/src/config/config.js @@ -250,6 +250,14 @@ export default { * @property {Array} knownNotations */ knownNotations: [], + /** + * If true, a salt notation is used to randomize 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. It is added to signatures of any algo for simplicity, and as it may also serve as protection in case of + * weaknesses in the hash algo, potentially hindering e.g. some chosen-prefix attacks. + * 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. + */ + nonDeterministicSignaturesViaNotation: 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 d479b081..9425ebf8 100644 --- a/src/key/helper.js +++ b/src/key/helper.js @@ -218,8 +218,8 @@ export async function createSignaturePacket(dataToSign, privateKey, signingKeyPa Object.assign(signaturePacket, signatureProperties); signaturePacket.publicKeyAlgorithm = signingKeyPacket.algorithm; signaturePacket.hashAlgorithm = await getPreferredHashAlgo(privateKey, signingKeyPacket, date, userID, config); - signaturePacket.rawNotations = notations; - await signaturePacket.sign(signingKeyPacket, dataToSign, date, detached); + signaturePacket.rawNotations = [...notations]; + await signaturePacket.sign(signingKeyPacket, dataToSign, date, detached, config); return signaturePacket; } diff --git a/src/packet/signature.js b/src/packet/signature.js index b650ca99..43fc2002 100644 --- a/src/packet/signature.js +++ b/src/packet/signature.js @@ -26,6 +26,14 @@ 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 signatures. +// This is to protect EdDSA signatures in particular, which are known to be vulnerable to fault attacks +// leading to secret key extraction if two signatures over the same data can be collected (see https://github.com/jedisct1/libsodium/issues/170). +// For simplicity, we add the salt to all algos, as it may also serve as protection in case of weaknesses in the hash algo, potentially hindering e.g. +// some chosen-prefix attacks. +// 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'; + // 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 +203,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 +213,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.nonDeterministicSignaturesViaNotation) { + 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 +248,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..20bd0c30 100644 --- a/test/general/signature.js +++ b/test/general/signature.js @@ -1409,7 +1409,7 @@ DAAKCRDyMVUMT0fjjlnQAQDFHUs6TIcxrNTtEZFjUFm1M0PJ1Dng/cDW4xN80fsn }); expect(await sig.verified).to.be.true; const { packets: [{ rawNotations: notations }] } = await sig.signature; - expect(notations).to.have.length(2); + expect(notations).to.have.length(3); expect(notations[0].name).to.equal('test@example.com'); expect(notations[0].value).to.deep.equal(new Uint8Array([116, 101, 115, 116])); expect(notations[0].humanReadable).to.be.true; @@ -1418,6 +1418,71 @@ DAAKCRDyMVUMT0fjjlnQAQDFHUs6TIcxrNTtEZFjUFm1M0PJ1Dng/cDW4xN80fsn expect(notations[1].value).to.deep.equal(new Uint8Array([0, 1, 2, 3])); expect(notations[1].humanReadable).to.be.false; expect(notations[1].critical).to.be.false; + expect(notations[2].name).to.equal('salt@notations.openpgpjs.org'); + expect(notations[2].humanReadable).to.be.false; + expect(notations[2].critical).to.be.false; + }); + + it('v4 signatures are randomized via salt notation (`config.nonDeterministicSignaturesViaNotation`)', 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: { nonDeterministicSignaturesViaNotation: false } + }); + const armoredDeterministicSignature2 = await openpgp.sign({ + message: await openpgp.createMessage({ text }), + signingKeys: v4SigningKey, + date, + detached: true, + config: { nonDeterministicSignaturesViaNotation: 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() { 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