Expand remote keystore interface to allow for hybrid mode (#7628)

* update to latest master

* updates on docs, license, meta

* hide ssrs behind feature flag

* implement remaining functions on the server

* sign server line length fix

* fix tests

* fixup in-memory-keystore

* adding failsafe

* skipping ecdsa test for now

* remote keystore param

* remote sign urls made available

* integrating keystore remotes features

* don't forget the dependency

* remove old cruft

* reset local keystore

* applying suggestions

* Switch to single remote, minor grumbles

* minor grumbles, docs

bin/node-template/node/Cargo.toml

@@ -22,6 +22,7 @@ sc-cli = { version = "0.8.0", path = "../../../client/cli", features = ["wasmtim
 sp-core = { version = "2.0.0", path = "../../../primitives/core" }
 sc-executor = { version = "0.8.0", path = "../../../client/executor", features = ["wasmtime"] }
 sc-service = { version = "0.8.0", path = "../../../client/service", features = ["wasmtime"] }
+sc-keystore = { version = "2.0.0", path = "../../../client/keystore" }
 sp-inherents = { version = "2.0.0", path = "../../../primitives/inherents" }
 sc-transaction-pool = { version = "2.0.0", path = "../../../client/transaction-pool" }
 sp-transaction-pool = { version = "2.0.0", path = "../../../primitives/transaction-pool" }

bin/node-template/node/src/service.rs

@@ -10,6 +10,7 @@ use sc_executor::native_executor_instance;
 pub use sc_executor::NativeExecutor;
 use sp_consensus_aura::sr25519::{AuthorityPair as AuraPair};
 use sc_finality_grandpa::SharedVoterState;
+use sc_keystore::LocalKeystore;
 
 // Our native executor instance.
 native_executor_instance!(
@@ -37,6 +38,10 @@ pub fn new_partial(config: &Configuration) -> Result<sc_service::PartialComponen
 		sc_finality_grandpa::LinkHalf<Block, FullClient, FullSelectChain>
 	)
 >, ServiceError> {
+	if config.keystore_remote.is_some() {
+		return Err(ServiceError::Other(
+			format!("Remote Keystores are not supported.")))
+	}
 	let inherent_data_providers = sp_inherents::InherentDataProviders::new();
 
 	let (client, backend, keystore_container, task_manager) =
@@ -78,14 +83,30 @@ pub fn new_partial(config: &Configuration) -> Result<sc_service::PartialComponen
 	})
 }
 
+fn remote_keystore(_url: &String) -> Result<Arc<LocalKeystore>, &'static str> {
+	// FIXME: here would the concrete keystore be built,
+	//        must return a concrete type (NOT `LocalKeystore`) that
+	//        implements `CryptoStore` and `SyncCryptoStore`
+	Err("Remote Keystore not supported.")
+}
+
 /// Builds a new service for a full client.
 pub fn new_full(mut config: Configuration) -> Result<TaskManager, ServiceError> {
 	let sc_service::PartialComponents {
-		client, backend, mut task_manager, import_queue, keystore_container,
+		client, backend, mut task_manager, import_queue, mut keystore_container,
 		select_chain, transaction_pool, inherent_data_providers,
 		other: (block_import, grandpa_link),
 	} = new_partial(&config)?;
 
+	if let Some(url) = &config.keystore_remote {
+		match remote_keystore(url) {
+			Ok(k) => keystore_container.set_remote_keystore(k),
+			Err(e) => {
+				return Err(ServiceError::Other(
+					format!("Error hooking up remote keystore for {}: {}", url, e)))
+			}
+		};
+	}
 	config.network.notifications_protocols.push(sc_finality_grandpa::GRANDPA_PROTOCOL_NAME.into());
 
 	let (network, network_status_sinks, system_rpc_tx, network_starter) =

client/cli/src/commands/insert.rs

@@ -65,7 +65,7 @@ impl InsertCmd {
 			.ok_or_else(|| Error::MissingBasePath)?;
 
 		let (keystore, public) = match self.keystore_params.keystore_config(base_path)? {
-			KeystoreConfig::Path { path, password } => {
+			(_, KeystoreConfig::Path { path, password }) => {
 				let public = with_crypto_scheme!(
 					self.crypto_scheme.scheme,
 					to_vec(&suri, password.clone())

client/cli/src/config.rs

@@ -188,10 +188,10 @@ pub trait CliConfiguration<DCV: DefaultConfigurationValues = ()>: Sized {
 	///
 	/// Bu default this is retrieved from `KeystoreParams` if it is available. Otherwise it uses
 	/// `KeystoreConfig::InMemory`.
-	fn keystore_config(&self, base_path: &PathBuf) -> Result<KeystoreConfig> {
+	fn keystore_config(&self, base_path: &PathBuf) -> Result<(Option<String>, KeystoreConfig)> {
 		self.keystore_params()
 			.map(|x| x.keystore_config(base_path))
-			.unwrap_or(Ok(KeystoreConfig::InMemory))
+			.unwrap_or_else(|| Ok((None, KeystoreConfig::InMemory)))
 	}
 
 	/// Get the database cache size.
@@ -471,6 +471,7 @@ pub trait CliConfiguration<DCV: DefaultConfigurationValues = ()>: Sized {
 		let role = self.role(is_dev)?;
 		let max_runtime_instances = self.max_runtime_instances()?.unwrap_or(8);
 		let is_validator = role.is_network_authority();
+		let (keystore_remote, keystore) = self.keystore_config(&config_dir)?;
 
 		let unsafe_pruning = self
 			.import_params()
@@ -491,7 +492,8 @@ pub trait CliConfiguration<DCV: DefaultConfigurationValues = ()>: Sized {
 				node_key,
 				DCV::p2p_listen_port(),
 			)?,
-			keystore: self.keystore_config(&config_dir)?,
+			keystore_remote,
+			keystore,
 			database: self.database_config(&config_dir, database_cache_size, database)?,
 			state_cache_size: self.state_cache_size()?,
 			state_cache_child_ratio: self.state_cache_child_ratio()?,

client/cli/src/params/keystore_params.rs

@@ -30,6 +30,9 @@ const DEFAULT_KEYSTORE_CONFIG_PATH: &'static str = "keystore";
 /// Parameters of the keystore
 #[derive(Debug, StructOpt)]
 pub struct KeystoreParams {
+	/// Specify custom URIs to connect to for keystore-services
+	#[structopt(long = "keystore-uri")]
+	pub keystore_uri: Option<String>,
 	/// Specify custom keystore path.
 	#[structopt(long = "keystore-path", value_name = "PATH", parse(from_os_str))]
 	pub keystore_path: Option<PathBuf>,
@@ -67,7 +70,9 @@ pub fn secret_string_from_str(s: &str) -> std::result::Result<SecretString, Stri
 
 impl KeystoreParams {
 	/// Get the keystore configuration for the parameters
-	pub fn keystore_config(&self, base_path: &PathBuf) -> Result<KeystoreConfig> {
+	/// returns a vector of remote-urls and the local Keystore configuration
+	pub fn keystore_config(&self, base_path: &PathBuf) -> Result<(Option<String>, KeystoreConfig)> {
+
 		let password = if self.password_interactive {
 			#[cfg(not(target_os = "unknown"))]
 			{
@@ -89,7 +94,7 @@ impl KeystoreParams {
 			.clone()
 			.unwrap_or_else(|| base_path.join(DEFAULT_KEYSTORE_CONFIG_PATH));
 
-		Ok(KeystoreConfig::Path { path, password })
+		Ok((self.keystore_uri.clone(), KeystoreConfig::Path { path, password }))
 	}
 
 	/// helper method to fetch password from `KeyParams` or read from stdin

client/service/src/builder.rs

@@ -59,7 +59,7 @@ use sp_core::traits::{
 	CodeExecutor,
 	SpawnNamed,
 };
-use sp_keystore::{CryptoStore, SyncCryptoStorePtr};
+use sp_keystore::{CryptoStore, SyncCryptoStore, SyncCryptoStorePtr};
 use sp_runtime::BuildStorage;
 use sc_client_api::{
 	BlockBackend, BlockchainEvents,
@@ -205,12 +205,25 @@ pub type TLightClientWithBackend<TBl, TRtApi, TExecDisp, TBackend> = Client<
 	TRtApi,
 >;
 
-enum KeystoreContainerInner {
-	Local(Arc<LocalKeystore>)
+trait AsCryptoStoreRef {
+    fn keystore_ref(&self) -> Arc<dyn CryptoStore>;
+    fn sync_keystore_ref(&self) -> Arc<dyn SyncCryptoStore>;
+}
+
+impl<T> AsCryptoStoreRef for Arc<T> where T: CryptoStore + SyncCryptoStore + 'static {
+    fn keystore_ref(&self) -> Arc<dyn CryptoStore> {
+        self.clone()
+    }
+    fn sync_keystore_ref(&self) -> Arc<dyn SyncCryptoStore> {
+        self.clone()
+    }
 }
 
 /// Construct and hold different layers of Keystore wrappers
-pub struct KeystoreContainer(KeystoreContainerInner);
+pub struct KeystoreContainer {
+	remote: Option<Box<dyn AsCryptoStoreRef>>,
+	local: Arc<LocalKeystore>,
+}
 
 impl KeystoreContainer {
 	/// Construct KeystoreContainer
@@ -223,20 +236,35 @@ impl KeystoreContainer {
 			KeystoreConfig::InMemory => LocalKeystore::in_memory(),
 		});
 
-		Ok(Self(KeystoreContainerInner::Local(keystore)))
+		Ok(Self{remote: Default::default(), local: keystore})
+	}
+
+	/// Set the remote keystore.
+	/// Should be called right away at startup and not at runtime:
+	/// even though this overrides any previously set remote store, it
+	/// does not reset any references previously handed out - they will
+	/// stick araound.
+	pub fn set_remote_keystore<T>(&mut self, remote: Arc<T>)
+		where T: CryptoStore + SyncCryptoStore + 'static
+	{
+		self.remote = Some(Box::new(remote))
 	}
 
 	/// Returns an adapter to the asynchronous keystore that implements `CryptoStore`
 	pub fn keystore(&self) -> Arc<dyn CryptoStore> {
-		match self.0 {
-			KeystoreContainerInner::Local(ref keystore) => keystore.clone(),
+		if let Some(c) = self.remote.as_ref() {
+			c.keystore_ref()
+		} else {
+			self.local.clone()
 		}
 	}
 
 	/// Returns the synchrnous keystore wrapper
 	pub fn sync_keystore(&self) -> SyncCryptoStorePtr {
-		match self.0 {
-			KeystoreContainerInner::Local(ref keystore) => keystore.clone() as SyncCryptoStorePtr,
+		if let Some(c) = self.remote.as_ref() {
+			c.sync_keystore_ref()
+		} else {
+			self.local.clone() as SyncCryptoStorePtr
 		}
 	}
 
@@ -249,9 +277,7 @@ impl KeystoreContainer {
 	/// Using the [`LocalKeystore`] will result in loosing the ability to use any other keystore implementation, like
 	/// a remote keystore for example. Only use this if you a certain that you require it!
 	pub fn local_keystore(&self) -> Option<Arc<LocalKeystore>> {
-		match self.0 {
-			KeystoreContainerInner::Local(ref keystore) => Some(keystore.clone()),
-		}
+		Some(self.local.clone())
 	}
 }
 

client/service/src/config.rs

@@ -50,6 +50,8 @@ pub struct Configuration {
 	pub network: NetworkConfiguration,
 	/// Configuration for the keystore.
 	pub keystore: KeystoreConfig,
+	/// Remote URI to connect to for async keystore support
+	pub keystore_remote: Option<String>,
 	/// Configuration for the database.
 	pub database: DatabaseConfig,
 	/// Size of internal state cache in Bytes

client/service/test/src/lib.rs

@@ -239,6 +239,7 @@ fn node_config<G: RuntimeGenesis + 'static, E: ChainSpecExtension + Clone + 'sta
 		task_executor,
 		transaction_pool: Default::default(),
 		network: network_config,
+		keystore_remote: Default::default(),
 		keystore: KeystoreConfig::Path {
 			path: root.join("key"),
 			password: None

primitives/application-crypto/test/Cargo.toml

@@ -14,7 +14,7 @@ targets = ["x86_64-unknown-linux-gnu"]
 
 [dependencies]
 sp-core = { version = "2.0.0", default-features = false, path = "../../core" }
-sp-keystore = { version = "0.8.0", path = "../../keystore" }
+sp-keystore = { version = "0.8.0", path = "../../keystore", default-features = false }
 substrate-test-runtime-client = { version = "2.0.0", path = "../../../test-utils/runtime/client" }
 sp-runtime = { version = "2.0.0", path = "../../runtime" }
 sp-api = { version = "2.0.0", path = "../../api" }

primitives/core/Cargo.toml

@@ -96,11 +96,11 @@ std = [
 	"base58",
 	"substrate-bip39",
 	"tiny-bip39",
-	"serde",
 	"byteorder/std",
 	"rand",
 	"sha2/std",
 	"schnorrkel/std",
+	"schnorrkel/serde",
 	"regex",
 	"num-traits/std",
 	"tiny-keccak",

primitives/core/src/crypto.rs

@@ -1029,6 +1029,7 @@ pub trait CryptoType {
 	Copy, Clone, Default, PartialEq, Eq, PartialOrd, Ord, Hash, Encode, Decode, PassByInner,
 	crate::RuntimeDebug
 )]
+#[cfg_attr(feature = "std", derive(serde::Serialize, serde::Deserialize))]
 pub struct KeyTypeId(pub [u8; 4]);
 
 impl From<u32> for KeyTypeId {
@@ -1058,10 +1059,12 @@ impl<'a> TryFrom<&'a str> for KeyTypeId {
 
 /// An identifier for a specific cryptographic algorithm used by a key pair
 #[derive(Debug, Copy, Clone, Default, PartialEq, Eq, PartialOrd, Ord, Hash, Encode, Decode)]
+#[cfg_attr(feature = "std", derive(serde::Serialize, serde::Deserialize))]
 pub struct CryptoTypeId(pub [u8; 4]);
 
 /// A type alias of CryptoTypeId & a public key
 #[derive(Debug, Clone, Default, PartialEq, Eq, PartialOrd, Ord, Hash, Encode, Decode)]
+#[cfg_attr(feature = "std", derive(serde::Serialize, serde::Deserialize))]
 pub struct CryptoTypePublicPair(pub CryptoTypeId, pub Vec<u8>);
 
 #[cfg(feature = "std")]

primitives/keystore/Cargo.toml

@@ -20,10 +20,19 @@ futures = { version = "0.3.1" }
 schnorrkel = { version = "0.9.1", features = ["preaudit_deprecated", "u64_backend"], default-features = false }
 merlin = { version = "2.0", default-features = false }
 parking_lot = { version = "0.10.0", default-features = false }
-
+serde = { version = "1.0", optional = true}
 sp-core = { version = "2.0.0", path = "../core" }
 sp-externalities = { version = "0.8.0", path = "../externalities", default-features = false }
 
 [dev-dependencies]
 rand = "0.7.2"
 rand_chacha = "0.2.2"
+
+
+[features]
+default = ["std"]
+std = [
+	"serde",
+	"schnorrkel/std",
+	"schnorrkel/serde",
+]

primitives/keystore/src/vrf.rs

@@ -20,9 +20,11 @@
 use codec::Encode;
 use merlin::Transcript;
 use schnorrkel::vrf::{VRFOutput, VRFProof};
+
 /// An enum whose variants represent possible
 /// accepted values to construct the VRF transcript
 #[derive(Clone, Encode)]
+#[cfg_attr(feature = "std", derive(serde::Serialize, serde::Deserialize))]
 pub enum VRFTranscriptValue {
 	/// Value is an array of bytes
 	Bytes(Vec<u8>),
@@ -38,6 +40,7 @@ pub struct VRFTranscriptData {
 	pub items: Vec<(&'static str, VRFTranscriptValue)>,
 }
 /// VRF signature data
+#[cfg_attr(feature = "std", derive(serde::Serialize, serde::Deserialize))]
 pub struct VRFSignature {
 	/// The VRFOutput serialized
 	pub output: VRFOutput,

utils/browser/src/lib.rs

@@ -75,6 +75,7 @@ where
 
 			DatabaseConfig::Custom(sp_database::as_database(db))
 		},
+		keystore_remote: Default::default(),
 		keystore: KeystoreConfig::InMemory,
 		default_heap_pages: Default::default(),
 		dev_key_seed: Default::default(),