]> git.ipfire.org Git - thirdparty/bind9.git/commitdiff
2282. [bug] Acl code fixups. [RT #17346]
authorMark Andrews <marka@isc.org>
Thu, 20 Dec 2007 01:48:29 +0000 (01:48 +0000)
committerMark Andrews <marka@isc.org>
Thu, 20 Dec 2007 01:48:29 +0000 (01:48 +0000)
CHANGES
bin/tests/system/xfer/ns2/named.conf
lib/dns/acl.c
lib/isc/radix.c

diff --git a/CHANGES b/CHANGES
index 75785d52e1ce74b729bf4f54683be79b9850f2ec..8ff50008662c70a0bf6437dfd41dff0e0acfb93f 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -1,3 +1,5 @@
+2282.  [bug]           Acl code fixups. [RT #17346]
+
 2281.  [bug]           Attempts to use undefined acls were not being logged.
                        [RT #17307]
 
index 2bf1b5221656396c9fb333398a5c26a4f7cf6816..53a64ab96267819d66ee25b4b21e4c0c8fc45f38 100644 (file)
@@ -15,7 +15,7 @@
  * PERFORMANCE OF THIS SOFTWARE.
  */
 
-/* $Id: named.conf,v 1.23 2007/06/19 23:47:07 tbox Exp $ */
+/* $Id: named.conf,v 1.24 2007/12/20 01:48:29 marka Exp $ */
 
 controls { /* empty */ };
 
@@ -36,8 +36,12 @@ options {
 include "../../common/controls.conf";
 
 key tsigzone. {
-        algorithm hmac-md5;
-        secret "1234abcd8765";
+       algorithm hmac-md5;
+       secret "1234abcd8765";
+};
+
+acl tzkey {
+       key tsigzone.;
 };
 
 zone "." {
@@ -53,5 +57,5 @@ zone "example" {
 zone "tsigzone" {
        type master;
        file "tsigzone.db";
-       allow-transfer { key tsigzone.; };
+       allow-transfer { tzkey; };
 };
index 6bb169c79590a72302a508cb3a4b4098b3ce2314..908a315de826be5f97c58bfbf11d6ac4fa35884d 100644 (file)
@@ -15,7 +15,7 @@
  * PERFORMANCE OF THIS SOFTWARE.
  */
 
-/* $Id: acl.c,v 1.35 2007/09/19 03:03:29 marka Exp $ */
+/* $Id: acl.c,v 1.36 2007/12/20 01:48:29 marka Exp $ */
 
 /*! \file */
 
@@ -57,12 +57,12 @@ dns_acl_create(isc_mem_t *mctx, int n, dns_acl_t **target) {
                return (result);
        }
 
-        result = dns_iptable_create(mctx, &acl->iptable);
+       result = dns_iptable_create(mctx, &acl->iptable);
        if (result != ISC_R_SUCCESS) {
                isc_mem_put(mctx, acl, sizeof(*acl));
                return (result);
        }
-        
+
        acl->elements = NULL;
        acl->alloc = 0;
        acl->length = 0;
@@ -101,7 +101,7 @@ dns_acl_anyornone(isc_mem_t *mctx, isc_boolean_t neg, dns_acl_t **target) {
        result = dns_acl_create(mctx, 0, &acl);
        if (result != ISC_R_SUCCESS)
                return (result);
-        dns_iptable_addprefix(acl->iptable, NULL, 0, ISC_TF(!neg));
+       dns_iptable_addprefix(acl->iptable, NULL, 0, ISC_TF(!neg));
        *target = acl;
        return (result);
 }
@@ -129,13 +129,13 @@ dns_acl_none(isc_mem_t *mctx, dns_acl_t **target) {
 static isc_boolean_t
 dns_acl_isanyornone(dns_acl_t *acl, isc_boolean_t pos)
 {
-        /* Should never happen but let's be safe */
-        if (acl == NULL ||
-            acl->iptable == NULL ||
-            acl->iptable->radix == NULL ||
-            acl->iptable->radix->head == NULL ||
-            acl->iptable->radix->head->prefix == NULL)
-                return (ISC_FALSE);
+       /* Should never happen but let's be safe */
+       if (acl == NULL ||
+           acl->iptable == NULL ||
+           acl->iptable->radix == NULL ||
+           acl->iptable->radix->head == NULL ||
+           acl->iptable->radix->head->prefix == NULL)
+               return (ISC_FALSE);
 
        if (acl->length != 0 && acl->node_count != 1)
                return (ISC_FALSE);
@@ -153,7 +153,7 @@ dns_acl_isanyornone(dns_acl_t *acl, isc_boolean_t pos)
 isc_boolean_t
 dns_acl_isany(dns_acl_t *acl)
 {
-        return (dns_acl_isanyornone(acl, ISC_TRUE));
+       return (dns_acl_isanyornone(acl, ISC_TRUE));
 }
 
 /*
@@ -162,7 +162,7 @@ dns_acl_isany(dns_acl_t *acl)
 isc_boolean_t
 dns_acl_isnone(dns_acl_t *acl)
 {
-        return (dns_acl_isanyornone(acl, ISC_FALSE));
+       return (dns_acl_isanyornone(acl, ISC_FALSE));
 }
 
 /*
@@ -182,10 +182,10 @@ dns_acl_match(const isc_netaddr_t *reqaddr,
        isc_uint16_t bitlen;
        isc_prefix_t pfx;
        isc_radix_node_t *node;
-        const isc_netaddr_t *addr;
-        isc_netaddr_t v4addr;
+       const isc_netaddr_t *addr;
+       isc_netaddr_t v4addr;
        isc_result_t result;
-        int match_num = -1;
+       int match_num = -1;
        unsigned int i;
 
        REQUIRE(reqaddr != NULL);
@@ -194,7 +194,7 @@ dns_acl_match(const isc_netaddr_t *reqaddr,
        if (env == NULL || env->match_mapped == ISC_FALSE ||
            reqaddr->family != AF_INET6 ||
            !IN6_IS_ADDR_V4MAPPED(&reqaddr->type.in6))
-                addr = reqaddr;
+               addr = reqaddr;
        else {
                isc_netaddr_fromv4mapped(&v4addr, reqaddr);
                addr = &v4addr;
@@ -213,24 +213,24 @@ dns_acl_match(const isc_netaddr_t *reqaddr,
        /* Found a match. */
        if (result == ISC_R_SUCCESS && node != NULL) {
                match_num = node->node_num;
-                if (*(isc_boolean_t *) node->data == ISC_TRUE)
-                        *match = match_num;
-                else
-                        *match = -match_num;
-        }
+               if (*(isc_boolean_t *) node->data == ISC_TRUE)
+                       *match = match_num;
+               else
+                       *match = -match_num;
+       }
 
-        /* Now search non-radix elements for a match with a lower node_num. */
+       /* Now search non-radix elements for a match with a lower node_num. */
        for (i = 0; i < acl->length; i++) {
                dns_aclelement_t *e = &acl->elements[i];
 
                if (dns_aclelement_match(reqaddr, reqsigner,
                                         e, env, matchelt)) {
-                        if (match_num == -1 || e->node_num < match_num) {
-                                if (e->negative)
-                                       *match = -e->node_num;
-                                else
-                                       *match = e->node_num;
-                        }
+                       if (match_num == -1 || e->node_num < match_num) {
+                               if (e->negative == ISC_TRUE)
+                                       *match = -e->node_num;
+                               else
+                                       *match = e->node_num;
+                       }
                        return (ISC_R_SUCCESS);
                }
        }
@@ -251,63 +251,90 @@ isc_result_t
 dns_acl_merge(dns_acl_t *dest, dns_acl_t *source, isc_boolean_t pos)
 {
        isc_result_t result;
-        unsigned int newalloc, nelem, i;
-        int max_node = 0, nodes;
-
-        /* Resize the element array if needed. */
-        if (dest->length + source->length > dest->alloc) {
-                void *newmem;
-
-                newalloc = dest->alloc + source->alloc;
-                if (newalloc < 4)
-                        newalloc = 4;
-
-                newmem = isc_mem_get(dest->mctx,
-                                     newalloc * sizeof(dns_aclelement_t));
-                if (newmem == NULL)
-                        return (ISC_R_NOMEMORY);
-
-                /* Copy in the original elements */
-                memcpy(newmem, dest->elements,
-                       dest->length * sizeof(dns_aclelement_t));
-
-                /* Release the memory for the old elements array */
-                isc_mem_put(dest->mctx, dest->elements,
-                            dest->alloc * sizeof(dns_aclelement_t));
-                dest->elements = newmem;
-                dest->alloc = newalloc;
-        }
-
-        /*
-         * Now copy in the new elements, increasing their node_num
-         * values so as to keep the new ACL consistent.  If we're
-         * negating, then negate positive elements, but keep negative
-         * elements the same for security reasons.
-         */
-        nelem = dest->length;
-        memcpy(&dest->elements[nelem], source->elements,
-               (source->length * sizeof(dns_aclelement_t)));
-        for (i = 0; i < source->length; i++) {
-                dest->elements[nelem + i].node_num = 
-                        source->elements[i].node_num + dest->node_count;
-                if (source->elements[i].node_num > max_node)
-                        max_node = source->elements[i].node_num;
-                if (!pos && source->elements[i].negative == ISC_FALSE)
-                        dest->elements[nelem + i].negative = ISC_TRUE;
-        }
-
-        /*
-         * Merge the iptables.  Make sure the destination ACL's
-         * node_count value is set correctly afterward.
-         */
-        nodes = max_node + dest->node_count;
-        result = dns_iptable_merge(dest->iptable, source->iptable, pos);
-        if (result != ISC_R_SUCCESS)
-                return (result);
-        if (nodes > dest->node_count)
-                dest->node_count = nodes;
-
-        return (ISC_R_SUCCESS);
+       unsigned int newalloc, nelem, i;
+       int max_node = 0, nodes;
+
+       /* Resize the element array if needed. */
+       if (dest->length + source->length > dest->alloc) {
+               void *newmem;
+
+               newalloc = dest->alloc + source->alloc;
+               if (newalloc < 4)
+                       newalloc = 4;
+
+               newmem = isc_mem_get(dest->mctx,
+                                    newalloc * sizeof(dns_aclelement_t));
+               if (newmem == NULL)
+                       return (ISC_R_NOMEMORY);
+
+               /* Copy in the original elements */
+               memcpy(newmem, dest->elements,
+                      dest->length * sizeof(dns_aclelement_t));
+
+               /* Release the memory for the old elements array */
+               isc_mem_put(dest->mctx, dest->elements,
+                           dest->alloc * sizeof(dns_aclelement_t));
+               dest->elements = newmem;
+               dest->alloc = newalloc;
+       }
+
+       /*
+        * Now copy in the new elements, increasing their node_num
+        * values so as to keep the new ACL consistent.  If we're
+        * negating, then negate positive elements, but keep negative
+        * elements the same for security reasons.
+        */
+       nelem = dest->length;
+       dest->length += source->length;
+       for (i = 0; i < source->length; i++) {
+               if (source->elements[i].node_num > max_node)
+                       max_node = source->elements[i].node_num;
+
+               /* Copy type. */
+               dest->elements[nelem + i].type = source->elements[i].type;
+
+               /* Adjust node numbering. */
+               dest->elements[nelem + i].node_num = 
+                       source->elements[i].node_num + dest->node_count;
+
+               /* Duplicate nested acl. */
+               if(source->elements[i].type == dns_aclelementtype_nestedacl &&
+                  source->elements[i].nestedacl != NULL)
+                       dns_acl_attach(source->elements[i].nestedacl,
+                                      &dest->elements[nelem + i].nestedacl);
+
+               /* Duplicate key name. */
+               if (source->elements[i].type == dns_aclelementtype_keyname) {
+                       dns_name_init(&dest->elements[nelem+i].keyname, NULL);
+                       result = dns_name_dup(&source->elements[i].keyname,
+                                             dest->mctx,
+                                             &dest->elements[nelem+i].keyname);
+                       if (result != ISC_R_SUCCESS)
+                               return result;
+               }
+
+               /* reverse sense of positives if this is a negative acl */
+               if (!pos && source->elements[i].negative == ISC_FALSE) {
+                       dest->elements[nelem + i].negative = ISC_TRUE;
+               } else {
+                       dest->elements[nelem + i].negative = 
+                               source->elements[i].negative;
+               }
+       }
+
+
+       /*
+        * Merge the iptables.  Make sure the destination ACL's
+        * node_count value is set correctly afterward.
+        */
+       nodes = max_node + dest->node_count;
+       result = dns_iptable_merge(dest->iptable, source->iptable, pos);
+       if (result != ISC_R_SUCCESS)
+               return (result);
+       if (nodes > dest->node_count)
+               dest->node_count = nodes;
+
+       return (ISC_R_SUCCESS);
 }
 
 /*
@@ -330,62 +357,62 @@ dns_aclelement_match(const isc_netaddr_t *reqaddr,
        int indirectmatch;
        isc_result_t result;
 
-        switch (e->type) {
-           case dns_aclelementtype_keyname:
+       switch (e->type) {
+       case dns_aclelementtype_keyname:
                if (reqsigner != NULL &&
                    dns_name_equal(reqsigner, &e->keyname)) {
-                       if (matchelt != NULL)
-                               *matchelt = e;
-                       return (ISC_TRUE);
-                } else {
-                        return (ISC_FALSE);
-                }
-
-           case dns_aclelementtype_nestedacl:
-                inner = e->nestedacl;
-                break;
-
-            case dns_aclelementtype_localhost:
-                if (env == NULL || env->localhost == NULL)
-                        return (ISC_FALSE);
-                inner = env->localhost;
-                break;
-
-            case dns_aclelementtype_localnets:
-                if (env == NULL || env->localnets == NULL)
-                        return (ISC_FALSE);
-                inner = env->localnets;
-                break;
-
-            default:
-                /* Should be impossible */
-                INSIST(0);
-        }
+                       if (matchelt != NULL)
+                               *matchelt = e;
+                       return (ISC_TRUE);
+               } else {
+                       return (ISC_FALSE);
+               }
+
+       case dns_aclelementtype_nestedacl:
+               inner = e->nestedacl;
+               break;
+
+       case dns_aclelementtype_localhost:
+               if (env == NULL || env->localhost == NULL)
+                       return (ISC_FALSE);
+               inner = env->localhost;
+               break;
+
+       case dns_aclelementtype_localnets:
+               if (env == NULL || env->localnets == NULL)
+                       return (ISC_FALSE);
+               inner = env->localnets;
+               break;
+
+       default:
+               /* Should be impossible. */
+               INSIST(0);
+       }
                
-        result = dns_acl_match(reqaddr, reqsigner, inner, env,
-                               &indirectmatch, matchelt);
-        INSIST(result == ISC_R_SUCCESS);
-
-        /*
-         * Treat negative matches in indirect ACLs as "no match".
-         * That way, a negated indirect ACL will never become a
-         * surprise positive match through double negation.
-         * XXXDCL this should be documented.
-         */
-
-        if (indirectmatch > 0) {
-                if (matchelt != NULL)
-                        *matchelt = e;
-                return (ISC_TRUE);
-        }
+       result = dns_acl_match(reqaddr, reqsigner, inner, env,
+                              &indirectmatch, matchelt);
+       INSIST(result == ISC_R_SUCCESS);
+
+       /*
+        * Treat negative matches in indirect ACLs as "no match".
+        * That way, a negated indirect ACL will never become a
+        * surprise positive match through double negation.
+        * XXXDCL this should be documented.
+        */
+
+       if (indirectmatch > 0) {
+               if (matchelt != NULL)
+                       *matchelt = e;
+               return (ISC_TRUE);
+       }
                
-        /*
-         * A negative indirect match may have set *matchelt, but we don't
-         * want it set when we return.
-         */
+       /*
+        * A negative indirect match may have set *matchelt, but we don't
+        * want it set when we return.
+        */
 
-        if (matchelt != NULL)
-                *matchelt = NULL;
+       if (matchelt != NULL)
+               *matchelt = NULL;
                
        return (ISC_FALSE);
 }      
@@ -413,8 +440,8 @@ destroy(dns_acl_t *dacl) {
                            dacl->alloc * sizeof(dns_aclelement_t));
        if (dacl->name != NULL)
                isc_mem_free(dacl->mctx, dacl->name);
-        if (dacl->iptable != NULL)
-                dns_iptable_detach(&dacl->iptable);
+       if (dacl->iptable != NULL)
+               dns_iptable_detach(&dacl->iptable);
        isc_refcount_destroy(&dacl->refcount);
        dacl->magic = 0;
        isc_mem_put(dacl->mctx, dacl, sizeof(*dacl));
@@ -438,7 +465,7 @@ static isc_boolean_t        insecure_prefix_found;
 
 static void
 initialize_action(void) {
-        RUNTIME_CHECK(isc_mutex_init(&insecure_prefix_lock) == ISC_R_SUCCESS);
+       RUNTIME_CHECK(isc_mutex_init(&insecure_prefix_lock) == ISC_R_SUCCESS);
 }
 
 /*
@@ -449,30 +476,30 @@ static void
 is_insecure(isc_prefix_t *prefix, void *data) {
        isc_boolean_t secure = * (isc_boolean_t *)data;
 
-        /* Negated entries are always secure */
-        if (!secure) {
-                return; 
-        }
+       /* Negated entries are always secure. */
+       if (!secure) {
+               return; 
+       }
 
-        /* If loopback prefix found, return */
+       /* If loopback prefix found, return */
        switch (prefix->family) {
-           case AF_INET:
+       case AF_INET:
                if (prefix->bitlen == 32 &&
                    htonl(prefix->add.sin.s_addr) == INADDR_LOOPBACK)
-                        return;
+                       return;
                break;
-           case AF_INET6:
+       case AF_INET6:
                if (prefix->bitlen == 128 &&
                    IN6_IS_ADDR_LOOPBACK(&prefix->add.sin6))
-                        return;
+                       return;
                break;
-           default:
+       default:
                break;
        }
 
-        /* Non-negated, non-loopback */
-        insecure_prefix_found = ISC_TRUE;
-        return;
+       /* Non-negated, non-loopback */
+       insecure_prefix_found = ISC_TRUE;
+       return;
 }
 
 /*
@@ -491,19 +518,19 @@ dns_acl_isinsecure(const dns_acl_t *a) {
        RUNTIME_CHECK(isc_once_do(&insecure_prefix_once,
                                  initialize_action) == ISC_R_SUCCESS);
 
-        /*
-         * Walk radix tree to find out if there are any non-negated,
-         * non-loopback prefixes.
-         */
+       /*
+        * Walk radix tree to find out if there are any non-negated,
+        * non-loopback prefixes.
+        */
        LOCK(&insecure_prefix_lock);
-        insecure_prefix_found = ISC_FALSE;
-        isc_radix_process(a->iptable->radix, is_insecure);
+       insecure_prefix_found = ISC_FALSE;
+       isc_radix_process(a->iptable->radix, is_insecure);
        insecure = insecure_prefix_found;
        UNLOCK(&insecure_prefix_lock);
-        if (insecure)
-                return(ISC_TRUE);
+       if (insecure)
+               return(ISC_TRUE);
                        
-        /* Now check non-radix elements */
+       /* Now check non-radix elements */
        for (i = 0; i < a->length; i++) {
                dns_aclelement_t *e = &a->elements[i];
 
@@ -512,19 +539,19 @@ dns_acl_isinsecure(const dns_acl_t *a) {
                        continue;
 
                switch (e->type) {
-                   case dns_aclelementtype_keyname:
-                   case dns_aclelementtype_localhost:
+               case dns_aclelementtype_keyname:
+               case dns_aclelementtype_localhost:
                        continue;
 
-                    case dns_aclelementtype_nestedacl:
-                        if (dns_acl_isinsecure(e->nestedacl))
-                                return (ISC_TRUE);
-                        continue;
+               case dns_aclelementtype_nestedacl:
+                       if (dns_acl_isinsecure(e->nestedacl))
+                               return (ISC_TRUE);
+                       continue;
 
-                   case dns_aclelementtype_localnets:
+               case dns_aclelementtype_localnets:
                        return (ISC_TRUE);
 
-                   default:
+               default:
                        INSIST(0);
                        return (ISC_TRUE);
                }
index f9a2c5075391e409852ce65e24ed7eea65d2f94a..dc01dede26975200d9c63ba68532ce2c3d3aee77 100644 (file)
@@ -14,7 +14,7 @@
  * PERFORMANCE OF THIS SOFTWARE.
  */
 
-/* $Id: radix.c,v 1.8 2007/11/27 19:14:45 each Exp $ */
+/* $Id: radix.c,v 1.9 2007/12/20 01:48:29 marka Exp $ */
 
 /*
  * This source was adapted from MRT's RCS Ids:
@@ -115,9 +115,9 @@ _ref_prefix(isc_mem_t *mctx, isc_prefix_t **target, isc_prefix_t *prefix) {
 static int 
 _comp_with_mask(void *addr, void *dest, u_int mask) {
 
-        /* Mask length of zero matches everything */
-        if (mask == 0)
-                return (1);
+       /* Mask length of zero matches everything */
+       if (mask == 0)
+               return (1);
 
        if (memcmp(addr, dest, mask / 8) == 0) {
                int n = mask / 8;
@@ -315,7 +315,20 @@ isc_radix_insert(isc_radix_tree_t *radix, isc_radix_node_t **target,
                node->parent = NULL;
                node->l = node->r = NULL;
                node->data = NULL;
-               node->node_num = ++radix->num_added_node;
+               if (source != NULL) {
+                       /*
+                        * If source is non-NULL, then we're merging in a
+                        * node from an existing radix tree.  To keep
+                        * the node_num values consistent, the calling
+                        * function will add the total number of nodes
+                        * added to num_added_node at the end of
+                        * the merge operation--we don't do it here.
+                        */
+                       node->node_num = radix->num_added_node +
+                                        source->node_num;
+               } else {
+                       node->node_num = ++radix->num_added_node;
+               }
                radix->head = node;
                radix->num_active_node++;
                *target = node;
@@ -382,7 +395,13 @@ isc_radix_insert(isc_radix_tree_t *radix, isc_radix_node_t **target,
                if (result != ISC_R_SUCCESS)
                        return (result);
                INSIST(node->data == NULL && node->node_num == -1);
-               node->node_num = ++radix->num_added_node;
+               if (source != NULL) {
+                       /* Merging node */
+                       node->node_num = radix->num_added_node +
+                                        source->node_num;
+               } else {
+                       node->node_num = ++radix->num_added_node;
+               }
                *target = node;
                return (ISC_R_SUCCESS);
        }
@@ -412,17 +431,7 @@ isc_radix_insert(isc_radix_tree_t *radix, isc_radix_node_t **target,
        radix->num_active_node++;
 
        if (source != NULL) {
-               /*
-                * If source is non-NULL, then we're merging in a node
-                * from an existing radix tree.  Node_num values have to
-                * remain consistent; they can't just be added in whatever
-                * order came from walking the tree.  So we don't increment
-                * num_added_node here; instead, we add it to the node-num
-                * values for each node from the nested tree, and then when
-                * the whole tree is done, the calling function will bump
-                * num_added_node by the highest value of node_num in the
-                * tree.
-                */
+               /* Merging node */
                new_node->node_num = radix->num_added_node + source->node_num;
                new_node->data = source->data;
        } else {