| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAMOXUJnFN_Fu07S_cGBLC3+k4jOtBGKH15fqZQP0JnTqZ6oJpg@mail.gmail.com>
Date: Mon, 16 Oct 2017 12:03:32 -0700
From: Chenbo Feng <fengc@...gle.com>
To: Stephen Smalley <sds@...ho.nsa.gov>,
David Miller <davem@...emloft.net>
Cc: Chenbo Feng <chenbofeng.kernel@...il.com>, netdev@...r.kernel.org,
Jeffrey Vander Stoep <jeffv@...gle.com>,
Alexei Starovoitov <alexei.starovoitov@...il.com>,
Lorenzo Colitti <lorenzo@...gle.com>,
Daniel Borkmann <daniel@...earbox.net>,
James Morris <james.l.morris@...cle.com>,
Paul Moore <paul@...l-moore.com>
Subject: Re: [PATCH net-next v5 5/5] selinux: bpf: Add addtional check for bpf
object file receive
On Mon, Oct 16, 2017 at 9:34 AM, Stephen Smalley <sds@...ho.nsa.gov> wrote:
> On Thu, 2017-10-12 at 13:55 -0700, Chenbo Feng wrote:
>> From: Chenbo Feng <fengc@...gle.com>
>>
>> Introduce a bpf object related check when sending and receiving files
>> through unix domain socket as well as binder. It checks if the
>> receiving
>> process have privilege to read/write the bpf map or use the bpf
>> program.
>> This check is necessary because the bpf maps and programs are using a
>> anonymous inode as their shared inode so the normal way of checking
>> the
>> files and sockets when passing between processes cannot work properly
>> on
>> eBPF object. This check only works when the BPF_SYSCALL is
>> configured.
>>
>> Signed-off-by: Chenbo Feng <fengc@...gle.com>
>> ---
>> include/linux/bpf.h | 3 +++
>> kernel/bpf/syscall.c | 4 ++--
>> security/selinux/hooks.c | 49
>> ++++++++++++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 54 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
>> index 225740688ab7..81d6c01b8825 100644
>> --- a/include/linux/bpf.h
>> +++ b/include/linux/bpf.h
>> @@ -285,6 +285,9 @@ int bpf_prog_array_copy_to_user(struct
>> bpf_prog_array __rcu *progs,
>> #ifdef CONFIG_BPF_SYSCALL
>> DECLARE_PER_CPU(int, bpf_prog_active);
>>
>> +extern const struct file_operations bpf_map_fops;
>> +extern const struct file_operations bpf_prog_fops;
>> +
>> #define BPF_PROG_TYPE(_id, _ops) \
>> extern const struct bpf_verifier_ops _ops;
>> #define BPF_MAP_TYPE(_id, _ops) \
>> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
>> index d3e152e282d8..8bdb98aa7f34 100644
>> --- a/kernel/bpf/syscall.c
>> +++ b/kernel/bpf/syscall.c
>> @@ -313,7 +313,7 @@ static ssize_t bpf_dummy_write(struct file *filp,
>> const char __user *buf,
>> return -EINVAL;
>> }
>>
>> -static const struct file_operations bpf_map_fops = {
>> +const struct file_operations bpf_map_fops = {
>> #ifdef CONFIG_PROC_FS
>> .show_fdinfo = bpf_map_show_fdinfo,
>> #endif
>> @@ -967,7 +967,7 @@ static void bpf_prog_show_fdinfo(struct seq_file
>> *m, struct file *filp)
>> }
>> #endif
>>
>> -static const struct file_operations bpf_prog_fops = {
>> +const struct file_operations bpf_prog_fops = {
>> #ifdef CONFIG_PROC_FS
>> .show_fdinfo = bpf_prog_show_fdinfo,
>> #endif
>> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
>> index 12cf7de8cbed..ef7e5c1de640 100644
>> --- a/security/selinux/hooks.c
>> +++ b/security/selinux/hooks.c
>> @@ -1815,6 +1815,10 @@ static inline int file_path_has_perm(const
>> struct cred *cred,
>> return inode_has_perm(cred, file_inode(file), av, &ad);
>> }
>>
>> +#ifdef CONFIG_BPF_SYSCALL
>> +static int bpf_fd_pass(struct file *file, u32 sid);
>> +#endif
>> +
>> /* Check whether a task can use an open file descriptor to
>> access an inode in a given way. Check access to the
>> descriptor itself, and then use dentry_has_perm to
>> @@ -1845,6 +1849,12 @@ static int file_has_perm(const struct cred
>> *cred,
>> goto out;
>> }
>>
>> +#ifdef CONFIG_BPF_SYSCALL
>> + rc = bpf_fd_pass(file, cred_sid(cred));
>> + if (rc)
>> + return rc;
>> +#endif
>> +
>> /* av is zero if only checking access to the descriptor. */
>> rc = 0;
>> if (av)
>> @@ -2165,6 +2175,12 @@ static int selinux_binder_transfer_file(struct
>> task_struct *from,
>> return rc;
>> }
>>
>> +#ifdef CONFIG_BPF_SYSCALL
>> + rc = bpf_fd_pass(file, sid);
>> + if (rc)
>> + return rc;
>> +#endif
>> +
>> if (unlikely(IS_PRIVATE(d_backing_inode(dentry))))
>> return 0;
>>
>> @@ -6288,6 +6304,39 @@ static u32 bpf_map_fmode_to_av(fmode_t fmode)
>> return av;
>> }
>>
>> +/* This function will check the file pass through unix socket or
>> binder to see
>> + * if it is a bpf related object. And apply correspinding checks on
>> the bpf
>> + * object based on the type. The bpf maps and programs, not like
>> other files and
>> + * socket, are using a shared anonymous inode inside the kernel as
>> their inode.
>> + * So checking that inode cannot identify if the process have
>> privilege to
>> + * access the bpf object and that's why we have to add this
>> additional check in
>> + * selinux_file_receive and selinux_binder_transfer_files.
>> + */
>> +static int bpf_fd_pass(struct file *file, u32 sid)
>> +{
>> + struct bpf_security_struct *bpfsec;
>> + struct bpf_prog *prog;
>> + struct bpf_map *map;
>> + int ret;
>> +
>> + if (file->f_op == &bpf_map_fops) {
>> + map = file->private_data;
>> + bpfsec = map->security;
>> + ret = avc_has_perm(sid, bpfsec->sid,
>> SECCLASS_BPF_MAP,
>> + bpf_map_fmode_to_av(file-
>> >f_mode), NULL);
>> + if (ret)
>> + return ret;
>> + } else if (file->f_op == &bpf_prog_fops) {
>> + prog = file->private_data;
>> + bpfsec = prog->aux->security;
>> + ret = avc_has_perm(sid, bpfsec->sid,
>> SECCLASS_BPF_PROG,
>> + BPF__PROG_RUN, NULL);
>> + if (ret)
>> + return ret;
>> + }
>> + return 0;
>> +}
>
> My apologies, I should have caught this earlier, but you didn't update
> the class/permission symbols used above when you re-spun patch 4/5 with
> a single bpf class with distinct permissions defined.
>
> Please, ensure that the entire series compiles and works before
> sending.
>
> Also, you likely should be cc'ing linux-security-module mailing list
> and selinux mailing list; I thought you were doing so on earlier
> versions
>
Sorry, forget to run format-patch again after I fix these errors. New
patch submitted and added back linux-security-module mailing list and
selinux mailing list to the recipient.
>> +
>> static int selinux_bpf_map(struct bpf_map *map, fmode_t fmode)
>> {
>> u32 sid = current_sid();
Powered by blists - more mailing lists