Merge pull request #294 from MetaMask/SignFullDataNotHash

Sign full data not hash

CHANGELOG.md

@@ -6,6 +6,7 @@
 - Remove nonfunctional QR code button.
 - Make network loading indicator clickable to select accessible network.
 - Show more characters of addresses when space permits.
+- Fixed eth.sign behavior.
 - Fixed bug when signing messages under 64 hex characters long.
 - Add disclaimer view with placeholder text for first time users.
 

app/scripts/lib/id-management.js

@@ -0,0 +1,76 @@
+const ethUtil = require('ethereumjs-util')
+
+module.exports = IdManagement
+
+function IdManagement(opts) {
+  if (!opts) opts = {}
+
+  this.keyStore = opts.keyStore
+  this.derivedKey = opts.derivedKey
+  this.hdPathString = "m/44'/60'/0'/0"
+
+  this.getAddresses =  function(){
+    return keyStore.getAddresses(this.hdPathString).map(function(address){ return '0x'+address })
+  }
+
+  this.signTx = function(txParams){
+    // normalize values
+    txParams.to = ethUtil.addHexPrefix(txParams.to)
+    txParams.from = ethUtil.addHexPrefix(txParams.from)
+    txParams.value = ethUtil.addHexPrefix(txParams.value)
+    txParams.data = ethUtil.addHexPrefix(txParams.data)
+    txParams.gasLimit = ethUtil.addHexPrefix(txParams.gasLimit || txParams.gas)
+    txParams.nonce = ethUtil.addHexPrefix(txParams.nonce)
+    var tx = new Transaction(txParams)
+
+    // sign tx
+    var privKeyHex = this.exportPrivateKey(txParams.from)
+    var privKey = ethUtil.toBuffer(privKeyHex)
+    tx.sign(privKey)
+
+    // Add the tx hash to the persisted meta-tx object
+    var txHash = ethUtil.bufferToHex(tx.hash())
+    var metaTx = configManager.getTx(txParams.metamaskId)
+    metaTx.hash = txHash
+    configManager.updateTx(metaTx)
+
+    // return raw serialized tx
+    var rawTx = ethUtil.bufferToHex(tx.serialize())
+    return rawTx
+  }
+
+  this.signMsg = function (address, message) {
+    // sign message
+    var privKeyHex = this.exportPrivateKey(address);
+    var privKey = ethUtil.toBuffer(privKeyHex);
+    var msgSig = ethUtil.ecsign(new Buffer(message.replace('0x',''), 'hex'), privKey);
+    var rawMsgSig = ethUtil.bufferToHex(concatSig(msgSig.v, msgSig.r, msgSig.s));
+    return rawMsgSig;
+  };
+
+  this.getSeed = function(){
+    return this.keyStore.getSeed(this.derivedKey)
+  }
+
+  this.exportPrivateKey = function(address) {
+    var privKeyHex = ethUtil.addHexPrefix(this.keyStore.exportPrivateKey(address, this.derivedKey, this.hdPathString))
+    return privKeyHex
+  }
+}
+
+function pad_with_zeroes(number, length){
+  var my_string = '' + number;
+  while (my_string.length < length) {
+    my_string = '0' + my_string;
+  }
+  return my_string;
+}
+
+function concatSig(v, r, s) {
+  r = pad_with_zeroes(ethUtil.fromSigned(r), 64)
+  s = pad_with_zeroes(ethUtil.fromSigned(s), 64)
+  r = ethUtil.stripHexPrefix(r.toString('hex'))
+  s = ethUtil.stripHexPrefix(s.toString('hex'))
+  v = ethUtil.stripHexPrefix(ethUtil.intToHex(v))
+  return ethUtil.addHexPrefix(r.concat(s, v))
+}

app/scripts/lib/idStore.js

@@ -13,6 +13,7 @@ const autoFaucet = require('./auto-faucet')
 const configManager = require('./config-manager-singleton')
 const messageManager = require('./message-manager')
 const DEFAULT_RPC = 'https://testrpc.metamask.io/'
+const IdManagement = require('./id-management')
 
 
 module.exports = IdentityStore
@@ -478,82 +479,6 @@ IdentityStore.prototype._autoFaucet = function() {
   autoFaucet(addresses[0])
 }
 
-function IdManagement(opts) {
-  if (!opts) opts = {}
-
-  this.keyStore = opts.keyStore
-  this.derivedKey = opts.derivedKey
-  this.hdPathString = "m/44'/60'/0'/0"
-
-  this.getAddresses =  function(){
-    return keyStore.getAddresses(this.hdPathString).map(function(address){ return '0x'+address })
-  }
-
-  this.signTx = function(txParams){
-    // normalize values
-    txParams.to = ethUtil.addHexPrefix(txParams.to)
-    txParams.from = ethUtil.addHexPrefix(txParams.from)
-    txParams.value = ethUtil.addHexPrefix(txParams.value)
-    txParams.data = ethUtil.addHexPrefix(txParams.data)
-    txParams.gasLimit = ethUtil.addHexPrefix(txParams.gasLimit || txParams.gas)
-    txParams.nonce = ethUtil.addHexPrefix(txParams.nonce)
-    var tx = new Transaction(txParams)
-
-    // sign tx
-    var privKeyHex = this.exportPrivateKey(txParams.from)
-    var privKey = ethUtil.toBuffer(privKeyHex)
-    tx.sign(privKey)
-
-    // Add the tx hash to the persisted meta-tx object
-    var txHash = ethUtil.bufferToHex(tx.hash())
-    var metaTx = configManager.getTx(txParams.metamaskId)
-    metaTx.hash = txHash
-    configManager.updateTx(metaTx)
-
-    // return raw serialized tx
-    var rawTx = ethUtil.bufferToHex(tx.serialize())
-    return rawTx
-  }
-
-  this.signMsg = function(address, message){
-    // sign message
-    var privKeyHex = this.exportPrivateKey(address)
-    var privKey = ethUtil.toBuffer(privKeyHex)
-    var msgHash = ethUtil.sha3(message)
-    var msgSig = ethUtil.ecsign(msgHash, privKey)
-    var rawMsgSig = ethUtil.bufferToHex(concatSig(msgSig.v, msgSig.r, msgSig.s))
-    return rawMsgSig
-  }
-
-  this.getSeed = function(){
-    return this.keyStore.getSeed(this.derivedKey)
-  }
-
-  this.exportPrivateKey = function(address) {
-    var privKeyHex = ethUtil.addHexPrefix(this.keyStore.exportPrivateKey(address, this.derivedKey, this.hdPathString))
-    return privKeyHex
-  }
-}
-
-
 // util
 
 function noop(){}
-
-function pad_with_zeroes(number, length){
-  var my_string = '' + number;
-  while (my_string.length < length) {
-    my_string = '0' + my_string;
-  }
-  return my_string;
-}
-
-function concatSig(v, r, s) {
-  r = pad_with_zeroes(ethUtil.fromSigned(r), 64)
-  s = pad_with_zeroes(ethUtil.fromSigned(s), 64)
-  v = ethUtil.bufferToInt(v)
-  r = ethUtil.toUnsigned(r).toString('hex')
-  s = ethUtil.toUnsigned(s).toString('hex')
-  v = ethUtil.stripHexPrefix(ethUtil.intToHex(v))
-  return ethUtil.addHexPrefix(r.concat(s, v).toString("hex"))
-}

test/unit/id-management-test.js

@@ -0,0 +1,33 @@
+var assert = require('assert')
+var IdManagement = require('../../app/scripts/lib/id-management')
+var sinon = require('sinon')
+
+describe('IdManagement', function() {
+
+  beforeEach(function() {
+    // sinon allows stubbing methods that are easily verified
+    this.sinon = sinon.sandbox.create()
+    window.localStorage = {} // Hacking localStorage support into JSDom
+  })
+
+  afterEach(function() {
+    // sinon requires cleanup otherwise it will overwrite context
+    this.sinon.restore()
+  })
+
+  describe('#signMsg', function () {
+    const address = '0x926cD0393816429a580037475ec23eD65fDC893B'
+    const message = '0x96b8d442f4c09a08d266bf37b18219465cfb341c1b3ab9792a6103a93583fdf7'
+    const privateKey = '0xd291f7aa01b94941b446f260bca42c0752762571428ad4ed6239613c66365cf4'
+    const expectedResult = '0x04881196121781472543750166203264808665659193717384627772472141185319786561270240926993050673320157359365329096037150419976876479876332927284781689204045461c'
+
+    const idManagement = new IdManagement()
+    const exportKeyStub = sinon.stub(idManagement, 'exportPrivateKey', (addr) => {
+      assert.equal(addr, address)
+      return privateKey
+    })
+
+    const result = idManagement.signMsg(address, message)
+    assert.equal(result, expectedResult)
+  })
+})