From 4a5b8cc003f603acc050cfa65f514a9ff55f1625 Mon Sep 17 00:00:00 2001 From: Simon Vareille Date: Fri, 12 Jun 2020 21:21:29 +0200 Subject: [PATCH] Prevention of flooding keys with signatures Manage unusual signatures. --- locales/en.json | 4 +- src/app/index.js | 2 +- src/email/templates.js | 10 ++-- src/route/rest.js | 55 ++++++++++++++++-- src/service/pgp.js | 42 +++++++++++++- src/service/public-key.js | 115 +++++++++++++++++++++++++++++++------- 6 files changed, 192 insertions(+), 36 deletions(-) diff --git a/locales/en.json b/locales/en.json index 6f30ed6..4e88879 100644 --- a/locales/en.json +++ b/locales/en.json @@ -7,6 +7,6 @@ "verify_removal_subject": "Verify key removal", "verify_removal_text": "Hello {0},\n\nplease verify removal of your email address {1} from our key server ({2}) by clicking on the following link:\n\n{3}\n\nGreetings from the Mailvelope Team", "removal_success": "Email address {0} removed from the key directory", - "confirm_signatures_subject": "Confirm new signatures", - "confirm_signatures_text": "Hello {0},\n\n{1} new signature(s) have been uploaded to your keys on our keyserver ({3}) ! Please select the ones you want to add by clicking on the following link:\n\n{2}\n\nGreetings from the Mailvelope Team" + "check_signatures_subject": "Confirm new signatures", + "check_signatures_text": "Hello {0},\n\n{1} new signature(s) have been uploaded to your keys on our keyserver ({3}) ! Please select the ones you want to add by clicking on the following link:\n\n{2}\n\nGreetings from the Mailvelope Team" } diff --git a/src/app/index.js b/src/app/index.js index 7ce71c8..b0c59f6 100644 --- a/src/app/index.js +++ b/src/app/index.js @@ -62,7 +62,7 @@ router.get('/manage.html', ctx => ctx.render('manage')); // HKP and REST api routes router.post('/pks/add', ctx => hkp.add(ctx)); router.get('/pks/lookup', ctx => hkp.lookup(ctx)); -router.post('/api/v1/key', ctx => rest.create(ctx)); +router.post('/api/v1/key', ctx => rest.postHandler(ctx)); router.get('/api/v1/key', ctx => rest.query(ctx)); router.del('/api/v1/key', ctx => rest.remove(ctx)); diff --git a/src/email/templates.js b/src/email/templates.js index 87f5217..3462f41 100644 --- a/src/email/templates.js +++ b/src/email/templates.js @@ -18,12 +18,12 @@ function verifyRemove(ctx, {name, email, nonce, origin, keyId}) { }; } -function confirmNewSigs(ctx, {name, sigsNb, nonce, origin, keyId}) { - const link = `${util.url(origin)}/api/v1/key?op=confirmSignatures&keyId=${keyId}&nonce=${nonce}`; +function checkNewSigs(ctx, {name, sigsNb, nonce, origin, keyId}) { + const link = `${util.url(origin)}/api/v1/key?op=checkSignatures&keyId=${keyId}&nonce=${nonce}`; return { - subject: ctx.__('confirm_signatures_subject'), - text: ctx.__('confirm_signatures_text', [name, sigsNb, link, origin.host]) + subject: ctx.__('check_signatures_subject'), + text: ctx.__('check_signatures_text', [name, sigsNb, link, origin.host]) }; } -module.exports = {verifyKey, verifyRemove, confirmNewSigs}; +module.exports = {verifyKey, verifyRemove, checkNewSigs}; diff --git a/src/route/rest.js b/src/route/rest.js index 04f51ed..4bf1fba 100644 --- a/src/route/rest.js +++ b/src/route/rest.js @@ -32,13 +32,26 @@ class REST { constructor(publicKey) { this._publicKey = publicKey; } - + + /** + * http POST handler + * @param {Object} ctx The koa request/response context + */ + async postHandler(ctx) { + const json = await parse.json(ctx, {limit: '1mb'}); + if(json.op === 'confirmSignatures') + return this[json.op](ctx, json);//delegate operation + + await this.create(ctx, json); + } + /** * Public key / user ID upload via http POST * @param {Object} ctx The koa request/response context + * @param {Object} json The json content of the request */ - async create(ctx) { - const {emails, publicKeyArmored} = await parse.json(ctx, {limit: '1mb'}); + async create(ctx, json) { + const {emails, publicKeyArmored} = json || await parse.json(ctx, {limit: '1mb'}); if (!publicKeyArmored) { ctx.throw(400, 'Invalid request!'); } @@ -54,7 +67,8 @@ class REST { */ async query(ctx) { const op = ctx.query.op; - if (op === 'verify' || op === 'verifyRemove') { + if (op === 'verify' || op === 'verifyRemove' || op === 'confirmSignatures' || + op === 'checkSignatures') { return this[op](ctx); // delegate operation } // do READ if no 'op' provided @@ -79,7 +93,38 @@ class REST { const link = util.url(util.origin(ctx), `/pks/lookup?op=get&search=${email}`); await ctx.render('verify-success', {email, link}); } - + + /** + * Check public key's signatures via http GET + * @param {Object} ctx The koa request/response context + */ + async checkSignatures(ctx) { + const q = {keyId: ctx.query.keyId, nonce: ctx.query.nonce}; + if (!util.isKeyId(q.keyId) || !util.isString(q.nonce)) { + ctx.throw(400, 'Invalid request!'); + } + + const sigs = await this._publicKey.getPendingSignatures(q, ctx); + // create link for confirmation + const link = util.url(util.origin(ctx), `/api/v1/key`); + await ctx.render('verify-certs', {keyId: q.keyId, link, nonce: q.nonce, sigs}); + } + + /** + * Confirm public key's signatures via http POST + * @param {Object} ctx The koa request/response context + * @param {Object} json The json content of the request + */ + async confirmSignatures(ctx, json) { + const post = json || await parse.json(ctx, {limit: '1mb'}); + const q = {keyId: post.keyId, nonce: post.nonce, sigs: post.sig}; + const {email} = await this._publicKey.verifySignatures(q, util.origin(ctx), ctx); + // create link for sharing + const link = util.url(util.origin(ctx), `/pks/lookup?op=get&search=${email}`); + ctx.body = `Update successful. You can find your key here.`; + ctx.status = 201; + } + /** * Request public key removal via http DELETE * @param {Object} ctx The koa request/response context diff --git a/src/service/pgp.js b/src/service/pgp.js index 4bdcbe2..0cd7648 100644 --- a/src/service/pgp.js +++ b/src/service/pgp.js @@ -178,8 +178,8 @@ class PGP { /** * Remove signatures from source armored key which are not in compared armored key * @param {String} srcArmored armored key block to be filtered - * @param {String} cmpArmored armored key block to be compare with - * @return {String, newSigs} filterd armored key block, list of new signatures + * @param {String} cmpArmored armored key block to be compared with + * @return {String, newSigs} filtered {armored key block, list of new signatures} */ async filterKeyBySignatures(srcArmored, cmpArmored) { const {keys: [srcKey], err: srcErr} = await openpgp.key.readArmored(srcArmored); @@ -210,7 +210,7 @@ class PGP { // list new signatures let userId = (srcUser.userId) ? srcUser.userId.userid : null; let userAttribute = (srcUser.userAttribute) ? srcUser.userAttribute : null; - newSigs.push({user: {userId: userId, userAttribute: userAttribute}, signature: sourceSig}); + newSigs.push({user: {userId: userId, userAttribute: userAttribute}, signature: Buffer.from(sourceSig.write()).toString('base64')}); // do not add new signatures source.splice(i, 1); } @@ -243,6 +243,42 @@ class PGP { return dstKey.armor(); } + /** + * Add new signature to key + * @param {String} publicKeyArmored source amored key block + * @param {Object} signature signature to add + * @return {String} updated armored key block + */ + async addSignature(publicKeyArmored, {user, signature}) { + const {keys: [key], err: srcErr} = await openpgp.key.readArmored(publicKeyArmored); + const signaturePacket = await this.getSignatureFromBase64(signature); + if (srcErr) { + log.error('pgp', 'Failed to parse source PGP key for update:\n%s', publicKeyArmored, srcErr); + util.throw(500, 'Failed to parse PGP key'); + } + for(const srcUser of key.users) { + if((srcUser.userId && user.userId === srcUser.userId.userid) || + (user.userAttribute && user.userAttribute === srcUser.userAttribute)) { + if(!srcUser.otherCertifications.some(certSig => util.equalsUint8Array(certSig.signature, signaturePacket.signature))) { + srcUser.otherCertifications.push(signaturePacket); + } + } + } + + return key.armor(); + } + + /** + * Get openpgp.packet.Signature object from base64 encoded signature + * @param {String} signature base64 encoded signature + * @return {openpgp.packet.Signature} Signature object + */ + async getSignatureFromBase64(signature) { + const signaturePacket = new openpgp.packet.Signature(); + signaturePacket.read(new Uint8Array(Buffer.from(signature, 'base64'))); + return signaturePacket; + } + /** * Returns primary user and most significant (latest valid) self signature * - if multiple primary users exist, returns the one with the latest self signature diff --git a/src/service/public-key.js b/src/service/public-key.js index 41aecc1..2895e8a 100644 --- a/src/service/public-key.js +++ b/src/service/public-key.js @@ -20,6 +20,7 @@ const config = require('config'); const util = require('./util'); const tpl = require('../email/templates'); +const crypto = require('crypto'); /** * Database documents have the format: @@ -96,13 +97,12 @@ class PublicKey { key.publicKeyArmored = await this._pgp.updateKey(verified.publicKeyArmored, filteredPublicKeyArmored); // store pending signatures in key and generate nounce for confirmation if(newSigs.length) { - await this._formatArrays(newSigs); if(!verified.pendingSignatures) key.pendingSignatures = {sigs: newSigs, nonce: util.random()}; else { key.pendingSignatures = verified.pendingSignatures; key.pendingSignatures.sigs = verified.pendingSignatures.sigs.concat(newSigs.filter(sourceSig => !verified.pendingSignatures.sigs.some(function(pendingSig) { - return pendingSig.signature.signature === sourceSig.signature.signature; + return pendingSig.signature === sourceSig.signature; }))); } } @@ -188,22 +188,6 @@ class PublicKey { return users.find(({email}) => email === user.email); } - /** - * Convert all Uint8Array in every signatures to base64. - * @param {Array} signatures list of signatures to convert - * @return {Promise} - */ - async _formatArrays(signatures) { - signatures.map(function(sig) { - const signature = sig.signature; - const attributes = ['signatureData', 'unhashedSubpackets', 'signedHashValue', 'preferredSymmetricAlgorithms', 'revocationKeyFingerprint', 'preferredHashAlgorithms', 'preferredCompressionAlgorithms', 'keyServerPreferences', 'keyFlags', 'features', 'issuerFingerprint', 'preferredAeadAlgorithms', 'signature']; - for (const attrib of attributes) { - if(signature[attrib] != null) - signature[attrib] = Buffer.from(signature[attrib]).toString('base64'); - } - }); - } - /** * Send verification emails to the public keys user ids for verification. * If a primary email address is provided only one email will be sent. @@ -241,7 +225,7 @@ class PublicKey { } /** - * Send confirmation email to the public keys primary user ids for confirmation + * Send email to the public key's primary user ids for confirmation * of new signatures addition. * @param {Object} key key documents containg all the needed data * @param {Object} origin the server's origin (required for email links) @@ -252,7 +236,7 @@ class PublicKey { if(key.pendingSignatures && key.pendingSignatures.sigs.length){ let primaryUser = await this._pgp.getPrimaryUser(key.publicKeyArmored); const userId = primaryUser.user.userId; - await this._email.send({template: tpl.confirmNewSigs.bind(null, ctx), userId, keyId: key.keyId, data: {name: userId.name, sigsNb: key.pendingSignatures.sigs.length, nonce: key.pendingSignatures.nonce}, origin, publicKeyArmored: key.publicKeyArmored}); + await this._email.send({template: tpl.checkNewSigs.bind(null, ctx), userId, keyId: key.keyId, data: {name: userId.name, sigsNb: key.pendingSignatures.sigs.length, nonce: key.pendingSignatures.nonce}, origin, publicKeyArmored: key.publicKeyArmored}); } } @@ -341,6 +325,46 @@ class PublicKey { }, DB_TYPE); return {email}; } + + /** + * Verify signatures by proving knowledge of the nonce. + * @param {string} keyId Correspronding public key id + * @param {string} nonce The verification nonce proving email address ownership + * @param {Array} sigs The list of signatures to verify + * @param {Object} origin The server's origin (required for email links) + * @param {Object} ctx Context + * @return {Promise} The email that has been verified + */ + async verifySignatures({keyId, nonce, sigs}, origin, ctx) { + // look for verification nonce in database + const query = {keyId, 'pendingSignatures.nonce': nonce}; + const key = await this._mongo.get(query, DB_TYPE); + if (!key) { + util.throw(404, 'Signatures not found on key'); + } + + let publicKeyArmored = key.publicKeyArmored; + + for(const {user, signature} of key.pendingSignatures.sigs) { + // update armored key + let hash = crypto.createHash('md5'); + hash.update(signature, 'base64'); + hash = hash.digest('hex'); + if(sigs.includes(hash)) { + publicKeyArmored = await this._pgp.addSignature(key.publicKeyArmored, {user, signature}); + publicKeyArmored = await this._pgp.updateKey(key.publicKeyArmored, publicKeyArmored); + } + } + + key.pendingSignatures = null; + + await this._mongo.update(query, { + publicKeyArmored, + 'pendingSignatures': null + }, DB_TYPE); + const email = (await this._pgp.getPrimaryUser(publicKeyArmored)).user.userId.email; + return {email}; + } /** * Removes keys with the same email address @@ -423,6 +447,57 @@ class PublicKey { return key; } + /** + * Fetch all pending signatures of a public key from the database. Either the + * key fingerprint, id or the email address muss be provided. + * @param {string} keyId Correspronding public key id + * @param {string} nonce The verification nonce proving legitimity of the request + * @param {Object} ctx Context + * @return {Map} The list of userId and associated signatures + */ + async getPendingSignatures({fingerprint, keyId, email, nonce}, ctx) { + // look for verified key + const userIds = email ? [{email}] : undefined; + const key = await this.getVerified({keyId, fingerprint, userIds}); + if (!key) { + util.throw(404, ctx.__('key_not_found')); + } + if(!key.pendingSignatures) + util.throw(404, "No pending signatures"); + if(key.pendingSignatures.nonce != nonce) + util.throw(403, "Invalid nonce"); + + const signatures = new Map(); + + for(const {user, signature} of key.pendingSignatures.sigs) { + const signedUserID = user.userId; + + let hash = crypto.createHash('md5'); + hash.update(signature, 'base64'); + hash = hash.digest('hex') + + const signaturePacket = await this._pgp.getSignatureFromBase64(signature); + + const fingerprint = Buffer.from(signaturePacket.issuerFingerprint).toString('HEX'); + + const verified = await this.getVerified({fingerprint: fingerprint}); + + const issuerUID = (verified)? await this._pgp.getPrimaryUser(verified.publicKeyArmored): "[unknown identity]"; + + const sig = {issuerFingerprint: fingerprint, + created: signaturePacket.created.toDateString(), + userId: issuerUID, + hash: hash + }; + if(!signatures.has(signedUserID)) { + signatures.set(signedUserID, []); + } + signatures.get(signedUserID).push(sig); + } + + return signatures; + } + /** * Request removal of the public key by flagging all user ids and sending * a verification email to the primary email address. Only one email