From 83650f725e274dcd00ccad155183f1ca491794ab Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Tue, 14 Apr 2026 12:24:33 +1000 Subject: [PATCH 1/2] 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) --- 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 c168fed..fcebd3d 100644 --- a/lib/dns/dnssec.c +++ b/lib/dns/dnssec.c @@ -139,11 +139,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); @@ -197,12 +197,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); @@ -242,7 +244,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--; } @@ -386,11 +388,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); @@ -405,6 +410,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); @@ -484,10 +504,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)); } @@ -498,7 +517,7 @@ again: * Create an envelope for each rdata: . */ 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); @@ -594,7 +613,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 40506d7..053eef6 100644 --- a/lib/dns/rdata/generic/rrsig_46.c +++ b/lib/dns/rdata/generic/rrsig_46.c @@ -21,12 +21,12 @@ static inline 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; @@ -59,8 +59,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. @@ -70,8 +70,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. @@ -142,12 +142,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. @@ -276,6 +284,7 @@ static inline isc_result_t fromwire_rrsig(ARGS_FROMWIRE) { isc_region_t sr; dns_name_t name; + unsigned char labels; REQUIRE(type == dns_rdatatype_rrsig); @@ -298,6 +307,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)); @@ -307,6 +318,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. */ From c62a6deaeaf37c8f0867f2174e9dc62fb2b68389 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Tue, 14 Apr 2026 15:14:06 +1000 Subject: [PATCH 2/2] Don't sign out of zone records in dnssec-signzone dnssec-signzone was signing extraneous records that were not within the namespace of the zone. This no longer occurs. (cherry picked from commit e45c9af7051421fd370f20ba8325199c606223fd) Don't sign out of zone records in dnssec-signzone dnssec-signzone was signing extraneous records that were not within the namespace of the zone. This no longer occurs. (cherry picked from commit e45c9af7051421fd370f20ba8325199c606223fd) --- bin/dnssec/dnssec-signzone.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/bin/dnssec/dnssec-signzone.c b/bin/dnssec/dnssec-signzone.c index 48aafdd..0ae6fae 100644 --- a/bin/dnssec/dnssec-signzone.c +++ b/bin/dnssec/dnssec-signzone.c @@ -1624,6 +1624,11 @@ assignwork(isc_task_t *task, isc_task_t *worker) { dns_db_detachnode(gdb, &node); goto next; } + if (!dns_name_issubdomain(name, gorigin)) { + dumpnode(name, node); + dns_db_detachnode(gdb, &node); + goto next; + } /* * Sort the zone data from the glue and out-of-zone data. * For NSEC zones nodes with zone data have NSEC records.