From be1b4df14053942dd946045980e446bebbc6350e Mon Sep 17 00:00:00 2001 From: Daniel Huigens Date: Mon, 29 Apr 2019 16:04:21 +0200 Subject: [PATCH 1/9] Use tweetnacl's Ed25519 implementation --- npm-shrinkwrap.json | 109 +++++++++++++++++------ package.json | 1 + src/crypto/public_key/elliptic/curves.js | 4 - src/crypto/public_key/elliptic/eddsa.js | 22 +++-- src/crypto/public_key/index.js | 6 +- src/crypto/signature.js | 9 +- test/general/x25519.js | 9 +- 7 files changed, 111 insertions(+), 49 deletions(-) diff --git a/npm-shrinkwrap.json b/npm-shrinkwrap.json index 63809934..3b1fb6a4 100644 --- a/npm-shrinkwrap.json +++ b/npm-shrinkwrap.json @@ -7,7 +7,8 @@ "@mattiasbuelens/web-streams-polyfill": { "version": "0.3.1", "resolved": "https://registry.npmjs.org/@mattiasbuelens/web-streams-polyfill/-/web-streams-polyfill-0.3.1.tgz", - "integrity": "sha512-IW+tCurEH2NL2tA3KKFAMXa90WWmTJMZksnQWMABe3bMijV7hEiOLthy4tKGTnUTV8qVf8WTCApiGmKb75Bxkg==" + "integrity": "sha512-IW+tCurEH2NL2tA3KKFAMXa90WWmTJMZksnQWMABe3bMijV7hEiOLthy4tKGTnUTV8qVf8WTCApiGmKb75Bxkg==", + "dev": true }, "@sinonjs/formatio": { "version": "2.0.0", @@ -107,6 +108,7 @@ "version": "2.0.3", "resolved": "https://registry.npmjs.org/address-rfc2822/-/address-rfc2822-2.0.3.tgz", "integrity": "sha1-LzDSHYEVCGLBSJpHL0lARQIa+IE=", + "dev": true, "requires": { "email-addresses": "^3.0.0" } @@ -152,6 +154,7 @@ "resolved": "https://registry.npmjs.org/align-text/-/align-text-0.1.4.tgz", "integrity": "sha1-DNkKVhCT810KmSVsIrcGlDP60Rc=", "dev": true, + "optional": true, "requires": { "kind-of": "^3.0.2", "longest": "^1.0.1", @@ -269,7 +272,8 @@ }, "asmcrypto.js": { "version": "github:openpgpjs/asmcrypto#6e4e407b9b8ae317925a9e677cc7b4de3e447e83", - "from": "github:openpgpjs/asmcrypto#6e4e407b9b8ae317925a9e677cc7b4de3e447e83" + "from": "github:openpgpjs/asmcrypto#6e4e407b9b8ae317925a9e677cc7b4de3e447e83", + "dev": true }, "asn1": { "version": "0.2.3", @@ -1132,7 +1136,8 @@ "base64-js": { "version": "1.2.1", "resolved": "https://registry.npmjs.org/base64-js/-/base64-js-1.2.1.tgz", - "integrity": "sha512-dwVUVIXsBZXwTuwnXI9RK8sBmgq09NDHzyR9SAph9eqk76gKK2JSQmZARC2zRC81JC2QTtxD0ARU5qTS25gIGw==" + "integrity": "sha512-dwVUVIXsBZXwTuwnXI9RK8sBmgq09NDHzyR9SAph9eqk76gKK2JSQmZARC2zRC81JC2QTtxD0ARU5qTS25gIGw==", + "dev": true }, "basic-auth": { "version": "2.0.0", @@ -1157,6 +1162,15 @@ "optional": true, "requires": { "tweetnacl": "^0.14.3" + }, + "dependencies": { + "tweetnacl": { + "version": "0.14.5", + "resolved": "https://registry.npmjs.org/tweetnacl/-/tweetnacl-0.14.5.tgz", + "integrity": "sha1-WuaBd/GS1EViadEIr6k/+HQ/T2Q=", + "dev": true, + "optional": true + } } }, "binary-extensions": { @@ -1212,7 +1226,8 @@ "brorand": { "version": "1.1.0", "resolved": "https://registry.npmjs.org/brorand/-/brorand-1.1.0.tgz", - "integrity": "sha1-EsJe/kCkXjwyPrhnWgoM5XsiNx8=" + "integrity": "sha1-EsJe/kCkXjwyPrhnWgoM5XsiNx8=", + "dev": true }, "browser-pack": { "version": "6.1.0", @@ -1529,6 +1544,7 @@ "version": "5.0.8", "resolved": "https://registry.npmjs.org/buffer/-/buffer-5.0.8.tgz", "integrity": "sha512-xXvjQhVNz50v2nPeoOsNqWCLGfiv4ji/gXZM28jnVwdLJxH4mFyqgqCKfaK9zf1KUbG6zTkjLOy7ou+jSMarGA==", + "dev": true, "requires": { "base64-js": "^1.0.2", "ieee754": "^1.1.4" @@ -2292,6 +2308,7 @@ "elliptic": { "version": "github:openpgpjs/elliptic#ad81845f693effa5b4b6d07db2e82112de222f48", "from": "github:openpgpjs/elliptic#ad81845f693effa5b4b6d07db2e82112de222f48", + "dev": true, "requires": { "bn.js": "^4.4.0", "brorand": "^1.0.1", @@ -2305,7 +2322,8 @@ "email-addresses": { "version": "3.0.1", "resolved": "https://registry.npmjs.org/email-addresses/-/email-addresses-3.0.1.tgz", - "integrity": "sha1-wfwgwYnn+W1AEtN121/qzN0kORw=" + "integrity": "sha1-wfwgwYnn+W1AEtN121/qzN0kORw=", + "dev": true }, "encodeurl": { "version": "1.0.1", @@ -3116,7 +3134,8 @@ "ansi-regex": { "version": "2.1.1", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "aproba": { "version": "1.2.0", @@ -3137,12 +3156,14 @@ "balanced-match": { "version": "1.0.0", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "brace-expansion": { "version": "1.1.11", "bundled": true, "dev": true, + "optional": true, "requires": { "balanced-match": "^1.0.0", "concat-map": "0.0.1" @@ -3157,17 +3178,20 @@ "code-point-at": { "version": "1.1.0", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "concat-map": { "version": "0.0.1", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "console-control-strings": { "version": "1.1.0", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "core-util-is": { "version": "1.0.2", @@ -3284,7 +3308,8 @@ "inherits": { "version": "2.0.3", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "ini": { "version": "1.3.5", @@ -3296,6 +3321,7 @@ "version": "1.0.0", "bundled": true, "dev": true, + "optional": true, "requires": { "number-is-nan": "^1.0.0" } @@ -3310,6 +3336,7 @@ "version": "3.0.4", "bundled": true, "dev": true, + "optional": true, "requires": { "brace-expansion": "^1.1.7" } @@ -3317,12 +3344,14 @@ "minimist": { "version": "0.0.8", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "minipass": { "version": "2.2.4", "bundled": true, "dev": true, + "optional": true, "requires": { "safe-buffer": "^5.1.1", "yallist": "^3.0.0" @@ -3341,6 +3370,7 @@ "version": "0.5.1", "bundled": true, "dev": true, + "optional": true, "requires": { "minimist": "0.0.8" } @@ -3421,7 +3451,8 @@ "number-is-nan": { "version": "1.0.1", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "object-assign": { "version": "4.1.1", @@ -3433,6 +3464,7 @@ "version": "1.4.0", "bundled": true, "dev": true, + "optional": true, "requires": { "wrappy": "1" } @@ -3518,7 +3550,8 @@ "safe-buffer": { "version": "5.1.1", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "safer-buffer": { "version": "2.1.2", @@ -3554,6 +3587,7 @@ "version": "1.0.2", "bundled": true, "dev": true, + "optional": true, "requires": { "code-point-at": "^1.0.0", "is-fullwidth-code-point": "^1.0.0", @@ -3573,6 +3607,7 @@ "version": "3.0.1", "bundled": true, "dev": true, + "optional": true, "requires": { "ansi-regex": "^2.0.0" } @@ -3616,12 +3651,14 @@ "wrappy": { "version": "1.0.2", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "yallist": { "version": "3.0.2", "bundled": true, - "dev": true + "dev": true, + "optional": true } } }, @@ -3773,7 +3810,8 @@ "graceful-readlink": { "version": "1.0.1", "resolved": "https://registry.npmjs.org/graceful-readlink/-/graceful-readlink-1.0.1.tgz", - "integrity": "sha1-TK+tdrxi8C+gObL5Tpo906ORpyU=" + "integrity": "sha1-TK+tdrxi8C+gObL5Tpo906ORpyU=", + "dev": true }, "growl": { "version": "1.10.3", @@ -4308,6 +4346,7 @@ "version": "1.1.3", "resolved": "https://registry.npmjs.org/hash.js/-/hash.js-1.1.3.tgz", "integrity": "sha512-/UETyP0W22QILqS+6HowevwhEFJ3MBJnwTf75Qob9Wz9t0DPuisL8kW8YZMK62dHAKE1c1p+gY1TtOLY+USEHA==", + "dev": true, "requires": { "inherits": "^2.0.3", "minimalistic-assert": "^1.0.0" @@ -4323,6 +4362,7 @@ "version": "1.0.1", "resolved": "https://registry.npmjs.org/hmac-drbg/-/hmac-drbg-1.0.1.tgz", "integrity": "sha1-0nRXAQJabHdabFRXk+1QL8DGSaE=", + "dev": true, "requires": { "hash.js": "^1.0.3", "minimalistic-assert": "^1.0.0", @@ -4418,7 +4458,8 @@ "ieee754": { "version": "1.1.8", "resolved": "https://registry.npmjs.org/ieee754/-/ieee754-1.1.8.tgz", - "integrity": "sha1-vjPUCsEO8ZJnAfbwii2G+/0a0+Q=" + "integrity": "sha1-vjPUCsEO8ZJnAfbwii2G+/0a0+Q=", + "dev": true }, "ignore": { "version": "3.3.7", @@ -5139,7 +5180,8 @@ "version": "1.0.1", "resolved": "https://registry.npmjs.org/longest/-/longest-1.0.1.tgz", "integrity": "sha1-MKCy2jj3N3DoKUoNIuZiXtd9AJc=", - "dev": true + "dev": true, + "optional": true }, "loose-envify": { "version": "1.3.1", @@ -5289,7 +5331,8 @@ "minimalistic-crypto-utils": { "version": "1.0.1", "resolved": "https://registry.npmjs.org/minimalistic-crypto-utils/-/minimalistic-crypto-utils-1.0.1.tgz", - "integrity": "sha1-9sAMHAsIIkblxNmd+4x8CDsrWCo=" + "integrity": "sha1-9sAMHAsIIkblxNmd+4x8CDsrWCo=", + "dev": true }, "minimatch": { "version": "3.0.4", @@ -5698,7 +5741,8 @@ "pako": { "version": "1.0.6", "resolved": "https://registry.npmjs.org/pako/-/pako-1.0.6.tgz", - "integrity": "sha512-lQe48YPsMJAig+yngZ87Lus+NF+3mtu7DVOBu6b/gHO1YpKwIj5AWjZ/TOS7i46HD/UixzWb1zeWDZfGZ3iYcg==" + "integrity": "sha512-lQe48YPsMJAig+yngZ87Lus+NF+3mtu7DVOBu6b/gHO1YpKwIj5AWjZ/TOS7i46HD/UixzWb1zeWDZfGZ3iYcg==", + "dev": true }, "parents": { "version": "1.0.1", @@ -6454,6 +6498,7 @@ "seek-bzip": { "version": "github:openpgpjs/seek-bzip#3aca608ffedc055a1da1d898ecb244804ef32209", "from": "github:openpgpjs/seek-bzip#3aca608ffedc055a1da1d898ecb244804ef32209", + "dev": true, "requires": { "commander": "~2.8.1" }, @@ -6462,6 +6507,7 @@ "version": "2.8.1", "resolved": "https://registry.npmjs.org/commander/-/commander-2.8.1.tgz", "integrity": "sha1-Br42f+v9oMMwqh4qBy09yXYkJdQ=", + "dev": true, "requires": { "graceful-readlink": ">= 1.0.0" } @@ -6731,6 +6777,15 @@ "jsbn": "~0.1.0", "safer-buffer": "^2.0.2", "tweetnacl": "~0.14.0" + }, + "dependencies": { + "tweetnacl": { + "version": "0.14.5", + "resolved": "https://registry.npmjs.org/tweetnacl/-/tweetnacl-0.14.5.tgz", + "integrity": "sha1-WuaBd/GS1EViadEIr6k/+HQ/T2Q=", + "dev": true, + "optional": true + } } }, "statuses": { @@ -7180,11 +7235,10 @@ } }, "tweetnacl": { - "version": "0.14.5", - "resolved": "https://registry.npmjs.org/tweetnacl/-/tweetnacl-0.14.5.tgz", - "integrity": "sha1-WuaBd/GS1EViadEIr6k/+HQ/T2Q=", - "dev": true, - "optional": true + "version": "1.0.1", + "resolved": "https://registry.npmjs.org/tweetnacl/-/tweetnacl-1.0.1.tgz", + "integrity": "sha512-kcoMoKTPYnoeS50tzoqjPY3Uv9axeuuFAZY9M/9zFnhoVvRfxz9K29IMPD7jGmt2c8SW7i3gT9WqDl2+nV7p4A==", + "dev": true }, "type-check": { "version": "0.3.2", @@ -7368,7 +7422,8 @@ }, "web-stream-tools": { "version": "github:openpgpjs/web-stream-tools#84a497715c9df271a673f8616318264ab42ab3cc", - "from": "github:openpgpjs/web-stream-tools#84a497715c9df271a673f8616318264ab42ab3cc" + "from": "github:openpgpjs/web-stream-tools#84a497715c9df271a673f8616318264ab42ab3cc", + "dev": true }, "websocket-driver": { "version": "0.7.0", diff --git a/package.json b/package.json index 7a04d51e..4936a643 100644 --- a/package.json +++ b/package.json @@ -80,6 +80,7 @@ "hash.js": "^1.1.3", "pako": "^1.0.6", "seek-bzip": "github:openpgpjs/seek-bzip#3aca608ffedc055a1da1d898ecb244804ef32209", + "tweetnacl": "^1.0.1", "web-stream-tools": "github:openpgpjs/web-stream-tools#84a497715c9df271a673f8616318264ab42ab3cc" }, "dependencies": { diff --git a/src/crypto/public_key/elliptic/curves.js b/src/crypto/public_key/elliptic/curves.js index 2186ea9c..742cd5c7 100644 --- a/src/crypto/public_key/elliptic/curves.js +++ b/src/crypto/public_key/elliptic/curves.js @@ -172,10 +172,6 @@ Curve.prototype.keyFromPrivate = function (priv) { // Not for ed25519 return new KeyPair(this, { priv: priv }); }; -Curve.prototype.keyFromSecret = function (secret) { // Only for ed25519 - return new KeyPair(this, { secret: secret }); -}; - Curve.prototype.keyFromPublic = function (pub) { const keyPair = new KeyPair(this, { pub: pub }); if ( diff --git a/src/crypto/public_key/elliptic/eddsa.js b/src/crypto/public_key/elliptic/eddsa.js index 0b09b036..c05b1b47 100644 --- a/src/crypto/public_key/elliptic/eddsa.js +++ b/src/crypto/public_key/elliptic/eddsa.js @@ -17,11 +17,15 @@ /** * @fileoverview Implementation of EdDSA following RFC4880bis-03 for OpenPGP + * @requires tweetnacl * @requires crypto/public_key/elliptic/curve + * @requires util * @module crypto/public_key/elliptic/eddsa */ +import nacl from 'tweetnacl'; import Curve from './curves'; +import util from '../../../util'; /** * Sign a message using the provided key @@ -35,12 +39,13 @@ import Curve from './curves'; * @async */ async function sign(oid, hash_algo, m, d, hashed) { - const curve = new Curve(oid); - const key = curve.keyFromSecret(d); - const signature = await key.sign(m, hash_algo, hashed); + const { secretKey } = nacl.sign.keyPair.fromSeed(d); + const signature = nacl.sign.detached(hashed, secretKey); // EdDSA signature params are returned in little-endian format - return { R: new Uint8Array(signature.Rencoded()), - S: new Uint8Array(signature.Sencoded()) }; + return { + R: signature.subarray(0, 32), + S: signature.subarray(32) + }; } /** @@ -50,12 +55,15 @@ async function sign(oid, hash_algo, m, d, hashed) { * @param {{R: Uint8Array, S: Uint8Array}} signature Signature to verify the message * @param {Uint8Array} m Message to verify - * @param {Uint8Array} Q Public key used to verify the message + * @param {Uint8Array} publicKey Public key used to verify the message * @param {Uint8Array} hashed The hashed message * @returns {Boolean} * @async */ -async function verify(oid, hash_algo, signature, m, Q, hashed) { +async function verify(oid, hash_algo, { R, S }, m, publicKey, hashed) { + const signature = util.concatUint8Array([R, S]); + return nacl.sign.detached.verify(hashed, signature, publicKey.subarray(1)); + const curve = new Curve(oid); const key = curve.keyFromPublic(Q); return key.verify(m, signature, hash_algo, hashed); diff --git a/src/crypto/public_key/index.js b/src/crypto/public_key/index.js index bd8656ff..23bceddb 100644 --- a/src/crypto/public_key/index.js +++ b/src/crypto/public_key/index.js @@ -1,5 +1,6 @@ /** * @fileoverview Asymmetric cryptography functions + * @requires tweetnacl * @requires crypto/public_key/dsa * @requires crypto/public_key/elgamal * @requires crypto/public_key/elliptic @@ -7,6 +8,7 @@ * @module crypto/public_key */ +import nacl from 'tweetnacl'; import rsa from './rsa'; import elgamal from './elgamal'; import elliptic from './elliptic'; @@ -20,5 +22,7 @@ export default { /** @see module:crypto/public_key/elliptic */ elliptic: elliptic, /** @see module:crypto/public_key/dsa */ - dsa: dsa + dsa: dsa, + /** @see tweetnacl */ + nacl: nacl }; diff --git a/src/crypto/signature.js b/src/crypto/signature.js index 9b13d944..da7a8df4 100644 --- a/src/crypto/signature.js +++ b/src/crypto/signature.js @@ -58,11 +58,10 @@ export default { } case enums.publicKey.eddsa: { const oid = pub_MPIs[0]; - // TODO refactor elliptic to accept Uint8Array // EdDSA signature params are expected in little-endian format - const signature = { R: Array.from(msg_MPIs[0].toUint8Array('le', 32)), - S: Array.from(msg_MPIs[1].toUint8Array('le', 32)) }; - const Q = Array.from(pub_MPIs[1].toUint8Array('be', 33)); + const signature = { R: msg_MPIs[0].toUint8Array('le', 32), + S: msg_MPIs[1].toUint8Array('le', 32) }; + const Q = pub_MPIs[1].toUint8Array('be', 33); return publicKey.elliptic.eddsa.verify(oid, hash_algo, signature, data, Q, hashed); } default: @@ -120,7 +119,7 @@ export default { } case enums.publicKey.eddsa: { const oid = key_params[0]; - const d = Array.from(key_params[2].toUint8Array('be', 32)); + const d = key_params[2].toUint8Array('be', 32); const signature = await publicKey.elliptic.eddsa.sign(oid, hash_algo, data, d, hashed); return util.concatUint8Array([ util.Uint8Array_to_MPI(signature.R), diff --git a/test/general/x25519.js b/test/general/x25519.js index 0acc602c..660b7054 100644 --- a/test/general/x25519.js +++ b/test/general/x25519.js @@ -336,9 +336,8 @@ describe('X25519 Cryptography', function () { const util = openpgp.util; function testVector(vector) { const curve = new elliptic.Curve('ed25519'); - const S = curve.keyFromSecret(vector.SECRET_KEY); - const P = curve.keyFromPublic('40'+vector.PUBLIC_KEY); - expect(S.getPublic()).to.deep.equal(P.getPublic()); + const { publicKey } = openpgp.crypto.publicKey.nacl.sign.keyPair.fromSeed(openpgp.util.hex_to_Uint8Array(vector.SECRET_KEY)); + expect(publicKey).to.deep.equal(openpgp.util.hex_to_Uint8Array(vector.PUBLIC_KEY)); const data = util.str_to_Uint8Array(vector.MESSAGE); const keyIntegers = [ openpgp.OID.fromClone(curve), @@ -350,12 +349,12 @@ describe('X25519 Cryptography', function () { new openpgp.MPI(util.Uint8Array_to_str(util.hex_to_Uint8Array(vector.SIGNATURE.S).reverse())) ]; return Promise.all([ - signature.sign(22, undefined, keyIntegers, data).then(signed => { + signature.sign(22, undefined, keyIntegers, undefined, data).then(signed => { const len = ((signed[0] << 8| signed[1]) + 7) / 8; expect(util.hex_to_Uint8Array(vector.SIGNATURE.R)).to.deep.eq(signed.slice(2, 2 + len)); expect(util.hex_to_Uint8Array(vector.SIGNATURE.S)).to.deep.eq(signed.slice(4 + len)); }), - signature.verify(22, undefined, msg_MPIs, keyIntegers, data).then(result => { + signature.verify(22, undefined, msg_MPIs, keyIntegers, undefined, data).then(result => { expect(result).to.be.true; }) ]); From ca0322bbea36e30cdf764d0cc9e3373260cd78af Mon Sep 17 00:00:00 2001 From: Daniel Huigens Date: Wed, 1 May 2019 07:56:39 +0200 Subject: [PATCH 2/9] Use tweetnacl's X25519 implementation --- src/crypto/public_key/elliptic/ecdh.js | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/src/crypto/public_key/elliptic/ecdh.js b/src/crypto/public_key/elliptic/ecdh.js index 87562060..3a1be448 100644 --- a/src/crypto/public_key/elliptic/ecdh.js +++ b/src/crypto/public_key/elliptic/ecdh.js @@ -18,6 +18,7 @@ /** * @fileoverview Key encryption and decryption for RFC 6637 ECDH * @requires bn.js + * @requires tweetnacl * @requires crypto/public_key/elliptic/curve * @requires crypto/aes_kw * @requires crypto/cipher @@ -29,6 +30,7 @@ */ import BN from 'bn.js'; +import nacl from 'tweetnacl'; import Curve from './curves'; import aes_kw from '../../aes_kw'; import cipher from '../../cipher'; @@ -85,6 +87,19 @@ async function kdf(hash_algo, S, length, param, curve, stripLeading=false, strip * @async */ async function genPublicEphemeralKey(curve, Q) { + if (curve.name === 'curve25519') { + const { secretKey } = nacl.box.keyPair(); + const one = curve.curve.curve.one; + const mask = one.ushln(255 - 3).sub(one).ushln(3); + let priv = new BN(secretKey); + priv = priv.or(one.ushln(255 - 1)); + priv = priv.and(mask); + priv = priv.toArrayLike(Uint8Array, 'le', 32); + const S = nacl.scalarMult(priv, Q.subarray(1)); + const { publicKey } = nacl.box.keyPair.fromSecretKey(priv); + const ret = { V: util.concatUint8Array([new Uint8Array([0x40]), publicKey]), S: new BN(S, 'le') }; + return ret; + } const v = await curve.genKeyPair(); Q = curve.keyFromPublic(Q); const V = new Uint8Array(v.getPublic()); @@ -124,6 +139,15 @@ async function encrypt(oid, cipher_algo, hash_algo, m, Q, fingerprint) { * @async */ async function genPrivateEphemeralKey(curve, V, d) { + if (curve.name === 'curve25519') { + const one = curve.curve.curve.one; + const mask = one.ushln(255 - 3).sub(one).ushln(3); + let priv = new BN(d); + priv = priv.or(one.ushln(255 - 1)); + priv = priv.and(mask); + const S = nacl.scalarMult(priv.toArrayLike(Uint8Array, 'le', 32), V.subarray(1)); + return new BN(S, 'le'); + } V = curve.keyFromPublic(V); d = curve.keyFromPrivate(d); return d.derive(V); From e637e758916c10a47fbf6709a53099368a0cb9e3 Mon Sep 17 00:00:00 2001 From: Daniel Huigens Date: Wed, 1 May 2019 10:42:35 +0200 Subject: [PATCH 3/9] Clean up ECDH API --- src/crypto/crypto.js | 120 +++++++++--------- src/crypto/public_key/elliptic/ecdh.js | 60 +++++---- .../public_key_encrypted_session_key.js | 4 +- test/crypto/crypto.js | 4 +- test/crypto/elliptic.js | 8 +- 5 files changed, 95 insertions(+), 101 deletions(-) diff --git a/src/crypto/crypto.js b/src/crypto/crypto.js index 46675409..f2107cfc 100644 --- a/src/crypto/crypto.js +++ b/src/crypto/crypto.js @@ -69,36 +69,34 @@ export default { */ publicKeyEncrypt: async function(algo, pub_params, data, fingerprint) { const types = this.getEncSessionKeyParamTypes(algo); - return (async function() { - switch (algo) { - case enums.publicKey.rsa_encrypt: - case enums.publicKey.rsa_encrypt_sign: { - const m = data.toBN(); - const n = pub_params[0].toBN(); - const e = pub_params[1].toBN(); - const res = await publicKey.rsa.encrypt(m, n, e); - return constructParams(types, [res]); - } - case enums.publicKey.elgamal: { - const m = data.toBN(); - const p = pub_params[0].toBN(); - const g = pub_params[1].toBN(); - const y = pub_params[2].toBN(); - const res = await publicKey.elgamal.encrypt(m, p, g, y); - return constructParams(types, [res.c1, res.c2]); - } - case enums.publicKey.ecdh: { - const oid = pub_params[0]; - const Q = pub_params[1].toUint8Array(); - const kdf_params = pub_params[2]; - const { V, C } = await publicKey.elliptic.ecdh.encrypt( - oid, kdf_params.cipher, kdf_params.hash, data, Q, fingerprint); - return constructParams(types, [new BN(V), C]); - } - default: - return []; + switch (algo) { + case enums.publicKey.rsa_encrypt: + case enums.publicKey.rsa_encrypt_sign: { + const m = data.toBN(); + const n = pub_params[0].toBN(); + const e = pub_params[1].toBN(); + const res = await publicKey.rsa.encrypt(m, n, e); + return constructParams(types, [res]); } - }()); + case enums.publicKey.elgamal: { + const m = data.toBN(); + const p = pub_params[0].toBN(); + const g = pub_params[1].toBN(); + const y = pub_params[2].toBN(); + const res = await publicKey.elgamal.encrypt(m, p, g, y); + return constructParams(types, [res.c1, res.c2]); + } + case enums.publicKey.ecdh: { + const oid = pub_params[0]; + const Q = pub_params[1].toUint8Array(); + const kdf_params = pub_params[2]; + const { publicKey: V, wrappedKey: C } = await publicKey.elliptic.ecdh.encrypt( + oid, kdf_params.cipher, kdf_params.hash, data, Q, fingerprint); + return constructParams(types, [new BN(V), C]); + } + default: + return []; + } }, /** @@ -112,43 +110,41 @@ export default { module:type/ecdh_symkey>} data_params encrypted session key parameters * @param {String} fingerprint Recipient fingerprint - * @returns {module:type/mpi} An MPI containing the decrypted data + * @returns {BN} A BN containing the decrypted data * @async */ publicKeyDecrypt: async function(algo, key_params, data_params, fingerprint) { - return new type_mpi(await (async function() { - switch (algo) { - case enums.publicKey.rsa_encrypt_sign: - case enums.publicKey.rsa_encrypt: { - const c = data_params[0].toBN(); - const n = key_params[0].toBN(); // n = pq - const e = key_params[1].toBN(); - const d = key_params[2].toBN(); // de = 1 mod (p-1)(q-1) - const p = key_params[3].toBN(); - const q = key_params[4].toBN(); - const u = key_params[5].toBN(); // q^-1 mod p - return publicKey.rsa.decrypt(c, n, e, d, p, q, u); - } - case enums.publicKey.elgamal: { - const c1 = data_params[0].toBN(); - const c2 = data_params[1].toBN(); - const p = key_params[0].toBN(); - const x = key_params[3].toBN(); - return publicKey.elgamal.decrypt(c1, c2, p, x); - } - case enums.publicKey.ecdh: { - const oid = key_params[0]; - const kdf_params = key_params[2]; - const V = data_params[0].toUint8Array(); - const C = data_params[1].data; - const d = key_params[3].toUint8Array(); - return publicKey.elliptic.ecdh.decrypt( - oid, kdf_params.cipher, kdf_params.hash, V, C, d, fingerprint); - } - default: - throw new Error('Invalid public key encryption algorithm.'); + switch (algo) { + case enums.publicKey.rsa_encrypt_sign: + case enums.publicKey.rsa_encrypt: { + const c = data_params[0].toBN(); + const n = key_params[0].toBN(); // n = pq + const e = key_params[1].toBN(); + const d = key_params[2].toBN(); // de = 1 mod (p-1)(q-1) + const p = key_params[3].toBN(); + const q = key_params[4].toBN(); + const u = key_params[5].toBN(); // q^-1 mod p + return publicKey.rsa.decrypt(c, n, e, d, p, q, u); } - }())); + case enums.publicKey.elgamal: { + const c1 = data_params[0].toBN(); + const c2 = data_params[1].toBN(); + const p = key_params[0].toBN(); + const x = key_params[3].toBN(); + return publicKey.elgamal.decrypt(c1, c2, p, x); + } + case enums.publicKey.ecdh: { + const oid = key_params[0]; + const kdf_params = key_params[2]; + const V = data_params[0].toUint8Array(); + const C = data_params[1].data; + const d = key_params[3].toUint8Array(); + return publicKey.elliptic.ecdh.decrypt( + oid, kdf_params.cipher, kdf_params.hash, V, C, d, fingerprint); + } + default: + throw new Error('Invalid public key encryption algorithm.'); + } }, /** Returns the types comprising the private key of an algorithm diff --git a/src/crypto/public_key/elliptic/ecdh.js b/src/crypto/public_key/elliptic/ecdh.js index 3a1be448..59831073 100644 --- a/src/crypto/public_key/elliptic/ecdh.js +++ b/src/crypto/public_key/elliptic/ecdh.js @@ -52,13 +52,10 @@ function buildEcdhParam(public_algo, oid, cipher_algo, hash_algo, fingerprint) { } // Key Derivation Function (RFC 6637) -async function kdf(hash_algo, S, length, param, curve, stripLeading=false, stripTrailing=false) { - const len = curve.curve.curve.p.byteLength(); - // Note: this is not ideal, but the RFC's are unclear +async function kdf(hash_algo, X, length, param, stripLeading=false, stripTrailing=false) { + // Note: X is little endian for Curve25519, big-endian for all others. + // This is not ideal, but the RFC's are unclear // https://tools.ietf.org/html/draft-ietf-openpgp-rfc4880bis-02#appendix-B - let X = curve.curve.curve.type === 'mont' ? - S.toArrayLike(Uint8Array, 'le', len) : - S.toArrayLike(Uint8Array, 'be', len); let i; if (stripLeading) { // Work around old go crypto bug @@ -88,23 +85,19 @@ async function kdf(hash_algo, S, length, param, curve, stripLeading=false, strip */ async function genPublicEphemeralKey(curve, Q) { if (curve.name === 'curve25519') { - const { secretKey } = nacl.box.keyPair(); - const one = curve.curve.curve.one; - const mask = one.ushln(255 - 3).sub(one).ushln(3); - let priv = new BN(secretKey); - priv = priv.or(one.ushln(255 - 1)); - priv = priv.and(mask); - priv = priv.toArrayLike(Uint8Array, 'le', 32); - const S = nacl.scalarMult(priv, Q.subarray(1)); - const { publicKey } = nacl.box.keyPair.fromSecretKey(priv); - const ret = { V: util.concatUint8Array([new Uint8Array([0x40]), publicKey]), S: new BN(S, 'le') }; - return ret; + const { secretKey: d } = nacl.box.keyPair(); + const { secretKey, sharedKey } = await genPrivateEphemeralKey(curve, Q, d); + let { publicKey } = nacl.box.keyPair.fromSecretKey(secretKey); + publicKey = util.concatUint8Array([new Uint8Array([0x40]), publicKey]); + return { publicKey, sharedKey }; // Note: sharedKey is little-endian here, unlike below } const v = await curve.genKeyPair(); Q = curve.keyFromPublic(Q); - const V = new Uint8Array(v.getPublic()); + const publicKey = new Uint8Array(v.getPublic()); const S = v.derive(Q); - return { V, S }; + const len = curve.curve.curve.p.byteLength(); + const sharedKey = S.toArrayLike(Uint8Array, 'be', len); + return { publicKey, sharedKey }; } /** @@ -121,12 +114,12 @@ async function genPublicEphemeralKey(curve, Q) { */ async function encrypt(oid, cipher_algo, hash_algo, m, Q, fingerprint) { const curve = new Curve(oid); - const { V, S } = await genPublicEphemeralKey(curve, Q); + const { publicKey, sharedKey } = await genPublicEphemeralKey(curve, Q); const param = buildEcdhParam(enums.publicKey.ecdh, oid, cipher_algo, hash_algo, fingerprint); cipher_algo = enums.read(enums.symmetric, cipher_algo); - const Z = await kdf(hash_algo, S, cipher[cipher_algo].keySize, param, curve); - const C = aes_kw.wrap(Z, m.toString()); - return { V, C }; + const Z = await kdf(hash_algo, sharedKey, cipher[cipher_algo].keySize, param); + const wrappedKey = aes_kw.wrap(Z, m.toString()); + return { publicKey, wrappedKey }; } /** @@ -142,15 +135,20 @@ async function genPrivateEphemeralKey(curve, V, d) { if (curve.name === 'curve25519') { const one = curve.curve.curve.one; const mask = one.ushln(255 - 3).sub(one).ushln(3); - let priv = new BN(d); - priv = priv.or(one.ushln(255 - 1)); - priv = priv.and(mask); - const S = nacl.scalarMult(priv.toArrayLike(Uint8Array, 'le', 32), V.subarray(1)); - return new BN(S, 'le'); + let secretKey = new BN(d); + secretKey = secretKey.or(one.ushln(255 - 1)); + secretKey = secretKey.and(mask); + secretKey = secretKey.toArrayLike(Uint8Array, 'le', 32); + const sharedKey = nacl.scalarMult(secretKey, V.subarray(1)); + return { secretKey, sharedKey }; // Note: sharedKey is little-endian here, unlike below } V = curve.keyFromPublic(V); d = curve.keyFromPrivate(d); - return d.derive(V); + const secretKey = new Uint8Array(d.getPrivate()); + const S = d.derive(V); + const len = curve.curve.curve.p.byteLength(); + const sharedKey = S.toArrayLike(Uint8Array, 'be', len); + return { secretKey, sharedKey }; } /** @@ -168,14 +166,14 @@ async function genPrivateEphemeralKey(curve, V, d) { */ async function decrypt(oid, cipher_algo, hash_algo, V, C, d, fingerprint) { const curve = new Curve(oid); - const S = await genPrivateEphemeralKey(curve, V, d); + const { sharedKey } = await genPrivateEphemeralKey(curve, V, d); const param = buildEcdhParam(enums.publicKey.ecdh, oid, cipher_algo, hash_algo, fingerprint); cipher_algo = enums.read(enums.symmetric, cipher_algo); let err; for (let i = 0; i < 3; i++) { try { // Work around old go crypto bug and old OpenPGP.js bug, respectively. - const Z = await kdf(hash_algo, S, cipher[cipher_algo].keySize, param, curve, i === 1, i === 2); + const Z = await kdf(hash_algo, sharedKey, cipher[cipher_algo].keySize, param, i === 1, i === 2); return new BN(aes_kw.unwrap(Z, C)); } catch (e) { err = e; diff --git a/src/packet/public_key_encrypted_session_key.js b/src/packet/public_key_encrypted_session_key.js index fc93e430..8bab822c 100644 --- a/src/packet/public_key_encrypted_session_key.js +++ b/src/packet/public_key_encrypted_session_key.js @@ -137,8 +137,8 @@ PublicKeyEncryptedSessionKey.prototype.encrypt = async function (key) { */ PublicKeyEncryptedSessionKey.prototype.decrypt = async function (key) { const algo = enums.write(enums.publicKey, this.publicKeyAlgorithm); - const result = await crypto.publicKeyDecrypt( - algo, key.params, this.encrypted, key.getFingerprintBytes()); + const result = new type_mpi(await crypto.publicKeyDecrypt( + algo, key.params, this.encrypted, key.getFingerprintBytes())); let checksum; let decoded; diff --git a/test/crypto/crypto.js b/test/crypto/crypto.js index 6459d346..53a0a321 100644 --- a/test/crypto/crypto.js +++ b/test/crypto/crypto.js @@ -364,7 +364,7 @@ describe('API functional testing', function() { return crypto.publicKeyDecrypt( 1, RSApubMPIs.concat(RSAsecMPIs), RSAEncryptedData ).then(data => { - data = data.write(); + data = new openpgp.MPI(data).write(); data = util.Uint8Array_to_str(data.subarray(2, data.length)); const result = crypto.pkcs1.eme.decode(data, RSApubMPIs[0].byteLength()); @@ -383,7 +383,7 @@ describe('API functional testing', function() { return crypto.publicKeyDecrypt( 16, ElgamalpubMPIs.concat(ElgamalsecMPIs), ElgamalEncryptedData ).then(data => { - data = data.write(); + data = new openpgp.MPI(data).write(); data = util.Uint8Array_to_str(data.subarray(2, data.length)); const result = crypto.pkcs1.eme.decode(data, ElgamalpubMPIs[0].byteLength()); diff --git a/test/crypto/elliptic.js b/test/crypto/elliptic.js index a8da94ec..f8ac83f9 100644 --- a/test/crypto/elliptic.js +++ b/test/crypto/elliptic.js @@ -429,7 +429,7 @@ describe('Elliptic Curve Cryptography', function () { async function genPublicEphemeralKey(curve, Q, fingerprint) { const curveObj = new openpgp.crypto.publicKey.elliptic.Curve(curve); const oid = new openpgp.OID(curveObj.oid); - const { V, S } = await openpgp.crypto.publicKey.elliptic.ecdh.genPublicEphemeralKey( + const { publicKey: V, sharedKey } = await openpgp.crypto.publicKey.elliptic.ecdh.genPublicEphemeralKey( curveObj, Q ); let cipher_algo = curveObj.cipher; @@ -439,14 +439,14 @@ describe('Elliptic Curve Cryptography', function () { ); cipher_algo = openpgp.enums.read(openpgp.enums.symmetric, cipher_algo); const Z = await openpgp.crypto.publicKey.elliptic.ecdh.kdf( - hash_algo, S, openpgp.crypto.cipher[cipher_algo].keySize, param, curveObj, false + hash_algo, sharedKey, openpgp.crypto.cipher[cipher_algo].keySize, param, curveObj, false ); return { V, Z }; } async function genPrivateEphemeralKey(curve, V, d, fingerprint) { const curveObj = new openpgp.crypto.publicKey.elliptic.Curve(curve); const oid = new openpgp.OID(curveObj.oid); - const S = await openpgp.crypto.publicKey.elliptic.ecdh.genPrivateEphemeralKey( + const { sharedKey } = await openpgp.crypto.publicKey.elliptic.ecdh.genPrivateEphemeralKey( curveObj, V, d ); let cipher_algo = curveObj.cipher; @@ -456,7 +456,7 @@ describe('Elliptic Curve Cryptography', function () { ); cipher_algo = openpgp.enums.read(openpgp.enums.symmetric, cipher_algo); const Z = await openpgp.crypto.publicKey.elliptic.ecdh.kdf( - hash_algo, S, openpgp.crypto.cipher[cipher_algo].keySize, param, curveObj, false + hash_algo, sharedKey, openpgp.crypto.cipher[cipher_algo].keySize, param, curveObj, false ); return Z; } From caa712c3376e6922ae7415fe178f1f00ab79554f Mon Sep 17 00:00:00 2001 From: Daniel Huigens Date: Wed, 1 May 2019 13:07:06 +0200 Subject: [PATCH 4/9] Fix using local dependencies --- Gruntfile.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gruntfile.js b/Gruntfile.js index 94afa86b..5c784f8c 100644 --- a/Gruntfile.js +++ b/Gruntfile.js @@ -56,7 +56,7 @@ module.exports = function(grunt) { "transform-runtime" ] : [], ignore: ['*.min.js'], - presets: [["env", { + presets: [[require.resolve('babel-preset-env'), { targets: { browsers: compat ? [ 'IE >= 11', From ffa83448092f9f4698b7123efa691267e15508c0 Mon Sep 17 00:00:00 2001 From: Daniel Huigens Date: Wed, 1 May 2019 14:27:18 +0200 Subject: [PATCH 5/9] Only include tweetnacl functions we need --- npm-shrinkwrap.json | 5 ++--- package.json | 2 +- src/crypto/public_key/elliptic/ecdh.js | 4 ++-- src/crypto/public_key/elliptic/eddsa.js | 11 +++++------ src/crypto/public_key/index.js | 2 +- 5 files changed, 11 insertions(+), 13 deletions(-) diff --git a/npm-shrinkwrap.json b/npm-shrinkwrap.json index 3b1fb6a4..9bbc9b40 100644 --- a/npm-shrinkwrap.json +++ b/npm-shrinkwrap.json @@ -7235,9 +7235,8 @@ } }, "tweetnacl": { - "version": "1.0.1", - "resolved": "https://registry.npmjs.org/tweetnacl/-/tweetnacl-1.0.1.tgz", - "integrity": "sha512-kcoMoKTPYnoeS50tzoqjPY3Uv9axeuuFAZY9M/9zFnhoVvRfxz9K29IMPD7jGmt2c8SW7i3gT9WqDl2+nV7p4A==", + "version": "github:openpgpjs/tweetnacl-js#1ef755f2b252a3e328ac739848d00e0dad76be2d", + "from": "github:openpgpjs/tweetnacl-js#1ef755f2b252a3e328ac739848d00e0dad76be2d", "dev": true }, "type-check": { diff --git a/package.json b/package.json index 4936a643..c07298de 100644 --- a/package.json +++ b/package.json @@ -80,7 +80,7 @@ "hash.js": "^1.1.3", "pako": "^1.0.6", "seek-bzip": "github:openpgpjs/seek-bzip#3aca608ffedc055a1da1d898ecb244804ef32209", - "tweetnacl": "^1.0.1", + "tweetnacl": "github:openpgpjs/tweetnacl-js#1ef755f2b252a3e328ac739848d00e0dad76be2d", "web-stream-tools": "github:openpgpjs/web-stream-tools#84a497715c9df271a673f8616318264ab42ab3cc" }, "dependencies": { diff --git a/src/crypto/public_key/elliptic/ecdh.js b/src/crypto/public_key/elliptic/ecdh.js index 59831073..47579141 100644 --- a/src/crypto/public_key/elliptic/ecdh.js +++ b/src/crypto/public_key/elliptic/ecdh.js @@ -30,7 +30,7 @@ */ import BN from 'bn.js'; -import nacl from 'tweetnacl'; +import nacl from 'tweetnacl/nacl-fast-light.js'; import Curve from './curves'; import aes_kw from '../../aes_kw'; import cipher from '../../cipher'; @@ -133,7 +133,7 @@ async function encrypt(oid, cipher_algo, hash_algo, m, Q, fingerprint) { */ async function genPrivateEphemeralKey(curve, V, d) { if (curve.name === 'curve25519') { - const one = curve.curve.curve.one; + const one = new BN(1); const mask = one.ushln(255 - 3).sub(one).ushln(3); let secretKey = new BN(d); secretKey = secretKey.or(one.ushln(255 - 1)); diff --git a/src/crypto/public_key/elliptic/eddsa.js b/src/crypto/public_key/elliptic/eddsa.js index c05b1b47..fdde149c 100644 --- a/src/crypto/public_key/elliptic/eddsa.js +++ b/src/crypto/public_key/elliptic/eddsa.js @@ -17,16 +17,19 @@ /** * @fileoverview Implementation of EdDSA following RFC4880bis-03 for OpenPGP + * @requires hash.js * @requires tweetnacl * @requires crypto/public_key/elliptic/curve * @requires util * @module crypto/public_key/elliptic/eddsa */ -import nacl from 'tweetnacl'; -import Curve from './curves'; +import sha512 from 'hash.js/lib/hash/sha/512'; +import nacl from 'tweetnacl/nacl-fast-light.js'; import util from '../../../util'; +nacl.hash = bytes => new Uint8Array(sha512().update(bytes).digest()); + /** * Sign a message using the provided key * @param {module:type/oid} oid Elliptic curve object identifier @@ -63,10 +66,6 @@ async function sign(oid, hash_algo, m, d, hashed) { async function verify(oid, hash_algo, { R, S }, m, publicKey, hashed) { const signature = util.concatUint8Array([R, S]); return nacl.sign.detached.verify(hashed, signature, publicKey.subarray(1)); - - const curve = new Curve(oid); - const key = curve.keyFromPublic(Q); - return key.verify(m, signature, hash_algo, hashed); } export default { sign, verify }; diff --git a/src/crypto/public_key/index.js b/src/crypto/public_key/index.js index 23bceddb..4e2edbcf 100644 --- a/src/crypto/public_key/index.js +++ b/src/crypto/public_key/index.js @@ -8,7 +8,7 @@ * @module crypto/public_key */ -import nacl from 'tweetnacl'; +import nacl from 'tweetnacl/nacl-fast-light.js'; import rsa from './rsa'; import elgamal from './elgamal'; import elliptic from './elliptic'; From ecc8ae2a090ed0ff309e3cdb657d74df642ec6f9 Mon Sep 17 00:00:00 2001 From: Daniel Huigens Date: Wed, 1 May 2019 18:30:26 +0200 Subject: [PATCH 6/9] Don't include package.json --- npm-shrinkwrap.json | 8 ++++---- package.json | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/npm-shrinkwrap.json b/npm-shrinkwrap.json index 9bbc9b40..415112c6 100644 --- a/npm-shrinkwrap.json +++ b/npm-shrinkwrap.json @@ -2306,8 +2306,8 @@ "dev": true }, "elliptic": { - "version": "github:openpgpjs/elliptic#ad81845f693effa5b4b6d07db2e82112de222f48", - "from": "github:openpgpjs/elliptic#ad81845f693effa5b4b6d07db2e82112de222f48", + "version": "github:openpgpjs/elliptic#1beea74833b48bd5698ed079c75fd21f0eb70b1c", + "from": "github:openpgpjs/elliptic#1beea74833b48bd5698ed079c75fd21f0eb70b1c", "dev": true, "requires": { "bn.js": "^4.4.0", @@ -6496,8 +6496,8 @@ } }, "seek-bzip": { - "version": "github:openpgpjs/seek-bzip#3aca608ffedc055a1da1d898ecb244804ef32209", - "from": "github:openpgpjs/seek-bzip#3aca608ffedc055a1da1d898ecb244804ef32209", + "version": "github:openpgpjs/seek-bzip#6187fc025851d35c4e104a25ea15a10b9b8d6f7d", + "from": "github:openpgpjs/seek-bzip#6187fc025851d35c4e104a25ea15a10b9b8d6f7d", "dev": true, "requires": { "commander": "~2.8.1" diff --git a/package.json b/package.json index c07298de..7a4578f9 100644 --- a/package.json +++ b/package.json @@ -76,10 +76,10 @@ "asmcrypto.js": "github:openpgpjs/asmcrypto#6e4e407b9b8ae317925a9e677cc7b4de3e447e83", "bn.js": "^4.11.8", "buffer": "^5.0.8", - "elliptic": "github:openpgpjs/elliptic#ad81845f693effa5b4b6d07db2e82112de222f48", + "elliptic": "github:openpgpjs/elliptic#1beea74833b48bd5698ed079c75fd21f0eb70b1c", "hash.js": "^1.1.3", "pako": "^1.0.6", - "seek-bzip": "github:openpgpjs/seek-bzip#3aca608ffedc055a1da1d898ecb244804ef32209", + "seek-bzip": "github:openpgpjs/seek-bzip#6187fc025851d35c4e104a25ea15a10b9b8d6f7d", "tweetnacl": "github:openpgpjs/tweetnacl-js#1ef755f2b252a3e328ac739848d00e0dad76be2d", "web-stream-tools": "github:openpgpjs/web-stream-tools#84a497715c9df271a673f8616318264ab42ab3cc" }, From 34e6eacb2f22a3e4a7654d765843dcfbf6e06e4c Mon Sep 17 00:00:00 2001 From: Daniel Huigens Date: Wed, 1 May 2019 22:03:37 +0200 Subject: [PATCH 7/9] Don't attempt to use workers if they fail to load --- README.md | 4 ++-- src/openpgp.js | 13 ++++++++++--- src/worker/async_proxy.js | 19 ++++++++++++++++++- src/worker/worker.js | 5 +++++ test/general/brainpool.js | 4 ++-- test/general/ecc_nist.js | 4 ++-- test/general/openpgp.js | 38 ++++++++++++++++++++++---------------- test/general/x25519.js | 4 ++-- test/worker/async_proxy.js | 10 ++++++---- 9 files changed, 69 insertions(+), 32 deletions(-) diff --git a/README.md b/README.md index c90e4019..bc320419 100644 --- a/README.md +++ b/README.md @@ -119,7 +119,7 @@ Here are some examples of how to use the v2.x+ API. For more elaborate examples ```js var openpgp = require('openpgp'); // use as CommonJS, AMD, ES6 module or via window.openpgp -openpgp.initWorker({ path:'openpgp.worker.js' }) // set the relative web worker path +await openpgp.initWorker({ path:'openpgp.worker.js' }) // set the relative web worker path ``` #### Encrypt and decrypt *Uint8Array* data with a password @@ -159,7 +159,7 @@ Encryption will use the algorithm preferred by the public key (defaults to aes25 ```js const openpgp = require('openpgp') // use as CommonJS, AMD, ES6 module or via window.openpgp -openpgp.initWorker({ path:'openpgp.worker.js' }) // set the relative web worker path +await openpgp.initWorker({ path:'openpgp.worker.js' }) // set the relative web worker path // put keys in backtick (``) to avoid errors caused by spaces or tabs const pubkey = `-----BEGIN PGP PUBLIC KEY BLOCK----- diff --git a/src/openpgp.js b/src/openpgp.js index 436d7a54..435b0ad0 100644 --- a/src/openpgp.js +++ b/src/openpgp.js @@ -63,12 +63,19 @@ let asyncProxy; // instance of the asyncproxy * @param {String} path relative path to the worker scripts, default: 'openpgp.worker.js' * @param {Number} n number of workers to initialize * @param {Array} workers alternative to path parameter: web workers initialized with 'openpgp.worker.js' + * @returns {Promise} returns a promise that resolves to true if all workers have succesfully finished loading + * @async */ -export function initWorker({ path='openpgp.worker.js', n = 1, workers = [] } = {}) { +export async function initWorker({ path='openpgp.worker.js', n = 1, workers = [] } = {}) { if (workers.length || (typeof window !== 'undefined' && window.Worker && window.MessageChannel)) { - asyncProxy = new AsyncProxy({ path, n, workers, config }); - return true; + const proxy = new AsyncProxy({ path, n, workers, config }); + const loaded = await proxy.loaded(); + if (loaded) { + asyncProxy = proxy; + return true; + } } + return false; } /** diff --git a/src/worker/async_proxy.js b/src/worker/async_proxy.js index 35ed6990..ce809206 100644 --- a/src/worker/async_proxy.js +++ b/src/worker/async_proxy.js @@ -49,6 +49,9 @@ function AsyncProxy({ path='openpgp.worker.js', n = 1, workers = [], config } = const handleMessage = workerId => event => { const msg = event.data; switch (msg.event) { + case 'loaded': + this.workers[workerId].loadedResolve(true); + break; case 'method-return': if (msg.err) { // fail @@ -83,10 +86,15 @@ function AsyncProxy({ path='openpgp.worker.js', n = 1, workers = [], config } = let workerId = 0; this.workers.forEach(worker => { + worker.loadedPromise = new Promise(resolve => { + worker.loadedResolve = resolve; + }); worker.requests = 0; worker.onmessage = handleMessage(workerId++); worker.onerror = e => { - throw new Error('Unhandled error in openpgp worker: ' + e.message + ' (' + e.filename + ':' + e.lineno + ')'); + worker.loadedResolve(false); + console.error('Unhandled error in openpgp worker: ' + e.message + ' (' + e.filename + ':' + e.lineno + ')'); + return false; }; if (config) { @@ -99,6 +107,15 @@ function AsyncProxy({ path='openpgp.worker.js', n = 1, workers = [], config } = this.currentID = 0; } +/** + * Returns a promise that resolves when all workers have finished loading + * @returns {Promise} Resolves to true if all workers have loaded succesfully; false otherwise +*/ +AsyncProxy.prototype.loaded = async function() { + const loaded = await Promise.all(this.workers.map(worker => worker.loadedPromise)); + return loaded.every(Boolean); +}; + /** * Get new request ID * @returns {integer} New unique request ID diff --git a/src/worker/worker.js b/src/worker/worker.js index 845b1a6b..a9281372 100644 --- a/src/worker/worker.js +++ b/src/worker/worker.js @@ -135,3 +135,8 @@ function delegate(id, method, options) { function response(event) { self.postMessage(event, openpgp.util.getTransferables(event.data, true)); } + +/** + * Let the main window know the worker has loaded. + */ +postMessage({ event: 'loaded' }); diff --git a/test/general/brainpool.js b/test/general/brainpool.js index 1a50ce3c..a8a2a9c6 100644 --- a/test/general/brainpool.js +++ b/test/general/brainpool.js @@ -273,8 +273,8 @@ describe('Brainpool Cryptography', function () { tryTests('Brainpool Worker Tests', omnibus, { if: typeof window !== 'undefined' && window.Worker, - before: function() { - openpgp.initWorker({ path:'../dist/openpgp.worker.js' }); + before: async function() { + await openpgp.initWorker({ path:'../dist/openpgp.worker.js' }); }, beforeEach: function() { openpgp.config.use_native = true; diff --git a/test/general/ecc_nist.js b/test/general/ecc_nist.js index e6faff65..f079dd16 100644 --- a/test/general/ecc_nist.js +++ b/test/general/ecc_nist.js @@ -297,8 +297,8 @@ describe('Elliptic Curve Cryptography', function () { tryTests('ECC Worker Tests', omnibus, { if: typeof window !== 'undefined' && window.Worker, - before: function() { - openpgp.initWorker({ path:'../dist/openpgp.worker.js' }); + before: async function() { + await openpgp.initWorker({ path:'../dist/openpgp.worker.js' }); }, beforeEach: function() { openpgp.config.use_native = true; diff --git a/test/general/openpgp.js b/test/general/openpgp.js index fa2f04f5..24cf3741 100644 --- a/test/general/openpgp.js +++ b/test/general/openpgp.js @@ -444,13 +444,16 @@ describe('[Sauce Labs Group 2] OpenPGP.js public api tests', function() { openpgp.destroyWorker(); // cleanup worker in case of failure }); - it('should work', function() { + it('should work', async function() { const workerStub = { postMessage: function() {} }; - openpgp.initWorker({ - workers: [workerStub] - }); + await Promise.all([ + openpgp.initWorker({ + workers: [workerStub] + }), + workerStub.onmessage({ data: { event: 'loaded' } }) + ]); expect(openpgp.getWorker()).to.exist; openpgp.destroyWorker(); expect(openpgp.getWorker()).to.not.exist; @@ -595,13 +598,16 @@ describe('[Sauce Labs Group 2] OpenPGP.js public api tests', function() { }); }); - it('should delegate to async proxy', function() { + it('should delegate to async proxy', async function() { const workerStub = { postMessage: function() {} }; - openpgp.initWorker({ - workers: [workerStub] - }); + await Promise.all([ + openpgp.initWorker({ + workers: [workerStub] + }), + workerStub.onmessage({ data: { event: 'loaded' } }) + ]); const proxyGenStub = stub(openpgp.getWorker(), 'delegate'); getWebCryptoAllStub.returns(); @@ -643,9 +649,9 @@ describe('[Sauce Labs Group 2] OpenPGP.js public api tests', function() { }); }); - it('should work in JS (with worker)', function() { + it('should work in JS (with worker)', async function() { openpgp.config.use_native = false; - openpgp.initWorker({ path:'../dist/openpgp.worker.js' }); + await openpgp.initWorker({ path:'../dist/openpgp.worker.js' }); const opt = { userIds: [{ name: 'Test User', email: 'text@example.com' }], numBits: 512 @@ -727,11 +733,11 @@ describe('[Sauce Labs Group 2] OpenPGP.js public api tests', function() { openpgp.config.aead_chunk_size_byte = aead_chunk_size_byteVal; }); - it('Configuration', function() { + it('Configuration', async function() { openpgp.config.show_version = false; openpgp.config.commentstring = 'different'; if (openpgp.getWorker()) { // init again to trigger config event - openpgp.initWorker({ path:'../dist/openpgp.worker.js' }); + await openpgp.initWorker({ path:'../dist/openpgp.worker.js' }); } return openpgp.encrypt({ publicKeys:publicKey.keys, message:openpgp.message.fromText(plaintext) }).then(function(encrypted) { expect(encrypted.data).to.exist; @@ -749,7 +755,7 @@ describe('[Sauce Labs Group 2] OpenPGP.js public api tests', function() { const { workers } = openpgp.getWorker(); try { await privateKey.keys[0].decrypt(passphrase) - openpgp.initWorker({path: '../dist/openpgp.worker.js', workers, n: 2}); + await openpgp.initWorker({path: '../dist/openpgp.worker.js', workers, n: 2}); const workerTest = (_, index) => { const plaintext = input.createSomeMessage() + index; @@ -770,7 +776,7 @@ describe('[Sauce Labs Group 2] OpenPGP.js public api tests', function() { }; await Promise.all(Array(10).fill(null).map(workerTest)); } finally { - openpgp.initWorker({path: '../dist/openpgp.worker.js', workers, n: 1 }); + await openpgp.initWorker({path: '../dist/openpgp.worker.js', workers, n: 1 }); } }); @@ -828,8 +834,8 @@ describe('[Sauce Labs Group 2] OpenPGP.js public api tests', function() { tryTests('CFB mode (asm.js, worker)', tests, { if: typeof window !== 'undefined' && window.Worker, - before: function() { - openpgp.initWorker({ path:'../dist/openpgp.worker.js' }); + before: async function() { + await openpgp.initWorker({ path:'../dist/openpgp.worker.js' }); }, beforeEach: function() { openpgp.config.aead_protect = false; diff --git a/test/general/x25519.js b/test/general/x25519.js index 660b7054..ab78dfec 100644 --- a/test/general/x25519.js +++ b/test/general/x25519.js @@ -319,8 +319,8 @@ describe('X25519 Cryptography', function () { tryTests('X25519 Worker Tests', omnibus, { if: typeof window !== 'undefined' && window.Worker, - before: function() { - openpgp.initWorker({ path:'../dist/openpgp.worker.js' }); + before: async function() { + await openpgp.initWorker({ path:'../dist/openpgp.worker.js' }); }, beforeEach: function() { openpgp.config.use_native = true; diff --git a/test/worker/async_proxy.js b/test/worker/async_proxy.js index ed8d30a4..0fe02bb0 100644 --- a/test/worker/async_proxy.js +++ b/test/worker/async_proxy.js @@ -37,7 +37,7 @@ let pubKey; tryTests('Async Proxy', tests, { if: typeof window !== 'undefined' && window.Worker && window.MessageChannel, before: async function() { - openpgp.initWorker({ path:'../dist/openpgp.worker.js' }); + await openpgp.initWorker({ path:'../dist/openpgp.worker.js' }); pubKey = (await openpgp.key.readArmored(pub_key)).keys[0]; }, after: function() { @@ -48,11 +48,13 @@ tryTests('Async Proxy', tests, { function tests() { describe('Random number pipeline', function() { - it('Random number buffer automatically reseeded', function() { + it('Random number buffer automatically reseeded', async function() { const worker = new Worker('../dist/openpgp.worker.js'); const wProxy = new openpgp.AsyncProxy({ path:'../dist/openpgp.worker.js', workers: [worker] }); - - return wProxy.delegate('encrypt', { publicKeys:[pubKey], message:openpgp.message.fromText(plaintext) }); + const loaded = await wProxy.loaded(); + if (loaded) { + return wProxy.delegate('encrypt', { publicKeys:[pubKey], message:openpgp.message.fromText(plaintext) }); + } }); }); From f322aaf715b50689fb4abf6f848fe91fe577db53 Mon Sep 17 00:00:00 2001 From: Daniel Huigens Date: Wed, 1 May 2019 22:13:28 +0200 Subject: [PATCH 8/9] CI: Require Chrome instead of Firefox to succeed Sauce Labs seems to be having issues with Firefox. --- .travis.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index 9ce19610..1a088d58 100644 --- a/.travis.yml +++ b/.travis.yml @@ -17,11 +17,11 @@ matrix: env: OPENPGP_NODE_JS='12' OPENPGPJSTEST='unit' - node_js: "9" env: BROWSER='Firefox' VERSION='26' PLATFORM='macOS 10.13' OPENPGPJSTEST='saucelabs' COMPAT=1 - - node_js: "10" + - node_js: "9" env: BROWSER='Firefox' VERSION='61' PLATFORM='macOS 10.13' OPENPGPJSTEST='saucelabs' - node_js: "9" env: BROWSER='Chrome' VERSION='49' PLATFORM='macOS 10.13' OPENPGPJSTEST='saucelabs' COMPAT=1 - - node_js: "9" + - node_js: "10" env: BROWSER='Chrome' VERSION='68' PLATFORM='macOS 10.13' OPENPGPJSTEST='saucelabs' - node_js: "9" env: BROWSER='Internet Explorer' VERSION='11.103' PLATFORM='Windows 10' OPENPGPJSTEST='saucelabs' COMPAT=1 From 6689f934653b67512843fe3f3e3c0f29da6bde52 Mon Sep 17 00:00:00 2001 From: Daniel Huigens Date: Thu, 2 May 2019 12:54:17 +0200 Subject: [PATCH 9/9] Fix flaky test in Edge --- test/general/streaming.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/general/streaming.js b/test/general/streaming.js index 4ddb4351..e9ee2a1b 100644 --- a/test/general/streaming.js +++ b/test/general/streaming.js @@ -640,7 +640,7 @@ function tests() { const reader = openpgp.stream.getReader(signed.signature); expect((await reader.readBytes(31)).toString('utf8')).to.equal('-----BEGIN PGP SIGNATURE-----\r\n'); dataArrived(); - await new Promise(resolve => setTimeout(resolve, 1000)); + await new Promise(resolve => setTimeout(resolve, 3000)); expect(i).to.be.greaterThan(100); });