Merge remote-tracking branch 'asn/bug4865_take2'

8 files changed, 115 insertions, 23 deletions

diff --git a/changes/bug4865 b/changes/bug4865
new file mode 100644
index 0000000000..e165c41f71
--- /dev/null
+++ b/changes/bug4865
@@ -0,0 +1,4 @@
+  o Major bugfixes:
+    - Pass correct OR address to managed proxies, even when
+      ORListenAddress is used. Fixes bug #4865; bugfix on
+      0.2.3.9-alpha.
diff --git a/src/common/address.c b/src/common/address.c
index 676c485897..7f78d1e4d3 100644
--- a/src/common/address.c
+++ b/src/common/address.c
@@ -986,16 +986,21 @@ tor_dup_addr(const tor_addr_t *addr)
   }
 }
 
-/** Return a string representing the address <b>addr</b>.  This string is
- * statically allocated, and must not be freed.  Each call to
- * <b>fmt_addr</b> invalidates the last result of the function.  This
- * function is not thread-safe. */
+/** Return a string representing the address <b>addr</b>.  This string
+ * is statically allocated, and must not be freed.  Each call to
+ * <b>fmt_addr_impl</b> invalidates the last result of the function.
+ * This function is not thread-safe. If <b>decorate</b> is set, add
+ * brackets to IPv6 addresses.
+ *
+ * It's better to use the wrapper macros of this function:
+ * <b>fmt_addr()</b> and <b>fmt_and_decorate_addr()</b>.
+ */
 const char *
-fmt_addr(const tor_addr_t *addr)
+fmt_addr_impl(const tor_addr_t *addr, int decorate)
 {
   static char buf[TOR_ADDR_BUF_LEN];
   if (!addr) return "<null>";
-  if (tor_addr_to_str(buf, addr, sizeof(buf), 0))
+  if (tor_addr_to_str(buf, addr, sizeof(buf), decorate))
     return buf;
   else
     return "???";
diff --git a/src/common/address.h b/src/common/address.h
index ede035d642..761eed661c 100644
--- a/src/common/address.h
+++ b/src/common/address.h
@@ -135,7 +135,14 @@ tor_addr_eq_ipv4h(const tor_addr_t *a, uint32_t u)
 
 int tor_addr_lookup(const char *name, uint16_t family, tor_addr_t *addr_out);
 char *tor_dup_addr(const tor_addr_t *addr) ATTR_MALLOC;
-const char *fmt_addr(const tor_addr_t *addr);
+
+/** Wrapper function of fmt_addr_impl(). It does not decorate IPv6
+ *  addresses. */
+#define fmt_addr(a) fmt_addr_impl((a), 0)
+/** Wrapper function of fmt_addr_impl(). It decorates IPv6
+ *  addresses. */
+#define fmt_and_decorate_addr(a) fmt_addr_impl((a), 1)
+const char *fmt_addr_impl(const tor_addr_t *addr, int decorate);
 const char * fmt_addr32(uint32_t addr);
 int get_interface_address6(int severity, sa_family_t family, tor_addr_t *addr);
 
diff --git a/src/or/config.c b/src/or/config.c
index ab4f160bf2..d11835463b 100644
--- a/src/or/config.c
+++ b/src/or/config.c
@@ -6040,6 +6040,65 @@ get_configured_ports(void)
   return configured_ports;
 }
 
+/** Return an <address>:<port> string representation of the address
+ *  where the first <b>listener_type</b> listener waits for
+ *  connections. Return NULL if we couldn't find a listener. The
+ *  string is allocated on the heap and it's the responsibility of the
+ *  caller to free it after use.
+ *
+ *  This function is meant to be used by the pluggable transport proxy
+ *  spawning code, please make sure that it fits your purposes before
+ *  using it. */
+char *
+get_first_listener_addrport_string(int listener_type)
+{
+  static const char *ipv4_localhost = "127.0.0.1";
+  static const char *ipv6_localhost = "[::1]";
+  const char *address;
+  uint16_t port;
+  char *string = NULL;
+
+  if (!configured_ports)
+    return NULL;
+
+  SMARTLIST_FOREACH_BEGIN(configured_ports, const port_cfg_t *, cfg) {
+    if (cfg->no_listen)
+      continue;
+
+    if (cfg->type == listener_type &&
+        tor_addr_family(&cfg->addr) != AF_UNSPEC) {
+
+      /* We found the first listener of the type we are interested in! */
+
+      /* If a listener is listening on INADDR_ANY, assume that it's
+         also listening on 127.0.0.1, and point the transport proxy
+         there: */
+      if (tor_addr_is_null(&cfg->addr))
+        address = tor_addr_is_v4(&cfg->addr) ? ipv4_localhost : ipv6_localhost;
+      else
+        address = fmt_and_decorate_addr(&cfg->addr);
+
+      /* If a listener is configured with port 'auto', we are forced
+         to iterate all listener connections and find out in which
+         port it ended up listening: */
+      if (cfg->port == CFG_AUTO_PORT) {
+        port = router_get_active_listener_port_by_type(listener_type);
+        if (!port)
+          return NULL;
+      } else {
+        port = cfg->port;
+      }
+
+      tor_asprintf(&string, "%s:%u", address, port);
+
+      return string;
+    }
+
+  } SMARTLIST_FOREACH_END(cfg);
+
+  return NULL;
+}
+
 /** Return the first advertised port of type <b>listener_type</b> in
     <b>address_family</b>.  */
 int
diff --git a/src/or/config.h b/src/or/config.h
index 0f7c618861..0495186514 100644
--- a/src/or/config.h
+++ b/src/or/config.h
@@ -72,6 +72,8 @@ int get_first_advertised_port_by_type_af(int listener_type,
 #define get_primary_dir_port() \
   (get_first_advertised_port_by_type_af(CONN_TYPE_DIR_LISTENER, AF_INET))
 
+char *get_first_listener_addrport_string(int listener_type);
+
 int options_need_geoip_info(const or_options_t *options,
                             const char **reason_out);
 
diff --git a/src/or/router.c b/src/or/router.c
index 35955f3ee0..4468c7b26a 100644
--- a/src/or/router.c
+++ b/src/or/router.c
@@ -1228,6 +1228,22 @@ consider_publishable_server(int force)
   }
 }
 
+/** Return the port of the first active listener of type
+ *  <b>listener_type</b>. */
+/** XXX not a very good interface. it's not reliable when there are
+    multiple listeners. */
+uint16_t
+router_get_active_listener_port_by_type(int listener_type)
+{
+  /* Iterate all connections, find one of the right kind and return
+     the port. Not very sophisticated or fast, but effective. */
+  const connection_t *c = connection_get_by_type(listener_type);
+  if (c)
+    return c->port;
+
+  return 0;
+}
+
 /** Return the port that we should advertise as our ORPort; this is either
  * the one configured in the ORPort option, or the one we actually bound to
  * if ORPort is "auto".
@@ -1238,12 +1254,11 @@ router_get_advertised_or_port(const or_options_t *options)
   int port = get_primary_or_port();
   (void)options;
 
-  if (port == CFG_AUTO_PORT) {
-    connection_t *c = connection_get_by_type(CONN_TYPE_OR_LISTENER);
-    if (c)
-      return c->port;
-    return 0;
-  }
+  /* If the port is in 'auto' mode, we have to use
+     router_get_listener_port_by_type(). */
+  if (port == CFG_AUTO_PORT)
+    return router_get_active_listener_port_by_type(CONN_TYPE_OR_LISTENER);
+
   return port;
 }
 
@@ -1260,12 +1275,10 @@ router_get_advertised_dir_port(const or_options_t *options, uint16_t dirport)
 
   if (!dirport_configured)
     return dirport;
-  if (dirport_configured == CFG_AUTO_PORT) {
-    connection_t *c = connection_get_by_type(CONN_TYPE_DIR_LISTENER);
-    if (c)
-      return c->port;
-    return 0;
-  }
+
+  if (dirport_configured == CFG_AUTO_PORT)
+    return router_get_active_listener_port_by_type(CONN_TYPE_DIR_LISTENER);
+
   return dirport_configured;
 }
 
diff --git a/src/or/router.h b/src/or/router.h
index fb914349f3..294736e70c 100644
--- a/src/or/router.h
+++ b/src/or/router.h
@@ -53,6 +53,7 @@ int authdir_mode_publishes_statuses(const or_options_t *options);
 int authdir_mode_tests_reachability(const or_options_t *options);
 int authdir_mode_bridge(const or_options_t *options);
 
+uint16_t router_get_active_listener_port_by_type(int listener_type);
 uint16_t router_get_advertised_or_port(const or_options_t *options);
 uint16_t router_get_advertised_dir_port(const or_options_t *options,
                                         uint16_t dirport);
diff --git a/src/or/transports.c b/src/or/transports.c
index 07b9371158..0082b6a9e4 100644
--- a/src/or/transports.c
+++ b/src/or/transports.c
@@ -964,8 +964,6 @@ get_bindaddr_for_server_proxy(const managed_proxy_t *mp)
 static process_environment_t *
 create_managed_proxy_environment(const managed_proxy_t *mp)
 {
-  const or_options_t *options = get_options();
-
   /* Environment variables to be added to or set in mp's environment. */
   smartlist_t *envs = smartlist_new();
   /* XXXX The next time someone touches this code, shorten the name of
@@ -1000,8 +998,11 @@ create_managed_proxy_environment(const managed_proxy_t *mp)
   }
 
   if (mp->is_server) {
-    smartlist_add_asprintf(envs, "TOR_PT_ORPORT=127.0.0.1:%s",
-                           options->ORPort->value);
+    {
+      char *orport_tmp = get_first_listener_addrport_string(CONN_TYPE_OR_LISTENER);
+      smartlist_add_asprintf(envs, "TOR_PT_ORPORT=%s", orport_tmp);
+      tor_free(orport_tmp);
+    }
 
     {
       char *bindaddr_tmp = get_bindaddr_for_server_proxy(mp);