From 6f7bad5d73ead1b43fb3b278c73739e26ffdcccb Mon Sep 17 00:00:00 2001 From: Sara Dickinson Date: Fri, 17 Mar 2017 14:43:26 +0000 Subject: [PATCH] Add new configuration parameters for TLS back off time and connection retries --- src/const-info.c | 4 ++ src/context.c | 78 +++++++++++++++++++++++++++++++----- src/context.h | 4 ++ src/getdns/getdns_extra.h.in | 18 +++++++++ src/libgetdns.symbols | 4 ++ src/stub.c | 3 +- src/types-internal.h | 1 - 7 files changed, 101 insertions(+), 11 deletions(-) diff --git a/src/const-info.c b/src/const-info.c index f105b756..c78f4816 100644 --- a/src/const-info.c +++ b/src/const-info.c @@ -74,6 +74,8 @@ static struct const_info consts_info[] = { { 620, "GETDNS_CONTEXT_CODE_TLS_QUERY_PADDING_BLOCKSIZE", GETDNS_CONTEXT_CODE_TLS_QUERY_PADDING_BLOCKSIZE_TEXT }, { 621, "GETDNS_CONTEXT_CODE_PUBKEY_PINSET", GETDNS_CONTEXT_CODE_PUBKEY_PINSET_TEXT }, { 622, "GETDNS_CONTEXT_CODE_ROUND_ROBIN_UPSTREAMS", GETDNS_CONTEXT_CODE_ROUND_ROBIN_UPSTREAMS_TEXT }, + { 623, "GETDNS_CONTEXT_CODE_TLS_BACKOFF_TIME", GETDNS_CONTEXT_CODE_TLS_BACKOFF_TIME_TEXT }, + { 624, "GETDNS_CONTEXT_CODE_TLS_CONNECTION_RETRIES", GETDNS_CONTEXT_CODE_TLS_CONNECTION_RETRIES_TEXT }, { 700, "GETDNS_CALLBACK_COMPLETE", GETDNS_CALLBACK_COMPLETE_TEXT }, { 701, "GETDNS_CALLBACK_CANCEL", GETDNS_CALLBACK_CANCEL_TEXT }, { 702, "GETDNS_CALLBACK_TIMEOUT", GETDNS_CALLBACK_TIMEOUT_TEXT }, @@ -162,6 +164,8 @@ static struct const_name_info consts_name_info[] = { { "GETDNS_CONTEXT_CODE_SUFFIX", 608 }, { "GETDNS_CONTEXT_CODE_TIMEOUT", 616 }, { "GETDNS_CONTEXT_CODE_TLS_AUTHENTICATION", 618 }, + { "GETDNS_CONTEXT_CODE_TLS_BACKOFF_TIME", 623 }, + { "GETDNS_CONTEXT_CODE_TLS_CONNECTION_RETRIES", 624 }, { "GETDNS_CONTEXT_CODE_TLS_QUERY_PADDING_BLOCKSIZE", 620 }, { "GETDNS_CONTEXT_CODE_UPSTREAM_RECURSIVE_SERVERS", 603 }, { "GETDNS_DNSSEC_BOGUS", 401 }, diff --git a/src/context.c b/src/context.c index 2bc6963a..d80e5091 100644 --- a/src/context.c +++ b/src/context.c @@ -89,9 +89,6 @@ typedef unsigned short in_port_t; #define GETDNS_STR_PORT_ZERO "0" #define GETDNS_STR_PORT_DNS "53" #define GETDNS_STR_PORT_DNS_OVER_TLS "853" -/* How long to wait in seconds before re-trying a connection based backed-off - upstream. Using 1 hour for all transports - based on RFC7858 value for for TLS.*/ -#define BACKOFF_RETRY 3600 #ifdef HAVE_PTHREADS static pthread_mutex_t ssl_init_lock = PTHREAD_MUTEX_INITIALIZER; @@ -648,6 +645,8 @@ upstreams_create(getdns_context *context, size_t size) r->count = 0; r->current_udp = 0; r->current_stateful = 0; + r->tls_backoff_time = context->tls_backoff_time; + r->tls_connection_retries = context->tls_connection_retries; return r; } @@ -744,16 +743,16 @@ _getdns_upstream_shutdown(getdns_upstream *upstream) Leave choice between working upstreams to the stub. This back-off should be time based for TLS according to RFC7858. For now, use the same basis if we simply can't get TCP service either.*/ - + uint16_t conn_retries = upstream->upstreams->tls_connection_retries; /* [TLS1]TODO: This arbitrary logic at the moment - review and improve!*/ - if (upstream->conn_setup_failed >= GETDNS_CONN_ATTEMPTS || - (upstream->conn_shutdowns >= GETDNS_CONN_ATTEMPTS*GETDNS_TRANSPORT_FAIL_MULT - && upstream->total_responses == 0) || - (upstream->conn_completed >= GETDNS_CONN_ATTEMPTS && + if (upstream->conn_setup_failed >= conn_retries + || (upstream->conn_shutdowns >= conn_retries*GETDNS_TRANSPORT_FAIL_MULT + && upstream->total_responses == 0) + || (upstream->conn_completed >= conn_retries && upstream->total_responses == 0 && upstream->total_timeouts > GETDNS_TRANSPORT_FAIL_MULT)) { upstream->conn_state = GETDNS_CONN_BACKOFF; - upstream->conn_retry_time = time(NULL) + BACKOFF_RETRY; + upstream->conn_retry_time = time(NULL) + upstream->upstreams->tls_backoff_time; upstream->total_responses = 0; upstream->total_timeouts = 0; upstream->conn_completed = 0; @@ -1430,6 +1429,8 @@ getdns_context_create_with_extended_memory_functions( result->tls_auth = GETDNS_AUTHENTICATION_NONE; result->tls_auth_min = GETDNS_AUTHENTICATION_NONE; result->round_robin_upstreams = 0; + result->tls_backoff_time = 3600; + result->tls_connection_retries = 2; result->limit_outstanding_queries = 0; /* unbound context is initialized here */ @@ -2053,6 +2054,41 @@ getdns_context_set_round_robin_upstreams(getdns_context *context, uint8_t value) return GETDNS_RETURN_GOOD; } /* getdns_context_set_round_robin_upstreams */ +/* + * getdns_context_set_tls_backoff_time + * + */ +getdns_return_t +getdns_context_set_tls_backoff_time(getdns_context *context, uint16_t value) +{ + RETURN_IF_NULL(context, GETDNS_RETURN_INVALID_PARAMETER); + /* Value is in seconds. Should we have a lower limit? 1 second?*/ + context->tls_backoff_time = value; + + dispatch_updated(context, GETDNS_CONTEXT_CODE_TLS_BACKOFF_TIME); + + return GETDNS_RETURN_GOOD; +} /* getdns_context_set_tls_backoff_time */ + +/* + * getdns_context_set_tls_connection_retries + * + */ +getdns_return_t +getdns_context_set_tls_connection_retries(getdns_context *context, uint16_t value) +{ + RETURN_IF_NULL(context, GETDNS_RETURN_INVALID_PARAMETER); + /* Should we put a sensible upper limit on this? 10?*/ + // if (value > 10) { + // return GETDNS_RETURN_CONTEXT_UPDATE_FAIL; + // } + + context->tls_connection_retries = value; + + dispatch_updated(context, GETDNS_CONTEXT_CODE_TLS_CONNECTION_RETRIES); + + return GETDNS_RETURN_GOOD; +} /* getdns_context_set_tls_connection retries */ #ifdef HAVE_LIBUNBOUND static void @@ -3493,6 +3529,10 @@ _get_context_settings(getdns_context* context) context->tls_auth) || getdns_dict_set_int(result, "round_robin_upstreams", context->round_robin_upstreams)) + || getdns_dict_set_int(result, "tls_backoff_time", + context->tls_backoff_time) + || getdns_dict_set_int(result, "tls_connection_retries", + context->tls_connection_retries)) goto error; /* list fields */ @@ -3797,6 +3837,24 @@ getdns_context_get_round_robin_upstreams(getdns_context *context, return GETDNS_RETURN_GOOD; } +getdns_return_t +getdns_context_get_tls_backoff_time(getdns_context *context, + uint16_t* value) { + RETURN_IF_NULL(context, GETDNS_RETURN_INVALID_PARAMETER); + RETURN_IF_NULL(value, GETDNS_RETURN_INVALID_PARAMETER); + *value = context->tls_backoff_time; + return GETDNS_RETURN_GOOD; +} + +getdns_return_t +getdns_context_get_tls_connection_retries(getdns_context *context, + uint16_t* value) { + RETURN_IF_NULL(context, GETDNS_RETURN_INVALID_PARAMETER); + RETURN_IF_NULL(value, GETDNS_RETURN_INVALID_PARAMETER); + *value = context->tls_connection_retries; + return GETDNS_RETURN_GOOD; +} + getdns_return_t getdns_context_get_limit_outstanding_queries(getdns_context *context, uint16_t* value) { @@ -4193,6 +4251,8 @@ _getdns_context_config_setting(getdns_context *context, CONTEXT_SETTING_INT(edns_client_subnet_private) CONTEXT_SETTING_INT(tls_authentication) CONTEXT_SETTING_INT(round_robin_upstreams) + CONTEXT_SETTING_INT(tls_backoff_time) + CONTEXT_SETTING_INT(tls_connection_retries) CONTEXT_SETTING_INT(tls_query_padding_blocksize) /**************************************/ diff --git a/src/context.h b/src/context.h index 3f4bf5df..91250648 100644 --- a/src/context.h +++ b/src/context.h @@ -218,6 +218,8 @@ typedef struct getdns_upstreams { size_t count; size_t current_udp; size_t current_stateful; + uint16_t tls_backoff_time; + uint16_t tls_connection_retries; getdns_upstream upstreams[]; } getdns_upstreams; @@ -250,6 +252,8 @@ struct getdns_context { getdns_tls_authentication_t tls_auth; /* What user requested for TLS*/ getdns_tls_authentication_t tls_auth_min; /* Derived minimum auth allowed*/ uint8_t round_robin_upstreams; + uint16_t tls_backoff_time; + uint16_t tls_connection_retries; getdns_transport_list_t *dns_transports; size_t dns_transport_count; diff --git a/src/getdns/getdns_extra.h.in b/src/getdns/getdns_extra.h.in index 2c03dcfc..8926ede2 100644 --- a/src/getdns/getdns_extra.h.in +++ b/src/getdns/getdns_extra.h.in @@ -78,6 +78,10 @@ extern "C" { #define GETDNS_CONTEXT_CODE_PUBKEY_PINSET_TEXT "Change related to getdns_context_set_pubkey_pinset" #define GETDNS_CONTEXT_CODE_ROUND_ROBIN_UPSTREAMS 622 #define GETDNS_CONTEXT_CODE_ROUND_ROBIN_UPSTREAMS_TEXT "Change related to getdns_context_set_pubkey_pinset" +#define GETDNS_CONTEXT_CODE_TLS_BACKOFF_TIME 623 +#define GETDNS_CONTEXT_CODE_TLS_BACKOFF_TIME_TEXT "Change related to getdns_context_set_pubkey_pinset" +#define GETDNS_CONTEXT_CODE_TLS_CONNECTION_RETRIES 624 +#define GETDNS_CONTEXT_CODE_TLS_CONNECTION_RETRIES_TEXT "Change related to getdns_context_set_pubkey_pinset" /** @} */ @@ -270,6 +274,12 @@ getdns_context_set_tls_authentication( getdns_return_t getdns_context_set_round_robin_upstreams(getdns_context *context, uint8_t value); +getdns_return_t +getdns_context_set_tls_backoff_time(getdns_context *context, uint16_t value); + +getdns_return_t +getdns_context_set_tls_connection_reuse(getdns_context *context, uint16_t value); + getdns_return_t getdns_context_set_edns_client_subnet_private(getdns_context *context, uint8_t value); @@ -365,6 +375,14 @@ getdns_return_t getdns_context_get_round_robin_upstreams(getdns_context *context, uint8_t* value); +getdns_return_t +getdns_context_get_tls_backoff_time(getdns_context *context, + uint16_t* value); + +getdns_return_t +getdns_context_get_tls_connection_retries(getdns_context *context, + uint16_t* value); + /** * Get the currently registered callback function and user defined argument * for context changes. diff --git a/src/libgetdns.symbols b/src/libgetdns.symbols index be63ec40..069a97e7 100644 --- a/src/libgetdns.symbols +++ b/src/libgetdns.symbols @@ -29,6 +29,8 @@ getdns_context_get_resolution_type getdns_context_get_suffix getdns_context_get_timeout getdns_context_get_tls_authentication +getdns_context_get_tls_backoff_time +getdns_context_get_tls_connection_retries getdns_context_get_tls_query_padding_blocksize getdns_context_get_round_robin_upstreams getdns_context_get_update_callback @@ -60,6 +62,8 @@ getdns_context_set_return_dnssec_status getdns_context_set_suffix getdns_context_set_timeout getdns_context_set_tls_authentication +getdns_context_set_tls_backoff_time +getdns_context_set_tls_connection_retries getdns_context_set_tls_query_padding_blocksize getdns_context_set_round_robin_upstreams getdns_context_set_update_callback diff --git a/src/stub.c b/src/stub.c index 6931e418..790b74cd 100644 --- a/src/stub.c +++ b/src/stub.c @@ -1669,7 +1669,8 @@ upstream_working_ok(getdns_upstream *upstream) { /* [TLS1]TODO: This arbitrary logic at the moment - review and improve!*/ return (upstream->responses_timeouts > - upstream->responses_received*GETDNS_CONN_ATTEMPTS ? 0 : 1); + upstream->responses_received* + upstream->upstreams->tls_connection_retries ? 0 : 1); } static int diff --git a/src/types-internal.h b/src/types-internal.h index 78f1935a..17e5b3fb 100644 --- a/src/types-internal.h +++ b/src/types-internal.h @@ -124,7 +124,6 @@ struct getdns_upstream; #define GETDNS_TRANSPORTS_MAX 3 #define GETDNS_UPSTREAM_TRANSPORTS 2 -#define GETDNS_CONN_ATTEMPTS 2 #define GETDNS_TRANSPORT_FAIL_MULT 5