From a7f1242961b352b3a09f2d879dcb5e2a5355ef42 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jon=20H=C3=A4ggblad?= Date: Thu, 12 Jan 2023 11:35:34 +0100 Subject: [PATCH] client-core: clean up gateway registration (#2827) * client-core: clean up gateway registration * client-core: tidy docs and clippy * clients: tidy output --- clients/client-core/src/config/mod.rs | 1 + clients/client-core/src/init/helpers.rs | 4 +- clients/client-core/src/init/mod.rs | 122 ++++++++++-------------- clients/native/src/commands/init.rs | 13 ++- clients/socks5/src/commands/init.rs | 13 ++- nym-connect/src-tauri/src/config/mod.rs | 12 +-- nym-connect/src-tauri/src/error.rs | 2 + sdk/rust/nym-sdk/src/error.rs | 3 + sdk/rust/nym-sdk/src/mixnet/client.rs | 13 ++- 9 files changed, 90 insertions(+), 93 deletions(-) diff --git a/clients/client-core/src/config/mod.rs b/clients/client-core/src/config/mod.rs index 1d3547dbd3..df1e0aa250 100644 --- a/clients/client-core/src/config/mod.rs +++ b/clients/client-core/src/config/mod.rs @@ -88,6 +88,7 @@ impl Config { Config::default().with_id(id) } + #[must_use] pub fn with_id>(mut self, id: S) -> Self where T: NymConfig, diff --git a/clients/client-core/src/init/helpers.rs b/clients/client-core/src/init/helpers.rs index 6ffa4bfd97..cd22774ecb 100644 --- a/clients/client-core/src/init/helpers.rs +++ b/clients/client-core/src/init/helpers.rs @@ -18,7 +18,7 @@ use url::Url; pub(super) async fn query_gateway_details( validator_servers: Vec, - chosen_gateway_id: Option, + chosen_gateway_id: Option, ) -> Result { let nym_api = validator_servers .choose(&mut thread_rng()) @@ -40,7 +40,7 @@ pub(super) async fn query_gateway_details( if let Some(gateway_id) = chosen_gateway_id { filtered_gateways .iter() - .find(|gateway| gateway.identity_key.to_base58_string() == gateway_id) + .find(|gateway| gateway.identity_key == gateway_id) .ok_or_else(|| ClientCoreError::NoGatewayWithId(gateway_id.to_string())) .cloned() } else { diff --git a/clients/client-core/src/init/mod.rs b/clients/client-core/src/init/mod.rs index f02d228242..6a84a3d37a 100644 --- a/clients/client-core/src/init/mod.rs +++ b/clients/client-core/src/init/mod.rs @@ -69,42 +69,21 @@ pub fn new_client_keys() -> KeyManager { KeyManager::new(&mut rng) } -/// Convenience function for setting up the gateway for a client. Depending on the arguments given -/// it will do the sensible thing. -pub async fn setup_gateway( - register_gateway: bool, - // TODO: this should get refactored to instead take Option - user_chosen_gateway_id: Option, - config: &Config, -) -> Result -where - C: NymConfig + ClientCoreConfigTrait, - T: NymConfig, -{ - let id = config.get_id(); - if register_gateway { - register_with_gateway_and_store(user_chosen_gateway_id, config).await - } else if let Some(user_chosen_gateway_id) = user_chosen_gateway_id { - config_gateway_with_existing_keys(user_chosen_gateway_id, config).await - } else { - reuse_existing_gateway_config::(&id) - } -} - -/// Get the gateway details by querying the validator-api. Either pick one at random or use -/// the chosen one if it's among the available ones. +/// Authenticate and register with a gateway. +/// Either pick one at random by querying the available gateways from the nym-api, or use the +/// chosen one if it's among the available ones. +/// The shared key is added to the supplied `KeyManager` and the endpoint details are returned. pub async fn register_with_gateway( key_manager: &mut KeyManager, nym_api_endpoints: Vec, - chosen_gateway_id: Option, + chosen_gateway_id: Option, ) -> Result { - // Our identity is derived from our key - let our_identity = key_manager.identity_keypair(); - // Get the gateway details of the gateway we will use let gateway = helpers::query_gateway_details(nym_api_endpoints, chosen_gateway_id).await?; log::debug!("Querying gateway gives: {}", gateway); + let our_identity = key_manager.identity_keypair(); + // Establish connection, authenticate and generate keys for talking with the gateway let shared_keys = helpers::register_with_gateway(&gateway, our_identity).await?; key_manager.insert_gateway_shared_key(shared_keys); @@ -112,59 +91,64 @@ pub async fn register_with_gateway( Ok(gateway.into()) } -/// Get the gateway details by querying the validator-api. Either pick one at random or use -/// the chosen one if it's among the available ones. -/// Saves keys to disk, specified by the paths in `config`. -pub async fn register_with_gateway_and_store( - chosen_gateway_id: Option, +/// Convenience function for setting up the gateway for a client given a `Config`. Depending on the +/// arguments given it will do the sensible thing. Either it will +/// +/// a. Reuse existing gateway configuration from storage. +/// b. Create a new gateway configuration but keep existing keys. This assumes that the caller +/// knows what they are doing and that the keys match the requested gateway. +/// c. Create a new gateway configuration with a newly registered gateway and keys. +pub async fn setup_gateway_from_config( + register_gateway: bool, + user_chosen_gateway_id: Option, config: &Config, ) -> Result where + C: NymConfig + ClientCoreConfigTrait, T: NymConfig, { - println!("Configuring gateway"); - let mut key_manager = new_client_keys(); + let id = config.get_id(); - let gateway = register_with_gateway( - &mut key_manager, - config.get_nym_api_endpoints(), - chosen_gateway_id, - ) - .await?; + // If we are not going to register gateway, and an explicitly chosed gateway is not passed in, + // load the existing configuration file + if !register_gateway && user_chosen_gateway_id.is_none() { + println!("Not registering gateway, will reuse existing config and keys"); + return load_existing_gateway_config::(&id); + } - helpers::store_keys(&key_manager, config)?; - println!("Saved all generated keys"); - - Ok(gateway) -} - -/// Set the gateway using the usual procedue of querying the validator-api, but don't register or -/// create any keys. -/// This assumes that the user knows what they are doing, and that the existing keys are valid for -/// the gateway being used -pub async fn config_gateway_with_existing_keys( - user_chosen_gateway_id: String, - config: &Config, -) -> Result -where - T: NymConfig, -{ - println!("Using gateway provided by user, keeping existing keys"); - let gateway = helpers::query_gateway_details( - config.get_nym_api_endpoints(), - Some(user_chosen_gateway_id), - ) - .await?; + // Else, we preceed by querying the nym-api + let gateway = + helpers::query_gateway_details(config.get_nym_api_endpoints(), user_chosen_gateway_id) + .await?; log::debug!("Querying gateway gives: {}", gateway); + + // If we are not registering, just return this and assume the caller has the keys already and + // wants to keep the, + if user_chosen_gateway_id.is_some() { + println!("Using gateway provided by user, keeping existing keys"); + return Ok(gateway.into()); + } + + // Create new keys and derive our identity + let mut key_manager = new_client_keys(); + let our_identity = key_manager.identity_keypair(); + + // Establish connection, authenticate and generate keys for talking with the gateway + println!("Registering with new gateway"); + let shared_keys = helpers::register_with_gateway(&gateway, our_identity).await?; + key_manager.insert_gateway_shared_key(shared_keys); + + // Write all keys to storage and just return the gateway endpoint config. It is assumed that we + // will load keys from storage when actually connecting. + helpers::store_keys(&key_manager, config)?; Ok(gateway.into()) } /// Read and reuse the existing gateway configuration from a file that was generate earlier. -pub fn reuse_existing_gateway_config(id: &str) -> Result +pub fn load_existing_gateway_config(id: &str) -> Result where T: NymConfig + ClientCoreConfigTrait, { - println!("Not registering gateway, will reuse existing config and keys"); T::load_from_file(Some(id)) .map(|existing_config| existing_config.get_gateway_endpoint().clone()) .map_err(|err| { @@ -242,9 +226,9 @@ where pub fn output_to_json(init_results: &T, output_file: &str) { match std::fs::File::create(output_file) { Ok(file) => match serde_json::to_writer_pretty(file, init_results) { - Ok(_) => println!("Saved: {}", output_file), - Err(err) => eprintln!("Could not save {}: {err}", output_file), + Ok(_) => println!("Saved: {output_file}"), + Err(err) => eprintln!("Could not save {output_file}: {err}"), }, - Err(err) => eprintln!("Could not save {}: {err}", output_file), + Err(err) => eprintln!("Could not save {output_file}: {err}"), } } diff --git a/clients/native/src/commands/init.rs b/clients/native/src/commands/init.rs index bb8295a34e..1515b24e62 100644 --- a/clients/native/src/commands/init.rs +++ b/clients/native/src/commands/init.rs @@ -114,16 +114,15 @@ pub(crate) async fn execute(args: &Init) -> Result<(), ClientError> { let already_init = Config::default_config_file_path(Some(id)).exists(); if already_init { - println!( - "Client \"{}\" was already initialised before! \ - Config information will be overwritten (but keys will be kept)!", - id - ); + println!("Client \"{id}\" was already initialised before"); } // Usually you only register with the gateway on the first init, however you can force // re-registering if wanted. let user_wants_force_register = args.force_register_gateway; + if user_wants_force_register { + println!("Instructed to force registering gateway. This might overwrite keys!"); + } // If the client was already initialized, don't generate new keys and don't re-register with // the gateway (because this would create a new shared key). @@ -138,9 +137,9 @@ pub(crate) async fn execute(args: &Init) -> Result<(), ClientError> { // Setup gateway by either registering a new one, or creating a new config from the selected // one but with keys kept, or reusing the gateway configuration. - let gateway = client_core::init::setup_gateway::( + let gateway = client_core::init::setup_gateway_from_config::( register_gateway, - user_chosen_gateway_id.map(|id| id.to_base58_string()), + user_chosen_gateway_id, config.get_base(), ) .await diff --git a/clients/socks5/src/commands/init.rs b/clients/socks5/src/commands/init.rs index f9bcf219a8..ebe01ad24d 100644 --- a/clients/socks5/src/commands/init.rs +++ b/clients/socks5/src/commands/init.rs @@ -124,16 +124,15 @@ pub(crate) async fn execute(args: &Init) -> Result<(), Socks5ClientError> { let already_init = Config::default_config_file_path(Some(id)).exists(); if already_init { - println!( - "SOCKS5 client \"{}\" was already initialised before! \ - Config information will be overwritten (but keys will be kept)!", - id - ); + println!("SOCKS5 client \"{id}\" was already initialised before"); } // Usually you only register with the gateway on the first init, however you can force // re-registering if wanted. let user_wants_force_register = args.force_register_gateway; + if user_wants_force_register { + println!("Instructed to force registering gateway. This might overwrite keys!"); + } // If the client was already initialized, don't generate new keys and don't re-register with // the gateway (because this would create a new shared key). @@ -151,9 +150,9 @@ pub(crate) async fn execute(args: &Init) -> Result<(), Socks5ClientError> { // Setup gateway by either registering a new one, or creating a new config from the selected // one but with keys kept, or reusing the gateway configuration. - let gateway = client_core::init::setup_gateway::( + let gateway = client_core::init::setup_gateway_from_config::( register_gateway, - user_chosen_gateway_id.map(|id| id.to_base58_string()), + user_chosen_gateway_id, config.get_base(), ) .await diff --git a/nym-connect/src-tauri/src/config/mod.rs b/nym-connect/src-tauri/src/config/mod.rs index 4cf4570207..4cde4ebaa4 100644 --- a/nym-connect/src-tauri/src/config/mod.rs +++ b/nym-connect/src-tauri/src/config/mod.rs @@ -4,6 +4,7 @@ use crate::{ }; use client_core::config::Config as BaseConfig; use config_common::NymConfig; +use crypto::asymmetric::identity; use nym_socks5::client::config::Config as Socks5Config; use std::path::PathBuf; use std::sync::Arc; @@ -109,11 +110,7 @@ pub async fn init_socks5_config(provider_address: String, chosen_gateway_id: Str ); let already_init = Config::config_file_location(&id)?.exists(); if already_init { - log::info!( - "SOCKS5 client \"{}\" was already initialised before! \ - Config information will be overwritten (but keys will be kept)!", - id - ); + log::info!("SOCKS5 client \"{id}\" was already initialised before"); } // Future proofing. This flag exists for the other clients @@ -133,8 +130,11 @@ pub async fn init_socks5_config(provider_address: String, chosen_gateway_id: Str .set_custom_nym_apis(config_common::parse_urls(&raw_validators)); } + let chosen_gateway_id = identity::PublicKey::from_base58_string(chosen_gateway_id) + .map_err(|_| BackendError::UnableToParseGateway)?; + // Setup gateway by either registering a new one, or reusing exiting keys - let gateway = client_core::init::setup_gateway::( + let gateway = client_core::init::setup_gateway_from_config::( register_gateway, Some(chosen_gateway_id), config.get_base(), diff --git a/nym-connect/src-tauri/src/error.rs b/nym-connect/src-tauri/src/error.rs index 124eeeddc4..53b8c9df44 100644 --- a/nym-connect/src-tauri/src/error.rs +++ b/nym-connect/src-tauri/src/error.rs @@ -68,6 +68,8 @@ pub enum BackendError { CouldNotLoadExistingGatewayConfiguration(std::io::Error), #[error("Unable to open a new window")] NewWindowError, + #[error("Unable to parse the specified gateway")] + UnableToParseGateway, } impl Serialize for BackendError { diff --git a/sdk/rust/nym-sdk/src/error.rs b/sdk/rust/nym-sdk/src/error.rs index 8f059ef001..6003256ad8 100644 --- a/sdk/rust/nym-sdk/src/error.rs +++ b/sdk/rust/nym-sdk/src/error.rs @@ -8,6 +8,9 @@ pub enum Error { TomlSerializationError(#[from] toml::ser::Error), #[error("toml deserialization error: {0}")] TomlDeserializationError(#[from] toml::de::Error), + #[error("Ed25519 error: {0}")] + Ed25519RecoveryError(#[from] crypto::asymmetric::identity::Ed25519RecoveryError), + #[error(transparent)] ClientCoreError(#[from] client_core::error::ClientCoreError), diff --git a/sdk/rust/nym-sdk/src/mixnet/client.rs b/sdk/rust/nym-sdk/src/mixnet/client.rs index 826e6aea23..99fca9e424 100644 --- a/sdk/rust/nym-sdk/src/mixnet/client.rs +++ b/sdk/rust/nym-sdk/src/mixnet/client.rs @@ -13,13 +13,15 @@ use client_core::{ }, config::{persistence::key_pathfinder::ClientKeyPathfinder, GatewayEndpointConfig}, }; -use futures::StreamExt; +use crypto::asymmetric::identity; use nymsphinx::{ addressing::clients::{ClientIdentity, Recipient}, receiver::ReconstructedMessage, }; use task::TaskManager; +use futures::StreamExt; + use super::{connection_state::BuilderState, Config, GatewayKeyMode, Keys, KeysArc, StoragePaths}; use crate::error::{Error, Result}; @@ -124,10 +126,17 @@ impl ClientBuilder { "can only setup gateway when in `New` connection state" ); + let user_chosen_gateway = self + .config + .user_chosen_gateway + .as_ref() + .map(identity::PublicKey::from_base58_string) + .transpose()?; + let gateway_config = client_core::init::register_with_gateway( &mut self.key_manager, self.config.nym_api_endpoints.clone(), - self.config.user_chosen_gateway.clone(), + user_chosen_gateway, ) .await?;