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

Merge branch 'guards_as_bridges_part2' into 'main' 

Use ByRelayIds to hold guards in GuardSet

See merge request tpo/core/arti!808
This commit is contained in:
Nick Mathewson 2022-10-27 21:09:47 +00:00
parent fdbd213a6d 557a07677e
commit 29c46d08b5
6 changed files with 373 additions and 141 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -15,7 +15,7 @@ use crate::skew::SkewObservation;
use crate::util::randomize_time;
use crate::{ids::GuardId, GuardParams, GuardRestriction, GuardUsage};
use crate::{ExternalActivity, GuardSetSelector, GuardUsageKind};
use tor_linkspec::{HasAddrs, HasRelayIds};
use tor_linkspec::{HasAddrs, HasRelayIds, RelayIds};
use tor_persist::{Futureproof, JsonValue};
/// Tri-state to represent whether a guard is believed to be reachable or not.
@ -68,24 +68,28 @@ impl CrateId {
/// A single guard node, as held by the guard manager.
///
/// A Guard is a Tor relay that clients use for the first hop of their
/// circuits. It doesn't need to be a relay that's currently on the
/// network (that is, one that we could represent as a [`Relay`]):
/// guards might be temporarily unlisted.
/// A Guard is a Tor relay that clients use for the first hop of their circuits.
/// It doesn't need to be a relay that's currently on the network (that is, one
/// that we could represent as a [`Relay`]): guards might be temporarily
/// unlisted.
///
/// Some fields in guards are persistent; others are reset with every
/// process.
/// Some fields in guards are persistent; others are reset with every process.
///
/// # Identity
///
/// Every guard has at least one `RelayId`. A guard may _gain_ identities over
/// time, as we learn more about it, but it should never _lose_ or _change_ its
/// identities of a given type.
///
/// # TODO
///
/// This structure uses [`Instant`] to represent non-persistent points
/// in time, and [`SystemTime`] to represent points in time that need
/// to be persistent. That's possibly undesirable; maybe we should
/// come up with a better solution.
/// This structure uses [`Instant`] to represent non-persistent points in time,
/// and [`SystemTime`] to represent points in time that need to be persistent.
/// That's possibly undesirable; maybe we should come up with a better solution.
#[derive(Clone, Debug, Serialize, Deserialize)]
pub(crate) struct Guard {
/// The identity keys for this guard.
id: GuardId, // TODO: Maybe refactor this out as redundant someday.
id: GuardId,
/// The most recently seen addresses for making OR connections to this
/// guard.
@ -298,12 +302,27 @@ impl Guard {
/// Copy all _non-persistent_ status from `other` to self.
///
/// Requires that the two `Guard`s have the same ID.
pub(crate) fn copy_status_from(self, other: Guard) -> Guard {
debug_assert_eq!(self.id, other.id);
/// We do this when we were not the owner of our persistent state, and we
/// have just reloaded it (as `self`), but we have some ephemeral knowledge
/// about this guard (as `other`).
///
/// You should not invent new uses for this function; instead we should come
/// up with alternatives.
///
/// # Panics
///
/// Panics if the identities in `self` are not exactly the same as the
/// identities in `other`.
pub(crate) fn copy_ephemeral_status_into_newly_loaded_state(self, other: Guard) -> Guard {
// It is not safe to copy failure information unless these identities
// are a superset of those in `other`; but it is not safe to copy success
// information unless these identities are a subset of those in `other`.
//
// To simplify matters, we just insist that the identities have to be the same.
assert!(self.same_relay_ids(&other));
Guard {
// All persistent fields are taken from `self`.
// All other persistent fields are taken from `self`.
id: self.id,
orports: self.orports,
added_at: self.added_at,
@ -428,14 +447,17 @@ impl Guard {
netdir.ids_listed(&self.id.0)
}
/// Change this guard's status based on a newly received or newly
/// updated [`NetDir`].
/// Change this guard's status based on a newly received or newly updated
/// [`NetDir`].
///
/// A guard may become "listed" or "unlisted": a listed guard is
/// one that appears in the consensus with the Guard flag.
/// A guard may become "listed" or "unlisted": a listed guard is one that
/// appears in the consensus with the Guard flag.
///
/// Additionally, a guard's orports may change, if the directory
/// lists a new address for the relay.
/// A guard may acquire additional identities if we learned them from the
/// netdir.
///
/// Additionally, a guard's orports may change, if the directory lists a new
/// address for the relay.
pub(crate) fn update_from_netdir(&mut self, netdir: &NetDir) {
// This is a tricky check, since if we're missing a microdescriptor
// for the RSA id, we won't know whether the ed25519 id is listed or
@ -450,6 +472,9 @@ impl Guard {
self.orports = relay.addrs().into();
// Check whether we can currently use it as a directory cache.
self.is_dir_cache = relay.is_dir_cache();
// Update our IDs: the Relay will have strictly more.
assert!(relay.has_all_relay_ids_from(self));
self.id = GuardId(RelayIds::from_relay_ids(&relay));
relay.is_flagged_guard()
}

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

@ -64,6 +64,15 @@ impl GuardId {
}
}
impl HasRelayIds for GuardId {
fn identity(
&self,
key_type: tor_linkspec::RelayIdType,
) -> Option<tor_linkspec::RelayIdRef<'_>> {
self.0.identity(key_type)
}
}
/// Implementation type held inside of FirstHopId.
///
/// This exists as a separate type from FirstHopId because Rust requires that a pub enum's variants are all public.
@ -79,11 +88,6 @@ pub(crate) enum FirstHopIdInner {
/// directory.
///
/// (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);

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

@ -641,7 +641,9 @@ impl GuardSets {
use strum::IntoEnumIterator;
for sample in GuardSetSelector::iter() {
self.guards_mut(&sample)
.copy_status_from(std::mem::take(other.guards_mut(&sample)));
.copy_ephemeral_status_into_newly_loaded_state(std::mem::take(
other.guards_mut(&sample),
));
}
}
}
@ -1055,7 +1057,11 @@ impl GuardMgrInner {
}
/// Return every currently extant FirstHopId for a guard or fallback
/// directory matching the provided keys.
/// directory matching (or possibly matching) the provided keys.
///
/// An identity is _possibly matching_ if it contains some of the IDs in the
/// provided identity, and it has no _contradictory_ identities, but it does
/// not necessarily contain _all_ of those identities.
///
/// # TODO
///
@ -1064,6 +1070,10 @@ 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.
///
/// TODO relay: we will have to make the change above when we implement
/// relays; otherwise, it would be possible for an attacker to exploit it to
/// mislead us about our guard status.
fn lookup_ids<T>(&self, identity: &T) -> Vec<FirstHopId>
where
T: tor_linkspec::HasRelayIds + ?Sized,
@ -1073,9 +1083,12 @@ impl GuardMgrInner {
let id = ids::GuardId::from_relay_ids(identity);
for sample in GuardSetSelector::iter() {
if self.guards.guards(&sample).contains(&id) {
vec.push(FirstHopId(FirstHopIdInner::Guard(sample, id.clone())));
}
let guard_id = match self.guards.guards(&sample).contains(&id) {
Ok(true) => &id,
Err(other) => other,
Ok(false) => continue,
};
vec.push(FirstHopId(FirstHopIdInner::Guard(sample, guard_id.clone())));
}
let id = ids::FallbackId::from_relay_ids(identity);

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

@ -13,6 +13,7 @@ use crate::{
};
use crate::{FirstHop, GuardSetSelector};
use tor_basic_utils::iter::{FilterCount, IteratorExt as _};
use tor_linkspec::{ByRelayIds, HasRelayIds};
use tor_netdir::{NetDir, Relay};
use itertools::Itertools;
@ -47,21 +48,78 @@ use tracing::{debug, info};
#[serde(from = "GuardSample")]
pub(crate) struct GuardSet {
/// Map from identities to guards, for every guard in this sample.
guards: HashMap<GuardId, Guard>,
///
/// The key for each entry is a set of identities which we have
/// good (trustworthy-enough) reason to link together.
///
/// When we connect to a guard we require it to demonstrate
/// that it has *all* of these identities;
/// and we do pinning, so that we note down the other identities we discover it has,
/// with the intent that we will require them in future.
///
/// ### Sources of linkage:
///
/// * If we connect to a relay and it proves a set of identities,
/// that necessarily will include at least the ones we have already.
/// We can add any other identities we have discovered.
/// Justification: the owners of the old ids have made a statement
/// (via the connection protocols) that these other ids are also theirs,
/// and should be required in future.
///
/// * If we obtain a (full) descriptor for a relay, and check the
/// self-signatures by all the identities we have already,
/// we can add any other identities listed in the descriptor.
/// Justification: the owners of the old ids have made an explicit statement
/// that these other ids are also theirs,
/// and should be required in future.
///
/// * For a relay in the netdir, if the netdir links some ids together,
/// we can combine the entries.
/// Justification: the netdir is authoritative for netdir-based relays.
///
/// * For a configured bridge, if our configuration links some identities,
/// we must insist on all those identities.
/// So we combine them.
///
/// ### Handling of conflicting entries:
///
/// `ByRelayIds` will implicitly delete conflicting entries,
/// simply forgetting about them.
/// This is OK for netdir relays, since we do not expect this to occur in practice.
///
/// For bridges, conflicts may in fact occur,
/// since bridge lines are not issued by a single authority,
/// and should be afforded limited trust.
///
/// * If the configuration contains bridge lines that mutually conflict,
/// affected bridge lines should be disregarded,
/// or the configuration rejected.
///
/// * If the configuration contains information which is inconsistent with
/// our past experience, we should discard the past experiences which
/// aren't reconcilable with the configuration.
///
/// * We may discover a linkage which demonstrates that the configuration
/// is wrong: for example, two bridge lines for identities X and Y,
/// but in fact there is only one bridge with both identities.
/// In this situation it is OK to effectively disregard some the configuration
/// entries which are at variance with reality, maybe with a warning,
/// but keeping at least one of every usable id set (actually existing bridge)
/// would be good.
guards: ByRelayIds<Guard>,
/// Identities of all the guards in the sample, in sample order.
///
/// This contains the same elements as `self.guards.keys()`, and
/// only exists to define an ordering on the guards.
/// This contains the same elements as the keys of `guards`
sample: Vec<GuardId>,
/// Identities of all the confirmed guards in the sample, in
/// confirmed order.
///
/// This contains a subset of the values in `self.guards.keys()`.
/// This contains a subset of the values in `sample`.
confirmed: Vec<GuardId>,
/// Identities of all the primary guards, in preference order
/// (from best to worst).
///
/// This contains a subset of the values in `self.guards.keys()`.
/// This contains a subset of the values in `sample`.
primary: Vec<GuardId>,
/// Currently active filter that restricts which guards we can use.
///
@ -126,21 +184,34 @@ impl GuardSet {
)
}
/// Remove all elements from this `GuardSet` that ought to be
/// referenced by another element, but which are not.
/// Remove all elements from this `GuardSet` that ought to be referenced by
/// another element, but which are not.
///
/// This method only removes corrupted elements; it doesn't add or
/// fix anything. It won't do anything if the `GuardSet` is
/// well-formed.
/// This method only removes corrupted elements and updates IDs in the ID
/// list (possibly adding new IDs); it doesn't add guards or other data.
/// It won't do anything if the `GuardSet` is well-formed.
fn fix_consistency(&mut self) {
/// Remove every element of `id_list` that does not belong to some guard
/// in `guards`, and update the others to have any extra identities
/// listed in `guards`.
fn fix_id_list(guards: &ByRelayIds<Guard>, id_list: &mut Vec<GuardId>) {
// TODO: Use Vec::retain_mut when our MSRV >= 1.61
#![allow(deprecated)]
use retain_mut::RetainMut;
RetainMut::retain_mut(id_list, |id| match guards.by_all_ids(id) {
Some(guard) => {
*id = guard.guard_id().clone();
true
}
None => false,
});
}
let sample_set: HashSet<_> = self.sample.iter().collect();
self.guards
.retain(|id, g| g.guard_id() == id && sample_set.contains(id));
let guards = &self.guards; // avoid borrow issues
// TODO: We should potentially de-duplicate these.
self.sample.retain(|id| guards.contains_key(id));
self.confirmed.retain(|id| guards.contains_key(id));
self.primary.retain(|id| guards.contains_key(id));
self.guards.retain(|g| sample_set.contains(g.guard_id()));
fix_id_list(&self.guards, &mut self.sample);
fix_id_list(&self.guards, &mut self.confirmed);
fix_id_list(&self.guards, &mut self.primary);
}
/// Assert that this `GuardSet` is internally consistent.
@ -153,9 +224,9 @@ impl GuardSet {
assert_eq!(len_pre, len_post);
}
/// Return the guard whose id is `id`, if any.
/// Return the guard that has every identity in `id`, if any.
pub(crate) fn get(&self, id: &GuardId) -> Option<&Guard> {
self.guards.get(id)
self.guards.by_all_ids(id)
}
/// Replace the filter used by this `GuardSet` with `filter`.
@ -173,7 +244,7 @@ impl GuardSet {
let filt = &self.active_filter;
self.primary.retain(|id| {
guards
.get(id)
.by_all_ids(id)
.map(|g| g.usable() && filt.permits(g))
.unwrap_or(false)
});
@ -187,16 +258,23 @@ impl GuardSet {
}
/// Copy non-persistent status from every guard shared with `other`.
pub(crate) fn copy_status_from(&mut self, mut other: GuardSet) {
let mut old_guards = HashMap::new();
std::mem::swap(&mut old_guards, &mut self.guards);
///
/// This is used as part of our reload process when we don't own our state
/// files, and we're reloading in order to find out what the other Arti
/// instance thinks the guards are. At that point, `self` is the set of
/// guards that we just loaded from state, and `other` is our old guards,
/// which we are using only for their status information.
pub(crate) fn copy_ephemeral_status_into_newly_loaded_state(&mut self, mut other: GuardSet) {
let old_guards = std::mem::take(&mut self.guards);
self.guards = old_guards
.into_iter()
.map(|(id, guard)| {
if let Some(other_guard) = other.guards.remove(&id) {
(id, guard.copy_status_from(other_guard))
.into_values()
.map(|guard| {
let id = guard.guard_id();
if let Some(other_guard) = other.guards.remove_exact(id) {
guard.copy_ephemeral_status_into_newly_loaded_state(other_guard)
} else {
(id, guard)
guard
}
})
.collect();
@ -208,7 +286,7 @@ impl GuardSet {
let guards = self
.sample
.iter()
.map(|id| Cow::Borrowed(self.guards.get(id).expect("Inconsistent state")))
.map(|id| Cow::Borrowed(self.guards.by_all_ids(id).expect("Inconsistent state")))
.collect();
GuardSample {
@ -220,11 +298,11 @@ impl GuardSet {
/// Reconstruct a guard state from its serialized representation.
fn from_state(state: GuardSample<'_>) -> Self {
let mut guards = HashMap::new();
let mut guards = ByRelayIds::new();
let mut sample = Vec::new();
for guard in state.guards {
sample.push(guard.guard_id().clone());
guards.insert(guard.guard_id().clone(), guard.into_owned());
guards.insert(guard.into_owned());
}
let confirmed = state.confirmed.into_owned();
let primary = Vec::new();
@ -258,17 +336,30 @@ impl GuardSet {
guard_set
}
/// Return true if `relay` is a member of this set.
fn contains_relay(&self, relay: &Relay<'_>) -> bool {
// Note: Could implement Borrow instead, but I don't think it'll
// matter.
let id = GuardId::from_relay_ids(relay);
self.contains(&id)
/// Return false if `relay` (or some other relay that shares an ID with it)
/// is a member if this set.
fn can_add_relay(&self, relay: &Relay<'_>) -> bool {
self.guards.all_overlapping(relay).is_empty()
}
/// Return true if `id` is a member of this set.
pub(crate) fn contains(&self, id: &GuardId) -> bool {
self.guards.contains_key(id)
/// Return `Ok(true)` if `id` is definitely a member of this set, and
/// `Ok(false)` if it is definitely not a member.
///
/// If we cannot tell, it's because there is a guard in this sample that has
/// a _subset_ of the IDs in `id`. In that case, we return
/// `Err(guard_ident)`, where `guard_ident` is the identity of that guard.
pub(crate) fn contains(&self, id: &GuardId) -> Result<bool, &GuardId> {
let overlapping = self.guards.all_overlapping(id);
match &overlapping[..] {
[singleton] => {
if singleton.has_all_relay_ids_from(id) {
Ok(true)
} else {
Err(singleton.guard_id())
}
}
_ => Ok(false),
}
}
/// If there are not enough filter-permitted usable guards in this
@ -367,7 +458,7 @@ impl GuardSet {
filter_ok
&& relay.is_flagged_guard()
&& relay.is_dir_cache()
&& !self.contains_relay(relay)
&& self.can_add_relay(relay)
},
);
@ -409,12 +500,12 @@ 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_relay_ids(relay);
if self.guards.contains_key(&id) {
if self.guards.by_all_ids(&id).is_some() {
return;
}
debug!(guard_id=?id, "Adding guard to sample.");
let guard = Guard::from_relay(relay, now, params);
self.guards.insert(id.clone(), guard);
self.guards.insert(guard);
self.sample.push(id);
self.primary_guards_invalidated = true;
}
@ -425,7 +516,10 @@ impl GuardSet {
self.primary
.iter()
.filter(|id| {
let g = self.guards.get(id).expect("Inconsistent guard state");
let g = self
.guards
.by_all_ids(*id)
.expect("Inconsistent guard state");
g.listed_in(dir).is_none()
})
.count()
@ -434,9 +528,16 @@ impl GuardSet {
/// Update the status of every guard in this sample from a network
/// directory.
pub(crate) fn update_status_from_netdir(&mut self, dir: &NetDir) {
for g in self.guards.values_mut() {
g.update_from_netdir(dir);
}
let old_guards = std::mem::take(&mut self.guards);
self.guards = old_guards
.into_values()
.map(|mut guard| {
guard.update_from_netdir(dir);
guard
})
.collect();
// Call "fix consistency", in case any guards got a new ID.
self.fix_consistency();
}
/// Re-build the list of primary guards.
@ -469,7 +570,10 @@ impl GuardSet {
.unique()
// We only consider usable guards that the filter allows.
.filter_map(|id| {
let g = self.guards.get(id).expect("Inconsistent guard state");
let g = self
.guards
.by_all_ids(id)
.expect("Inconsistent guard state");
if g.usable() && self.active_filter.permits(g) {
Some(id.clone())
} else {
@ -486,9 +590,9 @@ impl GuardSet {
// Clear exploratory_circ_pending for all primary guards.
for id in &self.primary {
if let Some(guard) = self.guards.get_mut(id) {
self.guards.modify_by_all_ids(id, |guard| {
guard.note_exploratory_circ(false);
}
});
}
// TODO: Recalculate retry times, perhaps, since we may have changed
@ -503,11 +607,11 @@ impl GuardSet {
pub(crate) fn expire_old_guards(&mut self, params: &GuardParams, now: SystemTime) {
self.assert_consistency();
let n_pre = self.guards.len();
self.guards.retain(|_, g| !g.is_expired(params, now));
let guards = &self.guards; // to avoid borrowing issue
self.sample.retain(|id| guards.contains_key(id));
self.confirmed.retain(|id| guards.contains_key(id));
self.primary.retain(|id| guards.contains_key(id));
self.guards.retain(|g| !g.is_expired(params, now));
let guards = &self.guards;
self.sample.retain(|id| guards.by_all_ids(id).is_some());
self.confirmed.retain(|id| guards.by_all_ids(id).is_some());
self.primary.retain(|id| guards.by_all_ids(id).is_some());
self.assert_consistency();
if self.guards.len() < n_pre {
@ -521,7 +625,10 @@ impl GuardSet {
/// is not known to be Unreachable.
fn reachable_sample_ids(&self) -> impl Iterator<Item = &GuardId> {
self.sample.iter().filter(move |id| {
let g = self.guards.get(id).expect("Inconsistent guard state");
let g = self
.guards
.by_all_ids(*id)
.expect("Inconsistent guard state");
g.reachable() != Reachable::Unreachable
})
}
@ -547,21 +654,30 @@ impl GuardSet {
/// Like `preference_order_ids`, but yields `&Guard` instead of `&GuardId`.
fn preference_order(&self) -> impl Iterator<Item = (ListKind, &Guard)> + '_ {
self.preference_order_ids()
.filter_map(move |(p, id)| self.guards.get(id).map(|g| (p, g)))
.filter_map(move |(p, id)| self.guards.by_all_ids(id).map(|g| (p, g)))
}
/// Return true if `guard_id` is the identity for a primary guard.
/// Return true if `guard_id` is an identity subset for any primary guard in this set.
fn guard_is_primary(&self, guard_id: &GuardId) -> bool {
// (This could be yes/no/maybe.)
// This is O(n), but the list is short.
self.primary.contains(guard_id)
self.primary
.iter()
.any(|p| p.has_all_relay_ids_from(guard_id))
}
/// For every guard that has been marked as `Unreachable` for too long,
/// mark it as `Unknown`.
pub(crate) fn consider_all_retries(&mut self, now: Instant) {
for guard in self.guards.values_mut() {
guard.consider_retry(now);
}
let old_guards = std::mem::take(&mut self.guards);
self.guards = old_guards
.into_values()
.map(|mut guard| {
guard.consider_retry(now);
guard
})
.collect();
}
/// Return the earliest time at which any guard will be retriable.
@ -575,9 +691,8 @@ impl GuardSet {
/// Mark every `Unreachable` primary guard as `Unknown`.
pub(crate) fn mark_primary_guards_retriable(&mut self) {
for id in &self.primary {
if let Some(g) = self.guards.get_mut(id) {
g.mark_retriable();
}
self.guards
.modify_by_all_ids(id, |guard| guard.mark_retriable());
}
}
@ -586,28 +701,33 @@ impl GuardSet {
pub(crate) fn all_primary_guards_are_unreachable(&mut self) -> bool {
self.primary
.iter()
.flat_map(|id| self.guards.get(id))
.flat_map(|id| self.guards.by_all_ids(id))
.all(|g| g.reachable() == Reachable::Unreachable)
}
/// Mark every `Unreachable` guard as `Unknown`.
pub(crate) fn mark_all_guards_retriable(&mut self) {
for g in self.guards.values_mut() {
g.mark_retriable();
}
let old_guards = std::mem::take(&mut self.guards);
self.guards = old_guards
.into_values()
.map(|mut guard| {
guard.mark_retriable();
guard
})
.collect();
}
/// Record that an attempt has begun to use the guard with
/// `guard_id`.
pub(crate) fn record_attempt(&mut self, guard_id: &GuardId, now: Instant) {
let is_primary = self.guard_is_primary(guard_id);
if let Some(guard) = self.guards.get_mut(guard_id) {
self.guards.modify_by_all_ids(guard_id, |guard| {
guard.record_attempt(now);
if !is_primary {
guard.note_exploratory_circ(true);
}
}
});
}
/// Record that an attempt to use the guard with `guard_id` has just
@ -623,20 +743,18 @@ impl GuardSet {
now: SystemTime,
) {
self.assert_consistency();
if let Some(guard) = self.guards.get_mut(guard_id) {
match how {
Some(external) => guard.record_external_success(external),
None => {
let newly_confirmed = guard.record_success(now, params);
self.guards.modify_by_all_ids(guard_id, |guard| match how {
Some(external) => guard.record_external_success(external),
None => {
let newly_confirmed = guard.record_success(now, params);
if newly_confirmed == NewlyConfirmed::Yes {
self.confirmed.push(guard_id.clone());
self.primary_guards_invalidated = true;
}
if newly_confirmed == NewlyConfirmed::Yes {
self.confirmed.push(guard_id.clone());
self.primary_guards_invalidated = true;
}
}
self.assert_consistency();
}
});
self.assert_consistency();
}
/// Record that an attempt to use the guard with `guard_id` has just failed.
@ -649,37 +767,33 @@ impl GuardSet {
) {
// TODO use instant uniformly for in-process, and systemtime for storage?
let is_primary = self.guard_is_primary(guard_id);
if let Some(guard) = self.guards.get_mut(guard_id) {
match how {
Some(external) => guard.record_external_failure(external, now),
None => guard.record_failure(now, is_primary),
}
}
self.guards.modify_by_all_ids(guard_id, |guard| match how {
Some(external) => guard.record_external_failure(external, now),
None => guard.record_failure(now, is_primary),
});
}
/// Record that an attempt to use the guard with `guard_id` has
/// just been abandoned, without learning whether it succeeded or failed.
pub(crate) fn record_attempt_abandoned(&mut self, guard_id: &GuardId) {
if let Some(guard) = self.guards.get_mut(guard_id) {
guard.note_exploratory_circ(false);
}
self.guards
.modify_by_all_ids(guard_id, |guard| guard.note_exploratory_circ(false));
}
/// Record that an attempt to use the guard with `guard_id` has
/// just failed in a way that we could not definitively attribute to
/// the guard.
pub(crate) fn record_indeterminate_result(&mut self, guard_id: &GuardId) {
if let Some(guard) = self.guards.get_mut(guard_id) {
self.guards.modify_by_all_ids(guard_id, |guard| {
guard.note_exploratory_circ(false);
guard.record_indeterminate_result();
}
});
}
/// Record that a given guard has told us about clock skew.
pub(crate) fn record_skew(&mut self, guard_id: &GuardId, observation: SkewObservation) {
if let Some(guard) = self.guards.get_mut(guard_id) {
guard.note_skew(observation);
}
self.guards
.modify_by_all_ids(guard_id, |guard| guard.note_skew(observation));
}
/// Return an iterator over all stored clock skew observations.
@ -956,12 +1070,12 @@ mod test {
guards.assert_consistency();
// make sure all the guards are okay.
for (g, guard) in &guards.guards {
let id = FirstHopId::in_sample(GuardSetSelector::Default, g.clone());
for guard in guards.guards.values() {
let id = FirstHopId::in_sample(GuardSetSelector::Default, guard.guard_id().clone());
let relay = id.get_relay(&netdir).unwrap();
assert!(relay.is_flagged_guard());
assert!(relay.is_dir_cache());
assert!(guards.contains_relay(&relay));
assert!(guards.guards.by_all_ids(&relay).is_some());
{
assert!(!guard.is_expired(&params, SystemTime::now()));
}
@ -1007,11 +1121,19 @@ mod test {
assert_eq!(&guards2.confirmed, &guards.confirmed);
assert_eq!(&guards2.confirmed, &[id1]);
assert_eq!(
guards.guards.keys().collect::<HashSet<_>>(),
guards2.guards.keys().collect::<HashSet<_>>()
guards
.guards
.values()
.map(Guard::guard_id)
.collect::<HashSet<_>>(),
guards2
.guards
.values()
.map(Guard::guard_id)
.collect::<HashSet<_>>()
);
for (k, g) in &guards.guards {
let g2 = guards2.guards.get(k).unwrap();
for g in guards.guards.values() {
let g2 = guards2.guards.by_all_ids(g.guard_id()).unwrap();
assert_eq!(format!("{:?}", g), format!("{:?}", g2));
}
}
@ -1363,7 +1485,7 @@ mod test {
guards2.record_failure(&id2, None, Instant::now());
// Copy status: make sure non-persistent status changed, and persistent didn't.
guards1.copy_status_from(guards2);
guards1.copy_ephemeral_status_into_newly_loaded_state(guards2);
{
let g1 = guards1.get(&id1).unwrap();
let g2 = guards1.get(&id2).unwrap();
@ -1376,14 +1498,22 @@ mod test {
// Now make a new set of unrelated guards, and make sure that copying
// from it doesn't change the membership of guards1.
let mut guards3 = GuardSet::default();
let g1_set: HashSet<_> = guards1.guards.keys().map(Clone::clone).collect();
let g1_set: HashSet<_> = guards1
.guards
.values()
.map(|g| g.guard_id().clone())
.collect();
let mut g3_set: HashSet<_> = HashSet::new();
for _ in 0..4 {
// There is roughly a 1-in-5000 chance of getting the same set
// twice, so we loop until that doesn't happen.
guards3.extend_sample_as_needed(SystemTime::now(), &params, &netdir);
guards3.select_primary_guards(&params);
g3_set = guards3.guards.keys().map(Clone::clone).collect();
g3_set = guards3
.guards
.values()
.map(|g| g.guard_id().clone())
.collect();
// There is roughly a 1-in-5000 chance of getting the same set twice, so
if g1_set == g3_set {
@ -1394,8 +1524,12 @@ mod test {
}
assert_ne!(g1_set, g3_set);
// Do the copy; make sure that the membership is unchanged.
guards1.copy_status_from(guards3);
let g1_set_new: HashSet<_> = guards1.guards.keys().map(Clone::clone).collect();
guards1.copy_ephemeral_status_into_newly_loaded_state(guards3);
let g1_set_new: HashSet<_> = guards1
.guards
.values()
.map(|g| g.guard_id().clone())
.collect();
assert_eq!(g1_set, g1_set_new);
}
}

51
crates/tor-linkspec/src/ids/by_id.rs
View File

@ -51,6 +51,20 @@ impl<H: HasRelayIds> ByRelayIds<H> {
}
}
/// Modify the value in this set (if any) that has the key `key`.
///
/// Return values are as for [`modify_by_ed25519`](Self::modify_by_ed25519)
pub fn modify_by_id<'a, T, F>(&mut self, key: T, func: F) -> Vec<H>
where
T: Into<RelayIdRef<'a>>,
F: FnOnce(&mut H),
{
match key.into() {
RelayIdRef::Ed25519(ed) => self.modify_by_ed25519(ed, func),
RelayIdRef::Rsa(rsa) => self.modify_by_rsa(rsa, func),
}
}
/// Return the value in this set (if any) that has _all_ the relay IDs
/// that `key` does.
///
@ -64,6 +78,26 @@ impl<H: HasRelayIds> ByRelayIds<H> {
.filter(|val| val.has_all_relay_ids_from(key))
}
/// Modify the value in this set (if any) that has _all_ the relay IDs
/// that `key` does.
///
/// Return values are as for [`modify_by_ed25519`](Self::modify_by_ed25519)
pub fn modify_by_all_ids<T, F>(&mut self, key: &T, func: F) -> Vec<H>
where
T: HasRelayIds,
F: FnOnce(&mut H),
{
let any_id = match key.identities().next() {
Some(id) => id,
None => return Vec::new(),
};
self.modify_by_id(any_id, |val| {
if val.has_all_relay_ids_from(key) {
func(val);
}
})
}
/// Remove the single value in this set (if any) that has _exactly the same_
/// relay IDs that `key` does
pub fn remove_exact<T>(&mut self, key: &T) -> Option<H>
@ -82,6 +116,23 @@ impl<H: HasRelayIds> ByRelayIds<H> {
}
}
/// Remove the single value in this set (if any) that has all the same relay ids that
/// relay IDs that `key` does.
pub fn remove_by_all_ids<T>(&mut self, key: &T) -> Option<H>
where
T: HasRelayIds,
{
let any_id = key.identities().next()?;
if self
.by_id(any_id)
.filter(|ent| ent.has_all_relay_ids_from(key))
.is_some()
{
self.remove_by_id(any_id)
} else {
None
}
}
/// Return a reference to every element in this set that shares _any_ ID
/// with `key`.
///

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

@ -2,6 +2,7 @@
//! that Tor can connect to, directly or indirectly.
use std::{iter::FusedIterator, net::SocketAddr};
use strum::IntoEnumIterator;
use tor_llcrypto::pk;
use crate::{ChannelMethod, OwnedChanTarget, RelayIdRef, RelayIdType, RelayIdTypeIter};
@ -61,6 +62,11 @@ pub trait HasRelayIds {
self.identity(id.id_type()).map(|my_id| my_id == id) == Some(true)
}
/// Return true if this object has any known identity.
fn has_any_identity(&self) -> bool {
RelayIdType::iter().any(|id_type| self.identity(id_type).is_some())
}
/// 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
@ -101,7 +107,6 @@ pub trait HasRelayIds {
/// If additional identities are added in the future, they may taken into
/// consideration before _or_ after the current identity types.
fn cmp_by_relay_ids<T: HasRelayIds + ?Sized>(&self, other: &T) -> std::cmp::Ordering {
use strum::IntoEnumIterator;
for key_type in RelayIdType::iter() {
let ordering = Ord::cmp(&self.identity(key_type), &other.identity(key_type));
if ordering.is_ne() {