[PATCH net-next v2 4/5] selinux: bpf: Add selinux check for eBPF syscall operations

[Stephen Smalley]

On Mon, 2017-10-09 at 15:20 -0700, Chenbo Feng wrote:
> From: Chenbo Feng <fengc at google.com>
> 
> Implement the actual checks introduced to eBPF related syscalls. This
> implementation use the security field inside bpf object to store a
> sid that
> identify the bpf object. And when processes try to access the object,
> selinux will check if processes have the right privileges. The
> creation
> of eBPF object are also checked at the general bpf check hook and new
> cmd introduced to eBPF domain can also be checked there.
> 
> Signed-off-by: Chenbo Feng <fengc at google.com>
> Acked-by: Alexei Starovoitov <ast at kernel.org>
> ---
>  security/selinux/hooks.c            | 111
> ++++++++++++++++++++++++++++++++++++
>  security/selinux/include/classmap.h |   2 +
>  security/selinux/include/objsec.h   |   4 ++
>  3 files changed, 117 insertions(+)
> 
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index f5d304736852..41aba4e3d57c 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -85,6 +85,7 @@
>  #include <linux/export.h>
>  #include <linux/msg.h>
>  #include <linux/shm.h>
> +#include <linux/bpf.h>
>  
>  #include "avc.h"
>  #include "objsec.h"
> @@ -6252,6 +6253,106 @@ static void selinux_ib_free_security(void
> *ib_sec)
>  }
>  #endif
>  
> +#ifdef CONFIG_BPF_SYSCALL
> +static int selinux_bpf(int cmd, union bpf_attr *attr,
> +				     unsigned int size)
> +{
> +	u32 sid = current_sid();
> +	int ret;
> +
> +	switch (cmd) {
> +	case BPF_MAP_CREATE:
> +		ret = avc_has_perm(sid, sid, SECCLASS_BPF_MAP,
> BPF_MAP__CREATE,
> +				   NULL);
> +		break;
> +	case BPF_PROG_LOAD:
> +		ret = avc_has_perm(sid, sid, SECCLASS_BPF_PROG,
> BPF_PROG__LOAD,
> +				   NULL);
> +		break;
> +	default:
> +		ret = 0;
> +		break;
> +	}
> +
> +	return ret;
> +}
> +
> +static u32 bpf_map_fmode_to_av(fmode_t fmode)
> +{
> +	u32 av = 0;
> +
> +	if (f_mode & FMODE_READ)
> +		av |= BPF_MAP__READ;
> +	if (f_mode & FMODE_WRITE)
> +		av |= BPF_MAP__WRITE;
> +	return av;
> +}
> +
> +static int selinux_bpf_map(struct bpf_map *map, fmode_t fmode)
> +{
> +	u32 sid = current_sid();
> +	struct bpf_security_struct *bpfsec;
> +
> +	bpfsec = map->security;
> +	return avc_has_perm(sid, bpfsec->sid, SECCLASS_BPF_MAP,
> +			    bpf_map_fmode_to_av(fmode), NULL);
> +}
> +
> +static int selinux_bpf_prog(struct bpf_prog *prog)
> +{
> +	u32 sid = current_sid();
> +	struct bpf_security_struct *bpfsec;
> +
> +	bpfsec = prog->aux->security;
> +	return avc_has_perm(sid, bpfsec->sid, SECCLASS_BPF_PROG,
> +			    BPF_PROG__USE, NULL);
> +}
> +
> +static int selinux_bpf_map_alloc(struct bpf_map *map)
> +{
> +	struct bpf_security_struct *bpfsec;
> +
> +	bpfsec = kzalloc(sizeof(*bpfsec), GFP_KERNEL);
> +	if (!bpfsec)
> +		return -ENOMEM;
> +
> +	bpfsec->sid = current_sid();
> +	map->security = bpfsec;
> +
> +	return 0;
> +}
> +
> +static void selinux_bpf_map_free(struct bpf_map *map)
> +{
> +	struct bpf_security_struct *bpfsec = map->security;
> +
> +	map->security = NULL;
> +	kfree(bpfsec);
> +}
> +
> +static int selinux_bpf_prog_alloc(struct bpf_prog_aux *aux)
> +{
> +	struct bpf_security_struct *bpfsec;
> +
> +	bpfsec = kzalloc(sizeof(*bpfsec), GFP_KERNEL);
> +	if (!bpfsec)
> +		return -ENOMEM;
> +
> +	bpfsec->sid = current_sid();
> +	aux->security = bpfsec;
> +
> +	return 0;
> +}
> +
> +static void selinux_bpf_prog_free(struct bpf_prog_aux *aux)
> +{
> +	struct bpf_security_struct *bpfsec = aux->security;
> +
> +	aux->security = NULL;
> +	kfree(bpfsec);
> +}
> +#endif
> +
>  static struct security_hook_list selinux_hooks[] __lsm_ro_after_init
> = {
>  	LSM_HOOK_INIT(binder_set_context_mgr,
> selinux_binder_set_context_mgr),
>  	LSM_HOOK_INIT(binder_transaction,
> selinux_binder_transaction),
> @@ -6471,6 +6572,16 @@ static struct security_hook_list
> selinux_hooks[] __lsm_ro_after_init = {
>  	LSM_HOOK_INIT(audit_rule_match, selinux_audit_rule_match),
>  	LSM_HOOK_INIT(audit_rule_free, selinux_audit_rule_free),
>  #endif
> +
> +#ifdef CONFIG_BPF_SYSCALL
> +	LSM_HOOK_INIT(bpf, selinux_bpf),
> +	LSM_HOOK_INIT(bpf_map, selinux_bpf_map),
> +	LSM_HOOK_INIT(bpf_prog, selinux_bpf_prog),
> +	LSM_HOOK_INIT(bpf_map_alloc_security,
> selinux_bpf_map_alloc),
> +	LSM_HOOK_INIT(bpf_prog_alloc_security,
> selinux_bpf_prog_alloc),
> +	LSM_HOOK_INIT(bpf_map_free_security, selinux_bpf_map_free),
> +	LSM_HOOK_INIT(bpf_prog_free_security,
> selinux_bpf_prog_free),
> +#endif
>  };
>  
>  static __init int selinux_init(void)
> diff --git a/security/selinux/include/classmap.h
> b/security/selinux/include/classmap.h
> index 35ffb29a69cb..7253c5eea59c 100644
> --- a/security/selinux/include/classmap.h
> +++ b/security/selinux/include/classmap.h
> @@ -237,6 +237,8 @@ struct security_class_mapping secclass_map[] = {
>  	  { "access", NULL } },
>  	{ "infiniband_endport",
>  	  { "manage_subnet", NULL } },
> +	{ "bpf_map", {"create", "read", "write"} },
> +	{ "bpf_prog", {"load", "use"} },

Again I have to ask: do you truly need/want two separate classes, or
would a single class with distinct permissions suffice, ala:
        { "bpf", { "create_map", "read_map", "write_map", "prog_load",
"prog_use" } },

and then allow A self:bpf { create_map read_map write_map prog_load
prog_use }; would be stored in a single policy avtab rule, and be
cached in a single AVC entry.

>  	{ NULL }
>    };
>  
> diff --git a/security/selinux/include/objsec.h
> b/security/selinux/include/objsec.h
> index 1649cd18eb0b..3d54468ce334 100644
> --- a/security/selinux/include/objsec.h
> +++ b/security/selinux/include/objsec.h
> @@ -150,6 +150,10 @@ struct pkey_security_struct {
>  	u32	sid;	/* SID of pkey */
>  };
>  
> +struct bpf_security_struct {
> +	u32 sid;  /*SID of bpf obj creater*/
> +};
> +
>  extern unsigned int selinux_checkreqprot;
>  
>  #endif /* _SELINUX_OBJSEC_H_ */
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html