[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

[tor-commits] [stem/master] Remove get_config_policy



commit 8dddc2ab8473937bb6853e6f91db8b817a36cdd9
Author: Damian Johnson <atagar@xxxxxxxxxxxxxx>
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.



_______________________________________________
tor-commits mailing list
tor-commits@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits