Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix CleartextMessage signature generation over text with trailing whitespace and \r\n line endings #1548

Merged
merged 1 commit into from Aug 2, 2022

Conversation

larabr
Copy link
Collaborator

@larabr larabr commented Jul 29, 2022

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 (ebeedd3). Note that these broken signatures were unverifiable even in the OpenPGP.js version(s) that generated them.

The problem is that the data was incorrectly normalised before being signed, as trailing whitespace preceding \r\n would not be stripped (but still correctly normalised on verification).

Example of data that would be incorrectly signed:

'this text    \r\nused to be incorrectly normalised'

If you have access to the original data that was signed (with whitespaces preserved), it's possible to verify an affected cleartext signature as a detached signature, by manually normalising the input message data as follows:

function removeTrailingSpaces(text) {
    return text.split('\n').map(line => {
      let i = line.length - 1;
      for (; i >= 0 && (line[i] === ' ' || line[i] === '\t'); i--);
      return line.substr(0, i + 1);
    }).join('\n');
}

// To verify:
// -----BEGIN PGP SIGNED MESSAGE-----
// Hash: SHA256
//
// this text    
// used to be incorrectly normalised
//
// -----BEGIN PGP SIGNATURE-----
// Version: OpenPGP.js v5.3.1
//
// wrMEAQEIAAYFAmLjqsQAIQkQ4IT3RGwgLJcWIQTX0bHexsqUZwA0RcXghPdE
// bCAsl2TvBADLOHYXevDSc3LtLRYR1HteijL/MssCCoZIfuGihd5AzJpD2h2j
// L8UuxlfERJn15RlFsDzlkMNMefJp5ltC8kKcf+HTuBUi+xf2t2nvlf8CrdjY
// vcEqswjkODDxxZ842h0sC0ZtbzWuMXIvODEdzZxBjhlmZmv9VKQ5uyb0oD/5
// WQ==
// =o2gq
// -----END PGP SIGNATURE-----

const message = await openpgp.createMessage({
    // manually normalise original input (with whitespaces preserved)
    text: removeTrailingSpaces('this text   \r\nused to be incorrectly normalised')
});

const signature = await openpgp.readSignature({ armoredSignature: `
-----BEGIN PGP SIGNATURE-----
Version: OpenPGP.js v5.3.1

wrMEAQEIAAYFAmLjqsQAIQkQ4IT3RGwgLJcWIQTX0bHexsqUZwA0RcXghPdE
bCAsl2TvBADLOHYXevDSc3LtLRYR1HteijL/MssCCoZIfuGihd5AzJpD2h2j
L8UuxlfERJn15RlFsDzlkMNMefJp5ltC8kKcf+HTuBUi+xf2t2nvlf8CrdjY
vcEqswjkODDxxZ842h0sC0ZtbzWuMXIvODEdzZxBjhlmZmv9VKQ5uyb0oD/5
WQ==
=o2gq
-----END PGP SIGNATURE-----
` })

const result = await openpgp.verify({
      message,
      signature,
      verificationKeys
});

@larabr larabr merged commit dc85a50 into openpgpjs:main Aug 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants