From 9f3985d39859a99f18d45aa16b978e9fc05c22e1 Mon Sep 17 00:00:00 2001
From: Daniel Huigens <d.huigens@protonmail.com>
Date: Tue, 30 Aug 2022 16:36:17 +0200
Subject: [PATCH] Hash all signature subpackets (#1562)

Move the Issuer, Issuer Fingerprint, and Embedded Signature subpackets
to the hashed subpackets for new signatures. While we allow these to be
unhashed, it's safer to hash them, and this simplifies the code as well.
---
 src/packet/signature.js | 60 ++++++++++++-----------------------------
 1 file changed, 17 insertions(+), 43 deletions(-)

diff --git a/src/packet/signature.js b/src/packet/signature.js
index b28cd9a6..f07a7310 100644
--- a/src/packet/signature.js
+++ b/src/packet/signature.js
@@ -188,8 +188,10 @@ class SignaturePacket {
     // Add hashed subpackets
     arr.push(this.writeHashedSubPackets());
 
-    // Set unhashed subpackets for serialization
-    this.unhashedSubpackets = this.createUnhashedSubPackets();
+    // Remove unhashed subpackets, in case some allowed unhashed
+    // subpackets existed, in order not to duplicate them (in both
+    // the hashed and unhashed subpackets) when re-signing.
+    this.unhashedSubpackets = [];
 
     this.signatureData = util.concat(arr);
 
@@ -253,6 +255,11 @@ class SignaturePacket {
       bytes = util.concat([bytes, this.revocationKeyFingerprint]);
       arr.push(writeSubPacket(sub.revocationKey, bytes));
     }
+    if (!this.issuerKeyID.isNull() && this.issuerKeyVersion !== 5) {
+      // If the version of [the] key is greater than 4, this subpacket
+      // MUST NOT be included in the signature.
+      arr.push(writeSubPacket(sub.issuer, this.issuerKeyID.write()));
+    }
     this.rawNotations.forEach(([{ name, value, humanReadable }]) => {
       bytes = [new Uint8Array([humanReadable ? 0x80 : 0, 0, 0, 0])];
       // 2 octets of name length
@@ -306,6 +313,14 @@ class SignaturePacket {
       bytes = util.concat(bytes);
       arr.push(writeSubPacket(sub.signatureTarget, bytes));
     }
+    if (this.embeddedSignature !== null) {
+      arr.push(writeSubPacket(sub.embeddedSignature, this.embeddedSignature.write()));
+    }
+    if (this.issuerFingerprint !== null) {
+      bytes = [new Uint8Array([this.issuerKeyVersion]), this.issuerFingerprint];
+      bytes = util.concat(bytes);
+      arr.push(writeSubPacket(sub.issuerFingerprint, bytes));
+    }
     if (this.preferredAEADAlgorithms !== null) {
       bytes = util.stringToUint8Array(util.uint8ArrayToString(this.preferredAEADAlgorithms));
       arr.push(writeSubPacket(sub.preferredAEADAlgorithms, bytes));
@@ -317,31 +332,6 @@ class SignaturePacket {
     return util.concat([length, result]);
   }
 
-  /**
-   * Returns the Issuer, Issuer Fingperprint, and Embedded Signature subpacket bodies
-   * @returns {Array<Uint8Array>} Subpackets.
-   */
-  createUnhashedSubPackets() {
-    const sub = enums.signatureSubpacket;
-    const arr = [];
-    let bytes;
-    if (!this.issuerKeyID.isNull() && this.issuerKeyVersion !== 5) {
-      // If the version of [the] key is greater than 4, this subpacket
-      // MUST NOT be included in the signature.
-      arr.push(writeSubPacketBody(sub.issuer, this.issuerKeyID.write()));
-    }
-    if (this.embeddedSignature !== null) {
-      arr.push(writeSubPacketBody(sub.embeddedSignature, this.embeddedSignature.write()));
-    }
-    if (this.issuerFingerprint !== null) {
-      bytes = [new Uint8Array([this.issuerKeyVersion]), this.issuerFingerprint];
-      bytes = util.concat(bytes);
-      arr.push(writeSubPacketBody(sub.issuerFingerprint, bytes));
-    }
-
-    return arr;
-  }
-
   /**
    * Creates an Uint8Array containing the unhashed subpackets
    * @returns {Uint8Array} Subpacket data.
@@ -777,19 +767,3 @@ function writeSubPacket(type, data) {
   arr.push(data);
   return util.concat(arr);
 }
-
-/**
- * Creates a string representation of the body of a sub-packet (without length)
- * @see {@link https://tools.ietf.org/html/rfc4880#section-5.2.3.1|RFC4880 5.2.3.1}
- * @see {@link https://tools.ietf.org/html/rfc4880#section-5.2.3.2|RFC4880 5.2.3.2}
- * @param {Integer} type - Subpacket signature type.
- * @param {String} data - Data to be included
- * @returns {Uint8Array} A string-representation of a sub signature packet.
- * @private
- */
-function writeSubPacketBody(type, data) {
-  const arr = [];
-  arr.push(new Uint8Array([type]));
-  arr.push(data);
-  return util.concat(arr);
-}