elliptic/eddsa: pay more attention to edge cases.

LICENSE

@@ -1336,3 +1336,26 @@ proprietary programs.  If your program is a subroutine library, you may
 consider it more useful to permit linking proprietary applications with the
 library.  If this is what you want to do, use the GNU Lesser General Public
 License instead of this License.
+
+---
+
+Parts of this software are based on jedisct1/libsodium:
+
+https://github.com/jedisct1/libsodium
+
+ISC License
+
+Copyright (c) 2013-2019
+Frank Denis <j at pureftpd dot org>
+
+Permission to use, copy, modify, and/or distribute this software for any
+purpose with or without fee is hereby granted, provided that the above
+copyright notice and this permission notice appear in all copies.
+
+THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES WITH
+REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND
+FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY SPECIAL, DIRECT,
+INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR
+OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+PERFORMANCE OF THIS SOFTWARE.

lib/js/eddsa.js

@@ -32,6 +32,39 @@ const BN = require('../bn.js');
 const ChaCha20 = require('../chacha20');
 const rng = require('../random');
 
+// Notes about our EdDSA implementation:
+//
+// - All functions are completely unaware of points of small
+//   order and torsion components (in other words, points will
+//   not be explicitly checked for this, anywhere).
+//
+// - To keep the interface consistent with the ECDSA API, we
+//   _do_ disallow points at infinity (in all functions).
+//
+// - We do "cofactor-less" verification by default. This means
+//   if one side of the equation has a torsion component, the
+//   other side must as well.
+//
+// - `verifySingle`/`verifyBatch` do cofactor verification.
+//   Do not use `verifyBatch` expecting the same results
+//   as the regular `verify` call[1].
+//
+// - `deriveWithScalar` and `exchangeWithScalar` automatically
+//   clamp scalars before multiplying (meaning torsion
+//   components are removed from the result and points of
+//   small order will be normalized to infinity).
+//
+// - The HD function, `publicKeyTweakMul`, _does not_ clamp
+//   automatically. It is possible to end up with a torsion
+//   component in the resulting point (assuming the input
+//   point had one).
+//
+// - These notes also spell out why you should avoid using
+//   Edwards curves on a blockchain[2].
+//
+// [1] https://moderncrypto.org/mail-archive/curves/2016/000836.html
+// [2] https://src.getmonero.org/2017/05/17/disclosure-of-a-major-bug-in-cryptonote-based-currencies.html
+
 /*
  * Constants
  */
@@ -164,12 +197,7 @@ class EDDSA {
     assert(Buffer.isBuffer(scalar));
     assert(scalar.length === this.curve.scalarSize);
 
-    if (!this.scalarVerify(scalar)) {
-      scalar = Buffer.from(scalar);
-      scalar = this.curve.clamp(scalar);
-    }
-
-    return scalar;
+    return this.curve.clamp(Buffer.from(scalar));
   }
 
   privateKeyExport(secret) {
@@ -360,7 +388,7 @@ class EDDSA {
   }
 
   publicKeyTweakMul(key, tweak) {
-    const t = this.curve.decodeScalar(tweak).imod(this.curve.n);
+    const t = this.curve.decodeScalar(tweak);
     const A = this.curve.decodePoint(key);
     const point = A.mul(t);
 
@@ -497,16 +525,61 @@ class EDDSA {
     //   G*S == R + A*e
     // But we can use shamir's trick to check:
     //   R == G*S - A*e
-    const Rp = G.mulAdd(S, A, e.ineg().imod(N));
+    return G.mulAdd(S, A.neg(), e).eq(R);
+  }
+
+  verifySingle(msg, sig, key, ph, ctx) {
+    assert(Buffer.isBuffer(msg));
+    assert(Buffer.isBuffer(sig));
+    assert(Buffer.isBuffer(key));
+    assert(ph == null || typeof ph === 'boolean');
+    assert(ctx == null || Buffer.isBuffer(ctx));
+    assert(!ctx || ctx.length <= 255);
+
+    if (!this.curve.context && ctx != null)
+      assert(ph != null, 'Must pass pre-hash flag with context.');
+
+    if (sig.length !== this.curve.fieldSize * 2)
+      return false;
+
+    if (key.length !== this.curve.fieldSize)
+      return false;
+
+    try {
+      return this._verifySingle(msg, sig, key, ph, ctx);
+    } catch (e) {
+      return false;
+    }
+  }
 
-    // We don't allow points at infinity.
-    // This is technically enforced by the
-    // decodePoint call above, but put it
-    // here to be more explicit.
-    if (Rp.isInfinity())
+  _verifySingle(msg, sig, key, ph, ctx) {
+    const N = this.curve.n;
+    const h = this.curve.h.toNumber();
+    const G = this.curve.g;
+    const Rraw = sig.slice(0, this.curve.fieldSize);
+    const Sraw = sig.slice(this.curve.fieldSize);
+    const R = this.curve.decodePoint(Rraw);
+    const S = this.curve.decodeField(Sraw);
+    const A = this.curve.decodePoint(key);
+
+    // Note: S is technically a scalar, but decode
+    // as a field element due to the useless byte.
+    if (S.cmp(N) >= 0)
       return false;
 
-    return Rp.eq(R);
+    // e = H(R, A, m).
+    const e = this.hashInt(ph, ctx, Rraw, key, msg);
+
+    // Ensure terms are multiples of `h`.
+    const Sh = S.imuln(h);
+    const eh = e.imuln(h);
+    const Rh = R.mulH();
+
+    // The spec says to check:
+    //   G*S*h == (R + A*e)*h
+    // But we can use shamir's trick to check:
+    //   R*h == G*S*h - A*e*h
+    return G.mulAdd(Sh, A.neg(), eh).eq(Rh);
   }
 
   verifyBatch(batch, ph, ctx) {
@@ -543,6 +616,7 @@ class EDDSA {
 
   _verifyBatch(batch, ph, ctx) {
     const N = this.curve.n;
+    const h = this.curve.h.toNumber();
     const G = this.curve.g;
     const rng = new RNG(this);
     const points = new Array(1 + batch.length * 2);
@@ -580,16 +654,17 @@ class EDDSA {
 
       // rhs = R * a + A * ((e * a) mod n) + ...
       points[1 + i * 2 + 0] = R;
-      coeffs[1 + i * 2 + 0] = a;
+      coeffs[1 + i * 2 + 0] = a.imuln(h);
       points[1 + i * 2 + 1] = A;
-      coeffs[1 + i * 2 + 1] = ea;
+      coeffs[1 + i * 2 + 1] = ea.imuln(h);
     }
 
     // In concept, we can do:
     //   G * lhs == rhs
-    // But to avoid affinization we do:
+    // But to avoid scaling we do:
     //   G * -lhs + rhs == O
     sum.ineg().imod(N);
+    sum.imuln(h);
 
     return this.curve.mulAll(points, coeffs).isInfinity();
   }
@@ -600,9 +675,10 @@ class EDDSA {
   }
 
   deriveWithScalar(pub, scalar) {
-    const s = this.curve.decodeScalar(scalar).imod(this.curve.n);
+    const s = this.curve.decodeScalar(scalar);
     const A = this.curve.decodePoint(pub);
-    const point = A.mulConst(s, rng);
+    const k = this.curve.reduce(s);
+    const point = A.mulConst(k, rng);
 
     return point.encode();
   }
@@ -631,6 +707,7 @@ class RNG {
     this.curve = eddsa.curve;
     this.hash = eddsa.hash;
     this.chacha = new ChaCha20();
+    // Nothing up my sleeve.
     this.iv = Buffer.from('EDDSARNG');
   }
 

lib/js/elliptic.js

@@ -81,6 +81,7 @@ class Curve {
     this.three = null;
     this.n = null;
     this.h = null;
+    this.q = null;
     this.g = null;
     this.nh = null;
     this.scalarSize = 0;
@@ -144,7 +145,8 @@ class Curve {
 
     // Curve configuration, optional.
     this.n = conf.n ? BN.fromJSON(conf.n) : new BN(0);
-    this.h = conf.h ? BN.fromJSON(conf.h) : new BN(0);
+    this.h = conf.h ? BN.fromJSON(conf.h) : new BN(1);
+    this.q = this.n.mul(this.h);
     this.g = null;
     this.nh = this.n.ushrn(1);
     this.scalarSize = Math.max(this.n.byteLength(), this.p.byteLength());
@@ -346,7 +348,7 @@ class Curve {
     //   N=256 => 5632M + 4096S + 3584A
     //          + 512*a + 1024*4 + 512*3
     //          + 1024*2
-    const [sign, bits, exp] = getLadderBits(k, this.n, this.h);
+    const [sign, bits, exp] = getLadderBits(k, this.q);
 
     // Clone points (for safe swapping).
     let a = p.toJ().clone();
@@ -428,7 +430,7 @@ class Curve {
     //   N * (11M + 3S + 21A + 1*2)
     //
     //   N=256 => 2817M + 773S + 5384A + 256*2 + 4*2 + 1*8
-    const [sign, bits, exp] = getCOZBits(k, this.n, this.h);
+    const [sign, bits, exp] = getCOZBits(k, this.q);
 
     // Initial double (we assume k[n-1] == 1).
     let [a, b] = p.toJ().zdblu();
@@ -1316,10 +1318,6 @@ class Point {
     const bits = this.curve.h.bitLength();
     const word = this.curve.h.andln(-1);
 
-    // Unknown cofactor.
-    if (bits === 0)
-      return this.toJ();
-
     // Optimize for powers of two.
     if (bits <= 26 && (word & (word - 1)) === 0)
       return this.toJ().dblp(bits - 1);
@@ -6941,18 +6939,9 @@ function getJNAF(c1, c2, max) {
   return naf;
 }
 
-function getLadderBits(k, n, h) {
+function getLadderBits(k, n) {
   assert(k instanceof BN);
   assert(n instanceof BN);
-  assert(h instanceof BN);
-
-  // Multiply order by cofactor
-  // to get the true group order.
-  // This avoids us computing an
-  // improper result for a point
-  // with a torsion component.
-  if (h.cmpn(1) > 0)
-    n = n.mul(h);
 
   // Ensure positive.
   const k0 = k.abs();
@@ -6986,11 +6975,9 @@ function getLadderBits(k, n, h) {
   return [sign, bits, exp];
 }
 
-function getCOZBits(k, n, h) {
+function getCOZBits(k, n) {
   assert(k instanceof BN);
   assert(n instanceof BN);
-  assert(h instanceof BN);
-  assert(h.cmpn(1) <= 0);
 
   // Ensure positive.
   const u = k.abs().imod(n);

lib/js/schnorr.js

@@ -253,10 +253,7 @@ class Schnorr {
     const coeffs = new Array(1 + batch.length * 2);
     const sum = new BN(0);
 
-    // Seed the RNG with our batch. This
-    // code assumes the signers do not
-    // have complete knowledge of the
-    // other signatures in the set.
+    // Seed the RNG with our batch.
     this.rng.init(batch);
 
     // Setup multiplication for lhs*G.

lib/native/ed25519.js

@@ -107,12 +107,11 @@ function scalarClamp(scalar) {
   assert(Buffer.isBuffer(scalar));
   assert(scalar.length === 32);
 
-  if (!scalarVerify(scalar)) {
-    scalar = Buffer.from(scalar);
-    scalar[0] &= -8;
-    scalar[31] &= 0x7f;
-    scalar[31] |= 0x40;
-  }
+  scalar = Buffer.from(scalar);
+
+  scalar[0] &= -8;
+  scalar[31] &= 0x7f;
+  scalar[31] |= 0x40;
 
   return scalar;
 }
@@ -503,6 +502,20 @@ function verify(msg, sig, key, ph, ctx) {
   return binding.verify(msg, sig, key, ph, ctx);
 }
 
+/**
+ * Verify a signature (cofactor verification).
+ * @param {Buffer} msg
+ * @param {Buffer} sig
+ * @param {Buffer} key
+ * @param {Boolean|null} ph
+ * @param {Buffer|null} ctx
+ * @returns {Boolean}
+ */
+
+function verifySingle(msg, sig, key, ph, ctx) {
+  return binding.verifySingle(msg, sig, key, ph, ctx);
+}
+
 /**
  * Batch verify signatures.
  * @param {Object[]} batch
@@ -605,6 +618,7 @@ exports.signWithScalar = signWithScalar;
 exports.signTweakAdd = signTweakAdd;
 exports.signTweakMul = signTweakMul;
 exports.verify = verify;
+exports.verifySingle = verifySingle;
 exports.verifyBatch = verifyBatch;
 exports.derive = derive;
 exports.deriveWithScalar = deriveWithScalar;