From c6daa090b3708633dea0f4585801d275e1d39b65 Mon Sep 17 00:00:00 2001 From: larabr <7375870+larabr@users.noreply.github.com> Date: Thu, 28 Mar 2024 15:35:20 +0100 Subject: [PATCH] Add salt notation to signatures of any algo --- src/config/config.js | 7 ++++--- src/packet/signature.js | 7 ++++--- test/general/signature.js | 11 +++++++---- 3 files changed, 15 insertions(+), 10 deletions(-) diff --git a/src/config/config.js b/src/config/config.js index b142d342..2d12dcc7 100644 --- a/src/config/config.js +++ b/src/config/config.js @@ -256,12 +256,13 @@ export default { */ 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). + * 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. + * 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. */ - nonDeterministicEdDSASignaturesViaNotation: true, + 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/packet/signature.js b/src/packet/signature.js index 674f3ad8..940a49ee 100644 --- a/src/packet/signature.js +++ b/src/packet/signature.js @@ -26,11 +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 +// A salt notation is used to randomize signatures; EdDSA signatures in particular 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). +// 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'; -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. @@ -219,7 +220,7 @@ class SignaturePacket { } 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)) { + } 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 diff --git a/test/general/signature.js b/test/general/signature.js index ccc0b64e..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,9 +1418,12 @@ 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('EdDSA v4 signatures are randomized via salt notation (`config.nonDeterministicEdDSASignaturesViaNotation`)', async function() { + it('v4 signatures are randomized via salt notation (`config.nonDeterministicSignaturesViaNotation`)', async function() { const v4SigningKey = await openpgp.readKey({ armoredKey: `-----BEGIN PGP PRIVATE KEY BLOCK----- @@ -1466,14 +1469,14 @@ GBgpBmrf6IVv484jBswGDA== signingKeys: v4SigningKey, date, detached: true, - config: { nonDeterministicEdDSASignaturesViaNotation: false } + config: { nonDeterministicSignaturesViaNotation: false } }); const armoredDeterministicSignature2 = await openpgp.sign({ message: await openpgp.createMessage({ text }), signingKeys: v4SigningKey, date, detached: true, - config: { nonDeterministicEdDSASignaturesViaNotation: false } + config: { nonDeterministicSignaturesViaNotation: false } }); const deterministicSignature1 = await openpgp.readSignature({ armoredSignature: armoredDeterministicSignature1 }); const deterministicSignature2 = await openpgp.readSignature({ armoredSignature: armoredDeterministicSignature2 });