token_bucket_ctr: replace 32-bit wallclock time with monotime

This started as a response to ticket #40792 where Coverity is
complaining about a potential year 2038 bug where we cast time_t from
approx_time() to uint32_t for use in token_bucket_ctr.

There was a larger can of worms though, since token_bucket really
doesn't want to be using wallclock time here. I audited the call sites
for approx_time() and changed any that used a 32-bit cast or made
inappropriate use of wallclock time. Things like certificate lifetime,
consensus intervals, etc. need wallclock time. Measurements of rates
over time, however, are better served with a monotonic timer that does
not try and sync with wallclock ever.

Looking closer at token_bucket, its design is a bit odd because it was
initially intended for use with tick units but later forked into
token_bucket_rw which uses ticks to count bytes per second, and
token_bucket_ctr which uses seconds to count slower events. The rates
represented by either token bucket can't be lower than 1 per second, so
the slower timer in 'ctr' is necessary to represent the slower rates of
things like connections or introduction packets or rendezvous attempts.

I considered modifying token_bucket to use 64-bit timestamps overall
instead of 32-bit, but that seemed like an unnecessarily invasive change
that would grant some peace of mind but probably not help much. I was
more interested in removing the dependency on wallclock time. The
token_bucket_rw timer already uses monotonic time. This patch converts
token_bucket_ctr to use monotonic time as well. It introduces a new
monotime_coarse_absolute_sec(), which is currently the same as nsec
divided by a billion but could be optimized easily if we ever need to.

This patch also might fix a rollover bug.. I haven't tested this
extensively but I don't think the previous version of the rollover code
on either token bucket was correct, and I would expect it to get stuck
after the first rollover.

Signed-off-by: Micah Elizabeth Scott <beth@torproject.org>

4 files changed, 9 insertions, 5 deletions

diff --git a/src/feature/hs/hs_circuit.c b/src/feature/hs/hs_circuit.c
index 4c27f417c5..4904f3ddf9 100644
--- a/src/feature/hs/hs_circuit.c
+++ b/src/feature/hs/hs_circuit.c
@@ -35,6 +35,7 @@
 #include "lib/crypt_ops/crypto_dh.h"
 #include "lib/crypt_ops/crypto_rand.h"
 #include "lib/crypt_ops/crypto_util.h"
+#include "lib/time/compat_time.h"
 
 /* Trunnel. */
 #include "trunnel/ed25519_cert.h"
@@ -794,7 +795,7 @@ handle_rend_pqueue_cb(mainloop_event_t *ev, void *arg)
 
     if (pow_state->using_pqueue_bucket) {
       token_bucket_ctr_refill(&pow_state->pqueue_bucket,
-                              (uint32_t) approx_time());
+                              (uint32_t) monotime_coarse_absolute_sec());
 
       if (token_bucket_ctr_get(&pow_state->pqueue_bucket) > 0) {
         token_bucket_ctr_dec(&pow_state->pqueue_bucket, 1);
diff --git a/src/feature/hs/hs_dos.c b/src/feature/hs/hs_dos.c
index 6323dbeeac..80ad3b1daa 100644
--- a/src/feature/hs/hs_dos.c
+++ b/src/feature/hs/hs_dos.c
@@ -28,6 +28,7 @@
 #include "feature/relay/routermode.h"
 
 #include "lib/evloop/token_bucket.h"
+#include "lib/time/compat_time.h"
 
 #include "feature/hs/hs_dos.h"
 
@@ -143,7 +144,7 @@ hs_dos_setup_default_intro2_defenses(or_circuit_t *circ)
   token_bucket_ctr_init(&circ->introduce2_bucket,
                         consensus_param_introduce_rate_per_sec,
                         consensus_param_introduce_burst_per_sec,
-                        (uint32_t) approx_time());
+                        (uint32_t) monotime_coarse_absolute_sec());
 }
 
 /** Called when the consensus has changed. We might have new consensus
@@ -188,7 +189,7 @@ hs_dos_can_send_intro2(or_circuit_t *s_intro_circ)
 
   /* Refill INTRODUCE2 bucket. */
   token_bucket_ctr_refill(&s_intro_circ->introduce2_bucket,
-                          (uint32_t) approx_time());
+                          (uint32_t) monotime_coarse_absolute_sec());
 
   /* Decrement the bucket for this valid INTRODUCE1 cell we just got. Don't
    * underflow else we end up with a too big of a bucket. */
diff --git a/src/feature/hs/hs_intropoint.c b/src/feature/hs/hs_intropoint.c
index 0a656b78dd..52bd0cd499 100644
--- a/src/feature/hs/hs_intropoint.c
+++ b/src/feature/hs/hs_intropoint.c
@@ -17,6 +17,7 @@
 #include "feature/rend/rendmid.h"
 #include "feature/stats/rephist.h"
 #include "lib/crypt_ops/crypto_format.h"
+#include "lib/time/compat_time.h"
 
 /* Trunnel */
 #include "trunnel/ed25519_cert.h"
@@ -316,7 +317,7 @@ handle_establish_intro_cell_dos_extension(
   token_bucket_ctr_init(&circ->introduce2_bucket,
                         (uint32_t) intro2_rate_per_sec,
                         (uint32_t) intro2_burst_per_sec,
-                        (uint32_t) approx_time());
+                        (uint32_t) monotime_coarse_absolute_sec());
   log_info(LD_REND, "Intro point DoS defenses enabled. Rate is %" PRIu64
                     " and Burst is %" PRIu64,
            intro2_rate_per_sec, intro2_burst_per_sec);
diff --git a/src/feature/hs/hs_service.c b/src/feature/hs/hs_service.c
index 3ef2a9120c..777cc85fea 100644
--- a/src/feature/hs/hs_service.c
+++ b/src/feature/hs/hs_service.c
@@ -34,6 +34,7 @@
 #include "lib/crypt_ops/crypto_rand.h"
 #include "lib/crypt_ops/crypto_util.h"
 #include "lib/time/tvdiff.h"
+#include "lib/time/compat_time.h"
 
 #include "feature/hs/hs_circuit.h"
 #include "feature/hs/hs_common.h"
@@ -290,7 +291,7 @@ initialize_pow_defenses(hs_service_t *service)
     token_bucket_ctr_init(&pow_state->pqueue_bucket,
                           service->config.pow_queue_rate,
                           service->config.pow_queue_burst,
-                          (uint32_t) approx_time());
+                          (uint32_t) monotime_coarse_absolute_sec());
 
     pow_state->pqueue_low_level = MAX(8, service->config.pow_queue_rate / 4);
     pow_state->pqueue_high_level =