February 2018 - tor-commits - lists.torproject.org

[tor/maint-0.2.9] test: Add unit tests for overflows and underflows in cc_stats_refill_bucket
by nickm＠torproject.org 16 Feb '18

16 Feb '18

commit 1f4a73133cf864774c017e2c50b347727519c18f Author: teor <teor2345(a)gmail.com> Date: Wed Jan 31 11:22:20 2018 +1100 test: Add unit tests for overflows and underflows in cc_stats_refill_bucket Closes #25094. Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- src/test/test_dos.c | 146 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 146 insertions(+) diff --git a/src/test/test_dos.c b/src/test/test_dos.c index 7fe549560..071926c28 100644 --- a/src/test/test_dos.c +++ b/src/test/test_dos.c @@ -234,6 +234,152 @@ test_dos_bucket_refill(void *arg) current_circ_count += max_circuit_count; tt_uint_op(dos_stats->cc_stats.circuit_bucket, OP_EQ, current_circ_count); + /* Now send as many CREATE cells as needed to deplete our token bucket + * completely */ + for (; current_circ_count != 0; current_circ_count--) { + dos_cc_new_create_cell(chan); + } + tt_uint_op(current_circ_count, OP_EQ, 0); + tt_uint_op(dos_stats->cc_stats.circuit_bucket, OP_EQ, current_circ_count); + + /* Now use a very large time, and check that the token bucket does not have + * more than max_circs allowance, even tho we let it simmer for so long. */ + now = INT32_MAX; /* 2038? */ + update_approx_time(now); + cc_stats_refill_bucket(&dos_stats->cc_stats, addr); + current_circ_count += max_circuit_count; + tt_uint_op(dos_stats->cc_stats.circuit_bucket, OP_EQ, current_circ_count); + + /* Now send as many CREATE cells as needed to deplete our token bucket + * completely */ + for (; current_circ_count != 0; current_circ_count--) { + dos_cc_new_create_cell(chan); + } + tt_uint_op(current_circ_count, OP_EQ, 0); + tt_uint_op(dos_stats->cc_stats.circuit_bucket, OP_EQ, current_circ_count); + + /* Now use a very small time, and check that the token bucket has exactly + * the max_circs allowance, because backward clock jumps are rare. */ + now = INT32_MIN; /* 19?? */ + update_approx_time(now); + cc_stats_refill_bucket(&dos_stats->cc_stats, addr); + current_circ_count += max_circuit_count; + tt_uint_op(dos_stats->cc_stats.circuit_bucket, OP_EQ, current_circ_count); + + /* Now send as many CREATE cells as needed to deplete our token bucket + * completely */ + for (; current_circ_count != 0; current_circ_count--) { + dos_cc_new_create_cell(chan); + } + tt_uint_op(current_circ_count, OP_EQ, 0); + tt_uint_op(dos_stats->cc_stats.circuit_bucket, OP_EQ, current_circ_count); + + /* Progress time forward one sec again, refill the bucket and check that the + * refill happened correctly. */ + now += 1; + update_approx_time(now); + cc_stats_refill_bucket(&dos_stats->cc_stats, addr); + /* check refill */ + current_circ_count += circ_rate; + tt_uint_op(dos_stats->cc_stats.circuit_bucket, OP_EQ, current_circ_count); + + /* Now send as many CREATE cells as needed to deplete our token bucket + * completely */ + for (; current_circ_count != 0; current_circ_count--) { + dos_cc_new_create_cell(chan); + } + tt_uint_op(current_circ_count, OP_EQ, 0); + tt_uint_op(dos_stats->cc_stats.circuit_bucket, OP_EQ, current_circ_count); + + /* Now use a very large time (again), and check that the token bucket does + * not have more than max_circs allowance, even tho we let it simmer for so + * long. */ + now = INT32_MAX; /* 2038? */ + update_approx_time(now); + cc_stats_refill_bucket(&dos_stats->cc_stats, addr); + current_circ_count += max_circuit_count; + tt_uint_op(dos_stats->cc_stats.circuit_bucket, OP_EQ, current_circ_count); + + /* Now send as many CREATE cells as needed to deplete our token bucket + * completely */ + for (; current_circ_count != 0; current_circ_count--) { + dos_cc_new_create_cell(chan); + } + tt_uint_op(current_circ_count, OP_EQ, 0); + tt_uint_op(dos_stats->cc_stats.circuit_bucket, OP_EQ, current_circ_count); + + /* This code resets the time to zero with 32-bit time_t, which triggers the + * code that initialises the bucket. */ +#if SIZEOF_TIME_T == 8 + /* Now use a very very small time, and check that the token bucket has + * exactly the max_circs allowance, because backward clock jumps are rare. + */ + now = (time_t)INT64_MIN; /* ???? */ + update_approx_time(now); + cc_stats_refill_bucket(&dos_stats->cc_stats, addr); + current_circ_count += max_circuit_count; + tt_uint_op(dos_stats->cc_stats.circuit_bucket, OP_EQ, current_circ_count); + + /* Now send as many CREATE cells as needed to deplete our token bucket + * completely */ + for (; current_circ_count != 0; current_circ_count--) { + dos_cc_new_create_cell(chan); + } + tt_uint_op(current_circ_count, OP_EQ, 0); + tt_uint_op(dos_stats->cc_stats.circuit_bucket, OP_EQ, current_circ_count); + + /* Progress time forward one sec again, refill the bucket and check that the + * refill happened correctly. */ + now += 1; + update_approx_time(now); + cc_stats_refill_bucket(&dos_stats->cc_stats, addr); + /* check refill */ + current_circ_count += circ_rate; + tt_uint_op(dos_stats->cc_stats.circuit_bucket, OP_EQ, current_circ_count); + + /* Now send as many CREATE cells as needed to deplete our token bucket + * completely */ + for (; current_circ_count != 0; current_circ_count--) { + dos_cc_new_create_cell(chan); + } + tt_uint_op(current_circ_count, OP_EQ, 0); + tt_uint_op(dos_stats->cc_stats.circuit_bucket, OP_EQ, current_circ_count); + + /* Now use a very very small time, and check that the token bucket has + * exactly the max_circs allowance, because backward clock jumps are rare. + */ + now = (time_t)INT64_MIN; /* ???? */ + update_approx_time(now); + cc_stats_refill_bucket(&dos_stats->cc_stats, addr); + current_circ_count += max_circuit_count; + tt_uint_op(dos_stats->cc_stats.circuit_bucket, OP_EQ, current_circ_count); + + /* Now send as many CREATE cells as needed to deplete our token bucket + * completely */ + for (; current_circ_count != 0; current_circ_count--) { + dos_cc_new_create_cell(chan); + } + tt_uint_op(current_circ_count, OP_EQ, 0); + tt_uint_op(dos_stats->cc_stats.circuit_bucket, OP_EQ, current_circ_count); + + /* Now use a very very large time, and check that the token bucket does not + * have more than max_circs allowance, even tho we let it simmer for so + * long. */ + now = (time_t)INT64_MAX; /* ???? */ + update_approx_time(now); + cc_stats_refill_bucket(&dos_stats->cc_stats, addr); + current_circ_count += max_circuit_count; + tt_uint_op(dos_stats->cc_stats.circuit_bucket, OP_EQ, current_circ_count); + + /* Now send as many CREATE cells as needed to deplete our token bucket + * completely */ + for (; current_circ_count != 0; current_circ_count--) { + dos_cc_new_create_cell(chan); + } + tt_uint_op(current_circ_count, OP_EQ, 0); + tt_uint_op(dos_stats->cc_stats.circuit_bucket, OP_EQ, current_circ_count); +#endif + done: tor_free(chan); dos_free_all();

1 0

[tor/maint-0.2.9] dos: Make circuit rate limit per second, not tenths anymore
by nickm＠torproject.org 16 Feb '18

16 Feb '18

commit e58a4fc6cfcdeafc2ebfb61fd3cf6d163ce2436c Author: David Goulet <dgoulet(a)torproject.org> Date: Mon Jan 29 11:50:11 2018 -0500 dos: Make circuit rate limit per second, not tenths anymore Because this touches too many commits at once, it is made into one single commit. Remove the use of "tenths" for the circuit rate to simplify things. We can only refill the buckets at best once every second because of the use of approx_time() and our token system is set to be 1 token = 1 circuit so make the rate a flat integer of circuit per second. Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- doc/tor.1.txt | 8 +++----- src/or/config.c | 2 +- src/or/dos.c | 32 ++++++++------------------------ src/or/dos.h | 2 +- src/or/or.h | 5 ++--- 5 files changed, 15 insertions(+), 34 deletions(-) diff --git a/doc/tor.1.txt b/doc/tor.1.txt index a2bbb8ab6..58997cdf3 100644 --- a/doc/tor.1.txt +++ b/doc/tor.1.txt @@ -2466,12 +2466,10 @@ Denial of Service mitigation subsystem. parameter. (Default: 0) -[[DoSCircuitCreationRateTenths]] **DoSCircuitCreationRateTenths** __NUM__:: +[[DoSCircuitCreationRate]] **DoSCircuitCreationRate** __NUM__:: - The allowed circuit creation rate in tenths of circuit per second applied - per client IP address. For example, if you want to set a rate of 5 - circuits per second allowed per IP address, this value should be set to - 50. If this option is 0, it obeys a consensus parameter. (Default: 0) + The allowed circuit creation rate per second applied per client IP + address. If this option is 0, it obeys a consensus parameter. (Default: 0) [[DoSCircuitCreationBurst]] **DoSCircuitCreationBurst** __NUM__:: diff --git a/src/or/config.c b/src/or/config.c index c651c202e..3b4027433 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -245,7 +245,7 @@ static config_var_t option_vars_[] = { /* DoS circuit creation options. */ V(DoSCircuitCreationEnabled, AUTOBOOL, "auto"), V(DoSCircuitCreationMinConnections, UINT, "0"), - V(DoSCircuitCreationRateTenths, UINT, "0"), + V(DoSCircuitCreationRate, UINT, "0"), V(DoSCircuitCreationBurst, UINT, "0"), V(DoSCircuitCreationDefenseType, INT, "0"), V(DoSCircuitCreationDefenseTimePeriod, INTERVAL, "0"), diff --git a/src/or/dos.c b/src/or/dos.c index 5af75ca57..a614d1231 100644 --- a/src/or/dos.c +++ b/src/or/dos.c @@ -31,7 +31,7 @@ static unsigned int dos_cc_enabled = 0; /* Consensus parameters. They can be changed when a new consensus arrives. * They are initialized with the hardcoded default values. */ static uint32_t dos_cc_min_concurrent_conn; -static uint32_t dos_cc_circuit_rate_tenths; +static uint32_t dos_cc_circuit_rate; static uint32_t dos_cc_circuit_burst; static dos_cc_defense_type_t dos_cc_defense_type; static int32_t dos_cc_defense_time_period; @@ -93,14 +93,14 @@ get_param_cc_min_concurrent_connection(const networkstatus_t *ns) /* Return the parameter for the time rate that is how many circuits over this * time span. */ static uint32_t -get_param_cc_circuit_rate_tenths(const networkstatus_t *ns) +get_param_cc_circuit_rate(const networkstatus_t *ns) { /* This is in seconds. */ - if (get_options()->DoSCircuitCreationRateTenths) { - return get_options()->DoSCircuitCreationRateTenths; + if (get_options()->DoSCircuitCreationRate) { + return get_options()->DoSCircuitCreationRate; } - return networkstatus_get_param(ns, "DoSCircuitCreationRateTenths", - DOS_CC_CIRCUIT_RATE_TENTHS_DEFAULT, + return networkstatus_get_param(ns, "DoSCircuitCreationRate", + DOS_CC_CIRCUIT_RATE_DEFAULT, 1, INT32_MAX); } @@ -189,7 +189,7 @@ set_dos_parameters(const networkstatus_t *ns) /* Get the default consensus param values. */ dos_cc_enabled = get_param_cc_enabled(ns); dos_cc_min_concurrent_conn = get_param_cc_min_concurrent_connection(ns); - dos_cc_circuit_rate_tenths = get_param_cc_circuit_rate_tenths(ns); + dos_cc_circuit_rate = get_param_cc_circuit_rate(ns); dos_cc_circuit_burst = get_param_cc_circuit_burst(ns); dos_cc_defense_time_period = get_param_cc_defense_time_period(ns); dos_cc_defense_type = get_param_cc_defense_type(ns); @@ -225,23 +225,7 @@ cc_consensus_has_changed(const networkstatus_t *ns) STATIC uint32_t get_circuit_rate_per_second(void) { - int64_t circ_rate; - - /* We take the burst divided by the rate which is in tenths of a second so - * convert to get a circuit rate per second. */ - circ_rate = dos_cc_circuit_rate_tenths / 10; - if (circ_rate < 0) { - /* Safety check, never allow it to go below 0 else the bucket will always - * be empty resulting in every address to be detected. */ - circ_rate = 1; - } - - /* Clamp it down to a 32 bit value because a rate of 2^32 circuits per - * second is just too much in any circumstances. */ - if (circ_rate > UINT32_MAX) { - circ_rate = UINT32_MAX; - } - return (uint32_t) circ_rate; + return dos_cc_circuit_rate; } /* Given the circuit creation client statistics object, refill the circuit diff --git a/src/or/dos.h b/src/or/dos.h index 9ce1baddb..8695512ea 100644 --- a/src/or/dos.h +++ b/src/or/dos.h @@ -70,7 +70,7 @@ void dos_note_refuse_single_hop_client(void); /* DoSCircuitCreationMinConnections default */ #define DOS_CC_MIN_CONCURRENT_CONN_DEFAULT 3 /* DoSCircuitCreationRateTenths is 3 per seconds. */ -#define DOS_CC_CIRCUIT_RATE_TENTHS_DEFAULT (3 * 10) +#define DOS_CC_CIRCUIT_RATE_DEFAULT 3 /* DoSCircuitCreationBurst default. */ #define DOS_CC_CIRCUIT_BURST_DEFAULT 90 /* DoSCircuitCreationDefenseTimePeriod in seconds. */ diff --git a/src/or/or.h b/src/or/or.h index 454d05ed5..024a9cff0 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -4520,9 +4520,8 @@ typedef struct { /** Minimum concurrent connection needed from one single address before any * defense is used. */ int DoSCircuitCreationMinConnections; - /** Circuit rate, in tenths of a second, that is used to refill the token - * bucket at this given rate. */ - int DoSCircuitCreationRateTenths; + /** Circuit rate used to refill the token bucket. */ + int DoSCircuitCreationRate; /** Maximum allowed burst of circuits. Reaching that value, the address is * detected as malicious and a defense might be used. */ int DoSCircuitCreationBurst;

1 0

[tor/maint-0.2.9] dos: Add the DoSRefuseSingleHopClientRendezvous option
by nickm＠torproject.org 16 Feb '18

16 Feb '18

commit 36a0ae151f8f85c76b4bd91a8fc2871dd88b6005 Author: David Goulet <dgoulet(a)torproject.org> Date: Thu Jan 25 16:32:28 2018 -0500 dos: Add the DoSRefuseSingleHopClientRendezvous option This option refuses any ESTABLISH_RENDEZVOUS cell arriving from a client connection. Its default value is "auto" for which we can turn it on or off with a consensus parameter. Default value is 0. Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- src/or/dos.c | 31 +++++++++++++++++++++++++++++++ src/or/dos.h | 3 +++ src/or/rendmid.c | 12 ++++++++++++ 3 files changed, 46 insertions(+) diff --git a/src/or/dos.c b/src/or/dos.c index 7e3a2ab7f..d98d3db16 100644 --- a/src/or/dos.c +++ b/src/or/dos.c @@ -14,6 +14,7 @@ #include "geoip.h" #include "main.h" #include "networkstatus.h" +#include "router.h" #include "dos.h" @@ -60,6 +61,9 @@ static uint64_t conn_num_addr_rejected; * General interface of the denial of service mitigation subsystem. */ +/* Keep stats for the heartbeat. */ +static uint64_t num_single_hop_client_refused; + /* Return true iff the circuit creation mitigation is enabled. We look at the * consensus for this else a default value is returned. */ MOCK_IMPL(STATIC unsigned int, @@ -524,6 +528,33 @@ dos_conn_addr_get_defense_type(const tor_addr_t *addr) /* General API */ +/* Note down that we've just refused a single hop client. This increments a + * counter later used for the heartbeat. */ +void +dos_note_refuse_single_hop_client(void) +{ + num_single_hop_client_refused++; +} + +/* Return true iff single hop client connection (ESTABLISH_RENDEZVOUS) should + * be refused. */ +int +dos_should_refuse_single_hop_client(void) +{ + /* If we aren't a public relay, this shouldn't apply to anything. */ + if (!public_server_mode(get_options())) { + return 0; + } + + if (get_options()->DoSRefuseSingleHopClientRendezvous != -1) { + return get_options()->DoSRefuseSingleHopClientRendezvous; + } + + return (int) networkstatus_get_param(NULL, + "DoSRefuseSingleHopClientRendezvous", + 0 /* default */, 0, 1); +} + /* Called when a new client connection has been established on the given * address. */ void diff --git a/src/or/dos.h b/src/or/dos.h index cc7749836..ec4c033ae 100644 --- a/src/or/dos.h +++ b/src/or/dos.h @@ -51,6 +51,9 @@ int dos_enabled(void); void dos_new_client_conn(or_connection_t *or_conn); void dos_close_client_conn(const or_connection_t *or_conn); +int dos_should_refuse_single_hop_client(void); +void dos_note_refuse_single_hop_client(void); + /* * Circuit creation DoS mitigation subsystemn interface. */ diff --git a/src/or/rendmid.c b/src/or/rendmid.c index ca0ad7b0d..441d5043c 100644 --- a/src/or/rendmid.c +++ b/src/or/rendmid.c @@ -8,9 +8,11 @@ **/ #include "or.h" +#include "channel.h" #include "circuitlist.h" #include "circuituse.h" #include "config.h" +#include "dos.h" #include "relay.h" #include "rendmid.h" #include "rephist.h" @@ -246,6 +248,16 @@ rend_mid_establish_rendezvous(or_circuit_t *circ, const uint8_t *request, goto err; } + /* Check if we are configured to accept established rendezvous cells from + * client or in other words tor2web clients. */ + if (channel_is_client(circ->p_chan) && + dos_should_refuse_single_hop_client()) { + /* Note it down for the heartbeat log purposes. */ + dos_note_refuse_single_hop_client(); + /* Silent drop so the client has to time out before moving on. */ + return 0; + } + if (circ->base_.n_chan) { log_warn(LD_PROTOCOL, "Tried to establish rendezvous on non-edge circuit");

1 0

[tor/maint-0.2.9] dos: Add a heartbeat log
by nickm＠torproject.org 16 Feb '18

16 Feb '18

commit 14a8b87852887f8c20a424ff32a2b6746105dd6c Author: David Goulet <dgoulet(a)torproject.org> Date: Thu Jan 25 16:36:05 2018 -0500 dos: Add a heartbeat log Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- src/or/dos.c | 45 +++++++++++++++++++++++++++++++++++++++++++++ src/or/dos.h | 1 + src/or/status.c | 2 ++ 3 files changed, 48 insertions(+) diff --git a/src/or/dos.c b/src/or/dos.c index d98d3db16..40e88aead 100644 --- a/src/or/dos.c +++ b/src/or/dos.c @@ -555,6 +555,51 @@ dos_should_refuse_single_hop_client(void) 0 /* default */, 0, 1); } +/* Log a heartbeat message with some statistics. */ +void +dos_log_heartbeat(void) +{ + char *conn_msg = NULL; + char *cc_msg = NULL; + char *single_hop_client_msg = NULL; + + if (!dos_is_enabled()) { + goto end; + } + + if (dos_cc_enabled) { + tor_asprintf(&cc_msg, + " %" PRIu64 " circuits rejected," + " %" PRIu32 " marked addresses.", + cc_num_rejected_cells, cc_num_marked_addrs); + } + + if (dos_conn_enabled) { + tor_asprintf(&conn_msg, + " %" PRIu64 " connections closed.", + conn_num_addr_rejected); + } + + if (dos_should_refuse_single_hop_client()) { + tor_asprintf(&single_hop_client_msg, + " %" PRIu64 " single hop clients refused.", + num_single_hop_client_refused); + } + + log_notice(LD_HEARTBEAT, + "DoS mitigation since startup:%s%s%s", + (cc_msg != NULL) ? cc_msg : " [cc not enabled]", + (conn_msg != NULL) ? conn_msg : " [conn not enabled]", + (single_hop_client_msg != NULL) ? single_hop_client_msg : ""); + + tor_free(conn_msg); + tor_free(cc_msg); + tor_free(single_hop_client_msg); + + end: + return; +} + /* Called when a new client connection has been established on the given * address. */ void diff --git a/src/or/dos.h b/src/or/dos.h index ec4c033ae..56835169d 100644 --- a/src/or/dos.h +++ b/src/or/dos.h @@ -47,6 +47,7 @@ void dos_init(void); void dos_free_all(void); void dos_consensus_has_changed(const networkstatus_t *ns); int dos_enabled(void); +void dos_log_heartbeat(void); void dos_new_client_conn(or_connection_t *or_conn); void dos_close_client_conn(const or_connection_t *or_conn); diff --git a/src/or/status.c b/src/or/status.c index fce6a1015..fa2238b9f 100644 --- a/src/or/status.c +++ b/src/or/status.c @@ -27,6 +27,7 @@ #include "hibernate.h" #include "rephist.h" #include "statefile.h" +#include "dos.h" static void log_accounting(const time_t now, const or_options_t *options); #include "geoip.h" @@ -145,6 +146,7 @@ log_heartbeat(time_t now) if (public_server_mode(options)) { rep_hist_log_circuit_handshake_stats(now); rep_hist_log_link_protocol_counts(); + dos_log_heartbeat(); } circuit_log_ancient_one_hop_circuits(1800);

1 0

[tor/maint-0.2.9] dos: Clear connection tracked flag if geoip entry is removed
by nickm＠torproject.org 16 Feb '18

16 Feb '18

commit 82de4ea900c5d3513214b127421890595343bfaa Author: David Goulet <dgoulet(a)torproject.org> Date: Thu Jan 25 09:44:21 2018 -0500 dos: Clear connection tracked flag if geoip entry is removed Imagine this scenario. We had 10 connections over the 24h lifetime of a geoip cache entry. The lifetime of the entry has been reached so it is about to get freed but 2 connections remain for it. After the free, a third connection comes in thus making us create a new geoip entry for that address matching the 2 previous ones that are still alive. If they end up being closed, we'll have a concurrent count desynch from what the reality is. To mitigate this probably very rare scenario in practice, when we free a geoip entry and it has a concurrent count above 0, we'll go over all connections matching the address and clear out the tracked flag. So once they are closed, we don't try to decrement the count. Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- src/or/dos.c | 35 +++++++++++++++++++++++++++++++++++ src/or/dos.h | 4 ++++ src/or/geoip.c | 4 ++++ 3 files changed, 43 insertions(+) diff --git a/src/or/dos.c b/src/or/dos.c index 40e88aead..5af75ca57 100644 --- a/src/or/dos.c +++ b/src/or/dos.c @@ -528,6 +528,41 @@ dos_conn_addr_get_defense_type(const tor_addr_t *addr) /* General API */ +/* Take any appropriate actions for the given geoip entry that is about to get + * freed. This is called for every entry that is being freed. + * + * This function will clear out the connection tracked flag if the concurrent + * count of the entry is above 0 so if those connections end up being seen by + * this subsystem, we won't try to decrement the counter for a new geoip entry + * that might have been added after this call for the same address. */ +void +dos_geoip_entry_about_to_free(const clientmap_entry_t *geoip_ent) +{ + tor_assert(geoip_ent); + + /* The count is down to 0 meaning no connections right now, we can safely + * clear the geoip entry from the cache. */ + if (geoip_ent->dos_stats.concurrent_count == 0) { + goto end; + } + + /* For each connection matching the geoip entry address, we'll clear the + * tracked flag because the entry is about to get removed from the geoip + * cache. We do not try to decrement if the flag is not set. */ + SMARTLIST_FOREACH_BEGIN(get_connection_array(), connection_t *, conn) { + if (conn->type == CONN_TYPE_OR) { + or_connection_t *or_conn = TO_OR_CONN(conn); + if (!tor_addr_compare(&geoip_ent->addr, &or_conn->real_addr, + CMP_EXACT)) { + or_conn->tracked_for_dos_mitigation = 0; + } + } + } SMARTLIST_FOREACH_END(conn); + + end: + return; +} + /* Note down that we've just refused a single hop client. This increments a * counter later used for the heartbeat. */ void diff --git a/src/or/dos.h b/src/or/dos.h index 56835169d..9ce1baddb 100644 --- a/src/or/dos.h +++ b/src/or/dos.h @@ -43,11 +43,15 @@ typedef struct dos_client_stats_t { /* General API. */ +/* Stub. */ +struct clientmap_entry_t; + void dos_init(void); void dos_free_all(void); void dos_consensus_has_changed(const networkstatus_t *ns); int dos_enabled(void); void dos_log_heartbeat(void); +void dos_geoip_entry_about_to_free(const struct clientmap_entry_t *geoip_ent); void dos_new_client_conn(or_connection_t *or_conn); void dos_close_client_conn(const or_connection_t *or_conn); diff --git a/src/or/geoip.c b/src/or/geoip.c index 5f0b04b56..4e4f6e639 100644 --- a/src/or/geoip.c +++ b/src/or/geoip.c @@ -516,6 +516,10 @@ clientmap_entry_free(clientmap_entry_t *ent) if (!ent) return; + /* This entry is about to be freed so pass it to the DoS subsystem to see if + * any actions can be taken about it. */ + dos_geoip_entry_about_to_free(ent); + tor_free(ent->transport_name); tor_free(ent); }

1 0

[tor/maint-0.2.9] test: Remove a redundant round from test_dos_bucket_refill
by nickm＠torproject.org 16 Feb '18

16 Feb '18

commit b45ae1b00237f0209c4ccce777de59581fda5e39 Author: teor <teor2345(a)gmail.com> Date: Wed Jan 31 11:11:08 2018 +1100 test: Remove a redundant round from test_dos_bucket_refill This round is left over from the tenths of a second code. Part of #25094. --- src/test/test_dos.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/test_dos.c b/src/test/test_dos.c index 5a8474ad8..80abc1937 100644 --- a/src/test/test_dos.c +++ b/src/test/test_dos.c @@ -176,7 +176,7 @@ test_dos_bucket_refill(void *arg) /* Initialize DoS subsystem and get relevant limits */ dos_init(); uint32_t max_circuit_count = get_param_cc_circuit_burst(NULL); - int circ_rate = tor_lround(get_circuit_rate_per_second()); + int circ_rate = get_circuit_rate_per_second(); /* Check that the circuit rate is a positive number and smaller than the max * circuit count */ tt_int_op(circ_rate, OP_GT, 1);

1 0

[tor/maint-0.2.9] test: Add unit tests for the DoS subsystem
by nickm＠torproject.org 16 Feb '18

16 Feb '18

commit c3c2b55decc80028728780422fe2766ec6517246 Author: George Kadianakis <desnacked(a)riseup.net> Date: Thu Jan 25 16:38:59 2018 -0500 test: Add unit tests for the DoS subsystem Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- src/or/channel.c | 4 +- src/or/channel.h | 3 +- src/test/include.am | 1 + src/test/test.c | 1 + src/test/test.h | 1 + src/test/test_dos.c | 248 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 255 insertions(+), 3 deletions(-) diff --git a/src/or/channel.c b/src/or/channel.c index fdd3f81e8..54e10666d 100644 --- a/src/or/channel.c +++ b/src/or/channel.c @@ -3845,8 +3845,8 @@ channel_get_canonical_remote_descr(channel_t *chan) * supports this operation, and return 1. Return 0 if the underlying transport * doesn't let us do this. */ -int -channel_get_addr_if_possible(channel_t *chan, tor_addr_t *addr_out) +MOCK_IMPL(int, +channel_get_addr_if_possible,(channel_t *chan, tor_addr_t *addr_out)) { tor_assert(chan); tor_assert(addr_out); diff --git a/src/or/channel.h b/src/or/channel.h index a711b56d4..bcd345e8d 100644 --- a/src/or/channel.h +++ b/src/or/channel.h @@ -550,7 +550,8 @@ MOCK_DECL(void, channel_dump_statistics, (channel_t *chan, int severity)); void channel_dump_transport_statistics(channel_t *chan, int severity); const char * channel_get_actual_remote_descr(channel_t *chan); const char * channel_get_actual_remote_address(channel_t *chan); -int channel_get_addr_if_possible(channel_t *chan, tor_addr_t *addr_out); +MOCK_DECL(int, channel_get_addr_if_possible, (channel_t *chan, + tor_addr_t *addr_out)); const char * channel_get_canonical_remote_descr(channel_t *chan); int channel_has_queued_writes(channel_t *chan); int channel_is_bad_for_new_circs(channel_t *chan); diff --git a/src/test/include.am b/src/test/include.am index 8ecfaf10c..91b0a5910 100644 --- a/src/test/include.am +++ b/src/test/include.am @@ -87,6 +87,7 @@ src_test_test_SOURCES = \ src/test/test_controller.c \ src/test/test_controller_events.c \ src/test/test_crypto.c \ + src/test/test_dos.c \ src/test/test_data.c \ src/test/test_dir.c \ src/test/test_dir_common.c \ diff --git a/src/test/test.c b/src/test/test.c index 9a41b976b..f66dee2d0 100644 --- a/src/test/test.c +++ b/src/test/test.c @@ -1197,6 +1197,7 @@ struct testgroup_t testgroups[] = { { "control/", controller_tests }, { "control/event/", controller_event_tests }, { "crypto/", crypto_tests }, + { "dos/", dos_tests }, { "dir/", dir_tests }, { "dir_handle_get/", dir_handle_get_tests }, { "dir/md/", microdesc_tests }, diff --git a/src/test/test.h b/src/test/test.h index 25336ac83..41df6b134 100644 --- a/src/test/test.h +++ b/src/test/test.h @@ -190,6 +190,7 @@ extern struct testcase_t container_tests[]; extern struct testcase_t controller_tests[]; extern struct testcase_t controller_event_tests[]; extern struct testcase_t crypto_tests[]; +extern struct testcase_t dos_tests[]; extern struct testcase_t dir_tests[]; extern struct testcase_t dir_handle_get_tests[]; extern struct testcase_t entryconn_tests[]; diff --git a/src/test/test_dos.c b/src/test/test_dos.c new file mode 100644 index 000000000..5a8474ad8 --- /dev/null +++ b/src/test/test_dos.c @@ -0,0 +1,248 @@ +/* Copyright (c) 2018, The Tor Project, Inc. */ +/* See LICENSE for licensing information */ + +#define DOS_PRIVATE +#define TOR_CHANNEL_INTERNAL_ +#define CIRCUITLIST_PRIVATE + +#include "or.h" +#include "dos.h" +#include "circuitlist.h" +#include "geoip.h" +#include "channel.h" +#include "test.h" +#include "log_test_helpers.h" + +static unsigned int +mock_enable_dos_protection(const networkstatus_t *ns) +{ + (void) ns; + return 1; +} + +/** Test that the connection tracker of the DoS subsystem will block clients + * who try to establish too many connections */ +static void +test_dos_conn_creation(void *arg) +{ + (void) arg; + + MOCK(get_param_cc_enabled, mock_enable_dos_protection); + MOCK(get_param_conn_enabled, mock_enable_dos_protection); + + /* Initialize test data */ + or_connection_t or_conn; + time_t now = 1281533250; /* 2010-08-11 13:27:30 UTC */ + tt_int_op(AF_INET,OP_EQ, tor_addr_parse(&or_conn.real_addr, + "18.0.0.1")); + tor_addr_t *addr = &or_conn.real_addr; + + /* Get DoS subsystem limits */ + dos_init(); + uint32_t max_concurrent_conns = get_param_conn_max_concurrent_count(NULL); + + /* Introduce new client */ + geoip_note_client_seen(GEOIP_CLIENT_CONNECT, addr, NULL, now); + { /* Register many conns from this client but not enough to get it blocked */ + unsigned int i; + for (i = 0; i < max_concurrent_conns; i++) { + dos_new_client_conn(&or_conn); + } + } + + /* Check that new conns are still permitted */ + tt_int_op(DOS_CONN_DEFENSE_NONE, OP_EQ, + dos_conn_addr_get_defense_type(addr)); + + /* Register another conn and check that new conns are not allowed anymore */ + dos_new_client_conn(&or_conn); + tt_int_op(DOS_CONN_DEFENSE_CLOSE, OP_EQ, + dos_conn_addr_get_defense_type(addr)); + + /* Close a client conn and see that a new conn will be permitted again */ + dos_close_client_conn(&or_conn); + tt_int_op(DOS_CONN_DEFENSE_NONE, OP_EQ, + dos_conn_addr_get_defense_type(addr)); + + /* Register another conn and see that defense measures get reactivated */ + dos_new_client_conn(&or_conn); + tt_int_op(DOS_CONN_DEFENSE_CLOSE, OP_EQ, + dos_conn_addr_get_defense_type(addr)); + + done: + dos_free_all(); +} + +/** Helper mock: Place a fake IP addr for this channel in addr_out */ +static int +mock_channel_get_addr_if_possible(channel_t *chan, tor_addr_t *addr_out) +{ + (void)chan; + tt_int_op(AF_INET,OP_EQ, tor_addr_parse(addr_out, "18.0.0.1"));; + return 1; + + done: + return 0; +} + +/** Test that the circuit tracker of the DoS subsystem will block clients who + * try to establish too many circuits. */ +static void +test_dos_circuit_creation(void *arg) +{ + (void) arg; + unsigned int i; + + MOCK(get_param_cc_enabled, mock_enable_dos_protection); + MOCK(get_param_conn_enabled, mock_enable_dos_protection); + MOCK(channel_get_addr_if_possible, + mock_channel_get_addr_if_possible); + + /* Initialize channels/conns/circs that will be used */ + channel_t *chan = tor_malloc_zero(sizeof(channel_t)); + channel_init(chan); + chan->is_client = 1; + + /* Initialize test data */ + or_connection_t or_conn; + time_t now = 1281533250; /* 2010-08-11 13:27:30 UTC */ + tt_int_op(AF_INET,OP_EQ, tor_addr_parse(&or_conn.real_addr, + "18.0.0.1")); + tor_addr_t *addr = &or_conn.real_addr; + + /* Get DoS subsystem limits */ + dos_init(); + uint32_t max_circuit_count = get_param_cc_circuit_burst(NULL); + uint32_t min_conc_conns_for_cc = + get_param_cc_min_concurrent_connection(NULL); + + /* Introduce new client and establish enough connections to activate the + * circuit counting subsystem */ + geoip_note_client_seen(GEOIP_CLIENT_CONNECT, addr, NULL, now); + for (i = 0; i < min_conc_conns_for_cc ; i++) { + dos_new_client_conn(&or_conn); + } + + /* Register new circuits for this client and conn, but not enough to get + * detected as dos */ + for (i=0; i < max_circuit_count-1; i++) { + dos_cc_new_create_cell(chan); + } + /* see that we didn't get detected for dosing */ + tt_int_op(DOS_CC_DEFENSE_NONE, OP_EQ, dos_cc_get_defense_type(chan)); + + /* Register another CREATE cell that will push us over the limit. Check that + * the cell gets refused. */ + dos_cc_new_create_cell(chan); + tt_int_op(DOS_CC_DEFENSE_REFUSE_CELL, OP_EQ, dos_cc_get_defense_type(chan)); + + /* TODO: Wait a few seconds before sending the cell, and check that the + buckets got refilled properly. */ + /* TODO: Actually send a Tor cell (instead of calling the DoS function) and + * check that it will get refused */ + + done: + tor_free(chan); + dos_free_all(); +} + +/** Test that the DoS subsystem properly refills the circuit token buckets. */ +static void +test_dos_bucket_refill(void *arg) +{ + (void) arg; + int i; + /* For this test, this variable is set to the current circ count of the token + * bucket. */ + uint32_t current_circ_count; + + MOCK(get_param_cc_enabled, mock_enable_dos_protection); + MOCK(get_param_conn_enabled, mock_enable_dos_protection); + MOCK(channel_get_addr_if_possible, + mock_channel_get_addr_if_possible); + + time_t now = 1281533250; /* 2010-08-11 13:27:30 UTC */ + update_approx_time(now); + + /* Initialize channels/conns/circs that will be used */ + channel_t *chan = tor_malloc_zero(sizeof(channel_t)); + channel_init(chan); + chan->is_client = 1; + or_connection_t or_conn; + tt_int_op(AF_INET,OP_EQ, tor_addr_parse(&or_conn.real_addr, + "18.0.0.1")); + tor_addr_t *addr = &or_conn.real_addr; + + /* Initialize DoS subsystem and get relevant limits */ + dos_init(); + uint32_t max_circuit_count = get_param_cc_circuit_burst(NULL); + int circ_rate = tor_lround(get_circuit_rate_per_second()); + /* Check that the circuit rate is a positive number and smaller than the max + * circuit count */ + tt_int_op(circ_rate, OP_GT, 1); + tt_int_op(circ_rate, OP_LT, max_circuit_count); + + /* Register this client */ + geoip_note_client_seen(GEOIP_CLIENT_CONNECT, addr, NULL, now); + dos_new_client_conn(&or_conn); + + /* Fetch this client from the geoip cache and get its DoS structs */ + clientmap_entry_t *entry = geoip_lookup_client(addr, NULL, + GEOIP_CLIENT_CONNECT); + tt_assert(entry); + dos_client_stats_t* dos_stats = &entry->dos_stats; + /* Check that the circuit bucket is still uninitialized */ + tt_uint_op(dos_stats->cc_stats.circuit_bucket, OP_EQ, 0); + + /* Send a create cell: then check that the circ token bucket got initialized + * and one circ was subtracted. */ + dos_cc_new_create_cell(chan); + current_circ_count = max_circuit_count - 1; + tt_uint_op(dos_stats->cc_stats.circuit_bucket, OP_EQ, current_circ_count); + + /* Now send 29 more CREATEs and ensure that the bucket is missing 30 + * tokens */ + for (i=0; i < 29; i++) { + dos_cc_new_create_cell(chan); + current_circ_count--; + } + tt_uint_op(dos_stats->cc_stats.circuit_bucket, OP_EQ, current_circ_count); + + /* OK! Progress time forward one sec, refill the bucket and check that the + * refill happened correctly. */ + now += 1; + update_approx_time(now); + cc_stats_refill_bucket(&dos_stats->cc_stats, addr); + /* check refill */ + current_circ_count += circ_rate; + tt_uint_op(dos_stats->cc_stats.circuit_bucket, OP_EQ, current_circ_count); + + /* Now send as many CREATE cells as needed to deplete our token bucket + * completely */ + for (; current_circ_count != 0; current_circ_count--) { + dos_cc_new_create_cell(chan); + } + tt_uint_op(current_circ_count, OP_EQ, 0); + tt_uint_op(dos_stats->cc_stats.circuit_bucket, OP_EQ, current_circ_count); + + /* Now progress time a week forward, and check that the token bucket does not + * have more than max_circs allowance, even tho we let it simmer for so + * long. */ + now += 604800; /* a week */ + update_approx_time(now); + cc_stats_refill_bucket(&dos_stats->cc_stats, addr); + current_circ_count += max_circuit_count; + tt_uint_op(dos_stats->cc_stats.circuit_bucket, OP_EQ, current_circ_count); + + done: + tor_free(chan); + dos_free_all(); +} + +struct testcase_t dos_tests[] = { + { "conn_creation", test_dos_conn_creation, TT_FORK, NULL, NULL }, + { "circuit_creation", test_dos_circuit_creation, TT_FORK, NULL, NULL }, + { "bucket_refill", test_dos_bucket_refill, TT_FORK, NULL, NULL }, + END_OF_TESTCASES +}; +

1 0

[tor/maint-0.2.9] dos: Make sure cc_stats_refill_bucket can't overflow while calculating
by nickm＠torproject.org 16 Feb '18

16 Feb '18

commit a09d5f5735abe2e1d16cf0ee9389ae096d5e7ef1 Author: teor <teor2345(a)gmail.com> Date: Wed Jan 31 11:13:17 2018 +1100 dos: Make sure cc_stats_refill_bucket can't overflow while calculating Debug log the elapsed time in cc_stats_refill_bucket Part of #25094. Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- src/or/dos.c | 82 +++++++++++++++++++++++++++++++++++++---------------- src/or/dos.h | 2 +- src/test/test_dos.c | 2 +- 3 files changed, 59 insertions(+), 27 deletions(-) diff --git a/src/or/dos.c b/src/or/dos.c index a614d1231..c221e5ecd 100644 --- a/src/or/dos.c +++ b/src/or/dos.c @@ -222,7 +222,7 @@ cc_consensus_has_changed(const networkstatus_t *ns) /** Return the number of circuits we allow per second under the current * configuration. */ -STATIC uint32_t +STATIC uint64_t get_circuit_rate_per_second(void) { return dos_cc_circuit_rate; @@ -234,31 +234,40 @@ get_circuit_rate_per_second(void) STATIC void cc_stats_refill_bucket(cc_client_stats_t *stats, const tor_addr_t *addr) { - uint32_t new_circuit_bucket_count, circuit_rate = 0, num_token; - time_t now, elapsed_time_last_refill; + uint32_t new_circuit_bucket_count; + uint64_t num_token, elapsed_time_last_refill = 0, circuit_rate = 0; + time_t now; + int64_t last_refill_ts; tor_assert(stats); tor_assert(addr); now = approx_time(); + last_refill_ts = (int64_t)stats->last_circ_bucket_refill_ts; - /* We've never filled the bucket so fill it with the maximum being the burst - * and we are done. */ - if (stats->last_circ_bucket_refill_ts == 0) { - num_token = dos_cc_circuit_burst; - goto end; + /* If less than a second has elapsed, don't add any tokens. + * Note: If a relay's clock is ever 0, any new clients won't get a refill + * until the next second. But a relay that thinks it is 1970 will never + * validate the public consensus. */ + if ((int64_t)now == last_refill_ts) { + goto done; } /* At this point, we know we might need to add token to the bucket. We'll - * first compute the circuit rate that is how many circuit are we allowed to - * do per second. */ + * first get the circuit rate that is how many circuit are we allowed to do + * per second. */ circuit_rate = get_circuit_rate_per_second(); - /* How many seconds have elapsed between now and the last refill? */ - elapsed_time_last_refill = now - stats->last_circ_bucket_refill_ts; + /* We've never filled the bucket so fill it with the maximum being the burst + * and we are done. + * Note: If a relay's clock is ever 0, all clients that were last refilled + * in that zero second will get a full refill here. */ + if (last_refill_ts == 0) { + num_token = dos_cc_circuit_burst; + goto end; + } - /* If the elapsed time is below 0 it means our clock jumped backward so in - * that case, lets be safe and fill it up to the maximum. Not filling it + /* Our clock jumped backward so fill it up to the maximum. Not filling it * could trigger a detection for a valid client. Also, if the clock jumped * negative but we didn't notice until the elapsed time became positive * again, then we potentially spent many seconds not refilling the bucket @@ -266,28 +275,51 @@ cc_stats_refill_bucket(cc_client_stats_t *stats, const tor_addr_t *addr) * until now means that no circuit creation requests came in during that * time, so the client doesn't end up punished that much from this hopefully * rare situation.*/ - if (elapsed_time_last_refill < 0) { - /* Dividing the burst by the circuit rate gives us the time span that will - * give us the maximum allowed value of token. */ - elapsed_time_last_refill = (dos_cc_circuit_burst / circuit_rate); + if ((int64_t)now < last_refill_ts) { + /* Use the maximum allowed value of token. */ + num_token = dos_cc_circuit_burst; + goto end; + } + + /* How many seconds have elapsed between now and the last refill? + * This subtraction can't underflow, because now >= last_refill_ts. + * And it can't overflow, because INT64_MAX - (-INT64_MIN) == UINT64_MAX. */ + elapsed_time_last_refill = (uint64_t)now - last_refill_ts; + + /* If the elapsed time is very large, it means our clock jumped forward. + * If the multiplication would overflow, use the maximum allowed value. */ + if (elapsed_time_last_refill > UINT32_MAX) { + num_token = dos_cc_circuit_burst; + goto end; } /* Compute how many circuits we are allowed in that time frame which we'll - * add to the bucket. This can be big but it is cap to a maximum after. */ + * add to the bucket. This can't overflow, because both multiplicands + * are less than or equal to UINT32_MAX, and num_token is uint64_t. */ num_token = elapsed_time_last_refill * circuit_rate; end: - /* We cap the bucket to the burst value else this could grow to infinity - * over time. */ - new_circuit_bucket_count = MIN(stats->circuit_bucket + num_token, - dos_cc_circuit_burst); + /* If the sum would overflow, use the maximum allowed value. */ + if (num_token > UINT32_MAX - stats->circuit_bucket) { + new_circuit_bucket_count = dos_cc_circuit_burst; + } else { + /* We cap the bucket to the burst value else this could overflow uint32_t + * over time. */ + new_circuit_bucket_count = MIN(stats->circuit_bucket + (uint32_t)num_token, + dos_cc_circuit_burst); + } + /* This function is not allowed to make the bucket count smaller */ + tor_assert_nonfatal(new_circuit_bucket_count >= stats->circuit_bucket); log_debug(LD_DOS, "DoS address %s has its circuit bucket value: %" PRIu32 - ". Filling it to %" PRIu32 ". Circuit rate is %" PRIu32, + ". Filling it to %" PRIu32 ". Circuit rate is %" PRIu64 + ". Elapsed time is %" PRIi64, fmt_addr(addr), stats->circuit_bucket, new_circuit_bucket_count, - circuit_rate); + circuit_rate, (int64_t)elapsed_time_last_refill); stats->circuit_bucket = new_circuit_bucket_count; stats->last_circ_bucket_refill_ts = now; + + done: return; } diff --git a/src/or/dos.h b/src/or/dos.h index 8695512ea..5d35a2b12 100644 --- a/src/or/dos.h +++ b/src/or/dos.h @@ -125,7 +125,7 @@ STATIC uint32_t get_param_cc_circuit_burst(const networkstatus_t *ns); STATIC uint32_t get_param_cc_min_concurrent_connection( const networkstatus_t *ns); -STATIC uint32_t get_circuit_rate_per_second(void); +STATIC uint64_t get_circuit_rate_per_second(void); STATIC void cc_stats_refill_bucket(cc_client_stats_t *stats, const tor_addr_t *addr); diff --git a/src/test/test_dos.c b/src/test/test_dos.c index 80abc1937..7fe549560 100644 --- a/src/test/test_dos.c +++ b/src/test/test_dos.c @@ -176,7 +176,7 @@ test_dos_bucket_refill(void *arg) /* Initialize DoS subsystem and get relevant limits */ dos_init(); uint32_t max_circuit_count = get_param_cc_circuit_burst(NULL); - int circ_rate = get_circuit_rate_per_second(); + uint64_t circ_rate = get_circuit_rate_per_second(); /* Check that the circuit rate is a positive number and smaller than the max * circuit count */ tt_int_op(circ_rate, OP_GT, 1);

1 0

[tor/maint-0.2.9] dos: Add changes file for ticket 24902
by nickm＠torproject.org 16 Feb '18

16 Feb '18

commit 9aca7d47306222f2870ec16a7291a8215d6c3316 Author: David Goulet <dgoulet(a)torproject.org> Date: Tue Jan 30 09:15:33 2018 -0500 dos: Add changes file for ticket 24902 Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- changes/ticket24902 | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/changes/ticket24902 b/changes/ticket24902 new file mode 100644 index 000000000..1a2ef95cc --- /dev/null +++ b/changes/ticket24902 @@ -0,0 +1,13 @@ + o Major features (denial of service mitigation): + - Give relays some defenses against the recent network overload. We start + with three defenses (default parameters in parentheses). First: if a + single client address makes too many concurrent connections (>100), hang + up on further connections. Second: if a single client address makes + circuits too quickly (more than 3 per second, with an allowed burst of + 90) while also having too many connections open (3), refuse new create + cells for the next while (1-2 hours). Third: if a client asks to + establish a rendezvous point to you directly, ignore the request. These + defenses can be manually controlled by new torrc options, but relays + will also take guidance from consensus parameters, so there's no need to + configure anything manually. Implements ticket 24902. +

1 0

[tor/maint-0.2.9] geoip: Hook the client history cache into the OOM handler
by nickm＠torproject.org 16 Feb '18

16 Feb '18

commit 51839f47650463f59bd2cc84da05d5bc535d699d Author: David Goulet <dgoulet(a)torproject.org> Date: Fri Feb 2 10:15:28 2018 -0500 geoip: Hook the client history cache into the OOM handler If the cache is using 20% of our maximum allowed memory, clean 10% of it. Same behavior as the HS descriptor cache. Closes #25122 Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- changes/ticket25122 | 4 +++ src/or/geoip.c | 91 +++++++++++++++++++++++++++++++++++++++++++++++++++++ src/or/geoip.h | 2 ++ src/or/relay.c | 16 ++++++++-- src/test/test.c | 18 +++++++++++ 5 files changed, 128 insertions(+), 3 deletions(-) diff --git a/changes/ticket25122 b/changes/ticket25122 new file mode 100644 index 000000000..2921811b2 --- /dev/null +++ b/changes/ticket25122 @@ -0,0 +1,4 @@ + o Minor feature (geoip cache): + - Make our OOM handler aware of the geoip client history cache so it + doesn't fill up the memory which is especially important for IPv6 and + our DoS mitigation subsystem. Closes ticket 25122. diff --git a/src/or/geoip.c b/src/or/geoip.c index 00c055bbe..c5e8cdab9 100644 --- a/src/or/geoip.c +++ b/src/or/geoip.c @@ -72,6 +72,10 @@ static smartlist_t *geoip_ipv4_entries = NULL, *geoip_ipv6_entries = NULL; static char geoip_digest[DIGEST_LEN]; static char geoip6_digest[DIGEST_LEN]; +/* Total size in bytes of the geoip client history cache. Used by the OOM + * handler. */ +static size_t geoip_client_history_cache_size; + /** Return the index of the country's entry in the GeoIP * country list if it is a valid 2-letter country code, otherwise * return -1. */ @@ -526,6 +530,15 @@ HT_PROTOTYPE(clientmap, clientmap_entry_t, node, clientmap_entry_hash, HT_GENERATE2(clientmap, clientmap_entry_t, node, clientmap_entry_hash, clientmap_entries_eq, 0.6, tor_reallocarray_, tor_free_) +/** Return the size of a client map entry. */ +static inline size_t +clientmap_entry_size(const clientmap_entry_t *ent) +{ + tor_assert(ent); + return (sizeof(clientmap_entry_t) + + (ent->transport_name ? strlen(ent->transport_name) : 0)); +} + /** Free all storage held by ent. */ static void clientmap_entry_free(clientmap_entry_t *ent) @@ -533,6 +546,8 @@ clientmap_entry_free(clientmap_entry_t *ent) if (!ent) return; + geoip_client_history_cache_size -= clientmap_entry_size(ent); + tor_free(ent->transport_name); tor_free(ent); } @@ -595,6 +610,7 @@ geoip_note_client_seen(geoip_client_action_t action, ent->transport_name = tor_strdup(transport_name); ent->action = (int)action; HT_INSERT(clientmap, &client_history, ent); + geoip_client_history_cache_size += clientmap_entry_size(ent); } if (now / 60 <= (int)MAX_LAST_SEEN_IN_MINUTES && now >= 0) ent->last_seen_in_minutes = (unsigned)(now/60); @@ -635,6 +651,81 @@ geoip_remove_old_clients(time_t cutoff) &cutoff); } +/* Cleanup client entries older than the cutoff. Used for the OOM. Return the + * number of bytes freed. If 0 is returned, nothing was freed. */ +static size_t +oom_clean_client_entries(time_t cutoff) +{ + size_t bytes = 0; + clientmap_entry_t **ent, **ent_next; + + for (ent = HT_START(clientmap, &client_history); ent; ent = ent_next) { + clientmap_entry_t *entry = *ent; + if (entry->last_seen_in_minutes < (cutoff / 60)) { + ent_next = HT_NEXT_RMV(clientmap, &client_history, ent); + bytes += clientmap_entry_size(entry); + clientmap_entry_free(entry); + } else { + ent_next = HT_NEXT(clientmap, &client_history, ent); + } + } + return bytes; +} + +/* Below this minimum lifetime, the OOM won't cleanup any entries. */ +#define GEOIP_CLIENT_CACHE_OOM_MIN_CUTOFF (4 * 60 * 60) +/* The OOM moves the cutoff by that much every run. */ +#define GEOIP_CLIENT_CACHE_OOM_STEP (15 * 50) + +/* Cleanup the geoip client history cache called from the OOM handler. Return + * the amount of bytes removed. This can return a value below or above + * min_remove_bytes but will stop as oon as the min_remove_bytes has been + * reached. */ +size_t +geoip_client_cache_handle_oom(time_t now, size_t min_remove_bytes) +{ + time_t k; + size_t bytes_removed = 0; + + /* Our OOM handler called with 0 bytes to remove is a code flow error. */ + tor_assert(min_remove_bytes != 0); + + /* Set k to the initial cutoff of an entry. We then going to move it by step + * to try to remove as much as we can. */ + k = WRITE_STATS_INTERVAL; + + do { + time_t cutoff; + + /* If k has reached the minimum lifetime, we have to stop else we might + * remove every single entries which would be pretty bad for the DoS + * mitigation subsystem if by just filling the geoip cache, it was enough + * to trigger the OOM and clean every single entries. */ + if (k <= GEOIP_CLIENT_CACHE_OOM_MIN_CUTOFF) { + break; + } + + cutoff = now - k; + bytes_removed += oom_clean_client_entries(cutoff); + k -= GEOIP_CLIENT_CACHE_OOM_STEP; + } while (bytes_removed < min_remove_bytes); + + return bytes_removed; +} + +/* Return the total size in bytes of the client history cache. */ +size_t +geoip_client_cache_total_allocation(void) +{ + size_t bytes = 0; + clientmap_entry_t **ent; + + HT_FOREACH(ent, clientmap, &client_history) { + bytes += clientmap_entry_size(*ent); + } + return bytes; +} + /** How many responses are we giving to clients requesting v3 network * statuses? */ static uint32_t ns_v3_responses[GEOIP_NS_RESPONSE_NUM]; diff --git a/src/or/geoip.h b/src/or/geoip.h index 070296dd0..42d0c1cfd 100644 --- a/src/or/geoip.h +++ b/src/or/geoip.h @@ -33,6 +33,8 @@ void geoip_note_client_seen(geoip_client_action_t action, const tor_addr_t *addr, const char *transport_name, time_t now); void geoip_remove_old_clients(time_t cutoff); +size_t geoip_client_cache_total_allocation(void); +size_t geoip_client_cache_handle_oom(time_t now, size_t min_remove_bytes); void geoip_note_ns_response(geoip_ns_response_t response); char *geoip_get_transport_history(void); diff --git a/src/or/relay.c b/src/or/relay.c index 29f34ca03..22ce76752 100644 --- a/src/or/relay.c +++ b/src/or/relay.c @@ -2469,24 +2469,34 @@ static time_t last_time_under_memory_pressure = 0; STATIC int cell_queues_check_size(void) { + time_t now = time(NULL); size_t alloc = cell_queues_get_total_allocation(); alloc += buf_get_total_allocation(); alloc += tor_zlib_get_total_allocation(); const size_t rend_cache_total = rend_cache_get_total_allocation(); alloc += rend_cache_total; + const size_t geoip_client_cache_total = + geoip_client_cache_total_allocation(); + alloc += geoip_client_cache_total; if (alloc >= get_options()->MaxMemInQueues_low_threshold) { last_time_under_memory_pressure = approx_time(); if (alloc >= get_options()->MaxMemInQueues) { /* If we're spending over 20% of the memory limit on hidden service - * descriptors, free them until we're down to 10%. - */ + * descriptors, free them until we're down to 10%. Do the same for geoip + * client cache. */ if (rend_cache_total > get_options()->MaxMemInQueues / 5) { const size_t bytes_to_remove = rend_cache_total - (size_t)(get_options()->MaxMemInQueues / 10); - rend_cache_clean_v2_descs_as_dir(time(NULL), bytes_to_remove); + rend_cache_clean_v2_descs_as_dir(now, bytes_to_remove); alloc -= rend_cache_total; alloc += rend_cache_get_total_allocation(); } + if (geoip_client_cache_total > get_options()->MaxMemInQueues / 5) { + const size_t bytes_to_remove = + geoip_client_cache_total - + (size_t)(get_options()->MaxMemInQueues / 10); + alloc -= geoip_client_cache_handle_oom(now, bytes_to_remove); + } circuits_handle_oom(alloc); return 1; } diff --git a/src/test/test.c b/src/test/test.c index 9a41b976b..e2fbfd21b 100644 --- a/src/test/test.c +++ b/src/test/test.c @@ -912,6 +912,24 @@ test_geoip(void *arg) tt_str_op(entry_stats_2,OP_EQ, s); tor_free(s); + /* Test the OOM handler. Add a client, run the OOM. */ + geoip_entry_stats_init(now); + SET_TEST_ADDRESS(100); + geoip_note_client_seen(GEOIP_CLIENT_CONNECT, &addr, NULL, + now - (12 * 60 * 60)); + /* We've seen this 12 hours ago. Run the OOM, it should clean the entry + * because it is above the minimum cutoff of 4 hours. */ + size_t bytes_removed = geoip_client_cache_handle_oom(now, 1000); + tt_size_op(bytes_removed, OP_GT, 0); + + /* Do it again but this time with an entry with a lower cutoff. */ + geoip_entry_stats_init(now); + SET_TEST_ADDRESS(100); + geoip_note_client_seen(GEOIP_CLIENT_CONNECT, &addr, NULL, + now - (3 * 60 * 60)); + bytes_removed = geoip_client_cache_handle_oom(now, 1000); + tt_size_op(bytes_removed, OP_EQ, 0); + /* Stop collecting entry statistics. */ geoip_entry_stats_term(); get_options_mutable()->EntryStatistics = 0;

1 0