From cee14ba99c4e1e53f6e285be57284b719fa3c14a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Thomas=20Obernd=C3=B6rfer?= <thomas@mailvelope.com>
Date: Wed, 6 Mar 2019 09:52:08 +0100
Subject: [PATCH] Extend error logging

---
 src/email/email.js        |  6 +++++-
 src/service/pgp.js        | 19 +++++++++++++++----
 src/service/public-key.js |  9 ++++++---
 test/unit/pgp-test.js     |  4 ++--
 4 files changed, 28 insertions(+), 10 deletions(-)

diff --git a/src/email/email.js b/src/email/email.js
index 7fab760..c932323 100644
--- a/src/email/email.js
+++ b/src/email/email.js
@@ -82,9 +82,13 @@ class Email {
    * @return {string}                    the encrypted PGP message block
    */
   async _pgpEncrypt(plaintext, publicKeyArmored) {
+    const {keys, err} = await openpgp.key.readArmored(publicKeyArmored);
+    if (err) {
+      log.error('email', 'Reading armored key failed.', err, publicKeyArmored);
+    }
     const ciphertext = await openpgp.encrypt({
       message: openpgp.message.fromText(plaintext),
-      publicKeys: (await openpgp.key.readArmored(publicKeyArmored)).keys,
+      publicKeys: keys,
     });
     return ciphertext.data;
   }
diff --git a/src/service/pgp.js b/src/service/pgp.js
index 8808ab3..616d475 100644
--- a/src/service/pgp.js
+++ b/src/service/pgp.js
@@ -53,6 +53,9 @@ class PGP {
     // verify primary key
     const key = r.keys[0];
     const primaryKey = key.primaryKey;
+    if (primaryKey.created > new Date()) {
+      log.error('pgp', 'Key creation date is in the future', primaryKey.created);
+    }
     if (await key.verifyPrimaryKey() !== openpgp.enums.keyStatus.valid) {
       util.throw(400, 'Invalid PGP key: primary key verification failed');
     }
@@ -67,7 +70,7 @@ class PGP {
     // check for at least one valid user id
     const userIds = await this.parseUserIds(key.users, primaryKey);
     if (!userIds.length) {
-      util.throw(400, 'Invalid PGP key: invalid user ids');
+      util.throw(400, 'Invalid PGP key: invalid user IDs');
     }
 
     // get algorithm details from primary key
@@ -119,7 +122,7 @@ class PGP {
    */
   async parseUserIds(users, primaryKey) {
     if (!users || !users.length) {
-      util.throw(400, 'Invalid PGP key: no user id found');
+      util.throw(400, 'Invalid PGP key: no user ID found');
     }
     // at least one user id must be valid, revoked or expired
     const result = [];
@@ -161,8 +164,16 @@ class PGP {
    * @return {String}            merged armored key block
    */
   async updateKey(srcArmored, dstArmored) {
-    const {keys: [srcKey]} = await openpgp.key.readArmored(srcArmored);
-    const {keys: [dstKey]} = await openpgp.key.readArmored(dstArmored);
+    const {keys: [srcKey], err: srcErr} = await openpgp.key.readArmored(srcArmored);
+    if (srcErr) {
+      log.error('pgp', 'Failed to parse source PGP key for update:\n%s', srcArmored, srcErr);
+      util.throw(500, 'Failed to parse PGP key');
+    }
+    const {keys: [dstKey], err: dstErr} = await openpgp.key.readArmored(dstArmored);
+    if (dstErr) {
+      log.error('pgp', 'Failed to parse destination PGP key for update:\n%s', dstArmored, dstErr);
+      util.throw(500, 'Failed to parse PGP key');
+    }
     await dstKey.update(srcKey);
     return dstKey.armor();
   }
diff --git a/src/service/public-key.js b/src/service/public-key.js
index 3c2203d..21ac05f 100644
--- a/src/service/public-key.js
+++ b/src/service/public-key.js
@@ -91,6 +91,9 @@ class PublicKey {
       key.publicKeyArmored = await this._pgp.updateKey(verified.publicKeyArmored, filteredPublicKeyArmored);
     } else {
       key.userIds = key.userIds.filter(userId => userId.status === KEY_STATUS_VALID);
+      if (!key.userIds.length) {
+        util.throw(400, 'Invalid PGP key: no valid user IDs found');
+      }
       await this._addKeyArmored(key.userIds, key.publicKeyArmored);
       // new key, set armored to null
       key.publicKeyArmored = null;
@@ -203,7 +206,7 @@ class PublicKey {
     const query = {keyId, 'userIds.nonce': nonce};
     const key = await this._mongo.get(query, DB_TYPE);
     if (!key) {
-      util.throw(404, 'User id not found');
+      util.throw(404, 'User ID not found');
     }
     await this._removeKeysWithSameEmail(key, nonce);
     let {publicKeyArmored} = key.userIds.find(userId => userId.nonce === nonce);
@@ -312,7 +315,7 @@ class PublicKey {
     // flag user ids for removal
     const key = await this._flagForRemove(keyId, email);
     if (!key) {
-      util.throw(404, 'User id not found');
+      util.throw(404, 'User ID not found');
     }
     // send verification mails
     keyId = key.keyId; // get keyId in case request was by email
@@ -364,7 +367,7 @@ class PublicKey {
     // check if key exists in database
     const flagged = await this._mongo.get({keyId, 'userIds.nonce': nonce}, DB_TYPE);
     if (!flagged) {
-      util.throw(404, 'User id not found');
+      util.throw(404, 'User ID not found');
     }
     if (flagged.userIds.length === 1) {
       // delete the key
diff --git a/test/unit/pgp-test.js b/test/unit/pgp-test.js
index 2b765ae..409747d 100644
--- a/test/unit/pgp-test.js
+++ b/test/unit/pgp-test.js
@@ -87,7 +87,7 @@ describe('PGP Unit Tests', () => {
 
     it('should only accept valid user ids', () => {
       sandbox.stub(pgp, 'parseUserIds').returns([]);
-      return expect(pgp.parseKey(key3Armored)).to.eventually.be.rejectedWith(/invalid user ids/);
+      return expect(pgp.parseKey(key3Armored)).to.eventually.be.rejectedWith(/invalid user IDs/);
     });
 
     it('should be able to parse RSA key', async () => {
@@ -180,7 +180,7 @@ describe('PGP Unit Tests', () => {
     });
 
     it('should throw for an empty user ids array', () =>
-      expect(pgp.parseUserIds([], key.primaryKey)).to.eventually.be.rejectedWith(/no user id/)
+      expect(pgp.parseUserIds([], key.primaryKey)).to.eventually.be.rejectedWith(/no user ID/)
     );
 
     it('should return no user id for an invalid signature', async () => {