commit cd9f82fbbe6ec4205fa3c8695cd277a03da39833 Author: juga0 juga@riseup.net Date: Mon Feb 8 15:04:58 2021 +0000

fix: relaylist: Remove duplicated can exit methods

After refactoring and making clear when we were using exit(s) that can exit to all public IPs (and a port) or only some, refactor them removing the duplicated code and adding the `strict` argument. --- sbws/core/scanner.py | 8 ++-- sbws/lib/relaylist.py | 64 +++++++++++-------------------- tests/integration/lib/test_destination.py | 6 +-- 3 files changed, 29 insertions(+), 49 deletions(-)

diff --git a/sbws/core/scanner.py b/sbws/core/scanner.py index 1499264..903f09f 100644 --- a/sbws/core/scanner.py +++ b/sbws/core/scanner.py @@ -215,7 +215,7 @@ def _pick_ideal_second_hop(relay, dest, rl, cont, is_exit): # In the case that a concrete exit can't exit to the Web server, it is not # a problem since the relay will be measured in the next loop with other # random exit. - candidates = rl.exits_not_bad_allowing_port_some_ips(dest.port) \ + candidates = rl.exits_not_bad_allowing_port(dest.port) \ if is_exit else rl.non_exits if not len(candidates): return None @@ -340,7 +340,7 @@ def measure_relay(args, conf, destinations, cb, rl, relay): # exit, then pick a non-exit. Otherwise pick an exit. # Instead of ensuring that the relay can exit to all IPs, try first with # the relay as an exit, if it can exit to some IPs. - if relay.is_exit_not_bad_allowing_port_some_ips(dest.port): + if relay.is_exit_not_bad_allowing_port(dest.port): circ_fps, nicknames = create_path_relay_as_exit(relay, dest, rl, cb) else: circ_fps, nicknames = create_path_relay_as_entry(relay, dest, rl, cb) @@ -365,7 +365,7 @@ def measure_relay(args, conf, destinations, cb, rl, relay): # to the Web server, try again using it as entry, to avoid that it would # always fail when there's only one Web server. if not is_usable and \ - relay.is_exit_not_bad_allowing_port_all_ips(dest.port): + relay.is_exit_not_bad_allowing_port(dest.port): log.info( "Exit %s (%s) that can't exit all ips failed to connect to " " %s via circuit %s (%s). Trying again with it as entry.", @@ -377,7 +377,7 @@ def measure_relay(args, conf, destinations, cb, rl, relay): "Exit %s (%s) that can't exit all ips, failed to create " " circuit as entry: %s (%s).", relay.fingerprint, relay.nickname, circ_fps, nicknames) - return error_no_circuit(relay, circ_fps, nicknames, reason, dest, + return error_no_circuit(circ_fps, nicknames, reason, relay, dest, our_nick)

log.debug('Built circuit with path %s (%s) to measure %s (%s)', diff --git a/sbws/lib/relaylist.py b/sbws/lib/relaylist.py index 3ff1f73..9c6d12a 100644 --- a/sbws/lib/relaylist.py +++ b/sbws/lib/relaylist.py @@ -178,21 +178,32 @@ class Relay: """Number of times the relay was in a conensus.""" return len(self.relay_in_recent_consensus)

- def can_exit_to_port_all_ips(self, port): + def can_exit_to_port(self, port, strict=False): """ Returns True if the relay has an exit policy and the policy accepts - exiting to the given portself or False otherwise. + exiting to the given port or False otherwise. + + If ``strict`` is true, it only returns the exits that can exit to all + IPs and that port.

The exits that are IPv6 only or IPv4 but rejecting some public networks will return false. On July 2020, there were 67 out of 1095 exits like this. + + If ``strict`` is false, it returns any exit that can exit to some + public IPs and that port. + + Note that the EXIT flag exists when the relay can exit to 443 **and** + 80. Currently all Web servers are using 443, so it would not be needed + to check the EXIT flag too, using this function. + """ assert isinstance(port, int) # if dind't get the descriptor, there isn't exit policy # When the attribute is gotten in getattr(self._desc, "exit_policy"), # is possible that stem's _input_rules is None and raises an exception # (#29899): - # File "/usr/lib/python3/dist-packages/sbws/lib/relaylist.py", line 117, in can_exit_to_port_all_ips # noqa + # File "/usr/lib/python3/dist-packages/sbws/lib/relaylist.py", line 117, in can_exit_to_port # noqa # if not self.exit_policy: # File "/usr/lib/python3/dist-packages/stem/exit_policy.py", line 512, in __len__ # noqa # return len(self._get_rules()) @@ -202,50 +213,23 @@ class Relay: # Therefore, catch the exception here. try: if self.exit_policy: - # Using `strict` to ensure it can exit to ALL domains - # and ips and that port. See #40006. # Using `strip_private` to ignore reject rules to private # networks. - # We could increase the chances that the exit can exit - # checking IPv6 with: - # ``or self.exit_policy_v6.can_exit_to(port=443, strict=True)`` - # But if it can still not exit to our Web server, then we - # should retry to measure it as entry. - return ( - self.exit_policy.strip_private() - .can_exit_to(port=port, strict=True) - ) - except TypeError: - return False - return False - - def can_exit_to_port_some_ips(self, port): - """ - Returns True if the relay has an exit policy and the policy accepts - exiting to the given port and some public IPs or False otherwise. - """ - assert isinstance(port, int) - try: - if self.exit_policy: - # Not using argument `strict`, to know whether it can exit - # some public IPs, though not all. + # When ``strict`` is true, We could increase the chances that + # the exit can exit via IPv6 too (``exit_policy_v6``). However, + # in theory that is only known using microdescriptors. return ( self.exit_policy.strip_private() - .can_exit_to(port=port) + .can_exit_to(port=port, strict=strict) ) except TypeError: return False return False

- def is_exit_not_bad_allowing_port_all_ips(self, port): + def is_exit_not_bad_allowing_port(self, port, strict=False): return (Flag.BADEXIT not in self.flags and Flag.EXIT in self.flags and - self.can_exit_to_port_all_ips(port)) - - def is_exit_not_bad_allowing_port_some_ips(self, port): - return (Flag.BADEXIT not in self.flags and - Flag.EXIT in self.flags and - self.can_exit_to_port_some_ips(port)) + self.can_exit_to_port(port, strict))

def increment_relay_recent_measurement_attempt(self): """ @@ -476,13 +460,9 @@ class RelayList: """Number of times a new consensus was obtained.""" return len(self._recent_consensus)

- def exits_not_bad_allowing_port_all_ips(self, port): - return [r for r in self.exits - if r.is_exit_not_bad_allowing_port_all_ips(port)] - - def exits_not_bad_allowing_port_some_ips(self, port): + def exits_not_bad_allowing_port(self, port, strict=False): return [r for r in self.exits - if r.is_exit_not_bad_allowing_port_some_ips(port)] + if r.is_exit_not_bad_allowing_port(port, strict)]

def increment_recent_measurement_attempt(self): """ diff --git a/tests/integration/lib/test_destination.py b/tests/integration/lib/test_destination.py index d305b0e..98ed89f 100644 --- a/tests/integration/lib/test_destination.py +++ b/tests/integration/lib/test_destination.py @@ -26,7 +26,7 @@ def test_connect_to_destination_over_circuit_success(persistent_launch_tor, relay = [r for r in rl.relays if r.nickname == 'relay1mbyteMAB'][0] # Choose an exit, for this test it does not matter the bandwidth - helper = rl.exits_not_bad_allowing_port_some_ips(destination.port)[0] + helper = rl.exits_not_bad_allowing_port(destination.port)[0] circuit_path = [relay.fingerprint, helper.fingerprint] # build a circuit circuit_id, _ = cb.build_circuit(circuit_path) @@ -46,7 +46,7 @@ def test_connect_to_destination_over_circuit_fail(persistent_launch_tor, relay = [r for r in rl.relays if r.nickname == 'relay1mbyteMAB'][0] # Choose an exit, for this test it does not matter the bandwidth - helper = rl.exits_not_bad_allowing_port_some_ips(bad_destination.port)[0] + helper = rl.exits_not_bad_allowing_port(bad_destination.port)[0] circuit_path = [relay.fingerprint, helper.fingerprint] # Build a circuit. circuit_id, _ = cb.build_circuit(circuit_path) @@ -75,7 +75,7 @@ def test_functional_destinations(conf, cb, rl, persistent_launch_tor): relay = [r for r in rl.relays if r.nickname == 'relay1mbyteMAB'][0] # Choose an exit, for this test it does not matter the bandwidth - helper = rl.exits_not_bad_allowing_port_some_ips(bad_destination.port)[0] + helper = rl.exits_not_bad_allowing_port(bad_destination.port)[0] circuit_path = [relay.fingerprint, helper.fingerprint] # Build a circuit. circuit_id, _ = cb.build_circuit(circuit_path)