Commit 9650d0b7 authored by Nick Mathewson's avatar Nick Mathewson 🥔
Browse files

netdir: Consider RelayFamilyIds based on family rules 

This implements the client side of proposal 321.

It's a breaking change in netdir, since in_same_family now takes an
extra argument.
parent 5ce84e59

crates/tor-netdir/semver.md

0 → 100644
+1 −0
Original line number Diff line number Diff line 
BREAKING: in_same_family now takes a FamilyRules.

crates/tor-netdir/src/details.rs

+19 −3
Original line number Diff line number Diff line
@@ -36,7 +36,7 @@ use std::sync::Arc; 
use tor_linkspec::HasRelayIds;

use tor_netdoc::{doc::netstatus, types::policy::PortPolicy};



use crate::{Relay, SubnetConfig};

use crate::{FamilyRules, Relay, SubnetConfig};



/// A view for lower-level details about a [`Relay`].

///
@@ -96,11 +96,27 @@ impl<'a> RelayDetails<'a> { 
    /// Return true if both relays are in the same family.

    ///

    /// (Every relay is considered to be in the same family as itself.)

    pub fn in_same_family(&self, other: &Relay<'_>) -> bool {

    pub fn in_same_family(&self, other: &Relay<'_>, family_rules: FamilyRules) -> bool {

        #![allow(clippy::collapsible_if)] // I prefer this style here.

        if self.0.same_relay_ids(other) {

            return true;

        }

        self.0.md.family().contains(other.rsa_id()) && other.md.family().contains(self.0.rsa_id())

        if family_rules.use_family_lists {

            if self.0.md.family().contains(other.rsa_id())

                && other.md.family().contains(self.0.rsa_id())

            {

                return true;

            }

        }

        if family_rules.use_family_ids {

            let my_ids = self.0.md.family_ids();

            let their_ids = other.md.family_ids();

            if my_ids.iter().any(|my_id| their_ids.contains(my_id)) {

                return true;

            }

        }



        false

    }



    /// Return true if there are any ports for which this Relay can be

crates/tor-netdir/src/lib.rs

+9 −9
Original line number Diff line number Diff line
@@ -250,7 +250,6 @@ impl SubnetConfig { 
///

/// 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,
@@ -2429,6 +2428,7 @@ mod test { 
        )

        .unwrap();

        let subnet_config = SubnetConfig::default();

        let all_family_info = FamilyRules::all_family_info();

        let mut dir = PartialNetDir::new(consensus, None);

        for md in microdescs.into_iter() {

            let wanted = dir.add_microdesc(md.clone());
@@ -2478,14 +2478,14 @@ mod test { 
        assert!(!r3.low_level_details().policies_allow_some_port());

        assert!(r10.low_level_details().policies_allow_some_port());



        assert!(r0.low_level_details().in_same_family(&r0));

        assert!(r0.low_level_details().in_same_family(&r1));

        assert!(r1.low_level_details().in_same_family(&r0));

        assert!(r1.low_level_details().in_same_family(&r1));

        assert!(!r0.low_level_details().in_same_family(&r2));

        assert!(!r2.low_level_details().in_same_family(&r0));

        assert!(r2.low_level_details().in_same_family(&r2));

        assert!(r2.low_level_details().in_same_family(&r3));

        assert!(r0.low_level_details().in_same_family(&r0, all_family_info));

        assert!(r0.low_level_details().in_same_family(&r1, all_family_info));

        assert!(r1.low_level_details().in_same_family(&r0, all_family_info));

        assert!(r1.low_level_details().in_same_family(&r1, all_family_info));

        assert!(!r0.low_level_details().in_same_family(&r2, all_family_info));

        assert!(!r2.low_level_details().in_same_family(&r0, all_family_info));

        assert!(r2.low_level_details().in_same_family(&r2, all_family_info));

        assert!(r2.low_level_details().in_same_family(&r3, all_family_info));



        assert!(r0.low_level_details().in_same_subnet(&r10, &subnet_config));

        assert!(r10.low_level_details().in_same_subnet(&r10, &subnet_config));

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

+45 −15
Original line number Diff line number Diff line
@@ -349,12 +349,9 @@ impl<'a> LowLevelRelayPredicate for RelayExclusion<'a> { 
            return false;

        }



        if self

            .exclude_relay_families

            .0

            .iter()

            .any(|r| relays_in_same_extended_family(&self.subnet_config, relay, r))

        {

        if self.exclude_relay_families.0.iter().any(|r| {

            relays_in_same_extended_family(&self.subnet_config, relay, r, self.family_rules)

        }) {

            return false;

        }
@@ -369,8 +366,10 @@ fn relays_in_same_extended_family( 
    subnet_config: &SubnetConfig,

    r1: &Relay<'_>,

    r2: &Relay<'_>,

    family_rules: FamilyRules,

) -> bool {

    r1.low_level_details().in_same_family(r2) || subnet_config.any_addrs_in_same_subnet(r1, r2)

    r1.low_level_details().in_same_family(r2, family_rules)

        || subnet_config.any_addrs_in_same_subnet(r1, r2)

}



#[cfg(test)]
@@ -390,6 +389,7 @@ mod test { 
    //! <!-- @@ end test lint list maintained by maint/add_warning @@ -->



    use tor_linkspec::RelayId;

    use tor_netdir::testnet::construct_custom_netdir;



    use super::*;

    use crate::testing::{cfg, split_netdir, testnet};
@@ -441,10 +441,9 @@ mod test { 
        assert!(no.iter().all(|r| !p(r)));

    }



    #[test]

    fn exclude_families() {

        let all_families = FamilyRules::all_family_info();

        let nd = testnet();

    /// Helper for testing family exclusions.  Requires a netdir where,

    /// for every N, relays 2N and 2N+1 are in a family.

    fn exclude_families_impl(nd: &NetDir, family_rules: FamilyRules) {

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

        let id_5: RelayId = "ed25519:BQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQU"

            .parse()
@@ -466,11 +465,11 @@ mod test { 
        };



        let (yes, no) = split_netdir(

            &nd,

            nd,

            &RelayExclusion::exclude_relays_in_same_family(

                &cfg_no_subnet,

                excluding_relays.clone(),

                all_families,

                family_rules,

            ),

        );

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



        let (yes, no) = split_netdir(

            &nd,

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

            nd,

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

        );

        for r in &no {

            dbg!(r.rsa_identity().unwrap());
@@ -515,6 +514,37 @@ mod test { 
        assert!(no.iter().all(|r| { !p(r) }));

    }



    #[test]

    fn exclude_families_by_list() {

        exclude_families_impl(

            &testnet(),

            *FamilyRules::ignore_declared_families().use_family_lists(true),

        );

    }



    #[test]

    fn exclude_families_by_id() {

        // Here we construct a network that matches our default testnet,

        // but without any family lists.

        // Instead we use "happy family" IDs to match the families from that default testnest.

        let netdir = construct_custom_netdir(|pos, nb, _| {

            // Clear the family list.

            nb.md.family("".parse().unwrap());

            // This will create an "Unrecognized" family id such that

            // pos:N  will be shared by nodes in positions 2N and 2N+1.

            let fam_id = format!("pos:{}", pos / 2);

            nb.md.add_family_id(fam_id.parse().unwrap());

        })

        .unwrap()

        .unwrap_if_sufficient()

        .unwrap();



        exclude_families_impl(

            &netdir,

            *FamilyRules::ignore_declared_families().use_family_ids(true),

        );

    }



    #[test]

    fn filter_addresses() {

        let nd = testnet();