Merge remote-tracking branch 'teor/bug-13163-AlternateAuthorities-type-handling-fixed'

changes/bug13163-bitwise-check-BRIDGE-DIRINFO

@@ -0,0 +1,5 @@
+  o Minor bugfixes:
+    - Bitwise check the BRIDGE_DIRINFO flag rather than using equality.
+      Fixes a (potential) bug where directories offering BRIDGE_DIRINFO and
+      some other flag (i.e. microdescriptors or extrainfo) would be ignored
+      when looking for bridge directories. Partially fixes bug 13163.

changes/bug13163-stop-AlternateAuthorities-always-using-default-authorities

@@ -0,0 +1,4 @@
+  o Minor bugfixes:
+    - Stop using the default authorities in networks which provide both
+      AlternateDirAuthority and AlternateBridgeAuthority.
+      Partially fixes bug 13163.

changes/issue13163-improve-DIRINFO-flags-comments

@@ -0,0 +1,5 @@
+  o Minor refactoring:
+    - Document usage of the NO_DIRINFO and ALL_DIRINFO flags clearly in
+      functions which take them as arguments. Replace 0 with NO_DIRINFO
+      in a function call for clarity.
+      Seeks to prevent future issues like 13163.

src/or/config.c

@@ -818,7 +818,9 @@ escaped_safe_str(const char *address)
 }
 
 /** Add the default directory authorities directly into the trusted dir list,
- * but only add them insofar as they share bits with <b>type</b>. */
+ * but only add them insofar as they share bits with <b>type</b>.
+ * Each authority's bits are restricted to the bits shared with <b>type</b>.
+ * If <b>type</b> is ALL_DIRINFO or NO_DIRINFO (zero), add all authorities. */
 static void
 add_default_trusted_dir_authorities(dirinfo_type_t type)
 {
@@ -960,7 +962,10 @@ consider_adding_dir_servers(const or_options_t *options,
       type |= BRIDGE_DIRINFO;
     if (!options->AlternateDirAuthority)
       type |= V3_DIRINFO | EXTRAINFO_DIRINFO | MICRODESC_DIRINFO;
-    add_default_trusted_dir_authorities(type);
+    /* if type == NO_DIRINFO, we don't want to add any of the
+     * default authorities, because we've replaced them all */
+    if (type != NO_DIRINFO)
+      add_default_trusted_dir_authorities(type);
   }
   if (!options->FallbackDir)
     add_default_fallback_dir_servers();
@@ -5192,8 +5197,9 @@ parse_server_transport_line(const or_options_t *options,
 /** Read the contents of a DirAuthority line from <b>line</b>. If
  * <b>validate_only</b> is 0, and the line is well-formed, and it
  * shares any bits with <b>required_type</b> or <b>required_type</b>
- * is 0, then add the dirserver described in the line (minus whatever
+ * is NO_DIRINFO (zero), then add the dirserver described in the line
- * bits it's missing) as a valid authority. Return 0 on success,
+ * (minus whatever bits it's missing) as a valid authority.
+ * Return 0 on success or filtering out by type,
  * or -1 if the line isn't well-formed or if we can't add it. */
 static int
 parse_dir_authority_line(const char *line, dirinfo_type_t required_type,

src/or/directory.c

@@ -452,7 +452,7 @@ directory_get_from_dirserver(uint8_t dir_purpose, uint8_t router_purpose,
     return;
 
   if (!get_via_tor) {
-    if (options->UseBridges && type != BRIDGE_DIRINFO) {
+    if (options->UseBridges && !(type & BRIDGE_DIRINFO)) {
       /* We want to ask a running bridge for which we have a descriptor.
        *
        * When we ask choose_random_entry() for a bridge, we specify what
@@ -479,7 +479,7 @@ directory_get_from_dirserver(uint8_t dir_purpose, uint8_t router_purpose,
                            "nodes are available yet.");
       return;
     } else {
-      if (prefer_authority || type == BRIDGE_DIRINFO) {
+      if (prefer_authority || (type & BRIDGE_DIRINFO)) {
         /* only ask authdirservers, and don't ask myself */
         rs = router_pick_trusteddirserver(type, pds_flags);
         if (rs == NULL && (pds_flags & (PDS_NO_EXISTING_SERVERDESC_FETCH|
@@ -506,7 +506,7 @@ directory_get_from_dirserver(uint8_t dir_purpose, uint8_t router_purpose,
           return;
         }
       }
-      if (!rs && type != BRIDGE_DIRINFO) {
+      if (!rs && !(type & BRIDGE_DIRINFO)) {
         /* */
         rs = directory_pick_generic_dirserver(type, pds_flags,
                                               dir_purpose);
@@ -523,12 +523,12 @@ directory_get_from_dirserver(uint8_t dir_purpose, uint8_t router_purpose,
       /* anybody with a non-zero dirport will do. Disregard firewalls. */
       pds_flags |= PDS_IGNORE_FASCISTFIREWALL;
       rs = router_pick_directory_server(type, pds_flags);
-      /* If we have any hope of building an indirect conn, we know some router
-       * descriptors.  If (rs==NULL), we can't build circuits anyway, so
-       * there's no point in falling back to the authorities in this case. */
     }
   }
 
+  /* If we have any hope of building an indirect conn, we know some router
+   * descriptors.  If (rs==NULL), we can't build circuits anyway, so
+   * there's no point in falling back to the authorities in this case. */
   if (rs) {
     const dir_indirection_t indirection =
       get_via_tor ? DIRIND_ANONYMOUS : DIRIND_ONEHOP;

src/or/entrynodes.c

@@ -1003,7 +1003,8 @@ node_understands_microdescriptors(const node_t *node)
 }
 
 /** Return true iff <b>node</b> is able to answer directory questions
- * of type <b>dirinfo</b>. */
+ * of type <b>dirinfo</b>. Always returns true if <b>dirinfo</b> is
+ * NO_DIRINFO (zero). */
 static int
 node_can_handle_dirinfo(const node_t *node, dirinfo_type_t dirinfo)
 {
@@ -1025,13 +1026,13 @@ node_can_handle_dirinfo(const node_t *node, dirinfo_type_t dirinfo)
  * <b>state</b> is non-NULL, this is for a specific circuit --
  * make sure not to pick this circuit's exit or any node in the
  * exit's family. If <b>state</b> is NULL, we're looking for a random
- * guard (likely a bridge).  If <b>dirinfo</b> is not NO_DIRINFO, then
+ * guard (likely a bridge).  If <b>dirinfo</b> is not NO_DIRINFO (zero),
- * only select from nodes that know how to answer directory questions
+ * then only select from nodes that know how to answer directory questions
  * of that type. */
 const node_t *
 choose_random_entry(cpath_build_state_t *state)
 {
-  return choose_random_entry_impl(state, 0, 0, NULL);
+  return choose_random_entry_impl(state, 0, NO_DIRINFO, NULL);
 }
 
 /** Pick a live (up and listed) directory guard from entry_guards for
@@ -1139,7 +1140,9 @@ populate_live_entry_guards(smartlist_t *live_entry_guards,
  * If <b>for_directory</b> is set, we are looking for a directory guard.
  *
  * <b>dirinfo_type</b> contains the kind of directory information we
- * are looking for in our node.
+ * are looking for in our node, or NO_DIRINFO (zero) if we are not
+ * looking for any particular directory information (when set to
+ * NO_DIRINFO, the <b>dirinfo_type</b> filter is ignored).
  *
  * If <b>n_options_out</b> is set, we set it to the number of
  * candidate guard nodes we had before picking a specific guard node.

src/or/routerlist.c

@@ -2534,7 +2534,7 @@ router_is_named(const routerinfo_t *router)
 
 /** Return true iff <b>digest</b> is the digest of the identity key of a
  * trusted directory matching at least one bit of <b>type</b>.  If <b>type</b>
- * is zero, any authority is okay. */
+ * is zero (NO_DIRINFO), or ALL_DIRINFO, any authority is okay. */
 int
 router_digest_is_trusted_dir_type(const char *digest, dirinfo_type_t type)
 {