| Message ID | 20220831152414.171484-1-ykaliuta@redhat.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | BPF |
| Headers | show |
| Series | [RFC,v2] bpf: use bpf_capable() instead of CAP_SYS_ADMIN for blinding decision | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
| bpf/vmtest-bpf-next-PR | fail | PR summary |
| bpf/vmtest-bpf-next-VM_Test-1 | success | Logs for build for s390x with gcc |
| bpf/vmtest-bpf-next-VM_Test-2 | success | Logs for build for x86_64 with gcc |
| bpf/vmtest-bpf-next-VM_Test-3 | success | Logs for build for x86_64 with llvm-16 |
| bpf/vmtest-bpf-next-VM_Test-4 | success | Logs for llvm-toolchain |
| bpf/vmtest-bpf-next-VM_Test-5 | success | Logs for set-matrix |
| bpf/vmtest-bpf-next-VM_Test-6 | success | Logs for test_maps on s390x with gcc |
| bpf/vmtest-bpf-next-VM_Test-7 | success | Logs for test_maps on x86_64 with gcc |
| bpf/vmtest-bpf-next-VM_Test-8 | success | Logs for test_maps on x86_64 with llvm-16 |
| bpf/vmtest-bpf-next-VM_Test-9 | fail | Logs for test_progs on s390x with gcc |
| bpf/vmtest-bpf-next-VM_Test-10 | success | Logs for test_progs on x86_64 with gcc |
| bpf/vmtest-bpf-next-VM_Test-11 | success | Logs for test_progs on x86_64 with llvm-16 |
| bpf/vmtest-bpf-next-VM_Test-12 | success | Logs for test_progs_no_alu32 on s390x with gcc |
| bpf/vmtest-bpf-next-VM_Test-13 | success | Logs for test_progs_no_alu32 on x86_64 with gcc |
| bpf/vmtest-bpf-next-VM_Test-14 | success | Logs for test_progs_no_alu32 on x86_64 with llvm-16 |
| bpf/vmtest-bpf-next-VM_Test-15 | success | Logs for test_verifier on s390x with gcc |
| bpf/vmtest-bpf-next-VM_Test-16 | success | Logs for test_verifier on x86_64 with gcc |
| bpf/vmtest-bpf-next-VM_Test-17 | success | Logs for test_verifier on x86_64 with llvm-16 |
On Wed, Aug 31, 2022 at 06:24:14PM +0300, Yauheni Kaliuta wrote: > The capability check can cause SELinux denial. > > For example, in ptp4l, setsockopt() with the SO_ATTACH_FILTER option > raises sk_attach_filter() to run a bpf program. SELinux hooks into > capable() calls and performs an additional check if the task's > SELinux domain has permission to "use" the given capability. ptp4l_t > already has CAP_BPF granted by SELinux, so if the function used > bpf_capable() as most BPF code does, there would be no change needed > in selinux-policy. The selinux mentions probably aren't really necessary. The more concise way to say it is that bpf_jit_blinding_enabled() should be permitted with CAP_BPF, that full CAP_SYS_ADMIN is not needed. (Assuming that that is the case) > Signed-off-by: Yauheni Kaliuta <ykaliuta@redhat.com> > --- > > v2: put the reasoning in the commit message > > --- > include/linux/filter.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/include/linux/filter.h b/include/linux/filter.h > index a5f21dc3c432..3de96b1a736b 100644 > --- a/include/linux/filter.h > +++ b/include/linux/filter.h > @@ -1100,7 +1100,7 @@ static inline bool bpf_jit_blinding_enabled(struct bpf_prog *prog) > return false; > if (!bpf_jit_harden) > return false; > - if (bpf_jit_harden == 1 && capable(CAP_SYS_ADMIN)) > + if (bpf_jit_harden == 1 && bpf_capable()) > return false; > > return true; > -- > 2.34.1
On 8/31/22 8:50 PM, Serge E. Hallyn wrote: > On Wed, Aug 31, 2022 at 06:24:14PM +0300, Yauheni Kaliuta wrote: >> The capability check can cause SELinux denial. >> >> For example, in ptp4l, setsockopt() with the SO_ATTACH_FILTER option >> raises sk_attach_filter() to run a bpf program. SELinux hooks into >> capable() calls and performs an additional check if the task's >> SELinux domain has permission to "use" the given capability. ptp4l_t >> already has CAP_BPF granted by SELinux, so if the function used >> bpf_capable() as most BPF code does, there would be no change needed >> in selinux-policy. > > The selinux mentions probably aren't really necessary. The more > concise way to say it is that bpf_jit_blinding_enabled() should > be permitted with CAP_BPF, that full CAP_SYS_ADMIN is not needed. > (Assuming that that is the case) > >> Signed-off-by: Yauheni Kaliuta <ykaliuta@redhat.com> >> --- >> >> v2: put the reasoning in the commit message >> >> --- >> include/linux/filter.h | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/include/linux/filter.h b/include/linux/filter.h >> index a5f21dc3c432..3de96b1a736b 100644 >> --- a/include/linux/filter.h >> +++ b/include/linux/filter.h >> @@ -1100,7 +1100,7 @@ static inline bool bpf_jit_blinding_enabled(struct bpf_prog *prog) >> return false; >> if (!bpf_jit_harden) >> return false; >> - if (bpf_jit_harden == 1 && capable(CAP_SYS_ADMIN)) >> + if (bpf_jit_harden == 1 && bpf_capable()) I think lowering this requirement is fine here. These days given unpriv eBPF is disabled by default, I see the main users for constant blinding coming from unpriv in particular via cBPF -> eBPF migration (e.g. old-style socket filters). >> return false; >> >> return true; Please also update Documentation/admin-guide/sysctl/net.rst to clarify cap details. Thanks, Daniel
diff --git a/include/linux/filter.h b/include/linux/filter.h index a5f21dc3c432..3de96b1a736b 100644 --- a/include/linux/filter.h +++ b/include/linux/filter.h @@ -1100,7 +1100,7 @@ static inline bool bpf_jit_blinding_enabled(struct bpf_prog *prog) return false; if (!bpf_jit_harden) return false; - if (bpf_jit_harden == 1 && capable(CAP_SYS_ADMIN)) + if (bpf_jit_harden == 1 && bpf_capable()) return false; return true;
The capability check can cause SELinux denial. For example, in ptp4l, setsockopt() with the SO_ATTACH_FILTER option raises sk_attach_filter() to run a bpf program. SELinux hooks into capable() calls and performs an additional check if the task's SELinux domain has permission to "use" the given capability. ptp4l_t already has CAP_BPF granted by SELinux, so if the function used bpf_capable() as most BPF code does, there would be no change needed in selinux-policy. Signed-off-by: Yauheni Kaliuta <ykaliuta@redhat.com> --- v2: put the reasoning in the commit message --- include/linux/filter.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)