224 lines
6.5 KiB
Diff
224 lines
6.5 KiB
Diff
From c30c944e424a6c2281e0b1d53e25fc8ed3f71d41 Mon Sep 17 00:00:00 2001
|
|
From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= <ondrej@isc.org>
|
|
Date: Tue, 22 Jul 2025 08:07:02 +0200
|
|
Subject: [PATCH] Fail the DNSSEC validation if matching but invalid DNSKEY is
|
|
found
|
|
|
|
If a matching but cryptographically invalid key was encountered during
|
|
the DNSSEC validation, the key would be just skipped and not counted
|
|
towards validation failures. Treat such DNSSEC keys as hard failures
|
|
and fail the DNSSEC validation immediatelly instead of continuing the
|
|
DNSSEC validation with the next DNSKEYs in the RRset.
|
|
|
|
Co-authored-by: Matthijs Mekking <matthijs@isc.org>
|
|
|
|
(cherry picked from commit f00117a4226be90d1bc865aff19bddf114242914)
|
|
(cherry picked from commit 7c5b8ef055900224f0424c341927562c5a9ebe19)
|
|
|
|
Fix an issue with selfsigned_dnskey() return value
|
|
|
|
The selfsigned_dnskey() function currently returns boolean. There
|
|
was a recent change to make it return a isc_result_t error code,
|
|
which is implicitly converted to bool, which is obviously an error.
|
|
|
|
If instead of the result code we return true/false, it still doesn't
|
|
indicate the error to the caller that has happened before.
|
|
|
|
Change the function to return isc_result_t, and change the caller
|
|
routine to process the new return type.
|
|
|
|
(cherry picked from commit 40c396ba2d955c32d70db04e900e40bf96519c59)
|
|
---
|
|
lib/dns/validator.c | 79 +++++++++++++++++++++++++++++----------------
|
|
1 file changed, 51 insertions(+), 28 deletions(-)
|
|
|
|
diff --git a/lib/dns/validator.c b/lib/dns/validator.c
|
|
index 696a464..1d84b75 100644
|
|
--- a/lib/dns/validator.c
|
|
+++ b/lib/dns/validator.c
|
|
@@ -431,6 +431,8 @@ fetch_callback_dnskey(isc_task_t *task, isc_event_t *event) {
|
|
result = select_signing_key(val, rdataset);
|
|
if (result == ISC_R_SUCCESS) {
|
|
val->keyset = &val->frdataset;
|
|
+ } else {
|
|
+ val->failed = true;
|
|
}
|
|
}
|
|
result = validate_answer(val, true);
|
|
@@ -1161,6 +1163,8 @@ select_signing_key(dns_validator_t *val, dns_rdataset_t *rdataset) {
|
|
goto done;
|
|
}
|
|
dst_key_free(&val->key);
|
|
+ } else {
|
|
+ break;
|
|
}
|
|
dns_rdata_reset(&rdata);
|
|
result = dns_rdataset_next(rdataset);
|
|
@@ -1285,13 +1289,15 @@ seek_dnskey(dns_validator_t *val) {
|
|
"keyset with trust %s",
|
|
dns_trust_totext(val->frdataset.trust));
|
|
result = select_signing_key(val, val->keyset);
|
|
- if (result != ISC_R_SUCCESS) {
|
|
+ if (result == ISC_R_NOTFOUND) {
|
|
/*
|
|
- * Either the key we're looking for is not
|
|
- * in the rrset, or something bad happened.
|
|
- * Give up.
|
|
+ * The key we're looking for is not
|
|
+ * in the rrset
|
|
*/
|
|
result = DNS_R_CONTINUE;
|
|
+ } else if (result != ISC_R_SUCCESS) {
|
|
+ /* Something bad happened. Give up. */
|
|
+ break;
|
|
}
|
|
}
|
|
break;
|
|
@@ -1352,17 +1358,17 @@ compute_keytag(dns_rdata_t *rdata) {
|
|
/*%
|
|
* Is the DNSKEY rrset in val->event->rdataset self-signed?
|
|
*/
|
|
-static bool
|
|
+static isc_result_t
|
|
selfsigned_dnskey(dns_validator_t *val) {
|
|
dns_rdataset_t *rdataset = val->event->rdataset;
|
|
dns_rdataset_t *sigrdataset = val->event->sigrdataset;
|
|
dns_name_t *name = val->event->name;
|
|
isc_result_t result;
|
|
isc_mem_t *mctx = val->view->mctx;
|
|
- bool answer = false;
|
|
+ bool match = false;
|
|
|
|
if (rdataset->type != dns_rdatatype_dnskey) {
|
|
- return (false);
|
|
+ return DNS_R_NOKEYMATCH;
|
|
}
|
|
|
|
for (result = dns_rdataset_first(rdataset); result == ISC_R_SUCCESS;
|
|
@@ -1384,8 +1390,6 @@ selfsigned_dnskey(dns_validator_t *val) {
|
|
result == ISC_R_SUCCESS;
|
|
result = dns_rdataset_next(sigrdataset))
|
|
{
|
|
- dst_key_t *dstkey = NULL;
|
|
-
|
|
dns_rdata_reset(&sigrdata);
|
|
dns_rdataset_current(sigrdataset, &sigrdata);
|
|
result = dns_rdata_tostruct(&sigrdata, &sig, NULL);
|
|
@@ -1400,18 +1404,16 @@ selfsigned_dnskey(dns_validator_t *val) {
|
|
|
|
/*
|
|
* If the REVOKE bit is not set we have a
|
|
- * theoretically self signed DNSKEY RRset.
|
|
- * This will be verified later.
|
|
+ * theoretically self-signed DNSKEY RRset;
|
|
+ * this will be verified later.
|
|
+ *
|
|
+ * We don't return the answer yet, though,
|
|
+ * because we need to check the remaining keys
|
|
+ * and possbly remove them if they're revoked.
|
|
*/
|
|
if ((key.flags & DNS_KEYFLAG_REVOKE) == 0) {
|
|
- answer = true;
|
|
- continue;
|
|
- }
|
|
-
|
|
- result = dns_dnssec_keyfromrdata(name, &keyrdata, mctx,
|
|
- &dstkey);
|
|
- if (result != ISC_R_SUCCESS) {
|
|
- continue;
|
|
+ match = true;
|
|
+ break;
|
|
}
|
|
|
|
/*
|
|
@@ -1421,6 +1423,14 @@ selfsigned_dnskey(dns_validator_t *val) {
|
|
if (DNS_TRUST_PENDING(rdataset->trust) &&
|
|
dns_view_istrusted(val->view, name, &key))
|
|
{
|
|
+ dst_key_t *dstkey = NULL;
|
|
+
|
|
+ result = dns_dnssec_keyfromrdata(
|
|
+ name, &keyrdata, mctx, &dstkey);
|
|
+ if (result != ISC_R_SUCCESS) {
|
|
+ break;
|
|
+ }
|
|
+
|
|
result = dns_dnssec_verify(
|
|
name, rdataset, dstkey, true,
|
|
val->view->maxbits, mctx, &sigrdata,
|
|
@@ -1433,6 +1443,8 @@ selfsigned_dnskey(dns_validator_t *val) {
|
|
*/
|
|
dns_view_untrust(val->view, name, &key);
|
|
}
|
|
+
|
|
+ dst_key_free(&dstkey);
|
|
} else if (rdataset->trust >= dns_trust_secure) {
|
|
/*
|
|
* We trust this RRset so if the key is
|
|
@@ -1440,12 +1452,14 @@ selfsigned_dnskey(dns_validator_t *val) {
|
|
*/
|
|
dns_view_untrust(val->view, name, &key);
|
|
}
|
|
-
|
|
- dst_key_free(&dstkey);
|
|
}
|
|
}
|
|
|
|
- return (answer);
|
|
+ if (!match) {
|
|
+ return DNS_R_NOKEYMATCH;
|
|
+ }
|
|
+
|
|
+ return ISC_R_SUCCESS;
|
|
}
|
|
|
|
/*%
|
|
@@ -1680,10 +1694,7 @@ check_signer(dns_validator_t *val, dns_rdata_t *keyrdata, uint16_t keyid,
|
|
val->event->name, keyrdata, val->view->mctx,
|
|
&dstkey);
|
|
if (result != ISC_R_SUCCESS) {
|
|
- /*
|
|
- * This really shouldn't happen, but...
|
|
- */
|
|
- continue;
|
|
+ return result;
|
|
}
|
|
}
|
|
result = verify(val, dstkey, &rdata, sig.keyid);
|
|
@@ -3064,11 +3075,22 @@ validator_start(isc_task_t *task, isc_event_t *event) {
|
|
|
|
INSIST(dns_rdataset_isassociated(val->event->rdataset));
|
|
INSIST(dns_rdataset_isassociated(val->event->sigrdataset));
|
|
- if (selfsigned_dnskey(val)) {
|
|
+
|
|
+ result = selfsigned_dnskey(val);
|
|
+ switch (result) {
|
|
+ case ISC_R_SUCCESS:
|
|
result = validate_dnskey(val);
|
|
- } else {
|
|
+ break;
|
|
+ case DNS_R_NOKEYMATCH:
|
|
result = validate_answer(val, false);
|
|
+ break;
|
|
+ default:
|
|
+ validator_log(val, ISC_LOG_INFO,
|
|
+ "invalid selfsigned DNSKEY: %s",
|
|
+ isc_result_totext(result));
|
|
+ goto cleanup;
|
|
}
|
|
+
|
|
if (result == DNS_R_NOVALIDSIG &&
|
|
(val->attributes & VALATTR_TRIEDVERIFY) == 0)
|
|
{
|
|
@@ -3137,6 +3159,7 @@ validator_start(isc_task_t *task, isc_event_t *event) {
|
|
UNREACHABLE();
|
|
}
|
|
|
|
+cleanup:
|
|
if (result != DNS_R_WAIT) {
|
|
want_destroy = exit_check(val);
|
|
validator_done(val, result);
|
|
--
|
|
2.51.1
|
|
|