Refactor to use purpose_needs_anonymity and remove is_sensitive_dir_purpose

changes/ticket20077

@@ -0,0 +1,3 @@
+  o Code simplification and refactoring:
+    - Remove redundant behavior of is_sensitive_dir_purpose, refactor to use
+      only purpose_needs_anonymity

src/or/connection_edge.c

@@ -2434,7 +2434,7 @@ connection_ap_handshake_send_begin(entry_connection_t *ap_conn)
      * Otherwise, directory connections are typically one-hop.
      * This matches the earlier check for directory connection path anonymity
      * in directory_initiate_command_rend(). */
-    if (is_sensitive_dir_purpose(linked_dir_conn_base->purpose)) {
+    if (purpose_needs_anonymity(linked_dir_conn_base->purpose, 0)) {
       assert_circ_anonymity_ok(circ, options);
     }
   } else {

src/or/directory.c

@@ -120,17 +120,22 @@ static void connection_dir_close_consensus_fetches(
 
 /********* END VARIABLES ************/
 
-/** Return true iff the directory purpose <b>dir_purpose</b> (and if it's
+/** Return false if the directory purpose <b>dir_purpose</b>
- * fetching descriptors, it's fetching them for <b>router_purpose</b>)
+ * does not require an anonymous (three-hop) connection.
- * must use an anonymous connection to a directory. */
+ *
+ * Return true 1) by default, 2) if all directory actions have
+ * specifically been configured to be over an anonymous connection,
+ * or 3) if the router is a bridge */
 int
 purpose_needs_anonymity(uint8_t dir_purpose, uint8_t router_purpose)
 {
   if (get_options()->AllDirActionsPrivate)
     return 1;
+
   if (router_purpose == ROUTER_PURPOSE_BRIDGE)
     return 1; /* if no circuits yet, this might break bootstrapping, but it's
                * needed to be safe. */
+
   if (dir_purpose == DIR_PURPOSE_UPLOAD_DIR ||
       dir_purpose == DIR_PURPOSE_UPLOAD_VOTE ||
       dir_purpose == DIR_PURPOSE_UPLOAD_SIGNATURES ||
@@ -1078,18 +1083,6 @@ directory_initiate_command(const tor_addr_t *or_addr, uint16_t or_port,
                              if_modified_since, NULL);
 }
 
-/** Return non-zero iff a directory connection with purpose
- * <b>dir_purpose</b> reveals sensitive information about a Tor
- * instance's client activities.  (Such connections must be performed
- * through normal three-hop Tor circuits.) */
-int
-is_sensitive_dir_purpose(uint8_t dir_purpose)
-{
-  return ((dir_purpose == DIR_PURPOSE_HAS_FETCHED_RENDDESC_V2) ||
-          (dir_purpose == DIR_PURPOSE_UPLOAD_RENDDESC_V2) ||
-          (dir_purpose == DIR_PURPOSE_FETCH_RENDDESC_V2));
-}
-
 /** Same as directory_initiate_command(), but accepts rendezvous data to
  * fetch a hidden service descriptor, and takes its address & port arguments
  * as tor_addr_port_t. */
@@ -1137,7 +1130,7 @@ directory_initiate_command_rend(const tor_addr_port_t *or_addr_port,
 
   log_debug(LD_DIR, "Initiating %s", dir_conn_purpose_to_string(dir_purpose));
 
-  if (is_sensitive_dir_purpose(dir_purpose)) {
+  if (purpose_needs_anonymity(dir_purpose, router_purpose)) {
     tor_assert(anonymized_connection ||
                rend_non_anonymous_mode_enabled(options));
   }

src/or/directory.h

@@ -132,10 +132,7 @@ int download_status_get_n_failures(const download_status_t *dls);
 int download_status_get_n_attempts(const download_status_t *dls);
 time_t download_status_get_next_attempt_at(const download_status_t *dls);
 
-/* Yes, these two functions are confusingly similar.
- * Let's sort that out in #20077. */
 int purpose_needs_anonymity(uint8_t dir_purpose, uint8_t router_purpose);
-int is_sensitive_dir_purpose(uint8_t dir_purpose);
 
 #ifdef TOR_UNIT_TESTS
 /* Used only by directory.c and test_dir.c */

src/test/test_dir.c

@@ -3253,16 +3253,51 @@ test_dir_http_handling(void *args)
 }
 
 static void
-test_dir_purpose_needs_anonymity(void *arg)
+test_dir_purpose_needs_anonymity_returns_true_for_bridges(void *arg)
 {
   (void)arg;
+
   tt_int_op(1, ==, purpose_needs_anonymity(0, ROUTER_PURPOSE_BRIDGE));
-  tt_int_op(1, ==, purpose_needs_anonymity(0, ROUTER_PURPOSE_GENERAL));
+  tt_int_op(1, ==, purpose_needs_anonymity(DIR_PURPOSE_HAS_FETCHED_RENDDESC_V2,
-  tt_int_op(0, ==, purpose_needs_anonymity(DIR_PURPOSE_FETCH_MICRODESC,
+                                   ROUTER_PURPOSE_BRIDGE));
-                                            ROUTER_PURPOSE_GENERAL));
  done: ;
 }
 
+static void
+test_dir_purpose_needs_anonymity_returns_true_for_sensitive_purpose(void *arg)
+{
+  (void)arg;
+
+  tt_int_op(1, ==, purpose_needs_anonymity(
+                    DIR_PURPOSE_HAS_FETCHED_RENDDESC_V2,
+                    ROUTER_PURPOSE_GENERAL));
+  tt_int_op(1, ==, purpose_needs_anonymity(
+                      DIR_PURPOSE_UPLOAD_RENDDESC_V2, 0));
+  tt_int_op(1, ==, purpose_needs_anonymity(
+                      DIR_PURPOSE_FETCH_RENDDESC_V2, 0));
+ done: ;
+}
+
+static void
+test_dir_purpose_needs_anonymity_ret_false_for_non_sensitive_conn(void *arg)
+{
+  (void)arg;
+
+  tt_int_op(0, ==, purpose_needs_anonymity(DIR_PURPOSE_UPLOAD_DIR,
+                                           ROUTER_PURPOSE_GENERAL));
+  tt_int_op(0, ==, purpose_needs_anonymity(DIR_PURPOSE_UPLOAD_VOTE, 0));
+  tt_int_op(0, ==, purpose_needs_anonymity(DIR_PURPOSE_UPLOAD_SIGNATURES, 0));
+  tt_int_op(0, ==, purpose_needs_anonymity(DIR_PURPOSE_FETCH_STATUS_VOTE, 0));
+  tt_int_op(0, ==, purpose_needs_anonymity(
+                    DIR_PURPOSE_FETCH_DETACHED_SIGNATURES, 0));
+  tt_int_op(0, ==, purpose_needs_anonymity(DIR_PURPOSE_FETCH_CONSENSUS, 0));
+  tt_int_op(0, ==, purpose_needs_anonymity(DIR_PURPOSE_FETCH_CERTIFICATE, 0));
+  tt_int_op(0, ==, purpose_needs_anonymity(DIR_PURPOSE_FETCH_SERVERDESC, 0));
+  tt_int_op(0, ==, purpose_needs_anonymity(DIR_PURPOSE_FETCH_EXTRAINFO, 0));
+  tt_int_op(0, ==, purpose_needs_anonymity(DIR_PURPOSE_FETCH_MICRODESC, 0));
+  done: ;
+}
+
 static void
 test_dir_fetch_type(void *arg)
 {
@@ -5464,7 +5499,9 @@ struct testcase_t dir_tests[] = {
   DIR(fmt_control_ns, 0),
   DIR(dirserv_set_routerstatus_testing, 0),
   DIR(http_handling, 0),
-  DIR(purpose_needs_anonymity, 0),
+  DIR(purpose_needs_anonymity_returns_true_for_bridges, 0),
+  DIR(purpose_needs_anonymity_returns_true_for_sensitive_purpose, 0),
+  DIR(purpose_needs_anonymity_ret_false_for_non_sensitive_conn, 0),
   DIR(fetch_type, 0),
   DIR(packages, 0),
   DIR(download_status_schedule, 0),