Thread (37 messages) 37 messages, 10 authors, 2021-06-09

Re: [PATCH v2] lockdown,selinux: avoid bogus SELinux lockdown permission checks

From: Paul Moore <paul@paul-moore.com>
Date: 2021-05-28 22:52:53
Also in: bpf, linux-fsdevel, linuxppc-dev, lkml, netdev, selinux

On Fri, May 28, 2021 at 2:28 PM Daniel Borkmann [off-list ref] wrote:
On 5/28/21 5:47 PM, Paul Moore wrote:
quoted
Let's reset.
Sure, yep, lets shortly take one step back. :)
quoted
What task_struct is running the BPF tracing program which is calling
into security_locked_down()?  My current feeling is that it is this
context/domain/cred that should be used for the access control check;
in the cases where it is a kernel thread, I think passing NULL is
reasonable, but I think the proper thing for SELinux is to interpret
NULL as kernel_t.
If this was a typical LSM hook and, say, your app calls into bind(2) where
we then invoke security_socket_bind() and check 'current' task, then I'm all
with you, because this was _explicitly initiated_ by the httpd app, so that
allow/deny policy belongs in the context of httpd.

In the case of tracing, it's different. You install small programs that are
triggered when certain events fire. Random example from bpftrace's README [0],
you want to generate a histogram of syscall counts by program. One-liner is:

   bpftrace -e 'tracepoint:raw_syscalls:sys_enter { @[comm] = count(); }'

bpftrace then goes and generates a BPF prog from this internally. One way of
doing it could be to call bpf_get_current_task() helper and then access
current->comm via one of bpf_probe_read_kernel{,_str}() helpers. So the
program itself has nothing to do with httpd or any other random app doing
a syscall here. The BPF prog _explicitly initiated_ the lockdown check.
The allow/deny policy belongs in the context of bpftrace: meaning, you want
to grant bpftrace access to use these helpers, but other tracers on the
systems like my_random_tracer not. While this works for prior mentioned
cases of security_locked_down() with open_kcore() for /proc/kcore access
or the module_sig_check(), it is broken for tracing as-is, and the patch
I sent earlier fixes this.
Sigh.

Generally it's helpful when someone asks a question if you answer it
directly before going off and answering your own questions.  Listen, I
get it, you wrote a patch and it fixes your problem (you've mentioned
that already) and it's wonderful and all that, but the rest of us
(maybe just me) need to sort this out too and talking past questions
isn't a great way to help us get there (once again, maybe just me).  I
think I can infer an answer from you, but you've made me grumpy now so
I'm not ACK'ing or NACK'ing anything right now; I clearly need to go
spend some time reading through BPF code.  Woo.
   [0] https://github.com/iovisor/bpftrace
-- 
paul moore
www.paul-moore.com
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help