Revised how we handle ClientTransportPlugin and Bridge lines.

Multiple Bridge lines can point to the same one ClientTransportPlugin
line, and we can have multiple ClientTransportPlugin lines in our
configuration file that don't match with a bridge. We also issue a
warning when we have a Bridge line with a pluggable transport but we
can't match it to a ClientTransportPlugin line.

src/or/circuitbuild.c

@@ -4607,16 +4607,15 @@ static transport_t *
 transport_get_by_name(const char *name)
 {
   tor_assert(name);
-  if (transport_list) {
-    SMARTLIST_FOREACH_BEGIN(transport_list, transport_t *, transport) {
-      if (!strcmp(transport->name, name))
-        return transport;
-    } SMARTLIST_FOREACH_END(transport);
-  }
 
-  log_notice(LD_GENERAL, "We were asked to match '%s' to a pluggable "
-             "transport, and we failed. If you didn't expect this, please "
-             "check your configuration.", name);
+  if (!transport_list)
+    return NULL;
+
+  SMARTLIST_FOREACH_BEGIN(transport_list, transport_t *, transport) {
+    if (!strcmp(transport->name, name))
+      return transport;
+  } SMARTLIST_FOREACH_END(transport);
+
   return NULL;
 }
 
@@ -4653,6 +4652,27 @@ transport_add_from_config(const tor_addr_t *addr, uint16_t port,
   return -1;
 }
 
+/** Warns the user of possible pluggable transport misconfiguration. */
+void
+validate_pluggable_transports_config(void)
+{
+  if (bridge_list) {
+    SMARTLIST_FOREACH_BEGIN(bridge_list, bridge_info_t *, b)
+      {
+        /* Skip bridges without transports. */
+        if (!b->transport_name)
+          continue;
+        /* See if the user has Bridges that specify nonexistent 
+           pluggable transports. We should warn the user in such case,
+           since it's probably misconfiguration. */
+        if (!transport_get_by_name(b->transport_name))
+          log_warn(LD_CONFIG, "You have a Bridge line using the %s "
+                   "pluggable transport, but there doesn't seem to be a "
+                   "corresponding ClientTransportPlugin line.", b->transport_name);
+      }  SMARTLIST_FOREACH_END(b);
+  }
+}
+
 /** Return a bridge pointer if <b>ri</b> is one of our known bridges
  * (either by comparing keys if possible, else by comparing addr/port).
  * Else return NULL. */
@@ -4789,24 +4809,41 @@ find_bridge_by_digest(const char *digest)
   return NULL;
 }
 
-/** If <b>addr</b> and <b>port</b> match one of our known bridges,
- *  returns its transport protocol if it has one, else returns NULL. */
-transport_t *
-find_transport_by_bridge_addrport(const tor_addr_t *addr, uint16_t port)
+/** If <b>addr</b> and <b>port</b> match the address and port of a
+ * bridge of ours that uses pluggable transports, place it's transport
+ * in <b>transport</b>.
+ *  
+ * Return:
+ * 0: if transport was found successfully.
+ * 1: if <b>addr</b>:<b>port</b> did not match a bridge,
+ *    or if matched bridge was not using transports.
+ * -1: if we should be using a transport, but the transport could not be found.
+ */
+int
+find_transport_by_bridge_addrport(const tor_addr_t *addr, uint16_t port, 
+                                  transport_t **transport)
 {
-  tor_assert(bridge_list);
+  if (!bridge_list)
+    return 1;
+
   SMARTLIST_FOREACH_BEGIN(bridge_list, bridge_info_t *, bridge)
     {
       if (tor_addr_eq(&bridge->addr, addr) &&
-          (bridge->port == port)) {
-        if (bridge->transport_name) {
-          return transport_get_by_name(bridge->transport_name);
-        } else /* bridge found, but it had no transport */
+          (bridge->port == port)) { /* bridge matched */
+        if (bridge->transport_name) { /* it also uses pluggable transports */
+          *transport = transport_get_by_name(bridge->transport_name);
+          if (*transport == NULL) { /* it uses pluggable transports, but
+                                       the transport could not be found! */
+            return -1;
+          }
+          return 0;
+        } else { /* bridge matched, but it doesn't use transports. */
           break;
+        }
       }
     } SMARTLIST_FOREACH_END(bridge);
 
-  return NULL;
+  return 1;
 }
 
 /** We need to ask <b>bridge</b> for its server descriptor. */
@@ -5116,6 +5153,8 @@ entry_guards_free_all(void)
   clear_bridge_list();
   clear_transport_list();
   smartlist_free(bridge_list);
+  smartlist_free(transport_list);
   bridge_list = NULL;
+  transport_list = NULL;
 }
 

src/or/circuitbuild.h

@@ -142,8 +142,9 @@ int circuit_build_times_get_bw_scale(networkstatus_t *ns);
 void clear_transport_list(void);
 int transport_add_from_config(const tor_addr_t *addr, uint16_t port,
                                const char *name, int socks_ver);
-transport_t *
-find_transport_by_bridge_addrport(const tor_addr_t *addr, uint16_t port);
+int find_transport_by_bridge_addrport(const tor_addr_t *addr, uint16_t port,
+                                      transport_t **transport);
+void validate_pluggable_transports_config(void);
 
 #endif
 

src/or/config.c

@@ -1232,6 +1232,11 @@ options_act(or_options_t *old_options)
     sweep_bridge_list();
   }
 
+  /* If we have pluggable transport related options enabled, see if we
+     should warn the user about potential configuration problems. */
+  if (options->Bridges || options->ClientTransportPlugin)
+    validate_pluggable_transports_config();
+
   if (running_tor && rend_config_services(options, 0)<0) {
     log_warn(LD_BUG,
        "Previously validated hidden services line could not be added!");
@@ -3683,11 +3688,6 @@ options_validate(or_options_t *old_options, or_options_t *options,
     REJECT("TunnelDirConns set to 0 only works with UseBridges set to 0");
 
   if (options->ClientTransportPlugin) {
-    if (!options->Bridges)
-      REJECT("ClientTransportPlugin found without any bridges.");
-    if (server_mode(options))
-      REJECT("ClientTransportPlugin found but we are a server.");
-
     for (cl = options->ClientTransportPlugin; cl; cl = cl->next) {
       if (parse_client_transport_line(cl->value, 1)<0)
         REJECT("Transport line did not parse. See logs for details.");
@@ -4604,12 +4604,6 @@ parse_bridge_line(const char *line, int validate_only,
   smartlist_del_keeporder(items, 0);
 
   if (!strstr(field1, ".")) { /* new-style bridge line */
-    if (!options->ClientTransportPlugin) {
-      log_warn(LD_CONFIG, "Pluggable transports protocol found "
-               "in bridge line, but no ClientTransportPlugin lines found.");
-      goto err;
-    }
-
     transport_name = field1;
     addrport = smartlist_get(items, 0);
     smartlist_del_keeporder(items, 0);

src/or/connection.c

@@ -67,6 +67,7 @@ static const char *connection_proxy_state_to_string(int state);
 static int connection_read_https_proxy_response(connection_t *conn);
 static void connection_send_socks5_connect(connection_t *conn);
 static const char *proxy_type_to_string(int proxy_type);
+static int get_proxy_type(void);
 
 /** The last IPv4 address that our network interface seemed to have been
  * binding to, in host order.  We use this to detect when our IP changes. */
@@ -4103,44 +4104,47 @@ assert_connection_ok(connection_t *conn, time_t now)
 /** Fills <b>addr</b> and <b>port</b> with the details of the proxy
  *  server of type <b>proxy_type</b> we are using.
  *  <b>conn</b> contains the connection_t we are using the proxy for.
- *  Returns 0 if we were successfull or returns 1 if we are not using
- *  a proxy. */
+ *  Returns 0 if we were successfull, 1 if we are not using
+ *  a proxy, -1 if we are using a proxy but his addrport could not be
+ *  found. */
 int
-get_proxy_addrport(int proxy_type, tor_addr_t *addr, uint16_t *port,
+get_proxy_addrport(tor_addr_t *addr, uint16_t *port,
                    connection_t *conn)
 {
-  or_options_t *options;
-
-  if (proxy_type == PROXY_NONE)
-    return 1;
-
-  options = get_options();
+  or_options_t *options = get_options();
 
-  if (proxy_type == PROXY_CONNECT) {
+  if (options->HTTPSProxy) {
     tor_addr_copy(addr, &options->HTTPSProxyAddr);
     *port = options->HTTPSProxyPort;
-  } else if (proxy_type == PROXY_SOCKS4) {
+    goto done;
+  } else if (options->Socks4Proxy) {
     tor_addr_copy(addr, &options->Socks4ProxyAddr);
     *port = options->Socks4ProxyPort;
-  } else if (proxy_type == PROXY_SOCKS5) {
+    goto done;
+  } else if (options->Socks5Proxy) {
     tor_addr_copy(addr, &options->Socks5ProxyAddr);
     *port = options->Socks5ProxyPort;
-  } else if (proxy_type == PROXY_PLUGGABLE) {
-    transport_t *transport;
-    transport = find_transport_by_bridge_addrport(&conn->addr, conn->port);
-    if (transport) {
+    goto done;
+  } else if (options->ClientTransportPlugin ||
+             options->Bridges) {
+    transport_t *transport=NULL;
+    int r;
+    r = find_transport_by_bridge_addrport(&conn->addr, conn->port, &transport);
+    if (r == 0) { /* transport found */
       tor_addr_copy(addr, &transport->addr);
       *port = transport->port;
-    } else { /* no transport for this bridge. */
-      return 1;
     }
+    return r;
   }
 
+  return 1;
+
+ done:
   return 0;
 }
 
 /** Returns the proxy type used by tor. */
-int
+static int
 get_proxy_type(void)
 {
   or_options_t *options = get_options();
@@ -4162,18 +4166,16 @@ get_proxy_type(void)
 void
 log_failed_proxy_connection(connection_t *conn)
 {
-  int proxy_type;
   tor_addr_t proxy_addr;
   uint16_t proxy_port;
 
-  proxy_type = get_proxy_type();
-  if (get_proxy_addrport(proxy_type, &proxy_addr, &proxy_port, conn) != 0)
+  if (get_proxy_addrport(&proxy_addr, &proxy_port, conn) != 0)
     return; /* if we have no proxy set up leave this function. */
 
   log_warn(LD_NET,
            "The connection to the %s proxy server at %s:%u just failed. "
            "Make sure that the proxy server is up and running.",
-           proxy_type_to_string(proxy_type), fmt_addr(&proxy_addr),
+           proxy_type_to_string(get_proxy_type()), fmt_addr(&proxy_addr),
            proxy_port);
 }
 

src/or/connection.h

@@ -58,9 +58,8 @@ int connection_connect(connection_t *conn, const char *address,
 int connection_proxy_connect(connection_t *conn, int type);
 int connection_read_proxy_handshake(connection_t *conn);
 void log_failed_proxy_connection(connection_t *conn);
-int get_proxy_addrport(int proxy_type, tor_addr_t *addr,
+int get_proxy_addrport(tor_addr_t *addr,
                        uint16_t *port, connection_t *conn);
-int get_proxy_type(void);
 
 int retry_all_listeners(smartlist_t *replaced_conns,
                         smartlist_t *new_conns);

src/or/connection_or.c

@@ -336,11 +336,15 @@ connection_or_finished_connecting(or_connection_t *or_conn)
   else if (get_options()->Socks5Proxy)
     proxy_type = PROXY_SOCKS5;
   else if (get_options()->ClientTransportPlugin) {
-    transport_t *transport;
-    transport = find_transport_by_bridge_addrport(&conn->addr,conn->port);
-    if (transport) { /* this bridge supports transports. use proxy. */
+    transport_t *transport=NULL;
+    int r;
+    r = find_transport_by_bridge_addrport(&conn->addr,conn->port,&transport);
+    if (r == 0) {
+      tor_assert(transport);
       log_debug(LD_GENERAL, "Found transport. Setting proxy type!\n");
       proxy_type = transport->socks_version;
+    } else if (r == -1) {
+      return -1;
     }
   }
 
@@ -840,7 +844,6 @@ connection_or_connect(const tor_addr_t *_addr, uint16_t port,
   tor_addr_t addr;
 
   int r;
-  int proxy_type;
   tor_addr_t proxy_addr;
   uint16_t proxy_port;
 
@@ -861,12 +864,16 @@ connection_or_connect(const tor_addr_t *_addr, uint16_t port,
   control_event_or_conn_status(conn, OR_CONN_EVENT_LAUNCHED, 0);
 
   /* If we are using a proxy server, find it and use it. */
-  proxy_type = get_proxy_type();
-  r = get_proxy_addrport(proxy_type, &proxy_addr, &proxy_port, TO_CONN(conn));
+  r = get_proxy_addrport(&proxy_addr, &proxy_port, TO_CONN(conn));
   if (r == 0) { /* proxy found. */
     tor_addr_copy(&addr, &proxy_addr);
     port = proxy_port;
     conn->_base.proxy_state = PROXY_INFANT;
+  } else if (r == -1) { /* proxy could not be found. */
+    log_warn(LD_GENERAL, "Tried to connect through proxy, but proxy address "
+             "could not be found.");
+    connection_free(TO_CONN(conn));
+    return NULL;
   }
 
   switch (connection_connect(TO_CONN(conn), conn->_base.address,