IPv6 conntrack support for neighbour discovery

IPv6 conntrack support for neighbour discovery

[Marek Szuba]

[-- Attachment #1: Type: text/plain, Size: 2402 bytes --]

Hello everyone,

Following a suggestion I was given at the kernel Bugzilla I am
forwarding this here.

It has come to my attention lately that the only two ICMPv6 packet
types which IPv6 conntrack allows at present to initiate new
connections are "echo request" and "node information query"; all other
types appearing in this context are considered invalid. In particular,
this includes two neighbour-discovery types specified in RFC 2461:
"router solicitation" and "neighbour solicitation" - meaning that if a
server drops all INVALID packets early in its INPUT or OUTPUT chain of
IPv6 netfilter's filter table, neither of the two can be received from
clients. Given that router solicitation is used for stateless network
autoconfiguration and neighbour solicitation is an IPv6 equivalent of
ARP requests, having these two dropped can be quite bad - especially
the latter, as it pretty much kills IPv6 over Ethernet.

Steps to reproduce:

1. Obtain an IPv6 address assigned to an Ethernet interface of host A
(link-local will do);
2. If necesssary, flush all ip6tables filter chains on host A and set
their policies to ACCEPT;
3. From host B on the same Ethernet network, ping6 the aforementioned IP
address to verify that everything works;
4. On host A, run something like "ip6tables -I INPUT 1 -m state --state
INVALID -j DROP";
5. Try to repeat step 3. This time the ping should fail and counters
associated with the aforementioned rule should increment;
6. Repeat all the above using OUTPUT instead of INPUT, with the same
final result.

BTW. This problem was discovered and (hopefully) fixed collaboratively
by Łukasz Stelmach <steelman@post.pl> and myself, please attribute both
of us should the patch make it into code repositories.

Attaching a proposed solution to the problem - the enclosed patch adds
"router solicitation" and "neighbour solicitation" to the list of
ICMPv6 types IPv6 conntrack considers allowed to initiate new
connections. Note that in principle "router advertisement" ICMPv6
packets could also be considered valid-new, as e.g. radvd periodically
advertises its presence to the local network - as however it is not
necessary to let this through as new to have things work (plus we
aren't that big on broadcasting, even on supposedly-secure networks),
the patch only adds solicitations to the list.

Best regards,
-- 
MS

[-- Attachment #2: nf_conntrack_proto_icmpv6.c-ndiscIsValid.patch --]
[-- Type: text/x-patch, Size: 459 bytes --]

--- a/net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c	2008-04-17 04:49:44.000000000 +0200
+++ b/net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c	2008-10-20 16:49:09.000000000 +0200
@@ -108,6 +108,8 @@
 {
 	static const u_int8_t valid_new[] = {
 		[ICMPV6_ECHO_REQUEST - 128] = 1,
+		[NDISC_ROUTER_SOLICITATION - 128] = 1,
+		[NDISC_NEIGHBOUR_SOLICITATION - 128] = 1,
 		[ICMPV6_NI_QUERY - 128] = 1
 	};
 	int type = ct->tuplehash[0].tuple.dst.u.icmp.type - 128;

Re: IPv6 conntrack support for neighbour discovery

[Patrick McHardy]

Marek Szuba wrote:
> Hello everyone,
> 
> Following a suggestion I was given at the kernel Bugzilla I am
> forwarding this here.
> 
> It has come to my attention lately that the only two ICMPv6 packet
> types which IPv6 conntrack allows at present to initiate new
> connections are "echo request" and "node information query"; all other
> types appearing in this context are considered invalid. In particular,
> this includes two neighbour-discovery types specified in RFC 2461:
> "router solicitation" and "neighbour solicitation" - meaning that if a
> server drops all INVALID packets early in its INPUT or OUTPUT chain of
> IPv6 netfilter's filter table, neither of the two can be received from
> clients. Given that router solicitation is used for stateless network
> autoconfiguration and neighbour solicitation is an IPv6 equivalent of
> ARP requests, having these two dropped can be quite bad - especially
> the latter, as it pretty much kills IPv6 over Ethernet.
> 
> Steps to reproduce:
> 
> 1. Obtain an IPv6 address assigned to an Ethernet interface of host A
> (link-local will do);
> 2. If necesssary, flush all ip6tables filter chains on host A and set
> their policies to ACCEPT;
> 3. From host B on the same Ethernet network, ping6 the aforementioned IP
> address to verify that everything works;
> 4. On host A, run something like "ip6tables -I INPUT 1 -m state --state
> INVALID -j DROP";
> 5. Try to repeat step 3. This time the ping should fail and counters
> associated with the aforementioned rule should increment;
> 6. Repeat all the above using OUTPUT instead of INPUT, with the same
> final result.
> 
> BTW. This problem was discovered and (hopefully) fixed collaboratively
> by Łukasz Stelmach <steelman@post.pl> and myself, please attribute both
> of us should the patch make it into code repositories.
> 
> Attaching a proposed solution to the problem - the enclosed patch adds
> "router solicitation" and "neighbour solicitation" to the list of
> ICMPv6 types IPv6 conntrack considers allowed to initiate new
> connections. Note that in principle "router advertisement" ICMPv6
> packets could also be considered valid-new, as e.g. radvd periodically
> advertises its presence to the local network - as however it is not
> necessary to let this through as new to have things work (plus we
> aren't that big on broadcasting, even on supposedly-secure networks),
> the patch only adds solicitations to the list.

This patch seems fine to me. Yasuyuki, what do you think?


--
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: IPv6 conntrack support for neighbour discovery

[Yasuyuki KOZAKAI]

> This patch seems fine to me. Yasuyuki, what do you think?

In short: nf_conntrack_proto_icmpv6.c does not help for protocols using
their messages. conntrack helper is better.


In detail:

A solicitation and advertisement does not construct 'connection'
in most cases. Because the destination of solicitation is typically
multicast address, and the source address of advertisement for replying is
typically unicast address.

In the first place, static configuration is better for their messages.

Even if you add their ICMPv6 types to nf_coonntrack_proto_icmpv6.c,
you will need to configure rules to allow IPv6 stack to receive
Router/Neighbor Solicitation/Advertisement with all-node or solicited
multicast address.

For example, IPv6 node sends neighbor solicitation to
solicited multicast address to resolve L2 address. If you block it
on your node, then other nodes (including routers) cannot resolve
L2 address of your node. Also Duplicated Address Detection (DAD) does not
work.

What threat you want to avoid ? If it is spoofed router advertisement
to all-nodes multicast address, conntrack helper is better.
It would expect advertisement with the specific destination address
(all-node multicast address or the link-local/global unicast address on your
node), only when your node sends router solicitation (with the all-node
multicast address or the unicast address of router as destination address).

But I don't recommend to block unsolicited router advertisement.
If many hosts block it on a link, that results in increasing traffic
on the link by solicitation. And hosts would not be able to know
the change of configuration on router in short time.

FYI: IIRC SEcure Neighbor Discovery (SEND) is the current solution from
IETF guys, but I don't know the implementation for Linux.

-- Yasuyuki Kozakai

From: Patrick McHardy <kaber@trash.net>
Date: Tue, 04 Nov 2008 15:06:55 +0100

> Marek Szuba wrote:
> > Hello everyone,
> > 
> > Following a suggestion I was given at the kernel Bugzilla I am
> > forwarding this here.
> > 
> > It has come to my attention lately that the only two ICMPv6 packet
> > types which IPv6 conntrack allows at present to initiate new
> > connections are "echo request" and "node information query"; all other
> > types appearing in this context are considered invalid. In particular,
> > this includes two neighbour-discovery types specified in RFC 2461:
> > "router solicitation" and "neighbour solicitation" - meaning that if a
> > server drops all INVALID packets early in its INPUT or OUTPUT chain of
> > IPv6 netfilter's filter table, neither of the two can be received from
> > clients. Given that router solicitation is used for stateless network
> > autoconfiguration and neighbour solicitation is an IPv6 equivalent of
> > ARP requests, having these two dropped can be quite bad - especially
> > the latter, as it pretty much kills IPv6 over Ethernet.
> > 
> > Steps to reproduce:
> > 
> > 1. Obtain an IPv6 address assigned to an Ethernet interface of host A
> > (link-local will do);
> > 2. If necesssary, flush all ip6tables filter chains on host A and set
> > their policies to ACCEPT;
> > 3. From host B on the same Ethernet network, ping6 the aforementioned IP
> > address to verify that everything works;
> > 4. On host A, run something like "ip6tables -I INPUT 1 -m state --state
> > INVALID -j DROP";
> > 5. Try to repeat step 3. This time the ping should fail and counters
> > associated with the aforementioned rule should increment;
> > 6. Repeat all the above using OUTPUT instead of INPUT, with the same
> > final result.
> > 
> > BTW. This problem was discovered and (hopefully) fixed collaboratively
> > by Łukasz Stelmach <steelman@post.pl> and myself, please attribute both
> > of us should the patch make it into code repositories.
> > 
> > Attaching a proposed solution to the problem - the enclosed patch adds
> > "router solicitation" and "neighbour solicitation" to the list of
> > ICMPv6 types IPv6 conntrack considers allowed to initiate new
> > connections. Note that in principle "router advertisement" ICMPv6
> > packets could also be considered valid-new, as e.g. radvd periodically
> > advertises its presence to the local network - as however it is not
> > necessary to let this through as new to have things work (plus we
> > aren't that big on broadcasting, even on supposedly-secure networks),
> > the patch only adds solicitations to the list.
> 
> This patch seems fine to me. Yasuyuki, what do you think?
> 
> 
> --
> 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
--
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: IPv6 conntrack support for neighbour discovery

[Patrick McHardy]

Yasuyuki KOZAKAI wrote:
>> This patch seems fine to me. Yasuyuki, what do you think?
> 
> In short: nf_conntrack_proto_icmpv6.c does not help for protocols using
> their messages. conntrack helper is better.
> 
> 
> In detail:
> 
> A solicitation and advertisement does not construct 'connection'
> in most cases. Because the destination of solicitation is typically
> multicast address, and the source address of advertisement for replying is
> typically unicast address.
> 
> In the first place, static configuration is better for their messages.
> 
> Even if you add their ICMPv6 types to nf_coonntrack_proto_icmpv6.c,
> you will need to configure rules to allow IPv6 stack to receive
> Router/Neighbor Solicitation/Advertisement with all-node or solicited
> multicast address.
> 
> For example, IPv6 node sends neighbor solicitation to
> solicited multicast address to resolve L2 address. If you block it
> on your node, then other nodes (including routers) cannot resolve
> L2 address of your node. Also Duplicated Address Detection (DAD) does not
> work.
> 
> What threat you want to avoid ? If it is spoofed router advertisement
> to all-nodes multicast address, conntrack helper is better.
> It would expect advertisement with the specific destination address
> (all-node multicast address or the link-local/global unicast address on your
> node), only when your node sends router solicitation (with the all-node
> multicast address or the unicast address of router as destination address).
> 
> But I don't recommend to block unsolicited router advertisement.
> If many hosts block it on a link, that results in increasing traffic
> on the link by solicitation. And hosts would not be able to know
> the change of configuration on router in short time.
> 
> FYI: IIRC SEcure Neighbor Discovery (SEND) is the current solution from
> IETF guys, but I don't know the implementation for Linux.

Thanks for the detailed explanation.

Marek, could you close the bugzilla report with a link to
Yasuyuki's explanation please? Thanks!

Re: IPv6 conntrack support for neighbour discovery

[Marek Szuba]

On Wed, 05 Nov 2008 13:44:05 +0900 (JST)
Yasuyuki KOZAKAI <yasuyuki.kozakai@toshiba.co.jp> wrote:

> In short: nf_conntrack_proto_icmpv6.c does not help for protocols
> using their messages. conntrack helper is better.
So what you are suggesting is, someone needs to write such a helper
module. Correct?

> In the first place, static configuration is better for their messages.
Do you mean pre-filling neighbour-discovery cache? If so, I agree with
you on this as far as router discovery is concerned but I don't think
such an approach is viable with neighbour discovery, except possibly on
very small and centrally-managed LANs - any addition or removal of
hosts and/or addresses on the network would require propagating the
change to all machines on it.

> Even if you add their ICMPv6 types to nf_coonntrack_proto_icmpv6.c,
> you will need to configure rules to allow IPv6 stack to receive
> Router/Neighbor Solicitation/Advertisement with all-node or solicited
> multicast address.
True. To be honest, some no-nonsense documentation of what data an IPv6
host must receive/return on what address in order to actually exist on
the network is clearly in order here... I've been working with
setting up IPv6 on various networks for quite a while and it takes much
more work here than with IPv4 to set up a "what isn't explicitly
allowed is forbidden" policy.

> For example, IPv6 node sends neighbor solicitation to
> solicited multicast address to resolve L2 address. If you block it
> on your node, then other nodes (including routers) cannot resolve
> L2 address of your node.
This is precisely the problem I have encountered here.

> What threat you want to avoid ? If it is spoofed router advertisement
> to all-nodes multicast address, conntrack helper is better.
I wasn't trying to avoid any specific threat here, it's just that my
iptables rules always contain a rule which kills packets with state
INVALID and in case of IPv6 on a LAN this breaks resolving of L2
addresses.

> But I don't recommend to block unsolicited router advertisement.
> If many hosts block it on a link, that results in increasing traffic
> on the link by solicitation. And hosts would not be able to know
> the change of configuration on router in short time.
Fair enough.

> FYI: IIRC SEcure Neighbor Discovery (SEND) is the current solution
> from IETF guys, but I don't know the implementation for Linux.
I haven't seen any such implementations yet, then again this doesn't
mean there are none of course.

-- 
MS

Re: IPv6 conntrack support for neighbour discovery

[Marek Szuba]

On Wed, 05 Nov 2008 10:50:29 +0100
Patrick McHardy <kaber@trash.net> wrote:

> Marek, could you close the bugzilla report with a link to
> Yasuyuki's explanation please? Thanks!
What resolution should I specify? I believe we've got the following
options: CODE_FIX, WILL_NOT_FIX (which rejects the bug) and
WILL_FIX_LATER (which defers the bug).

-- 
MS

Re: IPv6 conntrack support for neighbour discovery

[Patrick McHardy]

Marek Szuba wrote:
> On Wed, 05 Nov 2008 10:50:29 +0100
> Patrick McHardy <kaber@trash.net> wrote:
> 
>> Marek, could you close the bugzilla report with a link to
>> Yasuyuki's explanation please? Thanks!
> What resolution should I specify? I believe we've got the following
> options: CODE_FIX, WILL_NOT_FIX (which rejects the bug) and
> WILL_FIX_LATER (which defers the bug).

I'd suggest WILL_NOT_FIX with an appropriate comment.

Re: IPv6 conntrack support for neighbour discovery

[Yasuyuki KOZAKAI]

From: Marek Szuba <Marek.Szuba@student.if.pw.edu.pl>
Date: Wed, 5 Nov 2008 13:40:21 +0100

> On Wed, 05 Nov 2008 10:50:29 +0100
> Patrick McHardy <kaber@trash.net> wrote:
> 
> > Marek, could you close the bugzilla report with a link to
> > Yasuyuki's explanation please? Thanks!
> What resolution should I specify? I believe we've got the following
> options: CODE_FIX, WILL_NOT_FIX (which rejects the bug) and
> WILL_FIX_LATER (which defers the bug).

WILL_NOT_FIX. That is not bug of connection tracking.

Your patch cannot support protocols using their ICMPv6 messages
as I explained. Please feel free to implement my suggestion or propose
better way.

-- Yasuyuki Kozakai

[PATCH 0/2] IPv6 conntrack support for neighbour discovery

[Eric Leblond]

[-- Attachment #1: Type: text/plain, Size: 2364 bytes --]

Hi,

Le mercredi 05 novembre 2008 à 13:35 +0100, Marek Szuba a écrit :
> On Wed, 05 Nov 2008 13:44:05 +0900 (JST)
> Yasuyuki KOZAKAI <yasuyuki.kozakai@toshiba.co.jp> wrote:
> 
> > In short: nf_conntrack_proto_icmpv6.c does not help for protocols
> > using their messages. conntrack helper is better.
> So what you are suggesting is, someone needs to write such a helper
> module. Correct?

I'm reopening this thread because Victor Stinner (in CC) has opened a
similar bug in bugzilla:
http://bugzilla.netfilter.org/show_bug.cgi?id=567
Pablo has then pointed out this discussion.

I don't think that we have to handle these part of ICMPv6 protocol with
a connection tracking helper. For example, here's the trace of the
exchange which are needed to obtain a "public" IPv6 address on my setup:

15:14:42.377744 IP6 fe80::a00:27ff:feb3:e26c > ff02::2: ICMP6, router solicitation, length 16
15:14:42.405725 IP6 fe80::207:cbff:fe90:f8d1 > ff02::1: ICMP6, router advertisement, length 104
15:14:43.401395 IP6 :: > ff02::1:ffb3:e26c: ICMP6, neighbor solicitation, who has 2a01:e35:1394:5bd0:a00:27ff:feb3:e26c, length 24
15:14:46.628300 IP6 fe80::a00:27ff:feb3:e26c > ff02::16: HBH ICMP6, multicast listener report v2, 1 group record(s), length 28

If we only analyse the first two, we see we can't easily invert the
tuple of the connection: we don't know which address will answer and the
destination address of the packet are changing. But as pointed by
Yasuyuki the main point is not here, we need to accept the router
advertisement to be able to detect changes in the routing. In fact,
theses protocols are stateless and should be considered as such.

IMHO, the main issue here is to tag the packets as INVALID. They are
valid even we can't track them. I thus propose a patchset which adds the
capability to avoid connection tracking the packets of these protocols.
A sysctl flag can be set to activate this feature.

With this patchset, the concerned packet don't get blocked by the
INVALID rule and are not seen by the conntrack. They can be cleanly
filtered in the ruleset.

Patchset statistics:
 net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c |   35 +++++++++++++++++++++++-
 1 files changed, 34 insertions(+), 1 deletions(-)

BR,
-- 
Eric Leblond <eric@inl.fr>
INL: http://www.inl.fr/
NuFW: http://www.nufw.org/

[-- Attachment #2: Ceci est une partie de message numériquement signée --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

[PATCH 1/2] netfilter: use sysctl to choose icmpv6 autoconf behaviour.

[Eric Leblond]

This patches adds a sysctl flag to decide wheither or not detect icmpv6
autoconfiguration packet as INVALID.

Signed-off-by: Eric Leblond <eric@inl.fr>
---
 net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c b/net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c
index 0dbac72..4aa80ba 100644
--- a/net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c
+++ b/net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c
@@ -27,6 +27,7 @@
 #include <net/netfilter/nf_log.h>
 
 static unsigned long nf_ct_icmpv6_timeout __read_mostly = 30*HZ;
+static unsigned long nf_ct_icmpv6_autoconf __read_mostly = 0;
 
 static bool icmpv6_pkt_to_tuple(const struct sk_buff *skb,
 				unsigned int dataoff,
@@ -259,6 +260,13 @@ static struct ctl_table icmpv6_sysctl_table[] = {
 		.proc_handler	= proc_dointvec_jiffies,
 	},
 	{
+		.procname	= "nf_conntrack_icmpv6_autoconf",
+		.data		= &nf_ct_icmpv6_autoconf,
+		.maxlen		= sizeof(unsigned int),
+		.mode		= 0644,
+		.proc_handler	= proc_dointvec_jiffies,
+	},
+	{
 		.ctl_name	= 0
 	}
 };
-- 
1.5.6.3

[PATCH 2/2] netfilter: don't track ICMPv6 negotiation message.

[Eric Leblond]

This patch removes connection tracking handling for ICMPv6 messages
related to autoconfiguration. They can be tracked because they are
massively using multicast (on pre-defined address). But they are not
invalid.

Signed-off-by: Eric Leblond <eric@inl.fr>
---
 net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c |   27 +++++++++++++++++++++++-
 1 files changed, 26 insertions(+), 1 deletions(-)

diff --git a/net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c b/net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c
index 4aa80ba..34548c3 100644
--- a/net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c
+++ b/net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c
@@ -54,12 +54,25 @@ static const u_int8_t invmap[] = {
 	[ICMPV6_NI_REPLY - 128]		= ICMPV6_NI_QUERY +1
 };
 
+static const u_int8_t noct_valid_new[] = {
+	[ICMPV6_MGM_QUERY - 130] = 1,
+	[ICMPV6_MGM_REPORT -130] = 1,
+	[ICMPV6_MGM_REDUCTION - 130] = 1,
+	[NDISC_ROUTER_SOLICITATION - 130] = 1,
+	[NDISC_ROUTER_ADVERTISEMENT - 130] = 1,
+	[NDISC_NEIGHBOUR_SOLICITATION - 130] = 1,
+	[NDISC_NEIGHBOUR_ADVERTISEMENT - 130] = 1,
+	[ICMPV6_MLD2_REPORT - 130] = 1
+};
+
 static bool icmpv6_invert_tuple(struct nf_conntrack_tuple *tuple,
 				const struct nf_conntrack_tuple *orig)
 {
 	int type = orig->dst.u.icmp.type - 128;
-	if (type < 0 || type >= sizeof(invmap) || !invmap[type])
+
+	if (type < 0 || type >= sizeof(invmap) || !invmap[type]) {
 		return false;
+	}
 
 	tuple->src.u.icmp.id   = orig->src.u.icmp.id;
 	tuple->dst.u.icmp.type = invmap[type] - 1;
@@ -109,6 +122,7 @@ static bool icmpv6_new(struct nf_conn *ct, const struct sk_buff *skb,
 		[ICMPV6_ECHO_REQUEST - 128] = 1,
 		[ICMPV6_NI_QUERY - 128] = 1
 	};
+
 	int type = ct->tuplehash[0].tuple.dst.u.icmp.type - 128;
 
 	if (type < 0 || type >= sizeof(valid_new) || !valid_new[type]) {
@@ -198,6 +212,17 @@ icmpv6_error(struct net *net, struct sk_buff *skb, unsigned int dataoff,
 		return -NF_ACCEPT;
 	}
 
+	/* autoconf message handling */
+	if (nf_ct_icmpv6_autoconf) {
+		int type = icmp6h->icmp6_type - 130;
+		if (type >= 0 && type < sizeof(noct_valid_new)
+		    && noct_valid_new[type]) {
+			skb->nfct = &nf_conntrack_untracked.ct_general;
+			skb->nfctinfo = IP_CT_NEW;
+			nf_conntrack_get(skb->nfct);
+			return -NF_ACCEPT;
+		}
+	}
 	/* is not error message ? */
 	if (icmp6h->icmp6_type >= 128)
 		return NF_ACCEPT;
-- 
1.5.6.3

Re: [PATCH 2/2] netfilter: don't track ICMPv6 negotiation message.

[Eric Leblond]

[-- Attachment #1: Type: text/plain, Size: 2807 bytes --]

Hi,

Oups, sorry, this patch is not clean (indentation change and spacing). I
send an other one in reply to this mail.

Le jeudi 22 janvier 2009 à 00:43 +0100, Eric Leblond a écrit :
> This patch removes connection tracking handling for ICMPv6 messages
> related to autoconfiguration. They can be tracked because they are
> massively using multicast (on pre-defined address). But they are not
> invalid.
> 
> Signed-off-by: Eric Leblond <eric@inl.fr>
> ---
>  net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c |   27 +++++++++++++++++++++++-
>  1 files changed, 26 insertions(+), 1 deletions(-)
> 
> diff --git a/net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c b/net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c
> index 4aa80ba..34548c3 100644
> --- a/net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c
> +++ b/net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c
> @@ -54,12 +54,25 @@ static const u_int8_t invmap[] = {
>  	[ICMPV6_NI_REPLY - 128]		= ICMPV6_NI_QUERY +1
>  };
>  
> +static const u_int8_t noct_valid_new[] = {
> +	[ICMPV6_MGM_QUERY - 130] = 1,
> +	[ICMPV6_MGM_REPORT -130] = 1,
> +	[ICMPV6_MGM_REDUCTION - 130] = 1,
> +	[NDISC_ROUTER_SOLICITATION - 130] = 1,
> +	[NDISC_ROUTER_ADVERTISEMENT - 130] = 1,
> +	[NDISC_NEIGHBOUR_SOLICITATION - 130] = 1,
> +	[NDISC_NEIGHBOUR_ADVERTISEMENT - 130] = 1,
> +	[ICMPV6_MLD2_REPORT - 130] = 1
> +};
> +
>  static bool icmpv6_invert_tuple(struct nf_conntrack_tuple *tuple,
>  				const struct nf_conntrack_tuple *orig)
>  {
>  	int type = orig->dst.u.icmp.type - 128;
> -	if (type < 0 || type >= sizeof(invmap) || !invmap[type])
> +
> +	if (type < 0 || type >= sizeof(invmap) || !invmap[type]) {
>  		return false;
> +	}
>  
>  	tuple->src.u.icmp.id   = orig->src.u.icmp.id;
>  	tuple->dst.u.icmp.type = invmap[type] - 1;
> @@ -109,6 +122,7 @@ static bool icmpv6_new(struct nf_conn *ct, const struct sk_buff *skb,
>  		[ICMPV6_ECHO_REQUEST - 128] = 1,
>  		[ICMPV6_NI_QUERY - 128] = 1
>  	};
> +
>  	int type = ct->tuplehash[0].tuple.dst.u.icmp.type - 128;
>  
>  	if (type < 0 || type >= sizeof(valid_new) || !valid_new[type]) {
> @@ -198,6 +212,17 @@ icmpv6_error(struct net *net, struct sk_buff *skb, unsigned int dataoff,
>  		return -NF_ACCEPT;
>  	}
>  
> +	/* autoconf message handling */
> +	if (nf_ct_icmpv6_autoconf) {
> +		int type = icmp6h->icmp6_type - 130;
> +		if (type >= 0 && type < sizeof(noct_valid_new)
> +		    && noct_valid_new[type]) {
> +			skb->nfct = &nf_conntrack_untracked.ct_general;
> +			skb->nfctinfo = IP_CT_NEW;
> +			nf_conntrack_get(skb->nfct);
> +			return -NF_ACCEPT;
> +		}
> +	}
>  	/* is not error message ? */
>  	if (icmp6h->icmp6_type >= 128)
>  		return NF_ACCEPT;
-- 
Eric Leblond <eric@inl.fr>
INL: http://www.inl.fr/
NuFW: http://www.nufw.org/

[-- Attachment #2: Ceci est une partie de message numériquement signée --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

[Resend PATCH 2/2] netfilter: don't track ICMPv6 negotiation message.

[Eric Leblond]

This patch removes connection tracking handling for ICMPv6 messages
related to autoconfiguration. They can be tracked because they are
massively using multicast (on pre-defined address). But they are not
invalid.

Signed-off-by: Eric Leblond <eric@inl.fr>
---
 net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c |   22 ++++++++++++++++++++++
 1 files changed, 22 insertions(+), 0 deletions(-)

diff --git a/net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c b/net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c
index 4aa80ba..a178270 100644
--- a/net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c
+++ b/net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c
@@ -54,6 +54,17 @@ static const u_int8_t invmap[] = {
 	[ICMPV6_NI_REPLY - 128]		= ICMPV6_NI_QUERY +1
 };
 
+static const u_int8_t noct_valid_new[] = {
+	[ICMPV6_MGM_QUERY - 130] = 1,
+	[ICMPV6_MGM_REPORT -130] = 1,
+	[ICMPV6_MGM_REDUCTION - 130] = 1,
+	[NDISC_ROUTER_SOLICITATION - 130] = 1,
+	[NDISC_ROUTER_ADVERTISEMENT - 130] = 1,
+	[NDISC_NEIGHBOUR_SOLICITATION - 130] = 1,
+	[NDISC_NEIGHBOUR_ADVERTISEMENT - 130] = 1,
+	[ICMPV6_MLD2_REPORT - 130] = 1
+};
+
 static bool icmpv6_invert_tuple(struct nf_conntrack_tuple *tuple,
 				const struct nf_conntrack_tuple *orig)
 {
@@ -198,6 +209,17 @@ icmpv6_error(struct net *net, struct sk_buff *skb, unsigned int dataoff,
 		return -NF_ACCEPT;
 	}
 
+	/* autoconf message handling */
+	if (nf_ct_icmpv6_autoconf) {
+		int type = icmp6h->icmp6_type - 130;
+		if (type >= 0 && type < sizeof(noct_valid_new)
+		    && noct_valid_new[type]) {
+			skb->nfct = &nf_conntrack_untracked.ct_general;
+			skb->nfctinfo = IP_CT_NEW;
+			nf_conntrack_get(skb->nfct);
+			return -NF_ACCEPT;
+		}
+	}
 	/* is not error message ? */
 	if (icmp6h->icmp6_type >= 128)
 		return NF_ACCEPT;
-- 
1.5.6.3

Re: [PATCH 0/2] IPv6 conntrack support for neighbour discovery

[Yasuyuki KOZAKAI]

Hi, Eric,

How about just using NOTRACK target ?

-- Yasuyuki Kozakai

From: Eric Leblond <eric@inl.fr>
Date: Thu, 22 Jan 2009 00:41:27 +0100

> Hi,
> 
> Le mercredi 05 novembre 2008 à 13:35 +0100, Marek Szuba a écrit :
> > On Wed, 05 Nov 2008 13:44:05 +0900 (JST)
> > Yasuyuki KOZAKAI <yasuyuki.kozakai@toshiba.co.jp> wrote:
> > 
> > > In short: nf_conntrack_proto_icmpv6.c does not help for protocols
> > > using their messages. conntrack helper is better.
> > So what you are suggesting is, someone needs to write such a helper
> > module. Correct?
> 
> I'm reopening this thread because Victor Stinner (in CC) has opened a
> similar bug in bugzilla:
> http://bugzilla.netfilter.org/show_bug.cgi?id=567
> Pablo has then pointed out this discussion.
> 
> I don't think that we have to handle these part of ICMPv6 protocol with
> a connection tracking helper. For example, here's the trace of the
> exchange which are needed to obtain a "public" IPv6 address on my setup:
> 
> 15:14:42.377744 IP6 fe80::a00:27ff:feb3:e26c > ff02::2: ICMP6, router solicitation, length 16
> 15:14:42.405725 IP6 fe80::207:cbff:fe90:f8d1 > ff02::1: ICMP6, router advertisement, length 104
> 15:14:43.401395 IP6 :: > ff02::1:ffb3:e26c: ICMP6, neighbor solicitation, who has 2a01:e35:1394:5bd0:a00:27ff:feb3:e26c, length 24
> 15:14:46.628300 IP6 fe80::a00:27ff:feb3:e26c > ff02::16: HBH ICMP6, multicast listener report v2, 1 group record(s), length 28
> 
> If we only analyse the first two, we see we can't easily invert the
> tuple of the connection: we don't know which address will answer and the
> destination address of the packet are changing. But as pointed by
> Yasuyuki the main point is not here, we need to accept the router
> advertisement to be able to detect changes in the routing. In fact,
> theses protocols are stateless and should be considered as such.
> 
> IMHO, the main issue here is to tag the packets as INVALID. They are
> valid even we can't track them. I thus propose a patchset which adds the
> capability to avoid connection tracking the packets of these protocols.
> A sysctl flag can be set to activate this feature.
> 
> With this patchset, the concerned packet don't get blocked by the
> INVALID rule and are not seen by the conntrack. They can be cleanly
> filtered in the ruleset.
> 
> Patchset statistics:
>  net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c |   35 +++++++++++++++++++++++-
>  1 files changed, 34 insertions(+), 1 deletions(-)
--
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 0/2] IPv6 conntrack support for neighbour discovery

[Eric Leblond]

[-- Attachment #1: Type: text/plain, Size: 3474 bytes --]

Hi,

Le vendredi 23 janvier 2009 à 16:40 +0900, Yasuyuki KOZAKAI a écrit :
> Hi, Eric,
> 
> How about just using NOTRACK target ?

This is a possible solution but you will then have to be a Netfilter
Jedi to build a correct IPv6 filtering ruleset. I think even Padawan
have no clue about the existence of the NOTRACK target.

Seriously speaking, the NOTRACK target is tagged "NETFILTER_ADVANCED"
and IPv6 is becoming more and more frequent. I don't think we should use
such a complicated solution for simply obtaining an IPv6 address. We
will loose quit a bunch of users on this.

BR,
--
Eric

> 
> -- Yasuyuki Kozakai
> 
> From: Eric Leblond <eric@inl.fr>
> Date: Thu, 22 Jan 2009 00:41:27 +0100
> 
> > Hi,
> > 
> > Le mercredi 05 novembre 2008 à 13:35 +0100, Marek Szuba a écrit :
> > > On Wed, 05 Nov 2008 13:44:05 +0900 (JST)
> > > Yasuyuki KOZAKAI <yasuyuki.kozakai@toshiba.co.jp> wrote:
> > > 
> > > > In short: nf_conntrack_proto_icmpv6.c does not help for protocols
> > > > using their messages. conntrack helper is better.
> > > So what you are suggesting is, someone needs to write such a helper
> > > module. Correct?
> > 
> > I'm reopening this thread because Victor Stinner (in CC) has opened a
> > similar bug in bugzilla:
> > http://bugzilla.netfilter.org/show_bug.cgi?id=567
> > Pablo has then pointed out this discussion.
> > 
> > I don't think that we have to handle these part of ICMPv6 protocol with
> > a connection tracking helper. For example, here's the trace of the
> > exchange which are needed to obtain a "public" IPv6 address on my setup:
> > 
> > 15:14:42.377744 IP6 fe80::a00:27ff:feb3:e26c > ff02::2: ICMP6, router solicitation, length 16
> > 15:14:42.405725 IP6 fe80::207:cbff:fe90:f8d1 > ff02::1: ICMP6, router advertisement, length 104
> > 15:14:43.401395 IP6 :: > ff02::1:ffb3:e26c: ICMP6, neighbor solicitation, who has 2a01:e35:1394:5bd0:a00:27ff:feb3:e26c, length 24
> > 15:14:46.628300 IP6 fe80::a00:27ff:feb3:e26c > ff02::16: HBH ICMP6, multicast listener report v2, 1 group record(s), length 28
> > 
> > If we only analyse the first two, we see we can't easily invert the
> > tuple of the connection: we don't know which address will answer and the
> > destination address of the packet are changing. But as pointed by
> > Yasuyuki the main point is not here, we need to accept the router
> > advertisement to be able to detect changes in the routing. In fact,
> > theses protocols are stateless and should be considered as such.
> > 
> > IMHO, the main issue here is to tag the packets as INVALID. They are
> > valid even we can't track them. I thus propose a patchset which adds the
> > capability to avoid connection tracking the packets of these protocols.
> > A sysctl flag can be set to activate this feature.
> > 
> > With this patchset, the concerned packet don't get blocked by the
> > INVALID rule and are not seen by the conntrack. They can be cleanly
> > filtered in the ruleset.
> > 
> > Patchset statistics:
> >  net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c |   35 +++++++++++++++++++++++-
> >  1 files changed, 34 insertions(+), 1 deletions(-)
> --
> 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
-- 
Éric Leblond <eric@inl.fr>
INL, http://www.inl.fr/
NuFW, http://www.nufw.org

[-- Attachment #2: Ceci est une partie de message numériquement signée --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH 0/2] IPv6 conntrack support for neighbour discovery

[Jozsef Kadlecsik]

On Fri, 23 Jan 2009, Eric Leblond wrote:

> > How about just using NOTRACK target ?
> 
> This is a possible solution but you will then have to be a Netfilter
> Jedi to build a correct IPv6 filtering ruleset. I think even Padawan
> have no clue about the existence of the NOTRACK target.
> 
> Seriously speaking, the NOTRACK target is tagged "NETFILTER_ADVANCED"
> and IPv6 is becoming more and more frequent. I don't think we should use
> such a complicated solution for simply obtaining an IPv6 address. We
> will loose quit a bunch of users on this.

RFC4890, Recommendations for Filtering ICMPv6 Messages in Firewalls, deals 
with ICMPv6 filtering on firewalls in details and even contains a sample 
ip6tables script.

Wouldn't it make sense to add simply the RFC to the iptables source tree, 
and a sentence to the ip6tables manpage saying:

"Please read RFC 4890 and the sample script in it on how to filter ICMPv6
properly in an IPv6 environment."

Best regards,
Jozsef
-
E-mail  : kadlec@blackhole.kfki.hu, kadlec@mail.kfki.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : KFKI Research Institute for Particle and Nuclear Physics
          H-1525 Budapest 114, POB. 49, Hungary

Re: [PATCH 0/2] IPv6 conntrack support for neighbour discovery

[Jozsef Kadlecsik]

On Fri, 23 Jan 2009, Jozsef Kadlecsik wrote:

> On Fri, 23 Jan 2009, Eric Leblond wrote:
> 
> > > How about just using NOTRACK target ?
> > 
> > This is a possible solution but you will then have to be a Netfilter
> > Jedi to build a correct IPv6 filtering ruleset. I think even Padawan
> > have no clue about the existence of the NOTRACK target.
> > 
> > Seriously speaking, the NOTRACK target is tagged "NETFILTER_ADVANCED"
> > and IPv6 is becoming more and more frequent. I don't think we should use
> > such a complicated solution for simply obtaining an IPv6 address. We
> > will loose quit a bunch of users on this.
> 
> RFC4890, Recommendations for Filtering ICMPv6 Messages in Firewalls, deals 
> with ICMPv6 filtering on firewalls in details and even contains a sample 
> ip6tables script.
> 
> Wouldn't it make sense to add simply the RFC to the iptables source tree, 
> and a sentence to the ip6tables manpage saying:
> 
> "Please read RFC 4890 and the sample script in it on how to filter ICMPv6
> properly in an IPv6 environment."

Looking through the sample script again, it is incomplete as it does not 
handle the ICMPv6 traffic targeted to/sent from the firewall itself. But 
it'd be not hard to add the missing rules.

Best regards,
Jozsef
-
E-mail  : kadlec@blackhole.kfki.hu, kadlec@mail.kfki.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : KFKI Research Institute for Particle and Nuclear Physics
          H-1525 Budapest 114, POB. 49, Hungary

Re: [PATCH 0/2] IPv6 conntrack support for neighbour discovery

[Yasuyuki KOZAKAI]

Hi,

From: Eric Leblond <eric@inl.fr>
Date: Fri, 23 Jan 2009 10:02:27 +0100

> Hi,
> 
> Le vendredi 23 janvier 2009 à 16:40 +0900, Yasuyuki KOZAKAI a écrit :
> > Hi, Eric,
> > 
> > How about just using NOTRACK target ?
> 
> This is a possible solution but you will then have to be a Netfilter
> Jedi to build a correct IPv6 filtering ruleset. I think even Padawan
> have no clue about the existence of the NOTRACK target.
> 
> Seriously speaking, the NOTRACK target is tagged "NETFILTER_ADVANCED"
> and IPv6 is becoming more and more frequent. I don't think we should use
> such a complicated solution for simply obtaining an IPv6 address. We
> will loose quit a bunch of users on this.

It's reasonable. One solution is to change it and to make distributer add
NOTRACK rule to their configuration, but changing nf_conntrack would be
faster ;)


From: Eric Leblond <eric@inl.fr>
Date: Thu, 22 Jan 2009 00:43:50 +0100

> This patches adds a sysctl flag to decide wheither or not detect icmpv6
> autoconfiguration packet as INVALID.
> 
> Signed-off-by: Eric Leblond <eric@inl.fr>
> ---
>  net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c |    8 ++++++++
>  1 files changed, 8 insertions(+), 0 deletions(-)
> 
> diff --git a/net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c b/net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c
> index 0dbac72..4aa80ba 100644
> --- a/net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c
> +++ b/net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c
> @@ -27,6 +27,7 @@
>  #include <net/netfilter/nf_log.h>
>  
>  static unsigned long nf_ct_icmpv6_timeout __read_mostly = 30*HZ;
> +static unsigned long nf_ct_icmpv6_autoconf __read_mostly = 0;
>  
>  static bool icmpv6_pkt_to_tuple(const struct sk_buff *skb,
>  				unsigned int dataoff,
> @@ -259,6 +260,13 @@ static struct ctl_table icmpv6_sysctl_table[] = {
>  		.proc_handler	= proc_dointvec_jiffies,
>  	},
>  	{
> +		.procname	= "nf_conntrack_icmpv6_autoconf",
> +		.data		= &nf_ct_icmpv6_autoconf,
> +		.maxlen		= sizeof(unsigned int),
> +		.mode		= 0644,
> +		.proc_handler	= proc_dointvec_jiffies,
> +	},
> +	{
>  		.ctl_name	= 0
>  	}
>  };
> -- 
> 1.5.6.3

I think this is not needed. User would not be aware of this configuration.


From: Eric Leblond <eric@inl.fr>
Date: Thu, 22 Jan 2009 00:43:51 +0100

> This patch removes connection tracking handling for ICMPv6 messages
> related to autoconfiguration. They can be tracked because they are
> massively using multicast (on pre-defined address). But they are not
> invalid.
>
> Signed-off-by: Eric Leblond <eric@inl.fr>

<snip>

> +static const u_int8_t noct_valid_new[] = {
> +	[ICMPV6_MGM_QUERY - 130] = 1,
> +	[ICMPV6_MGM_REPORT -130] = 1,
> +	[ICMPV6_MGM_REDUCTION - 130] = 1,
> +	[NDISC_ROUTER_SOLICITATION - 130] = 1,
> +	[NDISC_ROUTER_ADVERTISEMENT - 130] = 1,
> +	[NDISC_NEIGHBOUR_SOLICITATION - 130] = 1,
> +	[NDISC_NEIGHBOUR_ADVERTISEMENT - 130] = 1,
> +	[ICMPV6_MLD2_REPORT - 130] = 1
> +};

MLD is not part of auto configuration. But I agree to handle it.


>  static bool icmpv6_invert_tuple(struct nf_conntrack_tuple *tuple,
>  				const struct nf_conntrack_tuple *orig)
>  {
>  	int type = orig->dst.u.icmp.type - 128;
> -	if (type < 0 || type >= sizeof(invmap) || !invmap[type])
> +
> +	if (type < 0 || type >= sizeof(invmap) || !invmap[type]) {
>  		return false;
> +	}

Really is this change necessary ?


> @@ -198,6 +212,17 @@ icmpv6_error(struct net *net, struct sk_buff *skb, unsigned int dataoff,
>  		return -NF_ACCEPT;
>  	}
>  
> +	/* autoconf message handling */
> +	if (nf_ct_icmpv6_autoconf) {
> +		int type = icmp6h->icmp6_type - 130;
> +		if (type >= 0 && type < sizeof(noct_valid_new)
> +		    && noct_valid_new[type]) {
> +			skb->nfct = &nf_conntrack_untracked.ct_general;
> +			skb->nfctinfo = IP_CT_NEW;
> +			nf_conntrack_get(skb->nfct);
> +			return -NF_ACCEPT;
> +		}
> +	}

I prefer 'NEW' rather than 'UNTRACKED' as other protocols which
validation is unclear. So another solution is to let the connection
tracking subsystem to create a new conntrack and to make
nf_contrack_proto_icmpv6 assign 0 as timeout. How do you think ?

-- Yasuyuki Kozakai
--
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 0/2] IPv6 conntrack support for neighbour discovery

[Yasuyuki KOZAKAI]

From: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>
Date: Fri, 23 Jan 2009 10:50:59 +0100 (CET)

> On Fri, 23 Jan 2009, Jozsef Kadlecsik wrote:
> 
> > On Fri, 23 Jan 2009, Eric Leblond wrote:
> > 
> > > > How about just using NOTRACK target ?
> > > 
> > > This is a possible solution but you will then have to be a Netfilter
> > > Jedi to build a correct IPv6 filtering ruleset. I think even Padawan
> > > have no clue about the existence of the NOTRACK target.
> > > 
> > > Seriously speaking, the NOTRACK target is tagged "NETFILTER_ADVANCED"
> > > and IPv6 is becoming more and more frequent. I don't think we should use
> > > such a complicated solution for simply obtaining an IPv6 address. We
> > > will loose quit a bunch of users on this.
> > 
> > RFC4890, Recommendations for Filtering ICMPv6 Messages in Firewalls, deals 
> > with ICMPv6 filtering on firewalls in details and even contains a sample 
> > ip6tables script.
> > 
> > Wouldn't it make sense to add simply the RFC to the iptables source tree, 
> > and a sentence to the ip6tables manpage saying:
> > 
> > "Please read RFC 4890 and the sample script in it on how to filter ICMPv6
> > properly in an IPv6 environment."
> 
> Looking through the sample script again, it is incomplete as it does not 
> handle the ICMPv6 traffic targeted to/sent from the firewall itself. But 
> it'd be not hard to add the missing rules.

I looks not to show the recommended rules but just the usage of ip6tables.

-- Yasuyuki Kozakai

Re: [PATCH 0/2] IPv6 conntrack support for neighbour discovery

[Eric Leblond]

[-- Attachment #1: Type: text/plain, Size: 5144 bytes --]

Le vendredi 23 janvier 2009 à 19:21 +0900, Yasuyuki KOZAKAI a écrit :
> Hi,
> 
> From: Eric Leblond <eric@inl.fr>
> Date: Fri, 23 Jan 2009 10:02:27 +0100
> 
> > Hi,
> > 
> > Le vendredi 23 janvier 2009 à 16:40 +0900, Yasuyuki KOZAKAI a écrit :
> > > Hi, Eric,
> > > 
> > > How about just using NOTRACK target ?
> > 
> > This is a possible solution but you will then have to be a Netfilter
> > Jedi to build a correct IPv6 filtering ruleset. I think even Padawan
> > have no clue about the existence of the NOTRACK target.
> > 
> > Seriously speaking, the NOTRACK target is tagged "NETFILTER_ADVANCED"
> > and IPv6 is becoming more and more frequent. I don't think we should use
> > such a complicated solution for simply obtaining an IPv6 address. We
> > will loose quit a bunch of users on this.
> 
> It's reasonable. One solution is to change it and to make distributer add
> NOTRACK rule to their configuration, but changing nf_conntrack would be
> faster ;)
> 
> 
> From: Eric Leblond <eric@inl.fr>
> Date: Thu, 22 Jan 2009 00:43:50 +0100
> 
> > This patches adds a sysctl flag to decide wheither or not detect icmpv6
> > autoconfiguration packet as INVALID.
> > 
> > Signed-off-by: Eric Leblond <eric@inl.fr>
> > ---
> >  net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c |    8 ++++++++
> >  1 files changed, 8 insertions(+), 0 deletions(-)
> > 
> > diff --git a/net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c b/net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c
> > index 0dbac72..4aa80ba 100644
> > --- a/net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c
> > +++ b/net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c
> > @@ -27,6 +27,7 @@
> >  #include <net/netfilter/nf_log.h>
> >  
> >  static unsigned long nf_ct_icmpv6_timeout __read_mostly = 30*HZ;
> > +static unsigned long nf_ct_icmpv6_autoconf __read_mostly = 0;
> >  
> >  static bool icmpv6_pkt_to_tuple(const struct sk_buff *skb,
> >  				unsigned int dataoff,
> > @@ -259,6 +260,13 @@ static struct ctl_table icmpv6_sysctl_table[] = {
> >  		.proc_handler	= proc_dointvec_jiffies,
> >  	},
> >  	{
> > +		.procname	= "nf_conntrack_icmpv6_autoconf",
> > +		.data		= &nf_ct_icmpv6_autoconf,
> > +		.maxlen		= sizeof(unsigned int),
> > +		.mode		= 0644,
> > +		.proc_handler	= proc_dointvec_jiffies,
> > +	},
> > +	{
> >  		.ctl_name	= 0
> >  	}
> >  };
> > -- 
> > 1.5.6.3
> 
> I think this is not needed. User would not be aware of this configuration.

Ok, I'm really fine with it.

> From: Eric Leblond <eric@inl.fr>
> Date: Thu, 22 Jan 2009 00:43:51 +0100
> 
> > This patch removes connection tracking handling for ICMPv6 messages
> > related to autoconfiguration. They can be tracked because they are
> > massively using multicast (on pre-defined address). But they are not
> > invalid.
> >
> > Signed-off-by: Eric Leblond <eric@inl.fr>
> 
> <snip>
> 
> > +static const u_int8_t noct_valid_new[] = {
> > +	[ICMPV6_MGM_QUERY - 130] = 1,
> > +	[ICMPV6_MGM_REPORT -130] = 1,
> > +	[ICMPV6_MGM_REDUCTION - 130] = 1,
> > +	[NDISC_ROUTER_SOLICITATION - 130] = 1,
> > +	[NDISC_ROUTER_ADVERTISEMENT - 130] = 1,
> > +	[NDISC_NEIGHBOUR_SOLICITATION - 130] = 1,
> > +	[NDISC_NEIGHBOUR_ADVERTISEMENT - 130] = 1,
> > +	[ICMPV6_MLD2_REPORT - 130] = 1
> > +};
> 
> MLD is not part of auto configuration. But I agree to handle it.

Ok.

> 
> 
> >  static bool icmpv6_invert_tuple(struct nf_conntrack_tuple *tuple,
> >  				const struct nf_conntrack_tuple *orig)
> >  {
> >  	int type = orig->dst.u.icmp.type - 128;
> > -	if (type < 0 || type >= sizeof(invmap) || !invmap[type])
> > +
> > +	if (type < 0 || type >= sizeof(invmap) || !invmap[type]) {
> >  		return false;
> > +	}
> 
> Really is this change necessary ?

I've resend the patch without this change. It was forgotten during a
merge of previous work. Sorry for that.

> > @@ -198,6 +212,17 @@ icmpv6_error(struct net *net, struct sk_buff *skb, unsigned int dataoff,
> >  		return -NF_ACCEPT;
> >  	}
> >  
> > +	/* autoconf message handling */
> > +	if (nf_ct_icmpv6_autoconf) {
> > +		int type = icmp6h->icmp6_type - 130;
> > +		if (type >= 0 && type < sizeof(noct_valid_new)
> > +		    && noct_valid_new[type]) {
> > +			skb->nfct = &nf_conntrack_untracked.ct_general;
> > +			skb->nfctinfo = IP_CT_NEW;
> > +			nf_conntrack_get(skb->nfct);
> > +			return -NF_ACCEPT;
> > +		}
> > +	}
> 
> I prefer 'NEW' rather than 'UNTRACKED' as other protocols which
> validation is unclear. So another solution is to let the connection
> tracking subsystem to create a new conntrack and to make
> nf_contrack_proto_icmpv6 assign 0 as timeout. How do you think ?

If we do that, we can have nfnetlink messages (NEW, DESTROY) send to
userspace. Personnaly, I don't think they are necessary. But there is an
other issue: as we can't invert the tuple, the information provided to
userspace will be false.

Once we agree on this last point, I will send a reworked patchset (with
at least the removal of sysctl stuff).

BR,
-- 
Éric Leblond <eric@inl.fr>
INL, http://www.inl.fr/
NuFW, http://www.nufw.org

[-- Attachment #2: Ceci est une partie de message numériquement signée --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH 0/2] IPv6 conntrack support for neighbour discovery

[Yasuyuki KOZAKAI]

Hi,

From: Eric Leblond <eric@inl.fr>
Date: Fri, 23 Jan 2009 11:51:30 +0100

> > >  static bool icmpv6_invert_tuple(struct nf_conntrack_tuple *tuple,
> > >  				const struct nf_conntrack_tuple *orig)
> > >  {
> > >  	int type = orig->dst.u.icmp.type - 128;
> > > -	if (type < 0 || type >= sizeof(invmap) || !invmap[type])
> > > +
> > > +	if (type < 0 || type >= sizeof(invmap) || !invmap[type]) {
> > >  		return false;
> > > +	}
> > 
> > Really is this change necessary ?
> 
> I've resend the patch without this change. It was forgotten during a
> merge of previous work. Sorry for that.

Ah, sorry & thank you for noticing.


> > > @@ -198,6 +212,17 @@ icmpv6_error(struct net *net, struct sk_buff *skb, unsigned int dataoff,
> > >  		return -NF_ACCEPT;
> > >  	}
> > >  
> > > +	/* autoconf message handling */
> > > +	if (nf_ct_icmpv6_autoconf) {
> > > +		int type = icmp6h->icmp6_type - 130;
> > > +		if (type >= 0 && type < sizeof(noct_valid_new)
> > > +		    && noct_valid_new[type]) {
> > > +			skb->nfct = &nf_conntrack_untracked.ct_general;
> > > +			skb->nfctinfo = IP_CT_NEW;
> > > +			nf_conntrack_get(skb->nfct);
> > > +			return -NF_ACCEPT;
> > > +		}
> > > +	}
> > 
> > I prefer 'NEW' rather than 'UNTRACKED' as other protocols which
> > validation is unclear. So another solution is to let the connection
> > tracking subsystem to create a new conntrack and to make
> > nf_contrack_proto_icmpv6 assign 0 as timeout. How do you think ?
> 
> If we do that, we can have nfnetlink messages (NEW, DESTROY) send to
> userspace. Personnaly, I don't think they are necessary. But there is an
> other issue: as we can't invert the tuple, the information provided to
> userspace will be false.
>
> Once we agree on this last point, I will send a reworked patchset (with
> at least the removal of sysctl stuff).

Thank you. I understand why ICMPv6 packets are special here and
I agree to assign UNTRACKED to them. Indeed non-invertible tuple might
bring issues.

-- Yasuyuki Kozakai

[PATCH] netfilter: don't track ICMPv6 negotiation message.

[Eric Leblond]

This patch removes connection tracking handling for ICMPv6 messages
related to autoconfiguration. They can be tracked because they are
massively using multicast (on pre-defined address). But they are not
invalid.

Signed-off-by: Eric Leblond <eric@inl.fr>
---
 net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c |   21 +++++++++++++++++++++
 1 files changed, 21 insertions(+), 0 deletions(-)

diff --git a/net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c b/net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c
index 6f859c1..94ace19 100644
--- a/net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c
+++ b/net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c
@@ -53,6 +53,17 @@ static const u_int8_t invmap[] = {
 	[ICMPV6_NI_REPLY - 128]		= ICMPV6_NI_QUERY +1
 };
 
+static const u_int8_t noct_valid_new[] = {
+	[ICMPV6_MGM_QUERY - 130] = 1,
+	[ICMPV6_MGM_REPORT -130] = 1,
+	[ICMPV6_MGM_REDUCTION - 130] = 1,
+	[NDISC_ROUTER_SOLICITATION - 130] = 1,
+	[NDISC_ROUTER_ADVERTISEMENT - 130] = 1,
+	[NDISC_NEIGHBOUR_SOLICITATION - 130] = 1,
+	[NDISC_NEIGHBOUR_ADVERTISEMENT - 130] = 1,
+	[ICMPV6_MLD2_REPORT - 130] = 1
+};
+
 static bool icmpv6_invert_tuple(struct nf_conntrack_tuple *tuple,
 				const struct nf_conntrack_tuple *orig)
 {
@@ -182,6 +193,7 @@ icmpv6_error(struct net *net, struct sk_buff *skb, unsigned int dataoff,
 {
 	const struct icmp6hdr *icmp6h;
 	struct icmp6hdr _ih;
+	int type;
 
 	icmp6h = skb_header_pointer(skb, dataoff, sizeof(_ih), &_ih);
 	if (icmp6h == NULL) {
@@ -199,6 +211,15 @@ icmpv6_error(struct net *net, struct sk_buff *skb, unsigned int dataoff,
 		return -NF_ACCEPT;
 	}
 
+	type = icmp6h->icmp6_type - 130;
+	if (type >= 0 && type < sizeof(noct_valid_new)
+	    && noct_valid_new[type]) {
+		skb->nfct = &nf_conntrack_untracked.ct_general;
+		skb->nfctinfo = IP_CT_NEW;
+		nf_conntrack_get(skb->nfct);
+		return NF_ACCEPT;
+	}
+
 	/* is not error message ? */
 	if (icmp6h->icmp6_type >= 128)
 		return NF_ACCEPT;
-- 
1.5.6.3

Re: [PATCH 0/2] IPv6 conntrack support for neighbour discovery

[Patrick McHardy]

Yasuyuki KOZAKAI wrote:
> From: Eric Leblond <eric@inl.fr>
> Date: Fri, 23 Jan 2009 11:51:30 +0100
> 
>>> I prefer 'NEW' rather than 'UNTRACKED' as other protocols which
>>> validation is unclear. So another solution is to let the connection
>>> tracking subsystem to create a new conntrack and to make
>>> nf_contrack_proto_icmpv6 assign 0 as timeout. How do you think ?
>> If we do that, we can have nfnetlink messages (NEW, DESTROY) send to
>> userspace. Personnaly, I don't think they are necessary. But there is an
>> other issue: as we can't invert the tuple, the information provided to
>> userspace will be false.
>>
>> Once we agree on this last point, I will send a reworked patchset (with
>> at least the removal of sysctl stuff).
> 
> Thank you. I understand why ICMPv6 packets are special here and
> I agree to assign UNTRACKED to them. Indeed non-invertible tuple might
> bring issues.

How about adding a flag to indicate that only one direction of
the tuple exists? It makes sense to support this for other kinds
of simplex flows as well in my opinion and it somewhat goes in the
same direction as the patch I talked about during the workshop
to have only a single tuple within the conntrack and have reply
tuples or potentially other tuples that relate to a connection
within the ct_extend area. And using NEW and having netlink
events seems more consistent to me.

Re: [PATCH] netfilter: don't track ICMPv6 negotiation message.

[Yasuyuki KOZAKAI]

Can you s/autoconfiguration/Stateless Address Autoconfiguration, MLD, and MLDv2/
in commit log ?

Others are fine to me.

-- Yasuyuki Kozakai

From: Eric Leblond <eric@inl.fr>
Date: Sat, 24 Jan 2009 11:32:58 +0100

> This patch removes connection tracking handling for ICMPv6 messages
> related to autoconfiguration. They can be tracked because they are
> massively using multicast (on pre-defined address). But they are not
> invalid.
> 
> Signed-off-by: Eric Leblond <eric@inl.fr>
> ---
>  net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c |   21 +++++++++++++++++++++
>  1 files changed, 21 insertions(+), 0 deletions(-)
> 
> diff --git a/net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c b/net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c
> index 6f859c1..94ace19 100644
> --- a/net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c
> +++ b/net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c
> @@ -53,6 +53,17 @@ static const u_int8_t invmap[] = {
>  	[ICMPV6_NI_REPLY - 128]		= ICMPV6_NI_QUERY +1
>  };
>  
> +static const u_int8_t noct_valid_new[] = {
> +	[ICMPV6_MGM_QUERY - 130] = 1,
> +	[ICMPV6_MGM_REPORT -130] = 1,
> +	[ICMPV6_MGM_REDUCTION - 130] = 1,
> +	[NDISC_ROUTER_SOLICITATION - 130] = 1,
> +	[NDISC_ROUTER_ADVERTISEMENT - 130] = 1,
> +	[NDISC_NEIGHBOUR_SOLICITATION - 130] = 1,
> +	[NDISC_NEIGHBOUR_ADVERTISEMENT - 130] = 1,
> +	[ICMPV6_MLD2_REPORT - 130] = 1
> +};
> +
>  static bool icmpv6_invert_tuple(struct nf_conntrack_tuple *tuple,
>  				const struct nf_conntrack_tuple *orig)
>  {
> @@ -182,6 +193,7 @@ icmpv6_error(struct net *net, struct sk_buff *skb, unsigned int dataoff,
>  {
>  	const struct icmp6hdr *icmp6h;
>  	struct icmp6hdr _ih;
> +	int type;
>  
>  	icmp6h = skb_header_pointer(skb, dataoff, sizeof(_ih), &_ih);
>  	if (icmp6h == NULL) {
> @@ -199,6 +211,15 @@ icmpv6_error(struct net *net, struct sk_buff *skb, unsigned int dataoff,
>  		return -NF_ACCEPT;
>  	}
>  
> +	type = icmp6h->icmp6_type - 130;
> +	if (type >= 0 && type < sizeof(noct_valid_new)
> +	    && noct_valid_new[type]) {
> +		skb->nfct = &nf_conntrack_untracked.ct_general;
> +		skb->nfctinfo = IP_CT_NEW;
> +		nf_conntrack_get(skb->nfct);
> +		return NF_ACCEPT;
> +	}
> +
>  	/* is not error message ? */
>  	if (icmp6h->icmp6_type >= 128)
>  		return NF_ACCEPT;
> -- 
> 1.5.6.3
> 

Re: [PATCH 0/2] IPv6 conntrack support for neighbour discovery

[Yasuyuki KOZAKAI]

From: Patrick McHardy <kaber@trash.net>
Date: Mon, 26 Jan 2009 14:11:37 +0100

> Yasuyuki KOZAKAI wrote:
> > From: Eric Leblond <eric@inl.fr>
> > Date: Fri, 23 Jan 2009 11:51:30 +0100
> > 
> >>> I prefer 'NEW' rather than 'UNTRACKED' as other protocols which
> >>> validation is unclear. So another solution is to let the connection
> >>> tracking subsystem to create a new conntrack and to make
> >>> nf_contrack_proto_icmpv6 assign 0 as timeout. How do you think ?
> >> If we do that, we can have nfnetlink messages (NEW, DESTROY) send to
> >> userspace. Personnaly, I don't think they are necessary. But there is an
> >> other issue: as we can't invert the tuple, the information provided to
> >> userspace will be false.
> >>
> >> Once we agree on this last point, I will send a reworked patchset (with
> >> at least the removal of sysctl stuff).
> > 
> > Thank you. I understand why ICMPv6 packets are special here and
> > I agree to assign UNTRACKED to them. Indeed non-invertible tuple might
> > bring issues.
> 
> How about adding a flag to indicate that only one direction of
> the tuple exists? It makes sense to support this for other kinds
> of simplex flows as well in my opinion and it somewhat goes in the
> same direction as the patch I talked about during the workshop
> to have only a single tuple within the conntrack and have reply
> tuples or potentially other tuples that relate to a connection
> within the ct_extend area. And using NEW and having netlink
> events seems more consistent to me.

It sounds good for long term solution. For now Eric's patch is enough,
I think.

And sorry, I don't remember your patch in detail since maybe nftables talk
was impressive to me ;) but it sounds that it will make easier to implement
a module to track protocols using broadcast. 

-- Yasuyuki Kozakai

Re: [PATCH 0/2] IPv6 conntrack support for neighbour discovery

[Patrick McHardy]

Yasuyuki KOZAKAI wrote:
> From: Patrick McHardy <kaber@trash.net>
> Date: Mon, 26 Jan 2009 14:11:37 +0100
> 
>>> Thank you. I understand why ICMPv6 packets are special here and
>>> I agree to assign UNTRACKED to them. Indeed non-invertible tuple might
>>> bring issues.
>> How about adding a flag to indicate that only one direction of
>> the tuple exists? It makes sense to support this for other kinds
>> of simplex flows as well in my opinion and it somewhat goes in the
>> same direction as the patch I talked about during the workshop
>> to have only a single tuple within the conntrack and have reply
>> tuples or potentially other tuples that relate to a connection
>> within the ct_extend area. And using NEW and having netlink
>> events seems more consistent to me.
> 
> It sounds good for long term solution. For now Eric's patch is enough,
> I think.

OK, thanks.

> And sorry, I don't remember your patch in detail since maybe nftables talk
> was impressive to me ;) but it sounds that it will make easier to implement
> a module to track protocols using broadcast. 

:) Yes, that was one of the ideas. The other one was that f.i. protocols
like SIP don't actually care about the network identitities of their
flows and might send traffic related to a single logical connection on
multiple different flows. I'm hoping that allowing more than two tuples
per conntrack will help with proper tracking.