From: Michał Kępień Date: Wed, 8 Aug 2018 05:56:29 +0000 (+0200) Subject: Restore zone database and zone node if cache search results are to be ignored X-Git-Tag: v9.13.3~75^2~3 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=b6c77202cba8570677925026365b4f332c093603;p=thirdparty%2Fbind9.git Restore zone database and zone node if cache search results are to be ignored When query processing hits a delegation from a locally configured zone, an attempt may be made to look for a better answer in the cache. In such a case, the zone-sourced delegation data is set aside and the lookup is retried using the cache database. When that lookup is completed, a decision is made whether the answer found in the cache is better than the answer found in the zone. Currently, if the zone-sourced answer turns out to be better than the one found in the cache: - qctx->zdb is not restored into qctx->db, - qctx->node, holding the zone database node found, is not even saved. Thus, in such a case both qctx->db and qctx->node will point at cache data. This is not an issue for BIND versions which do not support mirror zones because in these versions non-recursive queries always cause the zone-sourced delegation to be returned and thus the non-recursive part of query_delegation() is never reached if the delegation is coming from a zone. With mirror zones, however, non-recursive queries may cause cache lookups even after a zone delegation is found. Leaving qctx->db assigned to the cache database when query_delegation() determines that the zone-sourced delegation is the best answer to the client's query prevents DS records from being added to delegations coming from mirror zones. Fix this issue by keeping the zone database and zone node in qctx while the cache is searched for an answer and then restoring them into qctx->db and qctx->node, respectively, if the zone-sourced delegation turns out to be the best answer. Since this change means that qctx->zdb cannot be used as the glue database any more as it will be reset to NULL by RESTORE(), ensure that qctx->db is not a cache database before attaching it to qctx->client->query.gluedb. Furthermore, current code contains a conditional statement which prevents a mirror zone from being used as a source of glue records. Said statement was added to prevent assertion failures caused by attempting to use a zone database's glue cache for finding glue for an NS RRset coming from a cache database. However, that check is overly strict since it completely prevents glue from being added to delegations coming from mirror zones. With the changes described above in place, the scenario this check was preventing can no longer happen, so remove the aforementioned check. If qctx->zdb is not NULL, qctx->zfname will also not be NULL; qctx->zsigrdataset may be NULL in such a case, but query_putrdataset() handles pointers to NULL pointers gracefully. Remove redundant conditional expressions to make the cleanup code in query_freedata() match the corresponding sequences of SAVE() / RESTORE() macros more closely. --- diff --git a/bin/tests/system/mirror/ns2/named.conf.in b/bin/tests/system/mirror/ns2/named.conf.in index 7141ec7e00f..ebcce5729aa 100644 --- a/bin/tests/system/mirror/ns2/named.conf.in +++ b/bin/tests/system/mirror/ns2/named.conf.in @@ -36,7 +36,7 @@ zone "example" { zone "sub.example" { type master; - file "sub.example.db.in"; + file "sub.example.db.signed"; }; zone "initially-unavailable" { diff --git a/bin/tests/system/mirror/ns2/sign.sh b/bin/tests/system/mirror/ns2/sign.sh index 827a11c897e..e8239b14e03 100644 --- a/bin/tests/system/mirror/ns2/sign.sh +++ b/bin/tests/system/mirror/ns2/sign.sh @@ -14,7 +14,7 @@ SYSTEMTESTTOP=../.. keys_to_trust="" -for zonename in example initially-unavailable; do +for zonename in sub.example example initially-unavailable; do zone=$zonename infile=$zonename.db.in zonefile=$zonename.db @@ -24,7 +24,7 @@ for zonename in example initially-unavailable; do cat $infile $keyname1.key $keyname2.key > $zonefile - $SIGNER -P -o $zone $zonefile > /dev/null + $SIGNER -P -g -o $zone $zonefile > /dev/null done # Only add the key for "initially-unavailable" to the list of keys trusted by diff --git a/bin/tests/system/mirror/tests.sh b/bin/tests/system/mirror/tests.sh index 6b6ab1b4068..73b01f7ae40 100644 --- a/bin/tests/system/mirror/tests.sh +++ b/bin/tests/system/mirror/tests.sh @@ -204,6 +204,21 @@ grep "${UPDATED_SERIAL_GOOD}.*; serial" dig.out.ns3.test$n > /dev/null || ret=1 if [ $ret != 0 ]; then echo_i "failed"; fi status=`expr $status + $ret` +n=`expr $n + 1` +echo_i "checking delegations sourced from a mirror zone ($n)" +ret=0 +$DIG $DIGOPTS @10.53.0.3 foo.example A +norec > dig.out.ns3.test$n 2>&1 || ret=1 +# Check response code and flags in the answer. +grep "NOERROR" dig.out.ns3.test$n > /dev/null || ret=1 +grep "flags:.* ad" dig.out.ns3.test$n > /dev/null && ret=1 +# Check that a delegation containing a DS RRset and glue is present. +grep "ANSWER: 0" dig.out.ns3.test$n > /dev/null || ret=1 +grep "example.*IN.*NS" dig.out.ns3.test$n > /dev/null || ret=1 +grep "example.*IN.*DS" dig.out.ns3.test$n > /dev/null || ret=1 +grep "ns2.example.*A.*10.53.0.2" dig.out.ns3.test$n > /dev/null || ret=1 +if [ $ret != 0 ]; then echo_i "failed"; fi +status=`expr $status + $ret` + n=`expr $n + 1` echo_i "checking that resolution involving a mirror zone works as expected ($n)" ret=0 @@ -238,14 +253,16 @@ ret=0 $DIG $DIGOPTS @10.53.0.3 sub.example. NS > dig.out.ns3.test$n.1 2>&1 || ret=1 # Ensure the child-side NS RRset is returned. grep "NOERROR" dig.out.ns3.test$n.1 > /dev/null || ret=1 -grep "ANSWER: 1" dig.out.ns3.test$n.1 > /dev/null || ret=1 +grep "ANSWER: 2" dig.out.ns3.test$n.1 > /dev/null || ret=1 grep "sub.example.*IN.*NS" dig.out.ns3.test$n.1 > /dev/null || ret=1 # Issue a non-recursive query for something below the cached zone cut. $DIG $DIGOPTS @10.53.0.3 +norec foo.sub.example. A > dig.out.ns3.test$n.2 2>&1 || ret=1 -# Ensure the cached NS RRset is returned in a delegation. +# Ensure the cached NS RRset is returned in a delegation, along with the +# parent-side DS RRset. grep "NOERROR" dig.out.ns3.test$n.2 > /dev/null || ret=1 grep "ANSWER: 0" dig.out.ns3.test$n.2 > /dev/null || ret=1 grep "sub.example.*IN.*NS" dig.out.ns3.test$n.2 > /dev/null || ret=1 +grep "sub.example.*IN.*DS" dig.out.ns3.test$n.2 > /dev/null || ret=1 if [ $ret != 0 ]; then echo_i "failed"; fi status=`expr $status + $ret` diff --git a/lib/ns/include/ns/query.h b/lib/ns/include/ns/query.h index c9d0cfb43c4..e6ca2cf4ae0 100644 --- a/lib/ns/include/ns/query.h +++ b/lib/ns/include/ns/query.h @@ -153,8 +153,9 @@ typedef struct query_ctx { dns_dbnode_t *node; /* DB node */ dns_db_t *zdb; /* zone DB values, saved */ - dns_name_t *zfname; /* while searching cache */ - dns_dbversion_t *zversion; /* for a better answer */ + dns_dbnode_t *znode; /* while searching cache */ + dns_name_t *zfname; /* for a better answer */ + dns_dbversion_t *zversion; dns_rdataset_t *zrdataset; dns_rdataset_t *zsigrdataset; diff --git a/lib/ns/query.c b/lib/ns/query.c index e86608ff740..58ffe40c1ec 100644 --- a/lib/ns/query.c +++ b/lib/ns/query.c @@ -5012,6 +5012,7 @@ qctx_init(ns_client_t *client, dns_fetchevent_t *event, qctx->zsigrdataset = NULL; qctx->zversion = NULL; qctx->node = NULL; + qctx->znode = NULL; qctx->db = NULL; qctx->zdb = NULL; qctx->version = NULL; @@ -5077,11 +5078,10 @@ qctx_freedata(query_ctx_t *qctx) { } if (qctx->zdb != NULL) { + query_putrdataset(qctx->client, &qctx->zsigrdataset); query_putrdataset(qctx->client, &qctx->zrdataset); - if (qctx->zsigrdataset != NULL) - query_putrdataset(qctx->client, &qctx->zsigrdataset); - if (qctx->zfname != NULL) - query_releasename(qctx->client, &qctx->zfname); + query_releasename(qctx->client, &qctx->zfname); + dns_db_detachnode(qctx->zdb, &qctx->znode); dns_db_detach(&qctx->zdb); } @@ -7803,8 +7803,8 @@ query_zone_delegation(query_ctx_t *qctx) { * we'll restore these values there. */ query_keepname(qctx->client, qctx->fname, qctx->dbuf); - dns_db_detachnode(qctx->db, &qctx->node); SAVE(qctx->zdb, qctx->db); + SAVE(qctx->znode, qctx->node); SAVE(qctx->zfname, qctx->fname); SAVE(qctx->zversion, qctx->version); SAVE(qctx->zrdataset, qctx->rdataset); @@ -7923,16 +7923,14 @@ query_delegation(query_ctx_t *qctx) { &qctx->sigrdataset); qctx->version = NULL; + dns_db_detachnode(qctx->db, &qctx->node); + dns_db_detach(&qctx->db); + RESTORE(qctx->db, qctx->zdb); + RESTORE(qctx->node, qctx->znode); RESTORE(qctx->fname, qctx->zfname); RESTORE(qctx->version, qctx->zversion); RESTORE(qctx->rdataset, qctx->zrdataset); RESTORE(qctx->sigrdataset, qctx->zsigrdataset); - - /* - * We don't clean up zdb here because we - * may still need it. It will get cleaned - * up by the main cleanup code in query_done(). - */ } if (RECURSIONOK(qctx->client)) { @@ -8010,10 +8008,8 @@ query_delegation(query_ctx_t *qctx) { qctx->client->query.attributes |= NS_QUERYATTR_CACHEGLUEOK; qctx->client->query.isreferral = ISC_TRUE; - if (qctx->zdb != NULL && qctx->client->query.gluedb == NULL && - !(qctx->zone != NULL && dns_zone_ismirror(qctx->zone))) - { - dns_db_attach(qctx->zdb, &qctx->client->query.gluedb); + if (!dns_db_iscache(qctx->db) && qctx->client->query.gluedb == NULL) { + dns_db_attach(qctx->db, &qctx->client->query.gluedb); detach = ISC_TRUE; }