From c1dd43d823c74f59f8ba0eff90f6ddab6763f408 Mon Sep 17 00:00:00 2001 From: teor Date: Wed, 1 Oct 2014 18:37:19 +1000 Subject: [PATCH 1/3] Stop using default authorities with both Alternate Dir and Bridge Authority Stop using the default authorities in networks which provide both AlternateDirAuthority and AlternateBridgeAuthority. This bug occurred due to an ambiguity around the use of NO_DIRINFO. (Does it mean "any" or "none"?) Partially fixes bug 13163. --- ...top-AlternateAuthorities-always-using-default-authorities | 4 ++++ src/or/config.c | 5 ++++- 2 files changed, 8 insertions(+), 1 deletion(-) create mode 100644 changes/bug13163-stop-AlternateAuthorities-always-using-default-authorities diff --git a/changes/bug13163-stop-AlternateAuthorities-always-using-default-authorities b/changes/bug13163-stop-AlternateAuthorities-always-using-default-authorities new file mode 100644 index 0000000000..eeaca926a2 --- /dev/null +++ b/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. diff --git a/src/or/config.c b/src/or/config.c index 921d032529..493cfcd2bc 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -959,7 +959,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(); From ff42222845f5d79f27b653990cab4f20ad91a068 Mon Sep 17 00:00:00 2001 From: teor Date: Wed, 1 Oct 2014 18:54:26 +1000 Subject: [PATCH 2/3] Improve DIRINFO flags' usage comments 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. --- changes/issue13163-improve-DIRINFO-flags-comments | 5 +++++ src/or/config.c | 9 ++++++--- src/or/directory.c | 6 +++--- src/or/entrynodes.c | 13 ++++++++----- src/or/routerlist.c | 2 +- 5 files changed, 23 insertions(+), 12 deletions(-) create mode 100644 changes/issue13163-improve-DIRINFO-flags-comments diff --git a/changes/issue13163-improve-DIRINFO-flags-comments b/changes/issue13163-improve-DIRINFO-flags-comments new file mode 100644 index 0000000000..3acb1f3caf --- /dev/null +++ b/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. diff --git a/src/or/config.c b/src/or/config.c index 493cfcd2bc..cacf52a3fa 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -817,7 +817,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 type. */ + * but only add them insofar as they share bits with type. + * Each authority's bits are restricted to the bits shared with type. + * If type is ALL_DIRINFO or NO_DIRINFO (zero), add all authorities. */ static void add_default_trusted_dir_authorities(dirinfo_type_t type) { @@ -5194,8 +5196,9 @@ parse_server_transport_line(const or_options_t *options, /** Read the contents of a DirAuthority line from line. If * validate_only is 0, and the line is well-formed, and it * shares any bits with required_type or required_type - * is 0, then add the dirserver described in the line (minus whatever - * bits it's missing) as a valid authority. Return 0 on success, + * is NO_DIRINFO (zero), then add the dirserver described in the line + * (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, diff --git a/src/or/directory.c b/src/or/directory.c index 1aaa75ccee..12717ebfed 100644 --- a/src/or/directory.c +++ b/src/or/directory.c @@ -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; diff --git a/src/or/entrynodes.c b/src/or/entrynodes.c index b1fd310f97..b160235289 100644 --- a/src/or/entrynodes.c +++ b/src/or/entrynodes.c @@ -1003,7 +1003,8 @@ node_understands_microdescriptors(const node_t *node) } /** Return true iff node is able to answer directory questions - * of type dirinfo. */ + * of type dirinfo. Always returns true if dirinfo 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) * state 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 state is NULL, we're looking for a random - * guard (likely a bridge). If dirinfo is not NO_DIRINFO, then - * only select from nodes that know how to answer directory questions + * guard (likely a bridge). If dirinfo is not NO_DIRINFO (zero), + * 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 for_directory is set, we are looking for a directory guard. * * dirinfo_type 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 dirinfo_type filter is ignored). * * If n_options_out is set, we set it to the number of * candidate guard nodes we had before picking a specific guard node. diff --git a/src/or/routerlist.c b/src/or/routerlist.c index 22489a4476..e93482adec 100644 --- a/src/or/routerlist.c +++ b/src/or/routerlist.c @@ -2534,7 +2534,7 @@ router_is_named(const routerinfo_t *router) /** Return true iff digest is the digest of the identity key of a * trusted directory matching at least one bit of type. If type - * 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) { From 31bf8f26907945b8f26a7543e42ce0e92dd4918f Mon Sep 17 00:00:00 2001 From: teor Date: Wed, 1 Oct 2014 19:04:04 +1000 Subject: [PATCH 3/3] Bitwise check BRIDGE_DIRINFO Bitwise check for the BRIDGE_DIRINFO flag, rather than checking for 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. Final fix in series for bug 13163. --- changes/bug13163-bitwise-check-BRIDGE-DIRINFO | 5 +++++ src/or/directory.c | 6 +++--- 2 files changed, 8 insertions(+), 3 deletions(-) create mode 100644 changes/bug13163-bitwise-check-BRIDGE-DIRINFO diff --git a/changes/bug13163-bitwise-check-BRIDGE-DIRINFO b/changes/bug13163-bitwise-check-BRIDGE-DIRINFO new file mode 100644 index 0000000000..7f5ec05037 --- /dev/null +++ b/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. diff --git a/src/or/directory.c b/src/or/directory.c index 12717ebfed..83cc56f352 100644 --- a/src/or/directory.c +++ b/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);