| Message ID | 20231012222810.4120312-12-andrii@kernel.org (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Paul Moore |
| Headers | show |
| Series | BPF token and BPF FS-based delegation | expand |
On Oct 12, 2023 Andrii Nakryiko <andrii@kernel.org> wrote: > > Wire up bpf_token_create and bpf_token_free LSM hooks, which allow to > allocate LSM security blob (we add `void *security` field to struct > bpf_token for that), but also control who can instantiate BPF token. > This follows existing pattern for BPF map and BPF prog. > > Signed-off-by: Andrii Nakryiko <andrii@kernel.org> > --- > include/linux/bpf.h | 3 +++ > include/linux/lsm_hook_defs.h | 3 +++ > include/linux/security.h | 11 +++++++++++ > kernel/bpf/bpf_lsm.c | 2 ++ > kernel/bpf/token.c | 6 ++++++ > security/security.c | 28 ++++++++++++++++++++++++++++ > 6 files changed, 53 insertions(+) ... > diff --git a/kernel/bpf/token.c b/kernel/bpf/token.c > index d4e0cc8075d3..18fd1e04f92d 100644 > --- a/kernel/bpf/token.c > +++ b/kernel/bpf/token.c > @@ -7,6 +7,7 @@ > #include <linux/idr.h> > #include <linux/namei.h> > #include <linux/user_namespace.h> > +#include <linux/security.h> > > bool bpf_token_capable(const struct bpf_token *token, int cap) > { > @@ -28,6 +29,7 @@ void bpf_token_inc(struct bpf_token *token) > > static void bpf_token_free(struct bpf_token *token) > { > + security_bpf_token_free(token); > put_user_ns(token->userns); > kvfree(token); > } > @@ -183,6 +185,10 @@ int bpf_token_create(union bpf_attr *attr) > token->allowed_progs = mnt_opts->delegate_progs; > token->allowed_attachs = mnt_opts->delegate_attachs; > > + err = security_bpf_token_create(token, attr, &path); > + if (err) > + goto out_token; > + > fd = get_unused_fd_flags(O_CLOEXEC); > if (fd < 0) { > err = fd; As long as bpf_token_alloc() remains separate from bpf_token_create() I'm not comfortable not having a security_bpf_token_alloc() hook in bpf_token_alloc(). If you really don't want a LSM token alloc hook can you fold bpf_token_alloc() into bpf_token_create()? -- paul-moore.com
On Fri, Oct 13, 2023 at 2:15 PM Paul Moore <paul@paul-moore.com> wrote: > > On Oct 12, 2023 Andrii Nakryiko <andrii@kernel.org> wrote: > > > > Wire up bpf_token_create and bpf_token_free LSM hooks, which allow to > > allocate LSM security blob (we add `void *security` field to struct > > bpf_token for that), but also control who can instantiate BPF token. > > This follows existing pattern for BPF map and BPF prog. > > > > Signed-off-by: Andrii Nakryiko <andrii@kernel.org> > > --- > > include/linux/bpf.h | 3 +++ > > include/linux/lsm_hook_defs.h | 3 +++ > > include/linux/security.h | 11 +++++++++++ > > kernel/bpf/bpf_lsm.c | 2 ++ > > kernel/bpf/token.c | 6 ++++++ > > security/security.c | 28 ++++++++++++++++++++++++++++ > > 6 files changed, 53 insertions(+) > > ... > > > diff --git a/kernel/bpf/token.c b/kernel/bpf/token.c > > index d4e0cc8075d3..18fd1e04f92d 100644 > > --- a/kernel/bpf/token.c > > +++ b/kernel/bpf/token.c > > @@ -7,6 +7,7 @@ > > #include <linux/idr.h> > > #include <linux/namei.h> > > #include <linux/user_namespace.h> > > +#include <linux/security.h> > > > > bool bpf_token_capable(const struct bpf_token *token, int cap) > > { > > @@ -28,6 +29,7 @@ void bpf_token_inc(struct bpf_token *token) > > > > static void bpf_token_free(struct bpf_token *token) > > { > > + security_bpf_token_free(token); > > put_user_ns(token->userns); > > kvfree(token); > > } > > @@ -183,6 +185,10 @@ int bpf_token_create(union bpf_attr *attr) > > token->allowed_progs = mnt_opts->delegate_progs; > > token->allowed_attachs = mnt_opts->delegate_attachs; > > > > + err = security_bpf_token_create(token, attr, &path); > > + if (err) > > + goto out_token; > > + > > fd = get_unused_fd_flags(O_CLOEXEC); > > if (fd < 0) { > > err = fd; > > As long as bpf_token_alloc() remains separate from bpf_token_create() > I'm not comfortable not having a security_bpf_token_alloc() hook in > bpf_token_alloc(). If you really don't want a LSM token alloc hook > can you fold bpf_token_alloc() into bpf_token_create()? Yeah, that's easy, I'll just inline it into bpf_token_create(), which is the only place where I was intending to use it anyways. I just want to keep all this consistent between map, token, and progs. > > -- > paul-moore.com
diff --git a/include/linux/bpf.h b/include/linux/bpf.h index c87af564f464..dfcac60f1857 100644 --- a/include/linux/bpf.h +++ b/include/linux/bpf.h @@ -1585,6 +1585,9 @@ struct bpf_token { u64 allowed_maps; u64 allowed_progs; u64 allowed_attachs; +#ifdef CONFIG_SECURITY + void *security; +#endif }; struct bpf_struct_ops_value; diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h index 0adfb136521a..d776c9b7b856 100644 --- a/include/linux/lsm_hook_defs.h +++ b/include/linux/lsm_hook_defs.h @@ -404,6 +404,9 @@ LSM_HOOK(void, LSM_RET_VOID, bpf_map_free, struct bpf_map *map) LSM_HOOK(int, 0, bpf_prog_load, struct bpf_prog *prog, union bpf_attr *attr, struct bpf_token *token) LSM_HOOK(void, LSM_RET_VOID, bpf_prog_free, struct bpf_prog *prog) +LSM_HOOK(int, 0, bpf_token_create, struct bpf_token *token, union bpf_attr *attr, + struct path *path) +LSM_HOOK(void, LSM_RET_VOID, bpf_token_free, struct bpf_token *token) #endif /* CONFIG_BPF_SYSCALL */ LSM_HOOK(int, 0, locked_down, enum lockdown_reason what) diff --git a/include/linux/security.h b/include/linux/security.h index 59c5fab2c4d6..b9cb1446bb78 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -2031,6 +2031,9 @@ extern void security_bpf_map_free(struct bpf_map *map); extern int security_bpf_prog_load(struct bpf_prog *prog, union bpf_attr *attr, struct bpf_token *token); extern void security_bpf_prog_free(struct bpf_prog *prog); +extern int security_bpf_token_create(struct bpf_token *token, union bpf_attr *attr, + struct path *path); +extern void security_bpf_token_free(struct bpf_token *token); #else static inline int security_bpf(int cmd, union bpf_attr *attr, unsigned int size) @@ -2065,6 +2068,14 @@ static inline int security_bpf_prog_load(struct bpf_prog *prog, union bpf_attr * static inline void security_bpf_prog_free(struct bpf_prog *prog) { } + +static inline int security_bpf_token_create(struct bpf_token *token, union bpf_attr *attr, + struct path *path) +{ + return 0; +} +static inline void security_bpf_token_free(struct bpf_token *token) +{ } #endif /* CONFIG_SECURITY */ #endif /* CONFIG_BPF_SYSCALL */ diff --git a/kernel/bpf/bpf_lsm.c b/kernel/bpf/bpf_lsm.c index 9e4e615f11eb..2a019528953e 100644 --- a/kernel/bpf/bpf_lsm.c +++ b/kernel/bpf/bpf_lsm.c @@ -265,6 +265,8 @@ BTF_ID(func, bpf_lsm_bpf_map_free) BTF_ID(func, bpf_lsm_bpf_prog) BTF_ID(func, bpf_lsm_bpf_prog_load) BTF_ID(func, bpf_lsm_bpf_prog_free) +BTF_ID(func, bpf_lsm_bpf_token_create) +BTF_ID(func, bpf_lsm_bpf_token_free) BTF_ID(func, bpf_lsm_bprm_check_security) BTF_ID(func, bpf_lsm_bprm_committed_creds) BTF_ID(func, bpf_lsm_bprm_committing_creds) diff --git a/kernel/bpf/token.c b/kernel/bpf/token.c index d4e0cc8075d3..18fd1e04f92d 100644 --- a/kernel/bpf/token.c +++ b/kernel/bpf/token.c @@ -7,6 +7,7 @@ #include <linux/idr.h> #include <linux/namei.h> #include <linux/user_namespace.h> +#include <linux/security.h> bool bpf_token_capable(const struct bpf_token *token, int cap) { @@ -28,6 +29,7 @@ void bpf_token_inc(struct bpf_token *token) static void bpf_token_free(struct bpf_token *token) { + security_bpf_token_free(token); put_user_ns(token->userns); kvfree(token); } @@ -183,6 +185,10 @@ int bpf_token_create(union bpf_attr *attr) token->allowed_progs = mnt_opts->delegate_progs; token->allowed_attachs = mnt_opts->delegate_attachs; + err = security_bpf_token_create(token, attr, &path); + if (err) + goto out_token; + fd = get_unused_fd_flags(O_CLOEXEC); if (fd < 0) { err = fd; diff --git a/security/security.c b/security/security.c index 145e8082b9a6..83a2403b7261 100644 --- a/security/security.c +++ b/security/security.c @@ -5201,6 +5201,23 @@ int security_bpf_prog_load(struct bpf_prog *prog, union bpf_attr *attr, return call_int_hook(bpf_prog_load, 0, prog, attr, token); } +/** + * security_bpf_token_create() - Check if creating of BPF token is allowed + * @token BPF token object + * @attr: BPF syscall attributes used to create BPF token + * @path: path pointing to BPF FS mount point from which BPF token is created + * + * Do a check when the kernel instantiates a new BPF token object from BPF FS + * instance. This is also the point where LSM blob can be allocated for LSMs. + * + * Return: Returns 0 on success, error on failure. + */ +int security_bpf_token_create(struct bpf_token *token, union bpf_attr *attr, + struct path *path) +{ + return call_int_hook(bpf_token_create, 0, token, attr, path); +} + /** * security_bpf_map_free() - Free a bpf map's LSM blob * @map: bpf map @@ -5222,6 +5239,17 @@ void security_bpf_prog_free(struct bpf_prog *prog) { call_void_hook(bpf_prog_free, prog); } + +/** + * security_bpf_token_free() - Free a BPF token's LSM blob + * @token: BPF token struct + * + * Clean up the security information stored inside BPF token. + */ +void security_bpf_token_free(struct bpf_token *token) +{ + call_void_hook(bpf_token_free, token); +} #endif /* CONFIG_BPF_SYSCALL */ /**
Wire up bpf_token_create and bpf_token_free LSM hooks, which allow to allocate LSM security blob (we add `void *security` field to struct bpf_token for that), but also control who can instantiate BPF token. This follows existing pattern for BPF map and BPF prog. Signed-off-by: Andrii Nakryiko <andrii@kernel.org> --- include/linux/bpf.h | 3 +++ include/linux/lsm_hook_defs.h | 3 +++ include/linux/security.h | 11 +++++++++++ kernel/bpf/bpf_lsm.c | 2 ++ kernel/bpf/token.c | 6 ++++++ security/security.c | 28 ++++++++++++++++++++++++++++ 6 files changed, 53 insertions(+)