diff --git a/src/crypto/hash/index.js b/src/crypto/hash/index.js index 617b59a9..0943b82f 100644 --- a/src/crypto/hash/index.js +++ b/src/crypto/hash/index.js @@ -101,7 +101,7 @@ export default { case enums.hash.sha3_512: return this.sha3_512(data); default: - throw new Error('Invalid hash function.'); + throw new Error('Unsupported hash function'); } }, diff --git a/src/packet/one_pass_signature.js b/src/packet/one_pass_signature.js index 9018e79f..5676c8e6 100644 --- a/src/packet/one_pass_signature.js +++ b/src/packet/one_pass_signature.js @@ -16,7 +16,7 @@ // Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA import * as stream from '@openpgp/web-stream-tools'; -import SignaturePacket, { saltLengthForHash } from './signature'; +import SignaturePacket from './signature'; import KeyID from '../type/keyid'; import enums from '../enums'; import util from '../util'; @@ -116,10 +116,8 @@ class OnePassSignaturePacket { // A one-octet salt size. The value MUST match the value defined // for the hash algorithm as specified in Table 23 (Hash algorithm registry). + // To allow parsing unknown hash algos, we only check the expected salt length when verifying. const saltLength = bytes[mypos++]; - if (saltLength !== saltLengthForHash(this.hashAlgorithm)) { - throw new Error('Unexpected salt size for the hash algorithm'); - } // The salt; a random value value of the specified size. this.salt = bytes.subarray(mypos, mypos + saltLength); diff --git a/src/packet/signature.js b/src/packet/signature.js index 6ab5a6a4..a66fbd49 100644 --- a/src/packet/signature.js +++ b/src/packet/signature.js @@ -145,10 +145,8 @@ class SignaturePacket { if (this.version === 6) { // A one-octet salt size. The value MUST match the value defined // for the hash algorithm as specified in Table 23 (Hash algorithm registry). + // To allow parsing unknown hash algos, we only check the expected salt length when verifying. const saltLength = bytes[i++]; - if (saltLength !== saltLengthForHash(this.hashAlgorithm)) { - throw new Error('Unexpected salt size for the hash algorithm'); - } // The salt; a random value value of the specified size. this.salt = bytes.subarray(i, i + saltLength); @@ -699,6 +697,11 @@ class SignaturePacket { } async hash(signatureType, data, toHash, detached = false) { + if (this.version === 6 && this.salt.length !== saltLengthForHash(this.hashAlgorithm)) { + // avoid hashing unexpected salt size + throw new Error('Signature salt does not have the expected length'); + } + if (!toHash) toHash = this.toHash(signatureType, data, detached); return crypto.hash.digest(this.hashAlgorithm, toHash); } @@ -827,7 +830,7 @@ function writeSubPacket(type, critical, data) { * @returns {Integer} Salt length. * @private */ -export function saltLengthForHash(hashAlgorithm) { +function saltLengthForHash(hashAlgorithm) { switch (hashAlgorithm) { case enums.hash.sha256: return 16; case enums.hash.sha384: return 24; @@ -835,6 +838,6 @@ export function saltLengthForHash(hashAlgorithm) { case enums.hash.sha224: return 16; case enums.hash.sha3_256: return 16; case enums.hash.sha3_512: return 32; - default: throw new Error('Unsupported hash function for V6 signatures'); + default: throw new Error('Unsupported hash function'); } } diff --git a/test/general/signature.js b/test/general/signature.js index 5a58dd1b..5580fedd 100644 --- a/test/general/signature.js +++ b/test/general/signature.js @@ -790,6 +790,31 @@ kCNcH9WI6idSzFjuYegECf+ZA1xOCjS9oLTGbSeT7jNfC8dH5+E92qlBLq4Ctt7k await expect(openpgp.readSignature({ armoredSignature })).to.be.rejectedWith(/Missing signature creation time/); }); + it('Supports parsing v6 signature with unknown hash algo', async function () { + // v6 signatures must check that the salt size is correct. + // but we want to support parsing signatures with unknown hash algos. + // this test checks that the parsing succeeds, while signing/verification fails. + + // key with direct signature of unknown hash algo 99 + const armoredKey = `-----BEGIN PGP PRIVATE KEY BLOCK----- + +xUsGZN8edBsAAAAgdUMlFMFCVKNo7sdUd6FVBos6NNjpUpSdrodk6BfPb/kA ++3buA2+WY2LwyxlX5o07WR2VSn+wuegC3v28yO0tClHCtwYfG2MAAABIBYJk +3x50BAsJCAcHFQ4MCgkICwIWAAIXgAKbAwIeCSIhBpbSe0QWuaCNSSLaePhX +EP3BxQ2VHX3WpW1U6svHvCUiBScJAgcCAAAAACMZIP8aHixoyC9wS3q/TNV/ +IfOQa81f+U5Ucz6H4I+c5bWRYUzH/piBB4n5FoYlld+/SViCQIBCQ+fynLma +j5wlf22+mISTt/9je1ZfYWlJ+WSJyi5gY5EH9DubfuIU3VaqCM0aQmVybmFk +ZXR0ZSA8YkBleGFtcGxlLm9yZz7CugYTGw4AAABLBYJk3x50BAsJCAcHFQ4M +CgkICwIWAAIXgAIZAQKbAwIeCSIhBpbSe0QWuaCNSSLaePhXEP3BxQ2VHX3W +pW1U6svHvCUiBScJAgcCAAAAAMMGIJEi9+yqkFKsNwX1H5II0riPudFpwBx2 +ypVjNk4aNb7Exl56Aac4tXEhz4fH41q0dAzFww2erZaiUqmohQ4AFSw1jN/W +OiDfb1DkjT/HJ8vXMGpwWdgFPoqsWzTNhd5VCQ== +-----END PGP PRIVATE KEY BLOCK-----`; + + const key = await openpgp.readKey({ armoredKey }); + await expect(key.getSigningKey()).to.be.rejectedWith(/Unsupported hash function/); + }); + it('Ignores marker packets when verifying signatures', async function () { const signatureWithMarkerPacket = `-----BEGIN PGP SIGNATURE----- @@ -914,9 +939,7 @@ SlcdMBDgwngEGBYIAAkFAmFppjQCGwwAIQkQDmTSjoPv10MWIQRqj/4SGmAk ibGeE60OZNKOg+/XQx/EAQCM0UYrObp60YbOCxu07Dm6XjCVylbOcsaxCnE7 2eMU4AD+OkgajZgbqSIdAR1ud76FW+W+3xlDi/SMFdU7D49SbQI= =ASQu ------END PGP PRIVATE KEY BLOCK----- - -`; +-----END PGP PRIVATE KEY BLOCK-----`; const armoredMessage = `-----BEGIN PGP MESSAGE----- xA0DAQoWDmTSjoPv10MByw91AGFpplFwbGFpbnRleHTCdQQBFgoABgUCYWml @@ -1062,6 +1085,39 @@ eSvSZutLuKKbidSYMLhWROPlwKc2GU2ws6PrLZAyCAel/lU= expect(await sigInfo.verified).to.be.true; }); + it('Verification fails if v6 signature has unexpected salt length', async function() { + // signature with salt shorter than expected + const armoredMessage = `-----BEGIN PGP MESSAGE----- + +xEQGAQoWHgTCf3OkPcYPPB6GmoMeaOz1wYXbuSvHxW/PVbRIynPv5yU3YApt +KDJPb4mCbmxvCoKjGx6CMjDpDsVB+wDFAcsLdQBlEWcKaGVsbG/CmgYBFgoA +AAApBYJlEWcKIqEGc+/nJTdgCm0oMk9viYJubG8KgqMbHoIyMOkOxUH7AMUA +AAAA5GYeBMJ/c6Q9xg88Hoaagx5o7PXBhdu5K8fFb89VtEjKAQCW/XwAPo2V +ugvc1634oGA/74j7KonU2qdl0LvxVJuB2wEAtutHh3wry/SNkc+japCGO4u4 +XjIVmkzQNtymmOECUwI= +-----END PGP MESSAGE-----`; + const key = await openpgp.readKey({ armoredKey: `-----BEGIN PGP PRIVATE KEY BLOCK----- + +xVoGZRFjtxYAAAAtCSsGAQQB2kcPAQEHQJRcfAi8wlCCWAeBcvpRO6iL5YK8 +1e8BVcOkAGVXKDguAAEAxIUb1xswIKPfVEyOZkqSFukVOegoArxIeEuDaoK0 +feXCrQYfFgoAAAA6BYJlEWO3AwsJBwMVCAoCFgACmwMCHgEioQZz7+clN2AK +bSgyT2+Jgm5sbwqCoxsegjIw6Q7FQfsAxQAAAACBKyDA5Ih9cWlc9o5NUzmo +jSCtKhy54bBzfRX0t9Jha4BfZwD9FvmhOEpJAnYRDmBrEiaO4okM3D6eNZz9 +rmGZkLT9oJMBAI6UbwsjgWw42W85Kb57tfYdF/779TrLHcNRZLNV0p8NzQDC +nwYQFgoAAAAsBYJlEWO3AhkBIqEGc+/nJTdgCm0oMk9viYJubG8KgqMbHoIy +MOkOxUH7AMUAAAAAV2kgOkNvj/g+Q6hFcHcpRFekCUxOons+JgXE+lxuKnbt +l10BAO7pYlHAee5dxkzQI3WPiiYFt/OYrnr7fT5QadRZhAutAP9n5bvQaoLX +vfHp79dKJnU1qDnSTEshB7ytt9I3Ze+DAQ== +-----END PGP PRIVATE KEY BLOCK-----` }); + + const { signatures } = await openpgp.verify({ + message: await openpgp.readMessage({ armoredMessage }), + verificationKeys: key + }); + expect(signatures).to.have.length(1); + await expect(signatures[0].verified).to.be.rejectedWith(/Signature salt does not have the expected length/); + }); + it('Reject cleartext message with arbitrary text added around hash headers (spoofed cleartext message)', async function() { await expect(openpgp.readCleartextMessage({ cleartextMessage: `-----BEGIN PGP SIGNED MESSAGE----- This is not signed but you might think it is Hash: SHA512