diff --git a/CHANGELOG.md b/CHANGELOG.md index d98cc798f..f4f2148b9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,8 +4,15 @@ - net_version has been made synchronous. - Test suite for migrations expanded. +- Improve test coverage of eth.sign behavior, including a code example of verifying a signature. + +## 3.2.2 2017-2-8 + +- Revert eth.sign behavior to the previous one with a big warning. We will be gradually implementing the new behavior over the coming time. https://github.com/ethereum/wiki/wiki/JSON-RPC#eth_sign + ## 3.2.1 2017-2-8 +- Revert back to old style message signing. - Fixed some build errors that were causing a variety of bugs. ## 3.2.0 2017-2-8 diff --git a/app/manifest.json b/app/manifest.json index c5d34e6ea..95475ddf1 100644 --- a/app/manifest.json +++ b/app/manifest.json @@ -1,7 +1,7 @@ { "name": "MetaMask", "short_name": "Metamask", - "version": "3.2.1", + "version": "3.2.2", "manifest_version": 2, "author": "https://metamask.io", "description": "Ethereum Browser Extension", diff --git a/app/scripts/keyrings/hd.js b/app/scripts/keyrings/hd.js index 2e3b74192..3a66f7868 100644 --- a/app/scripts/keyrings/hd.js +++ b/app/scripts/keyrings/hd.js @@ -74,7 +74,18 @@ class HdKeyring extends EventEmitter { } // For eth_sign, we need to sign transactions: - signMessage (withAccount, msgHex) { + // hd + signMessage (withAccount, data) { + const wallet = this._getWalletForAccount(withAccount) + const message = ethUtil.stripHexPrefix(data) + var privKey = wallet.getPrivateKey() + var msgSig = ethUtil.ecsign(new Buffer(message, 'hex'), privKey) + var rawMsgSig = ethUtil.bufferToHex(sigUtil.concatSig(msgSig.v, msgSig.r, msgSig.s)) + return Promise.resolve(rawMsgSig) + } + + // For eth_sign, we need to sign transactions: + newGethSignMessage (withAccount, msgHex) { const wallet = this._getWalletForAccount(withAccount) const privKey = wallet.getPrivateKey() const msgBuffer = ethUtil.toBuffer(msgHex) @@ -101,9 +112,11 @@ class HdKeyring extends EventEmitter { _getWalletForAccount (account) { + const targetAddress = sigUtil.normalize(account) return this.wallets.find((w) => { const address = w.getAddress().toString('hex') - return ((address === account) || (sigUtil.normalize(address) === account)) + return ((address === targetAddress) || + (sigUtil.normalize(address) === targetAddress)) }) } } diff --git a/app/scripts/keyrings/simple.js b/app/scripts/keyrings/simple.js index fa8e9fd78..82881aa2d 100644 --- a/app/scripts/keyrings/simple.js +++ b/app/scripts/keyrings/simple.js @@ -58,7 +58,18 @@ class SimpleKeyring extends EventEmitter { } // For eth_sign, we need to sign transactions: - signMessage (withAccount, msgHex) { + signMessage (withAccount, data) { + const wallet = this._getWalletForAccount(withAccount) + const message = ethUtil.stripHexPrefix(data) + var privKey = wallet.getPrivateKey() + var msgSig = ethUtil.ecsign(new Buffer(message, 'hex'), privKey) + var rawMsgSig = ethUtil.bufferToHex(sigUtil.concatSig(msgSig.v, msgSig.r, msgSig.s)) + return Promise.resolve(rawMsgSig) + } + + // For eth_sign, we need to sign transactions: + + newGethSignMessage (withAccount, msgHex) { const wallet = this._getWalletForAccount(withAccount) const privKey = wallet.getPrivateKey() const msgBuffer = ethUtil.toBuffer(msgHex) @@ -77,7 +88,8 @@ class SimpleKeyring extends EventEmitter { /* PRIVATE METHODS */ _getWalletForAccount (account) { - let wallet = this.wallets.find(w => ethUtil.bufferToHex(w.getAddress()) === account) + const address = sigUtil.normalize(account) + let wallet = this.wallets.find(w => ethUtil.bufferToHex(w.getAddress()) === address) if (!wallet) throw new Error('Simple Keyring - Unable to find matching address.') return wallet } diff --git a/test/unit/id-management-test.js b/test/unit/id-management-test.js index 25eea8777..cbc6403bc 100644 --- a/test/unit/id-management-test.js +++ b/test/unit/id-management-test.js @@ -16,7 +16,7 @@ describe('IdManagement', function() { }) describe('#signMsg', function () { - it.skip('passes the dennis test', function() { + it('passes the dennis test', function() { const address = '0x9858e7d8b79fc3e6d989636721584498926da38a' const message = '0x879a053d4800c6354e76c7985a865d2922c82fb5b3f4577b2fe08b998954f2e0' const privateKey = '0x7dd98753d7b4394095de7d176c58128e2ed6ee600abe97c9f6d9fd65015d9b18' diff --git a/test/unit/keyrings/simple-test.js b/test/unit/keyrings/simple-test.js index 5fe29a67d..ba7dd448a 100644 --- a/test/unit/keyrings/simple-test.js +++ b/test/unit/keyrings/simple-test.js @@ -1,5 +1,7 @@ const assert = require('assert') const extend = require('xtend') +const Web3 = require('web3') +const web3 = new Web3() const ethUtil = require('ethereumjs-util') const SimpleKeyring = require('../../../app/scripts/keyrings/simple') const TYPE_STR = 'Simple Key Pair' @@ -55,7 +57,7 @@ describe('simple-keyring', function() { const privateKey = '0x7dd98753d7b4394095de7d176c58128e2ed6ee600abe97c9f6d9fd65015d9b18' const expectedResult = '0x28fcb6768e5110144a55b2e6ce9d1ea5a58103033632d272d2b5cf506906f7941a00b539383fd872109633d8c71c404e13dba87bc84166ee31b0e36061a69e161c' - it.skip('passes the dennis test', function(done) { + it('passes the dennis test', function(done) { keyring.deserialize([ privateKey ]) .then(() => { return keyring.signMessage(address, message) @@ -65,6 +67,44 @@ describe('simple-keyring', function() { done() }) }) + + it('reliably can decode messages it signs', function (done) { + + const message = 'hello there!' + const msgHashHex = web3.sha3(message) + let address + let addresses = [] + + keyring.deserialize([ privateKey ]) + .then(() => { + keyring.addAccounts(9) + }) + .then(() => { + return keyring.getAccounts() + }) + .then((addrs) => { + addresses = addrs + return Promise.all(addresses.map((address) => { + return keyring.signMessage(address, msgHashHex) + })) + }) + .then((signatures) => { + + signatures.forEach((sgn, index) => { + const address = addresses[index] + + var r = ethUtil.toBuffer(sgn.slice(0,66)) + var s = ethUtil.toBuffer('0x' + sgn.slice(66,130)) + var v = ethUtil.bufferToInt(ethUtil.toBuffer('0x' + sgn.slice(130,132))) + var m = ethUtil.toBuffer(msgHashHex) + var pub = ethUtil.ecrecover(m, v, r, s) + var adr = '0x' + ethUtil.pubToAddress(pub).toString('hex') + + assert.equal(adr, address, 'recovers address from signature correctly') + }) + done() + }) + }) }) describe('#addAccounts', function() { diff --git a/ui/app/components/pending-msg.js b/ui/app/components/pending-msg.js index f4bde91dc..b2cac164a 100644 --- a/ui/app/components/pending-msg.js +++ b/ui/app/components/pending-msg.js @@ -28,6 +28,15 @@ PendingMsg.prototype.render = function () { }, }, 'Sign Message'), + h('.error', { + style: { + margin: '10px', + }, + }, `Signing this message can have + dangerous side effects. Only sign messages from + sites you fully trust with your entire account. + This will be fixed in a future version.`), + // message details h(PendingTxDetails, state),