netdev - Re: [RFC][PATCH kernel_bpf] honor CAP_NET_ADMIN for BPF_PROG

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]

Message-ID: <7b488768fb9ce1825597b510550ed6f8e9c88193.camel@domdv.de>
Date:   Wed, 05 Jun 2019 12:56:39 +0200
From:   Andreas Steinmetz <ast@...dv.de>
To:     Song Liu <liu.song.a23@...il.com>
Cc:     Networking <netdev@...r.kernel.org>, bpf <bpf@...r.kernel.org>
Subject: Re: [RFC][PATCH kernel_bpf] honor CAP_NET_ADMIN for BPF_PROG_LOAD

On Tue, 2019-05-28 at 14:04 -0700, Song Liu wrote:
> >          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.

Reworking the if-statement is possible but the result is something like:

        if ((type != BPF_PROG_TYPE_SOCKET_FILTER &&
             type != BPF_PROG_TYPE_CGROUP_SKB &&
             !capable(CAP_SYS_ADMIN)) &&
            !((type == BPF_PROG_TYPE_SCHED_CLS ||
               type == BPF_PROG_TYPE_XDP) &&
              capable(CAP_NET_ADMIN)))
                return -EPERM;

This is not really readable and I do prefer an easy to verify code
when it comes to security, so how about the following version:

Signed-off-by: Andreas Steinmetz <ast@...dv.de>

--- a/kernel/bpf/syscall.c	2019-05-28 18:00:40.472841432 +0200
+++ b/kernel/bpf/syscall.c	2019-06-05 12:34:48.197107612 +0200
@@ -1559,10 +1559,18 @@ static int bpf_prog_load(union bpf_attr
 
 	if (attr->insn_cnt == 0 || attr->insn_cnt > BPF_MAXINSNS)
 		return -E2BIG;
-	if (type != BPF_PROG_TYPE_SOCKET_FILTER &&
-	    type != BPF_PROG_TYPE_CGROUP_SKB &&
-	    !capable(CAP_SYS_ADMIN))
-		return -EPERM;
+	switch (type) {
+	case BPF_PROG_TYPE_SOCKET_FILTER:
+	case BPF_PROG_TYPE_CGROUP_SKB:
+		break;
+	case BPF_PROG_TYPE_SCHED_CLS:
+	case BPF_PROG_TYPE_XDP:
+		if (capable(CAP_NET_ADMIN))
+			break;
+	default:
+		if (!capable(CAP_SYS_ADMIN))
+			return -EPERM;
+	}
 
 	bpf_prog_load_fixup_attach_type(attr);
 	if (bpf_prog_load_check_attach_type(type, attr->expected_attach_type))