Merge pull request #337 from bitcoinjs/rfc6979fix

RF6979 compliance in regards to invalid ECDSA signatures

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,10 +9,38 @@ 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)
 
+  // FIXME: remove/uncomment for 2.0.0
+//  typeForce('Function', checkSig)
+
+  if (typeof checkSig !== 'function') {
+    console.warn('deterministicGenerateK requires a checkSig callback in 2.0.0, see #337 for more information')
+
+    checkSig = function(k) {
+      var G = curve.G
+      var n = curve.n
+      var e = BigInteger.fromBuffer(hash)
+
+      var Q = G.multiply(k)
+
+      if (curve.isInfinity(Q))
+        return false
+
+      var r = Q.affineX.mod(n)
+      if (r.signum() === 0)
+        return false
+
+      var s = k.modInverse(n).multiply(e.add(d.multiply(r))).mod(n)
+      if (s.signum() === 0)
+        return false
+
+      return true
+    }
+  }
+
   // sanity check
   assert.equal(hash.length, 32, 'Hash must be 256 bit')
 
@@ -20,6 +48,7 @@ function deterministicGenerateK(curve, hash, d) {
   var k = new Buffer(32)
   var v = new Buffer(32)
 
+  // Step A, ignored as hash already provided
   // Step B
   v.fill(1)
 
@@ -54,8 +83,8 @@ function deterministicGenerateK(curve, hash, d) {
 
   var T = BigInteger.fromBuffer(v)
 
-  // Step H3, repeat until T is within the interval [1, n - 1]
+  // 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)) {
+  while ((T.signum() <= 0) || (T.compareTo(curve.n) >= 0) || !checkSig(T)) {
     k = crypto.createHmac('sha256', k)
       .update(v)
       .update(ZERO)
@@ -63,6 +92,9 @@ function deterministicGenerateK(curve, hash, d) {
 
     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)
   }
 
@@ -70,18 +102,28 @@ function deterministicGenerateK(curve, hash, d) {
 }
 
 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
+
+  deterministicGenerateK(curve, hash, d, function(k) {
     var Q = G.multiply(k)
-  var e = BigInteger.fromBuffer(hash)
 
-  var r = Q.affineX.mod(n)
+    if (curve.isInfinity(Q))
-  assert.notEqual(r.signum(), 0, 'Invalid R value')
+      return false
 
-  var s = k.modInverse(n).multiply(e.add(d.multiply(r))).mod(n)
+    r = Q.affineX.mod(n)
-  assert.notEqual(s.signum(), 0, 'Invalid S value')
+    if (r.signum() === 0)
+      return false
+
+    s = k.modInverse(n).multiply(e.add(d.multiply(r))).mod(n)
+    if (s.signum() === 0)
+      return false
+
+    return true
+  })
 
   var N_OVER_TWO = n.shiftRight(1)
 

test/ecdsa.js

@@ -15,12 +15,25 @@ var fixtures = require('./fixtures/ecdsa.json')
 
 describe('ecdsa', function() {
   describe('deterministicGenerateK', function() {
-    fixtures.valid.forEach(function(f) {
+    function checkSig() { return true }
+
+    fixtures.valid.ecdsa.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)
+      })
+    })
+
+    // FIXME: remove in 2.0.0
+    fixtures.valid.ecdsa.forEach(function(f) {
+      it('(deprecated) for \"' + f.message + '\"', function() {
+        var d = BigInteger.fromHex(f.d)
+        var h1 = crypto.sha256(f.message)
+
+        var k = ecdsa.deterministicGenerateK(curve, h1, d) // default checkSig
         assert.equal(k.toHex(), f.k)
       })
     })
@@ -28,21 +41,58 @@ describe('ecdsa', function() {
     it('loops until an appropriate k value is found', sinon.test(function() {
       this.mock(BigInteger).expects('fromBuffer')
         .exactly(3)
-        .onCall(0).returns(new BigInteger('0'))
+        .onCall(0).returns(new BigInteger('0')) // < 1
-        .onCall(1).returns(curve.n)
+        .onCall(1).returns(curve.n) // > n-1
-        .onCall(2).returns(new BigInteger('42'))
+        .onCall(2).returns(new BigInteger('42')) // valid
 
       var d = new BigInteger('1')
       var h1 = new Buffer(32)
-
+      var k = ecdsa.deterministicGenerateK(curve, h1, d, checkSig)
-      var k = ecdsa.deterministicGenerateK(curve, h1, d)
 
       assert.equal(k.toString(), '42')
     }))
+
+    it('loops until a suitable signature is found', sinon.test(function() {
+      this.mock(BigInteger).expects('fromBuffer')
+        .exactly(4)
+        .onCall(0).returns(new BigInteger('0')) // < 1
+        .onCall(1).returns(curve.n) // > n-1
+        .onCall(2).returns(new BigInteger('42')) // valid, but 'bad' signature
+        .onCall(3).returns(new BigInteger('53')) // valid, good signature
+
+      var checkSig = this.mock()
+      checkSig.exactly(2)
+      checkSig.onCall(0).returns(false) // bad signature
+      checkSig.onCall(1).returns(true) // good signature
+
+      var d = new BigInteger('1')
+      var h1 = new Buffer(32)
+      var k = ecdsa.deterministicGenerateK(curve, h1, d, checkSig)
+
+      assert.equal(k.toString(), '53')
+    }))
+
+    fixtures.valid.rfc6979.forEach(function(f) {
+      it('produces the expected k values for ' + f.message + ' if k wasn\'t suitable', function() {
+        var d = BigInteger.fromHex(f.d)
+        var h1 = crypto.sha256(f.message)
+
+        var results = []
+        ecdsa.deterministicGenerateK(curve, h1, d, function(k) {
+          results.push(k)
+
+          return results.length === 16
+        })
+
+        assert.equal(results[0].toHex(), f.k0)
+        assert.equal(results[1].toHex(), f.k1)
+        assert.equal(results[15].toHex(), f.k15)
+      })
+    })
   })
 
   describe('recoverPubKey', function() {
-    fixtures.valid.forEach(function(f) {
+    fixtures.valid.ecdsa.forEach(function(f) {
       it('recovers the pubKey for ' + f.d, function() {
         var d = BigInteger.fromHex(f.d)
         var Q = curve.G.multiply(d)
@@ -94,7 +144,7 @@ describe('ecdsa', function() {
   })
 
   describe('sign', function() {
-    fixtures.valid.forEach(function(f) {
+    fixtures.valid.ecdsa.forEach(function(f) {
       it('produces a deterministic signature for \"' + f.message + '\"', function() {
         var d = BigInteger.fromHex(f.d)
         var hash = crypto.sha256(f.message)
@@ -116,7 +166,7 @@ describe('ecdsa', function() {
   })
 
   describe('verify/verifyRaw', function() {
-    fixtures.valid.forEach(function(f) {
+    fixtures.valid.ecdsa.forEach(function(f) {
       it('verifies a valid signature for \"' + f.message + '\"', function() {
         var d = BigInteger.fromHex(f.d)
         var H = crypto.sha256(f.message)

test/fixtures/ecdsa.json

@@ -1,5 +1,6 @@
 {
-  "valid": [
+  "valid": {
+    "ecdsa": [
       {
         "d": "01",
         "k": "ec633bd56a5774a0940cb97e27a9e4e51dc94af737596a0c5cbb3d30332d92a5",
@@ -71,6 +72,79 @@
         }
       }
     ],
+    "rfc6979": [
+      {
+        "message": "test data",
+        "d": "fee0a1f7afebf9d2a5a80c0c98a31c709681cce195cbcd06342b517970c0be1e",
+        "k0": "fcce1de7a9bcd6b2d3defade6afa1913fb9229e3b7ddf4749b55c4848b2a196e",
+        "k1": "727fbcb59eb48b1d7d46f95a04991fc512eb9dbf9105628e3aec87428df28fd8",
+        "k15": "398f0e2c9f79728f7b3d84d447ac3a86d8b2083c8f234a0ffa9c4043d68bd258"
+      },
+      {
+        "message": "Everything should be made as simple as possible, but not simpler.",
+        "d": "0000000000000000000000000000000000000000000000000000000000000001",
+        "k0": "ec633bd56a5774a0940cb97e27a9e4e51dc94af737596a0c5cbb3d30332d92a5",
+        "k1": "df55b6d1b5c48184622b0ead41a0e02bfa5ac3ebdb4c34701454e80aabf36f56",
+        "k15": "def007a9a3c2f7c769c75da9d47f2af84075af95cadd1407393dc1e26086ef87"
+      },
+      {
+        "message": "Satoshi Nakamoto",
+        "d": "0000000000000000000000000000000000000000000000000000000000000002",
+        "k0": "d3edc1b8224e953f6ee05c8bbf7ae228f461030e47caf97cde91430b4607405e",
+        "k1": "f86d8e43c09a6a83953f0ab6d0af59fb7446b4660119902e9967067596b58374",
+        "k15": "241d1f57d6cfd2f73b1ada7907b199951f95ef5ad362b13aed84009656e0254a"
+      },
+      {
+        "message": "Diffie Hellman",
+        "d": "7f7f7f7f7f7f7f7f7f7f7f7f7f7f7f7f7f7f7f7f7f7f7f7f7f7f7f7f7f7f7f7f",
+        "k0": "c378a41cb17dce12340788dd3503635f54f894c306d52f6e9bc4b8f18d27afcc",
+        "k1": "90756c96fef41152ac9abe08819c4e95f16da2af472880192c69a2b7bac29114",
+        "k15": "7b3f53300ab0ccd0f698f4d67db87c44cf3e9e513d9df61137256652b2e94e7c"
+      },
+      {
+        "message": "Japan",
+        "d": "8080808080808080808080808080808080808080808080808080808080808080",
+        "k0": "f471e61b51d2d8db78f3dae19d973616f57cdc54caaa81c269394b8c34edcf59",
+        "k1": "6819d85b9730acc876fdf59e162bf309e9f63dd35550edf20869d23c2f3e6d17",
+        "k15": "d8e8bae3ee330a198d1f5e00ad7c5f9ed7c24c357c0a004322abca5d9cd17847"
+      },
+      {
+        "message": "Bitcoin",
+        "d": "fffffffffffffffffffffffffffffffebaaedce6af48a03bbfd25e8cd0364140",
+        "k0": "36c848ffb2cbecc5422c33a994955b807665317c1ce2a0f59c689321aaa631cc",
+        "k1": "4ed8de1ec952a4f5b3bd79d1ff96446bcd45cabb00fc6ca127183e14671bcb85",
+        "k15": "56b6f47babc1662c011d3b1f93aa51a6e9b5f6512e9f2e16821a238d450a31f8"
+      },
+      {
+        "message": "i2FLPP8WEus5WPjpoHwheXOMSobUJVaZM1JPMQZq",
+        "d": "fffffffffffffffffffffffffffffffebaaedce6af48a03bbfd25e8cd0364140",
+        "k0": "6e9b434fcc6bbb081a0463c094356b47d62d7efae7da9c518ed7bac23f4e2ed6",
+        "k1": "ae5323ae338d6117ce8520a43b92eacd2ea1312ae514d53d8e34010154c593bb",
+        "k15": "3eaa1b61d1b8ab2f1ca71219c399f2b8b3defa624719f1e96fe3957628c2c4ea"
+      },
+      {
+        "message": "lEE55EJNP7aLrMtjkeJKKux4Yg0E8E1SAJnWTCEh",
+        "d": "3881e5286abc580bb6139fe8e83d7c8271c6fe5e5c2d640c1f0ed0e1ee37edc9",
+        "k0": "5b606665a16da29cc1c5411d744ab554640479dd8abd3c04ff23bd6b302e7034",
+        "k1": "f8b25263152c042807c992eacd2ac2cc5790d1e9957c394f77ea368e3d9923bd",
+        "k15": "ea624578f7e7964ac1d84adb5b5087dd14f0ee78b49072aa19051cc15dab6f33"
+      },
+      {
+        "message": "2SaVPvhxkAPrayIVKcsoQO5DKA8Uv5X/esZFlf+y",
+        "d": "7259dff07922de7f9c4c5720d68c9745e230b32508c497dd24cb95ef18856631",
+        "k0": "3ab6c19ab5d3aea6aa0c6da37516b1d6e28e3985019b3adb388714e8f536686b",
+        "k1": "19af21b05004b0ce9cdca82458a371a9d2cf0dc35a813108c557b551c08eb52e",
+        "k15": "117a32665fca1b7137a91c4739ac5719fec0cf2e146f40f8e7c21b45a07ebc6a"
+      },
+      {
+        "message": "00A0OwO2THi7j5Z/jp0FmN6nn7N/DQd6eBnCS+/b",
+        "d": "0d6ea45d62b334777d6995052965c795a4f8506044b4fd7dc59c15656a28f7aa",
+        "k0": "79487de0c8799158294d94c0eb92ee4b567e4dc7ca18addc86e49d31ce1d2db6",
+        "k1": "9561d2401164a48a8f600882753b3105ebdd35e2358f4f808c4f549c91490009",
+        "k15": "b0d273634129ff4dbdf0df317d4062a1dbc58818f88878ffdb4ec511c77976c0"
+      }
+    ]
+  },
   "invalid": {
     "recoverPubKey": [
       {