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

[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

[tor/maint-0.2.9] dos: Man page entry for DoS mitigation
by nickm＠torproject.org 16 Feb '18

16 Feb '18

commit a3714268f659998dc879ed723852440cd8be1b04 Author: David Goulet <dgoulet(a)torproject.org> Date: Fri Jan 26 09:00:17 2018 -0500 dos: Man page entry for DoS mitigation Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- doc/tor.1.txt | 90 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 90 insertions(+) diff --git a/doc/tor.1.txt b/doc/tor.1.txt index 4c5d5359a..a2bbb8ab6 100644 --- a/doc/tor.1.txt +++ b/doc/tor.1.txt @@ -2441,6 +2441,96 @@ The following options are used to configure a hidden service. including setting SOCKSPort to "0". (Default: 0) +DENIAL OF SERVICE MITIGATION OPTIONS +------------------------------------ + +The following options are useful only for a public relay. They control the +Denial of Service mitigation subsystem. + +[[DoSCircuitCreationEnabled]] **DoSCircuitCreationEnabled** **0**|**1**|**auto**:: + + Enable circuit creation DoS mitigation. If enabled, tor will cache client + IPs along with statistics in order to detect circuit DoS attacks. If an + address is positively identified, tor will activate defenses against the + address. See the DoSCircuitCreationDefenseType option for more details. + This is a client to relay detection only. "auto" means use the consensus + parameter. + (Default: auto) + +[[DoSCircuitCreationMinConnections]] **DoSCircuitCreationMinConnections** __NUM__:: + + Minimum threshold of concurrent connections before a client address can be + flagged as executing a circuit creation DoS. In other words, once a client + address reaches the circuit rate and has a minimum of NUM concurrent + connections, a detection is positive. "0" means use the consensus + parameter. + (Default: 0) + +[[DoSCircuitCreationRateTenths]] **DoSCircuitCreationRateTenths** __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) + +[[DoSCircuitCreationBurst]] **DoSCircuitCreationBurst** __NUM__:: + + The allowed circuit creation burst per client IP address. If the circuit + rate and the burst are reached, a client is marked as executing a circuit + creation DoS. "0" means use the consensus parameter. + (Default: 0) + +[[DoSCircuitCreationDefenseType]] **DoSCircuitCreationDefenseType** __NUM__:: + + This is the type of defense applied to a detected client address. The + possible values are: + + 1: No defense. + 2: Refuse circuit creation for the DoSCircuitCreationDefenseTimePeriod period of time. ++ + "0" means use the consensus parameter. + (Default: 0) + +[[DoSCircuitCreationDefenseTimePeriod]] **DoSCircuitCreationDefenseTimePeriod** __NUM__:: + + The base time period that the DoS defense is activated for. The actual + value is selected randomly for each activation from NUM+1 to 3/2 * NUM. + "0" means use the consensus parameter. + (Default: 0) + +[[DoSConnectionEnabled]] **DoSConnectionEnabled** **0**|**1**|**auto**:: + + Enable the connection DoS mitigation. For client address only, this allows + tor to mitigate against large number of concurrent connections made by a + single IP address. "auto" means use the consensus parameter. + (Default: auto) + +[[DoSConnectionMaxConcurrentCount]] **DoSConnectionMaxConcurrentCount** __NUM__:: + + The maximum threshold of concurrent connection from a client IP address. + Above this limit, a defense selected by DoSConnectionDefenseType is + applied. "0" means use the consensus parameter. + (Default: 0) + +[[DoSConnectionDefenseType]] **DoSConnectionDefenseType** __NUM__:: + + This is the type of defense applied to a detected client address for the + connection mitigation. The possible values are: + + 1: No defense. + 2: Immediately close new connections. ++ + "0" means use the consensus parameter. + (Default: 0) + +[[DoSRefuseSingleHopClientRendezvous]] **DoSRefuseSingleHopClientRendezvous** **0**|**1**|**auto**:: + + Refuse establishment of rendezvous points for single hop clients. In other + words, if a client directly connects to the relay and sends an + ESTABLISH_RENDEZVOUS cell, it is silently dropped. "auto" means use the + consensus parameter. + (Default: auto) + TESTING NETWORK OPTIONS -----------------------

1 0

[tor/maint-0.2.9] channel_tls_get_remote_addr_method now returns real_addr.
by nickm＠torproject.org 16 Feb '18

16 Feb '18

commit 33d9889a2bdd611bcc255c68c43d60b8919ab663 Author: Fernando Fernandez Mancera <ffmancera(a)riseup.net> Date: Mon Jan 22 15:20:17 2018 +0100 channel_tls_get_remote_addr_method now returns real_addr. The accurate address of a connection is real_addr, not the addr member. channel_tls_get_remote_addr_method() now returns real_addr instead. Fixes #24952; bugfix on 707c1e2 in 0.2.4.11-alpha. Signed-off-by: Fernando Fernandez Mancera <ffmancera(a)riseup.net> --- changes/bug24952 | 5 +++++ src/or/channeltls.c | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/changes/bug24952 b/changes/bug24952 new file mode 100644 index 000000000..93174c04f --- /dev/null +++ b/changes/bug24952 @@ -0,0 +1,5 @@ + o Minor bugfix (channel connection): + - The accurate address of a connection is real_addr, not the addr member. + TLS Channel remote address is now real_addr content instead of addr + member. Fixes bug 24952; bugfix on 707c1e2e26 in 0.2.4.11-alpha. + Patch by "ffmancera". diff --git a/src/or/channeltls.c b/src/or/channeltls.c index 09cca95b6..890646989 100644 --- a/src/or/channeltls.c +++ b/src/or/channeltls.c @@ -514,7 +514,7 @@ channel_tls_get_remote_addr_method(channel_t *chan, tor_addr_t *addr_out) tor_assert(addr_out); if (tlschan->conn) { - tor_addr_copy(addr_out, &(TO_CONN(tlschan->conn)->addr)); + tor_addr_copy(addr_out, &(tlschan->conn->real_addr)); rv = 1; } else tor_addr_make_unspec(addr_out);

1 0

[tor/maint-0.2.9] Merge branch 'ticket25122_029_02' into ticket24902_029_05
by nickm＠torproject.org 16 Feb '18

16 Feb '18

commit 475218c108ad84aa302f0daec013faab9ff778f2 Merge: 33d9889a2 e758d659a Author: David Goulet <dgoulet(a)torproject.org> Date: Fri Feb 2 14:55:01 2018 -0500 Merge branch 'ticket25122_029_02' into ticket24902_029_05 changes/ticket25122 | 4 ++ src/or/geoip.c | 148 ++++++++++++++++++++++++++++++++++++++++++++++++++-- src/or/geoip.h | 2 + src/or/relay.c | 16 ++++-- src/test/test.c | 18 +++++++ 5 files changed, 180 insertions(+), 8 deletions(-) diff --cc src/or/geoip.c index 4e4f6e639,76fca43f6..20dad5f15 --- a/src/or/geoip.c +++ b/src/or/geoip.c @@@ -516,9 -574,7 +557,10 @@@ clientmap_entry_free(clientmap_entry_t 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); + geoip_decrement_client_history_cache_size(clientmap_entry_size(ent)); tor_free(ent->transport_name); tor_free(ent); @@@ -573,13 -651,13 +640,9 @@@ geoip_note_client_seen(geoip_client_act safe_str_client(fmt_addr((addr))), transport_name ? transport_name : "<no transport>"); - tor_addr_copy(&lookup.addr, addr); - lookup.action = (int)action; - lookup.transport_name = (char*) transport_name; - ent = HT_FIND(clientmap, &client_history, &lookup); - + ent = geoip_lookup_client(addr, transport_name, action); if (! ent) { - ent = tor_malloc_zero(sizeof(clientmap_entry_t)); - tor_addr_copy(&ent->addr, addr); - if (transport_name) - ent->transport_name = tor_strdup(transport_name); - ent->action = (int)action; + ent = clientmap_entry_new(action, addr, transport_name); HT_INSERT(clientmap, &client_history, ent); } if (now / 60 <= (int)MAX_LAST_SEEN_IN_MINUTES && now >= 0) @@@ -621,25 -699,81 +684,100 @@@ geoip_remove_old_clients(time_t cutoff &cutoff); } +/* Return a client entry object matching the given address, transport name and + * geoip action from the clientmap. NULL if not found. The transport_name can + * be NULL. */ +clientmap_entry_t * +geoip_lookup_client(const tor_addr_t *addr, const char *transport_name, + geoip_client_action_t action) +{ + clientmap_entry_t lookup; + + tor_assert(addr); + + /* We always look for a client connection with no transport. */ + tor_addr_copy(&lookup.addr, addr); + lookup.action = action; + lookup.transport_name = (char *) transport_name; + + return HT_FIND(clientmap, &client_history, &lookup); +} + + /* 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 --cc src/or/geoip.h index aa0fca50f,42d0c1cfd..c8ea9f85e --- a/src/or/geoip.h +++ b/src/or/geoip.h @@@ -57,9 -33,8 +57,11 @@@ void geoip_note_client_seen(geoip_clien const tor_addr_t *addr, const char *transport_name, time_t now); void geoip_remove_old_clients(time_t cutoff); +clientmap_entry_t *geoip_lookup_client(const tor_addr_t *addr, + const char *transport_name, + geoip_client_action_t action); + 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);

1 0

[tor/maint-0.2.9] 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.2.9] 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.2.9] 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.2.9] Add an address-set backend using a bloom filter.
by nickm＠torproject.org 16 Feb '18

16 Feb '18

commit 46bd2aed915f17d520f9ff237262d1510fe25e12 Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Feb 7 09:49:35 2018 -0500 Add an address-set backend using a bloom filter. We're going to need this to make our anti-DoS code (see 24902) more robust. --- src/common/address.c | 22 +++++++++ src/common/address.h | 2 + src/common/address_set.c | 120 +++++++++++++++++++++++++++++++++++++++++++++++ src/common/address_set.h | 33 +++++++++++++ src/common/include.am | 2 + 5 files changed, 179 insertions(+) diff --git a/src/common/address.c b/src/common/address.c index 773e68855..1bd52d24b 100644 --- a/src/common/address.c +++ b/src/common/address.c @@ -1200,6 +1200,28 @@ tor_addr_hash(const tor_addr_t *addr) } } +/** As tor_addr_hash, but use a particular siphash key. */ +uint64_t +tor_addr_keyed_hash(const struct sipkey *key, const tor_addr_t *addr) +{ + /* This is duplicate code with tor_addr_hash, since this function needs to + * be backportable all the way to 0.2.9. */ + + switch (tor_addr_family(addr)) { + case AF_INET: + return siphash24(&addr->addr.in_addr.s_addr, 4, key); + case AF_UNSPEC: + return 0x4e4d5342; + case AF_INET6: + return siphash24(&addr->addr.in6_addr.s6_addr, 16, key); + default: + /* LCOV_EXCL_START */ + tor_fragile_assert(); + return 0; + /* LCOV_EXCL_END */ + } +} + /** Return a newly allocated string with a representation of addr. */ char * tor_addr_to_str_dup(const tor_addr_t *addr) diff --git a/src/common/address.h b/src/common/address.h index 51db42c31..d57abd0d9 100644 --- a/src/common/address.h +++ b/src/common/address.h @@ -228,6 +228,8 @@ int tor_addr_compare_masked(const tor_addr_t *addr1, const tor_addr_t *addr2, #define tor_addr_eq(a,b) (0==tor_addr_compare((a),(b),CMP_EXACT)) uint64_t tor_addr_hash(const tor_addr_t *addr); +struct sipkey; +uint64_t tor_addr_keyed_hash(const struct sipkey *key, const tor_addr_t *addr); int tor_addr_is_v4(const tor_addr_t *addr); int tor_addr_is_internal_(const tor_addr_t *ip, int for_listening, const char *filename, int lineno); diff --git a/src/common/address_set.c b/src/common/address_set.c new file mode 100644 index 000000000..df7022174 --- /dev/null +++ b/src/common/address_set.c @@ -0,0 +1,120 @@ +/* Copyright (c) 2018, The Tor Project, Inc. */ +/* See LICENSE for licensing information */ + +/** + * \file address_set.c + * \brief Implementation for a set of addresses. + * + * This module was first written on a semi-emergency basis to improve the + * robustness of the anti-DoS module. As such, it's written in a pretty + * conservative way, and should be susceptible to improvement later on. + **/ + +#include "orconfig.h" +#include "address_set.h" +#include "address.h" +#include "compat.h" +#include "container.h" +#include "crypto.h" +#include "util.h" +#include "siphash.h" + +/** How many 64-bit siphash values to extract per address */ +#define N_HASHES 2 +/** How many bloom-filter bits we set per address. This is twice the N_HASHES + * value, since we split the siphash outcome two 32-bit values. */ +#define N_BITS_PER_ITEM (N_HASHES * 2) + +/* XXXX This code is largely duplicated with digestset_t. We should merge + * them together into a common bloom-filter implementation. I'm keeping + * them separate for now, though, since this module needs to be backported + * all the way to 0.2.9. + * + * The main difference between digestset_t and this code is that we use + * independent siphashes rather than messing around with bit-shifts. The + * approach here is probably more sound, and we should prefer it if&when we + * unify the implementations. + **/ + +struct address_set_t { + /** siphash keys to make N_HASHES independent hashes for each address. */ + struct sipkey key[N_HASHES]; + int mask; /**< One less than the number of bits in ba; always one less + * than a power of two. */ + bitarray_t *ba; /**< A bit array to implement the Bloom filter. */ +}; + +/** + * Allocate and return an address_set, suitable for holding up to + * max_address_guess distinct values. + */ +address_set_t * +address_set_new(int max_addresses_guess) +{ + /* See digestset_new() for rationale on this equation. */ + int n_bits = 1u << (tor_log2(max_addresses_guess)+5); + + address_set_t *set = tor_malloc_zero(sizeof(address_set_t)); + set->mask = n_bits - 1; + set->ba = bitarray_init_zero(n_bits); + crypto_rand((char*) set->key, sizeof(set->key)); + + return set; +} + +/** + * Release all storage associated with set + */ +void +address_set_free(address_set_t *set) +{ + if (! set) + return; + + bitarray_free(set->ba); + tor_free(set); +} + +/** Yield the bit index corresponding to 'val' for set. */ +#define BIT(set, val) ((val) & (set)->mask) + +/** + * Add addr to set. + * + * All future queries for addr in set will return true. Removing + * items is not possible. + */ +void +address_set_add(address_set_t *set, const struct tor_addr_t *addr) +{ + int i; + for (i = 0; i < N_HASHES; ++i) { + uint64_t h = tor_addr_keyed_hash(&set->key[i], addr); + uint32_t high_bits = (uint32_t)(h >> 32); + uint32_t low_bits = (uint32_t)(h); + bitarray_set(set->ba, BIT(set, high_bits)); + bitarray_set(set->ba, BIT(set, low_bits)); + } +} + +/** + * Return true if addr if a member of set. (And probably, + * return false if addr is not a member of set.) + */ +int +address_set_probably_contains(address_set_t *set, + const struct tor_addr_t *addr) +{ + int i, matches = 0; + for (i = 0; i < N_HASHES; ++i) { + uint64_t h = tor_addr_keyed_hash(&set->key[i], addr); + uint32_t high_bits = (uint32_t)(h >> 32); + uint32_t low_bits = (uint32_t)(h); + // Note that !! is necessary here, since bitarray_is_set does not + // necessarily return 1 on true. + matches += !! bitarray_is_set(set->ba, BIT(set, high_bits)); + matches += !! bitarray_is_set(set->ba, BIT(set, low_bits)); + } + return matches == N_BITS_PER_ITEM; +} + diff --git a/src/common/address_set.h b/src/common/address_set.h new file mode 100644 index 000000000..568528c89 --- /dev/null +++ b/src/common/address_set.h @@ -0,0 +1,33 @@ +/* Copyright (c) 2018, The Tor Project, Inc. */ +/* See LICENSE for licensing information */ + +/** + * \file addressset.h + * \brief Types to handle sets of addresses. + * + * This module was first written on a semi-emergency basis to improve the + * robustness of the anti-DoS module. As such, it's written in a pretty + * conservative way, and should be susceptible to improvement later on. + **/ + +#ifndef TOR_ADDRESS_SET_H +#define TOR_ADDRESS_SET_H + +#include "orconfig.h" + +/** + * An address_set_t represents a set of tor_addr_t values. The implementation + * is probabilistic: false negatives cannot occur but false positives are + * possible. + */ +typedef struct address_set_t address_set_t; +struct tor_addr_t; + +address_set_t *address_set_new(int max_addresses_guess); +void address_set_free(address_set_t *set); +void address_set_add(address_set_t *set, const struct tor_addr_t *addr); +int address_set_probably_contains(address_set_t *set, + const struct tor_addr_t *addr); + +#endif + diff --git a/src/common/include.am b/src/common/include.am index 40c463c9d..cb307e9d5 100644 --- a/src/common/include.am +++ b/src/common/include.am @@ -80,6 +80,7 @@ src_common_libor_ctime_testing_a_CFLAGS = @CFLAGS_CONSTTIME@ $(TEST_CFLAGS) LIBOR_A_SRC = \ src/common/address.c \ + src/common/address_set.c \ src/common/backtrace.c \ src/common/compat.c \ src/common/compat_threads.c \ @@ -135,6 +136,7 @@ src_common_libor_event_testing_a_CFLAGS = $(AM_CFLAGS) $(TEST_CFLAGS) COMMONHEADERS = \ src/common/address.h \ + src/common/address_set.h \ src/common/backtrace.h \ src/common/aes.h \ src/common/ciphers.inc \

1 0

[tor/maint-0.2.9] remove a redundant semicolon
by nickm＠torproject.org 16 Feb '18

16 Feb '18

commit 22a5d3dd2ab793336e911d3aceb8cacd278e0f48 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Jan 30 18:11:16 2018 -0500 remove a redundant semicolon --- 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 9496b0735..6db98b9ed 100644 --- a/src/test/test_dos.c +++ b/src/test/test_dos.c @@ -78,7 +78,7 @@ 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"));; + tt_int_op(AF_INET,OP_EQ, tor_addr_parse(addr_out, "18.0.0.1")); return 1; done:

1 0

[tor/maint-0.2.9] geoip: Add clientmap_entry_new() function
by nickm＠torproject.org 16 Feb '18

16 Feb '18

commit e758d659a0bc8b9a0e2bd6a0126755fd1fb58e0a Author: David Goulet <dgoulet(a)torproject.org> Date: Fri Feb 2 13:24:37 2018 -0500 geoip: Add clientmap_entry_new() function Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- src/or/geoip.c | 32 ++++++++++++++++++++++++++------ 1 file changed, 26 insertions(+), 6 deletions(-) diff --git a/src/or/geoip.c b/src/or/geoip.c index 92db9742e..76fca43f6 100644 --- a/src/or/geoip.c +++ b/src/or/geoip.c @@ -580,6 +580,31 @@ clientmap_entry_free(clientmap_entry_t *ent) tor_free(ent); } +/* Return a newly allocated clientmap entry with the given action and address + * that are mandatory. The transport_name can be optional. This can't fail. */ +static clientmap_entry_t * +clientmap_entry_new(geoip_client_action_t action, const tor_addr_t *addr, + const char *transport_name) +{ + clientmap_entry_t *entry; + + tor_assert(action == GEOIP_CLIENT_CONNECT || + action == GEOIP_CLIENT_NETWORKSTATUS); + tor_assert(addr); + + entry = tor_malloc_zero(sizeof(clientmap_entry_t)); + entry->action = action; + tor_addr_copy(&entry->addr, addr); + if (transport_name) { + entry->transport_name = tor_strdup(transport_name); + } + + /* Allocated and initialized, note down its size for the OOM handler. */ + geoip_increment_client_history_cache_size(clientmap_entry_size(entry)); + + return entry; +} + /** Clear history of connecting clients used by entry and bridge stats. */ static void client_history_clear(void) @@ -632,13 +657,8 @@ geoip_note_client_seen(geoip_client_action_t action, ent = HT_FIND(clientmap, &client_history, &lookup); if (! ent) { - ent = tor_malloc_zero(sizeof(clientmap_entry_t)); - tor_addr_copy(&ent->addr, addr); - if (transport_name) - ent->transport_name = tor_strdup(transport_name); - ent->action = (int)action; + ent = clientmap_entry_new(action, addr, transport_name); HT_INSERT(clientmap, &client_history, 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