From c028f106b1924b6656bbd1f66d7fbdf836e219f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Wed, 30 Nov 2016 16:11:41 +0100 Subject: [PATCH] RPC for confirming with token --- ethcore/src/account_provider.rs | 24 +++++- rpc/src/v1/helpers/dispatch.rs | 114 +++++++++++++++++++++++------ rpc/src/v1/impls/personal.rs | 4 +- rpc/src/v1/impls/signer.rs | 61 +++++++++------ rpc/src/v1/impls/signing.rs | 4 +- rpc/src/v1/impls/signing_unsafe.rs | 3 +- rpc/src/v1/tests/mocked/signer.rs | 46 ++++++++++++ rpc/src/v1/traits/signer.rs | 6 +- rpc/src/v1/types/confirmations.rs | 25 +++++++ rpc/src/v1/types/mod.rs.in | 4 +- 10 files changed, 237 insertions(+), 54 deletions(-) diff --git a/ethcore/src/account_provider.rs b/ethcore/src/account_provider.rs index 311204bb1..637a20401 100644 --- a/ethcore/src/account_provider.rs +++ b/ethcore/src/account_provider.rs @@ -401,6 +401,28 @@ impl AccountProvider { Ok((signature, new_token)) } + /// Decrypts a message with given token. Returns a token to use in next operation for this account. + pub fn decrypt_with_token(&self, account: Address, token: AccountToken, shared_mac: &[u8], message: &[u8]) + -> Result<(Vec, AccountToken), Error> + { + let is_std_password = try!(self.sstore.test_password(&account, &token)); + + let new_token = random_string(16); + let message = if is_std_password { + // Insert to transient store + try!(self.sstore.copy_account(&self.transient_sstore, &account, &token, &new_token)); + // decrypt + try!(self.sstore.decrypt(&account, &token, shared_mac, message)) + } else { + // check transient store + try!(self.transient_sstore.change_password(&account, &token, &new_token)); + // and decrypt + try!(self.transient_sstore.decrypt(&account, &token, shared_mac, message)) + }; + + Ok((message, new_token)) + } + /// Decrypts a message. If password is not provided the account must be unlocked. pub fn decrypt(&self, account: Address, password: Option, shared_mac: &[u8], message: &[u8]) -> Result, Error> { let password = try!(password.map(Ok).unwrap_or_else(|| self.password(&account))); @@ -477,8 +499,8 @@ mod tests { #[test] fn should_sign_and_return_token() { - let kp = Random.generate().unwrap(); // given + let kp = Random.generate().unwrap(); let ap = AccountProvider::transient_provider(); assert!(ap.insert_account(kp.secret().clone(), "test").is_ok()); diff --git a/rpc/src/v1/helpers/dispatch.rs b/rpc/src/v1/helpers/dispatch.rs index a66bc816d..3ac310be6 100644 --- a/rpc/src/v1/helpers/dispatch.rs +++ b/rpc/src/v1/helpers/dispatch.rs @@ -14,6 +14,7 @@ // You should have received a copy of the GNU General Public License // along with Parity. If not, see . +use std::fmt::Debug; use rlp; use util::{Address, H256, U256, Uint, Bytes}; use util::bytes::ToPretty; @@ -37,46 +38,101 @@ use v1::types::{ pub const DEFAULT_MAC: [u8; 2] = [0, 0]; -pub fn execute(client: &C, miner: &M, accounts: &AccountProvider, payload: ConfirmationPayload, pass: Option) -> Result +type AccountToken = String; + +#[derive(Debug, Clone, PartialEq)] +pub enum SignWith { + Nothing, + Password(String), + Token(AccountToken), +} + +#[derive(Debug)] +pub enum WithToken { + No(T), + Yes(T, AccountToken), +} + +impl WithToken { + pub fn map(self, f: F) -> WithToken where + S: Debug, + F: FnOnce(T) -> S, + { + match self { + WithToken::No(v) => WithToken::No(f(v)), + WithToken::Yes(v, token) => WithToken::Yes(f(v), token), + } + } + + pub fn into_value(self) -> T { + match self { + WithToken::No(v) => v, + WithToken::Yes(v, ..) => v, + } + } +} + +impl From<(T, AccountToken)> for WithToken { + fn from(tuple: (T, AccountToken)) -> Self { + WithToken::Yes(tuple.0, tuple.1) + } +} + +pub fn execute(client: &C, miner: &M, accounts: &AccountProvider, payload: ConfirmationPayload, pass: SignWith) -> Result, Error> where C: MiningBlockChainClient, M: MinerService { match payload { ConfirmationPayload::SendTransaction(request) => { sign_and_dispatch(client, miner, accounts, request, pass) - .map(RpcH256::from) - .map(ConfirmationResponse::SendTransaction) + .map(|result| result + .map(RpcH256::from) + .map(ConfirmationResponse::SendTransaction) + ) }, ConfirmationPayload::SignTransaction(request) => { sign_no_dispatch(client, miner, accounts, request, pass) - .map(RpcRichRawTransaction::from) - .map(ConfirmationResponse::SignTransaction) + .map(|result| result + .map(RpcRichRawTransaction::from) + .map(ConfirmationResponse::SignTransaction) + ) }, ConfirmationPayload::Signature(address, hash) => { signature(accounts, address, hash, pass) - .map(RpcH520::from) - .map(ConfirmationResponse::Signature) + .map(|result| result + .map(RpcH520::from) + .map(ConfirmationResponse::Signature) + ) }, ConfirmationPayload::Decrypt(address, data) => { decrypt(accounts, address, data, pass) - .map(RpcBytes) - .map(ConfirmationResponse::Decrypt) + .map(|result| result + .map(RpcBytes) + .map(ConfirmationResponse::Decrypt) + ) }, } } -fn signature(accounts: &AccountProvider, address: Address, hash: H256, password: Option) -> Result { - accounts.sign(address, password.clone(), hash).map_err(|e| match password { - Some(_) => errors::from_password_error(e), - None => errors::from_signing_error(e), +fn signature(accounts: &AccountProvider, address: Address, hash: H256, password: SignWith) -> Result, Error> { + match password.clone() { + SignWith::Nothing => accounts.sign(address, None, hash).map(WithToken::No), + SignWith::Password(pass) => accounts.sign(address, Some(pass), hash).map(WithToken::No), + SignWith::Token(token) => accounts.sign_with_token(address, token, hash).map(Into::into), + }.map_err(|e| match password { + SignWith::Nothing => errors::from_signing_error(e), + _ => errors::from_password_error(e), }) } -fn decrypt(accounts: &AccountProvider, address: Address, msg: Bytes, password: Option) -> Result { - accounts.decrypt(address, password.clone(), &DEFAULT_MAC, &msg) - .map_err(|e| match password { - Some(_) => errors::from_password_error(e), - None => errors::from_signing_error(e), - }) +fn decrypt(accounts: &AccountProvider, address: Address, msg: Bytes, password: SignWith) -> Result, Error> { + match password.clone() { + SignWith::Nothing => accounts.decrypt(address, None, &DEFAULT_MAC, &msg).map(WithToken::No), + SignWith::Password(pass) => accounts.decrypt(address, Some(pass), &DEFAULT_MAC, &msg).map(WithToken::No), + SignWith::Token(token) => accounts.decrypt_with_token(address, token, &DEFAULT_MAC, &msg).map(Into::into), + }.map_err(|e| match password { + SignWith::Nothing => errors::from_signing_error(e), + _ => errors::from_password_error(e), + }) } pub fn dispatch_transaction(client: &C, miner: &M, signed_transaction: SignedTransaction) -> Result @@ -88,7 +144,7 @@ pub fn dispatch_transaction(client: &C, miner: &M, signed_transaction: Sig .map(|_| hash) } -pub fn sign_no_dispatch(client: &C, miner: &M, accounts: &AccountProvider, filled: FilledTransactionRequest, password: Option) -> Result +pub fn sign_no_dispatch(client: &C, miner: &M, accounts: &AccountProvider, filled: FilledTransactionRequest, password: SignWith) -> Result, Error> where C: MiningBlockChainClient, M: MinerService { let network_id = client.signing_network_id(); @@ -110,20 +166,32 @@ pub fn sign_no_dispatch(client: &C, miner: &M, accounts: &AccountProvider, let hash = t.hash(network_id); let signature = try!(signature(accounts, address, hash, password)); - t.with_signature(signature, network_id) + signature.map(|sig| { + t.with_signature(sig, network_id) + }) }; Ok(signed_transaction) } -pub fn sign_and_dispatch(client: &C, miner: &M, accounts: &AccountProvider, filled: FilledTransactionRequest, password: Option) -> Result +pub fn sign_and_dispatch(client: &C, miner: &M, accounts: &AccountProvider, filled: FilledTransactionRequest, password: SignWith) -> Result, Error> where C: MiningBlockChainClient, M: MinerService { let network_id = client.signing_network_id(); let signed_transaction = try!(sign_no_dispatch(client, miner, accounts, filled, password)); + let (signed_transaction, token) = match signed_transaction { + WithToken::No(signed_transaction) => (signed_transaction, None), + WithToken::Yes(signed_transaction, token) => (signed_transaction, Some(token)), + }; + trace!(target: "miner", "send_transaction: dispatching tx: {} for network ID {:?}", rlp::encode(&signed_transaction).to_vec().pretty(), network_id); - dispatch_transaction(&*client, &*miner, signed_transaction) + dispatch_transaction(&*client, &*miner, signed_transaction).map(|hash| { + match token { + Some(ref token) => WithToken::Yes(hash, token.clone()), + None => WithToken::No(hash), + } + }) } pub fn fill_optional_fields(request: TransactionRequest, client: &C, miner: &M) -> FilledTransactionRequest diff --git a/rpc/src/v1/impls/personal.rs b/rpc/src/v1/impls/personal.rs index 1515e3fa1..48ed584cf 100644 --- a/rpc/src/v1/impls/personal.rs +++ b/rpc/src/v1/impls/personal.rs @@ -114,7 +114,7 @@ impl Personal for PersonalClient where C: MiningBl &*miner, &*accounts, request, - Some(password) - ).map(Into::into) + dispatch::SignWith::Password(password) + ).map(|v| v.into_value().into()) } } diff --git a/rpc/src/v1/impls/signer.rs b/rpc/src/v1/impls/signer.rs index 66f46ba01..bdb34dab7 100644 --- a/rpc/src/v1/impls/signer.rs +++ b/rpc/src/v1/impls/signer.rs @@ -26,7 +26,7 @@ use ethcore::miner::MinerService; use jsonrpc_core::Error; use v1::traits::Signer; -use v1::types::{TransactionModification, ConfirmationRequest, ConfirmationResponse, U256, Bytes}; +use v1::types::{TransactionModification, ConfirmationRequest, ConfirmationResponse, ConfirmationResponseWithToken, U256, Bytes}; use v1::helpers::{errors, SignerService, SigningQueue, ConfirmationPayload}; use v1::helpers::dispatch::{self, dispatch_transaction}; @@ -60,6 +60,35 @@ impl SignerClient where C: MiningBlockChainClient, take_weak!(self.client).keep_alive(); Ok(()) } + + fn confirm_internal(&self, id: U256, modification: TransactionModification, f: F) -> Result where + F: FnOnce(&C, &M, &AccountProvider, ConfirmationPayload) -> Result, + { + try!(self.active()); + + let id = id.into(); + let accounts = take_weak!(self.accounts); + let signer = take_weak!(self.signer); + let client = take_weak!(self.client); + let miner = take_weak!(self.miner); + + signer.peek(&id).map(|confirmation| { + let mut payload = confirmation.payload.clone(); + // Modify payload + match (&mut payload, modification.gas_price) { + (&mut ConfirmationPayload::SendTransaction(ref mut request), Some(gas_price)) => { + request.gas_price = gas_price.into(); + }, + _ => {}, + } + let result = f(&*client, &*miner, &*accounts, payload); + // Execute + if let Ok(ref response) = result { + signer.request_confirmed(id, Ok(response.clone())); + } + result + }).unwrap_or_else(|| Err(errors::invalid_params("Unknown RequestID", id))) + } } impl Signer for SignerClient where C: MiningBlockChainClient, M: MinerService { @@ -78,30 +107,14 @@ impl Signer for SignerClient where C: MiningBlockC // TODO [ToDr] TransactionModification is redundant for some calls // might be better to replace it in future fn confirm_request(&self, id: U256, modification: TransactionModification, pass: String) -> Result { - try!(self.active()); + self.confirm_internal(id, modification, move |client, miner, accounts, payload| { + dispatch::execute(client, miner, accounts, payload, dispatch::SignWith::Password(pass)) + .map(|v| v.into_value()) + }) + } - let id = id.into(); - let accounts = take_weak!(self.accounts); - let signer = take_weak!(self.signer); - let client = take_weak!(self.client); - let miner = take_weak!(self.miner); - - signer.peek(&id).map(|confirmation| { - let mut payload = confirmation.payload.clone(); - // Modify payload - match (&mut payload, modification.gas_price) { - (&mut ConfirmationPayload::SendTransaction(ref mut request), Some(gas_price)) => { - request.gas_price = gas_price.into(); - }, - _ => {}, - } - // Execute - let result = dispatch::execute(&*client, &*miner, &*accounts, payload, Some(pass)); - if let Ok(ref response) = result { - signer.request_confirmed(id, Ok(response.clone())); - } - result - }).unwrap_or_else(|| Err(errors::invalid_params("Unknown RequestID", id))) + fn confirm_request_with_token(&self, id: U256, modification: TransactionModification, token: String) -> Result { + unimplemented!() } fn confirm_request_raw(&self, id: U256, bytes: Bytes) -> Result { diff --git a/rpc/src/v1/impls/signing.rs b/rpc/src/v1/impls/signing.rs index 262e04dfb..c055628c0 100644 --- a/rpc/src/v1/impls/signing.rs +++ b/rpc/src/v1/impls/signing.rs @@ -99,7 +99,9 @@ impl SigningQueueClient where let sender = payload.sender(); if accounts.is_unlocked(sender) { - return dispatch::execute(&*client, &*miner, &*accounts, payload, None).map(DispatchResult::Value); + return dispatch::execute(&*client, &*miner, &*accounts, payload, dispatch::SignWith::Nothing) + .map(|v| v.into_value()) + .map(DispatchResult::Value); } take_weak!(self.signer).add_request(payload) diff --git a/rpc/src/v1/impls/signing_unsafe.rs b/rpc/src/v1/impls/signing_unsafe.rs index 46ffe6ded..3f03f7609 100644 --- a/rpc/src/v1/impls/signing_unsafe.rs +++ b/rpc/src/v1/impls/signing_unsafe.rs @@ -75,7 +75,8 @@ impl SigningUnsafeClient where let accounts = take_weak!(self.accounts); let payload = dispatch::from_rpc(payload, &*client, &*miner); - dispatch::execute(&*client, &*miner, &*accounts, payload, None) + dispatch::execute(&*client, &*miner, &*accounts, payload, dispatch::SignWith::Nothing) + .map(|v| v.into_value()) } } diff --git a/rpc/src/v1/tests/mocked/signer.rs b/rpc/src/v1/tests/mocked/signer.rs index e2ba580e0..c4df02606 100644 --- a/rpc/src/v1/tests/mocked/signer.rs +++ b/rpc/src/v1/tests/mocked/signer.rs @@ -209,6 +209,52 @@ fn should_confirm_transaction_and_dispatch() { assert_eq!(tester.miner.imported_transactions.lock().len(), 1); } +#[test] +fn should_confirm_transaction_with_token() { + // 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::SendTransaction(FilledTransactionRequest { + from: address, + to: Some(recipient), + gas_price: U256::from(10_000), + gas: U256::from(10_000_000), + value: U256::from(1), + data: vec![], + nonce: None, + })).unwrap(); + + 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, token) = tester.accounts.sign_with_token(address, "test".into(), t.hash(None)).unwrap(); + let t = t.with_signature(signature, None); + + assert_eq!(tester.signer.requests().len(), 1); + + // when + let request = r#"{ + "jsonrpc":"2.0", + "method":"signer_confirmRequestWithToken", + "params":["0x1", {"gasPrice":"0x1000"}, ""#.to_owned() + &token + r#""], + "id":1 + }"#; + let response = r#"{"jsonrpc":"2.0","result":{"result":""#.to_owned() + + format!("0x{:?}", t.hash()).as_ref() + + r#""token":""},"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(), 1); +} + #[test] fn should_confirm_transaction_with_rlp() { // given diff --git a/rpc/src/v1/traits/signer.rs b/rpc/src/v1/traits/signer.rs index eafa520d4..5014dc4a0 100644 --- a/rpc/src/v1/traits/signer.rs +++ b/rpc/src/v1/traits/signer.rs @@ -18,7 +18,7 @@ use jsonrpc_core::Error; use v1::helpers::auto_args::Wrap; -use v1::types::{U256, Bytes, TransactionModification, ConfirmationRequest, ConfirmationResponse}; +use v1::types::{U256, Bytes, TransactionModification, ConfirmationRequest, ConfirmationResponse, ConfirmationResponseWithToken}; build_rpc_trait! { @@ -33,6 +33,10 @@ build_rpc_trait! { #[rpc(name = "signer_confirmRequest")] fn confirm_request(&self, U256, TransactionModification, String) -> Result; + /// Confirm specific request with token. + #[rpc(name = "signer_confirmRequestWithToken")] + fn confirm_request_with_token(&self, U256, TransactionModification, String) -> Result; + /// Confirm specific request with already signed data. #[rpc(name = "signer_confirmRequestRaw")] fn confirm_request_raw(&self, U256, Bytes) -> Result; diff --git a/rpc/src/v1/types/confirmations.rs b/rpc/src/v1/types/confirmations.rs index bbbad83f3..795d24726 100644 --- a/rpc/src/v1/types/confirmations.rs +++ b/rpc/src/v1/types/confirmations.rs @@ -101,6 +101,15 @@ impl Serialize for ConfirmationResponse { } } +/// Confirmation response with additional token for further requests +#[derive(Debug, Clone, PartialEq, Serialize)] +pub struct ConfirmationResponseWithToken { + /// Actual response + pub result: ConfirmationResponse, + /// New token + pub token: String, +} + /// Confirmation payload, i.e. the thing to be confirmed #[derive(Debug, Clone, Eq, PartialEq, Hash, Serialize)] pub enum ConfirmationPayload { @@ -247,5 +256,21 @@ mod tests { gas_price: None, }); } + + #[test] + fn should_serialize_confirmation_response_with_token() { + // given + let response = ConfirmationResponseWithToken { + result: ConfirmationResponse::SendTransaction(H256::default()), + token: "test-token".into(), + }; + + // when + let res = serde_json::to_string(&response); + let expected = r#"{"result":"0x0000000000000000000000000000000000000000","token":"test-token"}"#; + + // then + assert_eq!(res.unwrap(), expected.to_owned()); + } } diff --git a/rpc/src/v1/types/mod.rs.in b/rpc/src/v1/types/mod.rs.in index 55e8fd27b..6b6f01443 100644 --- a/rpc/src/v1/types/mod.rs.in +++ b/rpc/src/v1/types/mod.rs.in @@ -38,7 +38,9 @@ pub use self::bytes::Bytes; pub use self::block::{RichBlock, Block, BlockTransactions}; pub use self::block_number::BlockNumber; pub use self::call_request::CallRequest; -pub use self::confirmations::{ConfirmationPayload, ConfirmationRequest, ConfirmationResponse, TransactionModification, SignRequest, DecryptRequest, Either}; +pub use self::confirmations::{ + ConfirmationPayload, ConfirmationRequest, ConfirmationResponse, ConfirmationResponseWithToken, TransactionModification, SignRequest, DecryptRequest, Either +}; pub use self::filter::{Filter, FilterChanges}; pub use self::hash::{H64, H160, H256, H512, H520, H2048}; pub use self::index::Index;