From a2efd8f6c17e0b90df569efb439ce53553827005 Mon Sep 17 00:00:00 2001 From: Willem Toorop Date: Sat, 25 Mar 2017 19:36:20 +0100 Subject: [PATCH 01/21] Report peer certificate in call_reporting --- src/request-internal.c | 2 ++ src/stub.c | 6 ++++++ src/types-internal.h | 1 + src/util-internal.c | 9 +++++++++ 4 files changed, 18 insertions(+) diff --git a/src/request-internal.c b/src/request-internal.c index a74fe8f0..d52ad585 100644 --- a/src/request-internal.c +++ b/src/request-internal.c @@ -182,6 +182,8 @@ network_req_init(getdns_network_req *net_req, getdns_dns_req *owner, net_req->write_queue_tail = NULL; /* Some fields to record info for return_call_reporting */ net_req->debug_tls_auth_status = GETDNS_AUTH_NONE; + net_req->debug_tls_peer_cert.size = 0; + net_req->debug_tls_peer_cert.data = NULL; net_req->debug_udp = 0; /* Scheduling, touch only via _getdns_netreq_change_state! diff --git a/src/stub.c b/src/stub.c index 29112cc3..17b7e1d6 100644 --- a/src/stub.c +++ b/src/stub.c @@ -1618,6 +1618,7 @@ upstream_write_cb(void *userarg) getdns_upstream *upstream = (getdns_upstream *)userarg; getdns_network_req *netreq = upstream->write_queue; int q; + X509 *cert; if (!netreq) { GETDNS_CLEAR_EVENT(upstream->loop, &upstream->event); @@ -1672,8 +1673,13 @@ upstream_write_cb(void *userarg) return; default: + cert = SSL_get_peer_certificate(netreq->upstream->tls_obj); + assert(netreq->debug_tls_peer_cert.data == NULL); + /* Need this because auth status is reset on connection close */ netreq->debug_tls_auth_status = netreq->upstream->tls_auth_state; + netreq->debug_tls_peer_cert.size = i2d_X509( + cert, &netreq->debug_tls_peer_cert.data); upstream->queries_sent++; netreq->query_id = (uint16_t) q; /* Unqueue the netreq from the write_queue */ diff --git a/src/types-internal.h b/src/types-internal.h index 57286bed..5e1961ac 100644 --- a/src/types-internal.h +++ b/src/types-internal.h @@ -240,6 +240,7 @@ typedef struct getdns_network_req uint64_t debug_start_time; uint64_t debug_end_time; getdns_auth_state_t debug_tls_auth_status; + getdns_bindata debug_tls_peer_cert; size_t debug_udp; /* When more space is needed for the wire_data response than is diff --git a/src/util-internal.c b/src/util-internal.c index 48242eb1..5c1d92c7 100644 --- a/src/util-internal.c +++ b/src/util-internal.c @@ -904,6 +904,15 @@ _getdns_create_call_reporting_dict( getdns_dict_destroy(netreq_debug); return NULL; } + if (getdns_dict_set_bindata(netreq_debug, "tls_peer_cert", + &netreq->debug_tls_peer_cert)) { + + getdns_dict_destroy(netreq_debug); + return NULL; + } + netreq->debug_tls_peer_cert.size = 0; + OPENSSL_free(netreq->debug_tls_peer_cert.data); + return netreq_debug; } From 5f6e47d091a15af0f4c5f7aa592ca4abff5b89be Mon Sep 17 00:00:00 2001 From: Willem Toorop Date: Sat, 25 Mar 2017 21:26:05 +0100 Subject: [PATCH 02/21] Only equip with peer cert when transport is TLS --- src/stub.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/stub.c b/src/stub.c index 17b7e1d6..068eeb72 100644 --- a/src/stub.c +++ b/src/stub.c @@ -1673,13 +1673,15 @@ upstream_write_cb(void *userarg) return; default: - cert = SSL_get_peer_certificate(netreq->upstream->tls_obj); - assert(netreq->debug_tls_peer_cert.data == NULL); + if (netreq->upstream->tls_obj && + (cert = SSL_get_peer_certificate(netreq->upstream->tls_obj))) { + assert(netreq->debug_tls_peer_cert.data == NULL); + netreq->debug_tls_peer_cert.size = i2d_X509( + cert, &netreq->debug_tls_peer_cert.data); + } /* Need this because auth status is reset on connection close */ netreq->debug_tls_auth_status = netreq->upstream->tls_auth_state; - netreq->debug_tls_peer_cert.size = i2d_X509( - cert, &netreq->debug_tls_peer_cert.data); upstream->queries_sent++; netreq->query_id = (uint16_t) q; /* Unqueue the netreq from the write_queue */ From 3eb6ebf5e4b7d1af92ec321a608b83870ac1d177 Mon Sep 17 00:00:00 2001 From: Willem Toorop Date: Sat, 25 Mar 2017 21:33:30 +0100 Subject: [PATCH 03/21] Fix memory leak --- src/request-internal.c | 3 +++ src/stub.c | 3 ++- src/util-internal.c | 2 +- 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/request-internal.c b/src/request-internal.c index d52ad585..7663e37c 100644 --- a/src/request-internal.c +++ b/src/request-internal.c @@ -110,6 +110,9 @@ network_req_cleanup(getdns_network_req *net_req) if (net_req->response && (net_req->response < net_req->wire_data || net_req->response > net_req->wire_data+ net_req->wire_data_sz)) GETDNS_FREE(net_req->owner->my_mf, net_req->response); + if (netreq->debug_tls_peer_cert.size && + netreq->debug_tls_peer_cert.data) + OPENSSL_free(netreq->debug_tls_peer_cert.data); } static uint8_t * diff --git a/src/stub.c b/src/stub.c index 068eeb72..c97789f6 100644 --- a/src/stub.c +++ b/src/stub.c @@ -1673,7 +1673,8 @@ upstream_write_cb(void *userarg) return; default: - if (netreq->upstream->tls_obj && + if (netreq->owner->return_call_reporting && + netreq->upstream->tls_obj && (cert = SSL_get_peer_certificate(netreq->upstream->tls_obj))) { assert(netreq->debug_tls_peer_cert.data == NULL); diff --git a/src/util-internal.c b/src/util-internal.c index 5c1d92c7..33c31745 100644 --- a/src/util-internal.c +++ b/src/util-internal.c @@ -912,7 +912,7 @@ _getdns_create_call_reporting_dict( } netreq->debug_tls_peer_cert.size = 0; OPENSSL_free(netreq->debug_tls_peer_cert.data); - + netreq->debug_tls_peer_cert.data = NULL; return netreq_debug; } From 6316c558bc299e523c25074ac23b2dd768829fb7 Mon Sep 17 00:00:00 2001 From: Willem Toorop Date: Sat, 25 Mar 2017 21:45:08 +0100 Subject: [PATCH 04/21] typo --- src/context.c | 5 +++-- src/request-internal.c | 6 +++--- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/context.c b/src/context.c index ba51d90c..c52f770b 100644 --- a/src/context.c +++ b/src/context.c @@ -693,9 +693,10 @@ _getdns_upstreams_dereference(getdns_upstreams *upstreams) upstream->finished_dnsreqs = dnsreq->finished_next; _getdns_context_cancel_request(dnsreq); } + if (upstream->tls_session != NULL) + SSL_SESSION_free(upstream->tls_session); + if (upstream->tls_obj != NULL) { - if (upstream->tls_session != NULL) - SSL_SESSION_free(upstream->tls_session); SSL_shutdown(upstream->tls_obj); SSL_free(upstream->tls_obj); } diff --git a/src/request-internal.c b/src/request-internal.c index 7663e37c..1bd5475e 100644 --- a/src/request-internal.c +++ b/src/request-internal.c @@ -110,9 +110,9 @@ network_req_cleanup(getdns_network_req *net_req) if (net_req->response && (net_req->response < net_req->wire_data || net_req->response > net_req->wire_data+ net_req->wire_data_sz)) GETDNS_FREE(net_req->owner->my_mf, net_req->response); - if (netreq->debug_tls_peer_cert.size && - netreq->debug_tls_peer_cert.data) - OPENSSL_free(netreq->debug_tls_peer_cert.data); + if (net_req->debug_tls_peer_cert.size && + net_req->debug_tls_peer_cert.data) + OPENSSL_free(net_req->debug_tls_peer_cert.data); } static uint8_t * From b7d16e3c8924536f1a488466a39aa151ba558dba Mon Sep 17 00:00:00 2001 From: Willem Toorop Date: Sat, 25 Mar 2017 17:00:02 -0500 Subject: [PATCH 05/21] One more leak --- src/stub.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/stub.c b/src/stub.c index c97789f6..d23fb61f 100644 --- a/src/stub.c +++ b/src/stub.c @@ -1680,6 +1680,7 @@ upstream_write_cb(void *userarg) netreq->debug_tls_peer_cert.size = i2d_X509( cert, &netreq->debug_tls_peer_cert.data); + X509_free(cert); } /* Need this because auth status is reset on connection close */ netreq->debug_tls_auth_status = netreq->upstream->tls_auth_state; From 9fa6ab5994ea72fafceb50b2c99a9b098eeb8663 Mon Sep 17 00:00:00 2001 From: Willem Toorop Date: Sat, 25 Mar 2017 20:22:34 -0500 Subject: [PATCH 06/21] Clang pragma's with clang only --- src/test/check_getdns.c | 4 ++++ src/test/check_getdns_common.c | 4 ++++ src/test/check_getdns_context_set_timeout.h | 4 ++++ src/test/check_getdns_libev.c | 4 ++++ src/test/check_getdns_libevent.c | 4 ++++ src/test/check_getdns_libuv.c | 4 ++++ src/test/check_getdns_transport.h | 4 ++++ 7 files changed, 28 insertions(+) diff --git a/src/test/check_getdns.c b/src/test/check_getdns.c index 8e39fe6e..a437374d 100644 --- a/src/test/check_getdns.c +++ b/src/test/check_getdns.c @@ -30,10 +30,14 @@ #include #include #include +#ifdef __clang__ #pragma clang diagnostic push #pragma clang diagnostic ignored "-Wgnu-zero-variadic-macro-arguments" +#endif #include +#ifdef __clang__ #pragma clang diagnostic pop +#endif #include "getdns/getdns.h" #include "check_getdns_common.h" #include "check_getdns_address.h" diff --git a/src/test/check_getdns_common.c b/src/test/check_getdns_common.c index 5a1409ad..8fe9f8c6 100644 --- a/src/test/check_getdns_common.c +++ b/src/test/check_getdns_common.c @@ -29,10 +29,14 @@ #include #include #include +#ifdef __clang__ #pragma clang diagnostic push #pragma clang diagnostic ignored "-Wgnu-zero-variadic-macro-arguments" +#endif #include +#ifdef __clang__ #pragma clang diagnostic pop +#endif #include "getdns/getdns.h" #include "config.h" #include "check_getdns_common.h" diff --git a/src/test/check_getdns_context_set_timeout.h b/src/test/check_getdns_context_set_timeout.h index c4ab14b8..fe89ee0f 100644 --- a/src/test/check_getdns_context_set_timeout.h +++ b/src/test/check_getdns_context_set_timeout.h @@ -27,10 +27,14 @@ #ifndef _check_getdns_context_set_timeout_h_ #define _check_getdns_context_set_timeout_h_ +#ifdef __clang__ #pragma clang diagnostic push #pragma clang diagnostic ignored "-Wgnu-zero-variadic-macro-arguments" +#endif #include +#ifdef __clang__ #pragma clang diagnostic pop +#endif Suite * getdns_context_set_timeout_suite (void); diff --git a/src/test/check_getdns_libev.c b/src/test/check_getdns_libev.c index 0939f083..ffdc6e22 100644 --- a/src/test/check_getdns_libev.c +++ b/src/test/check_getdns_libev.c @@ -41,10 +41,14 @@ #else #include #endif +#ifdef __clang__ #pragma clang diagnostic push #pragma clang diagnostic ignored "-Wgnu-zero-variadic-macro-arguments" +#endif #include +#ifdef __clang__ #pragma clang diagnostic pop +#endif #include "check_getdns_common.h" void run_event_loop_impl(struct getdns_context* context, void* eventloop) { diff --git a/src/test/check_getdns_libevent.c b/src/test/check_getdns_libevent.c index d3bd4f69..ee59854b 100644 --- a/src/test/check_getdns_libevent.c +++ b/src/test/check_getdns_libevent.c @@ -37,10 +37,14 @@ #include "getdns/getdns_ext_libevent.h" #include "check_getdns_libevent.h" +#ifdef __clang__ #pragma clang diagnostic push #pragma clang diagnostic ignored "-Wgnu-zero-variadic-macro-arguments" +#endif #include +#ifdef __clang__ #pragma clang diagnostic pop +#endif #include "check_getdns_common.h" void run_event_loop_impl(struct getdns_context* context, void* eventloop) { diff --git a/src/test/check_getdns_libuv.c b/src/test/check_getdns_libuv.c index 722406e7..7aa88b51 100644 --- a/src/test/check_getdns_libuv.c +++ b/src/test/check_getdns_libuv.c @@ -37,10 +37,14 @@ #include "getdns/getdns_ext_libuv.h" #include +#ifdef __clang__ #pragma clang diagnostic push #pragma clang diagnostic ignored "-Wgnu-zero-variadic-macro-arguments" +#endif #include +#ifdef __clang__ #pragma clang diagnostic pop +#endif #include "check_getdns_common.h" void run_event_loop_impl(struct getdns_context* context, void* eventloop) { diff --git a/src/test/check_getdns_transport.h b/src/test/check_getdns_transport.h index 6a18d0de..a5496a27 100644 --- a/src/test/check_getdns_transport.h +++ b/src/test/check_getdns_transport.h @@ -27,10 +27,14 @@ #ifndef _check_getdns_transport_h_ #define _check_getdns_transport_h_ +#ifdef __clang__ #pragma clang diagnostic push #pragma clang diagnostic ignored "-Wgnu-zero-variadic-macro-arguments" +#endif #include +#ifdef __clang__ #pragma clang diagnostic pop +#endif Suite * getdns_transport_suite (void); From 6f0b08a4008f36bc937452b302835238d6fec61d Mon Sep 17 00:00:00 2001 From: huitema Date: Sun, 26 Mar 2017 10:07:44 -0500 Subject: [PATCH 07/21] Fixing the select event loop so it does not give up for naked timers in Windows. Making sure the poll event loop works on windows. Fixing the poll event loop so it does not give up for naked timers in Windows. --- src/extension/poll_eventloop.c | 6 ++++++ src/extension/select_eventloop.c | 10 ++++++++++ src/stub.c | 4 ++++ 3 files changed, 20 insertions(+) diff --git a/src/extension/poll_eventloop.c b/src/extension/poll_eventloop.c index b59ad64d..cb629d18 100644 --- a/src/extension/poll_eventloop.c +++ b/src/extension/poll_eventloop.c @@ -30,8 +30,10 @@ #ifdef HAVE_SYS_POLL_H #include #else +#ifndef USE_WINSOCK #include #endif +#endif #ifdef HAVE_SYS_RESOURCE_H #include #endif @@ -402,6 +404,10 @@ poll_eventloop_run_once(getdns_eventloop *loop, int blocking) , poll_timeout ); #ifdef USE_WINSOCK + if (poll_loop->fd_events_free == 0) + { + Sleep(poll_timeout); + } else if (WSAPoll(poll_loop->pfds, poll_loop->fd_events_free, poll_timeout) < 0) { #else if (poll(poll_loop->pfds, poll_loop->fd_events_free, poll_timeout) < 0) { diff --git a/src/extension/select_eventloop.c b/src/extension/select_eventloop.c index 1b889fc1..47769afd 100644 --- a/src/extension/select_eventloop.c +++ b/src/extension/select_eventloop.c @@ -234,6 +234,16 @@ select_eventloop_run_once(getdns_eventloop *loop, int blocking) tv.tv_sec = (long)((timeout - now) / 1000000); tv.tv_usec = (long)((timeout - now) % 1000000); } +#ifdef USE_WINSOCK + if (max_fd == -1) + { + if (timeout != TIMEOUT_FOREVER) + { + uint32_t timeout_ms = (tv.tv_usec / 1000) + (tv.tv_sec * 1000); + Sleep(timeout_ms); + } + } else +#endif if (select(max_fd + 1, &readfds, &writefds, NULL, (timeout == TIMEOUT_FOREVER ? NULL : &tv)) < 0) { perror("select() failed"); diff --git a/src/stub.c b/src/stub.c index 29112cc3..322fa0d7 100644 --- a/src/stub.c +++ b/src/stub.c @@ -36,7 +36,11 @@ # ifdef HAVE_SYS_POLL_H # include # else +#ifdef USE_WINSOCK +#define poll(fdarray, nbsockets, timer) WSAPoll(fdarray, nbsockets, timer) +#else # include +#endif # endif #endif #include "debug.h" From 03efb66991cb234eaf724d8725b9dd90f9100ac8 Mon Sep 17 00:00:00 2001 From: Willem Toorop Date: Sun, 26 Mar 2017 10:16:25 -0500 Subject: [PATCH 08/21] Keep connections open with sync requests too --- src/sync.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/sync.c b/src/sync.c index bfec94c6..75f8cca3 100644 --- a/src/sync.c +++ b/src/sync.c @@ -123,18 +123,22 @@ getdns_sync_data_cleanup(getdns_sync_data *data) * synchronous request. */ GETDNS_CLEAR_EVENT(upstream->loop, &upstream->event); - (*upstream->event.timeout_cb)(upstream->event.userarg); - - /* This should have cleared the event */ - assert(!upstream->event.read_cb && - !upstream->event.write_cb && - !upstream->event.timeout_cb); + if (upstream->conn_state != GETDNS_CONN_OPEN || + upstream->keepalive_timeout == 0) + (*upstream->event.timeout_cb)(upstream->event.userarg); } upstream->loop = data->context->extension; upstream->is_sync_loop = 0; if (upstream->event.read_cb || upstream->event.write_cb) GETDNS_SCHEDULE_EVENT(upstream->loop, upstream->fd, TIMEOUT_FOREVER, &upstream->event); + + else if (upstream->event.timeout_cb && + upstream->conn_state == GETDNS_CONN_OPEN && + upstream->keepalive_timeout != 0) { + GETDNS_SCHEDULE_EVENT(upstream->loop, upstream->fd, + upstream->keepalive_timeout, &upstream->event); + } } } From 9de4d6537bdd2341eb257e822b2d6b1a0005fa06 Mon Sep 17 00:00:00 2001 From: Daniel Kahn Gillmor Date: Sun, 26 Mar 2017 14:06:29 -0500 Subject: [PATCH 09/21] Implement sensible default padding policy. This commit changes the semantics of tls_query_padding_blocksize() slightly. Where previously both 0 and 1 meant "no padding", this commit changes 1 to mean "pad using a sensible policy". At NDSS 2017's DNS privacy workshop, I presented an empirical study of DNS padding policies: https://www.internetsociety.org/events/ndss-symposium/ndss-symposium-2017/dns-privacy-workshop-2017-programme#session3 The slide deck is here: https://dns.cmrg.net/ndss2017-dprive-empirical-DNS-traffic-size.pdf The resulting recommendation from the research is that a simple padding policy is relatively cheap and still protective of metadata when DNS traffic is encrypted: * queries should be padded to a multiple of 128 octets * responses should be padded to a multiple of 468 octets Since getdns is only currently doing queries over tls, we only have to implement the first part of this policy :) --- src/context.c | 2 +- src/stub.c | 9 ++++++--- src/tools/getdns_query.c | 5 +++-- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/src/context.c b/src/context.c index c52f770b..0748f40c 100644 --- a/src/context.c +++ b/src/context.c @@ -1435,7 +1435,7 @@ getdns_context_create_with_extended_memory_functions( result->edns_version = 0; result->edns_do_bit = 0; result->edns_client_subnet_private = 0; - result->tls_query_padding_blocksize = 1; /* default is to not try to pad */ + result->tls_query_padding_blocksize = 1; /* default is to pad queries sensibly */ result->tls_ctx = NULL; result->extension = &result->default_eventloop.loop; diff --git a/src/stub.c b/src/stub.c index 2acf0708..30c08091 100644 --- a/src/stub.c +++ b/src/stub.c @@ -1284,12 +1284,15 @@ stub_tls_write(getdns_upstream *upstream, getdns_tcp_state *tcp, return STUB_OUT_OF_OPTIONS; netreq->keepalive_sent = 1; } - if (netreq->owner->tls_query_padding_blocksize > 1) { + if (netreq->owner->tls_query_padding_blocksize > 0) { + uint16_t blksz = netreq->owner->tls_query_padding_blocksize; + if (blksz == 1) /* use a sensible default policy */ + blksz = 128; pkt_len = netreq->response - netreq->query; pkt_len += 4; /* this accounts for the OPTION-CODE and OPTION-LENGTH of the padding */ - padding_sz = pkt_len % netreq->owner->tls_query_padding_blocksize; + padding_sz = pkt_len % blksz; if (padding_sz) - padding_sz = netreq->owner->tls_query_padding_blocksize - padding_sz; + padding_sz = blksz - padding_sz; if (_getdns_network_req_add_upstream_option(netreq, EDNS_PADDING_OPCODE, padding_sz, NULL)) diff --git a/src/tools/getdns_query.c b/src/tools/getdns_query.c index bb452929..350ba7cc 100644 --- a/src/tools/getdns_query.c +++ b/src/tools/getdns_query.c @@ -54,7 +54,7 @@ static const char *default_stubby_config = ", dns_transport_list: [ GETDNS_TRANSPORT_TLS, GETDNS_TRANSPORT_UDP, GETDNS_TRANSPORT_TCP ]" ", idle_timeout: 10000" ", listen_addresses: [ 127.0.0.1@53, 0::1@53 ]" -", tls_query_padding_blocksize: 256" +", tls_query_padding_blocksize: 1" ", edns_client_subnet_private : 1" "}"; static int clear_listen_list_on_arg = 0; @@ -243,7 +243,8 @@ print_usage(FILE *out, const char *progname) fprintf(out, "\t-n\tSet TLS authentication mode to NONE (default)\n"); fprintf(out, "\t-m\tSet TLS authentication mode to REQUIRED\n"); fprintf(out, "\t-p\tPretty print response dict\n"); - fprintf(out, "\t-P \tPad TLS queries to a multiple of blocksize\n"); + fprintf(out, "\t-P \tPad TLS queries to a multiple of blocksize\n" + "\t\t(special values: 0: no padding, 1: sensible default policy)\n"); fprintf(out, "\t-q\tQuiet mode - don't print response\n"); fprintf( out, "\t-r\tSet recursing resolution type%s\n" , i_am_stubby ? "(default = stub)" : ""); From f2a90925bc7c7074d3939fa257ad4ccc80faabe1 Mon Sep 17 00:00:00 2001 From: Daniel Kahn Gillmor Date: Sun, 26 Mar 2017 14:38:13 -0500 Subject: [PATCH 10/21] getdns-query: S is no longer a valid transport label. --- src/tools/getdns_query.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/getdns_query.c b/src/tools/getdns_query.c index bb452929..5b4d63c0 100644 --- a/src/tools/getdns_query.c +++ b/src/tools/getdns_query.c @@ -271,7 +271,7 @@ print_usage(FILE *out, const char *progname) fprintf(out, "\t-u\tSet transport to UDP with TCP fallback (default)\n"); fprintf(out, "\t-U\tSet transport to UDP only\n"); fprintf(out, "\t-l \tSet transport list. List can contain 1 of each of the characters\n"); - fprintf(out, "\t\t\t U T L S for UDP, TCP or TLS e.g 'UT' or 'LTU' \n"); + fprintf(out, "\t\t\t U T L for UDP, TCP or TLS e.g 'UT' or 'LTU' \n"); fprintf(out, "\t-z \n"); fprintf(out, "\t\tListen for DNS requests on the given IP address\n"); fprintf(out, "\t\t is in the same format as upstreams.\n"); From 67baa1d651a6e796ecfbad596459ad8f20163302 Mon Sep 17 00:00:00 2001 From: Willem Toorop Date: Wed, 5 Apr 2017 12:37:48 +0200 Subject: [PATCH 11/21] getdns_context_unset_edns_maximum_udp_payload_size --- src/context.c | 42 ++++++++++++++++++++---------------- src/getdns/getdns_extra.h.in | 3 +++ 2 files changed, 27 insertions(+), 18 deletions(-) diff --git a/src/context.c b/src/context.c index 0748f40c..ac96691e 100644 --- a/src/context.c +++ b/src/context.c @@ -153,8 +153,6 @@ static getdns_return_t set_ub_dns_transport(struct getdns_context*); static void set_ub_limit_outstanding_queries(struct getdns_context*, uint16_t); static void set_ub_dnssec_allowed_skew(struct getdns_context*, uint32_t); -static void set_ub_edns_maximum_udp_payload_size(struct getdns_context*, - int); #endif /* Stuff to make it compile pedantically */ @@ -1797,9 +1795,9 @@ rebuild_ub_ctx(struct getdns_context* context) { "target-fetch-policy:", "0 0 0 0 0"); #endif set_ub_dnssec_allowed_skew(context, - context->dnssec_allowed_skew); - set_ub_edns_maximum_udp_payload_size(context, - context->edns_maximum_udp_payload_size); + context->dnssec_allowed_skew); + set_ub_number_opt(context, "edns-buffer-size:", + context->edns_maximum_udp_payload_size); set_ub_dns_transport(context); context->ub_event.userarg = context; @@ -2832,15 +2830,26 @@ error: } /* getdns_context_set_upstream_recursive_servers */ +/* + * getdns_context_unset_edns_maximum_udp_payload_size + * + */ +getdns_return_t +getdns_context_unset_edns_maximum_udp_payload_size(getdns_context *context) +{ + if (!context) + return GETDNS_RETURN_INVALID_PARAMETER; + #ifdef HAVE_LIBUNBOUND -static void -set_ub_edns_maximum_udp_payload_size(struct getdns_context* context, - int value) { - /* edns-buffer-size */ - if (value >= 512 && value <= 65535) - set_ub_number_opt(context, "edns-buffer-size:", (uint16_t)value); -} + set_ub_number_opt(context, "edns-buffer-size:", 4096); #endif + if (context->edns_maximum_udp_payload_size != -1) { + context->edns_maximum_udp_payload_size = -1; + dispatch_updated(context, + GETDNS_CONTEXT_CODE_EDNS_MAXIMUM_UDP_PAYLOAD_SIZE); + } + return GETDNS_RETURN_GOOD; +} /* getdns_context_set_edns_maximum_udp_payload_size */ /* * getdns_context_set_edns_maximum_udp_payload_size @@ -2853,12 +2862,8 @@ getdns_context_set_edns_maximum_udp_payload_size(struct getdns_context *context, if (!context) return GETDNS_RETURN_INVALID_PARAMETER; - /* check for < 512. uint16_t won't let it go above max) */ - if (value < 512) - value = 512; - #ifdef HAVE_LIBUNBOUND - set_ub_edns_maximum_udp_payload_size(context, value); + set_ub_number_opt(context, "edns-buffer-size:", value); #endif if (value != context->edns_maximum_udp_payload_size) { context->edns_maximum_udp_payload_size = value; @@ -4151,7 +4156,8 @@ getdns_context_get_edns_maximum_udp_payload_size(getdns_context *context, uint16_t* value) { RETURN_IF_NULL(context, GETDNS_RETURN_INVALID_PARAMETER); RETURN_IF_NULL(value, GETDNS_RETURN_INVALID_PARAMETER); - *value = context->edns_maximum_udp_payload_size; + *value = context->edns_maximum_udp_payload_size == -1 ? 0 + : context->edns_maximum_udp_payload_size; return GETDNS_RETURN_GOOD; } diff --git a/src/getdns/getdns_extra.h.in b/src/getdns/getdns_extra.h.in index bbb305d1..c49113dc 100644 --- a/src/getdns/getdns_extra.h.in +++ b/src/getdns/getdns_extra.h.in @@ -275,6 +275,9 @@ getdns_context_set_edns_client_subnet_private(getdns_context *context, uint8_t v getdns_return_t getdns_context_set_tls_query_padding_blocksize(getdns_context *context, uint16_t value); + +getdns_return_t +getdns_context_unset_edns_maximum_udp_payload_size(getdns_context *context); /** @} */ From 2220c1a48d6a9bff27e2d3dd7572f3314c810f91 Mon Sep 17 00:00:00 2001 From: Willem Toorop Date: Wed, 5 Apr 2017 17:53:39 +0200 Subject: [PATCH 12/21] Options for request debugging --- configure.ac | 12 ++++++++++-- src/debug.h | 10 +++++++++- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/configure.ac b/configure.ac index 43d653e3..bc07ab09 100644 --- a/configure.ac +++ b/configure.ac @@ -149,15 +149,16 @@ fi ]) ACX_ARG_RPATH - +AC_ARG_ENABLE(debug-req, AC_HELP_STRING([--enable-debug-req], [Enable request debugging])) AC_ARG_ENABLE(debug-sched, AC_HELP_STRING([--enable-debug-sched], [Enable scheduling debugging messages])) AC_ARG_ENABLE(debug-stub, AC_HELP_STRING([--enable-debug-stub], [Enable stub debugging messages])) AC_ARG_ENABLE(debug-daemon, AC_HELP_STRING([--enable-debug-daemon], [Enable daemon debugging messages])) AC_ARG_ENABLE(debug-sec, AC_HELP_STRING([--enable-debug-sec], [Enable dnssec debugging messages])) AC_ARG_ENABLE(debug-server, AC_HELP_STRING([--enable-debug-server], [Enable server debugging messages])) -AC_ARG_ENABLE(all-debugging, AC_HELP_STRING([--enable-all-debugging], [Enable scheduling, stub and dnssec debugging])) +AC_ARG_ENABLE(all-debugging, AC_HELP_STRING([--enable-all-debugging], [Enable all debugging messages])) case "$enable_all_debugging" in yes) + enable_debug_req=yes enable_debug_sched=yes enable_debug_stub=yes enable_debug_daemon=yes @@ -167,6 +168,13 @@ case "$enable_all_debugging" in no|*) ;; esac +case "$enable_debug_req" in + yes) + AC_DEFINE_UNQUOTED([REQ_DEBUG], [1], [Define this to enable printing of request debugging messages.]) + ;; + no|*) + ;; +esac case "$enable_debug_sched" in yes) AC_DEFINE_UNQUOTED([SCHED_DEBUG], [1], [Define this to enable printing of scheduling debugging messages.]) diff --git a/src/debug.h b/src/debug.h index 9c5fa6f1..fff6b0cd 100644 --- a/src/debug.h +++ b/src/debug.h @@ -91,6 +91,13 @@ #define DEBUG_OFF(...) do {} while (0) +#if defined(REQ_DEBUG) && REQ_DEBUG +#include +#define DEBUG_REQ(...) DEBUG_ON(__VA_ARGS__) +#else +#define DEBUG_REQ(...) DEBUG_OFF(__VA_ARGS__) +#endif + #if defined(SCHED_DEBUG) && SCHED_DEBUG #include #define DEBUG_SCHED(...) DEBUG_ON(__VA_ARGS__) @@ -139,7 +146,8 @@ #define DEBUG_MDNS(...) DEBUG_OFF(__VA_ARGS__) #endif -#if (defined(SCHED_DEBUG) && SCHED_DEBUG) || \ +#if (defined(REQ_DEBUG) && REQ_DEBUG) || \ + (defined(SCHED_DEBUG) && SCHED_DEBUG) || \ (defined(STUB_DEBUG) && STUB_DEBUG) || \ (defined(DAEMON_DEBUG) && DAEMON_DEBUG) || \ (defined(SEC_DEBUG) && SEC_DEBUG) || \ From f8c7d8b5d597c54119fd0d1aca64480fe4f6f680 Mon Sep 17 00:00:00 2001 From: Willem Toorop Date: Wed, 5 Apr 2017 22:43:27 +0200 Subject: [PATCH 13/21] Network request submission and callback reporting --- src/context.c | 3 +++ src/debug.h | 19 ++++++++++++++++++- src/general.c | 10 ++++++++++ 3 files changed, 31 insertions(+), 1 deletion(-) diff --git a/src/context.c b/src/context.c index 0748f40c..ca3603cf 100644 --- a/src/context.c +++ b/src/context.c @@ -691,6 +691,7 @@ _getdns_upstreams_dereference(getdns_upstreams *upstreams) while (upstream->finished_dnsreqs) { dnsreq = upstream->finished_dnsreqs; upstream->finished_dnsreqs = dnsreq->finished_next; + debug_req("Destroy ", *dnsreq->netreqs); _getdns_context_cancel_request(dnsreq); } if (upstream->tls_session != NULL) @@ -3080,6 +3081,7 @@ getdns_cancel_callback(getdns_context *context, getdns_context_request_count_changed(context); + debug_req("CB Cancel ", *dnsreq->netreqs); if (dnsreq->user_callback) { dnsreq->context->processing = 1; dnsreq->user_callback(dnsreq->context, GETDNS_CALLBACK_CANCEL, @@ -3095,6 +3097,7 @@ _getdns_context_request_timed_out(getdns_dns_req *dnsreq) { DEBUG_SCHED("%s(%p)\n", __FUNC__, (void *)dnsreq); + debug_req("CB Timeout ", *dnsreq->netreqs); if (dnsreq->user_callback) { dnsreq->context->processing = 1; dnsreq->user_callback(dnsreq->context, GETDNS_CALLBACK_TIMEOUT, diff --git a/src/debug.h b/src/debug.h index fff6b0cd..fb74527c 100644 --- a/src/debug.h +++ b/src/debug.h @@ -36,7 +36,6 @@ #define DEBUG_H #include "config.h" - #define STUB_DEBUG_ENTRY "=> ENTRY: " #define STUB_DEBUG_SETUP "--- SETUP: " #define STUB_DEBUG_SETUP_TLS "--- SETUP(TLS): " @@ -94,8 +93,26 @@ #if defined(REQ_DEBUG) && REQ_DEBUG #include #define DEBUG_REQ(...) DEBUG_ON(__VA_ARGS__) +#include "gldns/wire2str.h" +#include "rr-dict.h" +#include "types-internal.h" +static inline void debug_req(const char *msg, getdns_network_req *netreq) +{ + char str[1024]; + struct timeval tv; + uint64_t t; + + (void) gettimeofday(&tv, NULL); + t = tv.tv_sec * 1000 + tv.tv_usec / 1000; + t = t >= netreq->owner->expires ? 0 : netreq->owner->expires - t; + (void) gldns_wire2str_dname_buf(netreq->owner->name, + netreq->owner->name_len, str, sizeof(str)); + DEBUG_REQ("NETREQ %s %4"PRIu64" %s %s\n", msg, t, + str, _getdns_rr_type_name(netreq->request_type)); +} #else #define DEBUG_REQ(...) DEBUG_OFF(__VA_ARGS__) +#define debug_req(...) DEBUG_OFF(__VA_ARGS__) #endif #if defined(SCHED_DEBUG) && SCHED_DEBUG diff --git a/src/general.c b/src/general.c index cc6e1fe9..4ac4e3f2 100644 --- a/src/general.c +++ b/src/general.c @@ -186,6 +186,14 @@ _getdns_check_dns_req_complete(getdns_dns_req *dns_req) return; } } +#if defined(REQ_DEBUG) && REQ_DEBUG + if (dns_req->internal_cb) + debug_req("CB Internal", *dns_req->netreqs); + else if (results_found) + debug_req("CB Complete", *dns_req->netreqs); + else + debug_req("CB Error ", *dns_req->netreqs); +#endif if (dns_req->internal_cb) { _getdns_context_clear_outbound_request(dns_req); dns_req->internal_cb(dns_req); @@ -373,6 +381,8 @@ _getdns_submit_netreq(getdns_network_req *netreq, uint64_t *now_ms) } _getdns_netreq_change_state(netreq, NET_REQ_IN_FLIGHT); + debug_req("Submitting ", netreq); + #ifdef STUB_NATIVE_DNSSEC # ifdef DNSSEC_ROADBLOCK_AVOIDANCE From e08d3592a069c3ce8c6a51f88adebac6d7f3102f Mon Sep 17 00:00:00 2001 From: Willem Toorop Date: Thu, 6 Apr 2017 11:20:08 +0200 Subject: [PATCH 14/21] Schedule timeout when collecting for dnssec chain --- src/context.c | 11 ++++-- src/dnssec.c | 37 +++++++++++++++++++ src/dnssec.h | 1 + src/general.c | 22 +++++++++-- src/request-internal.c | 1 + src/stub.c | 22 +++++++++++ .../070-coding-practice.test | 13 +++++++ src/types-internal.h | 1 + src/util-internal.c | 1 + 9 files changed, 102 insertions(+), 7 deletions(-) diff --git a/src/context.c b/src/context.c index ca3603cf..2af25a3f 100644 --- a/src/context.c +++ b/src/context.c @@ -691,8 +691,10 @@ _getdns_upstreams_dereference(getdns_upstreams *upstreams) while (upstream->finished_dnsreqs) { dnsreq = upstream->finished_dnsreqs; upstream->finished_dnsreqs = dnsreq->finished_next; - debug_req("Destroy ", *dnsreq->netreqs); - _getdns_context_cancel_request(dnsreq); + if (!dnsreq->internal_cb) { /* Not part of chain */ + debug_req("Destroy ", *dnsreq->netreqs); + _getdns_context_cancel_request(dnsreq); + } } if (upstream->tls_session != NULL) SSL_SESSION_free(upstream->tls_session); @@ -3088,7 +3090,10 @@ getdns_cancel_callback(getdns_context *context, NULL, dnsreq->user_pointer, dnsreq->trans_id); dnsreq->context->processing = 0; } - _getdns_context_cancel_request(dnsreq); + if (!dnsreq->internal_cb) { /* Not part of chain */ + debug_req("Destroy ", *dnsreq->netreqs); + _getdns_context_cancel_request(dnsreq); + } return GETDNS_RETURN_GOOD; } /* getdns_cancel_callback */ diff --git a/src/dnssec.c b/src/dnssec.c index b3e86bae..8c01a635 100644 --- a/src/dnssec.c +++ b/src/dnssec.c @@ -3113,6 +3113,43 @@ static void check_chain_complete(chain_head *chain) _getdns_call_user_callback(dnsreq, response_dict); } +void _getdns_validation_chain_timeout(getdns_dns_req *dnsreq) +{ + chain_head *head = dnsreq->chain, *next; + chain_node *node; + size_t node_count; + + while (head) { + next = head->next; + + for ( node_count = head->node_count, node = head->parent + ; node_count + ; node_count--, node = node->parent ) { + + if (!_getdns_netreq_finished(node->dnskey_req)) { + _getdns_context_cancel_request( + node->dnskey_req->owner); + node->dnskey_req = NULL; + } + + if (!_getdns_netreq_finished(node->ds_req)) { + _getdns_context_cancel_request( + node->ds_req->owner); + node->ds_req = NULL; + } + + if (!_getdns_netreq_finished(node->soa_req)) { + _getdns_context_cancel_request( + node->soa_req->owner); + node->soa_req = NULL; + } + } + head = next; + } + dnsreq->request_timed_out = 1; + check_chain_complete(dnsreq->chain); +} + void _getdns_cancel_validation_chain(getdns_dns_req *dnsreq) { chain_head *head = dnsreq->chain, *next; diff --git a/src/dnssec.h b/src/dnssec.h index b7becfe9..b0334d52 100644 --- a/src/dnssec.h +++ b/src/dnssec.h @@ -47,6 +47,7 @@ /* Do some additional requests to fetch the complete validation chain */ void _getdns_get_validation_chain(getdns_dns_req *dns_req); void _getdns_cancel_validation_chain(getdns_dns_req *dns_req); +void _getdns_validation_chain_timeout(getdns_dns_req *dns_req); uint16_t _getdns_parse_ta_file(time_t *ta_mtime, gldns_buffer *gbuf); diff --git a/src/general.c b/src/general.c index 4ac4e3f2..16a07128 100644 --- a/src/general.c +++ b/src/general.c @@ -61,8 +61,9 @@ void _getdns_call_user_callback(getdns_dns_req *dnsreq, getdns_dict *response) if (dnsreq->user_callback) { dnsreq->context->processing = 1; dnsreq->user_callback(dnsreq->context, - (response ? GETDNS_CALLBACK_COMPLETE - : GETDNS_CALLBACK_ERROR), + ( ! response ? GETDNS_CALLBACK_ERROR + : dnsreq->request_timed_out ? GETDNS_CALLBACK_TIMEOUT + : GETDNS_CALLBACK_COMPLETE ), response, dnsreq->user_pointer, dnsreq->trans_id); dnsreq->context->processing = 0; } @@ -214,9 +215,22 @@ _getdns_check_dns_req_complete(getdns_dns_req *dns_req) dns_req->dnssec_return_all_statuses )) #endif - )) + )) { + /* Reschedule timeout for this DNS request + */ + dns_req->timeout.userarg = dns_req; + dns_req->timeout.read_cb = NULL; + dns_req->timeout.write_cb = NULL; + dns_req->timeout.timeout_cb = + (getdns_eventloop_callback) + _getdns_validation_chain_timeout; + dns_req->timeout.ev = NULL; + (void) dns_req->loop->vmt->schedule(dns_req->loop, -1, + _getdns_ms_until_expiry2(dns_req->expires, &now_ms), + &dns_req->timeout); + _getdns_get_validation_chain(dns_req); - else + } else _getdns_call_user_callback( dns_req, _getdns_create_getdns_response(dns_req)); } diff --git a/src/request-internal.c b/src/request-internal.c index 1bd5475e..d91563f4 100644 --- a/src/request-internal.c +++ b/src/request-internal.c @@ -944,6 +944,7 @@ _getdns_dns_req_new(getdns_context *context, getdns_eventloop *loop, result->freed = NULL; result->validating = 0; result->is_dns_request = 1; + result->request_timed_out = 0; result->chain = NULL; network_req_init(result->netreqs[0], result, diff --git a/src/stub.c b/src/stub.c index 30c08091..90b44c96 100644 --- a/src/stub.c +++ b/src/stub.c @@ -32,6 +32,12 @@ */ #include "config.h" + +/* Intercept and do not sent out COM DS queries with TLS + * For debugging purposes only. Never commit with this turned on. + */ +#define INTERCEPT_COM_DS 0 + #ifdef USE_POLL_DEFAULT_EVENTLOOP # ifdef HAVE_SYS_POLL_H # include @@ -1306,6 +1312,22 @@ stub_tls_write(getdns_upstream *upstream, getdns_tcp_state *tcp, /* TODO[TLS]: Handle error cases, partial writes, renegotiation etc. */ ERR_clear_error(); +#if INTERCEPT_COM_DS + /* Intercept and do not sent out COM DS queries. For debugging + * purposes only. Never commit with this turned on. + */ + if (netreq->request_type == GETDNS_RRTYPE_DS && + netreq->owner->name_len == 5 && + netreq->owner->name[0] == 3 && + (netreq->owner->name[1] & 0xDF) == 'C' && + (netreq->owner->name[2] & 0xDF) == 'O' && + (netreq->owner->name[3] & 0xDF) == 'M' && + netreq->owner->name[4] == 0) { + + debug_req("Intercepting", netreq); + written = pkt_len + 2; + } else +#endif written = SSL_write(tls_obj, netreq->query - 2, pkt_len + 2); if (written <= 0) return STUB_TCP_ERROR; diff --git a/src/test/tpkg/070-coding-practice.tpkg/070-coding-practice.test b/src/test/tpkg/070-coding-practice.tpkg/070-coding-practice.test index b628a657..09473687 100644 --- a/src/test/tpkg/070-coding-practice.tpkg/070-coding-practice.test +++ b/src/test/tpkg/070-coding-practice.tpkg/070-coding-practice.test @@ -33,6 +33,19 @@ rm -f report.txt echo "" fi ) >> report.txt +( + cd ${SRCROOT}/src + if [ `grep '^#define[ ]*INTERCEPT_COM_DS[ ]*1' stub.c | wc -l` -gt 0 ] + then + echo "*** " + echo "*** The repo contained the COM DS queries interception" + echo "*** with TLS transports turned on, this should be off" + echo "*** " + grep -n '^#define[ ]INTERCEPT_COM_DS[ ]*1' stub.c + echo "" + fi +) >> report.txt + if [ -s report.txt ] then diff --git a/src/types-internal.h b/src/types-internal.h index 5e1961ac..e70d8911 100644 --- a/src/types-internal.h +++ b/src/types-internal.h @@ -314,6 +314,7 @@ typedef struct getdns_dns_req { unsigned dnssec_ok_checking_disabled : 1; unsigned is_sync_request : 1; unsigned is_dns_request : 1; + unsigned request_timed_out : 1; /* The validating and freed variables are used to make sure a single * code path is followed while processing a DNS request, even when diff --git a/src/util-internal.c b/src/util-internal.c index 33c31745..ddac7b51 100644 --- a/src/util-internal.c +++ b/src/util-internal.c @@ -1263,6 +1263,7 @@ _getdns_create_getdns_response(getdns_dns_req *completed_request) GETDNS_FREE(context->mf, srvs.rrs); } if (getdns_dict_set_int(result, GETDNS_STR_KEY_STATUS, + completed_request->request_timed_out || nreplies == 0 ? GETDNS_RESPSTATUS_ALL_TIMEOUT : completed_request->dnssec_return_only_secure && nsecure == 0 && ninsecure > 0 ? GETDNS_RESPSTATUS_NO_SECURE_ANSWERS : From 4ceec33d0826b9f5453875510735bf00d7ad8b75 Mon Sep 17 00:00:00 2001 From: Willem Toorop Date: Thu, 6 Apr 2017 11:46:10 +0200 Subject: [PATCH 15/21] Do something about TLS renegotiation. --- src/stub.c | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/src/stub.c b/src/stub.c index 90b44c96..e8a8ee52 100644 --- a/src/stub.c +++ b/src/stub.c @@ -1329,9 +1329,22 @@ stub_tls_write(getdns_upstream *upstream, getdns_tcp_state *tcp, } else #endif written = SSL_write(tls_obj, netreq->query - 2, pkt_len + 2); - if (written <= 0) - return STUB_TCP_ERROR; - + if (written <= 0) { + /* SSL_write will not do partial writes, because + * SSL_MODE_ENABLE_PARTIAL_WRITE is not default, + * but the write could fail because of renegotiation. + * In that case SSL_get_error() will return + * SSL_ERROR_WANT_READ or, SSL_ERROR_WANT_WRITE. + * Return for retry in such cases. + */ + switch (SSL_get_error(tls_obj, written)) { + case SSL_ERROR_WANT_READ: + case SSL_ERROR_WANT_WRITE: + return STUB_TCP_AGAIN; + default: + return STUB_TCP_ERROR; + } + } /* We were able to write everything! Start reading. */ return (int) query_id; @@ -2102,6 +2115,12 @@ upstream_reschedule_events(getdns_upstream *upstream, uint64_t idle_timeout) { else { DEBUG_STUB("%s %-35s: FD: %d Connection idle - timeout is %d\n", STUB_DEBUG_SCHEDULE, __FUNC__, upstream->fd, (int)idle_timeout); + /* TODO: Schedule a read also anyway, + * to digest timed out answers. + * Dont forget to schedule with upstream->fd then! + * + * upstream->event.read_cb = upstream_read_cb; + */ upstream->event.timeout_cb = upstream_idle_timeout_cb; if (upstream->conn_state != GETDNS_CONN_OPEN) idle_timeout = 0; From e35a2182a92bf9f747471144c6ed2197cecbb729 Mon Sep 17 00:00:00 2001 From: Willem Toorop Date: Thu, 6 Apr 2017 12:24:27 +0200 Subject: [PATCH 16/21] missing #include --- src/general.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/general.c b/src/general.c index 16a07128..b981470b 100644 --- a/src/general.c +++ b/src/general.c @@ -53,6 +53,7 @@ #include "stub.h" #include "dict.h" #include "mdns.h" +#include "debug.h" void _getdns_call_user_callback(getdns_dns_req *dnsreq, getdns_dict *response) { From c2edc94a3a25e835e18e094aab6c080a489986e6 Mon Sep 17 00:00:00 2001 From: Willem Toorop Date: Thu, 6 Apr 2017 15:18:12 +0200 Subject: [PATCH 17/21] Clear timeout event when getting dnssec chain With full recursion --- src/general.c | 16 +++++++--------- .../225-stub-only-valgrind-checks.dsc | 2 +- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/src/general.c b/src/general.c index b981470b..280df08d 100644 --- a/src/general.c +++ b/src/general.c @@ -219,16 +219,14 @@ _getdns_check_dns_req_complete(getdns_dns_req *dns_req) )) { /* Reschedule timeout for this DNS request */ - dns_req->timeout.userarg = dns_req; - dns_req->timeout.read_cb = NULL; - dns_req->timeout.write_cb = NULL; - dns_req->timeout.timeout_cb = - (getdns_eventloop_callback) - _getdns_validation_chain_timeout; - dns_req->timeout.ev = NULL; - (void) dns_req->loop->vmt->schedule(dns_req->loop, -1, + if (dns_req->timeout.timeout_cb && dns_req->timeout.ev) + GETDNS_CLEAR_EVENT(dns_req->loop, &dns_req->timeout); + + GETDNS_SCHEDULE_EVENT(dns_req->loop, -1, _getdns_ms_until_expiry2(dns_req->expires, &now_ms), - &dns_req->timeout); + getdns_eventloop_event_init(&dns_req->timeout, dns_req, + NULL, NULL, (getdns_eventloop_callback) + _getdns_validation_chain_timeout)); _getdns_get_validation_chain(dns_req); } else diff --git a/src/test/tpkg/225-stub-only-valgrind-checks.tpkg/225-stub-only-valgrind-checks.dsc b/src/test/tpkg/225-stub-only-valgrind-checks.tpkg/225-stub-only-valgrind-checks.dsc index d845ad44..7167a541 100644 --- a/src/test/tpkg/225-stub-only-valgrind-checks.tpkg/225-stub-only-valgrind-checks.dsc +++ b/src/test/tpkg/225-stub-only-valgrind-checks.tpkg/225-stub-only-valgrind-checks.dsc @@ -6,7 +6,7 @@ Maintainer: Willem Toorop Category: Component: CmdDepends: valgrind -Depends: 110-link.tpkg +Depends: 210-stub-only-link.tpkg Help: Pre: Post: From f0ee9202270aec03dc31b02ba89ce16f274bb3f7 Mon Sep 17 00:00:00 2001 From: Willem Toorop Date: Thu, 6 Apr 2017 16:13:15 +0200 Subject: [PATCH 18/21] Bump version, update ChangeLog --- ChangeLog | 9 ++++++++- configure.ac | 4 ++-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/ChangeLog b/ChangeLog index de4e70ca..f5bb6498 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,4 +1,11 @@ * 2017-04-??: Version 1.1.0 + * bugfix: Reschedule request timeout when getting the DNSSEC chain. + * Implement sensible default edns0 padding policy. Thanks DKG. + * Keep connections open with sync requests too. + * Fix of event loops so they do not give up with naked timers with + windows. Thanks Christian Huitema. + * Include peer certificate with DNS-over-TLS in combination with + the return_call_reporting extension. * More fine grained control over TLS upstream retry and back off behaviour with getdns_context_set_tls_backoff_time() and getdns_context_set_tls_connection_retries(). @@ -14,7 +21,7 @@ Thanks Neil Cook * bugfix: authentication failure for self signed cert + only pinset * bugfix: issue with session re-use making authentication appear to fail - + * 2017-01-13: Version 1.0.0 * edns0_cookies extension enabled by default (per RFC7873) * dnssec_roadblock_avoidance enabled by default (per RFC8027) diff --git a/configure.ac b/configure.ac index bc07ab09..c1a0c2fc 100644 --- a/configure.ac +++ b/configure.ac @@ -37,7 +37,7 @@ sinclude(./m4/ax_check_compile_flag.m4) sinclude(./m4/pkg.m4) AC_INIT([getdns], [1.1.0], [users@getdnsapi.net], [], [https://getdnsapi.net]) -AC_SUBST(RELEASE_CANDIDATE, [-rc1]) +AC_SUBST(RELEASE_CANDIDATE, [-rc2]) # Set current date from system if not set AC_ARG_WITH([current-date], @@ -47,7 +47,7 @@ AC_ARG_WITH([current-date], [CURRENT_DATE="`date -u +%Y-%m-%dT%H:%M:%SZ`"]) AC_SUBST(GETDNS_VERSION, ["AC_PACKAGE_VERSION$RELEASE_CANDIDATE"]) -AC_SUBST(GETDNS_NUMERIC_VERSION, [0x0100C100]) +AC_SUBST(GETDNS_NUMERIC_VERSION, [0x0100C200]) AC_SUBST(API_VERSION, ["December 2015"]) AC_SUBST(API_NUMERIC_VERSION, [0x07df0c00]) GETDNS_COMPILATION_COMMENT="AC_PACKAGE_NAME $GETDNS_VERSION configured on $CURRENT_DATE for the $API_VERSION version of the API" From a27915ccc97d3543568e4ca97f61b334ceaa03f8 Mon Sep 17 00:00:00 2001 From: Willem Toorop Date: Thu, 6 Apr 2017 19:47:15 +0200 Subject: [PATCH 19/21] One more ChangeLog update --- ChangeLog | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ChangeLog b/ChangeLog index f5bb6498..1299763c 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,7 @@ * 2017-04-??: Version 1.1.0 * bugfix: Reschedule request timeout when getting the DNSSEC chain. + * getdns_context_unset_edns_maximum_udp_payload_size() to reset + to default IPv4/IPv6 dependent edns max udp payload size. * Implement sensible default edns0 padding policy. Thanks DKG. * Keep connections open with sync requests too. * Fix of event loops so they do not give up with naked timers with From c9b3e3cf7b4d4a012241c4adc90c31c3d7244814 Mon Sep 17 00:00:00 2001 From: Willem Toorop Date: Thu, 6 Apr 2017 20:50:34 +0200 Subject: [PATCH 20/21] Allow cleanup of naked idle timeouts --- src/context.c | 34 +++++++++++++++++++++++++++------- src/stub.c | 3 +-- src/tools/getdns_query.c | 1 + 3 files changed, 29 insertions(+), 9 deletions(-) diff --git a/src/context.c b/src/context.c index 2e26b06c..5396fcc4 100644 --- a/src/context.c +++ b/src/context.c @@ -2210,18 +2210,38 @@ getdns_context_set_timeout(struct getdns_context *context, uint64_t timeout) * */ getdns_return_t -getdns_context_set_idle_timeout(struct getdns_context *context, uint64_t timeout) +getdns_context_set_idle_timeout(getdns_context *context, uint64_t timeout) { - RETURN_IF_NULL(context, GETDNS_RETURN_INVALID_PARAMETER); + size_t i; - /* Shuold we enforce maximum based on edns-tcp-keepalive spec? */ - /* 0 should be allowed as that is the default.*/ + if (!context) + return GETDNS_RETURN_INVALID_PARAMETER; - context->idle_timeout = timeout; + /* Shuold we enforce maximum based on edns-tcp-keepalive spec? */ + /* 0 should be allowed as that is the default.*/ - dispatch_updated(context, GETDNS_CONTEXT_CODE_IDLE_TIMEOUT); + context->idle_timeout = timeout; - return GETDNS_RETURN_GOOD; + dispatch_updated(context, GETDNS_CONTEXT_CODE_IDLE_TIMEOUT); + + if (timeout) + return GETDNS_RETURN_GOOD; + + /* If timeout == 0, call scheduled idle timeout events */ + for (i = 0; i < context->upstreams->count; i++) { + getdns_upstream *upstream = + &context->upstreams->upstreams[i]; + + if (!upstream->event.ev || + !upstream->event.timeout_cb || + upstream->event.read_cb || + upstream->event.write_cb) + continue; + + GETDNS_CLEAR_EVENT(upstream->loop, &upstream->event); + upstream->event.timeout_cb(upstream->event.userarg); + } + return GETDNS_RETURN_GOOD; } /* getdns_context_set_timeout */ diff --git a/src/stub.c b/src/stub.c index e8a8ee52..3597cb88 100644 --- a/src/stub.c +++ b/src/stub.c @@ -1717,9 +1717,8 @@ upstream_write_cb(void *userarg) default: if (netreq->owner->return_call_reporting && netreq->upstream->tls_obj && + netreq->debug_tls_peer_cert.data == NULL && (cert = SSL_get_peer_certificate(netreq->upstream->tls_obj))) { - assert(netreq->debug_tls_peer_cert.data == NULL); - netreq->debug_tls_peer_cert.size = i2d_X509( cert, &netreq->debug_tls_peer_cert.data); X509_free(cert); diff --git a/src/tools/getdns_query.c b/src/tools/getdns_query.c index 5a431484..7dca01fe 100644 --- a/src/tools/getdns_query.c +++ b/src/tools/getdns_query.c @@ -1230,6 +1230,7 @@ void read_line_cb(void *userarg) if (listen_count) (void) getdns_context_set_listen_addresses( context, NULL, NULL, NULL); + (void) getdns_context_set_idle_timeout(context, 0); return; } if (query_file) From e6696d9557770c7c1b0ebdaf92dbc7d16af1d214 Mon Sep 17 00:00:00 2001 From: Willem Toorop Date: Thu, 6 Apr 2017 20:53:18 +0200 Subject: [PATCH 21/21] getdns_context_unset_edns_maximum_udp_payload_size --- src/libgetdns.symbols | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libgetdns.symbols b/src/libgetdns.symbols index 890475ee..4df6a989 100644 --- a/src/libgetdns.symbols +++ b/src/libgetdns.symbols @@ -69,6 +69,7 @@ getdns_context_set_tls_query_padding_blocksize getdns_context_set_update_callback getdns_context_set_upstream_recursive_servers getdns_context_set_use_threads +getdns_context_unset_edns_maximum_udp_payload_size getdns_convert_alabel_to_ulabel getdns_convert_dns_name_to_fqdn getdns_convert_fqdn_to_dns_name