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] [day] [month] [year] [list]

Message-ID: <20150921190555.GD931@breakpoint.cc>
Date:	Mon, 21 Sep 2015 21:05:55 +0200
From:	Florian Westphal <fw@...len.de>
To:	Daniel Mack <daniel@...que.org>
Cc:	Florian Westphal <fw@...len.de>, 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

Daniel Mack <daniel@...que.org> wrote:
> >> 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.

Not sure if thats true.  What about Timewait sockets?

Its easy to imagine someone using this feature and then complaining
that it doesn't match some packets, at which point we'd have to grow
sock_common to accomondate all sk member we support matching for :-/

If we'd have kernel releases where we drop features this wouldn't be
much of an issue since we could back out in case it causes issues later.

But once we add your proposed feature we cannot go back...

I'm not sure; I dislike this feature proposal but I can't think of
any alternative [other than "don't do this"] :-(
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html