lib/52578: gethostbyname has EDNS fallback but getaddrinfo does not

[bemasc%google.com@localhost]

>Number:         52578
>Category:       lib
>Synopsis:       gethostbyname has EDNS fallback but getaddrinfo does not
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    lib-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Thu Sep 28 19:45:00 +0000 2017
>Originator:     Benjamin M. Schwartz
>Release:        master
>Organization:
Google LLC
>Environment:
I diagnosed this issuing on Android, which uses a fork of NetBSD's getaddrinfo.
>Description:
The EDNS0 extension allow dns clients (stub resolvers) to signal some additional metadata to the recursive resolver, especially the client's largest supported response size.  However, some old and broken DNS resolvers would return SERVFAIL or similar upon encountering any EDNS0 extension.  BIND therefore includes fallback logic (in the res_nquery function) to retry failed requests without EDNS0.  This logic is also present in NetBSD's resolver (based on BIND), and applies correctly to gethostbyname.

However, getaddrinfo doesn't use res_nquery.  Instead, it uses a function called res_queryN, which contains a modified copy of res_nquery.  This copy likely predates the introduction of the EDNS fallback mechanism, and has not been kept up to date.  As a result, getaddrinfo does not have the same fallback behavior as gethostbyname.

The obvious solution is to sync res_queryN to the current res_nquery, reducing version skew and making getaddrinfo's behavior match gethostbyname's.
>How-To-Repeat:

>Fix:
Apologies for the git-formatted patch.  I'm not used to CVS.

>From e046666a4141b85585ebc6a673de83e43bdef009 Mon Sep 17 00:00:00 2001
From: Ben Schwartz <bemasc%google.com@localhost>
Date: Thu, 28 Sep 2017 10:36:44 -0400
Subject: [PATCH 1/1] Extend EDNS retry logic to getaddrinfo

Currently, if EDNS is enabled, but the query fails,
gethostbyname will retry without EDNS.  This behavior
comes directly from upstream BIND, in the res_nquery
function.  However, the corresponding behavior is not
present in res_queryN, a copy of res_nquery that is
used by getaddrinfo.  This change extends res_queryN
to match res_nquery's retry behavior.

This change also sets the AD bit when DNSSEC is enabled
(RFC 6840 Section 5.7).
---
 lib/libc/net/getaddrinfo.c    | 17 ++++++++++++++++-
 lib/libc/resolv/res_mkquery.c |  1 +
 2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/lib/libc/net/getaddrinfo.c b/lib/libc/net/getaddrinfo.c
index 27146aeae..6f1c60137 100644
--- a/lib/libc/net/getaddrinfo.c
+++ b/lib/libc/net/getaddrinfo.c
@@ -2558,8 +2558,12 @@ res_queryN(const char *name, /* domain name */ struct res_target *target,
 		int class, type;
 		u_char *answer;
 		int anslen;
+		u_int oflags;
 
 		hp = (HEADER *)(void *)t->answer;
+		oflags = res->_flags;
+
+again:
 		hp->rcode = NOERROR;	/* default */
 
 		/* make it easier... */
@@ -2575,7 +2579,8 @@ res_queryN(const char *name, /* domain name */ struct res_target *target,
 		n = res_nmkquery(res, QUERY, name, class, type, NULL, 0, NULL,
 		    buf, (int)sizeof(buf));
 #ifdef RES_USE_EDNS0
-		if (n > 0 && (res->options & RES_USE_EDNS0) != 0)
+		if (n > 0 && (res->_flags & RES_F_EDNS0ERR) == 0 &&
+		    (res->options & (RES_USE_EDNS0|RES_USE_DNSSEC)) != 0)
 			n = res_nopt(res, n, buf, (int)sizeof(buf), anslen);
 #endif
 		if (n <= 0) {
@@ -2600,6 +2605,16 @@ res_queryN(const char *name, /* domain name */ struct res_target *target,
 
 		if (n < 0 || hp->rcode != NOERROR || ntohs(hp->ancount) == 0) {
 			rcode = hp->rcode;	/* record most recent error */
+#ifdef RES_USE_EDNS0
+			/* if the query choked with EDNS0, retry without EDNS0 */
+			if ((res->options & (RES_USE_EDNS0|RES_USE_DNSSEC)) != 0 &&
+			    ((oflags ^ res->_flags) & RES_F_EDNS0ERR) != 0) {
+				res->_flags |= RES_F_EDNS0ERR;
+				if (res->options & RES_DEBUG)
+					printf(";; res_nquery: retry without EDNS0\n");
+				goto again;
+			}
+#endif
 #ifdef DEBUG
 			if (res->options & RES_DEBUG)
 				printf(";; rcode = %u, ancount=%u\n", hp->rcode,
diff --git a/lib/libc/resolv/res_mkquery.c b/lib/libc/resolv/res_mkquery.c
index c7a17ead6..242ce0b14 100644
--- a/lib/libc/resolv/res_mkquery.c
+++ b/lib/libc/resolv/res_mkquery.c
@@ -142,6 +142,7 @@ res_nmkquery(res_state statp,
 	hp->id = htons(statp->id);
 	hp->opcode = op;
 	hp->rd = (statp->options & RES_RECURSE) != 0U;
+	hp->ad = (statp->options & RES_USE_DNSSEC) != 0U;
 	hp->rcode = NOERROR;
 	cp = buf + HFIXEDSZ;
 	ep = buf + buflen;
-- 
2.14.2.822.g60be5d43e6-goog