| Message ID | 20170308162658.5697-4-jengelh@inai.de |
|---|---|
| State | Not Applicable |
| Delegated to: | Pablo Neira |
| Headers | show |
On Wed, Mar 08, 2017 at 05:26:58PM +0100, Jan Engelhardt wrote: > A long-standing problem has been that `iptables -s any_host_here` > could yield multiple rules with the same address if the DNS was > indeed so populated. When did anyone report this problem out of the localhost case? -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wednesday 2017-03-08 17:45, Pablo Neira Ayuso wrote: >On Wed, Mar 08, 2017 at 05:26:58PM +0100, Jan Engelhardt wrote: >> A long-standing problem has been that `iptables -s any_host_here` >> could yield multiple rules with the same address if the DNS was >> indeed so populated. > >When did anyone report this problem out of the localhost case? It's been a long time. I think the issue was actually that one can specify multiple host names, and if those hostnames happen to resolve to the same address in the end, iptables would emit two rules of which one is essentially redundant. iptables -A INPUT -s www2.company.com,www3.company.com -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Mar 08, 2017 at 05:56:43PM +0100, Jan Engelhardt wrote: > On Wednesday 2017-03-08 17:45, Pablo Neira Ayuso wrote: > > >On Wed, Mar 08, 2017 at 05:26:58PM +0100, Jan Engelhardt wrote: > >> A long-standing problem has been that `iptables -s any_host_here` > >> could yield multiple rules with the same address if the DNS was > >> indeed so populated. > > > >When did anyone report this problem out of the localhost case? > > It's been a long time. I think the issue was actually that one can > specify multiple host names, and if those hostnames happen to resolve to > the same address in the end, iptables would emit two rules of which one > is essentially redundant. > > iptables -A INPUT -s www2.company.com,www3.company.com Got me thinking, are you sure we want to fix this? I think this rule expansion based on DNS is probably one of the most creepy features that we have in iptables... Probably if we leave it broken this will just scare people away from using this :). But your call. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/libxtables/xtables.c b/libxtables/xtables.c index 891d81a..d994306 100644 --- a/libxtables/xtables.c +++ b/libxtables/xtables.c @@ -1364,7 +1364,7 @@ static struct in_addr *host_to_ipaddr(const char *name, unsigned int *naddr) struct addrinfo hints; struct addrinfo *res, *p; int err; - unsigned int i; + size_t i, j; memset(&hints, 0, sizeof(hints)); hints.ai_family = AF_INET; @@ -1374,14 +1374,23 @@ static struct in_addr *host_to_ipaddr(const char *name, unsigned int *naddr) err = getaddrinfo(name, NULL, &hints, &res); if (err != 0) return NULL; - for (p = res; p != NULL; p = p->ai_next) - ++*naddr; addr = xtables_calloc(*naddr, sizeof(struct in_addr)); - for (i = 0, p = res; p != NULL; p = p->ai_next) - memcpy(&addr[i++], - &((const struct sockaddr_in *)p->ai_addr)->sin_addr, - sizeof(struct in_addr)); + for (i = 0, p = res; p != NULL; p = p->ai_next) { + const struct sockaddr_in *newaddr = (const void *)p->ai_addr; + bool seen = false; + + for (j = 0; j < i; ++j) { + if (memcmp(newaddr, &addr[j], sizeof(*newaddr)) == 0) { + seen = true; + break; + } + } + if (seen) + continue; + memcpy(&addr[i++], &newaddr->sin_addr, sizeof(newaddr->sin_addr)); + } freeaddrinfo(res); + *naddr = i; return addr; } @@ -1650,7 +1659,7 @@ host_to_ip6addr(const char *name, unsigned int *naddr) struct addrinfo hints; struct addrinfo *res, *p; int err; - unsigned int i; + size_t i, j; memset(&hints, 0, sizeof(hints)); hints.ai_family = AF_INET6; @@ -1665,11 +1674,22 @@ host_to_ip6addr(const char *name, unsigned int *naddr) ++*naddr; /* Copy each element of the address chain */ addr = xtables_calloc(*naddr, sizeof(struct in6_addr)); - for (i = 0, p = res; p != NULL; p = p->ai_next) - memcpy(&addr[i++], - &((const struct sockaddr_in6 *)p->ai_addr)->sin6_addr, - sizeof(struct in6_addr)); + for (i = 0, p = res; p != NULL; p = p->ai_next) { + const struct sockaddr_in6 *newaddr = (const void *)p->ai_addr; + bool seen = false; + + for (j = 0; j < i; ++j) { + if (memcmp(newaddr, &addr[j], sizeof(*newaddr)) == 0) { + seen = true; + break; + } + } + if (seen) + continue; + memcpy(&addr[i++], &newaddr->sin6_addr, sizeof(newaddr->sin6_addr)); + } freeaddrinfo(res); + *naddr = i; return addr; }
A long-standing problem has been that `iptables -s any_host_here` could yield multiple rules with the same address if the DNS was indeed so populated. Signed-off-by: Jan Engelhardt <jengelh@inai.de> --- libxtables/xtables.c | 44 ++++++++++++++++++++++++++++++++------------ 1 file changed, 32 insertions(+), 12 deletions(-)