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 7141ec7e00fa9e9f303a10e49c57994cf4d01837..ebcce5729aabb2b0e6d5864561d131c04c186ce9 100644 (file)
--- 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 827a11c897e370294ba66f66d6675318ab947a20..e8239b14e034bfc2fe7a8642e288d7a9a3f347a4 100644 (file)
--- 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 6b6ab1b4068548c3dd6ad1ed73defc605094d692..73b01f7ae4011b986c0507a42851f016087f9621 100644 (file)
--- 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 c9d0cfb43c4b11f67648accc9521b76b3837b898..e6ca2cf4ae0941ba1d108755fc5476bfd8d7b6e9 100644 (file)
--- 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 e86608ff740ae0d63c2a2b9bc19394a2fdc3e85c..58ffe40c1ecab6ff3200965070edbfd0cee941fa 100644 (file)
--- 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;
        }