Re: BPF LSM prevent program unload

Re: BPF LSM prevent program unload

[Yafang Shao]

On Wed, Dec 6, 2023 at 11:02 PM Frederick Lawler <fred@cloudflare.com> wrote:
>
> On Wed, Dec 06, 2023 at 10:42:50AM +0800, Yafang Shao wrote:
> > On Wed, Dec 6, 2023 at 4:39 AM 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.
> > >
> > > 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.
> > >
> >
> > All the above programs can also be removed by privileged users.
> >
>
> I should probably clarify the "BPF or otherwise" a bit better. Even a
> compiled in LSM module? If so, where can I find a bit more information
> about that?

Uncertain if it's feasible using the LSM module.
+security exports for help.

>
> We are aware of some of the shortcomings of policy cfg for the AppArmor &
> SELinux case.
>
> > > 5. Possible moonshot idea: introduce a interface to pin _specifically_
> > >    BPF LSM's to the kernel, and avoid the bpf sysfs problems all
> > >    together.
> >
> > Introducing non-auto-detachable lsm programs seems like a workable
> > solution.  That said, we can't remove the lsm program before it has
> > been detached explicitly by the task which attaches it.
> >
> > >
> > > 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
> > >
> >
> >
> > --
> > Regards
> > Yafang
>
> Fred



-- 
Regards
Yafang

Re: BPF LSM prevent program unload

[Tetsuo Handa]

On 2023/12/07 11:28, Yafang Shao wrote:
>>>> 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.

That is what I thought at
https://lkml.kernel.org/r/c588ca5d-c343-4ea2-a1f1-4efe67ebb8e3@I-love.SAKURA.ne.jp ,
though I didn't try it because the conclusion was that trying to re-implement TOMOYO
LSM module using BPF is not realistic.

While hooking security_sb_umount() from LSM modules will be possible,
unconditionally rejecting umount operation might confuse userspace programs
(e.g. retry until umount operation succeeds). Therefore, maybe introducing a
kernel thread who holds a refcount using a file descriptor ownded by that
kernel thread is better than trying to manage individual mount namepsaces
and inodes... Letting a kernel code to intentionally leak that refcount
instead of storing into somewhere might be possible, but that is considered
as a kernel bug.

Re: BPF LSM prevent program unload

[Paul Moore]

On Wed, Dec 6, 2023 at 9:28 PM Yafang Shao <laoar.shao@gmail.com> wrote:
> On Wed, Dec 6, 2023 at 11:02 PM Frederick Lawler <fred@cloudflare.com> wrote:
> > On Wed, Dec 06, 2023 at 10:42:50AM +0800, Yafang Shao wrote:
> > > On Wed, Dec 6, 2023 at 4:39 AM Frederick Lawler 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.

If you haven't already, I would suggest talking with KP Singh as he is
the BPF LSM maintainer; I see him on the To/CC line so I'm sure he'll
comment when he has the chance to do so.

> > > > 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.

I'm not a BPF expert, but this seems like the most obvious solution,
although as Tetsuo already mentioned you probably don't want to block
all unmount operations as that would be bad for obvious reasons.  I'm
guessing that a BPF LSM would have access to things like the current
task credentials and enough of the mounted filesystem's state (BPF
prog pinning?) to make a reasonable decision about granting or denying
the umount operation request.

> > > > 3. Leverage SELinux/Apparmor to possibly handle these cases.

SELinux has support for restricting unmount operations as well BPF
program loading.  I see that AppArmor also has controls around
unmount, but I am less familiar with how that works.  It is also worth
mentioning that Tomoyo and Landlock provide unmount hook
implementations although both LSMs are fairly unique so I can't say if
they would be a good fit for your proposed use case.

> > > > 4. Introduce a security_bpf_prog_unload() to target hopefully the
> > > >    umount and unlink cases at the same time.

At first glance that seems reasonable, but I guess we would need to
see it discussed a bit before I could promise to commit to that.

As a FYI, we have some documented guidelines on creating new LSM
hooks; it's worth a quick read if you haven't seen it already.

https://github.com/LinuxSecurityModule/kernel?tab=readme-ov-file#new-lsm-hook-guidelines

> > > All the above programs can also be removed by privileged users.
> > >
> >
> > I should probably clarify the "BPF or otherwise" a bit better. Even a
> > compiled in LSM module? If so, where can I find a bit more information
> > about that?

I'm not quite sure what you are asking about here, but we don't
currently support "unloading" built-in LSM modules and I don't see us
changing that anytime soon.  The closest one could get would be with a
LSM that supports runtime configuration of its security policy; one
could go from a restrictive or an allow-all, permissive policy
effectively disabling the LSM from an access control standpoint.

I don't want to speak for all the LSMs here, but at least SELinux has
the ability to restrict policy loading so that one could prevent
replacing a relatively strict policy with a more permissive policy.
Although it is worth noting that enabling this restriction has a
number of caveats, i.e. policy updates require a reboot, and isn't
something I would recommend for a general purpose system.

-- 
paul-moore.com

Re: BPF LSM prevent program unload

[Casey Schaufler]

On 12/7/2023 9:34 AM, Paul Moore wrote:
> On Wed, Dec 6, 2023 at 9:28 PM Yafang Shao <laoar.shao@gmail.com> wrote:
>> On Wed, Dec 6, 2023 at 11:02 PM Frederick Lawler <fred@cloudflare.com> wrote:
>>> On Wed, Dec 06, 2023 at 10:42:50AM +0800, Yafang Shao wrote:
>>>> On Wed, Dec 6, 2023 at 4:39 AM Frederick Lawler 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.
> If you haven't already, I would suggest talking with KP Singh as he is
> the BPF LSM maintainer; I see him on the To/CC line so I'm sure he'll
> comment when he has the chance to do so.
>
>>>>> 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.
> I'm not a BPF expert, but this seems like the most obvious solution,
> although as Tetsuo already mentioned you probably don't want to block
> all unmount operations as that would be bad for obvious reasons.  I'm
> guessing that a BPF LSM would have access to things like the current
> task credentials and enough of the mounted filesystem's state (BPF
> prog pinning?) to make a reasonable decision about granting or denying
> the umount operation request.
>
>>>>> 3. Leverage SELinux/Apparmor to possibly handle these cases.
> SELinux has support for restricting unmount operations as well BPF
> program loading.  I see that AppArmor also has controls around
> unmount, but I am less familiar with how that works.  It is also worth
> mentioning that Tomoyo and Landlock provide unmount hook
> implementations although both LSMs are fairly unique so I can't say if
> they would be a good fit for your proposed use case.
>
>>>>> 4. Introduce a security_bpf_prog_unload() to target hopefully the
>>>>>    umount and unlink cases at the same time.
> At first glance that seems reasonable, but I guess we would need to
> see it discussed a bit before I could promise to commit to that.
>
> As a FYI, we have some documented guidelines on creating new LSM
> hooks; it's worth a quick read if you haven't seen it already.
>
> https://github.com/LinuxSecurityModule/kernel?tab=readme-ov-file#new-lsm-hook-guidelines
>
>>>> All the above programs can also be removed by privileged users.
>>>>
>>> I should probably clarify the "BPF or otherwise" a bit better. Even a
>>> compiled in LSM module? If so, where can I find a bit more information
>>> about that?
> I'm not quite sure what you are asking about here, but we don't
> currently support "unloading" built-in LSM modules and I don't see us
> changing that anytime soon.  The closest one could get would be with a
> LSM that supports runtime configuration of its security policy; one
> could go from a restrictive or an allow-all, permissive policy
> effectively disabling the LSM from an access control standpoint.
>
> I don't want to speak for all the LSMs here, but at least SELinux has
> the ability to restrict policy loading so that one could prevent
> replacing a relatively strict policy with a more permissive policy.
> Although it is worth noting that enabling this restriction has a
> number of caveats, i.e. policy updates require a reboot, and isn't
> something I would recommend for a general purpose system.

You can prevent Smack from making policy changes using the "onlycap"
mechanism. It restricts what process labels can use CAP_MAC_ADMIN,
which is required to update Smack rules. As with SELinux, it isn't
recommended for general use.

Re: BPF LSM prevent program unload

[Frederick Lawler]

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!

Re: BPF LSM prevent program unload

[Song Liu]

Hi Frederick,

On Thu, Dec 7, 2023 at 3:30 PM Frederick Lawler <fred@cloudflare.com> wrote:
>
[...]
> > 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

Trying to understand the solution here. Does load-policies add multiple
policies to stop different ways to unload the LSM BPF program (unpin,
umount, etc.)? So the only way to unload these policies is reboot. If this
is the case, could you please share the list of hooks needed to achieve a
secure result? If the list is really long, we should probably add an option to
permanently load and attach a program (until reboot).

Thanks,
Song

Re: BPF LSM prevent program unload

[Frederick Lawler]

On Thu, Dec 07, 2023 at 03:42:49PM -0800, Song Liu wrote:
> Hi Frederick,
> 
> On Thu, Dec 7, 2023 at 3:30 PM Frederick Lawler <fred@cloudflare.com> wrote:
> >
> [...]
> > > 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
> 
> Trying to understand the solution here. Does load-policies add multiple
> policies to stop different ways to unload the LSM BPF program (unpin,
> umount, etc.)? So the only way to unload these policies is reboot. If this
> is the case, could you please share the list of hooks needed to achieve a
> secure result? 

./load-policies loads multiple BPF object files (policy) each containing
N programs. Then for each program, pin it to the bpffs and terminate. 
There's more there for atomic loads etc... but not relevant
for answering the question. For this hack, I created a bpf object file
with two programs:

	- lsm/sb_umount
	- lsm/inode_unlink

More could be added to this list as necessary depending on finding other
ways to unload. I've only found the filesystem to be the most consistent
way so far. libbpf's unpin functions seem to be also trapped by the
inode_unlink program, but more exploration syscalls is on my
TODO list.

And added the object file along with the rest to load in.

> If the list is really long, we should probably add an option to
> permanently load and attach a program (until reboot).

This is an interesting thought as well. I think that would fall under
idea (5) [1]. But the list isn't that long, and lonterm, I'd like the loader
to have permission to load/unload BPF LSM progs. But, this won't help folks that
leverage BPF's skeleton loading methods and users that rely on anon
inodes tied to the task. I think KP offered some ideas there [2]. 

[1] https://lore.kernel.org/all/ZW+KYViDT3HWtKI1@CMGLRV3/
[2] https://lore.kernel.org/all/CACYkzJ4QpQZ8JmdNXKWeSh8oc=jAyRh4Zj98Z+TG37Ce=cfE0w@mail.gmail.com/

> 
> Thanks,
> Song

Re: BPF LSM prevent program unload

[Song Liu]

Hi Frederick,

On Thu, Dec 7, 2023 at 4:21 PM Frederick Lawler <fred@cloudflare.com> wrote:
>
> On Thu, Dec 07, 2023 at 03:42:49PM -0800, Song Liu wrote:
> > Hi Frederick,
> >
> > On Thu, Dec 7, 2023 at 3:30 PM Frederick Lawler <fred@cloudflare.com> wrote:
> > >
[...]
> > Trying to understand the solution here. Does load-policies add multiple
> > policies to stop different ways to unload the LSM BPF program (unpin,
> > umount, etc.)? So the only way to unload these policies is reboot. If this
> > is the case, could you please share the list of hooks needed to achieve a
> > secure result?
>
> ./load-policies loads multiple BPF object files (policy) each containing
> N programs. Then for each program, pin it to the bpffs and terminate.
> There's more there for atomic loads etc... but not relevant
> for answering the question. For this hack, I created a bpf object file
> with two programs:
>
>         - lsm/sb_umount
>         - lsm/inode_unlink

Thanks for the information. This seems easy enough.

> More could be added to this list as necessary depending on finding other
> ways to unload. I've only found the filesystem to be the most consistent
> way so far. libbpf's unpin functions seem to be also trapped by the
> inode_unlink program, but more exploration syscalls is on my
> TODO list.
>
> And added the object file along with the rest to load in.
>
> > If the list is really long, we should probably add an option to
> > permanently load and attach a program (until reboot).
>
> This is an interesting thought as well. I think that would fall under
> idea (5) [1]. But the list isn't that long, and lonterm, I'd like the loader

Agreed this falls under idea (5).

> to have permission to load/unload BPF LSM progs. But, this won't help folks that
> leverage BPF's skeleton loading methods and users that rely on anon
> inodes tied to the task. I think KP offered some ideas there [2].

Thanks,
Song