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.rejectPublicKeyAlgorithms #1264

Merged
merged 13 commits into from Mar 25, 2021
4 changes: 2 additions & 2 deletions .eslintrc.js
Expand Up @@ -215,7 +215,7 @@ module.exports = {
"no-restricted-imports": "error",
"no-restricted-modules": "error",
"no-restricted-properties": "error",
"no-restricted-syntax": "error",
"no-restricted-syntax": ["error", "ForInStatement", "LabeledStatement", "WithStatement"],
"no-return-assign": "error",
"no-return-await": "error",
"no-script-url": "error",
Expand Down Expand Up @@ -344,7 +344,7 @@ module.exports = {
"no-use-before-define": [ 2, { "functions": false, "classes": true, "variables": false }],
"no-constant-condition": [ 2, { "checkLoops": false } ],
"new-cap": [ 2, { "properties": false, "capIsNewExceptionPattern": "EAX|OCB|GCM|CMAC|CBC|OMAC|CTR", "newIsCapExceptionPattern": "type|hash*"}],
"max-lines": [ 2, { "max": 600, "skipBlankLines": true, "skipComments": true } ],
"max-lines": [ 2, { "max": 620, "skipBlankLines": true, "skipComments": true } ],
"no-unused-expressions": 0,
"chai-friendly/no-unused-expressions": [ 2, { "allowShortCircuit": true } ],

Expand Down
4 changes: 3 additions & 1 deletion src/cleartext.js
Expand Up @@ -77,7 +77,9 @@ export class CleartextMessage {
* @param {Array<Key>} keys - Array of keys to verify signatures
* @param {Date} [date] - Verify the signature against the given date, i.e. check signature creation time < date < expiration time
* @param {Object} [config] - Full configuration, defaults to openpgp.config
* @returns {Array<{keyid: module:type/keyid~Keyid, valid: Boolean}>} List of signer's keyid and validity of signature.
* @returns {Array<{keyid: module:type/keyid~Keyid,
* signature: Promise<Signature>,
* verified: Promise<Boolean>}>} List of signer's keyid and validity of signature.
* @async
*/
verify(keys, date = new Date(), config = defaultConfig) {
Expand Down
18 changes: 13 additions & 5 deletions src/config/config.js
Expand Up @@ -105,7 +105,7 @@ export default {
checksumRequired: false,
/**
* @memberof module:config
* @property {Number} minRsaBits Minimum RSA key size allowed for key generation
* @property {Number} minRsaBits Minimum RSA key size allowed for key generation and message signing, verification and encryption
*/
minRsaBits: 2048,
/**
Expand Down Expand Up @@ -180,13 +180,21 @@ export default {
*/
useIndutnyElliptic: true,
/**
* Reject insecure hash algorithms
* @memberof module:config
* @property {Set<Integer>} reject_hash_algorithms Reject insecure hash algorithms {@link module:enums.hash}
* @property {Set<Integer>} rejectHashAlgorithms {@link module:enums.hash}
*/
rejectHashAlgorithms: new globalThis.Set([enums.hash.md5, enums.hash.ripemd]),
rejectHashAlgorithms: new Set([enums.hash.md5, enums.hash.ripemd]),
/**
* Reject insecure message hash algorithms
* @memberof module:config
* @property {Set<Integer>} reject_message_hash_algorithms Reject insecure message hash algorithms {@link module:enums.hash}
* @property {Set<Integer>} rejectMessageHashAlgorithms {@link module:enums.hash}
*/
rejectMessageHashAlgorithms: new globalThis.Set([enums.hash.md5, enums.hash.ripemd, enums.hash.sha1])
rejectMessageHashAlgorithms: new Set([enums.hash.md5, enums.hash.ripemd, enums.hash.sha1]),
/**
* Reject insecure public key algorithms for 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])
};
40 changes: 20 additions & 20 deletions src/key/factory.js
Expand Up @@ -67,42 +67,42 @@ export async function generate(options, config) {
*/
export async function reformat(options, config) {
options = sanitize(options);
const { privateKey } = options;

if (options.privateKey.primaryKey.isDummy()) {
if (privateKey.isPublic()) {
throw new Error('Cannot reformat a public key');
}

if (privateKey.primaryKey.isDummy()) {
throw new Error('Cannot reformat a gnu-dummy primary key');
}

const isDecrypted = options.privateKey.getKeys().every(({ keyPacket }) => keyPacket.isDecrypted());
const isDecrypted = privateKey.getKeys().every(({ keyPacket }) => keyPacket.isDecrypted());
if (!isDecrypted) {
throw new Error('Key is not decrypted');
}

const packetlist = options.privateKey.toPacketlist();
let secretKeyPacket;
const secretSubkeyPackets = [];
for (let i = 0; i < packetlist.length; i++) {
if (packetlist[i].tag === enums.packet.secretKey) {
secretKeyPacket = packetlist[i];
} else if (packetlist[i].tag === enums.packet.secretSubkey) {
secretSubkeyPackets.push(packetlist[i]);
}
}
if (!secretKeyPacket) {
throw new Error('Key does not contain a secret key packet');
}
const secretKeyPacket = privateKey.keyPacket;

if (!options.subkeys) {
options.subkeys = await Promise.all(secretSubkeyPackets.map(async secretSubkeyPacket => ({
sign: await options.privateKey.getSigningKey(secretSubkeyPacket.getKeyId(), null, undefined, config).catch(() => {}) &&
!await options.privateKey.getEncryptionKey(secretSubkeyPacket.getKeyId(), null, undefined, config).catch(() => {})
})));
options.subkeys = await Promise.all(privateKey.subKeys.map(async subkey => {
const secretSubkeyPacket = subkey.keyPacket;
const dataToVerify = { key: secretKeyPacket, bind: secretSubkeyPacket };
const bindingSignature = await (
helper.getLatestValidSignature(subkey.bindingSignatures, secretKeyPacket, enums.signature.subkeyBinding, dataToVerify, null, config)
).catch(() => ({}));
return {
sign: bindingSignature.keyFlags && (bindingSignature.keyFlags[0] & enums.keyFlags.signData)
};
}));
}

const secretSubkeyPackets = privateKey.subKeys.map(subkey => subkey.keyPacket);
if (options.subkeys.length !== secretSubkeyPackets.length) {
throw new Error('Number of subkey options does not match number of subkeys');
}

options.subkeys = options.subkeys.map(function(subkey, index) { return sanitize(options.subkeys[index], options); });
options.subkeys = options.subkeys.map(subkeyOptions => sanitize(subkeyOptions, options));

return wrapKeyObject(secretKeyPacket, secretSubkeyPackets, options, config);

Expand Down
31 changes: 23 additions & 8 deletions src/key/helper.js
Expand Up @@ -370,9 +370,11 @@ export function isValidSigningKeyPacket(keyPacket, signature) {
if (!signature.verified || signature.revoked !== false) { // Sanity check
throw new Error('Signature not verified');
}
return keyPacket.algorithm !== enums.read(enums.publicKey, enums.publicKey.rsaEncrypt) &&
keyPacket.algorithm !== enums.read(enums.publicKey, enums.publicKey.elgamal) &&
keyPacket.algorithm !== enums.read(enums.publicKey, enums.publicKey.ecdh) &&

const keyAlgo = enums.write(enums.publicKey, keyPacket.algorithm);
larabr marked this conversation as resolved.
Show resolved Hide resolved
return keyAlgo !== enums.publicKey.rsaEncrypt &&
keyAlgo !== enums.publicKey.elgamal &&
keyAlgo !== enums.publicKey.ecdh &&
(!signature.keyFlags ||
(signature.keyFlags[0] & enums.keyFlags.signData) !== 0);
}
Expand All @@ -381,10 +383,12 @@ export function isValidEncryptionKeyPacket(keyPacket, signature) {
if (!signature.verified || signature.revoked !== false) { // Sanity check
throw new Error('Signature not verified');
}
return keyPacket.algorithm !== enums.read(enums.publicKey, enums.publicKey.dsa) &&
keyPacket.algorithm !== enums.read(enums.publicKey, enums.publicKey.rsaSign) &&
keyPacket.algorithm !== enums.read(enums.publicKey, enums.publicKey.ecdsa) &&
keyPacket.algorithm !== enums.read(enums.publicKey, enums.publicKey.eddsa) &&

const keyAlgo = enums.write(enums.publicKey, keyPacket.algorithm);
return keyAlgo !== enums.publicKey.dsa &&
keyAlgo !== enums.publicKey.rsaSign &&
keyAlgo !== enums.publicKey.ecdsa &&
keyAlgo !== enums.publicKey.eddsa &&
(!signature.keyFlags ||
(signature.keyFlags[0] & enums.keyFlags.encryptCommunication) !== 0 ||
(signature.keyFlags[0] & enums.keyFlags.encryptStorage) !== 0);
Expand All @@ -396,11 +400,22 @@ export function isValidDecryptionKeyPacket(signature, config) {
}

if (config.allowInsecureDecryptionWithSigningKeys) {
// This is only relevant for RSA keys, all other signing ciphers cannot decrypt
// This is only relevant for RSA keys, all other signing algorithms cannot decrypt
return true;
}

return !signature.keyFlags ||
(signature.keyFlags[0] & enums.keyFlags.encryptCommunication) !== 0 ||
(signature.keyFlags[0] & enums.keyFlags.encryptStorage) !== 0;
}

export function checkKeyStrength(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.`);
}
}
87 changes: 53 additions & 34 deletions src/key/key.js
Expand Up @@ -291,29 +291,41 @@ class Key {
const primaryKey = this.keyPacket;
const subKeys = this.subKeys.slice().sort((a, b) => b.keyPacket.created - a.keyPacket.created);
let exception;
for (let i = 0; i < subKeys.length; i++) {
if (!keyId || subKeys[i].getKeyId().equals(keyId)) {
for (const subKey of subKeys) {
if (!keyId || subKey.getKeyId().equals(keyId)) {
try {
await subKeys[i].verify(primaryKey, date, config);
const dataToVerify = { key: primaryKey, bind: subKeys[i].keyPacket };
const bindingSignature = await helper.getLatestValidSignature(subKeys[i].bindingSignatures, primaryKey, enums.signature.subkeyBinding, dataToVerify, date, config);
if (
bindingSignature &&
bindingSignature.embeddedSignature &&
helper.isValidSigningKeyPacket(subKeys[i].keyPacket, bindingSignature) &&
await helper.getLatestValidSignature([bindingSignature.embeddedSignature], subKeys[i].keyPacket, enums.signature.keyBinding, dataToVerify, date, config)
) {
return subKeys[i];
await subKey.verify(primaryKey, date, config);
const dataToVerify = { key: primaryKey, bind: subKey.keyPacket };
const bindingSignature = await helper.getLatestValidSignature(
subKey.bindingSignatures, primaryKey, enums.signature.subkeyBinding, dataToVerify, date, config
);
if (!helper.isValidSigningKeyPacket(subKey.keyPacket, bindingSignature)) {
continue;
}
if (!bindingSignature.embeddedSignature) {
throw new Error('Missing embedded signature');
}
// verify embedded signature
await helper.getLatestValidSignature(
[bindingSignature.embeddedSignature], subKey.keyPacket, enums.signature.keyBinding, dataToVerify, date, config
);
helper.checkKeyStrength(subKey.keyPacket, config);
return subKey;
} catch (e) {
exception = e;
}
}
}
const primaryUser = await this.getPrimaryUser(date, userId, config);
if ((!keyId || primaryKey.getKeyId().equals(keyId)) &&
helper.isValidSigningKeyPacket(primaryKey, primaryUser.selfCertification)) {
return this;

try {
const primaryUser = await this.getPrimaryUser(date, userId, config);
if ((!keyId || primaryKey.getKeyId().equals(keyId)) &&
helper.isValidSigningKeyPacket(primaryKey, primaryUser.selfCertification, config)) {
helper.checkKeyStrength(primaryKey, config);
return this;
}
} catch (e) {
exception = e;
}
throw util.wrapError('Could not find valid signing key packet in key ' + this.getKeyId().toHex(), exception);
}
Expand All @@ -333,25 +345,32 @@ class Key {
// V4: by convention subkeys are preferred for encryption service
const subKeys = this.subKeys.slice().sort((a, b) => b.keyPacket.created - a.keyPacket.created);
let exception;
for (let i = 0; i < subKeys.length; i++) {
if (!keyId || subKeys[i].getKeyId().equals(keyId)) {
for (const subKey of subKeys) {
if (!keyId || subKey.getKeyId().equals(keyId)) {
try {
await subKeys[i].verify(primaryKey, date, config);
const dataToVerify = { key: primaryKey, bind: subKeys[i].keyPacket };
const bindingSignature = await helper.getLatestValidSignature(subKeys[i].bindingSignatures, primaryKey, enums.signature.subkeyBinding, dataToVerify, date, config);
if (bindingSignature && helper.isValidEncryptionKeyPacket(subKeys[i].keyPacket, bindingSignature)) {
return subKeys[i];
await subKey.verify(primaryKey, date, config);
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);
return subKey;
}
} catch (e) {
exception = e;
}
}
}
// if no valid subkey for encryption, evaluate primary key
const primaryUser = await this.getPrimaryUser(date, userId, config);
if ((!keyId || primaryKey.getKeyId().equals(keyId)) &&
helper.isValidEncryptionKeyPacket(primaryKey, primaryUser.selfCertification)) {
return this;

try {
// if no valid subkey for encryption, evaluate primary key
const primaryUser = await this.getPrimaryUser(date, userId, config);
if ((!keyId || primaryKey.getKeyId().equals(keyId)) &&
helper.isValidEncryptionKeyPacket(primaryKey, primaryUser.selfCertification)) {
helper.checkKeyStrength(primaryKey, config);
return this;
}
} catch (e) {
exception = e;
}
throw util.wrapError('Could not find valid encryption key packet in key ' + this.getKeyId().toHex(), exception);
}
Expand All @@ -374,7 +393,7 @@ class Key {
try {
const dataToVerify = { key: primaryKey, bind: this.subKeys[i].keyPacket };
const bindingSignature = await helper.getLatestValidSignature(this.subKeys[i].bindingSignatures, primaryKey, enums.signature.subkeyBinding, dataToVerify, date, config);
if (bindingSignature && helper.isValidDecryptionKeyPacket(bindingSignature, config)) {
if (helper.isValidDecryptionKeyPacket(bindingSignature, config)) {
keys.push(this.subKeys[i]);
}
} catch (e) {}
Expand Down Expand Up @@ -486,7 +505,7 @@ class Key {
* It is enough to validate any signing keys
* since its binding signatures are also checked
*/
const signingKey = await this.getSigningKey(null, null, undefined, config);
const signingKey = await this.getSigningKey(null, null, undefined, { ...config, rejectPublicKeyAlgorithms: new Set(), minRsaBits: 0 });
// This could again be a dummy key
if (signingKey && !signingKey.keyPacket.isDummy()) {
signingKeyPacket = signingKey.keyPacket;
Expand Down Expand Up @@ -581,16 +600,16 @@ class Key {
let expiry = keyExpiry < sigExpiry ? keyExpiry : sigExpiry;
if (capabilities === 'encrypt' || capabilities === 'encrypt_sign') {
const encryptKey =
await this.getEncryptionKey(keyId, expiry, userId, config).catch(() => {}) ||
await this.getEncryptionKey(keyId, null, userId, config).catch(() => {});
await this.getEncryptionKey(keyId, expiry, userId, { ...config, rejectPublicKeyAlgorithms: new Set(), minRsaBits: 0 }).catch(() => {}) ||
await this.getEncryptionKey(keyId, null, userId, { ...config, rejectPublicKeyAlgorithms: new Set(), minRsaBits: 0 }).catch(() => {});
if (!encryptKey) return null;
const encryptExpiry = await encryptKey.getExpirationTime(this.keyPacket, undefined, config);
if (encryptExpiry < expiry) expiry = encryptExpiry;
}
if (capabilities === 'sign' || capabilities === 'encrypt_sign') {
const signKey =
await this.getSigningKey(keyId, expiry, userId, config).catch(() => {}) ||
await this.getSigningKey(keyId, null, userId, config).catch(() => {});
await this.getSigningKey(keyId, expiry, userId, { ...config, rejectPublicKeyAlgorithms: new Set(), minRsaBits: 0 }).catch(() => {}) ||
await this.getSigningKey(keyId, null, userId, { ...config, rejectPublicKeyAlgorithms: new Set(), minRsaBits: 0 }).catch(() => {});
if (!signKey) return null;
const signExpiry = await signKey.getExpirationTime(this.keyPacket, undefined, config);
if (signExpiry < expiry) expiry = signExpiry;
Expand Down