Commit 5ce84e59 authored by Nick Mathewson's avatar Nick Mathewson 🥔
Browse files

relay-selection: Add FamilyRules to exclude_relays_in_same_family. 

When we're trying to exclude relays by family,
we need to know which lists to look at.
This information ultimately comes from the network parameters.

We could avoid this change if we just told clients
"look at all family information all the time",
but that's not what the proposal says.

This is a breaking change.
parent 3a8f5ed3

crates/tor-circmgr/src/path.rs

+4 −2
Original line number Diff line number Diff line
@@ -27,7 +27,7 @@ use tor_geoip::{CountryCode, HasCountryCode}; 
use tor_guardmgr::fallback::FallbackDir;

use tor_guardmgr::{GuardMgr, GuardMonitor, GuardUsable};

use tor_linkspec::{HasAddrs, HasRelayIds, OwnedChanTarget, OwnedCircTarget, RelayIdSet};

use tor_netdir::{NetDir, Relay};

use tor_netdir::{FamilyRules, NetDir, Relay};

use tor_relay_selection::{RelayExclusion, RelaySelectionConfig, RelaySelector, RelayUsage};

use tor_rtcompat::Runtime;
@@ -308,6 +308,7 @@ fn pick_path<'a, B: AnonymousPathBuilder, R: Rng, RT: Runtime>( 
        }

    };

    let rs_cfg = config.relay_selection_config();

    let family_rules = FamilyRules::from(netdir.params());



    let target_exclusion = match builder.compatible_with() {

        Some(ct) => {
@@ -331,6 +332,7 @@ fn pick_path<'a, B: AnonymousPathBuilder, R: Rng, RT: Runtime>( 
        MaybeOwnedRelay::Relay(r) => RelayExclusion::exclude_relays_in_same_family(

            &config.relay_selection_config(),

            vec![r.clone()],

            family_rules,

        ),

        MaybeOwnedRelay::Owned(ct) => RelayExclusion::exclude_channel_target_family(

            &config.relay_selection_config(),
@@ -344,7 +346,7 @@ fn pick_path<'a, B: AnonymousPathBuilder, R: Rng, RT: Runtime>( 
    let (exit, middle_usage) = builder.pick_exit(rng, netdir, exclusion, &rs_cfg)?;



    let mut family_exclusion =

        RelayExclusion::exclude_relays_in_same_family(&rs_cfg, vec![exit.clone()]);

        RelayExclusion::exclude_relays_in_same_family(&rs_cfg, vec![exit.clone()], family_rules);

    family_exclusion.extend(&guard_exclusion);

    let mut exclusion = family_exclusion;

    exclusion.extend(&target_exclusion);

crates/tor-circmgr/src/path/exitpath.rs

+18 −10
Original line number Diff line number Diff line
@@ -214,7 +214,7 @@ mod test { 
    use tor_basic_utils::test_rng::testing_rng;

    use tor_guardmgr::TestConfig;

    use tor_linkspec::{HasRelayIds, RelayIds};

    use tor_netdir::{testnet, SubnetConfig};

    use tor_netdir::{testnet, FamilyRules, SubnetConfig};

    use tor_persist::TestingStateMgr;

    use tor_relay_selection::LowLevelRelayPredicate;

    use tor_rtcompat::SleepProvider;
@@ -224,6 +224,7 @@ mod test { 
            &self,

            other: &MaybeOwnedRelay<'_>,

            subnet_config: SubnetConfig,

            family_rules: FamilyRules,

        ) -> bool {

            use MaybeOwnedRelay as M;

            match (self, other) {
@@ -235,7 +236,11 @@ mod test { 
                    };

                    // This use of "low_level_predicate_permits_relay" is okay because

                    // because we're in tests.

                    RelayExclusion::exclude_relays_in_same_family(&cfg, vec![a.clone()])

                    RelayExclusion::exclude_relays_in_same_family(

                        &cfg,

                        vec![a.clone()],

                        family_rules,

                    )

                    .low_level_predicate_permits_relay(b)

                }

                (a, b) => !subnet_config.any_addrs_in_same_subnet(a, b),
@@ -243,7 +248,7 @@ mod test { 
        }

    }



    fn assert_exit_path_ok(relays: &[MaybeOwnedRelay<'_>]) {

    fn assert_exit_path_ok(relays: &[MaybeOwnedRelay<'_>], family_rules: FamilyRules) {

        assert_eq!(relays.len(), 3);



        let r1 = &relays[0];
@@ -259,15 +264,16 @@ mod test { 
        assert!(!r2.same_relay_ids(r3));



        let subnet_config = SubnetConfig::default();

        assert!(r1.can_share_circuit(r2, subnet_config));

        assert!(r2.can_share_circuit(r3, subnet_config));

        assert!(r1.can_share_circuit(r3, subnet_config));

        assert!(r1.can_share_circuit(r2, subnet_config, family_rules));

        assert!(r2.can_share_circuit(r3, subnet_config, family_rules));

        assert!(r1.can_share_circuit(r3, subnet_config, family_rules));

    }



    #[test]

    fn by_ports() {

        tor_rtcompat::test_with_all_runtimes!(|rt| async move {

            let mut rng = testing_rng();

            let family_rules = FamilyRules::all_family_info();

            let netdir = testnet::construct_netdir().unwrap_if_sufficient().unwrap();

            let ports = vec![TargetPort::ipv4(443), TargetPort::ipv4(1119)];

            let dirinfo = (&netdir).into();
@@ -286,7 +292,7 @@ mod test { 
                assert_same_path_when_owned(&path);



                if let TorPathInner::Path(p) = path.inner {

                    assert_exit_path_ok(&p[..]);

                    assert_exit_path_ok(&p[..], family_rules);

                    let exit = match &p[2] {

                        MaybeOwnedRelay::Relay(r) => r,

                        MaybeOwnedRelay::Owned(_) => panic!("Didn't asked for an owned target!"),
@@ -303,6 +309,7 @@ mod test { 
    fn any_exit() {

        tor_rtcompat::test_with_all_runtimes!(|rt| async move {

            let mut rng = testing_rng();

            let family_rules = FamilyRules::all_family_info();

            let netdir = testnet::construct_netdir().unwrap_if_sufficient().unwrap();

            let dirinfo = (&netdir).into();

            let statemgr = TestingStateMgr::new();
@@ -318,7 +325,7 @@ mod test { 
                    .unwrap();

                assert_same_path_when_owned(&path);

                if let TorPathInner::Path(p) = path.inner {

                    assert_exit_path_ok(&p[..]);

                    assert_exit_path_ok(&p[..], family_rules);

                    let exit = match &p[2] {

                        MaybeOwnedRelay::Relay(r) => r,

                        MaybeOwnedRelay::Owned(_) => panic!("Didn't asked for an owned target!"),
@@ -391,6 +398,7 @@ mod test { 


        tor_rtcompat::test_with_all_runtimes!(|rt| async move {

            let netdir = testnet::construct_netdir().unwrap_if_sufficient().unwrap();

            let family_rules = FamilyRules::all_family_info();

            let mut rng = testing_rng();

            let dirinfo = (&netdir).into();

            let statemgr = TestingStateMgr::new();
@@ -413,7 +421,7 @@ mod test { 
                assert_eq!(path.len(), 3);

                assert_same_path_when_owned(&path);

                if let TorPathInner::Path(p) = path.inner {

                    assert_exit_path_ok(&p[..]);

                    assert_exit_path_ok(&p[..], family_rules);

                    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]));

crates/tor-netdir/src/lib.rs

+63 −0
Original line number Diff line number Diff line
@@ -245,6 +245,69 @@ impl SubnetConfig { 
    }

}



/// Configuration for which listed family information to use when deciding

/// whether relays belong to the same family.

///

/// Derived from network parameters.

#[derive(Clone, Copy, Debug)]

#[allow(unused)]

pub struct FamilyRules {

    /// If true, we use family information from lists of family members.

    use_family_lists: bool,

    /// If true, we use family information from lists of family IDs and from family certs.

    use_family_ids: bool,

}



impl<'a> From<&'a NetParameters> for FamilyRules {

    fn from(params: &'a NetParameters) -> Self {

        FamilyRules {

            use_family_lists: bool::from(params.use_family_lists),

            use_family_ids: bool::from(params.use_family_ids),

        }

    }

}



impl FamilyRules {

    /// Return a `FamilyRules` that will use all recognized kinds of family information.

    pub fn all_family_info() -> Self {

        Self {

            use_family_lists: true,

            use_family_ids: true,

        }

    }



    /// Return a `FamilyRules` that will ignore all family information declared by relays.

    pub fn ignore_declared_families() -> Self {

        Self {

            use_family_lists: false,

            use_family_ids: false,

        }

    }



    /// Configure this `FamilyRules` to use (or not use) family information from

    /// lists of family members.

    pub fn use_family_lists(&mut self, val: bool) -> &mut Self {

        self.use_family_lists = val;

        self

    }



    /// Configure this `FamilyRules` to use (or not use) family information from

    /// family IDs and family certs.

    pub fn use_family_ids(&mut self, val: bool) -> &mut Self {

        self.use_family_ids = val;

        self

    }



    /// Return a `FamilyRules` that will look at every source of information

    /// requested by `self` or by `other`.

    pub fn union(&self, other: &Self) -> Self {

        Self {

            use_family_lists: self.use_family_lists || other.use_family_lists,

            use_family_ids: self.use_family_ids || other.use_family_ids,

        }

    }

}



/// An opaque type representing the weight with which a relay or set of

/// relays will be selected for a given role.

///

crates/tor-relay-selection/semver.md

0 → 100644
+1 −0
Original line number Diff line number Diff line 
BREAKING: exclude_relays_in_same_family now expects a FamilyRules argument.

crates/tor-relay-selection/src/restriction.rs

+16 −3
Original line number Diff line number Diff line
@@ -3,7 +3,7 @@ 
#[cfg(feature = "geoip")]

use tor_geoip::HasCountryCode;

use tor_linkspec::{ChanTarget, HasAddrs, HasRelayIds, RelayIdSet};

use tor_netdir::{NetDir, Relay, SubnetConfig};

use tor_netdir::{FamilyRules, NetDir, Relay, SubnetConfig};

use tor_netdoc::types::policy::AddrPortPattern;



use crate::{LowLevelRelayPredicate, RelaySelectionConfig, RelayUsage};
@@ -187,6 +187,8 @@ pub struct RelayExclusion<'a> { 
    /// The configuration to use when deciding whether two addresses are in the

    /// same subnet.

    subnet_config: SubnetConfig,

    /// The rules to use when deciding whether two relays are in the same family.

    family_rules: FamilyRules,

}



/// Helper: wraps `Vec[Relay]`, but implements Debug.
@@ -216,6 +218,7 @@ impl<'a> RelayExclusion<'a> { 
            exclude_subnets: Vec::new(),

            exclude_relay_families: RelayList(Vec::new()),

            subnet_config: SubnetConfig::no_addresses_match(),

            family_rules: FamilyRules::ignore_declared_families(),

        }

    }
@@ -254,7 +257,11 @@ impl<'a> RelayExclusion<'a> { 
        netdir: &'a NetDir,

    ) -> Self {

        if let Some(r) = netdir.by_ids(ct) {

            return Self::exclude_relays_in_same_family(cfg, vec![r]);

            return Self::exclude_relays_in_same_family(

                cfg,

                vec![r],

                FamilyRules::from(netdir.params()),

            );

        }



        let exclude_ids = ct.identities().map(|id_ref| id_ref.to_owned()).collect();
@@ -280,10 +287,12 @@ impl<'a> RelayExclusion<'a> { 
    pub fn exclude_relays_in_same_family(

        cfg: &RelaySelectionConfig,

        relays: Vec<Relay<'a>>,

        family_rules: FamilyRules,

    ) -> Self {

        RelayExclusion {

            exclude_relay_families: RelayList(relays),

            subnet_config: cfg.subnet_config,

            family_rules,

            ..RelayExclusion::no_relays_excluded()

        }

    }
@@ -298,6 +307,7 @@ impl<'a> RelayExclusion<'a> { 
            exclude_subnets: exclude_addr_families,

            exclude_relay_families,

            subnet_config,

            family_rules,

        } = other;

        self.exclude_ids

            .extend(exclude_ids.iter().map(|id_ref| id_ref.to_owned()));
@@ -307,6 +317,7 @@ impl<'a> RelayExclusion<'a> { 
            .0

            .extend_from_slice(&exclude_relay_families.0[..]);

        self.subnet_config = self.subnet_config.union(subnet_config);

        self.family_rules = self.family_rules.union(family_rules);

    }



    /// Return a string describing why we rejected the relays that _don't_ match
@@ -432,6 +443,7 @@ mod test { 


    #[test]

    fn exclude_families() {

        let all_families = FamilyRules::all_family_info();

        let nd = testnet();

        let id_0: RelayId = "$0000000000000000000000000000000000000000".parse().unwrap();

        let id_5: RelayId = "ed25519:BQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQU"
@@ -458,6 +470,7 @@ mod test { 
            &RelayExclusion::exclude_relays_in_same_family(

                &cfg_no_subnet,

                excluding_relays.clone(),

                all_families,

            ),

        );

        let p = |r: &Relay<'_>| !r.identities().any(|id| expect_excluded_ids.contains(id));
@@ -487,7 +500,7 @@ mod test { 


        let (yes, no) = split_netdir(

            &nd,

            &RelayExclusion::exclude_relays_in_same_family(&cfg(), excluding_relays),

            &RelayExclusion::exclude_relays_in_same_family(&cfg(), excluding_relays, all_families),

        );

        for r in &no {

            dbg!(r.rsa_identity().unwrap());