Merge pull request #6168 from paritytech/secretstore_stresstest

SecretStore: bunch of fixes and improvements

parity/cli/config.full.toml

@@ -76,8 +76,10 @@ path = "$HOME/.parity/dapps"
 user = "test_user"
 pass = "test_pass"
 
-[secretstore]
+[secretstore]	
 disable = false
+disable_http = false
+disable_acl_check = false
 nodes = []
 http_interface = "local"
 http_port = 8082

parity/cli/mod.rs

@@ -217,6 +217,10 @@ usage! {
 		// Secret Store
 		flag_no_secretstore: bool = false,
 			or |c: &Config| otry!(c.secretstore).disable.clone(),
+		flag_no_secretstore_http: bool = false,
+			or |c: &Config| otry!(c.secretstore).disable_http.clone(),
+		flag_no_secretstore_acl_check: bool = false,
+			or |c: &Config| otry!(c.secretstore).disable_acl_check.clone(),
 		flag_secretstore_secret: Option<String> = None,
 			or |c: &Config| otry!(c.secretstore).self_secret.clone().map(Some),
 		flag_secretstore_nodes: String = "",
@@ -520,6 +524,8 @@ struct Dapps {
 #[derive(Default, Debug, PartialEq, Deserialize)]
 struct SecretStore {
 	disable: Option<bool>,
+	disable_http: Option<bool>,
+	disable_acl_check: Option<bool>,
 	self_secret: Option<String>,
 	nodes: Option<Vec<String>>,
 	interface: Option<String>,
@@ -796,6 +802,8 @@ mod tests {
 			flag_no_dapps: false,
 
 			flag_no_secretstore: false,
+			flag_no_secretstore_http: false,
+			flag_no_secretstore_acl_check: false,
 			flag_secretstore_secret: None,
 			flag_secretstore_nodes: "".into(),
 			flag_secretstore_interface: "local".into(),
@@ -1031,6 +1039,8 @@ mod tests {
 			}),
 			secretstore: Some(SecretStore {
 				disable: None,
+				disable_http: None,
+				disable_acl_check: None,
 				self_secret: None,
 				nodes: None,
 				interface: None,

parity/cli/usage.txt

@@ -234,6 +234,8 @@ API and Console Options:
 
 Secret Store Options:
   --no-secretstore                 Disable Secret Store functionality. (default: {flag_no_secretstore})
+  --no-secretstore-http            Disable Secret Store HTTP API. (default: {flag_no_secretstore_http})
+  --no-acl-check                   Disable ACL check (useful for test environments). (default: {flag_no_secretstore_acl_check})
   --secretstore-secret SECRET      Hex-encoded secret key of this node.
                                    (required, default: {flag_secretstore_secret:?}).
   --secretstore-nodes NODES        Comma-separated list of other secret store cluster nodes in form

parity/configuration.rs

@@ -609,6 +609,8 @@ impl Configuration {
 	fn secretstore_config(&self) -> Result<SecretStoreConfiguration, String> {
 		Ok(SecretStoreConfiguration {
 			enabled: self.secretstore_enabled(),
+			http_enabled: self.secretstore_http_enabled(),
+			acl_check_enabled: self.secretstore_acl_check_enabled(),
 			self_secret: self.secretstore_self_secret()?,
 			nodes: self.secretstore_nodes()?,
 			interface: self.secretstore_interface(),
@@ -1071,6 +1073,14 @@ impl Configuration {
 		!self.args.flag_no_secretstore && cfg!(feature = "secretstore")
 	}
 
+	fn secretstore_http_enabled(&self) -> bool {
+		!self.args.flag_no_secretstore_http && cfg!(feature = "secretstore")
+	}
+
+	fn secretstore_acl_check_enabled(&self) -> bool {
+		!self.args.flag_no_secretstore_acl_check
+	}
+
 	fn ui_enabled(&self) -> bool {
 		if self.args.flag_force_ui {
 			return true;
@@ -1376,6 +1386,7 @@ mod tests {
 			whisper: Default::default(),
 		};
 		expected.secretstore_conf.enabled = cfg!(feature = "secretstore");
+		expected.secretstore_conf.http_enabled = cfg!(feature = "secretstore");
 		assert_eq!(conf.into_command().unwrap().cmd, Cmd::Run(expected));
 	}
 

parity/secretstore.rs

@@ -37,6 +37,10 @@ pub enum NodeSecretKey {
 pub struct Configuration {
 	/// Is secret store functionality enabled?
 	pub enabled: bool,
+	/// Is HTTP API enabled?
+	pub http_enabled: bool,
+	/// Is ACL check enabled.
+	pub acl_check_enabled: bool,
 	/// This node secret.
 	pub self_secret: Option<NodeSecretKey>,
 	/// Other nodes IDs + addresses.
@@ -83,6 +87,7 @@ mod server {
 	use std::sync::Arc;
 	use ethcore_secretstore;
 	use ethkey::KeyPair;
+	use ansi_term::Colour::Red;
 	use super::{Configuration, Dependencies, NodeSecretKey};
 
 	/// Key server
@@ -93,6 +98,10 @@ mod server {
 	impl KeyServer {
 		/// Create new key server
 		pub fn new(mut conf: Configuration, deps: Dependencies) -> Result<Self, String> {
+			if !conf.acl_check_enabled {
+				warn!("Running SecretStore with disabled ACL check: {}", Red.bold().paint("everyone has access to stored keys"));
+			}
+
 			let self_secret: Arc<ethcore_secretstore::NodeKeyPair> = match conf.self_secret.take() {
 				Some(NodeSecretKey::Plain(secret)) => Arc::new(ethcore_secretstore::PlainNodeKeyPair::new(
 					KeyPair::from_secret(secret).map_err(|e| format!("invalid secret: {}", e))?)),
@@ -117,12 +126,14 @@ mod server {
 				None => return Err("self secret is required when using secretstore".into()),
 			};
 
-			let mut conf = ethcore_secretstore::ServiceConfiguration {
-				listener_address: ethcore_secretstore::NodeAddress {
+			let key_server_name = format!("{}:{}", conf.interface, conf.port);
+			let mut cconf = ethcore_secretstore::ServiceConfiguration {
+				listener_address: if conf.http_enabled { Some(ethcore_secretstore::NodeAddress {
 					address: conf.http_interface.clone(),
 					port: conf.http_port,
-				},
+				}) } else { None },
 				data_path: conf.data_path.clone(),
+				acl_check_enabled: conf.acl_check_enabled,
 				cluster_config: ethcore_secretstore::ClusterConfiguration {
 					threads: 4,
 					listener_address: ethcore_secretstore::NodeAddress {
@@ -137,10 +148,10 @@ mod server {
 				},
 			};
 
-			conf.cluster_config.nodes.insert(self_secret.public().clone(), conf.cluster_config.listener_address.clone());
+			cconf.cluster_config.nodes.insert(self_secret.public().clone(), cconf.cluster_config.listener_address.clone());
 
-			let key_server = ethcore_secretstore::start(deps.client, self_secret, conf)
-				.map_err(Into::<String>::into)?;
+			let key_server = ethcore_secretstore::start(deps.client, self_secret, cconf)
+				.map_err(|e| format!("Error starting KeyServer {}: {}", key_server_name, e))?;
 
 			Ok(KeyServer {
 				_key_server: key_server,
@@ -156,6 +167,8 @@ impl Default for Configuration {
 		let data_dir = default_data_path();
 		Configuration {
 			enabled: true,
+			http_enabled: true,
+			acl_check_enabled: true,
 			self_secret: None,
 			nodes: BTreeMap::new(),
 			interface: "127.0.0.1".to_owned(),