From 93b826faaa7cca351c68256ce60a7f7e6c5fda5b Mon Sep 17 00:00:00 2001 From: David Goulet Date: Thu, 25 Jan 2018 15:44:48 -0500 Subject: [PATCH 01/37] geoip: Add a lookup function for client map entry The upcoming DoS mitigation subsytem needs to keep information on a per-IP basis which is also what the geoip clientmap does. For another subsystem to access that clientmap, this commit adds a lookup function that returns the entry. For this, the clientmap_entry_t had to be moved to the header file. Signed-off-by: David Goulet --- src/or/geoip.c | 46 +++++++++++++++++++++------------------------- src/or/geoip.h | 22 ++++++++++++++++++++++ 2 files changed, 43 insertions(+), 25 deletions(-) diff --git a/src/or/geoip.c b/src/or/geoip.c index 00c055bbe7..e2a1b1cee4 100644 --- a/src/or/geoip.c +++ b/src/or/geoip.c @@ -472,24 +472,6 @@ geoip_db_digest(sa_family_t family) return hex_str(geoip6_digest, DIGEST_LEN); } -/** Entry in a map from IP address to the last time we've seen an incoming - * connection from that IP address. Used by bridges only, to track which - * countries have them blocked. */ -typedef struct clientmap_entry_t { - HT_ENTRY(clientmap_entry_t) node; - tor_addr_t addr; - /* Name of pluggable transport used by this client. NULL if no - pluggable transport was used. */ - char *transport_name; - - /** Time when we last saw this IP address, in MINUTES since the epoch. - * - * (This will run out of space around 4011 CE. If Tor is still in use around - * 4000 CE, please remember to add more bits to last_seen_in_minutes.) */ - unsigned int last_seen_in_minutes:30; - unsigned int action:2; -} clientmap_entry_t; - /** Largest allowable value for last_seen_in_minutes. (It's a 30-bit field, * so it can hold up to (1u<<30)-1, or 0x3fffffffu. */ @@ -564,8 +546,7 @@ geoip_note_client_seen(geoip_client_action_t action, time_t now) { const or_options_t *options = get_options(); - clientmap_entry_t lookup, *ent; - memset(&lookup, 0, sizeof(clientmap_entry_t)); + clientmap_entry_t *ent; if (action == GEOIP_CLIENT_CONNECT) { /* Only remember statistics as entry guard or as bridge. */ @@ -583,11 +564,7 @@ geoip_note_client_seen(geoip_client_action_t action, safe_str_client(fmt_addr((addr))), transport_name ? transport_name : ""); - 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); @@ -635,6 +612,25 @@ 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); +} + /** 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 070296dd07..b80efceb35 100644 --- a/src/or/geoip.h +++ b/src/or/geoip.h @@ -20,6 +20,25 @@ STATIC int geoip_get_country_by_ipv4(uint32_t ipaddr); STATIC int geoip_get_country_by_ipv6(const struct in6_addr *addr); STATIC void clear_geoip_db(void); #endif + +/** Entry in a map from IP address to the last time we've seen an incoming + * connection from that IP address. Used by bridges only to track which + * countries have them blocked, or the DoS mitigation subsystem if enabled. */ +typedef struct clientmap_entry_t { + HT_ENTRY(clientmap_entry_t) node; + tor_addr_t addr; + /* Name of pluggable transport used by this client. NULL if no + pluggable transport was used. */ + char *transport_name; + + /** Time when we last saw this IP address, in MINUTES since the epoch. + * + * (This will run out of space around 4011 CE. If Tor is still in use around + * 4000 CE, please remember to add more bits to last_seen_in_minutes.) */ + unsigned int last_seen_in_minutes:30; + unsigned int action:2; +} clientmap_entry_t; + int should_record_bridge_info(const or_options_t *options); int geoip_load_file(sa_family_t family, const char *filename); MOCK_DECL(int, geoip_get_country_by_addr, (const tor_addr_t *addr)); @@ -33,6 +52,9 @@ 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); +clientmap_entry_t *geoip_lookup_client(const tor_addr_t *addr, + const char *transport_name, + geoip_client_action_t action); void geoip_note_ns_response(geoip_ns_response_t response); char *geoip_get_transport_history(void); From 64149353dda6336488e7d011534a7132b3f01acc Mon Sep 17 00:00:00 2001 From: David Goulet Date: Thu, 25 Jan 2018 15:54:58 -0500 Subject: [PATCH 02/37] dos: Initial code of Denial of Service mitigation This commit introduces the src/or/dos.{c|h} files that contains the code for the Denial of Service mitigation subsystem. It currently contains basic functions to initialize and free the subsystem. They are used at this commit. The torrc options and consensus parameters are defined at this commit and getters are implemented. Signed-off-by: David Goulet --- src/common/log.c | 2 +- src/common/torlog.h | 4 +- src/or/config.c | 25 ++++ src/or/dos.c | 289 +++++++++++++++++++++++++++++++++++++++++ src/or/dos.h | 120 +++++++++++++++++ src/or/include.am | 2 + src/or/main.c | 2 + src/or/networkstatus.c | 13 +- src/or/or.h | 30 +++++ 9 files changed, 483 insertions(+), 4 deletions(-) create mode 100644 src/or/dos.c create mode 100644 src/or/dos.h diff --git a/src/common/log.c b/src/common/log.c index 56adc77f84..4db1c9f0d0 100644 --- a/src/common/log.c +++ b/src/common/log.c @@ -1177,7 +1177,7 @@ static const char *domain_list[] = { "GENERAL", "CRYPTO", "NET", "CONFIG", "FS", "PROTOCOL", "MM", "HTTP", "APP", "CONTROL", "CIRC", "REND", "BUG", "DIR", "DIRSERV", "OR", "EDGE", "ACCT", "HIST", "HANDSHAKE", "HEARTBEAT", "CHANNEL", - "SCHED", NULL + "SCHED", "DOS", NULL }; /** Return a bitmask for the log domain for which domain is the name, diff --git a/src/common/torlog.h b/src/common/torlog.h index 6732a42741..20b7d938f0 100644 --- a/src/common/torlog.h +++ b/src/common/torlog.h @@ -99,8 +99,10 @@ #define LD_CHANNEL (1u<<21) /** Scheduler */ #define LD_SCHED (1u<<22) +/** Denial of Service mitigation. */ +#define LD_DOS (1u<<23) /** Number of logging domains in the code. */ -#define N_LOGGING_DOMAINS 23 +#define N_LOGGING_DOMAINS 24 /** This log message is not safe to send to a callback-based logger * immediately. Used as a flag, not a log domain. */ diff --git a/src/or/config.c b/src/or/config.c index 42ff25877e..c651c202ec 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -29,6 +29,7 @@ #include "dirserv.h" #include "dirvote.h" #include "dns.h" +#include "dos.h" #include "entrynodes.h" #include "geoip.h" #include "hibernate.h" @@ -241,6 +242,19 @@ static config_var_t option_vars_[] = { OBSOLETE("DynamicDHGroups"), VPORT(DNSPort, LINELIST, NULL), V(DNSListenAddress, LINELIST, NULL), + /* DoS circuit creation options. */ + V(DoSCircuitCreationEnabled, AUTOBOOL, "auto"), + V(DoSCircuitCreationMinConnections, UINT, "0"), + V(DoSCircuitCreationRateTenths, UINT, "0"), + V(DoSCircuitCreationBurst, UINT, "0"), + V(DoSCircuitCreationDefenseType, INT, "0"), + V(DoSCircuitCreationDefenseTimePeriod, INTERVAL, "0"), + /* DoS connection options. */ + V(DoSConnectionEnabled, AUTOBOOL, "auto"), + V(DoSConnectionMaxConcurrentCount, UINT, "0"), + V(DoSConnectionDefenseType, INT, "0"), + /* DoS single hop client options. */ + V(DoSRefuseSingleHopClientRendezvous, AUTOBOOL, "auto"), V(DownloadExtraInfo, BOOL, "0"), V(TestingEnableConnBwEvent, BOOL, "0"), V(TestingEnableCellStatsEvent, BOOL, "0"), @@ -2039,6 +2053,17 @@ options_act(const or_options_t *old_options) } } + /* DoS mitigation subsystem only applies to public relay. */ + if (public_server_mode(options)) { + /* If we are configured as a relay, initialize the subsystem. Even on HUP, + * this is safe to call as it will load data from the current options + * or/and the consensus. */ + dos_init(); + } else if (old_options && public_server_mode(old_options)) { + /* Going from relay to non relay, clean it up. */ + dos_free_all(); + } + /* Load the webpage we're going to serve every time someone asks for '/' on our DirPort. */ tor_free(global_dirfrontpagecontents); diff --git a/src/or/dos.c b/src/or/dos.c new file mode 100644 index 0000000000..4b5983d16d --- /dev/null +++ b/src/or/dos.c @@ -0,0 +1,289 @@ +/* Copyright (c) 2018, The Tor Project, Inc. */ +/* See LICENSE for licensing information */ + +/* + * \file dos.c + * \brief Implement Denial of Service mitigation subsystem. + */ + +#define DOS_PRIVATE + +#include "or.h" +#include "channel.h" +#include "config.h" +#include "geoip.h" +#include "main.h" +#include "networkstatus.h" + +#include "dos.h" + +/* + * Circuit creation denial of service mitigation. + * + * Namespace used for this mitigation framework is "dos_cc_" where "cc" is for + * Circuit Creation. + */ + +/* Is the circuit creation DoS mitigation enabled? */ +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_burst; +static dos_cc_defense_type_t dos_cc_defense_type; +static int32_t dos_cc_defense_time_period; + +/* + * Concurrent connection denial of service mitigation. + * + * Namespace used for this mitigation framework is "dos_conn_". + */ + +/* Is the connection DoS mitigation enabled? */ +static unsigned int dos_conn_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_conn_max_concurrent_count; +static dos_conn_defense_type_t dos_conn_defense_type; + +/* + * General interface of the denial of service mitigation subsystem. + */ + +/* 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, +get_param_cc_enabled, (const networkstatus_t *ns)) +{ + if (get_options()->DoSCircuitCreationEnabled != -1) { + return get_options()->DoSCircuitCreationEnabled; + } + + return !!networkstatus_get_param(ns, "DoSCircuitCreationEnabled", + DOS_CC_ENABLED_DEFAULT, 0, 1); +} + +/* Return the parameter for the minimum concurrent connection at which we'll + * start counting circuit for a specific client address. */ +STATIC uint32_t +get_param_cc_min_concurrent_connection(const networkstatus_t *ns) +{ + if (get_options()->DoSCircuitCreationMinConnections) { + return get_options()->DoSCircuitCreationMinConnections; + } + return networkstatus_get_param(ns, "DoSCircuitCreationMinConnections", + DOS_CC_MIN_CONCURRENT_CONN_DEFAULT, + 1, INT32_MAX); +} + +/* 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) +{ + /* This is in seconds. */ + if (get_options()->DoSCircuitCreationRateTenths) { + return get_options()->DoSCircuitCreationRateTenths; + } + return networkstatus_get_param(ns, "DoSCircuitCreationRateTenths", + DOS_CC_CIRCUIT_RATE_TENTHS_DEFAULT, + 1, INT32_MAX); +} + +/* Return the parameter for the maximum circuit count for the circuit time + * rate. */ +STATIC uint32_t +get_param_cc_circuit_burst(const networkstatus_t *ns) +{ + if (get_options()->DoSCircuitCreationBurst) { + return get_options()->DoSCircuitCreationBurst; + } + return networkstatus_get_param(ns, "DoSCircuitCreationBurst", + DOS_CC_CIRCUIT_BURST_DEFAULT, + 1, INT32_MAX); +} + +/* Return the consensus parameter of the circuit creation defense type. */ +static uint32_t +get_param_cc_defense_type(const networkstatus_t *ns) +{ + if (get_options()->DoSCircuitCreationDefenseType) { + return get_options()->DoSCircuitCreationDefenseType; + } + return networkstatus_get_param(ns, "DoSCircuitCreationDefenseType", + DOS_CC_DEFENSE_TYPE_DEFAULT, + DOS_CC_DEFENSE_NONE, DOS_CC_DEFENSE_MAX); +} + +/* Return the consensus parameter of the defense time period which is how much + * time should we defend against a malicious client address. */ +static int32_t +get_param_cc_defense_time_period(const networkstatus_t *ns) +{ + /* Time in seconds. */ + if (get_options()->DoSCircuitCreationDefenseTimePeriod) { + return get_options()->DoSCircuitCreationDefenseTimePeriod; + } + return networkstatus_get_param(ns, "DoSCircuitCreationDefenseTimePeriod", + DOS_CC_DEFENSE_TIME_PERIOD_DEFAULT, + 0, INT32_MAX); +} + +/* Return true iff connection mitigation is enabled. We look at the consensus + * for this else a default value is returned. */ +MOCK_IMPL(STATIC unsigned int, +get_param_conn_enabled, (const networkstatus_t *ns)) +{ + if (get_options()->DoSConnectionEnabled != -1) { + return get_options()->DoSConnectionEnabled; + } + return !!networkstatus_get_param(ns, "DoSConnectionEnabled", + DOS_CONN_ENABLED_DEFAULT, 0, 1); +} + +/* Return the consensus parameter for the maximum concurrent connection + * allowed. */ +STATIC uint32_t +get_param_conn_max_concurrent_count(const networkstatus_t *ns) +{ + if (get_options()->DoSConnectionMaxConcurrentCount) { + return get_options()->DoSConnectionMaxConcurrentCount; + } + return networkstatus_get_param(ns, "DoSConnectionMaxConcurrentCount", + DOS_CONN_MAX_CONCURRENT_COUNT_DEFAULT, + 1, INT32_MAX); +} + +/* Return the consensus parameter of the connection defense type. */ +static uint32_t +get_param_conn_defense_type(const networkstatus_t *ns) +{ + if (get_options()->DoSConnectionDefenseType) { + return get_options()->DoSConnectionDefenseType; + } + return networkstatus_get_param(ns, "DoSConnectionDefenseType", + DOS_CONN_DEFENSE_TYPE_DEFAULT, + DOS_CONN_DEFENSE_NONE, DOS_CONN_DEFENSE_MAX); +} + +/* Set circuit creation parameters located in the consensus or their default + * if none are present. Called at initialization or when the consensus + * changes. */ +static void +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_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); + + /* Connection detection. */ + dos_conn_enabled = get_param_conn_enabled(ns); + dos_conn_max_concurrent_count = get_param_conn_max_concurrent_count(ns); + dos_conn_defense_type = get_param_conn_defense_type(ns); +} + +/* Free everything for the circuit creation DoS mitigation subsystem. */ +static void +cc_free_all(void) +{ + /* If everything is freed, the circuit creation subsystem is not enabled. */ + dos_cc_enabled = 0; +} + +/* Called when the consensus has changed. Do appropriate actions for the + * circuit creation subsystem. */ +static void +cc_consensus_has_changed(const networkstatus_t *ns) +{ + /* Looking at the consensus, is the circuit creation subsystem enabled? If + * not and it was enabled before, clean it up. */ + if (dos_cc_enabled && !get_param_cc_enabled(ns)) { + cc_free_all(); + } +} + +/* Concurrent connection private API. */ + +/* Free everything for the connection DoS mitigation subsystem. */ +static void +conn_free_all(void) +{ + dos_conn_enabled = 0; +} + +/* Called when the consensus has changed. Do appropriate actions for the + * connection mitigation subsystem. */ +static void +conn_consensus_has_changed(const networkstatus_t *ns) +{ + /* Looking at the consensus, is the connection mitigation subsystem enabled? + * If not and it was enabled before, clean it up. */ + if (dos_conn_enabled && !get_param_conn_enabled(ns)) { + conn_free_all(); + } +} + +/* General private API */ + +/* Return true iff we have at least one DoS detection enabled. This is used to + * decide if we need to allocate any kind of high level DoS object. */ +static inline int +dos_is_enabled(void) +{ + return (dos_cc_enabled || dos_conn_enabled); +} + +/* Circuit creation public API. */ + +/* Concurrent connection detection public API. */ + +/* General API */ + +/* Called when the consensus has changed. We might have new consensus + * parameters to look at. */ +void +dos_consensus_has_changed(const networkstatus_t *ns) +{ + cc_consensus_has_changed(ns); + conn_consensus_has_changed(ns); + + /* We were already enabled or we just became enabled but either way, set the + * consensus parameters for all subsystems. */ + set_dos_parameters(ns); +} + +/* Return true iff the DoS mitigation subsystem is enabled. */ +int +dos_enabled(void) +{ + return dos_is_enabled(); +} + +/* Free everything from the Denial of Service subsystem. */ +void +dos_free_all(void) +{ + /* Free the circuit creation mitigation subsystem. It is safe to do this + * even if it wasn't initialized. */ + cc_free_all(); + + /* Free the connection mitigation subsystem. It is safe to do this even if + * it wasn't initialized. */ + conn_free_all(); +} + +/* Initialize the Denial of Service subsystem. */ +void +dos_init(void) +{ + /* To initialize, we only need to get the parameters. */ + set_dos_parameters(NULL); +} + diff --git a/src/or/dos.h b/src/or/dos.h new file mode 100644 index 0000000000..dc36aaa406 --- /dev/null +++ b/src/or/dos.h @@ -0,0 +1,120 @@ +/* Copyright (c) 2018, The Tor Project, Inc. */ +/* See LICENSE for licensing information */ + +/* + * \file dos.h + * \brief Header file for dos.c + */ + +#ifndef TOR_DOS_H +#define TOR_DOS_H + +/* Structure that keeps stats of client connection per-IP. */ +typedef struct cc_client_stats_t { + /* Number of allocated circuits remaining for this address. It is + * decremented every time a new circuit is seen for this client address and + * if the count goes to 0, we have a positive detection. */ + uint32_t circuit_bucket; + + /* When was the last time we've refilled the circuit bucket? This is used to + * know if we need to refill the bucket when a new circuit is seen. It is + * synchronized using approx_time(). */ + time_t last_circ_bucket_refill_ts; + + /* This client address was detected to be above the circuit creation rate + * and this timestamp indicates until when it should remain marked as + * detected so we can apply a defense for the address. It is synchronized + * using the approx_time(). */ + time_t marked_until_ts; +} cc_client_stats_t; + +/* This object is a top level object that contains everything related to the + * per-IP client DoS mitigation. Because it is per-IP, it is used in the geoip + * clientmap_entry_t object. */ +typedef struct dos_client_stats_t { + /* Concurrent connection count from the specific address. 2^32 is most + * likely way too big for the amount of allowed file descriptors. */ + uint32_t concurrent_count; + + /* Circuit creation statistics. This is only used if the circuit creation + * subsystem has been enabled (dos_cc_enabled). */ + cc_client_stats_t cc_stats; +} dos_client_stats_t; + +/* General API. */ + +void dos_init(void); +void dos_free_all(void); +void dos_consensus_has_changed(const networkstatus_t *ns); +int dos_enabled(void); + +/* + * Circuit creation DoS mitigation subsystemn interface. + */ + +/* DoSCircuitCreationEnabled default. Disabled by default. */ +#define DOS_CC_ENABLED_DEFAULT 0 +/* DoSCircuitCreationDefenseType maps to the dos_cc_defense_type_t enum. */ +#define DOS_CC_DEFENSE_TYPE_DEFAULT DOS_CC_DEFENSE_REFUSE_CELL +/* DoSCircuitCreationMinConnections default */ +#define DOS_CC_MIN_CONCURRENT_CONN_DEFAULT 3 +/* DoSCircuitCreationRateTenths is 3 per seconds. */ +#define DOS_CC_CIRCUIT_RATE_TENTHS_DEFAULT (3 * 10) +/* DoSCircuitCreationBurst default. */ +#define DOS_CC_CIRCUIT_BURST_DEFAULT 90 +/* DoSCircuitCreationDefenseTimePeriod in seconds. */ +#define DOS_CC_DEFENSE_TIME_PERIOD_DEFAULT (60 * 60) + +/* Type of defense that we can use for the circuit creation DoS mitigation. */ +typedef enum dos_cc_defense_type_t { + /* No defense used. */ + DOS_CC_DEFENSE_NONE = 1, + /* Refuse any cells which means a DESTROY cell will be sent back. */ + DOS_CC_DEFENSE_REFUSE_CELL = 2, + + /* Maximum value that can be used. Useful for the boundaries of the + * consensus parameter. */ + DOS_CC_DEFENSE_MAX = 2, +} dos_cc_defense_type_t; + +/* + * Concurrent connection DoS mitigation interface. + */ + +/* DoSConnectionEnabled default. Disabled by default. */ +#define DOS_CONN_ENABLED_DEFAULT 0 +/* DoSConnectionMaxConcurrentCount default. */ +#define DOS_CONN_MAX_CONCURRENT_COUNT_DEFAULT 100 +/* DoSConnectionDefenseType maps to the dos_conn_defense_type_t enum. */ +#define DOS_CONN_DEFENSE_TYPE_DEFAULT DOS_CONN_DEFENSE_CLOSE + +/* Type of defense that we can use for the concurrent connection DoS + * mitigation. */ +typedef enum dos_conn_defense_type_t { + /* No defense used. */ + DOS_CONN_DEFENSE_NONE = 1, + /* Close immediately the connection meaning refuse it. */ + DOS_CONN_DEFENSE_CLOSE = 2, + + /* Maximum value that can be used. Useful for the boundaries of the + * consensus parameter. */ + DOS_CONN_DEFENSE_MAX = 2, +} dos_conn_defense_type_t; + +#ifdef DOS_PRIVATE + +STATIC uint32_t get_param_conn_max_concurrent_count( + const networkstatus_t *ns); +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); + +MOCK_DECL(STATIC unsigned int, get_param_cc_enabled, + (const networkstatus_t *ns)); +MOCK_DECL(STATIC unsigned int, get_param_conn_enabled, + (const networkstatus_t *ns)); + +#endif /* TOR_DOS_PRIVATE */ + +#endif /* TOR_DOS_H */ + diff --git a/src/or/include.am b/src/or/include.am index ae493b7225..5108a08e53 100644 --- a/src/or/include.am +++ b/src/or/include.am @@ -43,6 +43,7 @@ LIBTOR_A_SOURCES = \ src/or/dirvote.c \ src/or/dns.c \ src/or/dnsserv.c \ + src/or/dos.c \ src/or/fp_pair.c \ src/or/geoip.c \ src/or/entrynodes.c \ @@ -151,6 +152,7 @@ ORHEADERS = \ src/or/dns.h \ src/or/dns_structs.h \ src/or/dnsserv.h \ + src/or/dos.h \ src/or/ext_orport.h \ src/or/fallback_dirs.inc \ src/or/fp_pair.h \ diff --git a/src/or/main.c b/src/or/main.c index 187b255bfb..fcd8dc9024 100644 --- a/src/or/main.c +++ b/src/or/main.c @@ -34,6 +34,7 @@ #include "dirvote.h" #include "dns.h" #include "dnsserv.h" +#include "dos.h" #include "entrynodes.h" #include "geoip.h" #include "hibernate.h" @@ -2989,6 +2990,7 @@ tor_free_all(int postfork) control_free_all(); sandbox_free_getaddrinfo_cache(); protover_free_all(); + dos_free_all(); if (!postfork) { config_free_all(); or_state_free_all(); diff --git a/src/or/networkstatus.c b/src/or/networkstatus.c index 991cf80121..d9ae32560e 100644 --- a/src/or/networkstatus.c +++ b/src/or/networkstatus.c @@ -23,6 +23,7 @@ #include "directory.h" #include "dirserv.h" #include "dirvote.h" +#include "dos.h" #include "entrynodes.h" #include "main.h" #include "microdesc.h" @@ -1502,6 +1503,15 @@ notify_control_networkstatus_changed(const networkstatus_t *old_c, smartlist_free(changed); } +/* Called when the consensus has changed from old_c to new_c. */ +static void +notify_networkstatus_changed(const networkstatus_t *old_c, + const networkstatus_t *new_c) +{ + notify_control_networkstatus_changed(old_c, new_c); + dos_consensus_has_changed(new_c); +} + /** Copy all the ancillary information (like router download status and so on) * from old_c to new_c. */ static void @@ -1826,8 +1836,7 @@ networkstatus_set_current_consensus(const char *consensus, const int is_usable_flavor = flav == usable_consensus_flavor(); if (is_usable_flavor) { - notify_control_networkstatus_changed( - networkstatus_get_latest_consensus(), c); + notify_networkstatus_changed(networkstatus_get_latest_consensus(), c); } if (flav == FLAV_NS) { if (current_ns_consensus) { diff --git a/src/or/or.h b/src/or/or.h index 75a02a531e..2cf9e97356 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -4510,6 +4510,36 @@ typedef struct { /** If 1, we skip all OOS checks. */ int DisableOOSCheck; + + /** Autobool: Is the circuit creation DoS mitigation subsystem enabled? */ + int DoSCircuitCreationEnabled; + /** 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; + /** Maximum allowed burst of circuits. Reaching that value, the address is + * detected as malicious and a defense might be used. */ + int DoSCircuitCreationBurst; + /** When an address is marked as malicous, what defense should be used + * against it. See the dos_cc_defense_type_t enum. */ + int DoSCircuitCreationDefenseType; + /** For how much time (in seconds) the defense is applicable for a malicious + * address. A random time delta is added to the defense time of an address + * which will be between 1 second and half of this value. */ + int DoSCircuitCreationDefenseTimePeriod; + + /** Autobool: Is the DoS connection mitigation subsystem enabled? */ + int DoSConnectionEnabled; + /** Maximum concurrent connection allowed per address. */ + int DoSConnectionMaxConcurrentCount; + /** When an address is reaches the maximum count, what defense should be + * used against it. See the dos_conn_defense_type_t enum. */ + int DoSConnectionDefenseType; + + /** Autobool: Do we refuse single hop client rendezvous? */ + int DoSRefuseSingleHopClientRendezvous; } or_options_t; /** Persistent state for an onion router, as saved to disk. */ From 51fda85c23e5ff2cabbc66ea19b006c4cb04b1e2 Mon Sep 17 00:00:00 2001 From: David Goulet Date: Fri, 19 Jan 2018 13:15:07 -0500 Subject: [PATCH 03/37] geoip: Remember client stats if DoS mitigation is enabled Make the geoip cache track client address if the DoS subsystem is enabled. Signed-off-by: David Goulet --- src/or/geoip.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/or/geoip.c b/src/or/geoip.c index e2a1b1cee4..5f0b04b568 100644 --- a/src/or/geoip.c +++ b/src/or/geoip.c @@ -33,6 +33,7 @@ #include "config.h" #include "control.h" #include "dnsserv.h" +#include "dos.h" #include "geoip.h" #include "routerlist.h" @@ -549,10 +550,14 @@ geoip_note_client_seen(geoip_client_action_t action, clientmap_entry_t *ent; if (action == GEOIP_CLIENT_CONNECT) { - /* Only remember statistics as entry guard or as bridge. */ - if (!options->EntryStatistics && - (!(options->BridgeRelay && options->BridgeRecordUsageByCountry))) - return; + /* Only remember statistics if the DoS mitigation subsystem is enabled. If + * not, only if as entry guard or as bridge. */ + if (!dos_enabled()) { + if (!options->EntryStatistics && + (!(options->BridgeRelay && options->BridgeRecordUsageByCountry))) { + return; + } + } } else { /* Only gather directory-request statistics if configured, and * forcibly disable them on bridge authorities. */ From c05272783d0164363023ddd4b3ee93c2e12c8911 Mon Sep 17 00:00:00 2001 From: David Goulet Date: Thu, 25 Jan 2018 16:05:59 -0500 Subject: [PATCH 04/37] dos: Track new and closed OR client connections Implement a basic connection tracking that counts the number of concurrent connections when they open and close. This commit also adds the circuit creation mitigation data structure that will be needed at later commit to keep track of the circuit rate. Signed-off-by: David Goulet --- src/or/channel.c | 5 +++ src/or/connection.c | 8 +++++ src/or/dos.c | 75 +++++++++++++++++++++++++++++++++++++++++++++ src/or/dos.h | 3 ++ src/or/geoip.h | 5 +++ src/or/or.h | 4 +++ 6 files changed, 100 insertions(+) diff --git a/src/or/channel.c b/src/or/channel.c index f547aea1b3..fdd3f81e81 100644 --- a/src/or/channel.c +++ b/src/or/channel.c @@ -2583,6 +2583,7 @@ channel_do_open_actions(channel_t *chan) if (!router_get_by_id_digest(chan->identity_digest)) { if (channel_get_addr_if_possible(chan, &remote_addr)) { char *transport_name = NULL; + channel_tls_t *tlschan = BASE_CHAN_TO_TLS(chan); if (chan->get_transport_name(chan, &transport_name) < 0) transport_name = NULL; @@ -2590,6 +2591,10 @@ channel_do_open_actions(channel_t *chan) &remote_addr, transport_name, now); tor_free(transport_name); + /* Notify the DoS subsystem of a new client. */ + if (tlschan && tlschan->conn) { + dos_new_client_conn(tlschan->conn); + } } /* Otherwise the underlying transport can't tell us this, so skip it */ } diff --git a/src/or/connection.c b/src/or/connection.c index 8b00d637f6..15f489c6b4 100644 --- a/src/or/connection.c +++ b/src/or/connection.c @@ -78,6 +78,7 @@ #include "dirserv.h" #include "dns.h" #include "dnsserv.h" +#include "dos.h" #include "entrynodes.h" #include "ext_orport.h" #include "geoip.h" @@ -687,6 +688,13 @@ connection_free,(connection_t *conn)) "connection_free"); } #endif + + /* Notify the circuit creation DoS mitigation subsystem that an OR client + * connection has been closed. And only do that if we track it. */ + if (conn->type == CONN_TYPE_OR) { + dos_close_client_conn(TO_OR_CONN(conn)); + } + connection_unregister_events(conn); connection_free_(conn); } diff --git a/src/or/dos.c b/src/or/dos.c index 4b5983d16d..d1a2c6a281 100644 --- a/src/or/dos.c +++ b/src/or/dos.c @@ -246,6 +246,81 @@ dos_is_enabled(void) /* General API */ +/* Called when a new client connection has been established on the given + * address. */ +void +dos_new_client_conn(or_connection_t *or_conn) +{ + clientmap_entry_t *entry; + + tor_assert(or_conn); + + /* Past that point, we know we have at least one DoS detection subsystem + * enabled so we'll start allocating stuff. */ + if (!dos_is_enabled()) { + goto end; + } + + /* We are only interested in client connection from the geoip cache. */ + entry = geoip_lookup_client(&or_conn->real_addr, NULL, + GEOIP_CLIENT_CONNECT); + if (BUG(entry == NULL)) { + /* Should never happen because we note down the address in the geoip + * cache before this is called. */ + goto end; + } + + entry->dos_stats.concurrent_count++; + or_conn->tracked_for_dos_mitigation = 1; + log_debug(LD_DOS, "Client address %s has now %u concurrent connections.", + fmt_addr(&or_conn->real_addr), + entry->dos_stats.concurrent_count); + + end: + return; +} + +/* Called when a client connection for the given IP address has been closed. */ +void +dos_close_client_conn(const or_connection_t *or_conn) +{ + clientmap_entry_t *entry; + + tor_assert(or_conn); + + /* We have to decrement the count on tracked connection only even if the + * subsystem has been disabled at runtime because it might be re-enabled + * after and we need to keep a synchronized counter at all time. */ + if (!or_conn->tracked_for_dos_mitigation) { + goto end; + } + + /* We are only interested in client connection from the geoip cache. */ + entry = geoip_lookup_client(&or_conn->real_addr, NULL, + GEOIP_CLIENT_CONNECT); + if (entry == NULL) { + /* This can happen because we can close a connection before the channel + * got to be noted down in the geoip cache. */ + goto end; + } + + /* Extra super duper safety. Going below 0 means an underflow which could + * lead to most likely a false positive. In theory, this should never happen + * but lets be extra safe. */ + if (BUG(entry->dos_stats.concurrent_count == 0)) { + goto end; + } + + entry->dos_stats.concurrent_count--; + log_debug(LD_DOS, "Client address %s has lost a connection. Concurrent " + "connections are now at %u", + fmt_addr(&or_conn->real_addr), + entry->dos_stats.concurrent_count); + + end: + return; +} + /* Called when the consensus has changed. We might have new consensus * parameters to look at. */ void diff --git a/src/or/dos.h b/src/or/dos.h index dc36aaa406..3cc10d3f99 100644 --- a/src/or/dos.h +++ b/src/or/dos.h @@ -48,6 +48,9 @@ void dos_free_all(void); void dos_consensus_has_changed(const networkstatus_t *ns); 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); + /* * Circuit creation DoS mitigation subsystemn interface. */ diff --git a/src/or/geoip.h b/src/or/geoip.h index b80efceb35..aa0fca50f5 100644 --- a/src/or/geoip.h +++ b/src/or/geoip.h @@ -13,6 +13,7 @@ #define TOR_GEOIP_H #include "testsupport.h" +#include "dos.h" #ifdef GEOIP_PRIVATE STATIC int geoip_parse_entry(const char *line, sa_family_t family); @@ -37,6 +38,10 @@ typedef struct clientmap_entry_t { * 4000 CE, please remember to add more bits to last_seen_in_minutes.) */ unsigned int last_seen_in_minutes:30; unsigned int action:2; + + /* This object is used to keep some statistics per client address for the + * DoS mitigation subsystem. */ + dos_client_stats_t dos_stats; } clientmap_entry_t; int should_record_bridge_info(const or_options_t *options); diff --git a/src/or/or.h b/src/or/or.h index 2cf9e97356..454d05ed52 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -1500,6 +1500,10 @@ typedef struct or_connection_t { /** True iff this connection has had its bootstrap failure logged with * control_event_bootstrap_problem. */ unsigned int have_noted_bootstrap_problem:1; + /** True iff this is a client connection and its address has been put in the + * geoip cache and handled by the DoS mitigation subsystem. We use this to + * insure we have a coherent count of concurrent connection. */ + unsigned int tracked_for_dos_mitigation : 1; uint16_t link_proto; /**< What protocol version are we using? 0 for * "none negotiated yet." */ From 97abb3543b858afd27ed857903814175c1dfbf12 Mon Sep 17 00:00:00 2001 From: David Goulet Date: Thu, 25 Jan 2018 16:14:40 -0500 Subject: [PATCH 05/37] dos: Detect circuit creation denial of service Add a function that notifies the DoS subsystem that a new CREATE cell has arrived. The statistics are updated accordingly and the IP address can also be marked as malicious if it is above threshold. At this commit, no defense is applied, just detection with a circuit creation token bucket system. Signed-off-by: David Goulet --- src/or/command.c | 6 ++ src/or/dos.c | 179 +++++++++++++++++++++++++++++++++++++++++++++++ src/or/dos.h | 6 ++ 3 files changed, 191 insertions(+) diff --git a/src/or/command.c b/src/or/command.c index 5866c386e4..d2df55a4be 100644 --- a/src/or/command.c +++ b/src/or/command.c @@ -46,6 +46,7 @@ #include "config.h" #include "control.h" #include "cpuworker.h" +#include "dos.h" #include "hibernate.h" #include "nodelist.h" #include "onion.h" @@ -247,6 +248,11 @@ command_process_create_cell(cell_t *cell, channel_t *chan) (unsigned)cell->circ_id, U64_PRINTF_ARG(chan->global_identifier), chan); + /* First thing we do, even though the cell might be invalid, is inform the + * DoS mitigation subsystem layer of this event. Validation is done by this + * function. */ + dos_cc_new_create_cell(chan); + /* We check for the conditions that would make us drop the cell before * we check for the conditions that would make us send a DESTROY back, * since those conditions would make a DESTROY nonsensical. */ diff --git a/src/or/dos.c b/src/or/dos.c index d1a2c6a281..b83ea60298 100644 --- a/src/or/dos.c +++ b/src/or/dos.c @@ -35,6 +35,9 @@ 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; +/* Keep some stats for the heartbeat so we can report out. */ +static uint32_t cc_num_marked_addrs; + /* * Concurrent connection denial of service mitigation. * @@ -209,6 +212,117 @@ cc_consensus_has_changed(const networkstatus_t *ns) } } +/** Return the number of circuits we allow per second under the current + * configuration. */ +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; +} + +/* Given the circuit creation client statistics object, refill the circuit + * bucket if needed. This also works if the bucket was never filled in the + * first place. The addr is only used for logging purposes. */ +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; + + tor_assert(stats); + tor_assert(addr); + + now = approx_time(); + + /* 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; + } + + /* 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. */ + 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; + + /* 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 + * 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 + * when we should have been refilling it. But the fact that we didn't notice + * 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); + } + + /* 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. */ + 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); + log_debug(LD_DOS, "DoS address %s has its circuit bucket value: %" PRIu32 + ". Filling it to %" PRIu32 ". Circuit rate is %" PRIu32, + fmt_addr(addr), stats->circuit_bucket, new_circuit_bucket_count, + circuit_rate); + + stats->circuit_bucket = new_circuit_bucket_count; + stats->last_circ_bucket_refill_ts = now; + return; +} + +/* Return true iff the circuit bucket is down to 0 and the number of + * concurrent connections is greater or equal the minimum threshold set the + * consensus parameter. */ +static int +cc_has_exhausted_circuits(const dos_client_stats_t *stats) +{ + tor_assert(stats); + return stats->cc_stats.circuit_bucket == 0 && + stats->concurrent_count >= dos_cc_min_concurrent_conn; +} + +/* Mark client address by setting a timestamp in the stats object which tells + * us until when it is marked as positively detected. */ +static void +cc_mark_client(cc_client_stats_t *stats) +{ + tor_assert(stats); + /* We add a random offset of a maximum of half the defense time so it is + * less predictable. */ + stats->marked_until_ts = + approx_time() + dos_cc_defense_time_period + + crypto_rand_int_range(1, dos_cc_defense_time_period / 2); +} + /* Concurrent connection private API. */ /* Free everything for the connection DoS mitigation subsystem. */ @@ -242,6 +356,71 @@ dos_is_enabled(void) /* Circuit creation public API. */ +/* Called when a CREATE cell is received from the given channel. */ +void +dos_cc_new_create_cell(channel_t *chan) +{ + tor_addr_t addr; + clientmap_entry_t *entry; + + tor_assert(chan); + + /* Skip everything if not enabled. */ + if (!dos_cc_enabled) { + 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; + } + + /* General comment. Even though the client can already be marked as + * malicious, we continue to track statistics. If it keeps going above + * threshold while marked, the defense period time will grow longer. There + * is really no point at unmarking a client that keeps DoSing us. */ + + /* First of all, we'll try to refill the circuit bucket opportunistically + * before we assess. */ + cc_stats_refill_bucket(&entry->dos_stats.cc_stats, &addr); + + /* Take a token out of the circuit bucket if we are above 0 so we don't + * underflow the bucket. */ + if (entry->dos_stats.cc_stats.circuit_bucket > 0) { + entry->dos_stats.cc_stats.circuit_bucket--; + } + + /* This is the detection. Assess at every CREATE cell if the client should + * get marked as malicious. This should be kept as fast as possible. */ + if (cc_has_exhausted_circuits(&entry->dos_stats)) { + /* If this is the first time we mark this entry, log it a info level. + * Under heavy DDoS, logging each time we mark would results in lots and + * lots of logs. */ + if (entry->dos_stats.cc_stats.marked_until_ts == 0) { + log_debug(LD_DOS, "Detected circuit creation DoS by address: %s", + fmt_addr(&addr)); + cc_num_marked_addrs++; + } + cc_mark_client(&entry->dos_stats.cc_stats); + } + + end: + return; +} + /* Concurrent connection detection public API. */ /* General API */ diff --git a/src/or/dos.h b/src/or/dos.h index 3cc10d3f99..bb8d7d1a79 100644 --- a/src/or/dos.h +++ b/src/or/dos.h @@ -80,6 +80,8 @@ typedef enum dos_cc_defense_type_t { DOS_CC_DEFENSE_MAX = 2, } dos_cc_defense_type_t; +void dos_cc_new_create_cell(channel_t *channel); + /* * Concurrent connection DoS mitigation interface. */ @@ -112,6 +114,10 @@ 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 void cc_stats_refill_bucket(cc_client_stats_t *stats, + const tor_addr_t *addr); + MOCK_DECL(STATIC unsigned int, get_param_cc_enabled, (const networkstatus_t *ns)); MOCK_DECL(STATIC unsigned int, get_param_conn_enabled, From 1bfc91a029839f36e04c8204d1bccaa04a5c2afd Mon Sep 17 00:00:00 2001 From: David Goulet Date: Thu, 25 Jan 2018 16:20:52 -0500 Subject: [PATCH 06/37] 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 --- 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 d2df55a4be..0d2808e236 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 b83ea60298..8c00a2f310 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 bb8d7d1a79..fa86295cf6 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. From acf7ea77d8d76830924a14145afbcf3c95a06b0e Mon Sep 17 00:00:00 2001 From: David Goulet Date: Thu, 25 Jan 2018 16:28:54 -0500 Subject: [PATCH 07/37] dos: Add the connection DoS mitigation subsystem Defend against an address that has reached the concurrent connection count threshold. Signed-off-by: David Goulet --- 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 15f489c6b4..791fd95c27 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 8c00a2f310..7e3a2ab7f9 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 fa86295cf6..cc7749836f 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( From 36a0ae151f8f85c76b4bd91a8fc2871dd88b6005 Mon Sep 17 00:00:00 2001 From: David Goulet Date: Thu, 25 Jan 2018 16:32:28 -0500 Subject: [PATCH 08/37] 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 --- 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 7e3a2ab7f9..d98d3db16a 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 cc7749836f..ec4c033ae3 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 ca0ad7b0d4..441d5043ce 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"); From 14a8b87852887f8c20a424ff32a2b6746105dd6c Mon Sep 17 00:00:00 2001 From: David Goulet Date: Thu, 25 Jan 2018 16:36:05 -0500 Subject: [PATCH 09/37] dos: Add a heartbeat log Signed-off-by: David Goulet --- 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 d98d3db16a..40e88aead0 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 ec4c033ae3..56835169d2 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 fce6a10157..fa2238b9f9 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); From 82de4ea900c5d3513214b127421890595343bfaa Mon Sep 17 00:00:00 2001 From: David Goulet Date: Thu, 25 Jan 2018 09:44:21 -0500 Subject: [PATCH 10/37] 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 --- 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 40e88aead0..5af75ca57d 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 56835169d2..9ce1baddb8 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 5f0b04b568..4e4f6e639a 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); } From c3c2b55decc80028728780422fe2766ec6517246 Mon Sep 17 00:00:00 2001 From: George Kadianakis Date: Thu, 25 Jan 2018 16:38:59 -0500 Subject: [PATCH 11/37] test: Add unit tests for the DoS subsystem Signed-off-by: David Goulet --- 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(-) create mode 100644 src/test/test_dos.c diff --git a/src/or/channel.c b/src/or/channel.c index fdd3f81e81..54e10666d2 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 a711b56d44..bcd345e8d2 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 8ecfaf10c6..91b0a59101 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 9a41b976b8..f66dee2d0a 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 25336ac83e..41df6b1340 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 0000000000..5a8474ad8b --- /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 +}; + From a3714268f659998dc879ed723852440cd8be1b04 Mon Sep 17 00:00:00 2001 From: David Goulet Date: Fri, 26 Jan 2018 09:00:17 -0500 Subject: [PATCH 12/37] dos: Man page entry for DoS mitigation Signed-off-by: David Goulet --- doc/tor.1.txt | 90 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 90 insertions(+) diff --git a/doc/tor.1.txt b/doc/tor.1.txt index 4c5d5359af..a2bbb8ab6e 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 ----------------------- From e58a4fc6cfcdeafc2ebfb61fd3cf6d163ce2436c Mon Sep 17 00:00:00 2001 From: David Goulet Date: Mon, 29 Jan 2018 11:50:11 -0500 Subject: [PATCH 13/37] 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 --- 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 a2bbb8ab6e..58997cdf3d 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 c651c202ec..3b40274339 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 5af75ca57d..a614d12314 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 9ce1baddb8..8695512ea6 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 454d05ed52..024a9cff0f 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; From 9aca7d47306222f2870ec16a7291a8215d6c3316 Mon Sep 17 00:00:00 2001 From: David Goulet Date: Tue, 30 Jan 2018 09:15:33 -0500 Subject: [PATCH 14/37] dos: Add changes file for ticket 24902 Signed-off-by: David Goulet --- changes/ticket24902 | 13 +++++++++++++ 1 file changed, 13 insertions(+) create mode 100644 changes/ticket24902 diff --git a/changes/ticket24902 b/changes/ticket24902 new file mode 100644 index 0000000000..1a2ef95cc9 --- /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. + From b45ae1b00237f0209c4ccce777de59581fda5e39 Mon Sep 17 00:00:00 2001 From: teor Date: Wed, 31 Jan 2018 11:11:08 +1100 Subject: [PATCH 15/37] 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 5a8474ad8b..80abc19377 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); From a09d5f5735abe2e1d16cf0ee9389ae096d5e7ef1 Mon Sep 17 00:00:00 2001 From: teor Date: Wed, 31 Jan 2018 11:13:17 +1100 Subject: [PATCH 16/37] 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 --- src/or/dos.c | 84 +++++++++++++++++++++++++++++++-------------- src/or/dos.h | 2 +- src/test/test_dos.c | 2 +- 3 files changed, 60 insertions(+), 28 deletions(-) diff --git a/src/or/dos.c b/src/or/dos.c index a614d12314..c221e5ecdf 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; + + /* 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 get the circuit rate that is how many circuit are we allowed to do + * per second. */ + circuit_rate = get_circuit_rate_per_second(); /* 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) { + * 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; } - /* 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. */ - 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; - - /* 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 8695512ea6..5d35a2b12e 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 80abc19377..7fe5495603 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); From 1f4a73133cf864774c017e2c50b347727519c18f Mon Sep 17 00:00:00 2001 From: teor Date: Wed, 31 Jan 2018 11:22:20 +1100 Subject: [PATCH 17/37] test: Add unit tests for overflows and underflows in cc_stats_refill_bucket Closes #25094. Signed-off-by: David Goulet --- 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 7fe5495603..071926c287 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(); From 33d9889a2bdd611bcc255c68c43d60b8919ab663 Mon Sep 17 00:00:00 2001 From: Fernando Fernandez Mancera Date: Mon, 22 Jan 2018 15:20:17 +0100 Subject: [PATCH 18/37] 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 --- changes/bug24952 | 5 +++++ src/or/channeltls.c | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) create mode 100644 changes/bug24952 diff --git a/changes/bug24952 b/changes/bug24952 new file mode 100644 index 0000000000..93174c04f5 --- /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 09cca95b64..890646989e 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); From 51839f47650463f59bd2cc84da05d5bc535d699d Mon Sep 17 00:00:00 2001 From: David Goulet Date: Fri, 2 Feb 2018 10:15:28 -0500 Subject: [PATCH 19/37] 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 --- 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(-) create mode 100644 changes/ticket25122 diff --git a/changes/ticket25122 b/changes/ticket25122 new file mode 100644 index 0000000000..2921811b22 --- /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 00c055bbe7..c5e8cdab93 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 070296dd07..42d0c1cfd9 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 29f34ca033..22ce767523 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 9a41b976b8..e2fbfd21b4 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; From 4d812e29b9b1ec88fe268c150a826466b23a8762 Mon Sep 17 00:00:00 2001 From: David Goulet Date: Fri, 2 Feb 2018 13:14:50 -0500 Subject: [PATCH 20/37] 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 --- 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 c5e8cdab93..92db9742e8 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); From e758d659a0bc8b9a0e2bd6a0126755fd1fb58e0a Mon Sep 17 00:00:00 2001 From: David Goulet Date: Fri, 2 Feb 2018 13:24:37 -0500 Subject: [PATCH 21/37] geoip: Add clientmap_entry_new() function Signed-off-by: David Goulet --- 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 92db9742e8..76fca43f67 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); From 3bed8fdb91599b5e7c7946978c6221ba5db85463 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Fri, 2 Feb 2018 15:23:55 -0500 Subject: [PATCH 22/37] 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 071926c287..9496b0735c 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); From 78d6cb58707ff46464c591e45d81e83388427e2c Mon Sep 17 00:00:00 2001 From: David Goulet Date: Fri, 2 Feb 2018 17:04:12 -0500 Subject: [PATCH 23/37] 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 --- src/or/dos.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/or/dos.c b/src/or/dos.c index c221e5ecdf..88f1351a3f 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, From f08fa974600625e4ea0b21d0143d28fe280008d5 Mon Sep 17 00:00:00 2001 From: David Goulet Date: Mon, 5 Feb 2018 10:39:10 -0500 Subject: [PATCH 24/37] geoip: Make geoip_client_cache_total_allocation() return the counter The HT_FOREACH() is insanely heavy on the CPU and this is part of the fast path so make it return the nice memory size counter we added in 4d812e29b9b1ec88. Fixes #25148 Signed-off-by: David Goulet --- src/or/geoip.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/or/geoip.c b/src/or/geoip.c index 20dad5f159..a39366ed13 100644 --- a/src/or/geoip.c +++ b/src/or/geoip.c @@ -769,13 +769,7 @@ geoip_client_cache_handle_oom(time_t now, size_t min_remove_bytes) 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; + return geoip_client_history_cache_size; } /** How many responses are we giving to clients requesting v3 network From 22a5d3dd2ab793336e911d3aceb8cacd278e0f48 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 30 Jan 2018 18:11:16 -0500 Subject: [PATCH 25/37] 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 9496b0735c..6db98b9ed3 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: From 46bd2aed915f17d520f9ff237262d1510fe25e12 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 7 Feb 2018 09:49:35 -0500 Subject: [PATCH 26/37] 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(+) create mode 100644 src/common/address_set.c create mode 100644 src/common/address_set.h diff --git a/src/common/address.c b/src/common/address.c index 773e688554..1bd52d24b6 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 51db42c315..d57abd0d9e 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 0000000000..df7022174c --- /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 0000000000..568528c89e --- /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 40c463c9d9..cb307e9d5f 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 \ From 0640da42696a666382dd569839e98312d720a22a Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 8 Feb 2018 12:13:56 -0500 Subject: [PATCH 27/37] Function to add an ipv4 address to an address_set This is a convenience function, so callers don't need to wrap the IPv4 address. --- src/common/address_set.c | 9 +++++++++ src/common/address_set.h | 2 ++ 2 files changed, 11 insertions(+) diff --git a/src/common/address_set.c b/src/common/address_set.c index df7022174c..6fa942b0dc 100644 --- a/src/common/address_set.c +++ b/src/common/address_set.c @@ -97,6 +97,15 @@ address_set_add(address_set_t *set, const struct tor_addr_t *addr) } } +/** As address_set_add(), but take an ipv4 address in host order. */ +void +address_set_add_ipv4h(address_set_t *set, uint32_t addr) +{ + tor_addr_t a; + tor_addr_from_ipv4h(&a, addr); + address_set_add(set, &a); +} + /** * Return true if addr if a member of set. (And probably, * return false if addr is not a member of set.) diff --git a/src/common/address_set.h b/src/common/address_set.h index 568528c89e..aedf17fc66 100644 --- a/src/common/address_set.h +++ b/src/common/address_set.h @@ -14,6 +14,7 @@ #define TOR_ADDRESS_SET_H #include "orconfig.h" +#include "torint.h" /** * An address_set_t represents a set of tor_addr_t values. The implementation @@ -26,6 +27,7 @@ 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); +void address_set_add_ipv4h(address_set_t *set, uint32_t addr); int address_set_probably_contains(address_set_t *set, const struct tor_addr_t *addr); From 6892d3292121d02900ac9968e832353ecacca4ad Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 8 Feb 2018 12:14:42 -0500 Subject: [PATCH 28/37] Add an address_set to the nodelist. This set is rebuilt whenever a consensus arrives. In between consensuses, it is add-only. --- src/or/nodelist.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++ src/or/nodelist.h | 1 + 2 files changed, 67 insertions(+) diff --git a/src/or/nodelist.c b/src/or/nodelist.c index 0e9a651818..c2080db12c 100644 --- a/src/or/nodelist.c +++ b/src/or/nodelist.c @@ -14,6 +14,7 @@ #include "or.h" #include "address.h" +#include "address_set.h" #include "config.h" #include "control.h" #include "dirserv.h" @@ -52,6 +53,7 @@ static void count_usable_descriptors(int *num_present, static void update_router_have_minimum_dir_info(void); static double get_frac_paths_needed_for_circs(const or_options_t *options, const networkstatus_t *ns); +static void node_add_to_address_set(const node_t *node); /** A nodelist_t holds a node_t object for every router we're "willing to use * for something". Specifically, it should hold a node_t for every node that @@ -63,6 +65,8 @@ typedef struct nodelist_t { /* Hash table to map from node ID digest to node. */ HT_HEAD(nodelist_map, node_t) nodes_by_id; + /* Set of addresses that belong to nodes we believe in. */ + address_set_t *node_addrs; } nodelist_t; static inline unsigned int @@ -150,6 +154,50 @@ node_addrs_changed(node_t *node) node->country = -1; } +/** Add all address information about node to the current address + * set (if there is one). + */ +static void +node_add_to_address_set(const node_t *node) +{ + if (!the_nodelist || !the_nodelist->node_addrs) + return; + + /* These various address sources can be redundant, but it's likely faster + * to add them all than to compare them all for equality. */ + + if (node->rs) { + if (node->rs->addr) + address_set_add_ipv4h(the_nodelist->node_addrs, node->rs->addr); + if (!tor_addr_is_null(&node->rs->ipv6_addr)) + address_set_add(the_nodelist->node_addrs, &node->rs->ipv6_addr); + } + if (node->ri) { + if (node->ri->addr) + address_set_add_ipv4h(the_nodelist->node_addrs, node->ri->addr); + if (!tor_addr_is_null(&node->ri->ipv6_addr)) + address_set_add(the_nodelist->node_addrs, &node->ri->ipv6_addr); + } + if (node->md) { + if (!tor_addr_is_null(&node->md->ipv6_addr)) + address_set_add(the_nodelist->node_addrs, &node->md->ipv6_addr); + } +} + +/** Return true if addr is the address of some node in the nodelist. + * If not, probably return false. */ +int +nodelist_probably_contains_address(const tor_addr_t *addr) +{ + if (BUG(!addr)) + return 0; + + if (!the_nodelist || !the_nodelist->node_addrs) + return 0; + + return address_set_probably_contains(the_nodelist->node_addrs, addr); +} + /** Add ri to an appropriate node in the nodelist. If we replace an * old routerinfo, and ri_old_out is not NULL, set *ri_old_out * to the previous routerinfo. @@ -188,6 +236,8 @@ nodelist_set_routerinfo(routerinfo_t *ri, routerinfo_t **ri_old_out) dirserv_set_node_flags_from_authoritative_status(node, status); } + node_add_to_address_set(node); + return node; } @@ -219,6 +269,9 @@ nodelist_add_microdesc(microdesc_t *md) node->md = md; md->held_by_nodes++; } + + node_add_to_address_set(node); + return node; } @@ -240,6 +293,11 @@ nodelist_set_consensus(networkstatus_t *ns) SMARTLIST_FOREACH(the_nodelist->nodes, node_t *, node, node->rs = NULL); + /* Conservatively estimate that every node will have 2 addresses. */ + const int estimated_addresses = smartlist_len(ns->routerstatus_list) * 2; + address_set_free(the_nodelist->node_addrs); + the_nodelist->node_addrs = address_set_new(estimated_addresses); + SMARTLIST_FOREACH_BEGIN(ns->routerstatus_list, routerstatus_t *, rs) { node_t *node = node_get_or_create(rs->identity_digest); node->rs = rs; @@ -278,6 +336,11 @@ nodelist_set_consensus(networkstatus_t *ns) nodelist_purge(); + /* Now add all the nodes we have to the address set. */ + SMARTLIST_FOREACH_BEGIN(the_nodelist->nodes, node_t *, node) { + node_add_to_address_set(node); + } SMARTLIST_FOREACH_END(node); + if (! authdir) { SMARTLIST_FOREACH_BEGIN(the_nodelist->nodes, node_t *, node) { /* We have no routerstatus for this router. Clear flags so we can skip @@ -430,6 +493,9 @@ nodelist_free_all(void) smartlist_free(the_nodelist->nodes); + address_set_free(the_nodelist->node_addrs); + the_nodelist->node_addrs = NULL; + tor_free(the_nodelist); } diff --git a/src/or/nodelist.h b/src/or/nodelist.h index 71a91e107f..355057f398 100644 --- a/src/or/nodelist.h +++ b/src/or/nodelist.h @@ -22,6 +22,7 @@ const node_t *node_get_by_hex_id(const char *identity_digest); node_t *nodelist_set_routerinfo(routerinfo_t *ri, routerinfo_t **ri_old_out); node_t *nodelist_add_microdesc(microdesc_t *md); void nodelist_set_consensus(networkstatus_t *ns); +int nodelist_probably_contains_address(const tor_addr_t *addr); void nodelist_remove_microdesc(const char *identity_digest, microdesc_t *md); void nodelist_remove_routerinfo(routerinfo_t *ri); From a445327b80478c72093d8f1b0e205a279318f651 Mon Sep 17 00:00:00 2001 From: David Goulet Date: Thu, 8 Feb 2018 14:35:22 -0500 Subject: [PATCH 29/37] test: Add unit tests for addressset.c This also adds one that tests the integration with the nodelist. Signed-off-by: David Goulet --- src/or/nodelist.c | 14 ++- src/or/nodelist.h | 2 + src/test/include.am | 1 + src/test/test.c | 1 + src/test/test.h | 1 + src/test/test_address_set.c | 174 ++++++++++++++++++++++++++++++++++++ 6 files changed, 192 insertions(+), 1 deletion(-) create mode 100644 src/test/test_address_set.c diff --git a/src/or/nodelist.c b/src/or/nodelist.c index c2080db12c..5a02648c5c 100644 --- a/src/or/nodelist.c +++ b/src/or/nodelist.c @@ -275,6 +275,17 @@ nodelist_add_microdesc(microdesc_t *md) return node; } +/* Default value. */ +#define ESTIMATED_ADDRESS_PER_NODE 2 + +/* Return the estimated number of address per node_t. This is used for the + * size of the bloom filter in the nodelist (node_addrs). */ +MOCK_IMPL(int, +get_estimated_address_per_node, (void)) +{ + return ESTIMATED_ADDRESS_PER_NODE; +} + /** Tell the nodelist that the current usable consensus is ns. * This makes the nodelist change all of the routerstatus entries for * the nodes, drop nodes that no longer have enough info to get used, @@ -294,7 +305,8 @@ nodelist_set_consensus(networkstatus_t *ns) node->rs = NULL); /* Conservatively estimate that every node will have 2 addresses. */ - const int estimated_addresses = smartlist_len(ns->routerstatus_list) * 2; + const int estimated_addresses = smartlist_len(ns->routerstatus_list) * + get_estimated_address_per_node(); address_set_free(the_nodelist->node_addrs); the_nodelist->node_addrs = address_set_new(estimated_addresses); diff --git a/src/or/nodelist.h b/src/or/nodelist.h index 355057f398..098f1d1555 100644 --- a/src/or/nodelist.h +++ b/src/or/nodelist.h @@ -125,5 +125,7 @@ void router_dir_info_changed(void); const char *get_dir_info_status_string(void); int count_loading_descriptors_progress(void); +MOCK_DECL(int, get_estimated_address_per_node, (void)); + #endif diff --git a/src/test/include.am b/src/test/include.am index 8ecfaf10c6..cf29d4cb2b 100644 --- a/src/test/include.am +++ b/src/test/include.am @@ -72,6 +72,7 @@ src_test_test_SOURCES = \ src/test/test_accounting.c \ src/test/test_addr.c \ src/test/test_address.c \ + src/test/test_address_set.c \ src/test/test_buffers.c \ src/test/test_cell_formats.c \ src/test/test_cell_queue.c \ diff --git a/src/test/test.c b/src/test/test.c index 9a41b976b8..9b0775ce45 100644 --- a/src/test/test.c +++ b/src/test/test.c @@ -1182,6 +1182,7 @@ struct testgroup_t testgroups[] = { { "accounting/", accounting_tests }, { "addr/", addr_tests }, { "address/", address_tests }, + { "address_set/", address_set_tests }, { "buffer/", buffer_tests }, { "cellfmt/", cell_format_tests }, { "cellqueue/", cell_queue_tests }, diff --git a/src/test/test.h b/src/test/test.h index 25336ac83e..22b207207f 100644 --- a/src/test/test.h +++ b/src/test/test.h @@ -175,6 +175,7 @@ extern const struct testcase_setup_t ed25519_test_setup; extern struct testcase_t accounting_tests[]; extern struct testcase_t addr_tests[]; extern struct testcase_t address_tests[]; +extern struct testcase_t address_set_tests[]; extern struct testcase_t buffer_tests[]; extern struct testcase_t cell_format_tests[]; extern struct testcase_t cell_queue_tests[]; diff --git a/src/test/test_address_set.c b/src/test/test_address_set.c new file mode 100644 index 0000000000..df022f539a --- /dev/null +++ b/src/test/test_address_set.c @@ -0,0 +1,174 @@ +/* Copyright (c) 2017, The Tor Project, Inc. */ +/* See LICENSE for licensing information */ + +#include "or.h" +#include "address_set.h" +#include "microdesc.h" +#include "networkstatus.h" +#include "nodelist.h" +#include "routerlist.h" +#include "torcert.h" + +#include "test.h" + +static networkstatus_t *dummy_ns = NULL; +static networkstatus_t * +mock_networkstatus_get_latest_consensus(void) +{ + return dummy_ns; +} + +static networkstatus_t * +mock_networkstatus_get_latest_consensus_by_flavor(consensus_flavor_t f) +{ + tor_assert(f == FLAV_MICRODESC); + return dummy_ns; +} + +/* Number of address a single node_t can have. Default to the production + * value. This is to control the size of the bloom filter. */ +static int addr_per_node = 2; +static int +mock_get_estimated_address_per_node(void) +{ + return addr_per_node; +} + +static void +test_contains(void *arg) +{ + int ret; + address_set_t *set = NULL; + + (void) arg; + + /* Setup an IPv4 and IPv6 addresses. */ + tor_addr_t addr_v6; + tor_addr_parse(&addr_v6, "1:2:3:4::"); + tor_addr_t addr_v4; + tor_addr_parse(&addr_v4, "42.42.42.42"); + uint32_t ipv4h = tor_addr_to_ipv4h(&addr_v4); + + /* Make it very big so the chance of failing the contain test will be + * extremely rare. */ + set = address_set_new(1024); + tt_assert(set); + + /* Add and lookup IPv6. */ + address_set_add(set, &addr_v6); + ret = address_set_probably_contains(set, &addr_v6); + tt_int_op(ret, OP_EQ, 1); + + /* Add and lookup IPv4. */ + address_set_add_ipv4h(set, ipv4h); + ret = address_set_probably_contains(set, &addr_v4); + tt_int_op(ret, OP_EQ, 1); + + /* Try a lookup of rubbish. */ + tor_addr_t dummy_addr; + memset(&dummy_addr, 'A', sizeof(dummy_addr)); + dummy_addr.family = AF_INET; + ret = address_set_probably_contains(set, &dummy_addr); + tt_int_op(ret, OP_EQ, 0); + dummy_addr.family = AF_INET6; + ret = address_set_probably_contains(set, &dummy_addr); + tt_int_op(ret, OP_EQ, 0); + + done: + address_set_free(set); +} + +static void +test_nodelist(void *arg) +{ + int ret; + routerstatus_t *rs = NULL; microdesc_t *md = NULL; routerinfo_t *ri = NULL; + + (void) arg; + + MOCK(networkstatus_get_latest_consensus, + mock_networkstatus_get_latest_consensus); + MOCK(networkstatus_get_latest_consensus_by_flavor, + mock_networkstatus_get_latest_consensus_by_flavor); + MOCK(get_estimated_address_per_node, + mock_get_estimated_address_per_node); + + dummy_ns = tor_malloc_zero(sizeof(*dummy_ns)); + dummy_ns->flavor = FLAV_MICRODESC; + dummy_ns->routerstatus_list = smartlist_new(); + + tor_addr_t addr_v4, addr_v6, dummy_addr; + tor_addr_parse(&addr_v4, "42.42.42.42"); + uint32_t ipv4h = tor_addr_to_ipv4h(&addr_v4); + tor_addr_parse(&addr_v6, "1:2:3:4::"); + memset(&dummy_addr, 'A', sizeof(dummy_addr)); + + /* This will make the nodelist bloom filter very large + * (the_nodelist->node_addrs) so we will fail the contain test rarely. */ + addr_per_node = 1024; + + /* No node no nothing. The lookups should be empty. */ + nodelist_set_consensus(dummy_ns); + + /* The address set should be empty. */ + ret = nodelist_probably_contains_address(&addr_v4); + tt_int_op(ret, OP_EQ, 0); + ret = nodelist_probably_contains_address(&addr_v6); + tt_int_op(ret, OP_EQ, 0); + dummy_addr.family = AF_INET; + ret = nodelist_probably_contains_address(&dummy_addr); + tt_int_op(ret, OP_EQ, 0); + dummy_addr.family = AF_INET6; + ret = nodelist_probably_contains_address(&dummy_addr); + tt_int_op(ret, OP_EQ, 0); + + md = tor_malloc_zero(sizeof(*md)); + ri = tor_malloc_zero(sizeof(*ri)); + rs = tor_malloc_zero(sizeof(*rs)); + crypto_rand(rs->identity_digest, sizeof(rs->identity_digest)); + crypto_rand(md->digest, sizeof(md->digest)); + memcpy(rs->descriptor_digest, md->digest, DIGEST256_LEN); + + /* Setup the rs, ri and md addresses. */ + rs->addr = ipv4h; + tor_addr_parse(&rs->ipv6_addr, "1:2:3:4::"); + ri->addr = ipv4h; + tor_addr_parse(&ri->ipv6_addr, "1:2:3:4::"); + tor_addr_parse(&md->ipv6_addr, "1:2:3:4::"); + + /* Add the rs to the consensus becoming a node_t. */ + smartlist_add(dummy_ns->routerstatus_list, rs); + nodelist_set_consensus(dummy_ns); + + /* At this point, the address set should be initialized in the nodelist and + * we should be able to lookup. */ + ret = nodelist_probably_contains_address(&addr_v4); + tt_int_op(ret, OP_EQ, 1); + ret = nodelist_probably_contains_address(&addr_v6); + tt_int_op(ret, OP_EQ, 1); + /* Lookup unknown address. */ + dummy_addr.family = AF_INET; + ret = nodelist_probably_contains_address(&dummy_addr); + tt_int_op(ret, OP_EQ, 0); + dummy_addr.family = AF_INET6; + ret = nodelist_probably_contains_address(&dummy_addr); + tt_int_op(ret, OP_EQ, 0); + + done: + routerstatus_free(rs); routerinfo_free(ri); microdesc_free(md); + smartlist_clear(dummy_ns->routerstatus_list); + networkstatus_vote_free(dummy_ns); + UNMOCK(networkstatus_get_latest_consensus); + UNMOCK(networkstatus_get_latest_consensus_by_flavor); + UNMOCK(get_estimated_address_per_node); +} + +struct testcase_t address_set_tests[] = { + { "contains", test_contains, TT_FORK, + NULL, NULL }, + { "nodelist", test_nodelist, TT_FORK, + NULL, NULL }, + + END_OF_TESTCASES +}; + From 666582a679cdfb2d69620db6aadf55a57d430e23 Mon Sep 17 00:00:00 2001 From: David Goulet Date: Fri, 9 Feb 2018 11:11:41 -0500 Subject: [PATCH 30/37] dos: Exclude known relays from client connection count This is to avoid positively identifying Exit relays if tor client connection comes from them that is reentering the network. One thing to note is that this is done only in the DoS subsystem but we'll still add it to the geoip cache as a "client" seen. This is done that way so to avoid as much as possible changing the current behavior of the geoip client cache since this is being backported. Closes #25193 Signed-off-by: David Goulet --- src/or/dos.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/or/dos.c b/src/or/dos.c index 88f1351a3f..9e8a7a9abe 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 "nodelist.h" #include "router.h" #include "dos.h" @@ -664,6 +665,14 @@ dos_new_client_conn(or_connection_t *or_conn) goto end; } + /* We ignore any known address meaning an address of a known relay. The + * reason to do so is because network reentry is possible where a client + * connection comes from an Exit node. Even when we'll fix reentry, this is + * a robust defense to keep in place. */ + if (nodelist_probably_contains_address(&or_conn->real_addr)) { + goto end; + } + /* We are only interested in client connection from the geoip cache. */ entry = geoip_lookup_client(&or_conn->real_addr, NULL, GEOIP_CLIENT_CONNECT); From 1a4fc9cddf27595db6f5da981a557f768fa32f66 Mon Sep 17 00:00:00 2001 From: David Goulet Date: Fri, 9 Feb 2018 11:31:01 -0500 Subject: [PATCH 31/37] test: DoS test to make sure we exclude known relays Part of #25193 Signed-off-by: David Goulet --- src/test/test_dos.c | 103 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 103 insertions(+) diff --git a/src/test/test_dos.c b/src/test/test_dos.c index 6db98b9ed3..cb9d9e559c 100644 --- a/src/test/test_dos.c +++ b/src/test/test_dos.c @@ -10,9 +10,36 @@ #include "circuitlist.h" #include "geoip.h" #include "channel.h" +#include "microdesc.h" +#include "networkstatus.h" +#include "nodelist.h" +#include "routerlist.h" #include "test.h" #include "log_test_helpers.h" +static networkstatus_t *dummy_ns = NULL; +static networkstatus_t * +mock_networkstatus_get_latest_consensus(void) +{ + return dummy_ns; +} + +static networkstatus_t * +mock_networkstatus_get_latest_consensus_by_flavor(consensus_flavor_t f) +{ + tor_assert(f == FLAV_MICRODESC); + return dummy_ns; +} + +/* Number of address a single node_t can have. Default to the production + * value. This is to control the size of the bloom filter. */ +static int addr_per_node = 2; +static int +mock_get_estimated_address_per_node(void) +{ + return addr_per_node; +} + static unsigned int mock_enable_dos_protection(const networkstatus_t *ns) { @@ -385,10 +412,86 @@ test_dos_bucket_refill(void *arg) dos_free_all(); } +/* Test if we avoid counting a known relay. */ +static void +test_known_relay(void *arg) +{ + clientmap_entry_t *entry = NULL; + routerstatus_t *rs = NULL; microdesc_t *md = NULL; routerinfo_t *ri = NULL; + + (void) arg; + + MOCK(networkstatus_get_latest_consensus, + mock_networkstatus_get_latest_consensus); + MOCK(networkstatus_get_latest_consensus_by_flavor, + mock_networkstatus_get_latest_consensus_by_flavor); + MOCK(get_estimated_address_per_node, + mock_get_estimated_address_per_node); + MOCK(get_param_cc_enabled, mock_enable_dos_protection); + + dos_init(); + + dummy_ns = tor_malloc_zero(sizeof(*dummy_ns)); + dummy_ns->flavor = FLAV_MICRODESC; + dummy_ns->routerstatus_list = smartlist_new(); + + /* Setup an OR conn so we can pass it to the DoS subsystem. */ + or_connection_t or_conn; + tor_addr_parse(&or_conn.real_addr, "42.42.42.42"); + + rs = tor_malloc_zero(sizeof(*rs)); + rs->addr = tor_addr_to_ipv4h(&or_conn.real_addr); + crypto_rand(rs->identity_digest, sizeof(rs->identity_digest)); + smartlist_add(dummy_ns->routerstatus_list, rs); + + /* This will make the nodelist bloom filter very large + * (the_nodelist->node_addrs) so we will fail the contain test rarely. */ + addr_per_node = 1024; + nodelist_set_consensus(dummy_ns); + + /* We have now a node in our list so we'll make sure we don't count it as a + * client connection. */ + geoip_note_client_seen(GEOIP_CLIENT_CONNECT, &or_conn.real_addr, NULL, 0); + /* Suppose we have 5 connections in rapid succession, the counter should + * always be 0 because we should ignore this. */ + dos_new_client_conn(&or_conn); + dos_new_client_conn(&or_conn); + dos_new_client_conn(&or_conn); + dos_new_client_conn(&or_conn); + dos_new_client_conn(&or_conn); + entry = geoip_lookup_client(&or_conn.real_addr, NULL, GEOIP_CLIENT_CONNECT); + tt_assert(entry); + /* We should have a count of 0. */ + tt_uint_op(entry->dos_stats.concurrent_count, OP_EQ, 0); + + /* To make sure that his is working properly, make a unknown client + * connection and see if we do get it. */ + tor_addr_parse(&or_conn.real_addr, "42.42.42.43"); + geoip_note_client_seen(GEOIP_CLIENT_CONNECT, &or_conn.real_addr, NULL, 0); + dos_new_client_conn(&or_conn); + dos_new_client_conn(&or_conn); + entry = geoip_lookup_client(&or_conn.real_addr, NULL, GEOIP_CLIENT_CONNECT); + tt_assert(entry); + /* We should have a count of 2. */ + tt_uint_op(entry->dos_stats.concurrent_count, OP_EQ, 2); + + done: + routerstatus_free(rs); routerinfo_free(ri); microdesc_free(md); + smartlist_clear(dummy_ns->routerstatus_list); + networkstatus_vote_free(dummy_ns); + dos_free_all(); + UNMOCK(networkstatus_get_latest_consensus); + UNMOCK(networkstatus_get_latest_consensus_by_flavor); + UNMOCK(get_estimated_address_per_node); + UNMOCK(get_param_cc_enabled); +} + 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 }, + { "known_relay" , test_known_relay, TT_FORK, + NULL, NULL }, END_OF_TESTCASES }; From 99fbbc6c478d346a469e61663a319c8cf03fec44 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Mon, 12 Feb 2018 10:59:46 -0500 Subject: [PATCH 32/37] Fix a typo in an address_set.c comment. --- src/common/address_set.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/common/address_set.c b/src/common/address_set.c index 6fa942b0dc..4924cb65c2 100644 --- a/src/common/address_set.c +++ b/src/common/address_set.c @@ -22,7 +22,7 @@ /** 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. */ + * value, since we split the siphash output into two 32-bit values. */ #define N_BITS_PER_ITEM (N_HASHES * 2) /* XXXX This code is largely duplicated with digestset_t. We should merge From 1555946e202fef523b35e169c90892b57caea766 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Mon, 12 Feb 2018 11:08:33 -0500 Subject: [PATCH 33/37] Have tor_addr hashes return a randomized hash for AF_UNSPEC. We don't expect this to come up very much, but we may as well make sure that the value isn't predictable (as we do for the other addresses) in case the issue ever comes up. Spotted by teor. --- src/common/address.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/common/address.c b/src/common/address.c index 1bd52d24b6..68ad639411 100644 --- a/src/common/address.c +++ b/src/common/address.c @@ -1181,6 +1181,9 @@ tor_addr_compare_masked(const tor_addr_t *addr1, const tor_addr_t *addr2, } } +/** Input for siphash, to produce some output for an unspec value. */ +static const uint32_t unspec_hash_input[] = { 0x4e4df09f, 0x92985342 }; + /** Return a hash code based on the address addr. DOCDOC extra */ uint64_t tor_addr_hash(const tor_addr_t *addr) @@ -1189,7 +1192,7 @@ tor_addr_hash(const tor_addr_t *addr) case AF_INET: return siphash24g(&addr->addr.in_addr.s_addr, 4); case AF_UNSPEC: - return 0x4e4d5342; + return siphash24g(unspec_hash_input, sizeof(unspec_hash_input)); case AF_INET6: return siphash24g(&addr->addr.in6_addr.s6_addr, 16); default: @@ -1211,7 +1214,7 @@ tor_addr_keyed_hash(const struct sipkey *key, const tor_addr_t *addr) case AF_INET: return siphash24(&addr->addr.in_addr.s_addr, 4, key); case AF_UNSPEC: - return 0x4e4d5342; + return siphash24(unspec_hash_input, sizeof(unspec_hash_input), key); case AF_INET6: return siphash24(&addr->addr.in6_addr.s6_addr, 16, key); default: From 4fe4f8179fe81244319c7fdec64299b6506434a2 Mon Sep 17 00:00:00 2001 From: David Goulet Date: Tue, 13 Feb 2018 10:29:41 -0500 Subject: [PATCH 34/37] dos: Don't set consensus param if we aren't a public relay We had this safeguard around dos_init() but not when the consensus changes which can modify consensus parameters and possibly enable the DoS mitigation even if tor wasn't a public relay. Fixes #25223 Signed-off-by: David Goulet --- changes/bug25223 | 3 +++ src/or/dos.c | 8 ++++++++ 2 files changed, 11 insertions(+) create mode 100644 changes/bug25223 diff --git a/changes/bug25223 b/changes/bug25223 new file mode 100644 index 0000000000..2a7eb6b25d --- /dev/null +++ b/changes/bug25223 @@ -0,0 +1,3 @@ + o Minor bugfixes (DoS mitigation): + - Make sure we don't modify consensus parameters if we aren't a public + relay when a new consensus arrives. Fixes bug 25223. diff --git a/src/or/dos.c b/src/or/dos.c index 9e8a7a9abe..bfa415e7b5 100644 --- a/src/or/dos.c +++ b/src/or/dos.c @@ -738,6 +738,14 @@ dos_close_client_conn(const or_connection_t *or_conn) void dos_consensus_has_changed(const networkstatus_t *ns) { + /* There are two ways to configure this subsystem, one at startup through + * dos_init() which is called when the options are parsed. And this one + * through the consensus. We don't want to enable any DoS mitigation if we + * aren't a public relay. */ + if (!public_server_mode(get_options())) { + return; + } + cc_consensus_has_changed(ns); conn_consensus_has_changed(ns); From 305e39d0f8bcc39d45c2877495046bd927347106 Mon Sep 17 00:00:00 2001 From: David Goulet Date: Tue, 13 Feb 2018 10:41:21 -0500 Subject: [PATCH 35/37] dos: Add extra safety asserts in cc_stats_refill_bucket() Never allow the function to set a bucket value above the allowed circuit burst. Closes #25202 Signed-off-by: David Goulet --- changes/ticket25202 | 4 ++++ src/or/dos.c | 10 ++++++++++ 2 files changed, 14 insertions(+) create mode 100644 changes/ticket25202 diff --git a/changes/ticket25202 b/changes/ticket25202 new file mode 100644 index 0000000000..5edef44f0b --- /dev/null +++ b/changes/ticket25202 @@ -0,0 +1,4 @@ + o Minor bugfixes (DoS mitigation): + - Add extra safety checks when refilling the circuit creation bucket to + ensure we never set a value that is above the allowed burst. Fixes + ticket 25202. diff --git a/src/or/dos.c b/src/or/dos.c index 9e8a7a9abe..e7f3241ef4 100644 --- a/src/or/dos.c +++ b/src/or/dos.c @@ -309,6 +309,16 @@ 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 larger than the + * burst value */ + tor_assert_nonfatal(new_circuit_bucket_count <= dos_cc_circuit_burst); + /* This function is not allowed to make the bucket count smaller, unless it + * is decreasing it to a newly configured, lower burst value. We allow the + * bucket to stay the same size, in case the circuit rate is zero. */ + tor_assert_nonfatal(new_circuit_bucket_count >= stats->circuit_bucket || + new_circuit_bucket_count == dos_cc_circuit_burst); + 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, From 9cf8d669fa416c151f60cb795555b6ef2ab53ecf Mon Sep 17 00:00:00 2001 From: David Goulet Date: Tue, 13 Feb 2018 10:53:47 -0500 Subject: [PATCH 36/37] man: Document default values if not in the consensus for DoS mitigation Fixes #25236 Signed-off-by: David Goulet --- doc/tor.1.txt | 35 +++++++++++++++++++++-------------- 1 file changed, 21 insertions(+), 14 deletions(-) diff --git a/doc/tor.1.txt b/doc/tor.1.txt index 58997cdf3d..a7ee7d11ca 100644 --- a/doc/tor.1.txt +++ b/doc/tor.1.txt @@ -2454,7 +2454,7 @@ Denial of Service mitigation subsystem. 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. + parameter. If not defined in the consensus, the value is 0. (Default: auto) [[DoSCircuitCreationMinConnections]] **DoSCircuitCreationMinConnections** __NUM__:: @@ -2463,19 +2463,22 @@ Denial of Service mitigation subsystem. 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. + parameter. If not defined in the consensus, the value is 3. (Default: 0) [[DoSCircuitCreationRate]] **DoSCircuitCreationRate** __NUM__:: The allowed circuit creation rate per second applied per client IP - address. If this option is 0, it obeys a consensus parameter. (Default: 0) + address. If this option is 0, it obeys a consensus parameter. If not + defined in the consensus, the value is 3. + (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. + creation DoS. "0" means use the consensus parameter. If not defined in the + consensus, the value is 90. (Default: 0) [[DoSCircuitCreationDefenseType]] **DoSCircuitCreationDefenseType** __NUM__:: @@ -2486,28 +2489,31 @@ Denial of Service mitigation subsystem. 1: No defense. 2: Refuse circuit creation for the DoSCircuitCreationDefenseTimePeriod period of time. + - "0" means use the consensus parameter. + "0" means use the consensus parameter. If not defined in the consensus, + the value is 2. (Default: 0) -[[DoSCircuitCreationDefenseTimePeriod]] **DoSCircuitCreationDefenseTimePeriod** __NUM__:: +[[DoSCircuitCreationDefenseTimePeriod]] **DoSCircuitCreationDefenseTimePeriod** __N__ **seconds**|**minutes**|**hours**:: - 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) + The base time period in seconds that the DoS defense is activated for. The + actual value is selected randomly for each activation from N+1 to 3/2 * N. + "0" means use the consensus parameter. If not defined in the consensus, + the value is 3600 seconds (1 hour). (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. + single IP address. "auto" means use the consensus parameter. If not + defined in the consensus, the value is 0. (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. + applied. "0" means use the consensus parameter. If not defined in the + consensus, the value is 100. (Default: 0) [[DoSConnectionDefenseType]] **DoSConnectionDefenseType** __NUM__:: @@ -2518,7 +2524,8 @@ Denial of Service mitigation subsystem. 1: No defense. 2: Immediately close new connections. + - "0" means use the consensus parameter. + "0" means use the consensus parameter. If not defined in the consensus, + the value is 2. (Default: 0) [[DoSRefuseSingleHopClientRendezvous]] **DoSRefuseSingleHopClientRendezvous** **0**|**1**|**auto**:: @@ -2526,7 +2533,7 @@ Denial of Service mitigation subsystem. 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. + consensus parameter. If not defined in the consensus, the value is 0. (Default: auto) TESTING NETWORK OPTIONS From e7f631478254c38d6d8b1bea65840b4c6429e4f4 Mon Sep 17 00:00:00 2001 From: David Goulet Date: Tue, 13 Feb 2018 14:56:31 -0500 Subject: [PATCH 37/37] Make check-changes happy Signed-off-by: David Goulet --- changes/bug25223 | 3 ++- changes/ticket25202 | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/changes/bug25223 b/changes/bug25223 index 2a7eb6b25d..fdd5563500 100644 --- a/changes/bug25223 +++ b/changes/bug25223 @@ -1,3 +1,4 @@ o Minor bugfixes (DoS mitigation): - Make sure we don't modify consensus parameters if we aren't a public - relay when a new consensus arrives. Fixes bug 25223. + relay when a new consensus arrives. Fixes bug 25223; bugfix on + 0.3.3.2-alpha. diff --git a/changes/ticket25202 b/changes/ticket25202 index 5edef44f0b..ba64abad7b 100644 --- a/changes/ticket25202 +++ b/changes/ticket25202 @@ -1,4 +1,4 @@ o Minor bugfixes (DoS mitigation): - Add extra safety checks when refilling the circuit creation bucket to ensure we never set a value that is above the allowed burst. Fixes - ticket 25202. + bug 25202; bugfix on 0.3.3.2-alpha.