Cleanup pgp key parsing

src/service/pgp.js

@@ -47,35 +47,36 @@ class PGP {
       util.throw(400, 'Invalid PGP key: only one key can be uploaded');
     }
 
+    // verify primar key
     let key = r.keys[0];
     let primaryKey = key.primaryKey;
+    if (key.verifyPrimaryKey() !== openpgp.enums.keyStatus.valid) {
+      util.throw(400, 'Invalid PGP key: primary key verification failed');
+    }
+
+    // accept version 4 keys only
+    let keyId = primaryKey.getKeyId().toHex();
+    let fingerprint = primaryKey.fingerprint;
+    if (!util.isKeyId(keyId) || !util.isFingerPrint(fingerprint)) {
+      util.throw(400, 'Invalid PGP key: only v4 keys are accepted');
+    }
+
+    // check for at least one valid user id
+    let userIds = this.parseUserIds(key.users, primaryKey);
+    if (!userIds.length) {
+      util.throw(400, 'Invalid PGP key: invalid user ids');
+    }
 
     // public key document that is stored in the database
-    let keyDoc = {
+    return {
-      keyId: primaryKey.getKeyId().toHex(),
+      keyId,
-      fingerprint: primaryKey.fingerprint,
+      fingerprint,
-      userIds: this.parseUserIds(key.getUserIds()),
+      userIds,
       created: primaryKey.created,
       algorithm: primaryKey.algorithm,
       keySize: primaryKey.getBitSize(),
       publicKeyArmored
     };
-
-    // accept version 4 keys only
-    if (!util.isKeyId(keyDoc.keyId) || !util.isFingerPrint(keyDoc.fingerprint)) {
-      util.throw(400, 'Invalid PGP key: only v4 keys are accepted');
-    }
-
-    // verify user id signatures
-    for (let user of key.users) {
-      for (let cert of user.selfCertifications) {
-        if (!user.isValidSelfCertificate(primaryKey, cert)) {
-          util.throw(400, 'Invalid PGP key: invalid user id signatures');
-        }
-      }
-    }
-
-    return keyDoc;
   }
 
   /**
@@ -105,20 +106,31 @@ class PGP {
   }
 
   /**
-   * Parse an array of user id string to objects
+   * Parse an array of user ids and verify signatures
-   * @param  {Array} userIds   A list of user ids strings
+   * @param  {Array} users   A list of openpgp.js user objects
-   * @return {Array}           An array of user id objects
+   * @return {Array}         An array of user id objects
    */
-  parseUserIds(userIds) {
+  parseUserIds(users, primaryKey) {
-    if (!userIds.length) {
+    if (!users || !users.length) {
       util.throw(400, 'Invalid PGP key: no user id found');
     }
-
+    // at least one user id signature must be valid
     let result = [];
-    userIds.forEach(uid => result = result.concat(addressparser(uid)));
+    for (let user of users) {
+      let oneValid = false;
+      for (let cert of user.selfCertifications) {
+        if (user.isValidSelfCertificate(primaryKey, cert)) {
+          oneValid = true;
+        }
+      }
+      if (oneValid) {
+        result = result.concat(addressparser(user.userId.userid));
+      }
+    }
+    // map to local user id object format
     return result.map(uid => {
       if (!util.isEmail(uid.address)) {
-        util.throw(400, 'Invalid PGP key: invalid user id');
+        util.throw(400, 'Invalid PGP key: invalid email address');
       }
       return {
         name: uid.name,
@@ -127,7 +139,6 @@ class PGP {
       };
     });
   }
-
 }
 
 module.exports = PGP;

test/unit/pgp-test.js

@@ -2,25 +2,95 @@
 
 const fs = require('fs');
 const expect = require('chai').expect;
+const log = require('npmlog');
 const openpgp = require('openpgp');
 const PGP = require('../../src/service/pgp');
-
+const sinon = require('sinon');
 
 describe('PGP Unit Tests', () => {
-  let pgp, key1Armored, key2Armored;
+  let pgp, key1Armored, key2Armored, key3Armored;
 
   beforeEach(() => {
     key1Armored = fs.readFileSync(__dirname + '/../key1.asc', 'utf8');
     key2Armored = fs.readFileSync(__dirname + '/../key2.asc', 'utf8');
-    pgp = new PGP(openpgp);
+    key3Armored = fs.readFileSync(__dirname + '/../key3.asc', 'utf8');
+    pgp = new PGP();
   });
 
   describe('parseKey', () => {
+    it('should should throw error on key parsing', () => {
+      let readStub = sinon.stub(openpgp.key, 'readArmored').returns({err:[new Error()]});
+      sinon.stub(log, 'error');
+      expect(pgp.parseKey.bind(pgp, key3Armored)).to.throw(/Failed to parse/);
+      expect(log.error.calledOnce).to.be.true;
+      log.error.restore();
+      readStub.restore();
+    });
+
+    it('should should throw error when more than one key', () => {
+      let readStub = sinon.stub(openpgp.key, 'readArmored').returns({keys:[{},{}]});
+      expect(pgp.parseKey.bind(pgp, key3Armored)).to.throw(/only one key/);
+      readStub.restore();
+    });
+
+    it('should should throw error when more than one key', () => {
+      let readStub = sinon.stub(openpgp.key, 'readArmored').returns({
+        keys: [{
+          primaryKey: {},
+          verifyPrimaryKey: function() { return false; }
+        }]
+      });
+      expect(pgp.parseKey.bind(pgp, key3Armored)).to.throw(/primary key verification/);
+      readStub.restore();
+    });
+
+    it('should only accept 16 char key id', () => {
+      let readStub = sinon.stub(openpgp.key, 'readArmored').returns({
+        keys: [{
+          primaryKey: {
+            fingerprint: '4277257930867231ce393fb8dbc0b3d92b1b86e9',
+            getKeyId: function() {
+              return {
+                toHex:function() { return 'asdf'; }
+              };
+            }
+          },
+          verifyPrimaryKey: function() { return openpgp.enums.keyStatus.valid; }
+        }]
+      });
+      expect(pgp.parseKey.bind(pgp, key3Armored)).to.throw(/only v4 keys/);
+      readStub.restore();
+    });
+
+    it('should only accept version 4 fingerprint', () => {
+      let readStub = sinon.stub(openpgp.key, 'readArmored').returns({
+        keys: [{
+          primaryKey: {
+            fingerprint: '4277257930867231ce393fb8dbc0b3d92b1b86e',
+            getKeyId: function() {
+              return {
+                toHex:function() { return 'dbc0b3d92b1b86e9'; }
+              };
+            }
+          },
+          verifyPrimaryKey: function() { return openpgp.enums.keyStatus.valid; }
+        }]
+      });
+      expect(pgp.parseKey.bind(pgp, key3Armored)).to.throw(/only v4 keys/);
+      readStub.restore();
+    });
+
+    it('should only accept valid user ids', () => {
+      sinon.stub(pgp, 'parseUserIds').returns([]);
+      expect(pgp.parseKey.bind(pgp, key3Armored)).to.throw(/invalid user ids/);
+    });
+
     it('should be able to parse RSA key', () => {
       let params = pgp.parseKey(key1Armored);
       expect(params.keyId).to.equal('dbc0b3d92b1b86e9');
       expect(params.fingerprint).to.equal('4277257930867231ce393fb8dbc0b3d92b1b86e9');
-      expect(params.userIds.length).to.equal(1);
+      expect(params.userIds[0].name).to.equal('safewithme testuser');
+      expect(params.userIds[0].email).to.equal('safewithme.testuser@gmail.com');
       expect(params.created.getTime()).to.exist;
       expect(params.algorithm).to.equal('rsa_encrypt_sign');
       expect(params.keySize).to.equal(2048);
@@ -37,6 +107,17 @@ describe('PGP Unit Tests', () => {
       expect(params.keySize).to.equal(4096);
       expect(params.publicKeyArmored).to.equal(pgp.trimKey(key2Armored));
     });
+
+    it('should be able to parse komplex key', () => {
+      let params = pgp.parseKey(key3Armored);
+      expect(params.keyId).to.equal('4001a127a90de8e1');
+      expect(params.fingerprint).to.equal('04062c70b446e33016e219a74001a127a90de8e1');
+      expect(params.userIds.length).to.equal(4);
+      expect(params.created.getTime()).to.exist;
+      expect(params.algorithm).to.equal('rsa_encrypt_sign');
+      expect(params.keySize).to.equal(4096);
+      expect(params.publicKeyArmored).to.equal(pgp.trimKey(key3Armored));
+    });
   });
 
   describe('trimKey', () => {
@@ -77,35 +158,33 @@ describe('PGP Unit Tests', () => {
   });
 
   describe('parseUserIds', () => {
-    it('should parse a valid user id', () => {
+    let key;
-      let input = ['a <b@c.de>'];
+
-      let parsed = pgp.parseUserIds(input);
+    beforeEach(() => {
-      expect(parsed[0].name).to.equal('a');
+      key = openpgp.key.readArmored(key1Armored).keys[0];
-      expect(parsed[0].email).to.equal('b@c.de');
     });
 
     it('should parse a valid user id', () => {
-      let input = [' <b@c.de>'];
+      let parsed = pgp.parseUserIds(key.users, key.primaryKey);
-      let parsed = pgp.parseUserIds(input);
+      expect(parsed[0].name).to.equal('safewithme testuser');
-      expect(parsed[0].name).to.equal('');
+      expect(parsed[0].email).to.equal('safewithme.testuser@gmail.com');
-      expect(parsed[0].email).to.equal('b@c.de');
     });
 
-    it('should parse a valid user id', () => {
+    it('should throw for an empty user ids array', () => {
-      let input = ['<b@c.de>'];
+      expect(pgp.parseUserIds.bind(pgp, [], key.primaryKey)).to.throw(/no user id/);
-      let parsed = pgp.parseUserIds(input);
-      expect(parsed[0].name).to.equal('');
-      expect(parsed[0].email).to.equal('b@c.de');
     });
 
-    it('should throw for a invalid user id', () => {
+    it('should return no user id for an invalid signature', () => {
-      let input = ['a <@c.de>'];
+      key.users[0].userId.userid = 'fake@example.com';
-      expect(pgp.parseUserIds.bind(pgp, input)).to.throw(/invalid user id/);
+      let parsed = pgp.parseUserIds(key.users, key.primaryKey);
+      expect(parsed.length).to.equal(0);
     });
 
-    it('should throw for no user ids', () => {
+    it('should throw for a invalid email address', () => {
-      let input = [];
+      let verifyStub = sinon.stub(key.users[0], 'isValidSelfCertificate').returns(true);
-      expect(pgp.parseUserIds.bind(pgp, input)).to.throw(/no user id found/);
+      key.users[0].userId.userid = 'safewithme.testusergmail.com';
+      expect(pgp.parseUserIds.bind(pgp, key.users, key.primaryKey)).to.throw(/invalid email address/);
+      verifyStub.restore();
     });
   });