Le vendredi 23 janvier 2009 à 19:21 +0900, Yasuyuki KOZAKAI a écrit :
> Hi,
>
> From: Eric Leblond <eric@xxxxxx>
> 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@xxxxxx>
> 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@xxxxxx>
> > ---
> > 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@xxxxxx>
> 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@xxxxxx>
>
> <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@xxxxxx>
INL, http://www.inl.fr/
NuFW, http://www.nufw.org
Attachment:
signature.asc
Description: Ceci est une partie de message =?ISO-8859-1?Q?num=E9riquement?= =?ISO-8859-1?Q?_sign=E9e?=
