Change multiplicity of ChannelMethod and addresses

Now each `ChanTarget` has at most one `ChannelMethod`, and only
`Direct` `ChannelMethods` can have multiple addresses.

Closes #600.
This commit is contained in:
Nick Mathewson 2022-10-06 16:35:52 -04:00
parent 82af3a370e
commit 74d4c73d09
9 changed files with 63 additions and 60 deletions

View File

@ -9,7 +9,7 @@ use crate::{event::ChanMgrEventSender, Error};
use safelog::sensitive as sv;
use std::time::Duration;
use tor_error::{bad_api_usage, internal};
use tor_linkspec::{ChannelMethod, HasChanMethods, HasRelayIds, OwnedChanTarget};
use tor_linkspec::{ChannelMethod, HasChanMethod, HasRelayIds, OwnedChanTarget};
use tor_llcrypto::pk;
use tor_proto::channel::params::ChannelPaddingInstructionsUpdates;
use tor_rtcompat::{tls::TlsConnector, Runtime, TcpProvider, TlsProvider};
@ -148,15 +148,11 @@ impl<R: Runtime> ChanBuilder<R> {
}
// TODO pt-client: right now this only handles the Direct method.
let direct_addrs: Vec<_> = target
.chan_methods()
.iter()
.filter_map(|method| match method {
ChannelMethod::Direct(addr) => Some(*addr),
#[allow(unreachable_patterns)] // TODO pt-client
_ => None,
})
.collect();
let direct_addrs: Vec<_> = match target.chan_method() {
ChannelMethod::Direct(addrs) => addrs,
#[allow(unreachable_patterns)] // TODO pt-client
_ => vec![], // TODO pt-client
};
let (stream, addr) = connect_to_one(&self.runtime, &direct_addrs).await?;
let using_target = match target.restrict_addr(&addr) {
Ok(v) => v,
@ -199,7 +195,7 @@ impl<R: Runtime> ChanBuilder<R> {
// 2. Set up the channel.
let mut builder = ChannelBuilder::new();
builder.set_declared_method(tor_linkspec::ChannelMethod::Direct(addr));
builder.set_declared_method(tor_linkspec::ChannelMethod::Direct(vec![addr]));
let chan = builder
.launch(
tls,
@ -304,7 +300,7 @@ mod test {
let tls_cert = msgs::X509_CERT.into();
let target = OwnedChanTarget::builder()
.addrs(vec![orport])
.methods(vec![ChannelMethod::Direct(orport)])
.method(ChannelMethod::Direct(vec![orport]))
.ed_identity(ed)
.rsa_identity(rsa)
.build()

View File

@ -57,6 +57,8 @@ pub struct Bridge {
//
/// Address and transport via which the bridge can be reached, and
/// the parameters for those transports.
///
/// Restriction: This `addrs` may NOT contain more than one address.
addrs: ChannelMethod,
/// The RSA identity of the bridge.
@ -220,7 +222,7 @@ impl FromStr for Bridge {
word: word.to_string(),
addr_error,
})?;
ChannelMethod::Direct(addr)
ChannelMethod::Direct(vec![addr])
} else {
#[cfg(not(feature = "pt-client"))]
return Err(BPE::PluggableTransportsNotSupported {
@ -333,7 +335,11 @@ impl Display for Bridge {
let settings = match addrs {
ChannelMethod::Direct(a) => {
write!(f, "{}", a)?;
if a.len() == 1 {
write!(f, "{}", a[0])?;
} else {
panic!("Somehow created a Bridge config with multiple addrs.");
}
None
}
@ -388,7 +394,7 @@ mod test {
}
fn mk_direct(s: &str) -> ChannelMethod {
ChannelMethod::Direct(s.parse().unwrap())
ChannelMethod::Direct(vec![s.parse().unwrap()])
}
fn mk_rsa(s: &str) -> RsaIdentity {

View File

@ -3,7 +3,7 @@
use std::sync::Arc;
use tor_linkspec::{
ChanTarget, CircTarget, HasAddrs, HasChanMethods, HasRelayIds, RelayIdRef, RelayIdType,
ChanTarget, CircTarget, HasAddrs, HasChanMethod, HasRelayIds, RelayIdRef, RelayIdType,
};
use super::{Bridge, BridgeDesc};
@ -65,8 +65,8 @@ impl HasAddrs for BridgeRelay {
}
}
impl HasChanMethods for BridgeRelay {
fn chan_methods(&self) -> Vec<tor_linkspec::ChannelMethod> {
impl HasChanMethod for BridgeRelay {
fn chan_method(&self) -> tor_linkspec::ChannelMethod {
todo!()
}
}
@ -92,8 +92,8 @@ impl<'a> HasAddrs for BridgeRelayWithDesc<'a> {
&[]
}
}
impl<'a> HasChanMethods for BridgeRelayWithDesc<'a> {
fn chan_methods(&self) -> Vec<tor_linkspec::ChannelMethod> {
impl<'a> HasChanMethod for BridgeRelayWithDesc<'a> {
fn chan_method(&self) -> tor_linkspec::ChannelMethod {
todo!()
}
}

View File

@ -105,7 +105,7 @@ pub use ids::{
pub use ls::LinkSpec;
pub use owned::{OwnedChanTarget, OwnedChanTargetBuilder, OwnedCircTarget, RelayIds};
pub use traits::{
ChanTarget, CircTarget, DirectChanMethodsHelper, HasAddrs, HasChanMethods, HasRelayIds,
ChanTarget, CircTarget, DirectChanMethodsHelper, HasAddrs, HasChanMethod, HasRelayIds,
HasRelayIdsLegacy,
};
pub use transport::{ChannelMethod, PtAddrError, PtTargetAddr, TransportId, TransportIdError};

View File

@ -7,7 +7,7 @@ use tor_config::impl_standard_builder;
use tor_llcrypto::pk;
use crate::{
ChanTarget, ChannelMethod, CircTarget, HasAddrs, HasChanMethods, HasRelayIds, RelayIdRef,
ChanTarget, ChannelMethod, CircTarget, HasAddrs, HasChanMethod, HasRelayIds, RelayIdRef,
RelayIdType,
};
@ -89,8 +89,8 @@ pub struct OwnedChanTarget {
//
// TODO: in many cases this will be redundant with addrs; if we allocate a
// lot of these objects, we might want to handle that.
#[builder(default)]
methods: Vec<ChannelMethod>,
#[builder(default = "self.make_method()")]
method: ChannelMethod,
/// Identities that this relay provides.
#[builder(sub_builder)]
ids: RelayIds,
@ -109,6 +109,11 @@ impl OwnedChanTargetBuilder {
self.ids().rsa_identity(id);
self
}
/// Helper: make a channel method if none was specified.
fn make_method(&self) -> ChannelMethod {
ChannelMethod::Direct(self.addrs.clone().unwrap_or_default())
}
}
impl HasAddrs for OwnedChanTarget {
@ -117,9 +122,9 @@ impl HasAddrs for OwnedChanTarget {
}
}
impl HasChanMethods for OwnedChanTarget {
fn chan_methods(&self) -> Vec<ChannelMethod> {
self.methods.clone()
impl HasChanMethod for OwnedChanTarget {
fn chan_method(&self) -> ChannelMethod {
self.method.clone()
}
}
@ -139,7 +144,7 @@ impl OwnedChanTarget {
{
OwnedChanTarget {
addrs: target.addrs().to_vec(),
methods: target.chan_methods(),
method: target.chan_method(),
ids: RelayIds::from_relay_ids(target),
}
}
@ -154,7 +159,7 @@ impl OwnedChanTarget {
if self.addrs.contains(addr) {
Ok(OwnedChanTarget {
addrs: vec![*addr],
methods: self.methods.clone(),
method: self.method.clone(),
ids: self.ids.clone(),
})
} else {
@ -229,9 +234,9 @@ impl HasRelayIds for OwnedCircTarget {
self.chan_target.identity(key_type)
}
}
impl HasChanMethods for OwnedCircTarget {
fn chan_methods(&self) -> Vec<ChannelMethod> {
self.chan_target.chan_methods()
impl HasChanMethod for OwnedCircTarget {
fn chan_method(&self) -> ChannelMethod {
self.chan_target.chan_method()
}
}

View File

@ -144,12 +144,12 @@ pub trait HasAddrs {
}
/// An object that can be connected to via [`ChannelMethod`]s.
pub trait HasChanMethods {
pub trait HasChanMethod {
/// Return the known ways to contact this
// TODO: See notes on HasAddrs above. TODO pt-client: I don't like having
// this return a Vec, but I don't see a great alternative. Let's revisit
// that.-nickm.
fn chan_methods(&self) -> Vec<ChannelMethod>;
// TODO: See notes on HasAddrs above.
// TODO pt-client: I don't like having this return a new ChannelMethod, but I
// don't see a great alternative. Let's revisit that.-nickm.
fn chan_method(&self) -> ChannelMethod;
}
/// Implement `HasChanMethods` for an object with `HasAddr` whose addresses
@ -160,12 +160,9 @@ pub trait HasChanMethods {
// should instead suggest that we _only_ have direct methods?
pub trait DirectChanMethodsHelper: HasAddrs {}
impl<D: DirectChanMethodsHelper> HasChanMethods for D {
fn chan_methods(&self) -> Vec<ChannelMethod> {
self.addrs()
.iter()
.map(|a| ChannelMethod::Direct(*a))
.collect()
impl<D: DirectChanMethodsHelper> HasChanMethod for D {
fn chan_method(&self) -> ChannelMethod {
ChannelMethod::Direct(self.addrs().to_vec())
}
}
@ -174,7 +171,7 @@ impl<D: DirectChanMethodsHelper> HasChanMethods for D {
/// Anything that implements 'ChanTarget' can be used as the
/// identity of a relay for the purposes of launching a new
/// channel.
pub trait ChanTarget: HasRelayIds + HasAddrs + HasChanMethods {}
pub trait ChanTarget: HasRelayIds + HasAddrs + HasChanMethod {}
/// Information about a Tor relay used to extend a circuit to it.
///
@ -187,11 +184,8 @@ pub trait CircTarget: ChanTarget {
// doing so correctly would require default associated types.
fn linkspecs(&self) -> Vec<crate::LinkSpec> {
let mut result: Vec<_> = self.identities().map(|id| id.to_owned().into()).collect();
for method in self.chan_methods().iter() {
#[allow(irrefutable_let_patterns)] // TODO pt-client
if let ChannelMethod::Direct(addr) = method {
result.push(addr.into());
}
if let ChannelMethod::Direct(addrs) = self.chan_method() {
result.extend(addrs.into_iter().map(crate::LinkSpec::from));
}
result
}

View File

@ -383,18 +383,20 @@ impl PtTarget {
#[derive(Clone, Debug, Eq, PartialEq)]
#[allow(clippy::exhaustive_enums)]
// TODO pt-client: I am not in love with this enum name --nm.
// TODO pt-client: Maybe "ContactMethod" would be better?
pub enum ChannelMethod {
/// Connect to the relay directly at a given address.
Direct(std::net::SocketAddr),
/// Connect to the relay directly at one of several addresses.
Direct(Vec<std::net::SocketAddr>),
// TODO pt-client: We may want to have a third variant for "Direct" with a
// single address. Maybe?
/// Connect to a bridge relay via a pluggable transport.
#[cfg(feature = "pt-client")]
Pluggable(PtTarget),
}
impl ChannelMethod {
/// Return the advertised socket address that this method connects to, if
/// there is one.
/// Return an advertised socket address that this method connects to.
///
/// NOTE that this is not necessarily an address to which you can open a
/// TCP connection! If this `ChannelMethod` is using a non-`Direct`
@ -402,7 +404,7 @@ impl ChannelMethod {
/// implementation.
pub fn declared_peer_addr(&self) -> Option<&std::net::SocketAddr> {
match self {
ChannelMethod::Direct(addr) => Some(addr),
ChannelMethod::Direct(addr) if !addr.is_empty() => Some(&addr[0]),
#[cfg(feature = "pt-client")]
ChannelMethod::Pluggable(PtTarget {

View File

@ -295,7 +295,7 @@ impl ChannelBuilder {
/// connection to a given socket address.
#[deprecated(note = "use set_declared_method instead", since = "0.7.1")]
pub fn set_declared_addr(&mut self, target: std::net::SocketAddr) {
self.set_declared_method(tor_linkspec::ChannelMethod::Direct(target));
self.set_declared_method(tor_linkspec::ChannelMethod::Direct(vec![target]));
}
/// Set the declared target method of this channel.
@ -736,9 +736,9 @@ pub(crate) mod test {
fn chanbuilder() {
let rt = PreferredRuntime::create().unwrap();
let mut builder = ChannelBuilder::default();
builder.set_declared_method(tor_linkspec::ChannelMethod::Direct(
"127.0.0.1:9001".parse().unwrap(),
));
builder.set_declared_method(tor_linkspec::ChannelMethod::Direct(vec!["127.0.0.1:9001"
.parse()
.unwrap()]));
let tls = MsgBuf::new(&b""[..]);
let _outbound = builder.launch(tls, rt);
}

View File

@ -595,7 +595,7 @@ impl<T: AsyncRead + AsyncWrite + Send + Unpin + 'static, S: SleepProvider> Verif
if let Some(addr) = target_method.declared_peer_addr() {
peer_builder.addrs(vec![*addr]);
}
peer_builder.methods(vec![target_method]);
peer_builder.method(target_method);
}
let peer_id = peer_builder
.ed_identity(self.ed25519_id)
@ -1078,7 +1078,7 @@ pub(super) mod test {
link_protocol: 4,
tls: futures_codec::Framed::new(MsgBuf::new(&b""[..]), ChannelCodec::new(4)),
unique_id: UniqId::new(),
target_method: Some(ChannelMethod::Direct(peer_addr)),
target_method: Some(ChannelMethod::Direct(vec![peer_addr])),
ed25519_id,
rsa_id,
clock_skew: ClockSkew::None,