hs: Don't overwrite DoS parameters on circuit with consensus params

Turns out that the HS DoS defenses parameters were overwritten by the
consensus parameters everytime a new consensus would arrive.

This means that a service operator can still enable the defenses but as soon
as the intro point relay would get a new consensus, they would be overwritten.
And at this commit, the network is entirely disabling DoS defenses.

Fix this by introducing an "explicit" flag that indicate if the
ESTABLISH_INTRO cell DoS extension set those parameters or not. If set, avoid
using the consenus at once.

We are not bumping the protover HSIntro value for this because 0.4.2.x series
is EOL in 1 month and thus 0.4.3.x would be the only series with this bug. We
are confident that a backport and then upgrade path to the latest 0.4.4.x
stable coming up soon is enough to mitigate this problem in the coming months.

It avoids the upgrade path on the service side by keeping the requirement for
protover HSIntro=5.

Fixes #40109

Signed-off-by: David Goulet <dgoulet@torproject.org>

diff --git a/changes/ticket40109 b/changes/ticket40109
new file mode 100644 (file)
index 0000000..d99db65
--- /dev/null
+++ b/changes/ticket40109
@@ -0,0 +1,6 @@
+  o Major bugfixes (onion services, DoS):
+    - The consensus parameters for the onion service DoS defenses was
+      overwriting the circuit parameters that could have been set by the service
+      operator using HiddenServiceEnableIntroDoSDefense. Fixes bug 40109; bugfix
+      on 0.4.2.1-alpha.
+

diff --git a/src/core/or/or_circuit_st.h b/src/core/or/or_circuit_st.h
index 9bfe999728ac2f40d586542846a14f03634d4713..4e17b1c14328993bb5fd3607bf2b8c8c55d06bee 100644 (file)
--- a/src/core/or/or_circuit_st.h
+++ b/src/core/or/or_circuit_st.h
@@ -75,6 +75,10 @@ struct or_circuit_t {
   /** If set, the DoS defenses are enabled on this circuit meaning that the
    * introduce2_bucket is initialized and used. */
   unsigned int introduce2_dos_defense_enabled : 1;
+  /** If set, the DoS defenses were explicitly enabled through the
+   * ESTABLISH_INTRO cell extension. If unset, the consensus is used to learn
+   * if the defenses can be enabled or not. */
+  unsigned int introduce2_dos_defense_explicit : 1;
 
   /** INTRODUCE2 cell bucket controlling how much can go on this circuit. Only
    * used if this is a service introduction circuit at the intro point

diff --git a/src/feature/hs/hs_dos.c b/src/feature/hs/hs_dos.c
index 1f7415a280c0c2027cb26d487adcf06d0055f2de..04c2bfbb89adbeaa3455977456eb2728f5060bc1 100644 (file)
--- a/src/feature/hs/hs_dos.c
+++ b/src/feature/hs/hs_dos.c
@@ -93,6 +93,11 @@ update_intro_circuits(void)
   smartlist_t *intro_circs = hs_circuitmap_get_all_intro_circ_relay_side();
 
   SMARTLIST_FOREACH_BEGIN(intro_circs, circuit_t *, circ) {
+    /* Ignore circuit if the defenses were set explicitly through the
+     * ESTABLISH_INTRO cell DoS extension. */
+    if (TO_OR_CIRCUIT(circ)->introduce2_dos_defense_explicit) {
+      continue;
+    }
     /* Defenses might have been enabled or disabled. */
     TO_OR_CIRCUIT(circ)->introduce2_dos_defense_enabled =
       consensus_param_introduce_defense_enabled;

diff --git a/src/feature/hs/hs_intropoint.c b/src/feature/hs/hs_intropoint.c
index e282d1f1bd42329eeca9f07df012c39f833c1770..69d60f21c344998cec4294c510181ffd4554e889 100644 (file)
--- a/src/feature/hs/hs_intropoint.c
+++ b/src/feature/hs/hs_intropoint.c
@@ -285,6 +285,11 @@ handle_establish_intro_cell_dos_extension(
     }
   }
 
+  /* At this point, the extension is valid so any values out of it implies
+   * that it was set explicitly and thus flag the circuit that it should not
+   * look at the consensus for that reason for the defenses' values. */
+  circ->introduce2_dos_defense_explicit = 1;
+
   /* A value of 0 is valid in the sense that we accept it but we still disable
    * the defenses so return false. */
   if (intro2_rate_per_sec == 0 || intro2_burst_per_sec == 0) {