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

@@ -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<P: AsRef<Path>>(path: P) -> Result<Config, Error> {
+	pub fn load<P: AsRef<Path>>(path: P, allow_insecure_rpc_endpoints: bool) -> Result<Config, Error> {
 		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<Config, Error> {
+	fn load_from_str(s: &str, allow_insecure_rpc_endpoints: bool) -> Result<Config, Error> {
 		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<Config, Error> {
+	fn from_load_struct(config: load::Config, allow_insecure_rpc_endpoints: bool) -> Result<Config, Error> {
 		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<Node, Error> {
+	fn from_load_struct(node: load::Node, allow_insecure_rpc_endpoints: bool) -> Result<Node, Error> {
 		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(),

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)
+		}
 	}
 }
 

cli/src/main.rs

@@ -71,17 +71,19 @@ POA-Ethereum bridge.
     Copyright 2018 POA Networks Ltd.
 
 Usage:
-    bridge --config <config> --database <database>
+    bridge [options] --config <config> --database <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<S, I>(command: I, running: Arc<AtomicBool>) -> Result<String, UserFac
 		.and_then(|d| d.argv(command).deserialize()).map_err(|e| e.to_string())?;
 
 	info!(target: "bridge", "Loading config");
-	let config = Config::load(args.arg_config)?;
+	let config = Config::load(args.arg_config, args.flag_allow_insecure_rpc_endpoints)?;
 
 	info!(target: "bridge", "Starting event loop");
 	let mut event_loop = Core::new().unwrap();

integration-tests/tests/basic_deposit_then_withdraw.rs

@@ -163,6 +163,7 @@ fn test_basic_deposit_then_withdraw() {
 		.env("RUST_LOG", "info")
 		.arg("--config").arg("bridge_config.toml")
 		.arg("--database").arg("tmp/bridge1_db.txt")
+		.arg("--allow-insecure-rpc-endpoints")
 		.spawn()
 		.expect("failed to spawn bridge process");
 

integration-tests/tests/insufficient_funds.rs

@@ -194,6 +194,7 @@ fn test_insufficient_funds() {
 		.env("RUST_LOG", "info")
 		.arg("--config").arg("bridge_config_gas_price.toml")
 		.arg("--database").arg("tmp/bridge1_db.txt")
+		.arg("--allow-insecure-rpc-endpoints")
 		.spawn()
 		.expect("failed to spawn bridge process");