Make clients always use begindir for directory requests

This improves client anonymity and avoids directory header tampering.
The extra load on the authorities should be offset by the fallback
directories feature.

This also simplifies the fixes to #18809.

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".

src/or/directory.c

@@ -964,6 +964,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 +981,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 +1105,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 +1137,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 &&

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,