Merge remote-tracking branch 'public/bug15745_027_03'

9 files changed, 81 insertions, 21 deletions

diff --git a/src/common/crypto.c b/src/common/crypto.c
index 1b45639a7c..e723f3d5d2 100644
--- a/src/common/crypto.c
+++ b/src/common/crypto.c
@@ -2317,6 +2317,25 @@ crypto_rand_int(unsigned int max)
   }
 }
 
+/** Return a pseudorandom integer, chosen uniformly from the values <i>i</i>
+ * such that <b>min</b> &lt;= <i>i</i> &lt <b>max</b>.
+ *
+ * <b>min</b> MUST be in range [0, <b>max</b>).
+ * <b>max</b> MUST be in range (min, INT_MAX].
+ */
+int
+crypto_rand_int_range(unsigned int min, unsigned int max)
+{
+  tor_assert(min < max);
+  tor_assert(max <= INT_MAX);
+
+  /* The overflow is avoided here because crypto_rand_int() returns a value
+   * between 0 and (max - min - 1) with max being <= INT_MAX and min <= max.
+   * This is why we add 1 to the maximum value so we can actually get max as
+   * a return value. */
+  return min + crypto_rand_int(max - min);
+}
+
 /** Return a pseudorandom 64-bit integer, chosen uniformly from the values
  * between 0 and <b>max</b>-1. */
 uint64_t
@@ -2379,7 +2398,7 @@ crypto_random_hostname(int min_rand_len, int max_rand_len, const char *prefix,
   if (min_rand_len > max_rand_len)
     min_rand_len = max_rand_len;
 
-  randlen = min_rand_len + crypto_rand_int(max_rand_len - min_rand_len + 1);
+  randlen = crypto_rand_int_range(min_rand_len, max_rand_len+1);
 
   prefixlen = strlen(prefix);
   resultlen = prefixlen + strlen(suffix) + randlen + 16;
diff --git a/src/common/crypto.h b/src/common/crypto.h
index aedc51b844..aa587fd08b 100644
--- a/src/common/crypto.h
+++ b/src/common/crypto.h
@@ -254,6 +254,7 @@ int crypto_seed_rng(int startup);
 MOCK_DECL(int,crypto_rand,(char *to, size_t n));
 int crypto_strongest_rand(uint8_t *out, size_t out_len);
 int crypto_rand_int(unsigned int max);
+int crypto_rand_int_range(unsigned int min, unsigned int max);
 uint64_t crypto_rand_uint64(uint64_t max);
 double crypto_rand_double(void);
 struct tor_weak_rng_t;
diff --git a/src/common/tortls.c b/src/common/tortls.c
index 32106eb2df..7809c1adaa 100644
--- a/src/common/tortls.c
+++ b/src/common/tortls.c
@@ -659,7 +659,8 @@ tor_tls_create_certificate(crypto_pk_t *rsa,
    * than having it start right now. Don't choose quite uniformly, since
    * then we might pick a time where we're about to expire. Lastly, be
    * sure to start on a day boundary. */
-  start_time = time(NULL) - crypto_rand_int(cert_lifetime) + 2*24*3600;
+  time_t now = time(NULL);
+  start_time = crypto_rand_int_range(now - cert_lifetime, now) + 2*24*3600;
   start_time -= start_time % (24*3600);
 
   tor_assert(rsa);
diff --git a/src/or/entrynodes.c b/src/or/entrynodes.c
index 9663f34002..9f07d5ad69 100644
--- a/src/or/entrynodes.c
+++ b/src/or/entrynodes.c
@@ -440,7 +440,8 @@ add_an_entry_guard(const node_t *chosen, int reset_status, int prepend,
    * don't all select them on the same day, and b) avoid leaving a
    * precise timestamp in the state file about when we first picked
    * this guard. For details, see the Jan 2010 or-dev thread. */
-  entry->chosen_on_date = time(NULL) - crypto_rand_int(3600*24*30);
+  time_t now = time(NULL);
+  entry->chosen_on_date = crypto_rand_int_range(now - 3600*24*30, now);
   entry->chosen_by_version = tor_strdup(VERSION);
 
   /* Are we picking this guard because all of our current guards are
@@ -1439,8 +1440,9 @@ entry_guards_parse_state(or_state_t *state, int set, char **msg)
        }
      } else {
        if (state_version) {
+         time_t now = time(NULL);
+         e->chosen_on_date = crypto_rand_int_range(now - 3600*24*30, now);
          e->chosen_by_version = tor_strdup(state_version);
-         e->chosen_on_date = time(NULL) - crypto_rand_int(3600*24*30);
        }
      }
      if (e->path_bias_disabled && !e->bad_since)
diff --git a/src/or/main.c b/src/or/main.c
index 39c0f5c89e..b9009db151 100644
--- a/src/or/main.c
+++ b/src/or/main.c
@@ -1623,7 +1623,7 @@ run_scheduled_events(time_t now)
       time_to.check_for_correct_dns < now &&
       ! router_my_exit_policy_is_reject_star()) {
     if (!time_to.check_for_correct_dns) {
-      time_to.check_for_correct_dns = now + 60 + crypto_rand_int(120);
+      time_to.check_for_correct_dns = crypto_rand_int_range(now, now + 120) + 60;
     } else {
       dns_launch_correctness_checks();
       time_to.check_for_correct_dns = now + 12*3600 +
diff --git a/src/or/or.h b/src/or/or.h
index d548aeabb6..4fd6d1d9f6 100644
--- a/src/or/or.h
+++ b/src/or/or.h
@@ -4805,12 +4805,13 @@ typedef struct rend_encoded_v2_service_descriptor_t {
  * introduction point.  See also rend_intro_point_t.unreachable_count. */
 #define MAX_INTRO_POINT_REACHABILITY_FAILURES 5
 
-/** The maximum number of distinct INTRODUCE2 cells which a hidden
- * service's introduction point will receive before it begins to
- * expire.
- *
- * XXX023 Is this number at all sane? */
-#define INTRO_POINT_LIFETIME_INTRODUCTIONS 16384
+/** The minimum and maximum number of distinct INTRODUCE2 cells which a
+ * hidden service's introduction point will receive before it begins to
+ * expire. */
+#define INTRO_POINT_MIN_LIFETIME_INTRODUCTIONS 16384
+/* Double the minimum value so the interval is [min, min * 2]. */
+#define INTRO_POINT_MAX_LIFETIME_INTRODUCTIONS \
+  (INTRO_POINT_MIN_LIFETIME_INTRODUCTIONS * 2)
 
 /** The minimum number of seconds that an introduction point will last
  * before expiring due to old age.  (If it receives
@@ -4864,6 +4865,12 @@ typedef struct rend_intro_point_t {
    */
   int accepted_introduce2_count;
 
+  /** (Service side only) Number of maximum INTRODUCE2 cells that this IP
+   * will accept. This is a random value between
+   * INTRO_POINT_MIN_LIFETIME_INTRODUCTIONS and
+   * INTRO_POINT_MAX_LIFETIME_INTRODUCTIONS. */
+  unsigned int max_introductions;
+
   /** (Service side only) The time at which this intro point was first
    * published, or -1 if this intro point has not yet been
    * published. */
diff --git a/src/or/rendservice.c b/src/or/rendservice.c
index 0a41ed2db2..41bbd69c93 100644
--- a/src/or/rendservice.c
+++ b/src/or/rendservice.c
@@ -1156,16 +1156,17 @@ rend_service_note_removing_intro_point(rend_service_t *service,
     /* This intro point was never used.  Don't change
      * n_intro_points_wanted. */
   } else {
+
     /* We want to increase the number of introduction points service
      * operates if intro was heavily used, or decrease the number of
      * intro points if intro was lightly used.
      *
      * We consider an intro point's target 'usage' to be
-     * INTRO_POINT_LIFETIME_INTRODUCTIONS introductions in
+     * maximum of INTRODUCE2 cells divided by
      * INTRO_POINT_LIFETIME_MIN_SECONDS seconds.  To calculate intro's
-     * fraction of target usage, we divide the fraction of
-     * _LIFETIME_INTRODUCTIONS introductions that it has handled by
-     * the fraction of _LIFETIME_MIN_SECONDS for which it existed.
+     * fraction of target usage, we divide the amount of INTRODUCE2 cells
+     * that it has handled by the fraction of _LIFETIME_MIN_SECONDS for
+     * which it existed.
      *
      * Then we multiply that fraction of desired usage by a fudge
      * factor of 1.5, to decide how many new introduction points
@@ -1187,7 +1188,7 @@ rend_service_note_removing_intro_point(rend_service_t *service,
       intro_point_accepted_intro_count(intro) /
       (double)(now - intro->time_published);
     const double intro_point_target_usage =
-      INTRO_POINT_LIFETIME_INTRODUCTIONS /
+      intro->max_introductions /
       (double)INTRO_POINT_LIFETIME_MIN_SECONDS;
     const double fractional_n_intro_points_wanted_to_replace_this_one =
       (1.5 * (intro_point_usage / intro_point_target_usage));
@@ -3121,7 +3122,7 @@ intro_point_should_expire_now(rend_intro_point_t *intro,
   }
 
   if (intro_point_accepted_intro_count(intro) >=
-      INTRO_POINT_LIFETIME_INTRODUCTIONS) {
+      intro->max_introductions) {
     /* This intro point has been used too many times.  Expire it now. */
     return 1;
   }
@@ -3130,9 +3131,8 @@ intro_point_should_expire_now(rend_intro_point_t *intro,
     /* This intro point has been published, but we haven't picked an
      * expiration time for it.  Pick one now. */
     int intro_point_lifetime_seconds =
-      INTRO_POINT_LIFETIME_MIN_SECONDS +
-      crypto_rand_int(INTRO_POINT_LIFETIME_MAX_SECONDS -
-                      INTRO_POINT_LIFETIME_MIN_SECONDS);
+      crypto_rand_int_range(INTRO_POINT_LIFETIME_MIN_SECONDS,
+                            INTRO_POINT_LIFETIME_MAX_SECONDS);
 
     /* Start the expiration timer now, rather than when the intro
      * point was first published.  There shouldn't be much of a time
@@ -3333,6 +3333,9 @@ rend_services_introduce(void)
       intro->time_published = -1;
       intro->time_to_expire = -1;
       intro->time_expiring = -1;
+      intro->max_introductions =
+        crypto_rand_int_range(INTRO_POINT_MIN_LIFETIME_INTRODUCTIONS,
+                              INTRO_POINT_MAX_LIFETIME_INTRODUCTIONS);
       smartlist_add(service->intro_nodes, intro);
       log_info(LD_REND, "Picked router %s as an intro point for %s.",
                safe_str_client(node_describe(node)),
diff --git a/src/or/router.c b/src/or/router.c
index 253f251cb5..24b7c750f6 100644
--- a/src/or/router.c
+++ b/src/or/router.c
@@ -683,7 +683,9 @@ router_initialize_tls_context(void)
   if (!lifetime) { /* we should guess a good ssl cert lifetime */
 
     /* choose between 5 and 365 days, and round to the day */
-    lifetime = 5*24*3600 + crypto_rand_int(361*24*3600);
+    unsigned int five_days = 5*24*3600;
+    unsigned int one_year = 365*24*3600;
+    lifetime = crypto_rand_int_range(five_days, one_year);
     lifetime -= lifetime % (24*3600);
 
     if (crypto_rand_int(2)) {
diff --git a/src/test/test_crypto.c b/src/test/test_crypto.c
index 7f4aa13ac2..bcd7069b9d 100644
--- a/src/test/test_crypto.c
+++ b/src/test/test_crypto.c
@@ -107,6 +107,30 @@ test_crypto_rng(void *arg)
   ;
 }
 
+static void
+test_crypto_rng_range(void *arg)
+{
+  int got_smallest = 0, got_largest = 0;
+  int i;
+
+  (void)arg;
+  for (i = 0; i < 1000; ++i) {
+    int x = crypto_rand_int_range(5,9);
+    tt_int_op(x, OP_GE, 5);
+    tt_int_op(x, OP_LT, 9);
+    if (x == 5)
+      got_smallest = 1;
+    if (x == 8)
+      got_largest = 1;
+  }
+
+  /* These fail with probability 1/10^603. */
+  tt_assert(got_smallest);
+  tt_assert(got_largest);
+ done:
+  ;
+}
+
 /** Run unit tests for our AES functionality */
 static void
 test_crypto_aes(void *arg)
@@ -1637,6 +1661,7 @@ test_crypto_siphash(void *arg)
 struct testcase_t crypto_tests[] = {
   CRYPTO_LEGACY(formats),
   CRYPTO_LEGACY(rng),
+  { "rng_range", test_crypto_rng_range, 0, NULL, NULL },
   { "aes_AES", test_crypto_aes, TT_FORK, &passthrough_setup, (void*)"aes" },
   { "aes_EVP", test_crypto_aes, TT_FORK, &passthrough_setup, (void*)"evp" },
   CRYPTO_LEGACY(sha),