From 1e9848fb2b4573711c02696dc4ee6540de4c81f6 Mon Sep 17 00:00:00 2001
From: Mark Andrews <marka@isc.org>
Date: Tue, 20 Apr 2010 07:28:52 +0000
Subject: [PATCH] 2874.   [bug]           Cache lack of EDNS support only after
 the server                         successfully responds to the query using
 plain DNS.                         [RT #20930]

---
 CHANGES            |  4 +++
 lib/dns/resolver.c | 76 ++++++++++++++++++++++++++++++++++++++++------
 2 files changed, 70 insertions(+), 10 deletions(-)

diff --git a/CHANGES b/CHANGES
index cce109dae0..6c7e2f8437 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1,3 +1,7 @@
+2874.	[bug]		Cache lack of EDNS support only after the server
+			successfully responds to the query using plain DNS.
+			[RT #20930]
+
 2873.	[bug]		Canceling a dynamic update via the dns/client module
 			could trigger an assertion failure. [RT #21133]
 
diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c
index 12f89e68f3..8d4b5ed586 100644
--- a/lib/dns/resolver.c
+++ b/lib/dns/resolver.c
@@ -15,7 +15,7 @@
  * PERFORMANCE OF THIS SOFTWARE.
  */
 
-/* $Id: resolver.c,v 1.419 2010/03/04 22:25:31 marka Exp $ */
+/* $Id: resolver.c,v 1.420 2010/04/20 07:28:52 marka Exp $ */
 
 /*! \file */
 
@@ -203,6 +203,7 @@ struct fetchctx {
 	isc_sockaddrlist_t		bad;
 	isc_sockaddrlist_t		edns;
 	isc_sockaddrlist_t		edns512;
+	isc_sockaddrlist_t		bad_edns;
 	dns_validator_t			*validator;
 	ISC_LIST(dns_validator_t)       validators;
 	dns_db_t *			cache;
@@ -1561,6 +1562,36 @@ fctx_query(fetchctx_t *fctx, dns_adbaddrinfo_t *addrinfo,
 	return (result);
 }
 
+static isc_boolean_t
+bad_edns(fetchctx_t *fctx, isc_sockaddr_t *address) {
+	isc_sockaddr_t *sa;
+
+	for (sa = ISC_LIST_HEAD(fctx->bad_edns);
+	     sa != NULL;
+	     sa = ISC_LIST_NEXT(sa, link)) {
+		if (isc_sockaddr_equal(sa, address))
+			return (ISC_TRUE);
+	}
+
+	return (ISC_FALSE);
+}
+
+static void
+add_bad_edns(fetchctx_t *fctx, isc_sockaddr_t *address) {
+	isc_sockaddr_t *sa;
+
+	if (bad_edns(fctx, address))
+		return;
+
+	sa = isc_mem_get(fctx->res->buckets[fctx->bucketnum].mctx,
+			 sizeof(*sa));
+	if (sa == NULL)
+		return;
+
+	*sa = *address;
+	ISC_LIST_INITANDAPPEND(fctx->bad_edns, sa, link);
+}
+
 static isc_boolean_t
 triededns(fetchctx_t *fctx, isc_sockaddr_t *address) {
 	isc_sockaddr_t *sa;
@@ -3129,6 +3160,14 @@ fctx_destroy(fetchctx_t *fctx) {
 		isc_mem_put(res->buckets[bucketnum].mctx, sa, sizeof(*sa));
 	}
 
+	for (sa = ISC_LIST_HEAD(fctx->bad_edns);
+	     sa != NULL;
+	     sa = next_sa) {
+		next_sa = ISC_LIST_NEXT(sa, link);
+		ISC_LIST_UNLINK(fctx->bad_edns, sa, link);
+		isc_mem_put(res->buckets[bucketnum].mctx, sa, sizeof(*sa));
+	}
+
 	isc_timer_detach(&fctx->timer);
 	dns_message_destroy(&fctx->rmessage);
 	dns_message_destroy(&fctx->qmessage);
@@ -3500,6 +3539,7 @@ fctx_create(dns_resolver_t *res, dns_name_t *name, dns_rdatatype_t type,
 	ISC_LIST_INIT(fctx->bad);
 	ISC_LIST_INIT(fctx->edns);
 	ISC_LIST_INIT(fctx->edns512);
+	ISC_LIST_INIT(fctx->bad_edns);
 	ISC_LIST_INIT(fctx->validators);
 	fctx->validator = NULL;
 	fctx->find = NULL;
@@ -6620,7 +6660,26 @@ resquery_response(isc_task_t *task, isc_event_t *event) {
 	 * ensured by the dispatch code).
 	 */
 
-
+	/*
+	 * We have an affirmative response to the query and we have
+	 * previously got a response from this server which indicated
+	 * EDNS may not be supported so we can now cache the lack of
+	 * EDNS support.
+	 */
+	if (opt == NULL &&
+	    (message->rcode == dns_rcode_noerror ||
+	     message->rcode == dns_rcode_nxdomain ||
+	     message->rcode == dns_rcode_refused ||
+	     message->rcode == dns_rcode_yxdomain) &&
+	     bad_edns(fctx, &query->addrinfo->sockaddr)) {
+		char addrbuf[ISC_SOCKADDR_FORMATSIZE];
+		isc_sockaddr_format(&query->addrinfo->sockaddr, addrbuf,
+				    sizeof(addrbuf));
+		dns_adb_changeflags(fctx->adb, query->addrinfo,
+				    DNS_FETCHOPT_NOEDNS0,
+				    DNS_FETCHOPT_NOEDNS0);
+	}
+		
 	/*
 	 * Deal with truncated responses by retrying using TCP.
 	 */
@@ -6675,9 +6734,9 @@ resquery_response(isc_task_t *task, isc_event_t *event) {
 	if (message->rcode != dns_rcode_noerror &&
 	    message->rcode != dns_rcode_nxdomain) {
 		if (((message->rcode == dns_rcode_formerr ||
-		     message->rcode == dns_rcode_notimp) ||
-		    (message->rcode == dns_rcode_servfail &&
-		     dns_message_getopt(message) == NULL)) &&
+		      message->rcode == dns_rcode_notimp) ||
+		     (message->rcode == dns_rcode_servfail &&
+		      dns_message_getopt(message) == NULL)) &&
 		    (query->options & DNS_FETCHOPT_NOEDNS0) == 0) {
 			/*
 			 * It's very likely they don't like EDNS0.
@@ -6693,12 +6752,9 @@ resquery_response(isc_task_t *task, isc_event_t *event) {
 			options |= DNS_FETCHOPT_NOEDNS0;
 			resend = ISC_TRUE;
 			/*
-			 * Remember that they don't like EDNS0.
+			 * Remember that they may not like EDNS0.
 			 */
-			if (message->rcode != dns_rcode_servfail)
-				dns_adb_changeflags(fctx->adb, query->addrinfo,
-						    DNS_FETCHOPT_NOEDNS0,
-						    DNS_FETCHOPT_NOEDNS0);
+			add_bad_edns(fctx, &query->addrinfo->sockaddr);
 			inc_stats(fctx->res, dns_resstatscounter_edns0fail);
 		} else if (message->rcode == dns_rcode_formerr) {
 			if (ISFORWARDER(query->addrinfo)) {