From ef066183ddcaa04d070e9519390190794a2c5264 Mon Sep 17 00:00:00 2001 From: larabr Date: Tue, 7 Jun 2022 13:51:58 +0200 Subject: Throw `UnsupportedError` on unknown algorithm in keys, signatures and encrypted session keys (#1523) The relevant packets will be considered unsupported instead of malformed. Hence, parsing them will succeed by default (based on `config.ignoreUnsupportedPackets`). --- src/crypto/crypto.js | 32 +++++++++++++++++++------ src/crypto/public_key/elliptic/curves.js | 3 ++- src/crypto/signature.js | 7 +++--- src/key/helper.js | 2 +- src/packet/public_key.js | 10 +++----- src/packet/secret_key.js | 3 +++ test/crypto/ecdh.js | 2 +- test/crypto/elliptic.js | 4 ++-- test/general/packet.js | 40 ++++++++++++++++++++++++++++++++ 9 files changed, 81 insertions(+), 22 deletions(-) diff --git a/src/crypto/crypto.js b/src/crypto/crypto.js index 49804733..6662ee5a 100644 --- a/src/crypto/crypto.js +++ b/src/crypto/crypto.js @@ -34,6 +34,7 @@ import enums from '../enums'; import util from '../util'; import OID from '../type/oid'; import { Curve } from './public_key/elliptic/curves'; +import { UnsupportedError } from '../packet/packet'; /** * Encrypts data using specified algorithm and public key parameters. @@ -105,7 +106,7 @@ export async function publicKeyDecrypt(algo, publicKeyParams, privateKeyParams, oid, kdfParams, V, C.data, Q, d, fingerprint); } default: - throw new Error('Invalid public key encryption algorithm.'); + throw new Error('Unknown public key encryption algorithm.'); } } @@ -140,23 +141,26 @@ export function parsePublicKeyParams(algo, bytes) { } case enums.publicKey.ecdsa: { const oid = new OID(); read += oid.read(bytes); + checkSupportedCurve(oid); const Q = util.readMPI(bytes.subarray(read)); read += Q.length + 2; return { read: read, publicParams: { oid, Q } }; } case enums.publicKey.eddsa: { const oid = new OID(); read += oid.read(bytes); + checkSupportedCurve(oid); let Q = util.readMPI(bytes.subarray(read)); read += Q.length + 2; Q = util.leftPad(Q, 33); return { read: read, publicParams: { oid, Q } }; } case enums.publicKey.ecdh: { const oid = new OID(); read += oid.read(bytes); + checkSupportedCurve(oid); const Q = util.readMPI(bytes.subarray(read)); read += Q.length + 2; const kdfParams = new KDFParams(); read += kdfParams.read(bytes.subarray(read)); return { read: read, publicParams: { oid, Q, kdfParams } }; } default: - throw new Error('Invalid public key encryption algorithm.'); + throw new UnsupportedError('Unknown public key encryption algorithm.'); } } @@ -192,12 +196,13 @@ export function parsePrivateKeyParams(algo, bytes, publicParams) { return { read, privateParams: { d } }; } case enums.publicKey.eddsa: { + const curve = new Curve(publicParams.oid); let seed = util.readMPI(bytes.subarray(read)); read += seed.length + 2; - seed = util.leftPad(seed, 32); + seed = util.leftPad(seed, curve.payloadSize); return { read, privateParams: { seed } }; } default: - throw new Error('Invalid public key encryption algorithm.'); + throw new UnsupportedError('Unknown public key encryption algorithm.'); } } @@ -234,7 +239,7 @@ export function parseEncSessionKeyParams(algo, bytes) { return { V, C }; } default: - throw new Error('Invalid public key encryption algorithm.'); + throw new UnsupportedError('Unknown public key encryption algorithm.'); } } @@ -293,7 +298,7 @@ export function generateParams(algo, bits, oid) { case enums.publicKey.elgamal: throw new Error('Unsupported algorithm for key generation.'); default: - throw new Error('Invalid public key algorithm.'); + throw new Error('Unknown public key algorithm.'); } } @@ -340,7 +345,7 @@ export async function validateParams(algo, publicParams, privateParams) { return publicKey.elliptic.eddsa.validateParams(oid, Q, seed); } default: - throw new Error('Invalid public key algorithm.'); + throw new Error('Unknown public key algorithm.'); } } @@ -391,3 +396,16 @@ export function getCipher(algo) { const algoName = enums.read(enums.symmetric, algo); return cipher[algoName]; } + +/** + * Check whether the given curve OID is supported + * @param {module:type/oid} oid - EC object identifier + * @throws {UnsupportedError} if curve is not supported + */ +function checkSupportedCurve(oid) { + try { + oid.getName(); + } catch (e) { + throw new UnsupportedError('Unknown curve OID'); + } +} diff --git a/src/crypto/public_key/elliptic/curves.js b/src/crypto/public_key/elliptic/curves.js index 4403fbad..42ee1047 100644 --- a/src/crypto/public_key/elliptic/curves.js +++ b/src/crypto/public_key/elliptic/curves.js @@ -28,6 +28,7 @@ import util from '../../../util'; import { uint8ArrayToB64, b64ToUint8Array } from '../../../encoding/base64'; import OID from '../../../type/oid'; import { keyFromPublic, keyFromPrivate, getIndutnyCurve } from './indutnyKey'; +import { UnsupportedError } from '../../../packet/packet'; const webCrypto = util.getWebCrypto(); const nodeCrypto = util.getNodeCrypto(); @@ -145,7 +146,7 @@ class Curve { // by curve name or oid string this.name = enums.write(enums.curve, oidOrName); } catch (err) { - throw new Error('Not valid curve'); + throw new UnsupportedError('Unknown curve'); } params = params || curves[this.name]; diff --git a/src/crypto/signature.js b/src/crypto/signature.js index 012db203..aa027bbe 100644 --- a/src/crypto/signature.js +++ b/src/crypto/signature.js @@ -7,6 +7,7 @@ import publicKey from './public_key'; import enums from '../enums'; import util from '../util'; +import { UnsupportedError } from '../packet/packet'; /** * Parse signature in binary form to get the parameters. @@ -55,7 +56,7 @@ export function parseSignatureParams(algo, signature) { return { r, s }; } default: - throw new Error('Invalid signature algorithm.'); + throw new UnsupportedError('Unknown signature algorithm.'); } } @@ -101,7 +102,7 @@ export async function verify(algo, hashAlgo, signature, publicParams, data, hash return publicKey.elliptic.eddsa.verify(oid, hashAlgo, signature, data, Q, hashed); } default: - throw new Error('Invalid signature algorithm.'); + throw new Error('Unknown signature algorithm.'); } } @@ -151,6 +152,6 @@ export async function sign(algo, hashAlgo, publicKeyParams, privateKeyParams, da return publicKey.elliptic.eddsa.sign(oid, hashAlgo, data, Q, seed, hashed); } default: - throw new Error('Invalid signature algorithm.'); + throw new Error('Unknown signature algorithm.'); } } diff --git a/src/key/helper.js b/src/key/helper.js index 0720dde2..df618b1b 100644 --- a/src/key/helper.js +++ b/src/key/helper.js @@ -340,7 +340,7 @@ export function sanitizeKeyOptions(options, subkeyDefaults = {}) { try { options.curve = enums.write(enums.curve, options.curve); } catch (e) { - throw new Error('Invalid curve'); + throw new Error('Unknown curve'); } if (options.curve === enums.curve.ed25519 || options.curve === enums.curve.curve25519) { options.curve = options.sign ? enums.curve.ed25519 : enums.curve.curve25519; diff --git a/src/packet/public_key.js b/src/packet/public_key.js index 4a1ef788..14c188e6 100644 --- a/src/packet/public_key.js +++ b/src/packet/public_key.js @@ -123,13 +123,9 @@ class PublicKeyPacket { } // - A series of values comprising the key material. - try { - const { read, publicParams } = crypto.parsePublicKeyParams(this.algorithm, bytes.subarray(pos)); - this.publicParams = publicParams; - pos += read; - } catch (err) { - throw new Error('Error reading MPIs'); - } + const { read, publicParams } = crypto.parsePublicKeyParams(this.algorithm, bytes.subarray(pos)); + this.publicParams = publicParams; + pos += read; // we set the fingerprint and keyID already to make it possible to put together the key packets directly in the Key constructor await this.computeFingerprintAndKeyID(); diff --git a/src/packet/secret_key.js b/src/packet/secret_key.js index 016fbd4f..02b5631f 100644 --- a/src/packet/secret_key.js +++ b/src/packet/secret_key.js @@ -21,6 +21,7 @@ import crypto from '../crypto'; import enums from '../enums'; import util from '../util'; import defaultConfig from '../config'; +import { UnsupportedError } from './packet'; /** * A Secret-Key packet contains all the information that is found in a @@ -155,6 +156,8 @@ class SecretKeyPacket extends PublicKeyPacket { const { privateParams } = crypto.parsePrivateKeyParams(this.algorithm, cleartext, this.publicParams); this.privateParams = privateParams; } catch (err) { + if (err instanceof UnsupportedError) throw err; + // avoid throwing potentially sensitive errors throw new Error('Error reading MPIs'); } } diff --git a/test/crypto/ecdh.js b/test/crypto/ecdh.js index af06c374..e1814472 100644 --- a/test/crypto/ecdh.js +++ b/test/crypto/ecdh.js @@ -69,7 +69,7 @@ module.exports = () => describe('ECDH key exchange @lightweight', function () { it('Invalid curve oid', function (done) { expect(decrypt_message( '', 2, 7, [], [], [], [], [] - )).to.be.rejectedWith(Error, /Not valid curve/).notify(done); + )).to.be.rejectedWith(Error, /Unknown curve/).notify(done); }); it('Invalid ephemeral key', function (done) { if (!openpgp.config.useIndutnyElliptic && !util.getNodeCrypto()) { diff --git a/test/crypto/elliptic.js b/test/crypto/elliptic.js index c9cd4072..abe23a24 100644 --- a/test/crypto/elliptic.js +++ b/test/crypto/elliptic.js @@ -167,10 +167,10 @@ module.exports = () => describe('Elliptic Curve Cryptography @lightweight', func return Promise.all([ expect(verify_signature( 'invalid oid', 8, [], [], [], [] - )).to.be.rejectedWith(Error, /Not valid curve/), + )).to.be.rejectedWith(Error, /Unknown curve/), expect(verify_signature( '\x00', 8, [], [], [], [] - )).to.be.rejectedWith(Error, /Not valid curve/) + )).to.be.rejectedWith(Error, /Unknown curve/) ]); }); it('Invalid public key', async function () { diff --git a/test/general/packet.js b/test/general/packet.js index a99fe86b..c943bb3c 100644 --- a/test/general/packet.js +++ b/test/general/packet.js @@ -996,6 +996,46 @@ kePFjAnu9cpynKXu3usf8+FuBw2zLsg1Id1n7ttxoAte416KjBN9lFBt8mcu ).to.be.rejectedWith(/Version 1 of the signature packet is unsupported/); }); + it('Ignores unknown signature algorithm only with `config.ignoreUnsupportedPackets` enabled', async function() { + const binarySignature = util.hexToUint8Array('c2750401630a00060502628b8e2200210910f30ddfc2310b3560162104b9b0045c1930f842cb245566f30ddfc2310b35602ded0100bd69fe6a9f52499cd8b2fd2493dae91c997979890df4467cf31b197901590ff10100ead4c671487535b718a8428c8e6099e3873a41610aad9fcdaa06f6df5f404002'); + + const parsed = await openpgp.PacketList.fromBinary(binarySignature, allAllowedPackets, { ...openpgp.config, ignoreUnsupportedPackets: true }); + expect(parsed.length).to.equal(1); + expect(parsed[0]).instanceOf(openpgp.UnparseablePacket); + expect(parsed[0].tag).to.equal(openpgp.enums.packet.signature); + + await expect( + openpgp.PacketList.fromBinary(binarySignature, allAllowedPackets, { ...openpgp.config, ignoreUnsupportedPackets: false }) + ).to.be.rejectedWith(/Unknown signature algorithm/); + }); + + it('Ignores unknown key algorithm only with `config.ignoreUnsupportedPackets` enabled', async function() { + const binaryKey = util.hexToUint8Array('c55804628b944e63092b06010401da470f01010740d01ab8619b6dc6a36da5bff62ff416a974900f5a8c74d1bd1760d717d0aad8d50000ff516f8e3190aa5b394597655d7c32e16392e638da0e2a869fb7b1f429d9de263d1062cd0f3c7465737440746573742e636f6d3ec28c0410160a001d0502628b944e040b0907080315080a0416000201021901021b03021e01002109104803e40df201fa5b16210496dc42e91cc585e2f5e331644803e40df201fa5b340b0100812c47b60fa509e12e329fc37cc9c437cc6a6500915caa03ad8703db849846f900ff571b9a0d9e1dcc087d9fae04ec2906e60ef40ca02a387eb07ce1c37bedeecd0a'); + + const parsed = await openpgp.PacketList.fromBinary(binaryKey, allAllowedPackets, { ...openpgp.config, ignoreUnsupportedPackets: true }); + expect(parsed.length).to.equal(3); + expect(parsed[0]).instanceOf(openpgp.UnparseablePacket); + expect(parsed[0].tag).to.equal(openpgp.enums.packet.secretKey); + + await expect( + openpgp.PacketList.fromBinary(binaryKey, allAllowedPackets, { ...openpgp.config, ignoreUnsupportedPackets: false }) + ).to.be.rejectedWith(/Unknown public key encryption algorithm/); + }); + + it('Ignores unknown PKESK algorithm only with `config.ignoreUnsupportedPackets` enabled', async function() { + const binaryMessage = util.hexToUint8Array('c15e03c6a6737124ef0f5e63010740282956b4db64ea79e1b4b8e5c528241b5e1cf40b2f5df2a619692755d532353d30a8e044e7c96f51741c73e6c5c8f73db08daf66e49240afe90c9b50705d51e71ec2e7630c5bd86b002e1f6dbd638f61e2d23501830d9bb3711c66963363a6e5f8d9294210a0cd194174c3caa3f29865d33c6be4c09b437f906ca8d35e666f3ef53fd22e0d8ceade'); + + const parsed = await openpgp.PacketList.fromBinary(binaryMessage, allAllowedPackets, { ...openpgp.config, ignoreUnsupportedPackets: true }); + expect(parsed.length).to.equal(2); + expect(parsed[0]).instanceOf(openpgp.UnparseablePacket); + expect(parsed[0].tag).to.equal(openpgp.enums.packet.publicKeyEncryptedSessionKey); + + await expect( + openpgp.PacketList.fromBinary(binaryMessage, allAllowedPackets, { ...openpgp.config, ignoreUnsupportedPackets: false }) + ).to.be.rejectedWith(/Unknown public key encryption algorithm/); + }); + + it('Throws on disallowed packet even with tolerant mode enabled', async function() { const packets = new openpgp.PacketList(); packets.push(new openpgp.LiteralDataPacket()); -- cgit v1.2.3