Refactor FirstHopId into type-differentiated form (6282df34) · Commits · The Tor Project / Core / Arti

crates/tor-circmgr/src/lib.rs

+22 −10

Original line number	Diff line number	Diff line
		@@ -51,6 +51,7 @@
		#![deny(clippy::unwrap_used)]

		use tor_chanmgr::ChanMgr;
		use tor_linkspec::ChanTarget;
		use tor_netdir::{DirEvent, NetDir, NetDirProvider};
		use tor_proto::circuit::{CircParameters, ClientCirc, UniqId};
		use tor_rtcompat::Runtime;
		@@ -695,20 +696,31 @@ impl<R: Runtime> CircMgr<R> {

		/// Record that a failure occurred on a circuit with a given guard, in a way
		/// that makes us unwilling to use that guard for future circuits.
		pub fn note_external_failure(&self, id: &FirstHopId, external_failure: ExternalActivity) {
		self.mgr
		.peek_builder()
		.guardmgr()
		.note_external_failure(id, external_failure);
		///
		pub fn note_external_failure(
		&self,
		target: &impl ChanTarget,
		external_failure: ExternalActivity,
		) {
		self.mgr.peek_builder().guardmgr().note_external_failure(
		target.ed_identity(),
		target.rsa_identity(),
		external_failure,
		);
		}

		/// Record that a success occurred on a circuit with a given guard, in a way
		/// that makes us possibly willing to use that guard for future circuits.
		pub fn note_external_success(&self, id: &FirstHopId, external_activity: ExternalActivity) {
		self.mgr
		.peek_builder()
		.guardmgr()
		.note_external_success(id, external_activity);
		pub fn note_external_success(
		&self,
		target: &impl ChanTarget,
		external_activity: ExternalActivity,
		) {
		self.mgr.peek_builder().guardmgr().note_external_success(
		target.ed_identity(),
		target.rsa_identity(),
		external_activity,
		);
		}
		}

crates/tor-dirmgr/src/lib.rs

+4 −6

Original line number	Diff line number	Diff line
		@@ -981,7 +981,7 @@ impl<R: Runtime> DirMgr<R> {

		/// Record that a problem has occurred because of a failure in an answer from `source`.
		fn note_cache_error(&self, source: &tor_dirclient::SourceInfo, problem: &Error) {
		use tor_circmgr::{ExternalActivity, FirstHopId};
		use tor_circmgr::ExternalActivity;

		if !problem.indicates_cache_failure() {
		return;
		@@ -989,20 +989,18 @@ impl<R: Runtime> DirMgr<R> {

		if let Some(circmgr) = &self.circmgr {
		info!("Marking {:?} as failed: {}", source, problem);
		let guard_id = FirstHopId::from_chan_target(source.cache_id());
		circmgr.note_external_failure(&guard_id, ExternalActivity::DirCache);
		circmgr.note_external_failure(source.cache_id(), ExternalActivity::DirCache);
		circmgr.retire_circ(source.unique_circ_id());
		}
		}

		/// Record that `source` has successfully given us some directory info.
		fn note_cache_success(&self, source: &tor_dirclient::SourceInfo) {
		use tor_circmgr::{ExternalActivity, FirstHopId};
		use tor_circmgr::ExternalActivity;

		if let Some(circmgr) = &self.circmgr {
		trace!("Marking {:?} as successful", source);
		let guard_id = FirstHopId::from_chan_target(source.cache_id());
		circmgr.note_external_success(&guard_id, ExternalActivity::DirCache);
		circmgr.note_external_success(source.cache_id(), ExternalActivity::DirCache);
		}
		}
		}

crates/tor-guardmgr/src/fallback.rs

+2 −1

Original line number	Diff line number	Diff line
		@@ -13,6 +13,7 @@
		mod set;
		mod status;

		use crate::ids::FallbackId;
		use derive_builder::Builder;
		use tor_config::ConfigBuildError;
		use tor_llcrypto::pk::ed25519::Ed25519Identity;
		@@ -53,7 +54,7 @@ impl FallbackDir {
		/// Return a copy of this FallbackDir as a [`Guard`](crate::Guard)
		pub fn as_guard(&self) -> crate::FirstHop {
		crate::FirstHop {
		id: crate::FirstHopId::from_chan_target(self),
		id: FallbackId::from_chan_target(self).into(),
		orports: self.orports.clone(),
		}
		}

crates/tor-guardmgr/src/fallback/set.rs

+36 −11

Original line number	Diff line number	Diff line
		@@ -4,7 +4,7 @@ use rand::seq::IteratorRandom;
		use std::time::Instant;

		use super::{FallbackDir, Status};
		use crate::{FirstHopId, PickGuardError};
		use crate::{ids::FallbackId, PickGuardError};
		use serde::Deserialize;

		/// A list of fallback directories.
		@@ -62,6 +62,9 @@ pub(crate) struct FallbackState {
		#[derive(Debug, Clone)]
		pub(super) struct Entry {
		/// The inner fallback directory.
		///
		/// (TODO: We represent this as a `FirstHop`, which could technically hold a
		/// guard as well. Ought to fix that.)
		pub(super) fallback: crate::FirstHop,
		/// The status for the fallback directory.
		pub(super) status: Status,
		@@ -77,8 +80,12 @@ impl From<FallbackDir> for Entry {

		impl Entry {
		/// Return the identity for this fallback entry.
		fn id(&self) -> &FirstHopId {
		self.fallback.id()
		fn id(&self) -> &FallbackId {
		use crate::ids::FirstHopIdInner::*;
		match &self.fallback.id().0 {
		Fallback(id) => id,
		_ => panic!("Somehow we constructed a fallback object with a non-fallback id!"),
		}
		}
		}

		@@ -122,20 +129,33 @@ impl FallbackState {
		.min()
		}

		/// Return a reference to the entry whose identity is `id`, if there is one.
		fn get(&self, id: &FallbackId) -> Option<&Entry> {
		match self.fallbacks.binary_search_by(\|e\| e.id().cmp(id)) {
		Ok(idx) => Some(&self.fallbacks[idx]),
		Err(_) => None,
		}
		}

		/// Return a mutable reference to the entry whose identity is `id`, if there is one.
		fn get_mut(&mut self, id: &FirstHopId) -> Option<&mut Entry> {
		fn get_mut(&mut self, id: &FallbackId) -> Option<&mut Entry> {
		match self.fallbacks.binary_search_by(\|e\| e.id().cmp(id)) {
		Ok(idx) => Some(&mut self.fallbacks[idx]),
		Err(_) => None,
		}
		}

		/// Return true if this set contains some entry with the given `id`.
		pub(crate) fn contains(&self, id: &FallbackId) -> bool {
		self.get(id).is_some()
		}

		/// Record that a success has occurred for the fallback with the given
		/// identity.
		///
		/// Be aware that for fallbacks, we only count a successful directory
		/// operation as a success: a circuit success is not enough.
		pub(crate) fn note_success(&mut self, id: &FirstHopId) {
		pub(crate) fn note_success(&mut self, id: &FallbackId) {
		if let Some(entry) = self.get_mut(id) {
		entry.status.note_success();
		}
		@@ -143,7 +163,7 @@ impl FallbackState {

		/// Record that a failure has occurred for the fallback with the given
		/// identity.
		pub(crate) fn note_failure(&mut self, id: &FirstHopId, now: Instant) {
		pub(crate) fn note_failure(&mut self, id: &FallbackId, now: Instant) {
		if let Some(entry) = self.get_mut(id) {
		entry.status.note_failure(now);
		}
		@@ -171,6 +191,7 @@ impl FallbackState {
		mod test {
		#![allow(clippy::unwrap_used)]
		use super::*;
		use crate::FirstHopId;

		/// Construct a `FallbackDir` with random identity keys and addresses.
		///
		@@ -197,7 +218,7 @@ mod test {
		// fabricate some fallbacks.
		let fbs = vec![rand_fb(), rand_fb(), rand_fb(), rand_fb()];
		let fb_other = rand_fb();
		let id_other = FirstHopId::from_chan_target(&fb_other);
		let id_other = FallbackId::from_chan_target(&fb_other);

		// basic case: construct a set
		let list: FallbackList = fbs.clone().into();
		@@ -213,7 +234,7 @@ mod test {

		// use the constructed set a little.
		for fb in fbs.iter() {
		let id = FirstHopId::from_chan_target(fb);
		let id = FallbackId::from_chan_target(fb);
		assert_eq!(set.get_mut(&id).unwrap().id(), &id);
		}
		assert!(set.get_mut(&id_other).is_none());
		@@ -247,7 +268,11 @@ mod test {
		let now = Instant::now();

		fn lookup_idx(set: &FallbackState, id: &FirstHopId) -> Option<usize> {
		if let FirstHopId(crate::ids::FirstHopIdInner::Fallback(id)) = id {
		set.fallbacks.binary_search_by(\|ent\| ent.id().cmp(id)).ok()
		} else {
		None
		}
		}
		// Basic case: everybody is up.
		for _ in 0..100 {
		@@ -333,7 +358,7 @@ mod test {
		let mut fbs2: Vec<_> = fbs
		.into_iter()
		// (Remove the fallback with id==ids[2])
		.filter(\|fb\| FirstHopId::from_chan_target(fb) != ids[2])
		.filter(\|fb\| FallbackId::from_chan_target(fb) != ids[2])
		.collect();
		// add 2 new ones.
		let fbs_new = [rand_fb(), rand_fb(), rand_fb()];
		@@ -352,7 +377,7 @@ mod test {
		// Make sure that the new fbs are there.
		for new_fb in fbs_new {
		assert!(set2
		.get_mut(&FirstHopId::from_chan_target(&new_fb))
		.get_mut(&FallbackId::from_chan_target(&new_fb))
		.unwrap()
		.status
		.usable_at(now));

crates/tor-guardmgr/src/guard.rs

+29 −35

Original line number	Diff line number	Diff line
		@@ -13,7 +13,8 @@ use std::time::{Duration, Instant, SystemTime};
		use tracing::{trace, warn};

		use crate::util::randomize_time;
		use crate::{FirstHopId, GuardParams, GuardRestriction, GuardUsage};
		use crate::FirstHopId;
		use crate::{ids::GuardId, GuardParams, GuardRestriction, GuardUsage};
		use tor_persist::{Futureproof, JsonValue};

		/// Tri-state to represent whether a guard is believed to be reachable or not.
		@@ -83,7 +84,7 @@ impl CrateId {
		#[derive(Clone, Debug, Serialize, Deserialize)]
		pub(crate) struct Guard {
		/// The identity keys for this guard.
		id: FirstHopId, // TODO: Maybe refactor this out as redundant someday.
		id: GuardId, // TODO: Maybe refactor this out as redundant someday.

		/// The most recently seen addresses for making OR connections to this
		/// guard.
		@@ -195,14 +196,14 @@ impl Guard {
		);

		Self::new(
		FirstHopId::from_chan_target(relay),
		GuardId::from_chan_target(relay),
		relay.addrs().into(),
		added_at,
		)
		}

		/// Return a new, manually constructed [`Guard`].
		fn new(id: FirstHopId, orports: Vec<SocketAddr>, added_at: SystemTime) -> Self {
		fn new(id: GuardId, orports: Vec<SocketAddr>, added_at: SystemTime) -> Self {
		Guard {
		id,
		orports,
		@@ -225,7 +226,7 @@ impl Guard {
		}

		/// Return the identity of this Guard.
		pub(crate) fn guard_id(&self) -> &FirstHopId {
		pub(crate) fn guard_id(&self) -> &GuardId {
		&self.id
		}

		@@ -321,8 +322,8 @@ impl Guard {
		/// Return true if this guard obeys a single restriction.
		fn obeys_restriction(&self, r: &GuardRestriction) -> bool {
		match r {
		GuardRestriction::AvoidId(ed) => &self.id.ed25519 != ed,
		GuardRestriction::AvoidAllIds(ids) => !ids.contains(&self.id.ed25519),
		GuardRestriction::AvoidId(ed) => &self.id.0.ed25519 != ed,
		GuardRestriction::AvoidAllIds(ids) => !ids.contains(&self.id.0.ed25519),
		}
		}

		@@ -353,7 +354,7 @@ impl Guard {
		/// download another microdescriptor before we can be certain whether this
		/// guard is listed or not.
		pub(crate) fn listed_in(&self, netdir: &NetDir) -> Option<bool> {
		netdir.id_pair_listed(&self.id.ed25519, &self.id.rsa)
		netdir.id_pair_listed(&self.id.0.ed25519, &self.id.0.rsa)
		}

		/// Change this guard's status based on a newly received or newly
		@@ -370,11 +371,9 @@ impl Guard {
		// not.
		let listed_as_guard = match self.listed_in(netdir) {
		Some(true) => {
		let id: FirstHopId = self.id.clone().into();
		// Definitely listed.
		let relay = self
		.id
		.get_relay(netdir)
		.expect("Couldn't get a listed relay?!");
		let relay = id.get_relay(netdir).expect("Couldn't get a listed relay?!");
		// Update address information.
		self.orports = relay.addrs().into();
		// Check whether we can currently use it as a directory cache.
		@@ -570,13 +569,13 @@ 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.rsa, tor_netdir::WeightRole::Guard)
		dir.weight_by_rsa_id(&self.id.0.rsa, tor_netdir::WeightRole::Guard)
		}

		/// Return a [`crate::Guard`] object to represent this guard.
		pub(crate) fn get_external_rep(&self) -> crate::FirstHop {
		crate::FirstHop {
		id: self.id.clone(),
		id: self.id.clone().into(),
		orports: self.orports.clone(),
		}
		}
		@@ -587,10 +586,10 @@ impl tor_linkspec::ChanTarget for Guard {
		&self.orports[..]
		}
		fn ed_identity(&self) -> &Ed25519Identity {
		&self.id.ed25519
		&self.id.0.ed25519
		}
		fn rsa_identity(&self) -> &RsaIdentity {
		&self.id.rsa
		&self.id.0.rsa
		}
		}

		@@ -687,8 +686,8 @@ mod test {
		assert_eq!(Some(id.version.as_ref()), option_env!("CARGO_PKG_VERSION"));
		}

		fn basic_id() -> FirstHopId {
		FirstHopId::new([13; 32].into(), [37; 20].into())
		fn basic_id() -> GuardId {
		GuardId::new([13; 32].into(), [37; 20].into())
		}
		fn basic_guard() -> Guard {
		let id = basic_id();
		@@ -703,8 +702,8 @@ mod test {
		let g = basic_guard();

		assert_eq!(g.guard_id(), &id);
		assert_eq!(g.ed_identity(), &id.ed25519);
		assert_eq!(g.rsa_identity(), &id.rsa);
		assert_eq!(g.ed_identity(), &id.0.ed25519);
		assert_eq!(g.rsa_identity(), &id.0.rsa);
		assert_eq!(g.addrs(), &["127.0.0.7:7777".parse().unwrap()]);
		assert_eq!(g.reachable(), Reachable::Unknown);
		assert_eq!(g.reachable(), Reachable::default());
		@@ -910,7 +909,8 @@ mod test {
		assert!(Some(guard22.added_at) <= Some(now));

		// Can we still get the relay back?
		let r = guard22.id.get_relay(&netdir).unwrap();
		let id: FirstHopId = guard22.id.clone().into();
		let r = id.get_relay(&netdir).unwrap();
		assert_eq!(r.ed_identity(), relay22.ed_identity());

		// Can we check on the guard's weight?
		@@ -919,11 +919,12 @@ mod test {

		// Now try a guard that isn't in the netdir.
		let guard255 = Guard::new(
		FirstHopId::new([255; 32].into(), [255; 20].into()),
		GuardId::new([255; 32].into(), [255; 20].into()),
		vec![],
		now,
		);
		assert!(guard255.id.get_relay(&netdir).is_none());
		let id: FirstHopId = guard255.id.clone().into();
		assert!(id.get_relay(&netdir).is_none());
		assert!(guard255.get_weight(&netdir).is_none());
		}

		@@ -960,7 +961,7 @@ mod test {

		// Try a guard that isn't in the netdir at all.
		let mut guard255 = Guard::new(
		FirstHopId::new([255; 32].into(), [255; 20].into()),
		GuardId::new([255; 32].into(), [255; 20].into()),
		vec!["8.8.8.8:53".parse().unwrap()],
		now,
		);
		@@ -974,12 +975,9 @@ mod test {
		assert!(!guard255.orports.is_empty());

		// Try a guard that is in netdir, but not netdir2.
		let mut guard22 = Guard::new(
		FirstHopId::new([22; 32].into(), [22; 20].into()),
		vec![],
		now,
		);
		let relay22 = guard22.id.get_relay(&netdir).unwrap();
		let mut guard22 = Guard::new(GuardId::new([22; 32].into(), [22; 20].into()), vec![], now);
		let id22: FirstHopId = guard22.id.clone().into();
		let relay22 = id22.get_relay(&netdir).unwrap();
		assert_eq!(guard22.listed_in(&netdir), Some(true));
		guard22.update_from_netdir(&netdir);
		assert_eq!(guard22.unlisted_since, None); // It's listed.
		@@ -994,11 +992,7 @@ mod test {
		assert!(!guard22.microdescriptor_missing);

		// Now see what happens for a guard that's in the consensus, but missing an MD.
		let mut guard23 = Guard::new(
		FirstHopId::new([23; 32].into(), [23; 20].into()),
		vec![],
		now,
		);
		let mut guard23 = Guard::new(GuardId::new([23; 32].into(), [23; 20].into()), vec![], now);
		assert_eq!(guard23.listed_in(&netdir2), Some(true));
		assert_eq!(guard23.listed_in(&netdir3), None);
		guard23.update_from_netdir(&netdir3);