Re: [PATCH v2] netfilter: Parse ICMPv6 redirects

["Alin Năstac" <alin.nastac@gmail.com>]

On Mon, Mar 13, 2017 at 6:00 PM, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> On Mon, Mar 13, 2017 at 03:17:22PM +0100, Alin Năstac wrote:
>> On Mon, Mar 13, 2017 at 2:44 PM, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
>> > 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.
>>
>> People should not rely on buggy behaviour to keep them safe. Imagine
>> for instance there is a bug that prevents packets sent by HTTP servers
>> to match "-m conntrack --state ESTABLISHED" rules. Would you add a fix
>> that is operational only when an obscure procfs knob gets enabled?
>
> Come on, this behaviour has been there for more than 10 years...
>
>> Redirects are supposed to be sent to on-link hosts, so all we want in
>> fact is to allow these packets on INPUT. Would it be OK to restrict
>> RELATED status to redirects originated from link-local addresses? This
>> will be in line with RFC 4861 requirement that source address of
>> ICMPv6 redirects must be in link-local scope.
>
> I think restricting this to link-local, if possible, would be fine.

I take it would be fine but still not enough. :-(

Fine, drop this patch then. Better allow ICMPv6 redirects through an
ip6tables INPUT rule placed in front of the usual
   -m conntrack --ctstate RELATED,ESTABLISHED -j ACCEPT
than having to patch the kernel and use sysctl to alter default behaviour.

I only wish you told me earlier not to waste time on fixing a bug that
no one is interested to take it in.