From 8d923a05a94870fcfe2c3d0d7ad3a367a600e341 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Thu, 17 May 2018 20:05:25 -0700 Subject: [PATCH 1/2] ensure that we attempt to validate glue if it's signed - incidentally fixed a bug in the dnssec system test where TTLs in the answer section rather than the additional section were being checked --- bin/tests/system/dnssec/tests.sh | 25 +++++++++++++++++++++--- lib/ns/query.c | 33 ++++++++++++++++++++++++-------- 2 files changed, 47 insertions(+), 11 deletions(-) diff --git a/bin/tests/system/dnssec/tests.sh b/bin/tests/system/dnssec/tests.sh index 8cee92237e..a5bed94bb8 100644 --- a/bin/tests/system/dnssec/tests.sh +++ b/bin/tests/system/dnssec/tests.sh @@ -18,6 +18,7 @@ n=1 rm -f dig.out.* DIGOPTS="+tcp +noadd +nosea +nostat +nocmd +dnssec -p ${PORT}" +ADDITIONALOPTS="+noall +additional +dnssec -p ${PORT}" ANSWEROPTS="+noall +answer +dnssec -p ${PORT}" DELVOPTS="-a ns1/trusted.conf -p ${PORT}" RNDCCMD="$RNDC -c $SYSTEMTESTTOP/common/rndc.conf -p ${CONTROLPORT} -s" @@ -2767,12 +2768,30 @@ n=`expr $n + 1` if [ $ret != 0 ]; then echo_i "failed"; fi status=`expr $status + $ret` -echo_i "testing TTL is capped at RRSIG expiry time for records in the additional section ($n)" +echo_i "testing TTL is capped at RRSIG expiry time for records in the additional section (NS) ($n)" ret=0 $RNDCCMD 10.53.0.4 flush 2>&1 | sed 's/^/ns4 /' | cat_i sleep 1 -$DIG $ANSWEROPTS +cd expiring.example mx @10.53.0.4 > dig.out.ns4.1.$n -$DIG $ANSWEROPTS expiring.example mx @10.53.0.4 > dig.out.ns4.2.$n +$DIG $ADDITIONALOPTS +cd expiring.example ns @10.53.0.4 > dig.out.ns4.1.$n +$DIG $ADDITIONALOPTS expiring.example ns @10.53.0.4 > dig.out.ns4.2.$n +ttls=`awk '$1 != ";;" {print $2}' dig.out.ns4.1.$n` +ttls2=`awk '$1 != ";;" {print $2}' dig.out.ns4.2.$n` +for ttl in ${ttls:-300}; do + [ ${ttl:-0} -eq 300 ] || ret=1 +done +for ttl in ${ttls2:-0}; do + [ ${ttl:-0} -le 60 ] || ret=1 +done +n=`expr $n + 1` +if [ $ret != 0 ]; then echo_i "failed"; fi +status=`expr $status + $ret` + +echo_i "testing TTL is capped at RRSIG expiry time for records in the additional section (MX) ($n)" +ret=0 +$RNDCCMD 10.53.0.4 flush 2>&1 | sed 's/^/ns4 /' | cat_i +sleep 1 +$DIG $ADDITIONALOPTS +cd expiring.example mx @10.53.0.4 > dig.out.ns4.1.$n +$DIG $ADDITIONALOPTS expiring.example mx @10.53.0.4 > dig.out.ns4.2.$n ttls=`awk '$1 != ";;" {print $2}' dig.out.ns4.1.$n` ttls2=`awk '$1 != ";;" {print $2}' dig.out.ns4.2.$n` for ttl in ${ttls:-300}; do diff --git a/lib/ns/query.c b/lib/ns/query.c index a656ad7099..27705c433c 100644 --- a/lib/ns/query.c +++ b/lib/ns/query.c @@ -1882,12 +1882,19 @@ query_addadditional(void *arg, const dns_name_t *name, dns_rdatatype_t qtype) { dns_rdataset_isassociated(sigrdataset)) dns_rdataset_disassociate(sigrdataset); } else if (result == ISC_R_SUCCESS) { + isc_boolean_t invalid = ISC_FALSE; mname = NULL; have_a = ISC_TRUE; - if (additionaltype == dns_rdatasetadditional_fromcache && - DNS_TRUST_PENDING(rdataset->trust) && - !validate(client, db, fname, rdataset, sigrdataset)) + if (additionaltype == + dns_rdatasetadditional_fromcache && + (DNS_TRUST_PENDING(rdataset->trust) || + DNS_TRUST_GLUE(rdataset->trust))) { + /* validate() may change rdataset->trust */ + invalid = ISC_TF(!!validate(client, db, fname, + rdataset, sigrdataset)); + } + if (invalid && DNS_TRUST_PENDING(rdataset->trust)) { dns_rdataset_disassociate(rdataset); if (sigrdataset != NULL && dns_rdataset_isassociated(sigrdataset)) @@ -1896,7 +1903,8 @@ query_addadditional(void *arg, const dns_name_t *name, dns_rdatatype_t qtype) { dns_rdatatype_a, &mname)) { if (mname != fname) { if (mname != NULL) { - query_releasename(client, &fname); + query_releasename(client, + &fname); fname = mname; } else need_addname = ISC_TRUE; @@ -1938,6 +1946,7 @@ query_addadditional(void *arg, const dns_name_t *name, dns_rdatatype_t qtype) { dns_rdataset_isassociated(sigrdataset)) dns_rdataset_disassociate(sigrdataset); } else if (result == ISC_R_SUCCESS) { + isc_boolean_t invalid = ISC_FALSE; mname = NULL; /* * There's an A; check whether we're filtering AAAA @@ -1948,10 +1957,17 @@ query_addadditional(void *arg, const dns_name_t *name, dns_rdatatype_t qtype) { (!WANTDNSSEC(client) || sigrdataset == NULL || !dns_rdataset_isassociated(sigrdataset))))) goto addname; - if (additionaltype == dns_rdatasetadditional_fromcache && - DNS_TRUST_PENDING(rdataset->trust) && - !validate(client, db, fname, rdataset, sigrdataset)) + if (additionaltype == + dns_rdatasetadditional_fromcache && + (DNS_TRUST_PENDING(rdataset->trust) || + DNS_TRUST_GLUE(rdataset->trust))) { + /* validate() may change rdataset->trust */ + invalid = ISC_TF(!!validate(client, db, fname, + rdataset, sigrdataset)); + } + + if (invalid && DNS_TRUST_PENDING(rdataset->trust)) { dns_rdataset_disassociate(rdataset); if (sigrdataset != NULL && dns_rdataset_isassociated(sigrdataset)) @@ -1960,7 +1976,8 @@ query_addadditional(void *arg, const dns_name_t *name, dns_rdatatype_t qtype) { dns_rdatatype_aaaa, &mname)) { if (mname != fname) { if (mname != NULL) { - query_releasename(client, &fname); + query_releasename(client, + &fname); fname = mname; } else need_addname = ISC_TRUE; From bde9c2ec396dc19f27ba885049607f7e7ca9b3b4 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Tue, 5 Jun 2018 22:49:54 -0700 Subject: [PATCH 2/2] CHANGES --- CHANGES | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGES b/CHANGES index c6ee9d6fa5..2a03c7b511 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,6 @@ +4968. [bug] If glue records are signed, attempt to validate them. + [GL #209] + 4967. [cleanup] Add "answer-cookie" to the parser, marked obsolete. 4966. [placeholder]