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

[tor/maint-0.3.2] 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.3.2] 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.3.2] 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

[tor/maint-0.3.2] dos: Apply defense for circuit creation DoS
by nickm＠torproject.org 16 Feb '18

16 Feb '18

commit 1bfc91a029839f36e04c8204d1bccaa04a5c2afd Author: David Goulet <dgoulet(a)torproject.org> Date: Thu Jan 25 16:20:52 2018 -0500 dos: Apply defense for circuit creation DoS If the client address was detected as malicious, apply a defense which is at this commit to return a DESTROY cell. Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- src/or/command.c | 7 ++++++ src/or/dos.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ src/or/dos.h | 1 + 3 files changed, 73 insertions(+) diff --git a/src/or/command.c b/src/or/command.c index d2df55a4b..0d2808e23 100644 --- a/src/or/command.c +++ b/src/or/command.c @@ -290,6 +290,13 @@ command_process_create_cell(cell_t *cell, channel_t *chan) return; } + /* Check if we should apply a defense for this channel. */ + if (dos_cc_get_defense_type(chan) == DOS_CC_DEFENSE_REFUSE_CELL) { + channel_send_destroy(cell->circ_id, chan, + END_CIRC_REASON_RESOURCELIMIT); + return; + } + if (!server_mode(options) || (!public_server_mode(options) && channel_is_outgoing(chan))) { log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL, diff --git a/src/or/dos.c b/src/or/dos.c index b83ea6029..8c00a2f31 100644 --- a/src/or/dos.c +++ b/src/or/dos.c @@ -36,6 +36,7 @@ static dos_cc_defense_type_t dos_cc_defense_type; static int32_t dos_cc_defense_time_period; /* Keep some stats for the heartbeat so we can report out. */ +static uint64_t cc_num_rejected_cells; static uint32_t cc_num_marked_addrs; /* @@ -323,6 +324,44 @@ cc_mark_client(cc_client_stats_t *stats) crypto_rand_int_range(1, dos_cc_defense_time_period / 2); } +/* Return true iff the given channel address is marked as malicious. This is + * called a lot and part of the fast path of handling cells. It has to remain + * as fast as we can. */ +static int +cc_channel_addr_is_marked(channel_t *chan) +{ + time_t now; + tor_addr_t addr; + clientmap_entry_t *entry; + cc_client_stats_t *stats = NULL; + + if (chan == NULL) { + goto end; + } + /* Must be a client connection else we ignore. */ + if (!channel_is_client(chan)) { + goto end; + } + /* Without an IP address, nothing can work. */ + if (!channel_get_addr_if_possible(chan, &addr)) { + goto end; + } + + /* We are only interested in client connection from the geoip cache. */ + entry = geoip_lookup_client(&addr, NULL, GEOIP_CLIENT_CONNECT); + if (entry == NULL) { + /* We can have a connection creating circuits but not tracked by the geoip + * cache. Once this DoS subsystem is enabled, we can end up here with no + * entry for the channel. */ + goto end; + } + now = approx_time(); + stats = &entry->dos_stats.cc_stats; + + end: + return stats && stats->marked_until_ts >= now; +} + /* Concurrent connection private API. */ /* Free everything for the connection DoS mitigation subsystem. */ @@ -421,6 +460,32 @@ dos_cc_new_create_cell(channel_t *chan) return; } +/* Return the defense type that should be used for this circuit. + * + * This is part of the fast path and called a lot. */ +dos_cc_defense_type_t +dos_cc_get_defense_type(channel_t *chan) +{ + tor_assert(chan); + + /* Skip everything if not enabled. */ + if (!dos_cc_enabled) { + goto end; + } + + /* On an OR circuit, we'll check if the previous channel is a marked client + * connection detected by our DoS circuit creation mitigation subsystem. */ + if (cc_channel_addr_is_marked(chan)) { + /* We've just assess that this circuit should trigger a defense for the + * cell it just seen. Note it down. */ + cc_num_rejected_cells++; + return dos_cc_defense_type; + } + + end: + return DOS_CC_DEFENSE_NONE; +} + /* Concurrent connection detection public API. */ /* General API */ diff --git a/src/or/dos.h b/src/or/dos.h index bb8d7d1a7..fa86295cf 100644 --- a/src/or/dos.h +++ b/src/or/dos.h @@ -81,6 +81,7 @@ typedef enum dos_cc_defense_type_t { } dos_cc_defense_type_t; void dos_cc_new_create_cell(channel_t *channel); +dos_cc_defense_type_t dos_cc_get_defense_type(channel_t *chan); /* * Concurrent connection DoS mitigation interface.

1 0

[tor/maint-0.3.2] dos: Add the connection DoS mitigation subsystem
by nickm＠torproject.org 16 Feb '18

16 Feb '18

commit acf7ea77d8d76830924a14145afbcf3c95a06b0e Author: David Goulet <dgoulet(a)torproject.org> Date: Thu Jan 25 16:28:54 2018 -0500 dos: Add the connection DoS mitigation subsystem Defend against an address that has reached the concurrent connection count threshold. Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- src/or/connection.c | 8 ++++++++ src/or/dos.c | 34 ++++++++++++++++++++++++++++++++++ src/or/dos.h | 2 ++ 3 files changed, 44 insertions(+) diff --git a/src/or/connection.c b/src/or/connection.c index 15f489c6b..791fd95c2 100644 --- a/src/or/connection.c +++ b/src/or/connection.c @@ -1600,6 +1600,14 @@ connection_handle_listener_read(connection_t *conn, int new_type) return 0; } } + if (new_type == CONN_TYPE_OR) { + /* Assess with the connection DoS mitigation subsystem if this address + * can open a new connection. */ + if (dos_conn_addr_get_defense_type(&addr) == DOS_CONN_DEFENSE_CLOSE) { + tor_close_socket(news); + return 0; + } + } newconn = connection_new(new_type, conn->socket_family); newconn->s = news; diff --git a/src/or/dos.c b/src/or/dos.c index 8c00a2f31..7e3a2ab7f 100644 --- a/src/or/dos.c +++ b/src/or/dos.c @@ -53,6 +53,9 @@ static unsigned int dos_conn_enabled = 0; static uint32_t dos_conn_max_concurrent_count; static dos_conn_defense_type_t dos_conn_defense_type; +/* Keep some stats for the heartbeat so we can report out. */ +static uint64_t conn_num_addr_rejected; + /* * General interface of the denial of service mitigation subsystem. */ @@ -488,6 +491,37 @@ dos_cc_get_defense_type(channel_t *chan) /* Concurrent connection detection public API. */ +/* Return true iff the given address is permitted to open another connection. + * A defense value is returned for the caller to take appropriate actions. */ +dos_conn_defense_type_t +dos_conn_addr_get_defense_type(const tor_addr_t *addr) +{ + clientmap_entry_t *entry; + + tor_assert(addr); + + /* Skip everything if not enabled. */ + if (!dos_conn_enabled) { + goto end; + } + + /* We are only interested in client connection from the geoip cache. */ + entry = geoip_lookup_client(addr, NULL, GEOIP_CLIENT_CONNECT); + if (entry == NULL) { + goto end; + } + + /* Need to be above the maximum concurrent connection count to trigger a + * defense. */ + if (entry->dos_stats.concurrent_count > dos_conn_max_concurrent_count) { + conn_num_addr_rejected++; + return dos_conn_defense_type; + } + + end: + return DOS_CONN_DEFENSE_NONE; +} + /* General API */ /* Called when a new client connection has been established on the given diff --git a/src/or/dos.h b/src/or/dos.h index fa86295cf..cc7749836 100644 --- a/src/or/dos.h +++ b/src/or/dos.h @@ -107,6 +107,8 @@ typedef enum dos_conn_defense_type_t { DOS_CONN_DEFENSE_MAX = 2, } dos_conn_defense_type_t; +dos_conn_defense_type_t dos_conn_addr_get_defense_type(const tor_addr_t *addr); + #ifdef DOS_PRIVATE STATIC uint32_t get_param_conn_max_concurrent_count(

1 0

[tor/maint-0.3.2] dos: We can put less token than the current amount
by nickm＠torproject.org 16 Feb '18

16 Feb '18

commit 78d6cb58707ff46464c591e45d81e83388427e2c Author: David Goulet <dgoulet(a)torproject.org> Date: Fri Feb 2 17:04:12 2018 -0500 dos: We can put less token than the current amount Becasue the circuit creation burst and rate can change at runtime it is possible that between two refill of a bucket, we end up setting the bucket value to less than there currently is. Fixes #25128 Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- src/or/dos.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/or/dos.c b/src/or/dos.c index c221e5ecd..88f1351a3 100644 --- a/src/or/dos.c +++ b/src/or/dos.c @@ -308,8 +308,6 @@ cc_stats_refill_bucket(cc_client_stats_t *stats, const tor_addr_t *addr) 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 %" PRIu64 ". Elapsed time is %" PRIi64,

1 0

[tor/maint-0.3.2] Use tt_u64_op() for uint64_t inputs.
by nickm＠torproject.org 16 Feb '18

16 Feb '18

commit 3bed8fdb91599b5e7c7946978c6221ba5db85463 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Feb 2 15:23:55 2018 -0500 Use tt_u64_op() for uint64_t inputs. --- src/test/test_dos.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/test_dos.c b/src/test/test_dos.c index 071926c28..9496b0735 100644 --- a/src/test/test_dos.c +++ b/src/test/test_dos.c @@ -179,8 +179,8 @@ test_dos_bucket_refill(void *arg) 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); - tt_int_op(circ_rate, OP_LT, max_circuit_count); + tt_u64_op(circ_rate, OP_GT, 1); + tt_u64_op(circ_rate, OP_LT, max_circuit_count); /* Register this client */ geoip_note_client_seen(GEOIP_CLIENT_CONNECT, addr, NULL, now);

1 0

[tor/maint-0.3.2] geoip: Increment and decrement functions for the geoip client cache
by nickm＠torproject.org 16 Feb '18

16 Feb '18

commit 4d812e29b9b1ec88fe268c150a826466b23a8762 Author: David Goulet <dgoulet(a)torproject.org> Date: Fri Feb 2 13:14:50 2018 -0500 geoip: Increment and decrement functions for the geoip client cache These functions protect againts over and underflow. They BUG() in case we overflow the counter. Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- src/or/geoip.c | 32 ++++++++++++++++++++++++++++++-- 1 file changed, 30 insertions(+), 2 deletions(-) diff --git a/src/or/geoip.c b/src/or/geoip.c index c5e8cdab9..92db9742e 100644 --- a/src/or/geoip.c +++ b/src/or/geoip.c @@ -76,6 +76,34 @@ static char geoip6_digest[DIGEST_LEN]; * handler. */ static size_t geoip_client_history_cache_size; +/* Increment the geoip client history cache size counter with the given bytes. + * This prevents an overflow and set it to its maximum in that case. */ +static inline void +geoip_increment_client_history_cache_size(size_t bytes) +{ + /* This is shockingly high, lets log it so it can be reported. */ + IF_BUG_ONCE(geoip_client_history_cache_size > (SIZE_MAX - bytes)) { + geoip_client_history_cache_size = SIZE_MAX; + return; + } + geoip_client_history_cache_size += bytes; +} + +/* Decrement the geoip client history cache size counter with the given bytes. + * This prevents an underflow and set it to 0 in that case. */ +static inline void +geoip_decrement_client_history_cache_size(size_t bytes) +{ + /* Going below 0 means that we either allocated an entry without + * incrementing the counter or we have different sizes when allocating and + * freeing. It shouldn't happened so log it. */ + IF_BUG_ONCE(geoip_client_history_cache_size < bytes) { + geoip_client_history_cache_size = 0; + return; + } + geoip_client_history_cache_size -= bytes; +} + /** 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. */ @@ -546,7 +574,7 @@ clientmap_entry_free(clientmap_entry_t *ent) if (!ent) return; - geoip_client_history_cache_size -= clientmap_entry_size(ent); + geoip_decrement_client_history_cache_size(clientmap_entry_size(ent)); tor_free(ent->transport_name); tor_free(ent); @@ -610,7 +638,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); + geoip_increment_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);

1 0

[tor/maint-0.3.2] 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.3.2] 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