Merge branch 'reachable_addrs_v2' into 'main'

Implement support for reachable_addrs

Closes #491 and #93

See merge request tpo/core/arti!583
This commit is contained in:
Nick Mathewson 2022-06-17 13:16:16 +00:00
commit 9ae57e8bc4
15 changed files with 543 additions and 154 deletions

67
Cargo.lock generated
View File

@ -827,14 +827,38 @@ dependencies = [
"zeroize",
]
[[package]]
name = "darling"
version = "0.13.4"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "a01d95850c592940db9b8194bc39f4bc0e89dee5c4265e4b1807c34a9aba453c"
dependencies = [
"darling_core 0.13.4",
"darling_macro 0.13.4",
]
[[package]]
name = "darling"
version = "0.14.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "4529658bdda7fd6769b8614be250cdcfc3aeb0ee72fe66f9e41e5e5eb73eac02"
dependencies = [
"darling_core",
"darling_macro",
"darling_core 0.14.1",
"darling_macro 0.14.1",
]
[[package]]
name = "darling_core"
version = "0.13.4"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "859d65a907b6852c9361e3185c862aae7fafd2887876799fa55f5f99dc40d610"
dependencies = [
"fnv",
"ident_case",
"proc-macro2",
"quote",
"strsim 0.10.0",
"syn",
]
[[package]]
@ -851,13 +875,24 @@ dependencies = [
"syn",
]
[[package]]
name = "darling_macro"
version = "0.13.4"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "9c972679f83bdf9c42bd905396b6c3588a843a17f0f16dfcfa3e2c5d57441835"
dependencies = [
"darling_core 0.13.4",
"quote",
"syn",
]
[[package]]
name = "darling_macro"
version = "0.14.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "ddfc69c5bfcbd2fc09a0f38451d2daf0e372e367986a83906d1b0dbc88134fb5"
dependencies = [
"darling_core",
"darling_core 0.14.1",
"quote",
"syn",
]
@ -884,7 +919,7 @@ version = "0.11.2"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "24c1b715c79be6328caa9a5e1a387a196ea503740f0722ec3dd8f67a9e72314d"
dependencies = [
"darling",
"darling 0.14.1",
"proc-macro2",
"quote",
"syn",
@ -2893,6 +2928,28 @@ dependencies = [
"serde",
]
[[package]]
name = "serde_with"
version = "1.14.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "678b5a069e50bf00ecd22d0cd8ddf7c236f68581b03db652061ed5eb13a312ff"
dependencies = [
"serde",
"serde_with_macros",
]
[[package]]
name = "serde_with_macros"
version = "1.5.2"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "e182d6ec6f05393cc0e5ed1bf81ad6db3a8feedf8ee515ecdd369809bcce8082"
dependencies = [
"darling 0.13.4",
"proc-macro2",
"quote",
"syn",
]
[[package]]
name = "serial_test"
version = "0.7.0"
@ -3793,6 +3850,8 @@ dependencies = [
"phf",
"rand 0.8.5",
"serde",
"serde_json",
"serde_with",
"signature",
"thiserror",
"time",

View File

@ -148,6 +148,16 @@
#ipv4_subnet_family_prefix = 16
#ipv6_subnet_family_prefix = 32
# Which addresses are we willing to contact directly?
#
# This option can be used to specify a set of addresses or ports that are
# permitted: typically, because a local firewall blocks everything else. For
# example, [ "*:80", "*:443"] would only try to connect to relays on the network
# that support port 80 or port 443. You can use prefix lengths and port ranges,
# too: "198.51.100.0/24:1-1024" is a valid pattern.
#
# By default, all addresses and ports are permitted.
#reachable_addrs = [ "*:*" ]
# Configure preemptive circuit construction.
#

View File

@ -8,9 +8,11 @@ use tor_basic_utils::define_accessor_trait;
use tor_config::impl_standard_builder;
use tor_config::{define_list_builder_accessors, define_list_builder_helper, ConfigBuildError};
use tor_guardmgr::fallback::FallbackList;
use tor_guardmgr::GuardFilter;
use derive_builder::Builder;
use serde::{Deserialize, Serialize};
use tor_netdoc::types::policy::AddrPortPattern;
use std::time::Duration;
@ -40,9 +42,36 @@ pub struct PathConfig {
/// IPv6 addresses share at least this many initial bits.
#[builder(default = "ipv6_prefix_default()")]
ipv6_subnet_family_prefix: u8,
/// The set of addresses to which we're willing to make direct connections.
#[builder(sub_builder, setter(custom))]
pub(crate) reachable_addrs: ReachableAddrs,
}
impl_standard_builder! { PathConfig }
/// Type alias for a list of reachable addresses.
type ReachableAddrs = Vec<AddrPortPattern>;
/// Return the default list of reachable addresses (namely, "*:*")
fn default_reachable_addrs() -> ReachableAddrs {
vec![AddrPortPattern::new_all()]
}
define_list_builder_helper! {
struct ReachableAddrsBuilder {
pub(crate) patterns: [AddrPortPattern],
}
built: ReachableAddrs = patterns;
default = default_reachable_addrs();
item_build: |pat| Ok(pat.clone());
}
define_list_builder_accessors! {
struct PathConfigBuilder {
pub reachable_addrs: [AddrPortPattern],
}
}
/// Default value for ipv4_subnet_family_prefix.
fn ipv4_prefix_default() -> u8 {
16
@ -65,9 +94,21 @@ impl PathConfig {
///
/// In other words, in other words, return true if every circuit permitted
/// by `other` would also be permitted by this configuration.
///
/// We use this function to decide when circuits must be discarded.
/// Therefore, it is okay to return "false" inaccurately, but we should
/// never return "true" inaccurately.
pub(crate) fn at_least_as_permissive_as(&self, other: &Self) -> bool {
self.ipv4_subnet_family_prefix >= other.ipv4_subnet_family_prefix
&& self.ipv6_subnet_family_prefix >= other.ipv6_subnet_family_prefix
&& self.reachable_addrs == other.reachable_addrs
}
/// Return a new [`GuardFilter`] reflecting the rules in this configuration.
pub(crate) fn build_guard_filter(&self) -> GuardFilter {
let mut filt = GuardFilter::default();
filt.push_reachable_addresses(self.reachable_addrs.clone());
filt
}
}

View File

@ -198,6 +198,7 @@ impl<R: Runtime> CircMgr<R> {
storage.clone(),
config.fallbacks().clone(),
)?;
guardmgr.set_filter(config.path_rules().build_guard_filter(), None);
let storage_handle = storage.create_handle(PARETO_TIMEOUT_DATA_KEY);
@ -310,6 +311,12 @@ impl<R: Runtime> CircMgr<R> {
.guardmgr()
.replace_fallback_list(new_config.fallbacks().clone());
let new_reachable = &new_config.path_rules().reachable_addrs;
if new_reachable != &old_path_rules.reachable_addrs {
let filter = new_config.path_rules().build_guard_filter();
self.mgr.peek_builder().guardmgr().set_filter(filter, None);
}
let discard_circuits = !new_config
.path_rules()
.at_least_as_permissive_as(&old_path_rules);

View File

@ -39,6 +39,7 @@ tor-error = { path = "../tor-error", version = "0.3.1" }
tor-linkspec = { path = "../tor-linkspec", version = "0.3.0" }
tor-llcrypto = { path = "../tor-llcrypto", version = "0.3.0" }
tor-netdir = { path = "../tor-netdir", version = "0.3.0" }
tor-netdoc = { path = "../tor-netdoc", version = "0.4.0" } # for address pattern
tor-persist = { path = "../tor-persist", version = "0.4.0" }
tor-proto = { path = "../tor-proto", version = "0.3.1" }
tor-rtcompat = { path = "../tor-rtcompat", version = "0.4.0" }

View File

@ -0,0 +1,2 @@
BREAKING: GuardFilter is no longer an enum.
BREAKING: set_filter now takes an Option<&NetDir>

View File

@ -3,7 +3,7 @@
use futures::task::SpawnError;
use std::sync::Arc;
use std::time::Instant;
use tor_error::{ErrorKind, HasKind};
use tor_error::{Bug, ErrorKind, HasKind};
/// A error caused by a failure to pick a guard.
#[derive(Clone, Debug, thiserror::Error)]
@ -32,6 +32,10 @@ pub enum PickGuardError {
/// Tried to select guards or fallbacks from an empty list.
#[error("Tried to pick from an empty list")]
NoCandidatesAvailable,
/// An internal programming error occurred.
#[error("Internal error")]
Internal(#[from] Bug),
}
impl tor_error::HasKind for PickGuardError {
@ -41,6 +45,7 @@ impl tor_error::HasKind for PickGuardError {
match self {
E::AllFallbacksDown { .. } | E::AllGuardsDown { .. } => EK::TorAccessFailed,
E::NoGuardsUsable | E::NoCandidatesAvailable => EK::NoPath,
E::Internal(_) => EK::Internal,
}
}
}
@ -66,6 +71,9 @@ impl tor_error::HasRetryTime for PickGuardError {
// our current universe; that's not going to be come viable down the
// line.
E::NoGuardsUsable | E::NoCandidatesAvailable => RT::Never,
// Don't try to recover from internal errors.
E::Internal(_) => RT::Never,
}
}
}

View File

@ -129,6 +129,7 @@ impl FallbackState {
&self,
rng: &mut R,
now: Instant,
filter: &crate::GuardFilter,
) -> Result<&crate::FirstHop, PickGuardError> {
if self.fallbacks.is_empty() {
return Err(PickGuardError::NoCandidatesAvailable);
@ -136,7 +137,7 @@ impl FallbackState {
self.fallbacks
.iter()
.filter(|ent| ent.status.usable_at(now))
.filter(|ent| ent.status.usable_at(now) && filter.permits(&ent.fallback))
.choose(rng)
.map(|ent| &ent.fallback)
.ok_or_else(|| PickGuardError::AllFallbacksDown {
@ -315,6 +316,7 @@ mod test {
];
let list: FallbackList = fbs.into();
let mut set: FallbackState = list.into();
let filter = crate::GuardFilter::unfiltered();
let mut counts = [0_usize; 4];
let now = Instant::now();
@ -328,7 +330,7 @@ mod test {
}
// Basic case: everybody is up.
for _ in 0..100 {
let fb = set.choose(&mut rng, now).unwrap();
let fb = set.choose(&mut rng, now, &filter).unwrap();
let idx = lookup_idx(&set, fb.id()).unwrap();
counts[idx] += 1;
}
@ -340,7 +342,7 @@ mod test {
set.note_failure(&ids[2], now);
counts = [0; 4];
for _ in 0..100 {
let fb = set.choose(&mut rng, now).unwrap();
let fb = set.choose(&mut rng, now, &filter).unwrap();
let idx = lookup_idx(&set, fb.id()).unwrap();
counts[idx] += 1;
}
@ -352,14 +354,14 @@ mod test {
set.note_failure(id, now);
}
assert!(matches!(
set.choose(&mut rng, now),
set.choose(&mut rng, now, &filter),
Err(PickGuardError::AllFallbacksDown { .. })
));
// Construct an empty set; make sure we get the right error.
let empty_set = FallbackState::from(FallbackList::from(vec![]));
assert!(matches!(
empty_set.choose(&mut rng, now),
empty_set.choose(&mut rng, now, &filter),
Err(PickGuardError::NoCandidatesAvailable)
));

View File

@ -1,8 +1,9 @@
//! Implement GuardFilter and related types.
use educe::Educe;
use tor_linkspec::ChanTarget;
// TODO(nickm): Conceivably, this type should be exposed from a lower-level crate than
// tor-netdoc.
use tor_netdoc::types::policy::AddrPortPattern;
/// An object specifying which relays are eligible to be guards.
///
@ -11,48 +12,162 @@ use tor_linkspec::ChanTarget;
/// tagged with the `Guard` flag. But clients may narrow the eligible set
/// even further—for example, to those supporting only a given set of ports,
/// or to those in a given country.
///
/// # Limitations
///
/// Right now, only the `Unrestricted` filter is implemented or available.
/// This enumeration is just a place-holder, however, to make sure we're
/// checking our filter in the right places.
#[derive(Debug, Clone, Educe)]
#[educe(Default)]
#[non_exhaustive]
pub enum GuardFilter {
/// A filter representing no restrictions on the permissible guards
/// at all.
#[educe(Default)]
Unfiltered,
/// Testing only: checks whether the first byte of the rsa key is 0 modulo 4.
#[derive(Debug, Clone, Default, Eq, PartialEq)]
pub struct GuardFilter {
/// A list of filters to apply to guard or fallback selection. Each filter
/// restricts which guards may be used, and possibly how those guards may be
/// contacted.
///
/// TODO: remove this once real filters are implemented.
#[cfg(test)]
#[allow(dead_code)]
TestingLimitKeys,
/// This list of filters has "and" semantics: a relay is permitted by this
/// filter if ALL patterns in this list permit that first hop.
filters: Vec<SingleFilter>,
}
/// A single restriction places upon usable guards.
#[derive(Debug, Clone, Eq, PartialEq)]
enum SingleFilter {
/// A set of allowable addresses that we are willing to try to connect to.
///
/// This list of patterns has "or" semantics: a guard is permitted by this filter
/// if ANY pattern in this list permits one of the guard's addresses.
ReachableAddrs(Vec<AddrPortPattern>),
}
impl GuardFilter {
/// Create a new [`GuardFilter`] that doesn't restrict the set of
/// permissible guards at all.
pub fn unfiltered() -> Self {
GuardFilter::Unfiltered
GuardFilter::default()
}
/// Restrict this filter to only permit connections to an address permitted
/// by one of the patterns in `addrs`.
pub fn push_reachable_addresses(&mut self, addrs: impl IntoIterator<Item = AddrPortPattern>) {
self.filters
.push(SingleFilter::ReachableAddrs(addrs.into_iter().collect()));
}
/// Return true if this filter permits the provided `target`.
pub(crate) fn permits<C: ChanTarget>(&self, target: &C) -> bool {
let _ = target; // ignored for now, since only Unfiltered exists.
match self {
GuardFilter::Unfiltered => true,
#[cfg(test)]
GuardFilter::TestingLimitKeys => target.rsa_identity().as_bytes()[0] & 3 == 0,
self.filters.iter().all(|filt| filt.permits(target))
}
/// Modify `first_hop` so that it contains no elements not permitted by this
/// filter.
///
/// (For example, if we are restricted only to use certain addresses, then
/// `permits` will return true for a guard that has multiple addresses even
/// if _some_ of those addresses are not permitted. In that scenario, this
/// method will remove disallowed addresses from `first_hop`.)
pub(crate) fn modify_hop(
&self,
mut first_hop: crate::FirstHop,
) -> Result<crate::FirstHop, crate::PickGuardError> {
for filt in &self.filters {
first_hop = filt.modify_hop(first_hop)?;
}
Ok(first_hop)
}
/// Return true if this filter excludes no guards at all.
pub(crate) fn is_unfiltered(&self) -> bool {
matches!(self, GuardFilter::Unfiltered)
self.filters.is_empty()
}
/// Return a fraction between 0.0 and 1.0 describing what fraction of the
/// guard bandwidth this filter permits.
pub(crate) fn frac_bw_permitted(&self, netdir: &tor_netdir::NetDir) -> f64 {
use tor_netdir::{RelayWeight, WeightRole};
let mut guard_bw: RelayWeight = 0.into();
let mut permitted_bw: RelayWeight = 0.into();
for relay in netdir.relays() {
if relay.is_flagged_guard() {
let w = netdir.relay_weight(&relay, WeightRole::Guard);
guard_bw += w;
if self.permits(&relay) {
permitted_bw += w;
}
}
}
permitted_bw.checked_div(guard_bw).unwrap_or(1.0)
}
}
impl SingleFilter {
/// Return true if this filter permits the provided target.
fn permits<C: ChanTarget>(&self, target: &C) -> bool {
match self {
SingleFilter::ReachableAddrs(patterns) => patterns
.iter()
.any(|pat| target.addrs().iter().any(|addr| pat.matches_sockaddr(addr))),
}
}
/// Modify `first_hop` so that it contains no elements not permitted by this
/// filter.
///
/// It is an internal error to call this function on a guard not already
/// passed by `self.permits()`.
fn modify_hop(
&self,
mut first_hop: crate::FirstHop,
) -> Result<crate::FirstHop, crate::PickGuardError> {
match self {
SingleFilter::ReachableAddrs(patterns) => {
first_hop
.orports
.retain(|addr| patterns.iter().any(|pat| pat.matches_sockaddr(addr)));
if first_hop.orports.is_empty() {
// TODO(nickm): The fact that this check needs to be checked
// happen indicates a likely problem in our code design.
// Right now, we have `modify_hop` and `permits` as separate
// methods because our GuardSet logic needs a way to check
// whether a guard will be permitted by a filter without
// actually altering that guard (since another filter might
// be used in the future that would allow the same guard).
//
// To mitigate the risk of hitting this error, we try to
// make sure that modify_hop is always called right after
// (or at least soon after) the filter is checked, with the
// same filter object.
return Err(tor_error::internal!(
"Tried to apply an address filter to an unsupported guard"
)
.into());
}
}
}
Ok(first_hop)
}
}
#[cfg(test)]
mod test {
#![allow(clippy::unwrap_used)]
use super::*;
use float_eq::assert_float_eq;
use tor_netdir::testnet;
#[test]
fn permissiveness() {
let nd = testnet::construct_netdir().unwrap_if_sufficient().unwrap();
const TOL: f64 = 0.01;
let non_filter = GuardFilter::default();
assert_float_eq!(non_filter.frac_bw_permitted(&nd), 1.0, abs <= TOL);
let forbid_all = {
let mut f = GuardFilter::default();
f.push_reachable_addresses(vec!["*:1".parse().unwrap()]);
f
};
assert_float_eq!(forbid_all.frac_bw_permitted(&nd), 0.0, abs <= TOL);
let net_1_only = {
let mut f = GuardFilter::default();
f.push_reachable_addresses(vec!["1.0.0.0/8:*".parse().unwrap()]);
f
};
assert_float_eq!(net_1_only.frac_bw_permitted(&nd), 54.0 / 330.0, abs <= TOL);
}
}

View File

@ -75,9 +75,6 @@
//!
//! # Limitations
//!
//! * Only one guard selection is currently supported: we don't allow a
//! "filtered" or a "bridges" selection.
//!
//! * Our circuit blocking algorithm is simplified from the one that Tor uses.
//! See comments in `GuardSet::circ_usability_status` for more information.
//! See also [proposal 337](https://gitlab.torproject.org/tpo/core/torspec/-/blob/main/proposals/337-simpler-guard-usability.md).
@ -209,6 +206,13 @@ struct GuardMgrInner {
/// use, along with their relative priorities and statuses.
guards: GuardSets,
/// The current filter that we're using to decide which guards are
/// supported.
//
// TODO: This field is duplicated in the current active [`GuardSet`]; we
// should fix that.
filter: GuardFilter,
/// Configuration values derived from the consensus parameters.
///
/// This is updated whenever the consensus parameters change.
@ -267,17 +271,39 @@ struct GuardMgrInner {
netdir_provider: Option<Weak<dyn NetDirProvider>>,
}
/// A selector that tells us which [`GuardSet`] of several is currently in use.
#[derive(Clone, Debug, Eq, PartialEq, Educe)]
#[educe(Default)]
enum GuardSetSelector {
/// The default guard set is currently in use: that's the one that we use
/// when we have no filter installed, or the filter permits most of the
/// guards on the network.
#[educe(Default)]
Default,
/// A "restrictive" guard set is currently in use: that's the one that we
/// use when we have a filter that excludes a large fraction of the guards
/// on the network.
Restricted,
}
/// Persistent state for a guard manager, as serialized to disk.
#[derive(Debug, Default, Clone, Serialize, Deserialize)]
struct GuardSets {
/// Which set of guards is currently in use?
#[serde(skip)]
active_set: GuardSetSelector,
/// The default set of guards to use.
///
/// Right now, this is the _only_ `GuardSet` for each `GuardMgr`, but we
/// expect that to change: our algorithm specifies that there can
/// be multiple named guard sets, and we can swap between them
/// depending on the user's selected [`GuardFilter`].
/// We use this one when there is no filter, or the filter permits most of the
/// guards on the network.
default: GuardSet,
/// A guard set to use when we have a restrictive filter.
#[serde(default)]
restricted: GuardSet,
/// Unrecognized fields, including (possibly) other guard sets.
#[serde(flatten)]
remaining: HashMap<String, tor_persist::JsonValue>,
@ -316,6 +342,7 @@ impl<R: Runtime> GuardMgr<R> {
let inner = Arc::new(Mutex::new(GuardMgrInner {
guards: state,
filter: GuardFilter::unfiltered(),
last_primary_retry_time: runtime.now(),
params: GuardParams::default(),
ctrl,
@ -450,55 +477,15 @@ impl<R: Runtime> GuardMgr<R> {
}
/// Replace the current [`GuardFilter`] used by this `GuardMgr`.
///
/// (Since there is only one kind of filter right now, there's no
/// real reason to call this function, but at least it should work.
pub fn set_filter(&self, filter: GuardFilter, netdir: &NetDir) {
// First we have to see how much of the possible guard space
// this new filter allows. (We don't use this info yet, but we will
// one we have nontrivial filters.)
let n_guards = netdir.relays().filter(|r| r.is_flagged_guard()).count();
let n_permitted = netdir
.relays()
.filter(|r| r.is_flagged_guard() && filter.permits(r))
.count();
let frac_permitted = if n_guards > 0 {
n_permitted as f64 / (n_guards as f64)
} else {
1.0
};
pub fn set_filter(&self, filter: GuardFilter, netdir: Option<&NetDir>) {
let now = self.runtime.wallclock();
let mut inner = self.inner.lock().expect("Poisoned lock");
let restrictive_filter = frac_permitted < inner.params.filter_threshold;
// TODO: Once we support nontrivial filters, we might have to
// swap out "active_guards" depending on which set it is.
if frac_permitted < inner.params.extreme_threshold {
warn!(
"The number of guards permitted is smaller than the guard param minimum of {}%.",
inner.params.extreme_threshold * 100.0,
);
}
info!(
?filter,
restrictive = restrictive_filter,
"Guard filter replaced."
);
inner
.guards
.active_guards_mut()
.set_filter(filter, restrictive_filter);
inner.update(now, Some(netdir));
inner.set_filter(filter, netdir, now);
}
/// Select a guard for a given [`GuardUsage`].
///
/// On success, we return a [`FirstHopId`] object to identify which
/// On success, we return a [`FirstHop`] object to identify which
/// guard we have picked, a [`GuardMonitor`] object that the
/// caller can use to report whether its attempt to use the guard
/// succeeded or failed, and a [`GuardUsable`] future that the
@ -674,12 +661,18 @@ impl GuardSets {
/// complex filter types, and for bridge relays. Those will use separate
/// `GuardSet` instances, and this accessor will choose the right one.)
fn active_guards(&self) -> &GuardSet {
&self.default
match self.active_set {
GuardSetSelector::Default => &self.default,
GuardSetSelector::Restricted => &self.restricted,
}
}
/// Return a mutable reference to the currently active set of guards.
fn active_guards_mut(&mut self) -> &mut GuardSet {
&mut self.default
match self.active_set {
GuardSetSelector::Default => &mut self.default,
GuardSetSelector::Restricted => &mut self.restricted,
}
}
/// Update all non-persistent state for the guards in this object with the
@ -738,6 +731,20 @@ impl GuardMgrInner {
Ok(params) => self.params = params,
Err(e) => warn!("Unusable guard parameters from consensus: {}", e),
}
self.select_guard_set(netdir);
}
// Change the filter, if it doesn't match what the guards have.
//
// TODO(nickm): We could use a "dirty" flag or something to decide
// whether we need to call set_filter, if this comparison starts to show
// up in profiles.
if self.guards.active_guards().filter() != &self.filter {
let restrictive = self.guards.active_set == GuardSetSelector::Restricted;
self.guards
.active_guards_mut()
.set_filter(self.filter.clone(), restrictive);
}
// Then expire guards. Do that early, in case we need more.
@ -759,16 +766,9 @@ impl GuardMgrInner {
self.guards
.active_guards_mut()
.update_status_from_netdir(netdir);
loop {
let added_any = self.guards.active_guards_mut().extend_sample_as_needed(
now,
&self.params,
netdir,
);
if !added_any {
break;
}
}
self.guards
.active_guards_mut()
.extend_sample_as_needed(now, &self.params, netdir);
}
self.guards
@ -784,6 +784,48 @@ impl GuardMgrInner {
self.update(now, None);
}
/// Update which guard set is active based on the current filter and the
/// provided netdir.
///
/// After calling this function, the new guard set's filter may be
/// out-of-date: be sure to call `set_filter` as appropriate.
fn select_guard_set(&mut self, netdir: &NetDir) {
let frac_permitted = self.filter.frac_bw_permitted(netdir);
// In general, we'd like to use the restricted set if we're under the
// threshold, and the default set if we're over the threshold. But if
// we're sitting close to the threshold, we want to avoid flapping back
// and forth, so we only change when we're more than 5% "off" from
// whatever our current setting is.
//
// (See guard-spec section 2 for more information.)
let offset = match self.guards.active_set {
GuardSetSelector::Default => -0.05,
GuardSetSelector::Restricted => 0.05,
};
let threshold = self.params.filter_threshold + offset;
let new_choice = if frac_permitted < threshold {
GuardSetSelector::Restricted
} else {
GuardSetSelector::Default
};
if new_choice != self.guards.active_set {
info!(
"Guard selection changed; we are now using the {:?} guard set",
&new_choice
);
self.guards.active_set = new_choice;
if frac_permitted < self.params.extreme_threshold {
warn!(
"The number of guards permitted is smaller than the recommended minimum of {:.0}%.",
self.params.extreme_threshold * 100.0,
);
}
}
}
/// Mark all of our primary guards as retriable, if we haven't done
/// so since long enough before `now`.
///
@ -805,6 +847,16 @@ impl GuardMgrInner {
}
}
/// Replace the current GuardFilter with `filter`.
fn set_filter(&mut self, filter: GuardFilter, netdir: Option<&NetDir>, now: SystemTime) {
self.with_opt_netdir(netdir, |this, netdir| {
this.filter = filter;
// This call will invoke update_chosen_guard_set() if possible, and
// then call set_filter on the GuardSet.
this.update_internal(now, netdir);
});
}
/// Called when the circuit manager reports (via [`GuardMonitor`]) that
/// a guard succeeded or failed.
///
@ -1132,18 +1184,9 @@ impl GuardMgrInner {
usage: &GuardUsage,
now: Instant,
) -> Result<(sample::ListKind, FirstHop), PickGuardError> {
let (source, id) = self
.guards
self.guards
.active_guards()
.pick_guard(usage, &self.params, now)?;
let guard = self
.guards
.active_guards()
.get(&id)
.expect("Selected guard that wasn't in our sample!?")
.get_external_rep();
Ok((source, guard))
.pick_guard(usage, &self.params, now)
}
/// Helper: Select a fallback directory.
@ -1154,8 +1197,11 @@ impl GuardMgrInner {
&self,
now: Instant,
) -> Result<(sample::ListKind, FirstHop), PickGuardError> {
let fallback = self.fallbacks.choose(&mut rand::thread_rng(), now)?;
Ok((sample::ListKind::Fallback, fallback.clone()))
let filt = self.guards.active_guards().filter();
let fallback = self.fallbacks.choose(&mut rand::thread_rng(), now, filt)?;
let fallback = filt.modify_hop(fallback.clone())?;
Ok((sample::ListKind::Fallback, fallback))
}
}
@ -1199,8 +1245,6 @@ struct GuardParams {
internet_down_timeout: Duration,
/// What fraction of the guards can be can be filtered out before we
/// decide that our filter is "very restrictive"?
///
/// (Not fully implemented yet.)
filter_threshold: f64,
/// What fraction of the guards determine that our filter is "very
/// restrictive"?
@ -1384,9 +1428,20 @@ mod test {
assert!(have_lock.held());
let guardmgr = GuardMgr::new(rt, statemgr.clone(), [].into()).unwrap();
let (con, mds) = testnet::construct_network().unwrap();
let override_p = "guard-min-filtered-sample-size=5 guard-n-primary-guards=2"
.parse()
.unwrap();
let param_overrides = vec![
// We make the sample size smaller than usual to compensate for the
// small testing network. (Otherwise, we'd sample the whole network,
// and not be able to observe guards in the tests.)
"guard-min-filtered-sample-size=5",
// We choose only two primary guards, to make the tests easier to write.
"guard-n-primary-guards=2",
// We define any restriction that allows 75% or fewer of relays as "meaningful",
// so that we can test the "restrictive" guard sample behavior, and to avoid
"guard-meaningful-restriction-percent=75",
];
let param_overrides: String =
itertools::Itertools::intersperse(param_overrides.into_iter(), " ").collect();
let override_p = param_overrides.parse().unwrap();
let mut netdir = PartialNetDir::new(con, Some(&override_p));
for md in mds {
netdir.add_microdesc(md);
@ -1478,14 +1533,23 @@ mod test {
#[test]
fn filtering_basics() {
test_with_all_runtimes!(|rt| async move {
use tor_linkspec::ChanTarget;
let (guardmgr, _statemgr, netdir) = init(rt);
let u = GuardUsage::default();
let filter = {
let mut f = GuardFilter::default();
// All the addresses in the test network are {0,1,2,3,4}.0.0.3:9001.
// Limit to only 2.0.0.0/8
f.push_reachable_addresses(vec!["2.0.0.0/8:9001".parse().unwrap()]);
f
};
guardmgr.set_filter(filter, Some(&netdir));
guardmgr.update_network(&netdir);
guardmgr.set_filter(GuardFilter::TestingLimitKeys, &netdir);
let (guard, _mon, _usable) = guardmgr.select_guard(u, Some(&netdir)).unwrap();
// Make sure that the filter worked.
assert_eq!(guard.id().as_ref().rsa.as_bytes()[0] % 4, 0);
let addr = guard.addrs()[0];
assert_eq!(addr, "2.0.0.3:9001".parse().unwrap());
});
}

View File

@ -4,6 +4,7 @@
use crate::filter::GuardFilter;
use crate::guard::{Guard, NewlyConfirmed, Reachable};
use crate::skew::SkewObservation;
use crate::FirstHop;
use crate::{
ids::GuardId, ExternalActivity, GuardParams, GuardUsage, GuardUsageKind, PickGuardError,
};
@ -37,12 +38,6 @@ use tracing::{debug, info};
/// guards come first in preference order. Then come the non-primary
/// confirmed guards, in their confirmed order. Finally come the
/// non-primary, non-confirmed guards, in their sampled order.
///
/// # Limitations
///
/// Our current guard implementation in arti only uses
/// `GuardSet` at time, but eventually we may want to allow several to
/// exist, of which only one is "active".
#[derive(Default, Debug, Clone, Deserialize)]
#[serde(from = "GuardSample")]
pub(crate) struct GuardSet {
@ -181,6 +176,11 @@ impl GuardSet {
self.primary_guards_invalidated = true;
}
/// Return the current filter for this `GuardSet`.
pub(crate) fn filter(&self) -> &GuardFilter {
&self.active_filter
}
/// 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();
@ -275,21 +275,29 @@ impl GuardSet {
/// Guards always start out un-confirmed.
///
/// Return true if any guards were added.
///
/// # Complications
///
/// For spec conformance, we only consider our filter when
/// selecting new guards if the filter is "very restrictive".
/// That makes it possible that this will add fewer
/// filter-permitted guards than we had wanted. Because of that,
/// it's advisable to run this function in a loop until it returns
/// false.
pub(crate) fn extend_sample_as_needed(
&mut self,
now: SystemTime,
params: &GuardParams,
dir: &NetDir,
) -> bool {
let mut any_added = false;
while self.extend_sample_inner(now, params, dir) {
any_added = true;
}
any_added
}
/// Implementation helper for extend_sample_as_needed.
///
/// # Complications
///
/// For spec conformance, we only consider our filter when selecting new
/// guards if the filter is "very restrictive". That makes it possible that
/// this function will add fewer filter-permitted guards than we had wanted.
/// Because of that, this is a separate function, and
/// extend_sample_as_needed runs it in a loop until it returns false.
fn extend_sample_inner(&mut self, now: SystemTime, params: &GuardParams, dir: &NetDir) -> bool {
self.assert_consistency();
let n_filtered_usable = self
.guards
@ -742,12 +750,32 @@ impl GuardSet {
/// Try to select a guard for a given `usage`.
///
/// On success, returns the kind of guard that we got, and its identity.
/// On success, returns the kind of guard that we got, and its filtered
/// representation in a form suitable for use as a first hop.
pub(crate) fn pick_guard(
&self,
usage: &GuardUsage,
params: &GuardParams,
now: Instant,
) -> Result<(ListKind, FirstHop), PickGuardError> {
let (list_kind, id) = self.pick_guard_id(usage, params, now)?;
let first_hop = self
.get(&id)
.expect("Somehow selected a guard we don't know!")
.get_external_rep();
let first_hop = self.active_filter.modify_hop(first_hop)?;
Ok((list_kind, first_hop))
}
/// Try to select a guard for a given `usage`.
///
/// On success, returns the kind of guard that we got, and its identity.
fn pick_guard_id(
&self,
usage: &GuardUsage,
params: &GuardParams,
now: Instant,
) -> Result<(ListKind, GuardId), PickGuardError> {
debug_assert!(!self.primary_guards_invalidated);
let n_options = match usage.kind {
@ -1063,7 +1091,7 @@ mod test {
let usage = crate::GuardUsageBuilder::default().build().unwrap();
let id1 = guards.primary[0].clone();
let id2 = guards.primary[1].clone();
let (src, id) = guards.pick_guard(&usage, &params, i1).unwrap();
let (src, id) = guards.pick_guard_id(&usage, &params, i1).unwrap();
assert_eq!(src, ListKind::Primary);
assert_eq!(&id, &id1);
@ -1071,12 +1099,12 @@ mod test {
guards.record_failure(&id, None, i1 + sec);
// Second guard: try it, and try it again, and have it fail.
let (src, id) = guards.pick_guard(&usage, &params, i1 + sec).unwrap();
let (src, id) = guards.pick_guard_id(&usage, &params, i1 + sec).unwrap();
assert_eq!(src, ListKind::Primary);
assert_eq!(&id, &id2);
guards.record_attempt(&id, i1 + sec);
let (src, id_x) = guards.pick_guard(&usage, &params, i1 + sec).unwrap();
let (src, id_x) = guards.pick_guard_id(&usage, &params, i1 + sec).unwrap();
// We get the same guard this (second) time that we pick it too, since
// it is a primary guard, and is_pending won't block it.
assert_eq!(id_x, id);
@ -1086,14 +1114,14 @@ mod test {
guards.record_failure(&id, None, i1 + sec * 4);
// Third guard: this one won't be primary.
let (src, id3) = guards.pick_guard(&usage, &params, i1 + sec * 4).unwrap();
let (src, id3) = guards.pick_guard_id(&usage, &params, i1 + sec * 4).unwrap();
assert_eq!(src, ListKind::Sample);
assert!(!guards.primary.contains(&id3));
guards.record_attempt(&id3, i1 + sec * 5);
// Fourth guard: Third guard will be pending, so a different one gets
// handed out here.
let (src, id4) = guards.pick_guard(&usage, &params, i1 + sec * 5).unwrap();
let (src, id4) = guards.pick_guard_id(&usage, &params, i1 + sec * 5).unwrap();
assert_eq!(src, ListKind::Sample);
assert!(id3 != id4);
assert!(!guards.primary.contains(&id4));
@ -1130,7 +1158,9 @@ mod test {
assert_eq!(&guards.primary, &[id3.clone(), id4.clone()]);
// Next time we ask for a guard, we get a primary guard again.
let (src, id) = guards.pick_guard(&usage, &params, i1 + sec * 10).unwrap();
let (src, id) = guards
.pick_guard_id(&usage, &params, i1 + sec * 10)
.unwrap();
assert_eq!(src, ListKind::Primary);
assert_eq!(&id, &id3);
@ -1141,7 +1171,9 @@ mod test {
.build()
.unwrap();
for _ in 0..64 {
let (src, id) = guards.pick_guard(&usage, &params, i1 + sec * 10).unwrap();
let (src, id) = guards
.pick_guard_id(&usage, &params, i1 + sec * 10)
.unwrap();
assert_eq!(src, ListKind::Primary);
assert_eq!(
guards.circ_usability_status(&id, &usage, &params, i1 + sec * 10),
@ -1186,7 +1218,7 @@ mod test {
assert_eq!(guards.sample.len(), 5);
for _ in 0..5 {
let (_, id) = guards.pick_guard(&usage, &params, inst).unwrap();
let (_, id) = guards.pick_guard_id(&usage, &params, inst).unwrap();
guards.record_attempt(&id, inst);
guards.record_failure(&id, None, inst + sec);
@ -1194,7 +1226,7 @@ mod test {
st += sec * 2;
}
let e = guards.pick_guard(&usage, &params, inst);
let e = guards.pick_guard_id(&usage, &params, inst);
assert!(matches!(e, Err(PickGuardError::AllGuardsDown { .. })));
// Now in theory we should re-grow when we extend.
@ -1223,13 +1255,17 @@ mod test {
assert!(!guards.all_primary_guards_are_unreachable());
// Let one primary guard fail.
let (kind, p_id1) = guards.pick_guard(&usage, &params, Instant::now()).unwrap();
let (kind, p_id1) = guards
.pick_guard_id(&usage, &params, Instant::now())
.unwrap();
assert_eq!(kind, ListKind::Primary);
guards.record_failure(&p_id1, None, Instant::now());
assert!(!guards.all_primary_guards_are_unreachable());
// Now let the other one fail.
let (kind, p_id2) = guards.pick_guard(&usage, &params, Instant::now()).unwrap();
let (kind, p_id2) = guards
.pick_guard_id(&usage, &params, Instant::now())
.unwrap();
assert_eq!(kind, ListKind::Primary);
guards.record_failure(&p_id2, None, Instant::now());
assert!(guards.all_primary_guards_are_unreachable());
@ -1237,7 +1273,9 @@ mod test {
// Now mark the guards retriable.
guards.mark_primary_guards_retriable();
assert!(!guards.all_primary_guards_are_unreachable());
let (kind, p_id3) = guards.pick_guard(&usage, &params, Instant::now()).unwrap();
let (kind, p_id3) = guards
.pick_guard_id(&usage, &params, Instant::now())
.unwrap();
assert_eq!(kind, ListKind::Primary);
assert_eq!(p_id3, p_id1);
}
@ -1257,7 +1295,9 @@ mod test {
guards.select_primary_guards(&params);
assert_eq!(guards.primary.len(), 2);
let (_kind, p_id1) = guards.pick_guard(&usage, &params, Instant::now()).unwrap();
let (_kind, p_id1) = guards
.pick_guard_id(&usage, &params, Instant::now())
.unwrap();
guards.record_success(&p_id1, &params, None, SystemTime::now());
assert_eq!(guards.missing_primary_microdescriptors(&netdir), 0);

View File

@ -191,7 +191,7 @@ pub struct NetParameters {
/// If we have excluded so many possible guards that the
/// available fraction is below this threshold, we should use a different
/// guard sample. (TODO: not actually implemented)
/// guard sample.
pub guard_meaningful_restriction: Percentage<BoundedInt32<1,100>> = (20)
from "guard-meaningful-restriction-percent",

View File

@ -55,6 +55,7 @@ once_cell = "1"
phf = { version = "0.10.0", features = ["macros"] }
rand = { version = "0.8", optional = true }
serde = "1.0.103"
serde_with = "1.14.0"
signature = "1"
thiserror = "1"
time = { version = "0.3", features = ["std", "parsing", "macros"] }
@ -71,3 +72,4 @@ weak-table = "0.3.0"
[dev-dependencies]
hex-literal = "0.3"
serde_json = "1.0.50"

View File

@ -0,0 +1,2 @@
MODIFIED: AddrPortPattern and friends now impl Eq and PartialEq.
MODIFIED: AddrPortPattern now implements {Des,S}erialize

View File

@ -130,7 +130,9 @@ impl Display for AddrPolicyRule {
/// assert!(pat.matches(&localhost, 22));
/// assert!(! pat.matches(&not_localhost, 22));
/// ```
#[derive(Clone, Debug)]
#[derive(
Clone, Debug, Eq, PartialEq, serde_with::SerializeDisplay, serde_with::DeserializeFromStr,
)]
pub struct AddrPortPattern {
/// A pattern to match somewhere between zero and all IP addresses.
pattern: IpPattern,
@ -139,6 +141,14 @@ pub struct AddrPortPattern {
}
impl AddrPortPattern {
/// Return an AddrPortPattern matching all targets.
pub fn new_all() -> Self {
Self {
pattern: IpPattern::Star,
ports: PortRange::new_all(),
}
}
/// Return true iff this pattern matches a given address and port.
pub fn matches(&self, addr: &IpAddr, port: u16) -> bool {
self.pattern.matches(addr) && self.ports.contains(port)
@ -176,7 +186,12 @@ impl FromStr for AddrPortPattern {
}
/// A pattern that matches one or more IP addresses.
#[derive(Clone, Debug)]
//
// TODO(nickm): At present there is no way for Display or FromStr to distinguish
// V4Star, V6Star, and Star. If we decide it's important to have a syntax for
// "all IPv4 addresses" that isn't "0.0.0.0/0", we'll need to revisit that.
// At present, C tor allows '*', '*4', and '*6'.
#[derive(Clone, Debug, Eq, PartialEq)]
enum IpPattern {
/// Match all addresses.
Star,
@ -379,4 +394,25 @@ mod test {
);
Ok(())
}
#[test]
fn serde() {
#[derive(Clone, Debug, serde::Serialize, serde::Deserialize, Eq, PartialEq)]
struct X {
p1: AddrPortPattern,
p2: AddrPortPattern,
}
let x = X {
p1: "127.0.0.1/8:9-10".parse().unwrap(),
p2: "*:80".parse().unwrap(),
};
let encoded = serde_json::to_string(&x).unwrap();
let expected = r#"{"p1":"127.0.0.1/8:9-10","p2":"*:80"}"#;
let x2: X = serde_json::from_str(&encoded).unwrap();
let x3: X = serde_json::from_str(expected).unwrap();
assert_eq!(&x2, &x3);
assert_eq!(&x2, &x);
}
}