fix: dev: Do not cache signatures for rejected data

The cache has been updated so that if new data is rejected - for example, because there was already existing data at a higher trust level - then its covering RRSIG will also be rejected.

Closes #5132

Merge branch '5132-improve-cd-behavior' into 'main'

See merge request isc-projects/bind9!9999
This commit is contained in:
Evan Hunt 2025-02-20 02:12:12 +00:00
commit fc3a4d6f89
13 changed files with 133 additions and 12 deletions

View file

@ -43,3 +43,5 @@ dnskey-rrsigs-stripped. NS ns2.dnskey-rrsigs-stripped.
ns2.dnskey-rrsigs-stripped. A 10.53.0.2
ds-rrsigs-stripped. NS ns2.ds-rrsigs-stripped.
ns2.ds-rrsigs-stripped. A 10.53.0.2
inconsistent. NS ns2.inconsistent.
ns2.inconsistent. A 10.53.0.2

View file

@ -33,6 +33,7 @@ cp "../ns2/dsset-lazy-ksk." .
cp "../ns2/dsset-peer-ns-spoof." .
cp "../ns2/dsset-dnskey-rrsigs-stripped." .
cp "../ns2/dsset-ds-rrsigs-stripped." .
cp "../ns2/dsset-inconsistent." .
grep "$DEFAULT_ALGORITHM_NUMBER [12] " "../ns2/dsset-algroll." >"dsset-algroll."
cp "../ns6/dsset-optout-tld." .

View file

@ -0,0 +1,24 @@
; Copyright (C) Internet Systems Consortium, Inc. ("ISC")
;
; SPDX-License-Identifier: MPL-2.0
;
; This Source Code Form is subject to the terms of the Mozilla Public
; License, v. 2.0. If a copy of the MPL was not distributed with this
; file, you can obtain one at https://mozilla.org/MPL/2.0/.
;
; See the COPYRIGHT file distributed with this work for additional
; information regarding copyright ownership.
$TTL 300 ; 5 minutes
@ IN SOA ns2.example. . (
2010042407 ; serial
20 ; refresh (20 seconds)
20 ; retry (20 seconds)
1814400 ; expire (3 weeks)
3600 ; minimum (1 hour)
)
NS ns2.example.
NS ns3.example.
A 10.53.0.1
ns2 A 10.53.0.2
ns3 A 10.53.0.3

View file

@ -207,6 +207,11 @@ zone "too-many-iterations" {
file "too-many-iterations.db.signed";
};
zone "inconsistent" {
type primary;
file "inconsistent.db.signed";
};
zone "lazy-ksk" {
type primary;
file "lazy-ksk.db";

View file

@ -432,3 +432,14 @@ cat "$infile" "$ksk.key" "$zsk.key" >"$zonefile"
| awk '$4 == "SOA" { $7 = $7 + 1; print; next } { print }' >"$zonefile.next"
"$SIGNER" -g -o "$zone" -f "$zonefile.next" "$zonefile.next" >/dev/null 2>&1
cp "$zonefile.stripped" "$zonefile.signed"
#
# Inconsistent NS RRset between parent and child
#
zone=inconsistent
infile=inconsistent.db.in
zonefile=inconsistent.db
key1=$("$KEYGEN" -q -a "$DEFAULT_ALGORITHM" -b "$DEFAULT_BITS" -n zone -f KSK "$zone")
key2=$("$KEYGEN" -q -a "$DEFAULT_ALGORITHM" -b "$DEFAULT_BITS" -n zone "$zone")
cat "$infile" "$key1.key" "$key2.key" >"$zonefile"
"$SIGNER" -3 - -g -o "$zone" "$zonefile" >/dev/null 2>&1

View file

@ -26,6 +26,8 @@ options {
bindkeys-file "managed.conf";
dnssec-accept-expired yes;
minimal-responses no;
servfail-ttl 0;
disable-algorithms "digest-alg-unsupported.example." { ECDSAP384SHA384; };
disable-ds-digests "digest-alg-unsupported.example." { "SHA384"; "SHA-384";};
disable-ds-digests "ds-unsupported.example." { "SHA256"; "SHA-256"; "SHA384"; "SHA-384"; };

View file

@ -25,6 +25,7 @@ options {
dnssec-validation yes;
forward only;
forwarders { 10.53.0.4; };
servfail-ttl 0;
};
key rndc_key {

View file

@ -4666,5 +4666,21 @@ n=$((n + 1))
if [ "$ret" -ne 0 ]; then echo_i "failed"; fi
status=$((status + ret))
echo_i "checking validator behavior with mismatching NS ($n)"
ret=0
rndccmd 10.53.0.4 flush 2>&1 | sed 's/^/ns4 /' | cat_i
$DIG +tcp +cd -p "$PORT" -t ns inconsistent @10.53.0.4 >dig.out.ns4.test$n.1 || ret=1
grep "ANSWER: 1, AUTHORITY: 0, ADDITIONAL: 2" dig.out.ns4.test$n.1 >/dev/null || ret=1
grep "flags:.*ad.*QUERY" dig.out.ns4.test$n.1 >/dev/null && ret=1
$DIG +tcp +cd +dnssec -p "$PORT" -t ns inconsistent @10.53.0.4 >dig.out.ns4.test$n.2 || ret=1
grep "ANSWER: 1, AUTHORITY: 0, ADDITIONAL: 2" dig.out.ns4.test$n.2 >/dev/null || ret=1
grep "flags:.*ad.*QUERY" dig.out.ns4.test$n.2 >/dev/null && ret=1
$DIG +tcp +dnssec -p "$PORT" -t ns inconsistent @10.53.0.4 >dig.out.ns4.test$n.3 || ret=1
grep "ANSWER: 3, AUTHORITY: 0, ADDITIONAL: 1" dig.out.ns4.test$n.3 >/dev/null || ret=1
grep "flags:.*ad.*QUERY" dig.out.ns4.test$n.3 >/dev/null || ret=1
n=$((n + 1))
if [ "$ret" -ne 0 ]; then echo_i "failed"; fi
status=$((status + ret))
echo_i "exit status: $status"
[ $status -eq 0 ] || exit 1

View file

@ -70,6 +70,7 @@ pytestmark = pytest.mark.extra_artifacts(
"ns2/settime.out.updatecheck-kskonly.secure.zsk",
"ns2/single-nsec3.db",
"ns2/too-many-iterations.db",
"ns2/inconsistent.db",
"ns2/trusted.db",
"ns2/updatecheck-kskonly.secure.ksk.id",
"ns2/updatecheck-kskonly.secure.ksk.key",

View file

@ -100,6 +100,8 @@ struct dns_rdatasetmethods {
isc_result_t (*addglue)(dns_rdataset_t *rdataset,
dns_dbversion_t *version, dns_message_t *msg);
dns_slabheader_t *(*getheader)(const dns_rdataset_t *rdataset);
bool (*equals)(const dns_rdataset_t *rdataset1,
const dns_rdataset_t *rdataset2);
};
#define DNS_RDATASET_MAGIC ISC_MAGIC('D', 'N', 'S', 'R')
@ -676,3 +678,14 @@ dns_rdataset_getheader(const dns_rdataset_t *rdataset);
* Requires:
* \li 'rdataset' is a valid rdataset.
*/
bool
dns_rdataset_equals(const dns_rdataset_t *rdataset1,
const dns_rdataset_t *rdataset2);
/*%<
* Returns true if the rdata in the rdataset is equal.
*
* Requires:
* \li 'rdataset1' is a valid rdataset.
* \li 'rdataset2' is a valid rdataset.
*/

View file

@ -659,3 +659,18 @@ dns_rdataset_getheader(const dns_rdataset_t *rdataset) {
return NULL;
}
bool
dns_rdataset_equals(const dns_rdataset_t *rdataset1,
const dns_rdataset_t *rdataset2) {
REQUIRE(DNS_RDATASET_VALID(rdataset1));
REQUIRE(DNS_RDATASET_VALID(rdataset2));
if (rdataset1->methods->equals != NULL &&
rdataset1->methods->equals == rdataset2->methods->equals)
{
return (rdataset1->methods->equals)(rdataset1, rdataset2);
}
return false;
}

View file

@ -115,6 +115,9 @@ static void
rdataset_getownercase(const dns_rdataset_t *rdataset, dns_name_t *name);
static dns_slabheader_t *
rdataset_getheader(const dns_rdataset_t *rdataset);
static bool
rdataset_equals(const dns_rdataset_t *rdataset1,
const dns_rdataset_t *rdataset2);
/*% Note: the "const void *" are just to make qsort happy. */
static int
@ -943,6 +946,7 @@ dns_rdatasetmethods_t dns_rdataslab_rdatasetmethods = {
.setownercase = rdataset_setownercase,
.getownercase = rdataset_getownercase,
.getheader = rdataset_getheader,
.equals = rdataset_equals,
};
/* Fixed RRSet helper macros */
@ -1234,3 +1238,19 @@ rdataset_getheader(const dns_rdataset_t *rdataset) {
dns_slabheader_t *header = (dns_slabheader_t *)rdataset->slab.raw;
return header - 1;
}
static bool
rdataset_equals(const dns_rdataset_t *rdataset1,
const dns_rdataset_t *rdataset2) {
if (rdataset1->rdclass != rdataset2->rdclass ||
rdataset1->type != rdataset2->type)
{
return false;
}
dns_slabheader_t *header1 = (dns_slabheader_t *)rdataset1->slab.raw - 1;
dns_slabheader_t *header2 = (dns_slabheader_t *)rdataset2->slab.raw - 1;
return dns_rdataslab_equalx(header1, header2, rdataset1->rdclass,
rdataset2->type);
}

View file

@ -6096,33 +6096,43 @@ cache_name(fetchctx_t *fctx, dns_name_t *name, dns_message_t *message,
if (result == DNS_R_UNCHANGED) {
result = ISC_R_SUCCESS;
if (!need_validation &&
ardataset != NULL &&
NEGATIVE(ardataset))
ardataset != NULL)
{
/*
* The answer in the
* cache is better than
* the answer we found,
* and is a negative
* cache entry, so we
* the answer we found.
* If it's a negative
* cache entry, we
* must set eresult
* appropriately.
*/
if (NXDOMAIN(ardataset)) {
eresult =
DNS_R_NCACHENXDOMAIN;
} else {
} else if (NEGATIVE(ardataset))
{
eresult =
DNS_R_NCACHENXRRSET;
}
/*
* We have a negative
* response from the
* cache so don't
* attempt to add the
* RRSIG rrset.
* The cache wasn't updated
* because something was
* already there. If the
* data was the same as what
* we were trying to add,
* then sigrdataset might
* still be useful. If
* not, move on.
*/
continue;
if (sigrdataset != NULL &&
!dns_rdataset_equals(
rdataset,
addedrdataset))
{
continue;
}
}
}
if (result != ISC_R_SUCCESS) {