[PATCH] resolver: mitigate bad interactions concering inconsistent DNS search domains with ndots usage

[PATCH] resolver: mitigate bad interactions concering inconsistent DNS search domains with ndots usage

[William Pitcock]

When using kubernetes, the search path is configured like so:

  - containername.prod.env.whatever
  - prod.env.whatever
  - env.whatever
  - clusterwide domain

Kubernetes also typically configures the container's resolver with "options ndots:5", which
causes it to look for public domains in the private DNS domains (presumably so their DNS
records can be overridden somehow).

In certain cases where the Kubernetes guest is configured with a clusterwide domain that is
hosted by a certain large CDN provider (*ahem* Cloudflare), the resolver may process
erroneous replies sent from that CDN provider that have an empty A/AAAA record set.

Accordingly, if we detect a configuration that is exotic, as a mitigation, we force all DNS
queries to behave as if they were AF_UNSPEC and return only the records that were actually
requested (either A or AAAA).
---
 src/network/lookup_name.c | 35 ++++++++++++++++++++++++++++++-----
 1 file changed, 30 insertions(+), 5 deletions(-)

diff --git a/src/network/lookup_name.c b/src/network/lookup_name.c
index 209c20f0..861d5872 100644
--- a/src/network/lookup_name.c
+++ b/src/network/lookup_name.c
@@ -95,7 +95,9 @@ static int name_from_hosts(struct address buf[static MAXADDRS], char canon[stati
 struct dpc_ctx {
 	struct address *addrs;
 	char *canon;
+	int wanted;
 	int cnt;
+	int recordcnt[2];
 };
 
 int __dns_parse(const unsigned char *, int, int (*)(void *, int, const void *, int, const void *), void *);
@@ -115,12 +117,16 @@ static int dns_parse_callback(void *c, int rr, const void *data, int len, const
 	switch (rr) {
 	case RR_A:
 		if (len != 4) return -1;
+		ctx->recordcnt[0]++;
+		if (ctx->wanted && rr != ctx->wanted) return 0;
 		ctx->addrs[ctx->cnt].family = AF_INET;
 		ctx->addrs[ctx->cnt].scopeid = 0;
 		memcpy(ctx->addrs[ctx->cnt++].addr, data, 4);
 		break;
 	case RR_AAAA:
 		if (len != 16) return -1;
+		ctx->recordcnt[1]++;
+		if (ctx->wanted && rr != ctx->wanted) return 0;
 		ctx->addrs[ctx->cnt].family = AF_INET6;
 		ctx->addrs[ctx->cnt].scopeid = 0;
 		memcpy(ctx->addrs[ctx->cnt++].addr, data, 16);
@@ -134,7 +140,7 @@ static int dns_parse_callback(void *c, int rr, const void *data, int len, const
 	return 0;
 }
 
-static int name_from_dns(struct address buf[static MAXADDRS], char canon[static 256], const char *name, int family, const struct resolvconf *conf)
+static int name_from_dns(struct address buf[static MAXADDRS], char canon[static 256], const char *name, int family, const struct resolvconf *conf, int searchpath_len)
 {
 	unsigned char qbuf[2][280], abuf[2][512];
 	const unsigned char *qp[2] = { qbuf[0], qbuf[1] };
@@ -148,7 +154,12 @@ static int name_from_dns(struct address buf[static MAXADDRS], char canon[static
 	};
 
 	for (i=0; i<2; i++) {
-		if (family != afrr[i].af) {
+		if (family != afrr[i].af) ctx.wanted = afrr[i].rr;
+
+		/* If we are using search paths longer than 1 domain, or we have changed the
+		 * ndots setting to be greater than 1, then we need to always treat the query
+		 * as if it were AF_UNSPEC to ensure results are consistent. */
+		if (family != afrr[i].af || searchpath_len > 1 || conf->ndots > 1) {
 			qlens[nq] = __res_mkquery(0, name, 1, afrr[i].rr,
 				0, 0, 0, qbuf[nq], sizeof *qbuf);
 			if (qlens[nq] == -1)
@@ -165,7 +176,14 @@ static int name_from_dns(struct address buf[static MAXADDRS], char canon[static
 
 	if (ctx.cnt) return ctx.cnt;
 	if (alens[0] < 4 || (abuf[0][3] & 15) == 2) return EAI_AGAIN;
-	if ((abuf[0][3] & 15) == 0) return EAI_NONAME;
+	if ((abuf[0][3] & 15) == 0) {
+		/* A certain large CDN provider's DNS service erroneously responds to queries with
+		 * a NOERROR(0) response code, while also returning an empty record set. Accordingly,
+		 * check for this and handle it as we would an NXDOMAIN(3) if the record set is empty
+		 * for both A and AAAA records. */
+		if (nq == 2 && (ctx.recordcnt[0] - ctx.recordcnt[1]) == 0) return 0;
+		else return EAI_NONAME;
+	}
 	if ((abuf[0][3] & 15) == 3) return 0;
 	return EAI_FAIL;
 }
@@ -175,6 +193,7 @@ static int name_from_dns_search(struct address buf[static MAXADDRS], char canon[
 	char search[256];
 	struct resolvconf conf;
 	size_t l, dots;
+	int searchpath_len = 0;
 	char *p, *z;
 
 	if (__get_resolv_conf(&conf, search, sizeof search) < 0) return -1;
@@ -184,6 +203,12 @@ static int name_from_dns_search(struct address buf[static MAXADDRS], char canon[
 	for (dots=l=0; name[l]; l++) if (name[l]=='.') dots++;
 	if (dots >= conf.ndots || name[l-1]=='.') *search = 0;
 
+	/* Count the number of domains in the search path. */
+	if (*search) {
+		size_t n;
+		for (searchpath_len = n = 0; search[n]; n++) if (isspace(search[n])) searchpath_len++;
+	}
+
 	/* This can never happen; the caller already checked length. */
 	if (l >= 256) return EAI_NONAME;
 
@@ -201,13 +226,13 @@ static int name_from_dns_search(struct address buf[static MAXADDRS], char canon[
 		if (z-p < 256 - l - 1) {
 			memcpy(canon+l+1, p, z-p);
 			canon[z-p+1+l] = 0;
-			int cnt = name_from_dns(buf, canon, canon, family, &conf);
+			int cnt = name_from_dns(buf, canon, canon, family, &conf, searchpath_len);
 			if (cnt) return cnt;
 		}
 	}
 
 	canon[l] = 0;
-	return name_from_dns(buf, canon, name, family, &conf);
+	return name_from_dns(buf, canon, name, family, &conf, searchpath_len);
 }
 
 static const struct policy {
-- 
2.16.3