From 279dc0701965fd00ec15d90dcf1b259d8317eb9e Mon Sep 17 00:00:00 2001 From: larabr <7375870+larabr@users.noreply.github.com> Date: Mon, 8 Apr 2024 17:03:57 +0200 Subject: [PATCH] Add back armor checksum where allowed by the spec We need to include the checksum to workaround an GnuPG bug where data fails to be decoded if the base64 ends with no padding chars (=) (see https://dev.gnupg.org/T7071) --- src/cleartext.js | 8 ++-- src/encoding/armor.js | 88 +++++++++++++++++++++++++++++++++++++++++- src/key/key.js | 4 +- src/key/private_key.js | 4 +- src/key/public_key.js | 4 +- src/message.js | 8 +++- src/signature.js | 4 +- test/general/armor.js | 34 ++++++++++++++++ test/general/key.js | 2 +- 9 files changed, 146 insertions(+), 10 deletions(-) diff --git a/src/cleartext.js b/src/cleartext.js index 34b1bc68b..7ebe2ba2d 100644 --- a/src/cleartext.js +++ b/src/cleartext.js @@ -111,9 +111,9 @@ export class CleartextMessage { * @returns {String | ReadableStream} ASCII armor. */ armor(config = defaultConfig) { + const includesNonV6Signatures = this.signature.packets.some(packet => packet.version !== 6); // 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 ? + const hash = includesNonV6Signatures ? 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, includesNonV6Signatures, config); } } diff --git a/src/encoding/armor.js b/src/encoding/armor.js index 8686d1315..ed0a6be45 100644 --- a/src/encoding/armor.js +++ b/src/encoding/armor.js @@ -107,6 +107,78 @@ function addheader(customComment, config) { return result; } +/** + * Calculates a checksum over the given data and returns it base64 encoded + * @param {String | ReadableStream} data - Data to create a CRC-24 checksum for + * @returns {String | ReadableStream} Base64 encoded checksum. + * @private + */ +function getCheckSum(data) { + const crc = createcrc24(data); + return base64.encode(crc); +} + +// https://create.stephan-brumme.com/crc32/#slicing-by-8-overview + +const crc_table = [ + new Array(0xFF), + new Array(0xFF), + new Array(0xFF), + new Array(0xFF) +]; + +for (let i = 0; i <= 0xFF; i++) { + let crc = i << 16; + for (let j = 0; j < 8; j++) { + crc = (crc << 1) ^ ((crc & 0x800000) !== 0 ? 0x864CFB : 0); + } + crc_table[0][i] = + ((crc & 0xFF0000) >> 16) | + (crc & 0x00FF00) | + ((crc & 0x0000FF) << 16); +} +for (let i = 0; i <= 0xFF; i++) { + crc_table[1][i] = (crc_table[0][i] >> 8) ^ crc_table[0][crc_table[0][i] & 0xFF]; +} +for (let i = 0; i <= 0xFF; i++) { + crc_table[2][i] = (crc_table[1][i] >> 8) ^ crc_table[0][crc_table[1][i] & 0xFF]; +} +for (let i = 0; i <= 0xFF; i++) { + crc_table[3][i] = (crc_table[2][i] >> 8) ^ crc_table[0][crc_table[2][i] & 0xFF]; +} + +// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/DataView#Endianness +const isLittleEndian = (function() { + const buffer = new ArrayBuffer(2); + new DataView(buffer).setInt16(0, 0xFF, true /* littleEndian */); + // Int16Array uses the platform's endianness. + return new Int16Array(buffer)[0] === 0xFF; +}()); + +/** + * Internal function to calculate a CRC-24 checksum over a given string (data) + * @param {String | ReadableStream} input - Data to create a CRC-24 checksum for + * @returns {Uint8Array | ReadableStream} The CRC-24 checksum. + * @private + */ +function createcrc24(input) { + let crc = 0xCE04B7; + return stream.transform(input, value => { + const len32 = isLittleEndian ? Math.floor(value.length / 4) : 0; + const arr32 = new Uint32Array(value.buffer, value.byteOffset, len32); + for (let i = 0; i < len32; i++) { + crc ^= arr32[i]; + crc = + crc_table[0][(crc >> 24) & 0xFF] ^ + crc_table[1][(crc >> 16) & 0xFF] ^ + crc_table[2][(crc >> 8) & 0xFF] ^ + crc_table[3][(crc >> 0) & 0xFF]; + } + for (let i = len32 * 4; i < value.length; i++) { + crc = (crc >> 8) ^ crc_table[0][(crc & 0xFF) ^ value[i]]; + } + }, () => new Uint8Array([crc, crc >> 8, crc >> 16])); +} /** * Verify armored headers. crypto-refresh-06, section 6.2: @@ -262,10 +334,13 @@ export function unarmor(input) { * @param {Integer} [partIndex] * @param {Integer} [partTotal] * @param {String} [customComment] - Additional comment to add to the armored string + * @param {Boolean} [withChecksum] - 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} Armored text. * @static */ -export function armor(messageType, body, partIndex, partTotal, customComment, config = defaultConfig) { +export function armor(messageType, body, partIndex, partTotal, customComment, withChecksum = false, config = defaultConfig) { let text; let hash; if (messageType === enums.armor.signed) { @@ -273,18 +348,24 @@ export function armor(messageType, body, partIndex, partTotal, customComment, co hash = body.hash; body = body.data; } + // unless explicitly forbidden by the spec, we need to include the checksum to workaround an GnuPG bug + // where data fails to be decoded if the base64 ends with no padding chars (=) (see https://dev.gnupg.org/T7071) + const maybeBodyClone = withChecksum && 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: @@ -294,30 +375,35 @@ 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: result.push('-----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; } diff --git a/src/key/key.js b/src/key/key.js index 3c7475ada..54d2babde 100644 --- a/src/key/key.js +++ b/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 v6Key = this.keyPacket.version === 6; + return armor(enums.armor.publicKey, packetlist.write(), null, null, 'This is a revocation certificate', !v6Key); } /** diff --git a/src/key/private_key.js b/src/key/private_key.js index 3c6fe4741..943854202 100644 --- a/src/key/private_key.js +++ b/src/key/private_key.js @@ -64,7 +64,9 @@ class PrivateKey extends PublicKey { * @returns {ReadableStream} 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 v6Key = this.keyPacket.version === 6; + return armor(enums.armor.privateKey, this.toPacketList().write(), undefined, undefined, undefined, !v6Key, config); } /** diff --git a/src/key/public_key.js b/src/key/public_key.js index 66eac9240..cada0ef13 100644 --- a/src/key/public_key.js +++ b/src/key/public_key.js @@ -61,7 +61,9 @@ class PublicKey extends Key { * @returns {ReadableStream} 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 v6Key = this.keyPacket.version === 6; + return armor(enums.armor.publicKey, this.toPacketList().write(), undefined, undefined, undefined, !v6Key, config); } } diff --git a/src/message.js b/src/message.js index 159c003e4..e43162dfc 100644 --- a/src/message.js +++ b/src/message.js @@ -680,7 +680,13 @@ export class Message { * @returns {ReadableStream} 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 includeArmorChecksum = 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, includeArmorChecksum, config); } } diff --git a/src/signature.js b/src/signature.js index ae9e5623a..6a03d8bbb 100644 --- a/src/signature.js +++ b/src/signature.js @@ -49,7 +49,9 @@ export class Signature { * @returns {ReadableStream} 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 includesNonV6Signatures = this.packets.some(packet => packet.constructor.tag === SignaturePacket.tag && packet.version !== 6); + return armor(enums.armor.signature, this.write(), undefined, undefined, undefined, includesNonV6Signatures, config); } /** diff --git a/test/general/armor.js b/test/general/armor.js index 627737920..870f46692 100644 --- a/test/general/armor.js +++ b/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 workaround an 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----- diff --git a/test/general/key.js b/test/general/key.js index 1060ac873..570671fd4 100644 --- a/test/general/key.js +++ b/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, '')); });