4408. [func] Continue waiting for expected response when we the

response we get does not match the request. [RT #41026]

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]
 

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);
 

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) {

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 */

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.

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

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 <config.h>
@@ -16,6 +14,7 @@
 
 #include <unistd.h>
 
+#include <isc/app.h>
 #include <isc/buffer.h>
 #include <isc/socket.h>
 #include <isc/task.h>
@@ -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, &region, 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, &region, 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, &region, 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, &region, 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());
 }
-

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);
 }

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