From 68fba28dd95130e42880112c384319cc4149a894 Mon Sep 17 00:00:00 2001 From: Tankred Hase Date: Fri, 10 Jun 2016 17:47:05 +0200 Subject: [PATCH] Cleanup pgp key parsing --- src/service/pgp.js | 69 +++++++++++++---------- test/unit/pgp-test.js | 127 ++++++++++++++++++++++++++++++++++-------- 2 files changed, 143 insertions(+), 53 deletions(-) diff --git a/src/service/pgp.js b/src/service/pgp.js index 86ae518..b387c18 100644 --- a/src/service/pgp.js +++ b/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 = { - keyId: primaryKey.getKeyId().toHex(), - fingerprint: primaryKey.fingerprint, - userIds: this.parseUserIds(key.getUserIds()), + return { + keyId, + fingerprint, + 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 - * @param {Array} userIds A list of user ids strings - * @return {Array} An array of user id objects + * Parse an array of user ids and verify signatures + * @param {Array} users A list of openpgp.js user objects + * @return {Array} An array of user id objects */ - parseUserIds(userIds) { - if (!userIds.length) { + parseUserIds(users, primaryKey) { + 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; \ No newline at end of file diff --git a/test/unit/pgp-test.js b/test/unit/pgp-test.js index d3bd15b..4607237 100644 --- a/test/unit/pgp-test.js +++ b/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 input = ['a ']; - let parsed = pgp.parseUserIds(input); - expect(parsed[0].name).to.equal('a'); - expect(parsed[0].email).to.equal('b@c.de'); + let key; + + beforeEach(() => { + key = openpgp.key.readArmored(key1Armored).keys[0]; }); it('should parse a valid user id', () => { - let input = [' ']; - let parsed = pgp.parseUserIds(input); - expect(parsed[0].name).to.equal(''); - expect(parsed[0].email).to.equal('b@c.de'); + let parsed = pgp.parseUserIds(key.users, key.primaryKey); + expect(parsed[0].name).to.equal('safewithme testuser'); + expect(parsed[0].email).to.equal('safewithme.testuser@gmail.com'); }); - it('should parse a valid user id', () => { - let input = ['']; - let parsed = pgp.parseUserIds(input); - expect(parsed[0].name).to.equal(''); - expect(parsed[0].email).to.equal('b@c.de'); + it('should throw for an empty user ids array', () => { + expect(pgp.parseUserIds.bind(pgp, [], key.primaryKey)).to.throw(/no user id/); }); - it('should throw for a invalid user id', () => { - let input = ['a <@c.de>']; - expect(pgp.parseUserIds.bind(pgp, input)).to.throw(/invalid user id/); + it('should return no user id for an invalid signature', () => { + key.users[0].userId.userid = 'fake@example.com'; + let parsed = pgp.parseUserIds(key.users, key.primaryKey); + expect(parsed.length).to.equal(0); }); - it('should throw for no user ids', () => { - let input = []; - expect(pgp.parseUserIds.bind(pgp, input)).to.throw(/no user id found/); + it('should throw for a invalid email address', () => { + let verifyStub = sinon.stub(key.users[0], 'isValidSelfCertificate').returns(true); + key.users[0].userId.userid = 'safewithme.testusergmail.com'; + expect(pgp.parseUserIds.bind(pgp, key.users, key.primaryKey)).to.throw(/invalid email address/); + verifyStub.restore(); }); });