commit db0518c03e2b79500a9132fa82e077fa978972fa Author: Damian Johnson <atagar@torproject.org> Date: Wed Jan 28 10:15:51 2015 -0800 ExitPolicy's behavior around 'strictness' was incorrect Our docs state that... * 'strict' is true: we can exit to *all* instances of the given address or port * 'strict' is false: we can exit to *any* instances of the given address or port Caught thanks to nskinkel... https://trac.torproject.org/projects/tor/ticket/14314 --- docs/change_log.rst | 4 ++++ stem/exit_policy.py | 19 ++++++++++++++----- test/unit/exit_policy/policy.py | 11 +++++++++++ test/unit/exit_policy/rule.py | 32 ++++++++++++++++---------------- 4 files changed, 45 insertions(+), 21 deletions(-) diff --git a/docs/change_log.rst b/docs/change_log.rst index e60dd35..21c2105 100644 --- a/docs/change_log.rst +++ b/docs/change_log.rst @@ -49,6 +49,10 @@ performance Stem also now runs directly under both python2 and python3 without a 2to3 conversion (:trac:`14075`). + * **Controller** + + * The 'strict' argument of :func:`~stem.exit_policy.ExitPolicy.can_exit_to` didn't behave as documented (:trac:`14314`) + * **Descriptors** * Lazy-loading descriptors, improving performance by 25-70% depending on what type it is (:trac:`14011`) diff --git a/stem/exit_policy.py b/stem/exit_policy.py index 53d0043..16f8721 100644 --- a/stem/exit_policy.py +++ b/stem/exit_policy.py @@ -748,14 +748,21 @@ class ExitPolicyRule(object): if port is not None and not stem.util.connection.is_valid_port(port): raise ValueError("'%s' isn't a valid port" % port) + # If we're not matching against an address or port but the rule has one + # then we're a fuzzy match. When that happens... + # + # * If strict and a reject rule then we're a match ('can exit to *all* instances'). + # * If not strict and an accept rule then match ('an exit ot *any* instance'). + + fuzzy_match = False + if not self.is_address_wildcard(): # Already got the integer representation of our mask and our address # with the mask applied. Just need to check if this address with the # mask applied matches. if address is None: - if strict: - return False + fuzzy_match = True else: comparison_addr_bin = int(stem.util.connection._get_address_binary(address), 2) comparison_addr_bin &= self._get_mask_bin() @@ -765,12 +772,14 @@ class ExitPolicyRule(object): if not self.is_port_wildcard(): if port is None: - if strict: - return False + fuzzy_match = True elif port < self.min_port or port > self.max_port: return False - return True + if fuzzy_match: + return strict != self.is_accept + else: + return True def get_address_type(self): """ diff --git a/test/unit/exit_policy/policy.py b/test/unit/exit_policy/policy.py index d4eaa75..79f99e1 100644 --- a/test/unit/exit_policy/policy.py +++ b/test/unit/exit_policy/policy.py @@ -65,6 +65,17 @@ class TestExitPolicy(unittest.TestCase): self.assertEqual(expected_result, policy.can_exit_to(ip_addr, index)) self.assertEqual(expected_result, policy.can_exit_to(port = index)) + def test_can_exit_to_strictness(self): + # Check our 'strict' argument. + + policy = ExitPolicy('reject 1.0.0.0/8:80', 'accept *:*') + self.assertEqual(False, policy.can_exit_to(None, 80, strict = True)) # can't exit to *all* instances of port 80 + self.assertEqual(True, policy.can_exit_to(None, 80, strict = False)) # can exit to *an* instance of port 80 + + policy = ExitPolicy('accept 1.0.0.0/8:80', 'reject *:*') + self.assertEqual(False, policy.can_exit_to(None, 80, strict = True)) # can't exit to *all* instances of port 80 + self.assertEqual(True, policy.can_exit_to(None, 80, strict = False)) # can exit to *an* instance of port 80 + def test_is_exiting_allowed(self): test_inputs = { (): True, diff --git a/test/unit/exit_policy/rule.py b/test/unit/exit_policy/rule.py index 1661442..5458c10 100644 --- a/test/unit/exit_policy/rule.py +++ b/test/unit/exit_policy/rule.py @@ -239,10 +239,10 @@ class TestExitPolicyRule(unittest.TestCase): ('FE80:0000:0000:0000:0202:B3FF:FE1E:8329', 80): False, ('[FE80:0000:0000:0000:0202:B3FF:FE1E:8329]', 80): False, ('192.168.0.1', None): True, - (None, 80, False): True, - (None, 80, True): False, - (None, None, False): True, - (None, None, True): False, + (None, 80, False): False, + (None, 80, True): True, + (None, None, False): False, + (None, None, True): True, }, } @@ -265,8 +265,8 @@ class TestExitPolicyRule(unittest.TestCase): ('192.168.0.50', 80): True, ('192.168.0.51', 80): False, ('192.168.0.49', 80): False, - (None, 80, False): True, - (None, 80, True): False, + (None, 80, False): False, + (None, 80, True): True, ('192.168.0.50', None): True, }, 'reject 0.0.0.0/24:*': { @@ -276,8 +276,8 @@ class TestExitPolicyRule(unittest.TestCase): ('0.0.1.0', 80): False, ('0.1.0.0', 80): False, ('1.0.0.0', 80): False, - (None, 80, False): True, - (None, 80, True): False, + (None, 80, False): False, + (None, 80, True): True, ('0.0.0.0', None): True, }, } @@ -296,8 +296,8 @@ class TestExitPolicyRule(unittest.TestCase): ('[FE80:0000:0000:0000:0202:B3FF:FE1E:8329]', 80): True, ('FE80:0000:0000:0000:0202:B3FF:FE1E:8330', 80): False, ('FE80:0000:0000:0000:0202:B3FF:FE1E:8328', 80): False, - (None, 80, False): True, - (None, 80, True): False, + (None, 80, False): False, + (None, 80, True): True, ('FE80:0000:0000:0000:0202:B3FF:FE1E:8329', None): True, }, 'reject [FE80:0000:0000:0000:0202:B3FF:FE1E:8329]/112:*': { @@ -306,8 +306,8 @@ class TestExitPolicyRule(unittest.TestCase): ('FE80:0000:0000:0000:0202:B3FF:FE1E:FFFF', 80): True, ('FE80:0000:0000:0000:0202:B3FF:FE1F:8329', 80): False, ('FE81:0000:0000:0000:0202:B3FF:FE1E:8329', 80): False, - (None, 80, False): True, - (None, 80, True): False, + (None, 80, False): False, + (None, 80, True): True, ('FE80:0000:0000:0000:0202:B3FF:FE1E:8329', None, False): True, ('FE80:0000:0000:0000:0202:B3FF:FE1E:8329', None, True): True, }, @@ -326,8 +326,8 @@ class TestExitPolicyRule(unittest.TestCase): ('192.168.0.50', 81): False, ('192.168.0.50', 79): False, (None, 80): True, - ('192.168.0.50', None, False): True, - ('192.168.0.50', None, True): False, + ('192.168.0.50', None, False): False, + ('192.168.0.50', None, True): True, }, 'reject *:80-85': { ('192.168.0.50', 79): False, @@ -336,8 +336,8 @@ class TestExitPolicyRule(unittest.TestCase): ('192.168.0.50', 85): True, ('192.168.0.50', 86): False, (None, 83): True, - ('192.168.0.50', None, False): True, - ('192.168.0.50', None, True): False, + ('192.168.0.50', None, False): False, + ('192.168.0.50', None, True): True, }, }