config: Move relay port parsing into the relay module

This commit:
* creates feature/relay/relay_config.[ch],
* moves relay port parsing into them,
* exposes some code from src/app/config.c
  (we'll refactor it later in 29211), and
* adds thin wrappers to make the moved code compile.

No functional changes: the moved code is still enabled,
even if the relay module is disabled.

Part of 32213.

src/app/config/config.c

@@ -102,6 +102,7 @@
 #include "feature/relay/dns.h"
 #include "feature/relay/ext_orport.h"
 #include "feature/relay/routermode.h"
+#include "feature/relay/relay_config.h"
 #include "feature/rend/rendclient.h"
 #include "feature/rend/rendservice.h"
 #include "lib/geoip/geoip.h"
@@ -834,9 +835,6 @@ static char *get_bindaddr_from_transport_listen_line(const char *line,
 static int parse_ports(or_options_t *options, int validate_only,
                               char **msg_out, int *n_ports_out,
                               int *world_writable_control_socket);
-static int check_server_ports(const smartlist_t *ports,
-                              const or_options_t *options,
-                              int *num_low_ports_out);
 static int validate_data_directories(or_options_t *options);
 static int write_configuration_file(const char *fname,
                                     const or_options_t *options);
@@ -6492,7 +6490,7 @@ parse_dir_fallback_line(const char *line,
 }
 
 /** Allocate and return a new port_cfg_t with reasonable defaults. */
-STATIC port_cfg_t *
+port_cfg_t *
 port_cfg_new(size_t namelen)
 {
   tor_assert(namelen <= SIZE_T_CEILING - sizeof(port_cfg_t) - 1);
@@ -6506,7 +6504,7 @@ port_cfg_new(size_t namelen)
 }
 
 /** Free all storage held in <b>port</b> */
-STATIC void
+void
 port_cfg_free_(port_cfg_t *port)
 {
   tor_free(port);
@@ -6752,7 +6750,7 @@ check_bridge_distribution_setting(const char *bd)
  * <b>out</b> for every port that the client should listen on.  Return 0
  * on success, -1 on failure.
  */
-STATIC int
+int
 parse_port_config(smartlist_t *out,
                   const config_line_t *ports,
                   const char *portname,
@@ -7210,7 +7208,7 @@ parse_port_config(smartlist_t *out,
 /** Return the number of ports which are actually going to listen with type
  * <b>listenertype</b>.  Do not count no_listen ports.  Only count unix
  * sockets if count_sockets is true. */
-static int
+int
 count_real_listeners(const smartlist_t *ports, int listenertype,
                      int count_sockets)
 {
@@ -7319,40 +7317,9 @@ parse_ports(or_options_t *options, int validate_only,
       goto err;
     }
   }
-  if (! options->ClientOnly) {
-    if (parse_port_config(ports,
-                          options->ORPort_lines,
-                          "OR", CONN_TYPE_OR_LISTENER,
-                          "0.0.0.0", 0,
-                          CL_PORT_SERVER_OPTIONS) < 0) {
-      *msg = tor_strdup("Invalid ORPort configuration");
-      goto err;
-    }
-    if (parse_port_config(ports,
-                          options->ExtORPort_lines,
-                          "ExtOR", CONN_TYPE_EXT_OR_LISTENER,
-                          "127.0.0.1", 0,
-                          CL_PORT_SERVER_OPTIONS|CL_PORT_WARN_NONLOCAL) < 0) {
-      *msg = tor_strdup("Invalid ExtORPort configuration");
-      goto err;
-    }
-    if (parse_port_config(ports,
-                          options->DirPort_lines,
-                          "Dir", CONN_TYPE_DIR_LISTENER,
-                          "0.0.0.0", 0,
-                          CL_PORT_SERVER_OPTIONS) < 0) {
-      *msg = tor_strdup("Invalid DirPort configuration");
-      goto err;
-    }
-  }
 
-  int n_low_ports = 0;
+  if (parse_ports_relay(options, msg, ports, &have_low_ports) < 0)
-  if (check_server_ports(ports, options, &n_low_ports) < 0) {
-    *msg = tor_strdup("Misconfigured server ports");
     goto err;
-  }
-  if (have_low_ports < 0)
-    have_low_ports = (n_low_ports > 0);
 
   *n_ports_out = smartlist_len(ports);
 
@@ -7360,8 +7327,7 @@ parse_ports(or_options_t *options, int validate_only,
 
   /* Update the *Port_set options.  The !! here is to force a boolean out of
      an integer. */
-  options->ORPort_set =
+  update_port_set_relay(options, ports);
-    !! count_real_listeners(ports, CONN_TYPE_OR_LISTENER, 0);
   options->SocksPort_set =
     !! count_real_listeners(ports, CONN_TYPE_AP_LISTENER, 1);
   options->TransPort_set =
@@ -7373,12 +7339,8 @@ parse_ports(or_options_t *options, int validate_only,
   /* Use options->ControlSocket to test if a control socket is set */
   options->ControlPort_set =
     !! count_real_listeners(ports, CONN_TYPE_CONTROL_LISTENER, 0);
-  options->DirPort_set =
-    !! count_real_listeners(ports, CONN_TYPE_DIR_LISTENER, 0);
   options->DNSPort_set =
     !! count_real_listeners(ports, CONN_TYPE_AP_DNS_LISTENER, 1);
-  options->ExtORPort_set =
-    !! count_real_listeners(ports, CONN_TYPE_EXT_OR_LISTENER, 0);
 
   if (world_writable_control_socket) {
     SMARTLIST_FOREACH(ports, port_cfg_t *, p,
@@ -7409,7 +7371,7 @@ parse_ports(or_options_t *options, int validate_only,
 }
 
 /* Does port bind to IPv4? */
-static int
+int
 port_binds_ipv4(const port_cfg_t *port)
 {
   return tor_addr_family(&port->addr) == AF_INET ||
@@ -7418,7 +7380,7 @@ port_binds_ipv4(const port_cfg_t *port)
 }
 
 /* Does port bind to IPv6? */
-static int
+int
 port_binds_ipv6(const port_cfg_t *port)
 {
   return tor_addr_family(&port->addr) == AF_INET6 ||
@@ -7426,94 +7388,6 @@ port_binds_ipv6(const port_cfg_t *port)
           && !port->server_cfg.bind_ipv4_only);
 }
 
-/** Given a list of <b>port_cfg_t</b> in <b>ports</b>, check them for internal
- * consistency and warn as appropriate.  Set *<b>n_low_ports_out</b> to the
- * number of sub-1024 ports we will be binding. */
-static int
-check_server_ports(const smartlist_t *ports,
-                   const or_options_t *options,
-                   int *n_low_ports_out)
-{
-  int n_orport_advertised = 0;
-  int n_orport_advertised_ipv4 = 0;
-  int n_orport_listeners = 0;
-  int n_dirport_advertised = 0;
-  int n_dirport_listeners = 0;
-  int n_low_port = 0;
-  int r = 0;
-
-  SMARTLIST_FOREACH_BEGIN(ports, const port_cfg_t *, port) {
-    if (port->type == CONN_TYPE_DIR_LISTENER) {
-      if (! port->server_cfg.no_advertise)
-        ++n_dirport_advertised;
-      if (! port->server_cfg.no_listen)
-        ++n_dirport_listeners;
-    } else if (port->type == CONN_TYPE_OR_LISTENER) {
-      if (! port->server_cfg.no_advertise) {
-        ++n_orport_advertised;
-        if (port_binds_ipv4(port))
-          ++n_orport_advertised_ipv4;
-      }
-      if (! port->server_cfg.no_listen)
-        ++n_orport_listeners;
-    } else {
-      continue;
-    }
-#ifndef _WIN32
-    if (!port->server_cfg.no_listen && port->port < 1024)
-      ++n_low_port;
-#endif
-  } SMARTLIST_FOREACH_END(port);
-
-  if (n_orport_advertised && !n_orport_listeners) {
-    log_warn(LD_CONFIG, "We are advertising an ORPort, but not actually "
-             "listening on one.");
-    r = -1;
-  }
-  if (n_orport_listeners && !n_orport_advertised) {
-    log_warn(LD_CONFIG, "We are listening on an ORPort, but not advertising "
-             "any ORPorts. This will keep us from building a %s "
-             "descriptor, and make us impossible to use.",
-             options->BridgeRelay ? "bridge" : "router");
-    r = -1;
-  }
-  if (n_dirport_advertised && !n_dirport_listeners) {
-    log_warn(LD_CONFIG, "We are advertising a DirPort, but not actually "
-             "listening on one.");
-    r = -1;
-  }
-  if (n_dirport_advertised > 1) {
-    log_warn(LD_CONFIG, "Can't advertise more than one DirPort.");
-    r = -1;
-  }
-  if (n_orport_advertised && !n_orport_advertised_ipv4 &&
-      !options->BridgeRelay) {
-    log_warn(LD_CONFIG, "Configured public relay to listen only on an IPv6 "
-             "address. Tor needs to listen on an IPv4 address too.");
-    r = -1;
-  }
-
-  if (n_low_port && options->AccountingMax &&
-      (!have_capability_support() || options->KeepBindCapabilities == 0)) {
-    const char *extra = "";
-    if (options->KeepBindCapabilities == 0 && have_capability_support())
-      extra = ", and you have disabled KeepBindCapabilities.";
-    log_warn(LD_CONFIG,
-          "You have set AccountingMax to use hibernation. You have also "
-          "chosen a low DirPort or OrPort%s."
-          "This combination can make Tor stop "
-          "working when it tries to re-attach the port after a period of "
-          "hibernation. Please choose a different port or turn off "
-          "hibernation unless you know this combination will work on your "
-          "platform.", extra);
-  }
-
-  if (n_low_ports_out)
-    *n_low_ports_out = n_low_port;
-
-  return r;
-}
-
 /** Return a list of port_cfg_t for client ports parsed from the
  * options. */
 MOCK_IMPL(const smartlist_t *,

src/app/config/config.h

@@ -163,6 +163,8 @@ int write_to_data_subdir(const char* subdir, const char* fname,
 int get_num_cpus(const or_options_t *options);
 
 MOCK_DECL(const smartlist_t *,get_configured_ports,(void));
+int port_binds_ipv4(const port_cfg_t *port);
+int port_binds_ipv6(const port_cfg_t *port);
 int get_first_advertised_port_by_type_af(int listener_type,
                                          int address_family);
 #define get_primary_or_port() \
@@ -252,8 +254,13 @@ smartlist_t *get_options_for_server_transport(const char *transport);
 
 /* Port helper functions. */
 int options_any_client_port_set(const or_options_t *options);
-
+int parse_port_config(smartlist_t *out,
-#ifdef CONFIG_PRIVATE
+                      const struct config_line_t *ports,
+                      const char *portname,
+                      int listener_type,
+                      const char *defaultaddr,
+                      int defaultport,
+                      const unsigned flags);
 
 #define CL_PORT_NO_STREAM_OPTIONS (1u<<0)
 #define CL_PORT_WARN_NONLOCAL (1u<<1)
@@ -264,16 +271,23 @@ int options_any_client_port_set(const or_options_t *options);
 #define CL_PORT_IS_UNIXSOCKET (1u<<6)
 #define CL_PORT_DFLT_GROUP_WRITABLE (1u<<7)
 
+port_cfg_t *port_cfg_new(size_t namelen);
+#define port_cfg_free(port) \
+  FREE_AND_NULL(port_cfg_t, port_cfg_free_, (port))
+void port_cfg_free_(port_cfg_t *port);
+
+int count_real_listeners(const smartlist_t *ports,
+                         int listenertype,
+                         int count_sockets);
+
+#ifdef CONFIG_PRIVATE
+
 MOCK_DECL(STATIC int, options_act,(const or_options_t *old_options));
 MOCK_DECL(STATIC int, options_act_reversible,(const or_options_t *old_options,
                                              char **msg));
 struct config_mgr_t;
 STATIC const struct config_mgr_t *get_options_mgr(void);
 
-STATIC port_cfg_t *port_cfg_new(size_t namelen);
-#define port_cfg_free(port) \
-  FREE_AND_NULL(port_cfg_t, port_cfg_free_, (port))
-STATIC void port_cfg_free_(port_cfg_t *port);
 #define or_options_free(opt) \
   FREE_AND_NULL(or_options_t, or_options_free_, (opt))
 STATIC void or_options_free_(or_options_t *options);
@@ -292,13 +306,6 @@ STATIC int parse_dir_authority_line(const char *line,
 STATIC int parse_dir_fallback_line(const char *line, int validate_only);
 STATIC int have_enough_mem_for_dircache(const or_options_t *options,
                                         size_t total_mem, char **msg);
-STATIC int parse_port_config(smartlist_t *out,
-                  const struct config_line_t *ports,
-                  const char *portname,
-                  int listener_type,
-                  const char *defaultaddr,
-                  int defaultport,
-                  const unsigned flags);
 
 STATIC int check_bridge_distribution_setting(const char *bd);
 

src/core/include.am

@@ -143,6 +143,7 @@ LIBTOR_APP_A_SOURCES = 				\
 	src/feature/relay/dns.c			\
 	src/feature/relay/ext_orport.c		\
 	src/feature/relay/onion_queue.c		\
+	src/feature/relay/relay_config.c	\
 	src/feature/relay/relay_periodic.c	\
 	src/feature/relay/relay_sys.c		\
 	src/feature/relay/router.c		\
@@ -430,6 +431,7 @@ noinst_HEADERS +=					\
 	src/feature/relay/dns_structs.h			\
 	src/feature/relay/ext_orport.h			\
 	src/feature/relay/onion_queue.h			\
+	src/feature/relay/relay_config.h		\
 	src/feature/relay/relay_periodic.h		\
 	src/feature/relay/relay_sys.h			\
 	src/feature/relay/router.h			\

src/feature/relay/relay_config.c

@@ -0,0 +1,222 @@
+/* Copyright (c) 2001 Matej Pfajfar.
+ * Copyright (c) 2001-2004, Roger Dingledine.
+ * Copyright (c) 2004-2006, Roger Dingledine, Nick Mathewson.
+ * Copyright (c) 2007-2019, The Tor Project, Inc. */
+/* See LICENSE for licensing information */
+
+/**
+ * @file relay_config.c
+ * @brief Code to interpret the user's configuration of Tor's relay module.
+ **/
+
+#include "orconfig.h"
+#include "feature/relay/relay_config.h"
+
+#include "lib/encoding/confline.h"
+#include "lib/confmgt/confmgt.h"
+
+#include "lib/container/smartlist.h"
+#include "lib/process/setuid.h"
+
+/* Required for dirinfo_type_t in or_options_t */
+#include "core/or/or.h"
+#include "app/config/config.h"
+
+#include "core/mainloop/connection.h"
+#include "core/or/port_cfg_st.h"
+
+#include "feature/relay/dns.h"
+#include "feature/relay/ext_orport.h"
+#include "feature/relay/routermode.h"
+
+/** Given a list of <b>port_cfg_t</b> in <b>ports</b>, check them for internal
+ * consistency and warn as appropriate.  On Unix-based OSes, set
+ * *<b>n_low_ports_out</b> to the number of sub-1024 ports we will be
+ * binding, and warn if we may be unable to re-bind after hibernation. */
+static int
+check_server_ports(const smartlist_t *ports,
+                   const or_options_t *options,
+                   int *n_low_ports_out)
+{
+  if (BUG(!ports))
+    return -1;
+
+  if (BUG(!options))
+    return -1;
+
+  if (BUG(!n_low_ports_out))
+    return -1;
+
+  int n_orport_advertised = 0;
+  int n_orport_advertised_ipv4 = 0;
+  int n_orport_listeners = 0;
+  int n_dirport_advertised = 0;
+  int n_dirport_listeners = 0;
+  int n_low_port = 0;
+  int r = 0;
+
+  SMARTLIST_FOREACH_BEGIN(ports, const port_cfg_t *, port) {
+    if (port->type == CONN_TYPE_DIR_LISTENER) {
+      if (! port->server_cfg.no_advertise)
+        ++n_dirport_advertised;
+      if (! port->server_cfg.no_listen)
+        ++n_dirport_listeners;
+    } else if (port->type == CONN_TYPE_OR_LISTENER) {
+      if (! port->server_cfg.no_advertise) {
+        ++n_orport_advertised;
+        if (port_binds_ipv4(port))
+          ++n_orport_advertised_ipv4;
+      }
+      if (! port->server_cfg.no_listen)
+        ++n_orport_listeners;
+    } else {
+      continue;
+    }
+#ifndef _WIN32
+    if (!port->server_cfg.no_listen && port->port < 1024)
+      ++n_low_port;
+#endif
+  } SMARTLIST_FOREACH_END(port);
+
+  if (n_orport_advertised && !n_orport_listeners) {
+    log_warn(LD_CONFIG, "We are advertising an ORPort, but not actually "
+             "listening on one.");
+    r = -1;
+  }
+  if (n_orport_listeners && !n_orport_advertised) {
+    log_warn(LD_CONFIG, "We are listening on an ORPort, but not advertising "
+             "any ORPorts. This will keep us from building a %s "
+             "descriptor, and make us impossible to use.",
+             options->BridgeRelay ? "bridge" : "router");
+    r = -1;
+  }
+  if (n_dirport_advertised && !n_dirport_listeners) {
+    log_warn(LD_CONFIG, "We are advertising a DirPort, but not actually "
+             "listening on one.");
+    r = -1;
+  }
+  if (n_dirport_advertised > 1) {
+    log_warn(LD_CONFIG, "Can't advertise more than one DirPort.");
+    r = -1;
+  }
+  if (n_orport_advertised && !n_orport_advertised_ipv4 &&
+      !options->BridgeRelay) {
+    log_warn(LD_CONFIG, "Configured public relay to listen only on an IPv6 "
+             "address. Tor needs to listen on an IPv4 address too.");
+    r = -1;
+  }
+
+  if (n_low_port && options->AccountingMax &&
+      (!have_capability_support() || options->KeepBindCapabilities == 0)) {
+    const char *extra = "";
+    if (options->KeepBindCapabilities == 0 && have_capability_support())
+      extra = ", and you have disabled KeepBindCapabilities.";
+    log_warn(LD_CONFIG,
+          "You have set AccountingMax to use hibernation. You have also "
+          "chosen a low DirPort or OrPort%s."
+          "This combination can make Tor stop "
+          "working when it tries to re-attach the port after a period of "
+          "hibernation. Please choose a different port or turn off "
+          "hibernation unless you know this combination will work on your "
+          "platform.", extra);
+  }
+
+  if (n_low_ports_out)
+    *n_low_ports_out = n_low_port;
+
+  return r;
+}
+
+/** Parse all relay ports from <b>options</b>. On success, add parsed ports to
+ * <b>ports</b>, and return 0.  On failure, set *<b>msg</b> to a description
+ * of the problem and return -1.
+ **/
+int
+parse_ports_relay(or_options_t *options,
+                  char **msg,
+                  smartlist_t *ports_out,
+                  int *have_low_ports_out)
+{
+  int retval = -1;
+  smartlist_t *ports = smartlist_new();
+
+  if (BUG(!options))
+    goto err;
+
+  if (BUG(!msg))
+    goto err;
+
+  if (BUG(!ports_out))
+    goto err;
+
+  if (BUG(!have_low_ports_out))
+    goto err;
+
+  if (! options->ClientOnly) {
+    if (parse_port_config(ports,
+                          options->ORPort_lines,
+                          "OR", CONN_TYPE_OR_LISTENER,
+                          "0.0.0.0", 0,
+                          CL_PORT_SERVER_OPTIONS) < 0) {
+      *msg = tor_strdup("Invalid ORPort configuration");
+      goto err;
+    }
+    if (parse_port_config(ports,
+                          options->ExtORPort_lines,
+                          "ExtOR", CONN_TYPE_EXT_OR_LISTENER,
+                          "127.0.0.1", 0,
+                          CL_PORT_SERVER_OPTIONS|CL_PORT_WARN_NONLOCAL) < 0) {
+      *msg = tor_strdup("Invalid ExtORPort configuration");
+      goto err;
+    }
+    if (parse_port_config(ports,
+                          options->DirPort_lines,
+                          "Dir", CONN_TYPE_DIR_LISTENER,
+                          "0.0.0.0", 0,
+                          CL_PORT_SERVER_OPTIONS) < 0) {
+      *msg = tor_strdup("Invalid DirPort configuration");
+      goto err;
+    }
+  }
+
+  int n_low_ports = 0;
+  if (check_server_ports(ports, options, &n_low_ports) < 0) {
+    *msg = tor_strdup("Misconfigured server ports");
+    goto err;
+  }
+  if (*have_low_ports_out < 0)
+    *have_low_ports_out = (n_low_ports > 0);
+
+  smartlist_add_all(ports_out, ports);
+  smartlist_free(ports);
+  ports = NULL;
+  retval = 0;
+
+ err:
+  if (ports) {
+    SMARTLIST_FOREACH(ports, port_cfg_t *, p, port_cfg_free(p));
+    smartlist_free(ports);
+  }
+  return retval;
+}
+
+/** Update the relay *Port_set values in <b>options</b> from <b>ports</b>. */
+void
+update_port_set_relay(or_options_t *options,
+                      const smartlist_t *ports)
+{
+  if (BUG(!options))
+    return;
+
+  if (BUG(!ports))
+    return;
+
+  /* Update the relay *Port_set options.  The !! here is to force a boolean
+   * out of an integer. */
+  options->ORPort_set =
+    !! count_real_listeners(ports, CONN_TYPE_OR_LISTENER, 0);
+  options->DirPort_set =
+    !! count_real_listeners(ports, CONN_TYPE_DIR_LISTENER, 0);
+  options->ExtORPort_set =
+    !! count_real_listeners(ports, CONN_TYPE_EXT_OR_LISTENER, 0);
+}

src/feature/relay/relay_config.h

@@ -0,0 +1,25 @@
+/* Copyright (c) 2001 Matej Pfajfar.
+ * Copyright (c) 2001-2004, Roger Dingledine.
+ * Copyright (c) 2004-2006, Roger Dingledine, Nick Mathewson.
+ * Copyright (c) 2007-2019, The Tor Project, Inc. */
+/* See LICENSE for licensing information */
+
+/**
+ * @file relay_config.h
+ * @brief Header for feature/relay/relay_config.c
+ **/
+
+#ifndef TOR_FEATURE_RELAY_RELAY_CONFIG_H
+#define TOR_FEATURE_RELAY_RELAY_CONFIG_H
+
+typedef struct or_options_t or_options_t;
+typedef struct smartlist_t smartlist_t;
+
+int parse_ports_relay(or_options_t *options,
+                      char **msg,
+                      smartlist_t *ports_out,
+                      int *have_low_ports_out);
+void update_port_set_relay(or_options_t *options,
+                           const smartlist_t *ports);
+
+#endif /* !defined(TOR_FEATURE_RELAY_RELAY_CONFIG_H) */