From ff7fb6d6b4ab4f77d108b2d9b7fd010c77e613c7 Mon Sep 17 00:00:00 2001 From: Stan Bondi Date: Thu, 25 May 2023 10:17:14 +0200 Subject: [PATCH] fix(comms): only set final forward address if configured to port 0 (#5406) Description --- Sets the onion service forward address to the correct port only if configured to port 0. Motivation and Context --- Fixes #5405 How Has This Been Tested? --- Manually, default settings were also tested. What process can a PR reviewer use to test or verify this change? --- ```toml # This doesnt work (no incoming connections as expected) #tor.forward_address = "/dns4/localhost/tcp/12345" # This does work tor.forward_address = "/ip4/10.71.1.141/tcp/12345" tor.listener_address_override = "/ip4/10.71.1.141/tcp/12345" ``` Breaking Changes --- - [x] None - [ ] Requires data directory on base node to be deleted - [ ] Requires hard fork - [ ] Other - Please specify Co-authored-by: SW van Heerden --- base_layer/p2p/src/initialization.rs | 15 +++++++++++---- comms/core/src/builder/comms_node.rs | 14 +++++++++++++- comms/core/src/lib.rs | 2 +- comms/core/src/tor/control_client/types.rs | 8 +++++++- 4 files changed, 32 insertions(+), 7 deletions(-) diff --git a/base_layer/p2p/src/initialization.rs b/base_layer/p2p/src/initialization.rs index e4c3f9d667..ca990c4dd6 100644 --- a/base_layer/p2p/src/initialization.rs +++ b/base_layer/p2p/src/initialization.rs @@ -42,6 +42,7 @@ use tari_common::{ }; use tari_comms::{ backoff::ConstantBackoff, + multiaddr::multiaddr, peer_manager::{NodeIdentity, Peer, PeerFeatures, PeerFlags, PeerManagerError}, pipeline, protocol::{ @@ -249,14 +250,20 @@ pub async fn spawn_comms_using_transport( let tor_config = transport_config.tor; debug!(target: LOG_TARGET, "Building TOR comms stack ({:?})", tor_config); let listener_address_override = tor_config.listener_address_override.clone(); - let mut hidden_service_ctl = initialize_hidden_service(tor_config).await?; + let mut hidden_service_ctl = initialize_hidden_service(tor_config)?; // Set the listener address to be the address (usually local) to which tor will forward all traffic let transport = hidden_service_ctl.initialize_transport().await?; - debug!(target: LOG_TARGET, "Comms and DHT configured"); + + info!( + target: LOG_TARGET, + "Tor hidden service initialized. proxied_address = '{:?}', listener_override_address = {:?}", + hidden_service_ctl.proxied_address(), + listener_address_override, + ); comms .with_listener_address( - listener_address_override.unwrap_or_else(|| hidden_service_ctl.proxied_address()), + listener_address_override.unwrap_or_else(|| multiaddr![Ip4([127, 0, 0, 1]), Tcp(0u16)]), ) .with_hidden_service_controller(hidden_service_ctl) .spawn_with_transport(transport) @@ -275,7 +282,7 @@ pub async fn spawn_comms_using_transport( Ok(comms) } -async fn initialize_hidden_service( +fn initialize_hidden_service( mut config: TorTransportConfig, ) -> Result { let mut builder = tor::HiddenServiceBuilder::new() diff --git a/comms/core/src/builder/comms_node.rs b/comms/core/src/builder/comms_node.rs index ad5ff9a824..fe96a2e87f 100644 --- a/comms/core/src/builder/comms_node.rs +++ b/comms/core/src/builder/comms_node.rs @@ -23,6 +23,7 @@ use std::{iter, sync::Arc, time::Duration}; use log::*; +use multiaddr::{multiaddr, Protocol}; use tari_shutdown::ShutdownSignal; use tokio::{ io::{AsyncRead, AsyncWrite}, @@ -222,9 +223,20 @@ impl UnspawnedCommsNode { ); let listening_info = connection_manager_requester.wait_until_listening().await?; + + // Final setup of the hidden service. let mut hidden_service = None; if let Some(mut ctl) = hidden_service_ctl { - ctl.set_proxied_addr(listening_info.bind_address()); + // Only set the address to the bind address it is set to TCP port 0 + let mut proxied_addr = ctl.proxied_address(); + if proxied_addr.ends_with(&multiaddr!(Tcp(0u16))) { + // Remove the TCP port 0 address and replace it with the actual listener port + if let Some(Protocol::Tcp(port)) = listening_info.bind_address().iter().last() { + proxied_addr.pop(); + proxied_addr.push(Protocol::Tcp(port)); + ctl.set_proxied_addr(&proxied_addr); + } + } let hs = ctl.create_hidden_service().await?; let onion_addr = hs.get_onion_address(); if !node_identity.public_addresses().contains(&onion_addr) { diff --git a/comms/core/src/lib.rs b/comms/core/src/lib.rs index 69504784db..36779afa89 100644 --- a/comms/core/src/lib.rs +++ b/comms/core/src/lib.rs @@ -61,7 +61,7 @@ pub mod traits; pub mod multiaddr { // Re-export so that client code does not have to have multiaddr as a dependency - pub use ::multiaddr::{Error, Multiaddr, Protocol}; + pub use ::multiaddr::{multiaddr, Error, Multiaddr, Protocol}; } pub use async_trait::async_trait; diff --git a/comms/core/src/tor/control_client/types.rs b/comms/core/src/tor/control_client/types.rs index 9a12c5aad6..c04dc9da6b 100644 --- a/comms/core/src/tor/control_client/types.rs +++ b/comms/core/src/tor/control_client/types.rs @@ -90,7 +90,7 @@ pub enum PrivateKey { /// Represents a mapping between an onion port and a proxied address (usually 127.0.0.1:xxxx). /// If the proxied_address is not specified, the default `127.0.0.1:[onion_port]` will be used. -#[derive(Debug, Clone, Copy, Serialize, Deserialize)] +#[derive(Clone, Copy, Serialize, Deserialize)] pub struct PortMapping { onion_port: u16, proxied_address: SocketAddr, @@ -146,3 +146,9 @@ impl fmt::Display for PortMapping { write!(f, "PortMapping [{} -> {}]", self.onion_port, self.proxied_address) } } + +impl fmt::Debug for PortMapping { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + fmt::Display::fmt(self, f) + } +}