Re: [RFC][PATCH kernel_bpf] honor CAP_NET_ADMIN for BPF_PROG_LOAD

[Song Liu <liu.song.a23@gmail.com>]

On Tue, May 28, 2019 at 9:59 AM Andreas Steinmetz <ast@domdv.de> wrote:
>
> [sorry for crossposting but this affects both lists]
>
> BPF_PROG_TYPE_SCHED_CLS and BPF_PROG_TYPE_XDP should be allowed
> for CAP_NET_ADMIN capability. Nearly everything one can do with
> these program types can be done some other way with CAP_NET_ADMIN
> capability (e.g. NFQUEUE), but only slower.
>
> This change is similar in behaviour to the /proc/sys/net
> CAP_NET_ADMIN exemption.
>
> Overall chances are of increased security as network related
> applications do no longer require to keep CAP_SYS_ADMIN
> admin capability for network related eBPF operations.
>
> It may well be that other program types than BPF_PROG_TYPE_XDP
> and BPF_PROG_TYPE_SCHED_CLS do need the same exemption, though
> I do not have sufficient knowledge of other program types
> to be able to decide this.
>
> Preloading BPF programs is not possible in case of application
> modified or generated BPF programs, so this is no alternative.
> The verifier does prevent the BPF program from doing harmful
> things anyway.
>
> Signed-off-by: Andreas Steinmetz <ast@domdv.de>
>
> --- a/kernel/bpf/syscall.c      2019-05-28 18:00:40.472841432 +0200
> +++ b/kernel/bpf/syscall.c      2019-05-28 18:17:50.162811510 +0200
> @@ -1561,8 +1561,13 @@ static int bpf_prog_load(union bpf_attr
>                 return -E2BIG;
>         if (type != BPF_PROG_TYPE_SOCKET_FILTER &&
>             type != BPF_PROG_TYPE_CGROUP_SKB &&

You should extend this if () statement instead of adding another
if () below.

Thanks,
Song

> -           !capable(CAP_SYS_ADMIN))
> -               return -EPERM;
> +           !capable(CAP_SYS_ADMIN)) {
> +               if (type != BPF_PROG_TYPE_SCHED_CLS &&
> +                   type != BPF_PROG_TYPE_XDP)
> +                       return -EPERM;
> +               if(!capable(CAP_NET_ADMIN))
> +                       return -EPERM;
> +       }
>
>         bpf_prog_load_fixup_attach_type(attr);
>         if (bpf_prog_load_check_attach_type(type, attr->expected_attach_type))
>