Re: BPF LSM prevent program unload

[Frederick Lawler <fred@cloudflare.com>]

On Thu, Dec 07, 2023 at 03:01:33PM +0100, KP Singh wrote:
> On Tue, Dec 5, 2023 at 9:39 PM Frederick Lawler <fred@cloudflare.com> wrote:
> >
> > Hi,
> >
> > IIUC, LSMs are supposed to give us the ability to design policy around
> > unprivileged users and in addition to privileged users. As we expand
> > our usage of BPF LSM's, there are cases where we want to restrict
> > privileged users from unloading our progs. For instance, any privileged
> > user that wants to remove restrictions we've placed on privileged users.
> >
> > We currently have a loader application doesn't leverage BPF skeletons. We
> > instead load BPF object files, and then pin the progs to a mount point that
> > is a bpf filesystem. On next run, if we have new policies, load in new
> > policies, and finally unload the old.
> >
> > Here are some conditions a privileged user may unload programs:
> >
> >         umount /sys/fs/bpf
> >         rm -rf /sys/fs/bpf/lsm
> >         rm /sys/fs/bpf/lsm/some_prog
> >         unlink /sys/fs/bpf/lsm/some_prog
> >
> > This works because once we remove the last reference, the programs and
> > pinned maps are cleaned up.
> >
> > Moving individual pins or moving the mount entirely with mount --move
> > do not perform any clean up operations. Lastly, bpftool doesn't currently
> > have the ability to unload LSM's AFAIK.
> >
> > The few ideas I have floating around are:
> >
> > 1. Leverage some LSM hooks (BPF or otherwise) to restrict on the functions
> >    security_sb_umount(), security_path_unlink(), security_inode_unlink().
> >
> >    Both security_path_unlink() and security_inode_unlink() handle the
> >    unlink/remove case, but not the umount case.
> 
> IMHO this is the best option. Here:
> 
> * BPF LSM Program = MAC Policy
> * Removing / detaching / updating programs = Updating MAC policy
> 
> The decision around who can update MAC policy can be governed by the
> policy itself a.k.a. implemented with BPF LSM programs.  So we can
> update hooks (as suggested here inode_unlink, sb_unmount, path_unlink)
> to only allow this action for a subset of users (e.g. CAP_MAC_ADMIN or
> even further restricted)
> 
> While, I think this may be doable with existing LSM hooks but we need
> to probably have to cover multiple hook points needed to prevent one
> action which makes a good case for another LSM hook, perhaps something
> in the link->ops->detach path like
> https://elixir.bootlin.com/linux/latest/source/kernel/bpf/syscall.c#L5074
> 
> What do you think?

That's what I was thinking for option (4) "introduce a
security_bpf_prog_unload()". Anyway, I agree. Paul brought up a good
point that he'd like to see more discussion around this idea [1].
Mucking with the mounts (see below) is a bit of a mess, and there could
still exist other methods for unloading I'm not aware of yet.

Yesterday I whipped up a hack such that: 

	mkdir -p /run/fs/bpf-lsm
	mount -t bpf none /run/fs/bpf-lsm
	./load-policies /run/fs/bpf-lsm

Where the implemented hooks inode_unlink & sb_umount look for a "bpf"
filesystem type on the struct super_block for the mount, and then check
for a xattr or UUID to hone in on _just_ that special mount point. This is
so we can still umount /sys/fs/bpf. This hack so far is working out
quite well.

xattr is still under development, thus the UUID idea (we're still exploring
unique-path-agnostic identifiers). I also didn't prove the UUID
idea yet. For instance, I wouldn't want the UUID to change on mount
--move or mount -o remount.

Apropos the reboot problem, the idea is to force a reboot for policy change
for now. And then eventually leverage xattr + IMA, or something else to prove
the loader binary + progs once these features are available. Policy
versioning is also an idea we're floating around for handling the update
case with the loader to avoid reboot.

This is all to say, I think there is a use-case here for a new hook to
simplify the unload problem at least.

[1] https://lore.kernel.org/all/CAHC9VhSRdXLeJvS3tOmAAat+h8G7_cvAYnFvbrTwgG+sC+PRYg@mail.gmail.com/

+cc: discussion contributors
> 
> - KP
> 
> >
> > 3. Leverage SELinux/Apparmor to possibly handle these cases.
> >
> > 4. Introduce a security_bpf_prog_unload() to target hopefully the
> >    umount and unlink cases at the same time.
> 
> 
> 
> >
> > 5. Possible moonshot idea: introduce a interface to pin _specifically_
> >    BPF LSM's to the kernel, and avoid the bpf sysfs problems all
> >    together.
> >
> > We're making the assumption this problem has been thought about before,
> > and are wondering if there's anything obvious we're missing here.
> >
> > Fred
> >

Fred

P.S. This is an awesome discussion :) Thanks everyone!