Skip to content

Commit

Permalink
Allow decryption with revoked keys (#1135)
Browse files Browse the repository at this point in the history
However, when decrypting session keys, check that the public key
algorithm matches that of the decryption key.
  • Loading branch information
larabr authored Aug 18, 2020
1 parent 2e26509 commit cc1bdcb
Show file tree
Hide file tree
Showing 6 changed files with 58 additions and 10 deletions.
9 changes: 9 additions & 0 deletions src/key/helper.js
Original file line number Diff line number Diff line change
Expand Up @@ -361,3 +361,12 @@ export function isValidEncryptionKeyPacket(keyPacket, signature) {
(signature.keyFlags[0] & enums.keyFlags.encrypt_communication) !== 0 ||
(signature.keyFlags[0] & enums.keyFlags.encrypt_storage) !== 0);
}

export function isValidDecryptionKeyPacket(signature) {
if (!signature.verified) { // Sanity check
throw new Error('Signature not verified');
}
return !signature.keyFlags ||
(signature.keyFlags[0] & enums.keyFlags.encrypt_communication) !== 0 ||
(signature.keyFlags[0] & enums.keyFlags.encrypt_storage) !== 0;
}
6 changes: 2 additions & 4 deletions src/key/key.js
Original file line number Diff line number Diff line change
Expand Up @@ -349,16 +349,14 @@ Key.prototype.getEncryptionKey = async function(keyId, date = new Date(), userId
* @async
*/
Key.prototype.getDecryptionKeys = async function(keyId, date = new Date(), userId = {}) {
await this.verifyPrimaryKey(date, userId);
const primaryKey = this.keyPacket;
const keys = [];
for (let i = 0; i < this.subKeys.length; i++) {
if (!keyId || this.subKeys[i].getKeyId().equals(keyId, true)) {
try {
await this.subKeys[i].verify(primaryKey, date);
const dataToVerify = { key: primaryKey, bind: this.subKeys[i].keyPacket };
const bindingSignature = await helper.getLatestValidSignature(this.subKeys[i].bindingSignatures, primaryKey, enums.signature.subkey_binding, dataToVerify, date);
if (bindingSignature && helper.isValidEncryptionKeyPacket(this.subKeys[i].keyPacket, bindingSignature)) {
if (bindingSignature && helper.isValidDecryptionKeyPacket(bindingSignature)) {
keys.push(this.subKeys[i]);
}
} catch (e) {}
Expand All @@ -368,7 +366,7 @@ Key.prototype.getDecryptionKeys = async function(keyId, date = new Date(), userI
// evaluate primary key
const primaryUser = await this.getPrimaryUser(date, userId);
if ((!keyId || primaryKey.getKeyId().equals(keyId, true)) &&
helper.isValidEncryptionKeyPacket(primaryKey, primaryUser.selfCertification)) {
helper.isValidDecryptionKeyPacket(primaryUser.selfCertification)) {
keys.push(this);
}

Expand Down
6 changes: 3 additions & 3 deletions src/key/subkey.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,9 @@ SubKey.prototype.isRevoked = async function(primaryKey, signature, key, date = n
* Verify subkey. Checks for revocation signatures, expiration time
* and valid binding signature. Throws if the subkey is invalid.
* @param {module:packet.SecretKey|
* module:packet.PublicKey} primaryKey The primary key packet
* @param {Date} date Use the given date instead of the current time
* @returns {Promise<true>} The status of the subkey
* module:packet.PublicKey} primaryKey The primary key packet
* @param {Date} date Use the given date instead of the current time
* @returns {Promise<undefined>}
* @async
*/
SubKey.prototype.verify = async function(primaryKey, date = new Date()) {
Expand Down
5 changes: 5 additions & 0 deletions src/packet/public_key_encrypted_session_key.js
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,11 @@ PublicKeyEncryptedSessionKey.prototype.encrypt = async function (key) {
*/
PublicKeyEncryptedSessionKey.prototype.decrypt = async function (key) {
const algo = enums.write(enums.publicKey, this.publicKeyAlgorithm);
const keyAlgo = enums.write(enums.publicKey, key.algorithm);
// check that session key algo matches the secret key algo
if (algo !== keyAlgo) {
throw new Error('Decryption error');
}
const decoded = await crypto.publicKeyDecrypt(algo, key.params, this.encrypted, key.getFingerprintBytes());
const checksum = util.str_to_Uint8Array(decoded.substr(decoded.length - 2));
key = util.str_to_Uint8Array(decoded.substring(1, decoded.length - 2));
Expand Down
36 changes: 36 additions & 0 deletions test/general/openpgp.js
Original file line number Diff line number Diff line change
Expand Up @@ -2428,6 +2428,42 @@ describe('OpenPGP.js public api tests', function() {
});
});

it('should decrypt with revoked subkey', async function() {
const pubKeyDE = (await openpgp.key.readArmored(pub_key_de)).keys[0];
const privKeyDE = (await openpgp.key.readArmored(priv_key_de)).keys[0];
await privKeyDE.decrypt(passphrase);
const encrypted = await openpgp.encrypt({
message: openpgp.message.fromText(plaintext),
publicKeys: pubKeyDE
});
privKeyDE.subKeys[0] = await privKeyDE.subKeys[0].revoke(privKeyDE.primaryKey);
const decOpt = {
message: await openpgp.message.readArmored(encrypted.data),
privateKeys: privKeyDE
};
const decrypted = await openpgp.decrypt(decOpt);
expect(decrypted.data).to.equal(plaintext);
});

it('should not decrypt with corrupted subkey', async function() {
const pubKeyDE = (await openpgp.key.readArmored(pub_key_de)).keys[0];
const privKeyDE = (await openpgp.key.readArmored(priv_key_de)).keys[0];
// corrupt the public key params
privKeyDE.subKeys[0].keyPacket.params[0].data[0]++;
// validation will not check the decryption subkey and will succeed
await privKeyDE.decrypt(passphrase);
const encrypted = await openpgp.encrypt({
message: openpgp.message.fromText(plaintext),
publicKeys: pubKeyDE
});
const decOpt = {
message: await openpgp.message.readArmored(encrypted.data),
privateKeys: privKeyDE
};
// binding signature is invalid
await expect(openpgp.decrypt(decOpt)).to.be.rejectedWith(/Session key decryption failed/);
});

it('should decrypt with two passwords message which GPG fails on', async function() {
const decOpt = {
message: await openpgp.message.readArmored(twoPasswordGPGFail),
Expand Down
6 changes: 3 additions & 3 deletions test/general/packet.js
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,7 @@ describe("Packet", function() {
const msg2 = new openpgp.packet.List();

enc.sessionKey = new Uint8Array([1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2]);
enc.publicKeyAlgorithm = 'rsa_encrypt';
enc.publicKeyAlgorithm = 'rsa_encrypt_sign';
enc.sessionKeyAlgorithm = 'aes256';
enc.publicKeyId.bytes = '12345678';
return enc.encrypt({ params: mpi, getFingerprintBytes() {} }).then(async () => {
Expand All @@ -339,7 +339,7 @@ describe("Packet", function() {

await msg2.read(msg.write());

return msg2[0].decrypt({ params: mpi, getFingerprintBytes() {} }).then(() => {
return msg2[0].decrypt({ algorithm: 'rsa_encrypt_sign', params: mpi, getFingerprintBytes() {} }).then(() => {

expect(stringify(msg2[0].sessionKey)).to.equal(stringify(enc.sessionKey));
expect(msg2[0].sessionKeyAlgorithm).to.equal(enc.sessionKeyAlgorithm);
Expand Down Expand Up @@ -379,7 +379,7 @@ describe("Packet", function() {
const secret = new Uint8Array([1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,0,1,2]);

enc.sessionKey = secret;
enc.publicKeyAlgorithm = 'rsa_encrypt';
enc.publicKeyAlgorithm = 'rsa_encrypt_sign';
enc.sessionKeyAlgorithm = 'aes256';
enc.publicKeyId.bytes = '12345678';

Expand Down

0 comments on commit cc1bdcb

Please sign in to comment.