commit 8dddc2ab8473937bb6853e6f91db8b817a36cdd9
Author: Damian Johnson <atagar(a)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.
