[tor-commits] [tor/master] Fix small easy bugs all around

[nickm at torproject.org]

commit a561a10da726b426d326515ca7f75988b405bab7
Author: George Kadianakis <desnacked at riseup.net>
Date:   Thu Aug 3 16:04:25 2017 +0300

    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/
---
 src/or/hs_cell.c        |  4 ++--
 src/or/hs_circuit.c     |  4 ++++
 src/or/hs_intropoint.c  |  1 +
 src/or/hs_service.c     | 12 ++++++++----
 src/test/test_hs_cell.c |  2 +-
 5 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/src/or/hs_cell.c b/src/or/hs_cell.c
index 922ff7346..064e26334 100644
--- 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 75c946799..527439599 100644
--- 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 a0453841f..644611fea 100644
--- 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 592b1f255..f1592efa8 100644
--- 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 9c963bcb1..1686fccee 100644
--- 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);