[PATCH] DHCPv6 connection tracker helper

[PATCH] DHCPv6 connection tracker helper

[Darren Willis]

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);

Re: [PATCH] DHCPv6 connection tracker helper

[Pablo Neira Ayuso]

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.

Re: [PATCH] DHCPv6 connection tracker helper

[Darren Willis]

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)

Re: [PATCH] DHCPv6 connection tracker helper

[Jan Engelhardt]

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?

Re: [PATCH] DHCPv6 connection tracker helper

[Pablo Neira Ayuso]

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.

Re: [PATCH] DHCPv6 connection tracker helper

[Pablo Neira Ayuso]

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.

Re: [PATCH] DHCPv6 connection tracker helper

[Darren Willis]

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);

Re: [PATCH] DHCPv6 connection tracker helper

[Jan Engelhardt]

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.

Re: [PATCH] DHCPv6 connection tracker helper

[Darren Willis]

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);

Re: [PATCH] DHCPv6 connection tracker helper

[Pablo Neira Ayuso]

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.

Re: [PATCH] DHCPv6 connection tracker helper

[Darren Willis]

> 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

Re: [PATCH] DHCPv6 connection tracker helper

[Pablo Neira Ayuso]

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

Re: [PATCH] DHCPv6 connection tracker helper

[Darren Willis]

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.

Re: [PATCH] DHCPv6 connection tracker helper

[Pablo Neira Ayuso]

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.