openpgpjs · larabr · Apr 2, 2024 · Mar 26, 2024 · Mar 28, 2024 · Apr 2, 2024
diff --git a/src/config/config.js b/src/config/config.js
@@ -255,6 +255,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
@@ -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
@@ -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 = '[email protected]';
+
 // 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
@@ -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
@@ -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('[email protected]');
     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('[email protected]');
+    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
@@ -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