From: Ondřej Surý Date: Tue, 24 Feb 2026 12:30:56 +0000 (+0100) Subject: Change NSEC3 and NSEC3PARAM struct fields to use isc_region_t X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=14cebe4d61469f9db179071a8ec759a835cf3d23;p=thirdparty%2Fbind9.git Change NSEC3 and NSEC3PARAM struct fields to use isc_region_t Replace the separate pointer+length field pairs in dns_rdata_nsec3_t (salt/salt_length, next/next_length, typebits/len) and dns_rdata_nsec3param_t (salt/salt_length) with isc_region_t. This makes the structs self-describing and eliminates a class of length-mismatch bugs. The dns_zone_setnsec3param() signature is updated to take isc_region_t *salt instead of separate saltlen and salt arguments. Function signatures for dns_nsec3_addnsec3, dns_db_getnsec3parameters, and related internal functions still use separate pointer+length pairs and should be updated in a follow-up. --- diff --git a/bin/dnssec/dnssec-signzone.c b/bin/dnssec/dnssec-signzone.c index f16fcbf636b..33fc35ac019 100644 --- a/bin/dnssec/dnssec-signzone.c +++ b/bin/dnssec/dnssec-signzone.c @@ -1887,8 +1887,8 @@ addnsec3param(const unsigned char *salt, size_t salt_len, nsec3param.flags = 0; nsec3param.hash = unknownalg ? DNS_NSEC3_UNKNOWNALG : dns_hash_sha1; nsec3param.iterations = iterations; - nsec3param.salt_length = (unsigned char)salt_len; - nsec3param.salt = UNCONST(salt); + nsec3param.salt.length = (unsigned char)salt_len; + nsec3param.salt.base = UNCONST(salt); isc_buffer_init(&b, nsec3parambuf, sizeof(nsec3parambuf)); result = dns_rdata_fromstruct(&rdata, gclass, dns_rdatatype_nsec3param, @@ -2045,8 +2045,8 @@ nsec3clean(dns_name_t *name, dns_dbnode_t *node, unsigned int hashalg, check_result(result, "dns_rdata_tostruct"); if (exists && nsec3.hash == hashalg && nsec3.iterations == iterations && - nsec3.salt_length == salt_len && - isc_safe_memequal(nsec3.salt, salt, salt_len)) + nsec3.salt.length == salt_len && + isc_safe_memequal(nsec3.salt.base, salt, salt_len)) { continue; } diff --git a/bin/named/server.c b/bin/named/server.c index d594d7dc6ec..03e3dbf5e54 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -12996,8 +12996,9 @@ named_server_signing(named_server_t *server, isc_lex_t *lex, bool resalt = false; uint32_t serial = 0; char keystr[DNS_SECALG_FORMATSIZE + 7]; /* <5-digit keyid>/ */ - unsigned short hash = 0, flags = 0, iter = 0, saltlen = 0; - unsigned char salt[255]; + unsigned short hash = 0, flags = 0, iter = 0; + isc_region_t salt = { 0 }; + unsigned char saltbuf[255]; const char *ptr; size_t n; bool kasp = false; @@ -13081,14 +13082,15 @@ named_server_signing(named_server_t *server, isc_lex_t *lex, * 5155 (64 bits). It should be made * configurable. */ - saltlen = 8; + salt.length = 8; resalt = true; } else if (strcmp(ptr, "-") != 0) { isc_buffer_t buf; - isc_buffer_init(&buf, salt, sizeof(salt)); + isc_buffer_init(&buf, saltbuf, sizeof(saltbuf)); CHECK(isc_hex_decodestring(ptr, &buf)); - saltlen = isc_buffer_usedlength(&buf); + salt.base = saltbuf; + salt.length = isc_buffer_usedlength(&buf); } } } else if (strcasecmp(ptr, "-serial") == 0) { @@ -13116,9 +13118,9 @@ named_server_signing(named_server_t *server, isc_lex_t *lex, (void)putstr(text, "request queued"); (void)putnull(text); } else if (chain && !kasp) { - CHECK(dns_zone_setnsec3param( - zone, (uint8_t)hash, (uint8_t)flags, iter, - (uint8_t)saltlen, salt, true, resalt)); + CHECK(dns_zone_setnsec3param(zone, (uint8_t)hash, + (uint8_t)flags, iter, &salt, true, + resalt)); (void)putstr(text, "nsec3param request queued"); (void)putnull(text); } else if (setserial) { diff --git a/bin/named/zoneconf.c b/bin/named/zoneconf.c index 9a87bae73b2..cc300845bea 100644 --- a/bin/named/zoneconf.c +++ b/bin/named/zoneconf.c @@ -1639,11 +1639,14 @@ named_zone_configure(const cfg_obj_t *config, const cfg_obj_t *vconfig, result = dns_zone_setnsec3param( zone, 1, dns_kasp_nsec3flags(kasp), dns_kasp_nsec3iter(kasp), - dns_kasp_nsec3saltlen(kasp), NULL, true, - false); + &(isc_region_t){ + .length = dns_kasp_nsec3saltlen( + kasp), + }, + true, false); } else { result = dns_zone_setnsec3param( - zone, 0, 0, 0, 0, NULL, true, false); + zone, 0, 0, 0, NULL, true, false); } INSIST(result == ISC_R_SUCCESS); diff --git a/lib/dns/include/dns/zone.h b/lib/dns/include/dns/zone.h index 1ad5732191b..5e6bfb70ef9 100644 --- a/lib/dns/include/dns/zone.h +++ b/lib/dns/include/dns/zone.h @@ -1021,8 +1021,8 @@ dns_zone_keydone(dns_zone_t *zone, const char *data); isc_result_t dns_zone_setnsec3param(dns_zone_t *zone, uint8_t hash, uint8_t flags, - uint16_t iter, uint8_t saltlen, unsigned char *salt, - bool replace, bool resalt); + uint16_t iter, isc_region_t *salt, bool replace, + bool resalt); /*%< * Set the NSEC3 parameters for the zone. * diff --git a/lib/dns/nsec3.c b/lib/dns/nsec3.c index f19f45439f5..c4954c311f6 100644 --- a/lib/dns/nsec3.c +++ b/lib/dns/nsec3.c @@ -189,13 +189,13 @@ dns_nsec3_typepresent(dns_rdata_t *rdata, dns_rdatatype_t type) { INSIST(result == ISC_R_SUCCESS); present = false; - for (i = 0; i < nsec3.len; i += len) { - INSIST(i + 2 <= nsec3.len); - window = nsec3.typebits[i]; - len = nsec3.typebits[i + 1]; + for (i = 0; i < nsec3.typebits.length; i += len) { + INSIST(i + 2 <= nsec3.typebits.length); + window = nsec3.typebits.base[i]; + len = nsec3.typebits.base[i + 1]; INSIST(len > 0 && len <= 32); i += 2; - INSIST(i + len <= nsec3.len); + INSIST(i + len <= nsec3.typebits.length); if (window * 256 > type) { break; } @@ -203,7 +203,7 @@ dns_nsec3_typepresent(dns_rdata_t *rdata, dns_rdatatype_t type) { continue; } if (type < (window * 256) + len * 8) { - present = dns_nsec_isset(&nsec3.typebits[i], + present = dns_nsec_isset(&nsec3.typebits.base[i], type % 256); } break; @@ -372,8 +372,9 @@ match_nsec3param(const dns_rdata_nsec3_t *nsec3, const dns_rdata_nsec3param_t *nsec3param) { if (nsec3->hash == nsec3param->hash && nsec3->iterations == nsec3param->iterations && - nsec3->salt_length == nsec3param->salt_length && - !memcmp(nsec3->salt, nsec3param->salt, nsec3->salt_length)) + nsec3->salt.length == nsec3param->salt.length && + !memcmp(nsec3->salt.base, nsec3param->salt.base, + nsec3->salt.length)) { return true; } @@ -545,8 +546,8 @@ dns_nsec3_addnsec3(dns_db_t *db, dns_dbversion_t *version, */ hash = nsec3param->hash; iterations = nsec3param->iterations; - salt_length = nsec3param->salt_length; - salt = nsec3param->salt; + salt_length = nsec3param->salt.length; + salt = nsec3param->salt.base; /* * Default flags for a new chain. @@ -603,9 +604,9 @@ dns_nsec3_addnsec3(dns_db_t *db, dns_dbversion_t *version, if (!CREATE(nsec3param->flags)) { flags = nsec3.flags; } - next_length = nsec3.next_length; + next_length = nsec3.next.length; INSIST(next_length <= sizeof(nexthash)); - memmove(nexthash, nsec3.next, next_length); + memmove(nexthash, nsec3.next.base, next_length); dns_rdataset_disassociate(&rdataset); /* * If the NSEC3 is not for a unsecure delegation then @@ -679,8 +680,8 @@ find_previous: } } - old_next = nsec3.next; - old_length = nsec3.next_length; + old_next = nsec3.next.base; + old_length = nsec3.next.length; /* * Delete the old previous NSEC3. @@ -690,8 +691,8 @@ find_previous: /* * Fixup the previous NSEC3. */ - nsec3.next = nexthash; - nsec3.next_length = (unsigned char)next_length; + nsec3.next.base = nexthash; + nsec3.next.length = (unsigned char)next_length; isc_buffer_init(&buffer, nsec3buf, sizeof(nsec3buf)); CHECK(dns_rdata_fromstruct(&rdata, rdataset.rdclass, dns_rdatatype_nsec3, &nsec3, @@ -811,8 +812,8 @@ addnsec3: continue; } - old_next = nsec3.next; - old_length = nsec3.next_length; + old_next = nsec3.next.base; + old_length = nsec3.next.length; /* * Delete the old previous NSEC3. @@ -822,8 +823,8 @@ addnsec3: /* * Fixup the previous NSEC3. */ - nsec3.next = nexthash; - nsec3.next_length = (unsigned char)next_length; + nsec3.next.base = nexthash; + nsec3.next.length = (unsigned char)next_length; isc_buffer_init(&buffer, nsec3buf, sizeof(nsec3buf)); CHECK(dns_rdata_fromstruct(&rdata, rdataset.rdclass, dns_rdatatype_nsec3, &nsec3, @@ -1028,13 +1029,12 @@ cleanup: isc_result_t dns_nsec3param_salttotext(dns_rdata_nsec3param_t *nsec3param, char *dst, size_t dstlen) { - isc_region_t r; isc_buffer_t b; REQUIRE(nsec3param != NULL); REQUIRE(dst != NULL); - if (nsec3param->salt_length == 0) { + if (nsec3param->salt.length == 0) { if (dstlen < 2U) { return ISC_R_NOSPACE; } @@ -1042,11 +1042,10 @@ dns_nsec3param_salttotext(dns_rdata_nsec3param_t *nsec3param, char *dst, return ISC_R_SUCCESS; } - r.base = nsec3param->salt; - r.length = nsec3param->salt_length; isc_buffer_init(&b, dst, (unsigned int)dstlen); - RETERR(isc_hex_totext(&r, 2, "", &b)); + isc_region_t salt = nsec3param->salt; + RETERR(isc_hex_totext(&salt, 2, "", &b)); if (isc_buffer_availablelength(&b) < 1) { return ISC_R_NOSPACE; @@ -1346,8 +1345,8 @@ dns_nsec3_delnsec3(dns_db_t *db, dns_dbversion_t *version, */ hash = nsec3param->hash; iterations = nsec3param->iterations; - salt_length = nsec3param->salt_length; - salt = nsec3param->salt; + salt_length = nsec3param->salt.length; + salt = nsec3param->salt.base; /* * If this is the first NSEC3 in the chain nexthash will @@ -1381,9 +1380,9 @@ dns_nsec3_delnsec3(dns_db_t *db, dns_dbversion_t *version, */ result = find_nsec3(&nsec3, &rdataset, nsec3param); if (result == ISC_R_SUCCESS) { - next_length = nsec3.next_length; + next_length = nsec3.next.length; INSIST(next_length <= sizeof(nexthash)); - memmove(nexthash, nsec3.next, next_length); + memmove(nexthash, nsec3.next.base, next_length); } dns_rdataset_disassociate(&rdataset); if (result == ISC_R_NOTFOUND) { @@ -1424,8 +1423,8 @@ dns_nsec3_delnsec3(dns_db_t *db, dns_dbversion_t *version, /* * Fixup the previous NSEC3. */ - nsec3.next = nexthash; - nsec3.next_length = (unsigned char)next_length; + nsec3.next.base = nexthash; + nsec3.next.length = (unsigned char)next_length; if (CREATE(nsec3param->flags)) { nsec3.flags = nsec3param->flags & DNS_NSEC3FLAG_OPTOUT; } @@ -1487,9 +1486,9 @@ cleanup_orphaned_ents: result = find_nsec3(&nsec3, &rdataset, nsec3param); if (result == ISC_R_SUCCESS) { - next_length = nsec3.next_length; + next_length = nsec3.next.length; INSIST(next_length <= sizeof(nexthash)); - memmove(nexthash, nsec3.next, next_length); + memmove(nexthash, nsec3.next.base, next_length); } dns_rdataset_disassociate(&rdataset); if (result == ISC_R_NOTFOUND) { @@ -1527,8 +1526,8 @@ cleanup_orphaned_ents: /* * Fixup the previous NSEC3. */ - nsec3.next = nexthash; - nsec3.next_length = (unsigned char)next_length; + nsec3.next.base = nexthash; + nsec3.next.length = (unsigned char)next_length; isc_buffer_init(&buffer, nsec3buf, sizeof(nsec3buf)); CHECK(dns_rdata_fromstruct(&rdata, rdataset.rdclass, dns_rdatatype_nsec3, &nsec3, @@ -1872,7 +1871,7 @@ dns_nsec3_noexistnodata(dns_rdatatype_t type, const dns_name_t *name, /* * The hash lengths should match. If not ignore the record. */ - if (isc_buffer_usedlength(&buffer) != nsec3.next_length) { + if (isc_buffer_usedlength(&buffer) != nsec3.next.length) { return ISC_R_IGNORE; } @@ -1880,7 +1879,7 @@ dns_nsec3_noexistnodata(dns_rdatatype_t type, const dns_name_t *name, * Work out what this NSEC3 covers. * Inside (<0) or outside (>=0). */ - scope = memcmp(owner, nsec3.next, nsec3.next_length); + scope = memcmp(owner, nsec3.next.base, nsec3.next.length); /* * Prepare to compute all the hashes. @@ -1899,15 +1898,15 @@ dns_nsec3_noexistnodata(dns_rdatatype_t type, const dns_name_t *name, } length = isc_iterated_hash(hash, nsec3.hash, nsec3.iterations, - nsec3.salt, nsec3.salt_length, + nsec3.salt.base, nsec3.salt.length, qname->ndata, qname->length); /* * The computed hash length should match. */ - if (length != nsec3.next_length) { + if (length != nsec3.next.length) { (*logit)(arg, ISC_LOG_DEBUG(3), "ignoring NSEC bad length %u vs %u", length, - nsec3.next_length); + nsec3.next.length); return ISC_R_IGNORE; } @@ -2013,9 +2012,9 @@ dns_nsec3_noexistnodata(dns_rdatatype_t type, const dns_name_t *name, * above. */ if ((scope < 0 && order > 0 && - memcmp(hash, nsec3.next, length) < 0) || + memcmp(hash, nsec3.next.base, length) < 0) || (scope >= 0 && - (order > 0 || memcmp(hash, nsec3.next, length) < 0))) + (order > 0 || memcmp(hash, nsec3.next.base, length) < 0))) { dns_name_format(qname, namebuf, sizeof(namebuf)); (*logit)(arg, ISC_LOG_DEBUG(3), diff --git a/lib/dns/qpzone.c b/lib/dns/qpzone.c index 80dbfbc72d1..e8440579cfb 100644 --- a/lib/dns/qpzone.c +++ b/lib/dns/qpzone.c @@ -1242,10 +1242,10 @@ setnsec3parameters(dns_db_t *db, qpz_version_t *version) { continue; } - memmove(version->salt, nsec3param.salt, - nsec3param.salt_length); + memmove(version->salt, nsec3param.salt.base, + nsec3param.salt.length); version->hash = nsec3param.hash; - version->salt_length = nsec3param.salt_length; + version->salt_length = nsec3param.salt.length; version->iterations = nsec3param.iterations; version->flags = nsec3param.flags; version->havensec3 = true; @@ -2722,9 +2722,9 @@ matchparams(dns_vecheader_t *header, qpz_search_t *search) { if (nsec3.hash == search->version->hash && nsec3.iterations == search->version->iterations && - nsec3.salt_length == search->version->salt_length && - memcmp(nsec3.salt, search->version->salt, - nsec3.salt_length) == 0) + nsec3.salt.length == search->version->salt_length && + memcmp(nsec3.salt.base, search->version->salt, + nsec3.salt.length) == 0) { return true; } diff --git a/lib/dns/rdata/generic/nsec3_50.c b/lib/dns/rdata/generic/nsec3_50.c index 9429d5dd4fd..bcad9261255 100644 --- a/lib/dns/rdata/generic/nsec3_50.c +++ b/lib/dns/rdata/generic/nsec3_50.c @@ -286,7 +286,7 @@ fromstruct_nsec3(ARGS_FROMSTRUCT) { REQUIRE(nsec3 != NULL); REQUIRE(nsec3->common.rdtype == type); REQUIRE(nsec3->common.rdclass == rdclass); - REQUIRE(nsec3->typebits != NULL || nsec3->len == 0); + REQUIRE(nsec3->typebits.base != NULL || nsec3->typebits.length == 0); UNUSED(type); UNUSED(rdclass); @@ -294,17 +294,20 @@ fromstruct_nsec3(ARGS_FROMSTRUCT) { RETERR(uint8_tobuffer(nsec3->hash, target)); RETERR(uint8_tobuffer(nsec3->flags, target)); RETERR(uint16_tobuffer(nsec3->iterations, target)); - RETERR(uint8_tobuffer(nsec3->salt_length, target)); - RETERR(mem_tobuffer(target, nsec3->salt, nsec3->salt_length)); - RETERR(uint8_tobuffer(nsec3->next_length, target)); - RETERR(mem_tobuffer(target, nsec3->next, nsec3->next_length)); + RETERR(uint8_tobuffer(nsec3->salt.length, target)); + RETERR(mem_tobuffer(target, nsec3->salt.base, nsec3->salt.length)); + RETERR(uint8_tobuffer(nsec3->next.length, target)); + RETERR(mem_tobuffer(target, nsec3->next.base, nsec3->next.length)); - region.base = nsec3->typebits; - region.length = nsec3->len; + region.base = nsec3->typebits.base; + region.length = nsec3->typebits.length; RETERR(typemap_test(®ion, true)); - return mem_tobuffer(target, nsec3->typebits, nsec3->len); + return mem_tobuffer(target, nsec3->typebits.base, + nsec3->typebits.length); } +static void freestruct_nsec3(ARGS_FREESTRUCT); + static isc_result_t tostruct_nsec3(ARGS_TOSTRUCT) { isc_region_t region; @@ -322,18 +325,19 @@ tostruct_nsec3(ARGS_TOSTRUCT) { nsec3->flags = uint8_consume_fromregion(®ion); nsec3->iterations = uint16_consume_fromregion(®ion); - nsec3->salt_length = uint8_consume_fromregion(®ion); - INSIST(nsec3->salt_length <= region.length); - nsec3->salt = mem_maybedup(mctx, region.base, nsec3->salt_length); - isc_region_consume(®ion, nsec3->salt_length); + nsec3->salt.length = uint8_consume_fromregion(®ion); + INSIST(nsec3->salt.length <= region.length); + nsec3->salt.base = mem_maybedup(mctx, region.base, nsec3->salt.length); + isc_region_consume(®ion, nsec3->salt.length); - nsec3->next_length = uint8_consume_fromregion(®ion); - INSIST(nsec3->next_length <= region.length); - nsec3->next = mem_maybedup(mctx, region.base, nsec3->next_length); - isc_region_consume(®ion, nsec3->next_length); + nsec3->next.length = uint8_consume_fromregion(®ion); + INSIST(nsec3->next.length <= region.length); + nsec3->next.base = mem_maybedup(mctx, region.base, nsec3->next.length); + isc_region_consume(®ion, nsec3->next.length); - nsec3->len = region.length; - nsec3->typebits = mem_maybedup(mctx, region.base, region.length); + nsec3->typebits.length = region.length; + nsec3->typebits.base = mem_maybedup(mctx, region.base, + nsec3->typebits.length); nsec3->mctx = mctx; return ISC_R_SUCCESS; @@ -350,14 +354,17 @@ freestruct_nsec3(ARGS_FREESTRUCT) { return; } - if (nsec3->salt != NULL) { - isc_mem_free(nsec3->mctx, nsec3->salt); + if (nsec3->salt.base != NULL) { + isc_mem_free(nsec3->mctx, nsec3->salt.base); + nsec3->salt.length = 0; } - if (nsec3->next != NULL) { - isc_mem_free(nsec3->mctx, nsec3->next); + if (nsec3->next.base != NULL) { + isc_mem_free(nsec3->mctx, nsec3->next.base); + nsec3->next.length = 0; } - if (nsec3->typebits != NULL) { - isc_mem_free(nsec3->mctx, nsec3->typebits); + if (nsec3->typebits.base != NULL) { + isc_mem_free(nsec3->mctx, nsec3->typebits.base); + nsec3->typebits.length = 0; } nsec3->mctx = NULL; } diff --git a/lib/dns/rdata/generic/nsec3_50.h b/lib/dns/rdata/generic/nsec3_50.h index 3152c8534eb..0e5e2e3fb21 100644 --- a/lib/dns/rdata/generic/nsec3_50.h +++ b/lib/dns/rdata/generic/nsec3_50.h @@ -17,6 +17,7 @@ * \brief Per RFC 5155 */ #include +#include typedef struct dns_rdata_nsec3 { dns_rdatacommon_t common; @@ -24,12 +25,9 @@ typedef struct dns_rdata_nsec3 { dns_hash_t hash; unsigned char flags; dns_iterations_t iterations; - unsigned char salt_length; - unsigned char next_length; - uint16_t len; - unsigned char *salt; - unsigned char *next; - unsigned char *typebits; + isc_region_t salt; + isc_region_t next; + isc_region_t typebits; } dns_rdata_nsec3_t; /* diff --git a/lib/dns/rdata/generic/nsec3param_51.c b/lib/dns/rdata/generic/nsec3param_51.c index e1b8effe8b0..c0b15e217f2 100644 --- a/lib/dns/rdata/generic/nsec3param_51.c +++ b/lib/dns/rdata/generic/nsec3param_51.c @@ -214,8 +214,9 @@ fromstruct_nsec3param(ARGS_FROMSTRUCT) { RETERR(uint8_tobuffer(nsec3param->hash, target)); RETERR(uint8_tobuffer(nsec3param->flags, target)); RETERR(uint16_tobuffer(nsec3param->iterations, target)); - RETERR(uint8_tobuffer(nsec3param->salt_length, target)); - RETERR(mem_tobuffer(target, nsec3param->salt, nsec3param->salt_length)); + RETERR(uint8_tobuffer(nsec3param->salt.length, target)); + RETERR(mem_tobuffer(target, nsec3param->salt.base, + nsec3param->salt.length)); return ISC_R_SUCCESS; } @@ -236,11 +237,11 @@ tostruct_nsec3param(ARGS_TOSTRUCT) { nsec3param->flags = uint8_consume_fromregion(®ion); nsec3param->iterations = uint16_consume_fromregion(®ion); - nsec3param->salt_length = uint8_consume_fromregion(®ion); - INSIST(nsec3param->salt_length == region.length); - nsec3param->salt = mem_maybedup(mctx, region.base, - nsec3param->salt_length); - isc_region_consume(®ion, nsec3param->salt_length); + nsec3param->salt.length = uint8_consume_fromregion(®ion); + INSIST(nsec3param->salt.length == region.length); + nsec3param->salt.base = mem_maybedup(mctx, region.base, + nsec3param->salt.length); + isc_region_consume(®ion, nsec3param->salt.length); nsec3param->mctx = mctx; return ISC_R_SUCCESS; @@ -257,8 +258,9 @@ freestruct_nsec3param(ARGS_FREESTRUCT) { return; } - if (nsec3param->salt != NULL) { - isc_mem_free(nsec3param->mctx, nsec3param->salt); + if (nsec3param->salt.base != NULL) { + isc_mem_free(nsec3param->mctx, nsec3param->salt.base); + nsec3param->salt.length = 0; } nsec3param->mctx = NULL; } diff --git a/lib/dns/rdata/generic/nsec3param_51.h b/lib/dns/rdata/generic/nsec3param_51.h index 9288a96ae76..53c6767c3fa 100644 --- a/lib/dns/rdata/generic/nsec3param_51.h +++ b/lib/dns/rdata/generic/nsec3param_51.h @@ -24,6 +24,5 @@ typedef struct dns_rdata_nsec3param { dns_hash_t hash; unsigned char flags; /* DNS_NSEC3FLAG_* */ dns_iterations_t iterations; - unsigned char salt_length; - unsigned char *salt; + isc_region_t salt; } dns_rdata_nsec3param_t; diff --git a/lib/dns/validator.c b/lib/dns/validator.c index 44482328344..a0ec6dfe346 100644 --- a/lib/dns/validator.c +++ b/lib/dns/validator.c @@ -336,7 +336,7 @@ trynsec3: if (nsec3.hash != 1) { continue; } - if (nsec3.next_length > NSEC3_MAX_HASH_LENGTH) { + if (nsec3.next.length > NSEC3_MAX_HASH_LENGTH) { continue; } /* @@ -352,8 +352,9 @@ trynsec3: return ISC_R_SUCCESS; } length = isc_iterated_hash( - hash, nsec3.hash, nsec3.iterations, nsec3.salt, - nsec3.salt_length, name->ndata, name->length); + hash, nsec3.hash, nsec3.iterations, + nsec3.salt.base, nsec3.salt.length, name->ndata, + name->length); if (length != isc_buffer_usedlength(&buffer)) { continue; } @@ -370,12 +371,13 @@ trynsec3: /* * Does this optout span cover the name? */ - scope = memcmp(owner, nsec3.next, nsec3.next_length); + scope = memcmp(owner, nsec3.next.base, + nsec3.next.length); if ((scope < 0 && order > 0 && - memcmp(hash, nsec3.next, length) < 0) || + memcmp(hash, nsec3.next.base, length) < 0) || (scope >= 0 && (order > 0 || - memcmp(hash, nsec3.next, length) < 0))) + memcmp(hash, nsec3.next.base, length) < 0))) { dns_rdataset_disassociate(&set); return ISC_R_SUCCESS; diff --git a/lib/dns/zone.c b/lib/dns/zone.c index cd1cf63dc4c..b397a1672fd 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -2956,9 +2956,10 @@ zone_addnsec3chain(dns_zone_t *zone, dns_rdata_nsec3param_t *nsec3param) { nsec3chain->nsec3param.hash = nsec3param->hash; nsec3chain->nsec3param.iterations = nsec3param->iterations; nsec3chain->nsec3param.flags = nsec3param->flags; - nsec3chain->nsec3param.salt_length = nsec3param->salt_length; - memmove(nsec3chain->salt, nsec3param->salt, nsec3param->salt_length); - nsec3chain->nsec3param.salt = nsec3chain->salt; + nsec3chain->nsec3param.salt.length = nsec3param->salt.length; + memmove(nsec3chain->salt, nsec3param->salt.base, + nsec3param->salt.length); + nsec3chain->nsec3param.salt.base = nsec3chain->salt; nsec3chain->seen_nsec = false; nsec3chain->delete_nsec = false; nsec3chain->save_delete_nsec = false; @@ -3018,10 +3019,10 @@ zone_addnsec3chain(dns_zone_t *zone, dns_rdata_nsec3param_t *nsec3param) { (current->nsec3param.hash == nsec3param->hash) && (current->nsec3param.iterations == nsec3param->iterations) && - (current->nsec3param.salt_length == - nsec3param->salt_length) && - memcmp(current->nsec3param.salt, nsec3param->salt, - nsec3param->salt_length) == 0) + (current->nsec3param.salt.length == + nsec3param->salt.length) && + memcmp(current->nsec3param.salt.base, nsec3param->salt.base, + nsec3param->salt.length) == 0) { current->done = true; } @@ -6846,9 +6847,9 @@ fixup_nsec3param(dns_db_t *db, dns_dbversion_t *ver, dns_nsec3chain_t *chain, if (nsec3param.hash != chain->nsec3param.hash || (active && nsec3param.flags != 0) || nsec3param.iterations != chain->nsec3param.iterations || - nsec3param.salt_length != chain->nsec3param.salt_length || - memcmp(nsec3param.salt, chain->nsec3param.salt, - nsec3param.salt_length)) + nsec3param.salt.length != chain->nsec3param.salt.length || + memcmp(nsec3param.salt.base, chain->nsec3param.salt.base, + nsec3param.salt.length)) { /* * If the SOA minimum is different to the current TTL, @@ -6881,10 +6882,11 @@ fixup_nsec3param(dns_db_t *db, dns_dbversion_t *ver, dns_nsec3chain_t *chain, (active && nsec3param.flags != 0) || nsec3param.iterations != chain->nsec3param.iterations || - nsec3param.salt_length != - chain->nsec3param.salt_length || - memcmp(nsec3param.salt, chain->nsec3param.salt, - nsec3param.salt_length)) + nsec3param.salt.length != + chain->nsec3param.salt.length || + memcmp(nsec3param.salt.base, + chain->nsec3param.salt.base, + nsec3param.salt.length)) { CHECK(update_one_rr(db, ver, diff, DNS_DIFFOP_ADD, name, ttl, @@ -6932,9 +6934,9 @@ try_private: (nsec3param.flags & DNS_NSEC3FLAG_INITIAL) != 0) || nsec3param.hash != chain->nsec3param.hash || nsec3param.iterations != chain->nsec3param.iterations || - nsec3param.salt_length != chain->nsec3param.salt_length || - memcmp(nsec3param.salt, chain->nsec3param.salt, - nsec3param.salt_length)) + nsec3param.salt.length != chain->nsec3param.salt.length || + memcmp(nsec3param.salt.base, chain->nsec3param.salt.base, + nsec3param.salt.length)) { continue; } @@ -7024,8 +7026,9 @@ deletematchingnsec3(dns_db_t *db, dns_dbversion_t *ver, dns_dbnode_t *node, CHECK(dns_rdata_tostruct(&rdata, &nsec3, NULL)); if (nsec3.hash != param->hash || nsec3.iterations != param->iterations || - nsec3.salt_length != param->salt_length || - memcmp(nsec3.salt, param->salt, nsec3.salt_length)) + nsec3.salt.length != param->salt.length || + memcmp(nsec3.salt.base, param->salt.base, + nsec3.salt.length)) { continue; } @@ -7094,8 +7097,9 @@ need_nsec_chain(dns_db_t *db, dns_dbversion_t *ver, */ if (myparam.hash == param->hash && myparam.iterations == param->iterations && - myparam.salt_length == param->salt_length && - !memcmp(myparam.salt, param->salt, myparam.salt_length)) + myparam.salt.length == param->salt.length && + !memcmp(myparam.salt.base, param->salt.base, + myparam.salt.length)) { continue; } @@ -20826,7 +20830,7 @@ rss_post(void *arg) { unsigned char saltbuf[255]; isc_buffer_t b; - param.salt = NULL; + param.salt = (isc_region_t){ .base = NULL }; result = dns__zone_lookup_nsec3param(zone, &np->rdata, ¶m, saltbuf, np->resalt); if (result == ISC_R_SUCCESS) { @@ -20843,13 +20847,13 @@ rss_post(void *arg) { goto cleanup; } - INSIST(param.salt != NULL); + INSIST(param.salt.base != NULL); /* Update NSEC3 parameters. */ np->rdata.hash = param.hash; np->rdata.flags = param.flags; np->rdata.iterations = param.iterations; - np->rdata.salt_length = param.salt_length; + np->rdata.salt.length = param.salt.length; np->rdata.salt = param.salt; isc_buffer_init(&b, nbuf, sizeof(nbuf)); @@ -21067,12 +21071,12 @@ dns__zone_lookup_nsec3param(dns_zone_t *zone, dns_rdata_nsec3param_t *lookup, if (nsec3param.iterations != lookup->iterations) { continue; } - if (nsec3param.salt_length != lookup->salt_length) { + if (nsec3param.salt.length != lookup->salt.length) { continue; } - if (lookup->salt != NULL) { - if (memcmp(nsec3param.salt, lookup->salt, - lookup->salt_length) != 0) + if (lookup->salt.base != NULL) { + if (memcmp(nsec3param.salt.base, lookup->salt.base, + lookup->salt.length) != 0) { continue; } @@ -21082,7 +21086,6 @@ dns__zone_lookup_nsec3param(dns_zone_t *zone, dns_rdata_nsec3param_t *lookup, param->hash = nsec3param.hash; param->flags = nsec3param.flags; param->iterations = nsec3param.iterations; - param->salt_length = nsec3param.salt_length; param->salt = nsec3param.salt; break; } @@ -21093,7 +21096,6 @@ setparam: param->hash = lookup->hash; param->flags = lookup->flags; param->iterations = lookup->iterations; - param->salt_length = lookup->salt_length; param->salt = lookup->salt; } @@ -21101,31 +21103,31 @@ setparam: CHECK(result); } - if (param->salt_length == 0) { - param->salt = (unsigned char *)"-"; - } else if (resalt || param->salt == NULL) { + if (param->salt.length == 0) { + param->salt.base = (unsigned char *)"-"; + } else if (resalt || param->salt.base == NULL) { unsigned char *newsalt; unsigned char salttext[255 * 2 + 1]; do { /* Generate a new salt. */ result = dns_nsec3_generate_salt(saltbuf, - param->salt_length); + param->salt.length); if (result != ISC_R_SUCCESS) { break; } newsalt = saltbuf; - salt2text(newsalt, param->salt_length, salttext, + salt2text(newsalt, param->salt.length, salttext, sizeof(salttext)); dnssec_log(zone, ISC_LOG_INFO, "generated salt: %s", salttext); /* Check for salt conflict. */ - if (param->salt != NULL && - memcmp(newsalt, param->salt, param->salt_length) == - 0) + if (param->salt.base != NULL && + memcmp(newsalt, param->salt.base, + param->salt.length) == 0) { result = ISC_R_SUCCESS; } else { - param->salt = newsalt; + param->salt.base = newsalt; result = DNS_R_NSEC3RESALT; } } while (result == ISC_R_SUCCESS); @@ -21170,8 +21172,8 @@ cleanup: */ isc_result_t dns_zone_setnsec3param(dns_zone_t *zone, uint8_t hash, uint8_t flags, - uint16_t iter, uint8_t saltlen, unsigned char *salt, - bool replace, bool resalt) { + uint16_t iter, isc_region_t *salt, bool replace, + bool resalt) { isc_result_t result = ISC_R_SUCCESS; dns_rdata_nsec3param_t param, lookup; dns_rdata_t nrdata = DNS_RDATA_INIT; @@ -21195,9 +21197,8 @@ dns_zone_setnsec3param(dns_zone_t *zone, uint8_t hash, uint8_t flags, lookup.hash = hash; lookup.flags = flags; lookup.iterations = iter; - lookup.salt_length = saltlen; - lookup.salt = salt; - param.salt = NULL; + lookup.salt = *salt; + param.salt = (isc_region_t){ .base = NULL }; result = dns__zone_lookup_nsec3param(zone, &lookup, ¶m, saltbuf, resalt); if (result == ISC_R_SUCCESS) { @@ -21208,7 +21209,7 @@ dns_zone_setnsec3param(dns_zone_t *zone, uint8_t hash, uint8_t flags, * Schedule lookup if lookup above failed (may happen if * zone db is NULL for example). */ - do_lookup = (param.salt == NULL) ? true : false; + do_lookup = (param.salt.base == NULL) ? true : false; } npe = isc_mem_get(zone->mctx, sizeof(*npe)); @@ -21236,7 +21237,7 @@ dns_zone_setnsec3param(dns_zone_t *zone, uint8_t hash, uint8_t flags, */ isc_buffer_init(&b, nbuf, sizeof(nbuf)); - if (param.salt != NULL) { + if (param.salt.base != NULL) { CHECK(dns_rdata_fromstruct(&nrdata, zone->rdclass, dns_rdatatype_nsec3param, ¶m, &b)); @@ -21250,16 +21251,16 @@ dns_zone_setnsec3param(dns_zone_t *zone, uint8_t hash, uint8_t flags, if (isc_log_wouldlog(ISC_LOG_DEBUG(3))) { unsigned char salttext[255 * 2 + 1]; - if (param.salt != NULL) { - salt2text(param.salt, param.salt_length, + if (param.salt.base != NULL) { + salt2text(param.salt.base, param.salt.length, salttext, sizeof(salttext)); } dnssec_log(zone, ISC_LOG_DEBUG(3), "setnsec3param:nsec3 %u %u %u %u:%s", param.hash, param.flags, param.iterations, - param.salt_length, - param.salt == NULL ? "unknown" - : (char *)salttext); + param.salt.length, + param.salt.base == NULL ? "unknown" + : (char *)salttext); } } diff --git a/lib/dns/zoneverify.c b/lib/dns/zoneverify.c index b48752ceb0d..70209432a2c 100644 --- a/lib/dns/zoneverify.c +++ b/lib/dns/zoneverify.c @@ -90,8 +90,8 @@ struct nsec3_chain_fixed { * fields declared above for each NSEC3 chain element: * * unsigned char salt[salt_length]; - * unsigned char owner[next_length]; - * unsigned char next[next_length]; + * unsigned char owner[next.length]; + * unsigned char next[next.length]; */ }; @@ -394,21 +394,21 @@ record_nsec3(const vctx_t *vctx, const unsigned char *rawhash, unsigned char *cp = NULL; size_t len; - len = sizeof(*element) + nsec3->next_length * 2 + nsec3->salt_length; + len = sizeof(*element) + nsec3->next.length * 2 + nsec3->salt.length; element = isc_mem_get(vctx->mctx, len); *element = (struct nsec3_chain_fixed){ .hash = nsec3->hash, - .salt_length = nsec3->salt_length, - .next_length = nsec3->next_length, + .salt_length = nsec3->salt.length, + .next_length = nsec3->next.length, .iterations = nsec3->iterations, }; cp = (unsigned char *)(element + 1); - memmove(cp, nsec3->salt, nsec3->salt_length); - cp += nsec3->salt_length; - memmove(cp, rawhash, nsec3->next_length); - cp += nsec3->next_length; - memmove(cp, nsec3->next, nsec3->next_length); + memmove(cp, nsec3->salt.base, nsec3->salt.length); + cp += nsec3->salt.length; + memmove(cp, rawhash, nsec3->next.length); + cp += nsec3->next.length; + memmove(cp, nsec3->next.base, nsec3->next.length); isc_heap_insert(chains, element); } @@ -428,11 +428,11 @@ find_nsec3_match(const dns_rdata_nsec3param_t *nsec3param, dns_rdataset_current(rdataset, &rdata); dns_rdata_tostruct(&rdata, nsec3_match, NULL); if (nsec3_match->hash == nsec3param->hash && - nsec3_match->next_length == rhsize && + nsec3_match->next.length == rhsize && nsec3_match->iterations == nsec3param->iterations && - nsec3_match->salt_length == nsec3param->salt_length && - memcmp(nsec3_match->salt, nsec3param->salt, - nsec3param->salt_length) == 0) + nsec3_match->salt.length == nsec3param->salt.length && + memcmp(nsec3_match->salt.base, nsec3param->salt.base, + nsec3param->salt.length) == 0) { return ISC_R_SUCCESS; } @@ -466,7 +466,9 @@ match_nsec3(const vctx_t *vctx, const dns_name_t *name, * Check the type list. */ len = dns_nsec_compressbitmap(cbm, types, maxtype); - if (nsec3.len != len || memcmp(cbm, nsec3.typebits, len) != 0) { + if (nsec3.typebits.length != len || + memcmp(cbm, nsec3.typebits.base, len) != 0) + { dns_name_format(name, namebuf, sizeof(namebuf)); zoneverify_log_error(vctx, "Bad NSEC3 record for %s, bit map " @@ -494,9 +496,9 @@ match_nsec3(const vctx_t *vctx, const dns_name_t *name, RUNTIME_CHECK(result == ISC_R_SUCCESS); if (nsec3.hash == nsec3param->hash && nsec3.iterations == nsec3param->iterations && - nsec3.salt_length == nsec3param->salt_length && - memcmp(nsec3.salt, nsec3param->salt, nsec3.salt_length) == - 0) + nsec3.salt.length == nsec3param->salt.length && + memcmp(nsec3.salt.base, nsec3param->salt.base, + nsec3.salt.length) == 0) { dns_name_format(name, namebuf, sizeof(namebuf)); zoneverify_log_error(vctx, @@ -529,9 +531,9 @@ innsec3params(const dns_rdata_nsec3_t *nsec3, dns_rdataset_t *nsec3paramset) { RUNTIME_CHECK(result == ISC_R_SUCCESS); if (nsec3param.flags == 0 && nsec3param.hash == nsec3->hash && nsec3param.iterations == nsec3->iterations && - nsec3param.salt_length == nsec3->salt_length && - memcmp(nsec3param.salt, nsec3->salt, nsec3->salt_length) == - 0) + nsec3param.salt.length == nsec3->salt.length && + memcmp(nsec3param.salt.base, nsec3->salt.base, + nsec3->salt.length) == 0) { return true; } @@ -576,7 +578,7 @@ record_found(const vctx_t *vctx, const dns_name_t *name, dns_dbnode_t *node, dns_rdataset_current(&rdataset, &rdata); result = dns_rdata_tostruct(&rdata, &nsec3, NULL); RUNTIME_CHECK(result == ISC_R_SUCCESS); - if (nsec3.next_length != isc_buffer_usedlength(&b)) { + if (nsec3.next.length != isc_buffer_usedlength(&b)) { continue; } @@ -614,10 +616,10 @@ isoptout(const vctx_t *vctx, const dns_rdata_nsec3param_t *nsec3param, size_t rhsize = sizeof(rawhash); dns_fixedname_init(&fixed); - result = dns_nsec3_hashname(&fixed, rawhash, &rhsize, vctx->origin, - vctx->origin, nsec3param->hash, - nsec3param->iterations, nsec3param->salt, - nsec3param->salt_length); + result = dns_nsec3_hashname( + &fixed, rawhash, &rhsize, vctx->origin, vctx->origin, + nsec3param->hash, nsec3param->iterations, nsec3param->salt.base, + nsec3param->salt.length); if (result != ISC_R_SUCCESS) { zoneverify_log_error(vctx, "dns_nsec3_hashname(): %s", isc_result_totext(result)); @@ -698,9 +700,10 @@ verifynsec3(const vctx_t *vctx, const dns_name_t *name, RETERR(isoptout(vctx, &nsec3param, &optout)); dns_fixedname_init(&fixed); - result = dns_nsec3_hashname( - &fixed, rawhash, &rhsize, name, vctx->origin, nsec3param.hash, - nsec3param.iterations, nsec3param.salt, nsec3param.salt_length); + result = dns_nsec3_hashname(&fixed, rawhash, &rhsize, name, + vctx->origin, nsec3param.hash, + nsec3param.iterations, nsec3param.salt.base, + nsec3param.salt.length); if (result != ISC_R_SUCCESS) { zoneverify_log_error(vctx, "dns_nsec3_hashname(): %s", isc_result_totext(result)); diff --git a/tests/dns/nsec3param_test.c b/tests/dns/nsec3param_test.c index c8130d78bd9..1da5d12f491 100644 --- a/tests/dns/nsec3param_test.c +++ b/tests/dns/nsec3param_test.c @@ -79,14 +79,16 @@ copy_params(nsec3param_rdata_test_params_t from, dns_rdata_nsec3param_t *to, to->hash = from.hash; to->flags = from.flags; to->iterations = from.iterations; - to->salt_length = from.salt_length; if (from.salt == NULL) { - to->salt = NULL; + to->salt = (isc_region_t){ .base = NULL, + .length = from.salt_length }; } else if (strcmp(from.salt, "-") == 0) { - to->salt = (unsigned char *)"-"; + to->salt = (isc_region_t){ .base = (unsigned char *)"-", + .length = 0 }; } else { decode_salt(from.salt, saltbuf, saltlen); - to->salt = saltbuf; + to->salt = (isc_region_t){ .base = saltbuf, + .length = from.salt_length }; } } @@ -143,10 +145,11 @@ nsec3param_change_test(const nsec3param_change_test_params_t *test) { assert_int_equal(param.hash, expect.hash); assert_int_equal(param.flags, expect.flags); assert_int_equal(param.iterations, expect.iterations); - assert_int_equal(param.salt_length, expect.salt_length); - assert_non_null(param.salt); - if (expect.salt != NULL) { - int ret = memcmp(param.salt, expect.salt, expect.salt_length); + assert_int_equal(param.salt.length, expect.salt.length); + assert_non_null(param.salt.base); + if (expect.salt.base != NULL) { + int ret = memcmp(param.salt.base, expect.salt.base, + expect.salt.length); assert_true(ret == 0); } else { /* @@ -156,7 +159,7 @@ nsec3param_change_test(const nsec3param_change_test_params_t *test) { unsigned char salt[SALTLEN]; int ret; decode_salt(SALT, salt, SALTLEN); - ret = memcmp(param.salt, salt, SALTLEN); + ret = memcmp(param.salt.base, salt, SALTLEN); assert_false(ret == 0); } diff --git a/tests/dns/private_test.c b/tests/dns/private_test.c index 762bc0626bc..2ee5310b1a2 100644 --- a/tests/dns/private_test.c +++ b/tests/dns/private_test.c @@ -95,8 +95,7 @@ make_nsec3(nsec3_testcase_t *testcase, dns_rdata_t *private, params.common.rdtype = dns_rdatatype_nsec3param; params.hash = testcase->hash; params.iterations = testcase->iterations; - params.salt = sp; - params.salt_length = slen; + params.salt = (isc_region_t){ .base = sp, .length = slen }; params.flags = testcase->flags; if (testcase->remove) {