Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions bindings/ldk_node.udl
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ typedef dictionary EsploraSyncConfig;

typedef dictionary ElectrumSyncConfig;

typedef dictionary TorConfig;

typedef interface NodeEntropy;

typedef enum WordCount;
Expand Down Expand Up @@ -53,6 +55,8 @@ interface Builder {
[Throws=BuildError]
void set_announcement_addresses(sequence<SocketAddress> announcement_addresses);
[Throws=BuildError]
void set_tor_config(TorConfig tor_config);
[Throws=BuildError]
void set_node_alias(string node_alias);
[Throws=BuildError]
void set_async_payments_role(AsyncPaymentsRole? role);
Expand Down
48 changes: 45 additions & 3 deletions src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ use vss_client::headers::VssHeaderProvider;
use crate::chain::ChainSource;
use crate::config::{
default_user_config, may_announce_channel, AnnounceError, AsyncPaymentsRole,
BitcoindRestClientConfig, Config, ElectrumSyncConfig, EsploraSyncConfig,
BitcoindRestClientConfig, Config, ElectrumSyncConfig, EsploraSyncConfig, TorConfig,
DEFAULT_ESPLORA_SERVER_URL, DEFAULT_LOG_FILENAME, DEFAULT_LOG_LEVEL,
};
use crate::connection::ConnectionManager;
Expand Down Expand Up @@ -165,6 +165,8 @@ pub enum BuildError {
InvalidListeningAddresses,
/// The given announcement addresses are invalid, e.g. too many were passed.
InvalidAnnouncementAddresses,
/// The given tor proxy address is invalid, e.g. an onion address was passed.
InvalidTorProxyAddress,
/// The provided alias is invalid.
InvalidNodeAlias,
/// An attempt to setup a runtime has failed.
Expand Down Expand Up @@ -206,6 +208,7 @@ impl fmt::Display for BuildError {
Self::InvalidAnnouncementAddresses => {
write!(f, "Given announcement addresses are invalid.")
},
Self::InvalidTorProxyAddress => write!(f, "Given Tor proxy address is invalid."),
Self::RuntimeSetupFailed => write!(f, "Failed to setup a runtime."),
Self::ReadFailed => write!(f, "Failed to read from store."),
Self::WriteFailed => write!(f, "Failed to write to store."),
Expand Down Expand Up @@ -523,6 +526,23 @@ impl NodeBuilder {
Ok(self)
}

/// Configures the [`Node`] instance to use a Tor SOCKS proxy for outbound connections to peers with OnionV3 addresses.
/// Connections to clearnet addresses are not affected, and are not made over Tor.
/// The proxy address must not itself be an onion address.
///
/// **Note**: If unset, connecting to peer OnionV3 addresses will fail.
pub fn set_tor_config(&mut self, tor_config: TorConfig) -> Result<&mut Self, BuildError> {
match tor_config.proxy_address {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should move this check to the build step so it happens also when set via Builder::from_config? Not sure if we even need this setter then?

Copy link
Contributor Author

@jharveyb jharveyb Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fine with also having the check there, though I'll note that there are checks in other setters, like set_announcement_addresses() and set_listening_addresses(), that aren't mirrored in build_with_store_internal() right now.

I'd prefer to keep the setter (since I'm using the others as well), but can remove it if needed.

SocketAddress::OnionV2 { .. } | SocketAddress::OnionV3 { .. } => {
return Err(BuildError::InvalidTorProxyAddress);
},
_ => {},
}

self.config.tor_config = Some(tor_config);
Ok(self)
}

/// Sets the node alias that will be used when broadcasting announcements to the gossip
/// network.
///
Expand Down Expand Up @@ -957,6 +977,15 @@ impl ArcedNodeBuilder {
self.inner.write().unwrap().set_announcement_addresses(announcement_addresses).map(|_| ())
}

/// Configures the [`Node`] instance to use a Tor SOCKS proxy for outbound connections to peers with OnionV3 addresses.
/// Connections to clearnet addresses are not affected, and are not made over Tor.
/// The proxy address must not itself be an onion address.
///
/// **Note**: If unset, connecting to peer OnionV3 addresses will fail.
pub fn set_tor_config(&self, tor_config: TorConfig) -> Result<(), BuildError> {
self.inner.write().unwrap().set_tor_config(tor_config).map(|_| ())
}

/// Sets the node alias that will be used when broadcasting announcements to the gossip
/// network.
///
Expand Down Expand Up @@ -1146,6 +1175,15 @@ fn build_with_store_internal(
}
}

if let Some(tor_config) = &config.tor_config {
match tor_config.proxy_address {
SocketAddress::OnionV2 { .. } | SocketAddress::OnionV3 { .. } => {
return Err(BuildError::InvalidTorProxyAddress);
},
_ => {},
}
}
Comment on lines +1178 to +1185
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we put this here in addition to the validation in set_tor_config because of the NodeBuilder::from_config path ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes exactly.


let tx_broadcaster = Arc::new(TransactionBroadcaster::new(Arc::clone(&logger)));
let fee_estimator = Arc::new(OnchainFeeEstimator::new());

Expand Down Expand Up @@ -1779,8 +1817,12 @@ fn build_with_store_internal(

liquidity_source.as_ref().map(|l| l.set_peer_manager(Arc::downgrade(&peer_manager)));

let connection_manager =
Arc::new(ConnectionManager::new(Arc::clone(&peer_manager), Arc::clone(&logger)));
let connection_manager = Arc::new(ConnectionManager::new(
Arc::clone(&peer_manager),
config.tor_config.clone(),
Arc::clone(&keys_manager),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to confirm we are ok with using the same EntropySource for both passwords sent in plaintext to the SOCKS proxy, and the BOLT 8 per-connection ephemeral keys ? I had raised this concern earlier in this PR, but I double checked the security properties of EntropySource, and it now sounds reasonable to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at this again, we could also create a new set of random bytes from keys_manager here, use that to seed an instance of lightning::sign::RandomBytes, and then pass that in inside an Arc. IIUC a ChaCha20 stream is random enough for these passwords.

Then this entropy source should have no interaction with the one used for BOLT8 ephemeral keys.

Lmk if you prefer that approach, happy to make the change if so.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right @tnull you suggested we pass the KeysManager directly here, let me know what you think.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I think either is fine.

Arc::clone(&logger),
));

let output_sweeper = match sweeper_bytes_res {
Ok(output_sweeper) => Arc::new(output_sweeper),
Expand Down
21 changes: 20 additions & 1 deletion src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,8 @@ pub(crate) const LNURL_AUTH_TIMEOUT_SECS: u64 = 15;
/// | `probing_liquidity_limit_multiplier` | 3 |
/// | `log_level` | Debug |
/// | `anchor_channels_config` | Some(..) |
/// | `route_parameters` | None |
/// | `route_parameters` | None |
/// | `tor_config` | None |
///
/// See [`AnchorChannelsConfig`] and [`RouteParametersConfig`] for more information regarding their
/// respective default values.
Expand Down Expand Up @@ -196,6 +197,13 @@ pub struct Config {
/// **Note:** If unset, default parameters will be used, and you will be able to override the
/// parameters on a per-payment basis in the corresponding method calls.
pub route_parameters: Option<RouteParametersConfig>,
/// Configuration options for enabling peer connections via the Tor network.
///
/// Setting [`TorConfig`] enables connecting to peers with OnionV3 addresses. No other connections
/// are routed via Tor. Please refer to [`TorConfig`] for further information.
///
/// **Note**: If unset, connecting to peer OnionV3 addresses will fail.
pub tor_config: Option<TorConfig>,
}

impl Default for Config {
Expand All @@ -208,6 +216,7 @@ impl Default for Config {
trusted_peers_0conf: Vec::new(),
probing_liquidity_limit_multiplier: DEFAULT_PROBING_LIQUIDITY_LIMIT_MULTIPLIER,
anchor_channels_config: Some(AnchorChannelsConfig::default()),
tor_config: None,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the doc for pub struct Config we'll want to add the tor_config default

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah didn't notice that bit; updated + rebased

route_parameters: None,
node_alias: None,
}
Expand Down Expand Up @@ -487,6 +496,16 @@ pub struct BitcoindRestClientConfig {
pub rest_port: u16,
}

/// Configuration for connecting to peers via the Tor Network.
#[derive(Debug, Clone)]
#[cfg_attr(feature = "uniffi", derive(uniffi::Record))]
pub struct TorConfig {
/// Tor daemon SOCKS proxy address. Only connections to OnionV3 peers will be made
/// via this proxy; other connections (IPv4 peers, Electrum server) will not be
/// routed over Tor.
pub proxy_address: SocketAddress,
}

/// Options which apply on a per-channel basis and may change at runtime or based on negotiation
/// with our counterparty.
#[derive(Copy, Clone, Debug, PartialEq, Eq)]
Expand Down
142 changes: 116 additions & 26 deletions src/connection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,9 @@ use std::time::Duration;
use bitcoin::secp256k1::PublicKey;
use lightning::ln::msgs::SocketAddress;

use crate::config::TorConfig;
use crate::logger::{log_error, log_info, LdkLogger};
use crate::types::PeerManager;
use crate::types::{KeysManager, PeerManager};
use crate::Error;

pub(crate) struct ConnectionManager<L: Deref + Clone + Sync + Send>
Expand All @@ -25,16 +26,22 @@ where
pending_connections:
Mutex<HashMap<PublicKey, Vec<tokio::sync::oneshot::Sender<Result<(), Error>>>>>,
peer_manager: Arc<PeerManager>,
tor_proxy_config: Option<TorConfig>,
keys_manager: Arc<KeysManager>,
logger: L,
}

impl<L: Deref + Clone + Sync + Send> ConnectionManager<L>
where
L::Target: LdkLogger,
{
pub(crate) fn new(peer_manager: Arc<PeerManager>, logger: L) -> Self {
pub(crate) fn new(
peer_manager: Arc<PeerManager>, tor_proxy_config: Option<TorConfig>,
keys_manager: Arc<KeysManager>, logger: L,
) -> Self {
let pending_connections = Mutex::new(HashMap::new());
Self { pending_connections, peer_manager, logger }

Self { pending_connections, peer_manager, tor_proxy_config, keys_manager, logger }
}

pub(crate) async fn connect_peer_if_necessary(
Expand Down Expand Up @@ -64,27 +71,114 @@ where

log_info!(self.logger, "Connecting to peer: {}@{}", node_id, addr);

let socket_addr = addr
.to_socket_addrs()
.map_err(|e| {
log_error!(self.logger, "Failed to resolve network address {}: {}", addr, e);
self.propagate_result_to_subscribers(&node_id, Err(Error::InvalidSocketAddress));
Error::InvalidSocketAddress
})?
.next()
.ok_or_else(|| {
log_error!(self.logger, "Failed to resolve network address {}", addr);
let res = match addr {
SocketAddress::OnionV2(old_onion_addr) => {
log_error!(
self.logger,
"Failed to resolve network address {:?}: Resolution of OnionV2 addresses is currently unsupported.",
old_onion_addr
);
self.propagate_result_to_subscribers(&node_id, Err(Error::InvalidSocketAddress));
Error::InvalidSocketAddress
})?;
return Err(Error::InvalidSocketAddress);
},
SocketAddress::OnionV3 { .. } => {
let proxy_config = self.tor_proxy_config.as_ref().ok_or_else(|| {
log_error!(
self.logger,
"Failed to resolve network address {:?}: Tor usage is not configured.",
addr
);
self.propagate_result_to_subscribers(
&node_id,
Err(Error::InvalidSocketAddress),
);
Error::InvalidSocketAddress
})?;
let proxy_addr = proxy_config
.proxy_address
.to_socket_addrs()
.map_err(|e| {
log_error!(
self.logger,
"Failed to resolve Tor proxy network address {}: {}",
proxy_config.proxy_address,
e
);
self.propagate_result_to_subscribers(
&node_id,
Err(Error::InvalidSocketAddress),
);
Error::InvalidSocketAddress
})?
.next()
.ok_or_else(|| {
log_error!(
self.logger,
"Failed to resolve Tor proxy network address {}",
proxy_config.proxy_address
);
self.propagate_result_to_subscribers(
&node_id,
Err(Error::InvalidSocketAddress),
);
Error::InvalidSocketAddress
})?;
let connection_future = lightning_net_tokio::tor_connect_outbound(
Arc::clone(&self.peer_manager),
node_id,
addr.clone(),
proxy_addr,
Arc::clone(&self.keys_manager),
);
self.await_connection(connection_future, node_id, addr).await
},
_ => {
let socket_addr = addr
.to_socket_addrs()
.map_err(|e| {
log_error!(
self.logger,
"Failed to resolve network address {}: {}",
addr,
e
);
self.propagate_result_to_subscribers(
&node_id,
Err(Error::InvalidSocketAddress),
);
Error::InvalidSocketAddress
})?
.next()
.ok_or_else(|| {
log_error!(self.logger, "Failed to resolve network address {}", addr);
self.propagate_result_to_subscribers(
&node_id,
Err(Error::InvalidSocketAddress),
);
Error::InvalidSocketAddress
})?;
let connection_future = lightning_net_tokio::connect_outbound(
Arc::clone(&self.peer_manager),
node_id,
socket_addr,
);
self.await_connection(connection_future, node_id, addr).await
},
};

let connection_future = lightning_net_tokio::connect_outbound(
Arc::clone(&self.peer_manager),
node_id,
socket_addr,
);
self.propagate_result_to_subscribers(&node_id, res);

let res = match connection_future.await {
res
}

async fn await_connection<F, CF>(
&self, connection_future: F, node_id: PublicKey, addr: SocketAddress,
) -> Result<(), Error>
where
F: std::future::Future<Output = Option<CF>>,
CF: std::future::Future<Output = ()>,
{
match connection_future.await {
Some(connection_closed_future) => {
let mut connection_closed_future = Box::pin(connection_closed_future);
loop {
Expand All @@ -106,11 +200,7 @@ where
log_error!(self.logger, "Failed to connect to peer: {}@{}", node_id, addr);
Err(Error::ConnectionFailed)
},
};

self.propagate_result_to_subscribers(&node_id, res);

res
}
}

fn register_or_subscribe_pending_connection(
Expand Down
2 changes: 1 addition & 1 deletion src/ffi/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ impl VssClientHeaderProvider for VssHeaderProviderAdapter {
}

use crate::builder::sanitize_alias;
pub use crate::config::{default_config, ElectrumSyncConfig, EsploraSyncConfig};
pub use crate::config::{default_config, ElectrumSyncConfig, EsploraSyncConfig, TorConfig};
pub use crate::entropy::{generate_entropy_mnemonic, NodeEntropy, WordCount};
use crate::error::Error;
pub use crate::liquidity::LSPS1OrderStatus;
Expand Down
Loading