netdev - Re: [PATCH 07/31] netfilter: ctnetlink: Support L3 protocol-filter on flush

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20190502150637.6f7vqoxiheytg4le@salvia>
Date:   Thu, 2 May 2019 17:06:37 +0200
From:   Pablo Neira Ayuso <pablo@...filter.org>
To:     Nicolas Dichtel <nicolas.dichtel@...nd.com>
Cc:     Florian Westphal <fw@...len.de>,
        Kristian Evensen <kristian.evensen@...il.com>,
        Netfilter Development Mailing list 
        <netfilter-devel@...r.kernel.org>,
        David Miller <davem@...emloft.net>,
        Network Development <netdev@...r.kernel.org>
Subject: Re: [PATCH 07/31] netfilter: ctnetlink: Support L3 protocol-filter
 on flush

On Thu, May 02, 2019 at 02:56:42PM +0200, Nicolas Dichtel wrote:
> Le 02/05/2019 à 13:31, Pablo Neira Ayuso a écrit :
> > On Thu, May 02, 2019 at 09:46:42AM +0200, Florian Westphal wrote:
> >> Nicolas Dichtel <nicolas.dichtel@...nd.com> wrote:
> >>> I understand your point, but this is a regression. Ignoring a field/attribute of
> >>> a netlink message is part of the uAPI. This field exists for more than a decade
> >>> (probably two), so you cannot just use it because nobody was using it. Just see
> >>> all discussions about strict validation of netlink messages.
> >>> Moreover, the conntrack tool exists also for ages and is an official tool.
> >>
> >> FWIW I agree with Nicolas, we should restore old behaviour and flush
> >> everything when AF_INET is given.  We can add new netlink attr to
> >> restrict this.
> > 
> > Let's use nfgenmsg->version for this. This is so far set to zero. We
> > can just update userspace to set it to 1, so family is used.
> > 
> > The version field in the kernel size is ignored so far, so this should
> > be enough. So we avoid that extract netlink attribute.
>
> Why making such a hack? If any userspace app set this field (simply because it's
> not initialized), it will show up a new regression.
> What is the problem of adding another attribute?

The version field was meant to deal with this case.

It has been not unused so far because we had no good reason.