Randomise v4 and v5 signatures via custom notation, add config.nonDeterministicSignaturesViaNotation to disable feature (#1737)

EdDSA is known to be vulnerable to fault attacks which can lead to secret key
extraction if two signatures over the same data can be collected. Randomly
occurring bitflips in specific parts of the computation might in principle
result in vulnerable faulty signatures being generated.
To protect signatures generated using v4 and v5 keys from this possibility, we
randomise each signature by adding a custom notation with a random value,
functioning as a salt. 
For simplicity, we add the salt to all algos, not just EdDSA, 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, as they are non-deterministic by
design.

While this notation solution is interoperable, it will reveal that the
signature has been generated using OpenPGP.js, which may not be desirable in
some cases.
For this reason, the option `config.nonDeterministicSignaturesViaNotation`
(defaulting to true) has been added to turn off the feature.

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).

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;
 }
 

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);
 

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);

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() {

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