Parse prop171 options; refactor listener/port option code

Proposal 171 gives us a new syntax for parsing client port options.
You can now have as many FooPort options as you want (for Foo in
Socks, Trans, DNS, NATD), and they can have address:port arguments,
and you can specify the level of isolation on those ports.

Additionally, this patch refactors the client port parsing logic to
use a new type, port_cfg_t.  Previously, ports to be bound were
half-parsed in config.c, and later re-parsed in connection.c when
we're about to bind them.  Now, parsing a port means converting it
into a port_cfg_t, and binding it uses only a port_cfg_t, without
needing to parse the user-provided strings at all.

We should do a related refactoring on other port types.  For
control ports, that'll be easy enough.  For ORPort and DirPort,
we'll want to do this when we solve proposal 118 (letting servers
bind to and advertise multiple ports).

This implements tickets 3514 and 3515.

changes/prop171

@@ -0,0 +1,11 @@
+  o Minor features:
+    - There's a new syntax for specifying multiple client ports (such as
+      SOCKSPort, TransPort, DNSPort, NATDPort): you can now just declare
+      multiple ...Port entries with full addr:port syntax on each.
+      The old ...ListenAddress format is still supported, but you can't
+      mix it with the new SOCKSPort syntax.
+
+  o Code simplifications and refactoring:
+    - Rewrote the listener-selection logic so that parsing which ports
+      we want to listen on is now separate form binding to the ports
+      we want.

src/or/config.c

@@ -240,7 +240,7 @@ static config_var_t _option_vars[] = {
   VAR("DirServer",               LINELIST, DirServers, NULL),
   V(DisableAllSwap,              BOOL,     "0"),
   V(DisableIOCP,                 BOOL,     "1"),
-  V(DNSPort,                     PORT,     "0"),
+  V(DNSPort,                     LINELIST, NULL),
   V(DNSListenAddress,            LINELIST, NULL),
   V(DownloadExtraInfo,           BOOL,     "0"),
   V(EnforceDistinctSubnets,      BOOL,     "1"),
@@ -321,7 +321,7 @@ static config_var_t _option_vars[] = {
   V(NewCircuitPeriod,            INTERVAL, "30 seconds"),
   VAR("NamingAuthoritativeDirectory",BOOL, NamingAuthoritativeDir, "0"),
   V(NATDListenAddress,           LINELIST, NULL),
-  V(NATDPort,                    PORT,     "0"),
+  V(NATDPort,                    LINELIST, NULL),
   V(Nickname,                    STRING,   NULL),
   V(WarnUnsafeSocks,              BOOL,     "1"),
   OBSOLETE("NoPublish"),
@@ -374,7 +374,7 @@ static config_var_t _option_vars[] = {
   V(ShutdownWaitLength,          INTERVAL, "30 seconds"),
   V(SocksListenAddress,          LINELIST, NULL),
   V(SocksPolicy,                 LINELIST, NULL),
-  V(SocksPort,                   PORT,     "9050"),
+  V(SocksPort,                   LINELIST, NULL),
   V(SocksTimeout,                INTERVAL, "2 minutes"),
   OBSOLETE("StatusFetchPeriod"),
   V(StrictNodes,                 BOOL,     "0"),
@@ -385,7 +385,7 @@ static config_var_t _option_vars[] = {
   V(TrackHostExitsExpire,        INTERVAL, "30 minutes"),
   OBSOLETE("TrafficShaping"),
   V(TransListenAddress,          LINELIST, NULL),
-  V(TransPort,                   PORT,     "0"),
+  V(TransPort,                   LINELIST, NULL),
   V(TunnelDirConns,              BOOL,     "1"),
   V(UpdateBridgesFromAuthority,  BOOL,     "0"),
   V(UseBridges,                  BOOL,     "0"),
@@ -577,6 +577,8 @@ static int parse_client_transport_line(const char *line, int validate_only);
 static int parse_dir_server_line(const char *line,
                                  dirinfo_type_t required_type,
                                  int validate_only);
+static int parse_client_ports(const or_options_t *options, int validate_only,
+                              char **msg_out, int *n_ports_out);
 static int validate_data_directory(or_options_t *options);
 static int write_configuration_file(const char *fname,
                                     const or_options_t *options);
@@ -646,6 +648,8 @@ static or_state_t *global_state = NULL;
 static config_line_t *global_cmdline_options = NULL;
 /** Contents of most recently read DirPortFrontPage file. */
 static char *global_dirfrontpagecontents = NULL;
+/** List of port_cfg_t for client-level (SOCKS, DNS, Trans, NATD) ports. */
+static smartlist_t *configured_client_ports = NULL;
 
 /** Return the contents of our frontpage string, or NULL if not configured. */
 const char *
@@ -758,6 +762,13 @@ config_free_all(void)
   config_free_lines(global_cmdline_options);
   global_cmdline_options = NULL;
 
+  if (configured_client_ports) {
+    SMARTLIST_FOREACH(configured_client_ports,
+                      port_cfg_t *, p, tor_free(p));
+    smartlist_free(configured_client_ports);
+    configured_client_ports = NULL;
+  }
+
   tor_free(torrc_fname);
   tor_free(_version);
   tor_free(global_dirfrontpagecontents);
@@ -3027,6 +3038,7 @@ options_validate(or_options_t *old_options, or_options_t *options,
   int i;
   config_line_t *cl;
   const char *uname = get_uname();
+  int n_client_ports=0;
 #define REJECT(arg) \
   STMT_BEGIN *msg = tor_strdup(arg); return -1; STMT_END
 #define COMPLAIN(arg) STMT_BEGIN log(LOG_WARN, LD_CONFIG, arg); STMT_END
@@ -3050,57 +3062,8 @@ options_validate(or_options_t *old_options, or_options_t *options,
   if (options->DirPort == 0 && options->DirListenAddress != NULL)
     REJECT("DirPort must be defined if DirListenAddress is defined.");
 
-  if (options->DNSPort == 0 && options->DNSListenAddress != NULL)
-    REJECT("DNSPort must be defined if DNSListenAddress is defined.");
-
-  if (options->ControlPort == 0 && options->ControlListenAddress != NULL)
-    REJECT("ControlPort must be defined if ControlListenAddress is defined.");
-
-  if (options->TransPort == 0 && options->TransListenAddress != NULL)
-    REJECT("TransPort must be defined if TransListenAddress is defined.");
-
-  if (options->NATDPort == 0 && options->NATDListenAddress != NULL)
-    REJECT("NATDPort must be defined if NATDListenAddress is defined.");
-
-  /* Don't gripe about SocksPort 0 with SocksListenAddress set; a standard
-   * configuration does this. */
-
-  for (i = 0; i < 3; ++i) {
-    int is_socks = i==0;
-    int is_trans = i==1;
-    config_line_t *line, *opt, *old;
-    const char *tp;
-    if (is_socks) {
-      opt = options->SocksListenAddress;
-      old = old_options ? old_options->SocksListenAddress : NULL;
-      tp = "SOCKS proxy";
-    } else if (is_trans) {
-      opt = options->TransListenAddress;
-      old = old_options ? old_options->TransListenAddress : NULL;
-      tp = "transparent proxy";
-    } else {
-      opt = options->NATDListenAddress;
-      old = old_options ? old_options->NATDListenAddress : NULL;
-      tp = "natd proxy";
-    }
-
-    for (line = opt; line; line = line->next) {
-      char *address = NULL;
-      uint16_t port;
-      uint32_t addr;
-      if (parse_addr_port(LOG_WARN, line->value, &address, &addr, &port)<0)
-        continue; /* We'll warn about this later. */
-      if (!is_internal_IP(addr, 1) &&
-          (!old_options || !config_lines_eq(old, opt))) {
-        log_warn(LD_CONFIG,
-             "You specified a public address '%s' for a %s. Other "
-             "people on the Internet might find your computer and use it as "
-             "an open %s. Please don't allow this unless you have "
-             "a good reason.", address, tp, tp);
-      }
-      tor_free(address);
-    }
-  }
+  if (parse_client_ports(options, 1, msg, &n_client_ports) < 0)
+    return -1;
 
   if (validate_data_directory(options)<0)
     REJECT("Invalid DataDirectory");
@@ -3142,9 +3105,7 @@ options_validate(or_options_t *old_options, or_options_t *options,
     REJECT("Can't use a relative path to torrc when RunAsDaemon is set.");
 #endif
 
-  if (options->SocksPort == 0 && options->TransPort == 0 &&
-      options->NATDPort == 0 && options->ORPort == 0 &&
-      options->DNSPort == 0 && !options->RendConfigLines)
+  if (n_client_ports == 0 && options->ORPort == 0 && !options->RendConfigLines)
     log(LOG_WARN, LD_CONFIG,
         "SocksPort, TransPort, NATDPort, DNSPort, and ORPort are all "
         "undefined, and there aren't any hidden services configured.  "
@@ -4885,6 +4846,355 @@ parse_dir_server_line(const char *line, dirinfo_type_t required_type,
   return r;
 }
 
+/** Warn for every port in <b>ports</b> that is not on a loopback address. */
+static void
+warn_nonlocal_client_ports(const smartlist_t *ports, const char *portname)
+{
+  SMARTLIST_FOREACH_BEGIN(ports, const port_cfg_t *, port) {
+    if (!tor_addr_is_loopback(&port->addr)) {
+      log_warn(LD_CONFIG, "You specified a public address for %sPort. "
+               "Other people on the Internet might find your computer and "
+               "use it as an open proxy. Please don't allow this unless you "
+               "have a good reason.", portname);
+    }
+  } SMARTLIST_FOREACH_END(port);
+}
+
+#define CL_PORT_NO_OPTIONS    (1u<<0)
+#define CL_PORT_WARN_NONLOCAL (1u<<1)
+#define CL_PORT_ALLOW_EXTRA_LISTENADDR (1u<<2)
+
+/**
+ * Parse port configuration for a single client port type.
+ *
+ * Read entries of the "FooPort" type from the list <b>ports</b>, and
+ * entries of the "FooListenAddress" type from the list
+ * <b>listenaddrs</b>.  Two syntaxes are supported: a legacy syntax
+ * where FooPort is at most a single entry containing a port number and
+ * where FooListenAddress has any number of address:port combinations;
+ * and a new syntax where there are no FooListenAddress entries and
+ * where FooPort can have any number of entries of the format
+ * "[Address:][Port] IsolationOptions".
+ *
+ * In log messages, describe the port type as <b>portname</b>.
+ *
+ * If no address is specified, default to <b>defaultaddr</b>.  If no
+ * FooPort is given, default to defaultport (if 0, there is no default).
+ *
+ * If CL_PORT_NO_OPTIONS is set in <b>flags</b>, do not allow stream
+ * isolation options in the FooPort entries.
+ *
+ * If CL_PORT_WARN_NONLOCAL is set in <b>flags</b>, warn if any of the
+ * ports are not on a local address.
+ *
+ * Unless CL_PORT_ALLOW_EXTRA_LISTENADDR is set in <b>flags</b>, warn
+ * if FooListenAddress is set but FooPort is 0.
+ *
+ * On success, if <b>out</b> is given, add a new port_cfg_t entry to
+ * <b>out</b> for every port that the client should listen on.  Return 0
+ * on success, -1 on failure.
+ */
+static int
+parse_client_port_config(smartlist_t *out,
+                         const config_line_t *ports,
+                         const config_line_t *listenaddrs,
+                         const char *portname,
+                         int listener_type,
+                         const char *defaultaddr,
+                         int defaultport,
+                         unsigned flags)
+{
+  smartlist_t *elts;
+  int retval = -1;
+  const unsigned allow_client_options = !(flags & CL_PORT_NO_OPTIONS);
+  const unsigned warn_nonlocal = flags & CL_PORT_WARN_NONLOCAL;
+  const unsigned allow_spurious_listenaddr =
+    flags & CL_PORT_ALLOW_EXTRA_LISTENADDR;
+
+  /* FooListenAddress is deprecated; let's make it work like it used to work,
+   * though. */
+  if (listenaddrs) {
+    int mainport = defaultport;
+
+   if (ports && ports->next) {
+      log_warn(LD_CONFIG, "%sListenAddress can't be used when there are "
+               "multiple %sPort lines", portname, portname);
+      return -1;
+   } else if (ports) {
+     if (!strcmp(ports->value, "auto")) {
+       mainport = CFG_AUTO_PORT;
+     } else {
+       int ok;
+       mainport = (int)tor_parse_long(ports->value, 10, 0, 65535, &ok, NULL);
+       if (!ok) {
+         log_warn(LD_CONFIG, "%sListenAddress can only be used with a single "
+                  "%sPort with value \"auto\" or 65535.", portname, portname);
+         return -1;
+       }
+     }
+   }
+
+   if (mainport == 0) {
+     if (allow_spurious_listenaddr)
+       return 1;
+     log_warn(LD_CONFIG, "%sPort must be defined if %sListenAddress is used",
+              portname, portname);
+     return -1;
+   }
+
+   for (; listenaddrs; listenaddrs = listenaddrs->next) {
+     tor_addr_t addr;
+     uint16_t port = 0;
+     if (tor_addr_port_parse(listenaddrs->value, &addr, &port) < 0) {
+       log_warn(LD_CONFIG, "Unable to parse %sListenAddress '%s'",
+                portname, listenaddrs->value);
+       return -1;
+     }
+     if (out) {
+       port_cfg_t *cfg = tor_malloc_zero(sizeof(port_cfg_t));
+       cfg->type = listener_type;
+       cfg->port = port ? port : defaultport;
+       tor_addr_copy(&cfg->addr, &addr);
+       cfg->sessiongroup = -1;
+       cfg->isolate = ISO_DEFAULT;
+       smartlist_add(out, cfg);
+     }
+   }
+
+   if (warn_nonlocal && out)
+     warn_nonlocal_client_ports(out, portname);
+   return 0;
+  } /* end if (listenaddrs) */
+
+  /* No ListenAddress lines. If there's no FooPort, then maybe make a default
+   * one. */
+  if (! ports) {
+    if (defaultport && out) {
+       port_cfg_t *cfg = tor_malloc_zero(sizeof(port_cfg_t));
+       cfg->type = listener_type;
+       cfg->port = defaultport;
+       tor_addr_from_str(&cfg->addr, defaultaddr);
+       cfg->sessiongroup = -1;
+       cfg->isolate = ISO_DEFAULT;
+       smartlist_add(out, cfg);
+    }
+    return 0;
+  }
+
+  /* At last we can actually parse the FooPort lines.  The syntax is:
+   * [Addr:](Port|auto) [Options].*/
+  elts = smartlist_create();
+
+  for (; ports; ports = ports->next) {
+    tor_addr_t addr;
+    int port;
+    int sessiongroup = -1;
+    unsigned isolation = ISO_DEFAULT;
+
+    char *addrport;
+    uint16_t ptmp=0;
+    int ok;
+
+    smartlist_split_string(elts, ports->value, NULL,
+                           SPLIT_SKIP_SPACE|SPLIT_IGNORE_BLANK, 0);
+    if (smartlist_len(elts) == 0) {
+      log_warn(LD_CONFIG, "Invalid %sPort line with no value", portname);
+      goto err;
+    }
+
+    if (!allow_client_options && smartlist_len(elts) > 1) {
+      log_warn(LD_CONFIG, "Too many options on %sPort line", portname);
+      goto err;
+    }
+
+    /* Now parse the addr/port value */
+    addrport = smartlist_get(elts, 0);
+    if (!strcmp(addrport, "auto")) {
+      port = CFG_AUTO_PORT;
+      tor_addr_from_str(&addr, defaultaddr);
+    } else if (!strcasecmpend(addrport, ":auto")) {
+      char *addrtmp = tor_strndup(addrport, strlen(addrport)-5);
+      port = CFG_AUTO_PORT;
+      if (tor_addr_port_parse(addrtmp, &addr, &ptmp)<0 || ptmp) {
+        log_warn(LD_CONFIG, "Invalid address '%s' for %sPort",
+                 escaped(addrport), portname);
+        tor_free(addrtmp);
+        goto err;
+      }
+    } else if (tor_addr_port_parse(addrport, &addr, &ptmp) == 0) {
+      if (ptmp == 0) {
+        log_warn(LD_CONFIG, "%sPort line has address but no port", portname);
+        goto err;
+      }
+      port = ptmp;
+    } else {
+      port = (int) tor_parse_long(addrport, 10, 0, 65535, &ok, NULL);
+      if (!ok) {
+        log_warn(LD_CONFIG, "Couldn't parse address '%s' for %sPort",
+                 escaped(addrport), portname);
+        goto err;
+      }
+      tor_addr_from_str(&addr, defaultaddr);
+    }
+
+    /* Now parse the rest of the options, if any. */
+    SMARTLIST_FOREACH_BEGIN(elts, char *, elt) {
+      int no = 0, isoflag = 0;
+      const char *elt_orig = elt;
+      if (elt_sl_idx == 0)
+        continue; /* Skip addr:port */
+      if (!strcasecmpstart(elt, "SessionGroup=")) {
+        int group = tor_parse_long(elt+strlen("SessionGroup="),
+                                   10, 0, INT_MAX, &ok, NULL);
+        if (!ok) {
+          log_warn(LD_CONFIG, "Invalid %sPort option '%s'",
+                   portname, escaped(elt));
+          goto err;
+        }
+        if (sessiongroup >= 0) {
+          log_warn(LD_CONFIG, "Multiple SessionGroup options on %sPort",
+                   portname);
+          goto err;
+        }
+        sessiongroup = group;
+        continue;
+      }
+
+      if (!strcasecmpstart(elt, "No")) {
+        no = 1;
+        elt += 2;
+      }
+      if (!strcasecmpend(elt, "s"))
+        elt[strlen(elt)-1] = '\0'; /* kill plurals. */
+
+      if (!strcasecmp(elt, "IsolateDestPort")) {
+        isoflag = ISO_DESTPORT;
+      } else if (!strcasecmp(elt, "IsolateDestAddr")) {
+        isoflag = ISO_DESTADDR;
+      } else if (!strcasecmp(elt, "IsolateSOCKSAuth")) {
+        isoflag = ISO_SOCKSAUTH;
+      } else if (!strcasecmp(elt, "IsolateClientProtocol")) {
+        isoflag = ISO_CLIENTPROTO;
+      } else if (!strcasecmp(elt, "IsolateClientAddr")) {
+        isoflag = ISO_CLIENTADDR;
+      } else {
+        log_warn(LD_CONFIG, "Unrecognized %sPort option '%s'",
+                 portname, escaped(elt_orig));
+      }
+
+      if (no) {
+        isolation &= ~isoflag;
+      } else {
+        isolation |= isoflag;
+      }
+    } SMARTLIST_FOREACH_END(elt);
+
+    if (out) {
+      port_cfg_t *cfg = tor_malloc_zero(sizeof(port_cfg_t));
+      cfg->type = listener_type;
+      cfg->port = port;
+      tor_addr_copy(&cfg->addr, &addr);
+      cfg->sessiongroup = sessiongroup;
+      cfg->isolate = isolation;
+      smartlist_add(out, cfg);
+    }
+    SMARTLIST_FOREACH(elts, char *, cp, tor_free(cp));
+    smartlist_clear(elts);
+  }
+
+  if (warn_nonlocal && out)
+    warn_nonlocal_client_ports(out, portname);
+
+  retval = 0;
+ err:
+  SMARTLIST_FOREACH(elts, char *, cp, tor_free(cp));
+  smartlist_free(elts);
+  return retval;
+}
+
+/** Parse all client port types (Socks, DNS, Trans, NATD) from
+ * <b>options</b>. On success, set *<b>n_ports_out</b> to the number of
+ * ports that are listed and return 0.  On failure, set *<b>msg</b> to a
+ * description of the problem and return -1.
+ *
+ * If <b>validate_only</b> is false, set configured_client_ports to the
+ * new list of ports parsed from <b>options</b>.
+ **/
+static int
+parse_client_ports(const or_options_t *options, int validate_only,
+                   char **msg, int *n_ports_out)
+{
+  smartlist_t *ports;
+  int retval = -1;
+
+  ports = smartlist_create();
+
+  *n_ports_out = 0;
+
+  if (parse_client_port_config(ports,
+             options->SocksPort, options->SocksListenAddress,
+             "Socks", CONN_TYPE_AP_LISTENER,
+             "127.0.0.1", 9050,
+             CL_PORT_WARN_NONLOCAL|CL_PORT_ALLOW_EXTRA_LISTENADDR) < 0) {
+    *msg = tor_strdup("Invalid SocksPort/SocksListenAddress configuration");
+    goto err;
+  }
+  if (parse_client_port_config(ports,
+                               options->DNSPort, options->DNSListenAddress,
+                               "DNS", CONN_TYPE_AP_DNS_LISTENER,
+                               "127.0.0.1", 0,
+                               CL_PORT_WARN_NONLOCAL) < 0) {
+    *msg = tor_strdup("Invalid DNSPort/DNSListenAddress configuration");
+    goto err;
+  }
+  if (parse_client_port_config(ports,
+                               options->TransPort, options->TransListenAddress,
+                               "Trans", CONN_TYPE_AP_TRANS_LISTENER,
+                               "127.0.0.1", 0,
+                               CL_PORT_WARN_NONLOCAL) < 0) {
+    *msg = tor_strdup("Invalid TransPort/TransListenAddress configuration");
+    goto err;
+  }
+  if (parse_client_port_config(ports,
+                               options->NATDPort, options->NATDListenAddress,
+                               "NATD", CONN_TYPE_AP_NATD_LISTENER,
+                               "127.0.0.1", 0,
+                               CL_PORT_WARN_NONLOCAL) < 0) {
+    *msg = tor_strdup("Invalid NatdPort/NatdListenAddress configuration");
+    goto err;
+  }
+
+  *n_ports_out = smartlist_len(ports);
+
+  if (!validate_only) {
+    if (configured_client_ports) {
+      SMARTLIST_FOREACH(configured_client_ports,
+                        port_cfg_t *, p, tor_free(p));
+      smartlist_free(configured_client_ports);
+    }
+    configured_client_ports = ports;
+    ports = NULL; /* prevent free below. */
+  }
+
+  retval = 0;
+ err:
+  if (ports) {
+    SMARTLIST_FOREACH(ports, port_cfg_t *, p, tor_free(p));
+    smartlist_free(ports);
+  }
+  return retval;
+}
+
+/** Return a list of port_cfg_t for client ports parsed from the
+ * options. */
+const smartlist_t *
+get_configured_client_ports(void)
+{
+  if (!configured_client_ports)
+    configured_client_ports = smartlist_create();
+  return configured_client_ports;
+}
+
 /** Adjust the value of options->DataDirectory, or fill it in if it's
  * absent. Return 0 on success, -1 on failure. */
 static int

src/or/config.h

@@ -64,6 +64,8 @@ or_state_t *get_or_state(void);
 int did_last_state_file_write_fail(void);
 int or_state_save(time_t now);
 
+const smartlist_t *get_configured_client_ports(void);
+
 int options_need_geoip_info(const or_options_t *options,
                             const char **reason_out);
 int getinfo_helper_config(control_connection_t *conn,

src/or/connection.c

@@ -701,48 +701,6 @@ connection_expire_held_open(void)
   });
 }
 
-/** Create an AF_INET listenaddr struct.
- * <b>listenaddress</b> provides the host and optionally the port information
- * for the new structure.  If no port is provided in <b>listenaddress</b> then
- * <b>listenport</b> is used.
- *
- * If not NULL <b>readable_address</b> will contain a copy of the host part of
- * <b>listenaddress</b>.
- *
- * The listenaddr struct has to be freed by the caller.
- */
-static struct sockaddr_in *
-create_inet_sockaddr(const char *listenaddress, int listenport,
-                     char **readable_address, socklen_t *socklen_out) {
-  struct sockaddr_in *listenaddr = NULL;
-  uint32_t addr;
-  uint16_t usePort = 0;
-
-  if (parse_addr_port(LOG_WARN,
-                      listenaddress, readable_address, &addr, &usePort)<0) {
-    log_warn(LD_CONFIG,
-             "Error parsing/resolving ListenAddress %s", listenaddress);
-    goto err;
-  }
-  if (usePort==0) {
-    if (listenport != CFG_AUTO_PORT)
-      usePort = listenport;
-  }
-
-  listenaddr = tor_malloc_zero(sizeof(struct sockaddr_in));
-  listenaddr->sin_addr.s_addr = htonl(addr);
-  listenaddr->sin_family = AF_INET;
-  listenaddr->sin_port = htons((uint16_t) usePort);
-
-  *socklen_out = sizeof(struct sockaddr_in);
-
-  return listenaddr;
-
- err:
-  tor_free(listenaddr);
-  return NULL;
-}
-
 #ifdef HAVE_SYS_UN_H
 /** Create an AF_UNIX listenaddr struct.
  * <b>listenaddress</b> provides the path to the Unix socket.
@@ -1741,6 +1699,111 @@ connection_read_proxy_handshake(connection_t *conn)
   return ret;
 }
 
+/** Given a list of listener connections in <b>old_conns</b>, and list of
+ * port_cfg_t entries in <b>ports</b>, open a new listener for every port in
+ * <b>ports</b> that does not already have a listener in <b>old_conns</b>.
+ *
+ * Remove from <b>old_conns</b> every connection that has a corresponding
+ * entry in <b>ports</b>.  Add to <b>new_conns</b> new every connection we
+ * launch.
+ *
+ * Return 0 on success, -1 on failure.
+ **/
+static int
+retry_listener_ports(smartlist_t *old_conns,
+                     const smartlist_t *ports,
+                     smartlist_t *new_conns)
+{
+  smartlist_t *launch = smartlist_create();
+  int r = 0;
+
+  smartlist_add_all(launch, ports);
+
+  /* Iterate through old_conns, comparing it to launch: remove from both lists
+   * each pair of elements that corresponds to the same port. */
+  SMARTLIST_FOREACH_BEGIN(old_conns, connection_t *, conn) {
+    const port_cfg_t *found_port = NULL;
+
+    /* Okay, so this is a listener.  Is it configured? */
+    SMARTLIST_FOREACH_BEGIN(launch, const port_cfg_t *, wanted) {
+      if (conn->type != wanted->type)
+        continue;
+      if ((conn->socket_family != AF_UNIX && wanted->is_unix_addr) ||
+          (conn->socket_family == AF_UNIX && ! wanted->is_unix_addr))
+        continue;
+
+      if (wanted->is_unix_addr) {
+        if (conn->socket_family == AF_UNIX &&
+            !strcmp(wanted->unix_addr, conn->address)) {
+          found_port = wanted;
+          break;
+        }
+      } else {
+        int port_matches;
+        if (wanted->port == CFG_AUTO_PORT) {
+          port_matches = 1;
+        } else {
+          port_matches = (wanted->port == conn->port);
+        }
+        if (port_matches && tor_addr_eq(&wanted->addr, &conn->addr)) {
+          found_port = wanted;
+          break;
+        }
+      }
+    } SMARTLIST_FOREACH_END(wanted);
+
+    if (found_port) {
+      /* This listener is already running; we don't need to launch it. */
+//      log_debug(LD_NET, "Already have %s on %s:%d",
+//                conn_type_to_string(type), conn->address, conn->port);
+      smartlist_remove(launch, found_port);
+      /* And we can remove the connection from old_conns too. */
+      SMARTLIST_DEL_CURRENT(old_conns, conn);
+    }
+  } SMARTLIST_FOREACH_END(conn);
+
+  /* Now open all the listeners that are configured but not opened. */
+  SMARTLIST_FOREACH_BEGIN(launch, const port_cfg_t *, port) {
+    struct sockaddr *listensockaddr;
+    socklen_t listensocklen = 0;
+    char *address;
+    connection_t *conn;
+
+    if (port->is_unix_addr) {
+      listensockaddr = (struct sockaddr *)
+        create_unix_sockaddr(port->unix_addr,
+                             &address, &listensocklen);
+    } else {
+      listensockaddr = tor_malloc(sizeof(struct sockaddr_storage));
+      listensocklen = tor_addr_to_sockaddr(&port->addr,
+                                           port->port,
+                                           listensockaddr,
+                                           sizeof(struct sockaddr_storage));
+      address = tor_dup_addr(&port->addr);
+    }
+
+    if (listensockaddr) {
+      conn = connection_create_listener(listensockaddr, listensocklen,
+                                        port->type, address);
+      tor_free(listensockaddr);
+      tor_free(address);
+    } else {
+      conn = NULL;
+    }
+
+    if (!conn) {
+      r = -1;
+    } else {
+      if (new_conns)
+        smartlist_add(new_conns, conn);
+    }
+  } SMARTLIST_FOREACH_END(port);
+
+  smartlist_free(launch);
+
+  return r;
+}
+
 /**
  * Launch any configured listener connections of type <b>type</b>.  (A
  * listener is configured if <b>port_option</b> is non-zero.  If any
@@ -1748,168 +1811,73 @@ connection_read_proxy_handshake(connection_t *conn)
  * connection binding to each one.  Otherwise, create a single
  * connection binding to the address <b>default_addr</b>.)
  *
- * Only launch the listeners of this type that are not already open, and
- * only close listeners that are no longer wanted.  Existing listeners
- * that are still configured are not touched.
+ * We assume that we're starting with a list of existing listener connection_t
+ * pointers in <b>old_conns</b>: we do not launch listeners that are already
+ * in that list.  Instead, we just remove them from the list.
  *
- * If <b>disable_all_conns</b> is set, then never open new conns, and
- * close the existing ones.
- *
- * Add all old conns that should be closed to <b>replaced_conns</b>.
- * Add all new connections to <b>new_conns</b>.
+ * All new connections we launch are added to <b>new_conns</b>.
  */
 static int
-retry_listeners(int type, config_line_t *cfg,
+retry_listeners(smartlist_t *old_conns,
+                int type, const config_line_t *cfg,
                 int port_option, const char *default_addr,
-                smartlist_t *replaced_conns,
                 smartlist_t *new_conns,
-                int disable_all_conns,
-                int socket_family)
+                int is_sockaddr_un)
 {
-  smartlist_t *launch = smartlist_create(), *conns;
-  int free_launch_elts = 1;
-  int r;
-  config_line_t *c;
-  connection_t *conn;
-  config_line_t *line;
+  smartlist_t *ports = smartlist_create();
+  tor_addr_t dflt_addr;
+  int retval = 0;
 
-  tor_assert(socket_family == AF_INET || socket_family == AF_UNIX);
-
-  if (cfg && port_option) {
-    for (c = cfg; c; c = c->next) {
-      smartlist_add(launch, c);
-    }
-    free_launch_elts = 0;
-  } else if (port_option) {
-    line = tor_malloc_zero(sizeof(config_line_t));
-    line->key = tor_strdup("");
-    line->value = tor_strdup(default_addr);
-    smartlist_add(launch, line);
+  if (default_addr) {
+    tor_addr_from_str(&dflt_addr, default_addr);
+  } else {
+    tor_addr_make_unspec(&dflt_addr);
   }
 
-  /*
-  SMARTLIST_FOREACH(launch, config_line_t *, l,
-                    log_fn(LOG_NOTICE, "#%s#%s", l->key, l->value));
-  */
-
-  conns = get_connection_array();
-  SMARTLIST_FOREACH(conns, connection_t *, conn,
-  {
-    if (conn->type != type ||
-        conn->socket_family != socket_family ||
-        conn->marked_for_close)
-      continue;
-    /* Okay, so this is a listener.  Is it configured? */
-    line = NULL;
-    SMARTLIST_FOREACH(launch, config_line_t *, wanted,
-      {
-        char *address=NULL;
-        uint16_t port;
-        switch (socket_family) {
-          case AF_INET:
-            if (!parse_addr_port(LOG_WARN,
-                                 wanted->value, &address, NULL, &port)) {
-              int addr_matches = !strcasecmp(address, conn->address);
-              int port_matches;
-              tor_free(address);
-              if (port) {
-                /* The Listener line has a port */
-                port_matches = (port == conn->port);
-              } else if (port_option == CFG_AUTO_PORT) {
-                /* The Listener line has no port, and the Port line is "auto".
-                 * "auto" matches anything; transitions from any port to
-                 * "auto" succeed. */
-                port_matches = 1;
-              } else {
-                /*  The Listener line has no port, and the Port line is "auto".
-                 * "auto" matches anything; transitions from any port to
-                 * "auto" succeed. */
-                port_matches = (port_option == conn->port);
-              }
-              if (port_matches  && addr_matches) {
-                line = wanted;
-                break;
-              }
-            }
-            break;
-          case AF_UNIX:
-            if (!strcasecmp(wanted->value, conn->address)) {
-              line = wanted;
-              break;
-            }
-            break;
-          default:
-            tor_assert(0);
-        }
-      });
-    if (!line || disable_all_conns) {
-      /* This one isn't configured. Close it. */
-      log_notice(LD_NET, "Closing no-longer-configured %s on %s:%d",
-                 conn_type_to_string(type), conn->address, conn->port);
-      if (replaced_conns) {
-        smartlist_add(replaced_conns, conn);
-      } else {
-        connection_close_immediate(conn);
-        connection_mark_for_close(conn);
-      }
+  if (port_option) {
+    if (!cfg) {
+      port_cfg_t *port = tor_malloc_zero(sizeof(port_cfg_t));
+      tor_addr_copy(&port->addr, &dflt_addr);
+      port->port = port_option;
+      port->type = type;
+      smartlist_add(ports, port);
     } else {
-      /* It's configured; we don't need to launch it. */
-//      log_debug(LD_NET, "Already have %s on %s:%d",
-//                conn_type_to_string(type), conn->address, conn->port);
-      smartlist_remove(launch, line);
-      if (free_launch_elts)
-        config_free_lines(line);
-    }
-  });
-
-  /* Now open all the listeners that are configured but not opened. */
-  r = 0;
-  if (!disable_all_conns) {
-    SMARTLIST_FOREACH_BEGIN(launch, config_line_t *, cfg_line) {
-        char *address = NULL;
-        struct sockaddr *listensockaddr;
-        socklen_t listensocklen = 0;
-
-        switch (socket_family) {
-          case AF_INET:
-            listensockaddr = (struct sockaddr *)
-                             create_inet_sockaddr(cfg_line->value,
-                                                  port_option,
-                                                  &address, &listensocklen);
-            break;
-          case AF_UNIX:
-            listensockaddr = (struct sockaddr *)
-                             create_unix_sockaddr(cfg_line->value,
-                                                  &address, &listensocklen);
-            break;
-          default:
-            tor_assert(0);
-        }
-
-        if (listensockaddr) {
-          conn = connection_create_listener(listensockaddr, listensocklen,
-                                            type, address);
-          tor_free(listensockaddr);
-          tor_free(address);
-        } else
-          conn = NULL;
-
-        if (!conn) {
-          r = -1;
+      const config_line_t *c;
+      for (c = cfg; c; c = c->next) {
+        port_cfg_t *port;
+        tor_addr_t addr;
+        uint16_t portval = 0;
+        if (is_sockaddr_un) {
+          size_t len = strlen(c->value);
+          port = tor_malloc_zero(sizeof(port_cfg_t) + len + 1);
+          port->is_unix_addr = 1;
+          memcpy(port->unix_addr, c->value, len+1);
         } else {
-          if (new_conns)
-            smartlist_add(new_conns, conn);
+          if (tor_addr_port_parse(c->value, &addr, &portval) < 0) {
+            log_warn(LD_CONFIG, "Can't parse/resolve %s %s",
+                     c->key, c->value);
+            retval = -1;
+            continue;
+          }
+          port = tor_malloc_zero(sizeof(port_cfg_t));
+          tor_addr_copy(&port->addr, &addr);
         }
-    } SMARTLIST_FOREACH_END(cfg_line);
+        port->type = type;
+        port->port = portval ? portval : port_option;
+        smartlist_add(ports, port);
+      }
+    }
   }
 
-  if (free_launch_elts) {
-    SMARTLIST_FOREACH(launch, config_line_t *, cfg_line,
-                      config_free_lines(cfg_line));
-  }
-  smartlist_free(launch);
+  if (retval == -1)
+    goto cleanup;
 
-  return r;
+  retval = retry_listener_ports(old_conns, ports, new_conns);
+
+ cleanup:
+  SMARTLIST_FOREACH(ports, port_cfg_t *, p, tor_free(p));
+  smartlist_free(ports);
+  return retval;
 }
 
 /** Launch listeners for each port you should have open.  Only launch
@@ -1923,54 +1891,62 @@ int
 retry_all_listeners(smartlist_t *replaced_conns,
                     smartlist_t *new_conns)
 {
+  smartlist_t *listeners = smartlist_create();
   const or_options_t *options = get_options();
   int retval = 0;
   const uint16_t old_or_port = router_get_advertised_or_port(options);
   const uint16_t old_dir_port = router_get_advertised_dir_port(options, 0);
 
-  if (retry_listeners(CONN_TYPE_OR_LISTENER, options->ORListenAddress,
-                      options->ORPort, "0.0.0.0",
-                      replaced_conns, new_conns, options->ClientOnly,
-                      AF_INET)<0)
+  SMARTLIST_FOREACH_BEGIN(get_connection_array(), connection_t *, conn) {
+    if (connection_is_listener(conn) && !conn->marked_for_close)
+      smartlist_add(listeners, conn);
+  } SMARTLIST_FOREACH_END(conn);
+
+  if (! options->ClientOnly) {
+    if (retry_listeners(listeners,
+                        CONN_TYPE_OR_LISTENER, options->ORListenAddress,
+                        options->ORPort, "0.0.0.0",
+                        new_conns, 0) < 0)
+      retval = -1;
+    if (retry_listeners(listeners,
+                        CONN_TYPE_DIR_LISTENER, options->DirListenAddress,
+                        options->DirPort, "0.0.0.0",
+                        new_conns, 0) < 0)
+      retval = -1;
+  }
+
+  if (retry_listener_ports(listeners,
+                           get_configured_client_ports(),
+                           new_conns) < 0)
     retval = -1;
-  if (retry_listeners(CONN_TYPE_DIR_LISTENER, options->DirListenAddress,
-                      options->DirPort, "0.0.0.0",
-                      replaced_conns, new_conns, options->ClientOnly,
-                      AF_INET)<0)
-    retval = -1;
-  if (retry_listeners(CONN_TYPE_AP_LISTENER, options->SocksListenAddress,
-                      options->SocksPort, "127.0.0.1",
-                      replaced_conns, new_conns, 0,
-                      AF_INET)<0)
-    retval = -1;
-  if (retry_listeners(CONN_TYPE_AP_TRANS_LISTENER, options->TransListenAddress,
-                      options->TransPort, "127.0.0.1",
-                      replaced_conns, new_conns, 0,
-                      AF_INET)<0)
-    retval = -1;
-  if (retry_listeners(CONN_TYPE_AP_NATD_LISTENER, options->NATDListenAddress,
-                      options->NATDPort, "127.0.0.1",
-                      replaced_conns, new_conns, 0,
-                      AF_INET)<0)
-    retval = -1;
-  if (retry_listeners(CONN_TYPE_AP_DNS_LISTENER, options->DNSListenAddress,
-                      options->DNSPort, "127.0.0.1",
-                      replaced_conns, new_conns, 0,
-                      AF_INET)<0)
-    retval = -1;
-  if (retry_listeners(CONN_TYPE_CONTROL_LISTENER,
+  if (retry_listeners(listeners,
+                      CONN_TYPE_CONTROL_LISTENER,
                       options->ControlListenAddress,
                       options->ControlPort, "127.0.0.1",
-                      replaced_conns, new_conns, 0,
-                      AF_INET)<0)
+                      new_conns, 0) < 0)
     return -1;
-  if (retry_listeners(CONN_TYPE_CONTROL_LISTENER,
+  if (retry_listeners(listeners,
+                      CONN_TYPE_CONTROL_LISTENER,
                       options->ControlSocket,
                       options->ControlSocket ? 1 : 0, NULL,
-                      replaced_conns, new_conns, 0,
-                      AF_UNIX)<0)
+                      new_conns, 1) < 0)
     return -1;
 
+  /* Any members that were still in 'listeners' don't correspond to
+   * any configured port.  Kill 'em. */
+  SMARTLIST_FOREACH_BEGIN(listeners, connection_t *, conn) {
+    log_notice(LD_NET, "Closing no-longer-configured %s on %s:%d",
+               conn_type_to_string(conn->type), conn->address, conn->port);
+    if (replaced_conns) {
+      smartlist_add(replaced_conns, conn);
+    } else {
+      connection_close_immediate(conn);
+      connection_mark_for_close(conn);
+    }
+  } SMARTLIST_FOREACH_END(conn);
+
+  smartlist_free(listeners);
+
   if (old_or_port != router_get_advertised_or_port(options) ||
       old_dir_port != router_get_advertised_dir_port(options, 0)) {
     /* Our chosen ORPort or DirPort is not what it used to be: the

src/or/or.h

@@ -2542,6 +2542,47 @@ typedef enum invalid_router_usage_t {
 #define MIN_CONSTRAINED_TCP_BUFFER 2048
 #define MAX_CONSTRAINED_TCP_BUFFER 262144  /* 256k */
 
+/** @name Isolation flags
+
+    Ways to isolate client streams
+
+    @{
+*/
+/** Isolate based on destination port */
+#define ISO_DESTPORT    (1u<<0)
+/** Isolate based on destination address */
+#define ISO_DESTADDR    (1u<<1)
+/** Isolate based on SOCKS authentication */
+#define ISO_SOCKSAUTH   (1u<<2)
+/** Isolate based on client protocol choice */
+#define ISO_CLIENTPROTO (1u<<3)
+/** Isolate based on client address */
+#define ISO_CLIENTADDR  (1u<<4)
+/** Isolate based on session group (always on). */
+#define ISO_SESSIONGRP  (1u<<5)
+/**@}*/
+
+/** Default isolation level for ports. */
+#define ISO_DEFAULT (ISO_CLIENTADDR|ISO_SOCKSAUTH|ISO_SESSIONGRP)
+
+/** Configuration for a single port that we're listening on. */
+typedef struct port_cfg_t {
+  tor_addr_t addr; /**< The configured address to listen on. */
+  int port; /**< The configured port, or CFG_AUTO_PORT to tell Tor to pick its
+             * own port. */
+  uint8_t type; /**< One of CONN_TYPE_*_LISTENER */
+  unsigned is_unix_addr : 1; /**< True iff this is an AF_UNIX address. */
+
+  /* Client port types (socks, dns, trans, natd) only: */
+  uint8_t isolate; /**< Zero or more isolation flags */
+  int sessiongroup; /**< A session group, or -1 if this port is not in a
+                     * session group. */
+
+  /* Unix sockets only: */
+  /** Path for an AF_UNIX address */
+  char unix_addr[FLEXIBLE_ARRAY_MEMBER];
+} port_cfg_t;
+
 /** A linked list of lines in a config file. */
 typedef struct config_line_t {
   char *key;
@@ -2637,16 +2678,17 @@ typedef struct {
   char *User; /**< Name of user to run Tor as. */
   char *Group; /**< Name of group to run Tor as. */
   int ORPort; /**< Port to listen on for OR connections. */
-  int SocksPort; /**< Port to listen on for SOCKS connections. */
-  /** Port to listen on for transparent pf/netfilter connections. */
-  int TransPort;
-  int NATDPort; /**< Port to listen on for transparent natd connections. */
+  config_line_t *SocksPort; /**< Ports to listen on for SOCKS connections. */
+  /** Ports to listen on for transparent pf/netfilter connections. */
+  config_line_t *TransPort;
+  config_line_t *NATDPort; /**< Ports to listen on for transparent natd
+                            * connections. */
   int ControlPort; /**< Port to listen on for control connections. */
   config_line_t *ControlSocket; /**< List of Unix Domain Sockets to listen on
                                  * for control connections. */
   int ControlSocketsGroupWritable; /**< Boolean: Are control sockets g+rw? */
   int DirPort; /**< Port to listen on for directory connections. */
-  int DNSPort; /**< Port to listen on for DNS requests. */
+  config_line_t *DNSPort; /**< Port to listen on for DNS requests. */
   int AssumeReachable; /**< Whether to publish our descriptor regardless. */
   int AuthoritativeDir; /**< Boolean: is this an authoritative directory? */
   int V1AuthoritativeDir; /**< Boolean: is this an authoritative directory

src/or/router.c

@@ -1116,14 +1116,12 @@ set_server_advertised(int s)
   server_is_advertised = s;
 }
 
-/** Return true iff we are trying to be a socks proxy. */
+/** Return true iff we are trying to proxy client connections. */
 int
 proxy_mode(const or_options_t *options)
 {
-  return (options->SocksPort != 0 ||
-          options->TransPort != 0 ||
-          options->NATDPort != 0 ||
-          options->DNSPort != 0);
+  (void)options;
+  return smartlist_len(get_configured_client_ports()) > 0;
 }
 
 /** Decide if we're a publishable server. We are a publishable server if: