RE: [PATCH 00/18] bpf: Secure and authenticated preloading of eBPF programs

Roberto Sassu posted 18 patches 4 years, 2 months ago
Only 0 patches received!
There is a newer version of this series
RE: [PATCH 00/18] bpf: Secure and authenticated preloading of eBPF programs
Posted by Roberto Sassu 4 years, 2 months ago
> From: Djalal Harouni [mailto:tixxdz@gmail.com]
> Sent: Monday, April 4, 2022 9:45 AM
> On Sun, Apr 3, 2022 at 5:42 PM KP Singh <kpsingh@kernel.org> wrote:
> >
> > On Sat, Apr 2, 2022 at 1:55 AM Alexei Starovoitov
> > <alexei.starovoitov@gmail.com> wrote:
> ...
> > >
> > > > Pinning
> > > > them to unreachable inodes intuitively looked the
> > > > way to go for achieving the stated goal.
> > >
> > > We can consider inodes in bpffs that are not unlinkable by root
> > > in the future, but certainly not for this use case.
> >
> > Can this not be already done by adding a BPF_LSM program to the
> > inode_unlink LSM hook?
> >
> 
> Also, beside of the inode_unlink... and out of curiosity: making sysfs/bpffs/
> readonly after pinning, then using bpf LSM hooks
> sb_mount|remount|unmount...
> family combining bpf() LSM hook... isn't this enough to:
> 1. Restrict who can pin to bpffs without using a full MAC
> 2. Restrict who can delete or unmount bpf filesystem
> 
> ?

I'm thinking to implement something like this.

First, I add a new program flag called
BPF_F_STOP_ONCONFIRM, which causes the ref count
of the link to increase twice at creation time. In this way,
user space cannot make the link disappear, unless a
confirmation is explicitly sent via the bpf() system call.

Another advantage is that other LSMs can decide
whether or not they allow a program with this flag
(in the bpf security hook).

This would work regardless of the method used to
load the eBPF program (user space or kernel space).

Second, I extend the bpf() system call with a new
subcommand, BPF_LINK_CONFIRM_STOP, which
decreases the ref count for the link of the programs
with the BPF_F_STOP_ONCONFIRM flag. I will also
introduce a new security hook (something like
security_link_confirm_stop), so that an LSM has the
opportunity to deny the stop (the bpf security hook
would not be sufficient to determine exactly for
which link the confirmation is given, an LSM should
be able to deny the stop for its own programs).

What do you think?

Thanks

Roberto

HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Li Peng, Zhong Ronghua
Re: [PATCH 00/18] bpf: Secure and authenticated preloading of eBPF programs
Posted by Alexei Starovoitov 4 years, 2 months ago
On Mon, Apr 4, 2022 at 10:21 AM Roberto Sassu <roberto.sassu@huawei.com> wrote:
>
> > From: Djalal Harouni [mailto:tixxdz@gmail.com]
> > Sent: Monday, April 4, 2022 9:45 AM
> > On Sun, Apr 3, 2022 at 5:42 PM KP Singh <kpsingh@kernel.org> wrote:
> > >
> > > On Sat, Apr 2, 2022 at 1:55 AM Alexei Starovoitov
> > > <alexei.starovoitov@gmail.com> wrote:
> > ...
> > > >
> > > > > Pinning
> > > > > them to unreachable inodes intuitively looked the
> > > > > way to go for achieving the stated goal.
> > > >
> > > > We can consider inodes in bpffs that are not unlinkable by root
> > > > in the future, but certainly not for this use case.
> > >
> > > Can this not be already done by adding a BPF_LSM program to the
> > > inode_unlink LSM hook?
> > >
> >
> > Also, beside of the inode_unlink... and out of curiosity: making sysfs/bpffs/
> > readonly after pinning, then using bpf LSM hooks
> > sb_mount|remount|unmount...
> > family combining bpf() LSM hook... isn't this enough to:
> > 1. Restrict who can pin to bpffs without using a full MAC
> > 2. Restrict who can delete or unmount bpf filesystem
> >
> > ?
>
> I'm thinking to implement something like this.
>
> First, I add a new program flag called
> BPF_F_STOP_ONCONFIRM, which causes the ref count
> of the link to increase twice at creation time. In this way,
> user space cannot make the link disappear, unless a
> confirmation is explicitly sent via the bpf() system call.
>
> Another advantage is that other LSMs can decide
> whether or not they allow a program with this flag
> (in the bpf security hook).
>
> This would work regardless of the method used to
> load the eBPF program (user space or kernel space).
>
> Second, I extend the bpf() system call with a new
> subcommand, BPF_LINK_CONFIRM_STOP, which
> decreases the ref count for the link of the programs
> with the BPF_F_STOP_ONCONFIRM flag. I will also
> introduce a new security hook (something like
> security_link_confirm_stop), so that an LSM has the
> opportunity to deny the stop (the bpf security hook
> would not be sufficient to determine exactly for
> which link the confirmation is given, an LSM should
> be able to deny the stop for its own programs).
>
> What do you think?

Hack upon a hack? Makes no sense.
Re: [PATCH 00/18] bpf: Secure and authenticated preloading of eBPF programs
Posted by KP Singh 4 years, 2 months ago
On Tue, Apr 5, 2022 at 12:49 AM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Mon, Apr 4, 2022 at 10:21 AM Roberto Sassu <roberto.sassu@huawei.com> wrote:
> >
> > > From: Djalal Harouni [mailto:tixxdz@gmail.com]
> > > Sent: Monday, April 4, 2022 9:45 AM
> > > On Sun, Apr 3, 2022 at 5:42 PM KP Singh <kpsingh@kernel.org> wrote:
> > > >
> > > > On Sat, Apr 2, 2022 at 1:55 AM Alexei Starovoitov
> > > > <alexei.starovoitov@gmail.com> wrote:
> > > ...
> > > > >
> > > > > > Pinning
> > > > > > them to unreachable inodes intuitively looked the
> > > > > > way to go for achieving the stated goal.
> > > > >
> > > > > We can consider inodes in bpffs that are not unlinkable by root
> > > > > in the future, but certainly not for this use case.
> > > >
> > > > Can this not be already done by adding a BPF_LSM program to the
> > > > inode_unlink LSM hook?
> > > >
> > >
> > > Also, beside of the inode_unlink... and out of curiosity: making sysfs/bpffs/
> > > readonly after pinning, then using bpf LSM hooks
> > > sb_mount|remount|unmount...
> > > family combining bpf() LSM hook... isn't this enough to:
> > > 1. Restrict who can pin to bpffs without using a full MAC
> > > 2. Restrict who can delete or unmount bpf filesystem
> > >

I like this approach better, you will have to restrict the BPF, if you
want to implement MAC policy using BPF.

Can you please try implementing something using these hooks?

> > > ?
> >
> > I'm thinking to implement something like this.
> >
> > First, I add a new program flag called
> > BPF_F_STOP_ONCONFIRM, which causes the ref count
> > of the link to increase twice at creation time. In this way,
> > user space cannot make the link disappear, unless a
> > confirmation is explicitly sent via the bpf() system call.

I don't like this approach, this just sounds like an intentional
dangling reference, prone to refcounting errors and it does not
really solve the purpose you want to achieve.

And you will still need a policy around the BPF syscall,
so why not just use the LSM hooks as suggested above?

> >
> > Another advantage is that other LSMs can decide
> > whether or not they allow a program with this flag
> > (in the bpf security hook).
> >
> > This would work regardless of the method used to
> > load the eBPF program (user space or kernel space).
> >
> > Second, I extend the bpf() system call with a new
> > subcommand, BPF_LINK_CONFIRM_STOP, which
> > decreases the ref count for the link of the programs
> > with the BPF_F_STOP_ONCONFIRM flag. I will also
> > introduce a new security hook (something like
> > security_link_confirm_stop), so that an LSM has the
> > opportunity to deny the stop (the bpf security hook
> > would not be sufficient to determine exactly for
> > which link the confirmation is given, an LSM should
> > be able to deny the stop for its own programs).
> >
> > What do you think?
>
> Hack upon a hack? Makes no sense.
[POC][USER SPACE][PATCH] Introduce LSM to protect pinned objects
Posted by Roberto Sassu 4 years, 2 months ago
Introduce a new LSM to protect pinned objects in a bpf filesystem
instance. This is useful for example to ensure that an LSM will always
enforce its policy, even despite root tries to unload the corresponding
eBPF program.

Achieve the protection by denying inode unlink and unmount of the
protected bpf filesystem instance. Since protected inodes hold a
reference of the link of loaded programs (e.g. LSM hooks), denying
operations on them will prevent the ref count of the links from reaching
zero, ensuring that the programs remain always active.

Enable the protection only for the instance created by the user space
counterpart of the LSM, and don't interfere with other instances, so
that their behavior remains unchanged.

Suggested-by: Djalal Harouni <tixxdz@gmail.com>
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 .gitignore       |  4 +++
 Makefile         | 18 ++++++++++++++
 bpffs_lsm_kern.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++
 bpffs_lsm_user.c | 60 +++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 145 insertions(+)
 create mode 100644 .gitignore
 create mode 100644 Makefile
 create mode 100644 bpffs_lsm_kern.c
 create mode 100644 bpffs_lsm_user.c

diff --git a/.gitignore b/.gitignore
new file mode 100644
index 000000000000..7fa02964f1dc
--- /dev/null
+++ b/.gitignore
@@ -0,0 +1,4 @@
+*.o
+vmlinux.h
+bpffs_lsm_kern.skel.h
+bpffs_lsm_user
diff --git a/Makefile b/Makefile
new file mode 100644
index 000000000000..c3d805759db3
--- /dev/null
+++ b/Makefile
@@ -0,0 +1,18 @@
+# SPDX-License-Identifier: GPL-2.0
+all: bpffs_lsm_user
+
+clean:
+	rm -rf bpffs_lsm.skel.h vmlinux.h bpffs_lsm_kern.o bpffs_lsm_user
+
+vmlinux.h:
+	/usr/sbin/bpftool btf dump file /sys/kernel/btf/vmlinux format c > \
+			  vmlinux.h
+
+bpffs_lsm_kern.skel.h: bpffs_lsm_kern.o
+	bpftool gen skeleton $< > $@
+
+bpffs_lsm_kern.o: bpffs_lsm_kern.c vmlinux.h
+	clang -Wall -Werror -g -O2 -target bpf -c $< -o $@
+
+bpffs_lsm_user: bpffs_lsm_user.c bpffs_lsm_kern.skel.h bpffs_lsm_kern.o
+	cc -Wall -Werror -g -o $@ $< -lbpf
diff --git a/bpffs_lsm_kern.c b/bpffs_lsm_kern.c
new file mode 100644
index 000000000000..b3ccb2a75c95
--- /dev/null
+++ b/bpffs_lsm_kern.c
@@ -0,0 +1,63 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2022 Huawei Technologies Duesseldorf GmbH
+ *
+ * Authors:
+ * Roberto Sassu <roberto.sassu@huawei.com>
+ *
+ * Implement an LSM to protect a bpf filesystem instance.
+ */
+
+#include "vmlinux.h"
+#include <errno.h>
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+#include <bpf/bpf_core_read.h>
+
+char _license[] SEC("license") = "GPL";
+
+uint32_t monitored_pid = 0;
+
+struct {
+	__uint(type, BPF_MAP_TYPE_INODE_STORAGE);
+	__uint(map_flags, BPF_F_NO_PREALLOC);
+	__type(key, int);
+	__type(value, sizeof(uint8_t));
+} inode_storage_map SEC(".maps");
+
+SEC("lsm/sb_set_mnt_opts")
+int BPF_PROG(sb_set_mnt_opts, struct super_block *sb, void *mnt_opts,
+	     unsigned long kern_flags, unsigned long *set_kern_flags)
+{
+	u32 pid;
+
+	pid = bpf_get_current_pid_tgid() >> 32;
+	if (pid != monitored_pid)
+		return 0;
+
+	if (!bpf_inode_storage_get(&inode_storage_map, sb->s_root->d_inode, 0,
+				   BPF_LOCAL_STORAGE_GET_F_CREATE))
+		return -EPERM;
+
+	return 0;
+}
+
+SEC("lsm/inode_unlink")
+int BPF_PROG(inode_unlink, struct inode *dir, struct dentry *dentry)
+{
+	if (bpf_inode_storage_get(&inode_storage_map,
+				  dir->i_sb->s_root->d_inode, 0, 0))
+		return -EPERM;
+
+	return 0;
+}
+
+SEC("lsm/sb_umount")
+int BPF_PROG(sb_umount, struct vfsmount *mnt, int flags)
+{
+	if (bpf_inode_storage_get(&inode_storage_map,
+				  mnt->mnt_sb->s_root->d_inode, 0, 0))
+		return -EPERM;
+
+	return 0;
+}
diff --git a/bpffs_lsm_user.c b/bpffs_lsm_user.c
new file mode 100644
index 000000000000..e20180cc5db9
--- /dev/null
+++ b/bpffs_lsm_user.c
@@ -0,0 +1,60 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2022 Huawei Technologies Duesseldorf GmbH
+ *
+ * Author: Roberto Sassu <roberto.sassu@huawei.com>
+ *
+ * Implement the user space side of the LSM for bpffs.
+ */
+
+#include <fcntl.h>
+#include <unistd.h>
+#include <stdio.h>
+#include <errno.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <limits.h>
+#include <sys/mount.h>
+#include <sys/stat.h>
+
+#include "bpffs_lsm_kern.skel.h"
+
+#define MOUNT_FLAGS (MS_NOSUID | MS_NODEV | MS_NOEXEC | MS_RELATIME)
+
+int main(int argc, char *argv[])
+{
+	char mntpoint[] = "/tmp/bpf_private_mountXXXXXX";
+	char path[PATH_MAX];
+	struct bpffs_lsm_kern *skel;
+	int ret, i;
+
+	skel = bpffs_lsm_kern__open_and_load();
+	if (!skel)
+		return -EINVAL;
+
+	ret = bpffs_lsm_kern__attach(skel);
+	if (ret < 0)
+		goto out_destroy;
+
+	mkdtemp(mntpoint);
+
+	skel->bss->monitored_pid = getpid();
+	ret = mount(mntpoint, mntpoint, "bpf", MOUNT_FLAGS, NULL);
+	skel->bss->monitored_pid = 0;
+
+	if (ret < 0)
+		goto out_destroy;
+
+	for (i = 0; i < skel->skeleton->prog_cnt; i++) {
+		snprintf(path, sizeof(path), "%s/%s", mntpoint,
+			 skel->skeleton->progs[i].name);
+		ret = bpf_link__pin(*skel->skeleton->progs[i].link, path);
+		if (ret < 0)
+			goto out_destroy;
+	}
+
+	ret = 0;
+out_destroy:
+	bpffs_lsm_kern__destroy(skel);
+	return ret;
+}
-- 
2.32.0
Re: [POC][USER SPACE][PATCH] Introduce LSM to protect pinned objects
Posted by Casey Schaufler 4 years, 2 months ago
On 4/5/2022 6:11 AM, Roberto Sassu wrote:
> Introduce a new LSM to protect pinned objects in a bpf filesystem

This is *not an LSM*. Do not call it an LSM. It is a set of
eBPF programs. We have all the opportunities for confusion
that we need. I suggested that you call this a BPF security
module (BSM) earlier today. You have any number of things
you can call this that won't be objectionable.

> instance. This is useful for example to ensure that an LSM will always
> enforce its policy, even despite root tries to unload the corresponding
> eBPF program.

How is this going to ensure that SELinux enforces its policy?
AppArmor has no eBPF program that corresponds to its policy,
neither does any other existing LSM, save BPF. Your claim is
nonsensical in the face of LSM behavior.

> Achieve the protection by denying inode unlink and unmount of the
> protected bpf filesystem instance. Since protected inodes hold a
> reference of the link of loaded programs (e.g. LSM hooks), denying
> operations on them will prevent the ref count of the links from reaching
> zero, ensuring that the programs remain always active.
>
> Enable the protection only for the instance created by the user space
> counterpart of the LSM, and don't interfere with other instances, so
> that their behavior remains unchanged.
>
> Suggested-by: Djalal Harouni <tixxdz@gmail.com>
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> ---
>   .gitignore       |  4 +++
>   Makefile         | 18 ++++++++++++++
>   bpffs_lsm_kern.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++
>   bpffs_lsm_user.c | 60 +++++++++++++++++++++++++++++++++++++++++++++
>   4 files changed, 145 insertions(+)
>   create mode 100644 .gitignore
>   create mode 100644 Makefile
>   create mode 100644 bpffs_lsm_kern.c
>   create mode 100644 bpffs_lsm_user.c
>
> diff --git a/.gitignore b/.gitignore
> new file mode 100644
> index 000000000000..7fa02964f1dc
> --- /dev/null
> +++ b/.gitignore
> @@ -0,0 +1,4 @@
> +*.o
> +vmlinux.h
> +bpffs_lsm_kern.skel.h
> +bpffs_lsm_user
> diff --git a/Makefile b/Makefile
> new file mode 100644
> index 000000000000..c3d805759db3
> --- /dev/null
> +++ b/Makefile
> @@ -0,0 +1,18 @@
> +# SPDX-License-Identifier: GPL-2.0
> +all: bpffs_lsm_user
> +
> +clean:
> +	rm -rf bpffs_lsm.skel.h vmlinux.h bpffs_lsm_kern.o bpffs_lsm_user
> +
> +vmlinux.h:
> +	/usr/sbin/bpftool btf dump file /sys/kernel/btf/vmlinux format c > \
> +			  vmlinux.h
> +
> +bpffs_lsm_kern.skel.h: bpffs_lsm_kern.o
> +	bpftool gen skeleton $< > $@
> +
> +bpffs_lsm_kern.o: bpffs_lsm_kern.c vmlinux.h
> +	clang -Wall -Werror -g -O2 -target bpf -c $< -o $@
> +
> +bpffs_lsm_user: bpffs_lsm_user.c bpffs_lsm_kern.skel.h bpffs_lsm_kern.o
> +	cc -Wall -Werror -g -o $@ $< -lbpf
> diff --git a/bpffs_lsm_kern.c b/bpffs_lsm_kern.c
> new file mode 100644
> index 000000000000..b3ccb2a75c95
> --- /dev/null
> +++ b/bpffs_lsm_kern.c
> @@ -0,0 +1,63 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2022 Huawei Technologies Duesseldorf GmbH
> + *
> + * Authors:
> + * Roberto Sassu <roberto.sassu@huawei.com>
> + *
> + * Implement an LSM to protect a bpf filesystem instance.
> + */
> +
> +#include "vmlinux.h"
> +#include <errno.h>
> +#include <bpf/bpf_helpers.h>
> +#include <bpf/bpf_tracing.h>
> +#include <bpf/bpf_core_read.h>
> +
> +char _license[] SEC("license") = "GPL";
> +
> +uint32_t monitored_pid = 0;
> +
> +struct {
> +	__uint(type, BPF_MAP_TYPE_INODE_STORAGE);
> +	__uint(map_flags, BPF_F_NO_PREALLOC);
> +	__type(key, int);
> +	__type(value, sizeof(uint8_t));
> +} inode_storage_map SEC(".maps");
> +
> +SEC("lsm/sb_set_mnt_opts")
> +int BPF_PROG(sb_set_mnt_opts, struct super_block *sb, void *mnt_opts,
> +	     unsigned long kern_flags, unsigned long *set_kern_flags)
> +{
> +	u32 pid;
> +
> +	pid = bpf_get_current_pid_tgid() >> 32;
> +	if (pid != monitored_pid)
> +		return 0;
> +
> +	if (!bpf_inode_storage_get(&inode_storage_map, sb->s_root->d_inode, 0,
> +				   BPF_LOCAL_STORAGE_GET_F_CREATE))
> +		return -EPERM;
> +
> +	return 0;
> +}
> +
> +SEC("lsm/inode_unlink")
> +int BPF_PROG(inode_unlink, struct inode *dir, struct dentry *dentry)
> +{
> +	if (bpf_inode_storage_get(&inode_storage_map,
> +				  dir->i_sb->s_root->d_inode, 0, 0))
> +		return -EPERM;
> +
> +	return 0;
> +}
> +
> +SEC("lsm/sb_umount")
> +int BPF_PROG(sb_umount, struct vfsmount *mnt, int flags)
> +{
> +	if (bpf_inode_storage_get(&inode_storage_map,
> +				  mnt->mnt_sb->s_root->d_inode, 0, 0))
> +		return -EPERM;
> +
> +	return 0;
> +}
> diff --git a/bpffs_lsm_user.c b/bpffs_lsm_user.c
> new file mode 100644
> index 000000000000..e20180cc5db9
> --- /dev/null
> +++ b/bpffs_lsm_user.c
> @@ -0,0 +1,60 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2022 Huawei Technologies Duesseldorf GmbH
> + *
> + * Author: Roberto Sassu <roberto.sassu@huawei.com>
> + *
> + * Implement the user space side of the LSM for bpffs.
> + */
> +
> +#include <fcntl.h>
> +#include <unistd.h>
> +#include <stdio.h>
> +#include <errno.h>
> +#include <stdlib.h>
> +#include <unistd.h>
> +#include <limits.h>
> +#include <sys/mount.h>
> +#include <sys/stat.h>
> +
> +#include "bpffs_lsm_kern.skel.h"
> +
> +#define MOUNT_FLAGS (MS_NOSUID | MS_NODEV | MS_NOEXEC | MS_RELATIME)
> +
> +int main(int argc, char *argv[])
> +{
> +	char mntpoint[] = "/tmp/bpf_private_mountXXXXXX";
> +	char path[PATH_MAX];
> +	struct bpffs_lsm_kern *skel;
> +	int ret, i;
> +
> +	skel = bpffs_lsm_kern__open_and_load();
> +	if (!skel)
> +		return -EINVAL;
> +
> +	ret = bpffs_lsm_kern__attach(skel);
> +	if (ret < 0)
> +		goto out_destroy;
> +
> +	mkdtemp(mntpoint);
> +
> +	skel->bss->monitored_pid = getpid();
> +	ret = mount(mntpoint, mntpoint, "bpf", MOUNT_FLAGS, NULL);
> +	skel->bss->monitored_pid = 0;
> +
> +	if (ret < 0)
> +		goto out_destroy;
> +
> +	for (i = 0; i < skel->skeleton->prog_cnt; i++) {
> +		snprintf(path, sizeof(path), "%s/%s", mntpoint,
> +			 skel->skeleton->progs[i].name);
> +		ret = bpf_link__pin(*skel->skeleton->progs[i].link, path);
> +		if (ret < 0)
> +			goto out_destroy;
> +	}
> +
> +	ret = 0;
> +out_destroy:
> +	bpffs_lsm_kern__destroy(skel);
> +	return ret;
> +}
RE: [POC][USER SPACE][PATCH] Introduce LSM to protect pinned objects
Posted by Roberto Sassu 4 years, 2 months ago
> From: Casey Schaufler [mailto:casey@schaufler-ca.com]
> Sent: Wednesday, April 6, 2022 12:48 AM
> On 4/5/2022 6:11 AM, Roberto Sassu wrote:
> > Introduce a new LSM to protect pinned objects in a bpf filesystem
> 
> This is *not an LSM*. Do not call it an LSM. It is a set of
> eBPF programs. We have all the opportunities for confusion
> that we need. I suggested that you call this a BPF security
> module (BSM) earlier today. You have any number of things
> you can call this that won't be objectionable.
> 
> > instance. This is useful for example to ensure that an LSM will always
> > enforce its policy, even despite root tries to unload the corresponding
> > eBPF program.
> 
> How is this going to ensure that SELinux enforces its policy?

I should have said above: that an LSM implemented with eBPF.
Built-in LSMs are not affected by this change.

Ok, next time I call it BSM.

Thanks

Roberto

HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Li Peng, Zhong Ronghua

> AppArmor has no eBPF program that corresponds to its policy,
> neither does any other existing LSM, save BPF. Your claim is
> nonsensical in the face of LSM behavior.
> 
> > Achieve the protection by denying inode unlink and unmount of the
> > protected bpf filesystem instance. Since protected inodes hold a
> > reference of the link of loaded programs (e.g. LSM hooks), denying
> > operations on them will prevent the ref count of the links from reaching
> > zero, ensuring that the programs remain always active.
> >
> > Enable the protection only for the instance created by the user space
> > counterpart of the LSM, and don't interfere with other instances, so
> > that their behavior remains unchanged.
> >
> > Suggested-by: Djalal Harouni <tixxdz@gmail.com>
> > Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> > ---
> >   .gitignore       |  4 +++
> >   Makefile         | 18 ++++++++++++++
> >   bpffs_lsm_kern.c | 63
> ++++++++++++++++++++++++++++++++++++++++++++++++
> >   bpffs_lsm_user.c | 60
> +++++++++++++++++++++++++++++++++++++++++++++
> >   4 files changed, 145 insertions(+)
> >   create mode 100644 .gitignore
> >   create mode 100644 Makefile
> >   create mode 100644 bpffs_lsm_kern.c
> >   create mode 100644 bpffs_lsm_user.c
> >
> > diff --git a/.gitignore b/.gitignore
> > new file mode 100644
> > index 000000000000..7fa02964f1dc
> > --- /dev/null
> > +++ b/.gitignore
> > @@ -0,0 +1,4 @@
> > +*.o
> > +vmlinux.h
> > +bpffs_lsm_kern.skel.h
> > +bpffs_lsm_user
> > diff --git a/Makefile b/Makefile
> > new file mode 100644
> > index 000000000000..c3d805759db3
> > --- /dev/null
> > +++ b/Makefile
> > @@ -0,0 +1,18 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +all: bpffs_lsm_user
> > +
> > +clean:
> > +	rm -rf bpffs_lsm.skel.h vmlinux.h bpffs_lsm_kern.o bpffs_lsm_user
> > +
> > +vmlinux.h:
> > +	/usr/sbin/bpftool btf dump file /sys/kernel/btf/vmlinux format c > \
> > +			  vmlinux.h
> > +
> > +bpffs_lsm_kern.skel.h: bpffs_lsm_kern.o
> > +	bpftool gen skeleton $< > $@
> > +
> > +bpffs_lsm_kern.o: bpffs_lsm_kern.c vmlinux.h
> > +	clang -Wall -Werror -g -O2 -target bpf -c $< -o $@
> > +
> > +bpffs_lsm_user: bpffs_lsm_user.c bpffs_lsm_kern.skel.h
> bpffs_lsm_kern.o
> > +	cc -Wall -Werror -g -o $@ $< -lbpf
> > diff --git a/bpffs_lsm_kern.c b/bpffs_lsm_kern.c
> > new file mode 100644
> > index 000000000000..b3ccb2a75c95
> > --- /dev/null
> > +++ b/bpffs_lsm_kern.c
> > @@ -0,0 +1,63 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (C) 2022 Huawei Technologies Duesseldorf GmbH
> > + *
> > + * Authors:
> > + * Roberto Sassu <roberto.sassu@huawei.com>
> > + *
> > + * Implement an LSM to protect a bpf filesystem instance.
> > + */
> > +
> > +#include "vmlinux.h"
> > +#include <errno.h>
> > +#include <bpf/bpf_helpers.h>
> > +#include <bpf/bpf_tracing.h>
> > +#include <bpf/bpf_core_read.h>
> > +
> > +char _license[] SEC("license") = "GPL";
> > +
> > +uint32_t monitored_pid = 0;
> > +
> > +struct {
> > +	__uint(type, BPF_MAP_TYPE_INODE_STORAGE);
> > +	__uint(map_flags, BPF_F_NO_PREALLOC);
> > +	__type(key, int);
> > +	__type(value, sizeof(uint8_t));
> > +} inode_storage_map SEC(".maps");
> > +
> > +SEC("lsm/sb_set_mnt_opts")
> > +int BPF_PROG(sb_set_mnt_opts, struct super_block *sb, void
> *mnt_opts,
> > +	     unsigned long kern_flags, unsigned long *set_kern_flags)
> > +{
> > +	u32 pid;
> > +
> > +	pid = bpf_get_current_pid_tgid() >> 32;
> > +	if (pid != monitored_pid)
> > +		return 0;
> > +
> > +	if (!bpf_inode_storage_get(&inode_storage_map, sb->s_root-
> >d_inode, 0,
> > +				   BPF_LOCAL_STORAGE_GET_F_CREATE))
> > +		return -EPERM;
> > +
> > +	return 0;
> > +}
> > +
> > +SEC("lsm/inode_unlink")
> > +int BPF_PROG(inode_unlink, struct inode *dir, struct dentry *dentry)
> > +{
> > +	if (bpf_inode_storage_get(&inode_storage_map,
> > +				  dir->i_sb->s_root->d_inode, 0, 0))
> > +		return -EPERM;
> > +
> > +	return 0;
> > +}
> > +
> > +SEC("lsm/sb_umount")
> > +int BPF_PROG(sb_umount, struct vfsmount *mnt, int flags)
> > +{
> > +	if (bpf_inode_storage_get(&inode_storage_map,
> > +				  mnt->mnt_sb->s_root->d_inode, 0, 0))
> > +		return -EPERM;
> > +
> > +	return 0;
> > +}
> > diff --git a/bpffs_lsm_user.c b/bpffs_lsm_user.c
> > new file mode 100644
> > index 000000000000..e20180cc5db9
> > --- /dev/null
> > +++ b/bpffs_lsm_user.c
> > @@ -0,0 +1,60 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (C) 2022 Huawei Technologies Duesseldorf GmbH
> > + *
> > + * Author: Roberto Sassu <roberto.sassu@huawei.com>
> > + *
> > + * Implement the user space side of the LSM for bpffs.
> > + */
> > +
> > +#include <fcntl.h>
> > +#include <unistd.h>
> > +#include <stdio.h>
> > +#include <errno.h>
> > +#include <stdlib.h>
> > +#include <unistd.h>
> > +#include <limits.h>
> > +#include <sys/mount.h>
> > +#include <sys/stat.h>
> > +
> > +#include "bpffs_lsm_kern.skel.h"
> > +
> > +#define MOUNT_FLAGS (MS_NOSUID | MS_NODEV | MS_NOEXEC |
> MS_RELATIME)
> > +
> > +int main(int argc, char *argv[])
> > +{
> > +	char mntpoint[] = "/tmp/bpf_private_mountXXXXXX";
> > +	char path[PATH_MAX];
> > +	struct bpffs_lsm_kern *skel;
> > +	int ret, i;
> > +
> > +	skel = bpffs_lsm_kern__open_and_load();
> > +	if (!skel)
> > +		return -EINVAL;
> > +
> > +	ret = bpffs_lsm_kern__attach(skel);
> > +	if (ret < 0)
> > +		goto out_destroy;
> > +
> > +	mkdtemp(mntpoint);
> > +
> > +	skel->bss->monitored_pid = getpid();
> > +	ret = mount(mntpoint, mntpoint, "bpf", MOUNT_FLAGS, NULL);
> > +	skel->bss->monitored_pid = 0;
> > +
> > +	if (ret < 0)
> > +		goto out_destroy;
> > +
> > +	for (i = 0; i < skel->skeleton->prog_cnt; i++) {
> > +		snprintf(path, sizeof(path), "%s/%s", mntpoint,
> > +			 skel->skeleton->progs[i].name);
> > +		ret = bpf_link__pin(*skel->skeleton->progs[i].link, path);
> > +		if (ret < 0)
> > +			goto out_destroy;
> > +	}
> > +
> > +	ret = 0;
> > +out_destroy:
> > +	bpffs_lsm_kern__destroy(skel);
> > +	return ret;
> > +}

Re: [PATCH 00/18] bpf: Secure and authenticated preloading of eBPF programs
Posted by Casey Schaufler 4 years, 2 months ago
On 4/4/2022 10:20 AM, Roberto Sassu wrote:
>> From: Djalal Harouni [mailto:tixxdz@gmail.com]
>> Sent: Monday, April 4, 2022 9:45 AM
>> On Sun, Apr 3, 2022 at 5:42 PM KP Singh <kpsingh@kernel.org> wrote:
>>> On Sat, Apr 2, 2022 at 1:55 AM Alexei Starovoitov
>>> <alexei.starovoitov@gmail.com> wrote:
>> ...
>>>>> Pinning
>>>>> them to unreachable inodes intuitively looked the
>>>>> way to go for achieving the stated goal.
>>>> We can consider inodes in bpffs that are not unlinkable by root
>>>> in the future, but certainly not for this use case.
>>> Can this not be already done by adding a BPF_LSM program to the
>>> inode_unlink LSM hook?
>>>
>> Also, beside of the inode_unlink... and out of curiosity: making sysfs/bpffs/
>> readonly after pinning, then using bpf LSM hooks
>> sb_mount|remount|unmount...
>> family combining bpf() LSM hook... isn't this enough to:
>> 1. Restrict who can pin to bpffs without using a full MAC
>> 2. Restrict who can delete or unmount bpf filesystem
>>
>> ?
> I'm thinking to implement something like this.
>
> First, I add a new program flag called
> BPF_F_STOP_ONCONFIRM, which causes the ref count
> of the link to increase twice at creation time. In this way,
> user space cannot make the link disappear, unless a
> confirmation is explicitly sent via the bpf() system call.
>
> Another advantage is that other LSMs can decide
> whether or not they allow a program with this flag
> (in the bpf security hook).
>
> This would work regardless of the method used to
> load the eBPF program (user space or kernel space).
>
> Second, I extend the bpf() system call with a new
> subcommand, BPF_LINK_CONFIRM_STOP, which
> decreasres the ref count for the link of the programs
> with the BPF_F_STOP_ONCONFIRM flag. I will also
> introduce a new security hook (something like
> security_link_confirm_stop), so that an LSM has the
> opportunity to deny the stop (the bpf security hook
> would not be sufficient to determine exactly for
> which link the confirmation is given, an LSM should
> be able to deny the stop for its own programs).

Would you please stop referring to a set of eBPF programs
loaded into the BPF LSM as an LSM? Call it a BPF security
module (BSM) if you must use an abbreviation. An LSM is a
provider of security_ hooks. In your case that is BPF. When
you call the set of eBPF programs an LSM it is like calling
an SELinux policy an LSM.

>
> What do you think?
>
> Thanks
>
> Roberto
>
> HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
> Managing Director: Li Peng, Zhong Ronghua
RE: [PATCH 00/18] bpf: Secure and authenticated preloading of eBPF programs
Posted by Roberto Sassu 4 years, 2 months ago
> From: Casey Schaufler [mailto:casey@schaufler-ca.com]
> Sent: Tuesday, April 5, 2022 4:50 PM
> On 4/4/2022 10:20 AM, Roberto Sassu wrote:
> >> From: Djalal Harouni [mailto:tixxdz@gmail.com]
> >> Sent: Monday, April 4, 2022 9:45 AM
> >> On Sun, Apr 3, 2022 at 5:42 PM KP Singh <kpsingh@kernel.org> wrote:
> >>> On Sat, Apr 2, 2022 at 1:55 AM Alexei Starovoitov
> >>> <alexei.starovoitov@gmail.com> wrote:
> >> ...
> >>>>> Pinning
> >>>>> them to unreachable inodes intuitively looked the
> >>>>> way to go for achieving the stated goal.
> >>>> We can consider inodes in bpffs that are not unlinkable by root
> >>>> in the future, but certainly not for this use case.
> >>> Can this not be already done by adding a BPF_LSM program to the
> >>> inode_unlink LSM hook?
> >>>
> >> Also, beside of the inode_unlink... and out of curiosity: making
> sysfs/bpffs/
> >> readonly after pinning, then using bpf LSM hooks
> >> sb_mount|remount|unmount...
> >> family combining bpf() LSM hook... isn't this enough to:
> >> 1. Restrict who can pin to bpffs without using a full MAC
> >> 2. Restrict who can delete or unmount bpf filesystem
> >>
> >> ?
> > I'm thinking to implement something like this.
> >
> > First, I add a new program flag called
> > BPF_F_STOP_ONCONFIRM, which causes the ref count
> > of the link to increase twice at creation time. In this way,
> > user space cannot make the link disappear, unless a
> > confirmation is explicitly sent via the bpf() system call.
> >
> > Another advantage is that other LSMs can decide
> > whether or not they allow a program with this flag
> > (in the bpf security hook).
> >
> > This would work regardless of the method used to
> > load the eBPF program (user space or kernel space).
> >
> > Second, I extend the bpf() system call with a new
> > subcommand, BPF_LINK_CONFIRM_STOP, which
> > decreasres the ref count for the link of the programs
> > with the BPF_F_STOP_ONCONFIRM flag. I will also
> > introduce a new security hook (something like
> > security_link_confirm_stop), so that an LSM has the
> > opportunity to deny the stop (the bpf security hook
> > would not be sufficient to determine exactly for
> > which link the confirmation is given, an LSM should
> > be able to deny the stop for its own programs).
> 
> Would you please stop referring to a set of eBPF programs
> loaded into the BPF LSM as an LSM? Call it a BPF security
> module (BSM) if you must use an abbreviation. An LSM is a
> provider of security_ hooks. In your case that is BPF. When
> you call the set of eBPF programs an LSM it is like calling
> an SELinux policy an LSM.

An eBPF program could be a provider of security_ hooks
too. The bpf LSM is an aggregator, similarly to your
infrastructure to manage built-in LSMs. Maybe, calling
it second-level LSM or secondary LSM would better
represent this new class.

The only differences are the registration method, (SEC
directive instead of DEFINE_LSM), and what the hook
implementation can access.

The implementation of a security_ hook via eBPF can
follow the same structure of built-in LSMs, i.e. it can be
uniquely responsible for enforcing and be policy-agnostic,
and can retrieve the decisions based on a policy from a
component implemented somewhere else.

Hopefully, I understood the basic principles correctly.
I let the eBPF maintainers comment on this.

Thanks

Roberto

HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Li Peng, Zhong Ronghua

> > What do you think?
> >
> > Thanks
> >
> > Roberto
> >
> > HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
> > Managing Director: Li Peng, Zhong Ronghua

Re: [PATCH 00/18] bpf: Secure and authenticated preloading of eBPF programs
Posted by Casey Schaufler 4 years, 2 months ago
On 4/5/2022 8:29 AM, Roberto Sassu wrote:
>> From: Casey Schaufler [mailto:casey@schaufler-ca.com]
>> Sent: Tuesday, April 5, 2022 4:50 PM
>> On 4/4/2022 10:20 AM, Roberto Sassu wrote:
>>>> From: Djalal Harouni [mailto:tixxdz@gmail.com]
>>>> Sent: Monday, April 4, 2022 9:45 AM
>>>> On Sun, Apr 3, 2022 at 5:42 PM KP Singh <kpsingh@kernel.org> wrote:
>>>>> On Sat, Apr 2, 2022 at 1:55 AM Alexei Starovoitov
>>>>> <alexei.starovoitov@gmail.com> wrote:
>>>> ...
>>>>>>> Pinning
>>>>>>> them to unreachable inodes intuitively looked the
>>>>>>> way to go for achieving the stated goal.
>>>>>> We can consider inodes in bpffs that are not unlinkable by root
>>>>>> in the future, but certainly not for this use case.
>>>>> Can this not be already done by adding a BPF_LSM program to the
>>>>> inode_unlink LSM hook?
>>>>>
>>>> Also, beside of the inode_unlink... and out of curiosity: making
>> sysfs/bpffs/
>>>> readonly after pinning, then using bpf LSM hooks
>>>> sb_mount|remount|unmount...
>>>> family combining bpf() LSM hook... isn't this enough to:
>>>> 1. Restrict who can pin to bpffs without using a full MAC
>>>> 2. Restrict who can delete or unmount bpf filesystem
>>>>
>>>> ?
>>> I'm thinking to implement something like this.
>>>
>>> First, I add a new program flag called
>>> BPF_F_STOP_ONCONFIRM, which causes the ref count
>>> of the link to increase twice at creation time. In this way,
>>> user space cannot make the link disappear, unless a
>>> confirmation is explicitly sent via the bpf() system call.
>>>
>>> Another advantage is that other LSMs can decide
>>> whether or not they allow a program with this flag
>>> (in the bpf security hook).
>>>
>>> This would work regardless of the method used to
>>> load the eBPF program (user space or kernel space).
>>>
>>> Second, I extend the bpf() system call with a new
>>> subcommand, BPF_LINK_CONFIRM_STOP, which
>>> decreasres the ref count for the link of the programs
>>> with the BPF_F_STOP_ONCONFIRM flag. I will also
>>> introduce a new security hook (something like
>>> security_link_confirm_stop), so that an LSM has the
>>> opportunity to deny the stop (the bpf security hook
>>> would not be sufficient to determine exactly for
>>> which link the confirmation is given, an LSM should
>>> be able to deny the stop for its own programs).
>> Would you please stop referring to a set of eBPF programs
>> loaded into the BPF LSM as an LSM? Call it a BPF security
>> module (BSM) if you must use an abbreviation. An LSM is a
>> provider of security_ hooks. In your case that is BPF. When
>> you call the set of eBPF programs an LSM it is like calling
>> an SELinux policy an LSM.
> An eBPF program could be a provider of security_ hooks
> too.

No, it can't. If I look in /sys/kernel/security/lsm what
you see is "bpf". The LSM is BPF. What BPF does in its
hooks is up to it and its responsibility.

>   The bpf LSM is an aggregator, similarly to your
> infrastructure to manage built-in LSMs. Maybe, calling
> it second-level LSM or secondary LSM would better
> represent this new class.

It isn't an LSM, and adding a qualifier doesn't make it
one and only adds to the confusion.

> The only differences are the registration method, (SEC
> directive instead of DEFINE_LSM), and what the hook
> implementation can access.

Those two things pretty well define what an LSM is.

> The implementation of a security_ hook via eBPF can
> follow the same structure of built-in LSMs, i.e. it can be
> uniquely responsible for enforcing and be policy-agnostic,
> and can retrieve the decisions based on a policy from a
> component implemented somewhere else.

The BPF LSM provides mechanism. The eBPF programs provide policy.

>
> Hopefully, I understood the basic principles correctly.
> I let the eBPF maintainers comment on this.
>
> Thanks
>
> Roberto
>
> HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
> Managing Director: Li Peng, Zhong Ronghua
>
>>> What do you think?
>>>
>>> Thanks
>>>
>>> Roberto
>>>
>>> HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
>>> Managing Director: Li Peng, Zhong Ronghua
Re: [PATCH 00/18] bpf: Secure and authenticated preloading of eBPF programs
Posted by KP Singh 4 years, 2 months ago
On Tue, Apr 5, 2022 at 6:22 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>
> On 4/5/2022 8:29 AM, Roberto Sassu wrote:
> >> From: Casey Schaufler [mailto:casey@schaufler-ca.com]
> >> Sent: Tuesday, April 5, 2022 4:50 PM
> >> On 4/4/2022 10:20 AM, Roberto Sassu wrote:
> >>>> From: Djalal Harouni [mailto:tixxdz@gmail.com]
> >>>> Sent: Monday, April 4, 2022 9:45 AM
> >>>> On Sun, Apr 3, 2022 at 5:42 PM KP Singh <kpsingh@kernel.org> wrote:
> >>>>> On Sat, Apr 2, 2022 at 1:55 AM Alexei Starovoitov
> >>>>> <alexei.starovoitov@gmail.com> wrote:
> >>>> ...
> >>>>>>> Pinning
> >>>>>>> them to unreachable inodes intuitively looked the
> >>>>>>> way to go for achieving the stated goal.
> >>>>>> We can consider inodes in bpffs that are not unlinkable by root
> >>>>>> in the future, but certainly not for this use case.
> >>>>> Can this not be already done by adding a BPF_LSM program to the
> >>>>> inode_unlink LSM hook?
> >>>>>
> >>>> Also, beside of the inode_unlink... and out of curiosity: making
> >> sysfs/bpffs/
> >>>> readonly after pinning, then using bpf LSM hooks
> >>>> sb_mount|remount|unmount...
> >>>> family combining bpf() LSM hook... isn't this enough to:
> >>>> 1. Restrict who can pin to bpffs without using a full MAC
> >>>> 2. Restrict who can delete or unmount bpf filesystem
> >>>>
> >>>> ?
> >>> I'm thinking to implement something like this.
> >>>
> >>> First, I add a new program flag called
> >>> BPF_F_STOP_ONCONFIRM, which causes the ref count
> >>> of the link to increase twice at creation time. In this way,
> >>> user space cannot make the link disappear, unless a
> >>> confirmation is explicitly sent via the bpf() system call.
> >>>
> >>> Another advantage is that other LSMs can decide
> >>> whether or not they allow a program with this flag
> >>> (in the bpf security hook).
> >>>
> >>> This would work regardless of the method used to
> >>> load the eBPF program (user space or kernel space).
> >>>
> >>> Second, I extend the bpf() system call with a new
> >>> subcommand, BPF_LINK_CONFIRM_STOP, which
> >>> decreasres the ref count for the link of the programs
> >>> with the BPF_F_STOP_ONCONFIRM flag. I will also
> >>> introduce a new security hook (something like
> >>> security_link_confirm_stop), so that an LSM has the
> >>> opportunity to deny the stop (the bpf security hook
> >>> would not be sufficient to determine exactly for
> >>> which link the confirmation is given, an LSM should
> >>> be able to deny the stop for its own programs).
> >> Would you please stop referring to a set of eBPF programs
> >> loaded into the BPF LSM as an LSM? Call it a BPF security
> >> module (BSM) if you must use an abbreviation. An LSM is a
> >> provider of security_ hooks. In your case that is BPF. When
> >> you call the set of eBPF programs an LSM it is like calling
> >> an SELinux policy an LSM.
> > An eBPF program could be a provider of security_ hooks
> > too.
>
> No, it can't. If I look in /sys/kernel/security/lsm what
> you see is "bpf". The LSM is BPF. What BPF does in its
> hooks is up to it and its responsibility.
>
> >   The bpf LSM is an aggregator, similarly to your
> > infrastructure to manage built-in LSMs. Maybe, calling
> > it second-level LSM or secondary LSM would better
> > represent this new class.
>
> It isn't an LSM, and adding a qualifier doesn't make it
> one and only adds to the confusion.
>
> > The only differences are the registration method, (SEC
> > directive instead of DEFINE_LSM), and what the hook
> > implementation can access.
>
> Those two things pretty well define what an LSM is.
>
> > The implementation of a security_ hook via eBPF can
> > follow the same structure of built-in LSMs, i.e. it can be
> > uniquely responsible for enforcing and be policy-agnostic,
> > and can retrieve the decisions based on a policy from a
> > component implemented somewhere else.
>
> The BPF LSM provides mechanism. The eBPF programs provide policy.

Yeah, let's stick what we call an LSM in the kernel, Here,
"bpf" is the LSM like selinux,apparmor and this is what you set in
CONFIG_LSM or pass on cmdline as lsm= and can be seen
in /sys/kernel/security/lsm

Calling your BPF programs an LSM will just confuse people.

>
> >
> > Hopefully, I understood the basic principles correctly.
> > I let the eBPF maintainers comment on this.
> >
> > Thanks
> >
> > Roberto
> >
> > HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
> > Managing Director: Li Peng, Zhong Ronghua
> >
> >>> What do you think?
> >>>
> >>> Thanks
> >>>
> >>> Roberto
> >>>
> >>> HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
> >>> Managing Director: Li Peng, Zhong Ronghua