From 28dcbc642617b381e1bbddce17aef46e7bfb1d3e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Thu, 27 Apr 2017 18:23:22 +0200 Subject: [PATCH] Support external eth_sign (#5481) * Display a QR for eth_sign requests. * Support raw confirmation of eth_sign * Fix ethkey issue on nightly. * Fixing test. * Fixing test. --- Cargo.lock | 1 + ethkey/src/brain.rs | 4 +- ethkey/src/signature.rs | 42 +++++++- js/src/redux/providers/signerMiddleware.js | 23 ++-- js/src/util/qrscan.js | 18 +++- .../components/RequestOrigin/requestOrigin.js | 2 +- .../components/SignRequest/signRequest.js | 15 ++- .../TransactionPending/transactionPending.js | 7 +- .../transactionPendingFormConfirm.js | 62 +++++++---- .../transactionPendingFormConfirm.spec.js | 1 + .../transactionPendingForm.js | 15 ++- rpc/Cargo.toml | 1 + rpc/src/lib.rs | 4 + rpc/src/v1/helpers/dispatch.rs | 29 ++--- rpc/src/v1/impls/signer.rs | 79 +++++++++----- rpc/src/v1/tests/mocked/signer.rs | 101 +++++++++++++++++- rpc/src/v1/types/confirmations.rs | 2 +- 17 files changed, 313 insertions(+), 93 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d6b128491..b87285205 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1711,6 +1711,7 @@ dependencies = [ "order-stat 0.1.3 (registry+https://github.com/rust-lang/crates.io-index)", "parity-reactor 0.1.0", "parity-updater 1.7.0", + "pretty_assertions 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)", "rlp 0.1.0", "rust-crypto 0.2.36 (registry+https://github.com/rust-lang/crates.io-index)", "rustc-serialize 0.3.19 (registry+https://github.com/rust-lang/crates.io-index)", diff --git a/ethkey/src/brain.rs b/ethkey/src/brain.rs index 5ed23b404..cf675a843 100644 --- a/ethkey/src/brain.rs +++ b/ethkey/src/brain.rs @@ -57,8 +57,8 @@ mod tests { #[test] fn test_brain() { let words = "this is sparta!".to_owned(); - let first_keypair = Brain(words.clone()).generate().unwrap(); - let second_keypair = Brain(words.clone()).generate().unwrap(); + let first_keypair = Brain::new(words.clone()).generate().unwrap(); + let second_keypair = Brain::new(words.clone()).generate().unwrap(); assert_eq!(first_keypair.secret(), second_keypair.secret()); } } diff --git a/ethkey/src/signature.rs b/ethkey/src/signature.rs index 347222287..0c9e41ef5 100644 --- a/ethkey/src/signature.rs +++ b/ethkey/src/signature.rs @@ -25,6 +25,7 @@ use rustc_serialize::hex::{ToHex, FromHex}; use bigint::hash::{H520, H256}; use {Secret, Public, SECP256K1, Error, Message, public_to_address, Address}; +/// Signature encoded as RSV components #[repr(C)] pub struct Signature([u8; 65]); @@ -44,8 +45,32 @@ impl Signature { self.0[64] } + /// Encode the signature into VRS array (V altered to be in "Electrum" notation). + pub fn into_vrs(self) -> [u8; 65] { + let mut vrs = [0u8; 65]; + vrs[0] = self.v() + 27; + vrs[1..33].copy_from_slice(self.r()); + vrs[33..65].copy_from_slice(self.s()); + vrs + } + + /// Parse bytes as a signature encoded as VRS (V in "Electrum" notation). + /// May return empty (invalid) signature if given data has invalid length. + pub fn from_vrs(data: &[u8]) -> Self { + if data.len() != 65 || data[0] < 27 { + // fallback to empty (invalid) signature + return Signature::default(); + } + + let mut sig = [0u8; 65]; + sig[0..32].copy_from_slice(&data[1..33]); + sig[32..64].copy_from_slice(&data[33..65]); + sig[64] = data[0] - 27; + Signature(sig) + } + /// Create a signature object from the sig. - pub fn from_rsv(r: &H256, s: &H256, v: u8) -> Signature { + pub fn from_rsv(r: &H256, s: &H256, v: u8) -> Self { let mut sig = [0u8; 65]; sig[0..32].copy_from_slice(&r); sig[32..64].copy_from_slice(&s); @@ -222,6 +247,21 @@ mod tests { use {Generator, Random, Message}; use super::{sign, verify_public, verify_address, recover, Signature}; + #[test] + fn vrs_conversion() { + // given + let keypair = Random.generate().unwrap(); + let message = Message::default(); + let signature = sign(keypair.secret(), &message).unwrap(); + + // when + let vrs = signature.clone().into_vrs(); + let from_vrs = Signature::from_vrs(&vrs); + + // then + assert_eq!(signature, from_vrs); + } + #[test] fn signature_to_and_from_str() { let keypair = Random.generate().unwrap(); diff --git a/js/src/redux/providers/signerMiddleware.js b/js/src/redux/providers/signerMiddleware.js index f50057abe..869a7d802 100644 --- a/js/src/redux/providers/signerMiddleware.js +++ b/js/src/redux/providers/signerMiddleware.js @@ -87,14 +87,20 @@ export default class SignerMiddleware { return this._hwstore.signLedger(transaction); }) .then((rawTx) => { - return this.confirmRawTransaction(store, id, rawTx); + return this.confirmRawRequest(store, id, rawTx); }); } - confirmRawTransaction (store, id, rawTx) { + confirmRawRequest (store, id, rawData) { const handlePromise = this._createConfirmPromiseHandler(store, id); - return handlePromise(this._api.signer.confirmRequestRaw(id, rawTx)); + return handlePromise(this._api.signer.confirmRequestRaw(id, rawData)); + } + + confirmSignedData (store, id, dataSigned) { + const { signature } = dataSigned; + + return this.confirmRawRequest(store, id, signature); } confirmSignedTransaction (store, id, txSigned) { @@ -102,7 +108,7 @@ export default class SignerMiddleware { const { signature, tx } = txSigned; const { rlp } = createSignedTx(netVersion, signature, tx); - return this.confirmRawTransaction(store, id, rlp); + return this.confirmRawRequest(store, id, rlp); } confirmWalletTransaction (store, id, transaction, wallet, password) { @@ -138,7 +144,7 @@ export default class SignerMiddleware { return signer.signTransaction(txData); }) .then((rawTx) => { - return this.confirmRawTransaction(store, id, rawTx); + return this.confirmRawRequest(store, id, rawTx); }) .catch((error) => { console.error(error.message); @@ -147,7 +153,7 @@ export default class SignerMiddleware { } onConfirmStart = (store, action) => { - const { condition, gas = 0, gasPrice = 0, id, password, payload, txSigned, wallet } = action.payload; + const { condition, gas = 0, gasPrice = 0, id, password, payload, txSigned, dataSigned, wallet } = action.payload; const handlePromise = this._createConfirmPromiseHandler(store, id); const transaction = payload.sendTransaction || payload.signTransaction; @@ -170,6 +176,11 @@ export default class SignerMiddleware { } } + // TODO [ToDr] Support eth_sign for external wallet (wallet && !transction) + if (dataSigned) { + return this.confirmSignedData(store, id, dataSigned); + } + return handlePromise(this._api.signer.confirmRequest(id, { gas, gasPrice, condition }, password)); } diff --git a/js/src/util/qrscan.js b/js/src/util/qrscan.js index f3cf2f9e9..da73d5b8f 100644 --- a/js/src/util/qrscan.js +++ b/js/src/util/qrscan.js @@ -19,8 +19,8 @@ import Transaction from 'ethereumjs-tx'; import { inAddress, inHex, inNumber10 } from '~/api/format/input'; import { sha3 } from '~/api/util/sha3'; -export function createUnsignedTx (api, netVersion, gasStore, transaction) { - const { data, from, gas, gasPrice, to, value } = gasStore.overrideTransaction(transaction); +export function createUnsignedTx (api, netVersion, transaction) { + const { data, from, gas, gasPrice, to, value } = transaction; return api.parity .nextNonce(from) @@ -111,8 +111,18 @@ export function generateQr (from, tx, hash, rlp) { }); } -export function generateTxQr (api, netVersion, gasStore, transaction) { - return createUnsignedTx(api, netVersion, gasStore, transaction) +export function generateDataQr (data) { + return Promise.resolve({ + data, + value: JSON.stringify({ + action: 'signData', + data + }) + }); +} + +export function generateTxQr (api, netVersion, transaction) { + return createUnsignedTx(api, netVersion, transaction) .then((qr) => { qr.value = generateQr(transaction.from, qr.tx, qr.hash, qr.rlp); diff --git a/js/src/views/Signer/components/RequestOrigin/requestOrigin.js b/js/src/views/Signer/components/RequestOrigin/requestOrigin.js index 6582e9780..435d90a6b 100644 --- a/js/src/views/Signer/components/RequestOrigin/requestOrigin.js +++ b/js/src/views/Signer/components/RequestOrigin/requestOrigin.js @@ -85,7 +85,7 @@ export default class RequestOrigin extends Component { diff --git a/js/src/views/Signer/components/SignRequest/signRequest.js b/js/src/views/Signer/components/SignRequest/signRequest.js index 624e0931f..705b89965 100644 --- a/js/src/views/Signer/components/SignRequest/signRequest.js +++ b/js/src/views/Signer/components/SignRequest/signRequest.js @@ -19,6 +19,8 @@ import React, { Component, PropTypes } from 'react'; import { FormattedMessage } from 'react-intl'; import { connect } from 'react-redux'; +import HardwareStore from '~/mobx/hardwareStore'; + import Account from '../Account'; import TransactionPendingForm from '../TransactionPendingForm'; import RequestOrigin from '../RequestOrigin'; @@ -68,6 +70,8 @@ class SignRequest extends Component { } }; + hardwareStore = HardwareStore.get(this.context.api); + componentWillMount () { const { address, signerStore } = this.props; @@ -155,8 +159,9 @@ class SignRequest extends Component { } renderActions () { - const { accounts, address, focus, isFinished, status } = this.props; - const account = accounts[address]; + const { accounts, address, focus, isFinished, status, data } = this.props; + const account = accounts[address] || {}; + const disabled = account.hardware && !this.hardwareStore.isConnected(address); if (isFinished) { if (status === 'confirmed') { @@ -188,21 +193,23 @@ class SignRequest extends Component { ); } onConfirm = (data) => { const { id } = this.props; - const { password } = data; + const { password, dataSigned, wallet } = data; - this.props.onConfirm({ id, password }); + this.props.onConfirm({ id, password, dataSigned, wallet }); } onReject = () => { diff --git a/js/src/views/Signer/components/TransactionPending/transactionPending.js b/js/src/views/Signer/components/TransactionPending/transactionPending.js index 666ff16a1..b9cc8f684 100644 --- a/js/src/views/Signer/components/TransactionPending/transactionPending.js +++ b/js/src/views/Signer/components/TransactionPending/transactionPending.js @@ -98,7 +98,9 @@ class TransactionPending extends Component { } renderTransaction () { - const { accounts, className, focus, id, isSending, netVersion, origin, signerStore, transaction } = this.props; + const transaction = this.gasStore.overrideTransaction(this.props.transaction); + + const { accounts, className, focus, id, isSending, netVersion, origin, signerStore } = this.props; const { totalValue } = this.state; const { balances, externalLink } = signerStore; const { from, value } = transaction; @@ -127,12 +129,11 @@ class TransactionPending extends Component { address={ from } disabled={ disabled } focus={ focus } - gasStore={ this.gasStore } isSending={ isSending } netVersion={ netVersion } onConfirm={ this.onConfirm } onReject={ this.onReject } - transaction={ transaction } + dataToSign={ { transaction } } /> ); diff --git a/js/src/views/Signer/components/TransactionPendingForm/TransactionPendingFormConfirm/transactionPendingFormConfirm.js b/js/src/views/Signer/components/TransactionPendingForm/TransactionPendingFormConfirm/transactionPendingFormConfirm.js index 0152bb71d..8d4e4a9cc 100644 --- a/js/src/views/Signer/components/TransactionPendingForm/TransactionPendingFormConfirm/transactionPendingFormConfirm.js +++ b/js/src/views/Signer/components/TransactionPendingForm/TransactionPendingFormConfirm/transactionPendingFormConfirm.js @@ -22,7 +22,7 @@ import { FormattedMessage } from 'react-intl'; import ReactTooltip from 'react-tooltip'; import { Form, Input, IdentityIcon, QrCode, QrScan } from '~/ui'; -import { generateTxQr } from '~/util/qrscan'; +import { generateTxQr, generateDataQr } from '~/util/qrscan'; import styles from './transactionPendingFormConfirm.css'; @@ -40,11 +40,10 @@ export default class TransactionPendingFormConfirm extends Component { address: PropTypes.string.isRequired, disabled: PropTypes.bool, focus: PropTypes.bool, - gasStore: PropTypes.object.isRequired, netVersion: PropTypes.string.isRequired, isSending: PropTypes.bool.isRequired, onConfirm: PropTypes.func.isRequired, - transaction: PropTypes.object.isRequired + dataToSign: PropTypes.object.isRequired }; static defaultProps = { @@ -406,7 +405,7 @@ export default class TransactionPendingFormConfirm extends Component { } onScanTx = (signature) => { - const { chainId, rlp, tx } = this.state.qr; + const { chainId, rlp, tx, data } = this.state.qr; if (signature && signature.substr(0, 2) !== '0x') { signature = `0x${signature}`; @@ -414,12 +413,22 @@ export default class TransactionPendingFormConfirm extends Component { this.setState({ qrState: QR_COMPLETED }); + if (tx) { + this.props.onConfirm({ + txSigned: { + chainId, + rlp, + signature, + tx + } + }); + return; + } + this.props.onConfirm({ - txSigned: { - chainId, - rlp, - signature, - tx + dataSigned: { + data, + signature } }); } @@ -487,13 +496,20 @@ export default class TransactionPendingFormConfirm extends Component { }); } - generateTxQr = () => { + generateQr = () => { const { api } = this.context; - const { netVersion, gasStore, transaction } = this.props; - - generateTxQr(api, netVersion, gasStore, transaction).then((qr) => { + const { netVersion, dataToSign } = this.props; + const { transaction, data } = dataToSign; + const setState = qr => { this.setState({ qr }); - }); + }; + + if (transaction) { + generateTxQr(api, netVersion, transaction).then(setState); + return; + } + + generateDataQr(data).then(setState); } onKeyDown = (event) => { @@ -528,19 +544,25 @@ export default class TransactionPendingFormConfirm extends Component { readNonce = () => { const { api } = this.context; - const { account } = this.props; + const { account, dataToSign } = this.props; + const { qr } = this.state; - if (!account || !account.external || !api.transport.isConnected) { + if (dataToSign.data && qr && !qr.value) { + this.generateQr(); + return; + } + + if (!account || !account.external || !api.transport.isConnected || !dataToSign.transaction) { return; } return api.parity .nextNonce(account.address) - .then((nonce) => { - const { qr } = this.state; + .then((newNonce) => { + const { nonce } = this.state.qr; - if (!qr.nonce || !nonce.eq(qr.nonce)) { - this.generateTxQr(); + if (!nonce || !newNonce.eq(nonce)) { + this.generateQr(); } }); } diff --git a/js/src/views/Signer/components/TransactionPendingForm/TransactionPendingFormConfirm/transactionPendingFormConfirm.spec.js b/js/src/views/Signer/components/TransactionPendingForm/TransactionPendingFormConfirm/transactionPendingFormConfirm.spec.js index 1176a01f0..2e984400d 100644 --- a/js/src/views/Signer/components/TransactionPendingForm/TransactionPendingFormConfirm/transactionPendingFormConfirm.spec.js +++ b/js/src/views/Signer/components/TransactionPendingForm/TransactionPendingFormConfirm/transactionPendingFormConfirm.spec.js @@ -52,6 +52,7 @@ function render (address) { address={ address } onConfirm={ onConfirm } isSending={ false } + dataToSign={ {} } /> ); instance = component.instance(); diff --git a/js/src/views/Signer/components/TransactionPendingForm/transactionPendingForm.js b/js/src/views/Signer/components/TransactionPendingForm/transactionPendingForm.js index af360897a..e176b97e9 100644 --- a/js/src/views/Signer/components/TransactionPendingForm/transactionPendingForm.js +++ b/js/src/views/Signer/components/TransactionPendingForm/transactionPendingForm.js @@ -30,12 +30,18 @@ export default class TransactionPendingForm extends Component { className: PropTypes.string, disabled: PropTypes.bool, focus: PropTypes.bool, - gasStore: PropTypes.object.isRequired, netVersion: PropTypes.string.isRequired, isSending: PropTypes.bool.isRequired, onConfirm: PropTypes.func.isRequired, onReject: PropTypes.func.isRequired, - transaction: PropTypes.object.isRequired + dataToSign: PropTypes.oneOfType([ + PropTypes.shape({ + transaction: PropTypes.object.isRequired + }), + PropTypes.shape({ + data: PropTypes.string.isRequired + }) + ]).isRequired }; static defaultProps = { @@ -59,7 +65,7 @@ export default class TransactionPendingForm extends Component { } renderForm () { - const { account, address, disabled, focus, gasStore, isSending, netVersion, onConfirm, onReject, transaction } = this.props; + const { account, address, disabled, focus, isSending, netVersion, onConfirm, onReject, dataToSign } = this.props; if (this.state.isRejectOpen) { return ( @@ -73,11 +79,10 @@ export default class TransactionPendingForm extends Component { account={ account } disabled={ disabled } focus={ focus } - gasStore={ gasStore } netVersion={ netVersion } isSending={ isSending } onConfirm={ onConfirm } - transaction={ transaction } + dataToSign={ dataToSign } /> ); } diff --git a/rpc/Cargo.toml b/rpc/Cargo.toml index 87e340b02..70ff7202d 100644 --- a/rpc/Cargo.toml +++ b/rpc/Cargo.toml @@ -49,6 +49,7 @@ fetch = { path = "../util/fetch" } stats = { path = "../util/stats" } clippy = { version = "0.0.103", optional = true} +pretty_assertions = "0.1" [features] dev = ["clippy", "ethcore/dev", "ethcore-util/dev", "ethsync/dev"] diff --git a/rpc/src/lib.rs b/rpc/src/lib.rs index 53b8f994c..ae51d879c 100644 --- a/rpc/src/lib.rs +++ b/rpc/src/lib.rs @@ -66,6 +66,10 @@ extern crate ethjson; #[cfg(test)] extern crate ethcore_devtools as devtools; +#[cfg(test)] +#[macro_use] +extern crate pretty_assertions; + pub extern crate jsonrpc_ws_server as ws; mod metadata; diff --git a/rpc/src/v1/helpers/dispatch.rs b/rpc/src/v1/helpers/dispatch.rs index c6af8d7e7..5b25c3124 100644 --- a/rpc/src/v1/helpers/dispatch.rs +++ b/rpc/src/v1/helpers/dispatch.rs @@ -206,6 +206,17 @@ pub fn fetch_gas_price_corpus( } } +/// Returns a eth_sign-compatible hash of data to sign. +/// The data is prepended with special message to prevent +/// chosen-plaintext attacks. +pub fn eth_data_hash(mut data: Bytes) -> H256 { + let mut message_data = + format!("\x19Ethereum Signed Message:\n{}", data.len()) + .into_bytes(); + message_data.append(&mut data); + message_data.sha3() +} + /// Dispatcher for light clients -- fetches default gas price, next nonce, etc. from network. #[derive(Clone)] pub struct LightDispatcher { @@ -474,21 +485,11 @@ pub fn execute( .map(ConfirmationResponse::SignTransaction) ).boxed() }, - ConfirmationPayload::EthSignMessage(address, mut data) => { - let mut message_data = - format!("\x19Ethereum Signed Message:\n{}", data.len()) - .into_bytes(); - message_data.append(&mut data); - let res = signature(&accounts, address, message_data.sha3(), pass) + ConfirmationPayload::EthSignMessage(address, data) => { + let hash = eth_data_hash(data); + let res = signature(&accounts, address, hash, pass) .map(|result| result - .map(|rsv| { - let mut vrs = [0u8; 65]; - let rsv = rsv.as_ref(); - vrs[0] = rsv[64] + 27; - vrs[1..33].copy_from_slice(&rsv[0..32]); - vrs[33..65].copy_from_slice(&rsv[32..64]); - H520(vrs) - }) + .map(|rsv| H520(rsv.into_vrs())) .map(RpcH520::from) .map(ConfirmationResponse::Signature) ); diff --git a/rpc/src/v1/impls/signer.rs b/rpc/src/v1/impls/signer.rs index d46bfdcc5..e34743e1d 100644 --- a/rpc/src/v1/impls/signer.rs +++ b/rpc/src/v1/impls/signer.rs @@ -21,12 +21,13 @@ use std::sync::{Arc, Weak}; use rlp::UntrustedRlp; use ethcore::account_provider::AccountProvider; use ethcore::transaction::{SignedTransaction, PendingTransaction}; +use ethkey; use futures::{future, BoxFuture, Future, IntoFuture}; use jsonrpc_core::Error; -use v1::helpers::{errors, SignerService, SigningQueue, ConfirmationPayload}; -use v1::helpers::dispatch::{self, Dispatcher, WithToken}; use v1::helpers::accounts::unwrap_provider; +use v1::helpers::dispatch::{self, Dispatcher, WithToken, eth_data_hash}; +use v1::helpers::{errors, SignerService, SigningQueue, ConfirmationPayload, FilledTransactionRequest}; use v1::traits::Signer; use v1::types::{TransactionModification, ConfirmationRequest, ConfirmationResponse, ConfirmationResponseWithToken, U256, Bytes}; @@ -104,6 +105,37 @@ impl SignerClient { }) .unwrap_or_else(|| future::err(errors::invalid_params("Unknown RequestID", id)).boxed()) } + + fn verify_transaction(bytes: Bytes, request: FilledTransactionRequest, process: F) -> Result where + F: FnOnce(PendingTransaction) -> Result, + { + let signed_transaction = UntrustedRlp::new(&bytes.0).as_val().map_err(errors::from_rlp_error)?; + let signed_transaction = SignedTransaction::new(signed_transaction).map_err(|e| errors::invalid_params("Invalid signature.", e))?; + let sender = signed_transaction.sender(); + + // Verification + let sender_matches = sender == request.from; + let data_matches = signed_transaction.data == request.data; + let value_matches = signed_transaction.value == request.value; + let nonce_matches = match request.nonce { + Some(nonce) => signed_transaction.nonce == nonce, + None => true, + }; + + // Dispatch if everything is ok + if sender_matches && data_matches && value_matches && nonce_matches { + let pending_transaction = PendingTransaction::new(signed_transaction, request.condition.map(Into::into)); + process(pending_transaction) + } else { + let mut error = Vec::new(); + if !sender_matches { error.push("from") } + if !data_matches { error.push("data") } + if !value_matches { error.push("value") } + if !nonce_matches { error.push("nonce") } + + Err(errors::invalid_params("Sent transaction does not match the request.", error)) + } + } } impl Signer for SignerClient { @@ -149,38 +181,27 @@ impl Signer for SignerClient { signer.peek(&id).map(|confirmation| { let result = match confirmation.payload { ConfirmationPayload::SendTransaction(request) => { - let signed_transaction = UntrustedRlp::new(&bytes.0).as_val().map_err(errors::from_rlp_error)?; - let signed_transaction = SignedTransaction::new(signed_transaction).map_err(|e| errors::invalid_params("Invalid signature.", e))?; - let sender = signed_transaction.sender(); - - // Verification - let sender_matches = sender == request.from; - let data_matches = signed_transaction.data == request.data; - let value_matches = signed_transaction.value == request.value; - let nonce_matches = match request.nonce { - Some(nonce) => signed_transaction.nonce == nonce, - None => true, - }; - - // Dispatch if everything is ok - if sender_matches && data_matches && value_matches && nonce_matches { - let pending_transaction = PendingTransaction::new(signed_transaction, request.condition.map(Into::into)); + Self::verify_transaction(bytes, request, |pending_transaction| { self.dispatcher.dispatch_transaction(pending_transaction) .map(Into::into) .map(ConfirmationResponse::SendTransaction) - } else { - let mut error = Vec::new(); - if !sender_matches { error.push("from") } - if !data_matches { error.push("data") } - if !value_matches { error.push("value") } - if !nonce_matches { error.push("nonce") } - - Err(errors::invalid_params("Sent transaction does not match the request.", error)) + }) + }, + ConfirmationPayload::SignTransaction(request) => { + Self::verify_transaction(bytes, request, |pending_transaction| { + Ok(ConfirmationResponse::SignTransaction(pending_transaction.transaction.into())) + }) + }, + ConfirmationPayload::EthSignMessage(address, data) => { + let expected_hash = eth_data_hash(data); + let signature = ethkey::Signature::from_vrs(&bytes.0); + match ethkey::verify_address(&address, &signature, &expected_hash) { + Ok(true) => Ok(ConfirmationResponse::Signature(bytes.0.as_slice().into())), + Ok(false) => Err(errors::invalid_params("Sender address does not match the signature.", ())), + Err(err) => Err(errors::invalid_params("Invalid signature received.", err)), } }, - // TODO [ToDr]: - // 1. Sign - verify signature - // 2. Decrypt - pass through? + // TODO [ToDr]: Decrypt - pass through? _ => Err(errors::unimplemented(Some("Non-transaction requests does not support RAW signing yet.".into()))), }; if let Ok(ref response) = result { diff --git a/rpc/src/v1/tests/mocked/signer.rs b/rpc/src/v1/tests/mocked/signer.rs index 973e33528..473a7aa7e 100644 --- a/rpc/src/v1/tests/mocked/signer.rs +++ b/rpc/src/v1/tests/mocked/signer.rs @@ -20,7 +20,7 @@ use util::{U256, Uint, Address, ToPretty}; use ethcore::account_provider::AccountProvider; use ethcore::client::TestBlockChainClient; -use ethcore::transaction::{Transaction, Action}; +use ethcore::transaction::{Transaction, Action, SignedTransaction}; use rlp::encode; use serde_json; @@ -28,8 +28,9 @@ use jsonrpc_core::IoHandler; use v1::{SignerClient, Signer, Origin}; use v1::metadata::Metadata; use v1::tests::helpers::TestMinerService; +use v1::types::H520; use v1::helpers::{SigningQueue, SignerService, FilledTransactionRequest, ConfirmationPayload}; -use v1::helpers::dispatch::FullDispatcher; +use v1::helpers::dispatch::{FullDispatcher, eth_data_hash}; struct SignerTester { signer: Arc, @@ -359,7 +360,6 @@ fn should_confirm_transaction_with_rlp() { "params":["0x1", "0x"#.to_owned() + &rlp.to_hex() + r#""], "id":1 }"#; -println!("{:?}", request); let response = r#"{"jsonrpc":"2.0","result":""#.to_owned() + format!("0x{:?}", t.hash()).as_ref() + r#"","id":1}"#; // then @@ -415,6 +415,101 @@ fn should_return_error_when_sender_does_not_match() { assert_eq!(tester.signer.requests().len(), 1); } +#[test] +fn should_confirm_sign_transaction_with_rlp() { + // given + let tester = signer_tester(); + let address = tester.accounts.new_account("test").unwrap(); + let recipient = Address::from_str("d46e8dd67c5d32be8058bb8eb970870f07244567").unwrap(); + tester.signer.add_request(ConfirmationPayload::SignTransaction(FilledTransactionRequest { + from: address, + used_default_from: false, + to: Some(recipient), + gas_price: U256::from(10_000), + gas: U256::from(10_000_000), + value: U256::from(1), + data: vec![], + nonce: None, + condition: None, + }), Origin::Unknown).unwrap(); + assert_eq!(tester.signer.requests().len(), 1); + + let t = Transaction { + nonce: U256::zero(), + gas_price: U256::from(0x1000), + gas: U256::from(10_000_000), + action: Action::Call(recipient), + value: U256::from(0x1), + data: vec![] + }; + let signature = tester.accounts.sign(address, Some("test".into()), t.hash(None)).unwrap(); + let t = SignedTransaction::new(t.with_signature(signature.clone(), None)).unwrap(); + let rlp = encode(&t); + + // when + let request = r#"{ + "jsonrpc":"2.0", + "method":"signer_confirmRequestRaw", + "params":["0x1", "0x"#.to_owned() + &rlp.to_hex() + r#""], + "id":1 + }"#; + let response = r#"{"jsonrpc":"2.0","result":{"#.to_owned() + + r#""raw":"0x"# + &rlp.to_hex() + r#"","# + + r#""tx":{"# + + r#""blockHash":null,"blockNumber":null,"condition":null,"creates":null,"# + + &format!("\"from\":\"0x{:?}\",", &address) + + r#""gas":"0x989680","gasPrice":"0x1000","# + + &format!("\"hash\":\"0x{:?}\",", t.hash()) + + r#""input":"0x","# + + &format!("\"networkId\":{},", t.network_id().map_or("null".to_owned(), |n| format!("{}", n))) + + r#""nonce":"0x0","# + + &format!("\"publicKey\":\"0x{:?}\",", t.public_key().unwrap()) + + &format!("\"r\":\"0x{}\",", U256::from(signature.r()).to_hex()) + + &format!("\"raw\":\"0x{}\",", rlp.to_hex()) + + &format!("\"s\":\"0x{}\",", U256::from(signature.s()).to_hex()) + + &format!("\"standardV\":\"0x{}\",", U256::from(t.standard_v()).to_hex()) + + r#""to":"0xd46e8dd67c5d32be8058bb8eb970870f07244567","transactionIndex":null,"# + + &format!("\"v\":\"0x{}\",", U256::from(t.original_v()).to_hex()) + + r#""value":"0x1""# + + r#"}},"id":1}"#; + + // then + assert_eq!(tester.io.handle_request_sync(&request), Some(response.to_owned())); + assert_eq!(tester.signer.requests().len(), 0); + assert_eq!(tester.miner.imported_transactions.lock().len(), 0); +} + + +#[test] +fn should_confirm_data_sign_with_signature() { + // given + let tester = signer_tester(); + let address = tester.accounts.new_account("test").unwrap(); + tester.signer.add_request(ConfirmationPayload::EthSignMessage( + address, + vec![1, 2, 3, 4].into(), + ), Origin::Unknown).unwrap(); + assert_eq!(tester.signer.requests().len(), 1); + + let data_hash = eth_data_hash(vec![1, 2, 3, 4].into()); + let signature = H520(tester.accounts.sign(address, Some("test".into()), data_hash).unwrap().into_vrs()); + let signature = format!("0x{:?}", signature); + + // when + let request = r#"{ + "jsonrpc":"2.0", + "method":"signer_confirmRequestRaw", + "params":["0x1", ""#.to_owned() + &signature + r#""], + "id":1 + }"#; + let response = r#"{"jsonrpc":"2.0","result":""#.to_owned() + &signature + r#"","id":1}"#; + + // then + assert_eq!(tester.io.handle_request_sync(&request), Some(response.to_owned())); + assert_eq!(tester.signer.requests().len(), 0); + assert_eq!(tester.miner.imported_transactions.lock().len(), 0); +} + #[test] fn should_generate_new_token() { // given diff --git a/rpc/src/v1/types/confirmations.rs b/rpc/src/v1/types/confirmations.rs index ac1fba4fe..9a6d6837a 100644 --- a/rpc/src/v1/types/confirmations.rs +++ b/rpc/src/v1/types/confirmations.rs @@ -129,7 +129,7 @@ pub enum ConfirmationResponse { SendTransaction(H256), /// Transaction RLP SignTransaction(RichRawTransaction), - /// Signature + /// Signature (encoded as VRS) Signature(H520), /// Decrypted data Decrypt(Bytes),