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

Final (?) API revisions for tor-linkspec 

With this change, each individual identity type becomes optional.
The functions that expose them unconditionally are now in a "legacy"
trait that only some downstream types are expected to implement.

There are new convenience APIs in HasRelayIds:
  * to return Option<&keytype>,
  * to see if one identity-set contains another.

This commit will break several downstream crates!  For the
reviewer's convenience, I will put the fixes for those crates into a
series of squash! commits on this one.

tor-netdir
----------

Revise tor-netdir to accept optional identities.  This required some
caveats and workarounds about the cases where we have to deal with a
key type that the tor-netdir code does not currently recognize at
all.  If we start to add more identity types in the future, we may
well want more internal indices in this code.

tor-proto
---------

In order to make tor-proto support optional identities, there were
fewer changes than I thought.  Some "check" functions needed to start
looking at "all the ids we want" rather than at "the two known IDs";
they also needed to accommodate that case where we don't have an ID
that we demand.

This change will also help with bridges, since we want to be able to
connect to a bridge without knowing all of its IDs up front.

The protocol currently _requires_ the two current ID types in some
places. To deal with that, I added a new `MissingId` error.

I also removed a couple of unconditional identity accessors for
chanmgr; code should use `target().identity(...)` instead.

tor-chanmgr
-----------

This is an incomplete conversion: it does not at all handle channel
targets without Ed25519 identities yet.  It still uses those
identities to index its internal map from identity to channel; but
it gives a new `MissingId` error type if it's given a channel target
that doesn't have one.

We'll want to revise the map type again down the road when we
implement bridges, but I'd rather not step on the channel-padding
work in progress right now.

tor-guardmgr
------------

This change is mostly a matter of constructing owned identity types
more sensibly, rather than unwrapping them directly.

There are some places marked with TODOs where we still depend on
particular identity types, because of how the directory protocol
works.  This will need revisiting when we add bridge support here.

tor-circmgr
-----------

These changes are just relatively simple API changes in the tests.
This commit is contained in:
Nick Mathewson 2022-08-05 11:47:05 -04:00
parent 2c2224d6db
commit 2d4507ff35
22 changed files with 335 additions and 223 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
crates/tor-chanmgr/src/builder.rs
View File

@ -9,7 +9,7 @@ use crate::{event::ChanMgrEventSender, Error};
use std::result::Result as StdResult;
use std::time::Duration;
use tor_error::{bad_api_usage, internal};
use tor_linkspec::{HasAddrs, OwnedChanTarget};
use tor_linkspec::{HasAddrs, HasRelayIds, OwnedChanTarget};
use tor_llcrypto::pk;
use tor_proto::channel::params::ChannelsParamsUpdates;
use tor_rtcompat::{tls::TlsConnector, Runtime, TcpProvider, TlsProvider};
@ -242,7 +242,9 @@ impl<R: Runtime> ChanBuilder<R> {
impl crate::mgr::AbstractChannel for tor_proto::channel::Channel {
type Ident = pk::ed25519::Ed25519Identity;
fn ident(&self) -> &Self::Ident {
self.peer_ed25519_id()
self.target()
.ed_identity()
.expect("This channel had an Ed25519 identity when we created it, but now it doesn't!?")
}
fn is_usable(&self) -> bool {
!self.is_closing()

11
crates/tor-chanmgr/src/err.rs
View File

@ -76,6 +76,14 @@ pub enum Error {
#[source]
cause: Arc<SpawnError>,
},
/// A relay did not have the set of identity keys that we expected.
///
/// (Currently, `tor-chanmgr` only works to manage channels with a known
/// expected Ed25519 identity.)
#[error("Could not identify relay by identity key")]
MissingId,
/// An internal error of some kind that should never occur.
#[error("Internal error")]
Internal(#[from] tor_error::Bug),
@ -103,6 +111,7 @@ impl tor_error::HasKind for Error {
E::Proto { source, .. } => source.kind(),
E::PendingFailed { .. } => EK::TorAccessFailed,
E::UnusableTarget(_) | E::Internal(_) => EK::Internal,
E::MissingId => EK::BadApiUsage,
Error::ChannelBuild { .. } => EK::TorAccessFailed,
}
}
@ -134,7 +143,7 @@ impl tor_error::HasRetryTime for Error {
E::UnusableTarget(_) => RT::Never,
// These aren't recoverable at all.
E::Spawn { .. } | E::Internal(_) => RT::Never,
E::Spawn { .. } | E::MissingId | E::Internal(_) => RT::Never,
}
}
}

7
crates/tor-chanmgr/src/lib.rs
View File

@ -155,7 +155,12 @@ impl<R: Runtime> ChanMgr<R> {
&self,
target: &T,
) -> Result<(Channel, ChanProvenance)> {
let ed_identity = target.ed_identity();
// TODO(nickm): We will need to change the way that we index our map
// when we eventually support channels that are _not_ primarily
// identified by their ed25519 key. That could be in the distant future
// when we make Ed25519 keys optional: But more likely it will be when
// we implement bridges.
let ed_identity = target.ed_identity().ok_or(Error::MissingId)?;
let targetinfo = OwnedChanTarget::from_chan_target(target);
let (chan, provenance) = self.mgr.get_or_launch(*ed_identity, targetinfo).await?;

4
crates/tor-circmgr/src/path/dirpath.rs
View File

@ -100,7 +100,7 @@ mod test {
use std::collections::HashSet;
use tor_basic_utils::test_rng::testing_rng;
use tor_guardmgr::fallback::{FallbackDir, FallbackList};
use tor_linkspec::HasRelayIds;
use tor_linkspec::RelayIds;
use tor_netdir::testnet;
#[test]
@ -200,7 +200,7 @@ mod test {
.pick_path(&mut rng, dirinfo, Some(&guards))
.unwrap();
if let crate::path::TorPathInner::OwnedOneHop(relay) = path.inner {
distinct_guards.insert(relay.ed_identity().clone());
distinct_guards.insert(RelayIds::from_relay_ids(&relay));
mon.unwrap().succeeded();
assert!(usable.unwrap().await.unwrap());
} else {

12
crates/tor-circmgr/src/path/exitpath.rs
View File

@ -263,7 +263,7 @@ mod test {
use crate::test::OptDummyGuardMgr;
use std::collections::HashSet;
use tor_basic_utils::test_rng::testing_rng;
use tor_linkspec::HasRelayIds;
use tor_linkspec::{HasRelayIds, RelayIds};
use tor_netdir::testnet;
use tor_rtcompat::SleepProvider;
@ -431,9 +431,9 @@ mod test {
assert_same_path_when_owned(&path);
if let TorPathInner::Path(p) = path.inner {
assert_exit_path_ok(&p[..]);
distinct_guards.insert(p[0].ed_identity().clone());
distinct_mid.insert(p[1].ed_identity().clone());
distinct_exit.insert(p[2].ed_identity().clone());
distinct_guards.insert(RelayIds::from_relay_ids(&p[0]));
distinct_mid.insert(RelayIds::from_relay_ids(&p[1]));
distinct_exit.insert(RelayIds::from_relay_ids(&p[2]));
} else {
panic!("Wrong kind of path");
}
@ -450,9 +450,9 @@ mod test {
assert_ne!(distinct_exit.len(), 1);
let guard_relay = netdir
.by_id(distinct_guards.iter().next().unwrap())
.by_ids(distinct_guards.iter().next().unwrap())
.unwrap();
let exit_relay = netdir.by_id(distinct_exit.iter().next().unwrap()).unwrap();
let exit_relay = netdir.by_ids(distinct_exit.iter().next().unwrap()).unwrap();
// Now we'll try a forced exit that is not the same as our
// actual guard.

4
crates/tor-guardmgr/src/fallback.rs
View File

@ -58,7 +58,7 @@ impl FallbackDir {
/// Return a copy of this FallbackDir as a [`FirstHop`](crate::FirstHop)
pub fn as_guard(&self) -> crate::FirstHop {
crate::FirstHop {
id: FallbackId::from_chan_target(self).into(),
id: FallbackId::from_relay_ids(self).into(),
orports: self.orports.clone(),
}
}
@ -119,7 +119,7 @@ impl tor_linkspec::HasAddrs for FallbackDir {
&self.orports[..]
}
}
impl tor_linkspec::HasRelayIds for FallbackDir {
impl tor_linkspec::HasRelayIdsLegacy for FallbackDir {
fn ed_identity(&self) -> &Ed25519Identity {
&self.ed_identity
}

8
crates/tor-guardmgr/src/fallback/set.rs
View File

@ -278,7 +278,7 @@ mod test {
rand_fb(&mut rng),
];
let fb_other = rand_fb(&mut rng);
let id_other = FallbackId::from_chan_target(&fb_other);
let id_other = FallbackId::from_relay_ids(&fb_other);
// basic case: construct a set
let list: FallbackList = fbs.clone().into();
@ -294,7 +294,7 @@ mod test {
// use the constructed set a little.
for fb in fbs.iter() {
let id = FallbackId::from_chan_target(fb);
let id = FallbackId::from_relay_ids(fb);
assert_eq!(set.get_mut(&id).unwrap().id(), &id);
}
assert!(set.get_mut(&id_other).is_none());
@ -431,7 +431,7 @@ mod test {
let mut fbs2: Vec<_> = fbs
.into_iter()
// (Remove the fallback with id==ids[2])
.filter(|fb| FallbackId::from_chan_target(fb) != ids[2])
.filter(|fb| FallbackId::from_relay_ids(fb) != ids[2])
.collect();
// add 2 new ones.
let fbs_new = vec![rand_fb(&mut rng), rand_fb(&mut rng), rand_fb(&mut rng)];
@ -450,7 +450,7 @@ mod test {
// Make sure that the new fbs are there.
for new_fb in fbs_new {
assert!(set2
.get_mut(&FallbackId::from_chan_target(&new_fb))
.get_mut(&FallbackId::from_relay_ids(&new_fb))
.unwrap()
.status
.usable_at(now));

15
crates/tor-guardmgr/src/guard.rs
View File

@ -1,7 +1,6 @@
//! Code to represent its single guard node and track its status.
use tor_basic_utils::retry::RetryDelay;
use tor_llcrypto::pk::{ed25519::Ed25519Identity, rsa::RsaIdentity};
use tor_netdir::{NetDir, Relay, RelayWeight};
use educe::Educe;
@ -220,7 +219,7 @@ impl Guard {
);
Self::new(
GuardId::from_chan_target(relay),
GuardId::from_relay_ids(relay),
relay.addrs().into(),
added_at,
)
@ -659,7 +658,7 @@ impl Guard {
/// We use this information to decide whether we are about to sample
/// too much of the network as guards.
pub(crate) fn get_weight(&self, dir: &NetDir) -> Option<RelayWeight> {
dir.weight_by_rsa_id(self.id.0.rsa_identity(), tor_netdir::WeightRole::Guard)
dir.weight_by_rsa_id(self.id.0.rsa_identity()?, tor_netdir::WeightRole::Guard)
}
/// Return a [`FirstHop`](crate::FirstHop) object to represent this guard.
@ -695,11 +694,11 @@ impl tor_linkspec::HasAddrs for Guard {
}
impl tor_linkspec::HasRelayIds for Guard {
fn ed_identity(&self) -> &Ed25519Identity {
self.id.0.ed_identity()
}
fn rsa_identity(&self) -> &RsaIdentity {
self.id.0.rsa_identity()
fn identity(
&self,
key_type: tor_linkspec::RelayIdType,
) -> Option<tor_linkspec::RelayIdRef<'_>> {
self.id.0.identity(key_type)
}
}

34
crates/tor-guardmgr/src/ids.rs
View File

@ -2,7 +2,8 @@
use derive_more::AsRef;
use serde::{Deserialize, Serialize};
use tor_linkspec::{HasRelayIds, RelayIds};
use tor_linkspec::RelayIds;
#[cfg(test)]
use tor_llcrypto::pk;
/// An identifier for a fallback directory cache.
@ -14,11 +15,8 @@ pub(crate) struct FallbackId(pub(crate) RelayIds);
impl FallbackId {
/// Return a new, manually constructed `FallbackId`
pub(crate) fn new(ed25519: pk::ed25519::Ed25519Identity, rsa: pk::rsa::RsaIdentity) -> Self {
Self(RelayIds::new(ed25519, rsa))
}
/// Extract a `FallbackId` from a ChanTarget object.
pub(crate) fn from_chan_target<T: tor_linkspec::ChanTarget>(target: &T) -> Self {
pub(crate) fn from_relay_ids<T: tor_linkspec::HasRelayIds>(target: &T) -> Self {
Self(RelayIds::from_relay_ids(target))
}
}
@ -33,12 +31,13 @@ pub(crate) struct GuardId(pub(crate) RelayIds);
impl GuardId {
/// Return a new, manually constructed `GuardId`
#[cfg(test)]
pub(crate) fn new(ed25519: pk::ed25519::Ed25519Identity, rsa: pk::rsa::RsaIdentity) -> Self {
Self(RelayIds::new(ed25519, rsa))
}
/// Extract a `GuardId` from a ChanTarget object.
pub(crate) fn from_chan_target<T: tor_linkspec::ChanTarget>(target: &T) -> Self {
Self::new(*target.ed_identity(), *target.rsa_identity())
pub(crate) fn from_relay_ids<T: tor_linkspec::HasRelayIds>(target: &T) -> Self {
Self(RelayIds::from_relay_ids(target))
}
}
@ -56,8 +55,12 @@ pub(crate) enum FirstHopIdInner {
/// A unique cryptographic identifier for a selected guard or fallback
/// directory.
///
/// (This is implemented internally using both of the guard's Ed25519 and RSA
/// identities.)
/// (This is implemented internally using all of the guard's known identities.)
///
/// TODO(nickm): we may want a fuzzier match for this type in the future in our
/// maps, if we ever learn about more identity types. Right now we don't
/// recognize two `FirstHopId`s as "the same" if one has more IDs than the
/// other, even if all the IDs that they do have are the same.
#[derive(Clone, Debug, Eq, PartialEq, Ord, PartialOrd)]
pub struct FirstHopId(pub(crate) FirstHopIdInner);
@ -83,13 +86,12 @@ impl AsRef<RelayIds> for FirstHopId {
}
}
}
impl HasRelayIds for FirstHopId {
fn ed_identity(&self) -> &pk::ed25519::Ed25519Identity {
self.as_ref().ed_identity()
}
fn rsa_identity(&self) -> &pk::rsa::RsaIdentity {
self.as_ref().rsa_identity()
impl tor_linkspec::HasRelayIds for FirstHopId {
fn identity(
&self,
key_type: tor_linkspec::RelayIdType,
) -> Option<tor_linkspec::RelayIdRef<'_>> {
self.as_ref().identity(key_type)
}
}

40
crates/tor-guardmgr/src/lib.rs
View File

@ -148,7 +148,6 @@ use tracing::{debug, info, trace, warn};
use tor_config::impl_standard_builder;
use tor_config::{define_list_builder_accessors, define_list_builder_helper};
use tor_llcrypto::pk;
use tor_netdir::{params::NetParameters, NetDir, Relay};
use tor_persist::{DynStorageHandle, StateMgr};
use tor_rtcompat::Runtime;
@ -579,11 +578,9 @@ impl<R: Runtime> GuardMgr<R> {
identity: &impl tor_linkspec::HasRelayIds,
external_failure: ExternalActivity,
) {
let ed_identity = identity.ed_identity();
let rsa_identity = identity.rsa_identity();
let now = self.runtime.now();
let mut inner = self.inner.lock().expect("Poisoned lock");
let ids = inner.lookup_ids(ed_identity, rsa_identity);
let ids = inner.lookup_ids(identity);
for id in ids {
match &id.0 {
FirstHopIdInner::Guard(id) => {
@ -609,17 +606,9 @@ impl<R: Runtime> GuardMgr<R> {
identity: &impl tor_linkspec::HasRelayIds,
external_activity: ExternalActivity,
) {
let ed_identity = identity.ed_identity();
let rsa_identity = identity.rsa_identity();
let mut inner = self.inner.lock().expect("Poisoned lock");
inner.record_external_success(
ed_identity,
rsa_identity,
external_activity,
self.runtime.wallclock(),
);
inner.record_external_success(identity, external_activity, self.runtime.wallclock());
}
/// Return a stream of events about our estimated clock skew; these events
@ -984,12 +973,11 @@ impl GuardMgrInner {
/// we have `mut self` borrowed.)
fn record_external_success(
&mut self,
ed_identity: &pk::ed25519::Ed25519Identity,
rsa_identity: &pk::rsa::RsaIdentity,
identity: &impl tor_linkspec::HasRelayIds,
external_activity: ExternalActivity,
now: SystemTime,
) {
for id in self.lookup_ids(ed_identity, rsa_identity) {
for id in self.lookup_ids(identity) {
match &id.0 {
FirstHopIdInner::Guard(id) => {
self.guards.active_guards_mut().record_success(
@ -1104,19 +1092,15 @@ impl GuardMgrInner {
/// doesn't know whether its circuit came from a guard or a fallback. To
/// solve that, we'll need CircMgr to record and report which one it was
/// using, which will take some more plumbing.
fn lookup_ids(
&self,
ed_identity: &pk::ed25519::Ed25519Identity,
rsa_identity: &pk::rsa::RsaIdentity,
) -> Vec<FirstHopId> {
fn lookup_ids(&self, identity: &impl tor_linkspec::HasRelayIds) -> Vec<FirstHopId> {
let mut vec = Vec::with_capacity(2);
let id = ids::GuardId::new(*ed_identity, *rsa_identity);
let id = ids::GuardId::from_relay_ids(identity);
if self.guards.active_guards().contains(&id) {
vec.push(id.into());
}
let id = ids::FallbackId::new(*ed_identity, *rsa_identity);
let id = ids::FallbackId::from_relay_ids(identity);
if self.fallbacks.contains(&id) {
vec.push(id.into());
}
@ -1329,11 +1313,11 @@ impl tor_linkspec::HasAddrs for FirstHop {
}
}
impl tor_linkspec::HasRelayIds for FirstHop {
fn ed_identity(&self) -> &pk::ed25519::Ed25519Identity {
self.id.ed_identity()
}
fn rsa_identity(&self) -> &pk::rsa::RsaIdentity {
self.id.rsa_identity()
fn identity(
&self,
key_type: tor_linkspec::RelayIdType,
) -> Option<tor_linkspec::RelayIdRef<'_>> {
self.id.identity(key_type)
}
}
impl tor_linkspec::ChanTarget for FirstHop {}

4
crates/tor-guardmgr/src/sample.rs
View File

@ -258,7 +258,7 @@ impl GuardSet {
fn contains_relay(&self, relay: &Relay<'_>) -> bool {
// Note: Could implement Borrow instead, but I don't think it'll
// matter.
let id = GuardId::from_chan_target(relay);
let id = GuardId::from_relay_ids(relay);
self.contains(&id)
}
@ -404,7 +404,7 @@ impl GuardSet {
///
/// Does nothing if it is already a guard.
fn add_guard(&mut self, relay: &Relay<'_>, now: SystemTime, params: &GuardParams) {
let id = GuardId::from_chan_target(relay);
let id = GuardId::from_relay_ids(relay);
if self.guards.contains_key(&id) {
return;
}

24
crates/tor-linkspec/src/ids.rs
View File

@ -116,30 +116,6 @@ impl RelayId {
RelayId::Rsa(key) => key.as_bytes(),
}
}
/// Extract the RsaIdentity from a RelayId that is known to hold one.
///
/// # Panics
///
/// Panics if this is not an RSA identity.
pub(crate) fn unwrap_rsa(self) -> RsaIdentity {
match self {
RelayId::Rsa(rsa) => rsa,
_ => panic!("Not an RSA identity."),
}
}
/// Extract the Ed25519Identity from a RelayId that is known to hold one.
///
/// # Panics
///
/// Panics if this is not an Ed25519 identity.
pub(crate) fn unwrap_ed25519(self) -> Ed25519Identity {
match self {
RelayId::Ed25519(ed25519) => ed25519,
_ => panic!("Not an Ed25519 identity."),
}
}
}
impl<'a> RelayIdRef<'a> {

2
crates/tor-linkspec/src/lib.rs
View File

@ -80,4 +80,4 @@ mod traits;
pub use ids::{set::RelayIdSet, RelayId, RelayIdError, RelayIdRef, RelayIdType, RelayIdTypeIter};
pub use ls::LinkSpec;
pub use owned::{OwnedChanTarget, OwnedCircTarget, RelayIds};
pub use traits::{ChanTarget, CircTarget, HasAddrs, HasRelayIds};
pub use traits::{ChanTarget, CircTarget, HasAddrs, HasRelayIds, HasRelayIdsLegacy};

60
crates/tor-linkspec/src/owned.rs
View File

@ -5,26 +5,29 @@ use std::fmt::{self, Display};
use std::net::SocketAddr;
use tor_llcrypto::pk;
use crate::{ChanTarget, CircTarget, HasAddrs, HasRelayIds};
use crate::{ChanTarget, CircTarget, HasAddrs, HasRelayIds, RelayIdRef, RelayIdType};
/// RelayIds is an owned copy of the set of identities of a relay.
/// RelayIds is an owned copy of the set of known identities of a relay.
///
/// Note that an object of this type will not necessarily have every type of
/// identity: it's possible that we don't know all the identities, or that one
/// of the identity types has become optional.
#[derive(Debug, Clone, Eq, PartialEq, Hash, PartialOrd, Ord, Serialize, Deserialize)]
pub struct RelayIds {
/// Copy of the ed25519 id from the underlying ChanTarget.
#[serde(rename = "ed25519")]
ed_identity: pk::ed25519::Ed25519Identity,
ed_identity: Option<pk::ed25519::Ed25519Identity>,
/// Copy of the rsa id from the underlying ChanTarget.
#[serde(rename = "rsa")]
rsa_identity: pk::rsa::RsaIdentity,
rsa_identity: Option<pk::rsa::RsaIdentity>,
}
impl HasRelayIds for RelayIds {
fn ed_identity(&self) -> &pk::ed25519::Ed25519Identity {
&self.ed_identity
}
fn rsa_identity(&self) -> &pk::rsa::RsaIdentity {
&self.rsa_identity
fn identity(&self, key_type: RelayIdType) -> Option<crate::RelayIdRef<'_>> {
match key_type {
RelayIdType::Ed25519 => self.ed_identity.as_ref().map(RelayIdRef::from),
RelayIdType::Rsa => self.rsa_identity.as_ref().map(RelayIdRef::from),
}
}
}
@ -35,15 +38,23 @@ impl RelayIds {
rsa_identity: pk::rsa::RsaIdentity,
) -> Self {
Self {
ed_identity,
rsa_identity,
ed_identity: Some(ed_identity),
rsa_identity: Some(rsa_identity),
}
}
/// Construct a new RelayIds object from another object that impements
/// [`HasRelayIds`]
/// Construct a new `RelayIds` object from another object that implements
/// [`HasRelayIds`].
///
/// Note that it is possible to construct an _empty_ `RelayIds` object if
/// the input does not contain any recognized identity type.
pub fn from_relay_ids<T: HasRelayIds + ?Sized>(other: &T) -> Self {
Self::new(*other.ed_identity(), *other.rsa_identity())
Self {
ed_identity: other
.identity(RelayIdType::Ed25519)
.map(|r| *r.unwrap_ed25519()),
rsa_identity: other.identity(RelayIdType::Rsa).map(|r| *r.unwrap_rsa()),
}
}
}
@ -64,12 +75,8 @@ impl HasAddrs for OwnedChanTarget {
}
impl HasRelayIds for OwnedChanTarget {
fn ed_identity(&self) -> &pk::ed25519::Ed25519Identity {
self.ids.ed_identity()
}
fn rsa_identity(&self) -> &pk::rsa::RsaIdentity {
self.ids.rsa_identity()
fn identity(&self, key_type: RelayIdType) -> Option<RelayIdRef<'_>> {
self.ids.identity(key_type)
}
}
@ -124,7 +131,9 @@ impl Display for OwnedChanTarget {
[a] => write!(f, "{}", a)?,
[a, ..] => write!(f, "{}+", a)?,
};
write!(f, "{}", self.ed_identity())?; // short enough to print
for ident in self.identities() {
write!(f, " {}", ident)?;
}
write!(f, "]")?;
Ok(())
}
@ -186,11 +195,8 @@ impl HasAddrs for OwnedCircTarget {
}
impl HasRelayIds for OwnedCircTarget {
fn ed_identity(&self) -> &pk::ed25519::Ed25519Identity {
self.chan_target.ed_identity()
}
fn rsa_identity(&self) -> &pk::rsa::RsaIdentity {
self.chan_target.rsa_identity()
fn identity(&self, key_type: RelayIdType) -> Option<RelayIdRef<'_>> {
self.chan_target.identity(key_type)
}
}

76
crates/tor-linkspec/src/traits.rs
View File

@ -6,19 +6,30 @@ use tor_llcrypto::pk;
use crate::{RelayIdRef, RelayIdType, RelayIdTypeIter};
/// Legacy implementation helper for HasRelayIds.
///
/// Previously, we assumed that everything had these two identity types, which
/// is not an assumption we want to keep making in the future.
pub trait HasRelayIdsLegacy {
/// Return the ed25519 identity for this relay.
fn ed_identity(&self) -> &pk::ed25519::Ed25519Identity;
/// Return the RSA identity for this relay.
fn rsa_identity(&self) -> &pk::rsa::RsaIdentity;
}
/// An object containing information about a relay's identity keys.
///
/// This trait has a fairly large number of methods, most of which you're not
/// actually expected to implement. The only one that you need to provide is
/// [`identity`](HasRelayIds::identity).
pub trait HasRelayIds {
/// Return the identity of this relay whose type is `key_type`, or None if
/// the relay has no such identity.
///
/// (Currently all relays have all recognized identity types, but we might
/// implement or deprecate an identity type in the future.)
fn identity(&self, key_type: RelayIdType) -> Option<RelayIdRef<'_>> {
match key_type {
RelayIdType::Rsa => Some(self.rsa_identity().into()),
RelayIdType::Ed25519 => Some(self.ed_identity().into()),
}
}
fn identity(&self, key_type: RelayIdType) -> Option<RelayIdRef<'_>>;
/// Return an iterator over all of the identities held by this object.
fn identities(&self) -> RelayIdIter<'_, Self> {
RelayIdIter {
@ -26,6 +37,18 @@ pub trait HasRelayIds {
next_key: RelayIdType::all_types(),
}
}
/// Return the ed25519 identity for this relay if it has one.
fn ed_identity(&self) -> Option<&pk::ed25519::Ed25519Identity> {
self.identity(RelayIdType::Ed25519)
.map(RelayIdRef::unwrap_ed25519)
}
/// Return the RSA identity for this relay if it has one.
fn rsa_identity(&self) -> Option<&pk::rsa::RsaIdentity> {
self.identity(RelayIdType::Rsa).map(RelayIdRef::unwrap_rsa)
}
/// Check whether the provided Id is a known identity of this relay.
///
/// Remember that a given set of identity keys may be incomplete: some
@ -38,19 +61,44 @@ pub trait HasRelayIds {
self.identity(id.id_type()).map(|my_id| my_id == id) == Some(true)
}
/// Return the ed25519 identity for this relay.
fn ed_identity(&self) -> &pk::ed25519::Ed25519Identity;
/// Return the RSA identity for this relay.
fn rsa_identity(&self) -> &pk::rsa::RsaIdentity;
/// Return true if this object has exactly the same relay IDs as `other`.
//
// TODO: Once we make it so particular identity key types are optional, we
// should add a note saying that this function is usually not what you want
// for many cases, since you might want to know "could this be the same
// relay" vs "is this definitely the same relay."
fn same_relay_ids<T: HasRelayIds>(&self, other: &T) -> bool {
self.ed_identity() == other.ed_identity() && self.rsa_identity() == other.rsa_identity()
//
// NOTE: We don't make this an `Eq` method, since we want to make callers
// choose carefully among this method, `has_all_relay_ids_from`, and any
// similar methods we add in the future.
fn same_relay_ids<T: HasRelayIds + ?Sized>(&self, other: &T) -> bool {
RelayIdType::all_types().all(|key_type| self.identity(key_type) == other.identity(key_type))
}
/// Return true if this object has every relay ID that `other` does.
///
/// (It still returns true if there are some IDs in this object that are not
/// present in `other`.)
fn has_all_relay_ids_from<T: HasRelayIds + ?Sized>(&self, other: &T) -> bool {
RelayIdType::all_types().all(|key_type| {
match (self.identity(key_type), other.identity(key_type)) {
// If we both have the same key for this type, great.
(Some(mine), Some(theirs)) if mine == theirs => true,
// Uh oh. They do have a key for his type, but it's not ours.
(_, Some(_theirs)) => false,
// If they don't care what we have for this type, great.
(_, None) => true,
}
})
}
}
impl<T: HasRelayIdsLegacy> HasRelayIds for T {
fn identity(&self, key_type: RelayIdType) -> Option<RelayIdRef<'_>> {
match key_type {
RelayIdType::Rsa => Some(self.rsa_identity().into()),
RelayIdType::Ed25519 => Some(self.ed_identity().into()),
}
}
}
@ -136,7 +184,7 @@ mod test {
&self.addrs[..]
}
}
impl HasRelayIds for Example {
impl HasRelayIdsLegacy for Example {
fn ed_identity(&self) -> &pk::ed25519::Ed25519Identity {
&self.ed_id
}

124
crates/tor-netdir/src/lib.rs
View File

@ -68,7 +68,7 @@ mod weight;
#[cfg(any(test, feature = "testing"))]
pub mod testnet;
use tor_linkspec::{ChanTarget, HasAddrs, HasRelayIds};
use tor_linkspec::{ChanTarget, HasAddrs, HasRelayIds, RelayIdRef, RelayIdType};
use tor_llcrypto as ll;
use tor_llcrypto::pk::{ed25519::Ed25519Identity, rsa::RsaIdentity};
use tor_netdoc::doc::microdesc::{MdDigest, Microdesc};
@ -205,18 +205,26 @@ impl From<u64> for RelayWeight {
}
}
/// A view of the Tor directory, suitable for use in building
/// circuits.
/// A view of the Tor directory, suitable for use in building circuits.
///
/// Abstractly, a [`NetDir`] is a set of usable public [`Relay`]s,
/// each of which has its own properties, identity, and correct weighted
/// probability for use under different circumstances.
/// Abstractly, a [`NetDir`] is a set of usable public [`Relay`]s, each of which
/// has its own properties, identity, and correct weighted probability for use
/// under different circumstances.
///
/// A [`NetDir`] is constructed by making a [`PartialNetDir`] from a
/// consensus document, and then adding enough microdescriptors to
/// that `PartialNetDir` so that it can be used to build paths.
/// (Thus, if you have a NetDir, it is definitely adequate to build
/// paths.)
/// A [`NetDir`] is constructed by making a [`PartialNetDir`] from a consensus
/// document, and then adding enough microdescriptors to that `PartialNetDir` so
/// that it can be used to build paths. (Thus, if you have a NetDir, it is
/// definitely adequate to build paths.)
///
/// # Limitations
///
/// The current NetDir implementation assumes fairly strongly that every relay
/// has an Ed25519 identity and an RSA identity, that the consensus is indexed
/// by RSA identities, and that the Ed25519 identities are stored in
/// microdescriptors.
///
/// If these assumptions someday change, then we'll have to revise the
/// implementation.
#[derive(Debug, Clone)]
pub struct NetDir {
/// A microdescriptor consensus that lists the members of the network,
@ -630,15 +638,45 @@ impl NetDir {
Some(relay)
}
/// Return a relay matching a given identity, if we have a _usable_ relay
/// with that key.
///
/// (Does not return unusable relays.)
///
pub fn by_relay_id(&self, id: RelayIdRef<'_>) -> Option<Relay<'_>> {
match id {
RelayIdRef::Ed25519(ed25519) => self.by_id(ed25519),
RelayIdRef::Rsa(rsa) => self
.by_rsa_id_unchecked(rsa)
.and_then(UncheckedRelay::into_relay),
other_type => self.relays().find(|r| r.has_identity(other_type)),
}
}
/// Return a relay with the same identities as those in `target`, if one
/// exists.
///
/// Does not return unusable relays.
///
/// # Limitations
///
/// This will be very slow if `target` does not have an Ed25519 identity.
pub fn by_ids<T>(&self, target: &T) -> Option<Relay<'_>>
where
T: HasRelayIds + ?Sized,
{
self.by_id_pair(target.ed_identity(), target.rsa_identity())
if let Some(ed_id) = target.identity(RelayIdType::Ed25519) {
self.by_relay_id(ed_id).filter(|relay| {
// This is like "same_relay_ids", but it does not check the
// Ed25519 identity because that's already been checked.
RelayIdType::all_types().all(|id_type| {
id_type == RelayIdType::Ed25519
|| relay.identity(id_type) == target.identity(id_type)
})
})
} else {
self.relays().find(|r| r.same_relay_ids(target))
}
}
/// Return a relay matching a given Ed25519 identity and RSA identity,
@ -660,15 +698,14 @@ impl NetDir {
self.by_ids(chan_target)
}
/// Return a boolean if this consensus definitely has (or does not
/// have) a relay matching both the given Ed25519 and RSA
/// identity.
/// Return a boolean if this consensus definitely has (or does not have) a
/// relay matching the listed identities.
///
/// If we can't yet tell for sure, return None.
///
/// Once function has returned `Some(b)`, it will always return that
/// value for the same `ed_id` and `rsa_id` on this `NetDir`. A `None`
/// answer may later become `Some(b)` if a microdescriptor arrives.
/// If we can't yet tell for sure, return None. Once function has returned
/// `Some(b)`, it will always return that value for the same `ed_id` and
/// `rsa_id` on this `NetDir`. A `None` answer may later become `Some(b)`
/// if a microdescriptor arrives.
pub fn id_pair_listed(&self, ed_id: &Ed25519Identity, rsa_id: &RsaIdentity) -> Option<bool> {
let r = self.by_rsa_id_unchecked(rsa_id);
match r {
@ -689,11 +726,32 @@ impl NetDir {
/// As `id_pair_listed`, but check whether a relay exists (or may exist)
/// with the same identities as those in `target`.
///
/// # Limitations
///
/// This can be inefficient if the target does not have both an ed25519 and
/// an rsa identity key.
pub fn ids_listed<T>(&self, target: &T) -> Option<bool>
where
T: HasRelayIds + ?Sized,
{
self.id_pair_listed(target.ed_identity(), target.rsa_identity())
let rsa_id = target.rsa_identity();
let ed25519_id = target.ed_identity();
match (rsa_id, ed25519_id) {
(Some(r), Some(e)) => self.id_pair_listed(e, r),
(Some(r), None) => Some(self.rsa_id_is_listed(r)),
(None, Some(e)) => {
if self.rs_idx_by_ed.contains_key(e) {
Some(true)
} else {
None
}
}
// TODO: If we later support more identity key types, this will
// become incorrect.
(None, None) => None,
}
}
/// Return true if we are currently missing a micro descriptor for the
@ -1137,7 +1195,7 @@ impl<'a> HasAddrs for Relay<'a> {
self.rs.addrs()
}
}
impl<'a> HasRelayIds for Relay<'a> {
impl<'a> tor_linkspec::HasRelayIdsLegacy for Relay<'a> {
fn ed_identity(&self) -> &Ed25519Identity {
self.id()
}
@ -1146,6 +1204,18 @@ impl<'a> HasRelayIds for Relay<'a> {
}
}
impl<'a> HasRelayIds for UncheckedRelay<'a> {
fn identity(&self, key_type: RelayIdType) -> Option<RelayIdRef<'_>> {
match key_type {
RelayIdType::Ed25519 if self.rs.ed25519_id_is_usable() => {
self.md.map(|m| m.ed25519_id().into())
}
RelayIdType::Rsa => Some(self.rs.rsa_identity().into()),
_ => None,
}
}
}
impl<'a> ChanTarget for Relay<'a> {}
impl<'a> tor_linkspec::CircTarget for Relay<'a> {
@ -1173,6 +1243,7 @@ mod test {
use std::collections::HashSet;
use std::time::Duration;
use tor_basic_utils::test_rng;
use tor_linkspec::RelayIdType;
// Basic functionality for a partial netdir: Add microdescriptors,
// then you have a netdir.
@ -1351,7 +1422,7 @@ mod test {
r.supports_exit_port_ipv4(80)
});
let r = r.unwrap();
let id_byte = r.rsa_identity().as_bytes()[0];
let id_byte = r.identity(RelayIdType::Rsa).unwrap().as_bytes()[0];
picked[id_byte as usize] += 1;
}
// non-exits should never get picked.
@ -1383,7 +1454,7 @@ mod test {
});
assert_eq!(relays.len(), 4);
for r in relays {
let id_byte = r.rsa_identity().as_bytes()[0];
let id_byte = r.identity(RelayIdType::Rsa).unwrap().as_bytes()[0];
picked[id_byte as usize] += 1;
}
}
@ -1607,8 +1678,11 @@ mod test {
let r = netdir
.by_id_pair(&[14; 32].into(), &[14; 20].into())
.unwrap();
assert_eq!(r.rsa_identity(), &[14; 20].into());
assert_eq!(r.ed_identity(), &[14; 32].into());
assert_eq!(r.identity(RelayIdType::Rsa).unwrap().as_bytes(), &[14; 20]);
assert_eq!(
r.identity(RelayIdType::Ed25519).unwrap().as_bytes(),
&[14; 32]
);
let r = netdir.by_id_pair(&[14; 32].into(), &[99; 20].into());
assert!(r.is_none());

1
crates/tor-proto/semver.md Normal file
View File

@ -0,0 +1 @@
BREAKING: Remove key-specific accessors.

48
crates/tor-proto/src/channel.rs
View File

@ -77,9 +77,7 @@ use std::result::Result as StdResult;
use std::time::Duration;
use tor_cell::chancell::{msg, ChanCell, CircId};
use tor_error::internal;
use tor_linkspec::{ChanTarget, HasRelayIds, OwnedChanTarget};
use tor_llcrypto::pk::ed25519::Ed25519Identity;
use tor_llcrypto::pk::rsa::RsaIdentity;
use tor_linkspec::{HasRelayIds, OwnedChanTarget};
use tor_rtcompat::SleepProvider;
use asynchronous_codec as futures_codec;
@ -306,16 +304,6 @@ impl Channel {
self.details.unique_id
}
/// Return the Ed25519 identity for the peer of this channel.
pub fn peer_ed25519_id(&self) -> &Ed25519Identity {
self.details.peer_id.ed_identity()
}
/// Return the (legacy) RSA identity for the peer of this channel.
pub fn peer_rsa_id(&self) -> &RsaIdentity {
self.details.peer_id.rsa_identity()
}
/// Return an OwnedChanTarget representing the actual handshake used to
/// create this channel.
pub fn target(&self) -> &OwnedChanTarget {
@ -347,23 +335,25 @@ impl Channel {
/// Return an error if this channel is somehow mismatched with the
/// given target.
pub fn check_match<T: ChanTarget + ?Sized>(&self, target: &T) -> Result<()> {
if self.peer_ed25519_id() != target.ed_identity() {
return Err(Error::ChanMismatch(format!(
"Identity {} does not match target {}",
self.peer_ed25519_id(),
target.ed_identity()
)));
pub fn check_match<T: HasRelayIds + ?Sized>(&self, target: &T) -> Result<()> {
for desired in target.identities() {
let id_type = desired.id_type();
match self.details.peer_id.identity(id_type) {
Some(actual) if actual == desired => {}
Some(actual) => {
return Err(Error::ChanMismatch(format!(
"Identity {} does not match target {}",
actual, desired
)));
}
None => {
return Err(Error::ChanMismatch(format!(
"Peer does not have {} identity",
id_type
)))
}
}
}
if self.peer_rsa_id() != target.rsa_identity() {
return Err(Error::ChanMismatch(format!(
"Identity {} does not match target {}",
self.peer_rsa_id(),
target.rsa_identity()
)));
}
Ok(())
}

36
crates/tor-proto/src/channel/handshake.rs
View File

@ -19,7 +19,7 @@ use std::sync::Arc;
use std::time::SystemTime;
use tor_bytes::Reader;
use tor_linkspec::{ChanTarget, OwnedChanTarget};
use tor_linkspec::{ChanTarget, HasRelayIds, OwnedChanTarget, RelayIds};
use tor_llcrypto as ll;
use tor_llcrypto::pk::ed25519::Ed25519Identity;
use tor_llcrypto::pk::rsa::RsaIdentity;
@ -504,14 +504,30 @@ impl<T: AsyncRead + AsyncWrite + Send + Unpin + 'static, S: SleepProvider> Unver
// We do this _last_, since "this is the wrong peer" is
// usually a different situation than "this peer couldn't even
// identify itself right."
if peer.ed_identity() != identity_key {
return Err(Error::HandshakeProto(
"Peer ed25519 id not as expected".into(),
));
}
if *peer.rsa_identity() != rsa_id {
return Err(Error::HandshakeProto("Peer RSA id not as expected".into()));
// TODO: We'll need a different constructor if there are someday
// more/different identity keys.
let actual_identity = RelayIds::new(*identity_key, rsa_id);
// We enforce that the relay proved that it has every ID that we wanted:
// it may also have additional IDs that we didn't ask for.
for desired_id in peer.identities() {
let id_type = desired_id.id_type();
match actual_identity.identity(id_type) {
Some(actual) if actual == desired_id => {}
Some(_) => {
return Err(Error::HandshakeProto(format!(
"Peer {} id not as expected",
id_type
)))
}
None => {
return Err(Error::HandshakeProto(format!(
"Peer did not present a {} id.",
id_type
)))
}
}
}
// If we reach this point, the clock skew might be may now be considered
@ -928,7 +944,7 @@ pub(super) mod test {
assert_eq!(
format!("{}", err),
"Handshake protocol violation: Peer ed25519 id not as expected"
"Handshake protocol violation: Peer Ed25519 id not as expected"
);
let err = certs_test(
@ -944,7 +960,7 @@ pub(super) mod test {
assert_eq!(
format!("{}", err),
"Handshake protocol violation: Peer RSA id not as expected"
"Handshake protocol violation: Peer RSA (legacy) id not as expected"
);
let err = certs_test(

14
crates/tor-proto/src/circuit.rs
View File

@ -62,7 +62,7 @@ use tor_cell::{
};
use tor_error::{bad_api_usage, internal, into_internal};
use tor_linkspec::{CircTarget, LinkSpec, OwnedChanTarget};
use tor_linkspec::{CircTarget, LinkSpec, OwnedChanTarget, RelayIdType};
use futures::channel::{mpsc, oneshot};
@ -235,7 +235,9 @@ impl ClientCirc {
Tg: CircTarget,
{
let key = NtorPublicKey {
id: *target.rsa_identity(),
id: *target
.rsa_identity()
.ok_or(Error::MissingId(RelayIdType::Ed25519))?,
pk: *target.ntor_onion_key(),
};
let mut linkspecs = target.linkspecs();
@ -549,10 +551,14 @@ impl PendingClientCirc {
recv_created: self.recvcreated,
handshake: CircuitHandshake::Ntor {
public_key: NtorPublicKey {
id: *target.rsa_identity(),
id: *target
.rsa_identity()
.ok_or(Error::MissingId(RelayIdType::Rsa))?,
pk: *target.ntor_onion_key(),
},
ed_identity: *target.ed_identity(),
ed_identity: *target
.ed_identity()
.ok_or(Error::MissingId(RelayIdType::Ed25519))?,
},
require_sendme_auth,
params: params.clone(),

19
crates/tor-proto/src/circuit/reactor.rs
View File

@ -35,7 +35,7 @@ use crate::crypto::handshake::ntor::{NtorClient, NtorPublicKey};
use crate::crypto::handshake::{ClientHandshake, KeyGenerator};
use tor_cell::chancell;
use tor_cell::chancell::{ChanCell, CircId};
use tor_linkspec::{LinkSpec, OwnedChanTarget};
use tor_linkspec::{LinkSpec, OwnedChanTarget, RelayIds};
use tor_llcrypto::pk;
use tracing::{debug, trace, warn};
@ -781,21 +781,8 @@ impl Reactor {
params: &CircParameters,
) -> Result<()> {
// Exit now if we have an Ed25519 or RSA identity mismatch.
// FIXME(eta): this is copypasta from Channel::check_match!
if self.channel.peer_rsa_id() != &pubkey.id {
return Err(Error::ChanMismatch(format!(
"Identity {} does not match target {}",
self.channel.peer_rsa_id(),
pubkey.id,
)));
}
if self.channel.peer_ed25519_id() != &ed_identity {
return Err(Error::ChanMismatch(format!(
"Identity {} does not match target {}",
self.channel.peer_ed25519_id(),
ed_identity
)));
}
let target = RelayIds::new(ed_identity, pubkey.id);
self.channel.check_match(&target)?;
let wrap = Create2Wrap {
handshake_type: 0x0002, // ntor

9
crates/tor-proto/src/util/err.rs
View File

@ -3,6 +3,7 @@ use std::{sync::Arc, time::Duration};
use thiserror::Error;
use tor_cell::relaycell::msg::EndReason;
use tor_error::{ErrorKind, HasKind};
use tor_linkspec::RelayIdType;
/// An error type for the tor-proto crate.
///
@ -134,6 +135,10 @@ pub enum Error {
/// Remote DNS lookup failed.
#[error("Remote resolve failed")]
ResolveError(#[source] ResolveError),
/// We tried to do something with a that we couldn't, because of an identity key type
/// that the relay doesn't have.
#[error("Relay has no {0} identity")]
MissingId(RelayIdType),
}
/// Error which indicates that the channel was closed.
@ -222,7 +227,8 @@ impl From<Error> for std::io::Error {
| CellEncodeErr { .. }
| EncodeErr { .. }
| ChanMismatch(_)
| StreamProto(_) => ErrorKind::InvalidData,
| StreamProto(_)
| MissingId(_) => ErrorKind::InvalidData,
Bug(ref e) if e.kind() == tor_error::ErrorKind::BadApiUsage => ErrorKind::InvalidData,
@ -269,6 +275,7 @@ impl HasKind for Error {
E::ResolveError(ResolveError::Nontransient) => EK::RemoteHostNotFound,
E::ResolveError(ResolveError::Transient) => EK::RemoteHostResolutionFailed,
E::ResolveError(ResolveError::Unrecognized) => EK::RemoteHostResolutionFailed,
E::MissingId(_) => EK::BadApiUsage,
E::Bug(e) => e.kind(),
}
}