Mirroristas/openpgpjs
2
0
Fork 0
mirror of https://github.com/openpgpjs/openpgpjs.git synced 2025-03-30 15:08:32 +00:00
Code Issues Packages Projects Releases Wiki Activity

Detect invalid PKESK public point encoding on decryption

Browse Source 
We got a report of a message including a PKESK packet where
the ECDH x25519Legacy point was missing the leading byte (0x40).
While decryption naturally would naturally fail afterwards, this
change ensures we fail at a higher level, and do not blindly pass
down invalid data to the low-level crypto functions.
This commit is contained in:
larabr 2024-05-15 19:26:18 +02:00 committed by larabr
parent 9f5ff66c3d
commit 08b71487c5
3 changed files with 58 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
src/crypto/public_key/elliptic/ecdh.js
View File

@ -21,7 +21,7 @@
*/
import nacl from '@openpgp/tweetnacl';
import { CurveWithOID, jwkToRawPublic, rawPublicToJWK, privateToJWK, validateStandardParams } from './oid_curves';
import { CurveWithOID, jwkToRawPublic, rawPublicToJWK, privateToJWK, validateStandardParams, checkPublicPointEnconding } from './oid_curves';
import * as aesKW from '../../aes_kw';
import { getRandomBytes } from '../../random';
import hash from '../../hash';
@ -193,6 +193,7 @@ async function genPrivateEphemeralKey(curve, V, Q, d) {
*/
export async function decrypt(oid, kdfParams, V, C, Q, d, fingerprint) {
const curve = new CurveWithOID(oid);
checkPublicPointEnconding(oid, V);
const { sharedKey } = await genPrivateEphemeralKey(curve, V, Q, d);
const param = buildEcdhParam(enums.publicKey.ecdh, oid, kdfParams, fingerprint);
const { keySize } = getCipherParams(kdfParams.cipher);

44
src/crypto/public_key/elliptic/oid_curves.js
View File

@ -57,7 +57,8 @@ const curves = {
node: nodeCurves[enums.curve.nistP256],
web: webCurves[enums.curve.nistP256],
payloadSize: 32,
sharedSize: 256
sharedSize: 256,
wireFormatLeadingByte: 0x04
},
[enums.curve.nistP384]: {
oid: [0x06, 0x05, 0x2B, 0x81, 0x04, 0x00, 0x22],
@ -67,7 +68,8 @@ const curves = {
node: nodeCurves[enums.curve.nistP384],
web: webCurves[enums.curve.nistP384],
payloadSize: 48,
sharedSize: 384
sharedSize: 384,
wireFormatLeadingByte: 0x04
},
[enums.curve.nistP521]: {
oid: [0x06, 0x05, 0x2B, 0x81, 0x04, 0x00, 0x23],
@ -77,7 +79,8 @@ const curves = {
node: nodeCurves[enums.curve.nistP521],
web: webCurves[enums.curve.nistP521],
payloadSize: 66,
sharedSize: 528
sharedSize: 528,
wireFormatLeadingByte: 0x04
},
[enums.curve.secp256k1]: {
oid: [0x06, 0x05, 0x2B, 0x81, 0x04, 0x00, 0x0A],
@ -85,14 +88,16 @@ const curves = {
hash: enums.hash.sha256,
cipher: enums.symmetric.aes128,
node: nodeCurves[enums.curve.secp256k1],
payloadSize: 32
payloadSize: 32,
wireFormatLeadingByte: 0x04
},
[enums.curve.ed25519Legacy]: {
oid: [0x06, 0x09, 0x2B, 0x06, 0x01, 0x04, 0x01, 0xDA, 0x47, 0x0F, 0x01],
keyType: enums.publicKey.eddsaLegacy,
hash: enums.hash.sha512,
node: false, // nodeCurves.ed25519 TODO
payloadSize: 32
payloadSize: 32,
wireFormatLeadingByte: 0x40
},
[enums.curve.curve25519Legacy]: {
oid: [0x06, 0x0A, 0x2B, 0x06, 0x01, 0x04, 0x01, 0x97, 0x55, 0x01, 0x05, 0x01],
@ -100,7 +105,8 @@ const curves = {
hash: enums.hash.sha256,
cipher: enums.symmetric.aes128,
node: false, // nodeCurves.curve25519 TODO
payloadSize: 32
payloadSize: 32,
wireFormatLeadingByte: 0x40
},
[enums.curve.brainpoolP256r1]: {
oid: [0x06, 0x09, 0x2B, 0x24, 0x03, 0x03, 0x02, 0x08, 0x01, 0x01, 0x07],
@ -108,7 +114,8 @@ const curves = {
hash: enums.hash.sha256,
cipher: enums.symmetric.aes128,
node: nodeCurves[enums.curve.brainpoolP256r1],
payloadSize: 32
payloadSize: 32,
wireFormatLeadingByte: 0x04
},
[enums.curve.brainpoolP384r1]: {
oid: [0x06, 0x09, 0x2B, 0x24, 0x03, 0x03, 0x02, 0x08, 0x01, 0x01, 0x0B],
@ -116,7 +123,8 @@ const curves = {
hash: enums.hash.sha384,
cipher: enums.symmetric.aes192,
node: nodeCurves[enums.curve.brainpoolP384r1],
payloadSize: 48
payloadSize: 48,
wireFormatLeadingByte: 0x04
},
[enums.curve.brainpoolP512r1]: {
oid: [0x06, 0x09, 0x2B, 0x24, 0x03, 0x03, 0x02, 0x08, 0x01, 0x01, 0x0D],
@ -124,7 +132,8 @@ const curves = {
hash: enums.hash.sha512,
cipher: enums.symmetric.aes256,
node: nodeCurves[enums.curve.brainpoolP512r1],
payloadSize: 64
payloadSize: 64,
wireFormatLeadingByte: 0x04
}
};
@ -268,8 +277,23 @@ async function validateStandardParams(algo, oid, Q, d) {
return true;
}
/**
* Check whether the public point has a valid encoding.
* NB: this function does not check e.g. whether the point belongs to the curve.
*/
function checkPublicPointEnconding(oid, V) {
const curveName = oid.getName();
const { payloadSize, wireFormatLeadingByte } = curves[curveName];
const pointSize = (curveName === enums.curve.curve25519Legacy || curveName === enums.curve.ed25519Legacy) ? payloadSize : payloadSize * 2;
if (V[0] !== wireFormatLeadingByte || V.length !== pointSize + 1) {
throw new Error('Invalid point encoding');
}
}
export {
CurveWithOID, curves, webCurves, nodeCurves, generate, getPreferredHashAlgo, jwkToRawPublic, rawPublicToJWK, privateToJWK, validateStandardParams
CurveWithOID, curves, webCurves, nodeCurves, generate, getPreferredHashAlgo, jwkToRawPublic, rawPublicToJWK, privateToJWK, validateStandardParams, checkPublicPointEnconding
};
//////////////////////////

32
test/crypto/ecdh.js
View File

@ -72,14 +72,6 @@ export default () => describe('ECDH key exchange @lightweight', function () {
'', 2, 7, [], [], [], [], []
)).to.be.rejectedWith(Error, /Unknown curve/).notify(done);
});
it('Invalid ephemeral key', function (done) {
if (!openpgp.config.useEllipticFallback && !util.getNodeCrypto()) {
this.skip();
}
expect(decrypt_message(
'secp256k1', 2, 7, [], [], [], [], []
)).to.be.rejectedWith(Error, /Private key is not valid for specified curve|second arg must be public key/).notify(done);
});
it('Invalid elliptic public key', function (done) {
if (!openpgp.config.useEllipticFallback && !util.getNodeCrypto()) {
this.skip();
@ -145,8 +137,8 @@ export default () => describe('ECDH key exchange @lightweight', function () {
const kdfParams = new KDFParams({ hash: curve.hash, cipher: curve.cipher });
const data = random.getRandomBytes(16);
await expect(
ecdh.encrypt(oid, kdfParams, data, Q1.subarray(1), fingerprint1)
).to.be.rejectedWith(/Public key is not valid for specified curve|Failed to translate Buffer to a EC_POINT|second arg must be public key/);
ecdh.encrypt(oid, kdfParams, data, Q1, fingerprint1)
).to.be.rejectedWith(/Invalid point encoding/);
});
it('Different keys', async function () {
@ -212,6 +204,26 @@ export default () => describe('ECDH key exchange @lightweight', function () {
const { publicKey: V, wrappedKey: C } = await ecdh.encrypt(oid, kdfParams, data, Q, fingerprint1);
expect(await ecdh.decrypt(oid, kdfParams, V, C, Q, d, fingerprint1)).to.deep.equal(data);
});
it(`${curveName} - Detect invalid PKESK public point encoding on decryption`, async function () {
const curve = new elliptic_curves.CurveWithOID(curveName);
const oid = new OID(curve.oid);
const kdfParams = new KDFParams({ hash: curve.hash, cipher: curve.cipher });
const data = random.getRandomBytes(16);
const Q = key_data[curveName].pub;
const d = key_data[curveName].priv;
const { publicKey: V, wrappedKey: C } = await ecdh.encrypt(oid, kdfParams, data, Q, fingerprint1);
const publicPointWithoutPrefixByte = V.subarray(1);
const publicPointWithUnexpectedPrefixByte = new Uint8Array([0x1, ...publicPointWithoutPrefixByte]);
const publicPointWithUnexpectedSize = V.subarray(0, V.length - 1);
const expectedError = /Invalid point encoding/;
await expect(ecdh.decrypt(oid, kdfParams, publicPointWithoutPrefixByte, C, Q, d, fingerprint1)).to.be.rejectedWith(expectedError);
await expect(ecdh.decrypt(oid, kdfParams, publicPointWithUnexpectedPrefixByte, C, Q, d, fingerprint1)).to.be.rejectedWith(expectedError);
await expect(ecdh.decrypt(oid, kdfParams, publicPointWithUnexpectedSize, C, Q, d, fingerprint1)).to.be.rejectedWith(expectedError);
});
});
describe('Comparing decrypting with and without native crypto', () => {