diff --git a/CHANGES b/CHANGES index d4f2df86eb..28b3edd3af 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,7 @@ +5653. [bug] Fixed a bug that caused the NSEC3 salt to be changed + for KASP zones on restart. + [GL #2725] + 5652. [bug] Copy and paste error caused the socket option to be enabled instead of disabled. [GL #2746] diff --git a/bin/tests/system/nsec3/tests.sh b/bin/tests/system/nsec3/tests.sh index 2982be9188..62c49e5904 100644 --- a/bin/tests/system/nsec3/tests.sh +++ b/bin/tests/system/nsec3/tests.sh @@ -41,8 +41,8 @@ set_nsec3param() { FLAGS=$1 ITERATIONS=$2 SALTLEN=$3 + # Reset salt. SALT="" - test "$SALTLEN" = "0" && SALT="-" } # The apex NSEC3PARAM record indicates that it is signed. @@ -115,6 +115,10 @@ _check_nsec3_nsec3param() { dig_with_opts +noquestion @$SERVER "${ZONE}" NSEC3PARAM > "dig.out.test$n.nsec3param.$ZONE" || return 1 grep "${ZONE}.*0.*IN.*NSEC3PARAM.*1.*0.*${ITERATIONS}.*${SALT}" "dig.out.test$n.nsec3param.$ZONE" > /dev/null || return 1 + + if [ -z "$SALT" ]; then + SALT=`awk '$4 == "NSEC3PARAM" { print $8 }' dig.out.test$n.nsec3param.$ZONE` + fi return 0 } @@ -128,14 +132,14 @@ _check_nsec3_nxdomain() check_nsec3() { n=$((n+1)) - echo_i "check NSEC3PARAM response for zone ${ZONE} ($n)" + echo_i "check that NSEC3PARAM 1 0 ${ITERATIONS} is published zone ${ZONE} ($n)" ret=0 retry_quiet 10 _check_nsec3_nsec3param || log_error "bad NSEC3PARAM response for ${ZONE}" test "$ret" -eq 0 || echo_i "failed" status=$((status+ret)) n=$((n+1)) - echo_i "check NSEC3 response for zone ${ZONE} ($n)" + echo_i "check NXDOMAIN response has correct NSEC3 1 ${FLAGS} ${ITERATIONS} ${SALT} for zone ${ZONE} ($n)" ret=0 retry_quiet 10 _check_nsec3_nxdomain || log_error "bad NXDOMAIN response for zone ${ZONE}" test "$ret" -eq 0 || echo_i "failed" @@ -169,24 +173,28 @@ dnssec_verify # Zone: nsec3-change.kasp. set_zone_policy "nsec3-change.kasp" "nsec3" +set_nsec3param "0" "5" "8" echo_i "initial check zone ${ZONE}" check_nsec3 dnssec_verify # Zone: nsec3-dynamic-change.kasp. set_zone_policy "nsec3-dynamic-change.kasp" "nsec3" +set_nsec3param "0" "5" "8" echo_i "initial check zone ${ZONE}" check_nsec3 dnssec_verify # Zone: nsec3-to-nsec.kasp. set_zone_policy "nsec3-to-nsec.kasp" "nsec3" +set_nsec3param "0" "5" "8" echo_i "initial check zone ${ZONE}" check_nsec3 dnssec_verify # Zone: nsec3-to-optout.kasp. set_zone_policy "nsec3-to-optout.kasp" "nsec3" +set_nsec3param "0" "5" "8" echo_i "initial check zone ${ZONE}" check_nsec3 dnssec_verify @@ -205,7 +213,6 @@ echo_i "initial check zone ${ZONE}" check_nsec3 dnssec_verify - # Reconfig named. echo_i "reconfig dnssec-policy to trigger nsec3 rollovers" copy_setports ns3/named2.conf.in ns3/named.conf @@ -221,12 +228,14 @@ dnssec_verify # Zone: nsec3.kasp. (same) set_zone_policy "nsec3.kasp" "nsec3" +set_nsec3param "0" "5" "8" echo_i "check zone ${ZONE} after reconfig" check_nsec3 dnssec_verify # Zone: nsec3-dyamic.kasp. (same) set_zone_policy "nsec3-dynamic.kasp" "nsec3" +set_nsec3param "0" "5" "8" echo_i "check zone ${ZONE} after reconfig" check_nsec3 dnssec_verify @@ -247,6 +256,7 @@ dnssec_verify # Zone: nsec3-to-nsec.kasp. (reconfigured) set_zone_policy "nsec3-to-nsec.kasp" "nsec" +set_nsec3param "1" "11" "0" echo_i "check zone ${ZONE} after reconfig" check_nsec dnssec_verify @@ -286,5 +296,33 @@ grep "zone uses dnssec-policy, use rndc dnssec command instead" rndc.signing.tes check_nsec3 dnssec_verify +# Test NSEC3 and NSEC3PARAM is the same after restart +set_zone_policy "nsec3.kasp" "nsec3" +set_nsec3param "0" "5" "8" +echo_i "check zone ${ZONE} before restart" +check_nsec3 +dnssec_verify + +# Restart named, NSEC3 should stay the same. +ret=0 +echo "stop ns3" +$PERL ../stop.pl --use-rndc --port ${CONTROLPORT} nsec3 ${DIR} || ret=1 +test "$ret" -eq 0 || echo_i "failed" +status=$((status+ret)) + +ret=0 +echo "start ns3" +start_server --noclean --restart --port ${PORT} nsec3 ${DIR} +test "$ret" -eq 0 || echo_i "failed" +status=$((status+ret)) + +prevsalt="${SALT}" +set_zone_policy "nsec3.kasp" "nsec3" +set_nsec3param "0" "5" "8" +SALT="${prevsalt}" +echo_i "check zone ${ZONE} after restart has salt ${SALT}" +check_nsec3 +dnssec_verify + echo_i "exit status: $status" [ $status -eq 0 ] || exit 1 diff --git a/doc/notes/notes-current.rst b/doc/notes/notes-current.rst index 330a86c8be..92f0a693ed 100644 --- a/doc/notes/notes-current.rst +++ b/doc/notes/notes-current.rst @@ -76,3 +76,6 @@ Bug Fixes - Fix an error that would enable don't fragment socket option instead of disabling it leading to errors when sending the oversized UDP packets. [GL #2746] + +- Fixed a bug that caused the NSEC salt to be changed for KASP zones on + every startup. :gl:`#2725` diff --git a/lib/dns/zone.c b/lib/dns/zone.c index c4788ea203..74dfa82c3c 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -959,10 +959,13 @@ static const char *dbargv_default[] = { "rbt" }; typedef struct nsec3param nsec3param_t; struct nsec3param { + dns_rdata_nsec3param_t rdata; unsigned char data[DNS_NSEC3PARAM_BUFFERSIZE + 1]; unsigned int length; bool nsec; bool replace; + bool resalt; + bool lookup; ISC_LINK(nsec3param_t) link; }; typedef ISC_LIST(nsec3param_t) nsec3paramlist_t; @@ -21250,6 +21253,25 @@ setnsec3param(isc_task_t *task, isc_event_t *event) { dns_zone_idetach(&zone); } +static void +salt2text(unsigned char *salt, uint8_t saltlen, unsigned char *text, + unsigned int textlen) { + isc_region_t r; + isc_buffer_t buf; + isc_result_t result; + + r.base = salt; + r.length = (unsigned int)saltlen; + + isc_buffer_init(&buf, text, textlen); + result = isc_hex_totext(&r, 2, "", &buf); + if (result == ISC_R_SUCCESS) { + text[saltlen * 2] = 0; + } else { + text[0] = 0; + } +} + /* * Check whether NSEC3 chain addition or removal specified by the private-type * record passed with the event was already queued (or even fully performed). @@ -21301,6 +21323,53 @@ rss_post(dns_zone_t *zone, isc_event_t *event) { CHECK(dns_db_getoriginnode(db, &node)); + /* + * Do we need to look up the NSEC3 parameters? + */ + if (np->lookup) { + dns_rdata_nsec3param_t param; + dns_rdata_t nrdata = DNS_RDATA_INIT; + dns_rdata_t prdata = DNS_RDATA_INIT; + unsigned char nbuf[DNS_NSEC3PARAM_BUFFERSIZE]; + unsigned char saltbuf[255]; + isc_buffer_t b; + + param.salt = NULL; + result = dns__zone_lookup_nsec3param(zone, &np->rdata, ¶m, + saltbuf, np->resalt); + if (result == ISC_R_SUCCESS) { + /* + * Success because the NSEC3PARAM already exists, but + * function returns void, so goto failure to clean up. + */ + goto failure; + } + if (result != DNS_R_NSEC3RESALT && result != ISC_R_NOTFOUND) { + dnssec_log(zone, ISC_LOG_DEBUG(3), + "setnsec3param:lookup nsec3param -> %s", + isc_result_totext(result)); + goto failure; + } + + INSIST(param.salt != 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 = param.salt; + + isc_buffer_init(&b, nbuf, sizeof(nbuf)); + CHECK(dns_rdata_fromstruct(&nrdata, zone->rdclass, + dns_rdatatype_nsec3param, &np->rdata, + &b)); + dns_nsec3param_toprivate(&nrdata, &prdata, zone->privatetype, + np->data, sizeof(np->data)); + np->length = prdata.length; + np->nsec = false; + } + /* * Does a private-type record already exist for this chain? */ @@ -21443,25 +21512,6 @@ failure: INSIST(newver == NULL); } -static void -salt2text(unsigned char *salt, uint8_t saltlen, unsigned char *text, - unsigned int textlen) { - isc_region_t r; - isc_buffer_t buf; - isc_result_t result; - - r.base = salt; - r.length = (unsigned int)saltlen; - - isc_buffer_init(&buf, text, textlen); - result = isc_hex_totext(&r, 2, "", &buf); - if (result == ISC_R_SUCCESS) { - text[saltlen * 2] = 0; - } else { - text[0] = 0; - } -} - /* * Check if zone has NSEC3PARAM (and thus a chain) with the right parameters. * @@ -21493,14 +21543,17 @@ dns__zone_lookup_nsec3param(dns_zone_t *zone, dns_rdata_nsec3param_t *lookup, } ZONEDB_UNLOCK(&zone->dblock, isc_rwlocktype_read); if (db == NULL) { + result = ISC_R_FAILURE; goto setparam; } result = dns_db_findnode(db, &zone->origin, false, &node); if (result != ISC_R_SUCCESS) { dns_zone_log(zone, ISC_LOG_ERROR, - "nsec3param lookup failure: %s", + "dns__zone_lookup_nsec3param:" + "dns_db_findnode -> %s", dns_result_totext(result)); + result = ISC_R_FAILURE; goto setparam; } dns_db_currentversion(db, &version); @@ -21512,7 +21565,8 @@ dns__zone_lookup_nsec3param(dns_zone_t *zone, dns_rdata_nsec3param_t *lookup, INSIST(!dns_rdataset_isassociated(&rdataset)); if (result != ISC_R_NOTFOUND) { dns_zone_log(zone, ISC_LOG_ERROR, - "nsec3param lookup failure: %s", + "dns__zone_lookup_nsec3param:" + "dns_db_findrdataset -> %s", dns_result_totext(result)); } goto setparam; @@ -21552,10 +21606,13 @@ dns__zone_lookup_nsec3param(dns_zone_t *zone, dns_rdata_nsec3param_t *lookup, break; } + if (result == ISC_R_NOMORE) { + result = ISC_R_NOTFOUND; + } + setparam: if (result != ISC_R_SUCCESS) { /* Found no match. */ - result = ISC_R_NOTFOUND; param->hash = lookup->hash; param->flags = lookup->flags; param->iterations = lookup->iterations; @@ -21563,6 +21620,10 @@ setparam: param->salt = lookup->salt; } + if (result != ISC_R_NOTFOUND && result != ISC_R_SUCCESS) { + goto failure; + } + if (param->salt_length == 0) { DE_CONST("-", param->salt); } else if (resalt || param->salt == NULL) { @@ -21595,6 +21656,7 @@ setparam: INSIST(result != ISC_R_SUCCESS); } +failure: if (dns_rdataset_isassociated(&rdataset)) { dns_rdataset_disassociate(&rdataset); } @@ -21646,6 +21708,7 @@ dns_zone_setnsec3param(dns_zone_t *zone, uint8_t hash, uint8_t flags, dns_zone_t *dummy = NULL; isc_buffer_t b; isc_event_t *e = NULL; + bool do_lookup = false; REQUIRE(DNS_ZONE_VALID(zone)); @@ -21668,7 +21731,11 @@ dns_zone_setnsec3param(dns_zone_t *zone, uint8_t hash, uint8_t flags, UNLOCK_ZONE(zone); return (ISC_R_SUCCESS); } - INSIST(param.salt != NULL); + /* + * Schedule lookup if lookup above failed (may happen if zone + * db is NULL for example). + */ + do_lookup = (param.salt == NULL) ? true : false; } e = isc_event_allocate(zone->mctx, zone, DNS_EVENT_SETNSEC3PARAM, @@ -21676,8 +21743,9 @@ dns_zone_setnsec3param(dns_zone_t *zone, uint8_t hash, uint8_t flags, npe = (struct np3event *)e; np = &npe->params; - np->replace = replace; + np->resalt = resalt; + np->lookup = do_lookup; if (hash == 0) { np->length = 0; np->nsec = true; @@ -21689,22 +21757,32 @@ dns_zone_setnsec3param(dns_zone_t *zone, uint8_t hash, uint8_t flags, param.mctx = NULL; /* nsec3 specific param set in dns__zone_lookup_nsec3param() */ isc_buffer_init(&b, nbuf, sizeof(nbuf)); - CHECK(dns_rdata_fromstruct(&nrdata, zone->rdclass, - dns_rdatatype_nsec3param, ¶m, - &b)); - dns_nsec3param_toprivate(&nrdata, &prdata, zone->privatetype, - np->data, sizeof(np->data)); - np->length = prdata.length; + + if (param.salt != NULL) { + CHECK(dns_rdata_fromstruct(&nrdata, zone->rdclass, + dns_rdatatype_nsec3param, + ¶m, &b)); + dns_nsec3param_toprivate(&nrdata, &prdata, + zone->privatetype, np->data, + sizeof(np->data)); + np->length = prdata.length; + } + + np->rdata = param; np->nsec = false; if (isc_log_wouldlog(dns_lctx, ISC_LOG_DEBUG(3))) { unsigned char salttext[255 * 2 + 1]; - salt2text(param.salt, param.salt_length, salttext, - sizeof(salttext)); + if (param.salt != NULL) { + salt2text(param.salt, param.salt_length, + salttext, sizeof(salttext)); + } dnssec_log(zone, ISC_LOG_DEBUG(3), - "setnsec3param:nsec3 %u %u %u %s", + "setnsec3param:nsec3 %u %u %u %u:%s", param.hash, param.flags, param.iterations, - salttext); + param.salt_length, + param.salt == NULL ? "unknown" + : (char *)salttext); } }