Skip to content

Commit

Permalink
Check key strength and capabilities in separately
Browse files Browse the repository at this point in the history
  • Loading branch information
@larabr
larabr committed Mar 12, 2021
1 parent 5c63453 commit 851edee
Show file tree
Hide file tree
Showing 6 changed files with 70 additions and 65 deletions.
41 changes: 17 additions & 24 deletions src/key/factory.js
View file
Expand Up @@ -67,47 +67,40 @@ 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 => {
const canSign = await options.privateKey.getSigningKey(
secretSubkeyPacket.getKeyId(), null, undefined, { ...config, rejectPublicKeyAlgorithms: new Set(), minRsaBits: 0 }
).catch(() => {});
const canEncrypt = await options.privateKey.getEncryptionKey(
secretSubkeyPacket.getKeyId(), null, undefined, { ...config, rejectPublicKeyAlgorithms: new Set(), minRsaBits: 0 }
).catch(() => {});
return { sign: canSign && !canEncrypt };
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);
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
12 changes: 5 additions & 7 deletions src/key/helper.js
View file
Expand Up @@ -366,27 +366,25 @@ export function sanitizeKeyOptions(options, subkeyDefaults = {}) {
return options;
}

export function isValidSigningKeyPacket(keyPacket, signature, config) {
export function isValidSigningKeyPacket(keyPacket, signature) {
if (!signature.verified || signature.revoked !== false) { // Sanity check
throw new Error('Signature not verified');
}
const keyAlgo = enums.write(enums.publicKey, keyPacket.algorithm);
return checkKeyStrength(keyPacket, config) &&
keyAlgo !== enums.publicKey.rsaEncrypt &&
return keyAlgo !== enums.publicKey.rsaEncrypt &&
keyAlgo !== enums.publicKey.elgamal &&
keyAlgo !== enums.publicKey.ecdh &&
(!signature.keyFlags ||
(signature.keyFlags[0] & enums.keyFlags.signData) !== 0);
}

export function isValidEncryptionKeyPacket(keyPacket, signature, config) {
export function isValidEncryptionKeyPacket(keyPacket, signature) {
if (!signature.verified || signature.revoked !== false) { // Sanity check
throw new Error('Signature not verified');
}

const keyAlgo = enums.write(enums.publicKey, keyPacket.algorithm);
return checkKeyStrength(keyPacket, config) &&
keyAlgo !== enums.publicKey.dsa &&
return keyAlgo !== enums.publicKey.dsa &&
keyAlgo !== enums.publicKey.rsaSign &&
keyAlgo !== enums.publicKey.ecdsa &&
keyAlgo !== enums.publicKey.eddsa &&
Expand All @@ -410,7 +408,7 @@ export function isValidDecryptionKeyPacket(signature, config) {
(signature.keyFlags[0] & enums.keyFlags.encryptStorage) !== 0;
}

function checkKeyStrength(keyPacket, config) {
export function assertKeyStrength(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.`);
Expand Down
62 changes: 38 additions & 24 deletions src/key/key.js
View file
Expand Up @@ -283,20 +283,26 @@ 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, config) &&
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 (!bindingSignature || !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.assertKeyStrength(subKey.keyPacket, config);
return subKey;
} catch (e) {
exception = e;
}
Expand All @@ -305,6 +311,7 @@ class Key {
const primaryUser = await this.getPrimaryUser(date, userId, config);
if ((!keyId || primaryKey.getKeyId().equals(keyId)) &&
helper.isValidSigningKeyPacket(primaryKey, primaryUser.selfCertification, config)) {
helper.assertKeyStrength(primaryKey, config);
return this;
}
throw util.wrapError('Could not find valid signing key packet in key ' + this.getKeyId().toHex(), exception);
Expand All @@ -325,25 +332,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, 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 (bindingSignature && helper.isValidEncryptionKeyPacket(subKey.keyPacket, bindingSignature)) {
helper.assertKeyStrength(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, config)) {
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.assertKeyStrength(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 Down
4 changes: 2 additions & 2 deletions src/message.js
View file
Expand Up @@ -756,12 +756,12 @@ async function createVerificationObject(signature, literalDataList, keys, date =
}
}
if (!primaryKey) {
keyError = new Error(`Could not find key with id ${signature.issuerKeyId.toHex()}`);
keyError = new Error(`Could not find signing key with key ID ${signature.issuerKeyId.toHex()}`);
} else {
try {
signingKey = await primaryKey.getSigningKey(signature.issuerKeyId, null, undefined, config);
} catch (e) {
keyError = new Error(`Key with id ${signature.issuerKeyId.toHex()} is not suitable for verification: ${e.message}`);
keyError = e;
}
}
const signaturePacket = signature.correspondingSig || signature;
Expand Down
14 changes: 7 additions & 7 deletions test/general/openpgp.js
View file
Expand Up @@ -1630,7 +1630,7 @@ module.exports = () => describe('OpenPGP.js public api tests', function() {
}).then(async function ({ signatures, data }) {
expect(data).to.equal(plaintext);
expect(signatures[0].valid).to.be.false;
expect(signatures[0].error).to.match(/Could not find key/);
expect(signatures[0].error).to.match(/Could not find signing key/);
const signingKey = await privateKey.getSigningKey();
expect(signatures[0].keyid.toHex()).to.equal(signingKey.getKeyId().toHex());
expect(signatures[0].signature.packets.length).to.equal(1);
Expand All @@ -1653,7 +1653,7 @@ module.exports = () => describe('OpenPGP.js public api tests', function() {
}).then(async function ({ signatures, data }) {
expect(data).to.equal(plaintext);
expect(signatures[0].valid).to.be.false;
expect(signatures[0].error).to.match(/Could not find key/);
expect(signatures[0].error).to.match(/Could not find signing key/);
const signingKey = await privateKey.getSigningKey();
expect(signatures[0].keyid.toHex()).to.equal(signingKey.getKeyId().toHex());
expect(signatures[0].signature.packets.length).to.equal(1);
Expand All @@ -1676,7 +1676,7 @@ module.exports = () => describe('OpenPGP.js public api tests', function() {
}).then(async function ({ signatures, data }) {
expect(data).to.equal('');
expect(signatures[0].valid).to.be.false;
expect(signatures[0].error).to.match(/Could not find key/);
expect(signatures[0].error).to.match(/Could not find signing key/);
const signingKey = await privateKey.getSigningKey();
expect(signatures[0].keyid.toHex()).to.equal(signingKey.getKeyId().toHex());
expect(signatures[0].signature.packets.length).to.equal(1);
Expand All @@ -1698,7 +1698,7 @@ module.exports = () => describe('OpenPGP.js public api tests', function() {
}).then(async function ({ signatures, data }) {
expect(data).to.equal(plaintext);
expect(signatures[0].valid).to.be.false;
expect(signatures[0].error).to.match(/Could not find key/);
expect(signatures[0].error).to.match(/Could not find signing key/);
const signingKey = await privateKey.getSigningKey();
expect(signatures[0].keyid.toHex()).to.equal(signingKey.getKeyId().toHex());
expect(signatures[0].signature.packets.length).to.equal(1);
Expand All @@ -1723,7 +1723,7 @@ module.exports = () => describe('OpenPGP.js public api tests', function() {
});
expect(data).to.equal(plaintext);
expect(signatures[0].valid).to.be.false;
expect(signatures[0].error).to.match(/Could not find key/);
expect(signatures[0].error).to.match(/Could not find signing key/);
const signingKey = await privateKey.getSigningKey();
expect(signatures[0].keyid.toHex()).to.equal(signingKey.getKeyId().toHex());
expect(signatures[0].signature.packets.length).to.equal(1);
Expand Down Expand Up @@ -2184,7 +2184,7 @@ module.exports = () => describe('OpenPGP.js public api tests', function() {
}).then(async function ({ data, signatures }) {
expect(data).to.equal(plaintext.replace(/[ \t]+$/mg, ''));
expect(signatures[0].valid).to.be.false;
expect(signatures[0].error).to.match(/Could not find key/);
expect(signatures[0].error).to.match(/Could not find signing key/);
const signingKey = await privateKey.getSigningKey();
expect(signatures[0].keyid.toHex()).to.equal(signingKey.getKeyId().toHex());
expect(signatures[0].signature.packets.length).to.equal(1);
Expand All @@ -2208,7 +2208,7 @@ module.exports = () => describe('OpenPGP.js public api tests', function() {
}).then(async function ({ data, signatures }) {
expect(data).to.equal(plaintext);
expect(signatures[0].valid).to.be.false;
expect(signatures[0].error).to.match(/Could not find key/);
expect(signatures[0].error).to.match(/Could not find signing key/);
const signingKey = await privateKey.getSigningKey();
expect(signatures[0].keyid.toHex()).to.equal(signingKey.getKeyId().toHex());
expect(signatures[0].signature.packets.length).to.equal(1);
Expand Down
2 changes: 1 addition & 1 deletion test/general/streaming.js
View file
Expand Up @@ -508,7 +508,7 @@ function tests() {
dataArrived();
await expect(reader.readToEnd()).to.be.rejectedWith('Ascii armor integrity check on message failed');
expect(decrypted.signatures).to.exist.and.have.length(1);
await expect(decrypted.signatures[0].verified).to.be.eventually.rejectedWith(/Could not find key/);
await expect(decrypted.signatures[0].verified).to.be.eventually.rejectedWith(/Could not find signing key/);
} finally {
openpgp.config.allowUnauthenticatedStream = allowUnauthenticatedStreamValue;
}
Expand Down

0 comments on commit 851edee

Please sign in to comment.