Add back armor checksum for non-v6 artifacts (#1741)

We need to include the checksum to work around a GnuPG bug where data fails to
be decoded if the base64 ends with no padding chars (=) (see https://dev.gnupg.org/T7071).
Pure v6 artifacts are unaffected and won't include the checksum, as mandated by
the spec.

Breaking change:
`openpgp.armor` takes an additional `emitChecksum` argument (defaults to
false).
NB: some types of data must not include the checksum, but compliance is left as
responsibility of the caller: this function does not carry out any checks.
Refer to the crypto-refresh RFC for more details.

---------

Co-authored-by: Daniel Huigens <d.huigens@protonmail.com>

openpgp.d.ts

@@ -735,7 +735,7 @@ export interface VerifyMessageResult<T extends MaybeStream<Data> = MaybeStream<D
 /**
  * Armor an OpenPGP binary packet block
  */
-export function armor(messagetype: enums.armor, body: object, partindex?: number, parttotal?: number, customComment?: string, config?: Config): string;
+export function armor(messagetype: enums.armor, body: object, partindex?: number, parttotal?: number, customComment?: string, emitChecksum?: boolean, config?: Config): string;
 
 /**
  * DeArmor an OpenPGP armored message; verify the checksum and return the encoded bytes

src/cleartext.js

@@ -111,9 +111,9 @@ export class CleartextMessage {
    * @returns {String | ReadableStream<String>} ASCII armor.
    */
   armor(config = defaultConfig) {
-    // emit header if one of the signatures has a version not 6
-    const emitHeader = this.signature.packets.some(packet => packet.version !== 6);
-    const hash = emitHeader ?
+    // emit header and checksum if one of the signatures has a version not 6
+    const emitHeaderAndChecksum = this.signature.packets.some(packet => packet.version !== 6);
+    const hash = emitHeaderAndChecksum ?
       Array.from(new Set(this.signature.packets.map(
         packet => enums.read(enums.hash, packet.hashAlgorithm).toUpperCase()
       ))).join() :
@@ -124,7 +124,9 @@ export class CleartextMessage {
       text: this.text,
       data: this.signature.packets.write()
     };
-    return armor(enums.armor.signed, body, undefined, undefined, undefined, config);
+
+    // An ASCII-armored sequence of Signature packets that only includes v6 Signature packets MUST NOT contain a CRC24 footer.
+    return armor(enums.armor.signed, body, undefined, undefined, undefined, emitHeaderAndChecksum, config);
   }
 }
 

src/encoding/armor.js

@@ -334,29 +334,38 @@ export function unarmor(input) {
  * @param {Integer} [partIndex]
  * @param {Integer} [partTotal]
  * @param {String} [customComment] - Additional comment to add to the armored string
+ * @param {Boolean} [emitChecksum] - Whether to compute and include the CRC checksum
+ *  (NB: some types of data must not include it, but compliance is left as responsibility of the caller: this function does not carry out any checks)
+ * @param {Object} [config] - Full configuration, defaults to openpgp.config
  * @returns {String | ReadableStream<String>} Armored text.
  * @static
  */
-export function armor(messageType, body, partIndex, partTotal, customComment, config = defaultConfig) {
+export function armor(messageType, body, partIndex, partTotal, customComment, emitChecksum = false, config = defaultConfig) {
   let text;
   let hash;
   if (messageType === enums.armor.signed) {
     text = body.text;
     hash = body.hash;
     body = body.data;
   }
+  // unless explicitly forbidden by the spec, we need to include the checksum to work around a GnuPG bug
+  // where data fails to be decoded if the base64 ends with no padding chars (=) (see https://dev.gnupg.org/T7071)
+  const maybeBodyClone = emitChecksum && stream.passiveClone(body);
+
   const result = [];
   switch (messageType) {
     case enums.armor.multipartSection:
       result.push('-----BEGIN PGP MESSAGE, PART ' + partIndex + '/' + partTotal + '-----\n');
       result.push(addheader(customComment, config));
       result.push(base64.encode(body));
+      maybeBodyClone && result.push('=', getCheckSum(maybeBodyClone));
       result.push('-----END PGP MESSAGE, PART ' + partIndex + '/' + partTotal + '-----\n');
       break;
     case enums.armor.multipartLast:
       result.push('-----BEGIN PGP MESSAGE, PART ' + partIndex + '-----\n');
       result.push(addheader(customComment, config));
       result.push(base64.encode(body));
+      maybeBodyClone && result.push('=', getCheckSum(maybeBodyClone));
       result.push('-----END PGP MESSAGE, PART ' + partIndex + '-----\n');
       break;
     case enums.armor.signed:
@@ -366,38 +375,37 @@ export function armor(messageType, body, partIndex, partTotal, customComment, co
       result.push('\n-----BEGIN PGP SIGNATURE-----\n');
       result.push(addheader(customComment, config));
       result.push(base64.encode(body));
+      maybeBodyClone && result.push('=', getCheckSum(maybeBodyClone));
       result.push('-----END PGP SIGNATURE-----\n');
       break;
     case enums.armor.message:
       result.push('-----BEGIN PGP MESSAGE-----\n');
       result.push(addheader(customComment, config));
       result.push(base64.encode(body));
+      maybeBodyClone && result.push('=', getCheckSum(maybeBodyClone));
       result.push('-----END PGP MESSAGE-----\n');
       break;
     case enums.armor.publicKey:
       result.push('-----BEGIN PGP PUBLIC KEY BLOCK-----\n');
       result.push(addheader(customComment, config));
       result.push(base64.encode(body));
+      maybeBodyClone && result.push('=', getCheckSum(maybeBodyClone));
       result.push('-----END PGP PUBLIC KEY BLOCK-----\n');
       break;
     case enums.armor.privateKey:
       result.push('-----BEGIN PGP PRIVATE KEY BLOCK-----\n');
       result.push(addheader(customComment, config));
       result.push(base64.encode(body));
+      maybeBodyClone && result.push('=', getCheckSum(maybeBodyClone));
       result.push('-----END PGP PRIVATE KEY BLOCK-----\n');
       break;
-    case enums.armor.signature: {
-      const bodyClone = stream.passiveClone(body);
+    case enums.armor.signature:
       result.push('-----BEGIN PGP SIGNATURE-----\n');
       result.push(addheader(customComment, config));
       result.push(base64.encode(body));
-      // GPG v2 fails to parse signatures without checksums
-      result.push('=', getCheckSum(bodyClone));
+      maybeBodyClone && result.push('=', getCheckSum(maybeBodyClone));
       result.push('-----END PGP SIGNATURE-----\n');
       break;
-    }
-    default:
-      throw new Error('Unknown armor type');
   }
 
   return util.concat(result);

src/key/key.js

@@ -612,7 +612,9 @@ class Key {
     const revocationSignature = await helper.getLatestValidSignature(this.revocationSignatures, this.keyPacket, enums.signature.keyRevocation, dataToVerify, date, config);
     const packetlist = new PacketList();
     packetlist.push(revocationSignature);
-    return armor(enums.armor.publicKey, packetlist.write(), null, null, 'This is a revocation certificate');
+    // An ASCII-armored Transferable Public Key packet sequence of a v6 key MUST NOT contain a CRC24 footer.
+    const emitChecksum = this.keyPacket.version !== 6;
+    return armor(enums.armor.publicKey, packetlist.write(), null, null, 'This is a revocation certificate', emitChecksum);
   }
 
   /**

src/key/private_key.js

@@ -64,7 +64,9 @@ class PrivateKey extends PublicKey {
    * @returns {ReadableStream<String>} ASCII armor.
    */
   armor(config = defaultConfig) {
-    return armor(enums.armor.privateKey, this.toPacketList().write(), undefined, undefined, undefined, config);
+    // An ASCII-armored Transferable Public Key packet sequence of a v6 key MUST NOT contain a CRC24 footer.
+    const emitChecksum = this.keyPacket.version !== 6;
+    return armor(enums.armor.privateKey, this.toPacketList().write(), undefined, undefined, undefined, emitChecksum, config);
   }
 
   /**

src/key/public_key.js

@@ -61,7 +61,9 @@ class PublicKey extends Key {
    * @returns {ReadableStream<String>} ASCII armor.
    */
   armor(config = defaultConfig) {
-    return armor(enums.armor.publicKey, this.toPacketList().write(), undefined, undefined, undefined, config);
+    // An ASCII-armored Transferable Public Key packet sequence of a v6 key MUST NOT contain a CRC24 footer.
+    const emitChecksum = this.keyPacket.version !== 6;
+    return armor(enums.armor.publicKey, this.toPacketList().write(), undefined, undefined, undefined, emitChecksum, config);
   }
 }
 

src/message.js

@@ -680,7 +680,13 @@ export class Message {
    * @returns {ReadableStream<String>} ASCII armor.
    */
   armor(config = defaultConfig) {
-    return armor(enums.armor.message, this.write(), null, null, null, config);
+    const trailingPacket = this.packets[this.packets.length - 1];
+    // An ASCII-armored Encrypted Message packet sequence that ends in an v2 SEIPD packet MUST NOT contain a CRC24 footer.
+    // An ASCII-armored sequence of Signature packets that only includes v6 Signature packets MUST NOT contain a CRC24 footer.
+    const emitChecksum = trailingPacket.constructor.tag === SymEncryptedIntegrityProtectedDataPacket.tag ?
+      trailingPacket.version !== 2 :
+      this.packets.some(packet => packet.constructor.tag === SignaturePacket.tag && packet.version !== 6);
+    return armor(enums.armor.message, this.write(), null, null, null, emitChecksum, config);
   }
 }
 

src/signature.js

@@ -49,7 +49,9 @@ export class Signature {
    * @returns {ReadableStream<String>} ASCII armor.
    */
   armor(config = defaultConfig) {
-    return armor(enums.armor.signature, this.write(), undefined, undefined, undefined, config);
+    // An ASCII-armored sequence of Signature packets that only includes v6 Signature packets MUST NOT contain a CRC24 footer.
+    const emitChecksum = this.packets.some(packet => packet.constructor.tag === SignaturePacket.tag && packet.version !== 6);
+    return armor(enums.armor.signature, this.write(), undefined, undefined, undefined, emitChecksum, config);
   }
 
   /**

test/general/armor.js

@@ -255,6 +255,40 @@ export default () => describe('ASCII armor', function() {
     expect(msg.text).to.equal('\r\nsign this');
   });
 
+  it('Selectively output CRC checksum', async function () {
+    const includesArmorChecksum = armoredData => {
+      const lines = armoredData.split('\n');
+      const lastDataLine = lines[lines.length - 3];
+      return (lastDataLine[0] === '=' && lastDataLine.length === 5);
+    };
+
+    // unless explicitly forbidden by the spec, we include the checksum to work around a GnuPG bug (https://dev.gnupg.org/T7071)
+    const { privateKey: v4Key } = await openpgp.generateKey({ userIDs: { email: 'v4@armor.test' }, format: 'object' });
+    expect(includesArmorChecksum(v4Key.armor())).to.be.true;
+    const { privateKey: v6Key } = await openpgp.generateKey({ userIDs: { email: 'v6@armor.test' }, config: { v6Keys: true, aeadProtect: true }, format: 'object' });
+    expect(includesArmorChecksum(v6Key.armor())).to.be.false;
+
+    const messageWithSEIPDv1 = await openpgp.encrypt({ message: await openpgp.createMessage({ text: 'test' }), encryptionKeys: v4Key });
+    expect(includesArmorChecksum(messageWithSEIPDv1)).to.be.true;
+    const messageWithSEIPDv2 = await openpgp.encrypt({ message: await openpgp.createMessage({ text: 'test' }), encryptionKeys: v6Key });
+    expect(includesArmorChecksum(messageWithSEIPDv2)).to.be.false;
+
+    const signatureV4V6 = await openpgp.sign({ message: await openpgp.createMessage({ text: 'test' }), signingKeys: [v4Key, v6Key] });
+    expect(includesArmorChecksum(signatureV4V6)).to.be.true;
+    const signatureV6 = await openpgp.sign({ message: await openpgp.createMessage({ text: 'test' }), signingKeys: v6Key });
+    expect(includesArmorChecksum(signatureV6)).to.be.false;
+
+    const detachedSignatureV4V6 = await openpgp.sign({ message: await openpgp.createMessage({ text: 'test' }), signingKeys: [v4Key, v6Key], detached: true });
+    expect(includesArmorChecksum(detachedSignatureV4V6)).to.be.true;
+    const detachedSignatureV6 = await openpgp.sign({ message: await openpgp.createMessage({ text: 'test' }), signingKeys: v6Key, detached: true });
+    expect(includesArmorChecksum(detachedSignatureV6)).to.be.false;
+
+    const cleartextSignatureV4V6 = await openpgp.sign({ message: await openpgp.createCleartextMessage({ text: 'test' }), signingKeys: [v4Key, v6Key] });
+    expect(includesArmorChecksum(cleartextSignatureV4V6)).to.be.true;
+    const cleartextSignatureV6 = await openpgp.sign({ message: await openpgp.createCleartextMessage({ text: 'test' }), signingKeys: v6Key });
+    expect(includesArmorChecksum(cleartextSignatureV6)).to.be.false;
+  });
+
   it('Do not add extraneous blank line when base64 ends on line break', async function () {
     const pubKey = `-----BEGIN PGP PUBLIC KEY BLOCK-----
 

test/general/key.js

@@ -4024,7 +4024,7 @@ CNa5yq6lyexhsn2Vs8DsX+SOSUyNJiy5FyIJ
 
     const input = await openpgp.unarmor(revocation_certificate_arm4);
     const packetlist = await openpgp.PacketList.fromBinary(input.data, util.constructAllowedPackets([openpgp.SignaturePacket]), openpgp.config);
-    const armored = openpgp.armor(openpgp.enums.armor.publicKey, packetlist.write());
+    const armored = openpgp.armor(openpgp.enums.armor.publicKey, packetlist.write(), undefined, undefined, undefined, true);
 
     expect(revocationCertificate.replace(/^Comment: .*$\n/mg, '')).to.equal(armored.replace(/^Comment: .*$\n/mg, ''));
   });