From: Vladimír Čunát <vladimir.cunat@nic.cz>
Date: Tue, 6 Dec 2022 11:50:10 +0000 (+0100)
Subject: iterate: no longer accept DNS messages with trailing data
X-Git-Tag: v5.6.0~8^2
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=af4f44bd8a2a9fa76213fe2868600cc87be4ba32;p=thirdparty%2Fknot-resolver.git

iterate: no longer accept DNS messages with trailing data

We can get stricter here;
with negligible fraction of real-life names regressing.
---

diff --git a/NEWS b/NEWS
index 46cfe27e0..53b11e6ee 100644
--- a/NEWS
+++ b/NEWS
@@ -4,6 +4,7 @@ Knot Resolver 5.x.y (202y-mm-dd)
 Improvements
 ------------
 - depend on jemalloc, preferably, to improve memory usage (!1353)
+- no longer accept DNS messages with trailing data (!1365)
 - policy.STUB: avoid applying aggressive DNSSEC denial proofs (!1364)
 - policy.STUB: avoid copying +dnssec flag from client to upstream (!1364)
 
diff --git a/lib/layer/iterate.c b/lib/layer/iterate.c
index a73089932..03366acea 100644
--- a/lib/layer/iterate.c
+++ b/lib/layer/iterate.c
@@ -1039,12 +1039,6 @@ static int resolve(kr_layer_t *ctx, knot_pkt_t *pkt)
 	/* Check for packet processing errors first.
 	 * Note - we *MUST* check if it has at least a QUESTION,
 	 * otherwise it would crash on accessing QNAME. */
-#ifdef STRICT_MODE
-	if (pkt->parsed < pkt->size) {
-		VERBOSE_MSG("<= pkt contains excessive data\n");
-		return KR_STATE_FAIL;
-	} else
-#endif
 	if (pkt->parsed <= KNOT_WIRE_HEADER_SIZE) {
 		if (pkt->parsed == KNOT_WIRE_HEADER_SIZE && knot_wire_get_rcode(pkt->wire) == KNOT_RCODE_FORMERR) {
 			/* This is a special case where we get valid header with FORMERR and nothing else.
@@ -1080,6 +1074,7 @@ static int resolve(kr_layer_t *ctx, knot_pkt_t *pkt)
 	}
 
 	/* If exiting above here, there's no sense to put it into packet cache.
+	 * Having "extra bytes" at the end of DNS message is considered SANE here.
 	 * The most important part is to check for spoofing: is_paired_to_query() */
 	query->flags.PKT_IS_SANE = true;
 
@@ -1135,6 +1130,15 @@ static int resolve(kr_layer_t *ctx, knot_pkt_t *pkt)
 		break;
 	}
 
+	/* Check for "extra bytes" is deferred, so that RCODE-based failures take priority. */
+	if (ret != KR_STATE_FAIL && pkt->parsed < pkt->size) {
+		VERBOSE_MSG("<= malformed response with %zu extra bytes\n",
+				pkt->size - pkt->parsed);
+		ret = KR_STATE_FAIL;
+		if (selection_error == KR_SELECTION_OK)
+			selection_error = KR_SELECTION_MALFORMED;
+	}
+
 	if (query->server_selection.initialized) {
 		query->server_selection.error(query, req->upstream.transport, selection_error);
 	}