prop224: Expand the overlap period concept to be a full SRV protocol run

Because of #23387, we've realized that there is one scenario that makes
the client unable to reach the service because of a desynch in the time
period used. The scenario is as follows:

  +------------------------------------------------------------------+
  |                                                                  |
  | 00:00      12:00       00:00       12:00       00:00       12:00 |
  | SRV#1      TP#1        SRV#2       TP#2        SRV#3       TP#3  |
  |                                                                  |
  |  $==========|-----------$===========|-----------$===========|    |
  |                                    ^ ^                           |
  |                                    C S                           |
  +------------------------------------------------------------------+

In this scenario the HS has a newer consensus than the client, and the
HS just moved to the next TP but the client is still stuck on the old
one. However, the service is not in any sort of overlap mode so it
doesn't cover the old TP anymore, so the client is unable to fetch a
descriptor.

We've decided to solve this by extending the concept of overlap period
to be permanent so that the service always publishes two descriptors and
aims to cover clients with both older and newer consensuses. See the
spec patch in #23387 for more details.

src/or/hs_common.c

@@ -276,6 +276,14 @@ hs_get_next_time_period_num(time_t now)
   return hs_get_time_period_num(now) + 1;
 }
 
+/* Get the number of the _previous_ HS time period, given that the current
+ * time is <b>now</b>. */
+uint64_t
+hs_get_previous_time_period_num(time_t now)
+{
+  return hs_get_time_period_num(now) - 1;
+}
+
 /* Return the start time of the upcoming time period based on <b>now</b>. */
 time_t
 hs_get_start_time_of_next_time_period(time_t now)

src/or/hs_common.h

@@ -195,6 +195,7 @@ void hs_get_subcredential(const ed25519_public_key_t *identity_pk,
                           const ed25519_public_key_t *blinded_pk,
                           uint8_t *subcred_out);
 
+uint64_t hs_get_previous_time_period_num(time_t now);
 uint64_t hs_get_time_period_num(time_t now);
 uint64_t hs_get_next_time_period_num(time_t now);
 time_t hs_get_start_time_of_next_time_period(time_t now);

src/or/hs_descriptor.h

@@ -33,8 +33,11 @@ struct link_specifier_t;
  * which is 720 minutes or 43200 seconds. */
 #define HS_DESC_MAX_LIFETIME (12 * 60 * 60)
 /* Lifetime of certificate in the descriptor. This defines the lifetime of the
- * descriptor signing key and the cross certification cert of that key. */
+ * descriptor signing key and the cross certification cert of that key. It is
-#define HS_DESC_CERT_LIFETIME (36 * 60 * 60)
+ * set to 54 hours because a descriptor can be around for 48 hours and because
+ * consensuses are used after the hour, add an extra 6 hours to give some time
+ * for the service to stop using it. */
+#define HS_DESC_CERT_LIFETIME (54 * 60 * 60)
 /* Length of the salt needed for the encrypted section of a descriptor. */
 #define HS_DESC_ENCRYPTED_SALT_LEN 16
 /* Length of the secret input needed for the KDF construction which derives

src/or/hs_service.c

@@ -23,6 +23,7 @@
 #include "router.h"
 #include "routerkeys.h"
 #include "routerlist.h"
+#include "shared_random_state.h"
 #include "statefile.h"
 
 #include "hs_circuit.h"
@@ -769,7 +770,6 @@ move_hs_state(hs_service_t *src_service, hs_service_t *dst_service)
   /* Let's do a shallow copy */
   dst->intro_circ_retry_started_time = src->intro_circ_retry_started_time;
   dst->num_intro_circ_launched = src->num_intro_circ_launched;
-  dst->in_overlap_period = src->in_overlap_period;
   dst->replay_cache_rend_cookie = src->replay_cache_rend_cookie;
 
   src->replay_cache_rend_cookie = NULL; /* steal pointer reference */
@@ -1366,27 +1366,80 @@ build_service_descriptor(hs_service_t *service, time_t now,
   service_descriptor_free(desc);
 }
 
+/* Build both descriptors for the given service that has just booted up.
+ * Because it's a special case, it deserves its special function ;). */
+static void
+build_descriptors_for_new_service(hs_service_t *service, time_t now)
+{
+  uint64_t current_desc_tp, next_desc_tp;
+
+  tor_assert(service);
+  /* These are the conditions for a new service. */
+  tor_assert(!service->desc_current);
+  tor_assert(!service->desc_next);
+
+  /*
+   * +------------------------------------------------------------------+
+   * |                                                                  |
+   * | 00:00      12:00       00:00       12:00       00:00       12:00 |
+   * | SRV#1      TP#1        SRV#2       TP#2        SRV#3       TP#3  |
+   * |                                                                  |
+   * |  $==========|-----------$===========|-----------$===========|    |
+   * |                             ^         ^                          |
+   * |                             A         B                          |
+   * +------------------------------------------------------------------+
+   *
+   * Case A: The service boots up before a new time period, the current time
+   * period is thus TP#1 and the next is TP#2 which for both we have access to
+   * their SRVs.
+   *
+   * Case B: The service boots up inside TP#2, we can't use the TP#3 for the
+   * next descriptor because we don't have the SRV#3 so the current should be
+   * TP#1 and next TP#2.
+   */
+
+  if (hs_time_between_tp_and_srv(NULL, now)) {
+    /* Case B from the above, inside of the new time period. */
+    current_desc_tp = hs_get_previous_time_period_num(0); /* TP#1 */
+    next_desc_tp = hs_get_time_period_num(0);             /* TP#2 */
+  } else {
+    /* Case A from the above, outside of the new time period. */
+    current_desc_tp = hs_get_time_period_num(0);    /* TP#1 */
+    next_desc_tp = hs_get_next_time_period_num(0);  /* TP#2 */
+  }
+
+  /* Build descriptors. */
+  build_service_descriptor(service, now, current_desc_tp,
+                           &service->desc_current);
+  build_service_descriptor(service, now, next_desc_tp,
+                           &service->desc_next);
+  log_info(LD_REND, "Hidden service %s has just started. Both descriptors "
+                    "built. Now scheduled for upload.",
+           safe_str_client(service->onion_address));
+}
+
 /* Build descriptors for each service if needed. There are conditions to build
  * a descriptor which are details in the function. */
 STATIC void
 build_all_descriptors(time_t now)
 {
   FOR_EACH_SERVICE_BEGIN(service) {
-    if (service->desc_current == NULL) {
-      /* This means we just booted up because else this descriptor will never
-       * be NULL as it should always point to the descriptor that was in
-       * desc_next after rotation. */
-      build_service_descriptor(service, now, hs_get_time_period_num(0),
-                               &service->desc_current);
 
-      log_info(LD_REND, "Hidden service %s current descriptor successfully "
+    /* A service booting up will have both descriptors to NULL. No other cases
-                        "built. Now scheduled for upload.",
+     * makes both descriptor non existent. */
-               safe_str_client(service->onion_address));
+    if (service->desc_current == NULL && service->desc_next == NULL) {
+      build_descriptors_for_new_service(service, now);
+      continue;
     }
-    /* A next descriptor to NULL indicate that we need to build a fresh one if
+
-     * we are in the overlap period for the _next_ time period since it means
+    /* Reaching this point means we are pass bootup so at runtime. We should
-     * we either just booted or we just rotated our descriptors. */
+     * *never* have an empty current descriptor. If the next descriptor is
-    if (hs_overlap_mode_is_active(NULL, now) && service->desc_next == NULL) {
+     * empty, we'll try to build it for the next time period. This only
+     * happens when we rotate meaning that we are guaranteed to have a new SRV
+     * at that point for the next time period. */
+    tor_assert(service->desc_current);
+
+    if (service->desc_next == NULL) {
       build_service_descriptor(service, now, hs_get_next_time_period_num(0),
                                &service->desc_next);
       log_info(LD_REND, "Hidden service %s next descriptor successfully "
@@ -1716,10 +1769,62 @@ cleanup_intro_points(hs_service_t *service, time_t now)
   } FOR_EACH_DESCRIPTOR_END;
 }
 
-/** We just entered overlap period and we need to rotate our <b>service</b>
+/* Set the next rotation time of the descriptors for the given service for the
- *  descriptors */
+ * time now. */
 static void
-rotate_service_descriptors(hs_service_t *service)
+set_rotation_time(hs_service_t *service, time_t now)
+{
+  time_t valid_after;
+  const networkstatus_t *ns = networkstatus_get_live_consensus(now);
+  if (ns) {
+    valid_after = ns->valid_after;
+  } else {
+    valid_after = now;
+  }
+
+  tor_assert(service);
+  service->state.next_rotation_time =
+    sr_state_get_start_time_of_current_protocol_run(valid_after) +
+    sr_state_get_protocol_run_duration();
+
+  {
+    char fmt_time[ISO_TIME_LEN + 1];
+    format_local_iso_time(fmt_time, service->state.next_rotation_time);
+    log_info(LD_REND, "Next descriptor rotation time set to %s for %s",
+             fmt_time, safe_str_client(service->onion_address));
+  }
+}
+
+/* Return true iff the service should rotate its descriptor. The time now is
+ * only used to fetch the live consensus and if none can be found, this
+ * returns false. */
+static unsigned int
+should_rotate_descriptors(hs_service_t *service, time_t now)
+{
+  const networkstatus_t *ns;
+
+  tor_assert(service);
+
+  ns = networkstatus_get_live_consensus(now);
+  if (ns == NULL) {
+    goto no_rotation;
+  }
+
+  if (ns->valid_after >= service->state.next_rotation_time) {
+    goto rotation;
+  }
+
+ no_rotation:
+  return 0;
+ rotation:
+  return 1;
+}
+
+/* Rotate the service descriptors of the given service. The current descriptor
+ * will be freed, the next one put in as the current and finally the next
+ * descriptor pointer is NULLified. */
+static void
+rotate_service_descriptors(hs_service_t *service, time_t now)
 {
   if (service->desc_current) {
     /* Close all IP circuits for the descriptor. */
@@ -1732,37 +1837,13 @@ rotate_service_descriptors(hs_service_t *service)
    * a descriptor creation for it. */
   service->desc_current = service->desc_next;
   service->desc_next = NULL;
+
+  /* We've just rotated, set the next time for the rotation. */
+  set_rotation_time(service, now);
 }
 
-/** Return true if <b>service</b> **just** entered overlap mode. */
+/* Rotate descriptors for each service if needed. A non existing current
-static int
+ * descriptor will trigger a descriptor build for the next time period. */
-service_just_entered_overlap_mode(const hs_service_t *service,
-                                  int overlap_mode_is_active)
-{
-  if (overlap_mode_is_active && !service->state.in_overlap_period) {
-    return 1;
-  }
-
-  return 0;
-}
-
-/** Return true if <b>service</b> **just** left overlap mode. */
-static int
-service_just_left_overlap_mode(const hs_service_t *service,
-                               int overlap_mode_is_active)
-{
-  if (!overlap_mode_is_active && service->state.in_overlap_period) {
-    return 1;
-  }
-
-  return 0;
-}
-
-/* Rotate descriptors for each service if needed. If we are just entering or
- * leaving the overlap period, rotate them that is point the previous
- * descriptor to the current and cleanup the previous one. A non existing
- * current descriptor will trigger a descriptor build for the next time
- * period. */
 STATIC void
 rotate_all_descriptors(time_t now)
 {
@@ -1770,56 +1851,26 @@ rotate_all_descriptors(time_t now)
    *     be wise, to rotate service descriptors independently to hide that all
    *     those descriptors are on the same tor instance */
 
-  int overlap_mode_is_active = hs_overlap_mode_is_active(NULL, now);
-
   FOR_EACH_SERVICE_BEGIN(service) {
-    int service_entered_overlap = service_just_entered_overlap_mode(service,
+
-                                                      overlap_mode_is_active);
+    /* Note for a service booting up: Both descriptors are NULL in that case
-    int service_left_overlap = service_just_left_overlap_mode(service,
+     * so this function might return true if we are in the timeframe for a
-                                             overlap_mode_is_active);
+     * rotation leading to basically swapping two NULL pointers which is
-    /* This should not be possible */
+     * harmless. However, the side effect is that triggering a rotation will
-    if (BUG(service_entered_overlap && service_left_overlap)) {
+     * update the service state and avoid doing anymore rotations after the
-      return;
+     * two descriptors have been built. */
+    if (!should_rotate_descriptors(service, now)) {
+      continue;
     }
 
-    /* No changes in overlap mode: nothing to do here */
+    tor_assert(service->desc_current);
-    if (!service_entered_overlap && !service_left_overlap) {
+    tor_assert(service->desc_next);
-      return;
-    }
 
-    /* To get down there means that some change happened to overlap mode */
+    log_info(LD_REND, "Time to rotate our descriptors (%p / %p) for %s",
-    tor_assert(service_entered_overlap || service_left_overlap);
+             service->desc_current, service->desc_next,
+             safe_str_client(service->onion_address));
 
-    /* Update the overlap marks on this service */
+    rotate_service_descriptors(service, now);
-    if (service_entered_overlap) {
-      /* It's the first time the service encounters the overlap period so flag
-       * it in order to make sure we don't rotate at next check. */
-      service->state.in_overlap_period = 1;
-    } else if (service_left_overlap) {
-      service->state.in_overlap_period = 0;
-    }
-
-    if (service_entered_overlap) {
-      /* We just entered overlap period: recompute all HSDir indices. We need
-       * to do this otherwise nodes can get stuck with old HSDir indices until
-       * we fetch a new consensus, and we might need to reupload our desc
-       * before that. */
-      /* XXX find a better place than rotate_all_descriptors() to do this */
-      nodelist_recompute_all_hsdir_indices();
-    }
-
-    /* If we just entered or left overlap mode, rotate our descriptors */
-    log_info(LD_REND, "We just %s overlap period. About to rotate %s "
-             "descriptors (%p / %p).",
-             service_entered_overlap ? "entered" : "left",
-             safe_str_client(service->onion_address),
-             service->desc_current, service->desc_next);
-
-    /* If we have a next descriptor lined up, rotate the descriptors so that it
-     * becomes current. */
-    if (service->desc_next) {
-      rotate_service_descriptors(service);
-    }
   } FOR_EACH_SERVICE_END;
 }
 
@@ -1833,6 +1884,17 @@ run_housekeeping_event(time_t now)
    * simply moving things around or removing uneeded elements. */
 
   FOR_EACH_SERVICE_BEGIN(service) {
+
+    /* If the service is starting off, set the rotation time. We can't do that
+     * at configure time because the get_options() needs to be set for setting
+     * that time that uses the voting interval. */
+    if (service->state.next_rotation_time == 0) {
+      /* Set the next rotation time of the descriptors. If it's Oct 25th
+       * 23:47:00, the next rotation time is when the next SRV is computed
+       * which is at Oct 26th 00:00:00 that is in 13 minutes. */
+      set_rotation_time(service, now);
+    }
+
     /* Cleanup invalid intro points from the service descriptor. */
     cleanup_intro_points(service, now);
 
@@ -2504,9 +2566,8 @@ run_upload_descriptor_event(time_t now)
        * accurate because all circuits have been established. */
       build_desc_intro_points(service, desc, now);
 
-      /* If the service is in the overlap period and this descriptor is the
+      /* The next descriptor needs the next time period for computing the
-       * next one, it has to be uploaded for the next time period meaning
+       * corresponding hashring. */
-       * we'll use the next node_t hsdir_index to pick the HSDirs. */
       if (desc == service->desc_next) {
         for_next_period = 1;
       }
@@ -3091,6 +3152,7 @@ hs_service_new(const or_options_t *options)
   /* Allocate the CLIENT_PK replay cache in service state. */
   service->state.replay_cache_rend_cookie =
     replaycache_new(REND_REPLAY_TIME_INTERVAL, REND_REPLAY_TIME_INTERVAL);
+
   return service;
 }
 

src/or/hs_service.h

@@ -196,16 +196,16 @@ typedef struct hs_service_state_t {
    * should never go over MAX_INTRO_CIRCS_PER_PERIOD. */
   unsigned int num_intro_circ_launched;
 
-  /* Indicate that the service has entered the overlap period. We use this
-   * flag to check for descriptor rotation. */
-  unsigned int in_overlap_period : 1;
-
   /* Replay cache tracking the REND_COOKIE found in INTRODUCE2 cell to detect
    * repeats. Clients may send INTRODUCE1 cells for the same rendezvous point
    * through two or more different introduction points; when they do, this
    * keeps us from launching multiple simultaneous attempts to connect to the
    * same rend point. */
   replaycache_t *replay_cache_rend_cookie;
+
+  /* When is the next time we should rotate our descriptors. This is has to be
+   * done at the start time of the next SRV protocol run. */
+  time_t next_rotation_time;
 } hs_service_state_t;
 
 /* Representation of a service running on this tor instance. */
@@ -229,8 +229,7 @@ typedef struct hs_service_t {
 
   /* Current descriptor. */
   hs_service_descriptor_t *desc_current;
-  /* Next descriptor that we need for the overlap period for which we have to
+  /* Next descriptor. */
-   * keep two sets of opened introduction point circuits. */
   hs_service_descriptor_t *desc_next;
 
   /* XXX: Credential (client auth.) #20700. */

src/test/test_hs_common.c

@@ -250,117 +250,6 @@ test_start_time_of_next_time_period(void *arg)
   ;
 }
 
-/** Test that our HS overlap period functions work properly. */
-static void
-test_desc_overlap_period(void *arg)
-{
-  (void) arg;
-  int retval;
-  time_t now = time(NULL);
-  networkstatus_t *dummy_consensus = NULL;
-
-  /* First try with a consensus just inside the overlap period */
-  dummy_consensus = tor_malloc_zero(sizeof(networkstatus_t));
-  retval = parse_rfc1123_time("Wed, 13 Apr 2016 00:00:00 UTC",
-                              &dummy_consensus->valid_after);
-  tt_int_op(retval, OP_EQ, 0);
-
-  retval = hs_overlap_mode_is_active(dummy_consensus, now);
-  tt_int_op(retval, OP_EQ, 1);
-
-  /* Now increase the valid_after so that it goes to 11:00:00 UTC. Overlap
-     period is still active. */
-  dummy_consensus->valid_after += 3600*11;
-  retval = hs_overlap_mode_is_active(dummy_consensus, now);
-  tt_int_op(retval, OP_EQ, 1);
-
-  /* Now increase the valid_after so that it goes to 11:59:59 UTC. Overlap
-     period is still active. */
-  dummy_consensus->valid_after += 3599;
-  retval = hs_overlap_mode_is_active(dummy_consensus, now);
-  tt_int_op(retval, OP_EQ, 1);
-
-  /* Now increase the valid_after so that it drifts to noon, and check that
-     overlap mode is not active anymore. */
-  dummy_consensus->valid_after += 1;
-  retval = hs_overlap_mode_is_active(dummy_consensus, now);
-  tt_int_op(retval, OP_EQ, 0);
-
-  /* Check that overlap mode is also inactive at 23:59:59 UTC */
-  retval = parse_rfc1123_time("Wed, 13 Apr 2016 23:59:59 UTC",
-                              &dummy_consensus->valid_after);
-  tt_int_op(retval, OP_EQ, 0);
-  retval = hs_overlap_mode_is_active(dummy_consensus, now);
-  tt_int_op(retval, OP_EQ, 0);
-
- done:
-  tor_free(dummy_consensus);
-}
-
-/* Test the overlap period functions on a testnet with altered voting
- * schedule */
-static void
-test_desc_overlap_period_testnet(void *arg)
-{
-  int retval;
-  time_t now = approx_time();
-  networkstatus_t *dummy_consensus = NULL;
-  or_options_t *options = get_options_mutable();
-
-  (void) arg;
-
-  /* Set the testnet option and a 10-second voting interval */
-  options->TestingTorNetwork = 1;
-  options->V3AuthVotingInterval = 10;
-  options->TestingV3AuthInitialVotingInterval = 10;
-
-  dummy_consensus = tor_malloc_zero(sizeof(networkstatus_t));
-
-  /* A 10-second voting interval means that the lengths of an SRV run and of a
-   * time period are both 10*24 seconds (4 minutes). The SRV gets published at
-   * 00:00:00 and the TP starts at 00:02:00 (rotation offset: 2 mins). Those
-   * two minutes between SRV publish and TP start is the overlap period
-   * window. Let's test it: */
-  retval = parse_rfc1123_time("Wed, 13 Apr 2016 00:00:00 UTC",
-                              &dummy_consensus->valid_after);
-  tt_int_op(retval, OP_EQ, 0);
-  retval = hs_overlap_mode_is_active(dummy_consensus, now);
-  tt_int_op(retval, OP_EQ, 1);
-
-  retval = parse_rfc1123_time("Wed, 13 Apr 2016 00:01:59 UTC",
-                              &dummy_consensus->valid_after);
-  tt_int_op(retval, OP_EQ, 0);
-  retval = hs_overlap_mode_is_active(dummy_consensus, now);
-  tt_int_op(retval, OP_EQ, 1);
-
-  retval = parse_rfc1123_time("Wed, 13 Apr 2016 00:02:00 UTC",
-                              &dummy_consensus->valid_after);
-  tt_int_op(retval, OP_EQ, 0);
-  retval = hs_overlap_mode_is_active(dummy_consensus, now);
-  tt_int_op(retval, OP_EQ, 0);
-
-  retval = parse_rfc1123_time("Wed, 13 Apr 2016 00:04:00 UTC",
-                              &dummy_consensus->valid_after);
-  tt_int_op(retval, OP_EQ, 0);
-  retval = hs_overlap_mode_is_active(dummy_consensus, now);
-  tt_int_op(retval, OP_EQ, 1);
-
-  retval = parse_rfc1123_time("Wed, 13 Apr 2016 00:05:59 UTC",
-                              &dummy_consensus->valid_after);
-  tt_int_op(retval, OP_EQ, 0);
-  retval = hs_overlap_mode_is_active(dummy_consensus, now);
-  tt_int_op(retval, OP_EQ, 1);
-
-  retval = parse_rfc1123_time("Wed, 13 Apr 2016 00:06:00 UTC",
-                              &dummy_consensus->valid_after);
-  tt_int_op(retval, OP_EQ, 0);
-  retval = hs_overlap_mode_is_active(dummy_consensus, now);
-  tt_int_op(retval, OP_EQ, 0);
-
- done:
-  tor_free(dummy_consensus);
-}
-
 static void
 helper_add_hsdir_to_networkstatus(networkstatus_t *ns,
                                   int identity_idx,
@@ -860,10 +749,6 @@ struct testcase_t hs_common_tests[] = {
     NULL, NULL },
   { "start_time_of_next_time_period", test_start_time_of_next_time_period,
     TT_FORK, NULL, NULL },
-  { "desc_overlap_period", test_desc_overlap_period, TT_FORK,
-    NULL, NULL },
-  { "desc_overlap_period_testnet", test_desc_overlap_period_testnet, TT_FORK,
-    NULL, NULL },
   { "responsible_hsdirs", test_responsible_hsdirs, TT_FORK,
     NULL, NULL },
   { "desc_reupload_logic", test_desc_reupload_logic, TT_FORK,

src/test/test_hs_service.c

@@ -45,6 +45,7 @@
 #include "main.h"
 #include "rendservice.h"
 #include "statefile.h"
+#include "shared_random_state.h"
 
 /* Trunnel */
 #include "hs/cell_establish_intro.h"
@@ -98,17 +99,6 @@ mock_relay_send_command_from_edge(streamid_t stream_id, circuit_t *circ,
   return 0;
 }
 
-/* Mock function that always return true so we can test the descriptor
- * creation of the next time period deterministically. */
-static int
-mock_hs_overlap_mode_is_active_true(const networkstatus_t *consensus,
-                                    time_t now)
-{
-  (void) consensus;
-  (void) now;
-  return 1;
-}
-
 /* Helper: from a set of options in conf, configure a service which will add
  * it to the staging list of the HS subsytem. */
 static int
@@ -942,12 +932,12 @@ test_service_event(void *arg)
   UNMOCK(circuit_mark_for_close_);
 }
 
-/** Test that we rotate descriptors correctly in overlap period */
+/** Test that we rotate descriptors correctly. */
 static void
 test_rotate_descriptors(void *arg)
 {
   int ret;
-  time_t now = time(NULL);
+  time_t next_rotation_time;
   hs_service_t *service;
   hs_service_descriptor_t *desc_next;
   hs_service_intro_point_t *ip;
@@ -959,10 +949,11 @@ test_rotate_descriptors(void *arg)
   MOCK(networkstatus_get_live_consensus,
        mock_networkstatus_get_live_consensus);
 
-  /* Setup the valid_after time to be 13:00 UTC, not in overlap period. The
+  /* Descriptor rotation happens with a consensus with a new SRV. */
-   * overlap check doesn't care about the year. */
+
   ret = parse_rfc1123_time("Sat, 26 Oct 1985 13:00:00 UTC",
                            &mock_ns.valid_after);
+  tt_int_op(ret, OP_EQ, 0);
   ret = parse_rfc1123_time("Sat, 26 Oct 1985 14:00:00 UTC",
                            &mock_ns.fresh_until);
   tt_int_op(ret, OP_EQ, 0);
@@ -973,63 +964,48 @@ test_rotate_descriptors(void *arg)
   ip = helper_create_service_ip();
   service_intro_point_add(service->desc_current->intro_points.map, ip);
 
-  /* Nothing should happen because we are not in the overlap period. */
+  /* Tweak our service next rotation time so we can use a custom time. */
-  rotate_all_descriptors(now);
+  service->state.next_rotation_time = next_rotation_time =
-  tt_int_op(service->state.in_overlap_period, OP_EQ, 0);
+    mock_ns.valid_after + (11 * 60 * 60);
+
+  /* Nothing should happen, we are not at a new SRV. Our next rotation time
+   * should be untouched. */
+  rotate_all_descriptors(mock_ns.valid_after);
+  tt_u64_op(service->state.next_rotation_time, OP_EQ, next_rotation_time);
   tt_assert(service->desc_current);
   tt_int_op(digest256map_size(service->desc_current->intro_points.map),
             OP_EQ, 1);
 
-  /* Entering an overlap period. */
+  /* Going right after a new SRV. */
-  ret = parse_rfc1123_time("Sat, 26 Oct 1985 01:00:00 UTC",
+  ret = parse_rfc1123_time("Sat, 27 Oct 1985 01:00:00 UTC",
                            &mock_ns.valid_after);
-  ret = parse_rfc1123_time("Sat, 26 Oct 1985 02:00:00 UTC",
+  tt_int_op(ret, OP_EQ, 0);
+  ret = parse_rfc1123_time("Sat, 27 Oct 1985 02:00:00 UTC",
                            &mock_ns.fresh_until);
   tt_int_op(ret, OP_EQ, 0);
+
   desc_next = service_descriptor_new();
-  desc_next->next_upload_time = 42; /* Our marker to recognize it. */
   service->desc_next = desc_next;
-  /* We should have our state flagged to be in the overlap period, our current
+  /* Note down what to expect for the next rotation time which is 01:00 + 23h
-   * descriptor cleaned up and the next descriptor becoming the current. */
+   * meaning 00:00:00. */
-  rotate_all_descriptors(now);
+  next_rotation_time = mock_ns.valid_after + (23 * 60 * 60);
-  tt_int_op(service->state.in_overlap_period, OP_EQ, 1);
+  /* We should have our next rotation time modified, our current descriptor
+   * cleaned up and the next descriptor becoming the current. */
+  rotate_all_descriptors(mock_ns.valid_after);
+  tt_u64_op(service->state.next_rotation_time, OP_EQ, next_rotation_time);
   tt_mem_op(service->desc_current, OP_EQ, desc_next, sizeof(*desc_next));
   tt_int_op(digest256map_size(service->desc_current->intro_points.map),
             OP_EQ, 0);
   tt_assert(service->desc_next == NULL);
+
   /* A second time should do nothing. */
-  rotate_all_descriptors(now);
+  rotate_all_descriptors(mock_ns.valid_after);
-  tt_int_op(service->state.in_overlap_period, OP_EQ, 1);
+  tt_u64_op(service->state.next_rotation_time, OP_EQ, next_rotation_time);
   tt_mem_op(service->desc_current, OP_EQ, desc_next, sizeof(*desc_next));
   tt_int_op(digest256map_size(service->desc_current->intro_points.map),
             OP_EQ, 0);
   tt_assert(service->desc_next == NULL);
 
-  /* Now let's re-create desc_next and get out of overlap period. We should
-     test that desc_current gets replaced by desc_next, and desc_next becomes
-     NULL. */
-  desc_next = service_descriptor_new();
-  desc_next->next_upload_time = 240; /* Our marker to recognize it. */
-  service->desc_next = desc_next;
-
-  /* Going out of the overlap period. */
-  ret = parse_rfc1123_time("Sat, 26 Oct 1985 12:00:00 UTC",
-                           &mock_ns.valid_after);
-  ret = parse_rfc1123_time("Sat, 26 Oct 1985 13:00:00 UTC",
-                           &mock_ns.fresh_until);
-  /* This should reset the state and not touch the current descriptor. */
-  tt_int_op(ret, OP_EQ, 0);
-  rotate_all_descriptors(now);
-  tt_int_op(service->state.in_overlap_period, OP_EQ, 0);
-  tt_mem_op(service->desc_current, OP_EQ, desc_next, sizeof(*desc_next));
-  tt_assert(service->desc_next == NULL);
-
-  /* Calling rotate_all_descriptors() another time should do nothing */
-  rotate_all_descriptors(now);
-  tt_int_op(service->state.in_overlap_period, OP_EQ, 0);
-  tt_mem_op(service->desc_current, OP_EQ, desc_next, sizeof(*desc_next));
-  tt_assert(service->desc_next == NULL);
-
  done:
   hs_free_all();
   UNMOCK(circuit_mark_for_close_);
@@ -1050,7 +1026,6 @@ test_build_update_descriptors(void *arg)
   (void) arg;
 
   hs_init();
-  MOCK(hs_overlap_mode_is_active, mock_hs_overlap_mode_is_active_true);
   MOCK(get_or_state,
        get_or_state_replacement);
   MOCK(networkstatus_get_live_consensus,
@@ -1196,7 +1171,6 @@ test_build_update_descriptors(void *arg)
  done:
   hs_free_all();
   nodelist_free_all();
-  UNMOCK(hs_overlap_mode_is_active);
 }
 
 static void
@@ -1209,7 +1183,6 @@ test_upload_descriptors(void *arg)
   (void) arg;
 
   hs_init();
-  MOCK(hs_overlap_mode_is_active, mock_hs_overlap_mode_is_active_true);
   MOCK(get_or_state,
        get_or_state_replacement);
   MOCK(networkstatus_get_live_consensus,
@@ -1253,7 +1226,6 @@ test_upload_descriptors(void *arg)
 
  done:
   hs_free_all();
-  UNMOCK(hs_overlap_mode_is_active);
   UNMOCK(get_or_state);
 }