From 262c39b2366bf79062f7f86b218947523dd1cbac Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Mon, 21 Jan 2008 20:38:54 +0000 Subject: [PATCH] IPv6 addresses could match IPv4 ACL entries and vice versa. [RT #17462] --- CHANGES | 3 + bin/tests/system/acl/ns2/named2.conf | 16 +++-- lib/dns/acl.c | 36 +++++++----- lib/dns/iptable.c | 38 +++++++----- lib/isc/include/isc/radix.h | 27 ++++++--- lib/isc/radix.c | 87 +++++++++++++++++++--------- 6 files changed, 138 insertions(+), 69 deletions(-) diff --git a/CHANGES b/CHANGES index 25342da2ba..69b87ebf5d 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,6 @@ +2311. [bug] IPv6 addresses could match IPv4 ACL entries and + vice versa. [RT #17462] + 2310 [bug] dig, host, nslookup: flush stdout before emitting debug/fatal messages. [RT #17501] diff --git a/bin/tests/system/acl/ns2/named2.conf b/bin/tests/system/acl/ns2/named2.conf index a9940fed2c..bcd7e0df19 100644 --- a/bin/tests/system/acl/ns2/named2.conf +++ b/bin/tests/system/acl/ns2/named2.conf @@ -14,7 +14,7 @@ * PERFORMANCE OF THIS SOFTWARE. */ -/* $Id: named2.conf,v 1.2 2008/01/10 01:10:01 marka Exp $ */ +/* $Id: named2.conf,v 1.3 2008/01/21 20:38:54 each Exp $ */ controls { /* empty */ }; @@ -35,13 +35,13 @@ options { include "../../common/controls.conf"; key one { - algorithm hmac-md5; - secret "1234abcd8765"; + algorithm hmac-md5; + secret "1234abcd8765"; }; key two { - algorithm hmac-md5; - secret "1234abcd8765"; + algorithm hmac-md5; + secret "1234abcd8765"; }; zone "." { @@ -57,5 +57,9 @@ zone "example" { zone "tsigzone" { type master; file "tsigzone.db"; - allow-transfer { !10/8; key one; }; + /* + * 0a00::/8 and 10/8 are the same bits, but different address + * families. This should *not* match IPv4 queries from 10.*. + */ + allow-transfer { 0a00::/8; !10/8; key one; }; }; diff --git a/lib/dns/acl.c b/lib/dns/acl.c index 53ff261fb8..ecac508454 100644 --- a/lib/dns/acl.c +++ b/lib/dns/acl.c @@ -15,7 +15,7 @@ * PERFORMANCE OF THIS SOFTWARE. */ -/* $Id: acl.c,v 1.40 2008/01/18 23:46:58 tbox Exp $ */ +/* $Id: acl.c,v 1.41 2008/01/21 20:38:54 each Exp $ */ /*! \file */ @@ -186,7 +186,7 @@ dns_acl_match(const isc_netaddr_t *reqaddr, int *match, const dns_aclelement_t **matchelt) { - isc_uint16_t bitlen; + isc_uint16_t bitlen, family; isc_prefix_t pfx; isc_radix_node_t *node; const isc_netaddr_t *addr; @@ -208,7 +208,8 @@ dns_acl_match(const isc_netaddr_t *reqaddr, } /* Always match with host addresses. */ - bitlen = reqaddr->family == AF_INET6 ? 128 : 32; + family = reqaddr->family; + bitlen = family == AF_INET6 ? 128 : 32; NETADDR_TO_PREFIX_T(addr, pfx, bitlen); /* Assume no match. */ @@ -219,8 +220,8 @@ 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_num = node->node_num[ISC_IS6(family)]; + if (*(isc_boolean_t *) node->data[ISC_IS6(family)] == ISC_TRUE) *match = match_num; else *match = -match_num; @@ -309,7 +310,7 @@ dns_acl_merge(dns_acl_t *dest, dns_acl_t *source, isc_boolean_t pos) source->elements[i].node_num + dest->node_count; /* Duplicate nested acl. */ - if(source->elements[i].type == dns_aclelementtype_nestedacl && + 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); @@ -484,24 +485,29 @@ initialize_action(void) { * insecure. */ static void -is_insecure(isc_prefix_t *prefix, void *data) { - isc_boolean_t secure = * (isc_boolean_t *)data; +is_insecure(isc_prefix_t *prefix, void **data) { + isc_boolean_t secure; + int bitlen, family; + /* Bitlen 0 means "any" or "none", which is always treated as IPv4 */ + bitlen = prefix->bitlen; + family = bitlen ? prefix->family : AF_INET; + /* Negated entries are always secure. */ + secure = * (isc_boolean_t *)data[ISC_IS6(family)]; if (!secure) { - return; + return; } - + /* If loopback prefix found, return */ - switch (prefix->family) { + switch (family) { case AF_INET: - if (prefix->bitlen == 32 && + if (bitlen == 32 && htonl(prefix->add.sin.s_addr) == INADDR_LOOPBACK) return; break; case AF_INET6: - if (prefix->bitlen == 128 && - IN6_IS_ADDR_LOOPBACK(&prefix->add.sin6)) + if (bitlen == 128 && IN6_IS_ADDR_LOOPBACK(&prefix->add.sin6)) return; break; default: @@ -509,7 +515,7 @@ is_insecure(isc_prefix_t *prefix, void *data) { } /* Non-negated, non-loopback */ - insecure_prefix_found = ISC_TRUE; + insecure_prefix_found = ISC_TRUE; /* LOCKED */ return; } diff --git a/lib/dns/iptable.c b/lib/dns/iptable.c index 2d80112df7..4823bccc23 100644 --- a/lib/dns/iptable.c +++ b/lib/dns/iptable.c @@ -14,7 +14,7 @@ * PERFORMANCE OF THIS SOFTWARE. */ -/* $Id: iptable.c,v 1.8 2008/01/18 23:46:58 tbox Exp $ */ +/* $Id: iptable.c,v 1.9 2008/01/21 20:38:54 each Exp $ */ #include #include @@ -63,24 +63,27 @@ dns_iptable_addprefix(dns_iptable_t *tab, isc_netaddr_t *addr, isc_result_t result; isc_prefix_t pfx; isc_radix_node_t *node; + int family; INSIST(DNS_IPTABLE_VALID(tab)); INSIST(tab->radix); - INSIST(bitlen <= 32 || (addr->family == AF_INET6 && bitlen <= 128)); NETADDR_TO_PREFIX_T(addr, pfx, bitlen); + /* Bitlen 0 means "any" or "none", which is always treated as IPv4 */ + family = bitlen ? pfx.family : AF_INET; + result = isc_radix_insert(tab->radix, &node, NULL, &pfx); if (result != ISC_R_SUCCESS) return(result); /* If the node already contains data, don't overwrite it */ - if (node->data == NULL) { + if (node->data[ISC_IS6(family)] == NULL) { if (pos) - node->data = &dns_iptable_pos; + node->data[ISC_IS6(family)] = &dns_iptable_pos; else - node->data = &dns_iptable_neg; + node->data[ISC_IS6(family)] = &dns_iptable_neg; } return (ISC_R_SUCCESS); @@ -110,15 +113,24 @@ dns_iptable_merge(dns_iptable_t *tab, dns_iptable_t *source, isc_boolean_t pos) * could be a security risk. To prevent this, we * just leave the negative nodes negative. */ - if (!pos && - node->data && - *(isc_boolean_t *) node->data == ISC_TRUE) - new_node->data = &dns_iptable_neg; - else - new_node->data = node->data; + if (!pos) { + if (node->data[0] && + *(isc_boolean_t *) node->data[0] == ISC_TRUE) + new_node->data[0] = &dns_iptable_neg; + else + new_node->data[0] = node->data[0]; - if (node->node_num > max_node) - max_node = node->node_num; + if (node->data[1] && + *(isc_boolean_t *) node->data[1] == ISC_TRUE) + new_node->data[1] = &dns_iptable_neg; + else + new_node->data[1] = node->data[0]; + } + + if (node->node_num[0] > max_node) + max_node = node->node_num[0]; + if (node->node_num[1] > max_node) + max_node = node->node_num[1]; } RADIX_WALK_END; tab->radix->num_added_node += max_node; diff --git a/lib/isc/include/isc/radix.h b/lib/isc/include/isc/radix.h index 147c386c4f..4c38f53d86 100644 --- a/lib/isc/include/isc/radix.h +++ b/lib/isc/include/isc/radix.h @@ -14,7 +14,7 @@ * PERFORMANCE OF THIS SOFTWARE. */ -/* $Id: radix.h,v 1.8 2008/01/18 23:46:58 tbox Exp $ */ +/* $Id: radix.h,v 1.9 2008/01/21 20:38:54 each Exp $ */ /* * This source was adapted from MRT's RCS Ids: @@ -62,7 +62,7 @@ typedef struct isc_prefix { } isc_prefix_t; typedef void (*isc_radix_destroyfunc_t)(void *); -typedef void (*isc_radix_processfunc_t)(isc_prefix_t *, void *); +typedef void (*isc_radix_processfunc_t)(isc_prefix_t *, void **); #define isc_prefix_tochar(prefix) ((char *)&(prefix)->add.sin) #define isc_prefix_touchar(prefix) ((u_char *)&(prefix)->add.sin) @@ -72,19 +72,28 @@ typedef void (*isc_radix_processfunc_t)(isc_prefix_t *, void *); /* * We need "first match" when we search the radix tree to preserve * compatibility with the existing ACL implementation. Radix trees - * naturally lend themselves to "best match". In order to get "first - * match" behavior, we remember the entries are added to the tree, - * and when a search is made, we find all matching entries, and return - * the one that was added first. + * naturally lend themselves to "best match". In order to get "first match" + * behavior, we keep track of the order in which entries are added to the + * tree--and when a search is made, we find all matching entries, and + * return the one that was added first. + * + * An IPv4 prefix and an IPv6 prefix may share a radix tree node if they + * have the same length and bit pattern (e.g., 127/8 and 7f::/8). To + * disambiguate between them, node_num and data are two-element arrays; + * node_num[0] and data[0] are used for IPv4 addresses, node_num[1] + * and data[1] for IPv6 addresses. The only exception is a prefix of + * 0/0 (aka "any" or "none"), which is always stored as IPv4 but matches + * IPv6 addresses too. */ - + +#define ISC_IS6(family) ((family) == AF_INET6 ? 1 : 0) typedef struct isc_radix_node { isc_uint32_t bit; /* bit length of the prefix */ isc_prefix_t *prefix; /* who we are in radix tree */ struct isc_radix_node *l, *r; /* left and right children */ struct isc_radix_node *parent; /* may be used */ - void *data; /* pointer to data */ - int node_num; /* which node this was in the tree, + void *data[2]; /* pointers to IPv4 and IPV6 data */ + int node_num[2]; /* which node this was in the tree, or -1 for glue nodes */ } isc_radix_node_t; diff --git a/lib/isc/radix.c b/lib/isc/radix.c index dc01dede26..f2694f3f0c 100644 --- a/lib/isc/radix.c +++ b/lib/isc/radix.c @@ -14,7 +14,7 @@ * PERFORMANCE OF THIS SOFTWARE. */ -/* $Id: radix.c,v 1.9 2007/12/20 01:48:29 marka Exp $ */ +/* $Id: radix.c,v 1.10 2008/01/21 20:38:54 each Exp $ */ /* * This source was adapted from MRT's RCS Ids: @@ -173,10 +173,12 @@ _clear_radix(isc_radix_tree_t *radix, isc_radix_destroyfunc_t func) { if (Xrn->prefix != NULL) { _deref_prefix(radix->mctx, Xrn->prefix); - if (Xrn->data != NULL && func != NULL) + if (func != NULL && (Xrn->data[0] != NULL || + Xrn->data[1] != NULL)) func(Xrn->data); } else { - INSIST(Xrn->data == NULL); + INSIST(Xrn->data[0] == NULL && + Xrn->data[1] == NULL); } isc_mem_put(radix->mctx, Xrn, sizeof(*Xrn)); @@ -231,7 +233,7 @@ isc_radix_search(isc_radix_tree_t *radix, isc_radix_node_t **target, isc_radix_node_t *node; isc_radix_node_t *stack[RADIX_MAXBITS + 1]; u_char *addr; - isc_uint32_t bitlen; + isc_uint32_t bitlen, family; int cnt = 0; REQUIRE(radix != NULL); @@ -248,6 +250,9 @@ isc_radix_search(isc_radix_tree_t *radix, isc_radix_node_t **target, addr = isc_prefix_touchar(prefix); bitlen = prefix->bitlen; + /* Bitlen 0 means "any" or "none", which is always treated as IPv4 */ + family = bitlen ? prefix->family : AF_INET; + while (node->bit < bitlen) { if (node->prefix) stack[cnt++] = node; @@ -270,8 +275,10 @@ isc_radix_search(isc_radix_tree_t *radix, isc_radix_node_t **target, if (_comp_with_mask(isc_prefix_tochar(node->prefix), isc_prefix_tochar(prefix), node->prefix->bitlen)) { - if ((*target == NULL) || - (*target)->node_num > node->node_num) + if (node->node_num[ISC_IS6(family)] != -1 && + ((*target == NULL) || + (*target)->node_num[ISC_IS6(family)] > + node->node_num[ISC_IS6(family)])) *target = node; } } @@ -289,7 +296,7 @@ isc_radix_insert(isc_radix_tree_t *radix, isc_radix_node_t **target, { isc_radix_node_t *node, *new_node, *parent, *glue = NULL; u_char *addr, *test_addr; - isc_uint32_t bitlen, check_bit, differ_bit; + isc_uint32_t bitlen, family, check_bit, differ_bit; isc_uint32_t i, j, r; isc_result_t result; @@ -301,11 +308,17 @@ isc_radix_insert(isc_radix_tree_t *radix, isc_radix_node_t **target, prefix = source->prefix; INSIST(prefix != NULL); + + bitlen = prefix->bitlen; + + /* Bitlen 0 means "any" or "none", which is always treated as IPv4 */ + family = bitlen ? prefix->family : AF_INET; + if (radix->head == NULL) { node = isc_mem_get(radix->mctx, sizeof(isc_radix_node_t)); if (node == NULL) return (ISC_R_NOMEMORY); - node->bit = prefix->bitlen; + node->bit = bitlen; result = _ref_prefix(radix->mctx, &node->prefix, prefix); if (result != ISC_R_SUCCESS) { isc_mem_put(radix->mctx, node, @@ -314,7 +327,6 @@ isc_radix_insert(isc_radix_tree_t *radix, isc_radix_node_t **target, } node->parent = NULL; node->l = node->r = NULL; - node->data = NULL; if (source != NULL) { /* * If source is non-NULL, then we're merging in a @@ -324,10 +336,20 @@ isc_radix_insert(isc_radix_tree_t *radix, isc_radix_node_t **target, * 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; + if (source->node_num[0] != -1) + node->node_num[0] = radix->num_added_node + + source->node_num[0]; + if (source->node_num[1] != -1) + node->node_num[1] = radix->num_added_node + + source->node_num[1]; + node->data[0] = source->data[0]; + node->data[1] = source->data[1]; } else { - node->node_num = ++radix->num_added_node; + node->node_num[ISC_IS6(family)] = + ++radix->num_added_node; + node->node_num[!ISC_IS6(family)] = -1; + node->data[0] = NULL; + node->data[1] = NULL; } radix->head = node; radix->num_active_node++; @@ -336,7 +358,6 @@ isc_radix_insert(isc_radix_tree_t *radix, isc_radix_node_t **target, } addr = isc_prefix_touchar(prefix); - bitlen = prefix->bitlen; node = radix->head; while (node->bit < bitlen || node->prefix == NULL) { @@ -388,19 +409,26 @@ isc_radix_insert(isc_radix_tree_t *radix, isc_radix_node_t **target, if (differ_bit == bitlen && node->bit == bitlen) { if (node->prefix != NULL) { + /* Set node_num only if it hasn't been set before */ + if (node->node_num[ISC_IS6(family)] == -1) + node->node_num[ISC_IS6(family)] = + ++radix->num_added_node; *target = node; return (ISC_R_SUCCESS); } result = _ref_prefix(radix->mctx, &node->prefix, prefix); if (result != ISC_R_SUCCESS) return (result); - INSIST(node->data == NULL && node->node_num == -1); + INSIST(node->data[0] == NULL && node->node_num[0] == -1 && + node->data[1] == NULL && node->node_num[1] == -1); if (source != NULL) { /* Merging node */ - node->node_num = radix->num_added_node + - source->node_num; + node->node_num[ISC_IS6(family)] = + radix->num_added_node + + source->node_num[ISC_IS6(family)]; } else { - node->node_num = ++radix->num_added_node; + node->node_num[ISC_IS6(family)] = + ++radix->num_added_node; } *target = node; return (ISC_R_SUCCESS); @@ -417,7 +445,7 @@ isc_radix_insert(isc_radix_tree_t *radix, isc_radix_node_t **target, return (ISC_R_NOMEMORY); } } - new_node->bit = prefix->bitlen; + new_node->bit = bitlen; result = _ref_prefix(radix->mctx, &new_node->prefix, prefix); if (result != ISC_R_SUCCESS) { isc_mem_put(radix->mctx, new_node, sizeof(isc_radix_node_t)); @@ -428,15 +456,23 @@ isc_radix_insert(isc_radix_tree_t *radix, isc_radix_node_t **target, } new_node->parent = NULL; new_node->l = new_node->r = NULL; + new_node->node_num[0] = new_node->node_num[1] = -1; radix->num_active_node++; if (source != NULL) { /* Merging node */ - new_node->node_num = radix->num_added_node + source->node_num; - new_node->data = source->data; + if (source->node_num[0] != -1) + new_node->node_num[0] = radix->num_added_node + + source->node_num[0]; + if (source->node_num[1] != -1) + new_node->node_num[1] = radix->num_added_node + + source->node_num[1]; + new_node->data[0] = source->data[0]; + new_node->data[1] = source->data[1]; } else { - new_node->node_num = ++radix->num_added_node; - new_node->data = NULL; + new_node->node_num[ISC_IS6(family)] = ++radix->num_added_node; + new_node->data[0] = NULL; + new_node->data[1] = NULL; } if (node->bit == differ_bit) { @@ -478,8 +514,8 @@ isc_radix_insert(isc_radix_tree_t *radix, isc_radix_node_t **target, glue->bit = differ_bit; glue->prefix = NULL; glue->parent = node->parent; - glue->data = NULL; - glue->node_num = -1; + glue->data[0] = glue->data[1] = NULL; + glue->node_num[0] = glue->node_num[1] = -1; radix->num_active_node++; if (differ_bit < radix->maxbits && BIT_TEST(addr[differ_bit >> 3], 0x80 >> (differ_bit & 0x07))) { @@ -522,8 +558,7 @@ isc_radix_remove(isc_radix_tree_t *radix, isc_radix_node_t *node) { _deref_prefix(radix->mctx, node->prefix); node->prefix = NULL; - /* Also I needed to clear data pointer -- masaki */ - node->data = NULL; + node->data[0] = node->data[1] = NULL; return; }