From a35feea030628ae46d6312f036cacb7b6fe27a73 Mon Sep 17 00:00:00 2001 From: Yurii Rashkovskii Date: Wed, 23 May 2018 23:03:20 -0700 Subject: [PATCH] Problem: insecure RPCs are subject to MITM attacks Solution: by default, disallow use of non-TLS RPC endpoints For testing, there's an escape hatch of a command line argument `--allow-insecure-rpc-endpoints` (purposefully long) that will reduce the severity of using a non-TLS RPC endpoint to a warning in a log file. It was not made to be a configuration file option to reduce the risk of this option slipping into a production configuration file by mistake. Closes #79 --- bridge/src/config.rs | 34 ++++++++++++------- bridge/src/error.rs | 4 +++ cli/src/main.rs | 8 +++-- .../tests/basic_deposit_then_withdraw.rs | 1 + integration-tests/tests/insufficient_funds.rs | 1 + 5 files changed, 33 insertions(+), 15 deletions(-) diff --git a/bridge/src/config.rs b/bridge/src/config.rs index 2a845fe..acf92dc 100644 --- a/bridge/src/config.rs +++ b/bridge/src/config.rs @@ -8,13 +8,13 @@ use rustc_hex::FromHex; use web3::types::Address; #[cfg(feature = "deploy")] use web3::types::Bytes; -use error::{ResultExt, Error}; +use error::{ResultExt, Error, ErrorKind}; use {toml}; const DEFAULT_POLL_INTERVAL: u64 = 1; const DEFAULT_CONFIRMATIONS: usize = 12; const DEFAULT_TIMEOUT: u64 = 3600; -const DEFAULT_RPC_PORT: u16 = 8545; +const DEFAULT_RPC_PORT: u16 = 443; const DEFAULT_CONCURRENCY: usize = 100; const DEFAULT_GAS_PRICE_SPEED: GasPriceSpeed = GasPriceSpeed::Fast; const DEFAULT_GAS_PRICE_TIMEOUT_SECS: u64 = 10; @@ -33,22 +33,22 @@ pub struct Config { } impl Config { - pub fn load>(path: P) -> Result { + pub fn load>(path: P, allow_insecure_rpc_endpoints: bool) -> Result { let mut file = fs::File::open(path).chain_err(|| "Cannot open config")?; let mut buffer = String::new(); file.read_to_string(&mut buffer).expect("TODO"); - Self::load_from_str(&buffer) + Self::load_from_str(&buffer, allow_insecure_rpc_endpoints) } - fn load_from_str(s: &str) -> Result { + fn load_from_str(s: &str, allow_insecure_rpc_endpoints: bool) -> Result { let config: load::Config = toml::from_str(s).chain_err(|| "Cannot parse config")?; - Config::from_load_struct(config) + Config::from_load_struct(config, allow_insecure_rpc_endpoints) } - fn from_load_struct(config: load::Config) -> Result { + fn from_load_struct(config: load::Config, allow_insecure_rpc_endpoints: bool) -> Result { let result = Config { - home: Node::from_load_struct(config.home)?, - foreign: Node::from_load_struct(config.foreign)?, + home: Node::from_load_struct(config.home, allow_insecure_rpc_endpoints)?, + foreign: Node::from_load_struct(config.foreign, allow_insecure_rpc_endpoints)?, authorities: Authorities { accounts: config.authorities.accounts, required_signatures: config.authorities.required_signatures, @@ -104,9 +104,9 @@ impl PartialEq for NodeInfo { } impl Node { - fn from_load_struct(node: load::Node) -> Result { + fn from_load_struct(node: load::Node, allow_insecure_rpc_endpoints: bool) -> Result { let gas_price_oracle_url = node.gas_price_oracle_url.clone(); - + let gas_price_speed = match node.gas_price_speed { Some(ref s) => GasPriceSpeed::from_str(s).unwrap(), None => DEFAULT_GAS_PRICE_SPEED @@ -119,6 +119,16 @@ impl Node { let default_gas_price = node.default_gas_price.unwrap_or(DEFAULT_GAS_PRICE_WEI); + let rpc_host = node.rpc_host.unwrap(); + + if !rpc_host.starts_with("https://") { + if !allow_insecure_rpc_endpoints { + return Err(ErrorKind::ConfigError(format!("RPC endpoints must use TLS, {} doesn't", rpc_host)).into()); + } else { + warn!("RPC endpoints must use TLS, {} doesn't", rpc_host); + } + } + let result = Node { account: node.account, #[cfg(feature = "deploy")] @@ -133,7 +143,7 @@ impl Node { request_timeout: Duration::from_secs(node.request_timeout.unwrap_or(DEFAULT_TIMEOUT)), poll_interval: Duration::from_secs(node.poll_interval.unwrap_or(DEFAULT_POLL_INTERVAL)), required_confirmations: node.required_confirmations.unwrap_or(DEFAULT_CONFIRMATIONS), - rpc_host: node.rpc_host.unwrap(), + rpc_host, rpc_port: node.rpc_port.unwrap_or(DEFAULT_RPC_PORT), password: node.password, info: Default::default(), diff --git a/bridge/src/error.rs b/bridge/src/error.rs index 6413872..d77c22d 100644 --- a/bridge/src/error.rs +++ b/bridge/src/error.rs @@ -58,6 +58,10 @@ error_chain! { description("contextualized error") display("{:?} in {}", err, context) } + ConfigError(err: String) { + description("config error") + display("{}", err) + } } } diff --git a/cli/src/main.rs b/cli/src/main.rs index 38d48d6..91dffbf 100644 --- a/cli/src/main.rs +++ b/cli/src/main.rs @@ -71,17 +71,19 @@ POA-Ethereum bridge. Copyright 2018 POA Networks Ltd. Usage: - bridge --config --database + bridge [options] --config --database bridge -h | --help Options: - -h, --help Display help message and exit. + -h, --help Display help message and exit. + --allow-insecure-rpc-endpoints Allow non-HTTPS endpoints "#; #[derive(Debug, Deserialize)] pub struct Args { arg_config: PathBuf, arg_database: PathBuf, + flag_allow_insecure_rpc_endpoints: bool, } use std::sync::atomic::{AtomicBool, Ordering}; @@ -119,7 +121,7 @@ fn execute(command: I, running: Arc) -> Result