Commit 015db3d7 authored by Ian Jackson's avatar Ian Jackson
Browse files

GuardUsage: restrictions: Use list builder

Although these do not appear in the config, it does have a builder.
It seems sensible to get rid of this ad-hoc list manipulation site,
and replace it with our standard list builder API.

define_list_builder_helper requires that the builder element type be
Deserialize.  Currently GuardUsageRestriction is a transparent, public
enum, so we aren't really exposing anything.

We could introduce GuardUsageRestrictionBuilder now, but
since it's not in the config and thereofore only in the public API of
the lower crates, we can definitely put that off.
parent ab979b0b
Loading
Loading
Loading
Loading
+2 −1
Original line number Diff line number Diff line
@@ -160,7 +160,8 @@ impl<'a> ExitPathBuilder<'a> {
                    family.insert(*exit_relay.id());
                    // TODO(nickm): See "limitations" note on `known_family_members`.
                    family.extend(netdir.known_family_members(exit_relay).map(|r| *r.id()));
                    b.push_restriction(tor_guardmgr::GuardRestriction::AvoidAllIds(family));
                    b.restrictions()
                        .push(tor_guardmgr::GuardRestriction::AvoidAllIds(family));
                }
                let guard_usage = b.build().expect("Failed while building guard usage!");
                let (guard, mut mon, usable) = guardmgr.select_guard(guard_usage, Some(netdir))?;
+30 −27
Original line number Diff line number Diff line
@@ -814,34 +814,37 @@ mod test {
        assert_eq!(g.reachable(), Reachable::default());

        use crate::GuardUsageBuilder;
        let usage1 = GuardUsageBuilder::new()
            .push_restriction(GuardRestriction::AvoidId([22; 32].into()))
            .build()
            .unwrap();
        let usage2 = GuardUsageBuilder::new()
            .push_restriction(GuardRestriction::AvoidId([13; 32].into()))
            .build()
            .unwrap();
        let mut usage1 = GuardUsageBuilder::new();
        usage1
            .restrictions()
            .push(GuardRestriction::AvoidId([22; 32].into()));
        let usage1 = usage1.build().unwrap();
        let mut usage2 = GuardUsageBuilder::new();
        usage2
            .restrictions()
            .push(GuardRestriction::AvoidId([13; 32].into()));
        let usage2 = usage2.build().unwrap();
        let usage3 = GuardUsage::default();
        let usage4 = GuardUsageBuilder::new()
            .push_restriction(GuardRestriction::AvoidId([22; 32].into()))
            .push_restriction(GuardRestriction::AvoidId([13; 32].into()))
            .build()
            .unwrap();
        let usage5 = GuardUsageBuilder::new()
            .push_restriction(GuardRestriction::AvoidAllIds(
        let mut usage4 = GuardUsageBuilder::new();
        usage4
            .restrictions()
            .push(GuardRestriction::AvoidId([22; 32].into()));
        usage4
            .restrictions()
            .push(GuardRestriction::AvoidId([13; 32].into()));
        let usage4 = usage4.build().unwrap();
        let mut usage5 = GuardUsageBuilder::new();
        usage5.restrictions().push(GuardRestriction::AvoidAllIds(
            vec![[22; 32].into(), [13; 32].into()].into_iter().collect(),
            ))
            .build()
            .unwrap();
        let usage6 = GuardUsageBuilder::new()
            .push_restriction(GuardRestriction::AvoidAllIds(
        ));
        let usage5 = usage5.build().unwrap();
        let mut usage6 = GuardUsageBuilder::new();
        usage6.restrictions().push(GuardRestriction::AvoidAllIds(
            vec![[99; 32].into(), [100; 32].into()]
                .into_iter()
                .collect(),
            ))
            .build()
            .unwrap();
        ));
        let usage6 = usage6.build().unwrap();

        assert!(g.conforms_to_usage(&usage1));
        assert!(!g.conforms_to_usage(&usage2));
+24 −11
Original line number Diff line number Diff line
@@ -141,6 +141,7 @@ use std::time::{Duration, Instant, SystemTime};
use tor_proto::ClockSkew;
use tracing::{debug, info, trace, warn};

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};
@@ -1226,8 +1227,28 @@ pub struct GuardUsage {
    #[builder(default)]
    kind: GuardUsageKind,
    /// A list of restrictions on which guard may be used.
    #[builder(default)]
    restrictions: Vec<GuardRestriction>,
    ///
    /// The default is the empty list.
    #[builder(sub_builder, setter(custom))]
    restrictions: GuardRestrictionList,
}

/// List of socket restricteionesses, as configured
pub type GuardRestrictionList = Vec<GuardRestriction>;

define_list_builder_helper! {
    pub struct GuardRestrictionListBuilder {
        restrictions: [GuardRestriction],
    }
    built: GuardRestrictionList = restrictions;
    default = vec![];
    item_build: |restriction| Ok(restriction.clone());
}

define_list_builder_accessors! {
    struct GuardUsageBuilder {
        pub restrictions: [GuardRestriction],
    }
}

impl GuardUsageBuilder {
@@ -1235,14 +1256,6 @@ impl GuardUsageBuilder {
    pub fn new() -> Self {
        Self::default()
    }

    /// Add `restriction` to the list of restrictions on this guard usage.
    pub fn push_restriction(&mut self, restriction: GuardRestriction) -> &mut Self {
        self.restrictions
            .get_or_insert_with(Vec::new)
            .push(restriction);
        self
    }
}

/// A restriction that applies to a single request for a guard.
@@ -1252,7 +1265,7 @@ impl GuardUsageBuilder {
/// They're suitable for things like making sure that we don't start
/// and end a circuit at the same relay, or requiring a specific
/// subprotocol version for certain kinds of requests.
#[derive(Clone, Debug)]
#[derive(Clone, Debug, Deserialize)]
#[non_exhaustive]
pub enum GuardRestriction {
    /// Don't pick a guard with the provided Ed25519 identity.