Re: [PATCH bpf-next v4 3/8] bpf: lsm: provide attachment points for BPF LSM programs

[Alexei Starovoitov <alexei.starovoitov@gmail.com>]

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/

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.