Merge pull request #184 from dcousens/ecclean

ECDSA cleanup

src/ecdsa.js

@@ -1,40 +1,11 @@
 var assert = require('assert')
 var crypto = require('./crypto')
-var sec = require('./sec')
-var ecparams = sec("secp256k1")
 
 var BigInteger = require('bigi')
 var ECPointFp = require('./ec').ECPointFp
 
-function implShamirsTrick(P, k, Q, l) {
-  var m = Math.max(k.bitLength(), l.bitLength())
-  var Z = P.add2D(Q)
-  var R = P.curve.getInfinity()
-
-  for (var i = m - 1; i >= 0; --i) {
-    R = R.twice2D()
-
-    R.z = BigInteger.ONE
-
-    if (k.testBit(i)) {
-      if (l.testBit(i)) {
-        R = R.add2D(Z)
-      } else {
-        R = R.add2D(P)
-      }
-    } else {
-      if (l.testBit(i)) {
-        R = R.add2D(Q)
-      }
-    }
-  }
-
-  return R
-}
-
-var ecdsa = {
-  deterministicGenerateK: function(hash, D) {
-    assert(Buffer.isBuffer(hash), 'Hash must be a Buffer')
+function deterministicGenerateK(ecparams, hash, D) {
+  assert(Buffer.isBuffer(hash), 'Hash must be a Buffer, not ' + hash)
   assert.equal(hash.length, 32, 'Hash must be 256 bit')
   assert(D instanceof BigInteger, 'Private key must be a BigInteger')
 
@@ -57,10 +28,10 @@ var ecdsa = {
   assert(kB.compareTo(ecparams.getN()) < 0, 'Invalid k value')
 
   return kB
-  },
+}
 
-  sign: function (hash, D) {
-    var k = ecdsa.deterministicGenerateK(hash, D)
+function sign(ecparams, hash, D) {
+  var k = deterministicGenerateK(ecparams, hash, D)
 
   var n = ecparams.getN()
   var G = ecparams.getG()
@@ -80,36 +51,16 @@ var ecdsa = {
     s = n.subtract(s)
   }
 
-    return ecdsa.serializeSig(r, s)
-  },
-
-  verify: function (hash, sig, pubkey) {
-    var r,s
-    if (Array.isArray(sig) || Buffer.isBuffer(sig)) {
-      var obj = ecdsa.parseSig(sig)
-      r = obj.r
-      s = obj.s
-    } else if ("object" === typeof sig && sig.r && sig.s) {
-      r = sig.r
-      s = sig.s
-    } else {
-      throw new Error("Invalid value for signature")
+  return {r: r, s: s}
 }
 
-    var Q
-    if (pubkey instanceof ECPointFp) {
-      Q = pubkey
-    } else if (Array.isArray(pubkey) || Buffer.isBuffer(pubkey)) {
-      Q = ECPointFp.decodeFrom(ecparams.getCurve(), pubkey)
-    } else {
-      throw new Error("Invalid format for pubkey value, must be byte array or ECPointFp")
-    }
+function verify(ecparams, hash, r, s, Q) {
   var e = BigInteger.fromBuffer(hash)
 
-    return ecdsa.verifyRaw(e, r, s, Q)
-  },
+  return verifyRaw(ecparams, e, r, s, Q)
+}
 
-  verifyRaw: function (e, r, s, Q) {
+function verifyRaw(ecparams, e, r, s, Q) {
   var n = ecparams.getN()
   var G = ecparams.getG()
 
@@ -125,23 +76,18 @@ var ecdsa = {
   var u1 = e.multiply(c).mod(n)
   var u2 = r.multiply(c).mod(n)
 
-    // TODO(!!!): For some reason Shamir's trick isn't working with
-    // signed message verification!? Probably an implementation
-    // error!
-    //var point = implShamirsTrick(G, u1, Q, u2)
-    var point = G.multiply(u1).add(Q.multiply(u2))
-
+  var point = G.multiplyTwo(u1, Q, u2)
   var v = point.getX().toBigInteger().mod(n)
 
   return v.equals(r)
-  },
+}
 
 /**
   * Serialize a signature into DER format.
   *
   * Takes two BigIntegers representing r and s and returns a byte array.
   */
-  serializeSig: function (r, s) {
+function serializeSig(r, s) {
   var rBa = r.toByteArraySigned()
   var sBa = s.toByteArraySigned()
 
@@ -158,7 +104,7 @@ var ecdsa = {
   sequence.unshift(0x30); // SEQUENCE
 
   return sequence
-  },
+}
 
 /**
   * Parses a buffer containing a DER-encoded signature.
@@ -170,9 +116,7 @@ var ecdsa = {
   *   s: BigInteger
   * }
   */
-  parseSig: function (buffer) {
-    if (Array.isArray(buffer)) buffer = new Buffer(buffer) // FIXME: transitionary
-
+function parseSig(buffer) {
   assert.equal(buffer.readUInt8(0), 0x30, 'Not a DER sequence')
   assert.equal(buffer.readUInt8(1), buffer.length - 2, 'Invalid sequence length')
 
@@ -189,9 +133,9 @@ var ecdsa = {
     r: BigInteger.fromByteArraySigned(rB),
     s: BigInteger.fromByteArraySigned(sB)
   }
-  },
+}
 
-  serializeSigCompact: function(r, s, i, compressed) {
+function serializeSigCompact(r, s, i, compressed) {
   if (compressed) {
     i += 4
   }
@@ -204,9 +148,9 @@ var ecdsa = {
   s.toBuffer(32).copy(buffer, 33)
 
   return buffer
-  },
+}
 
-  parseSigCompact: function (buffer) {
+function parseSigCompact(buffer) {
   assert.equal(buffer.length, 65, 'Invalid signature length')
   var i = buffer.readUInt8(0) - 27
 
@@ -226,7 +170,7 @@ var ecdsa = {
     i: i,
     compressed: compressed
   }
-  },
+}
 
 /**
   * Recover a public key from a signature.
@@ -236,7 +180,7 @@ var ecdsa = {
   *
   * http://www.secg.org/download/aid-780/sec1-v2.pdf
   */
-  recoverPubKey: function (r, s, hash, i) {
+function recoverPubKey(ecparams, e, r, s, i) {
   assert.strictEqual(i & 3, i, 'The recovery param is more than two bits')
 
   // A set LSB signifies that the y-coordinate is odd
@@ -274,21 +218,22 @@ var ecdsa = {
   var R = new ECPointFp(curve, curve.fromBigInteger(x), curve.fromBigInteger(y))
   R.validate()
 
-    // 1.5 Compute e from M
-    var e = BigInteger.fromBuffer(hash)
-    var eNeg = BigInteger.ZERO.subtract(e).mod(n)
+  // 1.5 Compute -e from e
+  var eNeg = e.negate().mod(n)
 
   // 1.6 Compute Q = r^-1 (sR -  eG)
+  //             Q = r^-1 (sR + -eG)
   var rInv = r.modInverse(n)
-    var Q = implShamirsTrick(R, s, G, eNeg).multiply(rInv)
 
+  var Q = R.multiplyTwo(s, G, eNeg).multiply(rInv)
   Q.validate()
-    if (!ecdsa.verifyRaw(e, r, s, Q)) {
+
+  if (!verifyRaw(ecparams, e, r, s, Q)) {
     throw new Error("Pubkey recovery unsuccessful")
   }
 
   return Q
-  },
+}
 
 /**
   * Calculate pubkey extraction parameter.
@@ -301,17 +246,27 @@ var ecdsa = {
   * This function simply tries all four cases and returns the value
   * that resulted in a successful pubkey recovery.
   */
-  calcPubKeyRecoveryParam: function (origPubKey, r, s, hash) {
+function calcPubKeyRecoveryParam(ecparams, e, r, s, Q) {
   for (var i = 0; i < 4; i++) {
-      var pubKey = ecdsa.recoverPubKey(r, s, hash, i)
+    var Qprime = recoverPubKey(ecparams, e, r, s, i)
 
-      if (pubKey.equals(origPubKey)) {
+    if (Qprime.equals(Q)) {
       return i
     }
   }
 
-    throw new Error("Unable to find valid recovery factor")
-  }
+  throw new Error('Unable to find valid recovery factor')
 }
 
-module.exports = ecdsa
+module.exports = {
+calcPubKeyRecoveryParam: calcPubKeyRecoveryParam,
+deterministicGenerateK: deterministicGenerateK,
+recoverPubKey: recoverPubKey,
+sign: sign,
+verify: verify,
+verifyRaw: verifyRaw,
+serializeSig: serializeSig,
+parseSig: parseSig,
+serializeSigCompact: serializeSigCompact,
+parseSigCompact: parseSigCompact
+}

src/eckey.js

@@ -63,7 +63,7 @@ ECKey.prototype.toWIF = function(version) {
 
 // Operations
 ECKey.prototype.sign = function(hash) {
-  return ecdsa.sign(hash, this.D)
+  return ecdsa.sign(ecparams, hash, this.D)
 }
 
 module.exports = ECKey

src/ecpubkey.js

@@ -36,8 +36,8 @@ ECPubKey.prototype.getAddress = function(version) {
   return new Address(crypto.hash160(this.toBuffer()), version)
 }
 
-ECPubKey.prototype.verify = function(hash, sig) {
-  return ecdsa.verify(hash, sig, this.Q)
+ECPubKey.prototype.verify = function(hash, signature) {
+  return ecdsa.verify(ecparams, hash, signature.r, signature.s, this.Q)
 }
 
 // Export functions

src/message.js

@@ -1,5 +1,6 @@
 /// Implements Bitcoin's feature for signing arbitrary messages.
 var Address = require('./address')
+var BigInteger = require('bigi')
 var bufferutils = require('./bufferutils')
 var crypto = require('./crypto')
 var ecdsa = require('./ecdsa')
@@ -8,6 +9,9 @@ var networks = require('./networks')
 var Address = require('./address')
 var ECPubKey = require('./ecpubkey')
 
+var sec = require('./sec')
+var ecparams = sec('secp256k1')
+
 function magicHash(message, network) {
   var magicPrefix = new Buffer(network.magicPrefix)
   var messageBuffer = new Buffer(message)
@@ -24,8 +28,9 @@ function sign(key, message, network) {
   network = network || networks.bitcoin
 
   var hash = magicHash(message, network)
-  var sig = ecdsa.parseSig(key.sign(hash))
-  var i = ecdsa.calcPubKeyRecoveryParam(key.pub.Q, sig.r, sig.s, hash)
+  var sig = key.sign(hash)
+  var e = BigInteger.fromBuffer(hash)
+  var i = ecdsa.calcPubKeyRecoveryParam(ecparams, e, sig.r, sig.s, key.pub.Q)
 
   return ecdsa.serializeSigCompact(sig.r, sig.s, i, key.pub.compressed)
 }
@@ -40,7 +45,8 @@ function verify(address, compactSig, message, network) {
 
   var hash = magicHash(message, network)
   var sig = ecdsa.parseSigCompact(compactSig)
-  var Q = ecdsa.recoverPubKey(sig.r, sig.s, hash, sig.i)
+  var e = BigInteger.fromBuffer(hash)
+  var Q = ecdsa.recoverPubKey(ecparams, e, sig.r, sig.s, sig.i)
 
   var pubKey = new ECPubKey(Q, sig.compressed)
   return pubKey.getAddress(address.version).toString() === address.toString()

src/transaction.js

@@ -364,27 +364,35 @@ Transaction.prototype.sign = function(index, key, type) {
   this.setScriptSig(index, scriptSig)
 }
 
-Transaction.prototype.signScriptSig = function(index, script, key, type) {
+Transaction.prototype.signScriptSig = function(index, scriptPubKey, key, type) {
   type = type || SIGHASH_ALL
 
   assert((index >= 0), 'Invalid vin index')
-  assert(script instanceof Script, 'Invalid Script object')
+  assert(scriptPubKey instanceof Script, 'Invalid Script object')
   assert(key instanceof ECKey, 'Invalid private key')
 //  assert.equal(type & 0x7F, type, 'Invalid type') // TODO
 
-  var hash = this.hashForSignature(script, index, type)
-  return key.sign(hash).concat([type])
+  var hash = this.hashForSignature(scriptPubKey, index, type)
+  var sig = key.sign(hash)
+  var DERsig = ecdsa.serializeSig(sig.r, sig.s)
+
+  return Buffer.concat([
+    new Buffer(DERsig),
+    new Buffer([type])
+  ])
 }
 
 Transaction.prototype.setScriptSig = function(index, script) {
   this.ins[index].script = script
 }
 
-Transaction.prototype.validateSig = function(index, script, pub, sig) {
-  var type = sig[sig.length - 1]
+Transaction.prototype.validateSig = function(index, script, pub, DERsig) {
+  var type = DERsig.readUInt8(DERsig.length - 1)
+  DERsig = DERsig.slice(0, -1)
+
   var hash = this.hashForSignature(script, index, type)
+  var sig = ecdsa.parseSig(DERsig)
 
-  sig = sig.slice(0, -1)
   return pub.verify(hash, sig)
 }
 

test/ecdsa.js

@@ -17,10 +17,10 @@ describe('ecdsa', function() {
   describe('deterministicGenerateK', function() {
     it('matches the test vectors', function() {
       fixtures.valid.forEach(function(f) {
-        var priv = BigInteger.fromHex(f.D)
+        var D = BigInteger.fromHex(f.D)
         var h1 = crypto.sha256(f.message)
 
-        var k = ecdsa.deterministicGenerateK(h1, priv)
+        var k = ecdsa.deterministicGenerateK(ecparams, h1, D)
         assert.equal(k.toHex(), f.k)
       })
     })
@@ -28,14 +28,16 @@ describe('ecdsa', function() {
 
   describe('recoverPubKey', function() {
     it('succesfully recovers a public key', function() {
-      var signature = new Buffer('H0PG6+PUo96UPTJ/DVj8aBU5it+Nuli4YdsLuTMvfJxoHH9Jb7jYTQXCCOX2jrTChD5S1ic3vCrUQHdmB5/sEQY=', 'base64')
+      var D = BigInteger.ONE
+      var signature = new Buffer('INcvXVVEFyIfHLbDX+xoxlKFn3Wzj9g0UbhObXdMq+YMKC252o5RHFr0/cKdQe1WsBLUBi4morhgZ77obDJVuV0=', 'base64')
 
-      var obj = ecdsa.parseSigCompact(signature)
+      var Q = ecparams.getG().multiply(D)
       var hash = message.magicHash('1111', networks.bitcoin)
+      var e = BigInteger.fromBuffer(hash)
+      var psig = ecdsa.parseSigCompact(signature)
 
-      var pubKey = new ECPubKey(ecdsa.recoverPubKey(obj.r, obj.s, hash, obj.i))
-
-      assert.equal(pubKey.toHex(), '02e8fcf4d749b35879bc1f3b14b49e67ab7301da3558c5a9b74a54f1e6339c334c')
+      var Qprime = ecdsa.recoverPubKey(ecparams, e, psig.r, psig.s, psig.i)
+      assert(Q.equals(Qprime))
     })
   })
 
@@ -43,9 +45,8 @@ describe('ecdsa', function() {
     it('matches the test vectors', function() {
       fixtures.valid.forEach(function(f) {
         var D = BigInteger.fromHex(f.D)
-        var priv = new ECKey(D)
         var hash = crypto.sha256(f.message)
-        var sig = ecdsa.parseSig(priv.sign(hash))
+        var sig = ecdsa.sign(ecparams, hash, D)
 
         assert.equal(sig.r.toString(), f.signature.r)
         assert.equal(sig.s.toString(), f.signature.s)
@@ -53,14 +54,12 @@ describe('ecdsa', function() {
     })
 
     it('should sign with low S value', function() {
-      var priv = ECKey.makeRandom()
       var hash = crypto.sha256('Vires in numeris')
-      var signature = priv.sign(hash)
-      var psig = ecdsa.parseSig(signature)
+      var sig = ecdsa.sign(ecparams, hash, BigInteger.ONE)
 
       // See BIP62 for more information
       var N_OVER_TWO = ecparams.getN().shiftRight(1)
-      assert(psig.s.compareTo(N_OVER_TWO) <= 0)
+      assert(sig.s.compareTo(N_OVER_TWO) <= 0)
     })
   })
 
@@ -68,13 +67,13 @@ describe('ecdsa', function() {
     it('matches the test vectors', function() {
       fixtures.valid.forEach(function(f) {
         var D = BigInteger.fromHex(f.D)
-        var priv = new ECKey(D)
+        var Q = ecparams.getG().multiply(D)
 
         var r = new BigInteger(f.signature.r)
         var s = new BigInteger(f.signature.s)
         var e = BigInteger.fromBuffer(crypto.sha256(f.message))
 
-        assert(ecdsa.verifyRaw(e, r, s, priv.pub.Q))
+        assert(ecdsa.verifyRaw(ecparams, e, r, s, Q))
       })
     })
   })

test/transaction.js

@@ -197,7 +197,7 @@ describe('Transaction', function() {
         tx.sign(0, key)
 
         var script = prevTx.outs[0].script
-        var sig = tx.ins[0].script.chunks[0]
+        var sig = new Buffer(tx.ins[0].script.chunks[0])
 
         assert.equal(tx.validateSig(0, script, key.pub, sig), true)
       })
@@ -213,7 +213,7 @@ describe('Transaction', function() {
       it('returns true for valid signature', function(){
         var key = ECKey.fromWIF('L44f7zxJ5Zw4EK9HZtyAnzCYz2vcZ5wiJf9AuwhJakiV4xVkxBeb')
         var script = prevTx.outs[0].script
-        var sig = validTx.ins[0].script.chunks[0]
+        var sig = new Buffer(validTx.ins[0].script.chunks[0])
 
         assert.equal(validTx.validateSig(0, script, key.pub, sig), true)
       })