[tor-commits] [tor/master] Warn if ports are specified in {Socks, Dir}Policy

[nickm at torproject.org]

commit 4050dfa320e3322beb263bff3afc1db10df5af96
Author: Nick Mathewson <nickm at torproject.org>
Date:   Mon Mar 3 10:45:39 2014 -0500

    Warn if ports are specified in {Socks,Dir}Policy
    
    We have ignored any ports listed here since 80365b989 (0.0.7rc1),
    but we didn't warn the user that we were ignoring them.  This patch
    adds a warning if you put explicit ports in any of the options
    {Socks,Dir}Policy or AuthDir{Reject,Invalid,BadDir,BadExit}.  It
    also adjusts the manpage to say that ports are ignored.
    
    Fixes ticket 11108.
---
 changes/bug11108  |    8 ++++++++
 doc/tor.1.txt     |   11 ++++++++---
 src/or/policies.c |   22 +++++++++++++++-------
 3 files changed, 31 insertions(+), 10 deletions(-)

diff --git a/changes/bug11108 b/changes/bug11108
new file mode 100644
index 0000000..b2c1f50
--- /dev/null
+++ b/changes/bug11108
@@ -0,0 +1,8 @@
+  o Minor features:
+    - Warn the user if they put any ports in the SocksPolicy,
+      DirPolicy, AuthDirReject, AuthDirInvalid, AuthDirBadDir, or
+      AuthDirBadExit options. Fixes ticket #11108.
+
+  o Documentation:
+    - Explain that SocksPolicy, DirPolicy, and their allies don't take
+      port arguments. Fixes ticket #11108.
diff --git a/doc/tor.1.txt b/doc/tor.1.txt
index 2b6de2a..31a56e9 100644
--- a/doc/tor.1.txt
+++ b/doc/tor.1.txt
@@ -1025,7 +1025,7 @@ The following options are useful only for clients (that is, if
 [[SocksPolicy]] **SocksPolicy** __policy__,__policy__,__...__::
     Set an entrance policy for this server, to limit who can connect to the
     SocksPort and DNSPort ports. The policies have the same form as exit
-    policies below.
+    policies below, except that port specifiers are ignored.
 
 [[SocksTimeout]] **SocksTimeout** __NUM__::
     Let a socks connection wait NUM seconds handshaking, and NUM seconds
@@ -1822,7 +1822,8 @@ if DirPort is non-zero):
 
 [[DirPolicy]] **DirPolicy** __policy__,__policy__,__...__::
     Set an entrance policy for this server, to limit who can connect to the
-    directory ports. The policies have the same form as exit policies above.
+    directory ports. The policies have the same form as exit policies above,
+    except that port specifiers are ignored.
 
 [[FetchV2Networkstatus]] **FetchV2Networkstatus** **0**|**1**::
     If set, we try to fetch the (obsolete, unused) version 2 network status
@@ -1866,7 +1867,11 @@ DIRECTORY AUTHORITY SERVER OPTIONS
 [[AuthDirBadDir]] **AuthDirBadDir** __AddressPattern...__::
     Authoritative directories only. A set of address patterns for servers that
     will be listed as bad directories in any network status document this
-    authority publishes, if **AuthDirListBadDirs** is set.
+    authority publishes, if **AuthDirListBadDirs** is set. +
+ +
+    (The address pattern syntax here and in the options below
+    is the same as for exit policies, except that you don't need to say
+    "accept" or "reject", and ports are not needed.)
 
 [[AuthDirBadExit]] **AuthDirBadExit** __AddressPattern...__::
     Authoritative directories only. A set of address patterns for servers that
diff --git a/src/or/policies.c b/src/or/policies.c
index 05377ec..469a203 100644
--- a/src/or/policies.c
+++ b/src/or/policies.c
@@ -482,10 +482,12 @@ validate_addr_policies(const or_options_t *options, char **msg)
  * Ignore port specifiers.
  */
 static int
-load_policy_from_option(config_line_t *config, smartlist_t **policy,
+load_policy_from_option(config_line_t *config, const char *option_name,
+                        smartlist_t **policy,
                         int assume_action)
 {
   int r;
+  int killed_any_ports = 0;
   addr_policy_list_free(*policy);
   *policy = NULL;
   r = parse_addr_policy(config, policy, assume_action);
@@ -504,9 +506,13 @@ load_policy_from_option(config_line_t *config, smartlist_t **policy,
         c = addr_policy_get_canonical_entry(&newp);
         SMARTLIST_REPLACE_CURRENT(*policy, n, c);
         addr_policy_free(n);
+        killed_any_ports = 1;
       }
     } SMARTLIST_FOREACH_END(n);
   }
+  if (killed_any_ports) {
+    log_warn(LD_CONFIG, "Ignoring ports in %s option.", option_name);
+  }
   return 0;
 }
 
@@ -516,20 +522,22 @@ int
 policies_parse_from_options(const or_options_t *options)
 {
   int ret = 0;
-  if (load_policy_from_option(options->SocksPolicy, &socks_policy, -1) < 0)
+  if (load_policy_from_option(options->SocksPolicy, "SocksPolicy",
+                              &socks_policy, -1) < 0)
     ret = -1;
-  if (load_policy_from_option(options->DirPolicy, &dir_policy, -1) < 0)
+  if (load_policy_from_option(options->DirPolicy, "DirPolicy",
+                              &dir_policy, -1) < 0)
     ret = -1;
-  if (load_policy_from_option(options->AuthDirReject,
+  if (load_policy_from_option(options->AuthDirReject, "AuthDirReject",
                               &authdir_reject_policy, ADDR_POLICY_REJECT) < 0)
     ret = -1;
-  if (load_policy_from_option(options->AuthDirInvalid,
+  if (load_policy_from_option(options->AuthDirInvalid, "AuthDirInvalid",
                               &authdir_invalid_policy, ADDR_POLICY_REJECT) < 0)
     ret = -1;
-  if (load_policy_from_option(options->AuthDirBadDir,
+  if (load_policy_from_option(options->AuthDirBadDir, "AuthDirBadDir",
                               &authdir_baddir_policy, ADDR_POLICY_REJECT) < 0)
     ret = -1;
-  if (load_policy_from_option(options->AuthDirBadExit,
+  if (load_policy_from_option(options->AuthDirBadExit, "AuthDirBadExit",
                               &authdir_badexit_policy, ADDR_POLICY_REJECT) < 0)
     ret = -1;
   if (parse_reachable_addresses() < 0)