From d72cece54a913e288020ef19f68238a44a36a3ba Mon Sep 17 00:00:00 2001 From: larabr Date: Mon, 10 Jul 2023 15:23:47 +0200 Subject: [PATCH] Support parsing encrypted key with unknown s2k types or cipher algos (#1658) Such keys are still capable of encryption and signature verification. This change is relevant for forward compatibility of v4 keys encrypted using e.g. argon2. --- openpgp.d.ts | 1 + src/packet/secret_key.js | 94 +++++++++++++++++++++++++++------------- test/general/key.js | 38 ++++++++++++++++ test/general/openpgp.js | 72 ++++++++++++++++++++++++++++++ 4 files changed, 174 insertions(+), 31 deletions(-) diff --git a/openpgp.d.ts b/openpgp.d.ts index f7d86edf..b2c4c4ee 100644 --- a/openpgp.d.ts +++ b/openpgp.d.ts @@ -395,6 +395,7 @@ declare abstract class BaseSecretKeyPacket extends BasePublicKeyPacket { public decrypt(passphrase: string): Promise; // throws on error public validate(): Promise; // throws on error public isDummy(): boolean; + public isMissingSecretKeyMaterial(): boolean; public makeDummy(config?: Config): void; } diff --git a/src/packet/secret_key.js b/src/packet/secret_key.js index 8f9b0219..a0df643a 100644 --- a/src/packet/secret_key.js +++ b/src/packet/secret_key.js @@ -86,6 +86,7 @@ class SecretKeyPacket extends PublicKeyPacket { async read(bytes) { // - A Public-Key or Public-Subkey packet, as described above. let i = await this.readPublicKey(bytes); + const startOfSecretKeyData = i; // - One octet indicating string-to-key usage conventions. Zero // indicates that the secret-key data is not encrypted. 255 or 254 @@ -99,40 +100,47 @@ class SecretKeyPacket extends PublicKeyPacket { i++; } - // - [Optional] If string-to-key usage octet was 255, 254, or 253, a - // one-octet symmetric encryption algorithm. - if (this.s2kUsage === 255 || this.s2kUsage === 254 || this.s2kUsage === 253) { - this.symmetric = bytes[i++]; - - // - [Optional] If string-to-key usage octet was 253, a one-octet - // AEAD algorithm. - if (this.s2kUsage === 253) { - this.aead = bytes[i++]; - } - + try { // - [Optional] If string-to-key usage octet was 255, 254, or 253, a - // string-to-key specifier. The length of the string-to-key - // specifier is implied by its type, as described above. - this.s2k = new S2K(); - i += this.s2k.read(bytes.subarray(i, bytes.length)); + // one-octet symmetric encryption algorithm. + if (this.s2kUsage === 255 || this.s2kUsage === 254 || this.s2kUsage === 253) { + this.symmetric = bytes[i++]; - if (this.s2k.type === 'gnu-dummy') { - return; + // - [Optional] If string-to-key usage octet was 253, a one-octet + // AEAD algorithm. + if (this.s2kUsage === 253) { + this.aead = bytes[i++]; + } + + // - [Optional] If string-to-key usage octet was 255, 254, or 253, a + // string-to-key specifier. The length of the string-to-key + // specifier is implied by its type, as described above. + this.s2k = new S2K(); + i += this.s2k.read(bytes.subarray(i, bytes.length)); + + if (this.s2k.type === 'gnu-dummy') { + return; + } + } else if (this.s2kUsage) { + this.symmetric = this.s2kUsage; } - } else if (this.s2kUsage) { - this.symmetric = this.s2kUsage; - } - // - [Optional] If secret data is encrypted (string-to-key usage octet - // not zero), an Initial Vector (IV) of the same length as the - // cipher's block size. - if (this.s2kUsage) { - this.iv = bytes.subarray( - i, - i + crypto.getCipher(this.symmetric).blockSize - ); + // - [Optional] If secret data is encrypted (string-to-key usage octet + // not zero), an Initial Vector (IV) of the same length as the + // cipher's block size. + if (this.s2kUsage) { + this.iv = bytes.subarray( + i, + i + crypto.getCipher(this.symmetric).blockSize + ); - i += this.iv.length; + i += this.iv.length; + } + } catch (e) { + // if the s2k is unsupported, we still want to support encrypting and verifying with the given key + if (!this.s2kUsage) throw e; // always throw for decrypted keys + this.unparseableKeyMaterial = bytes.subarray(startOfSecretKeyData); + this.isEncrypted = true; } // - Only for a version 5 packet, a four-octet scalar octet count for @@ -168,8 +176,15 @@ class SecretKeyPacket extends PublicKeyPacket { * @returns {Uint8Array} A string of bytes containing the secret key OpenPGP packet. */ write() { - const arr = [this.writePublicKey()]; + const serializedPublicKey = this.writePublicKey(); + if (this.unparseableKeyMaterial) { + return util.concatUint8Array([ + serializedPublicKey, + this.unparseableKeyMaterial + ]); + } + const arr = [serializedPublicKey]; arr.push(new Uint8Array([this.s2kUsage])); const optionalFieldsArr = []; @@ -229,6 +244,18 @@ class SecretKeyPacket extends PublicKeyPacket { return this.isEncrypted === false; } + /** + * Check whether the key includes secret key material. + * Some secret keys do not include it, and can thus only be used + * for public-key operations (encryption and verification). + * Such keys are: + * - GNU-dummy keys, where the secret material has been stripped away + * - encrypted keys with unsupported S2K or cipher + */ + isMissingSecretKeyMaterial() { + return this.unparseableKeyMaterial !== undefined || this.isDummy(); + } + /** * Check whether this is a gnu-dummy key * @returns {Boolean} @@ -249,6 +276,7 @@ class SecretKeyPacket extends PublicKeyPacket { if (this.isDecrypted()) { this.clearPrivateParams(); } + delete this.unparseableKeyMaterial; this.isEncrypted = null; this.keyMaterial = null; this.s2k = new S2K(config); @@ -320,6 +348,10 @@ class SecretKeyPacket extends PublicKeyPacket { return false; } + if (this.unparseableKeyMaterial) { + throw new Error('Key packet cannot be decrypted: unsupported S2K or cipher algo'); + } + if (this.isDecrypted()) { throw new Error('Key packet is already decrypted.'); } @@ -404,7 +436,7 @@ class SecretKeyPacket extends PublicKeyPacket { * Clear private key parameters */ clearPrivateParams() { - if (this.isDummy()) { + if (this.isMissingSecretKeyMaterial()) { return; } diff --git a/test/general/key.js b/test/general/key.js index 8b294959..ed3f10a1 100644 --- a/test/general/key.js +++ b/test/general/key.js @@ -2217,6 +2217,25 @@ UGHMDD0RTiyoiQjvVdCRq3YDQtu38TdIKUurvfjeDjLBfuF1RmED9lCRREqRGwKU =kUWS -----END PGP PUBLIC KEY BLOCK-----`; +// key encrypted with invalid s2kType = 23, to test that it can still be used for encryption/verification +const encryptedKeyUnknownS2K = `-----BEGIN PGP PRIVATE KEY BLOCK----- + +xYYEZJ2H3RYJKwYBBAHaRw8BAQdA3V39Xv0+436Rpn/2UlcnOC1BGprmAlWY +RBKjAq0hAtD+CRcIdHzwqoLa54cAbBOEIgBh7Xa1Qh5wCGAmEVWnAldaqvk+ +NcvUL2bR6AQsGIT6YEihOS3xLKobMOd2XlO5ItQoWnONzkWgzjFvctgnlhmq +I80AwowEEBYKAD4FgmSdh90ECwkHCAmQaBT7gxSTsXwDFQgKBBYAAgECGQEC +mwMCHgEWIQSvRnJTQT6TtdZFk0NoFPuDFJOxfAAAT7kBALmmUEJt5HMAOWiW +7/8y4wllm8zNQ9vbl5Q0cWbeWj/8AP9HDa2rRxHY/37g5zXdmL9f/qNWr9Fk +EBRhLLwusumuDMeLBGSdh90SCisGAQQBl1UBBQEBB0Am2yjjialeIVXHJJ2P +b7KiapCC0mD95F0EFz6zz0l4DgMBCAf+CRcISMdt0OUFCNUABB/OD0UW7MPK +Y3t8RrUTYoiCuhuPRDLOJ5NnMNagVQLt3jQsI8JRjzmYbiTrA/V3iJIEDu5C +NWbnvCM7Hs7+OqPzJPJ2w8J4BBgWCAAqBYJknYfdCZBoFPuDFJOxfAKbDBYh +BK9GclNBPpO11kWTQ2gU+4MUk7F8AADwfwD8CsOVw/3zm1UwUbGUi+fuf6Pr +VFBLG8uc9IiaKann/DYBAJcZNZHRSfpDoV2pUA5EAEi2MdjxkRysFQnYPRAu +0pYO +=rWL8 +-----END PGP PRIVATE KEY BLOCK-----`; + function versionSpecificTests() { it('Preferences of generated key', function() { const testPref = function(key) { @@ -2928,6 +2947,17 @@ module.exports = () => describe('Key', function() { expect(primaryUser).to.exist; }); + it('Parsing and serialization of encrypted key with unknown S2K type (unparseableKeyMaterial)', async function() { + const key = await openpgp.readKey({ armoredKey: encryptedKeyUnknownS2K }); + expect(key.isPrivate()).to.equal(true); + expect(key.isDecrypted()).to.equal(false); + expect(key.getSubkeys()).to.have.length(1); + expect(key.keyPacket.isMissingSecretKeyMaterial()).to.equal(true); + + const expectedSerializedKey = await openpgp.unarmor(encryptedKeyUnknownS2K); + expect(key.write()).to.deep.equal(expectedSerializedKey.data); + }); + it('Parsing V5 public key packet', async function() { // Manually modified from https://gitlab.com/openpgp-wg/rfc4880bis/blob/00b2092/back.mkd#sample-eddsa-key const packetBytes = util.hexToUint8Array(` @@ -3218,6 +3248,14 @@ module.exports = () => describe('Key', function() { await openpgp.readKey({ armoredKey: decryptedKey.armor() }); }); + it('makeDummy() - should work for encrypted keys with unknown s2k (unparseableKeyMaterial)', async function() { + const key = await openpgp.readKey({ armoredKey: encryptedKeyUnknownS2K }); + expect(key.keyPacket.isDummy()).to.be.false; + expect(key.keyPacket.makeDummy()).to.not.throw; + expect(key.keyPacket.isDummy()).to.be.true; + expect(key.keyPacket.unparseableKeyMaterial).to.not.exist; + }); + it('clearPrivateParams() - check that private key can no longer be used', async function() { const key = await openpgp.decryptKey({ privateKey: await openpgp.readKey({ armoredKey: priv_key_rsa }), diff --git a/test/general/openpgp.js b/test/general/openpgp.js index 7d830960..2335664f 100644 --- a/test/general/openpgp.js +++ b/test/general/openpgp.js @@ -1154,6 +1154,31 @@ module.exports = () => describe('OpenPGP.js public api tests', function() { expect(privateKeyMismatchingParams).to.deep.equal(originalKey); }); }); + + it('should fail for encrypted key with unknown s2k (unparseableKeyMaterial)', async function() { + // key encrypted with invalid s2kType = 23, to test that it can still be used for encryption/verification + const encryptedKeyUnknownS2K = await openpgp.readKey({ armoredKey: `-----BEGIN PGP PRIVATE KEY BLOCK----- + +xYYEZJ2H3RYJKwYBBAHaRw8BAQdA3V39Xv0+436Rpn/2UlcnOC1BGprmAlWY +RBKjAq0hAtD+CRcIdHzwqoLa54cAbBOEIgBh7Xa1Qh5wCGAmEVWnAldaqvk+ +NcvUL2bR6AQsGIT6YEihOS3xLKobMOd2XlO5ItQoWnONzkWgzjFvctgnlhmq +I80AwowEEBYKAD4FgmSdh90ECwkHCAmQaBT7gxSTsXwDFQgKBBYAAgECGQEC +mwMCHgEWIQSvRnJTQT6TtdZFk0NoFPuDFJOxfAAAT7kBALmmUEJt5HMAOWiW +7/8y4wllm8zNQ9vbl5Q0cWbeWj/8AP9HDa2rRxHY/37g5zXdmL9f/qNWr9Fk +EBRhLLwusumuDMeLBGSdh90SCisGAQQBl1UBBQEBB0Am2yjjialeIVXHJJ2P +b7KiapCC0mD95F0EFz6zz0l4DgMBCAf+CRcISMdt0OUFCNUABB/OD0UW7MPK +Y3t8RrUTYoiCuhuPRDLOJ5NnMNagVQLt3jQsI8JRjzmYbiTrA/V3iJIEDu5C +NWbnvCM7Hs7+OqPzJPJ2w8J4BBgWCAAqBYJknYfdCZBoFPuDFJOxfAKbDBYh +BK9GclNBPpO11kWTQ2gU+4MUk7F8AADwfwD8CsOVw/3zm1UwUbGUi+fuf6Pr +VFBLG8uc9IiaKann/DYBAJcZNZHRSfpDoV2pUA5EAEi2MdjxkRysFQnYPRAu +0pYO +=rWL8 +-----END PGP PRIVATE KEY BLOCK-----` }); + await expect(openpgp.decryptKey({ + privateKey: encryptedKeyUnknownS2K, + passphrase: 'test' + })).to.be.rejectedWith(/Key packet cannot be decrypted: unsupported S2K or cipher algo/); + }); }); describe('encryptKey - unit tests', function() { @@ -1992,6 +2017,53 @@ aOU= const { data: streamedData } = await openpgp.decrypt({ message: objectMessage, passwords, verificationKeys: privateKey, expectSigned: true, config }); expect(await stream.readToEnd(streamedData)).to.equal(text); }); + + it('should support encrypting with encrypted key with unknown s2k (unparseableKeyMaterial)', async function() { + const originalDecryptedKey = await openpgp.readKey({ armoredKey: `-----BEGIN PGP PRIVATE KEY BLOCK----- + +xVgEZJ2H3RYJKwYBBAHaRw8BAQdA3V39Xv0+436Rpn/2UlcnOC1BGprmAlWY +RBKjAq0hAtAAAQCykslk/kEP7+O9sOsuvgX2Xfz4peQuNo2vD/w4dMZpchKj +zQDCjAQQFgoAPgWCZJ2H3QQLCQcICZBoFPuDFJOxfAMVCAoEFgACAQIZAQKb +AwIeARYhBK9GclNBPpO11kWTQ2gU+4MUk7F8AABPuQEAuaZQQm3kcwA5aJbv +/zLjCWWbzM1D29uXlDRxZt5aP/wA/0cNratHEdj/fuDnNd2Yv1/+o1av0WQQ +FGEsvC6y6a4Mx10EZJ2H3RIKKwYBBAGXVQEFAQEHQCbbKOOJqV4hVccknY9v +sqJqkILSYP3kXQQXPrPPSXgOAwEIBwAA/34s+u8hyLdzdLxjrEEN9zNb+C8d +EyBNxMpyZ/NJsUxoEIPCeAQYFggAKgWCZJ2H3QmQaBT7gxSTsXwCmwwWIQSv +RnJTQT6TtdZFk0NoFPuDFJOxfAAA8H8A/ArDlcP985tVMFGxlIvn7n+j61RQ +SxvLnPSImimp5/w2AQCXGTWR0Un6Q6FdqVAORABItjHY8ZEcrBUJ2D0QLtKW +Dg== +=wiwz +-----END PGP PRIVATE KEY BLOCK-----` }); + // `originalDecryptedKey` encrypted with invalid s2kType = 23, to test that it can still be used for encryption/verification + const encryptedKeyUnknownS2K = await openpgp.readKey({ armoredKey: `-----BEGIN PGP PRIVATE KEY BLOCK----- + +xYYEZJ2H3RYJKwYBBAHaRw8BAQdA3V39Xv0+436Rpn/2UlcnOC1BGprmAlWY +RBKjAq0hAtD+CRcIdHzwqoLa54cAbBOEIgBh7Xa1Qh5wCGAmEVWnAldaqvk+ +NcvUL2bR6AQsGIT6YEihOS3xLKobMOd2XlO5ItQoWnONzkWgzjFvctgnlhmq +I80AwowEEBYKAD4FgmSdh90ECwkHCAmQaBT7gxSTsXwDFQgKBBYAAgECGQEC +mwMCHgEWIQSvRnJTQT6TtdZFk0NoFPuDFJOxfAAAT7kBALmmUEJt5HMAOWiW +7/8y4wllm8zNQ9vbl5Q0cWbeWj/8AP9HDa2rRxHY/37g5zXdmL9f/qNWr9Fk +EBRhLLwusumuDMeLBGSdh90SCisGAQQBl1UBBQEBB0Am2yjjialeIVXHJJ2P +b7KiapCC0mD95F0EFz6zz0l4DgMBCAf+CRcISMdt0OUFCNUABB/OD0UW7MPK +Y3t8RrUTYoiCuhuPRDLOJ5NnMNagVQLt3jQsI8JRjzmYbiTrA/V3iJIEDu5C +NWbnvCM7Hs7+OqPzJPJ2w8J4BBgWCAAqBYJknYfdCZBoFPuDFJOxfAKbDBYh +BK9GclNBPpO11kWTQ2gU+4MUk7F8AADwfwD8CsOVw/3zm1UwUbGUi+fuf6Pr +VFBLG8uc9IiaKann/DYBAJcZNZHRSfpDoV2pUA5EAEi2MdjxkRysFQnYPRAu +0pYO +=rWL8 +-----END PGP PRIVATE KEY BLOCK-----` }); + const encrypted = await openpgp.encrypt({ + message: await openpgp.createMessage({ text: 'test' }), + encryptionKeys: encryptedKeyUnknownS2K + }); + + // decrypt with original key + const decrypted = await openpgp.decrypt({ + message: await openpgp.readMessage({ armoredMessage: encrypted }), + decryptionKeys: originalDecryptedKey + }); + expect(decrypted.data).to.equal('test'); + }); }); describe('encryptSessionKey - unit tests', function() {