From: Vladimír Čunát <vladimir.cunat@nic.cz>
Date: Wed, 5 Apr 2017 11:33:15 +0000 (+0200)
Subject: don't attempt to reuse cached nonvalidated records
X-Git-Tag: v1.3.0~23^2~22
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=9eeaf75011549d13da9fbf39aa6abbb36f514971;p=thirdparty%2Fknot-resolver.git

don't attempt to reuse cached nonvalidated records

at least for now (for queries without +cd).
It wasn't complete, and it turned out to need more changes,
and the benefits would be rather limited.
---

diff --git a/lib/layer/iterate.c b/lib/layer/iterate.c
index d7a21a590..0669ea28b 100644
--- a/lib/layer/iterate.c
+++ b/lib/layer/iterate.c
@@ -305,7 +305,7 @@ static inline uint8_t get_initial_rank(const knot_rrset_t *rr,
 		/* TODO: this classifier of authoritativity may not be perfect yet. */
 		rank |= KR_RANK_AUTH;
 	}
-	return (uint8_t)rank;
+	return rank;
 }
 
 static int pick_authority(knot_pkt_t *pkt, struct kr_request *req, bool to_wire)
diff --git a/lib/layer/pktcache.c b/lib/layer/pktcache.c
index b617e3ad3..f9eddc170 100644
--- a/lib/layer/pktcache.c
+++ b/lib/layer/pktcache.c
@@ -70,9 +70,9 @@ static int loot_pktcache(struct kr_cache *cache, knot_pkt_t *pkt,
 		return ret;
 	}
 
-	const uint8_t rank_val = rank_get_value(entry->rank);
-	if (!knot_wire_get_cd(req->answer->wire) && rank_val && rank_val != KR_RANK_OMIT) {
-		return kr_error(ENOENT); /* it would fail anyway */
+	if (!knot_wire_get_cd(req->answer->wire)
+	    && entry->rank < (KR_RANK_INSECURE|KR_RANK_AUTH)) {
+		return kr_error(ENOENT);
 	}
 
 	/* Copy answer, keep the original message id */
diff --git a/lib/layer/rrcache.c b/lib/layer/rrcache.c
index 939bc4503..60df23d5e 100644
--- a/lib/layer/rrcache.c
+++ b/lib/layer/rrcache.c
@@ -141,12 +141,14 @@ enomem:
 static int loot_rrcache(struct kr_cache *cache, knot_pkt_t *pkt,
 			struct kr_query *qry, uint16_t rrtype, const bool cdbit)
 {
-	/* Lookup direct match first; only consider authoritative records,
-	 * even unvalidated, as rank handling is better to do in the iterator
-	 * (QUERY_DNSSEC_* flags). */
+	/* Lookup direct match first; only consider authoritative records.
+	 * TODO: move rank handling into the iterator (QUERY_DNSSEC_* flags)? */
 	uint8_t rank  = 0;
 	uint8_t flags = 0;
 	uint8_t lowest_rank = KR_RANK_AUTH;
+	if (!cdbit) {
+		lowest_rank |= KR_RANK_INSECURE;
+	}
 
 	int ret = loot_rr(cache, pkt, qry->sname, qry->sclass, rrtype, qry,
 			  &rank, &flags, 0, lowest_rank);
@@ -159,6 +161,11 @@ static int loot_rrcache(struct kr_cache *cache, knot_pkt_t *pkt,
 	if (ret) {
 		return ret;
 	}
+	if (rank & KR_RANK_INSECURE) {
+		qry->flags |= QUERY_DNSSEC_INSECURE;
+		qry->flags &= ~QUERY_DNSSEC_WANT;
+	}
+
 	/* Record may have RRSIGs, try to find them. */
 	const bool dobit = (qry->flags & QUERY_DNSSEC_WANT);
 	if (cdbit || (dobit && (rank & KR_RANK_SECURE))) {
diff --git a/lib/layer/validate.c b/lib/layer/validate.c
index 49993228c..a07e088ad 100644
--- a/lib/layer/validate.c
+++ b/lib/layer/validate.c
@@ -108,7 +108,8 @@ static int validate_section(kr_rrset_validation_ctx_t *vctx, knot_mm_t *pool)
 		ranked_rr_array_entry_t *entry = vctx->rrs->at[i];
 		const knot_rrset_t *rr = entry->rr;
 		assert((entry->rank & (KR_RANK_SECURE | KR_RANK_INSECURE)) != (KR_RANK_SECURE | KR_RANK_INSECURE));
-		if (rank_test_flag(entry->rank, KR_RANK_SECURE) ||
+		if (rank_get_value(entry->rank) == KR_RANK_OMIT ||
+		    rank_test_flag(entry->rank, KR_RANK_SECURE) ||
 		    entry->yielded || vctx->qry_uid != entry->qry_uid) {
 			continue;
 		}
diff --git a/lib/resolve.h b/lib/resolve.h
index 442275801..acbeb146c 100644
--- a/lib/resolve.h
+++ b/lib/resolve.h
@@ -89,12 +89,11 @@
  *   https://tools.ietf.org/html/rfc4035#section-4.3
  */
 enum kr_rank {
-	KR_RANK_INITIAL = 0,
-
-	KR_RANK_BAD = 7,  /**< For simpler manipulation with the four values below. */
-	KR_RANK_OMIT = 1, /**< Do not validate. */
-	KR_RANK_INDET,    /**< Unable to determine whether it should be secure. */
-	KR_RANK_BOGUS,    /**< Ought to be secure but isn't. */
+	KR_RANK_BAD = 7,     /**< For simpler manipulation with the values below. */
+	KR_RANK_INITIAL = 0, /**< Did not attempt to validate. */
+	KR_RANK_OMIT = 1,    /**< Do not attempt to validate. */
+	KR_RANK_INDET,       /**< Unable to determine whether it should be secure. */
+	KR_RANK_BOGUS,       /**< Ought to be secure but isn't. */
 	KR_RANK_MISMATCH,
 
 	KR_RANK_INSECURE = 8, /**< Proven to be insecure. */