[prev in list] [next in list] [prev in thread] [next in thread] 

List:       oss-security
Subject:    [oss-security] Knot Resolver 4.1.0 security release
From:       Vladimír Čunát <vladimir.cunat () nic ! cz>
Date:       2019-07-14 7:27:13
Message-ID: 3464b63f-21cb-c894-a832-63e1a8d07f88 () nic ! cz
[Download RAW message or body]

[Attachment #2 (multipart/mixed)]

[Attachment #4 (multipart/mixed)]


Hello.

This Wednesday there was a Knot Resolver release and embargo lift for
two CVEs, both allowing the server to incorrectly accept DNS records:
CVE-2019-10190 and CVE-2019-10191; more details at the end of this e-mail.

We apologize for forgetting our responsibility to also post to
oss-security on that day.   Thanks to Salvatore Bonaccorso for notifying us.

Minimal patches are attached, but we generally do not recommend
backporting them.   Announcement:
https://lists.nic.cz/pipermail/knot-resolver-users/2019/000189.html

--Vladimir (upstream dev, discovered and fixed)

#### CVE-2019-10190

Impact
======
Under certain circumstances, improper input validation bug in DNS
resolver component of Knot Resolver allows remote attacker to bypass
DNSSEC validation for non-existence answer.

An NXDOMAIN answer would get passed through to the client even if its
DNSSEC validation failed, instead of sending a SERVFAIL packet.
Caching is not affected by this particular bug but see the other CVE.


[Affected version (required)]:
3.2.0 <= Knot Resolver <= 4.0.0

[Vulnerability type (required)]:
CWE-20: Improper Input Validation

[Affected component (required)]:
resolver

[Impact of exploitation (required)]:
Under certain circumstances this bug allows an attacker to hijack
DNS domains.

[Description of vulnerability]:
Under certain circumstances, improper input validation bug in DNS
resolver component of Knot Resolver allows remote attacker to bypass
DNSSEC validation for non-existence answer.

An NXDOMAIN answer would get passed through to the client even if its
DNSSEC validation failed, instead of sending a SERVFAIL packet.
Caching is not affected by this particular bug but see the other CVE.

Attack Vector (AV): Network
Attack Complexity (AC): Low
Privileges Required (PR): None
User Interaction (UI): None
Scope (S): Moderate
Confidentiality (C): None
Integrity (I): Medium
Availability (A): None

Technical Details:
CWE-20



#### CVE-2019-10191

Impact
======
Under certain circumstances this bug allows an network attacker with
ability to spoof packets to downgrade a DNSSEC-secured domain to
DNSSEC-insecure state, thus opening possibilities for further attacks.


[Affected version (required)]:
Knot Resolver <= 4.0.0
(probably since 2.0.0, we did not check older versions thoroughly)

[Vulnerability type (required)]:
CWE-20: Improper Input Validation

[Affected component (required)]:
resolver

[Impact of exploitation (required)]:
Under certain circumstances this bug allows an attacker to downgrade
DNSSEC-secure domains to DNSSEC-insecure state, opening possibility of
domain hijack using attacks against insecure DNS protocol.

[Description of vulnerability]:
Improper input validation bug in DNS resolver component of Knot Resolver
allows remote attacker to poison cache by an unsigned negative answer.

Attack Vector (AV): Network
Attack Complexity (AC): Low
Privileges Required (PR): None
User Interaction (UI): None
Scope (S): All
Confidentiality (C): None
Integrity (I): High
Availability (A): None

Technical Details:
CWE-20


["CVE-2019-10190.patch" (text/x-patch)]

diff --git a/lib/resolve.c b/lib/resolve.c
index 59420238..c39eab41 100644
--- a/lib/resolve.c
+++ b/lib/resolve.c
@@ -555,15 +555,17 @@ static int answer_padding(struct kr_request *request)
 
 static int answer_fail(struct kr_request *request)
 {
+	/* Note: OPT in SERVFAIL response is still useful for cookies/additional info. */
 	knot_pkt_t *answer = request->answer;
+	knot_rrset_t *opt_rr = answer->opt_rr; /* it gets NULLed below */
 	int ret = kr_pkt_clear_payload(answer);
 	knot_wire_clear_ad(answer->wire);
 	knot_wire_clear_aa(answer->wire);
 	knot_wire_set_rcode(answer->wire, KNOT_RCODE_SERVFAIL);
-	if (ret == 0 && answer->opt_rr) {
-		/* OPT in SERVFAIL response is still useful for cookies/additional info. */
+	if (ret == 0 && opt_rr) {
 		knot_pkt_begin(answer, KNOT_ADDITIONAL);
 		answer_padding(request); /* Ignore failed padding in SERVFAIL answer. */
+		answer->opt_rr = opt_rr;
 		ret = edns_put(answer, false);
 	}
 	return ret;
@@ -1592,9 +1594,7 @@ int kr_resolve_finish(struct kr_request *request, int state)
 {
 	/* Finalize answer and construct wire-buffer. */
 	ITERATE_LAYERS(request, NULL, answer_finalize);
-	if (request->state & KR_STATE_FAIL) {
-		state = KR_STATE_FAIL;
-	} else if (answer_finalize(request, state) != 0) {
+	if (answer_finalize(request, state) != 0) {
 		state = KR_STATE_FAIL;
 	}
 

["CVE-2019-10191.patch" (text/x-patch)]

diff --git a/daemon/lua/kres-gen.lua b/daemon/lua/kres-gen.lua
index 1d867682..b0a0e9d8 100644
--- 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 4142aa2b..dfcfb116 100644
--- 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 ac3b218b..069b34f0 100644
--- 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) == 1 &&
 	       query->sclass  == knot_pkt_qclass(answer) &&
@@ -1017,6 +1019,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) {
@@ -1060,6 +1063,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 6e93afc7..15ca5633 100644
--- 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. */

["signature.asc" (application/pgp-signature)]

[prev in list] [next in list] [prev in thread] [next in thread]