2282. [bug] Acl code fixups. [RT #17346] [RT #17374]

diff --git a/CHANGES b/CHANGES
index 8ff50008662c70a0bf6437dfd41dff0e0acfb93f..8be16b9b915980c50552f3119d9bfcf0d8f653a4 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -1,4 +1,4 @@
-2282.  [bug]           Acl code fixups. [RT #17346]
+2282.  [bug]           Acl code fixups. [RT #17346] [RT #17374]
 
 2281.  [bug]           Attempts to use undefined acls were not being logged.
                        [RT #17307]

diff --git a/lib/dns/acl.c b/lib/dns/acl.c
index 908a315de826be5f97c58bfbf11d6ac4fa35884d..4163b1561eb078839705ed0bb1bd2090ae8c2882 100644 (file)
--- a/lib/dns/acl.c
+++ b/lib/dns/acl.c
@@ -15,7 +15,7 @@
  * PERFORMANCE OF THIS SOFTWARE.
  */
 
-/* $Id: acl.c,v 1.36 2007/12/20 01:48:29 marka Exp $ */
+/* $Id: acl.c,v 1.37 2007/12/21 06:46:47 marka Exp $ */
 
 /*! \file */
 
@@ -66,6 +66,7 @@ dns_acl_create(isc_mem_t *mctx, int n, dns_acl_t **target) {
        acl->elements = NULL;
        acl->alloc = 0;
        acl->length = 0;
+       acl->has_negatives = ISC_FALSE;
 
        ISC_LINK_INIT(acl, nextincache);
        /*
@@ -223,6 +224,10 @@ dns_acl_match(const isc_netaddr_t *reqaddr,
        for (i = 0; i < acl->length; i++) {
                dns_aclelement_t *e = &acl->elements[i];
 
+               /* Already found a better match? */
+               if (match_num != -1 && match_num < e->node_num)
+                       return (ISC_R_SUCCESS);
+
                if (dns_aclelement_match(reqaddr, reqsigner,
                                         e, env, matchelt)) {
                        if (match_num == -1 || e->node_num < match_num) {

diff --git a/lib/dns/include/dns/acl.h b/lib/dns/include/dns/acl.h
index 058e4aacf92186c4b813861cb0e1b9351ebfde5f..94537a5ccd584fa1e1a3f784d2a2a3281a765d1a 100644 (file)
--- a/lib/dns/include/dns/acl.h
+++ b/lib/dns/include/dns/acl.h
@@ -15,7 +15,7 @@
  * PERFORMANCE OF THIS SOFTWARE.
  */
 
-/* $Id: acl.h,v 1.30 2007/09/12 01:09:08 each Exp $ */
+/* $Id: acl.h,v 1.31 2007/12/21 06:46:47 marka Exp $ */
 
 #ifndef DNS_ACL_H
 #define DNS_ACL_H 1
@@ -63,20 +63,21 @@ struct dns_aclipprefix {
 };
 
 struct dns_aclelement {
-       dns_aclelemettype_t type;
-       isc_boolean_t negative;
-       dns_name_t        keyname;
-        dns_acl_t         *nestedacl;
-        int node_num;
+       dns_aclelemettype_t     type;
+       isc_boolean_t           negative;
+       dns_name_t              keyname;
+       dns_acl_t               *nestedacl;
+       int                     node_num;
 };
 
 struct dns_acl {
        unsigned int            magic;
        isc_mem_t               *mctx;
        isc_refcount_t          refcount;
-       dns_iptable_t           *iptable;
-#define node_count              iptable->radix->num_added_node
+       dns_iptable_t           *iptable;
+#define node_count             iptable->radix->num_added_node
        dns_aclelement_t        *elements;
+       isc_boolean_t           has_negatives;
        unsigned int            alloc;          /*%< Elements allocated */
        unsigned int            length;         /*%< Elements initialized */
        char                    *name;          /*%< Temporary use only */
@@ -204,7 +205,7 @@ isc_boolean_t
 dns_aclelement_match(const isc_netaddr_t *reqaddr,
                     const dns_name_t *reqsigner,
                     const dns_aclelement_t *e,
-                    const dns_aclenv_t *env,                
+                    const dns_aclenv_t *env,
                     const dns_aclelement_t **matchelt);
 /*%<
  * Like dns_acl_match, but matches against the single ACL element 'e'

diff --git a/lib/isccfg/aclconf.c b/lib/isccfg/aclconf.c
index e99d98aedc40302a6a4d5b728b680bd58b11352a..253a8ce9a359cc1e81e3763537f9e049f489d57c 100644 (file)
--- a/lib/isccfg/aclconf.c
+++ b/lib/isccfg/aclconf.c
@@ -15,7 +15,7 @@
  * PERFORMANCE OF THIS SOFTWARE.
  */
 
-/* $Id: aclconf.c,v 1.16 2007/12/18 01:53:26 marka Exp $ */
+/* $Id: aclconf.c,v 1.17 2007/12/21 06:46:47 marka Exp $ */
 
 #include <config.h>
 
@@ -174,6 +174,10 @@ cfg_acl_fromconfig(const cfg_obj_t *caml,
        dns_aclelement_t *de;
        const cfg_listelt_t *elt;
        dns_iptable_t *iptab;
+       int new_nest_level = 0;
+
+       if (nest_level != 0)
+               new_nest_level = nest_level - 1;
 
        REQUIRE(target != NULL);
        REQUIRE(*target == NULL || DNS_ACL_VALID(*target));
@@ -214,6 +218,7 @@ cfg_acl_fromconfig(const cfg_obj_t *caml,
                        /* This must be a negated element. */
                        ce = cfg_tuple_get(ce, "value");
                        neg = ISC_TRUE;
+                       dacl->has_negatives = ISC_TRUE;
                } else
                        neg = ISC_FALSE;
 
@@ -240,16 +245,16 @@ cfg_acl_fromconfig(const cfg_obj_t *caml,
 
                        cfg_obj_asnetprefix(ce, &addr, &bitlen);
 
-                        /*
-                         * If nesting ACLs (nest_level != 0), we negate
-                         * the nestedacl element, not the iptable entry
-                         */
+                       /*
+                        * If nesting ACLs (nest_level != 0), we negate
+                        * the nestedacl element, not the iptable entry.
+                        */
                        result = dns_iptable_addprefix(iptab, &addr, bitlen,
-                                             ISC_TF(nest_level != 0 || !neg));
+                                             ISC_TF(nest_level != 0 || !neg));
                        if (result != ISC_R_SUCCESS)
                                goto cleanup;
 
-                       if (nest_level != 0) {
+                       if (nest_level > 0) {
                                de->type = dns_aclelementtype_nestedacl;
                                de->negative = neg;
                        } else
@@ -258,35 +263,35 @@ cfg_acl_fromconfig(const cfg_obj_t *caml,
                        /*
                         * If we're nesting ACLs, put the nested
                         * ACL onto the elements list; otherwise
-                        * merge it into *this* ACL.
+                        * merge it into *this* ACL.  We nest ACLs
+                        * in two cases: 1) sortlist, 2) if the
+                        * nested ACL contains negated members.
                         */
-                       if (nest_level == 0) {
-                               if (inneracl != NULL)
-                                       dns_acl_detach(&inneracl);
-
-                               result = cfg_acl_fromconfig(ce, cctx, lctx,
-                                                           ctx, mctx, 0,
-                                                           &inneracl);
-                               if (result != ISC_R_SUCCESS)
-                                       goto cleanup;
-
+                       if (inneracl != NULL)
+                               dns_acl_detach(&inneracl);
+                       result = cfg_acl_fromconfig(ce, cctx, lctx,
+                                                   ctx, mctx, new_nest_level,
+                                                   &inneracl);
+                       if (result != ISC_R_SUCCESS)
+                               goto cleanup;
+nested_acl:
+                       if (nest_level > 0 || inneracl->has_negatives) {
+                               de->type = dns_aclelementtype_nestedacl;
+                               de->negative = neg;
+                               if (de->nestedacl != NULL)
+                                       dns_acl_detach(&de->nestedacl);
+                               dns_acl_attach(inneracl,
+                                              &de->nestedacl);
+                               dns_acl_detach(&inneracl);
+                               /* Fall through. */
+                       } else {
                                dns_acl_merge(dacl, inneracl,
                                              ISC_TF(!neg));
                                dns_acl_detach(&inneracl);
                                continue;
-                       } else {
-                               de->type = dns_aclelementtype_nestedacl;
-                               de->negative = neg;
-                               result = cfg_acl_fromconfig(ce, cctx, lctx,
-                                                           ctx, mctx,
-                                                           nest_level - 1,
-                                                           &de->nestedacl);
-                               if (result != ISC_R_SUCCESS)
-                                       goto cleanup;
-                               /* Fall through */
                        }
                } else if (cfg_obj_istype(ce, &cfg_type_keyref)) {
-                       /* Key name */
+                       /* Key name. */
                        de->type = dns_aclelementtype_keyname;
                        de->negative = neg;
                        dns_name_init(&de->keyname, NULL);
@@ -295,32 +300,41 @@ cfg_acl_fromconfig(const cfg_obj_t *caml,
                        if (result != ISC_R_SUCCESS)
                                goto cleanup;
                } else if (cfg_obj_isstring(ce)) {
-                       /* ACL name */
+                       /* ACL name. */
                        const char *name = cfg_obj_asstring(ce);
                        if (strcasecmp(name, "any") == 0) {
-                               /* iptable entry with zero bit length */
+                               /* Iptable entry with zero bit length. */
                                result = dns_iptable_addprefix(iptab, NULL, 0,
-                                             ISC_TF(nest_level != 0 || !neg));
-                                if (result != ISC_R_SUCCESS)
-                                        goto cleanup;
-
-                                if (nest_level != 0) {
-                                        de->type = dns_aclelementtype_nestedacl;
-                                        de->negative = neg;
-                                } else
-                                        continue;
+                                             ISC_TF(nest_level != 0 || !neg));
+                               if (result != ISC_R_SUCCESS)
+                                       goto cleanup;
+
+                               if (nest_level != 0) {
+                                       de->type = dns_aclelementtype_nestedacl;
+                                       de->negative = neg;
+                               } else
+                                       continue;
                        } else if (strcasecmp(name, "none") == 0) {
-                               /* negated "any" */
+                               /* none == !any */
+                               /*
+                                * We don't unconditional set
+                                * dacl->has_negatives and
+                                * de->negative to true so we can handle
+                                * "!none;".
+                                */
                                result = dns_iptable_addprefix(iptab, NULL, 0,
-                                             ISC_TF(nest_level != 0 || neg));
-                                if (result != ISC_R_SUCCESS)
-                                        goto cleanup;
-
-                                if (nest_level != 0) {
-                                        de->type = dns_aclelementtype_nestedacl;
-                                        de->negative = !neg;
-                                } else
-                                        continue;
+                                             ISC_TF(nest_level != 0 || neg));
+                               if (result != ISC_R_SUCCESS)
+                                       goto cleanup;
+
+                               if (!neg)
+                                       dacl->has_negatives = !neg;
+
+                               if (nest_level != 0) {
+                                       de->type = dns_aclelementtype_nestedacl;
+                                       de->negative = !neg;
+                               } else
+                                       continue;
                        } else if (strcasecmp(name, "localhost") == 0) {
                                de->type = dns_aclelementtype_localhost;
                                de->negative = neg;
@@ -328,34 +342,15 @@ cfg_acl_fromconfig(const cfg_obj_t *caml,
                                de->type = dns_aclelementtype_localnets;
                                de->negative = neg;
                        } else {
-                               int new_nest_level;
                                if (inneracl != NULL)
                                        dns_acl_detach(&inneracl);
-                               if (nest_level != 0)
-                                       new_nest_level = nest_level - 1;
-                               else
-                                       new_nest_level = 0;
                                result = convert_named_acl(ce, cctx, lctx, ctx,
                                                           mctx, new_nest_level,
                                                           &inneracl);
                                if (result != ISC_R_SUCCESS)
                                        goto cleanup;
 
-                               if (nest_level != 0) {
-                                       de->type = dns_aclelementtype_nestedacl;
-                                       de->negative = neg;
-                                       if(de->nestedacl != NULL)
-                                               dns_acl_detach(&de->nestedacl);
-                                       dns_acl_attach(inneracl,
-                                                      &de->nestedacl);
-                                       dns_acl_detach(&inneracl);
-                                       /* Fall through */
-                               } else {
-                                       dns_acl_merge(dacl, inneracl,
-                                                     ISC_TF(!neg));
-                                       dns_acl_detach(&inneracl);
-                                       continue;
-                               }
+                               goto nested_acl;
                        }
                } else {
                        cfg_obj_log(ce, lctx, ISC_LOG_WARNING,
@@ -377,8 +372,8 @@ cfg_acl_fromconfig(const cfg_obj_t *caml,
                dacl->node_count++;
                de->node_num = dacl->node_count;
 
-               de++;
                dacl->length++;
+               de++;
                INSIST(dacl->length <= dacl->alloc);
        }