resolv: fix resolver to return TRY_AGAIN on timeout

This fixes the internal __dns_lookup to get a h_errno pointer so
it works nicely with the _r variants. Additionally the function is
modified to permanent error if the static buffer lengths are not
enough. And finally it fixed to return TRY_AGAIN if the nameservers
timeout.

res_search is fixed to continue searching if we receive TRY_AGAIN.
It could be a problem with the specific search domain's server
and not necessarily a problem in the recursive resolver we are
querying. For same reason, it does not make sense to differentiate
timeout or SERVFAIL error reply.

The biggest issue this fixes is that we now properly set h_errno
to TRY_AGAIN if upstream nameserver(s) timed out. Previously we
would have returned NETDB_INTERNAL.

Signed-off-by: Timo Teräs <timo.teras@iki.fi>
Signed-off-by: Natanael Copa <ncopa@alpinelinux.org>

1 files changed, 49 insertions, 46 deletions

diff --git a/libc/inet/resolv.c b/libc/inet/resolv.c
index 8781196c6..07dff18dd 100644
--- a/libc/inet/resolv.c
+++ b/libc/inet/resolv.c
@@ -456,7 +456,8 @@ extern int __read_etc_hosts_r(parser_t *parser,
 extern int __dns_lookup(const char *name,
 		int type,
 		unsigned char **outpacket,
-		struct resolv_answer *a) attribute_hidden;
+		struct resolv_answer *a,
+		int *h_errnop) attribute_hidden;
 extern int __encode_dotted(const char *dotted,
 		unsigned char *dest,
 		int maxlen) attribute_hidden;
@@ -1233,7 +1234,8 @@ static int __decode_answer(const unsigned char *message, /* packet */
 int attribute_hidden __dns_lookup(const char *name,
 		int type,
 		unsigned char **outpacket,
-		struct resolv_answer *a)
+		struct resolv_answer *a,
+		int *h_errnop)
 {
 	/* Protected by __resolv_lock: */
 	static int last_ns_num = 0;
@@ -1265,11 +1267,15 @@ int attribute_hidden __dns_lookup(const char *name,
 	fd = -1;
 	lookup = NULL;
 	name_len = strlen(name);
-	if ((unsigned)name_len >= MAXDNAME - MAXLEN_searchdomain - 2)
-		goto fail; /* paranoia */
+	if ((unsigned)name_len >= MAXDNAME - MAXLEN_searchdomain - 2) {
+		*h_errnop = NO_RECOVERY;
+		goto fail1; /* paranoia */
+	}
 	lookup = malloc(name_len + 1/*for '.'*/ + MAXLEN_searchdomain + 1);
-	if (!packet || !lookup || !name[0])
-		goto fail;
+	if (!packet || !lookup || !name[0]) {
+		*h_errnop = NO_RECOVERY;
+		goto fail1;
+	}
 	ends_with_dot = (name[name_len - 1] == '.');
 	/* no strcpy! paranoia, user might change name[] under us */
 	memcpy(lookup, name, name_len);
@@ -1337,8 +1343,10 @@ int attribute_hidden __dns_lookup(const char *name,
 		h.rd = 1;
 		DPRINTF("encoding header\n", h.rd);
 		i = __encode_header(&h, packet, PACKETSZ);
-		if (i < 0)
-			goto fail;
+		if (i < 0) {
+			*h_errnop = NO_RECOVERY;
+			goto fail1;
+		}
 
 		/* encode question */
 		DPRINTF("lookup name: %s\n", lookup);
@@ -1346,8 +1354,10 @@ int attribute_hidden __dns_lookup(const char *name,
 		q.qtype = type;
 		q.qclass = C_IN; /* CLASS_IN */
 		j = __encode_question(&q, packet+i, PACKETSZ-i);
-		if (j < 0)
-			goto fail;
+		if (j < 0) {
+			*h_errnop = NO_RECOVERY;
+			goto fail1;
+		}
 		packet_len = i + j;
 
 		/* send packet */
@@ -1473,7 +1483,7 @@ int attribute_hidden __dns_lookup(const char *name,
 				/* no more search domains to try */
 			}
 			/* dont loop, this is "no such host" situation */
-			h_errno = HOST_NOT_FOUND;
+			*h_errnop = HOST_NOT_FOUND;
 			goto fail1;
 		}
 		/* Insert other non-fatal errors here, which do not warrant
@@ -1485,7 +1495,7 @@ int attribute_hidden __dns_lookup(const char *name,
 
 		/* Code below won't work correctly with h.ancount == 0, so... */
 		if (h.ancount <= 0) {
-			h_errno = NO_DATA; /* [is this correct code to check for?] */
+			*h_errnop = NO_DATA; /* [is this correct code to check for?] */
 			goto fail1;
 		}
 		pos = HFIXEDSZ;
@@ -1564,8 +1574,7 @@ int attribute_hidden __dns_lookup(const char *name,
 		variant = -1;
 	} while (retries_left > 0);
 
- fail:
-	h_errno = NETDB_INTERNAL;
+	*h_errnop = TRY_AGAIN;
  fail1:
 	if (fd != -1)
 		close(fd);
@@ -2106,9 +2115,8 @@ int gethostbyname_r(const char *name,
 	 * we'll need space of one in_addr + two addr_list[] elems */
 	a.buflen = buflen - ((sizeof(addr_list[0]) * 2 + sizeof(struct in_addr)));
 	a.add_count = 0;
-	packet_len = __dns_lookup(name, T_A, &packet, &a);
+	packet_len = __dns_lookup(name, T_A, &packet, &a, h_errnop);
 	if (packet_len < 0) {
-		*h_errnop = HOST_NOT_FOUND;
 		DPRINTF("__dns_lookup returned < 0\n");
 		return TRY_AGAIN;
 	}
@@ -2292,9 +2300,8 @@ int gethostbyname2_r(const char *name,
 		int packet_len;
 
 /* Hmm why we memset(a) to zeros only once? */
-		packet_len = __dns_lookup(buf, T_AAAA, &packet, &a);
+		packet_len = __dns_lookup(buf, T_AAAA, &packet, &a, h_errnop);
 		if (packet_len < 0) {
-			*h_errnop = HOST_NOT_FOUND;
 			return TRY_AGAIN;
 		}
 		strncpy(buf, a.dotted, buflen);
@@ -2450,9 +2457,8 @@ int gethostbyaddr_r(const void *addr, socklen_t addrlen,
 	memset(&a, '\0', sizeof(a));
 	for (;;) {
 /* Hmm why we memset(a) to zeros only once? */
-		packet_len = __dns_lookup(buf, T_PTR, &packet, &a);
+		packet_len = __dns_lookup(buf, T_PTR, &packet, &a, h_errnop);
 		if (packet_len < 0) {
-			*h_errnop = HOST_NOT_FOUND;
 			return TRY_AGAIN;
 		}
 
@@ -3091,7 +3097,7 @@ int res_query(const char *dname, int class, int type,
 	}
 
 	memset(&a, '\0', sizeof(a));
-	i = __dns_lookup(dname, type, &packet, &a);
+	i = __dns_lookup(dname, type, &packet, &a, &h_errno);
 
 	if (i < 0) {
 		if (!h_errno) /* TODO: can this ever happen? */
@@ -3117,14 +3123,13 @@ libc_hidden_def(res_query)
  */
 #define __TRAILING_DOT	(1<<0)
 #define __GOT_NODATA	(1<<1)
-#define __GOT_SERVFAIL	(1<<2)
+#define __GOT_TRYAGAIN	(1<<2)
 #define __TRIED_AS_IS	(1<<3)
 int res_search(const char *name, int class, int type, u_char *answer,
 		int anslen)
 {
 	const char *cp;
 	char **domain;
-	HEADER *hp = (HEADER *)(void *)answer;
 	unsigned dots;
 	unsigned state;
 	int ret, saved_herrno;
@@ -3189,19 +3194,9 @@ int res_search(const char *name, int class, int type, u_char *answer,
 			if (ret > 0)
 				return ret;
 
-			/*
-			 * If no server present, give up.
-			 * If name isn't found in this domain,
-			 * keep trying higher domains in the search list
-			 * (if that's enabled).
-			 * On a NO_DATA error, keep trying, otherwise
-			 * a wildcard entry of another type could keep us
-			 * from finding this entry higher in the domain.
-			 * If we get some other error (negative answer or
-			 * server failure), then stop searching up,
-			 * but try the input name below in case it's
-			 * fully-qualified.
-			 */
+			/* our resolver refused to talk to us -
+			 * no sense to retry, as the retry would likely
+			 * fail too */
 			if (errno == ECONNREFUSED) {
 				h_errno = TRY_AGAIN;
 				return -1;
@@ -3209,21 +3204,29 @@ int res_search(const char *name, int class, int type, u_char *answer,
 
 			switch (h_errno) {
 				case NO_DATA:
+					/* Keep trying, otherwise a
+					 * wildcard entry of another type
+					 * could keep us from finding this
+					 * entry from higher in the domain
+					 * search. */
 					state |= __GOT_NODATA;
-					/* FALLTHROUGH */
+					break;
 				case HOST_NOT_FOUND:
-					/* keep trying */
+					/* Not found - keep trying higher
+					 * domains in the search list. */
 					break;
 				case TRY_AGAIN:
-					if (hp->rcode == SERVFAIL) {
-						/* try next search element, if any */
-						state |= __GOT_SERVFAIL;
-						break;
-					}
-					/* FALLTHROUGH */
+					/* Server error or timeout. Could
+					 * be caused by a problem in servers
+					 * our resolver queried. Keep trying
+					 * search, but remember that there
+					 * was a temporary problem. */
+					state |= __GOT_TRYAGAIN;
+					break;
 				default:
 					/* anything else implies that we're done */
 					done = 1;
+					break;
 			}
 			/*
 			 * if we got here for some reason other than DNSRCH,
@@ -3257,13 +3260,13 @@ int res_search(const char *name, int class, int type, u_char *answer,
 		h_errno = saved_herrno;
 	else if (state & __GOT_NODATA)
 		h_errno = NO_DATA;
-	else if (state & __GOT_SERVFAIL)
+	else if (state & __GOT_TRYAGAIN)
 		h_errno = TRY_AGAIN;
 	return -1;
 }
 #undef __TRAILING_DOT
 #undef __GOT_NODATA
-#undef __GOT_SERVFAIL
+#undef __GOT_TRYAGAIN
 #undef __TRIED_AS_IS
 /*
  * Perform a call on res_query on the concatenation of name and domain,