2963. [security] The allow-query acl was being applied instead of the

                        allow-query-cache acl to cache lookups. [RT #22114]

diff --git a/CHANGES b/CHANGES
index a300cec32f64375bbf67904a93356dc10fad51e2..40635f69f7ebb61bb84996d15d872ea4331fbe66 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -1,3 +1,6 @@
+2963.  [security]      The allow-query acl was being applied instead of the
+                       allow-query-cache acl to cache lookups. [RT #22114]
+
 2962.  [port]          win32: add more dependencies to BINDBuild.dsw.
                        [RT #22062]
 

diff --git a/bin/named/client.c b/bin/named/client.c
index 02efe4e37a49cec3115f7f213d941a532802f148..940c535d7ae4f9389e4290069f10b26885ec180d 100644 (file)
--- a/bin/named/client.c
+++ b/bin/named/client.c
@@ -15,7 +15,7 @@
  * PERFORMANCE OF THIS SOFTWARE.
  */
 
-/* $Id: client.c,v 1.268 2010/09/13 23:46:58 tbox Exp $ */
+/* $Id: client.c,v 1.269 2010/09/24 05:09:02 marka Exp $ */
 
 #include <config.h>
 
@@ -1865,13 +1865,13 @@ client_request(isc_task_t *task, isc_event_t *event) {
                                     client->view->recursionacl,
                                     ISC_TRUE) == ISC_R_SUCCESS &&
            ns_client_checkaclsilent(client, NULL,
-                                    client->view->queryacl,
+                                    client->view->cacheacl,
                                     ISC_TRUE) == ISC_R_SUCCESS &&
            ns_client_checkaclsilent(client, &client->destaddr,
                                     client->view->recursiononacl,
                                     ISC_TRUE) == ISC_R_SUCCESS &&
            ns_client_checkaclsilent(client, &client->destaddr,
-                                    client->view->queryonacl,
+                                    client->view->cacheonacl,
                                     ISC_TRUE) == ISC_R_SUCCESS)
                ra = ISC_TRUE;
 

diff --git a/bin/named/include/named/query.h b/bin/named/include/named/query.h
index 500b57714e4fde29014cdd83f1a0bbcd04c16e1d..5a9bd24cfe22bd9679ac14d9ea004104488a1371 100644 (file)
--- a/bin/named/include/named/query.h
+++ b/bin/named/include/named/query.h
@@ -15,7 +15,7 @@
  * PERFORMANCE OF THIS SOFTWARE.
  */
 
-/* $Id: query.h,v 1.40 2007/06/19 23:46:59 tbox Exp $ */
+/* $Id: query.h,v 1.41 2010/09/24 05:09:02 marka Exp $ */
 
 #ifndef NAMED_QUERY_H
 #define NAMED_QUERY_H 1
@@ -71,6 +71,8 @@ struct ns_query {
 #define NS_QUERYATTR_SECURE            0x0200
 #define NS_QUERYATTR_NOAUTHORITY       0x0400
 #define NS_QUERYATTR_NOADDITIONAL      0x0800
+#define NS_QUERYATTR_CACHEACLOKVALID   0x1000
+#define NS_QUERYATTR_CACHEACLOK                0x2000
 
 isc_result_t
 ns_query_init(ns_client_t *client);

diff --git a/bin/named/query.c b/bin/named/query.c
index da27c58556d8997f41c3f92c4ec62bd6fe074517..eb847edb2b57a831ff275b8bc14cdad7833b01e8 100644 (file)
--- a/bin/named/query.c
+++ b/bin/named/query.c
@@ -15,7 +15,7 @@
  * PERFORMANCE OF THIS SOFTWARE.
  */
 
-/* $Id: query.c,v 1.345 2010/09/15 12:07:55 marka Exp $ */
+/* $Id: query.c,v 1.346 2010/09/24 05:09:02 marka Exp $ */
 
 /*! \file */
 
@@ -820,17 +820,15 @@ query_getcachedb(ns_client_t *client, dns_name_t *name, dns_rdatatype_t qtype,
                return (DNS_R_REFUSED);
        dns_db_attach(client->view->cachedb, &db);
 
-       if ((client->query.attributes &
-            NS_QUERYATTR_QUERYOKVALID) != 0) {
+       if ((client->query.attributes & NS_QUERYATTR_CACHEACLOKVALID) != 0) {
                /*
-                * We've evaluated the view's queryacl already.  If
-                * NS_QUERYATTR_QUERYOK is set, then the client is
+                * We've evaluated the view's cacheacl already.  If
+                * NS_QUERYATTR_CACHEACLOK is set, then the client is
                 * allowed to make queries, otherwise the query should
                 * be refused.
                 */
                check_acl = ISC_FALSE;
-               if ((client->query.attributes &
-                    NS_QUERYATTR_QUERYOK) == 0)
+               if ((client->query.attributes & NS_QUERYATTR_CACHEACLOK) == 0)
                        goto refuse;
        } else {
                /*
@@ -844,16 +842,15 @@ query_getcachedb(ns_client_t *client, dns_name_t *name, dns_rdatatype_t qtype,
                char msg[NS_CLIENT_ACLMSGSIZE("query (cache)")];
 
                result = ns_client_checkaclsilent(client, NULL,
-                                                 client->view->queryacl,
+                                                 client->view->cacheacl,
                                                  ISC_TRUE);
                if (result == ISC_R_SUCCESS) {
                        /*
-                        * We were allowed by the default
-                        * "allow-query" ACL.  Remember this so we
-                        * don't have to check again.
+                        * We were allowed by the "allow-query-cache" ACL.
+                        * Remember this so we don't have to check again.
                         */
                        client->query.attributes |=
-                               NS_QUERYATTR_QUERYOK;
+                               NS_QUERYATTR_CACHEACLOK;
                        if (log && isc_log_wouldlog(ns_g_lctx,
                                                     ISC_LOG_DEBUG(3)))
                        {
@@ -876,9 +873,9 @@ query_getcachedb(ns_client_t *client, dns_name_t *name, dns_rdatatype_t qtype,
                }
                /*
                 * We've now evaluated the view's query ACL, and
-                * the NS_QUERYATTR_QUERYOK attribute is now valid.
+                * the NS_QUERYATTR_CACHEACLOKVALID attribute is now valid.
                 */
-               client->query.attributes |= NS_QUERYATTR_QUERYOKVALID;
+               client->query.attributes |= NS_QUERYATTR_CACHEACLOKVALID;
 
                if (result != ISC_R_SUCCESS)
                        goto refuse;

diff --git a/bin/named/server.c b/bin/named/server.c
index 04694b74377f9bd74c82fdcaea4e414ca202b6e3..42967757adc7f64343526d780f77becda8d3d9a9 100644 (file)
--- a/bin/named/server.c
+++ b/bin/named/server.c
@@ -15,7 +15,7 @@
  * PERFORMANCE OF THIS SOFTWARE.
  */
 
-/* $Id: server.c,v 1.583 2010/08/24 01:00:31 marka Exp $ */
+/* $Id: server.c,v 1.584 2010/09/24 05:09:02 marka Exp $ */
 
 /*! \file */
 
@@ -1444,6 +1444,9 @@ configure_view(dns_view_t *view, cfg_parser_t* parser,
                dns_acache_setcachesize(view->acache, max_acache_size);
        }
 
+       CHECK(configure_view_acl(NULL, ns_g_config, "allow-query", NULL, actx,
+                                ns_g_mctx, &view->queryacl));
+       
        /*
         * Configure the zones.
         */
@@ -2051,16 +2054,14 @@ configure_view(dns_view_t *view, cfg_parser_t* parser,
         * "allow-recursion", and "allow-recursion-on" acls if
         * configured in named.conf.
         */
-       if (view->queryacl == NULL)
-               CHECK(configure_view_acl(vconfig, config,
-                                        "allow-query-cache", NULL,
-                                        actx, ns_g_mctx, &view->queryacl));
+       CHECK(configure_view_acl(vconfig, config, "allow-query-cache", NULL,
+                                actx, ns_g_mctx, &view->cacheacl));
        CHECK(configure_view_acl(vconfig, config, "allow-query-cache-on", NULL,
-                                actx, ns_g_mctx, &view->queryonacl));
-       if (view->queryonacl == NULL)
+                                actx, ns_g_mctx, &view->cacheonacl));
+       if (view->cacheonacl == NULL)
                CHECK(configure_view_acl(NULL, ns_g_config,
                                         "allow-query-cache-on", NULL, actx,
-                                        ns_g_mctx, &view->queryonacl));
+                                        ns_g_mctx, &view->cacheonacl));
        if (strcmp(view->name, "_bind") != 0) {
                CHECK(configure_view_acl(vconfig, config, "allow-recursion",
                                         NULL, actx, ns_g_mctx,
@@ -2076,14 +2077,20 @@ configure_view(dns_view_t *view, cfg_parser_t* parser,
         * "allow-recursion" inherits from "allow-query-cache" if set,
         * otherwise from "allow-query" if set.
         */
-       if (view->queryacl == NULL && view->recursionacl != NULL)
-               dns_acl_attach(view->recursionacl, &view->queryacl);
-       if (view->queryacl == NULL && view->recursion)
+       if (view->cacheacl == NULL && view->recursionacl != NULL)
+               dns_acl_attach(view->recursionacl, &view->cacheacl);
+        /*
+         * XXXEACH: This call to configure_view_acl() is redundant.  We
+         * are leaving it as it is because we are making a minimal change
+         * for a patch release.  In the future this should be changed to
+         * dns_acl_attach(view->queryacl, &view->cacheacl).
+         */
+       if (view->cacheacl == NULL && view->recursion)
                CHECK(configure_view_acl(vconfig, config, "allow-query", NULL,
-                                        actx, ns_g_mctx, &view->queryacl));
+                                        actx, ns_g_mctx, &view->cacheacl));
        if (view->recursion &&
-           view->recursionacl == NULL && view->queryacl != NULL)
-               dns_acl_attach(view->queryacl, &view->recursionacl);
+           view->recursionacl == NULL && view->cacheacl != NULL)
+               dns_acl_attach(view->cacheacl, &view->recursionacl);
 
        /*
         * Set default "allow-recursion", "allow-recursion-on" and
@@ -2099,15 +2106,14 @@ configure_view(dns_view_t *view, cfg_parser_t* parser,
                                         "allow-recursion-on", NULL,
                                         actx, ns_g_mctx,
                                         &view->recursiononacl));
-       if (view->queryacl == NULL) {
+       if (view->cacheacl == NULL) {
                if (view->recursion)
                        CHECK(configure_view_acl(NULL, ns_g_config,
                                                 "allow-query-cache", NULL,
                                                 actx, ns_g_mctx,
-                                                &view->queryacl));
-               else if (view->new_zone_file == NULL) {
-                       CHECK(dns_acl_none(ns_g_mctx, &view->queryacl));
-               }
+                                                &view->cacheacl));
+                else
+                        CHECK(dns_acl_none(mctx, &view->cacheacl));
        }
 
        /*

diff --git a/bin/tests/system/addzone/ns2/named2.conf b/bin/tests/system/addzone/ns2/named2.conf
index bb4a89e717b1edcd077513c63f2d07e682dfb689..0ae37d93898eb71e487b0a1dc461ce3e7c335683 100644 (file)
--- a/bin/tests/system/addzone/ns2/named2.conf
+++ b/bin/tests/system/addzone/ns2/named2.conf
@@ -14,7 +14,7 @@
  * PERFORMANCE OF THIS SOFTWARE.
  */
 
-/* $Id: named2.conf,v 1.2 2010/08/11 18:14:19 each Exp $ */
+/* $Id: named2.conf,v 1.3 2010/09/24 05:09:03 marka Exp $ */
 
 controls { /* empty */ };
 
@@ -31,6 +31,7 @@ options {
 view internal {
        match-clients { 10.53.0.2; };
        allow-new-zones no;
+        recursion yes;
 
        zone "." {
                type hint;

diff --git a/bin/tests/system/addzone/tests.sh b/bin/tests/system/addzone/tests.sh
index 3c98812f7889a69d5aeea19fafae00582d2d219a..ccf9d906a9bc84a6e59c95ff74b15f281c1eb74f 100644 (file)
--- a/bin/tests/system/addzone/tests.sh
+++ b/bin/tests/system/addzone/tests.sh
@@ -14,7 +14,7 @@
 # OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 # PERFORMANCE OF THIS SOFTWARE.
 
-# $Id: tests.sh,v 1.3 2010/09/15 03:32:34 marka Exp $
+# $Id: tests.sh,v 1.4 2010/09/24 05:09:03 marka Exp $
 
 SYSTEMTESTTOP=..
 . $SYSTEMTESTTOP/conf.sh
@@ -54,11 +54,11 @@ status=`expr $status + $ret`
 echo "I:adding new zone with missing master file ($n)"
 ret=0
 $DIG $DIGOPTS +all @10.53.0.2 a.missing.example a > dig.out.ns2.pre.$n || ret=1
-grep "status: NOERROR" dig.out.ns2.pre.$n > /dev/null || ret=1
+grep "status: REFUSED" dig.out.ns2.pre.$n > /dev/null || ret=1
 $RNDC -c ../common/rndc.conf -s 10.53.0.2 -p 9953 addzone 'missing.example { type master; file "missing.db"; };' 2> rndc.out.ns2.$n
 grep "file not found" rndc.out.ns2.$n > /dev/null || ret=1
 $DIG $DIGOPTS +all @10.53.0.2 a.missing.example a > dig.out.ns2.post.$n || ret=1
-grep "status: NOERROR" dig.out.ns2.post.$n > /dev/null || ret=1
+grep "status: REFUSED" dig.out.ns2.post.$n > /dev/null || ret=1
 $PERL ../digcomp.pl dig.out.ns2.pre.$n dig.out.ns2.post.$n || ret=1
 n=`expr $n + 1`
 if [ $ret != 0 ]; then echo "I:failed"; fi
@@ -68,7 +68,7 @@ echo "I:deleting previously added zone ($n)"
 ret=0
 $RNDC -c ../common/rndc.conf -s 10.53.0.2 -p 9953 delzone previous.example 2>&1 | sed 's/^/I:ns2 /'
 $DIG $DIGOPTS @10.53.0.2 a.previous.example a > dig.out.ns2.$n
-grep 'status: NOERROR' dig.out.ns2.$n > /dev/null || ret=1
+grep 'status: REFUSED' dig.out.ns2.$n > /dev/null || ret=1
 grep '^a.previous.example' dig.out.ns2.$n > /dev/null && ret=1
 n=`expr $n + 1`
 if [ $ret != 0 ]; then echo "I:failed"; fi
@@ -78,7 +78,7 @@ echo "I:deleting newly added zone ($n)"
 ret=0
 $RNDC -c ../common/rndc.conf -s 10.53.0.2 -p 9953 delzone added.example 2>&1 | sed 's/^/I:ns2 /'
 $DIG $DIGOPTS @10.53.0.2 a.added.example a > dig.out.ns2.$n
-grep 'status: NOERROR' dig.out.ns2.$n > /dev/null || ret=1
+grep 'status: REFUSED' dig.out.ns2.$n > /dev/null || ret=1
 grep '^a.added.example' dig.out.ns2.$n > /dev/null && ret=1
 n=`expr $n + 1`
 if [ $ret != 0 ]; then echo "I:failed"; fi
@@ -102,11 +102,21 @@ $RNDC -c ../common/rndc.conf -s 10.53.0.2 -p 9953 reconfig 2>&1 | sed 's/^/I:ns2
 sleep 5
 
 echo "I:adding new zone to external view ($n)"
+# NOTE: The internal view has "recursion yes" set, and so queries for
+# nonexistent zones should return NOERROR.  The external view is
+# "recursion no", so queries for nonexistent zones should return
+# REFUSED.  This behavior should be the same regardless of whether
+# the zone does not exist because a) it has not yet been loaded, b)
+# it failed to load, or c) it has been deleted.
 ret=0
+$DIG +norec $DIGOPTS @10.53.0.2 -b 10.53.0.2 a.added.example a > dig.out.ns2.intpre.$n || ret=1
+grep 'status: NOERROR' dig.out.ns2.intpre.$n > /dev/null || ret=1
+$DIG +norec $DIGOPTS @10.53.0.4 -b 10.53.0.4 a.added.example a > dig.out.ns2.extpre.$n || ret=1
+grep 'status: REFUSED' dig.out.ns2.extpre.$n > /dev/null || ret=1
 $RNDC -c ../common/rndc.conf -s 10.53.0.2 -p 9953 addzone 'added.example in external { type master; file "added.db"; };' 2>&1 | sed 's/^/I:ns2 /'
-$DIG $DIGOPTS @10.53.0.2 -b 10.53.0.2 a.added.example a > dig.out.ns2.int.$n || ret=1
-$DIG $DIGOPTS @10.53.0.4 -b 10.53.0.4 a.added.example a > dig.out.ns2.ext.$n || ret=1
-grep 'status: REFUSED' dig.out.ns2.int.$n > /dev/null || ret=1
+$DIG +norec $DIGOPTS @10.53.0.2 -b 10.53.0.2 a.added.example a > dig.out.ns2.int.$n || ret=1
+grep 'status: NOERROR' dig.out.ns2.int.$n > /dev/null || ret=1
+$DIG +norec $DIGOPTS @10.53.0.4 -b 10.53.0.4 a.added.example a > dig.out.ns2.ext.$n || ret=1
 grep 'status: NOERROR' dig.out.ns2.ext.$n > /dev/null || ret=1
 grep '^a.added.example' dig.out.ns2.ext.$n > /dev/null || ret=1
 n=`expr $n + 1`
@@ -117,7 +127,7 @@ echo "I:deleting newly added zone ($n)"
 ret=0
 $RNDC -c ../common/rndc.conf -s 10.53.0.2 -p 9953 delzone 'added.example in external' 2>&1 | sed 's/^/I:ns2 /'
 $DIG $DIGOPTS @10.53.0.4 -b 10.53.0.4 a.added.example a > dig.out.ns2.$n || ret=1
-grep 'status: NOERROR' dig.out.ns2.$n > /dev/null || ret=1
+grep 'status: REFUSED' dig.out.ns2.$n > /dev/null || ret=1
 grep '^a.added.example' dig.out.ns2.$n > /dev/null && ret=1
 n=`expr $n + 1`
 if [ $ret != 0 ]; then echo "I:failed"; fi
@@ -125,10 +135,14 @@ status=`expr $status + $ret`
 
 echo "I:attempting to add zone to internal view ($n)"
 ret=0
+$DIG +norec $DIGOPTS @10.53.0.2 -b 10.53.0.2 a.added.example a > dig.out.ns2.pre.$n || ret=1
+grep 'status: NOERROR' dig.out.ns2.pre.$n > /dev/null || ret=1
 $RNDC -c ../common/rndc.conf -s 10.53.0.2 -p 9953 addzone 'added.example in internal { type master; file "added.db"; };' 2> rndc.out.ns2.$n
 grep "permission denied" rndc.out.ns2.$n > /dev/null || ret=1
-$DIG $DIGOPTS @10.53.0.2 -b 10.53.0.2 a.added.example a > dig.out.ns2.$n || ret=1
-grep 'status: REFUSED' dig.out.ns2.$n > /dev/null || ret=1
+$DIG $DIGOPTS @10.53.0.2 -b 10.53.0.2 a.added.example a > dig.out.ns2.int.$n || ret=1
+grep 'status: NOERROR' dig.out.ns2.int.$n > /dev/null || ret=1
+$DIG $DIGOPTS @10.53.0.4 -b 10.53.0.4 a.added.example a > dig.out.ns2.ext.$n || ret=1
+grep 'status: REFUSED' dig.out.ns2.ext.$n > /dev/null || ret=1
 n=`expr $n + 1`
 if [ $ret != 0 ]; then echo "I:failed"; fi
 status=`expr $status + $ret`

diff --git a/lib/dns/include/dns/view.h b/lib/dns/include/dns/view.h
index 4adfbd2ad4ff1460e6ca30af6d02f4eb992277fb..6835910f3faffa5230f366cc5ea20c0ae6b55a90 100644 (file)
--- a/lib/dns/include/dns/view.h
+++ b/lib/dns/include/dns/view.h
@@ -15,7 +15,7 @@
  * PERFORMANCE OF THIS SOFTWARE.
  */
 
-/* $Id: view.h,v 1.126 2010/08/11 18:14:19 each Exp $ */
+/* $Id: view.h,v 1.127 2010/09/24 05:09:03 marka Exp $ */
 
 #ifndef DNS_VIEW_H
 #define DNS_VIEW_H 1
@@ -125,6 +125,8 @@ struct dns_view {
        isc_boolean_t                   enablevalidation;
        isc_boolean_t                   acceptexpired;
        dns_transfer_format_t           transfer_format;
+       dns_acl_t *                     cacheacl;
+       dns_acl_t *                     cacheonacl;
        dns_acl_t *                     queryacl;
        dns_acl_t *                     queryonacl;
        dns_acl_t *                     recursionacl;

diff --git a/lib/dns/view.c b/lib/dns/view.c
index 3b71a4bcec8b472050535e339919f387568d8a77..8ca75136615cb607913f45b2bea566ab1bf08ef4 100644 (file)
--- a/lib/dns/view.c
+++ b/lib/dns/view.c
@@ -15,7 +15,7 @@
  * PERFORMANCE OF THIS SOFTWARE.
  */
 
-/* $Id: view.c,v 1.169 2010/09/06 04:31:11 marka Exp $ */
+/* $Id: view.c,v 1.170 2010/09/24 05:09:03 marka Exp $ */
 
 /*! \file */
 
@@ -158,6 +158,8 @@ dns_view_create(isc_mem_t *mctx, dns_rdataclass_t rdclass,
        view->acceptexpired = ISC_FALSE;
        view->minimalresponses = ISC_FALSE;
        view->transfer_format = dns_one_answer;
+       view->cacheacl = NULL;
+       view->cacheonacl = NULL;
        view->queryacl = NULL;
        view->queryonacl = NULL;
        view->recursionacl = NULL;
@@ -298,6 +300,10 @@ destroy(dns_view_t *view) {
                dns_acl_detach(&view->matchclients);
        if (view->matchdestinations != NULL)
                dns_acl_detach(&view->matchdestinations);
+       if (view->cacheacl != NULL)
+               dns_acl_detach(&view->cacheacl);
+       if (view->cacheonacl != NULL)
+               dns_acl_detach(&view->cacheonacl);
        if (view->queryacl != NULL)
                dns_acl_detach(&view->queryacl);
        if (view->queryonacl != NULL)