[tor-commits] [stem/master] ExitPolicy's 'private' keyword handling didn't account for our public address

[atagar at torproject.org]

commit c5e5d1ac2df52631d542d03d2f5c35db6a84f225
Author: Damian Johnson <atagar at torproject.org>
Date:   Fri Sep 5 08:34:43 2014 -0700

    ExitPolicy's 'private' keyword handling didn't account for our public address
    
    Our Controller included a rule for our public ip address, but the exit_policy
    module didn't. Moving this functionality down to that.
---
 stem/control.py                 |    7 +------
 stem/exit_policy.py             |   35 +++++++++++++++++++++++++++--------
 test/unit/exit_policy/policy.py |    3 ++-
 3 files changed, 30 insertions(+), 15 deletions(-)

diff --git a/stem/control.py b/stem/control.py
index 58dd119..1b33659 100644
--- a/stem/control.py
+++ b/stem/control.py
@@ -1033,17 +1033,12 @@ class Controller(BaseController):
           if self.get_conf('ExitPolicyRejectPrivate') == '1':
             policy.append('reject private:*')
 
-            public_addr = self.get_info('address', None)
-
-            if public_addr:
-              policy.append('reject %s:*' % public_addr)
-
           for policy_line in self.get_conf('ExitPolicy', multiple = True):
             policy += policy_line.split(',')
 
           policy += self.get_info('exit-policy/default').split(',')
 
-          config_policy = stem.exit_policy.get_config_policy(policy)
+          config_policy = stem.exit_policy.get_config_policy(policy, self.get_info('address'))
           self._set_cache({'exit_policy': config_policy})
 
         return config_policy
diff --git a/stem/exit_policy.py b/stem/exit_policy.py
index 2cade1d..86722af 100644
--- a/stem/exit_policy.py
+++ b/stem/exit_policy.py
@@ -61,6 +61,9 @@ exiting to a destination is permissible or not. For instance...
   ============ ===========
 """
 
+from __future__ import absolute_import
+
+import socket
 import zlib
 
 import stem.prereq
@@ -92,7 +95,7 @@ PRIVATE_ADDRESSES = (
 )
 
 
-def get_config_policy(rules):
+def get_config_policy(rules, ip_address = None):
   """
   Converts an ExitPolicy found in a torrc to a proper exit pattern. This
   accounts for...
@@ -101,12 +104,17 @@ def get_config_policy(rules):
   * the 'private' keyword
 
   :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)):
+    raise ValueError("%s isn't a valid IP address" % ip_address)
+
   if isinstance(rules, (bytes, unicode)):
     rules = rules.split(',')
 
@@ -125,7 +133,10 @@ def get_config_policy(rules):
       acceptance = rule.split(' ', 1)[0]
       port = rule.split(':', 1)[1]
 
-      for private_addr in PRIVATE_ADDRESSES:
+      if ip_address is None:
+        ip_address = socket.gethostbyname(socket.gethostname())
+
+      for private_addr in PRIVATE_ADDRESSES + (ip_address,):
         result.append(ExitPolicyRule("%s %s:%s" % (acceptance, private_addr, port)))
     else:
       result.append(ExitPolicyRule(rule))
@@ -149,7 +160,7 @@ def _flag_private_rules(rules):
   matches = []
 
   for i, rule in enumerate(rules):
-    if i + len(PRIVATE_ADDRESSES) > len(rules):
+    if i + len(PRIVATE_ADDRESSES) + 1 > len(rules):
       break
 
     rule_str = '%s/%s' % (rule.address, rule.get_masked_bits())
@@ -161,21 +172,29 @@ def _flag_private_rules(rules):
     # To match the private policy the following must all be true...
     #
     #   * series of addresses and bit masks match PRIVATE_ADDRESSES
-    #   * all these rules have the same port range and acceptance
-    #   * all these rules must be either accept or reject entries
+    #   * all rules have the same port range and acceptance
+    #   * all rules have the same acceptance (all accept or reject entries)
+
+    rule_set = rules[start_index:start_index + len(PRIVATE_ADDRESSES) + 1]
+    is_match = True
 
-    rule_set = rules[start_index:start_index + len(PRIVATE_ADDRESSES)]
     min_port, max_port = rule_set[0].min_port, rule_set[0].max_port
     is_accept = rule_set[0].is_accept
-    is_match = True
 
-    for i, rule in enumerate(rule_set):
+    for i, rule in enumerate(rule_set[:-1]):
       rule_str = '%s/%s' % (rule.address, rule.get_masked_bits())
 
       if rule_str != PRIVATE_ADDRESSES[i] or rule.min_port != min_port or rule.max_port != max_port or rule.is_accept != is_accept:
         is_match = False
         break
 
+    # The last rule is for the relay's public address, so it's dynamic.
+
+    last_rule = rule_set[-1]
+
+    if last_rule.is_address_wildcard() or last_rule.min_port != min_port or last_rule.max_port != max_port or last_rule.is_accept != is_accept:
+      is_match = False
+
     if is_match:
       for rule in rule_set:
         rule._is_private = True
diff --git a/test/unit/exit_policy/policy.py b/test/unit/exit_policy/policy.py
index d00954d..b4a35e5 100644
--- a/test/unit/exit_policy/policy.py
+++ b/test/unit/exit_policy/policy.py
@@ -225,6 +225,7 @@ class TestExitPolicy(unittest.TestCase):
         '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',
@@ -237,7 +238,7 @@ class TestExitPolicy(unittest.TestCase):
     }
 
     for test_input, expected in test_inputs.items():
-      self.assertEqual(expected, get_config_policy(test_input))
+      self.assertEqual(expected, get_config_policy(test_input, '12.34.56.78'))
 
     test_inputs = (
       'blarg',