netdev - Re: [PATCH RFC 0/3] Allow postponed netfilter handling for socket matches

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <560035B4.9010504@zonque.org>
Date:	Mon, 21 Sep 2015 18:52:04 +0200
From:	Daniel Mack <daniel@...que.org>
To:	Florian Westphal <fw@...len.de>
Cc:	pablo@...filter.org, daniel@...earbox.net,
	netfilter-devel@...r.kernel.org, netdev@...r.kernel.org,
	balazs.scheidler@...abit.com
Subject: Re: [PATCH RFC 0/3] Allow postponed netfilter handling for socket
 matches

Hi,

Thanks for your feedback, Florian!

On 09/17/2015 06:00 PM, Florian Westphal wrote:
> Daniel Mack <daniel@...que.org> wrote:

>> That would be a new netfilter hook then, something that is called after
>> LOCAL_IN, for ingress only? In a sense, it would be called from the
>> protocol handlers, just as my patches do right now, but instead of
>> conditionally re-iterating the same rules again, we would walk a
>> different chain?
> 
> Yes, something like that.  Obviously, you'll need to dru^W brib^W
> convince a LOT of people before that could ever fly.
> 
> I think we should not do this and that this 'match on ingress sk
> properties' is just bad[tm].
> 
> f.e. you'd also have to move all of the stuff you want into
> sock_common ... 8-(

Hmm, I'm not sure whether I understand which problems you see, or which
corner cases I am missing in my assessment. I did a quick test with the
attached 4 patches that

1) Allow hook callbacks to look at the socket passed to nf_hook(), so
   skb->sk does not have to be set

2) Make nft_meta look at pkt->sk rather that skb->sk (only for cgroups
   as proof of concept)

3) Introduce a new POST_DEMUX netfilter chain (the name is not
   perfect, admittedly)

4) Iterate POST_DEMUX chains for v4 TCP and UDP unicast+multicast
   sockets.

With some really trivial modifications to libnftnl/nftables (which just
map strings to the new enum value), this works fine in my tests.
Multicast receivers that match a netclass ID in the ruleset won't see
any packets, while others do.

Some more considerations: if we cannot determine a socket for a packet
and hence don't deliver it, it's IMO perfectly fine not to run the
netfilter rules for them. All we need to achieve with this chain is that
for packets that _are_ delivered to a socket, all the necessary rules
have been processed, at a time when we know who the final receiver of
the skb is.

I'm happy to discuss the side effects of such an approach.

Thanks,
Daniel

View attachment "0001-netfilter-add-socket-to-struct-nft_pktinfo.patch" of type "text/x-diff" (1509 bytes)

View attachment "0002-netfilter-nft_meta-look-at-pkt-sk-rather-than-skb-sk.patch" of type "text/x-diff" (1444 bytes)

View attachment "0003-netfilter-add-NF_INET_POST_DEMUX-chain-type.patch" of type "text/x-diff" (2936 bytes)

View attachment "0004-net-tcp_ipv4-udp_ipv4-hook-up-post-demux-netfilter-c.patch" of type "text/x-diff" (2261 bytes)