Merge branch 'tor-gitlab/mr/182' into master

changes/bug23126

@@ -0,0 +1,4 @@
+  o Major features (statistics):
+    - Relays will now also publish statistics about the number of v3 onion
+      services and volume of v3 onion service traffic, in the same manner they
+      already do for v2 onions.  Closes ticket 23126.

src/app/main/main.c

@@ -1080,6 +1080,7 @@ sandbox_init_filter(void)
     OPEN_DATADIR2_SUFFIX("stats", "buffer-stats", ".tmp");
     OPEN_DATADIR2_SUFFIX("stats", "conn-stats", ".tmp");
     OPEN_DATADIR2_SUFFIX("stats", "hidserv-stats", ".tmp");
+    OPEN_DATADIR2_SUFFIX("stats", "hidserv-v3-stats", ".tmp");
 
     OPEN_DATADIR("approved-routers");
     OPEN_DATADIR_SUFFIX("fingerprint", ".tmp");
@@ -1105,6 +1106,7 @@ sandbox_init_filter(void)
     RENAME_SUFFIX2("stats", "buffer-stats", ".tmp");
     RENAME_SUFFIX2("stats", "conn-stats", ".tmp");
     RENAME_SUFFIX2("stats", "hidserv-stats", ".tmp");
+    RENAME_SUFFIX2("stats", "hidserv-v3-stats", ".tmp");
     RENAME_SUFFIX("hashed-fingerprint", ".tmp");
     RENAME_SUFFIX("router-stability", ".tmp");
 

src/core/mainloop/mainloop.c

@@ -1937,7 +1937,11 @@ write_stats_file_callback(time_t now, const or_options_t *options)
       next_time_to_write_stats_files = next_write;
   }
   if (options->HiddenServiceStatistics) {
-    time_t next_write = rep_hist_hs_stats_write(now);
+    time_t next_write = rep_hist_hs_stats_write(now, false);
+    if (next_write && next_write < next_time_to_write_stats_files)
+      next_time_to_write_stats_files = next_write;
+
+    next_write = rep_hist_hs_stats_write(now, true);
     if (next_write && next_write < next_time_to_write_stats_files)
       next_time_to_write_stats_files = next_write;
   }

src/core/or/command.c

@@ -331,6 +331,13 @@ command_process_create_cell(cell_t *cell, channel_t *chan)
     return;
   }
 
+  /* Mark whether this circuit used TAP in case we need to use this
+   * information for onion service statistics later on. */
+  if (create_cell->handshake_type == ONION_HANDSHAKE_TYPE_FAST ||
+      create_cell->handshake_type == ONION_HANDSHAKE_TYPE_TAP) {
+    circ->used_legacy_circuit_handshake = true;
+  }
+
   if (!channel_is_client(chan)) {
     /* remember create types we've seen, but don't remember them from
      * clients, to be extra conservative about client statistics. */
@@ -587,11 +594,27 @@ command_process_relay_cell(cell_t *cell, channel_t *chan)
   }
 
   /* If this is a cell in an RP circuit, count it as part of the
-     hidden service stats */
+     onion service stats */
   if (options->HiddenServiceStatistics &&
       !CIRCUIT_IS_ORIGIN(circ) &&
-      TO_OR_CIRCUIT(circ)->circuit_carries_hs_traffic_stats) {
-    rep_hist_seen_new_rp_cell();
+      CONST_TO_OR_CIRCUIT(circ)->circuit_carries_hs_traffic_stats) {
+    /** We need to figure out of this is a v2 or v3 RP circuit to count it
+     *  appropriately. v2 services always use the TAP legacy handshake to
+     *  connect to the RP; we use this feature to distinguish between v2/v3. */
+    bool is_v2 = false;
+    if (CONST_TO_OR_CIRCUIT(circ)->used_legacy_circuit_handshake) {
+      is_v2 = true;
+    } else if (CONST_TO_OR_CIRCUIT(circ)->rend_splice) {
+      /* If this is a client->RP circuit we need to check the spliced circuit
+       * (which is the service->RP circuit) to see if it was using TAP and
+       * hence if it's a v2 circuit. That's because client->RP circuits can
+       * still use ntor even on v2; but service->RP will always use TAP. */
+      or_circuit_t *splice = CONST_TO_OR_CIRCUIT(circ)->rend_splice;
+      if (splice->used_legacy_circuit_handshake) {
+        is_v2 = true;
+      }
+    }
+    rep_hist_seen_new_rp_cell(is_v2);
   }
 }
 

src/core/or/or_circuit_st.h

@@ -63,6 +63,12 @@ struct or_circuit_t {
    *  statistics. */
   unsigned int circuit_carries_hs_traffic_stats : 1;
 
+  /** True iff this circuit was made with a CREATE_FAST cell, or a CREATE[2]
+   * cell with a TAP handshake. If this is the case and this is a rend circuit,
+   * this is a v2 circuit, otherwise if this is a rend circuit it's a v3
+   * circuit. */
+  bool used_legacy_circuit_handshake;
+
   /** Number of cells that were removed from circuit queue; reset every
    * time when writing buffer stats to disk. */
   uint32_t processed_cells;

src/feature/hs/hs_cache.c

@@ -19,6 +19,7 @@
 #include "feature/hs/hs_descriptor.h"
 #include "feature/nodelist/networkstatus.h"
 #include "feature/rend/rendcache.h"
+#include "feature/stats/rephist.h"
 
 #include "feature/hs/hs_cache.h"
 
@@ -174,7 +175,10 @@ cache_store_v3_as_dir(hs_cache_dir_descriptor_t *desc)
    * old HS protocol cache subsystem for which we are tied with. */
   rend_cache_increment_allocation(cache_get_dir_entry_size(desc));
 
-  /* XXX: Update HS statistics. We should have specific stats for v3. */
+  /* Update HSv3 statistics */
+  if (get_options()->HiddenServiceStatistics) {
+    rep_hist_hsdir_stored_maybe_new_v3_onion(desc->key);
+  }
 
   return 0;
 

src/feature/hs_common/shared_random_client.c

@@ -33,12 +33,11 @@ srv_to_control_string(const sr_srv_t *srv)
 }
 
 /**
- * If we have no consensus and we are not an authority, assume that this is
- * the voting interval.  We should never actually use this: only authorities
- * should be trying to figure out the schedule when they don't have a
- * consensus.
- **/
+ * If we have no consensus and we are not an authority, assume that this is the
+ * voting interval. This can be used while bootstrapping as a relay and we are
+ * asked to initialize HS stats (see rep_hist_hs_stats_init()) */
 #define DEFAULT_NETWORK_VOTING_INTERVAL (3600)
+#define TESTING_DEFAULT_NETWORK_VOTING_INTERVAL (20)
 
 /* This is an unpleasing workaround for tests.  Our unit tests assume that we
  * are scheduling all of our shared random stuff as if we were a directory
@@ -69,11 +68,13 @@ get_voting_interval(void)
      * It's better than falling back to the non-consensus case. */
     interval = (int)(consensus->fresh_until - consensus->valid_after);
   } else {
-    /* We should never be reaching this point, since a client should never
-     * call this code unless they have some kind of a consensus. All we can
-     * do is hope that this network is using the default voting interval. */
-    tor_assert_nonfatal_unreached_once();
-    interval = DEFAULT_NETWORK_VOTING_INTERVAL;
+    /* We can reach this as a relay when bootstrapping and we are asked to
+     * initialize HS stats (see rep_hist_hs_stats_init()). */
+    if (get_options()->TestingTorNetwork) {
+      interval = TESTING_DEFAULT_NETWORK_VOTING_INTERVAL;
+    } else {
+      interval = DEFAULT_NETWORK_VOTING_INTERVAL;
+    }
   }
   tor_assert(interval > 0);
   return interval;

src/feature/relay/router.c

@@ -3306,6 +3306,11 @@ extrainfo_dump_to_string_stats_helper(smartlist_t *chunks,
                         "hidserv-stats-end", now, &contents) > 0) {
       smartlist_add(chunks, contents);
     }
+    if (options->HiddenServiceStatistics &&
+        load_stats_file("stats"PATH_SEPARATOR"hidserv-v3-stats",
+                        "hidserv-v3-stats-end", now, &contents) > 0) {
+      smartlist_add(chunks, contents);
+    }
     if (options->EntryStatistics &&
         load_stats_file("stats"PATH_SEPARATOR"entry-stats",
                         "entry-stats-end", now, &contents) > 0) {

src/feature/rend/rendcache.c

@@ -718,7 +718,7 @@ rend_cache_store_v2_desc_as_dir(const char *desc)
              safe_str(desc_id_base32), (int)encoded_size);
     /* Statistics: Note down this potentially new HS. */
     if (options->HiddenServiceStatistics) {
-      rep_hist_stored_maybe_new_hs(e->parsed->pk);
+      rep_hist_hsdir_stored_maybe_new_v2_onion(e->parsed->pk);
     }
 
     number_stored++;

src/feature/stats/rephist.c

@@ -1710,103 +1710,63 @@ rep_hist_log_circuit_handshake_stats(time_t now)
 
 /** Start of the current hidden service stats interval or 0 if we're
  * not collecting hidden service statistics. */
-static time_t start_of_hs_stats_interval;
+static time_t start_of_hs_v2_stats_interval;
 
-/** Carries the various hidden service statistics, and any other
- *  information needed. */
-typedef struct hs_stats_t {
-  /** How many relay cells have we seen as rendezvous points? */
-  uint64_t rp_relay_cells_seen;
+/** Our v2 statistics structure singleton. */
+static hs_v2_stats_t *hs_v2_stats = NULL;
 
-  /** Set of unique public key digests we've seen this stat period
-   * (could also be implemented as sorted smartlist). */
-  digestmap_t *onions_seen_this_period;
-} hs_stats_t;
+/** HSv2 stats */
 
-/** Our statistics structure singleton. */
-static hs_stats_t *hs_stats = NULL;
-
-/** Allocate, initialize and return an hs_stats_t structure. */
-static hs_stats_t *
-hs_stats_new(void)
+/** Allocate, initialize and return an hs_v2_stats_t structure. */
+static hs_v2_stats_t *
+hs_v2_stats_new(void)
 {
-  hs_stats_t *new_hs_stats = tor_malloc_zero(sizeof(hs_stats_t));
-  new_hs_stats->onions_seen_this_period = digestmap_new();
+  hs_v2_stats_t *new_hs_v2_stats = tor_malloc_zero(sizeof(hs_v2_stats_t));
+  new_hs_v2_stats->v2_onions_seen_this_period = digestmap_new();
 
-  return new_hs_stats;
+  return new_hs_v2_stats;
 }
 
-#define hs_stats_free(val) \
-  FREE_AND_NULL(hs_stats_t, hs_stats_free_, (val))
+#define hs_v2_stats_free(val) \
+  FREE_AND_NULL(hs_v2_stats_t, hs_v2_stats_free_, (val))
 
-/** Free an hs_stats_t structure. */
+/** Free an hs_v2_stats_t structure. */
 static void
-hs_stats_free_(hs_stats_t *victim_hs_stats)
+hs_v2_stats_free_(hs_v2_stats_t *victim_hs_v2_stats)
 {
-  if (!victim_hs_stats) {
+  if (!victim_hs_v2_stats) {
     return;
   }
 
-  digestmap_free(victim_hs_stats->onions_seen_this_period, NULL);
-  tor_free(victim_hs_stats);
-}
-
-/** Initialize hidden service statistics. */
-void
-rep_hist_hs_stats_init(time_t now)
-{
-  if (!hs_stats) {
-    hs_stats = hs_stats_new();
-  }
-
-  start_of_hs_stats_interval = now;
+  digestmap_free(victim_hs_v2_stats->v2_onions_seen_this_period, NULL);
+  tor_free(victim_hs_v2_stats);
 }
 
 /** Clear history of hidden service statistics and set the measurement
  * interval start to <b>now</b>. */
 static void
-rep_hist_reset_hs_stats(time_t now)
+rep_hist_reset_hs_v2_stats(time_t now)
 {
-  if (!hs_stats) {
-    hs_stats = hs_stats_new();
+  if (!hs_v2_stats) {
+    hs_v2_stats = hs_v2_stats_new();
   }
 
-  hs_stats->rp_relay_cells_seen = 0;
+  hs_v2_stats->rp_v2_relay_cells_seen = 0;
 
-  digestmap_free(hs_stats->onions_seen_this_period, NULL);
-  hs_stats->onions_seen_this_period = digestmap_new();
+  digestmap_free(hs_v2_stats->v2_onions_seen_this_period, NULL);
+  hs_v2_stats->v2_onions_seen_this_period = digestmap_new();
 
-  start_of_hs_stats_interval = now;
+  start_of_hs_v2_stats_interval = now;
 }
 
-/** Stop collecting hidden service stats in a way that we can re-start
- * doing so in rep_hist_buffer_stats_init(). */
+/** As HSDirs, we saw another v2 onion with public key <b>pubkey</b>. Check
+ *  whether we have counted it before, if not count it now! */
 void
-rep_hist_hs_stats_term(void)
-{
-  rep_hist_reset_hs_stats(0);
-}
-
-/** We saw a new HS relay cell, Count it! */
-void
-rep_hist_seen_new_rp_cell(void)
-{
-  if (!hs_stats) {
-    return; // We're not collecting stats
-  }
-
-  hs_stats->rp_relay_cells_seen++;
-}
-
-/** As HSDirs, we saw another hidden service with public key
- *  <b>pubkey</b>. Check whether we have counted it before, if not
- *  count it now! */
-void
-rep_hist_stored_maybe_new_hs(const crypto_pk_t *pubkey)
+rep_hist_hsdir_stored_maybe_new_v2_onion(const crypto_pk_t *pubkey)
 {
   char pubkey_hash[DIGEST_LEN];
 
-  if (!hs_stats) {
+  if (!hs_v2_stats) {
     return; // We're not collecting stats
   }
 
@@ -1820,13 +1780,178 @@ rep_hist_stored_maybe_new_hs(const crypto_pk_t *pubkey)
 
   /* Check if this is the first time we've seen this hidden
      service. If it is, count it as new. */
-  if (!digestmap_get(hs_stats->onions_seen_this_period,
+  if (!digestmap_get(hs_v2_stats->v2_onions_seen_this_period,
                      pubkey_hash)) {
-    digestmap_set(hs_stats->onions_seen_this_period,
+    digestmap_set(hs_v2_stats->v2_onions_seen_this_period,
                   pubkey_hash, (void*)(uintptr_t)1);
   }
 }
 
+/*** HSv3 stats ******/
+
+/** Start of the current hidden service stats interval or 0 if we're not
+ *  collecting hidden service statistics.
+ *
+ *  This is particularly important for v3 statistics since this variable
+ *  controls the start time of initial v3 stats collection. It's initialized by
+ *  rep_hist_hs_stats_init() to the next time period start (i.e. 12:00UTC), and
+ *  should_collect_v3_stats() ensures that functions that collect v3 stats do
+ *  not do so sooner than that.
+ *
+ *  Collecting stats from 12:00UTC to 12:00UTC is extremely important for v3
+ *  stats because rep_hist_hsdir_stored_maybe_new_v3_onion() uses the blinded
+ *  key of each onion service as its double-counting index. Onion services
+ *  rotate their descriptor at around 00:00UTC which means that their blinded
+ *  key also changes around that time. However the precise time that onion
+ *  services rotate their descriptors is actually when they fetch a new
+ *  00:00UTC consensus and that happens at a random time (e.g. it can even
+ *  happen at 02:00UTC). This means that if we started keeping v3 stats at
+ *  around 00:00UTC we wouldn't be able to tell when onion services change
+ *  their blinded key and hence we would double count an unpredictable amount
+ *  of them (for example, if an onion service fetches the 00:00UTC consensus at
+ *  01:00UTC it would upload to its old HSDir at 00:45UTC, and then to a
+ *  different HSDir at 01:50UTC).
+ *
+ *  For this reason, we start collecting statistics at 12:00UTC. This way we
+ *  know that by the time we stop collecting statistics for that time period 24
+ *  hours later, all the onion services have switched to their new blinded
+ *  key. This way we can predict much better how much double counting has been
+ *  performed.
+ */
+static time_t start_of_hs_v3_stats_interval;
+
+/** Our v3 statistics structure singleton. */
+static hs_v3_stats_t *hs_v3_stats = NULL;
+
+/** Allocate, initialize and return an hs_v3_stats_t structure. */
+static hs_v3_stats_t *
+hs_v3_stats_new(void)
+{
+  hs_v3_stats_t *new_hs_v3_stats = tor_malloc_zero(sizeof(hs_v3_stats_t));
+  new_hs_v3_stats->v3_onions_seen_this_period = digest256map_new();
+
+  return new_hs_v3_stats;
+}
+
+#define hs_v3_stats_free(val) \
+  FREE_AND_NULL(hs_v3_stats_t, hs_v3_stats_free_, (val))
+
+/** Free an hs_v3_stats_t structure. */
+static void
+hs_v3_stats_free_(hs_v3_stats_t *victim_hs_v3_stats)
+{
+  if (!victim_hs_v3_stats) {
+    return;
+  }
+
+  digest256map_free(victim_hs_v3_stats->v3_onions_seen_this_period, NULL);
+  tor_free(victim_hs_v3_stats);
+}
+
+/** Clear history of hidden service statistics and set the measurement
+ * interval start to <b>now</b>. */
+static void
+rep_hist_reset_hs_v3_stats(time_t now)
+{
+  if (!hs_v3_stats) {
+    hs_v3_stats = hs_v3_stats_new();
+  }
+
+  digest256map_free(hs_v3_stats->v3_onions_seen_this_period, NULL);
+  hs_v3_stats->v3_onions_seen_this_period = digest256map_new();
+
+  hs_v3_stats->rp_v3_relay_cells_seen = 0;
+
+  start_of_hs_v3_stats_interval = now;
+}
+
+/** Return true if it's a good time to collect v3 stats.
+ *
+ *  v3 stats have a strict stats collection period (from 12:00UTC to 12:00UTC
+ *  on the real network). We don't want to collect statistics if (for example)
+ *  we just booted and it's 03:00UTC; we will wait until 12:00UTC before we
+ *  start collecting statistics to make sure that the final result represents
+ *  the whole collection period. This behavior is controlled by
+ *  rep_hist_hs_stats_init().
+ */
+MOCK_IMPL(STATIC bool,
+should_collect_v3_stats,(void))
+{
+  return start_of_hs_v3_stats_interval <= approx_time();
+}
+
+/** We just received a new descriptor with <b>blinded_key</b>. See if we've
+ * seen this blinded key before, and if not add it to the stats.  */
+void
+rep_hist_hsdir_stored_maybe_new_v3_onion(const uint8_t *blinded_key)
+{
+  /* Return early if we don't collect HSv3 stats, or if it's not yet the time
+   * to collect them. */
+  if (!hs_v3_stats || !should_collect_v3_stats()) {
+    return;
+  }
+
+  bool seen_before =
+    !!digest256map_get(hs_v3_stats->v3_onions_seen_this_period,
+                       blinded_key);
+
+  log_info(LD_GENERAL, "Considering v3 descriptor with %s (%sseen before)",
+           safe_str(hex_str((char*)blinded_key, 32)),
+           seen_before ? "" : "not ");
+
+  /* Count it if we haven't seen it before. */
+  if (!seen_before) {
+    digest256map_set(hs_v3_stats->v3_onions_seen_this_period,
+                  blinded_key, (void*)(uintptr_t)1);
+  }
+}
+
+/** We saw a new HS relay cell: count it!
+ *  If <b>is_v2</b> is set then it's a v2 RP cell, otherwise it's a v3. */
+void
+rep_hist_seen_new_rp_cell(bool is_v2)
+{
+  log_debug(LD_GENERAL, "New RP cell (%d)", is_v2);
+
+  if (is_v2 && hs_v2_stats) {
+    hs_v2_stats->rp_v2_relay_cells_seen++;
+  } else if (!is_v2 && hs_v3_stats && should_collect_v3_stats()) {
+    hs_v3_stats->rp_v3_relay_cells_seen++;
+  }
+}
+
+/** Generic HS stats code */
+
+/** Initialize v2 and v3 hidden service statistics. */
+void
+rep_hist_hs_stats_init(time_t now)
+{
+  if (!hs_v2_stats) {
+    hs_v2_stats = hs_v2_stats_new();
+  }
+
+  /* Start collecting v2 stats straight away */
+  start_of_hs_v2_stats_interval = now;
+
+  if (!hs_v3_stats) {
+    hs_v3_stats = hs_v3_stats_new();
+  }
+
+  /* Start collecting v3 stats at the next 12:00 UTC */
+  start_of_hs_v3_stats_interval = hs_get_start_time_of_next_time_period(now);
+}
+
+/** Stop collecting hidden service stats in a way that we can re-start
+ * doing so in rep_hist_buffer_stats_init(). */
+void
+rep_hist_hs_stats_term(void)
+{
+  rep_hist_reset_hs_v2_stats(0);
+  rep_hist_reset_hs_v3_stats(0);
+}
+
+/** Stats reporting code */
+
 /* The number of cells that are supposed to be hidden from the adversary
  * by adding noise from the Laplace distribution.  This value, divided by
  * EPSILON, is Laplace parameter b. It must be greater than 0. */
@@ -1851,58 +1976,69 @@ rep_hist_stored_maybe_new_hs(const crypto_pk_t *pubkey)
 #define ONIONS_SEEN_BIN_SIZE 8
 
 /** Allocate and return a string containing hidden service stats that
- *  are meant to be placed in the extra-info descriptor. */
-static char *
-rep_hist_format_hs_stats(time_t now)
+ *  are meant to be placed in the extra-info descriptor.
+ *
+ *  Function works for both v2 and v3 stats depending on <b>is_v3</b>. */
+STATIC char *
+rep_hist_format_hs_stats(time_t now, bool is_v3)
 {
   char t[ISO_TIME_LEN+1];
   char *hs_stats_string;
-  int64_t obfuscated_cells_seen;
-  int64_t obfuscated_onions_seen;
+  int64_t obfuscated_onions_seen, obfuscated_cells_seen;
+
+  uint64_t rp_cells_seen = is_v3 ?
+    hs_v3_stats->rp_v3_relay_cells_seen : hs_v2_stats->rp_v2_relay_cells_seen;
+  size_t onions_seen = is_v3 ?
+    digest256map_size(hs_v3_stats->v3_onions_seen_this_period) :
+    digestmap_size(hs_v2_stats->v2_onions_seen_this_period);
+  time_t start_of_hs_stats_interval = is_v3 ?
+    start_of_hs_v3_stats_interval : start_of_hs_v2_stats_interval;
 
   uint64_t rounded_cells_seen
-    = round_uint64_to_next_multiple_of(hs_stats->rp_relay_cells_seen,
-                                       REND_CELLS_BIN_SIZE);
+    = round_uint64_to_next_multiple_of(rp_cells_seen, REND_CELLS_BIN_SIZE);
   rounded_cells_seen = MIN(rounded_cells_seen, INT64_MAX);
   obfuscated_cells_seen = add_laplace_noise((int64_t)rounded_cells_seen,
                           crypto_rand_double(),
                           REND_CELLS_DELTA_F, REND_CELLS_EPSILON);
 
   uint64_t rounded_onions_seen =
-    round_uint64_to_next_multiple_of((size_t)digestmap_size(
-                                        hs_stats->onions_seen_this_period),
-                                     ONIONS_SEEN_BIN_SIZE);
+    round_uint64_to_next_multiple_of(onions_seen, ONIONS_SEEN_BIN_SIZE);
   rounded_onions_seen = MIN(rounded_onions_seen, INT64_MAX);
   obfuscated_onions_seen = add_laplace_noise((int64_t)rounded_onions_seen,
                            crypto_rand_double(), ONIONS_SEEN_DELTA_F,
                            ONIONS_SEEN_EPSILON);
 
   format_iso_time(t, now);
-  tor_asprintf(&hs_stats_string, "hidserv-stats-end %s (%d s)\n"
-               "hidserv-rend-relayed-cells %"PRId64" delta_f=%d "
-                                           "epsilon=%.2f bin_size=%d\n"
-               "hidserv-dir-onions-seen %"PRId64" delta_f=%d "
-                                        "epsilon=%.2f bin_size=%d\n",
-               t, (unsigned) (now - start_of_hs_stats_interval),
-               (obfuscated_cells_seen), REND_CELLS_DELTA_F,
+  tor_asprintf(&hs_stats_string, "%s %s (%ld s)\n"
+               "%s %"PRId64" delta_f=%d epsilon=%.2f bin_size=%d\n"
+               "%s %"PRId64" delta_f=%d epsilon=%.2f bin_size=%d\n",
+               is_v3 ? "hidserv-v3-stats-end" : "hidserv-stats-end",
+               t, now - start_of_hs_stats_interval,
+               is_v3 ?
+                "hidserv-rend-v3-relayed-cells" : "hidserv-rend-relayed-cells",
+               obfuscated_cells_seen, REND_CELLS_DELTA_F,
                REND_CELLS_EPSILON, REND_CELLS_BIN_SIZE,
-               (obfuscated_onions_seen),
-               ONIONS_SEEN_DELTA_F,
+               is_v3 ? "hidserv-dir-v3-onions-seen" :"hidserv-dir-onions-seen",
+               obfuscated_onions_seen, ONIONS_SEEN_DELTA_F,
                ONIONS_SEEN_EPSILON, ONIONS_SEEN_BIN_SIZE);
 
   return hs_stats_string;
 }
 
 /** If 24 hours have passed since the beginning of the current HS
- * stats period, write buffer stats to $DATADIR/stats/hidserv-stats
+ * stats period, write buffer stats to $DATADIR/stats/hidserv-v3-stats
  * (possibly overwriting an existing file) and reset counters.  Return
  * when we would next want to write buffer stats or 0 if we never want to
- * write. */
+ * write. Function works for both v2 and v3 stats depending on <b>is_v3</b>.
+ */
 time_t
-rep_hist_hs_stats_write(time_t now)
+rep_hist_hs_stats_write(time_t now, bool is_v3)
 {
   char *str = NULL;
 
+  time_t start_of_hs_stats_interval = is_v3 ?
+    start_of_hs_v3_stats_interval : start_of_hs_v2_stats_interval;
+
   if (!start_of_hs_stats_interval) {
     return 0; /* Not initialized. */
   }
@@ -1912,15 +2048,20 @@ rep_hist_hs_stats_write(time_t now)
   }
 
   /* Generate history string. */
-  str = rep_hist_format_hs_stats(now);
+  str = rep_hist_format_hs_stats(now, is_v3);
 
   /* Reset HS history. */
-  rep_hist_reset_hs_stats(now);
+  if (is_v3) {
+    rep_hist_reset_hs_v3_stats(now);
+  } else {
+    rep_hist_reset_hs_v2_stats(now);
+  }
 
   /* Try to write to disk. */
   if (!check_or_create_data_subdir("stats")) {
-    write_to_data_subdir("stats", "hidserv-stats", str,
-                         "hidden service stats");
+    write_to_data_subdir("stats",
+                         is_v3 ? "hidserv-v3-stats" : "hidserv-stats",
+                         str, "hidden service stats");
   }
 
  done:
@@ -2134,7 +2275,8 @@ rep_hist_log_link_protocol_counts(void)
 void
 rep_hist_free_all(void)
 {
-  hs_stats_free(hs_stats);
+  hs_v2_stats_free(hs_v2_stats);
+  hs_v3_stats_free(hs_v3_stats);
   digestmap_free(history_map, free_or_history);
 
   tor_free(exit_bytes_read);
@@ -2155,3 +2297,19 @@ rep_hist_free_all(void)
   tor_assert_nonfatal(rephist_total_alloc == 0);
   tor_assert_nonfatal_once(rephist_total_num == 0);
 }
+
+#ifdef TOR_UNIT_TESTS
+/* only exists for unit tests: get HSv2 stats object */
+const hs_v2_stats_t *
+rep_hist_get_hs_v2_stats(void)
+{
+  return hs_v2_stats;
+}
+
+/* only exists for unit tests: get HSv2 stats object */
+const hs_v3_stats_t *
+rep_hist_get_hs_v3_stats(void)
+{
+  return hs_v3_stats;
+}
+#endif /* defined(TOR_UNIT_TESTS) */

src/feature/stats/rephist.h

@@ -65,10 +65,14 @@ MOCK_DECL(int, rep_hist_get_circuit_handshake_assigned, (uint16_t type));
 
 void rep_hist_hs_stats_init(time_t now);
 void rep_hist_hs_stats_term(void);
-time_t rep_hist_hs_stats_write(time_t now);
-char *rep_hist_get_hs_stats_string(void);
-void rep_hist_seen_new_rp_cell(void);
-void rep_hist_stored_maybe_new_hs(const crypto_pk_t *pubkey);
+time_t rep_hist_hs_stats_write(time_t now, bool is_v3);
+
+char *rep_hist_get_hs_v2_stats_string(void);
+void rep_hist_seen_new_rp_cell(bool is_v2);
+void rep_hist_hsdir_stored_maybe_new_v2_onion(const crypto_pk_t *pubkey);
+
+char *rep_hist_get_hs_v3_stats_string(void);
+void rep_hist_hsdir_stored_maybe_new_v3_onion(const uint8_t *blinded_key);
 
 void rep_hist_free_all(void);
 
@@ -83,6 +87,40 @@ extern int onion_handshakes_requested[MAX_ONION_HANDSHAKE_TYPE+1];
 extern int onion_handshakes_assigned[MAX_ONION_HANDSHAKE_TYPE+1];
 #endif
 
+#ifdef REPHIST_PRIVATE
+/** Carries the various hidden service statistics, and any other
+ *  information needed. */
+typedef struct hs_v2_stats_t {
+  /** How many v2 relay cells have we seen as rendezvous points? */
+  uint64_t rp_v2_relay_cells_seen;
+
+  /** Set of unique public key digests we've seen this stat period
+   * (could also be implemented as sorted smartlist). */
+  digestmap_t *v2_onions_seen_this_period;
+} hs_v2_stats_t;
+
+/** Structure that contains the various statistics we keep about v3
+ *  services.
+ *
+ *  Because of the time period logic of v3 services, v3 statistics are more
+ *  sensitive to time than v2 stats. For this reason, we collect v3
+ *  statistics strictly from 12:00UTC to 12:00UTC as dictated by
+ *  'start_of_hs_v3_stats_interval'.
+ **/
+typedef struct hs_v3_stats_t {
+  /** How many v3 relay cells have we seen as a rendezvous point? */
+  uint64_t rp_v3_relay_cells_seen;
+
+  /* The number of unique v3 onion descriptors (actually, unique v3 blind keys)
+   * we've seen during the measurement period */
+  digest256map_t *v3_onions_seen_this_period;
+} hs_v3_stats_t;
+
+MOCK_DECL(STATIC bool, should_collect_v3_stats,(void));
+
+STATIC char *rep_hist_format_hs_stats(time_t now, bool is_v3);
+#endif /* defined(REPHIST_PRIVATE) */
+
 /**
  * Represents the type of a cell for padding accounting
  */
@@ -108,4 +146,11 @@ void rep_hist_reset_padding_counts(void);
 void rep_hist_prep_published_padding_counts(time_t now);
 void rep_hist_padding_count_timers(uint64_t num_timers);
 
+#ifdef TOR_UNIT_TESTS
+struct hs_v2_stats_t;
+const struct hs_v2_stats_t *rep_hist_get_hs_v2_stats(void);
+struct hs_v3_stats_t;
+const struct hs_v3_stats_t *rep_hist_get_hs_v3_stats(void);
+#endif
+
 #endif /* !defined(TOR_REPHIST_H) */

src/test/hs_test_helpers.c

@@ -134,7 +134,8 @@ hs_helper_build_intro_point(const ed25519_keypair_t *signing_kp, time_t now,
  * points are added. */
 static hs_descriptor_t *
 hs_helper_build_hs_desc_impl(unsigned int no_ip,
-                             const ed25519_keypair_t *signing_kp)
+                             const ed25519_keypair_t *signing_kp,
+                             uint64_t rev_counter)
 {
   int ret;
   int i;
@@ -161,7 +162,7 @@ hs_helper_build_hs_desc_impl(unsigned int no_ip,
                     &signing_kp->pubkey, now, 3600,
                     CERT_FLAG_INCLUDE_SIGNING_KEY);
   tt_assert(desc->plaintext_data.signing_key_cert);
-  desc->plaintext_data.revision_counter = 42;
+  desc->plaintext_data.revision_counter = rev_counter;
   desc->plaintext_data.lifetime_sec = 3 * 60 * 60;
 
   hs_get_subcredential(&signing_kp->pubkey, &blinded_kp.pubkey,
@@ -226,18 +227,26 @@ hs_helper_get_subcred_from_identity_keypair(ed25519_keypair_t *signing_kp,
                        subcred_out);
 }
 
+/* Build a descriptor with a specific rev counter. */
+hs_descriptor_t *
+hs_helper_build_hs_desc_with_rev_counter(const ed25519_keypair_t *signing_kp,
+                                         uint64_t revision_counter)
+{
+  return hs_helper_build_hs_desc_impl(0, signing_kp, revision_counter);
+}
+
 /* Build a descriptor with introduction points. */
 hs_descriptor_t *
 hs_helper_build_hs_desc_with_ip(const ed25519_keypair_t *signing_kp)
 {
-  return hs_helper_build_hs_desc_impl(0, signing_kp);
+  return hs_helper_build_hs_desc_impl(0, signing_kp, 42);
 }
 
 /* Build a descriptor without any introduction points. */
 hs_descriptor_t *
 hs_helper_build_hs_desc_no_ip(const ed25519_keypair_t *signing_kp)
 {
-  return hs_helper_build_hs_desc_impl(1, signing_kp);
+  return hs_helper_build_hs_desc_impl(1, signing_kp, 42);
 }
 
 hs_descriptor_t *
@@ -247,7 +256,7 @@ hs_helper_build_hs_desc_with_client_auth(
                         const ed25519_keypair_t *signing_kp)
 {
   curve25519_keypair_t auth_ephemeral_kp;
-  hs_descriptor_t *desc = hs_helper_build_hs_desc_impl(0, signing_kp);
+  hs_descriptor_t *desc = hs_helper_build_hs_desc_impl(0, signing_kp, 42);
   hs_desc_authorized_client_t *desc_client;
 
   /* The number of client authorized auth has tobe a multiple of

src/test/hs_test_helpers.h

@@ -17,6 +17,10 @@ hs_descriptor_t *hs_helper_build_hs_desc_no_ip(
                                  const ed25519_keypair_t *signing_kp);
 hs_descriptor_t *hs_helper_build_hs_desc_with_ip(
                                  const ed25519_keypair_t *signing_kp);
+hs_descriptor_t *
+hs_helper_build_hs_desc_with_rev_counter(const ed25519_keypair_t *signing_kp,
+                                         uint64_t revision_counter);
+
 hs_descriptor_t *hs_helper_build_hs_desc_with_client_auth(
                                  const uint8_t *descriptor_cookie,
                                  const curve25519_public_key_t *client_pk,

src/test/test_stats.c

@@ -12,6 +12,8 @@
 #include "lib/crypt_ops/crypto_rand.h"
 #include "app/config/or_state_st.h"
 #include "test/rng_test_helpers.h"
+#include "feature/hs/hs_cache.h"
+#include "test/hs_test_helpers.h"
 
 #include <stdio.h>
 
@@ -31,6 +33,7 @@
 #define MAINLOOP_PRIVATE
 #define STATEFILE_PRIVATE
 #define BWHIST_PRIVATE
+#define REPHIST_PRIVATE
 
 #include "core/or/or.h"
 #include "lib/err/backtrace.h"
@@ -493,6 +496,131 @@ test_get_bandwidth_lines(void *arg)
   bwhist_free_all();
 }
 
+static bool
+mock_should_collect_v3_stats(void)
+{
+  return true;
+}
+
+/* Test v3 metrics */
+static void
+test_rephist_v3_onions(void *arg)
+{
+  int ret;
+
+  char *stats_string = NULL;
+  char *desc1_str = NULL;
+  ed25519_keypair_t signing_kp1;
+  hs_descriptor_t *desc1 = NULL;
+
+  const hs_v3_stats_t *hs_v3_stats = NULL;
+
+  (void) arg;
+
+  MOCK(should_collect_v3_stats, mock_should_collect_v3_stats);
+
+  get_options_mutable()->HiddenServiceStatistics = 1;
+
+  /* Initialize the subsystems */
+  hs_cache_init();
+  rep_hist_hs_stats_init(0);
+  update_approx_time(10101010101);
+
+  /* HS stats should be zero here */
+  hs_v3_stats = rep_hist_get_hs_v3_stats();
+  tt_int_op(digest256map_size(hs_v3_stats->v3_onions_seen_this_period),
+            OP_EQ, 0);
+
+  /* Generate a valid descriptor */
+  ret = ed25519_keypair_generate(&signing_kp1, 0);
+  tt_int_op(ret, OP_EQ, 0);
+  desc1 = hs_helper_build_hs_desc_with_rev_counter(&signing_kp1, 42);
+  tt_assert(desc1);
+  ret = hs_desc_encode_descriptor(desc1, &signing_kp1, NULL, &desc1_str);
+  tt_int_op(ret, OP_EQ, 0);
+
+  /* Store descriptor and check that stats got updated */
+  ret = hs_cache_store_as_dir(desc1_str);
+  tt_int_op(ret, OP_EQ, 0);
+  hs_v3_stats = rep_hist_get_hs_v3_stats();
+  tt_int_op(digest256map_size(hs_v3_stats->v3_onions_seen_this_period),
+            OP_EQ, 1);
+
+  /* cleanup */
+  hs_descriptor_free(desc1);
+  tor_free(desc1_str);
+
+  /* Generate another valid descriptor */
+  ret = ed25519_keypair_generate(&signing_kp1, 0);
+  tt_int_op(ret, OP_EQ, 0);
+  desc1 = hs_helper_build_hs_desc_with_rev_counter(&signing_kp1, 42);
+  tt_assert(desc1);
+  ret = hs_desc_encode_descriptor(desc1, &signing_kp1, NULL, &desc1_str);
+  tt_int_op(ret, OP_EQ, 0);
+
+  /* Store descriptor and check that stats are updated */
+  ret = hs_cache_store_as_dir(desc1_str);
+  tt_int_op(ret, OP_EQ, 0);
+  hs_v3_stats = rep_hist_get_hs_v3_stats();
+  tt_int_op(digest256map_size(hs_v3_stats->v3_onions_seen_this_period),
+            OP_EQ, 2);
+
+  /* Check that storing the same descriptor twice does not work */
+  ret = hs_cache_store_as_dir(desc1_str);
+  tt_int_op(ret, OP_EQ, -1);
+
+  /* cleanup */
+  hs_descriptor_free(desc1);
+  tor_free(desc1_str);
+
+  /* Create a descriptor with the same identity key but diff rev counter and
+     same blinded key */
+  desc1 = hs_helper_build_hs_desc_with_rev_counter(&signing_kp1, 43);
+  tt_assert(desc1);
+  ret = hs_desc_encode_descriptor(desc1, &signing_kp1, NULL, &desc1_str);
+  tt_int_op(ret, OP_EQ, 0);
+
+  /* Store descriptor and check that stats are updated */
+  ret = hs_cache_store_as_dir(desc1_str);
+  tt_int_op(ret, OP_EQ, 0);
+  tt_int_op(digest256map_size(hs_v3_stats->v3_onions_seen_this_period),
+            OP_EQ, 2);
+
+  /* cleanup */
+  hs_descriptor_free(desc1);
+  tor_free(desc1_str);
+
+  /* Now let's skip to four days forward so that the blinded key rolls
+     forward */
+  update_approx_time(approx_time() + 345600);
+
+  /* Now create a descriptor with the same identity key but diff rev counter
+     and different blinded key */
+  desc1 = hs_helper_build_hs_desc_with_rev_counter(&signing_kp1, 44);
+  tt_assert(desc1);
+  ret = hs_desc_encode_descriptor(desc1, &signing_kp1, NULL, &desc1_str);
+  tt_int_op(ret, OP_EQ, 0);
+
+  /* Store descriptor and check that stats are updated */
+  ret = hs_cache_store_as_dir(desc1_str);
+  tt_int_op(ret, OP_EQ, 0);
+  tt_int_op(digest256map_size(hs_v3_stats->v3_onions_seen_this_period),
+            OP_EQ, 3);
+
+  /* cleanup */
+  hs_descriptor_free(desc1);
+  tor_free(desc1_str);
+
+  /* Because of differential privacy we can't actually check the stat value,
+     but let's just check that it's formatted correctly. */
+  stats_string = rep_hist_format_hs_stats(approx_time(), true);
+  tt_assert(strstr(stats_string, "hidserv-dir-v3-onions-seen"));
+
+ done:
+  UNMOCK(should_collect_v3_stats);
+  tor_free(stats_string);
+}
+
 #define ENT(name)                                                       \
   { #name, test_ ## name , 0, NULL, NULL }
 #define FORK(name)                                                      \
@@ -506,6 +634,7 @@ struct testcase_t stats_tests[] = {
   FORK(add_obs),
   FORK(fill_bandwidth_history),
   FORK(get_bandwidth_lines),
+  FORK(rephist_v3_onions),
 
   END_OF_TESTCASES
 };