Re: [PATCH 0/5] Return fds from privileged sockhash/sockmap lookup

[Alexei Starovoitov <alexei.starovoitov@gmail.com>]

On Fri, Mar 13, 2020 at 10:48:57AM +0000, Lorenz Bauer wrote:
> On Thu, 12 Mar 2020 at 17:58, Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
> >
> > but there it goes through ptrace checks and lsm hoooks, whereas here similar
> > security model cannot be enforced. bpf prog can put any socket into sockmap and
> > from bpf_lookup_elem side there is no way to figure out the owner task of the
> > socket to do ptrace checks. Just doing it all under CAP_NET_ADMIN is not a
> > great security answer.
> 
> Reading between the lines, you're concerned about something like a sock ops
> program "stealing" the socket and putting it in a sockmap, to be retrieved by an
> attacker later on?
> 
> How is that different than BPF_MAP_GET_FD_BY_ID, except that it's CAP_SYS_ADMIN?

It's different because it's crossing domains. FD_BY_ID returns FD for bpf
objects. Whereas here you're proposing bpf lookup to return FD from different
domain. If lookup was returning a socket cookie and separate api on the
networking side would convert cookie into FD I would be fine with that.

> > but bpf side may still need to insert them into old.
> > you gonna solve it with a flag for the prog to stop doing its job?
> > Or the prog will know that it needs to put sockets into second map now?
> > It's really the same problem as with classic so_reuseport
> > which was solved with BPF_MAP_TYPE_REUSEPORT_SOCKARRAY.
> 
> We don't modify the sockmap from eBPF:
>    receive a packet -> lookup sk in sockmap based on packet -> redirect
> 
> Why do you think we'll have to insert sockets from BPF?

sure, but sockmap allows socket insertion. Hence it's part of considerations.

> 
> > I think sockmap needs a redesign. Consider that today all sockets can be in any
> > number of sk_local_storage pseudo maps. They are 'defragmented' and resizable.
> > I think plugging socket redirect to use sk_local_storage-like infra is the
> > answer.
> 
> Maybe Jakub can speak more to this but I don't see how this solves our problem.
> We need a way to get at struct sk * from an eBPF program that runs on
> an skb context,
> to make BPF socket dispatch feasible. How would we use
> sk_local_storage if we don't
> have a sk?

I'm not following. There is skb->sk. Why do you need to lookup sk ? Because
your hook is before demux and skb->sk is not set? Then move your hook to after?

I think we're arguing in circles because in this thread I haven't seen the
explanation of the problem you're trying to solve. We argued about your
proposed solution and got stuck. Can we restart from the beginning with all
details?