vincenzopalazzo
/
arti
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Merge branch 'arti-client' into 'main' 

Sort out .onion handling in arti-client

See merge request tpo/core/arti!1257
This commit is contained in:
Alexander Færøy 2023-06-19 14:44:55 +00:00
parent c7fa6cb746 b34ba70070
commit f55187e065
5 changed files with 159 additions and 37 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
Cargo.lock generated
View File

@ -175,6 +175,7 @@ dependencies = [
"postage",
"safelog",
"serde",
"strum",
"tempfile",
"thiserror",
"tokio",

1
crates/arti-client/Cargo.toml
View File

@ -143,6 +143,7 @@ void = "1"
anyhow = "1.0.23"
once_cell = "1.9"
pin-project = "1"
strum = { version = "0.24", features = ["derive"] }
tempfile = "3.3"
tokio-crate = { package = "tokio", version = "1.7", features = [
"rt",

150
crates/arti-client/src/address.rs
View File

@ -2,6 +2,7 @@
//! Tor can connect to.
use crate::err::ErrorDetail;
use crate::StreamPrefs;
use std::fmt::Display;
use std::net::{IpAddr, Ipv4Addr, Ipv6Addr, SocketAddr, SocketAddrV4, SocketAddrV6};
use std::str::FromStr;
@ -230,8 +231,13 @@ impl TorAddr {
}
/// Get instructions for how to make a stream to this address
pub(crate) fn into_stream_instructions(self) -> Result<StreamInstructions, ErrorDetail> {
// TODO enforcement of the config should go here, not separately
pub(crate) fn into_stream_instructions(
self,
cfg: &crate::config::ClientAddrConfig,
prefs: &StreamPrefs,
) -> Result<StreamInstructions, ErrorDetail> {
self.enforce_config(cfg, prefs)?;
let port = self.port;
Ok(match self.host {
Host::Hostname(hostname) => StreamInstructions::Exit { hostname, port },
@ -258,13 +264,30 @@ impl TorAddr {
}
/// Get instructions for how to make a stream to this address
pub(crate) fn into_resolve_instructions(self) -> Result<ResolveInstructions, ErrorDetail> {
// TODO enforcement of the config should go here, not separately
Ok(match self.host {
Host::Hostname(hostname) => ResolveInstructions::Exit(hostname),
Host::Ip(ip) => ResolveInstructions::Return(vec![ip]),
Host::Onion(_) => return Err(ErrorDetail::OnionAddressResolveRequest),
})
pub(crate) fn into_resolve_instructions(
self,
cfg: &crate::config::ClientAddrConfig,
prefs: &StreamPrefs,
) -> Result<ResolveInstructions, ErrorDetail> {
// We defer enforcing the config until we see if this is a .onion,
// in which case it's always doomed and we want to return *our* error,
// not any problem with the configuration or preferences.
// But we must *calculate* the error now because instructions consumes self.
let enforce_config_result = self.enforce_config(cfg, prefs);
// This IEFE is so that any use of `return` doesn't bypass
// checking the the enforce_config result
let instructions = (move || {
Ok(match self.host {
Host::Hostname(hostname) => ResolveInstructions::Exit(hostname),
Host::Ip(ip) => ResolveInstructions::Return(vec![ip]),
Host::Onion(_) => return Err(ErrorDetail::OnionAddressResolveRequest),
})
})()?;
let () = enforce_config_result?;
Ok(instructions)
}
/// Return true if the `host` in this address is local.
@ -274,9 +297,11 @@ impl TorAddr {
/// Give an error if this address doesn't conform to the rules set in
/// `cfg`.
pub(crate) fn enforce_config(
fn enforce_config(
&self,
cfg: &crate::config::ClientAddrConfig,
#[allow(unused_variables)] // will only be used in certain configurations
prefs: &StreamPrefs,
) -> Result<(), ErrorDetail> {
if !cfg.allow_local_addrs && self.is_local() {
return Err(ErrorDetail::LocalAddress);
@ -293,6 +318,18 @@ impl TorAddr {
}
}
if let Host::Onion(_name) = &self.host {
cfg_if::cfg_if! {
if #[cfg(feature = "onion-service-client")] {
if !prefs.connect_to_onion_services {
return Err(ErrorDetail::OnionAddressDisabled);
}
} else {
return Err(ErrorDetail::OnionAddressNotSupported);
}
}
}
Ok(())
}
}
@ -549,7 +586,7 @@ mod test {
use crate::err::ErrorDetail;
fn val<A: IntoTorAddr>(addr: A) -> Result<TorAddr, ErrorDetail> {
let toraddr = addr.into_tor_addr()?;
toraddr.enforce_config(&Default::default())?;
toraddr.enforce_config(&Default::default(), &Default::default())?;
Ok(toraddr)
}
@ -559,8 +596,21 @@ mod test {
assert!(val("198.151.100.42:443").is_ok());
assert!(val("www.torproject.org:443").is_ok());
assert!(val(("www.torproject.org", 443)).is_ok());
assert!(val("example.onion:80").is_ok());
assert!(val(("example.onion", 80)).is_ok());
// When HS disabled, tested elsewhere, see: stream_instructions, prefs_onion_services
#[cfg(feature = "onion-service-client")]
{
assert!(val("example.onion:80").is_ok());
assert!(val(("example.onion", 80)).is_ok());
match val("eweiibe6tdjsdprb4px6rqrzzcsi22m4koia44kc5pcjr7nec2rlxyad.onion:443") {
Ok(TorAddr {
host: Host::Onion(_),
..
}) => {}
x => panic!("{x:?}"),
}
}
assert!(matches!(
val("-foobar.net:443"),
@ -575,13 +625,6 @@ mod test {
val("192.168.0.1:80"),
Err(ErrorDetail::LocalAddress)
));
match val("eweiibe6tdjsdprb4px6rqrzzcsi22m4koia44kc5pcjr7nec2rlxyad.onion:443") {
Ok(TorAddr {
host: Host::Onion(_),
..
}) => {}
x => panic!("{x:?}"),
}
}
#[test]
@ -618,7 +661,9 @@ mod test {
use StreamInstructions as SI;
fn sap(s: &str) -> Result<StreamInstructions, ErrorDetail> {
TorAddr::from(s).unwrap().into_stream_instructions()
TorAddr::from(s)
.unwrap()
.into_stream_instructions(&Default::default(), &Default::default())
}
assert_eq!(
@ -661,7 +706,9 @@ mod test {
use ResolveInstructions as RI;
fn sap(s: &str) -> Result<ResolveInstructions, ErrorDetail> {
TorAddr::from(s).unwrap().into_resolve_instructions()
TorAddr::from(s)
.unwrap()
.into_resolve_instructions(&Default::default(), &Default::default())
}
assert_eq!(
@ -690,6 +737,65 @@ mod test {
);
}
#[test]
fn prefs_onion_services() {
use crate::err::ErrorDetailDiscriminants;
use tor_error::{ErrorKind, HasKind as _};
use ErrorDetailDiscriminants as EDD;
use ErrorKind as EK;
#[allow(clippy::redundant_closure)] // for symmetry with prefs_of, below, and clarity
let prefs_def = || StreamPrefs::default();
let addr: TorAddr = "eweiibe6tdjsdprb4px6rqrzzcsi22m4koia44kc5pcjr7nec2rlxyad.onion:443"
.parse()
.unwrap();
fn map(
got: Result<impl Sized, ErrorDetail>,
) -> Result<(), (ErrorDetailDiscriminants, ErrorKind)> {
got.map(|_| ())
.map_err(|e| (ErrorDetailDiscriminants::from(&e), e.kind()))
}
let check_stream = |prefs, expected| {
let got = addr
.clone()
.into_stream_instructions(&Default::default(), &prefs);
assert_eq!(map(got), expected, "{prefs:?}");
};
let check_resolve = |prefs| {
let got = addr
.clone()
.into_resolve_instructions(&Default::default(), &prefs);
// This should be OnionAddressResolveRequest no matter if .onion is compiled in or enabled.
// Since compiling it in, or enabling it, won't help.
let expected = Err((EDD::OnionAddressResolveRequest, EK::NotImplemented));
assert_eq!(map(got), expected, "{prefs:?}");
};
cfg_if::cfg_if! {
if #[cfg(feature = "onion-service-client")] {
let prefs_of = |yn| {
let mut prefs = StreamPrefs::default();
prefs.connect_to_onion_services(yn);
prefs
};
check_stream(prefs_def(), Ok(()));
check_stream(prefs_of(true), Ok(()));
check_stream(prefs_of(false), Err((EDD::OnionAddressDisabled, EK::ForbiddenStreamTarget)));
check_resolve(prefs_def());
check_resolve(prefs_of(true));
check_resolve(prefs_of(false));
} else {
check_stream(prefs_def(), Err((EDD::OnionAddressNotSupported, EK::FeatureDisabled)));
check_resolve(prefs_def());
}
}
}
#[test]
fn convert_safe() {
fn check<A: IntoTorAddr>(a: A, s: &str) {

29
crates/arti-client/src/client.rs
View File

@ -112,7 +112,6 @@ pub struct TorClient<R: Runtime> {
#[cfg(feature = "pt-client")]
pt_mgr: Arc<tor_ptmgr::PtMgr<R>>,
/// HS client connector
#[allow(dead_code)] // TODO HS remove
#[cfg(feature = "onion-service-client")]
hsclient: HsClientConnector<R>,
/// The key manager.
@ -198,7 +197,8 @@ pub enum DormantMode {
}
/// Preferences for how to route a stream over the Tor network.
#[derive(Debug, Default, Clone)]
#[derive(Debug, Clone, Educe)]
#[educe(Default)]
pub struct StreamPrefs {
/// What kind of IPv6/IPv4 we'd prefer, and how strongly.
ip_ver_pref: IpVersionPreference,
@ -208,8 +208,8 @@ pub struct StreamPrefs {
optimistic_stream: bool,
/// Whether to try to make connections to onion services.
#[cfg(feature = "onion-service-client")]
#[allow(dead_code)]
connect_to_onion_services: bool, // TODO hs: this should default to "true".
#[educe(Default(true))]
pub(crate) connect_to_onion_services: bool,
}
/// Record of how we are isolating connections
@ -305,8 +305,17 @@ impl StreamPrefs {
self
}
// TODO hs: make setters for the `connect_to_onion_services` field.
/// Indicate whether connection to a hidden service (`.onion` service) should be allowed
///
/// If `false`, attempts to connect to Onion Services will be forced to fail with
/// an error of kind [`InvalidStreamTarget`](ErrorKind::InvalidStreamTarget).
///
/// By default this is enabled.
#[cfg(feature = "onion-service-client")]
pub fn connect_to_onion_services(&mut self, connect_to_onion_services: bool) -> &mut Self {
self.connect_to_onion_services = connect_to_onion_services;
self
}
/// Return a TargetPort to describe what kind of exit policy our
/// target circuit needs to support.
fn wrap_target_port(&self, port: u16) -> TargetPort {
@ -926,16 +935,14 @@ impl<R: Runtime> TorClient<R> {
/// Note that because Tor prefers to do DNS resolution on the remote
/// side of the network, this function takes its address as a string.
/// (See [`TorClient::connect()`] for more information.)
#[allow(clippy::missing_panics_doc)] // TODO HS remove
pub async fn connect_with_prefs<A: IntoTorAddr>(
&self,
target: A,
prefs: &StreamPrefs,
) -> crate::Result<DataStream> {
let addr = target.into_tor_addr().map_err(wrap_err)?;
addr.enforce_config(&self.addrcfg.get())?;
let (circ, addr, port) = match addr.into_stream_instructions()? {
let (circ, addr, port) = match addr.into_stream_instructions(&self.addrcfg.get(), prefs)? {
StreamInstructions::Exit {
hostname: addr,
port,
@ -958,7 +965,6 @@ impl<R: Runtime> TorClient<R> {
} => void::unreachable(hsid.0),
#[cfg(feature = "onion-service-client")]
#[allow(unused_variables)] // TODO HS remove
StreamInstructions::Hs {
hsid,
hostname,
@ -1073,9 +1079,8 @@ impl<R: Runtime> TorClient<R> {
// but I see no reason why it shouldn't be? Then `into_resolve_instructions`
// should be a method on `Host`, not `TorAddr`. -Diziet.
let addr = (hostname, 1).into_tor_addr().map_err(wrap_err)?;
addr.enforce_config(&self.addrcfg.get()).map_err(wrap_err)?;
match addr.into_resolve_instructions()? {
match addr.into_resolve_instructions(&self.addrcfg.get(), prefs)? {
ResolveInstructions::Exit(hostname) => {
let circ = self.get_or_launch_exit_circ(&[], prefs).await?;

15
crates/arti-client/src/err.rs
View File

@ -111,6 +111,8 @@ pub_if_error_detail! {
/// different kinds of [`Error`](crate::Error). If that doesn't provide enough information
/// for your use case, please let us know.
#[cfg_attr(docsrs, doc(cfg(feature = "error_detail")))]
#[cfg_attr(test, derive(strum::EnumDiscriminants))]
#[cfg_attr(test, strum_discriminants(vis(pub(crate))))]
#[derive(Error, Clone, Debug)]
#[non_exhaustive]
enum ErrorDetail {
@ -194,10 +196,15 @@ enum ErrorDetail {
#[error("Timed out while waiting for answer from exit")]
ExitTimeout,
/// Onion services are supported, but we were asked to connect to one.
#[error("Rejecting .onion address as unsupported")]
/// Onion services are not compiled in, but we were asked to connect to one.
#[error("Rejecting .onion address; feature onion-service-client not compiled in")]
OnionAddressNotSupported,
/// Onion services are supported, but we were asked to connect to one.
#[cfg(feature = "onion-service-client")]
#[error("Rejecting .onion address; connect_to_onion_services disabled in stream preferences")]
OnionAddressDisabled,
/// Error when trying to find the IP address of a hidden service
#[error("A .onion address cannot be resolved to an IP address")]
OnionAddressResolveRequest,
@ -354,8 +361,10 @@ impl tor_error::HasKind for ErrorDetail {
E::Configuration(e) => e.kind(),
E::Reconfigure(e) => e.kind(),
E::Spawn { cause, .. } => cause.kind(),
E::OnionAddressNotSupported => EK::NotImplemented,
E::OnionAddressNotSupported => EK::FeatureDisabled,
E::OnionAddressResolveRequest => EK::NotImplemented,
#[cfg(feature = "onion-service-client")]
E::OnionAddressDisabled => EK::ForbiddenStreamTarget,
// TODO Should delegate to TorAddrError EK
E::Address(_) | E::InvalidHostname => EK::InvalidStreamTarget,
E::LocalAddress => EK::ForbiddenStreamTarget,