Re: [PATCH bpf-next 3/7] bpf: Add socket assign support

From: Joe Stringer <joe@wand.net.nz>
To: Jakub Sitnicki <jakub@cloudflare.com>
Cc: Joe Stringer <joe@wand.net.nz>,
	bpf@vger.kernel.org, netdev <netdev@vger.kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Alexei Starovoitov <ast@kernel.org>,
	Eric Dumazet <eric.dumazet@gmail.com>,
	Lorenz Bauer <lmb@cloudflare.com>,
	Florian Westphal <fw@strlen.de>
Subject: Re: [PATCH bpf-next 3/7] bpf: Add socket assign support
Date: Mon, 16 Mar 2020 14:23:24 -0700	[thread overview]
Message-ID: <CAOftzPiUKa87U4UtxFMvWPpZYTTjvfgyb5E=u110jRCsjUh--g@mail.gmail.com> (raw)
In-Reply-To: <87mu8gy5m6.fsf@cloudflare.com>

On Mon, Mar 16, 2020 at 3:08 AM Jakub Sitnicki <jakub@cloudflare.com> wrote:
>
> [+CC Florian]
>
> Hey Joe,
>
> On Fri, Mar 13, 2020 at 12:36 AM CET, Joe Stringer wrote:
> > Add support for TPROXY via a new bpf helper, bpf_sk_assign().
> >
> > This helper requires the BPF program to discover the socket via a call
> > to bpf_sk*_lookup_*(), then pass this socket to the new helper. The
> > helper takes its own reference to the socket in addition to any existing
> > reference that may or may not currently be obtained for the duration of
> > BPF processing. For the destination socket to receive the traffic, the
> > traffic must be routed towards that socket via local route, the socket
> > must have the transparent option enabled out-of-band, and the socket
> > must not be closing. If all of these conditions hold, the socket will be
> > assigned to the skb to allow delivery to the socket.
>
> My impression from the last time we have been discussing TPROXY is that
> the check for IP_TRANSPARENT on ingress doesn't serve any purpose [0].
>
> The socket option only has effect on output, when there is a need to
> source traffic from a non-local address.
>
> Setting IP_TRANSPARENT requires CAP_NET_{RAW|ADMIN}, which grant a wider
> range of capabilities than needed to build a transparent proxy app. This
> is problematic because you to lock down your application with seccomp.
>
> It seems it should be enough to use a port number from a privileged
> range, if you want to ensure that only the designed process can receive
> the proxied traffic.

Thanks for looking this over. You're right, I neglected to fix up the
commit message here from an earlier iteration that enforced this
constraint. I can fix this up in a v2.

> Or, alternatively, instead of using socket lookup + IP_TRANSPARENT
> check, get the socket from sockmap and apply control to who can update
> the BPF map.

There's no IP_TRANSPARENT check in this iteration of the series.

Cheers,
Joe