ecdsa: fixes edge case presented in bitcoinjs#336

Author: dcousens

package.json

@@ -52,7 +52,7 @@
     "bs58check": "1.0.3",
     "crypto-browserify": "^3.2.6",
     "ecurve": "1.0.0",
-    "typeforce": "0.0.2"
+    "typeforce": "0.1.0"
   },
   "devDependencies": {
     "async": "^0.9.0",

src/ecdsa.js

@@ -9,9 +9,10 @@ var ZERO = new Buffer([0])
 var ONE = new Buffer([1])
 
 // https://tools.ietf.org/html/rfc6979#section-3.2
-function deterministicGenerateK(curve, hash, d) {
+function deterministicGenerateK(curve, hash, d, checkSig) {
   typeForce('Buffer', hash)
   typeForce('BigInteger', d)
+  typeForce('Function', checkSig)
 
   // sanity check
   assert.equal(hash.length, 32, 'Hash must be 256 bit')
@@ -55,34 +56,47 @@ function deterministicGenerateK(curve, hash, d) {
 
   var T = BigInteger.fromBuffer(v)
 
-  // Step H3, repeat until T is within the interval [1, n - 1]
-  while ((T.signum() <= 0) || (T.compareTo(curve.n) >= 0)) {
+  // Step H3, repeat until T is within the interval [1, n - 1] and is suitable for ECDSA
+  while ((T.signum() <= 0) || (T.compareTo(curve.n) >= 0) || !checkSig(T)) {
     k = crypto.createHmac('sha256', k)
       .update(v)
       .update(ZERO)
       .digest()
 
     v = crypto.createHmac('sha256', k).update(v).digest()
 
+    // Step H1/H2a, again, ignored as tlen === qlen (256 bit)
+    // Step H2b again
+    v = crypto.createHmac('sha256', k).update(v).digest()
     T = BigInteger.fromBuffer(v)
   }
 
   return T
 }
 
 function sign(curve, hash, d) {
-  var k = deterministicGenerateK(curve, hash, d)
+  var r, s
 
+  var e = BigInteger.fromBuffer(hash)
   var n = curve.n
   var G = curve.G
-  var Q = G.multiply(k)
-  var e = BigInteger.fromBuffer(hash)
 
-  var r = Q.affineX.mod(n)
-  assert.notEqual(r.signum(), 0, 'Invalid R value')
+  deterministicGenerateK(curve, hash, d, function(k) {
+    var Q = G.multiply(k)
+
+    if (curve.isInfinity(Q))
+      return false
+
+    r = Q.affineX.mod(n)
+    if (r.signum() === 0)
+      return false
+
+    s = k.modInverse(n).multiply(e.add(d.multiply(r))).mod(n)
+    if (s.signum() === 0)
+      return false
 
-  var s = k.modInverse(n).multiply(e.add(d.multiply(r))).mod(n)
-  assert.notEqual(s.signum(), 0, 'Invalid S value')
+    return true
+  })
 
   var N_OVER_TWO = n.shiftRight(1)
 

test/ecdsa.js

@@ -15,12 +15,14 @@ var fixtures = require('./fixtures/ecdsa.json')
 
 describe('ecdsa', function() {
   describe('deterministicGenerateK', function() {
+    function checkSig() { return true }
+
     fixtures.valid.forEach(function(f) {
       it('for \"' + f.message + '\"', function() {
         var d = BigInteger.fromHex(f.d)
         var h1 = crypto.sha256(f.message)
 
-        var k = ecdsa.deterministicGenerateK(curve, h1, d)
+        var k = ecdsa.deterministicGenerateK(curve, h1, d, checkSig)
         assert.equal(k.toHex(), f.k)
       })
     })
@@ -35,7 +37,7 @@ describe('ecdsa', function() {
       var d = new BigInteger('1')
       var h1 = new Buffer(32)
 
-      var k = ecdsa.deterministicGenerateK(curve, h1, d)
+      var k = ecdsa.deterministicGenerateK(curve, h1, d, checkSig)
 
       assert.equal(k.toString(), '42')
     }))