netdev - Re: [PATCH] Allow TCP connections to cache SYN packet for userspace inspection

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]

Date:	Fri, 1 May 2015 16:14:17 -0400
From:	Eric B Munson <emunson@...mai.com>
To:	Tom Herbert <tom@...bertland.com>
Cc:	Eric Dumazet <eric.dumazet@...il.com>,
	"David S. Miller" <davem@...emloft.net>,
	Alexey Kuznetsov <kuznet@....inr.ac.ru>,
	James Morris <jmorris@...ei.org>,
	Hideaki YOSHIFUJI <yoshfuji@...ux-ipv6.org>,
	Patrick McHardy <kaber@...sh.net>,
	Linux Kernel Network Developers <netdev@...r.kernel.org>,
	linux-api@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] Allow TCP connections to cache SYN packet for userspace
 inspection

On Fri, 01 May 2015, Tom Herbert wrote:

> On Fri, May 1, 2015 at 11:42 AM, Eric Dumazet <eric.dumazet@...il.com> wrote:
> > On Fri, 2015-05-01 at 13:43 -0400, Eric B Munson wrote:
> >> In order to enable policy decisions in userspace, the data contained in
> >> the SYN packet would be useful for tracking or identifying connections.
> >> Only parts of this data are available to userspace after the hand shake
> >> is completed.  This patch exposes a new setsockopt() option that will,
> >> when used with a listening socket, ask the kernel to cache the skb
> >> holding the SYN packet for retrieval later.  The SYN skbs will not be
> >> saved while the kernel is in syn cookie mode.
> >>
> >> The same option will ask the kernel for the packet headers when used
> >> with getsockopt() with the socket returned from accept().  The cached
> >> packet will only be available for the first getsockopt() call, the skb
> >> is consumed after the requested data is copied to userspace.  Subsequent
> >> calls will return -ENOENT.  Because of this behavior, getsockopt() will
> >> return -E2BIG if the caller supplied a buffer that is too small to hold
> >> the skb header.
> >>
> >> Signed-off-by: Eric B Munson <emunson@...mai.com>
> >> Cc: Alexey Kuznetsov <kuznet@....inr.ac.ru>
> >> Cc: James Morris <jmorris@...ei.org>
> >> Cc: Hideaki YOSHIFUJI <yoshfuji@...ux-ipv6.org>
> >> Cc: Patrick McHardy <kaber@...sh.net>
> >> Cc: netdev@...r.kernel.org
> >> Cc: linux-api@...r.kernel.org
> >> Cc: linux-kernel@...r.kernel.org
> >> ---
> >
> > We have a similar patch here at Google, but we do not hold one skb and
> > dst per saved syn. That can be ~4KB for some drivers.
> >
> > Only a kmalloc() with the needed part (headers), usually less than 128
> > bytes. We store the length in first byte of this allocation.
> >
> > This has a huge difference if you want to have ~4 million request socks.
> >
> +1 on kmalloc solution. I posted a similar patch a couple of years ago
> https://patchwork.ozlabs.org/patch/146034/. There was pushback on
> memory usage and this having to narrow of a use case.
> 
> Tom
> 

I cached the skb largely to take advantage of the built in reference
counting and avoid having to manage allocating memory and ownership of
said memory.  For V2, how about I keep the skb reference in the request
structure and kmalloc() a buffer, to be owned by the tcp sock structure,
when the new tcp socket is created?  This would also simplify the
getsockopt() so that the data was available to all callers until the
socket is closed.

Eric

Download attachment "signature.asc" of type "application/pgp-signature" (820 bytes)