Skip to content

Commit

Permalink
fix(libtor): prevent metrics port conflict (#6125)
Browse files Browse the repository at this point in the history
Description
---
Prevent conflict with metrics port when starting multiple nodes using
libtor
Run tor in background without an unnecessary task spawn
Write the control port to a file (not used in the node implementation)
Support for tor auto assigning it's own ports

Motivation and Context
---

Set metrics port to 0 (os -assigned) so that multiple libtor instances
do not conflict
Previously we'd wait on a shutdown signal in an async function, but tor
runs in the background in an OS thread and waiting on a shutdown does
nothing other than keep a task open in memory.

We could experience issues binding and dropping a listener quickly on
some platforms. It would be better for tor to assign its own port
however we could implement that if this is ever an issue, until then
this code just makes us aware of that option.

How Has This Been Tested?
---

Running two base nodes and a wallet with libtor enabled on each

What process can a PR reviewer use to test or verify this change?
---

<!-- Checklist -->
<!-- 1. Is the title of your PR in the form that would make nice release
notes? The title, excluding the conventional commit
tag, will be included exactly as is in the CHANGELOG, so please think
about it carefully. -->


Breaking Changes
---

- [x] None
- [ ] Requires data directory on base node to be deleted
- [ ] Requires hard fork
- [ ] Other - Please specify

<!-- Does this include a breaking change? If so, include this line as a
footer -->
<!-- BREAKING CHANGE: Description what the user should do, e.g. delete a
database, resync the chain -->
  • Loading branch information
sdbondi authored Feb 6, 2024
1 parent 0584782 commit 661af51
Show file tree
Hide file tree
Showing 5 changed files with 28 additions and 22 deletions.
1 change: 0 additions & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion applications/minotari_console_wallet/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ pub fn run_wallet_with_cli(
if config.wallet.use_libtor && config.wallet.p2p.transport.is_tor() {
let tor = Tor::initialize()?;
tor.update_comms_transport(&mut config.wallet.p2p.transport)?;
runtime.spawn(tor.run(shutdown.to_signal()));
tor.run_background();
debug!(
target: LOG_TARGET,
"Updated Tor comms transport: {:?}", config.wallet.p2p.transport
Expand Down
2 changes: 1 addition & 1 deletion applications/minotari_node/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ fn main_inner() -> Result<(), ExitError> {
if config.base_node.use_libtor && config.base_node.p2p.transport.is_tor() {
let tor = Tor::initialize()?;
tor.update_comms_transport(&mut config.base_node.p2p.transport)?;
runtime.spawn(tor.run(shutdown.to_signal()));
tor.run_background();
debug!(
target: LOG_TARGET,
"Updated Tor comms transport: {:?}", config.base_node.p2p.transport
Expand Down
1 change: 0 additions & 1 deletion infrastructure/libtor/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ license = "BSD-3-Clause"
[dependencies]
tari_common = { path = "../../common" }
tari_p2p = { path = "../../base_layer/p2p" }
tari_shutdown = { path = "../shutdown" }

derivative = "2.2.0"
log = "0.4.8"
Expand Down
44 changes: 26 additions & 18 deletions infrastructure/libtor/src/tor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,14 @@
// 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, io, net::TcpListener};
use std::{fmt, io, net::TcpListener, path::PathBuf, thread};

use derivative::Derivative;
use libtor::{LogDestination, LogLevel, TorFlag};
use log::*;
use rand::{distributions::Alphanumeric, thread_rng, Rng};
use tari_common::exit_codes::{ExitCode, ExitError};
use tari_p2p::{TorControlAuthentication, TransportConfig, TransportType};
use tari_shutdown::ShutdownSignal;
use tempfile::{tempdir, NamedTempFile, TempDir, TempPath};
use tor_hash_passwd::EncryptedKey;

Expand All @@ -46,7 +45,7 @@ impl fmt::Debug for TorPassword {
#[derivative(Debug)]
pub struct Tor {
control_port: u16,
data_dir: String,
data_dir: PathBuf,
log_destination: String,
log_level: LogLevel,
#[derivative(Debug = "ignore")]
Expand All @@ -59,12 +58,12 @@ pub struct Tor {
impl Default for Tor {
fn default() -> Tor {
Tor {
control_port: 19_051,
control_port: 0,
data_dir: "/tmp/tor-data".into(),
log_destination: "/tmp/tor.log".into(),
log_level: LogLevel::Err,
passphrase: TorPassword(None),
socks_port: 19_050,
socks_port: 0,
temp_dir: None,
temp_file: None,
}
Expand All @@ -83,6 +82,7 @@ impl Tor {

// check for unused ports to assign
let (socks_port, control_port) = get_available_ports()?;
debug!(target: LOG_TARGET, "Using socks port {socks_port} and control_port {control_port}");
instance.socks_port = socks_port;
instance.control_port = control_port;

Expand All @@ -96,9 +96,8 @@ impl Tor {

// data dir
let temp = tempdir()?;
let dir = temp.path().to_string_lossy().to_string();
instance.data_dir = temp.path().to_path_buf();
instance.temp_dir = Some(temp);
instance.data_dir = dir;

// log destination
let temp = NamedTempFile::new()?.into_temp_path();
Expand Down Expand Up @@ -128,8 +127,8 @@ impl Tor {
}
}

/// Run the Tor instance until the shutdown signal is received
pub async fn run(self, mut shutdown_signal: ShutdownSignal) -> Result<(), ExitError> {
/// Run the Tor instance in the background and return a handle to the thread.
pub fn run_background(self) -> thread::JoinHandle<Result<u8, libtor::Error>> {
info!(target: LOG_TARGET, "Starting Tor instance");

let Tor {
Expand All @@ -144,26 +143,35 @@ impl Tor {

let mut tor = libtor::Tor::new();

tor.flag(TorFlag::DataDirectory(data_dir.clone()))
.flag(TorFlag::SocksPort(socks_port))
.flag(TorFlag::ControlPort(control_port))
tor.flag(TorFlag::DataDirectory(data_dir.to_string_lossy().to_string()))
// Disable signal handlers so that ctrl+c can be handled by our application
// https://github.com/torproject/torspec/blob/8961bb4d83fccb2b987f9899ca83aa430f84ab0c/control-spec.txt#L3946
.flag(TorFlag::Custom("__DisableSignalHandlers 1".to_string()))
// Prevent conflicts with multiple instances using the same listener port for Prometheus metrics
.flag(TorFlag::Custom("MetricsPort 0".to_string()))
// Write the final control port to a file. This could be used to configure the node to use this port when auto is set.
.flag(TorFlag::ControlPortWriteToFile(data_dir.join("control_port").to_string_lossy().to_string()))
.flag(TorFlag::Hush())
.flag(TorFlag::LogTo(log_level, LogDestination::File(log_destination)));

if socks_port == 0 {
tor.flag(TorFlag::SocksPortAuto);
} else {
tor.flag(TorFlag::SocksPort(socks_port));
}

if control_port == 0 {
tor.flag(TorFlag::ControlPortAuto);
} else {
tor.flag(TorFlag::ControlPort(control_port));
}

if let Some(secret) = passphrase.0 {
let hash = EncryptedKey::hash_password(&secret).to_string();
tor.flag(TorFlag::HashedControlPassword(hash));
}

tor.start_background();

shutdown_signal.wait().await;
info!(target: LOG_TARGET, "Shutting down Tor instance");

Ok(())
tor.start_background()
}
}

Expand Down

0 comments on commit 661af51

Please sign in to comment.