Re: [PATCH v2] netfilter: Parse ICMPv6 redirects

[Pablo Neira Ayuso <pablo@netfilter.org>]

On Mon, Mar 13, 2017 at 02:17:39PM +0100, Alin Năstac wrote:
> Hi Pablo,
> 
> On Mon, Mar 13, 2017 at 1:40 PM, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > On Tue, Mar 07, 2017 at 11:00:43AM +0100, Alin Nastac wrote:
> >> Extract IPv6 packet that triggered the sending of redirect message from
> >> ICMPv6 Redirected Header option and check if conntrack table contain such
> >> connection. Mark redirect packet as RELATED if a matching connection is found.
> >
> > I think we need a sysctl to enable this on demand, eg.
> > 'nf_conntrack_icmpv6_accept_redirects'
> >
> > This is changing the default behaviour, my main concern here is that
> > filtering policies not accepting redirects will now make it via
> > RELATED.
> 
> net/ipv4/netfilter/nf_conntrack_proto_icmp.c  give RELATED status to
> all ICMP redirect messages that refer to valid conntracks. Why would
> ICMPv6 redirect case be any different?

That's very valid argument, but we have this asymmetry for long time
ago, basically since the beginning. As I said, I have concerns on
changing this default behaviour without an explicit knob. This
behaviour change will go through inadvertently for many people.

> Would you implement a similar sysctl switch for ICMP redirect
> RELATED state? And if you do, would you accept to enable these
> switches by default?

I don't think we shouldn't enable this by default. We have tried to be
conservative on that side so far. Is it a problem there to enable this
via sysctl.conf?

Am I missing any requirement there that is not described in your patch
description?

Thanks.