Merge branch 'tor-github/pr/1000'

Signed-off-by: David Goulet <dgoulet@torproject.org>

changes/bug30236

@@ -0,0 +1,3 @@
+  o Code simplification and refactoring:
+    - Refactor and encapsulate parts of the codebase that manipulate
+      crypt_path_t objects. Resolves issue 30236.

scripts/maint/practracker/exceptions.txt

@@ -54,8 +54,8 @@ problem function-size /src/app/main/main.c:sandbox_init_filter() 291
 problem function-size /src/app/main/main.c:run_tor_main_loop() 105
 problem function-size /src/app/main/ntmain.c:nt_service_install() 125
 problem include-count /src/app/main/shutdown.c 52
-problem file-size /src/core/mainloop/connection.c 5559
-problem include-count /src/core/mainloop/connection.c 61
+problem file-size /src/core/mainloop/connection.c 5560
+problem include-count /src/core/mainloop/connection.c 62
 problem function-size /src/core/mainloop/connection.c:connection_free_minimal() 185
 problem function-size /src/core/mainloop/connection.c:connection_listener_new() 328
 problem function-size /src/core/mainloop/connection.c:connection_handle_listener_read() 161
@@ -79,11 +79,11 @@ problem function-size /src/core/or/channeltls.c:channel_tls_process_netinfo_cell
 problem function-size /src/core/or/channeltls.c:channel_tls_process_certs_cell() 246
 problem function-size /src/core/or/channeltls.c:channel_tls_process_authenticate_cell() 202
 problem file-size /src/core/or/circuitbuild.c 3061
-problem include-count /src/core/or/circuitbuild.c 53
+problem include-count /src/core/or/circuitbuild.c 54
 problem function-size /src/core/or/circuitbuild.c:get_unique_circ_id_by_chan() 128
 problem function-size /src/core/or/circuitbuild.c:circuit_extend() 147
 problem function-size /src/core/or/circuitbuild.c:choose_good_exit_server_general() 206
-problem include-count /src/core/or/circuitlist.c 54
+problem include-count /src/core/or/circuitlist.c 55
 problem function-size /src/core/or/circuitlist.c:HT_PROTOTYPE() 128
 problem function-size /src/core/or/circuitlist.c:circuit_free_() 143
 problem function-size /src/core/or/circuitlist.c:circuit_find_to_cannibalize() 102
@@ -245,7 +245,7 @@ problem function-size /src/feature/rend/rendmid.c:rend_mid_establish_intro_legac
 problem function-size /src/feature/rend/rendparse.c:rend_parse_v2_service_descriptor() 187
 problem function-size /src/feature/rend/rendparse.c:rend_decrypt_introduction_points() 104
 problem function-size /src/feature/rend/rendparse.c:rend_parse_introduction_points() 131
-problem file-size /src/feature/rend/rendservice.c 4510
+problem file-size /src/feature/rend/rendservice.c 4511
 problem function-size /src/feature/rend/rendservice.c:rend_service_prune_list_impl_() 107
 problem function-size /src/feature/rend/rendservice.c:rend_config_service() 164
 problem function-size /src/feature/rend/rendservice.c:rend_service_load_auth_keys() 178

src/core/crypto/relay_crypto.c

@@ -6,6 +6,7 @@
 
 #include "core/or/or.h"
 #include "core/or/circuitlist.h"
+#include "core/or/crypt_path.h"
 #include "app/config/config.h"
 #include "lib/crypt_ops/crypto_cipher.h"
 #include "lib/crypt_ops/crypto_util.h"
@@ -21,7 +22,7 @@
 /** Update digest from the payload of cell. Assign integrity part to
  * cell.
  */
-static void
+void
 relay_set_digest(crypto_digest_t *digest, cell_t *cell)
 {
   char integrity[4];
@@ -85,7 +86,7 @@ relay_digest_matches(crypto_digest_t *digest, cell_t *cell)
  *
  * Note that we use the same operation for encrypting and for decrypting.
  */
-static void
+void
 relay_crypt_one_payload(crypto_cipher_t *cipher, uint8_t *in)
 {
   crypto_cipher_crypt_inplace(cipher, (char*) in, CELL_PAYLOAD_SIZE);
@@ -152,18 +153,18 @@ relay_decrypt_cell(circuit_t *circ, cell_t *cell,
         tor_assert(thishop);
 
         /* decrypt one layer */
-        relay_crypt_one_payload(thishop->crypto.b_crypto, cell->payload);
+        cpath_crypt_cell(thishop, cell->payload, true);
 
         relay_header_unpack(&rh, cell->payload);
         if (rh.recognized == 0) {
           /* it's possibly recognized. have to check digest to be sure. */
-          if (relay_digest_matches(thishop->crypto.b_digest, cell)) {
+          if (relay_digest_matches(cpath_get_incoming_digest(thishop), cell)) {
             *recognized = 1;
             *layer_hint = thishop;
             /* This cell is for us. Keep a record of this cell because we will
              * use it in the next SENDME cell. */
             if (sendme_circuit_cell_is_next(thishop->deliver_window)) {
-              sendme_circuit_record_inbound_cell(thishop);
+              cpath_sendme_circuit_record_inbound_cell(thishop);
             }
             return 0;
           }
@@ -210,14 +211,14 @@ relay_encrypt_cell_outbound(cell_t *cell,
                             crypt_path_t *layer_hint)
 {
   crypt_path_t *thishop; /* counter for repeated crypts */
-  relay_set_digest(layer_hint->crypto.f_digest, cell);
+  cpath_set_cell_forward_digest(layer_hint, cell);
 
   thishop = layer_hint;
   /* moving from farthest to nearest hop */
   do {
     tor_assert(thishop);
     log_debug(LD_OR,"encrypting a layer of the relay cell.");
-    relay_crypt_one_payload(thishop->crypto.f_crypto, cell->payload);
+    cpath_crypt_cell(thishop, cell->payload, false);
 
     thishop = thishop->prev;
   } while (thishop != circ->cpath->prev);

src/core/crypto/relay_crypto.h

@@ -29,6 +29,11 @@ void relay_crypto_assert_ok(const relay_crypto_t *crypto);
 
 uint8_t *relay_crypto_get_sendme_digest(relay_crypto_t *crypto);
 void relay_crypto_record_sendme_digest(relay_crypto_t *crypto);
+void
+relay_crypt_one_payload(crypto_cipher_t *cipher, uint8_t *in);
+
+void
+relay_set_digest(crypto_digest_t *digest, cell_t *cell);
 
 #endif /* !defined(TOR_RELAY_CRYPTO_H) */
 

src/core/include.am

@@ -39,6 +39,7 @@ LIBTOR_APP_A_SOURCES = 				\
 	src/core/or/circuitpadding.c		\
 	src/core/or/circuitstats.c		\
 	src/core/or/circuituse.c		\
+	src/core/or/crypt_path.c		\
 	src/core/or/command.c			\
 	src/core/or/connection_edge.c		\
 	src/core/or/connection_or.c		\
@@ -247,6 +248,7 @@ noinst_HEADERS +=					\
 	src/core/or/connection_edge.h			\
 	src/core/or/connection_or.h			\
 	src/core/or/connection_st.h			\
+	src/core/or/crypt_path.h      		\
 	src/core/or/cpath_build_state_st.h		\
 	src/core/or/crypt_path_reference_st.h		\
 	src/core/or/crypt_path_st.h			\

src/core/mainloop/connection.c

@@ -82,6 +82,7 @@
 #include "core/or/policies.h"
 #include "core/or/reasons.h"
 #include "core/or/relay.h"
+#include "core/or/crypt_path.h"
 #include "core/proto/proto_http.h"
 #include "core/proto/proto_socks.h"
 #include "feature/client/dnsserv.h"
@@ -5330,7 +5331,7 @@ assert_connection_ok(connection_t *conn, time_t now)
         tor_assert(entry_conn->socks_request->has_finished);
         if (!conn->marked_for_close) {
           tor_assert(ENTRY_TO_EDGE_CONN(entry_conn)->cpath_layer);
-          assert_cpath_layer_ok(ENTRY_TO_EDGE_CONN(entry_conn)->cpath_layer);
+          cpath_assert_layer_ok(ENTRY_TO_EDGE_CONN(entry_conn)->cpath_layer);
         }
       }
     }

src/core/or/circuitbuild.c

@@ -51,6 +51,7 @@
 #include "core/or/ocirc_event.h"
 #include "core/or/policies.h"
 #include "core/or/relay.h"
+#include "core/or/crypt_path.h"
 #include "feature/client/bridges.h"
 #include "feature/client/circpathbias.h"
 #include "feature/client/entrynodes.h"
@@ -90,8 +91,6 @@ static channel_t * channel_connect_for_circuit(const tor_addr_t *addr,
 static int circuit_deliver_create_cell(circuit_t *circ,
                                        const create_cell_t *create_cell,
                                        int relayed);
-static crypt_path_t *onion_next_hop_in_cpath(crypt_path_t *cpath);
-STATIC int onion_append_hop(crypt_path_t **head_ptr, extend_info_t *choice);
 static int circuit_send_first_onion_skin(origin_circuit_t *circ);
 static int circuit_build_no_more_hops(origin_circuit_t *circ);
 static int circuit_send_intermediate_onion_skin(origin_circuit_t *circ,
@@ -547,7 +546,7 @@ circuit_handle_first_hop(origin_circuit_t *circ)
   int should_launch = 0;
   const or_options_t *options = get_options();
 
-  firsthop = onion_next_hop_in_cpath(circ->cpath);
+  firsthop = cpath_get_next_non_open_hop(circ->cpath);
   tor_assert(firsthop);
   tor_assert(firsthop->extend_info);
 
@@ -948,7 +947,7 @@ circuit_send_next_onion_skin(origin_circuit_t *circ)
   tor_assert(circ->cpath->state == CPATH_STATE_OPEN);
   tor_assert(circ->base_.state == CIRCUIT_STATE_BUILDING);
 
-  crypt_path_t *hop = onion_next_hop_in_cpath(circ->cpath);
+  crypt_path_t *hop = cpath_get_next_non_open_hop(circ->cpath);
   circuit_build_times_handle_completed_hop(circ);
 
   circpad_machine_event_circ_added_hop(circ);
@@ -1360,34 +1359,6 @@ circuit_extend(cell_t *cell, circuit_t *circ)
   return 0;
 }
 
-/** Initialize cpath-\>{f|b}_{crypto|digest} from the key material in key_data.
- *
- * If <b>is_hs_v3</b> is set, this cpath will be used for next gen hidden
- * service circuits and <b>key_data</b> must be at least
- * HS_NTOR_KEY_EXPANSION_KDF_OUT_LEN bytes in length.
- *
- * If <b>is_hs_v3</b> is not set, key_data must contain CPATH_KEY_MATERIAL_LEN
- * bytes, which are used as follows:
- *   - 20 to initialize f_digest
- *   - 20 to initialize b_digest
- *   - 16 to key f_crypto
- *   - 16 to key b_crypto
- *
- * (If 'reverse' is true, then f_XX and b_XX are swapped.)
- *
- * Return 0 if init was successful, else -1 if it failed.
- */
-int
-circuit_init_cpath_crypto(crypt_path_t *cpath,
-                          const char *key_data, size_t key_data_len,
-                          int reverse, int is_hs_v3)
-{
-
-  tor_assert(cpath);
-  return relay_crypto_init(&cpath->crypto, key_data, key_data_len, reverse,
-                           is_hs_v3);
-}
-
 /** A "created" cell <b>reply</b> came back to us on circuit <b>circ</b>.
  * (The body of <b>reply</b> varies depending on what sort of handshake
  * this is.)
@@ -1413,7 +1384,7 @@ circuit_finish_handshake(origin_circuit_t *circ,
   if (circ->cpath->state == CPATH_STATE_AWAITING_KEYS) {
     hop = circ->cpath;
   } else {
-    hop = onion_next_hop_in_cpath(circ->cpath);
+    hop = cpath_get_next_non_open_hop(circ->cpath);
     if (!hop) { /* got an extended when we're all done? */
       log_warn(LD_PROTOCOL,"got extended when circ already built? Closing.");
       return - END_CIRC_REASON_TORPROTOCOL;
@@ -1437,7 +1408,7 @@ circuit_finish_handshake(origin_circuit_t *circ,
 
   onion_handshake_state_release(&hop->handshake_state);
 
-  if (circuit_init_cpath_crypto(hop, keys, sizeof(keys), 0, 0)<0) {
+  if (cpath_init_circuit_crypto(hop, keys, sizeof(keys), 0, 0)<0) {
     return -END_CIRC_REASON_TORPROTOCOL;
   }
 
@@ -1489,7 +1460,7 @@ circuit_truncated(origin_circuit_t *circ, int reason)
     }
 
     layer->next = victim->next;
-    circuit_free_cpath_node(victim);
+    cpath_free(victim);
   }
 
   log_info(LD_CIRC, "finished");
@@ -2308,7 +2279,7 @@ circuit_append_new_exit(origin_circuit_t *circ, extend_info_t *exit_ei)
   state->chosen_exit = extend_info_dup(exit_ei);
 
   ++circ->build_state->desired_path_len;
-  onion_append_hop(&circ->cpath, exit_ei);
+  cpath_append_hop(&circ->cpath, exit_ei);
   return 0;
 }
 
@@ -2373,47 +2344,6 @@ count_acceptable_nodes, (const smartlist_t *nodes, int direct))
   return num;
 }
 
-/** Add <b>new_hop</b> to the end of the doubly-linked-list <b>head_ptr</b>.
- * This function is used to extend cpath by another hop.
- */
-void
-onion_append_to_cpath(crypt_path_t **head_ptr, crypt_path_t *new_hop)
-{
-  if (*head_ptr) {
-    new_hop->next = (*head_ptr);
-    new_hop->prev = (*head_ptr)->prev;
-    (*head_ptr)->prev->next = new_hop;
-    (*head_ptr)->prev = new_hop;
-  } else {
-    *head_ptr = new_hop;
-    new_hop->prev = new_hop->next = new_hop;
-  }
-}
-
-#ifdef TOR_UNIT_TESTS
-
-/** Unittest helper function: Count number of hops in cpath linked list. */
-unsigned int
-cpath_get_n_hops(crypt_path_t **head_ptr)
-{
-  unsigned int n_hops = 0;
-  crypt_path_t *tmp;
-
-  if (!*head_ptr) {
-    return 0;
-  }
-
-  tmp = *head_ptr;
-  do {
-    n_hops++;
-    tmp = tmp->next;
-  } while (tmp != *head_ptr);
-
-  return n_hops;
-}
-
-#endif /* defined(TOR_UNIT_TESTS) */
-
 /**
  * Build the exclude list for vanguard circuits.
  *
@@ -2688,20 +2618,6 @@ choose_good_entry_server(uint8_t purpose, cpath_build_state_t *state,
   return choice;
 }
 
-/** Return the first non-open hop in cpath, or return NULL if all
- * hops are open. */
-static crypt_path_t *
-onion_next_hop_in_cpath(crypt_path_t *cpath)
-{
-  crypt_path_t *hop = cpath;
-  do {
-    if (hop->state != CPATH_STATE_OPEN)
-      return hop;
-    hop = hop->next;
-  } while (hop != cpath);
-  return NULL;
-}
-
 /** Choose a suitable next hop for the circuit <b>circ</b>.
  * Append the hop info to circ->cpath.
  *
@@ -2758,33 +2674,11 @@ onion_extend_cpath(origin_circuit_t *circ)
             extend_info_describe(info),
             cur_len+1, build_state_get_exit_nickname(state));
 
-  onion_append_hop(&circ->cpath, info);
+  cpath_append_hop(&circ->cpath, info);
   extend_info_free(info);
   return 0;
 }
 
-/** Create a new hop, annotate it with information about its
- * corresponding router <b>choice</b>, and append it to the
- * end of the cpath <b>head_ptr</b>. */
-STATIC int
-onion_append_hop(crypt_path_t **head_ptr, extend_info_t *choice)
-{
-  crypt_path_t *hop = tor_malloc_zero(sizeof(crypt_path_t));
-
-  /* link hop into the cpath, at the end. */
-  onion_append_to_cpath(head_ptr, hop);
-
-  hop->magic = CRYPT_PATH_MAGIC;
-  hop->state = CPATH_STATE_CLOSED;
-
-  hop->extend_info = extend_info_dup(choice);
-
-  hop->package_window = circuit_initial_package_window();
-  hop->deliver_window = CIRCWINDOW_START;
-
-  return 0;
-}
-
 /** Allocate a new extend_info object based on the various arguments. */
 extend_info_t *
 extend_info_new(const char *nickname,

src/core/or/circuitbuild.h

@@ -34,9 +34,6 @@ int circuit_timeout_want_to_count_circ(const origin_circuit_t *circ);
 int circuit_send_next_onion_skin(origin_circuit_t *circ);
 void circuit_note_clock_jumped(int64_t seconds_elapsed, bool was_idle);
 int circuit_extend(cell_t *cell, circuit_t *circ);
-int circuit_init_cpath_crypto(crypt_path_t *cpath,
-                              const char *key_data, size_t key_data_len,
-                              int reverse, int is_hs_v3);
 struct created_cell_t;
 int circuit_finish_handshake(origin_circuit_t *circ,
                              const struct created_cell_t *created_cell);
@@ -51,7 +48,6 @@ MOCK_DECL(int, circuit_all_predicted_ports_handled, (time_t now,
 
 int circuit_append_new_exit(origin_circuit_t *circ, extend_info_t *info);
 int circuit_extend_to_new_exit(origin_circuit_t *circ, extend_info_t *info);
-void onion_append_to_cpath(crypt_path_t **head_ptr, crypt_path_t *new_hop);
 extend_info_t *extend_info_new(const char *nickname,
                                const char *rsa_id_digest,
                                const struct ed25519_public_key_t *ed_id,
@@ -93,11 +89,6 @@ STATIC int
 onion_pick_cpath_exit(origin_circuit_t *circ, extend_info_t *exit_ei,
                       int is_hs_v3_rp_circuit);
 
-#if defined(TOR_UNIT_TESTS)
-unsigned int cpath_get_n_hops(crypt_path_t **head_ptr);
-
-#endif /* defined(TOR_UNIT_TESTS) */
-
 #endif /* defined(CIRCUITBUILD_PRIVATE) */
 
 #endif /* !defined(TOR_CIRCUITBUILD_H) */