diff mbox series

[v10,bpf-next,4/9] bpf: Add kfunc bpf_get_file_xattr

Message ID 20231103214535.2674059-5-song@kernel.org (mailing list archive)
State Superseded
Delegated to: BPF
Headers show
Series bpf: File verification with LSM and fsverity | expand

Checks

Context Check Description
bpf/vmtest-bpf-next-PR pending PR summary
bpf/vmtest-bpf-next-VM_Test-9 success Logs for aarch64-gcc / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-25 success Logs for x86_64-gcc / test (test_progs_parallel, true, 30) / test_progs_parallel on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-26 success Logs for x86_64-gcc / test (test_verifier, false, 360) / test_verifier on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-27 success Logs for x86_64-gcc / veristat / veristat on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-28 success Logs for x86_64-llvm-16 / build / build for x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-29 success Logs for x86_64-llvm-16 / test (test_maps, false, 360) / test_maps on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-30 success Logs for x86_64-llvm-16 / test (test_progs, false, 360) / test_progs on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-31 success Logs for x86_64-llvm-16 / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-32 success Logs for x86_64-llvm-16 / test (test_verifier, false, 360) / test_verifier on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-33 success Logs for x86_64-llvm-16 / veristat
bpf/vmtest-bpf-next-VM_Test-2 success Logs for Validate matrix.py
bpf/vmtest-bpf-next-VM_Test-1 success Logs for ShellCheck
bpf/vmtest-bpf-next-VM_Test-7 success Logs for aarch64-gcc / test (test_verifier, false, 360) / test_verifier on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-3 success Logs for aarch64-gcc / build / build for aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-8 success Logs for aarch64-gcc / veristat
bpf/vmtest-bpf-next-VM_Test-0 success Logs for Lint
bpf/vmtest-bpf-next-VM_Test-10 success Logs for set-matrix
bpf/vmtest-bpf-next-VM_Test-11 success Logs for x86_64-gcc / build / build for x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-12 success Logs for x86_64-gcc / test (test_maps, false, 360) / test_maps on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-17 success Logs for x86_64-gcc / test (test_verifier, false, 360) / test_verifier on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-19 success Logs for x86_64-llvm-16 / build / build for x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-20 success Logs for x86_64-llvm-16 / test (test_maps, false, 360) / test_maps on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-23 success Logs for x86_64-llvm-16 / test (test_verifier, false, 360) / test_verifier on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-24 success Logs for x86_64-llvm-16 / veristat
bpf/vmtest-bpf-next-VM_Test-6 success Logs for aarch64-gcc / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-4 success Logs for aarch64-gcc / test (test_maps, false, 360) / test_maps on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-5 success Logs for aarch64-gcc / test (test_progs, false, 360) / test_progs on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-13 success Logs for x86_64-gcc / test (test_progs, false, 360) / test_progs on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-14 success Logs for x86_64-gcc / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-15 success Logs for x86_64-gcc / test (test_progs_no_alu32_parallel, true, 30) / test_progs_no_alu32_parallel on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-16 success Logs for x86_64-gcc / test (test_progs_parallel, true, 30) / test_progs_parallel on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-18 success Logs for x86_64-gcc / veristat / veristat on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-21 success Logs for x86_64-llvm-16 / test (test_progs, false, 360) / test_progs on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-22 success Logs for x86_64-llvm-16 / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with llvm-16
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for bpf-next, async
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit fail Errors and warnings before: 1366 this patch: 1368
netdev/cc_maintainers warning 9 maintainers not CCed: sdf@google.com linux-trace-kernel@vger.kernel.org john.fastabend@gmail.com jolsa@kernel.org mhiramat@kernel.org yonghong.song@linux.dev rostedt@goodmis.org haoluo@google.com martin.lau@linux.dev
netdev/build_clang success Errors and warnings before: 1379 this patch: 1379
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn fail Errors and warnings before: 1394 this patch: 1396
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 76 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Song Liu Nov. 3, 2023, 9:45 p.m. UTC 
It is common practice for security solutions to store tags/labels in
xattrs. To implement similar functionalities in BPF LSM, add new kfunc
bpf_get_file_xattr().

The first use case of bpf_get_file_xattr() is to implement file
verifications with asymmetric keys. Specificially, security applications
could use fsverity for file hashes and use xattr to store file signatures.
(kfunc for fsverity hash will be added in a separate commit.)

Currently, only xattrs with "user." prefix can be read with kfunc
bpf_get_file_xattr(). As use cases evolve, we may add a dedicated prefix
for bpf_get_file_xattr().

To avoid recursion, bpf_get_file_xattr can be only called from LSM hooks.

Signed-off-by: Song Liu <song@kernel.org>
---
 kernel/trace/bpf_trace.c | 64 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 64 insertions(+)

Comments

Andrii Nakryiko Nov. 3, 2023, 10:10 p.m. UTC | #1 
On Fri, Nov 3, 2023 at 2:46 PM Song Liu <song@kernel.org> wrote:
>
> It is common practice for security solutions to store tags/labels in
> xattrs. To implement similar functionalities in BPF LSM, add new kfunc
> bpf_get_file_xattr().
>
> The first use case of bpf_get_file_xattr() is to implement file
> verifications with asymmetric keys. Specificially, security applications
> could use fsverity for file hashes and use xattr to store file signatures.
> (kfunc for fsverity hash will be added in a separate commit.)
>
> Currently, only xattrs with "user." prefix can be read with kfunc
> bpf_get_file_xattr(). As use cases evolve, we may add a dedicated prefix
> for bpf_get_file_xattr().
>
> To avoid recursion, bpf_get_file_xattr can be only called from LSM hooks.
>
> Signed-off-by: Song Liu <song@kernel.org>
> ---
>  kernel/trace/bpf_trace.c | 64 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 64 insertions(+)
>
> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index bfe6fb83e8d0..82eaa099053b 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -24,6 +24,7 @@
>  #include <linux/key.h>
>  #include <linux/verification.h>
>  #include <linux/namei.h>
> +#include <linux/fileattr.h>
>
>  #include <net/bpf_sk_storage.h>
>
> @@ -1431,6 +1432,69 @@ static int __init bpf_key_sig_kfuncs_init(void)
>  late_initcall(bpf_key_sig_kfuncs_init);
>  #endif /* CONFIG_KEYS */
>
> +/* filesystem kfuncs */
> +__diag_push();
> +__diag_ignore_all("-Wmissing-prototypes",
> +                 "kfuncs which will be used in BPF programs");
> +

please use __bpf_kfunc_{start,end}_defs macros, from [0]

  [0] https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf.git/commit/?id=391145ba2acc

> +/**
> + * bpf_get_file_xattr - get xattr of a file
> + * @file: file to get xattr from
> + * @name__str: name of the xattr
> + * @value_ptr: output buffer of the xattr value
> + *
> + * Get xattr *name__str* of *file* and store the output in *value_ptr*.
> + *
> + * For security reasons, only *name__str* with prefix "user." is allowed.
> + *
> + * Return: 0 on success, a negative value on error.
> + */
> +__bpf_kfunc int bpf_get_file_xattr(struct file *file, const char *name__str,
> +                                  struct bpf_dynptr_kern *value_ptr)
> +{
> +       struct dentry *dentry;
> +       void *value;
> +
> +       if (strncmp(name__str, XATTR_USER_PREFIX, XATTR_USER_PREFIX_LEN))
> +               return -EPERM;
> +
> +       value = __bpf_dynptr_data_rw(value_ptr, __bpf_dynptr_size(value_ptr));
> +       if (!value)
> +               return -EINVAL;
> +
> +       dentry = file_dentry(file);
> +       return __vfs_getxattr(dentry, dentry->d_inode, name__str,
> +                             value, __bpf_dynptr_size(value_ptr));

nit: probably cleaner to get size once into local variable and use
that throughout

> +}
> +
> +__diag_pop();
> +
> +BTF_SET8_START(fs_kfunc_set_ids)
> +BTF_ID_FLAGS(func, bpf_get_file_xattr, KF_SLEEPABLE | KF_TRUSTED_ARGS)
> +BTF_SET8_END(fs_kfunc_set_ids)
> +
> +static int bpf_get_file_xattr_filter(const struct bpf_prog *prog, u32 kfunc_id)
> +{
> +       if (!btf_id_set8_contains(&fs_kfunc_set_ids, kfunc_id))
> +               return 0;
> +
> +       /* Only allow to attach from LSM hooks, to avoid recursion */
> +       return prog->type != BPF_PROG_TYPE_LSM ? -EACCES : 0;
> +}
> +
> +const struct btf_kfunc_id_set bpf_fs_kfunc_set = {
> +       .owner = THIS_MODULE,
> +       .set = &fs_kfunc_set_ids,
> +       .filter = bpf_get_file_xattr_filter,
> +};
> +
> +static int __init bpf_fs_kfuncs_init(void)
> +{
> +       return register_btf_kfunc_id_set(BPF_PROG_TYPE_LSM, &bpf_fs_kfunc_set);
> +}
> +
> +late_initcall(bpf_fs_kfuncs_init);
> +
>  static const struct bpf_func_proto *
>  bpf_tracing_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
>  {
> --
> 2.34.1
>
Song Liu Nov. 3, 2023, 10:30 p.m. UTC | #2 
On Fri, Nov 3, 2023 at 3:10 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Fri, Nov 3, 2023 at 2:46 PM Song Liu <song@kernel.org> wrote:
> >
> > It is common practice for security solutions to store tags/labels in
> > xattrs. To implement similar functionalities in BPF LSM, add new kfunc
> > bpf_get_file_xattr().
> >
> > The first use case of bpf_get_file_xattr() is to implement file
> > verifications with asymmetric keys. Specificially, security applications
> > could use fsverity for file hashes and use xattr to store file signatures.
> > (kfunc for fsverity hash will be added in a separate commit.)
> >
> > Currently, only xattrs with "user." prefix can be read with kfunc
> > bpf_get_file_xattr(). As use cases evolve, we may add a dedicated prefix
> > for bpf_get_file_xattr().
> >
> > To avoid recursion, bpf_get_file_xattr can be only called from LSM hooks.
> >
> > Signed-off-by: Song Liu <song@kernel.org>
> > ---
> >  kernel/trace/bpf_trace.c | 64 ++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 64 insertions(+)
> >
> > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> > index bfe6fb83e8d0..82eaa099053b 100644
> > --- a/kernel/trace/bpf_trace.c
> > +++ b/kernel/trace/bpf_trace.c
> > @@ -24,6 +24,7 @@
> >  #include <linux/key.h>
> >  #include <linux/verification.h>
> >  #include <linux/namei.h>
> > +#include <linux/fileattr.h>
> >
> >  #include <net/bpf_sk_storage.h>
> >
> > @@ -1431,6 +1432,69 @@ static int __init bpf_key_sig_kfuncs_init(void)
> >  late_initcall(bpf_key_sig_kfuncs_init);
> >  #endif /* CONFIG_KEYS */
> >
> > +/* filesystem kfuncs */
> > +__diag_push();
> > +__diag_ignore_all("-Wmissing-prototypes",
> > +                 "kfuncs which will be used in BPF programs");
> > +
>
> please use __bpf_kfunc_{start,end}_defs macros, from [0]
>
>   [0] https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf.git/commit/?id=391145ba2acc

Nice! I was thinking about the same issue (-Wmissing-declarations).

But this patch is not pulled into bpf-next yet (only in bpf). How about we keep
__diag_ignore_all() etc for now?

Thanks,
Song
Song Liu Nov. 3, 2023, 10:38 p.m. UTC | #3 
On Fri, Nov 3, 2023 at 3:30 PM Song Liu <song@kernel.org> wrote:
>
> On Fri, Nov 3, 2023 at 3:10 PM Andrii Nakryiko
> <andrii.nakryiko@gmail.com> wrote:
> >
> > On Fri, Nov 3, 2023 at 2:46 PM Song Liu <song@kernel.org> wrote:
> > >
> > > It is common practice for security solutions to store tags/labels in
> > > xattrs. To implement similar functionalities in BPF LSM, add new kfunc
> > > bpf_get_file_xattr().
> > >
> > > The first use case of bpf_get_file_xattr() is to implement file
> > > verifications with asymmetric keys. Specificially, security applications
> > > could use fsverity for file hashes and use xattr to store file signatures.
> > > (kfunc for fsverity hash will be added in a separate commit.)
> > >
> > > Currently, only xattrs with "user." prefix can be read with kfunc
> > > bpf_get_file_xattr(). As use cases evolve, we may add a dedicated prefix
> > > for bpf_get_file_xattr().
> > >
> > > To avoid recursion, bpf_get_file_xattr can be only called from LSM hooks.
> > >
> > > Signed-off-by: Song Liu <song@kernel.org>
> > > ---
> > >  kernel/trace/bpf_trace.c | 64 ++++++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 64 insertions(+)
> > >
> > > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> > > index bfe6fb83e8d0..82eaa099053b 100644
> > > --- a/kernel/trace/bpf_trace.c
> > > +++ b/kernel/trace/bpf_trace.c
> > > @@ -24,6 +24,7 @@
> > >  #include <linux/key.h>
> > >  #include <linux/verification.h>
> > >  #include <linux/namei.h>
> > > +#include <linux/fileattr.h>
> > >
> > >  #include <net/bpf_sk_storage.h>
> > >
> > > @@ -1431,6 +1432,69 @@ static int __init bpf_key_sig_kfuncs_init(void)
> > >  late_initcall(bpf_key_sig_kfuncs_init);
> > >  #endif /* CONFIG_KEYS */
> > >
> > > +/* filesystem kfuncs */
> > > +__diag_push();
> > > +__diag_ignore_all("-Wmissing-prototypes",
> > > +                 "kfuncs which will be used in BPF programs");
> > > +
> >
> > please use __bpf_kfunc_{start,end}_defs macros, from [0]
> >
> >   [0] https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf.git/commit/?id=391145ba2acc
>
> Nice! I was thinking about the same issue (-Wmissing-declarations).
>
> But this patch is not pulled into bpf-next yet (only in bpf). How about we keep
> __diag_ignore_all() etc for now?

I will add __diag_ignore_all("-Wmissing-declarations",) so it is
equivalent to the marcos.

Thanks,
Song
Andrii Nakryiko Nov. 3, 2023, 10:53 p.m. UTC | #4 
On Fri, Nov 3, 2023 at 3:38 PM Song Liu <song@kernel.org> wrote:
>
> On Fri, Nov 3, 2023 at 3:30 PM Song Liu <song@kernel.org> wrote:
> >
> > On Fri, Nov 3, 2023 at 3:10 PM Andrii Nakryiko
> > <andrii.nakryiko@gmail.com> wrote:
> > >
> > > On Fri, Nov 3, 2023 at 2:46 PM Song Liu <song@kernel.org> wrote:
> > > >
> > > > It is common practice for security solutions to store tags/labels in
> > > > xattrs. To implement similar functionalities in BPF LSM, add new kfunc
> > > > bpf_get_file_xattr().
> > > >
> > > > The first use case of bpf_get_file_xattr() is to implement file
> > > > verifications with asymmetric keys. Specificially, security applications
> > > > could use fsverity for file hashes and use xattr to store file signatures.
> > > > (kfunc for fsverity hash will be added in a separate commit.)
> > > >
> > > > Currently, only xattrs with "user." prefix can be read with kfunc
> > > > bpf_get_file_xattr(). As use cases evolve, we may add a dedicated prefix
> > > > for bpf_get_file_xattr().
> > > >
> > > > To avoid recursion, bpf_get_file_xattr can be only called from LSM hooks.
> > > >
> > > > Signed-off-by: Song Liu <song@kernel.org>
> > > > ---
> > > >  kernel/trace/bpf_trace.c | 64 ++++++++++++++++++++++++++++++++++++++++
> > > >  1 file changed, 64 insertions(+)
> > > >
> > > > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> > > > index bfe6fb83e8d0..82eaa099053b 100644
> > > > --- a/kernel/trace/bpf_trace.c
> > > > +++ b/kernel/trace/bpf_trace.c
> > > > @@ -24,6 +24,7 @@
> > > >  #include <linux/key.h>
> > > >  #include <linux/verification.h>
> > > >  #include <linux/namei.h>
> > > > +#include <linux/fileattr.h>
> > > >
> > > >  #include <net/bpf_sk_storage.h>
> > > >
> > > > @@ -1431,6 +1432,69 @@ static int __init bpf_key_sig_kfuncs_init(void)
> > > >  late_initcall(bpf_key_sig_kfuncs_init);
> > > >  #endif /* CONFIG_KEYS */
> > > >
> > > > +/* filesystem kfuncs */
> > > > +__diag_push();
> > > > +__diag_ignore_all("-Wmissing-prototypes",
> > > > +                 "kfuncs which will be used in BPF programs");
> > > > +
> > >
> > > please use __bpf_kfunc_{start,end}_defs macros, from [0]
> > >
> > >   [0] https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf.git/commit/?id=391145ba2acc
> >
> > Nice! I was thinking about the same issue (-Wmissing-declarations).
> >
> > But this patch is not pulled into bpf-next yet (only in bpf). How about we keep
> > __diag_ignore_all() etc for now?
>
> I will add __diag_ignore_all("-Wmissing-declarations",) so it is
> equivalent to the marcos.

Ah, it's bpf tree, not bpf-next, I see. Ok, we'll have to leave it as
is for now and then clean up later.

>
> Thanks,
> Song
diff mbox series

Patch

diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index bfe6fb83e8d0..82eaa099053b 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -24,6 +24,7 @@ 
 #include <linux/key.h>
 #include <linux/verification.h>
 #include <linux/namei.h>
+#include <linux/fileattr.h>
 
 #include <net/bpf_sk_storage.h>
 
@@ -1431,6 +1432,69 @@  static int __init bpf_key_sig_kfuncs_init(void)
 late_initcall(bpf_key_sig_kfuncs_init);
 #endif /* CONFIG_KEYS */
 
+/* filesystem kfuncs */
+__diag_push();
+__diag_ignore_all("-Wmissing-prototypes",
+		  "kfuncs which will be used in BPF programs");
+
+/**
+ * bpf_get_file_xattr - get xattr of a file
+ * @file: file to get xattr from
+ * @name__str: name of the xattr
+ * @value_ptr: output buffer of the xattr value
+ *
+ * Get xattr *name__str* of *file* and store the output in *value_ptr*.
+ *
+ * For security reasons, only *name__str* with prefix "user." is allowed.
+ *
+ * Return: 0 on success, a negative value on error.
+ */
+__bpf_kfunc int bpf_get_file_xattr(struct file *file, const char *name__str,
+				   struct bpf_dynptr_kern *value_ptr)
+{
+	struct dentry *dentry;
+	void *value;
+
+	if (strncmp(name__str, XATTR_USER_PREFIX, XATTR_USER_PREFIX_LEN))
+		return -EPERM;
+
+	value = __bpf_dynptr_data_rw(value_ptr, __bpf_dynptr_size(value_ptr));
+	if (!value)
+		return -EINVAL;
+
+	dentry = file_dentry(file);
+	return __vfs_getxattr(dentry, dentry->d_inode, name__str,
+			      value, __bpf_dynptr_size(value_ptr));
+}
+
+__diag_pop();
+
+BTF_SET8_START(fs_kfunc_set_ids)
+BTF_ID_FLAGS(func, bpf_get_file_xattr, KF_SLEEPABLE | KF_TRUSTED_ARGS)
+BTF_SET8_END(fs_kfunc_set_ids)
+
+static int bpf_get_file_xattr_filter(const struct bpf_prog *prog, u32 kfunc_id)
+{
+	if (!btf_id_set8_contains(&fs_kfunc_set_ids, kfunc_id))
+		return 0;
+
+	/* Only allow to attach from LSM hooks, to avoid recursion */
+	return prog->type != BPF_PROG_TYPE_LSM ? -EACCES : 0;
+}
+
+const struct btf_kfunc_id_set bpf_fs_kfunc_set = {
+	.owner = THIS_MODULE,
+	.set = &fs_kfunc_set_ids,
+	.filter = bpf_get_file_xattr_filter,
+};
+
+static int __init bpf_fs_kfuncs_init(void)
+{
+	return register_btf_kfunc_id_set(BPF_PROG_TYPE_LSM, &bpf_fs_kfunc_set);
+}
+
+late_initcall(bpf_fs_kfuncs_init);
+
 static const struct bpf_func_proto *
 bpf_tracing_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 {