From 80a83c95d3168c8af18e492f79657fd69558821a Mon Sep 17 00:00:00 2001 From: Seun LanLege Date: Thu, 5 Dec 2019 09:38:14 +0100 Subject: [PATCH] only add transactions to signing-queue if it is enabled (#11272) * only add transactions to signing-queue if it is enabled * Update rpc/src/v1/helpers/errors.rs Co-Authored-By: David * use errors::codes::ACCOUNT_LOCKED * bail early if account isn't unlocked * use errors::signing * Update rpc/src/v1/helpers/errors.rs * Update rpc/src/v1/helpers/errors.rs * test * adds cli flag to enable signing queue. * use helper method siginig_queue_disabled instead of accounts::SignError * fix typo, use raw i64 * fixed tests --- parity/cli/mod.rs | 7 +++ parity/configuration.rs | 4 +- parity/rpc.rs | 2 +- rpc/src/v1/helpers/errors.rs | 10 ++++ rpc/src/v1/impls/signing.rs | 10 +++- rpc/src/v1/tests/mocked/signing.rs | 80 +++++++++++++++++++++++------- rpc/src/v1/types/confirmations.rs | 12 +++++ 7 files changed, 104 insertions(+), 21 deletions(-) diff --git a/parity/cli/mod.rs b/parity/cli/mod.rs index 4a9a999ca..d535efec8 100644 --- a/parity/cli/mod.rs +++ b/parity/cli/mod.rs @@ -348,6 +348,10 @@ usage! { "--unlock=[ACCOUNTS]", "Unlock ACCOUNTS for the duration of the execution. ACCOUNTS is a comma-delimited list of addresses.", + ARG arg_enable_signing_queue: (bool) = false, or |c: &Config| c.account.as_ref()?.enable_signing_queue, + "--enable-signing-queue=[BOOLEAN]", + "Enables the signing queue for external transaction signing either via CLI or personal_unlockAccount, turned off by default.", + ARG arg_password: (Vec) = Vec::new(), or |c: &Config| c.account.as_ref()?.password.clone(), "--password=[FILE]...", "Provide a file containing a password for unlocking an account. Leading and trailing whitespace is trimmed.", @@ -1194,6 +1198,7 @@ struct Operating { #[serde(deny_unknown_fields)] struct Account { unlock: Option>, + enable_signing_queue: Option, password: Option>, keys_iterations: Option, refresh_time: Option, @@ -1728,6 +1733,7 @@ mod tests { arg_restore_file: None, arg_tools_hash_file: None, + arg_enable_signing_queue: false, arg_signer_sign_id: None, arg_signer_reject_id: None, arg_dapp_path: None, @@ -2045,6 +2051,7 @@ mod tests { _legacy_public_node: None, }), account: Some(Account { + enable_signing_queue: None, unlock: Some(vec!["0x1".into(), "0x2".into(), "0x3".into()]), password: Some(vec!["passwdfile path".into()]), keys_iterations: None, diff --git a/parity/configuration.rs b/parity/configuration.rs index c6345fda0..8761cec88 100644 --- a/parity/configuration.rs +++ b/parity/configuration.rs @@ -898,7 +898,7 @@ impl Configuration { fn ws_config(&self) -> Result { let support_token_api = // enabled when not unlocking - self.args.arg_unlock.is_none(); + self.args.arg_unlock.is_none() && self.args.arg_enable_signing_queue; let conf = WsConfiguration { enabled: self.ws_enabled(), @@ -1388,7 +1388,7 @@ mod tests { origins: Some(vec!["parity://*".into(),"chrome-extension://*".into(), "moz-extension://*".into()]), hosts: Some(vec![]), signer_path: expected.into(), - support_token_api: true, + support_token_api: false, max_connections: 100, }, LogConfig { color: !cfg!(windows), diff --git a/parity/rpc.rs b/parity/rpc.rs index 0e1667d5b..4906365f3 100644 --- a/parity/rpc.rs +++ b/parity/rpc.rs @@ -119,7 +119,7 @@ impl Default for WsConfiguration { origins: Some(vec!["parity://*".into(),"chrome-extension://*".into(), "moz-extension://*".into()]), hosts: Some(Vec::new()), signer_path: replace_home(&data_dir, "$BASE/signer").into(), - support_token_api: true, + support_token_api: false, } } } diff --git a/rpc/src/v1/helpers/errors.rs b/rpc/src/v1/helpers/errors.rs index 791d7e3b9..e4e4f96ee 100644 --- a/rpc/src/v1/helpers/errors.rs +++ b/rpc/src/v1/helpers/errors.rs @@ -380,6 +380,16 @@ pub fn invalid_call_data(error: T) -> Error { } } +pub fn signing_queue_disabled() -> Error { + Error { + code: ErrorCode::ServerError(-32020), + message: "Your account is locked and the signing queue is disabled. \ + You can either Unlock the account via CLI, personal_unlockAccount or \ + enable the signing queue to use Trusted Signer.".into(), + data: None, + } +} + #[cfg(any(test, feature = "accounts"))] pub fn signing(error: ::accounts::SignError) -> Error { Error { diff --git a/rpc/src/v1/impls/signing.rs b/rpc/src/v1/impls/signing.rs index dc44e5bd4..be6e0ecb4 100644 --- a/rpc/src/v1/impls/signing.rs +++ b/rpc/src/v1/impls/signing.rs @@ -114,6 +114,12 @@ impl SigningQueueClient { fn dispatch(&self, payload: RpcConfirmationPayload, origin: Origin) -> BoxFuture { let default_account = self.accounts.default_account(); + let from = &payload.sender().unwrap_or(&default_account); + // bail early if the account isn't unlocked + if !self.accounts.is_unlocked(from) && !self.signer.is_enabled() { + return Box::new(future::done(Err(errors::signing_queue_disabled()))) + } + let accounts = self.accounts.clone(); let dispatcher = self.dispatcher.clone(); let signer = self.signer.clone(); @@ -127,7 +133,9 @@ impl SigningQueueClient { } else { Either::B(future::done( signer.add_request(payload, origin) - .map(|(id, future)| DispatchResult::Future(id, future)) + .map(|(id, future)| { + DispatchResult::Future(id, future) + }) .map_err(|_| errors::request_rejected_limit()) )) } diff --git a/rpc/src/v1/tests/mocked/signing.rs b/rpc/src/v1/tests/mocked/signing.rs index 184d3d8b6..21d0d77fe 100644 --- a/rpc/src/v1/tests/mocked/signing.rs +++ b/rpc/src/v1/tests/mocked/signing.rs @@ -50,10 +50,10 @@ struct SigningTester { pub io: IoHandler, } -impl Default for SigningTester { - fn default() -> Self { +impl SigningTester { + fn new(signing_queue_enabled: bool) ->Self { let runtime = Runtime::with_thread_count(1); - let signer = Arc::new(SignerService::new_test(false)); + let signer = Arc::new(SignerService::new_test(signing_queue_enabled)); let client = Arc::new(TestBlockChainClient::default()); let miner = Arc::new(TestMinerService::default()); let accounts = Arc::new(AccountProvider::transient_provider()); @@ -81,15 +81,15 @@ impl Default for SigningTester { } } -fn eth_signing() -> SigningTester { - SigningTester::default() +fn eth_signing(signing_queue_enabled: bool) -> SigningTester { + SigningTester::new(signing_queue_enabled) } #[test] fn rpc_eth_sign() { use rustc_hex::FromHex; - let tester = eth_signing(); + let tester = eth_signing(true); let account = tester.accounts.insert_account(Secret::from([69u8; 32]), &"abcd".into()).unwrap(); tester.accounts.unlock_account_permanently(account, "abcd".into()).unwrap(); @@ -112,7 +112,7 @@ fn rpc_eth_sign() { #[test] fn should_add_sign_to_queue() { // given - let tester = eth_signing(); + let tester = eth_signing(true); let address = Address::random(); assert_eq!(tester.signer.requests().len(), 0); @@ -150,7 +150,7 @@ fn should_add_sign_to_queue() { #[test] fn should_post_sign_to_queue() { // given - let tester = eth_signing(); + let tester = eth_signing(true); let address = Address::random(); assert_eq!(tester.signer.requests().len(), 0); @@ -174,7 +174,7 @@ fn should_post_sign_to_queue() { #[test] fn should_check_status_of_request() { // given - let tester = eth_signing(); + let tester = eth_signing(true); let address = Address::random(); let request = r#"{ "jsonrpc": "2.0", @@ -203,7 +203,7 @@ fn should_check_status_of_request() { #[test] fn should_check_status_of_request_when_its_resolved() { // given - let tester = eth_signing(); + let tester = eth_signing(true); let address = Address::random(); let request = r#"{ "jsonrpc": "2.0", @@ -234,10 +234,56 @@ fn should_check_status_of_request_when_its_resolved() { assert_eq!(tester.io.handle_request_sync(&request), Some(response.to_owned())); } +#[test] +fn eth_sign_locked_account() { + let secret = "8a283037bb19c4fed7b1c569e40c7dcff366165eb869110a1b11532963eb9cb2".parse().unwrap(); + let tester = eth_signing(false); + let address = tester.accounts.insert_account(secret, &"".into()).unwrap(); + + let req_send_trans = r#"{ + "jsonrpc": "2.0", + "method": "eth_sendTransaction", + "params": [{ + "from": ""#.to_owned() + format!("0x{:x}", address).as_ref() + r#"", + "to": "0xd46e8dd67c5d32be8058bb8eb970870f07244567", + "gas": "0x30000", + "gasPrice": "0x1", + "value": "0x9184e72a" + }], + "id": 16 + }"#; + + // expected error response + let error_res = r#"{ + "jsonrpc":"2.0", + "error": + { + "code":-32020, + "message":"Your account is locked and the signing queue is disabled. + You can either Unlock the account via CLI, personal_unlockAccount or + enable the signing queue to use Trusted Signer." + }, + "id":16 + }"#; + // dispatch the transaction, without unlocking the account. + assert_eq!( + error_res.replace("\t", "").replace("\n", ""), + tester.io.handle_request_sync(&req_send_trans).unwrap() + ); + // unlock the account + tester.accounts.unlock_account_permanently(address, "".into()).unwrap(); + + // try again, this time account is unlocked. + assert_eq!( + r#"{"jsonrpc":"2.0","result":"0x70df76392fba654351714803d99a90be1d58d165352e0008e376427284314c88","id":16}"#, + tester.io.handle_request_sync(&req_send_trans).unwrap() + ); +} + #[test] fn should_sign_if_account_is_unlocked() { // given - let tester = eth_signing(); + let tester = eth_signing(true); let data = vec![5u8]; let acc = tester.accounts.insert_account(Secret::from([69u8; 32]), &"test".into()).unwrap(); tester.accounts.unlock_account_permanently(acc, "test".into()).unwrap(); @@ -260,7 +306,7 @@ fn should_sign_if_account_is_unlocked() { #[test] fn should_add_transaction_to_queue() { // given - let tester = eth_signing(); + let tester = eth_signing(true); let address = Address::random(); assert_eq!(tester.signer.requests().len(), 0); @@ -301,7 +347,7 @@ fn should_add_transaction_to_queue() { #[test] fn should_add_sign_transaction_to_the_queue() { // given - let tester = eth_signing(); + let tester = eth_signing(true); let address = tester.accounts.new_account(&"test".into()).unwrap(); assert_eq!(tester.signer.requests().len(), 0); @@ -380,7 +426,7 @@ fn should_add_sign_transaction_to_the_queue() { #[test] fn should_dispatch_transaction_if_account_is_unlock() { // given - let tester = eth_signing(); + let tester = eth_signing(true); let acc = tester.accounts.new_account(&"test".into()).unwrap(); tester.accounts.unlock_account_permanently(acc, "test".into()).unwrap(); @@ -417,7 +463,7 @@ fn should_dispatch_transaction_if_account_is_unlock() { #[test] fn should_decrypt_message_if_account_is_unlocked() { // given - let mut tester = eth_signing(); + let mut tester = eth_signing(true); let parity = parity::Dependencies::new(); tester.io.extend_with(parity.client(None).to_delegate()); let (address, public) = tester.accounts.new_account_and_public(&"test".into()).unwrap(); @@ -449,7 +495,7 @@ fn should_decrypt_message_if_account_is_unlocked() { #[test] fn should_add_decryption_to_the_queue() { // given - let tester = eth_signing(); + let tester = eth_signing(true); let acc = Random.generate().unwrap(); assert_eq!(tester.signer.requests().len(), 0); @@ -486,7 +532,7 @@ fn should_add_decryption_to_the_queue() { #[test] fn should_compose_transaction() { // given - let tester = eth_signing(); + let tester = eth_signing(true); let acc = Random.generate().unwrap(); assert_eq!(tester.signer.requests().len(), 0); let from = format!("{:x}", acc.address()); diff --git a/rpc/src/v1/types/confirmations.rs b/rpc/src/v1/types/confirmations.rs index de54d0dec..e3d7d2665 100644 --- a/rpc/src/v1/types/confirmations.rs +++ b/rpc/src/v1/types/confirmations.rs @@ -229,6 +229,18 @@ impl From for ConfirmationPayload { } } +impl ConfirmationPayload { + pub fn sender(&self) -> Option<&H160> { + match *self { + ConfirmationPayload::SendTransaction(ref request) => request.from.as_ref(), + ConfirmationPayload::SignTransaction(ref request) => request.from.as_ref(), + ConfirmationPayload::EthSignMessage(ref request) => Some(&request.address), + ConfirmationPayload::EIP191SignMessage(ref request) => Some(&request.address), + ConfirmationPayload::Decrypt(ref request) => Some(&request.address), + } + } +} + /// Possible modifications to the confirmed transaction sent by `Trusted Signer` #[derive(Debug, PartialEq, Serialize, Deserialize)] #[serde(deny_unknown_fields)]