From a4134b9f55dc7f52db603cb90021b609c7d46c73 Mon Sep 17 00:00:00 2001 From: Mahrud Sayrafi Date: Wed, 31 Jan 2018 17:47:33 -0800 Subject: [PATCH] Addresses various review comments by @bartbutler + some cleanups --- Gruntfile.js | 10 ++++++---- package.json | 2 +- src/cleartext.js | 6 +++--- src/crypto/cipher/aes.js | 1 + src/crypto/crypto.js | 4 +++- src/crypto/public_key/elliptic/eddsa.js | 19 ++++++++++++++++++- src/crypto/public_key/elliptic/key.js | 24 +++++++++++------------- src/key.js | 8 ++++---- src/keyring/keyring.js | 4 ++-- src/keyring/localstore.js | 4 ++-- src/message.js | 23 +++++++++-------------- src/openpgp.js | 14 +++++--------- src/packet/clone.js | 8 ++++---- src/packet/signature.js | 2 +- src/type/kdf_params.js | 2 +- test/crypto/cipher/des.js | 3 ++- test/general/{ecc.js => ecc_nist.js} | 3 +-- test/general/index.js | 2 +- test/general/x25519.js | 1 - 19 files changed, 75 insertions(+), 65 deletions(-) rename test/general/{ecc.js => ecc_nist.js} (98%) diff --git a/Gruntfile.js b/Gruntfile.js index 77ec18a7..792fa5ea 100644 --- a/Gruntfile.js +++ b/Gruntfile.js @@ -50,7 +50,8 @@ module.exports = function(grunt) { browserifyOptions: { standalone: 'openpgp' }, - external: [ 'crypto', 'buffer', 'node-localstorage', 'node-fetch', 'asn1.js' ], + // Don't bundle these packages with openpgp.js + external: [ 'crypto', 'buffer', 'node-localstorage', 'node-fetch', 'asn1.js', 'bn.js', 'jwk-to-pem' ], transform: [ ["babelify", { plugins: ["transform-async-to-generator", @@ -73,7 +74,7 @@ module.exports = function(grunt) { debug: true, standalone: 'openpgp' }, - external: [ 'crypto', 'buffer', 'node-localstorage', 'node-fetch', 'asn1.js' ], + external: [ 'crypto', 'buffer', 'node-localstorage', 'node-fetch', 'asn1.js', 'bn.js', 'jwk-to-pem' ], transform: [ ["babelify", { plugins: ["transform-async-to-generator", @@ -87,6 +88,7 @@ module.exports = function(grunt) { plugin: [ 'browserify-derequire' ] } }, + // TODO: remove this, as it is only necessary to get browsertest working. openpgp_browser: { files: { 'dist/openpgp_browser.js': [ './src/index.js' ] @@ -95,7 +97,7 @@ module.exports = function(grunt) { browserifyOptions: { standalone: 'openpgp' }, - external: [ 'crypto' ], + external: [ 'crypto', 'buffer', 'node-localstorage', 'node-fetch' ], transform: [ ["babelify", { plugins: ["transform-async-to-generator", @@ -119,7 +121,7 @@ module.exports = function(grunt) { 'test/lib/unittests-bundle.js': [ './test/unittests.js' ] }, options: { - external: [ 'crypto', 'openpgp', '../../dist/openpgp' ] + external: [ 'buffer', 'openpgp', '../../dist/openpgp', '../../../dist/openpgp' ] } } }, diff --git a/package.json b/package.json index 7b0b161d..5fe7e38a 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "openpgp", "description": "OpenPGP.js is a Javascript implementation of the OpenPGP protocol. This is defined in RFC 4880.", - "version": "2.6.1", + "version": "3.0.0", "license": "LGPL-3.0+", "homepage": "http://openpgpjs.org/", "engines": { diff --git a/src/cleartext.js b/src/cleartext.js index b90e25b7..970621ea 100644 --- a/src/cleartext.js +++ b/src/cleartext.js @@ -27,10 +27,10 @@ 'use strict'; import config from './config'; -import armor from './encoding/armor.js'; -import enums from './enums.js'; +import armor from './encoding/armor'; +import enums from './enums'; import packet from './packet'; -import { Signature } from './signature.js'; +import { Signature } from './signature'; /** * @class diff --git a/src/crypto/cipher/aes.js b/src/crypto/cipher/aes.js index 4259d3bd..8fc9c2e6 100644 --- a/src/crypto/cipher/aes.js +++ b/src/crypto/cipher/aes.js @@ -6,6 +6,7 @@ import asmCrypto from 'asmcrypto-lite'; +// TODO use webCrypto or nodeCrypto when possible. export default function aes(length) { var c = function(key) { diff --git a/src/crypto/crypto.js b/src/crypto/crypto.js index 00dc28bd..17aa6f1b 100644 --- a/src/crypto/crypto.js +++ b/src/crypto/crypto.js @@ -56,6 +56,7 @@ export default { * @param {module:enums.publicKey} algo Algorithm to be used (See {@link http://tools.ietf.org/html/rfc4880#section-9.1|RFC 4880 9.1}) * @param {Array} publicParams Algorithm dependent params * @param {module:type/mpi} data Data to be encrypted as MPI + * @param {String} fingerprint Recipient fingerprint * @return {Array} encrypted session key parameters */ publicKeyEncrypt: async function(algo, publicParams, data, fingerprint) { @@ -99,7 +100,8 @@ export default { * Decrypts data using the specified public key multiprecision integers of the private key, * the specified secretMPIs of the private key and the specified algorithm. * @param {module:enums.publicKey} algo Algorithm to be used (See {@link http://tools.ietf.org/html/rfc4880#section-9.1|RFC 4880 9.1}) - * @param {Array} keyIntegers Algorithm dependent params + * @param {Array} dataIntegers encrypted session key parameters * @param {String} fingerprint Recipient fingerprint * @return {module:type/mpi} returns a big integer containing the decrypted data; otherwise null */ diff --git a/src/crypto/public_key/elliptic/eddsa.js b/src/crypto/public_key/elliptic/eddsa.js index 58a9512d..1927d72f 100644 --- a/src/crypto/public_key/elliptic/eddsa.js +++ b/src/crypto/public_key/elliptic/eddsa.js @@ -1,4 +1,21 @@ -// Implementation of EdDSA following RFC4880bis-02 for OpenPGP +// OpenPGP.js - An OpenPGP implementation in javascript +// Copyright (C) 2018 ProtonMail.ch +// +// This library is free software; you can redistribute it and/or +// modify it under the terms of the GNU Lesser General Public +// License as published by the Free Software Foundation; either +// version 3.0 of the License, or (at your option) any later version. +// +// This library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +// Lesser General Public License for more details. +// +// You should have received a copy of the GNU Lesser General Public +// License along with this library; if not, write to the Free Software +// Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + +// Implementation of EdDSA following RFC4880bis-03 for OpenPGP /** * @requires crypto/hash diff --git a/src/crypto/public_key/elliptic/key.js b/src/crypto/public_key/elliptic/key.js index 026beaae..a896d361 100644 --- a/src/crypto/public_key/elliptic/key.js +++ b/src/crypto/public_key/elliptic/key.js @@ -32,10 +32,6 @@ 'use strict'; -import BN from 'bn.js'; -import ASN1 from 'asn1.js'; -import jwkToPem from 'jwk-to-pem'; - import curves from './curves'; import hash from '../../hash'; import util from '../../../util'; @@ -48,12 +44,15 @@ const webCurves = curves.webCurves; const nodeCrypto = util.getNodeCrypto(); const nodeCurves = curves.nodeCurves; -var ECDSASignature = ASN1.define('ECDSASignature', function() { - this.seq().obj( - this.key('r').int(), - this.key('s').int() - ); -}); +const BN = nodeCrypto ? require('bn.js') : undefined; +const jwkToPem = nodeCrypto ? require('jwk-to-pem') : undefined; +const ECDSASignature = nodeCrypto ? + require('asn1.js').define('ECDSASignature', function() { + this.seq().obj( + this.key('r').int(), + this.key('s').int() + ); + }) : undefined; function KeyPair(curve, options) { this.curve = curve; @@ -62,7 +61,7 @@ function KeyPair(curve, options) { } KeyPair.prototype.sign = async function (message, hash_algo) { - if (typeof message === 'string') { + if (util.isString(message)) { message = util.str2Uint8Array(message); } if (webCrypto && config.use_native && this.curve.web) { @@ -76,7 +75,7 @@ KeyPair.prototype.sign = async function (message, hash_algo) { }; KeyPair.prototype.verify = async function (message, signature, hash_algo) { - if (typeof message === 'string') { + if (util.isString(message)) { message = util.str2Uint8Array(message); } if (webCrypto && config.use_native && this.curve.web) { @@ -240,7 +239,6 @@ async function nodeVerify(curve, hash_algo, {r, s}, message, publicKey) { { private: false } ); - // FIXME what happens when hash_algo = undefined? const verify = nodeCrypto.createVerify(enums.read(enums.hash, hash_algo)); verify.write(message); verify.end(); diff --git a/src/key.js b/src/key.js index c79daa7d..912f1abe 100644 --- a/src/key.js +++ b/src/key.js @@ -29,8 +29,8 @@ import config from './config'; import crypto from './crypto'; -import armor from './encoding/armor.js'; -import enums from './enums.js'; +import armor from './encoding/armor'; +import enums from './enums'; import util from './util'; import packet from './packet'; @@ -542,7 +542,7 @@ Key.prototype.getPrimaryUser = function(allowExpired=false) { // sort by primary user flag and signature creation time primaryUsers = primaryUsers.sort(function(a, b) { var A = a.selfCertificate, B = b.selfCertificate; - return A.isPrimaryUserID < B.isPrimaryUserID || A.created < B.created; + return (B.isPrimaryUserID - A.isPrimaryUserID) || (B.created - A.created); }); return primaryUsers.pop(); }; @@ -907,7 +907,7 @@ User.prototype.verify = async function(primaryKey) { } return enums.keyStatus.valid; }))); - return results.some(status => status === enums.keyStatus.valid)? + return results.some(status => status === enums.keyStatus.valid) ? enums.keyStatus.valid : results.pop(); }; diff --git a/src/keyring/keyring.js b/src/keyring/keyring.js index c7b860ca..7bb89ba9 100644 --- a/src/keyring/keyring.js +++ b/src/keyring/keyring.js @@ -25,8 +25,8 @@ 'use strict'; -import { readArmored } from '../key.js'; -import LocalStore from './localstore.js'; +import { readArmored } from '../key'; +import LocalStore from './localstore'; /** * Initialization routine for the keyring. This method reads the diff --git a/src/keyring/localstore.js b/src/keyring/localstore.js index aaa84c7e..4bbc8588 100644 --- a/src/keyring/localstore.js +++ b/src/keyring/localstore.js @@ -27,8 +27,8 @@ 'use strict'; import config from '../config'; -import { readArmored } from '../key.js'; -import util from '../util.js'; +import { readArmored } from '../key'; +import util from '../util'; export default function LocalStore(prefix) { prefix = prefix || 'openpgp-'; diff --git a/src/message.js b/src/message.js index 4546981c..87363d18 100644 --- a/src/message.js +++ b/src/message.js @@ -31,12 +31,12 @@ import config from './config'; import crypto from './crypto'; -import armor from './encoding/armor.js'; -import enums from './enums.js'; -import util from './util.js'; +import armor from './encoding/armor'; +import enums from './enums'; +import util from './util'; import packet from './packet'; -import { Signature } from './signature.js'; -import { getPreferredHashAlgo, getPreferredSymAlgo } from './key.js'; +import { Signature } from './signature'; +import { getPreferredHashAlgo, getPreferredSymAlgo } from './key'; /** @@ -145,19 +145,14 @@ Message.prototype.decryptSessionKey = function(privateKey, password) { }); } else if (privateKey) { - var encryptionKeyIds = this.getEncryptionKeyIds(); - if (!encryptionKeyIds.length) { - // nothing to decrypt - return; - } - var privateKeyPacket = privateKey.getKeyPacket(encryptionKeyIds); - if (!privateKeyPacket.isDecrypted) { - throw new Error('Private key is not decrypted.'); - } var pkESKeyPacketlist = this.packets.filterByTag(enums.packet.publicKeyEncryptedSessionKey); if (!pkESKeyPacketlist) { throw new Error('No public key encrypted session key packet found.'); } + var privateKeyPacket = privateKey.getKeyPacket(this.getEncryptionKeyIds()); + if (!privateKeyPacket.isDecrypted) { + throw new Error('Private key is not decrypted.'); + } // TODO replace when Promise.some or Promise.any are implemented // eslint-disable-next-line no-await-in-loop await pkESKeyPacketlist.some(async function(packet) { diff --git a/src/openpgp.js b/src/openpgp.js index 6a6daa31..22d331ed 100644 --- a/src/openpgp.js +++ b/src/openpgp.js @@ -34,16 +34,12 @@ 'use strict'; -import es6Promise from 'es6-promise'; - -import * as messageLib from './message.js'; -import { CleartextMessage } from './cleartext.js'; -import { generate, reformat } from './key.js'; -import config from './config/config.js'; +import * as messageLib from './message'; +import { CleartextMessage } from './cleartext'; +import { generate, reformat } from './key'; +import config from './config/config'; import util from './util'; -import AsyncProxy from './worker/async_proxy.js'; - -es6Promise.polyfill(); // load ES6 Promises polyfill +import AsyncProxy from './worker/async_proxy'; ////////////////////////// diff --git a/src/packet/clone.js b/src/packet/clone.js index 07e0a632..ab5e41a9 100644 --- a/src/packet/clone.js +++ b/src/packet/clone.js @@ -23,10 +23,10 @@ 'use strict'; -import { Key } from '../key.js'; -import { Message } from '../message.js'; -import { CleartextMessage } from '../cleartext.js'; -import { Signature } from '../signature.js' +import { Key } from '../key'; +import { Message } from '../message'; +import { CleartextMessage } from '../cleartext'; +import { Signature } from '../signature' import Packetlist from './packetlist.js'; import type_keyid from '../type/keyid.js'; diff --git a/src/packet/signature.js b/src/packet/signature.js index 291611aa..9aed991b 100644 --- a/src/packet/signature.js +++ b/src/packet/signature.js @@ -630,7 +630,7 @@ Signature.prototype.verify = async function (key, data) { if (publicKeyAlgorithm > 0 && publicKeyAlgorithm < 4) { mpicount = 1; } - // Algorithm-Specific Fields for DSA and ECDSA signatures: + // Algorithm-Specific Fields for DSA, ECDSA, and EdDSA signatures: // - MPI of DSA value r. // - MPI of DSA value s. else if (publicKeyAlgorithm === enums.publicKey.dsa || diff --git a/src/type/kdf_params.js b/src/type/kdf_params.js index 686830b5..8a33a026 100644 --- a/src/type/kdf_params.js +++ b/src/type/kdf_params.js @@ -67,4 +67,4 @@ KDFParams.prototype.write = function () { KDFParams.fromClone = function (clone) { return new KDFParams(clone.hash, clone.cipher); -}; \ No newline at end of file +}; diff --git a/test/crypto/cipher/des.js b/test/crypto/cipher/des.js index 044a0696..62f2122d 100644 --- a/test/crypto/cipher/des.js +++ b/test/crypto/cipher/des.js @@ -6,7 +6,8 @@ var util = openpgp.util, chai = require('chai'), expect = chai.expect; -describe('TripleDES (EDE) cipher test with test vectors from http://csrc.nist.gov/publications/nistpubs/800-20/800-20.pdf', function() { +describe('TripleDES (EDE) cipher test with test vectors from NIST SP 800-20', function() { + // see http://csrc.nist.gov/publications/nistpubs/800-20/800-20.pdf var key = new Uint8Array([1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1]); var testvectors = [[[0x80,0x00,0x00,0x00,0x00,0x00,0x00,0x00],[0x95,0xF8,0xA5,0xE5,0xDD,0x31,0xD9,0x00]], [[0x40,0x00,0x00,0x00,0x00,0x00,0x00,0x00],[0xDD,0x7F,0x12,0x1C,0xA5,0x01,0x56,0x19]], diff --git a/test/general/ecc.js b/test/general/ecc_nist.js similarity index 98% rename from test/general/ecc.js rename to test/general/ecc_nist.js index 0213b349..27149763 100644 --- a/test/general/ecc.js +++ b/test/general/ecc_nist.js @@ -168,10 +168,9 @@ describe('Elliptic Curve Cryptography', function () { expect(result.signatures[0].valid).to.be.true; }); }); - // FIXME is this pattern correct? it('Sign message', function () { var romeo = load_priv_key('romeo'); - openpgp.sign({privateKeys: [romeo], data: data.romeo.message + "\n"}).then(function (signed) { + return openpgp.sign({privateKeys: [romeo], data: data.romeo.message + "\n"}).then(function (signed) { var romeo = load_pub_key('romeo'); var msg = openpgp.cleartext.readArmored(signed.data); return openpgp.verify({publicKeys: [romeo], message: msg}).then(function (result) { diff --git a/test/general/index.js b/test/general/index.js index 49535869..0e9b6ff2 100644 --- a/test/general/index.js +++ b/test/general/index.js @@ -8,7 +8,7 @@ describe('General', function () { require('./openpgp.js'); require('./hkp.js'); require('./oid.js'); - require('./ecc.js'); + require('./ecc_nist.js'); require('./x25519.js'); }); diff --git a/test/general/x25519.js b/test/general/x25519.js index f8aa8122..3ba34002 100644 --- a/test/general/x25519.js +++ b/test/general/x25519.js @@ -153,7 +153,6 @@ describe('X25519 Cryptography', function () { expect(result.signatures[0].valid).to.be.true; }); }); - // FIXME is this pattern correct? it('Sign message', function () { var name = 'light' var priv = load_priv_key(name);