Fix small easy bugs all around

- Fix log message format string.
- Do extra circuit purpose check.
- wipe memory in a clear function
- Make sure we don't double add intro points in our list
- Make sure we don't double close intro circuits.
- s/tt_u64_op/tt_i64_op/

diff --git a/src/or/hs_cell.c b/src/or/hs_cell.c
index 922ff73468148bf48eeb5e1c910a28bda1259a68..064e26334d3398f15517247baeb44a33c258eb3f 100644 (file)
--- a/src/or/hs_cell.c
+++ b/src/or/hs_cell.c
@@ -169,9 +169,9 @@ parse_introduce2_encrypted(const uint8_t *decrypted_data,
 
   if (trn_cell_introduce_encrypted_getlen_onion_key(enc_cell) !=
       CURVE25519_PUBKEY_LEN) {
-    log_info(LD_REND, "INTRODUCE2 onion key length is invalid. Got %ld but "
+    log_info(LD_REND, "INTRODUCE2 onion key length is invalid. Got %u but "
                       "expected %d on circuit %u for service %s",
-             trn_cell_introduce_encrypted_getlen_onion_key(enc_cell),
+             (unsigned)trn_cell_introduce_encrypted_getlen_onion_key(enc_cell),
              CURVE25519_PUBKEY_LEN, TO_CIRCUIT(circ)->n_circ_id,
              safe_str_client(service->onion_address));
     goto err;

diff --git a/src/or/hs_circuit.c b/src/or/hs_circuit.c
index 75c946799f22371178da277c6596c8e1ebfd5293..527439599ad68e8b492060125884c5b59a77ae1c 100644 (file)
--- a/src/or/hs_circuit.c
+++ b/src/or/hs_circuit.c
@@ -898,6 +898,10 @@ hs_circ_handle_intro_established(const hs_service_t *service,
   tor_assert(circ);
   tor_assert(payload);
 
+  if (BUG(TO_CIRCUIT(circ)->purpose != CIRCUIT_PURPOSE_S_ESTABLISH_INTRO)) {
+    goto done;
+  }
+
   /* Try to parse the payload into a cell making sure we do actually have a
    * valid cell. For a legacy node, it's an empty payload so as long as we
    * have the cell, we are good. */

diff --git a/src/or/hs_intropoint.c b/src/or/hs_intropoint.c
index a0453841f946c37c50b59318d03246269b9d0d87..644611feab1a76c8b04961eba8a8b12c7894105e 100644 (file)
--- a/src/or/hs_intropoint.c
+++ b/src/or/hs_intropoint.c
@@ -607,5 +607,6 @@ hs_intropoint_clear(hs_intropoint_t *ip)
   SMARTLIST_FOREACH(ip->link_specifiers, hs_desc_link_specifier_t *, ls,
                     hs_desc_link_specifier_free(ls));
   smartlist_free(ip->link_specifiers);
+  memset(ip, 0, sizeof(hs_intropoint_t));
 }
 

diff --git a/src/or/hs_service.c b/src/or/hs_service.c
index 592b1f2552931e165ff8fc7d0bfcd08c20c55ec2..f1592efa81be3b11eded3e89c4943184e9308e58 100644 (file)
--- a/src/or/hs_service.c
+++ b/src/or/hs_service.c
@@ -433,14 +433,18 @@ service_intro_point_new(const extend_info_t *ei, unsigned int is_legacy)
 STATIC void
 service_intro_point_add(digest256map_t *map, hs_service_intro_point_t *ip)
 {
+  hs_service_intro_point_t *old_ip_entry;
+
   tor_assert(map);
   tor_assert(ip);
 
-  digest256map_set(map, ip->auth_key_kp.pubkey.pubkey, ip);
+  old_ip_entry = digest256map_set(map, ip->auth_key_kp.pubkey.pubkey, ip);
+  /* Make sure we didn't just try to double-add an intro point */
+  tor_assert_nonfatal(!old_ip_entry);
 }
 
-/* For a given service, remove the intro point from that service which will
- * look in both descriptors. */
+/* For a given service, remove the intro point from that service's descriptors
+ * (check both current and next descriptor) */
 STATIC void
 service_intro_point_remove(const hs_service_t *service,
                            const hs_service_intro_point_t *ip)
@@ -1623,7 +1627,7 @@ cleanup_intro_points(hs_service_t *service, time_t now)
          * descriptor created and uploaded. There is no difference to an
          * attacker between the timing of a new consensus and intro point
          * rotation (possibly?). */
-        if (ocirc) {
+        if (ocirc && !TO_CIRCUIT(ocirc)->marked_for_close) {
           /* After this, no new cells will be handled on the circuit. */
           circuit_mark_for_close(TO_CIRCUIT(ocirc), END_CIRC_REASON_FINISHED);
         }

diff --git a/src/test/test_hs_cell.c b/src/test/test_hs_cell.c
index 9c963bcb16090cf685c3b6373e3559fca3ce0339..1686fccee76b76fca3737df2fb6b743dbea28ecb 100644 (file)
--- a/src/test/test_hs_cell.c
+++ b/src/test/test_hs_cell.c
@@ -106,7 +106,7 @@ test_gen_establish_intro_cell_bad(void *arg)
   expect_log_msg_containing("Unable to make signature for "
                             "ESTABLISH_INTRO cell.");
   teardown_capture_of_logs();
-  tt_u64_op(cell_len, OP_EQ, -1);
+  tt_i64_op(cell_len, OP_EQ, -1);
 
  done:
   trn_cell_establish_intro_free(cell);