[tor-commits] [tor/master] hs: Fix the intro circuit max retry

[nickm at torproject.org]

commit 6507ecb7e8751b3b66886306bfae3ca846bd0f81
Author: David Goulet <dgoulet at torproject.org>
Date:   Mon Aug 28 14:31:32 2017 -0400

    hs: Fix the intro circuit max retry
    
    Some parentheses were missing making the rend_max_intro_circs_per_period()
    return a lower value than it was suppose to.
    
    The calculation is that a service at most will open a number of intro points
    that it wants which is 3 by default or HiddenServiceNumIntroductionPoints. Two
    extra are launched for performance reason. Finally, this can happen twice for
    two descriptors for the current and next time period.
    
    From:
      2 * n_intro_wanted + 2
    
    ...which resulted in 8 for 3 intro points, this commit fixes it to:
    
      (n_intro_wanted + 2) * 2
    
    ... resulting in 12 possible intro point circuit which is the correct maximum
    intro circuit allowed per period.
    
    Last, this commit rate limits the the log message if we ever go above that
    limit else over a INTRO_CIRC_RETRY_PERIOD, we can print it often!
    
    Fixes #22159
    
    Signed-off-by: David Goulet <dgoulet at torproject.org>
---
 changes/bug22159     |  7 +++++++
 src/or/rendservice.c | 34 +++++++++++++++++++++++-----------
 2 files changed, 30 insertions(+), 11 deletions(-)

diff --git a/changes/bug22159 b/changes/bug22159
new file mode 100644
index 000000000..c319c7e32
--- /dev/null
+++ b/changes/bug22159
@@ -0,0 +1,7 @@
+  o Minor bugfixes (hidden service):
+    - A service is allowed to open a maximum number of circuits for a specific
+      period of time. That value was lower than it should be (8 vs 12) in the
+      normal case of 3 introduction points. Fixes bug 22159.; bugfix on
+      tor-0.3.0.5-rc.
+    - Rate limit the log if we ever go above the maximum number of allowed
+      intro circuits. Fixes bug 22159.; bugfix on tor-0.3.1.1-alpha.
diff --git a/src/or/rendservice.c b/src/or/rendservice.c
index ede887945..2236fcca7 100644
--- a/src/or/rendservice.c
+++ b/src/or/rendservice.c
@@ -1077,16 +1077,23 @@ rend_log_intro_limit(const rend_service_t *service, int min_severity)
   }
   time_t intro_period_elapsed = time(NULL) - service->intro_period_started;
   tor_assert_nonfatal(intro_period_elapsed >= 0);
-  log_fn(severity, LD_REND, "Hidden service %s %s %d intro points in the last "
-         "%d seconds. Intro circuit launches are limited to %d per %d "
-         "seconds.",
-         service->service_id,
-         exceeded_limit ? "exceeded launch limit with" : "launched",
-         service->n_intro_circuits_launched,
-         (int)intro_period_elapsed,
-         rend_max_intro_circs_per_period(service->n_intro_points_wanted),
-         INTRO_CIRC_RETRY_PERIOD);
-  rend_service_dump_stats(severity);
+  {
+    char *msg;
+    static ratelim_t rlimit = RATELIM_INIT(INTRO_CIRC_RETRY_PERIOD);
+    if ((msg = rate_limit_log(&rlimit, approx_time()))) {
+      log_fn(severity, LD_REND,
+             "Hidden service %s %s %d intro points in the last %d seconds. "
+             "Intro circuit launches are limited to %d per %d seconds.%s",
+             service->service_id,
+             exceeded_limit ? "exceeded launch limit with" : "launched",
+             service->n_intro_circuits_launched,
+             (int)intro_period_elapsed,
+             rend_max_intro_circs_per_period(service->n_intro_points_wanted),
+             INTRO_CIRC_RETRY_PERIOD, msg);
+      rend_service_dump_stats(severity);
+      tor_free(msg);
+    }
+  }
 }
 
 /** Replace the old value of <b>service</b>-\>desc with one that reflects
@@ -4093,7 +4100,12 @@ rend_max_intro_circs_per_period(unsigned int n_intro_points_wanted)
   /* Allow all but one of the initial connections to fail and be
    * retried. (If all fail, we *want* to wait, because something is broken.) */
   tor_assert(n_intro_points_wanted <= NUM_INTRO_POINTS_MAX);
-  return (int)(2*n_intro_points_wanted + NUM_INTRO_POINTS_EXTRA);
+
+  /* For the normal use case, 3 intro points plus 2 extra for performance and
+   * allow that twice because once every 24h or so, we can do it twice for two
+   * descriptors that is the current one and the next one. So (3 + 2) * 2 ==
+   * 12 allowed attempts for one period. */
+  return ((n_intro_points_wanted + NUM_INTRO_POINTS_EXTRA) * 2);
 }
 
 /** For every service, check how many intro points it currently has, and: