diff --git a/lib/elliptic/ec/key.js b/lib/elliptic/ec/key.js index 8d075fb3..69054b3e 100644 --- a/lib/elliptic/ec/key.js +++ b/lib/elliptic/ec/key.js @@ -89,6 +89,9 @@ KeyPair.prototype._importPublic = function _importPublic(key, enc) { // ECDH KeyPair.prototype.derive = function derive(pub) { + if(!pub.validate()) { + assert(pub.validate(), 'public point not validated'); + } return pub.mul(this.priv).getX(); }; diff --git a/lib/elliptic/ec/key.js.orig b/lib/elliptic/ec/key.js.orig new file mode 100644 index 00000000..8d075fb3 --- /dev/null +++ b/lib/elliptic/ec/key.js.orig @@ -0,0 +1,107 @@ +'use strict'; + +var BN = require('bn.js'); + +function KeyPair(ec, options) { + this.ec = ec; + this.priv = null; + this.pub = null; + + // KeyPair(ec, { priv: ..., pub: ... }) + if (options.priv) + this._importPrivate(options.priv, options.privEnc); + if (options.pub) + this._importPublic(options.pub, options.pubEnc); +} +module.exports = KeyPair; + +KeyPair.fromPublic = function fromPublic(ec, pub, enc) { + if (pub instanceof KeyPair) + return pub; + + return new KeyPair(ec, { + pub: pub, + pubEnc: enc + }); +}; + +KeyPair.fromPrivate = function fromPrivate(ec, priv, enc) { + if (priv instanceof KeyPair) + return priv; + + return new KeyPair(ec, { + priv: priv, + privEnc: enc + }); +}; + +KeyPair.prototype.validate = function validate() { + var pub = this.getPublic(); + + if (pub.isInfinity()) + return { result: false, reason: 'Invalid public key' }; + if (!pub.validate()) + return { result: false, reason: 'Public key is not a point' }; + if (!pub.mul(this.ec.curve.n).isInfinity()) + return { result: false, reason: 'Public key * N != O' }; + + return { result: true, reason: null }; +}; + +KeyPair.prototype.getPublic = function getPublic(compact, enc) { + // compact is optional argument + if (typeof compact === 'string') { + enc = compact; + compact = null; + } + + if (!this.pub) + this.pub = this.ec.g.mul(this.priv); + + if (!enc) + return this.pub; + + return this.pub.encode(enc, compact); +}; + +KeyPair.prototype.getPrivate = function getPrivate(enc) { + if (enc === 'hex') + return this.priv.toString(16, 2); + else + return this.priv; +}; + +KeyPair.prototype._importPrivate = function _importPrivate(key, enc) { + this.priv = new BN(key, enc || 16); + + // Ensure that the priv won't be bigger than n, otherwise we may fail + // in fixed multiplication method + this.priv = this.priv.umod(this.ec.curve.n); +}; + +KeyPair.prototype._importPublic = function _importPublic(key, enc) { + if (key.x || key.y) { + this.pub = this.ec.curve.point(key.x, key.y); + return; + } + this.pub = this.ec.curve.decodePoint(key, enc); +}; + +// ECDH +KeyPair.prototype.derive = function derive(pub) { + return pub.mul(this.priv).getX(); +}; + +// ECDSA +KeyPair.prototype.sign = function sign(msg, enc, options) { + return this.ec.sign(msg, this, enc, options); +}; + +KeyPair.prototype.verify = function verify(msg, signature) { + return this.ec.verify(msg, signature, this); +}; + +KeyPair.prototype.inspect = function inspect() { + return ''; +}; diff --git a/lib/elliptic/ec/signature.js b/lib/elliptic/ec/signature.js index 165b179a..85c63701 100644 --- a/lib/elliptic/ec/signature.js +++ b/lib/elliptic/ec/signature.js @@ -33,11 +33,24 @@ function getLength(buf, p) { return initial; } var octetLen = initial & 0xf; + + // Indefinite length or overflow + if (octetLen === 0 || octetLen > 4) { + return false; + } + var val = 0; for (var i = 0, off = p.place; i < octetLen; i++, off++) { val <<= 8; val |= buf[off]; + val >>>= 0; } + + // Leading zeroes + if (val <= 0x7f) { + return false; + } + p.place = off; return val; } @@ -61,6 +74,9 @@ Signature.prototype._importDER = function _importDER(data, enc) { return false; } var len = getLength(data, p); + if (len === false) { + return false; + } if ((len + p.place) !== data.length) { return false; } @@ -68,21 +84,37 @@ Signature.prototype._importDER = function _importDER(data, enc) { return false; } var rlen = getLength(data, p); + if (rlen === false) { + return false; + } var r = data.slice(p.place, rlen + p.place); p.place += rlen; if (data[p.place++] !== 0x02) { return false; } var slen = getLength(data, p); + if (slen === false) { + return false; + } if (data.length !== slen + p.place) { return false; } var s = data.slice(p.place, slen + p.place); - if (r[0] === 0 && (r[1] & 0x80)) { - r = r.slice(1); + if (r[0] === 0) { + if (r[1] & 0x80) { + r = r.slice(1); + } else { + // Leading zeroes + return false; + } } - if (s[0] === 0 && (s[1] & 0x80)) { - s = s.slice(1); + if (s[0] === 0) { + if (s[1] & 0x80) { + s = s.slice(1); + } else { + // Leading zeroes + return false; + } } this.r = new BN(r); diff --git a/lib/elliptic/ec/signature.js.orig b/lib/elliptic/ec/signature.js.orig new file mode 100644 index 00000000..165b179a --- /dev/null +++ b/lib/elliptic/ec/signature.js.orig @@ -0,0 +1,135 @@ +'use strict'; + +var BN = require('bn.js'); + +var elliptic = require('../../elliptic'); +var utils = elliptic.utils; +var assert = utils.assert; + +function Signature(options, enc) { + if (options instanceof Signature) + return options; + + if (this._importDER(options, enc)) + return; + + assert(options.r && options.s, 'Signature without r or s'); + this.r = new BN(options.r, 16); + this.s = new BN(options.s, 16); + if (options.recoveryParam === undefined) + this.recoveryParam = null; + else + this.recoveryParam = options.recoveryParam; +} +module.exports = Signature; + +function Position() { + this.place = 0; +} + +function getLength(buf, p) { + var initial = buf[p.place++]; + if (!(initial & 0x80)) { + return initial; + } + var octetLen = initial & 0xf; + var val = 0; + for (var i = 0, off = p.place; i < octetLen; i++, off++) { + val <<= 8; + val |= buf[off]; + } + p.place = off; + return val; +} + +function rmPadding(buf) { + var i = 0; + var len = buf.length - 1; + while (!buf[i] && !(buf[i + 1] & 0x80) && i < len) { + i++; + } + if (i === 0) { + return buf; + } + return buf.slice(i); +} + +Signature.prototype._importDER = function _importDER(data, enc) { + data = utils.toArray(data, enc); + var p = new Position(); + if (data[p.place++] !== 0x30) { + return false; + } + var len = getLength(data, p); + if ((len + p.place) !== data.length) { + return false; + } + if (data[p.place++] !== 0x02) { + return false; + } + var rlen = getLength(data, p); + var r = data.slice(p.place, rlen + p.place); + p.place += rlen; + if (data[p.place++] !== 0x02) { + return false; + } + var slen = getLength(data, p); + if (data.length !== slen + p.place) { + return false; + } + var s = data.slice(p.place, slen + p.place); + if (r[0] === 0 && (r[1] & 0x80)) { + r = r.slice(1); + } + if (s[0] === 0 && (s[1] & 0x80)) { + s = s.slice(1); + } + + this.r = new BN(r); + this.s = new BN(s); + this.recoveryParam = null; + + return true; +}; + +function constructLength(arr, len) { + if (len < 0x80) { + arr.push(len); + return; + } + var octets = 1 + (Math.log(len) / Math.LN2 >>> 3); + arr.push(octets | 0x80); + while (--octets) { + arr.push((len >>> (octets << 3)) & 0xff); + } + arr.push(len); +} + +Signature.prototype.toDER = function toDER(enc) { + var r = this.r.toArray(); + var s = this.s.toArray(); + + // Pad values + if (r[0] & 0x80) + r = [ 0 ].concat(r); + // Pad values + if (s[0] & 0x80) + s = [ 0 ].concat(s); + + r = rmPadding(r); + s = rmPadding(s); + + while (!s[0] && !(s[1] & 0x80)) { + s = s.slice(1); + } + var arr = [ 0x02 ]; + constructLength(arr, r.length); + arr = arr.concat(r); + arr.push(0x02); + constructLength(arr, s.length); + var backHalf = arr.concat(s); + var res = [ 0x30 ]; + constructLength(res, backHalf.length); + res = res.concat(backHalf); + return utils.encode(res, enc); +}; diff --git a/test/ecdh-test.js b/test/ecdh-test.js index 9f8f6f0d..4eb4fe3b 100644 --- a/test/ecdh-test.js +++ b/test/ecdh-test.js @@ -25,3 +25,17 @@ describe('ECDH', function() { test('ed25519'); test('secp256k1'); }); + +describe('ECDH twist attack', () => { + it('should be able to prevent a twist attack for secp256k1', () => { + var bobEcdh = new elliptic.ec('secp256k1'); + var malloryEcdh = new elliptic.ec('secp256k1'); + var bob = bobEcdh.genKeyPair(); + // This is a bad point that shouldn't be able to be passed to derive. + // If a bad point can be passed it's possible to perform a twist attack. + var mallory = malloryEcdh.keyFromPublic({ x: 14, y: 16 }); + assert.throws(function () { + bob.derive(mallory.getPublic()); + }); + }); +}); diff --git a/test/ecdh-test.js.orig b/test/ecdh-test.js.orig new file mode 100644 index 00000000..9f8f6f0d --- /dev/null +++ b/test/ecdh-test.js.orig @@ -0,0 +1,27 @@ +var assert = require('assert'); +var elliptic = require('../'); +var hash = require('hash.js'); + +describe('ECDH', function() { + function test(name) { + it('should work with ' + name + ' curve', function() { + var ecdh = new elliptic.ec(name); + var s1 = ecdh.genKeyPair(); + var s2 = ecdh.genKeyPair(); + var sh1 = s1.derive(s2.getPublic()); + var sh2 = s2.derive(s1.getPublic()); + + assert.equal(sh1.toString(16), sh2.toString(16)); + + var sh1 = s1.derive(ecdh.keyFromPublic(s2.getPublic('hex'), 'hex') + .getPublic()); + var sh2 = s2.derive(ecdh.keyFromPublic(s1.getPublic('hex'), 'hex') + .getPublic()); + assert.equal(sh1.toString(16), sh2.toString(16)); + }); + } + + test('curve25519'); + test('ed25519'); + test('secp256k1'); +});