From 3fa778abe2902e75ae5b1e7f6327467eef91a761 Mon Sep 17 00:00:00 2001
From: larabr <larabr@protonmail.com>
Date: Thu, 19 Aug 2021 17:58:16 +0200
Subject: [PATCH] Add `config.rejectCurves` and prevent generating keys using
blacklisted algorithms (#1395)
Breaking changes:
- throw error on key generation if the requested public key algorithm is
included in `config.rejectPublicKeyAlgorithms`;
- add `config.rejectCurves` to blacklist a set of ECC curves, to prevent keys
using those curves from being generated, or being used to
encrypt/decrypt/sign/verify messages.
By default, `config.rejectCurves` includes the brainpool curves
(`brainpoolP256r1`, `brainpoolP384r1`, `brainpoolP512r1`) and the Bitcoin curve
(`secp256k1`). This is because it's unclear whether these curves will be
standardised[1], and we prefer to blacklist them already, rather than introduce
a breaking change after release.
[1] https://gitlab.com/openpgp-wg/rfc4880bis/-/merge_requests/47#note_634199141
---
openpgp.d.ts | 13 ++++++
src/config/config.js | 15 ++++--
src/key/helper.js | 30 ++++++++++--
src/key/key.js | 8 ++--
src/openpgp.js | 3 ++
test/crypto/validate.js | 3 +-
test/general/brainpool.js | 88 +++++++++++++++++++++--------------
test/general/config.js | 20 ++++++++
test/general/ecc_secp256k1.js | 11 +++++
test/general/openpgp.js | 7 +--
test/general/streaming.js | 14 ++++--
11 files changed, 158 insertions(+), 54 deletions(-)
diff --git a/openpgp.d.ts b/openpgp.d.ts
index a5972551..1854f88e 100644
--- a/openpgp.d.ts
+++ b/openpgp.d.ts
@@ -337,6 +337,7 @@ interface Config {
rejectHashAlgorithms: Set<enums.hash>;
rejectMessageHashAlgorithms: Set<enums.hash>;
rejectPublicKeyAlgorithms: Set<enums.publicKey>;
+ rejectCurves: Set<enums.curve>;
}
export var config: Config;
@@ -814,6 +815,18 @@ export namespace enums {
aedsa = 24,
}
+ enum curve {
+ p256 = 'p256',
+ p384 = 'p384',
+ p521 = 'p521',
+ ed25519 = 'ed25519',
+ curve25519 = 'curve25519',
+ secp256k1 = 'secp256k1',
+ brainpoolP256r1 = 'brainpoolP256r1',
+ brainpoolP384r1 = 'brainpoolP384r1',
+ brainpoolP512r1 = 'brainpoolP512r1'
+ }
+
export type symmetricNames = 'plaintext' | 'idea' | 'tripledes' | 'cast5' | 'blowfish' | 'aes128' | 'aes192' | 'aes256' | 'twofish';
enum symmetric {
plaintext = 0,
diff --git a/src/config/config.js b/src/config/config.js
index 1d56f6c9..9451063e 100644
--- a/src/config/config.js
+++ b/src/config/config.js
@@ -181,8 +181,11 @@ export default {
*/
knownNotations: ['preferred-email-encoding@pgp.com', 'pka-address@gnupg.org'],
/**
+ * Whether to use the indutny/elliptic library for curves (other than Curve25519) that are not supported by the available native crypto API.
+ * When false, certain standard curves will not be supported (depending on the platform).
+ * Note: the indutny/elliptic curve library is not designed to be constant time.
* @memberof module:config
- * @property {Boolean} useIndutnyElliptic Whether to use the indutny/elliptic library. When false, certain curves will not be supported.
+ * @property {Boolean} useIndutnyElliptic
*/
useIndutnyElliptic: true,
/**
@@ -198,9 +201,15 @@ export default {
*/
rejectMessageHashAlgorithms: new Set([enums.hash.md5, enums.hash.ripemd, enums.hash.sha1]),
/**
- * Reject insecure public key algorithms for message encryption, signing or verification
+ * Reject insecure public key algorithms for key generation and message encryption, signing or verification
* @memberof module:config
* @property {Set<Integer>} rejectPublicKeyAlgorithms {@link module:enums.publicKey}
*/
- rejectPublicKeyAlgorithms: new Set([enums.publicKey.elgamal, enums.publicKey.dsa])
+ rejectPublicKeyAlgorithms: new Set([enums.publicKey.elgamal, enums.publicKey.dsa]),
+ /**
+ * Reject non-standard curves for key generation, message encryption, signing or verification
+ * @memberof module:config
+ * @property {Set<String>} rejectCurves {@link module:enums.curve}
+ */
+ rejectCurves: new Set([enums.curve.brainpoolP256r1, enums.curve.brainpoolP384r1, enums.curve.brainpoolP512r1, enums.curve.secp256k1])
};
diff --git a/src/key/helper.js b/src/key/helper.js
index 6cb48d65..a24d3f1c 100644
--- a/src/key/helper.js
+++ b/src/key/helper.js
@@ -391,13 +391,35 @@ export function isValidDecryptionKeyPacket(signature, config) {
(signature.keyFlags[0] & enums.keyFlags.encryptStorage) !== 0;
}
-export function checkKeyStrength(keyPacket, config) {
+/**
+ * Check key against blacklisted algorithms and minimum strength requirements.
+ * @param {SecretKeyPacket|PublicKeyPacket|
+ * SecretSubkeyPacket|PublicSubkeyPacket} keyPacket
+ * @param {Config} config
+ * @throws {Error} if the key packet does not meet the requirements
+ */
+export function checkKeyRequirements(keyPacket, config) {
const keyAlgo = enums.write(enums.publicKey, keyPacket.algorithm);
if (config.rejectPublicKeyAlgorithms.has(keyAlgo)) {
throw new Error(`${keyPacket.algorithm} keys are considered too weak.`);
}
- const rsaAlgos = new Set([enums.publicKey.rsaEncryptSign, enums.publicKey.rsaSign, enums.publicKey.rsaEncrypt]);
- if (rsaAlgos.has(keyAlgo) && util.uint8ArrayBitLength(keyPacket.publicParams.n) < config.minRSABits) {
- throw new Error(`RSA keys shorter than ${config.minRSABits} bits are considered too weak.`);
+ const algoInfo = keyPacket.getAlgorithmInfo();
+ switch (keyAlgo) {
+ case enums.publicKey.rsaEncryptSign:
+ case enums.publicKey.rsaSign:
+ case enums.publicKey.rsaEncrypt:
+ if (algoInfo.bits < config.minRSABits) {
+ throw new Error(`RSA keys shorter than ${config.minRSABits} bits are considered too weak.`);
+ }
+ break;
+ case enums.publicKey.ecdsa:
+ case enums.publicKey.eddsa:
+ case enums.publicKey.ecdh:
+ if (config.rejectCurves.has(algoInfo.curve)) {
+ throw new Error(`Support for ${keyPacket.algorithm} keys using curve ${algoInfo.curve} is disabled.`);
+ }
+ break;
+ default:
+ break;
}
}
diff --git a/src/key/key.js b/src/key/key.js
index 2bb1bd2f..72dd09b8 100644
--- a/src/key/key.js
+++ b/src/key/key.js
@@ -254,7 +254,7 @@ class Key {
await helper.getLatestValidSignature(
[bindingSignature.embeddedSignature], subkey.keyPacket, enums.signature.keyBinding, dataToVerify, date, config
);
- helper.checkKeyStrength(subkey.keyPacket, config);
+ helper.checkKeyRequirements(subkey.keyPacket, config);
return subkey;
} catch (e) {
exception = e;
@@ -266,7 +266,7 @@ class Key {
const primaryUser = await this.getPrimaryUser(date, userID, config);
if ((!keyID || primaryKey.getKeyID().equals(keyID)) &&
helper.isValidSigningKeyPacket(primaryKey, primaryUser.selfCertification, config)) {
- helper.checkKeyStrength(primaryKey, config);
+ helper.checkKeyRequirements(primaryKey, config);
return this;
}
} catch (e) {
@@ -298,7 +298,7 @@ class Key {
const dataToVerify = { key: primaryKey, bind: subkey.keyPacket };
const bindingSignature = await helper.getLatestValidSignature(subkey.bindingSignatures, primaryKey, enums.signature.subkeyBinding, dataToVerify, date, config);
if (helper.isValidEncryptionKeyPacket(subkey.keyPacket, bindingSignature)) {
- helper.checkKeyStrength(subkey.keyPacket, config);
+ helper.checkKeyRequirements(subkey.keyPacket, config);
return subkey;
}
} catch (e) {
@@ -312,7 +312,7 @@ class Key {
const primaryUser = await this.getPrimaryUser(date, userID, config);
if ((!keyID || primaryKey.getKeyID().equals(keyID)) &&
helper.isValidEncryptionKeyPacket(primaryKey, primaryUser.selfCertification)) {
- helper.checkKeyStrength(primaryKey, config);
+ helper.checkKeyRequirements(primaryKey, config);
return this;
}
} catch (e) {
diff --git a/src/openpgp.js b/src/openpgp.js
index b3f4867e..a77739c1 100644
--- a/src/openpgp.js
+++ b/src/openpgp.js
@@ -21,6 +21,7 @@ import { CleartextMessage } from './cleartext';
import { generate, reformat, getPreferredAlgo } from './key';
import defaultConfig from './config';
import util from './util';
+import { checkKeyRequirements } from './key/helper';
//////////////////////
@@ -63,10 +64,12 @@ export async function generateKey({ userIDs = [], passphrase = '', type = 'ecc',
if (type === 'rsa' && rsaBits < config.minRSABits) {
throw new Error(`rsaBits should be at least ${config.minRSABits}, got: ${rsaBits}`);
}
+
const options = { userIDs, passphrase, type, rsaBits, curve, keyExpirationTime, date, subkeys };
try {
const { key, revocationCertificate } = await generate(options, config);
+ key.getKeys().forEach(({ keyPacket }) => checkKeyRequirements(keyPacket, config));
return {
privateKey: formatObject(key, format, config),
diff --git a/test/crypto/validate.js b/test/crypto/validate.js
index 691572d9..5475d990 100644
--- a/test/crypto/validate.js
+++ b/test/crypto/validate.js
@@ -81,7 +81,8 @@ async function cloneKeyPacket(key) {
}
async function generatePrivateKeyObject(options) {
- const { privateKey } = await openpgp.generateKey({ ...options, userIDs: [{ name: 'Test', email: 'test@test.com' }], format: 'object' });
+ const config = { rejectCurves: new Set() };
+ const { privateKey } = await openpgp.generateKey({ ...options, userIDs: [{ name: 'Test', email: 'test@test.com' }], format: 'object', config });
return privateKey;
}
diff --git a/test/general/brainpool.js b/test/general/brainpool.js
index 30c16b2c..5c0c958c 100644
--- a/test/general/brainpool.js
+++ b/test/general/brainpool.js
@@ -10,12 +10,23 @@ const input = require('./testInputs.js');
const expect = chai.expect;
module.exports = () => (openpgp.config.ci ? describe.skip : describe)('Brainpool Cryptography @lightweight', function () {
- //only x25519 crypto is fully functional in lightbuild
- if (!openpgp.config.useIndutnyElliptic && !util.getNodeCrypto()) {
- before(function() {
+ let rejectCurvesVal;
+ before(function() {
+ //only x25519 crypto is fully functional in lightbuild
+ if (!openpgp.config.useIndutnyElliptic && !util.getNodeCrypto()) {
this.skip(); // eslint-disable-line no-invalid-this
- });
- }
+ }
+ });
+
+ beforeEach(function () {
+ rejectCurvesVal = openpgp.config.rejectCurves;
+ openpgp.config.rejectCurves = new Set();
+ });
+
+ afterEach(function () {
+ openpgp.config.rejectCurves = rejectCurvesVal;
+ });
+
const data = {
romeo: {
id: 'fa3d64c9bcf338bc',
@@ -282,39 +293,46 @@ EJ4QcD/oQ6x1M/8X/iKQCtxZP8RnlrbH7ExkNON5s5g=
function omnibus() {
it('Omnibus BrainpoolP256r1 Test', async function() {
- const testData = input.createSomeMessage();
- const testData2 = input.createSomeMessage();
+ const { rejectCurves } = openpgp.config;
+ openpgp.config.rejectCurves = new Set();
- const { privateKey: hi, publicKey: pubHi } = await openpgp.generateKey({ userIDs: { name: 'Hi', email: 'hi@hel.lo' }, curve: 'brainpoolP256r1', format: 'object' });
- const { privateKey: bye, publicKey: pubBye } = await openpgp.generateKey({ userIDs: { name: 'Bye', email: 'bye@good.bye' }, curve: 'brainpoolP256r1', format: 'object' });
+ try {
+ const testData = input.createSomeMessage();
+ const testData2 = input.createSomeMessage();
- const cleartextMessage = await openpgp.sign({ message: await openpgp.createCleartextMessage({ text: testData }), signingKeys: hi });
- await openpgp.verify({
- message: await openpgp.readCleartextMessage({ cleartextMessage }),
- verificationKeys: pubHi
- }).then(output => expect(output.signatures[0].verified).to.eventually.be.true);
- // Verifying detached signature
- await openpgp.verify({
- message: await openpgp.createMessage({ text: util.removeTrailingSpaces(testData) }),
- verificationKeys: pubHi,
- signature: (await openpgp.readCleartextMessage({ cleartextMessage })).signature
- }).then(output => expect(output.signatures[0].verified).to.eventually.be.true);
+ const { privateKey: hi, publicKey: pubHi } = await openpgp.generateKey({ userIDs: { name: 'Hi', email: 'hi@hel.lo' }, curve: 'brainpoolP256r1', format: 'object' });
+ const { privateKey: bye, publicKey: pubBye } = await openpgp.generateKey({ userIDs: { name: 'Bye', email: 'bye@good.bye' }, curve: 'brainpoolP256r1', format: 'object' });
- // Encrypting and signing
- const encrypted = await openpgp.encrypt({
- message: await openpgp.createMessage({ text: testData2 }),
- encryptionKeys: [pubBye],
- signingKeys: [hi]
- });
- // Decrypting and verifying
- return openpgp.decrypt({
- message: await openpgp.readMessage({ armoredMessage: encrypted }),
- decryptionKeys: bye,
- verificationKeys: [pubHi]
- }).then(async output => {
- expect(output.data).to.equal(testData2);
- await expect(output.signatures[0].verified).to.eventually.be.true;
- });
+ const cleartextMessage = await openpgp.sign({ message: await openpgp.createCleartextMessage({ text: testData }), signingKeys: hi });
+ await openpgp.verify({
+ message: await openpgp.readCleartextMessage({ cleartextMessage }),
+ verificationKeys: pubHi
+ }).then(output => expect(output.signatures[0].verified).to.eventually.be.true);
+ // Verifying detached signature
+ await openpgp.verify({
+ message: await openpgp.createMessage({ text: util.removeTrailingSpaces(testData) }),
+ verificationKeys: pubHi,
+ signature: (await openpgp.readCleartextMessage({ cleartextMessage })).signature
+ }).then(output => expect(output.signatures[0].verified).to.eventually.be.true);
+
+ // Encrypting and signing
+ const encrypted = await openpgp.encrypt({
+ message: await openpgp.createMessage({ text: testData2 }),
+ encryptionKeys: [pubBye],
+ signingKeys: [hi]
+ });
+ // Decrypting and verifying
+ return openpgp.decrypt({
+ message: await openpgp.readMessage({ armoredMessage: encrypted }),
+ decryptionKeys: bye,
+ verificationKeys: [pubHi]
+ }).then(async output => {
+ expect(output.data).to.equal(testData2);
+ await expect(output.signatures[0].verified).to.eventually.be.true;
+ });
+ } finally {
+ openpgp.config.rejectCurves = rejectCurves;
+ }
});
}
diff --git a/test/general/config.js b/test/general/config.js
index 4c8814dd..acaffbeb 100644
--- a/test/general/config.js
+++ b/test/general/config.js
@@ -230,6 +230,15 @@ vAFM3jjrAQDgJPXsv8PqCrLGDuMa/2r6SgzYd03aw/xt1WM6hgUvhQD+J54Z
await expect(openpgp.encrypt({
message, encryptionKeys: [key], config: { rejectPublicKeyAlgorithms: new Set([openpgp.enums.publicKey.ecdh]) }
})).to.be.eventually.rejectedWith(/ecdh keys are considered too weak/);
+
+ await expect(openpgp.encrypt({
+ message, encryptionKeys: [key], config: { rejectCurves: new Set([openpgp.enums.curve.curve25519]) }
+ })).to.be.eventually.rejectedWith(/Support for ecdh keys using curve curve25519 is disabled/);
+
+ const echdEncrypted = await openpgp.encrypt({
+ message, encryptionKeys: [key], config: { rejectCurves: new Set([openpgp.enums.curve.ed25519]) }
+ });
+ expect(echdEncrypted).to.match(/---BEGIN PGP MESSAGE---/);
} finally {
openpgp.config.aeadProtect = aeadProtectVal;
openpgp.config.preferredCompressionAlgorithm = preferredCompressionAlgorithmVal;
@@ -295,6 +304,9 @@ vAFM3jjrAQDgJPXsv8PqCrLGDuMa/2r6SgzYd03aw/xt1WM6hgUvhQD+J54Z
await expect(openpgp.sign({
message, signingKeys: [key], config: { rejectPublicKeyAlgorithms: new Set([openpgp.enums.publicKey.eddsa]) }
})).to.be.eventually.rejectedWith(/eddsa keys are considered too weak/);
+ await expect(openpgp.sign({
+ message, signingKeys: [key], config: { rejectCurves: new Set([openpgp.enums.curve.ed25519]) }
+ })).to.be.eventually.rejectedWith(/Support for eddsa keys using curve ed25519 is disabled/);
});
it('openpgp.verify', async function() {
@@ -339,6 +351,14 @@ vAFM3jjrAQDgJPXsv8PqCrLGDuMa/2r6SgzYd03aw/xt1WM6hgUvhQD+J54Z
};
const { signatures: [sig4] } = await openpgp.verify(opt4);
await expect(sig4.verified).to.be.rejectedWith(/eddsa keys are considered too weak/);
+
+ const opt5 = {
+ message: await openpgp.readMessage({ armoredMessage: signed }),
+ verificationKeys: [key],
+ config: { rejectCurves: new Set([openpgp.enums.curve.ed25519]) }
+ };
+ const { signatures: [sig5] } = await openpgp.verify(opt5);
+ await expect(sig5.verified).to.be.eventually.rejectedWith(/Support for eddsa keys using curve ed25519 is disabled/);
});
describe('detects unknown config property', async function() {
diff --git a/test/general/ecc_secp256k1.js b/test/general/ecc_secp256k1.js
index 893d98c7..69ef38ee 100644
--- a/test/general/ecc_secp256k1.js
+++ b/test/general/ecc_secp256k1.js
@@ -12,6 +12,17 @@ module.exports = () => describe('Elliptic Curve Cryptography for secp256k1 curve
this.skip(); // eslint-disable-line no-invalid-this
});
}
+
+ let rejectCurvesVal;
+ beforeEach(() => {
+ rejectCurvesVal = openpgp.config.rejectCurves;
+ openpgp.config.rejectCurves = new Set();
+ });
+
+ afterEach(() => {
+ openpgp.config.rejectCurves = rejectCurvesVal;
+ });
+
const data = {
romeo: {
id: 'c2b12389b401a43d',
diff --git a/test/general/openpgp.js b/test/general/openpgp.js
index 993b2248..878f94b2 100644
--- a/test/general/openpgp.js
+++ b/test/general/openpgp.js
@@ -3724,10 +3724,11 @@ amnR6g==
const curves = ['secp256k1' , 'p256', 'p384', 'p521', 'curve25519', 'brainpoolP256r1', 'brainpoolP384r1', 'brainpoolP512r1'];
curves.forEach(curve => {
it(`sign/verify with ${curve}`, async function() {
+ const config = { rejectCurves: new Set() };
const plaintext = 'short message';
- const { privateKey: key } = await openpgp.generateKey({ curve, userIDs: { name: 'Alice', email: 'info@alice.com' }, format: 'object' });
- const signed = await openpgp.sign({ signingKeys:[key], message: await openpgp.createCleartextMessage({ text: plaintext }) });
- const verified = await openpgp.verify({ verificationKeys:[key], message: await openpgp.readCleartextMessage({ cleartextMessage: signed }) });
+ const { privateKey: key } = await openpgp.generateKey({ curve, userIDs: { name: 'Alice', email: 'info@alice.com' }, format: 'object', config });
+ const signed = await openpgp.sign({ signingKeys:[key], message: await openpgp.createCleartextMessage({ text: plaintext }), config });
+ const verified = await openpgp.verify({ verificationKeys:[key], message: await openpgp.readCleartextMessage({ cleartextMessage: signed }), config });
expect(await verified.signatures[0].verified).to.be.true;
});
});
diff --git a/test/general/streaming.js b/test/general/streaming.js
index ed5243c8..028dede4 100644
--- a/test/general/streaming.js
+++ b/test/general/streaming.js
@@ -383,11 +383,13 @@ function tests() {
});
try {
+ const config = { rejectCurves: new Set() };
const encrypted = await openpgp.encrypt({
message: await openpgp.createMessage({ binary: data }),
encryptionKeys: pub,
signingKeys: priv,
- format: 'binary'
+ format: 'binary',
+ config
});
expect(stream.isStream(encrypted)).to.equal(expectedType);
@@ -396,7 +398,8 @@ function tests() {
verificationKeys: pub,
decryptionKeys: priv,
message,
- format: 'binary'
+ format: 'binary',
+ config
});
expect(stream.isStream(decrypted.data)).to.equal(expectedType);
const reader = stream.getReader(decrypted.data);
@@ -706,11 +709,13 @@ function tests() {
privateKey: await openpgp.readKey({ armoredKey: brainpoolPriv }),
passphrase: brainpoolPass
});
+ const config = { rejectCurves: new Set() };
const signed = await openpgp.sign({
message: await openpgp.createMessage({ binary: data }),
signingKeys: priv,
- detached: true
+ detached: true,
+ config
});
expect(stream.isStream(signed)).to.equal(expectedType);
const armoredSignature = await stream.readToEnd(signed);
@@ -718,7 +723,8 @@ function tests() {
const verified = await openpgp.verify({
signature,
verificationKeys: pub,
- message: await openpgp.createMessage({ text: 'hello world' })
+ message: await openpgp.createMessage({ text: 'hello world' }),
+ config
});
expect(verified.data).to.equal('hello world');
expect(verified.signatures).to.exist.and.have.length(1);