commit 461d231289584110bde37ab498db3631fb6b0cf1 Author: David Goulet dgoulet@torproject.org Date: Tue Aug 20 09:38:13 2019 -0400
hs-v3: Refactor DoS cell extension parameters validation
Move everything to its own function in order to better log, document and tests the introduction point validation process.
Signed-off-by: David Goulet dgoulet@torproject.org --- src/feature/hs/hs_config.h | 3 +- src/feature/hs/hs_intropoint.c | 91 +++++++++++++++++++++++++++++++++--------- src/feature/hs/hs_intropoint.h | 3 ++ src/test/test_hs_dos.c | 43 ++++++++++++++++++++ 4 files changed, 120 insertions(+), 20 deletions(-)
diff --git a/src/feature/hs/hs_config.h b/src/feature/hs/hs_config.h index 249e19309..beefc7a61 100644 --- a/src/feature/hs/hs_config.h +++ b/src/feature/hs/hs_config.h @@ -15,7 +15,8 @@ #define HS_CONFIG_MAX_STREAMS_PER_RDV_CIRCUIT 65535 /* Maximum number of intro points per version 3 services. */ #define HS_CONFIG_V3_MAX_INTRO_POINTS 20 -/* Default value for the introduction DoS defenses. */ +/* Default value for the introduction DoS defenses. The MIN/MAX are inclusive + * meaning they can be used as valid values. */ #define HS_CONFIG_V3_DOS_DEFENSE_DEFAULT 0 #define HS_CONFIG_V3_DOS_DEFENSE_RATE_PER_SEC_DEFAULT 25 #define HS_CONFIG_V3_DOS_DEFENSE_RATE_PER_SEC_MIN 0 diff --git a/src/feature/hs/hs_intropoint.c b/src/feature/hs/hs_intropoint.c index fc7d96194..9b6a96628 100644 --- a/src/feature/hs/hs_intropoint.c +++ b/src/feature/hs/hs_intropoint.c @@ -182,6 +182,59 @@ hs_intro_send_intro_established_cell,(or_circuit_t *circ)) return ret; }
+/* Validate the cell DoS extension parameters. Return true iff they've been + * bound check and can be used. Else return false. See proposal 305 for + * details and reasons about this validation. */ +STATIC bool +validate_cell_dos_extension_parameters(uint64_t intro2_rate_per_sec, + uint64_t intro2_burst_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."); + goto end; + } + + /* 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); + goto end; + } + + /* 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); + goto end; + } + + /* In a rate limiting scenario, burst can never be smaller than the rate. At + * best it can be equal. */ + if (intro2_burst_per_sec < intro2_rate_per_sec) { + log_info(LD_REND, "Intro point DoS defenses burst is smaller than rate. " + "Rate: %" PRIu64 " vs Burst: %" PRIu64, + intro2_rate_per_sec, intro2_burst_per_sec); + goto end; + } + + /* Passing validation. */ + ret = true; + + end: + return ret; +} + +/* Parse the cell DoS extension and apply defenses on the given circuit if + * validation passes. If the cell extension is malformed or contains unusable + * values, the DoS defenses is disabled on the circuit. */ static void handle_establish_intro_cell_dos_extension( const trn_cell_extension_field_t *field, @@ -220,33 +273,33 @@ handle_establish_intro_cell_dos_extension( } }
- /* Validation. A value of 0 on either of them means the defenses are - * disabled so we ignore. */ - if ((intro2_rate_per_sec > HS_CONFIG_V3_DOS_DEFENSE_BURST_PER_SEC_MAX || - intro2_rate_per_sec <= HS_CONFIG_V3_DOS_DEFENSE_RATE_PER_SEC_MIN) || - (intro2_burst_per_sec > HS_CONFIG_V3_DOS_DEFENSE_RATE_PER_SEC_MAX || - intro2_burst_per_sec <= HS_CONFIG_V3_DOS_DEFENSE_BURST_PER_SEC_MIN) || - (intro2_burst_per_sec < intro2_rate_per_sec)) { + /* If invalid, we disable the defense on the circuit. */ + if (!validate_cell_dos_extension_parameters(intro2_rate_per_sec, + intro2_burst_per_sec)) { circ->introduce2_dos_defense_enabled = 0; - log_info(LD_REND, "Intro point DoS defenses disabled due to bad values"); - } else { - circ->introduce2_dos_defense_enabled = 1; - - /* Initialize the INTRODUCE2 token bucket for the rate limiting. */ - token_bucket_ctr_init(&circ->introduce2_bucket, - (uint32_t) intro2_rate_per_sec, - (uint32_t) intro2_burst_per_sec, - (uint32_t) approx_time()); - log_debug(LD_REND, "Intro point DoS defenses enabled. Rate is %" PRIu64 - " and Burst is %" PRIu64, intro2_rate_per_sec, - intro2_burst_per_sec); + log_info(LD_REND, "Disabling INTRO2 DoS defenses on circuit id %u", + circ->p_circ_id); + goto end; }
+ /* We passed validation, enable defenses and apply rate/burst. */ + circ->introduce2_dos_defense_enabled = 1; + + /* Initialize the INTRODUCE2 token bucket for the rate limiting. */ + token_bucket_ctr_init(&circ->introduce2_bucket, + (uint32_t) intro2_rate_per_sec, + (uint32_t) intro2_burst_per_sec, + (uint32_t) approx_time()); + log_info(LD_REND, "Intro point DoS defenses enabled. Rate is %" PRIu64 + " and Burst is %" PRIu64, + intro2_rate_per_sec, intro2_burst_per_sec); + end: trn_cell_extension_dos_free(dos); return; }
+/* Parse every cell extension in the given ESTABLISH_INTRO cell. */ static void handle_establish_intro_cell_extensions( const trn_cell_establish_intro_t *parsed_cell, diff --git a/src/feature/hs/hs_intropoint.h b/src/feature/hs/hs_intropoint.h index e82575f05..1bebcacd8 100644 --- a/src/feature/hs/hs_intropoint.h +++ b/src/feature/hs/hs_intropoint.h @@ -57,6 +57,9 @@ STATIC int handle_introduce1(or_circuit_t *client_circ, const uint8_t *request, size_t request_len); STATIC int validate_introduce1_parsed_cell(const trn_cell_introduce1_t *cell); STATIC int circuit_is_suitable_for_introduce1(const or_circuit_t *circ); +STATIC bool validate_cell_dos_extension_parameters( + uint64_t intro2_rate_per_sec, + uint64_t intro2_burst_per_sec);
#endif /* defined(HS_INTROPOINT_PRIVATE) */
diff --git a/src/test/test_hs_dos.c b/src/test/test_hs_dos.c index 370e12bf7..25a04d779 100644 --- a/src/test/test_hs_dos.c +++ b/src/test/test_hs_dos.c @@ -9,6 +9,7 @@ #define CIRCUITLIST_PRIVATE #define NETWORKSTATUS_PRIVATE #define HS_DOS_PRIVATE +#define HS_INTROPOINT_PRIVATE
#include "test/test.h" #include "test/test_helpers.h" @@ -21,6 +22,7 @@ #include "core/or/or_circuit_st.h"
#include "feature/hs/hs_dos.h" +#include "feature/hs/hs_intropoint.h" #include "feature/nodelist/networkstatus.h"
static void @@ -125,9 +127,50 @@ test_can_send_intro2(void *arg) free_mock_consensus(); }
+static void +test_validate_dos_extension_params(void *arg) +{ + bool ret; + + (void) arg; + + /* Validate the default values. */ + ret = validate_cell_dos_extension_parameters( + get_intro2_rate_consensus_param(NULL), + get_intro2_burst_consensus_param(NULL)); + tt_assert(ret); + + /* Valid custom rate/burst. */ + ret = validate_cell_dos_extension_parameters(17, 42); + tt_assert(ret); + + /* Invalid rate. */ + ret = validate_cell_dos_extension_parameters(UINT64_MAX, 42); + tt_assert(!ret); + + /* Invalid burst. */ + 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); + + /* Can't have burst smaller than rate. */ + ret = validate_cell_dos_extension_parameters(42, 40); + tt_assert(!ret); + + done: + return; +} + struct testcase_t hs_dos_tests[] = { { "can_send_intro2", test_can_send_intro2, TT_FORK, NULL, NULL }, + { "validate_dos_extension_params", test_validate_dos_extension_params, + TT_FORK, NULL, NULL },
END_OF_TESTCASES };