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>
This commit is contained in:
David Goulet 2020-08-19 09:47:34 -04:00
parent 7915b651d9
commit f5c9f6d432
4 changed files with 20 additions and 0 deletions

6
changes/ticket40109 Normal file
View file

@ -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.

View file

@ -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

View file

@ -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;

View file

@ -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) {