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 46cfe27e03659bae7c0cf32a169dfe4885335d14..53b11e6ee60b9213c8433df31027686484e536df 100644 (file)
--- 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 a730899325f68f06b5f07137b1dfdd2232c6f33a..03366acea34b6ddebfa72d5e9720cb3124b88f97 100644 (file)
--- 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);
        }