Merge branch 'ent-wc-fix' into 'master'

Fixed nsec3 proof validation with opt-out below wildcard



See merge request !17

lib/cache.c

@@ -38,7 +38,50 @@
 #define txn_api(txn) ((txn)->owner->api)
 #define txn_is_valid(txn) ((txn) && (txn)->owner && txn_api(txn))
 
-/** @internal Check cache internal data version. Clear if it doesn't match. */
+
+/** @internal Removes all records from cache. */
+static int cache_purge(struct kr_cache_txn *txn)
+{
+	int ret = kr_error(EINVAL);
+	if (txn_is_valid(txn)) {
+		txn->owner->stats.delete += 1;
+		ret = txn_api(txn)->clear(&txn->t);
+	}
+	return ret;
+}
+
+/** @internal	Check cache internal data version. Clear if it doesn't match.
+ * returns :	EEXIST - cache data version matched.
+ *		0 - cache recreated, txn has to be committed.
+ *		Otherwise - cache recreation fails.
+ */
+static int assert_right_version_txn(struct kr_cache_txn *txn)
+{
+	/* Check cache ABI version */
+	knot_db_val_t key = { KEY_VERSION, 2 };
+	knot_db_val_t val = { NULL, 0 };
+	int ret = txn_api(txn)->find(&txn->t, &key, &val, 0);
+	if (ret == 0) {
+		ret = kr_error(EEXIST);
+	} else {
+		/*
+		 * Version doesn't match.
+		 * Recreate cache and write version key.
+		 */
+		ret = txn_api(txn)->count(&txn->t);
+		if (ret != 0) { /* Non-empty cache, purge it. */
+			kr_log_info("[cache] purging cache\n");
+			ret = cache_purge(txn);
+		}
+		/* Either purged or empty. */
+		if (ret == 0) {
+			ret = txn_api(txn)->insert(&txn->t, &key, &val, 0);
+		}
+	}
+	return ret;
+}
+
+/** @internal Open cache db transaction and check internal data version. */
 static void assert_right_version(struct kr_cache *cache)
 {
 	/* Check cache ABI version */
@@ -47,25 +90,11 @@ static void assert_right_version(struct kr_cache *cache)
 	if (ret != 0) {
 		return; /* N/A, doesn't work. */
 	}
-	knot_db_val_t key = { KEY_VERSION, 2 };
-	knot_db_val_t val = { NULL, 0 };
-	ret = txn_api(&txn)->find(&txn.t, &key, &val, 0);
-	if (ret == 0) { /* Version is OK */
-		kr_cache_txn_abort(&txn);
-		return;
-	}
-	/* Recreate cache and write version key */
-	ret = txn_api(&txn)->count(&txn.t);
-	if (ret > 0) { /* Non-empty cache, purge it. */
-		kr_log_info("[cache] purging cache\n");
-		kr_cache_clear(&txn);
-		kr_cache_txn_commit(&txn);
-		ret = kr_cache_txn_begin(cache, &txn, 0);
-	}
-	/* Either purged or empty. */
-	if (ret == 0) {
-		txn_api(&txn)->insert(&txn.t, &key, &val, 0);
+	ret = assert_right_version_txn(&txn);
+	if (ret == 0) { /* Cache recreated, commit. */
 		kr_cache_txn_commit(&txn);
+	} else {
+		kr_cache_txn_abort(&txn);
 	}
 }
 
@@ -290,9 +319,15 @@ int kr_cache_clear(struct kr_cache_txn *txn)
 	if (!txn_is_valid(txn)) {
 		return kr_error(EINVAL);
 	}
-
-	txn->owner->stats.delete += 1;
-	return txn_api(txn)->clear(&txn->t);
+	int ret = cache_purge(txn);
+	if (ret == 0) {
+		/*
+		 * normally must return 0, never EEXIST
+		 * (due to cache_purge())
+		 */
+		ret = assert_right_version_txn(txn);
+	}
+	return ret;
 }
 
 int kr_cache_peek_rr(struct kr_cache_txn *txn, knot_rrset_t *rr, uint16_t *rank, uint32_t *timestamp)

lib/dnssec/nsec3.c

@@ -222,47 +222,44 @@ static int covers_name(int *flags, const knot_rrset_t *nsec3, const knot_dname_t
 	uint8_t *next_hash = NULL;
 	knot_nsec3_next_hashed(&nsec3->rrs, 0, &next_hash, &next_size);
 
-	if ((owner_hash.size != next_size) || (name_hash.size != next_size)) {
+	if ((owner_hash.size == next_size) && (name_hash.size == next_size)) {
 		/* All hash lengths must be same. */
-		goto fail;
-	}
-
-	const uint8_t *ownrd = owner_hash.data;
-	const uint8_t *nextd = next_hash;
-	if (memcmp(ownrd, nextd, next_size) < 0) {
-		/*
-		 * 0 (...) owner ... next (...) MAX
-		 *                ^
-		 *                name
-		 * ==>
-		 * (owner < name) && (name < next)
-		 */
-		if ((memcmp(ownrd, name_hash.data, next_size) >= 0) ||
-		    (memcmp(name_hash.data, nextd, next_size) >= 0)) {
-			goto fail;
-		}
-	} else {
-		/*
-		 * owner ... MAX, 0 ... next
-		 *        ^     ^    ^
-		 *        name  name name
-		 * =>
-		 * (owner < name) || (name < next)
-		 */
-		if ((memcmp(ownrd, name_hash.data, next_size) >= 0) &&
-		    (memcmp(name_hash.data, nextd, next_size) >= 0)) {
-			goto fail;
+		const uint8_t *ownrd = owner_hash.data;
+		const uint8_t *nextd = next_hash;
+		int covered = 0;
+		int greater_then_owner = (memcmp(ownrd, name_hash.data, next_size) < 0);
+		int less_then_next = (memcmp(name_hash.data, nextd, next_size) < 0);
+		if (memcmp(ownrd, nextd, next_size) < 0) {
+			/*
+			 * 0 (...) owner ... next (...) MAX
+			 *                ^
+			 *                name
+			 * ==>
+			 * (owner < name) && (name < next)
+			 */
+			covered = ((greater_then_owner) && (less_then_next));
+		} else {
+			/*
+			 * owner ... MAX, 0 ... next
+			 *        ^     ^    ^
+			 *        name  name name
+			 * =>
+			 * (owner < name) || (name < next)
+			 */
+			covered = ((greater_then_owner) || (less_then_next));
 		}
-	}
 
-	*flags |= FLG_NAME_COVERED;
+		if (covered) {
+			*flags |= FLG_NAME_COVERED;
 
-	uint8_t nsec3_flags = knot_nsec3_flags(&nsec3->rrs, 0);
-	if (nsec3_flags & ~OPT_OUT_BIT) {
-		/* RFC5155 3.1.2 */
-		ret = kr_error(EINVAL);
-	} else {
-		ret = kr_ok();
+			uint8_t nsec3_flags = knot_nsec3_flags(&nsec3->rrs, 0);
+			if (nsec3_flags & ~OPT_OUT_BIT) {
+				/* RFC5155 3.1.2 */
+				ret = kr_error(EINVAL);
+			} else {
+				ret = kr_ok();
+			}
+		}
 	}
 
 fail:
@@ -329,14 +326,12 @@ static int matches_name(int *flags, const knot_rrset_t *nsec3, const knot_dname_
 		goto fail;
 	}
 
-	if ((owner_hash.size != name_hash.size) ||
-	    (memcmp(owner_hash.data, name_hash.data, owner_hash.size) != 0)) {
-		goto fail;
+	if ((owner_hash.size == name_hash.size) &&
+	    (memcmp(owner_hash.data, name_hash.data, owner_hash.size) == 0)) {
+		*flags |= FLG_NAME_MATCHED;
+		ret = kr_ok();
 	}
 
-	*flags |= FLG_NAME_MATCHED;
-	ret = kr_ok();
-
 fail:
 	if (params.salt.data) {
 		dnssec_nsec3_params_free(&params);
@@ -505,7 +500,7 @@ int kr_nsec3_name_error_response_check(const knot_pkt_t *pkt, knot_section_t sec
  * @param type  Type to be checked.
  * @return      0 or error code.
  */
-static int maches_name_and_type(int *flags, const knot_rrset_t *nsec3,
+static int matches_name_and_type(int *flags, const knot_rrset_t *nsec3,
                                 const knot_dname_t *name, uint16_t type)
 {
 	assert(flags && nsec3 && name);
@@ -565,7 +560,7 @@ static int no_data_response_no_ds(const knot_pkt_t *pkt, knot_section_t section_
 		}
 		flags = 0;
 
-		int ret = maches_name_and_type(&flags, rrset, sname, stype);
+		int ret = matches_name_and_type(&flags, rrset, sname, stype);
 		if (ret != 0) {
 			return ret;
 		}
@@ -611,7 +606,7 @@ static int matches_closest_encloser_wildcard(const knot_pkt_t *pkt, knot_section
 		}
 		flags = 0;
 
-		int ret = maches_name_and_type(&flags, rrset, wildcard, stype);
+		int ret = matches_name_and_type(&flags, rrset, wildcard, stype);
 		if (ret != 0) {
 			return ret;
 		}
@@ -619,7 +614,10 @@ static int matches_closest_encloser_wildcard(const knot_pkt_t *pkt, knot_section
 		/* TODO -- The loop resembles no_data_response_no_ds() exept
 		 * the following condition.
 		 */
-		if ((flags & FLG_NAME_MATCHED) && (flags & FLG_TYPE_BIT_MISSING)) {
+		if ((flags & FLG_NAME_MATCHED) &&
+		    (flags & FLG_TYPE_BIT_MISSING) &&
+		    (flags & FLG_CNAME_BIT_MISSING)) {
+			/* rfc5155 8.7 */
 			return kr_ok();
 		}
 	}
@@ -659,13 +657,14 @@ int kr_nsec3_wildcard_answer_response_check(const knot_pkt_t *pkt, knot_section_
 	return kr_error(ENOENT);
 }
 
+
 int kr_nsec3_no_data(const knot_pkt_t *pkt, knot_section_t section_id,
                      const knot_dname_t *sname, uint16_t stype)
 {
 	/* DS record may be also matched by an existing NSEC3 RR. */
 	int ret = no_data_response_no_ds(pkt, section_id, sname, stype);
 	if (ret == 0) {
-		/* Satisfies RFC5155 8.5 and 8.6, first paragraph. */
+		/* Satisfies RFC5155 8.5 and 8.6, both first paragraph. */
 		return ret;
 	}
 
@@ -679,11 +678,28 @@ int kr_nsec3_no_data(const knot_pkt_t *pkt, knot_section_t section_id,
 	}
 
 	assert(encloser_name && covering_next_nsec3);
-	if ((stype == KNOT_RRTYPE_DS) && has_optout(covering_next_nsec3)) {
-		/* Satisfies RFC5155 8.6, second paragraph. */
-		return 0;
+	ret = matches_closest_encloser_wildcard(pkt, section_id,
+	                                         encloser_name, stype);
+	if (ret == 0) {
+		/* Satisfies RFC5155 8.7 */
+		return ret;
 	}
 
-	return matches_closest_encloser_wildcard(pkt, section_id,
-	                                         encloser_name, stype);
+	if (!has_optout(covering_next_nsec3)) {
+		/* Bogus */
+		ret = kr_error(ENOENT);
+	} else {
+		/* 
+		 * Satisfies RFC5155 8.6 (QTYPE == DS), 2nd paragraph.
+		 * Also satisfies ERRATA 3441 8.5 (QTYPE != DS), 3rd paragraph.
+		 * - (wildcard) empty nonterminal
+		 * derived from unsecure delegation.
+		 * Denial of existance can not be proven.
+		 * Set error code to proceed unsecure.
+		 */
+		ret = kr_error(DNSSEC_NOT_FOUND);
+	}
+	
+	return ret;
 }
+

lib/dnssec/nsec3.h

@@ -41,13 +41,15 @@ int kr_nsec3_wildcard_answer_response_check(const knot_pkt_t *pkt, knot_section_
                                             const knot_dname_t *sname, int trim_to_next);
 
 /**
- * Authenticated denial of existence according to RFC5155 8.5, 8.6 and 8.7.
+ * Authenticated denial of existence according to RFC5155 8.5 and 8.7.
  * @note No RRSIGs are validated.
  * @param pkt        Packet structure to be processed.
  * @param section_id Packet section to be processed.
  * @param sname      Queried domain name.
  * @param stype      Queried type.
- * @return           0 or error code.
+ * @return           0 or error code:
+ * 		     DNSSEC_NOT_FOUND - denial of existence can't be proven
+ *		     due to opt-out, otherwise - bogus.
  */
 int kr_nsec3_no_data(const knot_pkt_t *pkt, knot_section_t section_id,
                      const knot_dname_t *sname, uint16_t stype);

lib/layer/validate.c

@@ -23,6 +23,7 @@
 #include <libknot/packet/wire.h>
 #include <libknot/rrtype/rdname.h>
 #include <libknot/rrtype/rrsig.h>
+#include <dnssec/error.h>
 
 #include "lib/dnssec/nsec.h"
 #include "lib/dnssec/nsec3.h"
@@ -295,6 +296,10 @@ static int update_delegation(struct kr_request *req, struct kr_query *qry, knot_
 			ret = kr_nsec_existence_denial(answer, KNOT_AUTHORITY, proved_name, KNOT_RRTYPE_DS);
 		} else {
 			ret = kr_nsec3_no_data(answer, KNOT_AUTHORITY, proved_name, KNOT_RRTYPE_DS);
+			if (ret == kr_error(DNSSEC_NOT_FOUND)) {
+				/* Not bogus, but going insecure */
+				ret = 0;
+			}
 		}
 		if (ret != 0) {
 			DEBUG_MSG(qry, "<= bogus proof of DS non-existence\n");
@@ -424,9 +429,15 @@ static int validate(knot_layer_t *ctx, knot_pkt_t *pkt)
 				ret = kr_nsec3_no_data(pkt, KNOT_AUTHORITY, knot_pkt_qname(pkt), knot_pkt_qtype(pkt));
 			}
 			if (ret != 0) {
-				DEBUG_MSG(qry, "<= bad NODATA proof\n");
-				qry->flags |= QUERY_DNSSEC_BOGUS;
-				return KNOT_STATE_FAIL;
+				if (has_nsec3 && (ret == kr_error(DNSSEC_NOT_FOUND))) {
+					DEBUG_MSG(qry, "<= can't prove NODATA due to optout, going insecure\n");
+					qry->flags &= ~QUERY_DNSSEC_WANT;
+					qry->flags |= QUERY_DNSSEC_INSECURE;
+				} else {
+					DEBUG_MSG(qry, "<= bad NODATA proof\n");
+					qry->flags |= QUERY_DNSSEC_BOGUS;
+					return KNOT_STATE_FAIL;
+				}
 			}
 		}
 	}

deckard @ e73a0854

-Subproject commit 866b7b95ce7355d112b9e47504825c31c6fcb27a
+Subproject commit e73a0854552d94e8bc5306e75c3a2229bc739531

tests/test_cache.c

@@ -394,8 +394,11 @@ static void test_clear(void **state)
 	struct kr_cache_txn *txn = test_txn_write(state);
 	int preempt_ret = kr_cache_clear(txn);
 	int commit_ret = kr_cache_txn_commit(txn);
+	int count_ret = txn->owner->api->count(&txn->t);
+
 	assert_int_equal(preempt_ret, KNOT_EOK);
 	assert_int_equal(commit_ret, KNOT_EOK);
+	assert_int_equal(count_ret, 1); /* Version record */
 }
 
 int main(void)