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@xxxxxxxxx> 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?