Re: [PATCH bpf-next v4 3/8] bpf: lsm: provide attachment points for BPF LSM programs
From: Alexei Starovoitov <hidden>
Date: 2020-02-25 05:41:35
Also in:
bpf, lkml, netdev
Subsystem:
security subsystem, the rest · Maintainers:
Paul Moore, James Morris, "Serge E. Hallyn", Linus Torvalds
On Mon, Feb 24, 2020 at 01:41:19PM -0800, Kees Cook wrote:
But the LSM subsystem doesn't want special cases (Casey has worked very hard to generalize everything there for stacking). It is really hard to accept adding a new special case when there are still special cases yet to be worked out even in the LSM code itself[2]. [2] Casey's work to generalize the LSM interfaces continues and it quite complex: https://lore.kernel.org/linux-security-module/20200214234203.7086-1-casey@schaufler-ca.com/ (local)
I think the key mistake we made is that we classified KRSI as LSM. LSM stacking, lsmblobs that the above set is trying to do are not necessary for KRSI. I don't see anything in LSM infra that KRSI can reuse. The only thing BPF needs is a function to attach to. It can be a nop function or any other. security_*() functions are interesting from that angle only. Hence I propose to reconsider what I was suggesting earlier. No changes to secruity/ directory. Attach to security_*() funcs via bpf trampoline. The key observation vs what I was saying earlier is KRSI and LSM are wrong names. I think "security" is also loaded word that should be avoided. I'm proposing to rename BPF_PROG_TYPE_LSM into BPF_PROG_TYPE_OVERRIDE_RETURN.
So, unless James is going to take this over Casey's objections, the path forward I see here is: - land a "slow" KRSI (i.e. one that hooks every hook with a stub). - optimize calling for all LSMs
I'm very much surprised how 'slow' KRSI is an option at all. 'slow' KRSI means that CONFIG_SECURITY_KRSI=y adds indirect calls to nop functions for every place in the kernel that calls security_*(). This is not an acceptable overhead. Even w/o retpoline this is not something datacenter servers can use. Another option is to do this:
diff --git a/include/linux/security.h b/include/linux/security.h
index 64b19f050343..7887ce636fb1 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h@@ -240,7 +240,7 @@ static inline const char *kernel_load_data_id_str(enum kernel_load_data_id id) return kernel_load_data_str[id]; } -#ifdef CONFIG_SECURITY +#if defined(CONFIG_SECURITY) || defined(CONFIG_BPF_OVERRIDE_RETURN)
Single line change to security.h and new file kernel/bpf/override_security.c
that will look like:
int security_binder_set_context_mgr(struct task_struct *mgr)
{
return 0;
}
int security_binder_transaction(struct task_struct *from,
struct task_struct *to)
{
return 0;
}
Essentially it will provide BPF side with a set of nop functions.
CONFIG_SECURITY is off. It may seem as a downside that it will force a choice
on kernel users. Either they build the kernel with CONFIG_SECURITY and their
choice of LSMs or build the kernel with CONFIG_BPF_OVERRIDE_RETURN and use
BPF_PROG_TYPE_OVERRIDE_RETURN programs to enforce any kind of policy. I think
it's a pro not a con.