From: Mark Andrews Date: Tue, 14 Apr 2026 02:24:33 +0000 (+1000) Subject: Invalid signed wildcard records were being accepted X-Git-Tag: v9.21.24~12^2~2 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=084ca5ee10515e461d46b63df9660b8394bc7de9;p=thirdparty%2Fbind9.git 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. --- diff --git a/lib/dns/dnssec.c b/lib/dns/dnssec.c index 3b4c6672a6a..d71d1e68b97 100644 --- a/lib/dns/dnssec.c +++ b/lib/dns/dnssec.c @@ -127,7 +127,7 @@ 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(DNS_NAME_VALID(name)); INSIST(rdata != NULL); INSIST(mctx != NULL); INSIST(key != NULL); @@ -180,12 +180,14 @@ dns_dnssec_sign(const dns_name_t *name, dns_rdataset_t *set, dst_key_t *key, isc_result_t result; isc_buffer_t *databuf = NULL; char data[256 + 8]; + unsigned int labels; unsigned int sigsize; dns_fixedname_t fnewname; dns_fixedname_t fsigner; - REQUIRE(name != NULL); - REQUIRE(dns_name_countlabels(name) <= 255); + REQUIRE(DNS_NAME_VALID(name)); + labels = dns_name_countlabels(name); + REQUIRE(labels <= 255 && labels > 0); REQUIRE(set != NULL); REQUIRE(key != NULL); REQUIRE(inception != NULL); @@ -213,7 +215,7 @@ dns_dnssec_sign(const dns_name_t *name, dns_rdataset_t *set, dst_key_t *key, sig.covered = set->type; sig.algorithm = dst_algorithm_tosecalg(dst_key_alg(key)); - sig.labels = dns_name_countlabels(name) - 1; + sig.labels = labels - 1; if (dns_name_iswildcard(name)) { sig.labels--; } @@ -353,10 +355,13 @@ dns_dnssec_verify(const dns_name_t *name, dns_rdataset_t *set, dst_key_t *key, isc_result_t result; unsigned char data[300]; dst_context_t *ctx = NULL; - int labels = 0; + unsigned int labels; + unsigned int siglabels; bool downcase = false; - REQUIRE(name != NULL); + REQUIRE(DNS_NAME_VALID(name)); + labels = dns_name_countlabels(name); + REQUIRE(labels > 0); REQUIRE(set != NULL); REQUIRE(key != NULL); REQUIRE(mctx != NULL); @@ -368,6 +373,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; @@ -434,12 +454,11 @@ 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)) == ISC_R_SUCCESS); - if (labels - sig.labels > 0) { - dns_name_split(dns_fixedname_name(&fnewname), sig.labels + 1, - NULL, dns_fixedname_name(&fnewname)); + if (labels > siglabels) { + dns_name_split(dns_fixedname_name(&fnewname), siglabels, NULL, + dns_fixedname_name(&fnewname)); } dns_name_toregion(dns_fixedname_name(&fnewname), &r); @@ -448,7 +467,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); @@ -543,7 +562,7 @@ cleanup_struct: inc_stat(dns_dnssecstats_fail); } - if (result == ISC_R_SUCCESS && labels - sig.labels > 0) { + if (result == 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 647c45b519f..a4f9b4916b5 100644 --- a/lib/dns/rdata/generic/rrsig_46.c +++ b/lib/dns/rdata/generic/rrsig_46.c @@ -23,14 +23,16 @@ static isc_result_t fromtext_rrsig(ARGS_FROMTEXT) { isc_token_t token; - unsigned char alg, c; + unsigned char alg, labels; long i; dns_rdatatype_t covered; - char *e; + char *e = NULL; isc_result_t result; isc_buffer_t buffer; uint32_t time_signed, time_expire; unsigned int used; + dns_fixedname_t fixed; + dns_name_t *signer = dns_fixedname_initname(&fixed); REQUIRE(type == dns_rdatatype_rrsig); @@ -72,8 +74,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. @@ -148,7 +150,16 @@ fromtext_rrsig(ARGS_FROMTEXT) { if (origin == NULL) { origin = dns_rootname; } - RETTOK(dns_name_wirefromtext(&buffer, origin, options, target)); + RETTOK(dns_name_fromtext(signer, &buffer, origin, options)); + + /* + * (RRSIG labels doesn't include the root label, so add one + * to normalize it before checking against the signer.) + */ + if ((labels + 1) < dns_name_countlabels(signer)) { + RETTOK(ISC_R_RANGE); + } + RETERR(mem_tobuffer(target, signer->ndata, signer->length)); /* * Sig. @@ -295,6 +306,7 @@ fromwire_rrsig(ARGS_FROMWIRE) { isc_region_t sr; dns_name_t name; unsigned char algorithm; + unsigned char labels; REQUIRE(type == dns_rdatatype_rrsig); @@ -318,6 +330,7 @@ fromwire_rrsig(ARGS_FROMWIRE) { } algorithm = sr.base[2]; + labels = sr.base[3]; isc_buffer_forward(source, 18); RETERR(mem_tobuffer(target, sr.base, 18)); @@ -328,6 +341,14 @@ fromwire_rrsig(ARGS_FROMWIRE) { dns_name_init(&name); RETERR(dns_name_fromwire(&name, source, dctx, target)); + /* + * (RRSIG labels doesn't include the root label, so add one + * to normalize it before checking against the signer.) + */ + if ((labels + 1) < dns_name_countlabels(&name)) { + RETERR(DNS_R_FORMERR); + } + /* * Sig. */