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

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:   Mon, 16 Mar 2020 14:23:24 -0700
From:   Joe Stringer <joe@...d.net.nz>
To:     Jakub Sitnicki <jakub@...udflare.com>
Cc:     Joe Stringer <joe@...d.net.nz>, bpf@...r.kernel.org,
        netdev <netdev@...r.kernel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Alexei Starovoitov <ast@...nel.org>,
        Eric Dumazet <eric.dumazet@...il.com>,
        Lorenz Bauer <lmb@...udflare.com>,
        Florian Westphal <fw@...len.de>
Subject: Re: [PATCH bpf-next 3/7] bpf: Add socket assign support

On Mon, Mar 16, 2020 at 3:08 AM Jakub Sitnicki <jakub@...udflare.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