Speed up decryptKey and encryptKey (#1192)

Change `openpgp.decryptKey` and `openpgp.encryptKey` to avoid deep cloning the
original key.

openpgp.d.ts

@@ -16,7 +16,7 @@ export function readKeys(data: Uint8Array): Promise<Key[]>;
 export function generateKey(options: KeyOptions): Promise<KeyPair>;
 export function generateSessionKey(options: { publicKeys: Key[], date?: Date, toUserIds?: UserID[] }): Promise<SessionKey>;
 export function decryptKey(options: { privateKey: Key; passphrase?: string | string[]; }): Promise<Key>;
-export function encryptKey(options: { privateKey: Key; passphrase?: string | string[] }): Promise<Key>;
+export function encryptKey(options: { privateKey: Key; passphrase?: string | string[]; }): Promise<Key>;
 export function reformatKey(options: { privateKey: Key; userIds?: UserID|UserID[]; passphrase?: string; keyExpirationTime?: number; }): Promise<KeyPair>;
 
 export class Key {

src/index.js

@@ -9,7 +9,7 @@
  */
 export {
   encrypt, decrypt, sign, verify,
-  generateKey, reformatKey, revokeKey, decryptKey,
+  generateKey, reformatKey, revokeKey, decryptKey, encryptKey,
   generateSessionKey, encryptSessionKey, decryptSessionKeys
 } from './openpgp';
 

src/key/key.js

@@ -166,16 +166,10 @@ class Key {
 
   /**
    * Clones the key object
-   * @param  {Boolean} deep Whether to clone each packet, in addition to the list of packets
+   * @returns {Promise<module:key.Key>} shallow clone of the key
-   * @returns {Promise<module:key.Key>} cloned key
    * @async
    */
-  async clone(deep = false) {
+  async clone() {
-    if (deep) {
-      const packetlist = new PacketList();
-      await packetlist.read(this.toPacketlist().write(), helper.allowedKeyPackets);
-      return new Key(packetlist);
-    }
     return new Key(this.toPacketlist());
   }
 

src/openpgp.js

@@ -171,33 +171,62 @@ export function revokeKey({
 }
 
 /**
- * Unlock a private key with your passphrase.
+ * Unlock a private key with the given passphrase.
- * @param  {Key} privateKey                    the private key that is to be decrypted
+ * This method does not change the original key.
- * @param  {String|Array<String>} passphrase   the user's passphrase(s) chosen during key generation
+ * @param  {Key} privateKey                   the private key to decrypt
- * @returns {Promise<Object>}                  the unlocked key object in the form: { key:Key }
+ * @param  {String|Array<String>} passphrase  the user's passphrase(s)
+ * @returns {Promise<Key>}                    the unlocked key object
  * @async
  */
-export function decryptKey({ privateKey, passphrase }) {
+export async function decryptKey({ privateKey, passphrase }) {
-  return Promise.resolve().then(async function() {
+  const key = await privateKey.clone();
-    const key = await privateKey.clone(true);
+  // shallow clone is enough since the encrypted material is not changed in place by decryption
+  key.getKeys().forEach(k => {
+    k.keyPacket = Object.create(
+      Object.getPrototypeOf(k.keyPacket),
+      Object.getOwnPropertyDescriptors(k.keyPacket)
+    );
+  });
+  try {
     await key.decrypt(passphrase);
     return key;
-  }).catch(onError.bind(null, 'Error decrypting private key'));
+  } catch (err) {
+    key.clearPrivateParams();
+    return onError('Error decrypting private key', err);
+  }
 }
 
 /**
- * Lock a private key with your passphrase.
+ * Lock a private key with the given passphrase.
- * @param  {Key} privateKey                      the private key that is to be decrypted
+ * This method does not change the original key.
- * @param  {String|Array<String>} passphrase     the user's passphrase(s) chosen during key generation
+ * @param  {Key} privateKey                   the private key to encrypt
- * @returns {Promise<Object>}                    the locked key object in the form: { key:Key }
+ * @param  {String|Array<String>} passphrase  if multiple passphrases, they should be in the same order as the packets each should encrypt
+ * @returns {Promise<Key>}                    the locked key object
  * @async
  */
-export function encryptKey({ privateKey, passphrase }) {
+export async function encryptKey({ privateKey, passphrase }) {
-  return Promise.resolve().then(async function() {
+  const key = await privateKey.clone();
-    const key = await privateKey.clone(true);
+  key.getKeys().forEach(k => {
+    // shallow clone the key packets
+    k.keyPacket = Object.create(
+      Object.getPrototypeOf(k.keyPacket),
+      Object.getOwnPropertyDescriptors(k.keyPacket)
+    );
+    if (!k.keyPacket.isDecrypted()) return;
+    // deep clone the private params, which are cleared during encryption
+    const privateParams = {};
+    Object.keys(k.keyPacket.privateParams).forEach(name => {
+      privateParams[name] = new Uint8Array(k.keyPacket.privateParams[name]);
+    });
+    k.keyPacket.privateParams = privateParams;
+  });
+  try {
     await key.encrypt(passphrase);
     return key;
-  }).catch(onError.bind(null, 'Error encrypting private key'));
+  } catch (err) {
+    key.clearPrivateParams();
+    return onError('Error encrypting private key', err);
+  }
 }
 
 

test/general/openpgp.js

@@ -503,6 +503,40 @@ IMq6OV/eCedB8bF4bqoU+zGdGh+XwJkoYVVF6DtG+gIcceHUjC0eXHw=
 -----END PGP PRIVATE KEY BLOCK-----
 `;
 
+const gnuDummyKeySigningSubkey = `
+-----BEGIN PGP PRIVATE KEY BLOCK-----
+Version: OpenPGP.js VERSION
+Comment: https://openpgpjs.org
+
+xZUEWCC+hwEEALu8GwefswqZLoiKJk1Nd1yKmVWBL1ypV35FN0gCjI1NyyJX
+UfQZDdC2h0494OVAM2iqKepqht3tH2DebeFLnc2ivvIFmQJZDnH2/0nFG2gC
+rSySWHUjVfbMSpmTaXpit8EX/rjNauGOdbePbezOSsAhW7R9pBdtDjPnq2Zm
+vDXXABEBAAH+B2UAR05VAc0JR05VIER1bW15wrgEEwECACIFAlggvocCGwMG
+CwkIBwMCBhUIAgkKCwQWAgMBAh4BAheAAAoJEJ3XHFanUJgCeMYD/2zKefpl
+clQoBdDPJKCYJm8IhuWuoF8SnHAsbhD+U42Gbm+2EATTPj0jyGPkZzl7a0th
+S2rSjQ4JF0Ktgdr9585haknpGwr31t486KxXOY4AEsiBmRyvTbaQegwKaQ+C
+/0JQYo/XKpsaX7PMDBB9SNFSa8NkhxYseLaB7gbM8w+Lx8EYBFggvpwBBADF
+YeeJwp6MAVwVwXX/eBRKBIft6LC4E9czu8N2AbOW97WjWNtXi3OuM32OwKXq
+vSck8Mx8FLOAuvVq41NEboeknhptw7HzoQMB35q8NxA9lvvPd0+Ef+BvaVB6
+NmweHttt45LxYxLMdXdGoIt3wn/HBY81HnMqfV/KnggZ+imJ0wARAQABAAP7
+BA56WdHzb53HIzYgWZl04H3BJdB4JU6/FJo0yHpjeWRQ46Q7w2WJzjHS6eBB
+G+OhGzjAGYK7AUr8wgjqMq6LQHt2f80N/nWLusZ00a4lcMd7rvoHLWwRj80a
+RzviOvvhP7kZY1TrhbS+Sl+BWaNIDOxS2maEkxexztt4GEl2dWUCAMoJvyFm
+qPVqVx2Yug29vuJsDcr9XwnjrYI8PtszJI8Fr+5rKgWE3GJumheaXaug60dr
+mLMXdvT/0lj3sXquqR0CAPoZ1Mn7GaUKjPVJ7CiJ/UjqSurrGhruA5ikhehQ
+vUB+v4uIl7ICcX8zfiP+SMhWY9qdkmOvLSSSMcTkguMfe68B/j/qf2en5OHy
+6NJgMIjMrBHvrf34f6pxw5p10J6nxjooZQxV0P+9MoTHWsy0r6Er8IOSSTGc
+WyWJ8wmSqiq/dZSoJcLAfQQYAQIACQUCWCC+nAIbAgCoCRCd1xxWp1CYAp0g
+BBkBAgAGBQJYIL6cAAoJEOYZSGiVA/C9CT4D/2Vq2dKxHmzn/UD1MWSLXUbN
+ISd8tvHjoVg52RafdgHFmg9AbE0DW8ifwaai7FkifD0IXiN04nER3MuVhAn1
+gtMu03m1AQyX/X39tHz+otpwBn0g57NhFbHFmzKfr/+N+XsDRj4VXn13hhqM
+qQR8i1wgiWBUFJbpP5M1BPdH4Qfkcn8D/j8A3QKYGGETa8bNOdVTRU+sThXr
+imOfWu58V1yWCmLE1kK66qkqmgRVUefqacF/ieMqNmsAY+zmR9D4fg2wzu/d
+nPjJXp1670Vlzg7oT5XVYnfys7x4GLHsbaOSjXToILq+3GwI9UjNjtpobcfm
+mNG2ibD6lftLOtDsVSDY8a6a
+=KjxQ
+-----END PGP PRIVATE KEY BLOCK-----
+`;
 
 function withCompression(tests) {
   const compressionTypes = Object.keys(openpgp.enums.compression).map(k => openpgp.enums.compression[k]);
@@ -759,18 +793,26 @@ module.exports = () => describe('OpenPGP.js public api tests', function() {
     });
 
     describe('decryptKey', function() {
-      it('should work for correct passphrase', function() {
+      it('should work for correct passphrase', async function() {
+        const originalKey = await openpgp.readArmoredKey(privateKey.armor());
         return openpgp.decryptKey({
           privateKey: privateKey,
           passphrase: passphrase
         }).then(function(unlocked){
           expect(unlocked.getKeyId().toHex()).to.equal(privateKey.getKeyId().toHex());
+          expect(unlocked.subKeys[0].getKeyId().toHex()).to.equal(privateKey.subKeys[0].getKeyId().toHex());
           expect(unlocked.isDecrypted()).to.be.true;
+          expect(unlocked.keyPacket.privateParams).to.not.be.null;
+          // original key should be unchanged
           expect(privateKey.isDecrypted()).to.be.false;
+          expect(privateKey.keyPacket.privateParams).to.be.null;
+          originalKey.subKeys[0].getKeyId(); // fill in keyid
+          expect(privateKey).to.deep.equal(originalKey);
         });
       });
 
-      it('should fail for incorrect passphrase', function() {
+      it('should fail for incorrect passphrase', async function() {
+        const originalKey = await openpgp.readArmoredKey(privateKey.armor());
         return openpgp.decryptKey({
           privateKey: privateKey,
           passphrase: 'incorrect'
@@ -778,21 +820,103 @@ module.exports = () => describe('OpenPGP.js public api tests', function() {
           throw new Error('Should not decrypt with incorrect passphrase');
         }).catch(function(error){
           expect(error.message).to.match(/Incorrect key passphrase/);
+          // original key should be unchanged
+          expect(privateKey.isDecrypted()).to.be.false;
+          expect(privateKey.keyPacket.privateParams).to.be.null;
+          expect(privateKey).to.deep.equal(originalKey);
         });
       });
 
-      it('should fail for corrupted key', function() {
+      it('should fail for corrupted key', async function() {
+        const originalKey = await openpgp.readArmoredKey(privateKeyMismatchingParams.armor());
         return openpgp.decryptKey({
           privateKey: privateKeyMismatchingParams,
           passphrase: 'userpass'
         }).then(function() {
           throw new Error('Should not decrypt corrupted key');
-        }).catch(function(error){
+        }).catch(function(error) {
           expect(error.message).to.match(/Key is invalid/);
+          expect(privateKeyMismatchingParams.isDecrypted()).to.be.false;
+          expect(privateKeyMismatchingParams.keyPacket.privateParams).to.be.null;
+          expect(privateKeyMismatchingParams).to.deep.equal(originalKey);
         });
       });
     });
 
+    describe('encryptKey', function() {
+      it('should not change original key', async function() {
+        const { privateKeyArmored } = await openpgp.generateKey({ userIds: [{ name: 'test', email: 'test@test.com' }] });
+        // read both keys from armored data to make sure all fields are exactly the same
+        const key = await openpgp.readArmoredKey(privateKeyArmored);
+        const originalKey = await openpgp.readArmoredKey(privateKeyArmored);
+        return openpgp.encryptKey({
+          privateKey: key,
+          passphrase: passphrase
+        }).then(function(locked){
+          expect(locked.getKeyId().toHex()).to.equal(key.getKeyId().toHex());
+          expect(locked.subKeys[0].getKeyId().toHex()).to.equal(key.subKeys[0].getKeyId().toHex());
+          expect(locked.isDecrypted()).to.be.false;
+          expect(locked.keyPacket.privateParams).to.be.null;
+          // original key should be unchanged
+          expect(key.isDecrypted()).to.be.true;
+          expect(key.keyPacket.privateParams).to.not.be.null;
+          originalKey.subKeys[0].getKeyId(); // fill in keyid
+          expect(key).to.deep.equal(originalKey);
+        });
+      });
+
+      it('encrypted key can be decrypted', async function() {
+        const { key } = await openpgp.generateKey({ userIds: [{ name: 'test', email: 'test@test.com' }] });
+        const locked = await openpgp.encryptKey({
+          privateKey: key,
+          passphrase: passphrase
+        });
+        expect(locked.isDecrypted()).to.be.false;
+        const unlocked = await openpgp.decryptKey({
+          privateKey: locked,
+          passphrase: passphrase
+        });
+        expect(unlocked.isDecrypted()).to.be.true;
+      });
+
+      it('should support multiple passphrases', async function() {
+        const { key } = await openpgp.generateKey({ userIds: [{ name: 'test', email: 'test@test.com' }] });
+        const passphrases = ['123', '456'];
+        const locked = await openpgp.encryptKey({
+          privateKey: key,
+          passphrase: passphrases
+        });
+        expect(locked.isDecrypted()).to.be.false;
+        await expect(openpgp.decryptKey({
+          privateKey: locked,
+          passphrase: passphrases[0]
+        })).to.eventually.be.rejectedWith(/Incorrect key passphrase/);
+        const unlocked = await openpgp.decryptKey({
+          privateKey: locked,
+          passphrase: passphrases
+        });
+        expect(unlocked.isDecrypted()).to.be.true;
+      });
+
+      it('should encrypt gnu-dummy key', async function() {
+        const key = await openpgp.readArmoredKey(gnuDummyKeySigningSubkey);
+        const locked = await openpgp.encryptKey({
+          privateKey: key,
+          passphrase: passphrase
+        });
+        expect(key.isDecrypted()).to.be.true;
+        expect(locked.isDecrypted()).to.be.false;
+        expect(locked.primaryKey.isDummy()).to.be.true;
+        const unlocked = await openpgp.decryptKey({
+          privateKey: locked,
+          passphrase: passphrase
+        });
+        expect(key.isDecrypted()).to.be.true;
+        expect(unlocked.isDecrypted()).to.be.true;
+        expect(unlocked.primaryKey.isDummy()).to.be.true;
+      });
+    });
+
     it('Calling decrypt with not decrypted key leads to exception', async function() {
       const encOpt = {
         message: openpgp.Message.fromText(plaintext),