237 lines
7.1 KiB
Diff
237 lines
7.1 KiB
Diff
From 107fba6767a792bf9022dbfc8a75e3dc61c0d76d Mon Sep 17 00:00:00 2001
|
|
From: Mark Andrews <marka@isc.org>
|
|
Date: Tue, 14 Apr 2026 12:24:33 +1000
|
|
Subject: [PATCH] Invalid signed wildcard records were being accepted
|
|
|
|
An RRSIG whose Labels field indicates fewer labels than its signer
|
|
name requires was being accepted. When such a record covers a
|
|
wildcard, the validator reconstructs a wildcard owner name above the
|
|
signer's zone and caches it as secure. RFC 8198 cache synthesis
|
|
(synth-from-dnssec) then serves that forged wildcard for unrelated
|
|
names, poisoning the cache.
|
|
|
|
These records are now rejected, both when an RRSIG is parsed and when
|
|
its signature is verified.
|
|
|
|
(cherry picked from commit 084ca5ee10515e461d46b63df9660b8394bc7de9)
|
|
(cherry picked from commit 15089066b15f826d7487c3d160b5872820f84b83)
|
|
---
|
|
lib/dns/dnssec.c | 43 +++++++++++++++++++++++---------
|
|
lib/dns/rdata/generic/rrsig_46.c | 37 ++++++++++++++++++++-------
|
|
2 files changed, 59 insertions(+), 21 deletions(-)
|
|
|
|
diff --git a/lib/dns/dnssec.c b/lib/dns/dnssec.c
|
|
index 662136db87..86f03680e9 100644
|
|
--- a/lib/dns/dnssec.c
|
|
+++ b/lib/dns/dnssec.c
|
|
@@ -137,11 +137,11 @@ dns_dnssec_keyfromrdata(const dns_name_t *name, const dns_rdata_t *rdata,
|
|
isc_buffer_t b;
|
|
isc_region_t r;
|
|
|
|
- INSIST(name != NULL);
|
|
- INSIST(rdata != NULL);
|
|
- INSIST(mctx != NULL);
|
|
- INSIST(key != NULL);
|
|
- INSIST(*key == NULL);
|
|
+ REQUIRE(name != NULL);
|
|
+ REQUIRE(rdata != NULL);
|
|
+ REQUIRE(mctx != NULL);
|
|
+ REQUIRE(key != NULL);
|
|
+ REQUIRE(*key == NULL);
|
|
REQUIRE(rdata->type == dns_rdatatype_key ||
|
|
rdata->type == dns_rdatatype_dnskey);
|
|
|
|
@@ -195,12 +195,14 @@ dns_dnssec_sign(const dns_name_t *name, dns_rdataset_t *set, dst_key_t *key,
|
|
isc_buffer_t *databuf = NULL;
|
|
char data[256 + 8];
|
|
uint32_t flags;
|
|
+ unsigned int labels;
|
|
unsigned int sigsize;
|
|
dns_fixedname_t fnewname;
|
|
dns_fixedname_t fsigner;
|
|
|
|
REQUIRE(name != NULL);
|
|
- REQUIRE(dns_name_countlabels(name) <= 255);
|
|
+ labels = dns_name_countlabels(name);
|
|
+ REQUIRE(labels <= 255 && labels > 0);
|
|
REQUIRE(set != NULL);
|
|
REQUIRE(key != NULL);
|
|
REQUIRE(inception != NULL);
|
|
@@ -240,7 +242,7 @@ dns_dnssec_sign(const dns_name_t *name, dns_rdataset_t *set, dst_key_t *key,
|
|
|
|
sig.covered = set->type;
|
|
sig.algorithm = dst_key_alg(key);
|
|
- sig.labels = dns_name_countlabels(name) - 1;
|
|
+ sig.labels = labels - 1;
|
|
if (dns_name_iswildcard(name)) {
|
|
sig.labels--;
|
|
}
|
|
@@ -384,11 +386,14 @@ dns_dnssec_verify(const dns_name_t *name, dns_rdataset_t *set, dst_key_t *key,
|
|
isc_result_t ret;
|
|
unsigned char data[300];
|
|
dst_context_t *ctx = NULL;
|
|
- int labels = 0;
|
|
+ unsigned int labels;
|
|
+ unsigned int siglabels;
|
|
uint32_t flags;
|
|
bool downcase = false;
|
|
|
|
REQUIRE(name != NULL);
|
|
+ labels = dns_name_countlabels(name);
|
|
+ REQUIRE(labels > 0);
|
|
REQUIRE(set != NULL);
|
|
REQUIRE(key != NULL);
|
|
REQUIRE(mctx != NULL);
|
|
@@ -403,6 +408,21 @@ dns_dnssec_verify(const dns_name_t *name, dns_rdataset_t *set, dst_key_t *key,
|
|
return DNS_R_SIGINVALID;
|
|
}
|
|
|
|
+ /*
|
|
+ * The RRSIG labels field can't indicate fewer labels than the
|
|
+ * signer. Also the labels shouldn't be greater than that of
|
|
+ * the owner name.
|
|
+ *
|
|
+ * sig.labels doesn't include the root label, so add 1 to account
|
|
+ * for it.
|
|
+ */
|
|
+ siglabels = sig.labels + 1;
|
|
+ if (siglabels < dns_name_countlabels(&sig.signer) || siglabels > labels)
|
|
+ {
|
|
+ inc_stat(dns_dnssecstats_fail);
|
|
+ return DNS_R_SIGINVALID;
|
|
+ }
|
|
+
|
|
if (isc_serial_lt(sig.timeexpire, sig.timesigned)) {
|
|
inc_stat(dns_dnssecstats_fail);
|
|
return DNS_R_SIGINVALID;
|
|
@@ -482,10 +502,9 @@ again:
|
|
* If the name is an expanded wildcard, use the wildcard name.
|
|
*/
|
|
dns_fixedname_init(&fnewname);
|
|
- labels = dns_name_countlabels(name) - 1;
|
|
RUNTIME_CHECK(dns_name_downcase(name, dns_fixedname_name(&fnewname),
|
|
NULL) == ISC_R_SUCCESS);
|
|
- if (labels - sig.labels > 0) {
|
|
+ if (labels > siglabels) {
|
|
dns_name_split(dns_fixedname_name(&fnewname), sig.labels + 1,
|
|
NULL, dns_fixedname_name(&fnewname));
|
|
}
|
|
@@ -496,7 +515,7 @@ again:
|
|
* Create an envelope for each rdata: <name|type|class|ttl>.
|
|
*/
|
|
isc_buffer_init(&envbuf, data, sizeof(data));
|
|
- if (labels - sig.labels > 0) {
|
|
+ if (labels > siglabels) {
|
|
isc_buffer_putuint8(&envbuf, 1);
|
|
isc_buffer_putuint8(&envbuf, '*');
|
|
memmove(data + 2, r.base, r.length);
|
|
@@ -592,7 +611,7 @@ cleanup_struct:
|
|
inc_stat(dns_dnssecstats_fail);
|
|
}
|
|
|
|
- if (ret == ISC_R_SUCCESS && labels - sig.labels > 0) {
|
|
+ if (ret == ISC_R_SUCCESS && labels > siglabels) {
|
|
if (wild != NULL) {
|
|
RUNTIME_CHECK(dns_name_concatenate(
|
|
dns_wildcardname,
|
|
diff --git a/lib/dns/rdata/generic/rrsig_46.c b/lib/dns/rdata/generic/rrsig_46.c
|
|
index 2cc315bdea..53648f1bfa 100644
|
|
--- a/lib/dns/rdata/generic/rrsig_46.c
|
|
+++ b/lib/dns/rdata/generic/rrsig_46.c
|
|
@@ -23,12 +23,12 @@
|
|
static isc_result_t
|
|
fromtext_rrsig(ARGS_FROMTEXT) {
|
|
isc_token_t token;
|
|
- unsigned char c;
|
|
+ unsigned char alg, labels;
|
|
long i;
|
|
dns_rdatatype_t covered;
|
|
- char *e;
|
|
+ char *e = NULL;
|
|
isc_result_t result;
|
|
- dns_name_t name;
|
|
+ dns_name_t signer;
|
|
isc_buffer_t buffer;
|
|
uint32_t time_signed, time_expire;
|
|
|
|
@@ -61,8 +61,8 @@ fromtext_rrsig(ARGS_FROMTEXT) {
|
|
*/
|
|
RETERR(isc_lex_getmastertoken(lexer, &token, isc_tokentype_string,
|
|
false));
|
|
- RETTOK(dns_secalg_fromtext(&c, &token.value.as_textregion));
|
|
- RETERR(mem_tobuffer(target, &c, 1));
|
|
+ RETTOK(dns_secalg_fromtext(&alg, &token.value.as_textregion));
|
|
+ RETERR(mem_tobuffer(target, &alg, 1));
|
|
|
|
/*
|
|
* Labels.
|
|
@@ -72,8 +72,8 @@ fromtext_rrsig(ARGS_FROMTEXT) {
|
|
if (token.value.as_ulong > 0xffU) {
|
|
RETTOK(ISC_R_RANGE);
|
|
}
|
|
- c = (unsigned char)token.value.as_ulong;
|
|
- RETERR(mem_tobuffer(target, &c, 1));
|
|
+ labels = (unsigned char)token.value.as_ulong;
|
|
+ RETERR(mem_tobuffer(target, &labels, 1));
|
|
|
|
/*
|
|
* Original ttl.
|
|
@@ -144,12 +144,20 @@ fromtext_rrsig(ARGS_FROMTEXT) {
|
|
*/
|
|
RETERR(isc_lex_getmastertoken(lexer, &token, isc_tokentype_string,
|
|
false));
|
|
- dns_name_init(&name, NULL);
|
|
+ dns_name_init(&signer, NULL);
|
|
buffer_fromregion(&buffer, &token.value.as_region);
|
|
if (origin == NULL) {
|
|
origin = dns_rootname;
|
|
}
|
|
- RETTOK(dns_name_fromtext(&name, &buffer, origin, options, target));
|
|
+ RETTOK(dns_name_fromtext(&signer, &buffer, origin, options, target));
|
|
+
|
|
+ /*
|
|
+ * (RRSIG labels doesn't include the root label, so add one
|
|
+ * to normalize it before checking against the signer.)
|
|
+ */
|
|
+ if ((unsigned int)(labels + 1) < dns_name_countlabels(&signer)) {
|
|
+ RETTOK(ISC_R_RANGE);
|
|
+ }
|
|
|
|
/*
|
|
* Sig.
|
|
@@ -278,6 +286,7 @@ static isc_result_t
|
|
fromwire_rrsig(ARGS_FROMWIRE) {
|
|
isc_region_t sr;
|
|
dns_name_t name;
|
|
+ unsigned char labels;
|
|
|
|
REQUIRE(type == dns_rdatatype_rrsig);
|
|
|
|
@@ -300,6 +309,8 @@ fromwire_rrsig(ARGS_FROMWIRE) {
|
|
return ISC_R_UNEXPECTEDEND;
|
|
}
|
|
|
|
+ labels = sr.base[3];
|
|
+
|
|
isc_buffer_forward(source, 18);
|
|
RETERR(mem_tobuffer(target, sr.base, 18));
|
|
|
|
@@ -309,6 +320,14 @@ fromwire_rrsig(ARGS_FROMWIRE) {
|
|
dns_name_init(&name, NULL);
|
|
RETERR(dns_name_fromwire(&name, source, dctx, options, target));
|
|
|
|
+ /*
|
|
+ * (RRSIG labels doesn't include the root label, so add one
|
|
+ * to normalize it before checking against the signer.)
|
|
+ */
|
|
+ if ((unsigned int)(labels + 1) < dns_name_countlabels(&name)) {
|
|
+ RETERR(DNS_R_FORMERR);
|
|
+ }
|
|
+
|
|
/*
|
|
* Sig.
|
|
*/
|
|
--
|
|
2.55.0
|
|
|