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>
This commit is contained in:
larabr 2024-04-09 17:12:44 +02:00 committed by GitHub
parent e9e843280b
commit 90c8fbbf00
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
10 changed files with 77 additions and 19 deletions

2
openpgp.d.ts vendored
View File

@ -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

View File

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

View File

@ -334,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} [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) {
@ -345,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 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,39 +375,38 @@ 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);
}

View File

@ -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);
}
/**

View File

@ -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);
}
/**

View File

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

View File

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

View File

@ -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);
}
/**

View File

@ -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-----

View File

@ -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, ''));
});