commit f5c9f6d4327d7dcd36081426c0a862b6a06a2b61 Author: David Goulet <dgoulet@torproject.org> Date: Wed Aug 19 09:47:34 2020 -0400 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> --- changes/ticket40109 | 6 ++++++ src/core/or/or_circuit_st.h | 4 ++++ src/feature/hs/hs_dos.c | 5 +++++ src/feature/hs/hs_intropoint.c | 5 +++++ 4 files changed, 20 insertions(+) diff --git a/changes/ticket40109 b/changes/ticket40109 new file mode 100644 index 0000000000..d99db65aa4 --- /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 9bfe999728..4e17b1c143 100644 --- 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 1f7415a280..04c2bfbb89 100644 --- 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 e282d1f1bd..69d60f21c3 100644 --- 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) {