diff --git a/Cargo.lock b/Cargo.lock index afd8231418..847f10d72d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6660,6 +6660,7 @@ dependencies = [ "openssl-sys", "pin-project 1.0.8", "prost", + "prost-types", "rand 0.8.4", "serde 1.0.130", "serde_derive", diff --git a/applications/ffi_client/lib/index.js b/applications/ffi_client/lib/index.js index fb4d4ef50c..db922de1f0 100644 --- a/applications/ffi_client/lib/index.js +++ b/applications/ffi_client/lib/index.js @@ -17,6 +17,7 @@ const { u8ArrayPtr, byteVectorRef, publicKeyRef, + publicKeyArrPtr, strArray, strArrayPtr, } = require("./types"); @@ -30,7 +31,9 @@ const libWallet = ffi.Library("./libtari_wallet_ffi.dylib", { commsConfigRef, ["string", transportRef, "string", "string", u64, u64, errPtr], ], + comms_list_connected_public_keys: [publicKeyArrPtr, [walletRef, errPtr]], public_key_create: [publicKeyRef, [byteVectorRef, errPtr]], + public_keys_destroy: ["void", [publicKeyArrPtr]], public_key_get_bytes: [u8ArrayPtr, [publicKeyRef, errPtr]], seed_words_create: [strPtr, []], seed_words_get_at: ["string", [strArrayPtr, u32, errPtr]], diff --git a/applications/ffi_client/lib/types.js b/applications/ffi_client/lib/types.js index 7815eb5ab8..e8e7057938 100644 --- a/applications/ffi_client/lib/types.js +++ b/applications/ffi_client/lib/types.js @@ -17,7 +17,8 @@ const u8Array = ArrayType(ref.types.uint8); const u8ArrayPtr = ref.refType(u8Array); const byteVectorRef = ref.refType(u8Array); const publicKeyRef = ref.refType(ref.types.void); -const strArray = ArrayType("string"); +const publicKeyArrPtr = ref.refType(u8Array); +const strArray = ref.refType(ArrayType(ref.types.void)); const strArrayPtr = ref.refType(ArrayType("string")); module.exports = { @@ -37,6 +38,7 @@ module.exports = { u8ArrayPtr, byteVectorRef, publicKeyRef, + publicKeyArrPtr, strArray, strArrayPtr, }; diff --git a/applications/launchpad/docker_rig/config.toml b/applications/launchpad/docker_rig/config.toml index 9d97586e67..240e3a4a5e 100644 --- a/applications/launchpad/docker_rig/config.toml +++ b/applications/launchpad/docker_rig/config.toml @@ -232,7 +232,7 @@ console_wallet_tor_identity_file = "config/console_wallet_tor.json" # direcly over TCP. /ip4, /ip6, /dns, /dns4 and /dns6 are supported. # tor_proxy_bypass_addresses = ["/dns4/my-foo-base-node/tcp/9998"] # When using the tor transport and set to true, outbound TCP connections bypass the tor proxy. Defaults to false for better privacy -# tor_proxy_bypass_for_outbound_tcp = false; +# tor_proxy_bypass_for_outbound_tcp = false ######################################################################################################################## # # @@ -309,7 +309,7 @@ console_wallet_tor_identity_file = "config/igor/console_wallet_tor.json" # direcly over TCP. /ip4, /ip6, /dns, /dns4 and /dns6 are supported. # tor_proxy_bypass_addresses = ["/dns4/my-foo-base-node/tcp/9998"] # When using the tor transport and set to true, outbound TCP connections bypass the tor proxy. Defaults to false for better privacy -# tor_proxy_bypass_for_outbound_tcp = false; +# tor_proxy_bypass_for_outbound_tcp = false ######################################################################################################################## # # diff --git a/applications/tari_app_utilities/Cargo.toml b/applications/tari_app_utilities/Cargo.toml index aa7fca8910..04c9d940f1 100644 --- a/applications/tari_app_utilities/Cargo.toml +++ b/applications/tari_app_utilities/Cargo.toml @@ -16,11 +16,11 @@ config = { version = "0.9.3" } futures = { version = "^0.3.16", default-features = false, features = ["alloc"] } qrcode = { version = "0.12" } dirs-next = "1.0.2" -serde = "1.0.126" json5 = "0.2.2" log = { version = "0.4.8", features = ["std"] } rand = "0.8" tokio = { version = "1.11", features = ["signal"] } +serde = "1.0.126" structopt = { version = "0.3.13", default_features = false } strum = "^0.22" strum_macros = "^0.22" diff --git a/applications/tari_app_utilities/src/identity_management.rs b/applications/tari_app_utilities/src/identity_management.rs index 518e542d82..41fb3d24aa 100644 --- a/applications/tari_app_utilities/src/identity_management.rs +++ b/applications/tari_app_utilities/src/identity_management.rs @@ -29,9 +29,8 @@ use tari_common::{ configuration::{bootstrap::prompt, utils::get_local_ip}, exit_codes::ExitCodes, }; -use tari_common_types::types::PrivateKey; use tari_comms::{multiaddr::Multiaddr, peer_manager::PeerFeatures, NodeIdentity}; -use tari_crypto::{keys::SecretKey, tari_utilities::hex::Hex}; +use tari_crypto::tari_utilities::hex::Hex; pub const LOG_TARGET: &str = "tari_application"; @@ -59,7 +58,7 @@ pub fn setup_node_identity>( None => Ok(Arc::new(id)), }, Err(e) => { - debug!(target: LOG_TARGET, "Node id not found. {}. Creating new ID", e); + debug!(target: LOG_TARGET, "Failed to load node identity: {}", e); if !create_id { let prompt = prompt("Node identity does not exist.\nWould you like to to create one (Y/n)?"); if !prompt { @@ -79,6 +78,8 @@ pub fn setup_node_identity>( }; } + debug!(target: LOG_TARGET, "Existing node id not found. {}. Creating new ID", e); + match create_new_identity(&identity_file, public_address.clone(), peer_features) { Ok(id) => { info!( @@ -117,7 +118,7 @@ pub fn load_identity>(path: P) -> Result { )); } - let id_str = std::fs::read_to_string(path.as_ref()).map_err(|e| { + let id_str = fs::read_to_string(path.as_ref()).map_err(|e| { format!( "The node identity file, {}, could not be read. {}", path.as_ref().to_str().unwrap_or("?"), @@ -131,6 +132,10 @@ pub fn load_identity>(path: P) -> Result { e ) })?; + // Check whether the previous version has a signature and sign if necessary + if !id.is_signed() { + id.sign(); + } debug!( "Node ID loaded with public key {} and Node id {}", id.public_key().to_hex(), @@ -151,9 +156,8 @@ pub fn create_new_identity>( public_addr: Option, features: PeerFeatures, ) -> Result { - let private_key = PrivateKey::random(&mut OsRng); - let node_identity = NodeIdentity::new( - private_key, + let node_identity = NodeIdentity::random( + &mut OsRng, match public_addr { Some(public_addr) => public_addr, None => format!("{}/tcp/18141", get_local_ip().ok_or("Can't get local ip address")?) @@ -166,26 +170,6 @@ pub fn create_new_identity>( Ok(node_identity) } -/// Recover a node id from a given private key and save it to disk -/// ## Parameters -/// `private_key` - The private key -/// `path` - Reference to path to save the file -/// `public_addr` - Network address of the base node -/// `peer_features` - The features enabled for the base node -/// -/// ## Returns -/// A NodeIdentity wrapped in an atomic reference counter on success, the exit code indicating the reason on failure -pub fn recover_node_identity>( - private_key: PrivateKey, - path: P, - public_addr: &Multiaddr, - features: PeerFeatures, -) -> Result, ExitCodes> { - let node_identity = NodeIdentity::new(private_key, public_addr.clone(), features); - save_as_json(path, &node_identity).map_err(ExitCodes::IOError)?; - Ok(Arc::new(node_identity)) -} - /// Loads the node identity from json at the given path /// ## Parameters /// `path` - Path to file from which to load the node identity @@ -213,15 +197,14 @@ pub fn load_from_json, T: DeserializeOwned>(path: P) -> Result, T: Serialize>(path: P, object: &T) -> Result<(), String> { - let json = json5::to_string(object).unwrap(); + let json = json5::to_string(object).map_err(|err| err.to_string())?; if let Some(p) = path.as_ref().parent() { if !p.exists() { fs::create_dir_all(p).map_err(|e| format!("Could not save json to data folder. {}", e))?; } } let json_with_comment = format!( - "// The public address is overwritten by the config/environment variable \ - (TARI_BASE_NODE____PUBLIC_ADDRESS)\n{}", + "// This file is generated by the Tari base node. Any changes will be overwritten.\n{}", json ); fs::write(path.as_ref(), json_with_comment.as_bytes()).map_err(|e| { diff --git a/applications/tari_base_node/src/command_handler.rs b/applications/tari_base_node/src/command_handler.rs index 221032dcc9..d69be9a45b 100644 --- a/applications/tari_base_node/src/command_handler.rs +++ b/applications/tari_base_node/src/command_handler.rs @@ -419,7 +419,7 @@ impl CommandHandler { let peer = match peer_manager.find_all_starts_with(&partial).await { Ok(peers) if peers.is_empty() => { if let Some(pk) = parse_emoji_id_or_public_key(&original_str) { - if let Ok(peer) = peer_manager.find_by_public_key(&pk).await { + if let Ok(Some(peer)) = peer_manager.find_by_public_key(&pk).await { peer } else { println!("No peer matching '{}'", original_str); @@ -457,6 +457,9 @@ impl CommandHandler { if let Some(dt) = peer.last_seen() { println!("Last seen: {}", dt); } + if let Some(updated_at) = peer.identity_signature.map(|i| i.updated_at()) { + println!("Last updated: {} (UTC)", updated_at); + } }); } @@ -479,7 +482,7 @@ impl CommandHandler { let num_peers = peers.len(); println!(); let mut table = Table::new(); - table.set_titles(vec!["NodeId", "Public Key", "Flags", "Role", "User Agent", "Info"]); + table.set_titles(vec!["NodeId", "Public Key", "Role", "User Agent", "Info"]); for peer in peers { let info_str = { @@ -491,7 +494,7 @@ impl CommandHandler { } } else if let Some(dt) = peer.last_seen() { s.push(format!( - "LAST_SEEN = {}", + "LAST_SEEN: {}", Utc::now() .naive_utc() .signed_duration_since(dt) @@ -516,10 +519,11 @@ impl CommandHandler { .get_metadata(1) .and_then(|v| bincode::deserialize::(v).ok()) { - s.push(format!( - "chain height = {}", - metadata.metadata.height_of_longest_chain() - )); + s.push(format!("chain height: {}", metadata.metadata.height_of_longest_chain())); + } + + if let Some(updated_at) = peer.identity_signature.map(|i| i.updated_at()) { + s.push(format!("updated_at: {} (UTC)", updated_at)); } if s.is_empty() { @@ -531,7 +535,6 @@ impl CommandHandler { table.add_row(row![ peer.node_id, peer.public_key, - format!("{:?}", peer.flags), { if peer.features == PeerFeatures::COMMUNICATION_CLIENT { "Wallet" @@ -724,7 +727,8 @@ impl CommandHandler { let peer = peer_manager .find_by_node_id(conn.peer_node_id()) .await - .expect("Unexpected peer database error or peer not found"); + .expect("Unexpected peer database error") + .expect("Peer not found"); let chain_height = peer .get_metadata(1) diff --git a/applications/tari_base_node/src/grpc/base_node_grpc_server.rs b/applications/tari_base_node/src/grpc/base_node_grpc_server.rs index 7f1fb9e4ac..1d7c044316 100644 --- a/applications/tari_base_node/src/grpc/base_node_grpc_server.rs +++ b/applications/tari_base_node/src/grpc/base_node_grpc_server.rs @@ -1415,7 +1415,8 @@ impl tari_rpc::base_node_server::BaseNode for BaseNodeGrpcServer { peer_manager .find_by_node_id(peer.peer_node_id()) .await - .map_err(|err| Status::internal(err.to_string()))?, + .map_err(|err| Status::internal(err.to_string()))? + .ok_or_else(|| Status::not_found(format!("Peer {} not found", peer.peer_node_id())))?, ); } diff --git a/applications/tari_console_wallet/src/automation/commands.rs b/applications/tari_console_wallet/src/automation/commands.rs index 8469df0658..45547d3a8a 100644 --- a/applications/tari_console_wallet/src/automation/commands.rs +++ b/applications/tari_console_wallet/src/automation/commands.rs @@ -310,7 +310,7 @@ async fn set_base_node_peer( println!("Setting base node peer..."); println!("{}::{}", public_key, net_address); wallet - .set_base_node_peer(public_key.clone(), net_address.to_string()) + .set_base_node_peer(public_key.clone(), net_address.clone()) .await?; Ok((public_key, net_address)) } diff --git a/applications/tari_console_wallet/src/grpc/wallet_grpc_server.rs b/applications/tari_console_wallet/src/grpc/wallet_grpc_server.rs index 70ab54c629..47957c1152 100644 --- a/applications/tari_console_wallet/src/grpc/wallet_grpc_server.rs +++ b/applications/tari_console_wallet/src/grpc/wallet_grpc_server.rs @@ -819,12 +819,13 @@ impl wallet_server::Wallet for WalletGrpcServer { .map_err(|err| Status::internal(err.to_string()))?; let mut peers = Vec::with_capacity(connected_peers.len()); - for peer in connected_peers { + for conn in connected_peers { peers.push( peer_manager - .find_by_node_id(peer.peer_node_id()) + .find_by_node_id(conn.peer_node_id()) .await - .map_err(|err| Status::internal(err.to_string()))?, + .map_err(|err| Status::internal(err.to_string()))? + .ok_or_else(|| Status::not_found(format!("Peer '{}' not found", conn.peer_node_id())))?, ); } diff --git a/applications/tari_console_wallet/src/init/mod.rs b/applications/tari_console_wallet/src/init/mod.rs index e57983b231..19f7fe7e79 100644 --- a/applications/tari_console_wallet/src/init/mod.rs +++ b/applications/tari_console_wallet/src/init/mod.rs @@ -30,11 +30,12 @@ use tari_common::{exit_codes::ExitCodes, ConfigBootstrap, GlobalConfig}; use tari_comms::{ multiaddr::Multiaddr, peer_manager::{Peer, PeerFeatures}, - types::CommsSecretKey, + types::CommsPublicKey, NodeIdentity, }; use tari_comms_dht::{store_forward::SafConfig, DbConnectionUrl, DhtConfig}; use tari_core::transactions::CryptoFactories; +use tari_crypto::keys::PublicKey; use tari_key_manager::cipher_seed::CipherSeed; use tari_p2p::{ auto_update::AutoUpdateConfig, @@ -50,6 +51,7 @@ use tari_wallet::{ output_manager_service::config::OutputManagerServiceConfig, storage::{database::WalletDatabase, sqlite_utilities::initialize_sqlite_database_backends}, transaction_service::config::{TransactionRoutingMechanism, TransactionServiceConfig}, + wallet::{derive_comms_secret_key, read_or_create_master_seed}, Wallet, WalletConfig, WalletSqlite, @@ -309,24 +311,51 @@ pub async fn init_wallet( "Databases Initialized. Wallet encrypted? {}.", wallet_encrypted ); - let node_address = match wallet_db.get_node_address().await? { - None => match config.public_address.clone() { - Some(val) => val, + let node_address = match config.public_address.clone() { + Some(addr) => addr, + None => match wallet_db.get_node_address().await? { + Some(addr) => addr, None => Multiaddr::empty(), }, - Some(a) => a, }; - let node_features = match wallet_db.get_node_features().await? { - None => PeerFeatures::COMMUNICATION_CLIENT, - Some(nf) => nf, - }; + let node_features = wallet_db + .get_node_features() + .await? + .unwrap_or(PeerFeatures::COMMUNICATION_CLIENT); + + let identity_sig = wallet_db.get_comms_identity_signature().await?; + + let master_seed = read_or_create_master_seed(recovery_seed.clone(), &wallet_db).await?; + let comms_secret_key = derive_comms_secret_key(&master_seed)?; - let node_identity = Arc::new(NodeIdentity::new( - CommsSecretKey::default(), + // This checks if anything has changed by validating the previous signature and if invalid, setting identity_sig to + // None + let identity_sig = identity_sig.filter(|sig| { + let comms_public_key = CommsPublicKey::from_secret_key(&comms_secret_key); + sig.is_valid(&comms_public_key, node_features, [&node_address]) + }); + + // SAFETY: we are manually checking the validity of this signature before adding Some(..) + let node_identity = Arc::new(NodeIdentity::with_signature_unchecked( + comms_secret_key, node_address, node_features, + identity_sig, )); + if !node_identity.is_signed() { + node_identity.sign(); + // unreachable panic: signed above + wallet_db + .set_comms_identity_signature( + node_identity + .identity_signature_read() + .as_ref() + .expect("unreachable panic") + .clone(), + ) + .await?; + } let transport_type = create_transport_type(config); let transport_type = match transport_type { @@ -425,7 +454,7 @@ pub async fn init_wallet( output_manager_backend, contacts_backend, shutdown_signal, - recovery_seed.clone(), + master_seed, ) .await .map_err(|e| { @@ -499,11 +528,10 @@ pub async fn start_wallet( let net_address = base_node .addresses .first() - .ok_or_else(|| ExitCodes::ConfigError("Configured base node has no address!".to_string()))? - .to_string(); + .ok_or_else(|| ExitCodes::ConfigError("Configured base node has no address!".to_string()))?; wallet - .set_base_node_peer(base_node.public_key.clone(), net_address) + .set_base_node_peer(base_node.public_key.clone(), net_address.address.clone()) .await .map_err(|e| ExitCodes::WalletError(format!("Error setting wallet base node peer. {}", e)))?; diff --git a/applications/tari_console_wallet/src/ui/state/app_state.rs b/applications/tari_console_wallet/src/ui/state/app_state.rs index c11990e538..8a77875a53 100644 --- a/applications/tari_console_wallet/src/ui/state/app_state.rs +++ b/applications/tari_console_wallet/src/ui/state/app_state.rs @@ -762,7 +762,7 @@ impl AppStateInner { let peer_manager = self.wallet.comms.peer_manager(); let mut peers = Vec::with_capacity(connections.len()); for c in connections.iter() { - if let Ok(p) = peer_manager.find_by_node_id(c.peer_node_id()).await { + if let Ok(Some(p)) = peer_manager.find_by_node_id(c.peer_node_id()).await { peers.push(p); } } @@ -843,7 +843,7 @@ impl AppStateInner { self.wallet .set_base_node_peer( peer.public_key.clone(), - peer.clone().addresses.first().ok_or(UiError::NoAddress)?.to_string(), + peer.addresses.first().ok_or(UiError::NoAddress)?.address.clone(), ) .await?; @@ -867,7 +867,7 @@ impl AppStateInner { self.wallet .set_base_node_peer( peer.public_key.clone(), - peer.clone().addresses.first().ok_or(UiError::NoAddress)?.to_string(), + peer.addresses.first().ok_or(UiError::NoAddress)?.address.clone(), ) .await?; @@ -908,7 +908,7 @@ impl AppStateInner { self.wallet .set_base_node_peer( previous.public_key.clone(), - previous.addresses.first().ok_or(UiError::NoAddress)?.to_string(), + previous.addresses.first().ok_or(UiError::NoAddress)?.address.clone(), ) .await?; diff --git a/base_layer/p2p/src/initialization.rs b/base_layer/p2p/src/initialization.rs index dafdf1f62f..ee04b6ab4b 100644 --- a/base_layer/p2p/src/initialization.rs +++ b/base_layer/p2p/src/initialization.rs @@ -541,7 +541,7 @@ impl P2pInitializer { #[async_trait] impl ServiceInitializer for P2pInitializer { async fn initialize(&mut self, context: ServiceInitializerContext) -> Result<(), ServiceInitializationError> { - let config = self.config.clone(); + let mut config = self.config.clone(); let connector = self.connector.take().expect("P2pInitializer called more than once"); let mut builder = CommsBuilder::new() @@ -557,6 +557,8 @@ impl ServiceInitializer for P2pInitializer { if config.allow_test_addresses { builder = builder.allow_test_addresses(); } + // Ensure this setting always matches + config.dht.allow_test_addresses = config.allow_test_addresses; let (comms, dht) = configure_comms_and_dht(builder, &config, connector).await?; diff --git a/base_layer/wallet/src/error.rs b/base_layer/wallet/src/error.rs index c6ae2f5fa8..4145d8a08e 100644 --- a/base_layer/wallet/src/error.rs +++ b/base_layer/wallet/src/error.rs @@ -171,6 +171,8 @@ pub enum WalletStorageError { DeprecatedOperation, #[error("Key Manager Error: `{0}`")] KeyManagerError(#[from] KeyManagerError), + #[error("Recovery Seed Error: {0}")] + RecoverySeedError(String), } impl From for ExitCodes { diff --git a/base_layer/wallet/src/storage/database.rs b/base_layer/wallet/src/storage/database.rs index 7e9b926aeb..437637ca8d 100644 --- a/base_layer/wallet/src/storage/database.rs +++ b/base_layer/wallet/src/storage/database.rs @@ -28,7 +28,11 @@ use std::{ use aes_gcm::Aes256Gcm; use log::*; use tari_common_types::chain_metadata::ChainMetadata; -use tari_comms::{multiaddr::Multiaddr, peer_manager::PeerFeatures, tor::TorIdentity}; +use tari_comms::{ + multiaddr::Multiaddr, + peer_manager::{IdentitySignature, PeerFeatures}, + tor::TorIdentity, +}; use tari_key_manager::cipher_seed::CipherSeed; use crate::{error::WalletStorageError, utxo_scanner_service::service::ScannedBlock}; @@ -64,6 +68,7 @@ pub trait WalletBackend: Send + Sync + Clone { pub enum DbKey { CommsAddress, CommsFeatures, + CommsIdentitySignature, TorId, BaseNodeChainMetadata, ClientKey(String), @@ -76,6 +81,7 @@ pub enum DbKey { pub enum DbValue { CommsAddress(Multiaddr), CommsFeatures(PeerFeatures), + CommsIdentitySignature(Box), TorId(TorIdentity), ClientValue(String), ValueCleared, @@ -94,6 +100,7 @@ pub enum DbKeyValuePair { MasterSeed(CipherSeed), CommsAddress(Multiaddr), CommsFeatures(PeerFeatures), + CommsIdentitySignature(Box), } pub enum WriteOperation { @@ -217,6 +224,33 @@ where T: WalletBackend + 'static Ok(()) } + pub async fn get_comms_identity_signature(&self) -> Result, WalletStorageError> { + let db = self.db.clone(); + + let sig = tokio::task::spawn_blocking(move || match db.fetch(&DbKey::CommsIdentitySignature) { + Ok(None) => Ok(None), + Ok(Some(DbValue::CommsIdentitySignature(k))) => Ok(Some(*k)), + Ok(Some(other)) => unexpected_result(DbKey::CommsIdentitySignature, other), + Err(e) => log_error(DbKey::CommsIdentitySignature, e), + }) + .await + .map_err(|err| WalletStorageError::BlockingTaskSpawnError(err.to_string()))??; + Ok(sig) + } + + pub async fn set_comms_identity_signature(&self, sig: IdentitySignature) -> Result<(), WalletStorageError> { + let db = self.db.clone(); + + tokio::task::spawn_blocking(move || { + db.write(WriteOperation::Insert(DbKeyValuePair::CommsIdentitySignature( + Box::new(sig), + ))) + }) + .await + .map_err(|err| WalletStorageError::BlockingTaskSpawnError(err.to_string()))??; + Ok(()) + } + pub async fn get_chain_metadata(&self) -> Result, WalletStorageError> { let db_clone = self.db.clone(); @@ -398,15 +432,16 @@ where T: WalletBackend + 'static impl Display for DbKey { fn fmt(&self, f: &mut Formatter) -> Result<(), Error> { match self { - DbKey::MasterSeed => f.write_str(&"MasterSeed".to_string()), - DbKey::CommsAddress => f.write_str(&"CommsAddress".to_string()), - DbKey::CommsFeatures => f.write_str(&"Node features".to_string()), - DbKey::TorId => f.write_str(&"TorId".to_string()), + DbKey::MasterSeed => f.write_str("MasterSeed"), + DbKey::CommsAddress => f.write_str("CommsAddress"), + DbKey::CommsFeatures => f.write_str("Nod features"), + DbKey::TorId => f.write_str("TorId"), DbKey::ClientKey(k) => f.write_str(&format!("ClientKey: {:?}", k)), - DbKey::BaseNodeChainMetadata => f.write_str(&"Last seen Chain metadata from base node".to_string()), - DbKey::PassphraseHash => f.write_str(&"PassphraseHash".to_string()), - DbKey::EncryptionSalt => f.write_str(&"EncryptionSalt".to_string()), - DbKey::WalletBirthday => f.write_str(&"WalletBirthday".to_string()), + DbKey::BaseNodeChainMetadata => f.write_str("Last seen Chain metadata from basw node"), + DbKey::PassphraseHash => f.write_str("PassphraseHash"), + DbKey::EncryptionSalt => f.write_str("EncryptionSalt"), + DbKey::WalletBirthday => f.write_str("WalletBirthday"), + DbKey::CommsIdentitySignature => f.write_str("CommsIdentitySignature"), } } } @@ -416,14 +451,15 @@ impl Display for DbValue { match self { DbValue::MasterSeed(k) => f.write_str(&format!("MasterSeed: {:?}", k)), DbValue::ClientValue(v) => f.write_str(&format!("ClientValue: {:?}", v)), - DbValue::ValueCleared => f.write_str(&"ValueCleared".to_string()), - DbValue::CommsFeatures(_) => f.write_str(&"Node features".to_string()), - DbValue::CommsAddress(_) => f.write_str(&"Comms Address".to_string()), + DbValue::ValueCleared => f.write_str("ValueCleared"), + DbValue::CommsFeatures(_) => f.write_str("Node features"), + DbValue::CommsAddress(_) => f.write_str("Comms Address"), DbValue::TorId(v) => f.write_str(&format!("Tor ID: {}", v)), DbValue::BaseNodeChainMetadata(v) => f.write_str(&format!("Last seen Chain metadata from base node:{}", v)), DbValue::PassphraseHash(h) => f.write_str(&format!("PassphraseHash: {}", h)), DbValue::EncryptionSalt(s) => f.write_str(&format!("EncryptionSalt: {}", s)), DbValue::WalletBirthday(b) => f.write_str(&format!("WalletBirthday: {}", b)), + DbValue::CommsIdentitySignature(_) => f.write_str("CommsIdentitySignature"), } } } diff --git a/base_layer/wallet/src/storage/sqlite_db/wallet.rs b/base_layer/wallet/src/storage/sqlite_db/wallet.rs index 50a974ad25..36af04c23d 100644 --- a/base_layer/wallet/src/storage/sqlite_db/wallet.rs +++ b/base_layer/wallet/src/storage/sqlite_db/wallet.rs @@ -37,7 +37,11 @@ use argon2::{ use diesel::{prelude::*, SqliteConnection}; use log::*; use tari_common_types::chain_metadata::ChainMetadata; -use tari_comms::{multiaddr::Multiaddr, peer_manager::PeerFeatures, tor::TorIdentity}; +use tari_comms::{ + multiaddr::Multiaddr, + peer_manager::{IdentitySignature, PeerFeatures}, + tor::TorIdentity, +}; use tari_crypto::tari_utilities::{ hex::{from_hex, Hex}, message_format::MessageFormat, @@ -263,6 +267,14 @@ impl WalletSqliteDatabase { kvp_text = "CommsFeatures"; WalletSettingSql::new(DbKey::CommsFeatures.to_string(), cf.bits().to_string()).set(&conn)?; }, + DbKeyValuePair::CommsIdentitySignature(identity_sig) => { + kvp_text = "CommsIdentitySignature"; + WalletSettingSql::new( + DbKey::CommsIdentitySignature.to_string(), + identity_sig.to_bytes().to_hex(), + ) + .set(&conn)?; + }, } if start.elapsed().as_millis() > 0 { trace!( @@ -290,25 +302,16 @@ impl WalletSqliteDatabase { return Ok(Some(DbValue::ValueCleared)); } }, - DbKey::CommsFeatures => { - return Err(WalletStorageError::OperationNotSupported); - }, - DbKey::CommsAddress => { - return Err(WalletStorageError::OperationNotSupported); - }, - DbKey::BaseNodeChainMetadata => { - return Err(WalletStorageError::OperationNotSupported); - }, DbKey::TorId => { let _ = WalletSettingSql::clear(DbKey::TorId.to_string(), &conn)?; }, - DbKey::PassphraseHash => { - return Err(WalletStorageError::OperationNotSupported); - }, - DbKey::EncryptionSalt => { - return Err(WalletStorageError::OperationNotSupported); - }, - DbKey::WalletBirthday => { + DbKey::CommsFeatures | + DbKey::CommsAddress | + DbKey::BaseNodeChainMetadata | + DbKey::PassphraseHash | + DbKey::EncryptionSalt | + DbKey::WalletBirthday | + DbKey::CommsIdentitySignature => { return Err(WalletStorageError::OperationNotSupported); }, }; @@ -353,6 +356,11 @@ impl WalletBackend for WalletSqliteDatabase { DbKey::PassphraseHash => WalletSettingSql::get(key.to_string(), &conn)?.map(DbValue::PassphraseHash), DbKey::EncryptionSalt => WalletSettingSql::get(key.to_string(), &conn)?.map(DbValue::EncryptionSalt), DbKey::WalletBirthday => WalletSettingSql::get(key.to_string(), &conn)?.map(DbValue::WalletBirthday), + DbKey::CommsIdentitySignature => WalletSettingSql::get(key.to_string(), &conn)? + .and_then(|s| from_hex(&s).ok()) + .and_then(|bytes| IdentitySignature::from_bytes(&bytes).ok()) + .map(Box::new) + .map(DbValue::CommsIdentitySignature), }; if start.elapsed().as_millis() > 0 { trace!( diff --git a/base_layer/wallet/src/transaction_service/protocols/transaction_send_protocol.rs b/base_layer/wallet/src/transaction_service/protocols/transaction_send_protocol.rs index af1f0eba43..b09da36c21 100644 --- a/base_layer/wallet/src/transaction_service/protocols/transaction_send_protocol.rs +++ b/base_layer/wallet/src/transaction_service/protocols/transaction_send_protocol.rs @@ -658,16 +658,17 @@ where ) .await; }, + Ok(SendMessageResponse::Failed(e)) => warn!( + target: LOG_TARGET, + "Failed to send message ({}) for TxId: {}", e, self.id + ), + Ok(SendMessageResponse::PendingDiscovery(_)) => unreachable!(), Err(e) => { warn!( target: LOG_TARGET, "Error waiting for Discovery while sending message to TxId: {} {:?}", self.id, e ); }, - _ => warn!( - target: LOG_TARGET, - "Empty message received waiting for Discovery to complete TxId: {}", self.id, - ), } }, }, diff --git a/base_layer/wallet/src/transaction_service/tasks/send_finalized_transaction.rs b/base_layer/wallet/src/transaction_service/tasks/send_finalized_transaction.rs index 20436fd7ae..efae1b0944 100644 --- a/base_layer/wallet/src/transaction_service/tasks/send_finalized_transaction.rs +++ b/base_layer/wallet/src/transaction_service/tasks/send_finalized_transaction.rs @@ -188,16 +188,18 @@ pub async fn send_finalized_transaction_message_direct( ) .await; }, + + Ok(SendMessageResponse::Failed(e)) => warn!( + target: LOG_TARGET, + "Failed to send message ({}) Discovery failed for TxId: {}", e, tx_id + ), + Ok(SendMessageResponse::PendingDiscovery(_)) => unreachable!(), Err(e) => { warn!( target: LOG_TARGET, "Error waiting for Discovery while sending message to TxId: {} {:?}", tx_id, e ); }, - _ => warn!( - target: LOG_TARGET, - "Empty response received waiting for Discovery to complete TxId: {}", tx_id - ), } }, }, diff --git a/base_layer/wallet/src/transaction_service/tasks/send_transaction_reply.rs b/base_layer/wallet/src/transaction_service/tasks/send_transaction_reply.rs index c40fefec51..a8d4ee6a7c 100644 --- a/base_layer/wallet/src/transaction_service/tasks/send_transaction_reply.rs +++ b/base_layer/wallet/src/transaction_service/tasks/send_transaction_reply.rs @@ -170,16 +170,18 @@ pub async fn send_transaction_reply_direct( ) .await; }, + + Ok(SendMessageResponse::Failed(e)) => warn!( + target: LOG_TARGET, + "Failed to send message ({}) Discovery failed for TxId: {}", e, tx_id + ), + Ok(SendMessageResponse::PendingDiscovery(_)) => unreachable!(), Err(e) => { debug!( target: LOG_TARGET, "Error waiting for Discovery while sending message to TxId: {} {:?}", tx_id, e ); }, - _ => debug!( - target: LOG_TARGET, - "Empty message received waiting for Discovery to complete TxId: {}", tx_id - ), } }, }, diff --git a/base_layer/wallet/src/wallet.rs b/base_layer/wallet/src/wallet.rs index cff8e3a392..8594ca291e 100644 --- a/base_layer/wallet/src/wallet.rs +++ b/base_layer/wallet/src/wallet.rs @@ -128,26 +128,13 @@ where output_manager_backend: V, contacts_backend: W, shutdown_signal: ShutdownSignal, - recovery_seed: Option, + master_seed: CipherSeed, ) -> Result { - let master_seed = read_or_create_master_seed(recovery_seed, &mut wallet_database.clone()).await?; - let comms_secret_key = derive_comms_secret_key(&master_seed)?; - - let node_identity = Arc::new(NodeIdentity::new( - comms_secret_key, - config.comms_config.node_identity.public_address(), - config.comms_config.node_identity.features(), - )); - - let mut comms_config = config.comms_config.clone(); - comms_config.node_identity = node_identity.clone(); - - let bn_service_db = wallet_database.clone(); - let factories = config.factories.clone(); let (publisher, subscription_factory) = pubsub_connector(config.buffer_size, config.rate_limit); let peer_message_subscription_factory = Arc::new(subscription_factory); let transport_type = config.comms_config.transport_type.clone(); + let node_identity = config.comms_config.node_identity.clone(); debug!(target: LOG_TARGET, "Wallet Initializing"); info!( @@ -169,7 +156,7 @@ where config.rate_limit ); let stack = StackBuilder::new(shutdown_signal) - .add_initializer(P2pInitializer::new(comms_config, publisher)) + .add_initializer(P2pInitializer::new(config.comms_config.clone(), publisher)) .add_initializer(OutputManagerServiceInitializer::new( config.output_manager_service_config.unwrap_or_default(), output_manager_backend.clone(), @@ -189,7 +176,7 @@ where .add_initializer(ContactsServiceInitializer::new(contacts_backend)) .add_initializer(BaseNodeServiceInitializer::new( config.base_node_service_config.clone(), - bn_service_db, + wallet_database.clone(), )) .add_initializer(WalletConnectivityInitializer::new(config.base_node_service_config)) .add_initializer(UtxoScannerServiceInitializer::new( @@ -287,33 +274,55 @@ where pub async fn set_base_node_peer( &mut self, public_key: CommsPublicKey, - net_address: String, + address: Multiaddr, ) -> Result<(), WalletError> { info!( "Wallet setting base node peer, public key: {}, net address: {}.", - public_key, net_address - ); - - let address = net_address.parse::()?; - let peer = Peer::new( - public_key.clone(), - NodeId::from_key(&public_key), - vec![address].into(), - PeerFlags::empty(), - PeerFeatures::COMMUNICATION_NODE, - Default::default(), - String::new(), + public_key, address ); - self.comms.peer_manager().add_peer(peer.clone()).await?; if let Some(current_node) = self.wallet_connectivity.get_current_base_node_id() { self.comms .connectivity() .remove_peer_from_allow_list(current_node) .await?; } - self.wallet_connectivity.set_base_node(peer.clone()); - self.comms.connectivity().add_peer_to_allow_list(peer.node_id).await?; + + let addresses = vec![address].into(); + let peer_manager = self.comms.peer_manager(); + let mut connectivity = self.comms.connectivity(); + if let Some(mut current_peer) = peer_manager.find_by_public_key(&public_key).await? { + // Only invalidate the identity signature if addresses are different + if current_peer.addresses != addresses { + info!( + target: LOG_TARGET, + "Address for base node differs from storage. Was {}, setting to {}", + current_peer.addresses, + addresses + ); + current_peer.update(Some(addresses.into_vec()), None, None, None, None, None, None); + peer_manager.add_peer(current_peer.clone()).await?; + } + connectivity + .add_peer_to_allow_list(current_peer.node_id.clone()) + .await?; + self.wallet_connectivity.set_base_node(current_peer); + } else { + let node_id = NodeId::from_key(&public_key); + let peer = Peer::new( + public_key, + node_id, + addresses, + PeerFlags::empty(), + PeerFeatures::COMMUNICATION_NODE, + Default::default(), + String::new(), + ); + peer_manager.add_peer(peer.clone()).await?; + connectivity.add_peer_to_allow_list(peer.node_id.clone()).await?; + self.wallet_connectivity.set_base_node(peer); + } + Ok(()) } @@ -522,9 +531,9 @@ where } } -async fn read_or_create_master_seed( +pub async fn read_or_create_master_seed( recovery_seed: Option, - db: &mut WalletDatabase, + db: &WalletDatabase, ) -> Result { let db_master_seed = db.get_master_seed().await?; @@ -555,7 +564,7 @@ async fn read_or_create_master_seed( Ok(master_seed) } -fn derive_comms_secret_key(master_seed: &CipherSeed) -> Result { +pub fn derive_comms_secret_key(master_seed: &CipherSeed) -> Result { let comms_key_manager = KeyManager::::from( master_seed.clone(), KEY_MANAGER_COMMS_SECRET_KEY_BRANCH_KEY.to_string(), diff --git a/base_layer/wallet/tests/wallet.rs b/base_layer/wallet/tests/wallet.rs index ce7dc5dab2..ca8affed44 100644 --- a/base_layer/wallet/tests/wallet.rs +++ b/base_layer/wallet/tests/wallet.rs @@ -89,6 +89,7 @@ use tari_wallet::{ handle::TransactionEvent, storage::sqlite_db::TransactionServiceSqliteDatabase, }, + wallet::read_or_create_master_seed, Wallet, WalletConfig, WalletSqlite, @@ -181,14 +182,17 @@ async fn create_wallet( let _ = wallet_backend.write(WriteOperation::Insert(DbKeyValuePair::BaseNodeChainMetadata(metadata))); + let wallet_db = WalletDatabase::new(wallet_backend); + let master_seed = read_or_create_master_seed(recovery_seed, &wallet_db).await?; + Wallet::start( config, - WalletDatabase::new(wallet_backend), + wallet_db, transaction_backend, output_manager_backend, contacts_backend, shutdown_signal, - recovery_seed, + master_seed, ) .await } @@ -252,7 +256,7 @@ async fn test_wallet() { alice_wallet .set_base_node_peer( (*base_node_identity.public_key()).clone(), - get_next_memory_address().to_string(), + base_node_identity.public_address().clone(), ) .await .unwrap(); @@ -277,11 +281,11 @@ async fn test_wallet() { let delay = sleep(Duration::from_secs(60)); tokio::pin!(delay); - let mut reply_count = false; + let mut received_reply = false; loop { tokio::select! { event = alice_event_stream.recv() => if let TransactionEvent::ReceivedTransactionReply(_) = &*event.unwrap() { - reply_count = true; + received_reply = true; break; }, () = &mut delay => { @@ -289,7 +293,7 @@ async fn test_wallet() { }, } } - assert!(reply_count); + assert!(received_reply); let mut contacts = Vec::new(); for i in 0..2 { @@ -726,6 +730,7 @@ async fn test_import_utxo() { None, None, ); + let mut alice_wallet = Wallet::start( config, WalletDatabase::new(WalletSqliteDatabase::new(connection.clone(), None).unwrap()), @@ -733,7 +738,7 @@ async fn test_import_utxo() { OutputManagerSqliteDatabase::new(connection.clone(), None), ContactsServiceSqliteDatabase::new(connection), shutdown.to_signal(), - None, + CipherSeed::new(), ) .await .unwrap(); diff --git a/base_layer/wallet_ffi/src/lib.rs b/base_layer/wallet_ffi/src/lib.rs index bcd4ad62e9..920d9da16b 100644 --- a/base_layer/wallet_ffi/src/lib.rs +++ b/base_layer/wallet_ffi/src/lib.rs @@ -117,7 +117,7 @@ use tari_comms::{ socks, tor, transports::MemoryTransport, - types::CommsSecretKey, + types::{CommsPublicKey, CommsSecretKey}, }; use tari_comms_dht::{store_forward::SafConfig, DbConnectionUrl, DhtConfig}; use tari_core::transactions::{tari_amount::MicroTari, transaction::OutputFeatures, CryptoFactories}; @@ -152,6 +152,7 @@ use tari_wallet::{ }, }, utxo_scanner_service::{service::UtxoScannerService, RECOVERY_KEY}, + wallet::{derive_comms_secret_key, read_or_create_master_seed}, Wallet, WalletConfig, WalletSqlite, @@ -207,6 +208,9 @@ pub struct EmojiSet(Vec); #[derive(Debug, PartialEq)] pub struct TariSeedWords(Vec); +#[derive(Debug, PartialEq)] +pub struct TariPublicKeys(Vec); + pub struct TariWallet { wallet: WalletSqlite, runtime: Runtime, @@ -530,6 +534,23 @@ pub unsafe extern "C" fn public_key_destroy(pk: *mut TariPublicKey) { } } +/// Frees memory for TariPublicKeys +/// +/// ## Arguments +/// `pks` - The pointer to TariPublicKeys +/// +/// ## Returns +/// `()` - Does not return a value, equivalent to void in C +/// +/// # Safety +/// None +#[no_mangle] +pub unsafe extern "C" fn public_keys_destroy(pks: *mut TariPublicKeys) { + if !pks.is_null() { + Box::from_raw(pks); + } +} + /// Gets a ByteVector from a TariPublicKey /// /// ## Arguments @@ -2867,7 +2888,7 @@ pub unsafe extern "C" fn comms_config_create( Ok(selected_network) => { match public_address { Ok(public_address) => { - let ni = NodeIdentity::new( + let node_identity = NodeIdentity::new( CommsSecretKey::default(), public_address, PeerFeatures::COMMUNICATION_CLIENT, @@ -2875,7 +2896,7 @@ pub unsafe extern "C" fn comms_config_create( let config = TariCommsConfig { network: selected_network, - node_identity: Arc::new(ni), + node_identity: Arc::new(node_identity), transport_type: (*transport_type).clone(), auxilary_tcp_listener_address: None, datastore_path, @@ -2940,6 +2961,53 @@ pub unsafe extern "C" fn comms_config_destroy(wc: *mut TariCommsConfig) { } } +/// This function lists the public keys of all connected peers +/// +/// ## Arguments +/// `wallet` - The TariWallet pointer +/// `error_out` - Pointer to an int which will be modified to an error code should one occur, may not be null. Functions +/// as an out parameter. +/// +/// ## Returns +/// `TariPublicKeys` - Returns a list of connected public keys. Note the result will be null if there was an error +/// +/// # Safety +/// The caller is responsible for null checking and deallocating the returned object using public_keys_destroy. +#[no_mangle] +pub unsafe extern "C" fn comms_list_connected_public_keys( + wallet: *mut TariWallet, + error_out: *mut c_int, +) -> *mut TariPublicKeys { + let mut error = 0; + ptr::swap(error_out, &mut error as *mut c_int); + if wallet.is_null() { + error = LibWalletError::from(InterfaceError::NullError("wallet".to_string())).code; + ptr::swap(error_out, &mut error as *mut c_int); + return ptr::null_mut(); + } + + let mut connectivity = (*wallet).wallet.comms.connectivity(); + let peer_manager = (*wallet).wallet.comms.peer_manager(); + + match (*wallet).runtime.block_on(async move { + let connections = connectivity.get_active_connections().await?; + let mut public_keys = Vec::with_capacity(connections.len()); + for conn in connections { + if let Some(peer) = peer_manager.find_by_node_id(conn.peer_node_id()).await? { + public_keys.push(peer.public_key); + } + } + Result::<_, WalletError>::Ok(public_keys) + }) { + Ok(public_keys) => Box::into_raw(Box::new(TariPublicKeys(public_keys))), + Err(e) => { + error = LibWalletError::from(e).code; + ptr::swap(error_out, &mut error as *mut c_int); + ptr::null_mut() + }, + } +} + /// ---------------------------------------------------------------------------------------------- /// /// ------------------------------------- Wallet -------------------------------------------------/// @@ -3202,6 +3270,58 @@ pub unsafe extern "C" fn wallet_create( _ => comms_config.transport_type, }; + let result = runtime.block_on(async { + let master_seed = read_or_create_master_seed(recovery_seed, &wallet_database) + .await + .map_err(|err| WalletStorageError::RecoverySeedError(err.to_string()))?; + let comms_secret_key = derive_comms_secret_key(&master_seed) + .map_err(|err| WalletStorageError::RecoverySeedError(err.to_string()))?; + let node_features = comms_config.node_identity.features(); + let node_address = comms_config.node_identity.public_address(); + let identity_sig = wallet_database.get_comms_identity_signature().await?; + + // This checks if anything has changed by validating the previous signature and if invalid, setting identity_sig + // to None + let identity_sig = identity_sig.filter(|sig| { + let comms_public_key = CommsPublicKey::from_secret_key(&comms_secret_key); + sig.is_valid(&comms_public_key, node_features, [&node_address]) + }); + + // SAFETY: we are manually checking the validity of this signature before adding Some(..) + let node_identity = Arc::new(NodeIdentity::with_signature_unchecked( + comms_secret_key, + node_address, + node_features, + identity_sig, + )); + if !node_identity.is_signed() { + node_identity.sign(); + // unreachable panic: signed above + wallet_database + .set_comms_identity_signature( + node_identity + .identity_signature_read() + .as_ref() + .expect("unreachable panic") + .clone(), + ) + .await?; + } + Ok((master_seed, node_identity)) + }); + let master_seed; + match result { + Ok((seed, node_identity)) => { + master_seed = seed; + comms_config.node_identity = node_identity; + }, + Err(e) => { + error = LibWalletError::from(WalletError::WalletStorageError(e)).code; + ptr::swap(error_out, &mut error as *mut c_int); + return ptr::null_mut(); + }, + } + let shutdown = Shutdown::new(); let wallet_config = WalletConfig::new( comms_config, @@ -3233,7 +3353,7 @@ pub unsafe extern "C" fn wallet_create( output_manager_backend, contacts_backend, shutdown.to_signal(), - recovery_seed, + master_seed, )); match w { @@ -3532,11 +3652,19 @@ pub unsafe extern "C" fn wallet_add_base_node_peer( return false; } - let address_string; + let parsed_addr; if !address.is_null() { match CStr::from_ptr(address).to_str() { Ok(v) => { - address_string = v.to_owned(); + parsed_addr = match Multiaddr::from_str(v) { + Ok(v) => v, + Err(_) => { + error = LibWalletError::from(InterfaceError::InvalidArgument("address is invalid".to_string())) + .code; + ptr::swap(error_out, &mut error as *mut c_int); + return false; + }, + } }, _ => { error = LibWalletError::from(InterfaceError::PointerError("address".to_string())).code; @@ -3550,11 +3678,10 @@ pub unsafe extern "C" fn wallet_add_base_node_peer( return false; } - if let Err(e) = (*wallet).runtime.block_on( - (*wallet) - .wallet - .set_base_node_peer((*public_key).clone(), address_string), - ) { + if let Err(e) = (*wallet) + .runtime + .block_on((*wallet).wallet.set_base_node_peer((*public_key).clone(), parsed_addr)) + { error = LibWalletError::from(e).code; ptr::swap(error_out, &mut error as *mut c_int); return false; diff --git a/base_layer/wallet_ffi/wallet.h b/base_layer/wallet_ffi/wallet.h index b13281dc9b..eb78962bf6 100644 --- a/base_layer/wallet_ffi/wallet.h +++ b/base_layer/wallet_ffi/wallet.h @@ -51,6 +51,8 @@ struct TariWallet; struct TariPublicKey; +struct TariPublicKeys; + struct TariContacts; struct TariContact; @@ -137,12 +139,16 @@ struct TariPublicKey *public_key_from_hex(const char *hex, int *error_out); // Frees memory for a TariPublicKey pointer void public_key_destroy(struct TariPublicKey *pk); +// Frees memory for a TariPublicKeys pointer +void public_keys_destroy(struct TariPublicKeys *pk); + //Converts a TariPublicKey to char array in emoji format char *public_key_to_emoji_id(struct TariPublicKey *pk, int *error_out); // Converts a char array in emoji format to a public key struct TariPublicKey *emoji_id_to_public_key(const char *emoji, int *error_out); + /// -------------------------------- TariPrivateKey ----------------------------------------------- /// // Creates a TariPrivateKey from a ByteVector @@ -397,6 +403,8 @@ struct TariCommsConfig *comms_config_create(const char *public_address, // Frees memory for a TariCommsConfig void comms_config_destroy(struct TariCommsConfig *wc); +// Converts a char array in emoji format to a public key +struct TariPublicKeys *comms_list_connected_public_keys(struct TariWallet *wallet, int *error_out); /// -------------------------------- TariWallet ----------------------------------------------- // /// Creates a TariWallet diff --git a/common/config/presets/base_node.toml b/common/config/presets/base_node.toml index 88b234ed1d..aa1e51f6f3 100644 --- a/common/config/presets/base_node.toml +++ b/common/config/presets/base_node.toml @@ -146,7 +146,7 @@ console_wallet_tor_identity_file = "config/console_wallet_tor.json" # direcly over TCP. /ip4, /ip6, /dns, /dns4 and /dns6 are supported. # tor_proxy_bypass_addresses = ["/dns4/my-foo-base-node/tcp/9998"] # When using the tor transport and set to true, outbound TCP connections bypass the tor proxy. Defaults to false for better privacy -# tor_proxy_bypass_for_outbound_tcp = false; +# tor_proxy_bypass_for_outbound_tcp = false ######################################################################################################################## # # @@ -321,7 +321,7 @@ console_wallet_tor_identity_file = "config/console_wallet_tor.json" # direcly over TCP. /ip4, /ip6, /dns, /dns4 and /dns6 are supported. # tor_proxy_bypass_addresses = ["/dns4/my-foo-base-node/tcp/9998"] # When using the tor transport and set to true, outbound TCP connections bypass the tor proxy. Defaults to false for better privacy -# tor_proxy_bypass_for_outbound_tcp = false; +# tor_proxy_bypass_for_outbound_tcp = false ######################################################################################################################## # # diff --git a/common/config/presets/console_wallet.toml b/common/config/presets/console_wallet.toml index 7c5ea1dc59..21f2613016 100644 --- a/common/config/presets/console_wallet.toml +++ b/common/config/presets/console_wallet.toml @@ -151,7 +151,7 @@ tor_control_auth = "none" # or "password=xxxxxx" # direcly over TCP. /ip4, /ip6, /dns, /dns4 and /dns6 are supported. # tor_proxy_bypass_addresses = ["/dns4/my-foo-base-node/tcp/9998"] # When using the tor transport and set to true, outbound TCP connections bypass the tor proxy. Defaults to false for better privacy -# tor_proxy_bypass_for_outbound_tcp = false; +# tor_proxy_bypass_for_outbound_tcp = false # Wallet configuration options for igor [wallet.igor] @@ -201,7 +201,7 @@ tor_control_auth = "none" # or "password=xxxxxx" # direcly over TCP. /ip4, /ip6, /dns, /dns4 and /dns6 are supported. # tor_proxy_bypass_addresses = ["/dns4/my-foo-base-node/tcp/9998"] # When using the tor transport and set to true, outbound TCP connections bypass the tor proxy. Defaults to false for better privacy -# tor_proxy_bypass_for_outbound_tcp = false; +# tor_proxy_bypass_for_outbound_tcp = false diff --git a/comms/Cargo.toml b/comms/Cargo.toml index d684aca85f..c7a77d20bd 100644 --- a/comms/Cargo.toml +++ b/comms/Cargo.toml @@ -35,6 +35,7 @@ once_cell = "1.8.0" openssl-sys = { version = "=0.9.66", features = ["vendored"], optional = true } pin-project = "1.0.8" prost = "=0.9.0" +prost-types = "0.9.0" rand = "0.8" serde = "1.0.119" serde_derive = "1.0.119" diff --git a/comms/dht/src/actor.rs b/comms/dht/src/actor.rs index 618a70b457..5dc0a61ada 100644 --- a/comms/dht/src/actor.rs +++ b/comms/dht/src/actor.rs @@ -207,7 +207,7 @@ pub struct DhtActor { database: DhtDatabase, outbound_requester: OutboundMessageRequester, connectivity: ConnectivityRequester, - config: DhtConfig, + config: Arc, shutdown_signal: ShutdownSignal, request_rx: mpsc::Receiver, msg_hash_dedup_cache: DedupCacheDatabase, @@ -216,7 +216,7 @@ pub struct DhtActor { impl DhtActor { #[allow(clippy::too_many_arguments)] pub fn new( - config: DhtConfig, + config: Arc, conn: DbConnection, node_identity: Arc, peer_manager: Arc, @@ -352,7 +352,7 @@ impl DhtActor { let connectivity = self.connectivity.clone(); let config = self.config.clone(); Box::pin(async move { - match Self::select_peers(config, node_identity, peer_manager, connectivity, broadcast_strategy) + match Self::select_peers(&config, node_identity, peer_manager, connectivity, broadcast_strategy) .await { Ok(peers) => reply_tx.send(peers).map_err(|_| DhtActorError::ReplyCanceled), @@ -414,7 +414,7 @@ impl DhtActor { } async fn select_peers( - config: DhtConfig, + config: &DhtConfig, node_identity: Arc, peer_manager: Arc, mut connectivity: ConnectivityRequester, @@ -667,7 +667,7 @@ impl DhtActor { async fn select_closest_node_connected( closest_request: Box, - config: DhtConfig, + config: &DhtConfig, mut connectivity: ConnectivityRequester, peer_manager: Arc, ) -> Result, DhtActorError> { @@ -818,10 +818,10 @@ mod test { // Note: This must be equal or larger than the minimum dedup cache capacity for DedupCacheDatabase let capacity = 10; let actor = DhtActor::new( - DhtConfig { + Arc::new(DhtConfig { dedup_cache_capacity: capacity, ..Default::default() - }, + }), db_connection().await, node_identity, peer_manager, diff --git a/comms/dht/src/connectivity/mod.rs b/comms/dht/src/connectivity/mod.rs index 47b7cb7ccc..855ee776b3 100644 --- a/comms/dht/src/connectivity/mod.rs +++ b/comms/dht/src/connectivity/mod.rs @@ -65,7 +65,7 @@ pub enum DhtConnectivityError { /// The DHT connectivity actor monitors the connectivity state (using `ConnectivityEvent`s) and attempts /// to maintain connectivity to the network as peers come and go. pub struct DhtConnectivity { - config: DhtConfig, + config: Arc, peer_manager: Arc, node_identity: Arc, connectivity: ConnectivityRequester, @@ -89,7 +89,7 @@ pub struct DhtConnectivity { impl DhtConnectivity { #[allow(clippy::too_many_arguments)] pub fn new( - config: DhtConfig, + config: Arc, peer_manager: Arc, node_identity: Arc, connectivity: ConnectivityRequester, diff --git a/comms/dht/src/connectivity/test.rs b/comms/dht/src/connectivity/test.rs index fdc9ace313..fd09986337 100644 --- a/comms/dht/src/connectivity/test.rs +++ b/comms/dht/src/connectivity/test.rs @@ -72,7 +72,7 @@ async fn setup( let (event_publisher, _) = broadcast::channel(1); let dht_connectivity = DhtConnectivity::new( - config, + Arc::new(config), peer_manager.clone(), node_identity.clone(), connectivity, @@ -229,8 +229,11 @@ async fn insert_neighbour() { let node_identities = ordered_node_identities_by_distance(node_identity.node_id(), 10, PeerFeatures::COMMUNICATION_NODE); - let (mut dht_connectivity, _, _, _, _, _) = setup(Default::default(), node_identity.clone(), vec![]).await; - dht_connectivity.config.num_neighbouring_nodes = 8; + let config = DhtConfig { + num_neighbouring_nodes: 8, + ..Default::default() + }; + let (mut dht_connectivity, _, _, _, _, _) = setup(config, node_identity.clone(), vec![]).await; let shuffled = { let mut v = node_identities.clone(); diff --git a/comms/dht/src/dht.rs b/comms/dht/src/dht.rs index 0672d66482..fcdb8826a8 100644 --- a/comms/dht/src/dht.rs +++ b/comms/dht/src/dht.rs @@ -87,7 +87,7 @@ pub struct Dht { /// Comms peer manager peer_manager: Arc, /// Dht configuration - config: DhtConfig, + config: Arc, /// Used to create a OutboundMessageRequester. outbound_tx: mpsc::Sender, /// Sender for DHT requests @@ -127,7 +127,7 @@ impl Dht { node_identity, peer_manager, metrics_collector, - config, + config: Arc::new(config), outbound_tx, dht_sender, saf_sender, @@ -331,8 +331,9 @@ impl Dht { self.saf_response_signal_sender.clone(), )) .layer(inbound::DhtHandlerLayer::new( - Arc::clone(&self.node_identity), - Arc::clone(&self.peer_manager), + self.config.clone(), + self.node_identity.clone(), + self.peer_manager.clone(), self.discovery_service_requester(), self.outbound_requester(), )) diff --git a/comms/dht/src/discovery/service.rs b/comms/dht/src/discovery/service.rs index f1038a552e..1d883e468d 100644 --- a/comms/dht/src/discovery/service.rs +++ b/comms/dht/src/discovery/service.rs @@ -64,7 +64,7 @@ impl DiscoveryRequestState { } pub struct DhtDiscoveryService { - config: DhtConfig, + config: Arc, node_identity: Arc, outbound_requester: OutboundMessageRequester, peer_manager: Arc, @@ -75,7 +75,7 @@ pub struct DhtDiscoveryService { impl DhtDiscoveryService { pub fn new( - config: DhtConfig, + config: Arc, node_identity: Arc, peer_manager: Arc, outbound_requester: OutboundMessageRequester, @@ -321,6 +321,7 @@ impl DhtDiscoveryService { addresses: vec![self.node_identity.public_address().to_string()], peer_features: self.node_identity.features().bits(), nonce, + identity_signature: self.node_identity.identity_signature_read().as_ref().map(Into::into), }; debug!( target: LOG_TARGET, @@ -374,7 +375,7 @@ mod test { let shutdown = Shutdown::new(); DhtDiscoveryService::new( - DhtConfig::default(), + Default::default(), node_identity, peer_manager, outbound_requester, diff --git a/comms/dht/src/envelope.rs b/comms/dht/src/envelope.rs index 33b9c14bfc..215290886c 100644 --- a/comms/dht/src/envelope.rs +++ b/comms/dht/src/envelope.rs @@ -49,9 +49,9 @@ pub(crate) fn datetime_to_timestamp(datetime: DateTime) -> Timestamp { } /// Utility function that converts a `prost::Timestamp` to a `chrono::DateTime` -pub(crate) fn timestamp_to_datetime(timestamp: Timestamp) -> DateTime { - let naive = NaiveDateTime::from_timestamp(timestamp.seconds, cmp::max(0, timestamp.nanos) as u32); - DateTime::from_utc(naive, Utc) +pub(crate) fn timestamp_to_datetime(timestamp: Timestamp) -> Option> { + let naive = NaiveDateTime::from_timestamp_opt(timestamp.seconds, cmp::max(0, timestamp.nanos) as u32)?; + Some(DateTime::from_utc(naive, Utc)) } /// Utility function that converts a `chrono::DateTime` to a `EpochTime` @@ -186,7 +186,7 @@ impl TryFrom for DhtMessageHeader { ) }; - let expires: Option> = header.expires.map(timestamp_to_datetime); + let expires = header.expires.and_then(timestamp_to_datetime); let version = DhtProtocolVersion::try_from((header.major, header.minor))?; Ok(Self { diff --git a/comms/dht/src/inbound/decryption.rs b/comms/dht/src/inbound/decryption.rs index 5220e8485d..a69685ab7e 100644 --- a/comms/dht/src/inbound/decryption.rs +++ b/comms/dht/src/inbound/decryption.rs @@ -75,11 +75,11 @@ enum DecryptionError { pub struct DecryptionLayer { node_identity: Arc, connectivity: ConnectivityRequester, - config: DhtConfig, + config: Arc, } impl DecryptionLayer { - pub fn new(config: DhtConfig, node_identity: Arc, connectivity: ConnectivityRequester) -> Self { + pub fn new(config: Arc, node_identity: Arc, connectivity: ConnectivityRequester) -> Self { Self { node_identity, connectivity, @@ -104,7 +104,7 @@ impl Layer for DecryptionLayer { /// Responsible for decrypting InboundMessages and passing a DecryptedInboundMessage to the given service #[derive(Clone)] pub struct DecryptionService { - config: DhtConfig, + config: Arc, node_identity: Arc, connectivity: ConnectivityRequester, inner: S, @@ -112,7 +112,7 @@ pub struct DecryptionService { impl DecryptionService { pub fn new( - config: DhtConfig, + config: Arc, node_identity: Arc, connectivity: ConnectivityRequester, service: S, diff --git a/comms/dht/src/inbound/deserialize.rs b/comms/dht/src/inbound/deserialize.rs index 6c4df80f18..280cbfa5a9 100644 --- a/comms/dht/src/inbound/deserialize.rs +++ b/comms/dht/src/inbound/deserialize.rs @@ -25,7 +25,7 @@ use std::{convert::TryInto, sync::Arc, task::Poll}; use futures::{future::BoxFuture, task::Context}; use log::*; use prost::Message; -use tari_comms::{message::InboundMessage, pipeline::PipelineError, PeerManager}; +use tari_comms::{message::InboundMessage, pipeline::PipelineError, OrNotFound, PeerManager}; use tower::{layer::Layer, Service, ServiceExt}; use crate::{inbound::DhtInboundMessage, proto::envelope::DhtEnvelope}; @@ -84,7 +84,11 @@ where match DhtEnvelope::decode(&mut body) { Ok(dht_envelope) => { - let source_peer = peer_manager.find_by_node_id(&source_peer).await.map(Arc::new)?; + let source_peer = peer_manager + .find_by_node_id(&source_peer) + .await + .or_not_found() + .map(Arc::new)?; let inbound_msg = DhtInboundMessage::new(tag, dht_envelope.header.try_into()?, source_peer, dht_envelope.body); diff --git a/comms/dht/src/inbound/dht_handler/layer.rs b/comms/dht/src/inbound/dht_handler/layer.rs index bce1752b38..eebe5ffa1e 100644 --- a/comms/dht/src/inbound/dht_handler/layer.rs +++ b/comms/dht/src/inbound/dht_handler/layer.rs @@ -26,9 +26,10 @@ use tari_comms::peer_manager::{NodeIdentity, PeerManager}; use tower::layer::Layer; use super::middleware::DhtHandlerMiddleware; -use crate::{discovery::DhtDiscoveryRequester, outbound::OutboundMessageRequester}; +use crate::{discovery::DhtDiscoveryRequester, outbound::OutboundMessageRequester, DhtConfig}; pub struct DhtHandlerLayer { + config: Arc, peer_manager: Arc, node_identity: Arc, outbound_service: OutboundMessageRequester, @@ -37,15 +38,16 @@ pub struct DhtHandlerLayer { impl DhtHandlerLayer { pub fn new( + config: Arc, node_identity: Arc, peer_manager: Arc, discovery_requester: DhtDiscoveryRequester, outbound_service: OutboundMessageRequester, ) -> Self { Self { + config, peer_manager, node_identity, - outbound_service, discovery_requester, } @@ -62,6 +64,7 @@ impl Layer for DhtHandlerLayer { Arc::clone(&self.peer_manager), self.outbound_service.clone(), self.discovery_requester.clone(), + self.config.clone(), ) } } diff --git a/comms/dht/src/inbound/dht_handler/middleware.rs b/comms/dht/src/inbound/dht_handler/middleware.rs index 82304f97f1..8fe66f62e6 100644 --- a/comms/dht/src/inbound/dht_handler/middleware.rs +++ b/comms/dht/src/inbound/dht_handler/middleware.rs @@ -30,7 +30,12 @@ use tari_comms::{ use tower::Service; use super::task::ProcessDhtMessage; -use crate::{discovery::DhtDiscoveryRequester, inbound::DecryptedDhtMessage, outbound::OutboundMessageRequester}; +use crate::{ + discovery::DhtDiscoveryRequester, + inbound::DecryptedDhtMessage, + outbound::OutboundMessageRequester, + DhtConfig, +}; #[derive(Clone)] pub struct DhtHandlerMiddleware { @@ -39,6 +44,7 @@ pub struct DhtHandlerMiddleware { node_identity: Arc, outbound_service: OutboundMessageRequester, discovery_requester: DhtDiscoveryRequester, + config: Arc, } impl DhtHandlerMiddleware { @@ -47,16 +53,16 @@ impl DhtHandlerMiddleware { node_identity: Arc, peer_manager: Arc, outbound_service: OutboundMessageRequester, - discovery_requester: DhtDiscoveryRequester, + config: Arc, ) -> Self { Self { next_service, peer_manager, node_identity, - outbound_service, discovery_requester, + config, } } } @@ -83,6 +89,7 @@ where Arc::clone(&self.node_identity), self.discovery_requester.clone(), message, + self.config.clone(), ) .run(), ) diff --git a/comms/dht/src/inbound/dht_handler/task.rs b/comms/dht/src/inbound/dht_handler/task.rs index 88821deb50..2a44000ac8 100644 --- a/comms/dht/src/inbound/dht_handler/task.rs +++ b/comms/dht/src/inbound/dht_handler/task.rs @@ -20,14 +20,16 @@ // WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE // USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -use std::sync::Arc; +use std::{convert::TryFrom, str::FromStr, sync::Arc}; use log::*; use tari_comms::{ message::MessageExt, - peer_manager::{NodeId, NodeIdentity, PeerFeatures, PeerManager}, + multiaddr::Multiaddr, + peer_manager::{IdentitySignature, NodeId, NodeIdentity, Peer, PeerFeatures, PeerFlags, PeerManager}, pipeline::PipelineError, types::CommsPublicKey, + OrNotFound, }; use tari_utilities::{hex::Hex, ByteArray}; use tower::{Service, ServiceExt}; @@ -37,10 +39,12 @@ use crate::{ envelope::NodeDestination, inbound::{error::DhtInboundError, message::DecryptedDhtMessage}, outbound::{OutboundMessageRequester, SendMessageParams}, + peer_validator::PeerValidator, proto::{ dht::{DiscoveryMessage, DiscoveryResponseMessage, JoinMessage}, envelope::DhtMessageType, }, + DhtConfig, }; const LOG_TARGET: &str = "comms::dht::dht_handler"; @@ -52,6 +56,7 @@ pub struct ProcessDhtMessage { node_identity: Arc, message: Option, discovery_requester: DhtDiscoveryRequester, + config: Arc, } impl ProcessDhtMessage @@ -64,6 +69,7 @@ where S: Service node_identity: Arc, discovery_requester: DhtDiscoveryRequester, message: DecryptedDhtMessage, + config: Arc, ) -> Self { Self { next_service, @@ -72,6 +78,7 @@ where S: Service node_identity, discovery_requester, message: Some(message), + config, } } @@ -131,20 +138,6 @@ where S: Service Ok(()) } - fn validate_raw_node_id(&self, public_key: &CommsPublicKey, raw_node_id: &[u8]) -> Result { - // The reason that we check the given node id against what we expect instead of just using the given node id - // is in future the NodeId may not necessarily be derived from the public key (i.e. DAN node is registered on - // the base layer) - let expected_node_id = NodeId::from_key(public_key); - let node_id = NodeId::from_bytes(raw_node_id).map_err(|_| DhtInboundError::InvalidNodeId)?; - if expected_node_id == node_id { - Ok(expected_node_id) - } else { - // TODO: Misbehaviour? - Err(DhtInboundError::InvalidNodeId) - } - } - async fn handle_join(&mut self, message: DecryptedDhtMessage) -> Result<(), DhtInboundError> { let DecryptedDhtMessage { decryption_result, @@ -176,25 +169,33 @@ where S: Service let addresses = join_msg .addresses - .into_iter() - .filter_map(|addr| addr.parse().ok()) + .iter() + .filter_map(|addr| Multiaddr::from_str(addr).ok()) .collect::>(); if addresses.is_empty() { return Err(DhtInboundError::InvalidAddresses); } + let node_id = NodeId::from_public_key(&authenticated_pk); + let features = PeerFeatures::from_bits_truncate(join_msg.peer_features); + let mut new_peer = Peer::new( + authenticated_pk, + node_id.clone(), + addresses.into(), + PeerFlags::empty(), + features, + vec![], + String::new(), + ); + new_peer.identity_signature = join_msg + .identity_signature + .map(IdentitySignature::try_from) + .transpose() + .map_err(|err| DhtInboundError::InvalidPeerIdentitySignature(err.to_string()))?; - let node_id = self.validate_raw_node_id(&authenticated_pk, &join_msg.node_id)?; - - let origin_peer = self - .peer_manager - .add_or_update_online_peer( - &authenticated_pk, - node_id, - addresses, - PeerFeatures::from_bits_truncate(join_msg.peer_features), - ) - .await?; + let peer_validator = PeerValidator::new(&self.peer_manager, &self.config); + peer_validator.validate_and_add_peer(new_peer).await?; + let origin_peer = self.peer_manager.find_by_node_id(&node_id).await.or_not_found()?; // DO NOT propagate this peer if this node has banned them if origin_peer.is_banned() { @@ -288,30 +289,40 @@ where S: Service let addresses = discover_msg .addresses - .into_iter() - .filter_map(|addr| addr.parse().ok()) + .iter() + .filter_map(|addr| Multiaddr::from_str(addr).ok()) .collect::>(); if addresses.is_empty() { return Err(DhtInboundError::InvalidAddresses); } - let node_id = self.validate_raw_node_id(&authenticated_pk, &discover_msg.node_id)?; - let origin_peer = self - .peer_manager - .add_or_update_online_peer( - &authenticated_pk, - node_id, - addresses, - PeerFeatures::from_bits_truncate(discover_msg.peer_features), - ) - .await?; + let node_id = NodeId::from_public_key(&authenticated_pk); + let features = PeerFeatures::from_bits_truncate(discover_msg.peer_features); + let mut new_peer = Peer::new( + authenticated_pk, + node_id.clone(), + addresses.into(), + PeerFlags::empty(), + features, + vec![], + String::new(), + ); + new_peer.identity_signature = discover_msg + .identity_signature + .map(IdentitySignature::try_from) + .transpose() + .map_err(|err| DhtInboundError::InvalidPeerIdentitySignature(err.to_string()))?; + + let peer_validator = PeerValidator::new(&self.peer_manager, &self.config); + peer_validator.validate_and_add_peer(new_peer).await?; + let origin_peer = self.peer_manager.find_by_node_id(&node_id).await.or_not_found()?; // Don't send a join request to the origin peer if they are banned if origin_peer.is_banned() { warn!( target: LOG_TARGET, - "Received Discovery request for banned peer '{}'. This request will be ignored.", authenticated_pk + "Received Discovery request for banned peer '{}'. This request will be ignored.", node_id ); return Ok(()); } @@ -335,6 +346,7 @@ where S: Service addresses: vec![self.node_identity.public_address().to_string()], peer_features: self.node_identity.features().bits(), nonce, + identity_signature: self.node_identity.identity_signature_read().as_ref().map(Into::into), }; trace!(target: LOG_TARGET, "Sending discovery response to {}", dest_public_key); diff --git a/comms/dht/src/inbound/error.rs b/comms/dht/src/inbound/error.rs index 8d40ad5f4b..6681bdedc5 100644 --- a/comms/dht/src/inbound/error.rs +++ b/comms/dht/src/inbound/error.rs @@ -23,7 +23,7 @@ use tari_comms::{message::MessageError, peer_manager::PeerManagerError}; use thiserror::Error; -use crate::{discovery::DhtDiscoveryError, outbound::DhtOutboundError}; +use crate::{discovery::DhtDiscoveryError, outbound::DhtOutboundError, peer_validator::PeerValidatorError}; #[derive(Debug, Error)] pub enum DhtInboundError { @@ -35,12 +35,14 @@ pub enum DhtInboundError { DhtOutboundError(#[from] DhtOutboundError), #[error("Message body invalid")] InvalidMessageBody, - #[error("Node ID is invalid")] - InvalidNodeId, #[error("All given addresses were invalid")] InvalidAddresses, #[error("DhtDiscoveryError: {0}")] DhtDiscoveryError(#[from] DhtDiscoveryError), #[error("OriginRequired: {0}")] OriginRequired(String), + #[error("Invalid peer identity signature: {0}")] + InvalidPeerIdentitySignature(String), + #[error("Invalid peer: {0}")] + PeerValidatorError(#[from] PeerValidatorError), } diff --git a/comms/dht/src/lib.rs b/comms/dht/src/lib.rs index 5aa79b0baa..cb75344910 100644 --- a/comms/dht/src/lib.rs +++ b/comms/dht/src/lib.rs @@ -156,6 +156,7 @@ pub use dedup::DedupLayer; mod filter; mod logging_middleware; +mod peer_validator; mod proto; mod rpc; mod schema; diff --git a/comms/dht/src/network_discovery/discovering.rs b/comms/dht/src/network_discovery/discovering.rs index c543c87929..cc7b47e974 100644 --- a/comms/dht/src/network_discovery/discovering.rs +++ b/comms/dht/src/network_discovery/discovering.rs @@ -27,15 +27,15 @@ use log::*; use tari_comms::{ connectivity::ConnectivityError, peer_manager::{NodeDistance, NodeId, Peer, PeerFeatures}, - validate_peer_addresses, PeerConnection, + PeerManager, }; use super::{ state_machine::{DhtNetworkDiscoveryRoundInfo, DiscoveryParams, NetworkDiscoveryContext, StateEvent}, NetworkDiscoveryError, }; -use crate::{proto::rpc::GetPeersRequest, rpc, DhtConfig}; +use crate::{peer_validator::PeerValidator, proto::rpc::GetPeersRequest, rpc, DhtConfig}; const LOG_TARGET: &str = "comms::dht::network_discovery"; @@ -181,62 +181,55 @@ impl Discovering { Ok(()) } - async fn validate_and_add_peer(&mut self, sync_peer: &NodeId, peer: Peer) -> Result<(), NetworkDiscoveryError> { - if self.context.node_identity.node_id() == &peer.node_id { + async fn validate_and_add_peer(&mut self, sync_peer: &NodeId, new_peer: Peer) -> Result<(), NetworkDiscoveryError> { + if self.context.node_identity.node_id() == &new_peer.node_id { debug!(target: LOG_TARGET, "Received our own node from peer sync. Ignoring."); return Ok(()); } - let peer_manager = &self.context.peer_manager; - if peer_manager.exists_node_id(&peer.node_id).await { - self.stats.num_duplicate_peers += 1; - debug!( - target: LOG_TARGET, - "Peer `{}` already exists and will not be added to the peer list", peer.node_id - ); - return Ok(()); - } + let new_peer_node_id = new_peer.node_id.clone(); + let peer_validator = PeerValidator::new(self.peer_manager(), self.config()); - let peer_dist = peer.node_id.distance(self.context.node_identity.node_id()); + let peer_dist = new_peer.node_id.distance(self.context.node_identity.node_id()); let is_neighbour = peer_dist <= self.neighbourhood_threshold; - let addresses = peer.addresses.iter(); - match validate_peer_addresses(addresses, self.config().allow_test_addresses) { - Ok(_) => { + match peer_validator.validate_and_add_peer(new_peer).await { + Ok(true) => { if is_neighbour { self.stats.num_new_neighbours += 1; - debug!( - target: LOG_TARGET, - "Adding new neighbouring peer `{}`. A total of {} have been added this round.", - peer.node_id, - self.stats.num_new_neighbours - ); } - - debug!( - target: LOG_TARGET, - "Adding peer `{}` from `{}`", peer.node_id, sync_peer - ); self.stats.num_new_peers += 1; - peer_manager.add_peer(peer).await?; + Ok(()) + }, + Ok(false) => { + self.stats.num_duplicate_peers += 1; + Ok(()) }, Err(err) => { - // TODO: #banheuristic - debug!( + warn!( target: LOG_TARGET, - "Failed to validate peer received from `{}`: {}. Peer not added.", sync_peer, err + "Received invalid peer from sync peer '{}': {}. Banning sync peer.", sync_peer, err ); + self.context + .connectivity + .ban_peer( + sync_peer.clone(), + format!("Network discovery peer sent invalid peer '{}'", new_peer_node_id), + ) + .await?; + Err(err.into()) }, } - - Ok(()) } - #[inline] fn config(&self) -> &DhtConfig { &self.context.config } + fn peer_manager(&self) -> &PeerManager { + &self.context.peer_manager + } + fn dial_all_candidates(&self) -> impl Stream> + 'static { let pending_dials = self .params diff --git a/comms/dht/src/network_discovery/error.rs b/comms/dht/src/network_discovery/error.rs index 5d1fdbaf07..db92cfc805 100644 --- a/comms/dht/src/network_discovery/error.rs +++ b/comms/dht/src/network_discovery/error.rs @@ -22,6 +22,8 @@ use tari_comms::{connectivity::ConnectivityError, peer_manager::PeerManagerError, protocol::rpc::RpcError}; +use crate::peer_validator::PeerValidatorError; + #[derive(thiserror::Error, Debug)] pub enum NetworkDiscoveryError { #[error("RPC error: {0}")] @@ -32,4 +34,6 @@ pub enum NetworkDiscoveryError { ConnectivityError(#[from] ConnectivityError), #[error("No sync peers available")] NoSyncPeers, + #[error("Sync peer sent invalid peer")] + PeerValidationError(#[from] PeerValidatorError), } diff --git a/comms/dht/src/network_discovery/on_connect.rs b/comms/dht/src/network_discovery/on_connect.rs index 400bc11c23..121ad1635e 100644 --- a/comms/dht/src/network_discovery/on_connect.rs +++ b/comms/dht/src/network_discovery/on_connect.rs @@ -20,16 +20,11 @@ // WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE // USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -use std::convert::TryInto; +use std::{convert::TryInto, time::Duration}; use futures::StreamExt; use log::*; -use tari_comms::{ - connectivity::ConnectivityEvent, - peer_manager::{NodeId, Peer}, - validate_peer_addresses, - PeerConnection, -}; +use tari_comms::{connectivity::ConnectivityEvent, peer_manager::NodeId, PeerConnection}; use tokio::sync::broadcast; use crate::{ @@ -39,6 +34,7 @@ use crate::{ DhtNetworkDiscoveryRoundInfo, NetworkDiscoveryError, }, + peer_validator::PeerValidator, proto::rpc::GetPeersRequest, rpc, DhtConfig, @@ -88,6 +84,21 @@ impl OnConnect { match self.sync_peers(conn.clone()).await { Ok(_) => continue, + Err(err @ NetworkDiscoveryError::PeerValidationError(_)) => { + warn!(target: LOG_TARGET, "{}. Banning peer.", err); + if let Err(err) = self + .context + .connectivity + .ban_peer_until( + conn.peer_node_id().clone(), + Duration::from_secs(60 * 60), + err.to_string(), + ) + .await + { + return err.into(); + } + }, Err(err) => debug!( target: LOG_TARGET, "Failed to peer sync from `{}`: {}", @@ -125,12 +136,12 @@ impl OnConnect { let sync_peer = conn.peer_node_id(); let mut num_added = 0; + let peer_validator = PeerValidator::new(&self.context.peer_manager, self.config()); while let Some(resp) = peer_stream.next().await { match resp { Ok(resp) => match resp.peer.and_then(|peer| peer.try_into().ok()) { Some(peer) => { - let is_node_added = self.validate_and_add_peer(sync_peer, peer).await?; - if is_node_added { + if peer_validator.validate_and_add_peer(peer).await? { num_added += 1; } }, @@ -163,36 +174,6 @@ impl OnConnect { Ok(()) } - async fn validate_and_add_peer(&self, sync_peer: &NodeId, peer: Peer) -> Result { - if self.context.node_identity.node_id() == &peer.node_id { - return Ok(false); - } - let peer_manager = &self.context.peer_manager; - if peer_manager.exists_node_id(&peer.node_id).await { - return Ok(false); - } - - let addresses = peer.addresses.iter(); - match validate_peer_addresses(addresses, self.config().allow_test_addresses) { - Ok(_) => { - debug!( - target: LOG_TARGET, - "Adding peer `{}` from `{}`", peer.node_id, sync_peer - ); - peer_manager.add_peer(peer).await?; - Ok(true) - }, - Err(err) => { - // TODO: #banheuristic - debug!( - target: LOG_TARGET, - "Failed to validate peer received from `{}`: {}. Peer not added.", sync_peer, err - ); - Ok(false) - }, - } - } - #[inline] fn config(&self) -> &DhtConfig { &self.context.config diff --git a/comms/dht/src/network_discovery/state_machine.rs b/comms/dht/src/network_discovery/state_machine.rs index 2a42f15176..fe5b81dd3b 100644 --- a/comms/dht/src/network_discovery/state_machine.rs +++ b/comms/dht/src/network_discovery/state_machine.rs @@ -120,7 +120,7 @@ impl> From for StateEvent { #[derive(Debug, Clone)] pub(super) struct NetworkDiscoveryContext { - pub config: DhtConfig, + pub config: Arc, pub peer_manager: Arc, pub connectivity: ConnectivityRequester, pub node_identity: Arc, @@ -170,7 +170,7 @@ pub struct DhtNetworkDiscovery { impl DhtNetworkDiscovery { pub fn new( - config: DhtConfig, + config: Arc, node_identity: Arc, peer_manager: Arc, connectivity: ConnectivityRequester, diff --git a/comms/dht/src/network_discovery/test.rs b/comms/dht/src/network_discovery/test.rs index fad4aeabf7..31897a87a6 100644 --- a/comms/dht/src/network_discovery/test.rs +++ b/comms/dht/src/network_discovery/test.rs @@ -81,7 +81,7 @@ mod state_machine { let (event_tx, event_rx) = broadcast::channel(2); let network_discovery = DhtNetworkDiscovery::new( - config, + Arc::new(config), node_identity.clone(), peer_manager.clone(), connectivity, @@ -186,10 +186,10 @@ mod discovery_ready { let (connectivity, connectivity_mock) = create_connectivity_mock(); let (event_tx, _) = broadcast::channel(1); let context = NetworkDiscoveryContext { - config: DhtConfig { + config: Arc::new(DhtConfig { network_discovery: config, ..Default::default() - }, + }), peer_manager: peer_manager.clone(), connectivity, node_identity: node_identity.clone(), diff --git a/comms/dht/src/outbound/broadcast.rs b/comms/dht/src/outbound/broadcast.rs index 1b1b5d1a4e..deab640263 100644 --- a/comms/dht/src/outbound/broadcast.rs +++ b/comms/dht/src/outbound/broadcast.rs @@ -299,16 +299,11 @@ where S: Service .send(SendMessageResponse::PendingDiscovery(discovery_reply_rx)); match self.initiate_peer_discovery(target_public_key).await { - Ok(Some(peer)) => { + Ok(peer) => { // Set the reply_tx so that it can be used later reply_tx = Some(discovery_reply_tx); peers = vec![peer.node_id]; }, - Ok(None) => { - // Message sent to 0 peers - let _ = discovery_reply_tx.send(SendMessageResponse::Queued(vec![].into())); - return Ok(Vec::new()); - }, Err(err @ DhtOutboundError::DiscoveryFailed) => { let _ = discovery_reply_tx.send(SendMessageResponse::Failed(SendFailure::DiscoveryFailed)); return Err(err); @@ -376,7 +371,7 @@ where S: Service async fn initiate_peer_discovery( &mut self, dest_public_key: Box, - ) -> Result, DhtOutboundError> { + ) -> Result { trace!( target: LOG_TARGET, "Initiating peer discovery for public key '{}'", @@ -391,20 +386,11 @@ where S: Service { // Peer found! Ok(peer) => { - if peer.is_banned() { - warn!( - target: LOG_TARGET, - "Peer discovery succeeded however peer with public key '{}' is marked as banned.", - peer.public_key - ); - return Ok(None); - } - debug!( target: LOG_TARGET, "Peer discovery succeeded for public key '{}'.", peer.public_key ); - Ok(Some(peer)) + Ok(peer) }, // Error during discovery Err(err) => { diff --git a/comms/dht/src/outbound/message.rs b/comms/dht/src/outbound/message.rs index 5606e7b113..a9980b2be6 100644 --- a/comms/dht/src/outbound/message.rs +++ b/comms/dht/src/outbound/message.rs @@ -136,7 +136,7 @@ impl SendMessageResponse { Queued(send_states) if !send_states.is_empty() => Ok(send_states), Queued(_) => Err(SendFailure::NoMessagesQueued), Failed(err) => Err(err), - PendingDiscovery(_) => panic!("ok_or_failed() called on PendingDiscovery"), + PendingDiscovery(_) => panic!("queued_or_failed() called on PendingDiscovery"), } } } diff --git a/comms/dht/src/peer_validator.rs b/comms/dht/src/peer_validator.rs new file mode 100644 index 0000000000..5e03a2f7bc --- /dev/null +++ b/comms/dht/src/peer_validator.rs @@ -0,0 +1,129 @@ +// Copyright 2021, The Tari Project +// +// Redistribution and use in source and binary forms, with or without modification, are permitted provided that the +// following conditions are met: +// +// 1. Redistributions of source code must retain the above copyright notice, this list of conditions and the following +// disclaimer. +// +// 2. Redistributions in binary form must reproduce the above copyright notice, this list of conditions and the +// following disclaimer in the documentation and/or other materials provided with the distribution. +// +// 3. Neither the name of the copyright holder nor the names of its contributors may be used to endorse or promote +// products derived from this software without specific prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, +// INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE +// DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR +// SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, +// WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE +// USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +use log::*; +use tari_comms::{ + peer_manager::{NodeId, Peer, PeerManagerError}, + types::CommsPublicKey, + validate_peer_addresses, + PeerManager, +}; + +use crate::DhtConfig; + +const LOG_TARGET: &str = "dht::network_discovery::peer_validator"; + +#[derive(Debug, thiserror::Error)] +pub enum PeerValidatorError { + #[error("Node ID was invalid")] + InvalidNodeId, + #[error("Peer signature was invalid")] + InvalidPeerSignature, + #[error("One or more peer addresses were invalid")] + InvalidPeerAddresses, + #[error("Peer manager error: {0}")] + PeerManagerError(#[from] PeerManagerError), +} + +pub struct PeerValidator<'a> { + peer_manager: &'a PeerManager, + config: &'a DhtConfig, +} + +impl<'a> PeerValidator<'a> { + pub fn new(peer_manager: &'a PeerManager, config: &'a DhtConfig) -> Self { + Self { peer_manager, config } + } + + /// Validates the new peer against the current peer database. Returning true if a new peer was added and false if + /// the peer already exists. + pub async fn validate_and_add_peer(&self, new_peer: Peer) -> Result { + validate_node_id(&new_peer.public_key, &new_peer.node_id)?; + + if let Err(err) = validate_peer_addresses(new_peer.addresses.iter(), self.config.allow_test_addresses) { + warn!(target: LOG_TARGET, "Invalid peer address: {}", err); + return Err(PeerValidatorError::InvalidPeerAddresses); + } + + let can_update = match new_peer.is_valid_identity_signature() { + // Update/insert peer + Some(true) => true, + Some(false) => return Err(PeerValidatorError::InvalidPeerSignature), + // Insert new peer if it doesn't exist, do not update + None => false, + }; + + debug!(target: LOG_TARGET, "Adding peer `{}`", new_peer.node_id); + + match self.peer_manager.find_by_node_id(&new_peer.node_id).await? { + Some(mut current_peer) => { + let can_update = can_update && { + // Update/insert peer if newer + // unreachable panic: can_update is true only is identity_signature is present and valid + let new_dt = new_peer + .identity_signature + .as_ref() + .map(|i| i.updated_at()) + .expect("unreachable panic"); + + // Update if new_peer has newer timestamp than current_peer + current_peer + .identity_signature + .as_ref() + .map(|i| i.updated_at() < new_dt) + // If None, update to peer with valid signature + .unwrap_or(true) + }; + + if !can_update { + debug!( + target: LOG_TARGET, + "Peer `{}` already exists or is up to date and will not be updated", new_peer.node_id + ); + return Ok(false); + } + + debug!(target: LOG_TARGET, "Updating peer `{}`", new_peer.node_id); + current_peer.addresses.update_addresses(new_peer.addresses.into_vec()); + current_peer.identity_signature = new_peer.identity_signature; + current_peer.set_offline(false); + self.peer_manager.add_peer(current_peer).await?; + + Ok(false) + }, + None => { + debug!(target: LOG_TARGET, "Adding peer `{}`", new_peer.node_id); + self.peer_manager.add_peer(new_peer).await?; + Ok(true) + }, + } + } +} + +fn validate_node_id(public_key: &CommsPublicKey, node_id: &NodeId) -> Result { + let expected_node_id = NodeId::from_key(public_key); + if expected_node_id == *node_id { + Ok(expected_node_id) + } else { + Err(PeerValidatorError::InvalidNodeId) + } +} diff --git a/comms/dht/src/proto/common.proto b/comms/dht/src/proto/common.proto new file mode 100644 index 0000000000..e5591320af --- /dev/null +++ b/comms/dht/src/proto/common.proto @@ -0,0 +1,11 @@ +syntax = "proto3"; + +package tari.dht.common; + +message IdentitySignature { + uint32 version = 1; + bytes signature = 2; + bytes public_nonce = 3; + // The EPOCH timestamp used in the identity signature challenge + int64 updated_at = 4; +} diff --git a/comms/dht/src/proto/dht.proto b/comms/dht/src/proto/dht.proto index cbf231f0c0..8cde5ae8f1 100644 --- a/comms/dht/src/proto/dht.proto +++ b/comms/dht/src/proto/dht.proto @@ -2,6 +2,8 @@ syntax = "proto3"; package tari.dht; +import "common.proto"; + // JoinMessage contains the information required for a network join request. // // Message containing contact information for a node wishing to join the network. @@ -13,6 +15,7 @@ message JoinMessage { repeated string addresses = 2; uint64 peer_features = 3; uint64 nonce = 4; + tari.dht.common.IdentitySignature identity_signature = 5; } // The DiscoverMessage stores the information required for a network discover request. @@ -25,6 +28,7 @@ message DiscoveryMessage { repeated string addresses = 2; uint64 peer_features = 3; uint64 nonce = 4; + tari.dht.common.IdentitySignature identity_signature = 5; } message DiscoveryResponseMessage { @@ -32,4 +36,5 @@ message DiscoveryResponseMessage { repeated string addresses = 2; uint64 peer_features = 3; uint64 nonce = 4; + tari.dht.common.IdentitySignature identity_signature = 5; } diff --git a/comms/dht/src/proto/mod.rs b/comms/dht/src/proto/mod.rs index d4ee853da2..03b1b58477 100644 --- a/comms/dht/src/proto/mod.rs +++ b/comms/dht/src/proto/mod.rs @@ -20,24 +20,33 @@ // WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE // USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -use std::{convert::TryInto, fmt}; +use std::{ + convert::{TryFrom, TryInto}, + fmt, +}; +use chrono::NaiveDateTime; use rand::{rngs::OsRng, RngCore}; use tari_comms::{ multiaddr::Multiaddr, - peer_manager::{NodeId, Peer, PeerFeatures, PeerFlags}, - types::CommsPublicKey, + peer_manager::{IdentitySignature, NodeId, Peer, PeerFeatures, PeerFlags}, + types::{CommsPublicKey, CommsSecretKey, Signature}, NodeIdentity, }; use tari_utilities::{hex::Hex, ByteArray}; use crate::proto::dht::JoinMessage; +pub mod common { + tari_comms::outdir_include!("tari.dht.common.rs"); +} + pub mod envelope { tari_comms::outdir_include!("tari.dht.envelope.rs"); } pub mod dht { + use super::common; tari_comms::outdir_include!("tari.dht.rs"); } @@ -63,6 +72,7 @@ impl> From for JoinMessage { addresses: vec![node_identity.public_address().to_string()], peer_features: node_identity.features().bits(), nonce: OsRng.next_u64(), + identity_signature: node_identity.identity_signature_read().as_ref().map(Into::into), } } } @@ -92,6 +102,7 @@ impl From for rpc::Peer { .map(|addr| addr.address.to_string()) .collect(), peer_features: peer.features.bits(), + identity_signature: peer.identity_signature.as_ref().map(Into::into), } } } @@ -107,15 +118,44 @@ impl TryInto for rpc::Peer { .iter() .filter_map(|addr| addr.parse::().ok()) .collect::>(); - - Ok(Peer::new( + let mut peer = Peer::new( pk, node_id, addresses.into(), PeerFlags::NONE, PeerFeatures::from_bits_truncate(self.peer_features), Default::default(), - "".to_string(), - )) + String::new(), + ); + + peer.identity_signature = self.identity_signature.map(TryInto::try_into).transpose()?; + + Ok(peer) + } +} + +impl TryFrom for IdentitySignature { + type Error = anyhow::Error; + + fn try_from(value: common::IdentitySignature) -> Result { + let version = u8::try_from(value.version) + .map_err(|_| anyhow::anyhow!("Invalid peer identity signature version {}", value.version))?; + let public_nonce = CommsPublicKey::from_bytes(&value.public_nonce)?; + let signature = CommsSecretKey::from_bytes(&value.signature)?; + let updated_at = NaiveDateTime::from_timestamp_opt(value.updated_at, 0) + .ok_or_else(|| anyhow::anyhow!("updated_at overflowed"))?; + + Ok(Self::new(version, Signature::new(public_nonce, signature), updated_at)) + } +} + +impl From<&IdentitySignature> for common::IdentitySignature { + fn from(identity_sig: &IdentitySignature) -> Self { + common::IdentitySignature { + version: identity_sig.version() as u32, + signature: identity_sig.signature().get_signature().to_vec(), + public_nonce: identity_sig.signature().get_public_nonce().to_vec(), + updated_at: identity_sig.updated_at().timestamp(), + } } } diff --git a/comms/dht/src/proto/rpc.proto b/comms/dht/src/proto/rpc.proto index 0db02c249f..098695e8c0 100644 --- a/comms/dht/src/proto/rpc.proto +++ b/comms/dht/src/proto/rpc.proto @@ -2,6 +2,8 @@ syntax = "proto3"; package tari.dht.rpc; +import "common.proto"; + // `get_closer_peers` request message GetCloserPeersRequest { // The number of peers to return @@ -28,6 +30,5 @@ message Peer { bytes public_key = 1; repeated string addresses = 2; uint64 peer_features = 3; + tari.dht.common.IdentitySignature identity_signature = 4; } - - diff --git a/comms/dht/src/store_forward/saf_handler/task.rs b/comms/dht/src/store_forward/saf_handler/task.rs index 5b4eb0ff41..e852300b4c 100644 --- a/comms/dht/src/store_forward/saf_handler/task.rs +++ b/comms/dht/src/store_forward/saf_handler/task.rs @@ -174,11 +174,11 @@ where S: Service &mut self, message: DecryptedDhtMessage, ) -> Result<(), StoreAndForwardError> { - trace!( + debug!( target: LOG_TARGET, "Received request for stored message {} from {} (Trace: {})", message.tag, - message.source_peer.public_key, + message.source_peer.node_id, message.dht_header.message_tag ); let msg = message @@ -195,7 +195,7 @@ where S: Service // Compile a set of stored messages for the requesting peer let mut query = FetchStoredMessageQuery::new(source_pubkey, source_node_id.clone()); - let since: Option> = match retrieve_msgs.since.map(timestamp_to_datetime) { + let since = match retrieve_msgs.since.and_then(timestamp_to_datetime) { Some(since) => { debug!( target: LOG_TARGET, @@ -433,11 +433,13 @@ where S: Service let stored_at = message .stored_at .map(|t| { - DateTime::from_utc( - NaiveDateTime::from_timestamp(t.seconds, t.nanos.try_into().unwrap_or(u32::MAX)), + Result::<_, StoreAndForwardError>::Ok(DateTime::from_utc( + NaiveDateTime::from_timestamp_opt(t.seconds, t.nanos.try_into().unwrap_or(u32::MAX)) + .ok_or(StoreAndForwardError::InvalidStoreMessage)?, Utc, - ) + )) }) + .transpose()? .unwrap_or(chrono::MIN_DATETIME); if stored_at > Utc::now() { diff --git a/comms/dht/src/store_forward/store.rs b/comms/dht/src/store_forward/store.rs index c045cbe1af..d06e3bd976 100644 --- a/comms/dht/src/store_forward/store.rs +++ b/comms/dht/src/store_forward/store.rs @@ -379,7 +379,7 @@ where S: Service + Se // TODO: Remove this once we have a layer that filters out all messages to/from banned peers if let Some(origin_pk) = message.authenticated_origin() { - if let Ok(peer) = self.peer_manager.find_by_public_key(origin_pk).await { + if let Ok(Some(peer)) = self.peer_manager.find_by_public_key(origin_pk).await { if peer.is_banned() { log_not_eligible("source peer is banned by this node"); return Ok(None); @@ -410,7 +410,7 @@ where S: Service + Se } match peer_manager.find_by_node_id(&dest_node_id).await { - Ok(peer) if peer.is_banned() => { + Ok(Some(peer)) if peer.is_banned() => { log_not_eligible("destination peer is banned."); Ok(None) }, diff --git a/comms/dht/tests/dht.rs b/comms/dht/tests/dht.rs index 97f1de4af0..441a4a8942 100644 --- a/comms/dht/tests/dht.rs +++ b/comms/dht/tests/dht.rs @@ -294,6 +294,7 @@ async fn dht_join_propagation() { let node_A_peer = node_C_peer_manager .find_by_public_key(node_A.node_identity().public_key()) .await + .unwrap() .unwrap(); assert_eq!(node_A_peer.features, node_A.comms.node_identity().features()); @@ -750,6 +751,7 @@ async fn dht_do_not_store_invalid_message_in_dedup() { .peer_manager() .find_by_node_id(node_B.node_identity().node_id()) .await + .unwrap() .unwrap(); n.is_banned() }, diff --git a/comms/src/builder/comms_node.rs b/comms/src/builder/comms_node.rs index c90af4c4bc..a85706f527 100644 --- a/comms/src/builder/comms_node.rs +++ b/comms/src/builder/comms_node.rs @@ -212,7 +212,10 @@ impl UnspawnedCommsNode { if let Some(mut ctl) = hidden_service_ctl { ctl.set_proxied_addr(listening_info.bind_address().clone()); let hs = ctl.create_hidden_service().await?; - node_identity.set_public_address(hs.get_onion_address()); + let onion_addr = hs.get_onion_address(); + if node_identity.public_address() != onion_addr { + node_identity.set_public_address(onion_addr); + } hidden_service = Some(hs); } info!( diff --git a/comms/src/connection_manager/common.rs b/comms/src/connection_manager/common.rs index f835f507c3..f7caef7203 100644 --- a/comms/src/connection_manager/common.rs +++ b/comms/src/connection_manager/common.rs @@ -29,7 +29,8 @@ use super::types::ConnectionDirection; use crate::{ connection_manager::error::ConnectionManagerError, multiaddr::{Multiaddr, Protocol}, - peer_manager::{NodeId, NodeIdentity, Peer, PeerFeatures, PeerFlags}, + peer_manager::{IdentitySignature, NodeId, NodeIdentity, Peer, PeerFeatures, PeerFlags}, + proto, proto::identity::PeerIdentityMsg, protocol, protocol::{NodeNetworkInfo, ProtocolId}, @@ -121,6 +122,9 @@ pub async fn validate_and_add_peer_from_peer_identity( peer.features = PeerFeatures::from_bits_truncate(peer_identity.features); peer.supported_protocols = supported_protocols.clone(); peer.user_agent = peer_identity.user_agent; + if let Some(identity_signature) = peer_identity.identity_signature { + add_valid_identity_signature_to_peer(&mut peer, identity_signature)?; + } peer }, None => { @@ -139,6 +143,10 @@ pub async fn validate_and_add_peer_from_peer_identity( peer_identity.user_agent, ); new_peer.connection_stats.set_connection_success(); + // TODO(testnetreset): Require an identity signature once majority nodes are upgraded + if let Some(identity_sig) = peer_identity.identity_signature { + add_valid_identity_signature_to_peer(&mut new_peer, identity_sig)?; + } if let Some(addr) = dialed_addr { new_peer.addresses.mark_last_seen_now(addr); } @@ -151,14 +159,32 @@ pub async fn validate_and_add_peer_from_peer_identity( Ok((peer_node_id, supported_protocols)) } +fn add_valid_identity_signature_to_peer( + peer: &mut Peer, + identity_sig: proto::identity::IdentitySignature, +) -> Result<(), ConnectionManagerError> { + let identity_sig = + IdentitySignature::try_from(identity_sig).map_err(|_| ConnectionManagerError::PeerIdentityInvalidSignature)?; + + if !identity_sig.is_valid_for_peer(peer) { + warn!( + target: LOG_TARGET, + "Peer {} sent invalid identity signature", peer.node_id + ); + return Err(ConnectionManagerError::PeerIdentityInvalidSignature); + } + + peer.identity_signature = Some(identity_sig); + Ok(()) +} + pub async fn find_unbanned_peer( peer_manager: &PeerManager, authenticated_public_key: &CommsPublicKey, ) -> Result, ConnectionManagerError> { match peer_manager.find_by_public_key(authenticated_public_key).await { - Ok(peer) if peer.is_banned() => Err(ConnectionManagerError::PeerBanned), - Ok(peer) => Ok(Some(peer)), - Err(err) if err.is_peer_not_found() => Ok(None), + Ok(Some(peer)) if peer.is_banned() => Err(ConnectionManagerError::PeerBanned), + Ok(peer) => Ok(peer), Err(err) => Err(err.into()), } } @@ -167,9 +193,14 @@ pub fn validate_peer_addresses<'a, A: IntoIterator>( addresses: A, allow_test_addrs: bool, ) -> Result<(), ConnectionManagerError> { + let mut has_address = false; for addr in addresses.into_iter() { + has_address = true; validate_address(addr, allow_test_addrs)?; } + if !has_address { + return Err(ConnectionManagerError::PeerHasNoAddresses); + } Ok(()) } diff --git a/comms/src/connection_manager/error.rs b/comms/src/connection_manager/error.rs index 2c2eb44fcd..b4121f1eb5 100644 --- a/comms/src/connection_manager/error.rs +++ b/comms/src/connection_manager/error.rs @@ -82,6 +82,12 @@ pub enum ConnectionManagerError { NoiseProtocolTimeout, #[error("Listener oneshot cancelled")] ListenerOneshotCancelled, + #[error("Peer sent invalid identity signature")] + PeerIdentityInvalidSignature, + #[error("Peer did not provide the identity timestamp")] + PeerIdentityNoUpdatedTimestampProvided, + #[error("Peer did not provide any public addresses")] + PeerHasNoAddresses, } impl From for ConnectionManagerError { diff --git a/comms/src/connection_manager/manager.rs b/comms/src/connection_manager/manager.rs index 7c4cf9b0c7..1d0bb6b496 100644 --- a/comms/src/connection_manager/manager.rs +++ b/comms/src/connection_manager/manager.rs @@ -46,7 +46,7 @@ use crate::{ connection_manager::{metrics, ConnectionDirection}, multiplexing::Substream, noise::NoiseConfig, - peer_manager::{NodeId, NodeIdentity}, + peer_manager::{NodeId, NodeIdentity, PeerManagerError}, protocol::{NodeNetworkInfo, ProtocolEvent, ProtocolId, Protocols}, transports::{TcpTransport, Transport}, PeerManager, @@ -460,10 +460,18 @@ where reply: Option>>, ) { match self.peer_manager.find_by_node_id(&node_id).await { - Ok(peer) => { + Ok(Some(peer)) => { self.send_dialer_request(DialerRequest::Dial(Box::new(peer), reply)) .await; }, + Ok(None) => { + warn!(target: LOG_TARGET, "Peer not found for dial"); + if let Some(reply) = reply { + let _ = reply.send(Err(ConnectionManagerError::PeerManagerError( + PeerManagerError::PeerNotFoundError, + ))); + } + }, Err(err) => { warn!(target: LOG_TARGET, "Failed to fetch peer to dial because '{}'", err); if let Some(reply) = reply { diff --git a/comms/src/connection_manager/tests/listener_dialer.rs b/comms/src/connection_manager/tests/listener_dialer.rs index 24b6d7c2ea..1fc21c7c76 100644 --- a/comms/src/connection_manager/tests/listener_dialer.rs +++ b/comms/src/connection_manager/tests/listener_dialer.rs @@ -166,8 +166,16 @@ async fn smoke() { shutdown.trigger(); - let peer2 = peer_manager1.find_by_node_id(node_identity2.node_id()).await.unwrap(); - let peer1 = peer_manager2.find_by_node_id(node_identity1.node_id()).await.unwrap(); + let peer2 = peer_manager1 + .find_by_node_id(node_identity2.node_id()) + .await + .unwrap() + .unwrap(); + let peer1 = peer_manager2 + .find_by_node_id(node_identity1.node_id()) + .await + .unwrap() + .unwrap(); assert_eq!(&peer1.public_key, node_identity1.public_key()); assert_eq!(&peer2.public_key, node_identity2.public_key()); diff --git a/comms/src/connection_manager/tests/manager.rs b/comms/src/connection_manager/tests/manager.rs index c6ebf133b6..ce27bc96c9 100644 --- a/comms/src/connection_manager/tests/manager.rs +++ b/comms/src/connection_manager/tests/manager.rs @@ -155,7 +155,11 @@ async fn dial_success() { let mut conn_out = conn_man1.dial_peer(node_identity2.node_id().clone()).await.unwrap(); assert_eq!(conn_out.peer_node_id(), node_identity2.node_id()); - let peer2 = peer_manager1.find_by_node_id(conn_out.peer_node_id()).await.unwrap(); + let peer2 = peer_manager1 + .find_by_node_id(conn_out.peer_node_id()) + .await + .unwrap() + .unwrap(); assert_eq!(peer2.supported_protocols, [&TEST_PROTO]); assert_eq!(peer2.user_agent, "node2"); @@ -163,7 +167,11 @@ async fn dial_success() { unpack_enum!(ConnectionManagerEvent::PeerConnected(conn_in) = &*event); assert_eq!(conn_in.peer_node_id(), node_identity1.node_id()); - let peer1 = peer_manager2.find_by_node_id(node_identity1.node_id()).await.unwrap(); + let peer1 = peer_manager2 + .find_by_node_id(node_identity1.node_id()) + .await + .unwrap() + .unwrap(); assert_eq!(peer1.supported_protocols(), [&TEST_PROTO]); assert_eq!(peer1.user_agent, "node1"); diff --git a/comms/src/connectivity/manager.rs b/comms/src/connectivity/manager.rs index b902d781f9..f37b113cd5 100644 --- a/comms/src/connectivity/manager.rs +++ b/comms/src/connectivity/manager.rs @@ -312,6 +312,9 @@ impl ConnectivityManagerActor { let mut node_ids = Vec::with_capacity(self.pool.count_connected()); for mut state in self.pool.filter_drain(|_| true) { if let Some(conn) = state.connection_mut() { + if !conn.is_connected() { + continue; + } match conn.disconnect_silent().await { Ok(_) => { node_ids.push(conn.peer_node_id().clone()); @@ -456,7 +459,7 @@ impl ConnectivityManagerActor { self.publish_event(ConnectivityEvent::PeerOffline(node_id.clone())); } - if let Ok(peer) = self.peer_manager.find_by_node_id(node_id).await { + if let Some(peer) = self.peer_manager.find_by_node_id(node_id).await? { if !peer.is_banned() && peer.last_seen_since() // Haven't seen them in expire_peer_last_seen_duration diff --git a/comms/src/connectivity/test.rs b/comms/src/connectivity/test.rs index 0cbcf8e1c7..23ec64a3e4 100644 --- a/comms/src/connectivity/test.rs +++ b/comms/src/connectivity/test.rs @@ -271,7 +271,7 @@ async fn ban_peer() { unpack_enum!(ConnectivityEvent::PeerBanned(node_id) = event); assert_eq!(node_id, peer.node_id); - let peer = peer_manager.find_by_node_id(&peer.node_id).await.unwrap(); + let peer = peer_manager.find_by_node_id(&peer.node_id).await.unwrap().unwrap(); assert!(peer.is_banned()); let conn = connectivity.get_connection(peer.node_id.clone()).await.unwrap(); diff --git a/comms/src/lib.rs b/comms/src/lib.rs index 99d7ef2c1e..463fc0aadd 100644 --- a/comms/src/lib.rs +++ b/comms/src/lib.rs @@ -28,7 +28,7 @@ pub use connection_manager::{validate_peer_addresses, PeerConnection, PeerConnec pub mod connectivity; pub mod peer_manager; -pub use peer_manager::{NodeIdentity, PeerManager}; +pub use peer_manager::{NodeIdentity, OrNotFound, PeerManager}; pub mod framing; diff --git a/comms/src/net_address/mutliaddresses_with_stats.rs b/comms/src/net_address/mutliaddresses_with_stats.rs index 908b3199e6..1cb9cfaa62 100644 --- a/comms/src/net_address/mutliaddresses_with_stats.rs +++ b/comms/src/net_address/mutliaddresses_with_stats.rs @@ -1,13 +1,17 @@ -use std::{ops::Index, time::Duration}; +use std::{ + fmt::{Display, Formatter}, + ops::Index, + time::Duration, +}; use chrono::{DateTime, Utc}; use multiaddr::Multiaddr; use serde::{Deserialize, Serialize}; -use super::multiaddr_with_stats::MutliaddrWithStats; +use crate::net_address::MutliaddrWithStats; /// This struct is used to store a set of different net addresses such as IPv4, IPv6, Tor or I2P for a single peer. -#[derive(Debug, Clone, Deserialize, Serialize, PartialEq, Default, Eq)] +#[derive(Debug, Clone, Deserialize, Serialize, Default, Eq)] pub struct MultiaddressesWithStats { pub addresses: Vec, last_attempted: Option>, @@ -52,39 +56,31 @@ impl MultiaddressesWithStats { /// Adds a new net address to the peer. This function will not add a duplicate if the address /// already exists. - pub fn add_net_address(&mut self, net_address: &Multiaddr) { + pub fn add_address(&mut self, net_address: &Multiaddr) { if !self.addresses.iter().any(|x| x.address == *net_address) { self.addresses.push(net_address.clone().into()); self.addresses.sort(); } } - /// Compares the existing set of net_addresses to the provided net_address set and remove missing net_addresses and - /// add new net_addresses without discarding the usage stats of the existing and remaining net_addresses. - pub fn update_net_addresses(&mut self, net_addresses: Vec) { - // Remove missing elements - let mut remove_indices: Vec = Vec::new(); - for index in 0..self.addresses.len() { - if net_addresses - .iter() - .all(|new_net_address| *new_net_address != self.addresses[index].address) - { - remove_indices.push(index); - } - } - for index in remove_indices.iter().rev() { - self.addresses.remove(*index); - } - // Add new elements - for new_net_address in &net_addresses { - if self - .addresses - .iter() - .all(|curr_net_address| curr_net_address.address != *new_net_address) - { - self.add_net_address(new_net_address); - } + /// Compares the existing set of addresses to the provided address set and remove missing addresses and + /// add new addresses without discarding the usage stats of the existing and remaining addresses. + pub fn update_addresses(&mut self, addresses: Vec) { + self.addresses = self + .addresses + .drain(..) + .filter(|addr| addresses.contains(&addr.address)) + .collect(); + + let to_add = addresses + .iter() + .filter(|addr| !self.addresses.iter().any(|a| a.address == **addr)) + .collect::>(); + + for address in to_add { + self.addresses.push(address.clone().into()); } + self.addresses.sort(); } @@ -191,6 +187,16 @@ impl MultiaddressesWithStats { pub fn is_empty(&self) -> bool { self.addresses.is_empty() } + + pub fn into_vec(self) -> Vec { + self.addresses.into_iter().map(|addr| addr.address).collect() + } +} + +impl PartialEq for MultiaddressesWithStats { + fn eq(&self, other: &Self) -> bool { + self.addresses == other.addresses + } } impl Index for MultiaddressesWithStats { @@ -235,6 +241,20 @@ impl From> for MultiaddressesWithStats { } } +impl Display for MultiaddressesWithStats { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + write!( + f, + "{}", + self.addresses + .iter() + .map(|a| a.address.to_string()) + .collect::>() + .join(", ") + ) + } +} + #[cfg(test)] mod test { use multiaddr::Multiaddr; @@ -260,8 +280,8 @@ mod test { let net_address2 = "/ip4/125.1.54.254/tcp/7999".parse::().unwrap(); let net_address3 = "/ip4/175.6.3.145/tcp/8000".parse::().unwrap(); let mut net_addresses = MultiaddressesWithStats::from(net_address1.clone()); - net_addresses.add_net_address(&net_address2); - net_addresses.add_net_address(&net_address3); + net_addresses.add_address(&net_address2); + net_addresses.add_address(&net_address3); assert!(net_addresses.mark_last_seen_now(&net_address3)); assert!(net_addresses.mark_last_seen_now(&net_address1)); @@ -277,10 +297,10 @@ mod test { let net_address2 = "/ip4/125.1.54.254/tcp/7999".parse::().unwrap(); let net_address3 = "/ip4/175.6.3.145/tcp/8000".parse::().unwrap(); let mut net_addresses = MultiaddressesWithStats::from(net_address1.clone()); - net_addresses.add_net_address(&net_address2); - net_addresses.add_net_address(&net_address3); + net_addresses.add_address(&net_address2); + net_addresses.add_address(&net_address3); // Add duplicate address, test add_net_address is idempotent - net_addresses.add_net_address(&net_address2); + net_addresses.add_address(&net_address2); assert_eq!(net_addresses.addresses.len(), 3); assert_eq!(net_addresses.addresses[0].address, net_address1); assert_eq!(net_addresses.addresses[1].address, net_address2); @@ -293,8 +313,8 @@ mod test { let net_address2 = "/ip4/125.1.54.254/tcp/7999".parse::().unwrap(); let net_address3 = "/ip4/175.6.3.145/tcp/8000".parse::().unwrap(); let mut net_addresses = MultiaddressesWithStats::from(net_address1.clone()); - net_addresses.add_net_address(&net_address2); - net_addresses.add_net_address(&net_address3); + net_addresses.add_address(&net_address2); + net_addresses.add_address(&net_address3); let priority_address = net_addresses.iter().next().unwrap(); assert_eq!(priority_address, &net_address1); diff --git a/comms/src/peer_manager/error.rs b/comms/src/peer_manager/error.rs index b7ebe43a11..f307c6df19 100644 --- a/comms/src/peer_manager/error.rs +++ b/comms/src/peer_manager/error.rs @@ -29,12 +29,16 @@ use thiserror::Error; pub enum PeerManagerError { #[error("The requested peer does not exist")] PeerNotFoundError, + #[error("DB Data inconsistency: {0}")] + DataInconsistency(String), #[error("The peer has been banned")] BannedPeer, #[error("A problem has been encountered with the database: {0}")] DatabaseError(#[from] KeyValStoreError), #[error("An error occurred while migrating the database: {0}")] MigrationError(String), + #[error("Identity signature is invalid")] + InvalidIdentitySignature, } impl PeerManagerError { diff --git a/comms/src/peer_manager/identity_signature.rs b/comms/src/peer_manager/identity_signature.rs new file mode 100644 index 0000000000..902fb59cf8 --- /dev/null +++ b/comms/src/peer_manager/identity_signature.rs @@ -0,0 +1,251 @@ +// Copyright 2021, The Tari Project +// +// Redistribution and use in source and binary forms, with or without modification, are permitted provided that the +// following conditions are met: +// +// 1. Redistributions of source code must retain the above copyright notice, this list of conditions and the following +// disclaimer. +// +// 2. Redistributions in binary form must reproduce the above copyright notice, this list of conditions and the +// following disclaimer in the documentation and/or other materials provided with the distribution. +// +// 3. Neither the name of the copyright holder nor the names of its contributors may be used to endorse or promote +// products derived from this software without specific prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, +// INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE +// DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR +// SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, +// WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE +// USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +use std::convert::{TryFrom, TryInto}; + +use chrono::{NaiveDateTime, Utc}; +use digest::Digest; +use prost::Message; +use rand::rngs::OsRng; +use serde::{Deserialize, Serialize}; +use tari_crypto::{keys::SecretKey, tari_utilities::ByteArray}; + +use crate::{ + message::MessageExt, + multiaddr::Multiaddr, + peer_manager::{Peer, PeerFeatures, PeerManagerError}, + proto, + types::{Challenge, CommsPublicKey, CommsSecretKey, Signature}, +}; + +/// Signature that secures the peer identity +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] +pub struct IdentitySignature { + version: u8, + signature: Signature, + updated_at: NaiveDateTime, +} + +impl IdentitySignature { + pub const LATEST_VERSION: u8 = 0; + + pub fn new(version: u8, signature: Signature, updated_at: NaiveDateTime) -> Self { + Self { + version, + signature, + updated_at, + } + } + + pub fn sign_new<'a, I: IntoIterator>( + secret_key: &CommsSecretKey, + features: PeerFeatures, + addresses: I, + updated_at: NaiveDateTime, + ) -> Self { + let challenge = Self::construct_challenge(Self::LATEST_VERSION, features, addresses, updated_at); + let nonce = CommsSecretKey::random(&mut OsRng); + let signature = Signature::sign(secret_key.clone(), nonce, &challenge.finalize()) + .expect("unreachable panic: challenge hash digest is the correct length"); + Self { + version: Self::LATEST_VERSION, + signature, + updated_at, + } + } + + pub fn signature(&self) -> &Signature { + &self.signature + } + + pub fn updated_at(&self) -> NaiveDateTime { + self.updated_at + } + + pub fn version(&self) -> u8 { + self.version + } + + pub fn is_valid_for_peer(&self, peer: &Peer) -> bool { + self.is_valid(&peer.public_key, peer.features, peer.addresses.iter()) + } + + pub fn is_valid<'a, I: IntoIterator>( + &self, + public_key: &CommsPublicKey, + features: PeerFeatures, + addresses: I, + ) -> bool { + // A negative timestamp is considered invalid + if self.updated_at.timestamp() < 0 { + return false; + } + // Do not accept timestamp more than 1 day in the future + if self.updated_at.timestamp() > (Utc::now().timestamp() + 24 * 60 * 60) { + return false; + } + + let challenge = Self::construct_challenge(self.version, features, addresses, self.updated_at); + self.signature.verify_challenge(public_key, &challenge.finalize()) + } + + fn construct_challenge<'a, I: IntoIterator>( + version: u8, + features: PeerFeatures, + addresses: I, + updated_at: NaiveDateTime, + ) -> Challenge { + let challenge = Challenge::new() + .chain(version.to_le_bytes()) + .chain(u64::try_from(updated_at.timestamp()).unwrap_or(0).to_le_bytes()) + .chain(features.bits().to_le_bytes()); + addresses + .into_iter() + .fold(challenge, |challenge, addr| challenge.chain(&addr)) + } + + pub fn to_bytes(&self) -> Vec { + proto::identity::IdentitySignature::from(self).to_encoded_bytes() + } + + pub fn from_bytes(bytes: &[u8]) -> Result { + let sig = proto::identity::IdentitySignature::decode(bytes) + .map_err(|_| PeerManagerError::InvalidIdentitySignature)? + .try_into()?; + Ok(sig) + } +} + +impl TryFrom for IdentitySignature { + type Error = PeerManagerError; + + fn try_from(value: proto::identity::IdentitySignature) -> Result { + let version = u8::try_from(value.version).map_err(|_| PeerManagerError::InvalidIdentitySignature)?; + let public_nonce = + CommsPublicKey::from_bytes(&value.public_nonce).map_err(|_| PeerManagerError::InvalidIdentitySignature)?; + let signature = + CommsSecretKey::from_bytes(&value.signature).map_err(|_| PeerManagerError::InvalidIdentitySignature)?; + let updated_at = + NaiveDateTime::from_timestamp_opt(value.updated_at, 0).ok_or(PeerManagerError::InvalidIdentitySignature)?; + + Ok(Self { + version, + signature: Signature::new(public_nonce, signature), + updated_at, + }) + } +} + +impl From<&IdentitySignature> for proto::identity::IdentitySignature { + fn from(identity_sig: &IdentitySignature) -> Self { + proto::identity::IdentitySignature { + version: identity_sig.version as u32, + signature: identity_sig.signature.get_signature().to_vec(), + public_nonce: identity_sig.signature.get_public_nonce().to_vec(), + updated_at: identity_sig.updated_at.timestamp(), + } + } +} + +#[cfg(test)] +mod test { + use std::str::FromStr; + + use tari_crypto::keys::PublicKey; + + use super::*; + use crate::peer_manager::{NodeId, PeerFlags}; + + mod is_valid_for_peer { + use super::*; + + #[test] + fn it_returns_true_for_valid_signature() { + let secret = CommsSecretKey::random(&mut OsRng); + let public_key = CommsPublicKey::from_secret_key(&secret); + let address = Multiaddr::from_str("/ip4/127.0.0.1/tcp/1234").unwrap(); + let updated_at = Utc::now().naive_utc(); + let identity = + IdentitySignature::sign_new(&secret, PeerFeatures::COMMUNICATION_NODE, [&address], updated_at); + let node_id = NodeId::from_public_key(&public_key); + + let peer = Peer::new( + public_key, + node_id, + vec![address].into(), + PeerFlags::empty(), + PeerFeatures::COMMUNICATION_NODE, + vec![], + String::new(), + ); + assert!(identity.is_valid_for_peer(&peer)); + } + + #[test] + fn it_returns_false_for_tampered_address() { + let secret = CommsSecretKey::random(&mut OsRng); + let public_key = CommsPublicKey::from_secret_key(&secret); + let address = Multiaddr::from_str("/ip4/127.0.0.1/tcp/1234").unwrap(); + let updated_at = Utc::now().naive_utc(); + let identity = + IdentitySignature::sign_new(&secret, PeerFeatures::COMMUNICATION_NODE, [&address], updated_at); + let node_id = NodeId::from_public_key(&public_key); + + let tampered = Multiaddr::from_str("/ip4/127.0.0.1/tcp/4321").unwrap(); + + let peer = Peer::new( + public_key, + node_id, + vec![tampered].into(), + PeerFlags::empty(), + PeerFeatures::COMMUNICATION_NODE, + vec![], + String::new(), + ); + assert!(!identity.is_valid_for_peer(&peer)); + } + + #[test] + fn it_returns_false_for_tampered_features() { + let secret = CommsSecretKey::random(&mut OsRng); + let public_key = CommsPublicKey::from_secret_key(&secret); + let address = Multiaddr::from_str("/ip4/127.0.0.1/tcp/1234").unwrap(); + let updated_at = Utc::now().naive_utc(); + let identity = + IdentitySignature::sign_new(&secret, PeerFeatures::COMMUNICATION_NODE, [&address], updated_at); + let node_id = NodeId::from_public_key(&public_key); + + let tampered = PeerFeatures::COMMUNICATION_CLIENT; + + let peer = Peer::new( + public_key, + node_id, + vec![address].into(), + PeerFlags::empty(), + tampered, + vec![], + String::new(), + ); + assert!(!identity.is_valid_for_peer(&peer)); + } + } +} diff --git a/comms/src/peer_manager/manager.rs b/comms/src/peer_manager/manager.rs index 399a9548ae..e10b0be988 100644 --- a/comms/src/peer_manager/manager.rs +++ b/comms/src/peer_manager/manager.rs @@ -87,20 +87,20 @@ impl PeerManager { } /// Find the peer with the provided NodeID - pub async fn find_by_node_id(&self, node_id: &NodeId) -> Result { + pub async fn find_by_node_id(&self, node_id: &NodeId) -> Result, PeerManagerError> { self.peer_storage.read().await.find_by_node_id(node_id) } + /// Find the peer with the provided PublicKey + pub async fn find_by_public_key(&self, public_key: &CommsPublicKey) -> Result, PeerManagerError> { + self.peer_storage.read().await.find_by_public_key(public_key) + } + /// Find the peer with the provided substring. This currently only compares the given bytes to the NodeId pub async fn find_all_starts_with(&self, partial: &[u8]) -> Result, PeerManagerError> { self.peer_storage.read().await.find_all_starts_with(partial) } - /// Find the peer with the provided PublicKey - pub async fn find_by_public_key(&self, public_key: &CommsPublicKey) -> Result { - self.peer_storage.read().await.find_by_public_key(public_key) - } - /// Check if a peer exist using the specified public_key pub async fn exists(&self, public_key: &CommsPublicKey) -> bool { self.peer_storage.read().await.exists(public_key) @@ -126,7 +126,7 @@ impl PeerManager { peer_features: PeerFeatures, ) -> Result { match self.find_by_public_key(pubkey).await { - Ok(mut peer) => { + Ok(Some(mut peer)) => { peer.connection_stats.set_connection_success(); peer.addresses = addresses.into(); peer.set_offline(false); @@ -134,7 +134,7 @@ impl PeerManager { self.add_peer(peer.clone()).await?; Ok(peer) }, - Err(PeerManagerError::PeerNotFoundError) => { + Ok(None) => { self.add_peer(Peer::new( pubkey.clone(), node_id, @@ -146,7 +146,9 @@ impl PeerManager { )) .await?; - self.find_by_public_key(pubkey).await + self.find_by_public_key(pubkey) + .await? + .ok_or(PeerManagerError::PeerNotFoundError) }, Err(err) => Err(err), } @@ -292,7 +294,10 @@ impl PeerManager { } pub async fn get_peer_features(&self, node_id: &NodeId) -> Result { - let peer = self.find_by_node_id(node_id).await?; + let peer = self + .find_by_node_id(node_id) + .await? + .ok_or(PeerManagerError::PeerNotFoundError)?; Ok(peer.features) } @@ -397,6 +402,7 @@ mod test { .find_by_node_id(&peer_identity.node_id) .await .unwrap() + .unwrap() .is_banned(),); } diff --git a/comms/src/peer_manager/migrations.rs b/comms/src/peer_manager/migrations.rs index 82b3594abd..c05ae4dfe9 100644 --- a/comms/src/peer_manager/migrations.rs +++ b/comms/src/peer_manager/migrations.rs @@ -21,6 +21,7 @@ // USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. mod v4; +mod v5; use log::*; use tari_storage::lmdb_store::{LMDBDatabase, LMDBError}; @@ -30,7 +31,8 @@ const LOG_TARGET: &str = "comms::peer_manager::migrations"; pub(super) const MIGRATION_VERSION_KEY: u64 = u64::MAX; pub fn migrate(database: &LMDBDatabase) -> Result<(), LMDBError> { - let migrations = vec![v4::Migration.boxed()]; + // Add migrations here in version order + let migrations = vec![v4::Migration.boxed(), v5::Migration.boxed()]; if migrations.is_empty() { return Ok(()); } diff --git a/comms/src/peer_manager/migrations/v5.rs b/comms/src/peer_manager/migrations/v5.rs new file mode 100644 index 0000000000..e32ef3c01d --- /dev/null +++ b/comms/src/peer_manager/migrations/v5.rs @@ -0,0 +1,144 @@ +// Copyright 2020, The Tari Project +// +// Redistribution and use in source and binary forms, with or without modification, are permitted provided that the +// following conditions are met: +// +// 1. Redistributions of source code must retain the above copyright notice, this list of conditions and the following +// disclaimer. +// +// 2. Redistributions in binary form must reproduce the above copyright notice, this list of conditions and the +// following disclaimer in the documentation and/or other materials provided with the distribution. +// +// 3. Neither the name of the copyright holder nor the names of its contributors may be used to endorse or promote +// products derived from this software without specific prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, +// INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE +// DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR +// SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, +// WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE +// USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +use std::collections::HashMap; + +use chrono::NaiveDateTime; +use log::*; +use serde::{Deserialize, Serialize}; +use tari_crypto::tari_utilities::hex::serialize_to_hex; +use tari_storage::{ + lmdb_store::{LMDBDatabase, LMDBError}, + IterationResult, +}; + +use crate::{ + net_address::MultiaddressesWithStats, + peer_manager::{ + connection_stats::PeerConnectionStats, + migrations::MIGRATION_VERSION_KEY, + node_id::deserialize_node_id_from_hex, + IdentitySignature, + NodeId, + PeerFeatures, + PeerFlags, + PeerId, + }, + protocol::ProtocolId, + types::CommsPublicKey, +}; + +const LOG_TARGET: &str = "comms::peer_manager::migrations::v4"; + +#[derive(Debug, Deserialize, Serialize)] +pub struct PeerV4 { + pub(super) id: Option, + pub public_key: CommsPublicKey, + #[serde(serialize_with = "serialize_to_hex")] + #[serde(deserialize_with = "deserialize_node_id_from_hex")] + pub node_id: NodeId, + pub addresses: MultiaddressesWithStats, + pub flags: PeerFlags, + pub banned_until: Option, + pub banned_reason: String, + pub offline_at: Option, + pub last_seen: Option, + pub features: PeerFeatures, + pub connection_stats: PeerConnectionStats, + pub supported_protocols: Vec, + pub added_at: NaiveDateTime, + pub user_agent: String, + pub metadata: HashMap>, +} + +#[derive(Debug, Deserialize, Serialize)] +pub struct PeerV5 { + pub(super) id: Option, + pub public_key: CommsPublicKey, + #[serde(serialize_with = "serialize_to_hex")] + #[serde(deserialize_with = "deserialize_node_id_from_hex")] + pub node_id: NodeId, + pub addresses: MultiaddressesWithStats, + pub flags: PeerFlags, + pub banned_until: Option, + pub banned_reason: String, + pub offline_at: Option, + pub last_seen: Option, + pub features: PeerFeatures, + pub connection_stats: PeerConnectionStats, + pub supported_protocols: Vec, + pub added_at: NaiveDateTime, + pub user_agent: String, + pub metadata: HashMap>, + pub identity_signature: Option, +} + +pub struct Migration; + +impl super::Migration for Migration { + type Error = LMDBError; + + fn get_version(&self) -> u32 { + 5 + } + + fn migrate(&self, db: &LMDBDatabase) -> Result<(), Self::Error> { + db.for_each::(|old_peer| { + let result = old_peer.and_then(|(key, peer)| { + if key == MIGRATION_VERSION_KEY { + return Ok(()); + } + + debug!(target: LOG_TARGET, "Migrating peer `{}`", peer.node_id.short_str()); + db.insert(&key, &PeerV5 { + id: peer.id, + public_key: peer.public_key, + node_id: peer.node_id, + addresses: peer.addresses, + flags: peer.flags, + banned_until: peer.banned_until, + banned_reason: peer.banned_reason, + offline_at: peer.offline_at, + last_seen: peer.last_seen, + features: peer.features, + connection_stats: peer.connection_stats, + supported_protocols: peer.supported_protocols, + added_at: peer.added_at, + user_agent: peer.user_agent, + metadata: peer.metadata, + identity_signature: None, + }) + .map_err(Into::into) + }); + + if let Err(err) = result { + error!( + target: LOG_TARGET, + "Failed to deserialize peer: {} ** Database may be corrupt **", err + ); + } + IterationResult::Continue + })?; + + Ok(()) + } +} diff --git a/comms/src/peer_manager/mod.rs b/comms/src/peer_manager/mod.rs index 8c1ca6bfc5..ec8466d9ed 100644 --- a/comms/src/peer_manager/mod.rs +++ b/comms/src/peer_manager/mod.rs @@ -75,6 +75,9 @@ mod connection_stats; mod error; pub use error::PeerManagerError; +mod identity_signature; +pub use identity_signature::IdentitySignature; + pub mod node_id; pub use node_id::NodeId; @@ -104,4 +107,7 @@ pub use peer_storage::PeerStorage; mod migrations; +mod or_not_found; +pub use or_not_found::OrNotFound; + mod wrapper; diff --git a/comms/src/peer_manager/node_identity.rs b/comms/src/peer_manager/node_identity.rs index d063b55943..7d72aa0713 100644 --- a/comms/src/peer_manager/node_identity.rs +++ b/comms/src/peer_manager/node_identity.rs @@ -20,8 +20,12 @@ // WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE // USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -use std::{fmt, sync::RwLock}; +use std::{ + fmt, + sync::{RwLock, RwLockReadGuard}, +}; +use chrono::Utc; use multiaddr::Multiaddr; use rand::{CryptoRng, Rng}; use serde::{Deserialize, Serialize}; @@ -32,7 +36,7 @@ use tari_crypto::{ use super::node_id::deserialize_node_id_from_hex; use crate::{ - peer_manager::{node_id::NodeId, Peer, PeerFeatures, PeerFlags}, + peer_manager::{identity_signature::IdentitySignature, node_id::NodeId, Peer, PeerFeatures, PeerFlags}, types::{CommsPublicKey, CommsSecretKey}, }; @@ -46,6 +50,12 @@ pub struct NodeIdentity { features: PeerFeatures, secret_key: CommsSecretKey, public_address: RwLock, + #[serde(default = "rwlock_none")] + identity_signature: RwLock>, +} + +fn rwlock_none() -> RwLock> { + RwLock::new(None) } impl NodeIdentity { @@ -54,19 +64,29 @@ impl NodeIdentity { let public_key = CommsPublicKey::from_secret_key(&secret_key); let node_id = NodeId::from_key(&public_key); - NodeIdentity { + let node_identity = NodeIdentity { node_id, public_key, features, secret_key, public_address: RwLock::new(public_address), - } + identity_signature: RwLock::new(None), + }; + node_identity.sign(); + node_identity } - /// Generates a new random NodeIdentity for CommsPublicKey - pub fn random(rng: &mut R, public_address: Multiaddr, features: PeerFeatures) -> Self - where R: CryptoRng + Rng { - let secret_key = CommsSecretKey::random(rng); + /// Create a new NodeIdentity from the provided key pair and control service address. + /// + /// # Unchecked + /// It is up to the caller to ensure that the given signature is valid for the node identity. + /// Prefer using NodeIdentity::new over this function. + pub fn with_signature_unchecked( + secret_key: CommsSecretKey, + public_address: Multiaddr, + features: PeerFeatures, + identity_signature: Option, + ) -> Self { let public_key = CommsPublicKey::from_secret_key(&secret_key); let node_id = NodeId::from_key(&public_key); @@ -76,17 +96,26 @@ impl NodeIdentity { features, secret_key, public_address: RwLock::new(public_address), + identity_signature: RwLock::new(identity_signature), } } + /// Generates a new random NodeIdentity for CommsPublicKey + pub fn random(rng: &mut R, public_address: Multiaddr, features: PeerFeatures) -> Self + where R: CryptoRng + Rng { + let secret_key = CommsSecretKey::random(rng); + Self::new(secret_key, public_address, features) + } + /// Retrieve the publicly accessible address that peers must connect to establish a connection pub fn public_address(&self) -> Multiaddr { acquire_read_lock!(self.public_address).clone() } - /// Modify the control_service_address + /// Modify the public address. The account signature will be invalid pub fn set_public_address(&self, address: Multiaddr) { *acquire_write_lock!(self.public_address) = address; + self.sign() } /// This returns a random NodeIdentity for testing purposes. This function can panic. If public_address @@ -102,35 +131,50 @@ impl NodeIdentity { ) } - #[inline] pub fn node_id(&self) -> &NodeId { &self.node_id } - #[inline] pub fn public_key(&self) -> &CommsPublicKey { &self.public_key } - #[inline] pub fn secret_key(&self) -> &CommsSecretKey { &self.secret_key } - #[inline] pub fn features(&self) -> PeerFeatures { self.features } - #[inline] pub fn has_peer_features(&self, peer_features: PeerFeatures) -> bool { self.features().contains(peer_features) } + pub fn identity_signature_read(&self) -> RwLockReadGuard<'_, Option> { + acquire_read_lock!(self.identity_signature) + } + + pub fn is_signed(&self) -> bool { + self.identity_signature_read().is_some() + } + + /// Signs the peer using the peer secret key and replaces the peer account signature. + pub fn sign(&self) { + let identity_sig = IdentitySignature::sign_new( + self.secret_key(), + self.features, + Some(&*acquire_read_lock!(self.public_address)), + Utc::now().naive_utc(), + ); + + *acquire_write_lock!(self.identity_signature) = Some(identity_sig); + } + /// Returns a Peer with the same public key, node id, public address and features as represented in this /// NodeIdentity. _NOTE: PeerFlags, supported_protocols and user agent are empty._ pub fn to_peer(&self) -> Peer { - Peer::new( + let mut peer = Peer::new( self.public_key().clone(), self.node_id().clone(), self.public_address().into(), @@ -138,7 +182,10 @@ impl NodeIdentity { self.features(), Default::default(), Default::default(), - ) + ); + peer.identity_signature = acquire_read_lock!(self.identity_signature).clone(); + + peer } } @@ -150,6 +197,7 @@ impl Clone for NodeIdentity { features: self.features, secret_key: self.secret_key.clone(), public_address: RwLock::new(self.public_address()), + identity_signature: RwLock::new(self.identity_signature_read().as_ref().cloned()), } } } diff --git a/comms/src/peer_manager/or_not_found.rs b/comms/src/peer_manager/or_not_found.rs new file mode 100644 index 0000000000..8323206b3f --- /dev/null +++ b/comms/src/peer_manager/or_not_found.rs @@ -0,0 +1,38 @@ +// Copyright 2021, The Tari Project +// +// Redistribution and use in source and binary forms, with or without modification, are permitted provided that the +// following conditions are met: +// +// 1. Redistributions of source code must retain the above copyright notice, this list of conditions and the following +// disclaimer. +// +// 2. Redistributions in binary form must reproduce the above copyright notice, this list of conditions and the +// following disclaimer in the documentation and/or other materials provided with the distribution. +// +// 3. Neither the name of the copyright holder nor the names of its contributors may be used to endorse or promote +// products derived from this software without specific prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, +// INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE +// DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR +// SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, +// WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE +// USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +use crate::peer_manager::PeerManagerError; + +pub trait OrNotFound { + type Value; + type Error; + fn or_not_found(self) -> Result; +} + +impl OrNotFound for Result, PeerManagerError> { + type Error = PeerManagerError; + type Value = T; + + fn or_not_found(self) -> Result { + self.and_then(|val| val.ok_or(PeerManagerError::PeerNotFoundError)) + } +} diff --git a/comms/src/peer_manager/peer.rs b/comms/src/peer_manager/peer.rs index 7e6482cedf..c2c3fee8f2 100644 --- a/comms/src/peer_manager/peer.rs +++ b/comms/src/peer_manager/peer.rs @@ -41,6 +41,7 @@ use super::{ }; use crate::{ net_address::MultiaddressesWithStats, + peer_manager::identity_signature::IdentitySignature, protocol::ProtocolId, types::CommsPublicKey, utils::datetime::safe_future_datetime_from_duration, @@ -94,6 +95,9 @@ pub struct Peer { /// Metadata field. This field is for use by upstream clients to record extra info about a peer. /// We use a hashmap here so that we can use more than one "info set" pub metadata: HashMap>, + /// Signs the peer information with a timestamp to prevent malleability. This is optional for backward + /// compatibility, but without this, the identity (addresses etc) cannot be updated. + pub identity_signature: Option, } impl Peer { @@ -115,7 +119,7 @@ impl Peer { flags, features, banned_until: None, - banned_reason: "".to_string(), + banned_reason: String::new(), offline_at: None, last_seen: None, connection_stats: Default::default(), @@ -123,6 +127,7 @@ impl Peer { supported_protocols, user_agent, metadata: HashMap::new(), + identity_signature: None, } } @@ -185,7 +190,8 @@ impl Peer { supported_protocols: Option>, ) { if let Some(new_net_addresses) = net_addresses { - self.addresses.update_net_addresses(new_net_addresses) + self.addresses.update_addresses(new_net_addresses); + self.identity_signature = None; } if let Some(new_flags) = flags { self.flags = new_flags @@ -203,12 +209,20 @@ impl Peer { } if let Some(new_features) = features { self.features = new_features; + self.identity_signature = None; } if let Some(supported_protocols) = supported_protocols { self.supported_protocols = supported_protocols; } } + /// Returns `Some(true)` if the identity signature is valid, otherwise `Some(false)`. If no signature is present, + /// None is returned. + pub fn is_valid_identity_signature(&self) -> Option { + let identity_signature = self.identity_signature.as_ref()?; + Some(identity_signature.is_valid_for_peer(self)) + } + /// Provides that date time of the last successful interaction with the peer pub fn last_seen(&self) -> Option { self.last_seen diff --git a/comms/src/peer_manager/peer_storage.rs b/comms/src/peer_manager/peer_storage.rs index 248b66d5d6..0f9ca71ac3 100644 --- a/comms/src/peer_manager/peer_storage.rs +++ b/comms/src/peer_manager/peer_storage.rs @@ -205,21 +205,23 @@ where DS: KeyValueStore } /// Find the peer with the provided NodeID - pub fn find_by_node_id(&self, node_id: &NodeId) -> Result { - let peer_key = self - .node_id_index - .get(node_id) - .ok_or(PeerManagerError::PeerNotFoundError)?; - self.peer_db - .get(peer_key) - .map_err(PeerManagerError::DatabaseError)? - .ok_or_else(|| { - warn!( - target: LOG_TARGET, - "node_id_index and peer database are out of sync! (key={}, node_id={})", peer_key, node_id - ); - PeerManagerError::PeerNotFoundError - }) + pub fn find_by_node_id(&self, node_id: &NodeId) -> Result, PeerManagerError> { + match self.node_id_index.get(node_id) { + Some(peer_key) => { + let peer = self.peer_db.get(peer_key)?.ok_or_else(|| { + warn!( + target: LOG_TARGET, + "node_id_index and peer database are out of sync! (key={}, node_id={})", peer_key, node_id + ); + PeerManagerError::DataInconsistency(format!( + "node_id_index and peer database are out of sync! (key={}, node_id={})", + peer_key, node_id + )) + })?; + Ok(Some(peer)) + }, + None => Ok(None), + } } pub fn find_all_starts_with(&self, partial: &[u8]) -> Result, PeerManagerError> { @@ -240,23 +242,30 @@ where DS: KeyValueStore } /// Find the peer with the provided PublicKey - pub fn find_by_public_key(&self, public_key: &CommsPublicKey) -> Result { - let peer_key = self - .public_key_index - .get(public_key) - .ok_or(PeerManagerError::PeerNotFoundError)?; - self.peer_db - .get(peer_key) - .map_err(PeerManagerError::DatabaseError)? - .ok_or_else(|| { - warn!( - target: LOG_TARGET, - "public_key_index and peer database are out of sync! (key={}, public_key ={})", - peer_key, - public_key - ); - PeerManagerError::PeerNotFoundError - }) + pub fn find_by_public_key(&self, public_key: &CommsPublicKey) -> Result, PeerManagerError> { + match self.public_key_index.get(public_key) { + Some(peer_key) => { + let peer = self + .peer_db + .get(peer_key) + .map_err(PeerManagerError::DatabaseError)? + .ok_or_else(|| { + warn!( + target: LOG_TARGET, + "public_key_index and peer database are out of sync! (key={}, public_key ={})", + peer_key, + public_key + ); + PeerManagerError::DataInconsistency(format!( + "public_key_index and peer database are out of sync! (key={}, public_key ={})", + peer_key, public_key + )) + })?; + + Ok(Some(peer)) + }, + None => Ok(None), + } } /// Check if a peer exist using the specified public_key @@ -271,7 +280,9 @@ where DS: KeyValueStore /// Constructs a single NodeIdentity for the peer corresponding to the provided NodeId pub fn direct_identity_node_id(&self, node_id: &NodeId) -> Result { - let peer = self.find_by_node_id(node_id)?; + let peer = self + .find_by_node_id(node_id)? + .ok_or(PeerManagerError::PeerNotFoundError)?; if peer.is_banned() { Err(PeerManagerError::BannedPeer) @@ -282,7 +293,10 @@ where DS: KeyValueStore /// Constructs a single NodeIdentity for the peer corresponding to the provided NodeId pub fn direct_identity_public_key(&self, public_key: &CommsPublicKey) -> Result { - let peer = self.find_by_public_key(public_key)?; + let peer = self + .find_by_public_key(public_key)? + .ok_or(PeerManagerError::PeerNotFoundError)?; + if peer.is_banned() { Err(PeerManagerError::BannedPeer) } else { @@ -514,7 +528,7 @@ where DS: KeyValueStore .get(&peer_key) .map_err(PeerManagerError::DatabaseError)? .expect("node_id_index is out of sync with peer db"); - peer.addresses.add_net_address(net_address); + peer.addresses.add_address(net_address); self.peer_db .insert(peer_key, peer) .map_err(PeerManagerError::DatabaseError) @@ -545,7 +559,9 @@ where DS: KeyValueStore } pub fn mark_last_seen(&mut self, node_id: &NodeId) -> Result<(), PeerManagerError> { - let mut peer = self.find_by_node_id(node_id)?; + let mut peer = self + .find_by_node_id(node_id)? + .ok_or(PeerManagerError::PeerNotFoundError)?; peer.last_seen = Some(Utc::now().naive_utc()); peer.set_offline(false); self.peer_db @@ -585,8 +601,8 @@ mod test { let net_address2 = "/ip4/5.6.7.8/tcp/8000".parse::().unwrap(); let net_address3 = "/ip4/5.6.7.8/tcp/7000".parse::().unwrap(); let mut net_addresses = MultiaddressesWithStats::from(net_address1); - net_addresses.add_net_address(&net_address2); - net_addresses.add_net_address(&net_address3); + net_addresses.add_address(&net_address2); + net_addresses.add_address(&net_address3); let peer1 = Peer::new( pk, node_id, @@ -616,7 +632,7 @@ mod test { let net_address5 = "/ip4/13.14.15.16/tcp/6000".parse::().unwrap(); let net_address6 = "/ip4/17.18.19.20/tcp/8000".parse::().unwrap(); let mut net_addresses = MultiaddressesWithStats::from(net_address5); - net_addresses.add_net_address(&net_address6); + net_addresses.add_address(&net_address6); let peer3 = Peer::new( pk, node_id, @@ -664,8 +680,8 @@ mod test { let net_address2 = "/ip4/5.6.7.8/tcp/8000".parse::().unwrap(); let net_address3 = "/ip4/5.6.7.8/tcp/7000".parse::().unwrap(); let mut net_addresses = MultiaddressesWithStats::from(net_address1); - net_addresses.add_net_address(&net_address2); - net_addresses.add_net_address(&net_address3); + net_addresses.add_address(&net_address2); + net_addresses.add_address(&net_address3); let peer1 = Peer::new( pk, node_id, @@ -695,7 +711,7 @@ mod test { let net_address5 = "/ip4/13.14.15.16/tcp/6000".parse::().unwrap(); let net_address6 = "/ip4/17.18.19.20/tcp/8000".parse::().unwrap(); let mut net_addresses = MultiaddressesWithStats::from(net_address5); - net_addresses.add_net_address(&net_address6); + net_addresses.add_address(&net_address6); let peer3 = Peer::new( pk, node_id, @@ -713,34 +729,46 @@ mod test { assert_eq!(peer_storage.peer_db.len().unwrap(), 3); assert_eq!( - peer_storage.find_by_public_key(&peer1.public_key).unwrap().public_key, + peer_storage + .find_by_public_key(&peer1.public_key) + .unwrap() + .unwrap() + .public_key, peer1.public_key ); assert_eq!( - peer_storage.find_by_public_key(&peer2.public_key).unwrap().public_key, + peer_storage + .find_by_public_key(&peer2.public_key) + .unwrap() + .unwrap() + .public_key, peer2.public_key ); assert_eq!( - peer_storage.find_by_public_key(&peer3.public_key).unwrap().public_key, + peer_storage + .find_by_public_key(&peer3.public_key) + .unwrap() + .unwrap() + .public_key, peer3.public_key ); assert_eq!( - peer_storage.find_by_node_id(&peer1.node_id).unwrap().node_id, + peer_storage.find_by_node_id(&peer1.node_id).unwrap().unwrap().node_id, peer1.node_id ); assert_eq!( - peer_storage.find_by_node_id(&peer2.node_id).unwrap().node_id, + peer_storage.find_by_node_id(&peer2.node_id).unwrap().unwrap().node_id, peer2.node_id ); assert_eq!( - peer_storage.find_by_node_id(&peer3.node_id).unwrap().node_id, + peer_storage.find_by_node_id(&peer3.node_id).unwrap().unwrap().node_id, peer3.node_id ); - assert!(peer_storage.find_by_public_key(&peer1.public_key).is_ok()); - assert!(peer_storage.find_by_public_key(&peer2.public_key).is_ok()); - assert!(peer_storage.find_by_public_key(&peer3.public_key).is_ok()); + peer_storage.find_by_public_key(&peer1.public_key).unwrap().unwrap(); + peer_storage.find_by_public_key(&peer2.public_key).unwrap().unwrap(); + peer_storage.find_by_public_key(&peer3.public_key).unwrap().unwrap(); // Test delete of border case peer assert!(peer_storage.delete_peer(&peer3.node_id).is_ok()); @@ -748,28 +776,36 @@ mod test { assert_eq!(peer_storage.peer_db.len().unwrap(), 2); assert_eq!( - peer_storage.find_by_public_key(&peer1.public_key).unwrap().public_key, + peer_storage + .find_by_public_key(&peer1.public_key) + .unwrap() + .unwrap() + .public_key, peer1.public_key ); assert_eq!( - peer_storage.find_by_public_key(&peer2.public_key).unwrap().public_key, + peer_storage + .find_by_public_key(&peer2.public_key) + .unwrap() + .unwrap() + .public_key, peer2.public_key ); - assert!(peer_storage.find_by_public_key(&peer3.public_key).is_err()); + assert!(peer_storage.find_by_public_key(&peer3.public_key).unwrap().is_none()); assert_eq!( - peer_storage.find_by_node_id(&peer1.node_id).unwrap().node_id, + peer_storage.find_by_node_id(&peer1.node_id).unwrap().unwrap().node_id, peer1.node_id ); assert_eq!( - peer_storage.find_by_node_id(&peer2.node_id).unwrap().node_id, + peer_storage.find_by_node_id(&peer2.node_id).unwrap().unwrap().node_id, peer2.node_id ); - assert!(peer_storage.find_by_node_id(&peer3.node_id).is_err()); + assert!(peer_storage.find_by_node_id(&peer3.node_id).unwrap().is_none()); - assert!(peer_storage.find_by_public_key(&peer1.public_key).is_ok()); - assert!(peer_storage.find_by_public_key(&peer2.public_key).is_ok()); - assert!(peer_storage.find_by_public_key(&peer3.public_key).is_err()); + peer_storage.find_by_public_key(&peer1.public_key).unwrap().unwrap(); + peer_storage.find_by_public_key(&peer2.public_key).unwrap().unwrap(); + assert!(peer_storage.find_by_public_key(&peer3.public_key).unwrap().is_none()); // Test of delete with moving behaviour assert!(peer_storage.add_peer(peer3.clone()).is_ok()); @@ -778,28 +814,36 @@ mod test { assert_eq!(peer_storage.peer_db.len().unwrap(), 2); assert_eq!( - peer_storage.find_by_public_key(&peer1.public_key).unwrap().public_key, + peer_storage + .find_by_public_key(&peer1.public_key) + .unwrap() + .unwrap() + .public_key, peer1.public_key ); - assert!(peer_storage.find_by_public_key(&peer2.public_key).is_err()); + assert!(peer_storage.find_by_public_key(&peer2.public_key).unwrap().is_none()); assert_eq!( - peer_storage.find_by_public_key(&peer3.public_key).unwrap().public_key, + peer_storage + .find_by_public_key(&peer3.public_key) + .unwrap() + .unwrap() + .public_key, peer3.public_key ); assert_eq!( - peer_storage.find_by_node_id(&peer1.node_id).unwrap().node_id, + peer_storage.find_by_node_id(&peer1.node_id).unwrap().unwrap().node_id, peer1.node_id ); - assert!(peer_storage.find_by_node_id(&peer2.node_id).is_err()); + assert!(peer_storage.find_by_node_id(&peer2.node_id).unwrap().is_none()); assert_eq!( - peer_storage.find_by_node_id(&peer3.node_id).unwrap().node_id, + peer_storage.find_by_node_id(&peer3.node_id).unwrap().unwrap().node_id, peer3.node_id ); - assert!(peer_storage.find_by_public_key(&peer1.public_key).is_ok()); - assert!(peer_storage.find_by_public_key(&peer2.public_key).is_err()); - assert!(peer_storage.find_by_public_key(&peer3.public_key).is_ok()); + peer_storage.find_by_public_key(&peer1.public_key).unwrap().unwrap(); + assert!(peer_storage.find_by_public_key(&peer2.public_key).unwrap().is_none()); + peer_storage.find_by_public_key(&peer3.public_key).unwrap().unwrap(); } fn create_test_peer(features: PeerFeatures, ban: bool, offline: bool) -> Peer { diff --git a/comms/src/proto/identity.proto b/comms/src/proto/identity.proto index 9147b29f94..88aeb07e08 100644 --- a/comms/src/proto/identity.proto +++ b/comms/src/proto/identity.proto @@ -1,5 +1,7 @@ syntax = "proto3"; +import "google/protobuf/timestamp.proto"; + package tari.comms.identity; message PeerIdentityMsg { @@ -7,4 +9,14 @@ message PeerIdentityMsg { uint64 features = 2; repeated bytes supported_protocols = 3; string user_agent = 4; + // Signature that signs the peer identity + IdentitySignature identity_signature = 5; +} + +message IdentitySignature { + uint32 version = 1; + bytes signature = 2; + bytes public_nonce = 3; + // The EPOCH timestamp used in the identity signature challenge + int64 updated_at = 4; } diff --git a/comms/src/protocol/identity.rs b/comms/src/protocol/identity.rs index a3f2c44e26..3c96b36c06 100644 --- a/comms/src/protocol/identity.rs +++ b/comms/src/protocol/identity.rs @@ -63,6 +63,7 @@ where features: node_identity.features().bits(), supported_protocols, user_agent: network_info.user_agent, + identity_signature: node_identity.identity_signature_read().as_ref().map(Into::into), } .to_encoded_bytes(); diff --git a/comms/src/protocol/rpc/context.rs b/comms/src/protocol/rpc/context.rs index 4e1458758e..a0b2a2e6f3 100644 --- a/comms/src/protocol/rpc/context.rs +++ b/comms/src/protocol/rpc/context.rs @@ -27,7 +27,7 @@ use async_trait::async_trait; use super::RpcError; use crate::{ connectivity::{ConnectivityRequester, ConnectivitySelection}, - peer_manager::{NodeId, Peer}, + peer_manager::{NodeId, OrNotFound, Peer}, PeerConnection, PeerManager, }; @@ -63,7 +63,11 @@ impl RpcCommsBackend { #[async_trait] impl RpcCommsProvider for RpcCommsBackend { async fn fetch_peer(&self, node_id: &NodeId) -> Result { - self.peer_manager.find_by_node_id(node_id).await.map_err(Into::into) + self.peer_manager + .find_by_node_id(node_id) + .await + .or_not_found() + .map_err(Into::into) } async fn dial_peer(&mut self, node_id: &NodeId) -> Result { diff --git a/comms/src/types.rs b/comms/src/types.rs index b169e9c7eb..1e9bc1d4ba 100644 --- a/comms/src/types.rs +++ b/comms/src/types.rs @@ -20,7 +20,7 @@ // WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE // USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -use tari_crypto::{common::Blake256, keys::PublicKey, ristretto::RistrettoPublicKey}; +use tari_crypto::{common::Blake256, keys::PublicKey, ristretto::RistrettoPublicKey, signatures::SchnorrSignature}; use tari_storage::lmdb_store::LMDBStore; #[cfg(test)] use tari_storage::HashmapDatabase; @@ -29,13 +29,15 @@ use tari_storage::LMDBWrapper; use crate::peer_manager::{Peer, PeerId}; -/// Specify the digest type for the signature challenges -pub type Challenge = Blake256; - /// Public key type pub type CommsPublicKey = RistrettoPublicKey; pub type CommsSecretKey = ::K; +/// Specify the digest type for the signature challenges +pub type Challenge = Blake256; +/// Comms signature type +pub type Signature = SchnorrSignature; + /// Specify the RNG that should be used for random selection pub type CommsRng = rand::rngs::OsRng; diff --git a/comms/src/utils/signature.rs b/comms/src/utils/signature.rs index ca3038cd2a..54bd92dd3e 100644 --- a/comms/src/utils/signature.rs +++ b/comms/src/utils/signature.rs @@ -28,13 +28,13 @@ use tari_crypto::{ tari_utilities::message_format::MessageFormat, }; -use crate::types::{Challenge, CommsPublicKey}; +use crate::types::{Challenge, CommsPublicKey, CommsSecretKey, Signature}; pub fn sign_challenge( rng: &mut R, - secret_key: ::K, + secret_key: CommsSecretKey, challenge: Challenge, -) -> Result::K>, SchnorrSignatureError> +) -> Result where R: CryptoRng + Rng, { @@ -44,7 +44,7 @@ where /// Verify that the signature is valid for the challenge pub fn verify_challenge(public_key: &CommsPublicKey, signature: &[u8], challenge: Challenge) -> bool { - match SchnorrSignature::::K>::from_binary(signature) { + match Signature::from_binary(signature) { Ok(signature) => signature.verify_challenge(public_key, &challenge.finalize()), Err(_) => false, } diff --git a/integration_tests/features/WalletFFI.feature b/integration_tests/features/WalletFFI.feature index df04647db7..14e598c91b 100644 --- a/integration_tests/features/WalletFFI.feature +++ b/integration_tests/features/WalletFFI.feature @@ -44,17 +44,11 @@ Feature: Wallet FFI Scenario: As a client I want to set the base node Given I have a base node BASE1 - Given I have a base node BASE2 - And I have a ffi wallet FFI_WALLET connected to base node BASE1 - And I set base node BASE2 for ffi wallet FFI_WALLET - And I stop ffi wallet FFI_WALLET - And I stop node BASE2 - And I restart ffi wallet FFI_WALLET connected to base node BASE2 - And I wait 5 seconds - Then I wait for ffi wallet FFI_WALLET to receive EXACTLY 0 SAF message - And I start base node BASE2 - Then I wait for ffi wallet FFI_WALLET to receive AT_LEAST 1 SAF message - And I stop ffi wallet FFI_WALLET + And I have a base node BASE2 + Given I have a ffi wallet FFI_WALLET connected to base node BASE1 + Then I wait for ffi wallet FFI_WALLET to connect to BASE1 + Given I set base node BASE2 for ffi wallet FFI_WALLET + Then I wait for ffi wallet FFI_WALLET to connect to BASE2 Scenario: As a client I want to cancel a transaction Given I have a base node BASE diff --git a/integration_tests/features/support/ffi_steps.js b/integration_tests/features/support/ffi_steps.js index 1e41a71cb3..1853b77eda 100644 --- a/integration_tests/features/support/ffi_steps.js +++ b/integration_tests/features/support/ffi_steps.js @@ -292,12 +292,16 @@ Then( 120 ); - if (!(wallet.getCounters().received >= amount)) { - console.log("Counter not adequate!"); + if (wallet.getCounters().received < amount) { + console.log( + `Expected to receive at least ${amount} transaction(s) but got ${ + wallet.getCounters().received + }` + ); } else { console.log(wallet.getCounters()); } - expect(wallet.getCounters().received >= amount).to.equal(true); + expect(wallet.getCounters().received).to.be.gte(amount); } ); @@ -427,15 +431,19 @@ Then( 120 ); - if (!(wallet.getCounters().saf >= amount)) { - console.log("Counter not adequate!"); + if (wallet.getCounters().saf < amount) { + console.log( + `Expected to receive ${amount} SAF messages but received ${ + wallet.getCounters().saf + }` + ); } else { console.log(wallet.getCounters()); } if (comparison === atLeast) { - expect(wallet.getCounters().saf >= amount).to.equal(true); + expect(wallet.getCounters().saf).to.be.gte(amount); } else { - expect(wallet.getCounters().saf === amount).to.equal(true); + expect(wallet.getCounters().saf).to.be.equal(amount); } } ); @@ -532,10 +540,11 @@ When("I start ffi wallet {word}", async function (walletName) { When( "I restart ffi wallet {word} connected to base node {word}", async function (walletName, node) { + console.log(`Starting ${walletName}`); let wallet = this.getWallet(walletName); await wallet.restart(); - let peer = this.nodes[node].peerAddress().split("::"); - wallet.addBaseNodePeer(peer[0], peer[1]); + let [pub_key, address] = this.nodes[node].peerAddress().split("::"); + wallet.addBaseNodePeer(pub_key, address); } ); @@ -576,6 +585,7 @@ When( "I stop ffi wallet {word}", { timeout: 20 * 1000 }, async function (walletName) { + console.log(`Stopping ${walletName}`); let wallet = this.getWallet(walletName); await wallet.stop(); wallet.resetCounters(); @@ -605,3 +615,26 @@ Then( } } ); + +Then( + "I wait for ffi wallet {word} to connect to {word}", + { timeout: 125 * 1000 }, + async function (walletName, nodeName) { + const wallet = this.getWallet(walletName, true); + const client = this.getClient(nodeName); + // let client = await node.connectClient(); + let nodeIdentity = await client.identify(); + + await waitForIterate( + () => { + let publicKeys = wallet.listConnectedPublicKeys(); + return ( + publicKeys && publicKeys.some((p) => p === nodeIdentity.public_key) + ); + }, + true, + 1000, + 10 + ); + } +); diff --git a/integration_tests/features/support/steps.js b/integration_tests/features/support/steps.js index 8b155a5868..0392f4f6f4 100644 --- a/integration_tests/features/support/steps.js +++ b/integration_tests/features/support/steps.js @@ -3678,7 +3678,7 @@ Then( ); When( - /I wait for (.*) to connect to (.*)/, + "I wait for {word} to connect to {word}", async function (firstNode, secondNode) { const firstNodeClient = await this.getNodeOrWalletClient(firstNode); const secondNodeClient = await this.getNodeOrWalletClient(secondNode); diff --git a/integration_tests/helpers/ffi/ffiInterface.js b/integration_tests/helpers/ffi/ffiInterface.js index 8eb3b95ade..5ad36ff21c 100644 --- a/integration_tests/helpers/ffi/ffiInterface.js +++ b/integration_tests/helpers/ffi/ffiInterface.js @@ -73,9 +73,8 @@ class InterfaceFFI { static library = null; static async init() { - this.library = `${process.cwd()}/temp/out/${ - process.platform === "win32" ? "" : "lib" - }tari_wallet_ffi`; + let platform = process.platform === "win32" ? "" : "lib"; + this.library = `${process.cwd()}/temp/out/${platform}tari_wallet_ffi`; // Load the library this.fn = ffi.Library(this.loaded ? null : this.library, { transport_memory_create: [this.ptr, []], @@ -269,6 +268,7 @@ class InterfaceFFI { ], ], comms_config_destroy: [this.void, [this.ptr]], + comms_list_connected_public_keys: [this.ptr, [this.ptr, this.intPtr]], wallet_create: [ this.ptr, [ @@ -727,6 +727,13 @@ class InterfaceFFI { static commsConfigDestroy(ptr) { this.fn.comms_config_destroy(ptr); } + + static commsListConnectedPublicKeys(walletPtr) { + let error = this.initError(); + let result = this.fn.comms_list_connected_public_keys(walletPtr, error); + this.checkErrorResult(error, `commsListConnectedPublicKeys`); + return result; + } //endregion //region Contact diff --git a/integration_tests/helpers/ffi/wallet.js b/integration_tests/helpers/ffi/wallet.js index feae5535b8..d48a4e3cda 100644 --- a/integration_tests/helpers/ffi/wallet.js +++ b/integration_tests/helpers/ffi/wallet.js @@ -441,6 +441,10 @@ class Wallet { return InterfaceFFI.walletStartTransactionValidation(this.ptr); } + listConnectedPublicKeys() { + return InterfaceFFI.commsListConnectedPublicKeys(this.ptr); + } + destroy() { if (this.ptr) { InterfaceFFI.walletDestroy(this.ptr); diff --git a/integration_tests/helpers/walletFFIClient.js b/integration_tests/helpers/walletFFIClient.js index 2a84de67cf..6bfd907f12 100644 --- a/integration_tests/helpers/walletFFIClient.js +++ b/integration_tests/helpers/walletFFIClient.js @@ -130,6 +130,10 @@ class WalletFFIClient { this.wallet.startTxValidation(); } + listConnectedPublicKeys() { + this.wallet.listConnectedPublicKeys(); + } + getCounters() { return this.wallet.getCounters(); } diff --git a/integration_tests/helpers/walletProcess.js b/integration_tests/helpers/walletProcess.js index 1a88666a9a..23e49aa540 100644 --- a/integration_tests/helpers/walletProcess.js +++ b/integration_tests/helpers/walletProcess.js @@ -177,12 +177,13 @@ class WalletProcess { stop() { return new Promise((resolve) => { + let name = this.name; if (!this.ps) { return resolve(); } this.ps.on("close", (code) => { if (code) { - console.log(`child process exited with code ${code}`); + console.log(`child process (${name}) exited with code ${code}`); } resolve(); });