* [PATCH] DHCPv6 connection tracker helper @ 2012-02-10 2:30 Darren Willis 2012-02-10 11:18 ` Pablo Neira Ayuso 2012-02-13 9:55 ` Jan Engelhardt 0 siblings, 2 replies; 14+ messages in thread From: Darren Willis @ 2012-02-10 2:30 UTC (permalink / raw) To: netfilter-devel Adds a connection tracker helper for DHCPv6, which relies on UDP multicast solicitations to discover DHCPv6 servers. This allows DHCPv6 to work through ip6tables rulesets where non-related traffic is dropped (e.g., default fedora iptables). https://bugzilla.redhat.com/show_bug.cgi?id=656334 Signed-off-by: Darren Willis <djw@google.com> diff -rupN linux-orig/net/ipv6/netfilter/Kconfig linux-dhcpv6/net/ipv6/netfilter/Kconfig --- linux-orig/net/ipv6/netfilter/Kconfig 2011-10-24 07:10:05.000000000 +0000 +++ linux-dhcpv6/net/ipv6/netfilter/Kconfig 2012-02-10 02:05:54.000000000 +0000 @@ -25,6 +25,22 @@ config NF_CONNTRACK_IPV6 To compile it as a module, choose M here. If unsure, say N. +if NF_CONNTRACK_IPV6 + +config NF_CONNTRACK_DHCPV6 + tristate "DHCPv6 connection tracking support" + depends on NF_CONNTRACK_IPV6 + default m + help + DHCPv6 is an autoconfiguration protocol for IPv6 that makes use of + IPv6's mulitcast addresses to solicit configuration providers. This + module makes it possible for connection tracker to track replies to + these multicast solicitations, allowing DHCPv6 to function correctly. + + To compile it as a module, choose M here. If unsure, say N. + +endif # NF_CONNTRACK_IPV6 + config IP6_NF_QUEUE tristate "IP6 Userspace queueing via NETLINK (OBSOLETE)" depends on INET && IPV6 && NETFILTER diff -rupN linux-orig/net/ipv6/netfilter/Makefile linux-dhcpv6/net/ipv6/netfilter/Makefile --- linux-orig/net/ipv6/netfilter/Makefile 2011-10-24 07:10:05.000000000 +0000 +++ linux-dhcpv6/net/ipv6/netfilter/Makefile 2012-02-09 07:54:18.000000000 +0000 @@ -16,6 +16,9 @@ nf_conntrack_ipv6-y := nf_conntrack_l3 # l3 independent conntrack obj-$(CONFIG_NF_CONNTRACK_IPV6) += nf_conntrack_ipv6.o nf_defrag_ipv6.o +# IPv6-only connection tracker protocol. +obj-$(CONFIG_NF_CONNTRACK_DHCPV6) += nf_conntrack_dhcpv6.o + # defrag nf_defrag_ipv6-y := nf_defrag_ipv6_hooks.o nf_conntrack_reasm.o obj-$(CONFIG_NF_DEFRAG_IPV6) += nf_defrag_ipv6.o diff -rupN linux-orig/net/ipv6/netfilter/nf_conntrack_dhcpv6.c linux-dhcpv6/net/ipv6/netfilter/nf_conntrack_dhcpv6.c --- linux-orig/net/ipv6/netfilter/nf_conntrack_dhcpv6.c 1970-01-01 00:00:00.000000000 +0000 +++ linux-dhcpv6/net/ipv6/netfilter/nf_conntrack_dhcpv6.c 2012-02-09 07:54:32.000000000 +0000 @@ -0,0 +1,94 @@ +/* + * DHCPv6 multicast connection tracking helper. + * + * (c) 2012 Google Inc. + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version + * 2 of the License, or (at your option) any later version. + */ + +#include <linux/ip.h> +#include <net/addrconf.h> +#include <linux/skbuff.h> +#include <linux/ipv6.h> + +#include <net/netfilter/nf_conntrack.h> +#include <net/netfilter/nf_conntrack_expect.h> +#include <net/netfilter/nf_conntrack_helper.h> + +#define DHCPV6_SERVER_PORT 547 + +MODULE_AUTHOR("Darren Willis <djw@google.com>"); +MODULE_DESCRIPTION("DHCPv6 multicast connection tracking helper"); +MODULE_LICENSE("GPL"); +MODULE_ALIAS("ipv6_conntrack_dhcpv6"); +MODULE_ALIAS_NFCT_HELPER("dhcpv6"); + +static unsigned int timeout __read_mostly = 120; +module_param(timeout, uint, S_IRUSR); +MODULE_PARM_DESC(timeout, "timeout for DHCPv6 replies in seconds"); + +static int dhcpv6_help(struct sk_buff *skb, + unsigned int protoff, + struct nf_conn *ct, + enum ip_conntrack_info ctinfo) +{ + struct nf_conntrack_expect *exp; + struct iphdr *iph = ip_hdr(skb); + if (iph->version == 6) { + struct ipv6hdr *ip6h = ipv6_hdr(skb); + if (skb->sk == NULL) + goto out; + if (ipv6_addr_is_multicast(&ip6h->daddr)) { + exp = nf_ct_expect_alloc(ct); + if (exp == NULL) + goto out; + exp->tuple = ct->tuplehash[IP_CT_DIR_REPLY].tuple; + exp->tuple.src.u.udp.port = htons(DHCPV6_SERVER_PORT); + exp->mask.src.u.udp.port = htons(0xFFFF); + exp->mask.src.u.all = 0x0; + + exp->expectfn = NULL; + exp->flags = NF_CT_EXPECT_PERMANENT; + exp->class = NF_CT_EXPECT_CLASS_DEFAULT; + exp->helper = NULL; + + nf_ct_expect_related(exp); + nf_ct_expect_put(exp); + + nf_ct_refresh(ct, skb, timeout * HZ); + } + } + out: + return NF_ACCEPT; +} + +static struct nf_conntrack_expect_policy exp_policy = { + .max_expected = 1, +}; + +static struct nf_conntrack_helper helper __read_mostly = { + .name = "dhcpv6", + .tuple.src.l3num = AF_INET6, + .tuple.src.u.udp.port = htons(DHCPV6_SERVER_PORT), + .tuple.dst.protonum = IPPROTO_UDP, + .me = THIS_MODULE, + .help = dhcpv6_help, + .expect_policy = &exp_policy, +}; + +static int __init nf_conntrack_dhcpv6_init(void) +{ + exp_policy.timeout = timeout; + return nf_conntrack_helper_register(&helper); +} + +static void __exit nf_conntrack_dhcpv6_fini(void) +{ + nf_conntrack_helper_unregister(&helper); +} + +module_init(nf_conntrack_dhcpv6_init); +module_exit(nf_conntrack_dhcpv6_fini); ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] DHCPv6 connection tracker helper 2012-02-10 2:30 [PATCH] DHCPv6 connection tracker helper Darren Willis @ 2012-02-10 11:18 ` Pablo Neira Ayuso 2012-02-13 4:07 ` Darren Willis 2012-02-13 9:55 ` Jan Engelhardt 1 sibling, 1 reply; 14+ messages in thread From: Pablo Neira Ayuso @ 2012-02-10 11:18 UTC (permalink / raw) To: Darren Willis; +Cc: netfilter-devel Hi Darren, On Fri, Feb 10, 2012 at 11:30:33AM +0900, Darren Willis wrote: > Adds a connection tracker helper for DHCPv6, which relies on UDP > multicast solicitations to discover DHCPv6 servers. > > This allows DHCPv6 to work through ip6tables rulesets where > non-related traffic is dropped (e.g., default fedora iptables). > https://bugzilla.redhat.com/show_bug.cgi?id=656334 why not just adding the rule that allows udp traffic for this? I still don't see the need for this extra module if you can get it done with iptables itself. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] DHCPv6 connection tracker helper 2012-02-10 11:18 ` Pablo Neira Ayuso @ 2012-02-13 4:07 ` Darren Willis 2012-02-13 23:05 ` Pablo Neira Ayuso 0 siblings, 1 reply; 14+ messages in thread From: Darren Willis @ 2012-02-13 4:07 UTC (permalink / raw) To: Pablo Neira Ayuso; +Cc: netfilter-devel Hi Pablo, On Fri, Feb 10, 2012 at 20:18, Pablo Neira Ayuso <pablo@netfilter.org> wrote: > why not just adding the rule that allows udp traffic for this? Distros don't seem to want to (see the bug I linked where some red hat people have decided a module is the way to go). Possibly people are concerned that such a firewall rule leaves a port open on the local link permanently (and possibly with an /sbin/dhclient binary, or similar, listening on it). DHCPv4 seems to get away with it because, IIRC, it uses raw sockets and bypasses netfilter completely. So it's still open, but people don't tend to think/know about it (this isn't really a good thing...) > I still don't see the need for this extra module if you can get it > done with iptables itself. I think it's nice to firewall things as much as is feasible, and this particular case isn't really complex at all. All this module does (and all that needs doing) is lets through the first reply to the right port, and after that normal connection tracking takes care of it. Possibly in the future conntrack should have some kind of extendable broadcast/multicast helpers module that can set up simple helpers like this for various different protocols (mDNS, etc) ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] DHCPv6 connection tracker helper 2012-02-13 4:07 ` Darren Willis @ 2012-02-13 23:05 ` Pablo Neira Ayuso 0 siblings, 0 replies; 14+ messages in thread From: Pablo Neira Ayuso @ 2012-02-13 23:05 UTC (permalink / raw) To: Darren Willis; +Cc: netfilter-devel On Mon, Feb 13, 2012 at 01:07:18PM +0900, Darren Willis wrote: > Hi Pablo, > > On Fri, Feb 10, 2012 at 20:18, Pablo Neira Ayuso <pablo@netfilter.org> wrote: > > why not just adding the rule that allows udp traffic for this? > > Distros don't seem to want to (see the bug I linked where some red hat > people have decided a module is the way to go). Possibly people are > concerned that such a firewall rule leaves a port open on the local > link permanently (and possibly with an /sbin/dhclient binary, or > similar, listening on it). > DHCPv4 seems to get away with it because, IIRC, it uses raw sockets > and bypasses netfilter completely. So it's still open, but people > don't tend to think/know about it (this isn't really a good thing...) I see. > > I still don't see the need for this extra module if you can get it > > done with iptables itself. > > I think it's nice to firewall things as much as is feasible, and this > particular case isn't really complex at all. All this module does (and > all that needs doing) is lets through the first reply to the right > port, and after that normal connection tracking takes care of it. > > Possibly in the future conntrack should have some kind of extendable > broadcast/multicast helpers module that can set up simple helpers like > this for various different protocols (mDNS, etc) Yes, we need some appropriate broadcast/multicast tracking. I don't like the idea of using the expectation infrastructure for this, but well, it's what we have by now. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] DHCPv6 connection tracker helper 2012-02-10 2:30 [PATCH] DHCPv6 connection tracker helper Darren Willis 2012-02-10 11:18 ` Pablo Neira Ayuso @ 2012-02-13 9:55 ` Jan Engelhardt 2012-02-14 0:46 ` Pablo Neira Ayuso 2012-02-15 9:00 ` Darren Willis 1 sibling, 2 replies; 14+ messages in thread From: Jan Engelhardt @ 2012-02-13 9:55 UTC (permalink / raw) To: Darren Willis; +Cc: Netfilter Developer Mailing List, Pablo Neira Ayuso On Friday 2012-02-10 03:30, Darren Willis wrote: >+MODULE_ALIAS("ipv6_conntrack_dhcpv6"); I see no use for this one. >+static int dhcpv6_help(struct sk_buff *skb, >+ unsigned int protoff, >+ struct nf_conn *ct, >+ enum ip_conntrack_info ctinfo) >+{ >+ struct nf_conntrack_expect *exp; >+ struct iphdr *iph = ip_hdr(skb); >+ if (iph->version == 6) { >+ struct ipv6hdr *ip6h = ipv6_hdr(skb); >+ if (skb->sk == NULL) >+ goto out; >+ if (ipv6_addr_is_multicast(&ip6h->daddr)) { >+ exp = nf_ct_expect_alloc(ct); >+ if (exp == NULL) >+ goto out; >+ exp->tuple = ct->tuplehash[IP_CT_DIR_REPLY].tuple; This reeks of http://stackoverflow.com/a/57611/1091587 :) And an ip(v4)hdr is not very fruitful in the realm of IPv6. How about getting rid of the goto as well, as in struct nf_conntrack_expect *exp; const struct ipv6hdr *iph = ipv6_hdr(skb); if (iph->version == 6 || skb->sk == NULL || !ipv6_addr_is_multicast(&iph->daddr)) return NF_ACCEPT; exp = nf_ct_expect_alloc(ct); if (exp == NULL) return NF_ACCEPT; exp->tuple = ... ... return NF_ACCEPT; Something for further separate patches on top: You probably want to add a test for the skb actually being a DHCP message type that desires a reply (DHCPOFFER, etc.) before creating the exp. >+ nf_ct_refresh(ct, skb, timeout * HZ); > return NF_ACCEPT; >+} In all cases, NF_ACCEPT is returned - makes some eyebrows raise. Upon looking for it, nf_conntrack_ftp.c returns NF_DROP on exp==NULL, so should you. It is probably a sane thing to also check that the DHCP packet is looking valid (packet size, flag combinations, etc.) Thirdly, your module does not seem to make any attempt (yet) to handle the DHCP v6 RECONFIGURE message. The timeout value for the exp could depend on the message type; RFC 3315 defines different timeouts for different message types, up to 600 seconds for RENEW/REBOUND. (Is 600 seconds a wise thing?) >+static struct nf_conntrack_helper helper __read_mostly = { >+ .name = "dhcpv6", >+ .tuple.src.l3num = AF_INET6, >+ .tuple.src.u.udp.port = htons(DHCPV6_SERVER_PORT), >+ .tuple.dst.protonum = IPPROTO_UDP, >+ .me = THIS_MODULE, >+ .help = dhcpv6_help, >+ .expect_policy = &exp_policy, >+}; Would you know whether somebody is using UDPLITE for DHCP? ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] DHCPv6 connection tracker helper 2012-02-13 9:55 ` Jan Engelhardt @ 2012-02-14 0:46 ` Pablo Neira Ayuso 2012-02-15 9:00 ` Darren Willis 1 sibling, 0 replies; 14+ messages in thread From: Pablo Neira Ayuso @ 2012-02-14 0:46 UTC (permalink / raw) To: Jan Engelhardt; +Cc: Darren Willis, Netfilter Developer Mailing List On Mon, Feb 13, 2012 at 10:55:58AM +0100, Jan Engelhardt wrote: > On Friday 2012-02-10 03:30, Darren Willis wrote: > > >+MODULE_ALIAS("ipv6_conntrack_dhcpv6"); > > I see no use for this one. > > >+static int dhcpv6_help(struct sk_buff *skb, > >+ unsigned int protoff, > >+ struct nf_conn *ct, > >+ enum ip_conntrack_info ctinfo) > >+{ > >+ struct nf_conntrack_expect *exp; > >+ struct iphdr *iph = ip_hdr(skb); > >+ if (iph->version == 6) { > >+ struct ipv6hdr *ip6h = ipv6_hdr(skb); > >+ if (skb->sk == NULL) > >+ goto out; > >+ if (ipv6_addr_is_multicast(&ip6h->daddr)) { > >+ exp = nf_ct_expect_alloc(ct); > >+ if (exp == NULL) > >+ goto out; > >+ exp->tuple = ct->tuplehash[IP_CT_DIR_REPLY].tuple; > > This reeks of http://stackoverflow.com/a/57611/1091587 :) > And an ip(v4)hdr is not very fruitful in the realm of IPv6. > How about getting rid of the goto as well, as in > > struct nf_conntrack_expect *exp; > const struct ipv6hdr *iph = ipv6_hdr(skb); > if (iph->version == 6 || skb->sk == NULL || > !ipv6_addr_is_multicast(&iph->daddr)) > return NF_ACCEPT; > exp = nf_ct_expect_alloc(ct); > if (exp == NULL) > return NF_ACCEPT; > exp->tuple = ... > ... > return NF_ACCEPT; > > > > Something for further separate patches on top: > > You probably want to add a test for the skb actually being a > DHCP message type that desires a reply (DHCPOFFER, etc.) > before creating the exp. > > >+ nf_ct_refresh(ct, skb, timeout * HZ); > > return NF_ACCEPT; > >+} > > In all cases, NF_ACCEPT is returned - makes some eyebrows raise. > Upon looking for it, nf_conntrack_ftp.c returns NF_DROP on exp==NULL, so > should you. > It is probably a sane thing to also check that the DHCP packet is > looking valid (packet size, flag combinations, etc.) > > Thirdly, your module does not seem to make any attempt (yet) to handle > the DHCP v6 RECONFIGURE message. > > The timeout value for the exp could depend on the message type; RFC > 3315 defines different timeouts for different message types, up to 600 > seconds for RENEW/REBOUND. (Is 600 seconds a wise thing?) I agree with Jan's review. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] DHCPv6 connection tracker helper 2012-02-13 9:55 ` Jan Engelhardt 2012-02-14 0:46 ` Pablo Neira Ayuso @ 2012-02-15 9:00 ` Darren Willis 2012-02-15 17:13 ` Jan Engelhardt 1 sibling, 1 reply; 14+ messages in thread From: Darren Willis @ 2012-02-15 9:00 UTC (permalink / raw) To: Jan Engelhardt; +Cc: Netfilter Developer Mailing List, Pablo Neira Ayuso Thanks for the feedback! See below for a revised version. On Mon, Feb 13, 2012 at 18:55, Jan Engelhardt <jengelh@medozas.de> wrote: > You probably want to add a test for the skb actually being a > DHCP message type that desires a reply (DHCPOFFER, etc.) > before creating the exp. I think every (or almost every) message a client sends in DHCPv6 can expect a REPLY message. The code is now checking the message type though, just in case a client is doing something silly like sending out advertises to ff02::1:2 (in which case it won't get an expectation). > In all cases, NF_ACCEPT is returned - makes some eyebrows raise. > Upon looking for it, nf_conntrack_ftp.c returns NF_DROP on exp==NULL, so > should you. OK, doing that. > It is probably a sane thing to also check that the DHCP packet is > looking valid (packet size, flag combinations, etc.) > .... > The timeout value for the exp could depend on the message type; RFC > 3315 defines different timeouts for different message types, up to 600 > seconds for RENEW/REBOUND. (Is 600 seconds a wise thing?) I was trying to avoid digging around in the packet too much, but this is necessary (I had thought server_unicast was more widespread than it is). I've set it up to check the output message type and set the timeout appropriately. Since there's now 8 different timeouts I've dropped the parameter configuration. Client software doesn't seem too interested in letting people set these anyway. For checking the validity of a DHCP packet, there's not really very much that can be done without parsing the DHCPv6 options. I've added a length check to make sure the outgoing message has at least a message type and transaction ID, and the timeout setting-code implicitly checks that the message has a known DHCPv6 message type. It would be possible to capture outgoing messages' transaction IDs and require that incoming advertise/reply responses match it, but I'm not sure that's really going to be a big help (the clients are surely doing this already). > Thirdly, your module does not seem to make any attempt (yet) to handle > the DHCP v6 RECONFIGURE message. Right. I'm not sure of a good way to handle this; I suppose I could check initial ADVERTISE messages for the 'accept reconfigure' option, and allow the source of that advertise to have access to port 546. There's no time limit on reconfigure messages, though, so it'd basically be "throw open this port to this IP for <very long time period>." (possibly the timeout for accepting reconfiguration requests could be a parameter). Reconfigure messages require authentication, though, so this might not be so bad. > Would you know whether somebody is using UDPLITE for DHCP? I wouldn't expect it, and the DHCPv6 rfc specifically states that it uses UDP. Revised module source: ------ diff -ruN linux-orig/net/ipv6/netfilter/nf_conntrack_dhcpv6.c linux-dhcpv6/net/ipv6/netfilter/nf_conntrack_dhcpv6.c --- linux-orig/net/ipv6/netfilter/nf_conntrack_dhcpv6.c 1970-01-01 00:00:00.000000000 +0000 +++ linux-dhcpv6/net/ipv6/netfilter/nf_conntrack_dhcpv6.c 2012-02-11 08:13:49.000000000 +0000 @@ -0,0 +1,123 @@ +/* + * DHCPv6 multicast connection tracking helper. + * + * (c) 2012 Google Inc. + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version + * 2 of the License, or (at your option) any later version. + */ + +#include <linux/ip.h> +#include <net/addrconf.h> +#include <linux/skbuff.h> +#include <linux/ipv6.h> + +#include <net/netfilter/nf_conntrack.h> +#include <net/netfilter/nf_conntrack_expect.h> +#include <net/netfilter/nf_conntrack_helper.h> + +#define DHCPV6_SERVER_PORT 547 + +MODULE_AUTHOR("Darren Willis <djw@google.com>"); +MODULE_DESCRIPTION("DHCPv6 multicast connection tracking helper"); +MODULE_LICENSE("GPL"); +MODULE_ALIAS_NFCT_HELPER("dhcpv6"); + +/* Timeouts for DHCPv6 replies, in seconds, indexed by message type. */ +const static int dhcpv6_timeouts[] = { + 0, /* No message has type 0. */ + 120, /* Solicit. */ + 0, /* Advertise. */ + 30, /* Request. */ + 4, /* Confirm. */ + 600, /* Renew. */ + 600, /* Rebind. */ + 0, /* Reply. */ + 1, /* Release. */ + 1, /* Decline. */ + 0, /* Reconfigure. */ + 120, /* Information Request. */ + 0, /* Relay-forward. */ + 0 /* Relay-reply. */ +}; + +static int dhcpv6_help(struct sk_buff *skb, + unsigned int protoff, + struct nf_conn *ct, + enum ip_conntrack_info ctinfo) +{ + struct nf_conntrack_expect *exp; + struct ipv6hdr *iph = ipv6_hdr(skb); + const struct udphdr *udph; + struct udphdr _udph; + int timeout = 0; + short udp_len = 0; + char typebuf = 0; + unsigned char *type = 0; + + if (iph->version != 6 || skb->sk == NULL || + !ipv6_addr_is_multicast(&iph->daddr)) + return NF_ACCEPT; + + udph = skb_header_pointer(skb, protoff, sizeof(_udph), &_udph); + udp_len = ntohs(udph->len); + /* DHCPv6 messages are at least 4 bytes long. */ + if (udph == NULL || udp_len < 12) { + return NF_ACCEPT; + } + + type = skb_header_pointer(skb, protoff + sizeof(_udph), + sizeof(typebuf), &typebuf); + if (type == NULL || *type >= ARRAY_SIZE(dhcpv6_timeouts) || + dhcpv6_timeouts[*type] == 0) + return NF_ACCEPT; + timeout = dhcpv6_timeouts[*type]; + + exp = nf_ct_expect_alloc(ct); + if (exp == NULL) + return NF_DROP; + exp->tuple = ct->tuplehash[IP_CT_DIR_REPLY].tuple; + exp->tuple.src.u.udp.port = htons(DHCPV6_SERVER_PORT); + exp->mask.src.u.udp.port = htons(0xFFFF); + exp->mask.src.u.all = 0x0; + + exp->expectfn = NULL; + exp->flags = NF_CT_EXPECT_PERMANENT; + exp->class = NF_CT_EXPECT_CLASS_DEFAULT; + exp->helper = NULL; + + nf_ct_expect_related(exp); + nf_ct_expect_put(exp); + + nf_ct_refresh(ct, skb, timeout * HZ); + return NF_ACCEPT; +} + +static struct nf_conntrack_expect_policy exp_policy = { + .max_expected = 1, +}; + +static struct nf_conntrack_helper helper __read_mostly = { + .name = "dhcpv6", + .tuple.src.l3num = AF_INET6, + .tuple.src.u.udp.port = htons(DHCPV6_SERVER_PORT), + .tuple.dst.protonum = IPPROTO_UDP, + .me = THIS_MODULE, + .help = dhcpv6_help, + .expect_policy = &exp_policy, +}; + +static int __init nf_conntrack_dhcpv6_init(void) +{ + return nf_conntrack_helper_register(&helper); +} + +static void __exit nf_conntrack_dhcpv6_fini(void) +{ + nf_conntrack_helper_unregister(&helper); +} + +module_init(nf_conntrack_dhcpv6_init); +module_exit(nf_conntrack_dhcpv6_fini); ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] DHCPv6 connection tracker helper 2012-02-15 9:00 ` Darren Willis @ 2012-02-15 17:13 ` Jan Engelhardt 2012-02-16 4:56 ` Darren Willis 0 siblings, 1 reply; 14+ messages in thread From: Jan Engelhardt @ 2012-02-15 17:13 UTC (permalink / raw) To: Darren Willis; +Cc: Netfilter Developer Mailing List, Pablo Neira Ayuso On Wednesday 2012-02-15 10:00, Darren Willis wrote: > >For checking the validity of a DHCP packet, there's not really very >much that can be done without parsing the DHCPv6 options. I've added a >length check to make sure the outgoing message has at least a message >type and transaction ID, and the timeout setting-code implicitly >checks that the message has a known DHCPv6 message type. Seems sufficient indeed. >It would be possible to capture outgoing messages' transaction IDs and >require that incoming advertise/reply responses match it, but I'm not >sure that's really going to be a big help (the clients are surely >doing this already). Certainly would be on my wishlist if such an extension does not look too ugly. >> Thirdly, your module does not seem to make any attempt (yet) to handle >> the DHCP v6 RECONFIGURE message. > >Right. I'm not sure of a good way to handle this; I suppose I could >check initial ADVERTISE messages for the 'accept reconfigure' option, >and allow the source of that advertise to have access to port 546. >There's no time limit on reconfigure messages,[...] Irk. I would suggest doing like the ICMPv6 tracker does, marking it unconditionally as INVALID to indicate that the implementation chose to not track RECONFIGURE on purpose. >+static int dhcpv6_help(struct sk_buff *skb, >+ unsigned int protoff, >+ struct nf_conn *ct, >+ enum ip_conntrack_info ctinfo) >+{ >... >+ unsigned char *type = 0; = NULL. >+static struct nf_conntrack_helper helper __read_mostly = { >+ .name = "dhcpv6", >+ .tuple.src.l3num = AF_INET6, This should be NFPROTO_IPV6 actually. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] DHCPv6 connection tracker helper 2012-02-15 17:13 ` Jan Engelhardt @ 2012-02-16 4:56 ` Darren Willis 2012-02-24 17:54 ` Pablo Neira Ayuso 0 siblings, 1 reply; 14+ messages in thread From: Darren Willis @ 2012-02-16 4:56 UTC (permalink / raw) To: Jan Engelhardt; +Cc: Netfilter Developer Mailing List, Pablo Neira Ayuso On Thu, Feb 16, 2012 at 02:13, Jan Engelhardt <jengelh@medozas.de> wrote: >>It would be possible to capture outgoing messages' transaction IDs and >>require that incoming advertise/reply responses match it, but I'm not >>sure that's really going to be a big help (the clients are surely >>doing this already). > > Certainly would be on my wishlist if such an extension does not look > too ugly. Probably not too ugly. Transaction IDs are effectively one-shot, except for solicit/advertise, so just keeping a couple of arrays of sent transaction IDs and checking incoming messages for a match could work (one array for solicit transaction IDs, which can get multiple replies, and one array for 'normal' transaction IDs, which can't). Array size could reasonably be expected to be 1 entry per DHCPv6 server, I think. There'd need to be a scheme for evicting old IDs as well. I'm considering this a bit of a to-do-later though. Oh, right now this module has an expect_policy with max expected of 1, which I'm assuming is a hard limit of 1 expectation. Not sure what the usual decision is on this - typical case is going to be 1 DHCPv6 server, but more are allowed. > Irk. I would suggest doing like the ICMPv6 tracker does, > marking it unconditionally as INVALID to indicate that the > implementation chose to not track RECONFIGURE on purpose. The thing with reconfigure is I've got nowhere to track it (sensibly). RECONFIGURE just appears out of the blue from a DHCPv6 server. Right now this module's only deals with messages from the client, I don't examine incoming DHCPv6 packets at all, I just set up expectations for them. Another revision with your changes and a bugfix (I was dereferencing udph before checking it was NULL in the last one...) ---- diff -ruN linux-orig/net/ipv6/netfilter/Kconfig linux-dhcpv6/net/ipv6/netfilter/Kconfig --- linux-orig/net/ipv6/netfilter/Kconfig 2011-10-24 07:10:05.000000000 +0000 +++ linux-dhcpv6/net/ipv6/netfilter/Kconfig 2012-02-10 02:05:54.000000000 +0000 @@ -25,6 +25,22 @@ To compile it as a module, choose M here. If unsure, say N. +if NF_CONNTRACK_IPV6 + +config NF_CONNTRACK_DHCPV6 + tristate "DHCPv6 connection tracking support" + depends on NF_CONNTRACK_IPV6 + default m + help + DHCPv6 is an autoconfiguration protocol for IPv6 that makes use of + IPv6's mulitcast addresses to solicit configuration providers. This + module makes it possible for connection tracker to track replies to + these multicast solicitations, allowing DHCPv6 to function correctly. + + To compile it as a module, choose M here. If unsure, say N. + +endif # NF_CONNTRACK_IPV6 + config IP6_NF_QUEUE tristate "IP6 Userspace queueing via NETLINK (OBSOLETE)" depends on INET && IPV6 && NETFILTER diff -ruN linux-orig/net/ipv6/netfilter/Makefile linux-dhcpv6/net/ipv6/netfilter/Makefile --- linux-orig/net/ipv6/netfilter/Makefile 2011-10-24 07:10:05.000000000 +0000 +++ linux-dhcpv6/net/ipv6/netfilter/Makefile 2012-02-09 07:54:18.000000000 +0000 @@ -16,6 +16,9 @@ # l3 independent conntrack obj-$(CONFIG_NF_CONNTRACK_IPV6) += nf_conntrack_ipv6.o nf_defrag_ipv6.o +# IPv6-only connection tracker protocol. +obj-$(CONFIG_NF_CONNTRACK_DHCPV6) += nf_conntrack_dhcpv6.o + # defrag nf_defrag_ipv6-y := nf_defrag_ipv6_hooks.o nf_conntrack_reasm.o obj-$(CONFIG_NF_DEFRAG_IPV6) += nf_defrag_ipv6.o diff -ruN linux-orig/net/ipv6/netfilter/nf_conntrack_dhcpv6.c linux-dhcpv6/net/ipv6/netfilter/nf_conntrack_dhcpv6.c --- linux-orig/net/ipv6/netfilter/nf_conntrack_dhcpv6.c 1970-01-01 00:00:00.000000000 +0000 +++ linux-dhcpv6/net/ipv6/netfilter/nf_conntrack_dhcpv6.c 2012-02-12 04:47:32.000000000 +0000 @@ -0,0 +1,122 @@ +/* + * DHCPv6 multicast connection tracking helper. + * + * (c) 2012 Google Inc. + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version + * 2 of the License, or (at your option) any later version. + */ + +#include <linux/ip.h> +#include <net/addrconf.h> +#include <linux/skbuff.h> +#include <linux/ipv6.h> + +#include <net/netfilter/nf_conntrack.h> +#include <net/netfilter/nf_conntrack_expect.h> +#include <net/netfilter/nf_conntrack_helper.h> + +#define DHCPV6_SERVER_PORT 547 + +MODULE_AUTHOR("Darren Willis <djw@google.com>"); +MODULE_DESCRIPTION("DHCPv6 multicast connection tracking helper"); +MODULE_LICENSE("GPL"); +MODULE_ALIAS_NFCT_HELPER("dhcpv6"); + +/* Timeouts for DHCPv6 replies, in seconds, indexed by message type. */ +const static int dhcpv6_timeouts[] = { + 0, /* No message has type 0. */ + 120, /* Solicit. */ + 0, /* Advertise. */ + 30, /* Request. */ + 4, /* Confirm. */ + 600, /* Renew. */ + 600, /* Rebind. */ + 0, /* Reply. */ + 1, /* Release. */ + 1, /* Decline. */ + 0, /* Reconfigure. */ + 120, /* Information Request. */ + 0, /* Relay-forward. */ + 0 /* Relay-reply. */ +}; + +static int dhcpv6_help(struct sk_buff *skb, + unsigned int protoff, + struct nf_conn *ct, + enum ip_conntrack_info ctinfo) +{ + struct nf_conntrack_expect *exp; + struct ipv6hdr *iph = ipv6_hdr(skb); + const struct udphdr *udph; + struct udphdr _udph; + int typeint = 0; + int timeout = 0; + char typebuf = 0; + unsigned char *type = NULL; + + if (iph->version != 6 || skb->sk == NULL || + !ipv6_addr_is_multicast(&iph->daddr)) + return NF_ACCEPT; + + udph = skb_header_pointer(skb, protoff, sizeof(_udph), &_udph); + /* DHCPv6 messages are at least 4 bytes long. */ + if (udph == NULL || ntohs(udph->len) < 12 || + ntohs(udph->len) != skb->len - protoff) { + return NF_ACCEPT; + } + + type = skb_header_pointer(skb, protoff + sizeof(_udph), sizeof(typebuf), &typebuf); + if (type == NULL || *type >= ARRAY_SIZE(dhcpv6_timeouts) || + dhcpv6_timeouts[*type] == 0) { + return NF_ACCEPT; + } + timeout = dhcpv6_timeouts[*type]; + exp = nf_ct_expect_alloc(ct); + if (exp == NULL) + return NF_DROP; + exp->tuple = ct->tuplehash[IP_CT_DIR_REPLY].tuple; + exp->tuple.src.u.udp.port = htons(DHCPV6_SERVER_PORT); + exp->mask.src.u.udp.port = htons(0xFFFF); + exp->mask.src.u.all = 0x0; + + exp->expectfn = NULL; + exp->flags = NF_CT_EXPECT_PERMANENT; + exp->class = NF_CT_EXPECT_CLASS_DEFAULT; + exp->helper = NULL; + + nf_ct_expect_related(exp); + nf_ct_expect_put(exp); + + nf_ct_refresh(ct, skb, timeout * HZ); + return NF_ACCEPT; +} + +static struct nf_conntrack_expect_policy exp_policy = { + .max_expected = 1, +}; + +static struct nf_conntrack_helper helper __read_mostly = { + .name = "dhcpv6", + .tuple.src.l3num = NFPROTO_IPV6, + .tuple.src.u.udp.port = htons(DHCPV6_SERVER_PORT), + .tuple.dst.protonum = IPPROTO_UDP, + .me = THIS_MODULE, + .help = dhcpv6_help, + .expect_policy = &exp_policy, +}; + +static int __init nf_conntrack_dhcpv6_init(void) +{ + return nf_conntrack_helper_register(&helper); +} + +static void __exit nf_conntrack_dhcpv6_fini(void) +{ + nf_conntrack_helper_unregister(&helper); +} + +module_init(nf_conntrack_dhcpv6_init); +module_exit(nf_conntrack_dhcpv6_fini); ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] DHCPv6 connection tracker helper 2012-02-16 4:56 ` Darren Willis @ 2012-02-24 17:54 ` Pablo Neira Ayuso 2012-02-27 4:18 ` Darren Willis 0 siblings, 1 reply; 14+ messages in thread From: Pablo Neira Ayuso @ 2012-02-24 17:54 UTC (permalink / raw) To: Darren Willis; +Cc: Jan Engelhardt, Netfilter Developer Mailing List On Thu, Feb 16, 2012 at 01:56:01PM +0900, Darren Willis wrote: > On Thu, Feb 16, 2012 at 02:13, Jan Engelhardt <jengelh@medozas.de> wrote: > >>It would be possible to capture outgoing messages' transaction IDs and > >>require that incoming advertise/reply responses match it, but I'm not > >>sure that's really going to be a big help (the clients are surely > >>doing this already). > > > > Certainly would be on my wishlist if such an extension does not look > > too ugly. > > Probably not too ugly. Transaction IDs are effectively one-shot, > except for solicit/advertise, so just keeping a couple of arrays of > sent transaction IDs and checking incoming messages for a match could > work (one array for solicit transaction IDs, which can get multiple > replies, and one array for 'normal' transaction IDs, which can't). > Array size could reasonably be expected to be 1 entry per DHCPv6 > server, I think. There'd need to be a scheme for evicting old IDs as > well. I'm considering this a bit of a to-do-later though. > > Oh, right now this module has an expect_policy with max expected of 1, > which I'm assuming is a hard limit of 1 expectation. Not sure what the > usual decision is on this - typical case is going to be 1 DHCPv6 > server, but more are allowed. > > > Irk. I would suggest doing like the ICMPv6 tracker does, > > marking it unconditionally as INVALID to indicate that the > > implementation chose to not track RECONFIGURE on purpose. > > The thing with reconfigure is I've got nowhere to track it (sensibly). > RECONFIGURE just appears out of the blue from a DHCPv6 server. Right > now this module's only deals with messages from the client, I don't > examine incoming DHCPv6 packets at all, I just set up expectations for > them. > > Another revision with your changes and a bugfix (I was dereferencing > udph before checking it was NULL in the last one...) Thanks for the patch. I still have two concerns with this: 1) In the last Netfilter workshop, we decided that we're targeting towards explicit helper configuration via iptables, ie. something like: ip6tables -I OUTPUT -t raw -s $SRC -d $DST \ -p udp --dport 547 -j CT --helper dhcpv6 According to your report, this is exactly what distributors don't want to do. The reason why we're doing this was that we have detected problems in default configuration of non-broadcast helpers (read Eric Leblond article): http://home.regit.org/netfilter-en/secure-use-of-helpers/ If we move towards explicit helper configuration, distributors will have to add the rule above. 2) The helper infrastructure is allowing us to filter broadcast traffic but I think that it's been designed for a different purpose. I know, we don't have any better by now. But in the meanwhile, we're adding specific helpers to support each broadcast protocol. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] DHCPv6 connection tracker helper 2012-02-24 17:54 ` Pablo Neira Ayuso @ 2012-02-27 4:18 ` Darren Willis 2012-02-28 23:54 ` Pablo Neira Ayuso 0 siblings, 1 reply; 14+ messages in thread From: Darren Willis @ 2012-02-27 4:18 UTC (permalink / raw) To: Pablo Neira Ayuso; +Cc: Jan Engelhardt, Netfilter Developer Mailing List > 1) In the last Netfilter workshop, we decided that we're targeting > towards explicit helper configuration via iptables, ie. something > like: > > ip6tables -I OUTPUT -t raw -s $SRC -d $DST \ > -p udp --dport 547 -j CT --helper dhcpv6 > > According to your report, this is exactly what distributors don't > want to do. Interesting. Well, my impression is that distributions don't wan't to add rules, but if they can't avoid it, they'll just have to cope. Is this changeover coming in the immediate future? > 2) The helper infrastructure is allowing us to filter broadcast > traffic but I think that it's been designed for a different purpose. > I know, we don't have any better by now. But in the meanwhile, we're > adding specific helpers to support each broadcast protocol. Agreed, while I think for now this helper is fine, I think it'd be nice to have a more generic multicast/broadcast helper, although it'd still need to have specific protocols baked into it to work (maybe netbios, dhcpv6, mDNS, LLMNR, SSDP, neighbour discovery, other things). -- 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] DHCPv6 connection tracker helper 2012-02-27 4:18 ` Darren Willis @ 2012-02-28 23:54 ` Pablo Neira Ayuso 2012-03-02 3:59 ` Darren Willis 0 siblings, 1 reply; 14+ messages in thread From: Pablo Neira Ayuso @ 2012-02-28 23:54 UTC (permalink / raw) To: Darren Willis; +Cc: Jan Engelhardt, Netfilter Developer Mailing List On Mon, Feb 27, 2012 at 01:18:58PM +0900, Darren Willis wrote: > > 1) In the last Netfilter workshop, we decided that we're targeting > > towards explicit helper configuration via iptables, ie. something > > like: > > > > ip6tables -I OUTPUT -t raw -s $SRC -d $DST \ > > -p udp --dport 547 -j CT --helper dhcpv6 > > > > According to your report, this is exactly what distributors don't > > want to do. > > Interesting. Well, my impression is that distributions don't wan't to > add rules, but if they can't avoid it, they'll just have to cope. > Is this changeover coming in the immediate future? Yes. I'd like to send a patch for RFC to the mailing list any time soon. I'll include you in the CC. > > 2) The helper infrastructure is allowing us to filter broadcast > > traffic but I think that it's been designed for a different purpose. > > I know, we don't have any better by now. But in the meanwhile, we're > > adding specific helpers to support each broadcast protocol. > > Agreed, while I think for now this helper is fine, I think it'd be > nice to have a more generic multicast/broadcast helper, although it'd > still need to have specific protocols baked into it to work (maybe > netbios, dhcpv6, mDNS, LLMNR, SSDP, neighbour discovery, other > things). This is exactly what scares me. I don't like the idea of bloating the kernel with lots of helpers for each single protocol. I'm currently working on one user-space helper infrastructure. We can use that infrastructure to implement this helper and many others. I've got the patch in one branch of my kernel tree, it's still experimental stuff, but I expect to have it done soon. Would you be OK with we make this (and other helpers that will surely follow up) in user-space? -- 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] DHCPv6 connection tracker helper 2012-02-28 23:54 ` Pablo Neira Ayuso @ 2012-03-02 3:59 ` Darren Willis 2012-03-03 13:35 ` Pablo Neira Ayuso 0 siblings, 1 reply; 14+ messages in thread From: Darren Willis @ 2012-03-02 3:59 UTC (permalink / raw) To: Pablo Neira Ayuso; +Cc: Jan Engelhardt, Netfilter Developer Mailing List On Wed, Feb 29, 2012 at 08:54, Pablo Neira Ayuso <pablo@netfilter.org> wrote: > Yes. I'd like to send a patch for RFC to the mailing list any time > soon. I'll include you in the CC. Thanks, sounds interesting. > I'm currently working on one user-space helper infrastructure. We can > use that infrastructure to implement this helper and many others. > > I've got the patch in one branch of my kernel tree, it's still > experimental stuff, but I expect to have it done soon. > > Would you be OK with we make this (and other helpers that will surely > follow up) in user-space? Well, I think long-term that's the way to go. I'd still like to get this in its current form, if possible, since it's a simple thing in a pretty OK state right now, so it could be ready to go quite quickly. Whereas, although I think your helper framework sounds great and is the right thing to do, it's also an unknown amount of time away. Unless it's going to be landing in the next few weeks? Of course, I don't want to make a lot of work for you - I'll be more than happy to revisit this, remove the dhcpv6 helper and re-implement it for your new helper framework when it's ready for external contributions. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] DHCPv6 connection tracker helper 2012-03-02 3:59 ` Darren Willis @ 2012-03-03 13:35 ` Pablo Neira Ayuso 0 siblings, 0 replies; 14+ messages in thread From: Pablo Neira Ayuso @ 2012-03-03 13:35 UTC (permalink / raw) To: Darren Willis; +Cc: Jan Engelhardt, Netfilter Developer Mailing List On Fri, Mar 02, 2012 at 12:59:15PM +0900, Darren Willis wrote: > On Wed, Feb 29, 2012 at 08:54, Pablo Neira Ayuso <pablo@netfilter.org> wrote: > > Yes. I'd like to send a patch for RFC to the mailing list any time > > soon. I'll include you in the CC. > > Thanks, sounds interesting. > > > I'm currently working on one user-space helper infrastructure. We can > > use that infrastructure to implement this helper and many others. > > > > I've got the patch in one branch of my kernel tree, it's still > > experimental stuff, but I expect to have it done soon. > > > > Would you be OK with we make this (and other helpers that will surely > > follow up) in user-space? > > Well, I think long-term that's the way to go. I'd still like to get > this in its current form, if possible, since it's a simple thing in a > pretty OK state right now, so it could be ready to go quite quickly. > Whereas, although I think your helper framework sounds great and is > the right thing to do, it's also an unknown amount of time away. > Unless it's going to be landing in the next few weeks? If we apply this, we'll have to live with it for long time until we can remove. It's easy to get something it, but not evident to get it out because somebody will be using it. > Of course, I don't want to make a lot of work for you - I'll be more > than happy to revisit this, remove the dhcpv6 helper and re-implement > it for your new helper framework when it's ready for external > contributions. One firewall vendor is supporting the development of this helper user-space infrastructure, so we'll have the infrastructure soon (by next kernel release cycle). Please, ping me in three months or so if you see no advances in this regard. ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2012-03-03 13:35 UTC | newest] Thread overview: 14+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-02-10 2:30 [PATCH] DHCPv6 connection tracker helper Darren Willis 2012-02-10 11:18 ` Pablo Neira Ayuso 2012-02-13 4:07 ` Darren Willis 2012-02-13 23:05 ` Pablo Neira Ayuso 2012-02-13 9:55 ` Jan Engelhardt 2012-02-14 0:46 ` Pablo Neira Ayuso 2012-02-15 9:00 ` Darren Willis 2012-02-15 17:13 ` Jan Engelhardt 2012-02-16 4:56 ` Darren Willis 2012-02-24 17:54 ` Pablo Neira Ayuso 2012-02-27 4:18 ` Darren Willis 2012-02-28 23:54 ` Pablo Neira Ayuso 2012-03-02 3:59 ` Darren Willis 2012-03-03 13:35 ` Pablo Neira Ayuso
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).