diff --git a/CHANGES b/CHANGES index 8de006732d..74d4e5a2d2 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,8 @@ +5006. [cleanup] Code preparing a delegation response was extracted from + query_delegation() and query_zone_delegation() into a + separate function in order to decrease code + duplication. [GL #431] + 5005. [bug] dnssec-verify, and dnssec-signzone at the verification step, failed on some validly signed zones. [GL #442] diff --git a/bin/tests/system/mirror/ns2/named.conf.in b/bin/tests/system/mirror/ns2/named.conf.in index 7141ec7e00..ebcce5729a 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 827a11c897..e8239b14e0 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 6b6ab1b406..73b01f7ae4 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 c9d0cfb43c..054c95dfeb 100644 --- a/lib/ns/include/ns/query.h +++ b/lib/ns/include/ns/query.h @@ -101,7 +101,6 @@ struct ns_query { #define NS_QUERYATTR_PARTIALANSWER 0x0004 #define NS_QUERYATTR_NAMEBUFUSED 0x0008 #define NS_QUERYATTR_RECURSING 0x0010 -#define NS_QUERYATTR_CACHEGLUEOK 0x0020 #define NS_QUERYATTR_QUERYOKVALID 0x0040 #define NS_QUERYATTR_QUERYOK 0x0080 #define NS_QUERYATTR_WANTRECURSION 0x0100 @@ -153,8 +152,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 e86608ff74..659038b532 100644 --- a/lib/ns/query.c +++ b/lib/ns/query.c @@ -112,9 +112,6 @@ do { \ /*% Recursing? */ #define RECURSING(c) (((c)->query.attributes & \ NS_QUERYATTR_RECURSING) != 0) -/*% Cache glue ok? */ -#define CACHEGLUEOK(c) (((c)->query.attributes & \ - NS_QUERYATTR_CACHEGLUEOK) != 0) /*% Want Recursion? */ #define WANTRECURSION(c) (((c)->query.attributes & \ NS_QUERYATTR_WANTRECURSION) != 0) @@ -5012,6 +5009,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 +5075,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); } @@ -7728,6 +7725,54 @@ query_notfound(query_ctx_t *qctx) { return (query_delegation(qctx)); } +/*% + * We have a delegation but recursion is not allowed, so return the delegation + * to the client. + */ +static isc_result_t +query_prepare_delegation_response(query_ctx_t *qctx) { + dns_rdataset_t **sigrdatasetp = NULL; + isc_boolean_t detach = ISC_FALSE; + + /* + * qctx->fname could be released in query_addrrset(), so save a copy of + * it here in case we need it. + */ + dns_fixedname_init(&qctx->dsname); + dns_name_copy(qctx->fname, dns_fixedname_name(&qctx->dsname), NULL); + + /* + * This is the best answer. + */ + qctx->client->query.isreferral = ISC_TRUE; + + if (!dns_db_iscache(qctx->db) && qctx->client->query.gluedb == NULL) { + dns_db_attach(qctx->db, &qctx->client->query.gluedb); + detach = ISC_TRUE; + } + + /* + * We must ensure NOADDITIONAL is off, because the generation of + * additional data is required in delegations. + */ + qctx->client->query.attributes &= ~NS_QUERYATTR_NOADDITIONAL; + if (WANTDNSSEC(qctx->client) && qctx->sigrdataset != NULL) { + sigrdatasetp = &qctx->sigrdataset; + } + query_addrrset(qctx->client, &qctx->fname, &qctx->rdataset, + sigrdatasetp, qctx->dbuf, DNS_SECTION_AUTHORITY); + if (detach) { + dns_db_detach(&qctx->client->query.gluedb); + } + + /* + * Add a DS if needed. + */ + query_addds(qctx); + + return (query_done(qctx)); +} + /*% * Handle a delegation response from an authoritative lookup. This * may trigger additional lookups, e.g. from the cache database to @@ -7737,8 +7782,6 @@ query_notfound(query_ctx_t *qctx) { static isc_result_t query_zone_delegation(query_ctx_t *qctx) { isc_result_t result; - dns_rdataset_t **sigrdatasetp = NULL; - isc_boolean_t detach = ISC_FALSE; /* * If the query type is DS, look to see if we are @@ -7803,8 +7846,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); @@ -7815,57 +7858,7 @@ query_zone_delegation(query_ctx_t *qctx) { return (query_lookup(qctx)); } - /* - * qctx->fname could be released in - * query_addrrset(), so save a copy of it - * here in case we need it - */ - dns_fixedname_init(&qctx->dsname); - dns_name_copy(qctx->fname, dns_fixedname_name(&qctx->dsname), NULL); - - /* - * If we don't have a cache, this is the best - * answer. - * - * If the client is making a nonrecursive - * query we always give out the authoritative - * delegation. This way even if we get - * junk in our cache, we won't fail in our - * role as the delegating authority if another - * nameserver asks us about a delegated - * subzone. - * - * We enable the retrieval of glue for this - * database by setting client->query.gluedb. - */ - if (qctx->db != NULL && qctx->client->query.gluedb == NULL) { - dns_db_attach(qctx->db, &qctx->client->query.gluedb); - detach = ISC_TRUE; - } - qctx->client->query.isreferral = ISC_TRUE; - /* - * We must ensure NOADDITIONAL is off, - * because the generation of - * additional data is required in - * delegations. - */ - qctx->client->query.attributes &= - ~NS_QUERYATTR_NOADDITIONAL; - if (WANTDNSSEC(qctx->client) && qctx->sigrdataset != NULL) - sigrdatasetp = &qctx->sigrdataset; - query_addrrset(qctx->client, &qctx->fname, - &qctx->rdataset, sigrdatasetp, - qctx->dbuf, DNS_SECTION_AUTHORITY); - if (detach) { - dns_db_detach(&qctx->client->query.gluedb); - } - - /* - * Add a DS if needed - */ - query_addds(qctx); - - return (query_done(qctx)); + return (query_prepare_delegation_response(qctx)); } /*% @@ -7879,8 +7872,6 @@ query_zone_delegation(query_ctx_t *qctx) { static isc_result_t query_delegation(query_ctx_t *qctx) { isc_result_t result; - dns_rdataset_t **sigrdatasetp = NULL; - isc_boolean_t detach = ISC_FALSE; qctx->authoritative = ISC_FALSE; @@ -7923,16 +7914,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)) { @@ -7993,54 +7982,7 @@ query_delegation(query_ctx_t *qctx) { return (query_done(qctx)); } - /* - * We have a delegation but recursion is not - * allowed, so return the delegation to the client. - * - * qctx->fname could be released in - * query_addrrset(), so save a copy of it - * here in case we need it - */ - dns_fixedname_init(&qctx->dsname); - dns_name_copy(qctx->fname, dns_fixedname_name(&qctx->dsname), NULL); - - /* - * This is the best answer. - */ - 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); - detach = ISC_TRUE; - } - - /* - * We must ensure NOADDITIONAL is off, - * because the generation of - * additional data is required in - * delegations. - */ - qctx->client->query.attributes &= ~NS_QUERYATTR_NOADDITIONAL; - if (WANTDNSSEC(qctx->client) && qctx->sigrdataset != NULL) { - sigrdatasetp = &qctx->sigrdataset; - } - query_addrrset(qctx->client, &qctx->fname, - &qctx->rdataset, sigrdatasetp, - qctx->dbuf, DNS_SECTION_AUTHORITY); - qctx->client->query.attributes &= ~NS_QUERYATTR_CACHEGLUEOK; - if (detach) { - dns_db_detach(&qctx->client->query.gluedb); - } - - /* - * Add a DS if needed - */ - query_addds(qctx); - - return (query_done(qctx)); + return (query_prepare_delegation_response(qctx)); } /*%