Merge branch 'bug40578' into 'main'

new mode that only fetches bridge descs as needed

See merge request tpo/core/tor!688

changes/bug40578

@@ -0,0 +1,8 @@
+  o Major bugfixes (bridges):
+    - Add a new config option FetchBridgeDescsJIT to fetch bridge
+      descriptors "just in time" for the bridges that Tor plans to
+      use. Now if you have a large bridge list (such as when using
+      Tor Browser's built-in bridges) or you are using transports like
+      Snowflake where each descriptor fetch is an expensive operation,
+      you will only reach out to the bridges whose descriptors you
+      actually need. Fixes bug 40578; bugfix on 0.2.0.3-alpha.

changes/bug40746

@@ -0,0 +1,7 @@
+  o Minor bugfixes (bridge users):
+    - When choosing a bridge to fetch directory info from, only select
+      from among bridges whose descriptors we already have. In theory we
+      don't need the bridge's descriptor to ask it for directory info,
+      but in practice the code that does the fetching assumes we do. This
+      mistake only slows down bootstrap, but shouldn't prevent it. Fixes
+      bug 40746; bugfix on 0.3.0.3-alpha.

doc/man/tor.1.txt

@@ -594,6 +594,14 @@ forward slash (/) in the configuration file and on the command line.
     FallbackDir replaces Tor's default hard-coded FallbackDirs (if any).
     (See <<DirAuthority,DirAuthority>> for an explanation of each flag.)
 
+[[FetchBridgeDescsJIT]] **FetchBridgeDescsJIT** **0**|**1**::
+    If set to 1 along with UseBridges, Tor will only fetch descriptors for
+    the first NumEntryGuards (or guard-n-primary-guards-to-use) bridges,
+    rather than the default behavior of fetching descriptors for every
+    configured bridge. When this option is set, we change behavior to use
+    this more conservative value rather than the larger NumDirectoryGuards
+    (or guard-n-primary-dir-guards-to-use). (Default: 0)
+
 [[FetchDirInfoEarly]] **FetchDirInfoEarly** **0**|**1**::
     If set to 1, Tor will always fetch directory information like other
     directory caches, even if you don't meet the normal criteria for fetching

src/app/config/config.c

@@ -473,6 +473,7 @@ static const config_var_t option_vars_[] = {
   V(FascistFirewall,             BOOL,     "0"),
   V(FirewallPorts,               CSV,      ""),
   OBSOLETE("FastFirstHopPK"),
+  V(FetchBridgeDescsJIT,         BOOL,     "0"),
   V(FetchDirInfoEarly,           BOOL,     "0"),
   V(FetchDirInfoExtraEarly,      BOOL,     "0"),
   V(FetchServerDescriptors,      BOOL,     "1"),
@@ -3425,6 +3426,17 @@ options_validate_cb(const void *old_options_, void *options_, char **msg)
     REJECT("FetchDirInfoExtraEarly requires that you also set "
            "FetchDirInfoEarly");
 
+  if (options->FetchBridgeDescsJIT && !options->UseBridges)
+    REJECT("FetchBridgeDescsJIT requires that you also set UseBridges.");
+
+  if (options->FetchBridgeDescsJIT && options->UpdateBridgesFromAuthority) {
+    /* there's no reason in principle why we can't allow both, but we
+     * really should be making UpdateBridgesFromAuthority obsolete, so
+     * we skipped supporting it here. */
+    REJECT("Cannot set both FetchBridgeDescsJIT and "
+           "UpdateBridgesFromAuthority.");
+  }
+
   if (options->ConnLimit <= 0) {
     tor_asprintf(msg,
         "ConnLimit must be greater than 0, but was set to %d",

src/app/config/or_options_st.h

@@ -608,6 +608,11 @@ struct or_options_t {
   int SbwsExit;
 
   int RephistTrackTime; /**< How many seconds do we keep rephist info? */
+
+  /** When using bridges, should we fetch descriptors for only the bridges
+   * we plan to use? Off by default until it's gotten more testing. */
+  int FetchBridgeDescsJIT;
+
   /** Should we always fetch our dir info on the mirror schedule (which
    * means directly from the authorities) no matter our other config? */
   int FetchDirInfoEarly;

src/feature/client/bridges.c

@@ -691,12 +691,15 @@ get_socks_args_by_bridge_addrport(const tor_addr_t *addr, uint16_t port)
 }
 
 /** We need to ask <b>bridge</b> for its server descriptor. */
-static void
+void
 launch_direct_bridge_descriptor_fetch(bridge_info_t *bridge)
 {
   const or_options_t *options = get_options();
   circuit_guard_state_t *guard_state = NULL;
 
+  log_info(LD_GUARD, "Wanting descriptor fetch for bridge %s",
+           safe_str_client(fmt_and_decorate_addr(&bridge->addr)));
+
   if (connection_get_by_type_addr_port_purpose(
       CONN_TYPE_DIR, &bridge->addr, bridge->port,
       DIR_PURPOSE_FETCH_SERVERDESC))
@@ -765,6 +768,33 @@ retry_bridge_descriptor_fetch_directly(const char *digest)
   launch_direct_bridge_descriptor_fetch(bridge);
 }
 
+/** Decide whether we're ready to launch a descriptor fetch for this
+ * bridge, and if yes, prep the download fetch status for the launch.
+ * Return 1 if we want to launch, 0 if we don't want to.
+ */
+int
+prep_for_bridge_descriptor_fetch(bridge_info_t *bridge,
+                                 const or_options_t *options, time_t now)
+{
+  /* This resets the download status on first use */
+  if (!download_status_is_ready(&bridge->fetch_status, now))
+    return 0; /* don't bother, no need to retry yet */
+  if (routerset_contains_bridge(options->ExcludeNodes, bridge)) {
+    download_status_mark_impossible(&bridge->fetch_status);
+    log_warn(LD_APP, "Not using bridge at %s: it is in ExcludeNodes.",
+             safe_str_client(fmt_and_decorate_addr(&bridge->addr)));
+    return 0;
+  }
+  /* schedule the next attempt
+   * we can't increment after a failure, because sometimes we use the
+   * bridge authority, and sometimes we use the bridge direct */
+  download_status_increment_attempt(
+                    &bridge->fetch_status,
+                    safe_str_client(fmt_and_decorate_addr(&bridge->addr)),
+                    now);
+  return 1; /* all set, proceed with fetching */
+}
+
 /** For each bridge in our list for which we don't currently have a
  * descriptor, fetch a new copy of its descriptor -- either directly
  * from the bridge or via a bridge authority. */
@@ -783,25 +813,24 @@ fetch_bridge_descriptors(const or_options_t *options, time_t now)
   if (pt_proxies_configuration_pending())
     return;
 
-  SMARTLIST_FOREACH_BEGIN(bridge_list, bridge_info_t *, bridge)
+  /* At this point this function goes in one of two directions:
-    {
+   *
-      /* This resets the download status on first use */
+   * (A) If FetchBridgeDescsJIT is set, we ask the entryguard subsystem
-      if (!download_status_is_ready(&bridge->fetch_status, now))
+   * to step through the first n guards and launch descriptor fetches
-        continue; /* don't bother, no need to retry yet */
+   * for them if we don't yet have a descriptor and the timers are ready.
-      if (routerset_contains_bridge(options->ExcludeNodes, bridge)) {
+   */
-        download_status_mark_impossible(&bridge->fetch_status);
+  if (options->FetchBridgeDescsJIT) {
-        log_warn(LD_APP, "Not using bridge at %s: it is in ExcludeNodes.",
+    fetch_descriptors_for_first_guards(options, now);
-                 safe_str_client(fmt_and_decorate_addr(&bridge->addr)));
+    return;
-        continue;
   }
 
-      /* schedule the next attempt
+  /* (B) Else, we do the old behavior where we walk the entire bridge
-       * we can't increment after a failure, because sometimes we use the
+   * list and fetch descriptors for each of them as needed.
-       * bridge authority, and sometimes we use the bridge direct */
+   */
-      download_status_increment_attempt(
+  SMARTLIST_FOREACH_BEGIN(bridge_list, bridge_info_t *, bridge)
-                        &bridge->fetch_status,
+    {
-                        safe_str_client(fmt_and_decorate_addr(&bridge->addr)),
+      if (!prep_for_bridge_descriptor_fetch(bridge, options, now))
-                        now);
+        continue;
 
       can_use_bridge_authority = !tor_digest_is_zero(bridge->identity) &&
                                  num_bridge_auths;

src/feature/client/bridges.h

@@ -45,11 +45,14 @@ void learned_router_identity(const tor_addr_t *addr, uint16_t port,
 
 void bridge_add_from_config(struct bridge_line_t *bridge_line);
 void retry_bridge_descriptor_fetch_directly(const char *digest);
+int prep_for_bridge_descriptor_fetch(bridge_info_t *bridge,
+                                     const or_options_t *options, time_t now);
 void fetch_bridge_descriptors(const or_options_t *options, time_t now);
 void learned_bridge_descriptor(routerinfo_t *ri,
                                int from_cache, int desc_is_new);
 const smartlist_t *get_socks_args_by_bridge_addrport(const tor_addr_t *addr,
                                                      uint16_t port);
+void launch_direct_bridge_descriptor_fetch(bridge_info_t *bridge);
 
 int any_bridges_dont_support_microdescriptors(void);
 

src/feature/client/entrynodes.c

@@ -489,8 +489,13 @@ get_n_primary_guards_to_use(guard_usage_t usage)
   int param_default;
 
   /* If the user has explicitly configured the amount of guards, use
-     that. Otherwise, fall back to the default value. */
+   * that. Otherwise, fall back to the default value.
-  if (usage == GUARD_USAGE_DIRGUARD) {
+   *
+   * If we're planning to use a directory guard, use the bigger number;
+   * but as an exception use the more conservative pick if we're configured
+   * to minimize the number of bridges we touch. */
+  if (usage == GUARD_USAGE_DIRGUARD &&
+      !get_options()->FetchBridgeDescsJIT) {
     configured = get_options()->NumDirectoryGuards;
     param_name = "guard-n-primary-dir-guards-to-use";
     param_default = DFLT_N_PRIMARY_DIR_GUARDS_TO_USE;
@@ -2135,7 +2140,8 @@ select_primary_guard_for_circuit(guard_selection_t *gs,
                                  const entry_guard_restriction_t *rst,
                                  unsigned *state_out)
 {
-  const int need_descriptor = (usage == GUARD_USAGE_TRAFFIC);
+  const int need_descriptor = (usage == GUARD_USAGE_TRAFFIC) ||
+                              get_options()->UseBridges;
   entry_guard_t *chosen_guard = NULL;
 
   int num_entry_guards_to_consider = get_n_primary_guards_to_use(usage);
@@ -2229,7 +2235,8 @@ select_confirmed_guard_for_circuit(guard_selection_t *gs,
                                   const entry_guard_restriction_t *rst,
                                   unsigned *state_out)
 {
-  const int need_descriptor = (usage == GUARD_USAGE_TRAFFIC);
+  const int need_descriptor = (usage == GUARD_USAGE_TRAFFIC) ||
+                              get_options()->UseBridges;
 
   SMARTLIST_FOREACH_BEGIN(gs->confirmed_entry_guards, entry_guard_t *, guard) {
     if (guard->is_primary)
@@ -3975,6 +3982,39 @@ guards_retry_optimistic(const or_options_t *options)
   return 1;
 }
 
+/** Fetch descriptors but for the bridges that we intend to actually
+ * use, as sorted by the sampled guard list. Don't fetch descriptors
+ * for bridges later on the guard list. */
+void
+fetch_descriptors_for_first_guards(const or_options_t *options, time_t now)
+{
+  guard_selection_t *gs = get_guard_selection_info();
+  if (!gs->primary_guards_up_to_date)
+    entry_guards_update_primary(gs);
+
+  int n_considered = 0;
+  int num_primary_to_check = get_n_primary_guards_to_use(GUARD_USAGE_TRAFFIC);
+
+  SMARTLIST_FOREACH_BEGIN(gs->primary_entry_guards, entry_guard_t *, guard) {
+    entry_guard_consider_retry(guard);
+    if (guard->is_reachable == GUARD_REACHABLE_NO)
+      continue;
+    n_considered++;
+    if (!guard_has_descriptor(guard)) {
+      /* we want to try fetching it now */
+      bridge_info_t *bridge = get_bridge_info_for_guard(guard);
+      if (!prep_for_bridge_descriptor_fetch(bridge, options, now))
+        continue;
+      launch_direct_bridge_descriptor_fetch(bridge);
+    }
+    if (n_considered >= num_primary_to_check) {
+//      log_info(LD_GUARD, "Have looked at first %d guards; good enough.",
+//               n_considered);
+      break;
+    }
+  } SMARTLIST_FOREACH_END(guard);
+}
+
 /**
  * Check if we are missing any crucial dirinfo for the guard subsystem to
  * work. Return NULL if everything went well, otherwise return a newly
@@ -3994,11 +4034,10 @@ guard_selection_get_err_str_if_dir_info_missing(guard_selection_t *gs,
   int n_considered = 0;
   int num_primary_to_check;
 
-  /* We want to check for the descriptor of at least the first two primary
+  /* We want to check for the descriptor of the first n primary
    * guards in our list, since these are the guards that we typically use for
    * circuits. */
   num_primary_to_check = get_n_primary_guards_to_use(GUARD_USAGE_TRAFFIC);
-  num_primary_to_check++;
 
   SMARTLIST_FOREACH_BEGIN(gs->primary_entry_guards, entry_guard_t *, guard) {
     entry_guard_consider_retry(guard);

src/feature/client/entrynodes.h

@@ -634,6 +634,9 @@ int getinfo_helper_entry_guards(control_connection_t *conn,
 int entries_known_but_down(const or_options_t *options);
 void entries_retry_all(const or_options_t *options);
 
+void fetch_descriptors_for_first_guards(const or_options_t *options,
+                                        time_t now);
+
 char *entry_guards_get_err_str_if_dir_info_missing(int using_mds,
                                            int num_present, int num_usable);
 char *guard_selection_get_err_str_if_dir_info_missing(guard_selection_t *gs,

src/test/test_entrynodes.c

@@ -1744,7 +1744,7 @@ test_entry_guard_manage_primary(void *arg)
     memset(first_primary->identity, 9, sizeof(first_primary->identity));
     dir_info_str =guard_selection_get_err_str_if_dir_info_missing(gs, 1, 2, 3);
     tt_str_op(dir_info_str, OP_EQ,
-              "We're missing descriptors for 1/2 of our primary entry guards "
+              "We're missing descriptors for 1/1 of our primary entry guards "
               "(total microdescriptors: 2/3). That's ok. We will try to fetch "
               "missing descriptors soon.");
     tor_free(dir_info_str);