From ec5e01747a0b128039aba31b13616282e25fc9f8 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Mon, 11 Jul 2016 13:36:16 +1000 Subject: [PATCH] 4408. [func] Continue waiting for expected response when we the response we get does not match the request. [RT #41026] --- CHANGES | 3 + bin/named/statschannel.c | 1 + lib/dns/dispatch.c | 46 ++++++++- lib/dns/include/dns/dispatch.h | 11 +++ lib/dns/include/dns/stats.h | 3 +- lib/dns/resolver.c | 72 ++++++++++---- lib/dns/tests/dispatch_test.c | 176 ++++++++++++++++++++++++++++++++- lib/dns/tests/dnstest.c | 5 +- lib/dns/win32/libdns.def.in | 1 + 9 files changed, 290 insertions(+), 28 deletions(-) diff --git a/CHANGES b/CHANGES index ffc15da81f..3cc7370a01 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,6 @@ +4408. [func] Continue waiting for expected response when we the + response we get does not match the request. [RT #41026] + 4407. [performance] Use GCC builtin for clz in RPZ lookup code. [RT #42818] diff --git a/bin/named/statschannel.c b/bin/named/statschannel.c index 08089ffb55..162ea0b702 100644 --- a/bin/named/statschannel.c +++ b/bin/named/statschannel.c @@ -369,6 +369,7 @@ init_desc(void) { SET_RESSTATDESC(zonequota, "spilled due to zone quota", "ZoneQuota"); SET_RESSTATDESC(serverquota, "spilled due to server quota", "ServerQuota"); + SET_RESSTATDESC(nextitem, "waited for next item", "NextItem"); INSIST(i == dns_resstatscounter_max); diff --git a/lib/dns/dispatch.c b/lib/dns/dispatch.c index ce5d2295f7..33e4033b8e 100644 --- a/lib/dns/dispatch.c +++ b/lib/dns/dispatch.c @@ -1697,7 +1697,7 @@ open_socket(isc_socketmgr_t *mgr, isc_sockaddr_t *local, return (ISC_R_SUCCESS); } else { result = isc_socket_create(mgr, isc_sockaddr_pf(local), - isc_sockettype_udp, &sock); + isc_sockettype_udp, &sock); if (result != ISC_R_SUCCESS) return (result); } @@ -3406,6 +3406,48 @@ dns_dispatch_starttcp(dns_dispatch_t *disp) { UNLOCK(&disp->lock); } +isc_result_t +dns_dispatch_getnext(dns_dispentry_t *resp, dns_dispatchevent_t **sockevent) { + dns_dispatch_t *disp; + dns_dispatchevent_t *ev; + + REQUIRE(VALID_RESPONSE(resp)); + REQUIRE(sockevent != NULL && *sockevent != NULL); + + disp = resp->disp; + REQUIRE(VALID_DISPATCH(disp)); + + REQUIRE(resp->item_out == ISC_TRUE); + resp->item_out = ISC_FALSE; + + ev = *sockevent; + *sockevent = NULL; + + LOCK(&disp->lock); + if (ev->buffer.base != NULL) + free_buffer(disp, ev->buffer.base, ev->buffer.length); + free_devent(disp, ev); + + if (disp->shutting_down == 1) { + UNLOCK(&disp->lock); + return (ISC_R_SHUTTINGDOWN); + } + ev = ISC_LIST_HEAD(resp->items); + if (ev != NULL) { + ISC_LIST_UNLINK(resp->items, ev, ev_link); + ISC_EVENT_INIT(ev, sizeof(*ev), 0, NULL, DNS_EVENT_DISPATCH, + resp->action, resp->arg, resp, NULL, NULL); + request_log(disp, resp, LVL(90), + "[c] Sent event %p buffer %p len %d to task %p", + ev, ev->buffer.base, ev->buffer.length, + resp->task); + resp->item_out = ISC_TRUE; + isc_task_send(resp->task, ISC_EVENT_PTR(&ev)); + } + UNLOCK(&disp->lock); + return (ISC_R_SUCCESS); +} + void dns_dispatch_removeresponse(dns_dispentry_t **resp, dns_dispatchevent_t **sockevent) @@ -3503,7 +3545,7 @@ dns_dispatch_removeresponse(dns_dispentry_t **resp, } /* - * Free any buffered requests as well + * Free any buffered responses as well */ ev = ISC_LIST_HEAD(res->items); while (ev != NULL) { diff --git a/lib/dns/include/dns/dispatch.h b/lib/dns/include/dns/dispatch.h index 4869f9fabb..9e0e8f9825 100644 --- a/lib/dns/include/dns/dispatch.h +++ b/lib/dns/include/dns/dispatch.h @@ -590,6 +590,17 @@ dns_dispatch_getdscp(dns_dispatch_t *disp); *\li disp is valid. */ +isc_result_t +dns_dispatch_getnext(dns_dispentry_t *resp, dns_dispatchevent_t **sockevent); +/*%< + * Free the sockevent and trigger the sending of the next item off the + * dispatch queue if present. + * + * Requires: + *\li resp is valid + *\li *sockevent to be valid + */ + ISC_LANG_ENDDECLS #endif /* DNS_DISPATCH_H */ diff --git a/lib/dns/include/dns/stats.h b/lib/dns/include/dns/stats.h index 41ddac2578..33c859ff7a 100644 --- a/lib/dns/include/dns/stats.h +++ b/lib/dns/include/dns/stats.h @@ -65,7 +65,8 @@ enum { dns_resstatscounter_badcookie = 40, dns_resstatscounter_zonequota = 41, dns_resstatscounter_serverquota = 42, - dns_resstatscounter_max = 43, + dns_resstatscounter_nextitem = 43, + dns_resstatscounter_max = 44, /* * DNSSEC stats. diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index 4b7b738d38..043691b34b 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -7643,7 +7643,7 @@ resquery_response(isc_task_t *task, isc_event_t *event) { isc_result_t result = ISC_R_SUCCESS; resquery_t *query = event->ev_arg; dns_dispatchevent_t *devent = (dns_dispatchevent_t *)event; - isc_boolean_t keep_trying, get_nameservers, resend; + isc_boolean_t keep_trying, get_nameservers, resend, nextitem; isc_boolean_t truncated; dns_message_t *message; dns_rdataset_t *opt; @@ -7689,6 +7689,7 @@ resquery_response(isc_task_t *task, isc_event_t *event) { broken_server = ISC_R_SUCCESS; get_nameservers = ISC_FALSE; resend = ISC_FALSE; + nextitem = ISC_FALSE; truncated = ISC_FALSE; finish = NULL; no_response = ISC_FALSE; @@ -7897,14 +7898,49 @@ resquery_response(isc_task_t *task, isc_event_t *event) { if (message->cc_bad && (options & DNS_FETCHOPT_TCP) == 0) { /* - * If the COOKIE is bad assume it is a attack and retry. + * If the COOKIE is bad, assume it is an attack and + * keep listening for a good answer. */ - resend = ISC_TRUE; - /* XXXMPA log it */ - FCTXTRACE("bad cookie"); + nextitem = ISC_TRUE; + if (isc_log_wouldlog(dns_lctx, ISC_LOG_INFO)) { + char addrbuf[ISC_SOCKADDR_FORMATSIZE]; + isc_sockaddr_format(&query->addrinfo->sockaddr, + addrbuf, sizeof(addrbuf)); + isc_log_write(dns_lctx, DNS_LOGCATEGORY_RESOLVER, + DNS_LOGMODULE_RESOLVER, ISC_LOG_INFO, + "bad cookie from %s", addrbuf); + } goto done; } + /* + * Is the question the same as the one we asked? + * NOERROR/NXDOMAIN/YXDOMAIN/REFUSED/SERVFAIL/BADCOOKIE must have + * the same question. + * FORMERR/NOTIMP if they have a question section then it must match. + */ + switch (message->rcode) { + case dns_rcode_notimp: + case dns_rcode_formerr: + if (message->counts[DNS_SECTION_QUESTION] == 0) + break; + case dns_rcode_nxrrset: /* Not expected. */ + case dns_rcode_badcookie: + case dns_rcode_noerror: + case dns_rcode_nxdomain: + case dns_rcode_yxdomain: + case dns_rcode_refused: + case dns_rcode_servfail: + default: + result = same_question(fctx); + if (result != ISC_R_SUCCESS) { + FCTXTRACE3("response did not match question", result); + nextitem = ISC_TRUE; + goto done; + } + break; + } + /* * If the message is signed, check the signature. If not, this * returns success anyway. @@ -8213,18 +8249,6 @@ resquery_response(isc_task_t *task, isc_event_t *event) { goto done; } - /* - * Is the question the same as the one we asked? - */ - result = same_question(fctx); - if (result != ISC_R_SUCCESS) { - /* XXXRTH Log */ - if (result == DNS_R_FORMERR) - keep_trying = ISC_TRUE; - FCTXTRACE3("response did not match question", result); - goto done; - } - /* * Is the server lame? */ @@ -8488,7 +8512,9 @@ resquery_response(isc_task_t *task, isc_event_t *event) { * * XXXRTH Don't cancel the query if waiting for validation? */ - fctx_cancelquery(&query, &devent, finish, no_response, ISC_FALSE); + if (!nextitem) + fctx_cancelquery(&query, &devent, finish, + no_response, ISC_FALSE); #ifdef ENABLE_AFL if (fuzzing_resolver && (keep_trying || resend)) { @@ -8585,6 +8611,16 @@ resquery_response(isc_task_t *task, isc_event_t *event) { if (bucket_empty) empty_bucket(res); } + } else if (nextitem) { + /* + * Wait for next item. + */ + FCTXTRACE("nextitem"); + inc_stats(fctx->res, dns_resstatscounter_nextitem); + INSIST(query->dispentry != NULL); + result = dns_dispatch_getnext(query->dispentry, &devent); + if (result != ISC_R_SUCCESS) + fctx_done(fctx, result, __LINE__); } else if (result == ISC_R_SUCCESS && !HAVE_ANSWER(fctx)) { /* * All has gone well so far, but we are waiting for the diff --git a/lib/dns/tests/dispatch_test.c b/lib/dns/tests/dispatch_test.c index c26c1eadcd..f52a4c7fdd 100644 --- a/lib/dns/tests/dispatch_test.c +++ b/lib/dns/tests/dispatch_test.c @@ -6,8 +6,6 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -/* $Id$ */ - /*! \file */ #include @@ -16,6 +14,7 @@ #include +#include #include #include #include @@ -90,8 +89,6 @@ ATF_TC_BODY(dispatchset_create, tc) { dns_test_end(); } - - ATF_TC(dispatchset_get); ATF_TC_HEAD(dispatchset_get, tc) { atf_tc_set_md_var(tc, "descr", "test dispatch set round-robin"); @@ -140,6 +137,175 @@ ATF_TC_BODY(dispatchset_get, tc) { dns_test_end(); } +static void +senddone(isc_task_t *task, isc_event_t *event) { + isc_socket_t *socket = event->ev_arg; + + UNUSED(task); + + isc_socket_detach(&socket); + isc_event_free(&event); +} + +static void +nameserver(isc_task_t *task, isc_event_t *event) { + isc_result_t result; + isc_region_t region; + isc_socket_t *dummy; + isc_socket_t *socket = event->ev_arg; + isc_socketevent_t *ev = (isc_socketevent_t *)event; + static unsigned char buf1[16]; + static unsigned char buf2[16]; + + memcpy(buf1, ev->region.base, 12); + memset(buf1 + 12, 0, 4); + buf1[2] |= 0x80; /* qr=1 */ + + memcpy(buf2, ev->region.base, 12); + memset(buf2 + 12, 1, 4); + buf2[2] |= 0x80; /* qr=1 */ + + /* + * send message to be discarded. + */ + region.base = buf1; + region.length = sizeof(buf1); + dummy = NULL; + isc_socket_attach(socket, &dummy); + result = isc_socket_sendto(socket, ®ion, task, senddone, socket, + &ev->address, NULL); + if (result != ISC_R_SUCCESS) + isc_socket_detach(&dummy); + + /* + * send nextitem message. + */ + region.base = buf2; + region.length = sizeof(buf2); + dummy = NULL; + isc_socket_attach(socket, &dummy); + result = isc_socket_sendto(socket, ®ion, task, senddone, socket, + &ev->address, NULL); + if (result != ISC_R_SUCCESS) + isc_socket_detach(&dummy); + isc_event_free(&event); +} + +static dns_dispentry_t *dispentry = NULL; +static isc_boolean_t first = ISC_TRUE; +static unsigned int responses = 0; + +static void +response(isc_task_t *task, isc_event_t *event) { + dns_dispatchevent_t *devent = (dns_dispatchevent_t *)event; + isc_result_t result; + + UNUSED(task); + + if (first) { + result = dns_dispatch_getnext(dispentry, &devent); + ATF_CHECK_EQ(result, ISC_R_SUCCESS); + } else { + dns_dispatch_removeresponse(&dispentry, &devent); + isc_app_shutdown(); + } + first = ISC_FALSE; + responses++; +} + +ATF_TC(dispatch_getnext); +ATF_TC_HEAD(dispatch_getnext, tc) { + atf_tc_set_md_var(tc, "descr", "test dispatch getnext"); +} +ATF_TC_BODY(dispatch_getnext, tc) { + dns_dispatch_t *dispatch = NULL; + isc_region_t region; + isc_result_t result; + isc_sockaddr_t local; + isc_socket_t *dsocket = NULL; + isc_socket_t *socket = NULL; + isc_task_t *task = NULL; + isc_uint16_t id; + struct in_addr ina; + unsigned char message[12]; + unsigned int attrs; + unsigned char rbuf[12]; + + UNUSED(tc); + + result = dns_test_begin(NULL, ISC_TRUE); + ATF_REQUIRE_EQ(result, ISC_R_SUCCESS); + + result = isc_task_create(taskmgr, 0, &task); + ATF_REQUIRE_EQ(result, ISC_R_SUCCESS); + + result = dns_dispatchmgr_create(mctx, NULL, &dispatchmgr); + ATF_REQUIRE_EQ(result, ISC_R_SUCCESS); + + ina.s_addr = htonl(INADDR_LOOPBACK); + isc_sockaddr_fromin(&local, &ina, 0); + attrs = DNS_DISPATCHATTR_IPV4 | DNS_DISPATCHATTR_UDP; + result = dns_dispatch_getudp(dispatchmgr, socketmgr, taskmgr, + &local, 512, 6, 1024, 17, 19, attrs, + attrs, &dispatch); + ATF_REQUIRE_EQ(result, ISC_R_SUCCESS); + + /* + * Create a local udp nameserver on the loopback. + */ + result = isc_socket_create(socketmgr, AF_INET, isc_sockettype_udp, + &socket); + ATF_REQUIRE_EQ(result, ISC_R_SUCCESS); + + ina.s_addr = htonl(INADDR_LOOPBACK); + isc_sockaddr_fromin(&local, &ina, 0); + result = isc_socket_bind(socket, &local, 0); + ATF_REQUIRE_EQ(result, ISC_R_SUCCESS); + + result = isc_socket_getsockname(socket, &local); + ATF_REQUIRE_EQ(result, ISC_R_SUCCESS); + + first = ISC_TRUE; + region.base = rbuf; + region.length = sizeof(rbuf); + result = isc_socket_recv(socket, ®ion, 1, task, nameserver, socket); + ATF_REQUIRE_EQ(result, ISC_R_SUCCESS); + + result = dns_dispatch_addresponse(dispatch, &local, task, response, + NULL, &id, &dispentry); + ATF_REQUIRE_EQ(result, ISC_R_SUCCESS); + + memset(message, 0, sizeof(message)); + message[0] = (id >> 8) & 0xff; + message[1] = id & 0xff; + + isc_socket_attach(dns_dispatch_getsocket(dispatch), &dsocket); + region.base = message; + region.length = sizeof(message); + result = isc_socket_sendto(dsocket, ®ion, task, senddone, dsocket, + &local, NULL); + ATF_REQUIRE_EQ(result, ISC_R_SUCCESS); + + result = isc_app_run(); + ATF_REQUIRE_EQ(result, ISC_R_SUCCESS); + + ATF_CHECK_EQ(responses, 2); + + /* + * Shutdown nameserver. + */ + isc_socket_cancel(socket, task, ISC_SOCKCANCEL_RECV); + isc_socket_detach(&socket); + isc_task_detach(&task); + + /* + * Shutdown the dispatch. + */ + dns_dispatch_detach(&dispatch); + dns_dispatchmgr_destroy(&dispatchmgr); + + dns_test_end(); +} /* * Main @@ -147,6 +313,6 @@ ATF_TC_BODY(dispatchset_get, tc) { ATF_TP_ADD_TCS(tp) { ATF_TP_ADD_TC(tp, dispatchset_create); ATF_TP_ADD_TC(tp, dispatchset_get); + ATF_TP_ADD_TC(tp, dispatch_getnext); return (atf_no_error()); } - diff --git a/lib/dns/tests/dnstest.c b/lib/dns/tests/dnstest.c index 8b353c22cb..5167829547 100644 --- a/lib/dns/tests/dnstest.c +++ b/lib/dns/tests/dnstest.c @@ -162,8 +162,6 @@ dns_test_begin(FILE *logfile, isc_boolean_t start_managers) { void dns_test_end(void) { - if (lctx != NULL) - isc_log_destroy(&lctx); if (dst_active) { dst_lib_destroy(); dst_active = ISC_FALSE; @@ -177,6 +175,9 @@ dns_test_end(void) { cleanup_managers(); + if (lctx != NULL) + isc_log_destroy(&lctx); + if (mctx != NULL) isc_mem_destroy(&mctx); } diff --git a/lib/dns/win32/libdns.def.in b/lib/dns/win32/libdns.def.in index 918ebe1bf8..5760b4b461 100644 --- a/lib/dns/win32/libdns.def.in +++ b/lib/dns/win32/libdns.def.in @@ -294,6 +294,7 @@ dns_dispatch_getattributes dns_dispatch_getdscp dns_dispatch_getentrysocket dns_dispatch_getlocaladdress +dns_dispatch_getnext dns_dispatch_getsocket dns_dispatch_gettcp dns_dispatch_gettcp2