From b7e2afd5c0df8af8c874fefab9398039433f6a2e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Wed, 21 Sep 2016 12:44:49 +0200 Subject: [PATCH] New signer token RPC & Initial signer connection without token. (#2096) * Allowing signer to use initial token * Generating new tokens via RPC * Fixing RPC tests * Fixing signer doctest * whitespace [ci:skip] * whitespace [ci:skip] --- devtools/src/http_client.rs | 4 +- devtools/src/random_path.rs | 19 ++- parity/rpc_apis.rs | 12 +- parity/run.rs | 3 +- parity/signer.rs | 2 +- rpc/src/lib.rs | 2 +- rpc/src/v1/helpers/errors.rs | 7 ++ rpc/src/v1/helpers/mod.rs | 2 + rpc/src/v1/helpers/signer.rs | 61 +++++++++ rpc/src/v1/impls/eth_signing.rs | 16 +-- rpc/src/v1/impls/ethcore.rs | 12 +- rpc/src/v1/impls/personal_signer.rs | 36 ++++-- rpc/src/v1/mod.rs | 2 +- rpc/src/v1/tests/mocked/eth_signing.rs | 30 ++--- rpc/src/v1/tests/mocked/ethcore.rs | 6 +- rpc/src/v1/tests/mocked/personal_signer.rs | 55 +++++--- rpc/src/v1/traits/personal.rs | 4 + signer/src/authcode_store.rs | 44 ++++++- signer/src/tests/mod.rs | 140 +++++++++++++++++++-- signer/src/ws_server/mod.rs | 1 - signer/src/ws_server/session.rs | 13 +- 21 files changed, 377 insertions(+), 94 deletions(-) create mode 100644 rpc/src/v1/helpers/signer.rs diff --git a/devtools/src/http_client.rs b/devtools/src/http_client.rs index 27fa6ec50..f194c4004 100644 --- a/devtools/src/http_client.rs +++ b/devtools/src/http_client.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::time::Duration; use std::io::{Read, Write}; use std::str::{self, Lines}; use std::net::{TcpStream, SocketAddr}; @@ -43,10 +44,11 @@ pub fn read_block(lines: &mut Lines, all: bool) -> String { pub fn request(address: &SocketAddr, request: &str) -> Response { let mut req = TcpStream::connect(address).unwrap(); + req.set_read_timeout(Some(Duration::from_secs(1))).unwrap(); req.write_all(request.as_bytes()).unwrap(); let mut response = String::new(); - req.read_to_string(&mut response).unwrap(); + let _ = req.read_to_string(&mut response); let mut lines = response.lines(); let status = lines.next().unwrap().to_owned(); diff --git a/devtools/src/random_path.rs b/devtools/src/random_path.rs index d58042512..9c6c261a2 100644 --- a/devtools/src/random_path.rs +++ b/devtools/src/random_path.rs @@ -23,7 +23,8 @@ use std::ops::{Deref, DerefMut}; use rand::random; pub struct RandomTempPath { - path: PathBuf + path: PathBuf, + pub panic_on_drop_failure: bool, } pub fn random_filename() -> String { @@ -39,7 +40,8 @@ impl RandomTempPath { let mut dir = env::temp_dir(); dir.push(random_filename()); RandomTempPath { - path: dir.clone() + path: dir.clone(), + panic_on_drop_failure: true, } } @@ -48,7 +50,8 @@ impl RandomTempPath { dir.push(random_filename()); fs::create_dir_all(dir.as_path()).unwrap(); RandomTempPath { - path: dir.clone() + path: dir.clone(), + panic_on_drop_failure: true, } } @@ -72,12 +75,20 @@ impl AsRef for RandomTempPath { self.as_path() } } +impl Deref for RandomTempPath { + type Target = Path; + fn deref(&self) -> &Self::Target { + self.as_path() + } +} impl Drop for RandomTempPath { fn drop(&mut self) { if let Err(_) = fs::remove_dir_all(&self) { if let Err(e) = fs::remove_file(&self) { - panic!("Failed to remove temp directory. Here's what prevented this from happening: ({})", e); + if self.panic_on_drop_failure { + panic!("Failed to remove temp directory. Here's what prevented this from happening: ({})", e); + } } } } diff --git a/parity/rpc_apis.rs b/parity/rpc_apis.rs index 624c6b3f4..29b33b844 100644 --- a/parity/rpc_apis.rs +++ b/parity/rpc_apis.rs @@ -25,7 +25,7 @@ use ethcore::client::Client; use ethcore::account_provider::AccountProvider; use ethsync::{ManageNetwork, SyncProvider}; use ethcore_rpc::{Extendable, NetworkSettings}; -pub use ethcore_rpc::ConfirmationsQueue; +pub use ethcore_rpc::SignerService; #[derive(Debug, PartialEq, Clone, Eq, Hash)] @@ -94,7 +94,7 @@ impl FromStr for ApiSet { pub struct Dependencies { pub signer_port: Option, - pub signer_queue: Arc, + pub signer_service: Arc, pub client: Arc, pub sync: Arc, pub net: Arc, @@ -173,7 +173,7 @@ pub fn setup_rpc(server: T, deps: Arc, apis: ApiSet server.add_delegate(filter_client.to_delegate()); if deps.signer_port.is_some() { - server.add_delegate(EthSigningQueueClient::new(&deps.signer_queue, &deps.client, &deps.miner, &deps.secret_store).to_delegate()); + server.add_delegate(EthSigningQueueClient::new(&deps.signer_service, &deps.client, &deps.miner, &deps.secret_store).to_delegate()); } else { server.add_delegate(EthSigningUnsafeClient::new(&deps.client, &deps.secret_store, &deps.miner).to_delegate()); } @@ -182,11 +182,11 @@ pub fn setup_rpc(server: T, deps: Arc, apis: ApiSet server.add_delegate(PersonalClient::new(&deps.secret_store, &deps.client, &deps.miner, deps.signer_port, deps.geth_compatibility).to_delegate()); }, Api::Signer => { - server.add_delegate(SignerClient::new(&deps.secret_store, &deps.client, &deps.miner, &deps.signer_queue).to_delegate()); + server.add_delegate(SignerClient::new(&deps.secret_store, &deps.client, &deps.miner, &deps.signer_service).to_delegate()); }, Api::Ethcore => { - let queue = deps.signer_port.map(|_| deps.signer_queue.clone()); - server.add_delegate(EthcoreClient::new(&deps.client, &deps.miner, &deps.sync, &deps.net_service, deps.logger.clone(), deps.settings.clone(), queue).to_delegate()) + let signer = deps.signer_port.map(|_| deps.signer_service.clone()); + server.add_delegate(EthcoreClient::new(&deps.client, &deps.miner, &deps.sync, &deps.net_service, deps.logger.clone(), deps.settings.clone(), signer).to_delegate()) }, Api::EthcoreSet => { server.add_delegate(EthcoreSetClient::new(&deps.client, &deps.miner, &deps.net_service).to_delegate()) diff --git a/parity/run.rs b/parity/run.rs index cefd8bb21..6e368522e 100644 --- a/parity/run.rs +++ b/parity/run.rs @@ -206,9 +206,10 @@ pub fn execute(cmd: RunCmd) -> Result<(), String> { } // set up dependencies for rpc servers + let signer_path = cmd.signer_conf.signer_path.clone(); let deps_for_rpc_apis = Arc::new(rpc_apis::Dependencies { signer_port: cmd.signer_port, - signer_queue: Arc::new(rpc_apis::ConfirmationsQueue::default()), + signer_service: Arc::new(rpc_apis::SignerService::new(move || signer::new_token(signer_path.clone()))), client: client.clone(), sync: sync_provider.clone(), net: manage_network.clone(), diff --git a/parity/signer.rs b/parity/signer.rs index e6924dcef..b60bc7211 100644 --- a/parity/signer.rs +++ b/parity/signer.rs @@ -90,7 +90,7 @@ fn do_start(conf: Configuration, deps: Dependencies) -> Result Error { } } +pub fn token(e: String) -> Error { + Error { + code: ErrorCode::ServerError(codes::UNKNOWN_ERROR), + message: "There was an error when saving your authorization tokens.".into(), + data: Some(Value::String(e)), + } +} pub fn signer_disabled() -> Error { Error { diff --git a/rpc/src/v1/helpers/mod.rs b/rpc/src/v1/helpers/mod.rs index d71eaac41..f5b45b55b 100644 --- a/rpc/src/v1/helpers/mod.rs +++ b/rpc/src/v1/helpers/mod.rs @@ -21,6 +21,7 @@ pub mod params; mod poll_manager; mod poll_filter; mod requests; +mod signer; mod signing_queue; mod network_settings; @@ -28,4 +29,5 @@ pub use self::poll_manager::PollManager; pub use self::poll_filter::PollFilter; pub use self::requests::{TransactionRequest, FilledTransactionRequest, ConfirmationRequest, ConfirmationPayload, CallRequest}; pub use self::signing_queue::{ConfirmationsQueue, ConfirmationPromise, ConfirmationResult, SigningQueue, QueueEvent}; +pub use self::signer::SignerService; pub use self::network_settings::NetworkSettings; diff --git a/rpc/src/v1/helpers/signer.rs b/rpc/src/v1/helpers/signer.rs new file mode 100644 index 000000000..2cebc8261 --- /dev/null +++ b/rpc/src/v1/helpers/signer.rs @@ -0,0 +1,61 @@ +// Copyright 2015, 2016 Ethcore (UK) Ltd. +// This file is part of Parity. + +// Parity is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// Parity is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with Parity. If not, see . + +use std::sync::Arc; +use std::ops::Deref; +use v1::helpers::signing_queue::{ConfirmationsQueue}; + +/// Manages communication with Signer crate +pub struct SignerService { + queue: Arc, + generate_new_token: Box Result + Send + Sync + 'static>, +} + +impl SignerService { + + /// Creates new Signer Service given function to generate new tokens. + pub fn new(new_token: F) -> Self + where F: Fn() -> Result + Send + Sync + 'static { + SignerService { + queue: Arc::new(ConfirmationsQueue::default()), + generate_new_token: Box::new(new_token), + } + } + + /// Generates new token. + pub fn generate_token(&self) -> Result { + (self.generate_new_token)() + } + + /// Returns a reference to `ConfirmationsQueue` + pub fn queue(&self) -> Arc { + self.queue.clone() + } + + #[cfg(test)] + /// Creates new Signer Service for tests. + pub fn new_test() -> Self { + SignerService::new(|| Ok("new_token".into())) + } +} + +impl Deref for SignerService { + type Target = ConfirmationsQueue; + fn deref(&self) -> &Self::Target { + &self.queue + } +} + diff --git a/rpc/src/v1/impls/eth_signing.rs b/rpc/src/v1/impls/eth_signing.rs index c19b5819d..9ae243de4 100644 --- a/rpc/src/v1/impls/eth_signing.rs +++ b/rpc/src/v1/impls/eth_signing.rs @@ -23,7 +23,7 @@ use ethcore::client::MiningBlockChainClient; use util::{U256, Address, H256, Mutex}; use transient_hashmap::TransientHashMap; use ethcore::account_provider::AccountProvider; -use v1::helpers::{errors, SigningQueue, ConfirmationPromise, ConfirmationResult, ConfirmationsQueue, ConfirmationPayload, TransactionRequest as TRequest, FilledTransactionRequest as FilledRequest}; +use v1::helpers::{errors, SigningQueue, ConfirmationPromise, ConfirmationResult, ConfirmationPayload, TransactionRequest as TRequest, FilledTransactionRequest as FilledRequest, SignerService}; use v1::helpers::dispatch::{default_gas_price, sign_and_dispatch}; use v1::traits::EthSigning; use v1::types::{TransactionRequest, H160 as RpcH160, H256 as RpcH256, H520 as RpcH520, U256 as RpcU256}; @@ -43,7 +43,7 @@ fn fill_optional_fields(request: TRequest, client: &C, miner: &M) -> Fille /// Implementation of functions that require signing when no trusted signer is used. pub struct EthSigningQueueClient where C: MiningBlockChainClient, M: MinerService { - queue: Weak, + signer: Weak, accounts: Weak, client: Weak, miner: Weak, @@ -60,9 +60,9 @@ pub enum DispatchResult { impl EthSigningQueueClient where C: MiningBlockChainClient, M: MinerService { /// Creates a new signing queue client given shared signing queue. - pub fn new(queue: &Arc, client: &Arc, miner: &Arc, accounts: &Arc) -> Self { + pub fn new(signer: &Arc, client: &Arc, miner: &Arc, accounts: &Arc) -> Self { EthSigningQueueClient { - queue: Arc::downgrade(queue), + signer: Arc::downgrade(signer), accounts: Arc::downgrade(accounts), client: Arc::downgrade(client), miner: Arc::downgrade(miner), @@ -86,8 +86,8 @@ impl EthSigningQueueClient where C: MiningBlockChainClient, M: Miner return Ok(DispatchResult::Value(to_value(&accounts.sign(address, msg).ok().map_or_else(RpcH520::default, Into::into)))) } - let queue = take_weak!(self.queue); - queue.add_request(ConfirmationPayload::Sign(address, msg)) + let signer = take_weak!(self.signer); + signer.add_request(ConfirmationPayload::Sign(address, msg)) .map(DispatchResult::Promise) .map_err(|_| errors::request_rejected_limit()) }) @@ -105,9 +105,9 @@ impl EthSigningQueueClient where C: MiningBlockChainClient, M: Miner return sign_and_dispatch(&*client, &*miner, request, &*accounts, sender).map(DispatchResult::Value); } - let queue = take_weak!(self.queue); + let signer = take_weak!(self.signer); let request = fill_optional_fields(request, &*client, &*miner); - queue.add_request(ConfirmationPayload::Transaction(request)) + signer.add_request(ConfirmationPayload::Transaction(request)) .map(DispatchResult::Promise) .map_err(|_| errors::request_rejected_limit()) }) diff --git a/rpc/src/v1/impls/ethcore.rs b/rpc/src/v1/impls/ethcore.rs index ee352e65a..6ff7cff26 100644 --- a/rpc/src/v1/impls/ethcore.rs +++ b/rpc/src/v1/impls/ethcore.rs @@ -30,7 +30,7 @@ use ethcore::client::{MiningBlockChainClient}; use jsonrpc_core::*; use v1::traits::Ethcore; use v1::types::{Bytes, U256, H160, Peers}; -use v1::helpers::{errors, SigningQueue, ConfirmationsQueue, NetworkSettings}; +use v1::helpers::{errors, SigningQueue, SignerService, NetworkSettings}; use v1::helpers::params::expect_no_params; /// Ethcore implementation. @@ -45,7 +45,7 @@ pub struct EthcoreClient where net: Weak, logger: Arc, settings: Arc, - confirmations_queue: Option>, + signer: Option>, } impl EthcoreClient where C: MiningBlockChainClient, M: MinerService, S: SyncProvider { @@ -57,7 +57,7 @@ impl EthcoreClient where C: MiningBlockChainClient, M: net: &Arc, logger: Arc, settings: Arc, - queue: Option> + signer: Option> ) -> Self { EthcoreClient { client: Arc::downgrade(client), @@ -66,7 +66,7 @@ impl EthcoreClient where C: MiningBlockChainClient, M: net: Arc::downgrade(net), logger: logger, settings: settings, - confirmations_queue: queue, + signer: signer, } } @@ -198,9 +198,9 @@ impl Ethcore for EthcoreClient where M: MinerService + try!(self.active()); try!(expect_no_params(params)); - match self.confirmations_queue { + match self.signer { None => Err(errors::signer_disabled()), - Some(ref queue) => Ok(to_value(&queue.len())), + Some(ref signer) => Ok(to_value(&signer.len())), } } diff --git a/rpc/src/v1/impls/personal_signer.rs b/rpc/src/v1/impls/personal_signer.rs index 5cfda9a65..441ed679b 100644 --- a/rpc/src/v1/impls/personal_signer.rs +++ b/rpc/src/v1/impls/personal_signer.rs @@ -23,13 +23,13 @@ use ethcore::client::MiningBlockChainClient; use ethcore::miner::MinerService; use v1::traits::PersonalSigner; use v1::types::{TransactionModification, ConfirmationRequest, U256}; -use v1::helpers::{errors, SigningQueue, ConfirmationsQueue, ConfirmationPayload}; +use v1::helpers::{errors, SignerService, SigningQueue, ConfirmationPayload}; use v1::helpers::params::expect_no_params; use v1::helpers::dispatch::{unlock_sign_and_dispatch, signature_with_password}; /// Transactions confirmation (personal) rpc implementation. pub struct SignerClient where C: MiningBlockChainClient, M: MinerService { - queue: Weak, + signer: Weak, accounts: Weak, client: Weak, miner: Weak, @@ -38,9 +38,14 @@ pub struct SignerClient where C: MiningBlockChainClient, M: MinerService { impl SignerClient where C: MiningBlockChainClient, M: MinerService { /// Create new instance of signer client. - pub fn new(store: &Arc, client: &Arc, miner: &Arc, queue: &Arc) -> Self { + pub fn new( + store: &Arc, + client: &Arc, + miner: &Arc, + signer: &Arc, + ) -> Self { SignerClient { - queue: Arc::downgrade(queue), + signer: Arc::downgrade(signer), accounts: Arc::downgrade(store), client: Arc::downgrade(client), miner: Arc::downgrade(miner), @@ -59,8 +64,8 @@ impl PersonalSigner for SignerClient where C: Mini fn requests_to_confirm(&self, params: Params) -> Result { try!(self.active()); try!(expect_no_params(params)); - let queue = take_weak!(self.queue); - Ok(to_value(&queue.requests().into_iter().map(From::from).collect::>())) + let signer = take_weak!(self.signer); + Ok(to_value(&signer.requests().into_iter().map(From::from).collect::>())) } fn confirm_request(&self, params: Params) -> Result { @@ -71,11 +76,11 @@ impl PersonalSigner for SignerClient where C: Mini |(id, modification, pass)| { let id = id.into(); let accounts = take_weak!(self.accounts); - let queue = take_weak!(self.queue); + let signer = take_weak!(self.signer); let client = take_weak!(self.client); let miner = take_weak!(self.miner); - queue.peek(&id).map(|confirmation| { + signer.peek(&id).map(|confirmation| { let result = match confirmation.payload { ConfirmationPayload::Transaction(mut request) => { // apply modification @@ -90,7 +95,7 @@ impl PersonalSigner for SignerClient where C: Mini } }; if let Ok(ref response) = result { - queue.request_confirmed(id, Ok(response.clone())); + signer.request_confirmed(id, Ok(response.clone())); } result }).unwrap_or_else(|| Err(errors::invalid_params("Unknown RequestID", id))) @@ -102,11 +107,20 @@ impl PersonalSigner for SignerClient where C: Mini try!(self.active()); from_params::<(U256, )>(params).and_then( |(id, )| { - let queue = take_weak!(self.queue); - let res = queue.request_rejected(id.into()); + let signer = take_weak!(self.signer); + let res = signer.request_rejected(id.into()); Ok(to_value(&res.is_some())) } ) } + + fn generate_token(&self, params: Params) -> Result { + try!(self.active()); + try!(expect_no_params(params)); + let signer = take_weak!(self.signer); + signer.generate_token() + .map(|token| to_value(&token)) + .map_err(|e| errors::token(e)) + } } diff --git a/rpc/src/v1/mod.rs b/rpc/src/v1/mod.rs index 897fcf623..889b7840b 100644 --- a/rpc/src/v1/mod.rs +++ b/rpc/src/v1/mod.rs @@ -28,4 +28,4 @@ pub mod types; pub use self::traits::{Web3, Eth, EthFilter, EthSigning, Personal, PersonalSigner, Net, Ethcore, EthcoreSet, Traces, Rpc}; pub use self::impls::*; -pub use self::helpers::{SigningQueue, ConfirmationsQueue, NetworkSettings}; +pub use self::helpers::{SigningQueue, SignerService, ConfirmationsQueue, NetworkSettings}; diff --git a/rpc/src/v1/tests/mocked/eth_signing.rs b/rpc/src/v1/tests/mocked/eth_signing.rs index f06d4027a..1bf901e5f 100644 --- a/rpc/src/v1/tests/mocked/eth_signing.rs +++ b/rpc/src/v1/tests/mocked/eth_signing.rs @@ -19,7 +19,7 @@ use std::sync::Arc; use jsonrpc_core::{IoHandler, to_value}; use v1::impls::EthSigningQueueClient; use v1::traits::EthSigning; -use v1::helpers::{ConfirmationsQueue, SigningQueue}; +use v1::helpers::{SignerService, SigningQueue}; use v1::types::{H256 as RpcH256, H520 as RpcH520}; use v1::tests::helpers::TestMinerService; use util::{Address, FixedHash, Uint, U256, H256, H520}; @@ -28,7 +28,7 @@ use ethcore::client::TestBlockChainClient; use ethcore::transaction::{Transaction, Action}; struct EthSigningTester { - pub queue: Arc, + pub signer: Arc, pub client: Arc, pub miner: Arc, pub accounts: Arc, @@ -37,15 +37,15 @@ struct EthSigningTester { impl Default for EthSigningTester { fn default() -> Self { - let queue = Arc::new(ConfirmationsQueue::default()); + let signer = Arc::new(SignerService::new_test()); let client = Arc::new(TestBlockChainClient::default()); let miner = Arc::new(TestMinerService::default()); let accounts = Arc::new(AccountProvider::transient_provider()); let io = IoHandler::new(); - io.add_delegate(EthSigningQueueClient::new(&queue, &client, &miner, &accounts).to_delegate()); + io.add_delegate(EthSigningQueueClient::new(&signer, &client, &miner, &accounts).to_delegate()); EthSigningTester { - queue: queue, + signer: signer, client: client, miner: miner, accounts: accounts, @@ -63,7 +63,7 @@ fn should_add_sign_to_queue() { // given let tester = eth_signing(); let address = Address::random(); - assert_eq!(tester.queue.requests().len(), 0); + assert_eq!(tester.signer.requests().len(), 0); // when let request = r#"{ @@ -79,9 +79,9 @@ fn should_add_sign_to_queue() { // then let async_result = tester.io.handle_request(&request).unwrap(); - assert_eq!(tester.queue.requests().len(), 1); + assert_eq!(tester.signer.requests().len(), 1); // respond - tester.queue.request_confirmed(U256::from(1), Ok(to_value(&RpcH520::from(H520::default())))); + tester.signer.request_confirmed(U256::from(1), Ok(to_value(&RpcH520::from(H520::default())))); assert!(async_result.on_result(move |res| { assert_eq!(res, response.to_owned()); })); @@ -92,7 +92,7 @@ fn should_post_sign_to_queue() { // given let tester = eth_signing(); let address = Address::random(); - assert_eq!(tester.queue.requests().len(), 0); + assert_eq!(tester.signer.requests().len(), 0); // when let request = r#"{ @@ -108,7 +108,7 @@ fn should_post_sign_to_queue() { // then assert_eq!(tester.io.handle_request_sync(&request), Some(response.to_owned())); - assert_eq!(tester.queue.requests().len(), 1); + assert_eq!(tester.signer.requests().len(), 1); } #[test] @@ -155,7 +155,7 @@ fn should_check_status_of_request_when_its_resolved() { "id": 1 }"#; tester.io.handle_request_sync(&request).expect("Sent"); - tester.queue.request_confirmed(U256::from(1), Ok(to_value(&"Hello World!"))); + tester.signer.request_confirmed(U256::from(1), Ok(to_value(&"Hello World!"))); // when let request = r#"{ @@ -192,7 +192,7 @@ fn should_sign_if_account_is_unlocked() { }"#; let response = r#"{"jsonrpc":"2.0","result":""#.to_owned() + format!("0x{}", signature).as_ref() + r#"","id":1}"#; assert_eq!(tester.io.handle_request_sync(&request), Some(response.to_owned())); - assert_eq!(tester.queue.requests().len(), 0); + assert_eq!(tester.signer.requests().len(), 0); } #[test] @@ -200,7 +200,7 @@ fn should_add_transaction_to_queue() { // given let tester = eth_signing(); let address = Address::random(); - assert_eq!(tester.queue.requests().len(), 0); + assert_eq!(tester.signer.requests().len(), 0); // when let request = r#"{ @@ -219,9 +219,9 @@ fn should_add_transaction_to_queue() { // then let async_result = tester.io.handle_request(&request).unwrap(); - assert_eq!(tester.queue.requests().len(), 1); + assert_eq!(tester.signer.requests().len(), 1); // respond - tester.queue.request_confirmed(U256::from(1), Ok(to_value(&RpcH256::from(H256::default())))); + tester.signer.request_confirmed(U256::from(1), Ok(to_value(&RpcH256::from(H256::default())))); assert!(async_result.on_result(move |res| { assert_eq!(res, response.to_owned()); })); diff --git a/rpc/src/v1/tests/mocked/ethcore.rs b/rpc/src/v1/tests/mocked/ethcore.rs index d8121b6d6..a9d0c4e1d 100644 --- a/rpc/src/v1/tests/mocked/ethcore.rs +++ b/rpc/src/v1/tests/mocked/ethcore.rs @@ -22,7 +22,7 @@ use ethcore::client::{TestBlockChainClient}; use jsonrpc_core::IoHandler; use v1::{Ethcore, EthcoreClient}; -use v1::helpers::{ConfirmationsQueue, NetworkSettings}; +use v1::helpers::{SignerService, NetworkSettings}; use v1::tests::helpers::{TestSyncProvider, Config, TestMinerService}; use super::manage_network::TestManageNetwork; @@ -262,8 +262,8 @@ fn rpc_ethcore_unsigned_transactions_count() { let sync = sync_provider(); let net = network_service(); let io = IoHandler::new(); - let queue = Arc::new(ConfirmationsQueue::default()); - let ethcore = EthcoreClient::new(&client, &miner, &sync, &net, logger(), settings(), Some(queue)).to_delegate(); + let signer = Arc::new(SignerService::new_test()); + let ethcore = EthcoreClient::new(&client, &miner, &sync, &net, logger(), settings(), Some(signer)).to_delegate(); io.add_delegate(ethcore); let request = r#"{"jsonrpc": "2.0", "method": "ethcore_unsignedTransactionsCount", "params":[], "id": 1}"#; diff --git a/rpc/src/v1/tests/mocked/personal_signer.rs b/rpc/src/v1/tests/mocked/personal_signer.rs index 976b232cc..04ae829ee 100644 --- a/rpc/src/v1/tests/mocked/personal_signer.rs +++ b/rpc/src/v1/tests/mocked/personal_signer.rs @@ -23,10 +23,10 @@ use ethcore::client::TestBlockChainClient; use ethcore::transaction::{Transaction, Action}; use v1::{SignerClient, PersonalSigner}; use v1::tests::helpers::TestMinerService; -use v1::helpers::{SigningQueue, ConfirmationsQueue, FilledTransactionRequest, ConfirmationPayload}; +use v1::helpers::{SigningQueue, SignerService, FilledTransactionRequest, ConfirmationPayload}; struct PersonalSignerTester { - queue: Arc, + signer: Arc, accounts: Arc, io: IoHandler, miner: Arc, @@ -49,16 +49,16 @@ fn miner_service() -> Arc { } fn signer_tester() -> PersonalSignerTester { - let queue = Arc::new(ConfirmationsQueue::default()); + let signer = Arc::new(SignerService::new_test()); let accounts = accounts_provider(); let client = blockchain_client(); let miner = miner_service(); let io = IoHandler::new(); - io.add_delegate(SignerClient::new(&accounts, &client, &miner, &queue).to_delegate()); + io.add_delegate(SignerClient::new(&accounts, &client, &miner, &signer).to_delegate()); PersonalSignerTester { - queue: queue, + signer: signer, accounts: accounts, io: io, miner: miner, @@ -71,7 +71,7 @@ fn signer_tester() -> PersonalSignerTester { fn should_return_list_of_items_to_confirm() { // given let tester = signer_tester(); - tester.queue.add_request(ConfirmationPayload::Transaction(FilledTransactionRequest { + tester.signer.add_request(ConfirmationPayload::Transaction(FilledTransactionRequest { from: Address::from(1), to: Some(Address::from_str("d46e8dd67c5d32be8058bb8eb970870f07244567").unwrap()), gas_price: U256::from(10_000), @@ -80,7 +80,7 @@ fn should_return_list_of_items_to_confirm() { data: vec![], nonce: None, })).unwrap(); - tester.queue.add_request(ConfirmationPayload::Sign(1.into(), 5.into())).unwrap(); + tester.signer.add_request(ConfirmationPayload::Sign(1.into(), 5.into())).unwrap(); // when let request = r#"{"jsonrpc":"2.0","method":"personal_requestsToConfirm","params":[],"id":1}"#; @@ -100,7 +100,7 @@ fn should_return_list_of_items_to_confirm() { fn should_reject_transaction_from_queue_without_dispatching() { // given let tester = signer_tester(); - tester.queue.add_request(ConfirmationPayload::Transaction(FilledTransactionRequest { + tester.signer.add_request(ConfirmationPayload::Transaction(FilledTransactionRequest { from: Address::from(1), to: Some(Address::from_str("d46e8dd67c5d32be8058bb8eb970870f07244567").unwrap()), gas_price: U256::from(10_000), @@ -109,7 +109,7 @@ fn should_reject_transaction_from_queue_without_dispatching() { data: vec![], nonce: None, })).unwrap(); - assert_eq!(tester.queue.requests().len(), 1); + assert_eq!(tester.signer.requests().len(), 1); // when let request = r#"{"jsonrpc":"2.0","method":"personal_rejectRequest","params":["0x1"],"id":1}"#; @@ -117,7 +117,7 @@ fn should_reject_transaction_from_queue_without_dispatching() { // then assert_eq!(tester.io.handle_request_sync(&request), Some(response.to_owned())); - assert_eq!(tester.queue.requests().len(), 0); + assert_eq!(tester.signer.requests().len(), 0); assert_eq!(tester.miner.imported_transactions.lock().len(), 0); } @@ -125,7 +125,7 @@ fn should_reject_transaction_from_queue_without_dispatching() { fn should_not_remove_transaction_if_password_is_invalid() { // given let tester = signer_tester(); - tester.queue.add_request(ConfirmationPayload::Transaction(FilledTransactionRequest { + tester.signer.add_request(ConfirmationPayload::Transaction(FilledTransactionRequest { from: Address::from(1), to: Some(Address::from_str("d46e8dd67c5d32be8058bb8eb970870f07244567").unwrap()), gas_price: U256::from(10_000), @@ -134,7 +134,7 @@ fn should_not_remove_transaction_if_password_is_invalid() { data: vec![], nonce: None, })).unwrap(); - assert_eq!(tester.queue.requests().len(), 1); + assert_eq!(tester.signer.requests().len(), 1); // when let request = r#"{"jsonrpc":"2.0","method":"personal_confirmRequest","params":["0x1",{},"xxx"],"id":1}"#; @@ -142,15 +142,15 @@ fn should_not_remove_transaction_if_password_is_invalid() { // then assert_eq!(tester.io.handle_request_sync(&request), Some(response.to_owned())); - assert_eq!(tester.queue.requests().len(), 1); + assert_eq!(tester.signer.requests().len(), 1); } #[test] fn should_not_remove_sign_if_password_is_invalid() { // given let tester = signer_tester(); - tester.queue.add_request(ConfirmationPayload::Sign(0.into(), 5.into())).unwrap(); - assert_eq!(tester.queue.requests().len(), 1); + tester.signer.add_request(ConfirmationPayload::Sign(0.into(), 5.into())).unwrap(); + assert_eq!(tester.signer.requests().len(), 1); // when let request = r#"{"jsonrpc":"2.0","method":"personal_confirmRequest","params":["0x1",{},"xxx"],"id":1}"#; @@ -158,7 +158,7 @@ fn should_not_remove_sign_if_password_is_invalid() { // then assert_eq!(tester.io.handle_request_sync(&request), Some(response.to_owned())); - assert_eq!(tester.queue.requests().len(), 1); + assert_eq!(tester.signer.requests().len(), 1); } #[test] @@ -167,7 +167,7 @@ fn should_confirm_transaction_and_dispatch() { let tester = signer_tester(); let address = tester.accounts.new_account("test").unwrap(); let recipient = Address::from_str("d46e8dd67c5d32be8058bb8eb970870f07244567").unwrap(); - tester.queue.add_request(ConfirmationPayload::Transaction(FilledTransactionRequest { + tester.signer.add_request(ConfirmationPayload::Transaction(FilledTransactionRequest { from: address, to: Some(recipient), gas_price: U256::from(10_000), @@ -189,7 +189,7 @@ fn should_confirm_transaction_and_dispatch() { let signature = tester.accounts.sign(address, t.hash()).unwrap(); let t = t.with_signature(signature); - assert_eq!(tester.queue.requests().len(), 1); + assert_eq!(tester.signer.requests().len(), 1); // when let request = r#"{ @@ -202,7 +202,24 @@ fn should_confirm_transaction_and_dispatch() { // then assert_eq!(tester.io.handle_request_sync(&request), Some(response.to_owned())); - assert_eq!(tester.queue.requests().len(), 0); + assert_eq!(tester.signer.requests().len(), 0); assert_eq!(tester.miner.imported_transactions.lock().len(), 1); } +#[test] +fn should_generate_new_token() { + // given + let tester = signer_tester(); + + // when + let request = r#"{ + "jsonrpc":"2.0", + "method":"personal_generateAuthorizationToken", + "params":[], + "id":1 + }"#; + let response = r#"{"jsonrpc":"2.0","result":"new_token","id":1}"#; + + // then + assert_eq!(tester.io.handle_request_sync(&request), Some(response.to_owned())); +} diff --git a/rpc/src/v1/traits/personal.rs b/rpc/src/v1/traits/personal.rs index 89d63c863..988091958 100644 --- a/rpc/src/v1/traits/personal.rs +++ b/rpc/src/v1/traits/personal.rs @@ -92,12 +92,16 @@ pub trait PersonalSigner: Sized + Send + Sync + 'static { /// Reject the confirmation request. fn reject_request(&self, _: Params) -> Result; + /// Generates new authorization token. + fn generate_token(&self, _: Params) -> Result; + /// Should be used to convert object to io delegate. fn to_delegate(self) -> IoDelegate { let mut delegate = IoDelegate::new(Arc::new(self)); delegate.add_method("personal_requestsToConfirm", PersonalSigner::requests_to_confirm); delegate.add_method("personal_confirmRequest", PersonalSigner::confirm_request); delegate.add_method("personal_rejectRequest", PersonalSigner::reject_request); + delegate.add_method("personal_generateAuthorizationToken", PersonalSigner::generate_token); delegate } } diff --git a/signer/src/authcode_store.rs b/signer/src/authcode_store.rs index 7b9ff1d6b..d8068fc88 100644 --- a/signer/src/authcode_store.rs +++ b/signer/src/authcode_store.rs @@ -48,6 +48,7 @@ impl TimeProvider for DefaultTimeProvider { /// No of seconds the hash is valid const TIME_THRESHOLD: u64 = 7; const TOKEN_LENGTH: usize = 16; +const INITIAL_TOKEN: &'static str = "initial"; /// Manages authorization codes for `SignerUIs` pub struct AuthCodes { @@ -98,7 +99,7 @@ impl AuthCodes { } /// Checks if given hash is correct identifier of `SignerUI` - pub fn is_valid(&self, hash: &H256, time: u64) -> bool { + pub fn is_valid(&mut self, hash: &H256, time: u64) -> bool { let now = self.now.now(); // check time if time >= now + TIME_THRESHOLD || time <= now - TIME_THRESHOLD { @@ -106,9 +107,21 @@ impl AuthCodes { return false; } + let as_token = |code| format!("{}:{}", code, time).sha3(); + + // Check if it's the initial token. + if self.is_empty() { + let initial = &as_token(INITIAL_TOKEN) == hash; + // Initial token can be used only once. + if initial { + let _ = self.generate_new(); + } + return initial; + } + // look for code self.codes.iter() - .any(|code| &format!("{}:{}", code, time).sha3() == hash) + .any(|code| &as_token(code) == hash) } /// Generates and returns a new code that can be used by `SignerUIs` @@ -124,6 +137,11 @@ impl AuthCodes { self.codes.push(code); Ok(readable_code) } + + /// Returns true if there are no tokens in this store + pub fn is_empty(&self) -> bool { + self.codes.is_empty() + } } @@ -137,12 +155,28 @@ mod tests { format!("{}:{}", val, time).sha3() } + #[test] + fn should_return_true_if_code_is_initial_and_store_is_empty() { + // given + let code = "initial"; + let time = 99; + let mut codes = AuthCodes::new(vec![], || 100); + + // when + let res1 = codes.is_valid(&generate_hash(code, time), time); + let res2 = codes.is_valid(&generate_hash(code, time), time); + + // then + assert_eq!(res1, true); + assert_eq!(res2, false); + } + #[test] fn should_return_true_if_hash_is_valid() { // given let code = "23521352asdfasdfadf"; let time = 99; - let codes = AuthCodes::new(vec![code.into()], || 100); + let mut codes = AuthCodes::new(vec![code.into()], || 100); // when let res = codes.is_valid(&generate_hash(code, time), time); @@ -156,7 +190,7 @@ mod tests { // given let code = "23521352asdfasdfadf"; let time = 99; - let codes = AuthCodes::new(vec!["1".into()], || 100); + let mut codes = AuthCodes::new(vec!["1".into()], || 100); // when let res = codes.is_valid(&generate_hash(code, time), time); @@ -171,7 +205,7 @@ mod tests { let code = "23521352asdfasdfadf"; let time = 107; let time2 = 93; - let codes = AuthCodes::new(vec![code.into()], || 100); + let mut codes = AuthCodes::new(vec![code.into()], || 100); // when let res1 = codes.is_valid(&generate_hash(code, time), time); diff --git a/signer/src/tests/mod.rs b/signer/src/tests/mod.rs index eaed49de8..61b2ff1d3 100644 --- a/signer/src/tests/mod.rs +++ b/signer/src/tests/mod.rs @@ -14,24 +14,48 @@ // You should have received a copy of the GNU General Public License // along with Parity. If not, see . -use std::env; +use std::ops::{Deref, DerefMut}; use std::thread; -use std::time::Duration; +use std::time::{self, Duration}; use std::sync::Arc; -use devtools::http_client; +use devtools::{http_client, RandomTempPath}; use rpc::ConfirmationsQueue; +use util::Hashable; use rand; use ServerBuilder; use Server; +use AuthCodes; -pub fn serve() -> Server { +pub struct GuardedAuthCodes { + authcodes: AuthCodes, + path: RandomTempPath, +} +impl Deref for GuardedAuthCodes { + type Target = AuthCodes; + fn deref(&self) -> &Self::Target { + &self.authcodes + } +} +impl DerefMut for GuardedAuthCodes { + fn deref_mut(&mut self) -> &mut AuthCodes { + &mut self.authcodes + } +} + +pub fn serve() -> (Server, usize, GuardedAuthCodes) { + let mut path = RandomTempPath::new(); + path.panic_on_drop_failure = false; let queue = Arc::new(ConfirmationsQueue::default()); - let builder = ServerBuilder::new(queue, env::temp_dir()); + let builder = ServerBuilder::new(queue, path.to_path_buf()); let port = 35000 + rand::random::() % 10000; let res = builder.start(format!("127.0.0.1:{}", port).parse().unwrap()).unwrap(); thread::sleep(Duration::from_millis(25)); - res + + (res, port, GuardedAuthCodes { + authcodes: AuthCodes::from_file(&path).unwrap(), + path: path, + }) } pub fn request(server: Server, request: &str) -> http_client::Response { @@ -41,7 +65,7 @@ pub fn request(server: Server, request: &str) -> http_client::Response { #[test] fn should_reject_invalid_host() { // given - let server = serve(); + let server = serve().0; // when let response = request(server, @@ -62,7 +86,7 @@ fn should_reject_invalid_host() { #[test] fn should_serve_styles_even_on_disallowed_domain() { // given - let server = serve(); + let server = serve().0; // when let response = request(server, @@ -79,3 +103,103 @@ fn should_serve_styles_even_on_disallowed_domain() { assert_eq!(response.status, "HTTP/1.1 200 OK".to_owned()); } +#[test] +fn should_block_if_authorization_is_incorrect() { + // given + let (server, port, _) = serve(); + + // when + let response = request(server, + &format!("\ + GET / HTTP/1.1\r\n\ + Host: 127.0.0.1:{}\r\n\ + Connection: Upgrade\r\n\ + Sec-WebSocket-Key: x3JJHMbDL1EzLkh9GBhXDw==\r\n\ + Sec-WebSocket-Protocol: wrong\r\n\ + Sec-WebSocket-Version: 13\r\n\ + \r\n\ + {{}} + ", port) + ); + + // then + assert_eq!(response.status, "HTTP/1.1 403 FORBIDDEN".to_owned()); +} + +#[test] +fn should_allow_if_authorization_is_correct() { + // given + let (server, port, mut authcodes) = serve(); + let code = authcodes.generate_new().unwrap().replace("-", ""); + authcodes.to_file(&authcodes.path).unwrap(); + let timestamp = time::UNIX_EPOCH.elapsed().unwrap().as_secs(); + + // when + let response = request(server, + &format!("\ + GET / HTTP/1.1\r\n\ + Host: 127.0.0.1:{}\r\n\ + Connection: Close\r\n\ + Sec-WebSocket-Key: x3JJHMbDL1EzLkh9GBhXDw==\r\n\ + Sec-WebSocket-Protocol: {:?}_{}\r\n\ + Sec-WebSocket-Version: 13\r\n\ + \r\n\ + {{}} + ", + port, + format!("{}:{}", code, timestamp).sha3(), + timestamp, + ) + ); + + // then + assert_eq!(response.status, "HTTP/1.1 101 Switching Protocols".to_owned()); +} + +#[test] +fn should_allow_initial_connection_but_only_once() { + // given + let (server, port, authcodes) = serve(); + let code = "initial"; + let timestamp = time::UNIX_EPOCH.elapsed().unwrap().as_secs(); + assert!(authcodes.is_empty()); + + // when + let response1 = http_client::request(server.addr(), + &format!("\ + GET / HTTP/1.1\r\n\ + Host: 127.0.0.1:{}\r\n\ + Connection: Close\r\n\ + Sec-WebSocket-Key: x3JJHMbDL1EzLkh9GBhXDw==\r\n\ + Sec-WebSocket-Protocol:{:?}_{}\r\n\ + Sec-WebSocket-Version: 13\r\n\ + \r\n\ + {{}} + ", + port, + format!("{}:{}", code, timestamp).sha3(), + timestamp, + ) + ); + let response2 = http_client::request(server.addr(), + &format!("\ + GET / HTTP/1.1\r\n\ + Host: 127.0.0.1:{}\r\n\ + Connection: Close\r\n\ + Sec-WebSocket-Key: x3JJHMbDL1EzLkh9GBhXDw==\r\n\ + Sec-WebSocket-Protocol:{:?}_{}\r\n\ + Sec-WebSocket-Version: 13\r\n\ + \r\n\ + {{}} + ", + port, + format!("{}:{}", code, timestamp).sha3(), + timestamp, + ) + ); + + + // then + assert_eq!(response1.status, "HTTP/1.1 101 Switching Protocols".to_owned()); + assert_eq!(response2.status, "HTTP/1.1 403 FORBIDDEN".to_owned()); +} diff --git a/signer/src/ws_server/mod.rs b/signer/src/ws_server/mod.rs index 57223ccd9..697fbd4c7 100644 --- a/signer/src/ws_server/mod.rs +++ b/signer/src/ws_server/mod.rs @@ -180,7 +180,6 @@ impl Drop for Server { self.queue.finish(); self.broadcaster_handle.take().unwrap().join().unwrap(); self.handle.take().unwrap().join().unwrap(); - } } diff --git a/signer/src/ws_server/session.rs b/signer/src/ws_server/session.rs index cd3e2eee3..afc6606d7 100644 --- a/signer/src/ws_server/session.rs +++ b/signer/src/ws_server/session.rs @@ -59,7 +59,7 @@ fn origin_is_allowed(self_origin: &str, header: Option<&[u8]>) -> bool { } } -fn auth_is_valid(codes: &Path, protocols: ws::Result>) -> bool { +fn auth_is_valid(codes_path: &Path, protocols: ws::Result>) -> bool { match protocols { Ok(ref protocols) if protocols.len() == 1 => { protocols.iter().any(|protocol| { @@ -69,8 +69,15 @@ fn auth_is_valid(codes: &Path, protocols: ws::Result>) -> bool { if let (Some(auth), Some(time)) = (auth, time) { // Check if the code is valid - AuthCodes::from_file(codes) - .map(|codes| codes.is_valid(&auth, time)) + AuthCodes::from_file(codes_path) + .map(|mut codes| { + let res = codes.is_valid(&auth, time); + // make sure to save back authcodes - it might have been modified + if let Err(_) = codes.to_file(codes_path) { + warn!(target: "signer", "Couldn't save authorization codes to file."); + } + res + }) .unwrap_or(false) } else { false