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

README.md

@@ -54,6 +54,11 @@ bridge --config config.toml --database db.toml
 - `--config` - location of the configuration file. configuration file must exist
 - `--database` - location of the database file.
 
+Bridge forces TLS for RPC connections by default. However, in some limited scenarios (like local testing),
+this might be undesirable. In this case, you can use `--allow-insecure-rpc-endpoints` option to allow non-TLS
+endpoints to be used. Ensure, however, that this option is not going to be used in production.
+
+
 #### Exit Status Codes
 
 | Code | Meaning              |

bridge/src/config.rs

@@ -8,7 +8,7 @@ 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;
@@ -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 {
 				#[cfg(feature = "deploy")]
 				accounts: config.authorities.accounts,
@@ -106,7 +106,7 @@ 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 {
@@ -122,6 +122,16 @@ impl Node {
 		let default_gas_price = node.default_gas_price.unwrap_or(DEFAULT_GAS_PRICE_WEI);
 		let concurrent_http_requests = node.concurrent_http_requests.unwrap_or(DEFAULT_CONCURRENCY);
 
+		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")]
@@ -136,7 +146,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(),
@@ -398,7 +408,7 @@ required_signatures = 2
 			keystore: "/keys/".into(),
 		};
 
-		let config = Config::load_from_str(toml).unwrap();
+		let config = Config::load_from_str(toml, true).unwrap();
 		assert_eq!(expected, config);
 	}
 
@@ -461,7 +471,7 @@ required_signatures = 2
 			keystore: "/keys/".into(),
 		};
 
-		let config = Config::load_from_str(toml).unwrap();
+		let config = Config::load_from_str(toml, true).unwrap();
 		assert_eq!(expected, config);
 	}
 }

bridge/src/error.rs

@@ -61,6 +61,10 @@ error_chain! {
 		OtherError(error: String) {
 		    description("other error")
 		    display("{}", error)
+        }
+		ConfigError(err: String) {
+		    description("config error")
+		    display("{}", err)
 		}
 	}
 }

cli/src/main.rs

@@ -73,13 +73,14 @@ POA-Ethereum bridge.
     Copyright 2018 POA Networks Ltd.
 
 Usage:
-    bridge --config <config> --database <database>
+    bridge [options] --config <config> --database <database>
     bridge -h | --help
     bridge -v | --version
 
 Options:
-    -h, --help           Display help message and exit.
-    -v, --version        Print version and exit.
+    -h, --help                        Display help message and exit.
+    -v, --version                     Print version and exit.
+    --allow-insecure-rpc-endpoints    Allow non-HTTPS endpoints
 "#;
 
 #[derive(Debug, Deserialize)]
@@ -87,6 +88,7 @@ pub struct Args {
 	arg_config: PathBuf,
 	arg_database: PathBuf,
 	flag_version: bool,
+	flag_allow_insecure_rpc_endpoints: bool,
 }
 
 use std::sync::atomic::{AtomicBool, Ordering};
@@ -128,7 +130,7 @@ fn execute<S, I>(command: I, running: Arc<AtomicBool>) -> Result<String, UserFac
 	}
 
 	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");