Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add config.rejectCurves and prevent generating keys using blacklisted algos #1395

Merged
merged 4 commits into from Aug 19, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
13 changes: 13 additions & 0 deletions openpgp.d.ts
Expand Up @@ -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;

Expand Down Expand Up @@ -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,
Expand Down
15 changes: 12 additions & 3 deletions src/config/config.js
Expand Up @@ -179,8 +179,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,
/**
Expand All @@ -196,9 +199,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])
};
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
3 changes: 3 additions & 0 deletions src/openpgp.js
Expand Up @@ -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';


//////////////////////
Expand Down Expand Up @@ -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),
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,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() {
Expand Down
11 changes: 11 additions & 0 deletions test/general/ecc_secp256k1.js
Expand Up @@ -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',
Expand Down
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