Fix CleartextMessage
signature generation over text with trailing whitespace and \r\n line endings
Signing a `CleartextMessage` containing trailing whitespace and \r\n line endings (as opposed to \n) would result in an unverifiable signature. The issue seems to have been present since v3.0.9 . These broken signatures were unverifiable even in the OpenPGP.js version(s) that generated them.
This commit is contained in:
parent
e862d5f20b
commit
dc85a5088f
|
@ -36,7 +36,7 @@ export class CleartextMessage {
|
||||||
* @param {Signature} signature - The detached signature or an empty signature for unsigned messages
|
* @param {Signature} signature - The detached signature or an empty signature for unsigned messages
|
||||||
*/
|
*/
|
||||||
constructor(text, signature) {
|
constructor(text, signature) {
|
||||||
// normalize EOL to canonical form <CR><LF>
|
// remove trailing whitespace and normalize EOL to canonical form <CR><LF>
|
||||||
this.text = util.removeTrailingSpaces(text).replace(/\r?\n/g, '\r\n');
|
this.text = util.removeTrailingSpaces(text).replace(/\r?\n/g, '\r\n');
|
||||||
if (signature && !(signature instanceof Signature)) {
|
if (signature && !(signature instanceof Signature)) {
|
||||||
throw new Error('Invalid signature input');
|
throw new Error('Invalid signature input');
|
||||||
|
|
|
@ -520,12 +520,12 @@ const util = {
|
||||||
},
|
},
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Remove trailing spaces and tabs from each line
|
* Remove trailing spaces, carriage returns and tabs from each line
|
||||||
*/
|
*/
|
||||||
removeTrailingSpaces: function(text) {
|
removeTrailingSpaces: function(text) {
|
||||||
return text.split('\n').map(line => {
|
return text.split('\n').map(line => {
|
||||||
let i = line.length - 1;
|
let i = line.length - 1;
|
||||||
for (; i >= 0 && (line[i] === ' ' || line[i] === '\t'); i--);
|
for (; i >= 0 && (line[i] === ' ' || line[i] === '\t' || line[i] === '\r'); i--);
|
||||||
return line.substr(0, i + 1);
|
return line.substr(0, i + 1);
|
||||||
}).join('\n');
|
}).join('\n');
|
||||||
},
|
},
|
||||||
|
|
|
@ -1255,6 +1255,77 @@ zmuVOdNuWQqxT9Sqa84=
|
||||||
expect((await signatures[0].signature).packets.length).to.equal(1);
|
expect((await signatures[0].signature).packets.length).to.equal(1);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('Verify cleartext signed message with trailing spaces incorrectly normalised (from OpenPGP.js v3.0.9-v5.3.1)', async function() {
|
||||||
|
// We used to not strip trailing whitespace with \r\n line endings when signing cleartext messages
|
||||||
|
const armoredSignature = `-----BEGIN PGP SIGNATURE-----
|
||||||
|
Version: OpenPGP.js v5.3.1
|
||||||
|
|
||||||
|
wrMEAQEIAAYFAmLjqsQAIQkQ4IT3RGwgLJcWIQTX0bHexsqUZwA0RcXghPdE
|
||||||
|
bCAsl2TvBADLOHYXevDSc3LtLRYR1HteijL/MssCCoZIfuGihd5AzJpD2h2j
|
||||||
|
L8UuxlfERJn15RlFsDzlkMNMefJp5ltC8kKcf+HTuBUi+xf2t2nvlf8CrdjY
|
||||||
|
vcEqswjkODDxxZ842h0sC0ZtbzWuMXIvODEdzZxBjhlmZmv9VKQ5uyb0oD/5
|
||||||
|
WQ==
|
||||||
|
=o2gq
|
||||||
|
-----END PGP SIGNATURE-----`;
|
||||||
|
const cleartextMessage = `
|
||||||
|
-----BEGIN PGP SIGNED MESSAGE-----
|
||||||
|
Hash: SHA256
|
||||||
|
|
||||||
|
this text
|
||||||
|
used to be incorrectly normalised
|
||||||
|
${armoredSignature}
|
||||||
|
`;
|
||||||
|
|
||||||
|
const signedText = 'this text \r\nused to be incorrectly normalised';
|
||||||
|
const message = await openpgp.readCleartextMessage({ cleartextMessage });
|
||||||
|
const pubKey = await openpgp.readKey({ armoredKey: pub_key_arm2 });
|
||||||
|
|
||||||
|
// Direct verification won't work since the signed data was not stripped of the trailing whitespaces,
|
||||||
|
// as required for cleartext messages. Verification would always fail also in the affected OpenPGP.js versions.
|
||||||
|
await expect(openpgp.verify({
|
||||||
|
verificationKeys:[pubKey],
|
||||||
|
message,
|
||||||
|
config: { minRSABits: 1024 },
|
||||||
|
expectSigned: true
|
||||||
|
})).to.be.rejectedWith(/Signed digest did not match/);
|
||||||
|
|
||||||
|
// The signature should be verifiable over non-normalised text
|
||||||
|
const { signatures, data } = await openpgp.verify({
|
||||||
|
verificationKeys:[pubKey],
|
||||||
|
message: await openpgp.createMessage({ text: signedText }),
|
||||||
|
signature: await openpgp.readSignature({ armoredSignature }),
|
||||||
|
config: { minRSABits: 1024 },
|
||||||
|
expectSigned: true
|
||||||
|
});
|
||||||
|
expect(data).to.equal(signedText);
|
||||||
|
expect(signatures).to.have.length(1);
|
||||||
|
expect(await signatures[0].verified).to.be.true;
|
||||||
|
});
|
||||||
|
|
||||||
|
it('Sign and verify cleartext signed message with trailing spaces correctly normalised', async function() {
|
||||||
|
const pubKey = await openpgp.readKey({ armoredKey: pub_key_arm2 });
|
||||||
|
const privKey = await openpgp.decryptKey({
|
||||||
|
privateKey: await openpgp.readKey({ armoredKey: priv_key_arm2 }),
|
||||||
|
passphrase: 'hello world'
|
||||||
|
});
|
||||||
|
const config = { minRSABits: 1024 };
|
||||||
|
|
||||||
|
const message = await openpgp.createCleartextMessage({
|
||||||
|
text: 'this text \r\nused to be incorrectly normalised'
|
||||||
|
});
|
||||||
|
const expectedText = 'this text\nused to be incorrectly normalised';
|
||||||
|
expect(message.getText()).to.equal(expectedText);
|
||||||
|
const cleartextMessage = await openpgp.sign({ message, signingKeys: privKey, config, format: 'armored' });
|
||||||
|
const { signatures, data } = await openpgp.verify({
|
||||||
|
message: await openpgp.readCleartextMessage({ cleartextMessage }),
|
||||||
|
verificationKeys:[pubKey],
|
||||||
|
config
|
||||||
|
});
|
||||||
|
expect(data).to.equal(expectedText);
|
||||||
|
expect(signatures).to.have.length(1);
|
||||||
|
expect(await signatures[0].verified).to.be.true;
|
||||||
|
});
|
||||||
|
|
||||||
function tests() {
|
function tests() {
|
||||||
it('Verify signed message with trailing spaces from GPG', async function() {
|
it('Verify signed message with trailing spaces from GPG', async function() {
|
||||||
const armoredMessage =
|
const armoredMessage =
|
||||||
|
|
Loading…
Reference in New Issue
Block a user