Merge branch 'feature18483-028-v2-squashed' into maint-0.2.8

4 files changed, 71 insertions, 16 deletions

diff --git a/changes/feature18483 b/changes/feature18483
new file mode 100644
index 0000000000..b3c42e60fd
--- /dev/null
+++ b/changes/feature18483
@@ -0,0 +1,4 @@
+  o Minor features (clients):
+    - Make clients, onion services, and bridge relays always
+      use an encrypted begindir connection for directory requests.
+      Resolves #18483. Patch by "teor".
diff --git a/src/or/directory.c b/src/or/directory.c
index ca3b3e338b..8dc018a662 100644
--- a/src/or/directory.c
+++ b/src/or/directory.c
@@ -630,6 +630,7 @@ directory_choose_address_routerstatus(const routerstatus_t *status,
   tor_assert(use_or_ap != NULL);
   tor_assert(use_dir_ap != NULL);
 
+  const or_options_t *options = get_options();
   int have_or = 0, have_dir = 0;
 
   /* We expect status to have at least one reachable address if we're
@@ -671,10 +672,11 @@ directory_choose_address_routerstatus(const routerstatus_t *status,
   }
 
   /* DirPort connections
-   * DIRIND_ONEHOP uses ORPort, but may fall back to the DirPort */
+   * DIRIND_ONEHOP uses ORPort, but may fall back to the DirPort on relays */
   if (indirection == DIRIND_DIRECT_CONN ||
       indirection == DIRIND_ANON_DIRPORT ||
-      indirection == DIRIND_ONEHOP) {
+      (indirection == DIRIND_ONEHOP
+       && !directory_must_use_begindir(options))) {
     have_dir = fascist_firewall_choose_address_rs(status,
                                                   FIREWALL_DIR_CONNECTION, 0,
                                                   use_dir_ap);
@@ -964,6 +966,16 @@ connection_dir_download_cert_failed(dir_connection_t *conn, int status)
   update_certificate_downloads(time(NULL));
 }
 
+/* Should this tor instance only use begindir for all its directory requests?
+ */
+int
+directory_must_use_begindir(const or_options_t *options)
+{
+  /* Clients, onion services, and bridges must use begindir,
+   * relays and authorities do not have to */
+  return !public_server_mode(options);
+}
+
 /** Evaluate the situation and decide if we should use an encrypted
  * "begindir-style" connection for this directory request.
  * 1) If or_port is 0, or it's a direct conn and or_port is firewalled
@@ -971,23 +983,48 @@ connection_dir_download_cert_failed(dir_connection_t *conn, int status)
  * 2) If we prefer to avoid begindir conns, and we're not fetching or
  *    publishing a bridge relay descriptor, no.
  * 3) Else yes.
+ * If returning 0, return in *reason why we can't use begindir.
+ * reason must not be NULL.
  */
 static int
 directory_command_should_use_begindir(const or_options_t *options,
                                       const tor_addr_t *addr,
                                       int or_port, uint8_t router_purpose,
-                                      dir_indirection_t indirection)
+                                      dir_indirection_t indirection,
+                                      const char **reason)
 {
   (void) router_purpose;
-  if (!or_port)
+  tor_assert(reason);
+  *reason = NULL;
+
+  /* Reasons why we can't possibly use begindir */
+  if (!or_port) {
+    *reason = "directory with unknown ORPort";
     return 0; /* We don't know an ORPort -- no chance. */
-  if (indirection == DIRIND_DIRECT_CONN || indirection == DIRIND_ANON_DIRPORT)
+  }
+  if (indirection == DIRIND_DIRECT_CONN ||
+      indirection == DIRIND_ANON_DIRPORT) {
+    *reason = "DirPort connection";
     return 0;
-  if (indirection == DIRIND_ONEHOP)
+  }
+  if (indirection == DIRIND_ONEHOP) {
+    /* We're firewalled and want a direct OR connection */
     if (!fascist_firewall_allows_address_addr(addr, or_port,
-                                              FIREWALL_OR_CONNECTION, 0, 0) ||
-        directory_fetches_from_authorities(options))
-      return 0; /* We're firewalled or are acting like a relay -- also no. */
+                                              FIREWALL_OR_CONNECTION, 0, 0)) {
+      *reason = "ORPort not reachable";
+      return 0;
+    }
+  }
+  /* Reasons why we want to avoid using begindir */
+  if (indirection == DIRIND_ONEHOP) {
+    if (!directory_must_use_begindir(options)) {
+      *reason = "in relay mode";
+      return 0;
+    }
+  }
+  /* DIRIND_ONEHOP on a client, or DIRIND_ANONYMOUS
+   */
+  *reason = "(using begindir)";
   return 1;
 }
 
@@ -1070,11 +1107,13 @@ directory_initiate_command_rend(const tor_addr_port_t *or_addr_port,
   dir_connection_t *conn;
   const or_options_t *options = get_options();
   int socket_error = 0;
+  const char *begindir_reason = NULL;
   /* Should the connection be to a relay's OR port (and inside that we will
    * send our directory request)? */
   const int use_begindir = directory_command_should_use_begindir(options,
                                      &or_addr_port->addr, or_addr_port->port,
-                                     router_purpose, indirection);
+                                     router_purpose, indirection,
+                                     &begindir_reason);
   /* Will the connection go via a three-hop Tor circuit? Note that this
    * is separate from whether it will use_begindir. */
   const int anonymized_connection = dirind_is_anon(indirection);
@@ -1100,6 +1139,14 @@ directory_initiate_command_rend(const tor_addr_port_t *or_addr_port,
   (void)is_sensitive_dir_purpose;
 #endif
 
+  /* use encrypted begindir connections for everything except relays
+   * this provides better protection for directory fetches */
+  if (!use_begindir && directory_must_use_begindir(options)) {
+    log_warn(LD_BUG, "Client could not use begindir connection: %s",
+             begindir_reason ? begindir_reason : "(NULL)");
+    return;
+  }
+
   /* ensure that we don't make direct connections when a SOCKS server is
    * configured. */
   if (!anonymized_connection && !use_begindir && !options->HTTPProxy &&
diff --git a/src/or/directory.h b/src/or/directory.h
index 03c04c10c9..c4edbb5c0f 100644
--- a/src/or/directory.h
+++ b/src/or/directory.h
@@ -28,8 +28,8 @@ void directory_get_from_all_authorities(uint8_t dir_purpose,
 
 /** Enumeration of ways to connect to a directory server */
 typedef enum {
-  /** Default: connect over a one-hop Tor circuit but fall back to direct
-   * connection */
+  /** Default: connect over a one-hop Tor circuit. Relays fall back to direct
+   * DirPort connections, clients, onion services, and bridges do not */
   DIRIND_ONEHOP=0,
   /** Connect over a multi-hop anonymizing Tor circuit */
   DIRIND_ANONYMOUS=1,
@@ -39,6 +39,8 @@ typedef enum {
   DIRIND_ANON_DIRPORT,
 } dir_indirection_t;
 
+int directory_must_use_begindir(const or_options_t *options);
+
 MOCK_DECL(void, directory_initiate_command_routerstatus,
                 (const routerstatus_t *status,
                  uint8_t dir_purpose,
diff --git a/src/or/routerlist.c b/src/or/routerlist.c
index 2167ae2bf6..97512d7969 100644
--- a/src/or/routerlist.c
+++ b/src/or/routerlist.c
@@ -1687,6 +1687,7 @@ router_pick_directory_server_impl(dirinfo_type_t type, int flags,
 
   const int skip_or = router_skip_or_reachability(options, try_ip_pref);
   const int skip_dir = router_skip_dir_reachability(options, try_ip_pref);
+  const int must_have_or = directory_must_use_begindir(options);
 
   /* Find all the running dirservers we know about. */
   SMARTLIST_FOREACH_BEGIN(nodelist_get_list(), const node_t *, node) {
@@ -1743,9 +1744,9 @@ router_pick_directory_server_impl(dirinfo_type_t type, int flags,
                                    try_ip_pref))
       smartlist_add(is_trusted ? trusted_tunnel :
                     is_overloaded ? overloaded_tunnel : tunnel, (void*)node);
-    else if (skip_dir ||
+    else if (!must_have_or && (skip_dir ||
              fascist_firewall_allows_rs(status, FIREWALL_DIR_CONNECTION,
-                                        try_ip_pref))
+                                        try_ip_pref)))
       smartlist_add(is_trusted ? trusted_direct :
                     is_overloaded ? overloaded_direct : direct, (void*)node);
   } SMARTLIST_FOREACH_END(node);
@@ -1849,6 +1850,7 @@ router_pick_trusteddirserver_impl(const smartlist_t *sourcelist,
 
   const int skip_or = router_skip_or_reachability(options, try_ip_pref);
   const int skip_dir = router_skip_dir_reachability(options, try_ip_pref);
+  const int must_have_or = directory_must_use_begindir(options);
 
   SMARTLIST_FOREACH_BEGIN(sourcelist, const dir_server_t *, d)
     {
@@ -1888,9 +1890,9 @@ router_pick_trusteddirserver_impl(const smartlist_t *sourcelist,
           fascist_firewall_allows_dir_server(d, FIREWALL_OR_CONNECTION,
                                              try_ip_pref))
         smartlist_add(is_overloaded ? overloaded_tunnel : tunnel, (void*)d);
-      else if (skip_dir ||
+      else if (!must_have_or && (skip_dir ||
                fascist_firewall_allows_dir_server(d, FIREWALL_DIR_CONNECTION,
-                                                  try_ip_pref))
+                                                  try_ip_pref)))
         smartlist_add(is_overloaded ? overloaded_direct : direct, (void*)d);
     }
   SMARTLIST_FOREACH_END(d);