Skip to content

Commit

Permalink
Add config.rejectCurves, prevent generation of keys with blacklisted …
Browse files Browse the repository at this point in the history
…public key algo and curves
  • Loading branch information
larabr committed Aug 3, 2021
1 parent 1837077 commit efb2c04
Show file tree
Hide file tree
Showing 9 changed files with 135 additions and 54 deletions.
15 changes: 12 additions & 3 deletions src/config/config.js
Expand Up @@ -175,8 +175,11 @@ export default {
*/
knownNotations: ['preferred-email-encoding@pgp.com', 'pka-address@gnupg.org'],
/**
* Whether to use the indutny/elliptic library for NIST and Brainpool curves that are not supported by the available native crypto library.
* 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,
/**
Expand All @@ -192,9 +195,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])
};
30 changes: 26 additions & 4 deletions src/key/helper.js
Expand Up @@ -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;
}
}
8 changes: 4 additions & 4 deletions src/key/key.js
Expand Up @@ -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;
Expand All @@ -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) {
Expand Down Expand Up @@ -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) {
Expand All @@ -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) {
Expand Down
4 changes: 4 additions & 0 deletions src/openpgp.js
Expand Up @@ -21,6 +21,8 @@ import { CleartextMessage } from './cleartext';
import { generate, reformat, getPreferredAlgo } from './key';
import defaultConfig from './config';
import util from './util';
import enums from './enums';
import { checkKeyRequirements } from './key/helper';


//////////////////////
Expand Down Expand Up @@ -63,10 +65,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);
await checkKeyRequirements(key.keyPacket, config);

return {
privateKey: formatObject(key, format, config),
Expand Down
3 changes: 2 additions & 1 deletion test/crypto/validate.js
Expand Up @@ -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;
}

Expand Down
88 changes: 53 additions & 35 deletions test/general/brainpool.js
Expand Up @@ -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',
Expand Down Expand Up @@ -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;
}
});
}

Expand Down
20 changes: 20 additions & 0 deletions test/general/config.js
Expand Up @@ -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;
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -339,5 +351,13 @@ 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/);
});
});
7 changes: 4 additions & 3 deletions test/general/openpgp.js
Expand Up @@ -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;
});
});
Expand Down
14 changes: 10 additions & 4 deletions test/general/streaming.js
Expand Up @@ -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);

Expand All @@ -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);
Expand Down Expand Up @@ -706,19 +709,22 @@ 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);
const signature = await openpgp.readSignature({ armoredSignature });
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);
Expand Down

0 comments on commit efb2c04

Please sign in to comment.