netdev - Re: Design for sk_lookup helper function in context of sk

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] [day] [month] [year] [list]

Date:   Mon, 22 Mar 2021 10:59:22 -0700
From:   Martin KaFai Lau <kafai@...com>
To:     Shanti Lombard née Bouchez-Mongardé 
        <mildred@...dred.fr>
CC:     bpf <bpf@...r.kernel.org>, <netdev@...r.kernel.org>,
        <alexei.starovoitov@...il.com>
Subject: Re: Design for sk_lookup helper function in context of sk_lookup hook

On Fri, Mar 19, 2021 at 06:05:20PM +0100, Shanti Lombard née Bouchez-Mongardé wrote:
> Le 19/03/2021 à 17:55, Martin KaFai Lau a écrit :
> > On Wed, Mar 17, 2021 at 10:04:18AM +0100, Shanti Lombard née Bouchez-Mongardé wrote:
> > > Q1: How do we prevent socket lookup from triggering BPF sk_lookup causing a
> > > loop?
> > The bpf_sk_lookup_(tcp|udp) will be called from the BPF_PROG_TYPE_SK_LOOKUP program?
> 
> Yes, the idea is to allow the BPF program to redirect incoming connections
> for 0.0.0.0:1234 to a specific IP address such as 127.0.12.34:1234 or any
> other combinaison with a binding not done based on a predefined socket file
> descriptor but based on a listening IP address for a socket.
> 
> See linked discussion in the original message
> 
> > > - Solution A: We add a flag to the existing inet_lookup exported function
> > > (and similarly for inet6, udp4 and udp6). The INET_LOOKUP_SKIP_BPF_SK_LOOKUP
> > > flag, when set, would prevent BPF sk_lookup from happening. It also requires
> > > modifying every location making use of those functions.
> > > 
> > > - Solution B: We export a new symbol in inet_hashtables, a wrapper around
> > > static function inet_lhash2_lookup for inet4 and similar functions for inet6
> > > and udp4/6. Looking up specific IP/port and if not found looking up for
> > > INADDR_ANY could be done in the helper function in net/core/filters.c or in
> > > the BPF program.
For TCP, it is only for lhash lookup, right?

> > > 
> > > Q2: Should we reuse the bpf_sk_lokup_tcp and bpf_sk_lookup_udp helper
> > > functions or create new ones?
> > If the args passing to the bpf_sk_lookup_(tcp|udp) is the same,
> > it makes sense to reuse the same BPF_FUNC_sk_lookup_*.
> > The actual helper implementation could be different though.
> > Look at bpf_xdp_sk_lookup_tcp_proto and bpf_sk_lookup_tcp_proto.
> 
> I was thinking that perhaps a different helper method which would take
> IPPROTO_TCP or IPPROTO_UDP parameter would be better suited. it would avoid
> BPF code such as :
> 
>     switch(ctx->protocol){
>         case IPPROTO_TCP:
>             sk = bpf_sk_lookup_tcp(ctx, &tuple, tuple_size, -1, 0);
>             break;
>         case IPPROTO_UDP:
>             sk = bpf_sk_lookup_udp(ctx, &tuple, tuple_size, -1, 0);
>             break;
>         default:
>             return SK_PASS;
>     }
> 
> But then there is the limit of 5 arguments, isn't it, so perhaps the
> _tcp/_udp functions are not such a bad idea after all.
> 
> I saw already that the same helper functions could be given different
> implementations. And if there is no way to have more than 5 arguments then
> this is probably better to reuse the same helper function name and
> signature.