diff --git a/src/crypto/public_key/elliptic/ecdh_x.js b/src/crypto/public_key/elliptic/ecdh_x.js index a7bf3bd5..b293d4e5 100644 --- a/src/crypto/public_key/elliptic/ecdh_x.js +++ b/src/crypto/public_key/elliptic/ecdh_x.js @@ -174,14 +174,15 @@ export async function generateEphemeralEncryptionMaterial(algo, recipientA) { case enums.publicKey.x25519: { const ephemeralSecretKey = getRandomBytes(getPayloadSize(algo)); const sharedSecret = x25519.scalarMult(ephemeralSecretKey, recipientA); + assertNonZeroArray(sharedSecret); const { publicKey: ephemeralPublicKey } = x25519.box.keyPair.fromSecretKey(ephemeralSecretKey); - return { ephemeralPublicKey, sharedSecret }; } case enums.publicKey.x448: { const x448 = await util.getNobleCurve(enums.publicKey.x448); const ephemeralSecretKey = x448.utils.randomPrivateKey(); const sharedSecret = x448.getSharedSecret(ephemeralSecretKey, recipientA); + assertNonZeroArray(sharedSecret); const ephemeralPublicKey = x448.getPublicKey(ephemeralSecretKey); return { ephemeralPublicKey, sharedSecret }; } @@ -192,14 +193,34 @@ export async function generateEphemeralEncryptionMaterial(algo, recipientA) { export async function recomputeSharedSecret(algo, ephemeralPublicKey, A, k) { switch (algo) { - case enums.publicKey.x25519: - return x25519.scalarMult(k, ephemeralPublicKey); + case enums.publicKey.x25519: { + const sharedSecret = x25519.scalarMult(k, ephemeralPublicKey); + assertNonZeroArray(sharedSecret); + return sharedSecret; + } case enums.publicKey.x448: { const x448 = await util.getNobleCurve(enums.publicKey.x448); const sharedSecret = x448.getSharedSecret(k, ephemeralPublicKey); + assertNonZeroArray(sharedSecret); return sharedSecret; } default: throw new Error('Unsupported ECDH algorithm'); } } + +/** + * x25519 and x448 produce an all-zero value when given as input a point with small order. + * This does not lead to a security issue in the context of ECDH, but it is still unexpected, + * hence we throw. + * @param {Uint8Array} sharedSecret + */ +function assertNonZeroArray(sharedSecret) { + let acc = 0; + for (let i = 0; i < sharedSecret.length; i++) { + acc |= sharedSecret[i]; + } + if (acc === 0) { + throw new Error('Unexpected low order point'); + } +} diff --git a/test/crypto/ecdh.js b/test/crypto/ecdh.js index b44c8a3f..72a8892b 100644 --- a/test/crypto/ecdh.js +++ b/test/crypto/ecdh.js @@ -203,6 +203,76 @@ export default () => describe('ECDH key exchange @lightweight', function () { expect(await ecdhX.decrypt(openpgp.enums.publicKey.x448, ephemeralPublicKey, wrappedKey, K_B, b)).to.deep.equal(data); }); + it('Detect small order points in x25519', async () => { + const vectors = [ + { + 'order': '0', + 'vector': '0000000000000000000000000000000000000000000000000000000000000000' + }, + { + 'order': '1', + 'vector': '0100000000000000000000000000000000000000000000000000000000000000' + }, + { + 'order': '8', + 'vector': 'e0eb7a7c3b41b8ae1656e3faf19fc46ada098deb9c32b1fd866205165f49b800' + }, + { + 'order': 'p-1 (order 2)', + 'vector': 'ecffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff7f' + }, + { + 'order': 'p (=0, order 4)', + 'vector': 'edffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff7f' + }, + { + 'order': 'p+1 (=1, order 1)', + 'vector': 'eeffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff7f' + } + ]; + const data = random.getRandomBytes(16); + for (const { vector } of vectors) { + const lowOrderPoint = util.hexToUint8Array(vector); + const { A: K_A, k: a } = await elliptic_curves.ecdhX.generate(openpgp.enums.publicKey.x25519); + await expect(elliptic_curves.ecdhX.encrypt(openpgp.enums.publicKey.x25519, data, lowOrderPoint)).to.be.rejectedWith(/low order point/); + const dummyWrappedKey = new Uint8Array(32); // expected to be unused + await expect(elliptic_curves.ecdhX.decrypt(openpgp.enums.publicKey.x25519, lowOrderPoint, dummyWrappedKey, K_A, a)).to.be.rejectedWith(/low order point/); + } + }); + + it('Detect small order points in x448', async () => { + const vectors = [ + { + 'order': '0', + 'vector': '0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000' + }, + { + 'order': '1', + 'vector': '0100000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000' + }, + { + 'order': 'p-1 (order 2)', + 'vector': 'fefffffffffffffffffffffffffffffffffffffffffffffffffffffffeffffffffffffffffffffffffffffffffffffffffffffffffffffff' + }, + { + 'order': 'p (=0, order 4)', + 'vector': 'fffffffffffffffffffffffffffffffffffffffffffffffffffffffffeffffffffffffffffffffffffffffffffffffffffffffffffffffff' + }, + { + 'order': 'p+1 (=1, order 1)', + 'vector': '00000000000000000000000000000000000000000000000000000000ffffffffffffffffffffffffffffffffffffffffffffffffffffffff' + } + ]; + const data = random.getRandomBytes(16); + for (const { vector } of vectors) { + const lowOrderPoint = util.hexToUint8Array(vector); + const { A: K_A, k: a } = await elliptic_curves.ecdhX.generate(openpgp.enums.publicKey.x448); + await expect(elliptic_curves.ecdhX.encrypt(openpgp.enums.publicKey.x448, data, lowOrderPoint)).to.be.rejectedWith(/Invalid private or public key received|expected valid u|low order point/); + const dummyWrappedKey = new Uint8Array(32); // expected to be unused + await expect(elliptic_curves.ecdhX.decrypt(openpgp.enums.publicKey.x448, lowOrderPoint, dummyWrappedKey, K_A, a)).to.be.rejectedWith(/Invalid private or public key received|expected valid u|low order point/); + } + }); + const allCurves = ['secp256k1', 'nistP256', 'nistP384', 'nistP521', 'brainpoolP256r1', 'brainpoolP384r1', 'brainpoolP512r1']; allCurves.forEach(curveName => { it(`${curveName} - Successful exchange`, async function () {