prop224: Refactor rendclient.c to use the new hsdir_req code.

- Also add tests for the hidserv_req subsystem.
- Introduce purge_v2_hidserv_req() wrapper to simplify v2 code.

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

src/or/rendclient.c

@@ -562,6 +562,20 @@ directory_get_from_hs_dir(const char *desc_id,
   return 1;
 }
 
+/** Remove tracked HSDir requests from our history for this hidden service
+ *  descriptor <b>desc_id</b> (of size DIGEST_LEN) */
+static void
+purge_v2_hidserv_req(const char *desc_id)
+{
+  char desc_id_base32[REND_DESC_ID_V2_LEN_BASE32 + 1];
+
+  /* The hsdir request tracker stores v2 keys using the base32 encoded
+     desc_id. Do it: */
+  base32_encode(desc_id_base32, sizeof(desc_id_base32), desc_id,
+                DIGEST_LEN);
+  hs_purge_hid_serv_from_last_hid_serv_requests(desc_id_base32);
+}
+
 /** Fetch a v2 descriptor using the given descriptor id. If any hsdir(s) are
  * given, they will be used instead.
  *
@@ -636,8 +650,7 @@ fetch_v2_desc_by_addr(rend_data_t *rend_query, smartlist_t *hsdirs)
                    sizeof(descriptor_id)) != 0) {
       /* Not equal from what we currently have so purge the last hid serv
        * request cache and update the descriptor ID with the new value. */
-      hs_purge_hid_serv_from_last_hid_serv_requests(
-                                     rend_data->descriptor_id[chosen_replica]);
+      purge_v2_hidserv_req(rend_data->descriptor_id[chosen_replica]);
       memcpy(rend_data->descriptor_id[chosen_replica], descriptor_id,
              sizeof(rend_data->descriptor_id[chosen_replica]));
     }
@@ -1036,14 +1049,14 @@ rend_client_note_connection_attempt_ended(const rend_data_t *rend_data)
     for (replica = 0; replica < ARRAY_LENGTH(rend_data_v2->descriptor_id);
          replica++) {
       const char *desc_id = rend_data_v2->descriptor_id[replica];
-      hs_purge_hid_serv_from_last_hid_serv_requests(desc_id);
+      purge_v2_hidserv_req(desc_id);
     }
     log_info(LD_REND, "Connection attempt for %s has ended; "
              "cleaning up temporary state.",
              safe_str_client(onion_address));
   } else {
     /* We only have an ID for a fetch. Probably used by HSFETCH. */
-    hs_purge_hid_serv_from_last_hid_serv_requests(rend_data_v2->desc_id_fetch);
+    purge_v2_hidserv_req(rend_data_v2->desc_id_fetch);
   }
 }
 

src/test/test_hs_common.c

@@ -697,6 +697,104 @@ test_disaster_srv(void *arg)
   ;
 }
 
+/** Test our HS descriptor request tracker by making various requests and
+ *  checking whether they get tracked properly. */
+static void
+test_hid_serv_request_tracker(void *arg)
+{
+  (void) arg;
+  time_t retval;
+  routerstatus_t *hsdir = NULL, *hsdir2 = NULL;
+  time_t now = approx_time();
+
+  const char *req_key_str_first =
+ "vd4zb6zesaubtrjvdqcr2w7x7lhw2up4Xnw4526ThUNbL5o1go+EdUuEqlKxHkNbnK41pRzizzs";
+  const char *req_key_str_second =
+ "g53o7iavcd62oihswhr24u6czmqws5kpXnw4526ThUNbL5o1go+EdUuEqlKxHkNbnK41pRzizzs";
+
+  /*************************** basic test *******************************/
+
+  /* Get request tracker and make sure it's empty */
+  strmap_t *request_tracker = get_last_hid_serv_requests();
+  tt_int_op(strmap_size(request_tracker),OP_EQ, 0);
+
+  /* Let's register a hid serv request */
+  hsdir = tor_malloc_zero(sizeof(routerstatus_t));
+  memset(hsdir->identity_digest, 'Z', DIGEST_LEN);
+  retval = hs_lookup_last_hid_serv_request(hsdir, req_key_str_first,
+                                           now, 1);
+  tt_int_op(retval, OP_EQ, now);
+  tt_int_op(strmap_size(request_tracker),OP_EQ, 1);
+
+  /* Let's lookup a non-existent hidserv request */
+  retval = hs_lookup_last_hid_serv_request(hsdir, req_key_str_second,
+                                           now+1, 0);
+  tt_int_op(retval, OP_EQ, 0);
+  tt_int_op(strmap_size(request_tracker),OP_EQ, 1);
+
+  /* Let's lookup a real hidserv request */
+  retval = hs_lookup_last_hid_serv_request(hsdir, req_key_str_first,
+                                           now+2, 0);
+  tt_int_op(retval, OP_EQ, now); /* we got it */
+  tt_int_op(strmap_size(request_tracker),OP_EQ, 1);
+
+  /**********************************************************************/
+
+  /* Let's add another request for the same HS but on a different HSDir. */
+  hsdir2 = tor_malloc_zero(sizeof(routerstatus_t));
+  memset(hsdir->identity_digest, 2, DIGEST_LEN);
+  retval = hs_lookup_last_hid_serv_request(hsdir2, req_key_str_first,
+                                           now+3, 1);
+  tt_int_op(retval, OP_EQ, now+3);
+  tt_int_op(strmap_size(request_tracker),OP_EQ, 2);
+
+  /* Check that we can clean the first request based on time */
+  hs_clean_last_hid_serv_requests(now+3+REND_HID_SERV_DIR_REQUERY_PERIOD);
+  tt_int_op(strmap_size(request_tracker),OP_EQ, 1);
+  /* Check that it doesn't exist anymore */
+  retval = hs_lookup_last_hid_serv_request(hsdir, req_key_str_first,
+                                           now+2, 0);
+  tt_int_op(retval, OP_EQ, 0);
+
+  /*************************** deleting entries **************************/
+
+  /* Add another request with very short key */
+  retval = hs_lookup_last_hid_serv_request(hsdir, "l",  now, 1);
+
+  /* Try deleting entries with a dummy key. Check that our previous requests
+   * are still there */
+  tor_capture_bugs_(1);
+  hs_purge_hid_serv_from_last_hid_serv_requests("a");
+  tt_int_op(strmap_size(request_tracker),OP_EQ, 2);
+  tor_end_capture_bugs_();
+
+  /* Try another dummy key. Check that requests are still there */
+  {
+    char dummy[2000];
+    memset(dummy, 'Z', 2000);
+    dummy[1999] = '\x00';
+    hs_purge_hid_serv_from_last_hid_serv_requests(dummy);
+    tt_int_op(strmap_size(request_tracker),OP_EQ, 2);
+  }
+
+  /* Another dummy key! */
+  hs_purge_hid_serv_from_last_hid_serv_requests(req_key_str_second);
+  tt_int_op(strmap_size(request_tracker),OP_EQ, 2);
+
+  /* Now actually delete a request! */
+  hs_purge_hid_serv_from_last_hid_serv_requests(req_key_str_first);
+  tt_int_op(strmap_size(request_tracker),OP_EQ, 1);
+
+  /* Purge it all! */
+  hs_purge_last_hid_serv_requests();
+  request_tracker = get_last_hid_serv_requests();
+  tt_int_op(strmap_size(request_tracker),OP_EQ, 0);
+
+ done:
+  tor_free(hsdir);
+  tor_free(hsdir2);
+}
+
 static void
 test_parse_extended_hostname(void *arg)
 {
@@ -748,6 +846,8 @@ struct testcase_t hs_common_tests[] = {
     NULL, NULL },
   { "disaster_srv", test_disaster_srv, TT_FORK,
     NULL, NULL },
+  { "hid_serv_request_tracker", test_hid_serv_request_tracker, TT_FORK,
+    NULL, NULL },
   { "parse_extended_hostname", test_parse_extended_hostname, TT_FORK,
     NULL, NULL },