From: Vladimír Čunát <vladimir.cunat@nic.cz>
Date: Tue, 14 Sep 2021 15:46:19 +0000 (+0200)
Subject: lib/dnssec: make kr_dnskeys_trusted() cleaner
X-Git-Tag: v5.4.3~13^2~3
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=720aa89dbc076f593313ee35bb9d833a5e7a760b;p=thirdparty%2Fknot-resolver.git

lib/dnssec: make kr_dnskeys_trusted() cleaner

This way it will be easier to re-use (and more efficient).
I really disliked those searches for RRSIGs embedded deep inside.

Uh, I tried to keep the new function as clean as possible,
moving hacks to outside.
---

diff --git a/lib/dnssec.c b/lib/dnssec.c
index c1935b483..50436c5f3 100644
--- a/lib/dnssec.c
+++ b/lib/dnssec.c
@@ -348,12 +348,13 @@ bool kr_ds_algo_support(const knot_rrset_t *ta)
 	return false;
 }
 
-int kr_dnskeys_trusted(kr_rrset_validation_ctx_t *vctx, const knot_rrset_t *ta)
+int kr_dnskeys_trusted(kr_rrset_validation_ctx_t *vctx, const knot_rdataset_t *sigs,
+			const knot_rrset_t *ta)
 {
-	knot_rrset_t *keys            = vctx->keys;
-
+	knot_rrset_t *keys = vctx->keys;
 	const bool ok = keys && ta && ta->rrs.count && ta->rrs.rdata
-			&& ta->type == KNOT_RRTYPE_DS;
+			&& ta->type == KNOT_RRTYPE_DS
+			&& knot_dname_is_equal(ta->owner, keys->owner);
 	if (kr_fails_assert(ok))
 		return kr_error(EINVAL);
 
@@ -361,28 +362,24 @@ int kr_dnskeys_trusted(kr_rrset_validation_ctx_t *vctx, const knot_rrset_t *ta)
 	 * The supplied DS record has been authenticated.
 	 * It has been validated or is part of a configured trust anchor.
 	 */
-	memset(&vctx->rrs_counters, 0, sizeof(vctx->rrs_counters));
-	for (uint16_t i = 0; i < keys->rrs.count; ++i) {
+	knot_rdata_t *krr = keys->rrs.rdata;
+	for (int i = 0; i < keys->rrs.count; ++i, krr = knot_rdataset_next(krr)) {
 		/* RFC4035 5.3.1, bullet 8 */ /* ZSK */
-		/* LATER(optim.): more efficient way to iterate than _at() */
-		knot_rdata_t *krr = knot_rdataset_at(&keys->rrs, i);
 		if (!kr_dnssec_key_zsk(krr->data) || kr_dnssec_key_revoked(krr->data))
 			continue;
 
-		struct dnssec_key *key = NULL;
-		if (kr_dnssec_key_from_rdata(&key, keys->owner, krr->data, krr->len) != 0)
-			continue;
-		if (kr_authenticate_referral(ta, key) != 0) {
-			kr_dnssec_key_free(&key);
-			continue;
-		}
-		if (kr_rrset_validate_with_key(vctx, keys, i, key) != 0) {
-			kr_dnssec_key_free(&key);
-			continue;
+		kr_svldr_key_t key;
+		if (svldr_key_new(krr, keys->owner, &key) != 0)
+			continue; // it might e.g. be malformed
+
+		int ret = kr_authenticate_referral(ta, key.key);
+		if (ret == 0)
+			ret = kr_svldr_rrset_with_key(keys, sigs, vctx, &key);
+		svldr_key_del(&key);
+		if (ret == 0) {
+			kr_assert(vctx->result == 0);
+			return vctx->result;
 		}
-		kr_dnssec_key_free(&key);
-		kr_assert(vctx->result == 0);
-		return vctx->result;
 	}
 
 	/* No useable key found */
diff --git a/lib/dnssec.h b/lib/dnssec.h
index 731ddf7ad..d69d88d0f 100644
--- a/lib/dnssec.h
+++ b/lib/dnssec.h
@@ -76,11 +76,14 @@ bool kr_ds_algo_support(const knot_rrset_t *ta);
 
 /**
  * Check whether the DNSKEY rrset matches the supplied trust anchor RRSet.
+ *
  * @param vctx  Pointer to validation context.  Note that TTL of vctx->keys may get lowered.
- * @param ta    Trust anchor RRSet against which to validate the DNSKEY RRSet.
+ * @param sigs  RRSIGs for this DNSKEY set
+ * @param ta    Trusted DS RRSet against which to validate the DNSKEY RRSet.
  * @return      0 or error code, same as vctx->result.
  */
-int kr_dnskeys_trusted(kr_rrset_validation_ctx_t *vctx, const knot_rrset_t *ta);
+int kr_dnskeys_trusted(kr_rrset_validation_ctx_t *vctx, const knot_rdataset_t *sigs,
+			const knot_rrset_t *ta);
 
 /** Return true if the DNSKEY can be used as a ZSK.  */
 KR_EXPORT KR_PURE
diff --git a/lib/layer/validate.c b/lib/layer/validate.c
index fb906efdb..0d48514db 100644
--- a/lib/layer/validate.c
+++ b/lib/layer/validate.c
@@ -340,6 +340,26 @@ static int validate_keyset(struct kr_request *req, knot_pkt_t *answer, bool has_
 
 	/* Check if there's a key for current TA. */
 	if (updated_key && !(qry->flags.CACHED)) {
+		/* Find signatures for the DNSKEY; selected by iterator from ANSWER. */
+		int sig_index = -1;
+		for (int i = req->answ_selected.len - 1; i >= 0; --i) {
+			const knot_rrset_t *rrsig = req->answ_selected.at[i]->rr;
+			const bool ok = req->answ_selected.at[i]->qry_uid == qry->uid
+				&& rrsig->type == KNOT_RRTYPE_RRSIG
+				&& knot_rrsig_type_covered(rrsig->rrs.rdata)
+					== KNOT_RRTYPE_DNSKEY
+				&& rrsig->rclass == KNOT_CLASS_IN
+				&& knot_dname_is_equal(rrsig->owner,
+							qry->zone_cut.key->owner);
+			if (ok) {
+				sig_index = i;
+				break;
+			}
+		}
+		if (sig_index < 0) {
+			return kr_error(ENOENT);
+		}
+		const knot_rdataset_t *sig_rds = &req->answ_selected.at[sig_index]->rr->rrs;
 
 		kr_rrset_validation_ctx_t vctx = {
 			.pkt		= answer,
@@ -354,7 +374,12 @@ static int validate_keyset(struct kr_request *req, knot_pkt_t *answer, bool has_
 			.result		= 0,
 			.log_qry	= qry,
 		};
-		int ret = kr_dnskeys_trusted(&vctx, qry->zone_cut.trust_anchor);
+		int ret = kr_dnskeys_trusted(&vctx, sig_rds, qry->zone_cut.trust_anchor);
+		/* Set rank of the RRSIG.  This may be needed, but I don't know why.
+		 * In particular, black_ent.rpl may get broken otherwise. */
+		kr_rank_set(&req->answ_selected.at[sig_index]->rank,
+				ret == 0 ? KR_RANK_SECURE : KR_RANK_BOGUS);
+
 		if (ret != 0) {
 			if (ret != kr_error(DNSSEC_INVALID_DS_ALGORITHM) &&
 			    ret != kr_error(EAGAIN)) {