commit 8dddc2ab8473937bb6853e6f91db8b817a36cdd9 Author: Damian Johnson atagar@torproject.org Date: Mon Jan 6 16:06:32 2020 -0800

Remove get_config_policy

Tor exit policies are well specified, but torrcs accept a looser format that includes a 'private' keyword, makes ports optional, etc.

Stem 1.x made a best effort attempt to read these policies via a get_config_policy function, but their lack of a formal specification make them a pain. As such get_config_policy() has long been deprecated.

Dropping this function, and with it the ability to read torrc exit policies. As a result Controllers can no longer call get_exit_policy() when 'GETINFO exit-policy/full' fails. That said, it's probably better to fully rely on tor to get an exit policy rather than continuing to guess at this on our side. --- stem/control.py | 37 +----------------- stem/exit_policy.py | 69 +--------------------------------- test/unit/control/controller.py | 51 ------------------------- test/unit/exit_policy/policy.py | 83 +---------------------------------------- 4 files changed, 5 insertions(+), 235 deletions(-)

diff --git a/stem/control.py b/stem/control.py index 9fda9d34..5501d1c8 100644 --- a/stem/control.py +++ b/stem/control.py @@ -1295,41 +1295,8 @@ class Controller(BaseController): policy = self._get_cache('exit_policy')

if not policy: - try: - policy = stem.exit_policy.ExitPolicy(*self.get_info('exit-policy/full').splitlines()) - self._set_cache({'exit_policy': policy}) - except stem.OperationFailed: - # There's a few situations where 'GETINFO exit-policy/full' will fail, - # most commonly... - # - # * Error 551: Descriptor still rebuilding - not ready yet - # - # Tor hasn't yet finished making our server descriptor. This often - # arises when tor has first started. - # - # * Error 552: Not running in server mode - # - # We're not configured to be a relay (no ORPort), or haven't yet - # been able to determine our externally facing IP address. - # - # When these arise best we can do is infer our policy from the torrc. - # Skipping caching so we'll retry GETINFO policy resolution next time - # we're called. - - rules = [] - - if self.get_conf('ExitRelay') == '0': - rules.append('reject *:*') - - if self.get_conf('ExitPolicyRejectPrivate') == '1': - rules.append('reject private:*') - - for policy_line in self.get_conf('ExitPolicy', multiple = True): - rules += policy_line.split(',') - - rules += self.get_info('exit-policy/default').split(',') - - policy = stem.exit_policy.get_config_policy(rules, self.get_info('address', None)) + policy = stem.exit_policy.ExitPolicy(*self.get_info('exit-policy/full').splitlines()) + self._set_cache({'exit_policy': policy})

return policy

diff --git a/stem/exit_policy.py b/stem/exit_policy.py index 0d1e7e42..0150e190 100644 --- a/stem/exit_policy.py +++ b/stem/exit_policy.py @@ -50,8 +50,6 @@ exiting to a destination is permissible or not. For instance... |- is_private - flag indicating if this was expanded from a 'private' keyword +- __str__ - string representation for this rule

- get_config_policy - provides the ExitPolicy based on torrc rules - .. data:: AddressType (enum)

Enumerations for IP address types that can be in an exit policy. @@ -96,69 +94,6 @@ PRIVATE_ADDRESSES = ( )

-def get_config_policy(rules, ip_address = None): - """ - Converts an ExitPolicy found in a torrc to a proper exit pattern. This - accounts for... - - * ports being optional - * the 'private' keyword - - .. deprecated:: 1.7.0 - - Tor's torrc parameters lack a formal spec, making it difficult for this - method to be reliable. Callers are encouraged to move to - :func:`~stem.control.Controller.get_exit_policy` instead. - - :param str,list rules: comma separated rules or list to be converted - :param str ip_address: this relay's IP address for the 'private' policy if - it's present, this defaults to the local address - - :returns: :class:`~stem.exit_policy.ExitPolicy` reflected by the rules - - :raises: **ValueError** if input isn't a valid tor exit policy - """ - - if ip_address and not (stem.util.connection.is_valid_ipv4_address(ip_address) or stem.util.connection.is_valid_ipv6_address(ip_address, allow_brackets = True)): - raise ValueError("%s isn't a valid IP address" % ip_address) - elif ip_address and stem.util.connection.is_valid_ipv6_address(ip_address, allow_brackets = True) and not (ip_address[0] == '[' and ip_address[-1] == ']'): - ip_address = '[%s]' % ip_address # ExitPolicy validation expects IPv6 addresses to be bracketed - - if isinstance(rules, (bytes, str)): - rules = rules.split(',') - - result = [] - - for rule in rules: - rule = rule.strip() - - if not rule: - continue - - if not re.search(':[\d\-\*]+$', rule): - rule = '%s:*' % rule - - if 'private' in rule: - acceptance = rule.split(' ', 1)[0] - port = rule.rsplit(':', 1)[1] - addresses = list(PRIVATE_ADDRESSES) - - if ip_address: - addresses.append(ip_address) - else: - try: - addresses.append(socket.gethostbyname(socket.gethostname())) - except: - pass # we might not have a network connection - - for private_addr in addresses: - result.append(ExitPolicyRule('%s %s:%s' % (acceptance, private_addr, port))) - else: - result.append(ExitPolicyRule(rule)) - - return ExitPolicy(*result) - - def _flag_private_rules(rules): """ Determine if part of our policy was expanded from the 'private' keyword. This @@ -184,9 +119,7 @@ def _flag_private_rules(rules): # * all rules have the same port range # * all rules have the same acceptance (all accept or reject entries) # - # The last rule is dynamically based on the relay's public address. It may - # not be present if get_config_policy() created this policy and we couldn't - # resolve our address. + # The last rule is dynamically based on the relay's public address.

last_index = start_index + len(PRIVATE_ADDRESSES) rule_set = rules[start_index:last_index] diff --git a/test/unit/control/controller.py b/test/unit/control/controller.py index 9628c913..34429f49 100644 --- a/test/unit/control/controller.py +++ b/test/unit/control/controller.py @@ -196,57 +196,6 @@ class TestControl(unittest.TestCase):

@patch('stem.control.Controller.get_info') @patch('stem.control.Controller.get_conf') - def test_get_exit_policy_if_not_relaying(self, get_conf_mock, get_info_mock): - # If tor lacks an ORPort, resolved extrnal address, hasn't finished making - # our server descriptor (ie. tor just started), etc 'GETINFO - # exit-policy/full' will fail. - - get_conf_mock.side_effect = lambda param, **kwargs: { - 'ExitRelay': '1', - 'ExitPolicyRejectPrivate': '1', - 'ExitPolicy': ['accept *:80, accept *:443', 'accept 43.5.5.5,reject *:22'], - }[param] - - expected = ExitPolicy( - 'reject 0.0.0.0/8:*', - 'reject 169.254.0.0/16:*', - 'reject 127.0.0.0/8:*', - 'reject 192.168.0.0/16:*', - 'reject 10.0.0.0/8:*', - 'reject 172.16.0.0/12:*', - 'reject 1.2.3.4:*', - 'accept *:80', - 'accept *:443', - 'accept 43.5.5.5:*', - 'reject *:22', - ) - - # Unfortunate it's a bit tricky to have a mock that raises exceptions in - # response to some arguments, and returns a response for others. As such - # mapping it to the following function. - - exit_policy_exception = None - - def getinfo_response(param, default = None): - if param == 'address': - return '1.2.3.4' - elif param == 'exit-policy/default': - return '' - elif param == 'exit-policy/full' and exit_policy_exception: - raise exit_policy_exception - else: - raise ValueError("Unmocked request for 'GETINFO %s'" % param) - - get_info_mock.side_effect = getinfo_response - - exit_policy_exception = stem.OperationFailed('552', 'Not running in server mode') - self.assertEqual(str(expected), str(self.controller.get_exit_policy())) - - exit_policy_exception = stem.OperationFailed('551', 'Descriptor still rebuilding - not ready yet') - self.assertEqual(str(expected), str(self.controller.get_exit_policy())) - - @patch('stem.control.Controller.get_info') - @patch('stem.control.Controller.get_conf') def test_get_ports(self, get_conf_mock, get_info_mock): """ Exercises the get_ports() and get_listeners() methods. diff --git a/test/unit/exit_policy/policy.py b/test/unit/exit_policy/policy.py index 0cb755ae..f1e74aeb 100644 --- a/test/unit/exit_policy/policy.py +++ b/test/unit/exit_policy/policy.py @@ -9,7 +9,6 @@ from unittest.mock import Mock, patch

from stem.exit_policy import ( DEFAULT_POLICY_RULES, - get_config_policy, ExitPolicy, MicroExitPolicy, ExitPolicyRule, @@ -110,15 +109,8 @@ class TestExitPolicy(unittest.TestCase): policy = ExitPolicy('reject *:80-65535', 'accept *:1-65533', 'reject *:*') self.assertEqual('accept 1-79', policy.summary())

- def test_without_port(self): - policy = get_config_policy('accept 216.58.193.78, reject *') - self.assertEqual([ExitPolicyRule('accept 216.58.193.78:*'), ExitPolicyRule('reject *:*')], list(policy)) - - policy = get_config_policy('reject6 [2a00:1450:4001:081e:0000:0000:0000:200e]') - self.assertEqual([ExitPolicyRule('reject [2a00:1450:4001:081e:0000:0000:0000:200e]:*')], list(policy)) - def test_non_private_non_default_policy(self): - policy = get_config_policy('reject *:80-65535, accept *:1-65533, reject *:*') + policy = ExitPolicy('reject *:80-65535', 'accept *:1-65533', 'reject *:*')

for rule in policy: self.assertFalse(rule.is_private()) @@ -130,26 +122,6 @@ class TestExitPolicy(unittest.TestCase): self.assertEqual(policy, policy.strip_private()) self.assertEqual(policy, policy.strip_default())

- def test_all_private_policy(self): - for port in ('*', '80', '1-1024'): - private_policy = get_config_policy('reject private:%s' % port, '12.34.56.78') - - for rule in private_policy: - self.assertTrue(rule.is_private()) - - self.assertEqual(ExitPolicy(), private_policy.strip_private()) - - # though not commonly done, technically private policies can be accept rules too - - private_policy = get_config_policy('accept private:*') - self.assertEqual(ExitPolicy(), private_policy.strip_private()) - - @patch('socket.gethostname', Mock(side_effect = IOError('no address'))) - def test_all_private_policy_without_network(self): - for rule in get_config_policy('reject private:80, accept *:80'): - # all rules except the ending accept are part of the private policy - self.assertEqual(str(rule) != 'accept *:80', rule.is_private()) - def test_all_default_policy(self): policy = ExitPolicy(*DEFAULT_POLICY_RULES)

@@ -159,14 +131,6 @@ class TestExitPolicy(unittest.TestCase): self.assertTrue(policy.has_default()) self.assertEqual(ExitPolicy(), policy.strip_default())

- def test_mixed_private_policy(self): - policy = get_config_policy('accept *:80, reject private:1-65533, accept *:*') - - for rule in policy: - self.assertTrue(rule.is_accept != rule.is_private()) # only reject rules are the private ones - - self.assertEqual(get_config_policy('accept *:80, accept *:*'), policy.strip_private()) - def test_mixed_default_policy(self): policy = ExitPolicy('accept *:80', 'accept 127.0.0.1:1-65533', *DEFAULT_POLICY_RULES)

@@ -174,12 +138,7 @@ class TestExitPolicy(unittest.TestCase): # only accept-all and reject rules are the default ones self.assertTrue(rule.is_accept != rule.is_default() or (rule.is_accept and rule.is_address_wildcard() and rule.is_port_wildcard()))

- self.assertEqual(get_config_policy('accept *:80, accept 127.0.0.1:1-65533'), policy.strip_default()) - - def test_get_config_policy_with_ipv6(self): - # ensure our constructor accepts addresses both with and without brackets - self.assertTrue(get_config_policy('reject private:80', 'fe80:0000:0000:0000:0202:b3ff:fe1e:8329').is_exiting_allowed()) - self.assertTrue(get_config_policy('reject private:80', '[fe80:0000:0000:0000:0202:b3ff:fe1e:8329]').is_exiting_allowed()) + self.assertEqual(ExitPolicy('accept *:80', 'accept 127.0.0.1:1-65533'), policy.strip_default())

def test_str(self): # sanity test for our __str__ method @@ -268,44 +227,6 @@ class TestExitPolicy(unittest.TestCase): self.assertFalse(policy.can_exit_to('127.0.0.1', 79)) self.assertTrue(policy.can_exit_to('127.0.0.1', 80))

- def test_get_config_policy(self): - test_inputs = { - '': ExitPolicy(), - 'reject *': ExitPolicy('reject *:*'), - 'reject *:*': ExitPolicy('reject *:*'), - 'reject private': ExitPolicy( - 'reject 0.0.0.0/8:*', - 'reject 169.254.0.0/16:*', - 'reject 127.0.0.0/8:*', - 'reject 192.168.0.0/16:*', - 'reject 10.0.0.0/8:*', - 'reject 172.16.0.0/12:*', - 'reject 12.34.56.78:*', - ), - 'accept *:80, reject *': ExitPolicy( - 'accept *:80', - 'reject *:*', - ), - ' accept *:80, reject * ': ExitPolicy( - 'accept *:80', - 'reject *:*', - ), - } - - for test_input, expected in test_inputs.items(): - self.assertEqual(expected, get_config_policy(test_input, '12.34.56.78')) - - test_inputs = ( - 'blarg', - 'accept *:*:*', - 'acceptt *:80', - 'accept 257.0.0.1:80', - 'accept *:999999', - ) - - for test_input in test_inputs: - self.assertRaises(ValueError, get_config_policy, test_input) - def test_pickleability(self): """ Checks that we can unpickle ExitPolicy instances.