lib/cache: fix CVE-2019-10191

Don't stash a packet with mismatching QNAME+QTYPE.
When receiving an NXDOMAIN or NODATA packet in an insecure zone,
it would get cached with KR_RANK_INSECURE regardless of mismatch
in QNAME.  If the 0x20 pattern was preserved in the fake QNAME,
such packet would then be used to answer queries with matching QNAME,
even if there's no proof that this QNAME is insecure.

diff --git a/NEWS b/NEWS
index dcf525adb31a8abd2f5e113c6e2262973d9187bf..acca64d7055757443d01292604fa4a575b0ce783 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -1,6 +1,10 @@
 Knot Resolver 4.x.y (2019-0m-dd)
 ================================
 
+Security
+--------
+- fix CVE-2019-10191: caching negative answer with forged QNAME+QTYPE
+
 Improvements
 ------------
 - DNS-over-HTTPS: answers include `access-control-allow-origin: *` (!823)

diff --git a/daemon/lua/kres-gen.lua b/daemon/lua/kres-gen.lua
index 02e3ea2b8bf7b2ecef07cc5e75f1ca3f89b88c13..2565ac3f1ae1ea859067aee54568aad749f66b43 100644 (file)
--- a/daemon/lua/kres-gen.lua
+++ b/daemon/lua/kres-gen.lua
@@ -124,6 +124,7 @@ struct kr_qflags {
        _Bool DNS64_MARK : 1;
        _Bool CACHE_TRIED : 1;
        _Bool NO_NS_FOUND : 1;
+       _Bool PKT_IS_SANE : 1;
 };
 typedef struct {
        knot_rrset_t **at;

diff --git a/lib/cache/api.c b/lib/cache/api.c
index bb7560ec9aa32f012fc1d23cb12b08549302ffab..ece97b96d5f2f1c5e6d030186a76dda17bb4d394 100644 (file)
--- a/lib/cache/api.c
+++ b/lib/cache/api.c
@@ -414,7 +414,7 @@ int cache_stash(kr_layer_t *ctx, knot_pkt_t *pkt)
        /* LATER(optim.): typically we also have corresponding NS record in the list,
         * so we might save a cache operation. */
 
-       if (check_dname_for_lf(knot_pkt_qname(pkt), qry)) {
+       if (qry->flags.PKT_IS_SANE && check_dname_for_lf(knot_pkt_qname(pkt), qry)) {
                stash_pkt(pkt, qry, req, needs_pkt);
        }
 

diff --git a/lib/layer/iterate.c b/lib/layer/iterate.c
index bedc54c7bb4650659d9d818c58d1483aec29defd..518bd474d1615f9975bb24baa43bb7deec62e3ab 100644 (file)
--- a/lib/layer/iterate.c
+++ b/lib/layer/iterate.c
@@ -82,6 +82,8 @@ static bool is_paired_to_query(const knot_pkt_t *answer, struct kr_query *query)
        uint16_t qtype = query->stype;
        const knot_dname_t *qname = minimized_qname(query, &qtype);
 
+       /* ID should already match, thanks to session_tasklist_del_msgid()
+        * in worker_submit(), but it won't hurt to check again. */
        return query->id      == knot_wire_get_id(answer->wire) &&
               knot_wire_get_qdcount(answer->wire) > 0 &&
               query->sclass  == knot_pkt_qclass(answer) &&
@@ -1015,6 +1017,7 @@ static int resolve(kr_layer_t *ctx, knot_pkt_t *pkt)
        if (!query) {
                return ctx->state;
        }
+       query->flags.PKT_IS_SANE = false;
 
        WITH_VERBOSE(query) {
                if (query->flags.TRACE) {
@@ -1058,6 +1061,10 @@ static int resolve(kr_layer_t *ctx, knot_pkt_t *pkt)
                return KR_STATE_CONSUME;
        }
 
+       /* If exiting above here, there's no sense to put it into packet cache.
+        * The most important part is to check for spoofing: is_paired_to_query() */
+       query->flags.PKT_IS_SANE = true;
+
 #ifndef NOVERBOSELOG
        const knot_lookup_t *rcode = knot_lookup_by_id(knot_rcode_names, knot_wire_get_rcode(pkt->wire));
 #endif

diff --git a/lib/rplan.h b/lib/rplan.h
index 6e93afc71b003c9db84e3017784f0393c24b39cf..15ca5633faa504cfeafd984243e4e8860369b485 100644 (file)
--- a/lib/rplan.h
+++ b/lib/rplan.h
@@ -64,6 +64,8 @@ struct kr_qflags {
        bool DNS64_MARK : 1;     /**< Internal mark for dns64 module. */
        bool CACHE_TRIED : 1;    /**< Internal to cache module. */
        bool NO_NS_FOUND : 1;    /**< No valid NS found during last PRODUCE stage. */
+       bool PKT_IS_SANE : 1;    /**< Set by iterator in consume phase to indicate whether
+                                 * some basic aspects of the packet are OK, e.g. QNAME. */
 };
 
 /** Combine flags together.  This means set union for simple flags. */