commit 385f6bcfccbc327f42e5139ac8136086e79fbb17 Author: David Goulet dgoulet@torproject.org Date: Tue Aug 20 10:50:31 2019 -0400
hs-v3: Move DoS parameter check against 0
Move it outside of the validation function since 0 is a valid value but disables defenses.
Signed-off-by: David Goulet dgoulet@torproject.org --- src/feature/hs/hs_intropoint.c | 54 +++++++++++++++++++++++++++++------------- src/test/test_hs_dos.c | 11 ++++----- 2 files changed, 43 insertions(+), 22 deletions(-)
diff --git a/src/feature/hs/hs_intropoint.c b/src/feature/hs/hs_intropoint.c index 9b6a96628..fb2ac52e5 100644 --- a/src/feature/hs/hs_intropoint.c +++ b/src/feature/hs/hs_intropoint.c @@ -191,28 +191,40 @@ validate_cell_dos_extension_parameters(uint64_t intro2_rate_per_sec, { bool ret = false;
- /* 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) { - log_info(LD_REND, "Intro point DoS defenses parameter set to 0."); + /* Check that received value is not below the minimum. Don't check if minimum + is set to 0, since the param is a positive value and gcc will complain. */ +#if HS_CONFIG_V3_DOS_DEFENSE_RATE_PER_SEC_MIN > 0 + if (intro2_rate_per_sec < HS_CONFIG_V3_DOS_DEFENSE_RATE_PER_SEC_MIN) { + log_fn(LOG_PROTOCOL_WARN, LD_REND, + "Intro point DoS defenses rate per second is " + "too small. Received value: %" PRIu64, intro2_rate_per_sec); goto end; } +#endif
- /* Bound check the received rate per second. MIN/MAX are inclusive. */ - if (!(intro2_rate_per_sec <= HS_CONFIG_V3_DOS_DEFENSE_RATE_PER_SEC_MAX && - intro2_rate_per_sec > HS_CONFIG_V3_DOS_DEFENSE_RATE_PER_SEC_MIN)) { - log_info(LD_REND, "Intro point DoS defenses rate per second is " - "invalid. Received value: %" PRIu64, - intro2_rate_per_sec); + /* Check that received value is not above maximum */ + if (intro2_rate_per_sec > HS_CONFIG_V3_DOS_DEFENSE_RATE_PER_SEC_MAX) { + log_fn(LOG_PROTOCOL_WARN, LD_REND, + "Intro point DoS defenses rate per second is " + "too big. Received value: %" PRIu64, intro2_rate_per_sec); + goto end; + } + + /* Check that received value is not below the minimum */ +#if HS_CONFIG_V3_DOS_DEFENSE_BURST_PER_SEC_MIN > 0 + if (intro2_burst_per_sec < HS_CONFIG_V3_DOS_DEFENSE_BURST_PER_SEC_MIN) { + log_fn(LOG_PROTOCOL_WARN, LD_REND, + "Intro point DoS defenses burst per second is " + "too small. Received value: %" PRIu64, intro2_burst_per_sec); goto end; } +#endif
- /* Bound check the received burst per second. MIN/MAX are inclusive. */ - if (!(intro2_burst_per_sec <= HS_CONFIG_V3_DOS_DEFENSE_BURST_PER_SEC_MAX && - intro2_burst_per_sec > HS_CONFIG_V3_DOS_DEFENSE_BURST_PER_SEC_MIN)) { - log_info(LD_REND, "Intro point DoS defenses burst per second is " - "invalid. Received value: %" PRIu64, - intro2_burst_per_sec); + /* Check that received value is not above maximum */ + if (intro2_burst_per_sec > HS_CONFIG_V3_DOS_DEFENSE_BURST_PER_SEC_MAX) { + log_fn(LOG_PROTOCOL_WARN, LD_REND, + "Intro point DoS defenses burst per second is " + "too big. Received value: %" PRIu64, intro2_burst_per_sec); goto end; }
@@ -273,6 +285,16 @@ handle_establish_intro_cell_dos_extension( } }
+ /* 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) { + log_info(LD_REND, "Intro point DoS defenses parameter set to 0. " + "Disabling INTRO2 DoS defenses on circuit id %u", + circ->p_circ_id); + circ->introduce2_dos_defense_enabled = 0; + goto end; + } + /* If invalid, we disable the defense on the circuit. */ if (!validate_cell_dos_extension_parameters(intro2_rate_per_sec, intro2_burst_per_sec)) { diff --git a/src/test/test_hs_dos.c b/src/test/test_hs_dos.c index 25a04d779..03c755acb 100644 --- a/src/test/test_hs_dos.c +++ b/src/test/test_hs_dos.c @@ -143,6 +143,8 @@ test_validate_dos_extension_params(void *arg) /* Valid custom rate/burst. */ ret = validate_cell_dos_extension_parameters(17, 42); tt_assert(ret); + ret = cell_dos_extension_parameters_are_valid(INT32_MAX, INT32_MAX); + tt_assert(ret);
/* Invalid rate. */ ret = validate_cell_dos_extension_parameters(UINT64_MAX, 42); @@ -152,11 +154,9 @@ test_validate_dos_extension_params(void *arg) ret = validate_cell_dos_extension_parameters(42, UINT64_MAX); tt_assert(!ret);
- /* Value of 0 should return invalid so defenses can be disabled. */ - ret = validate_cell_dos_extension_parameters(0, 42); - tt_assert(!ret); - ret = validate_cell_dos_extension_parameters(42, 0); - tt_assert(!ret); + /* Value of 0 is valid (but should disable defenses) */ + ret = cell_dos_extension_parameters_are_valid(0, 0); + tt_assert(ret);
/* Can't have burst smaller than rate. */ ret = validate_cell_dos_extension_parameters(42, 40); @@ -174,4 +174,3 @@ struct testcase_t hs_dos_tests[] = {
END_OF_TESTCASES }; -