netdev - Re: [PATCH 1/7] bpf: Add missing fd modes check for map iterators

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
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAADnVQL7murY8G4SG5sF3855ETBpmrv0S-dueR8Rht4ucm6WwQ@mail.gmail.com>
Date:   Thu, 8 Sep 2022 08:17:19 -0700
From:   Alexei Starovoitov <alexei.starovoitov@...il.com>
To:     Roberto Sassu <roberto.sassu@...weicloud.com>
Cc:     Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Andrii Nakryiko <andrii@...nel.org>,
        Martin KaFai Lau <martin.lau@...ux.dev>,
        Song Liu <song@...nel.org>, Yonghong Song <yhs@...com>,
        John Fastabend <john.fastabend@...il.com>,
        KP Singh <kpsingh@...nel.org>,
        Stanislav Fomichev <sdf@...gle.com>,
        Hao Luo <haoluo@...gle.com>, Jiri Olsa <jolsa@...nel.org>,
        Mykola Lysenko <mykolal@...com>, Shuah Khan <shuah@...nel.org>,
        "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Jakub Sitnicki <jakub@...udflare.com>,
        bpf <bpf@...r.kernel.org>,
        Network Development <netdev@...r.kernel.org>,
        "open list:KERNEL SELFTEST FRAMEWORK" 
        <linux-kselftest@...r.kernel.org>,
        "open list:CONTROL GROUP (CGROUP)" <cgroups@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Hou Tao <houtao1@...wei.com>,
        Roberto Sassu <roberto.sassu@...wei.com>,
        stable <stable@...r.kernel.org>, Chenbo Feng <fengc@...gle.com>,
        LSM List <linux-security-module@...r.kernel.org>
Subject: Re: [PATCH 1/7] bpf: Add missing fd modes check for map iterators

On Thu, Sep 8, 2022 at 6:59 AM Roberto Sassu
<roberto.sassu@...weicloud.com> wrote:
>
> On Wed, 2022-09-07 at 09:02 -0700, Alexei Starovoitov wrote:
> >
>
> [...]
>
> > > Well, if you write a security module to prevent writes on a map,
> > > and
> > > user space is able to do it anyway with an iterator, what is the
> > > purpose of the security module then?
> >
> > sounds like a broken "security module" and nothing else.
>
> Ok, if a custom security module does not convince you, let me make a
> small example with SELinux.
>
> I created a small map iterator that sets every value of a map to 5:
>
> SEC("iter/bpf_map_elem")
> int write_bpf_hash_map(struct bpf_iter__bpf_map_elem *ctx)
> {
>         u32 *key = ctx->key;
>         u8 *val = ctx->value;
>
>         if (key == NULL || val == NULL)
>                 return 0;
>
>         *val = 5;
>         return 0;
> }
>
> I create and pin a map:
>
> # bpftool map create /sys/fs/bpf/map type array key 4 value 1 entries 1
> name test
>
> Initially, the content of the map looks like:
>
> # bpftool map dump pinned /sys/fs/bpf/map
> key: 00 00 00 00  value: 00
> Found 1 element
>
> I then created a new SELinux type bpftool_test_t, which has only read
> permission on maps:
>
> # sesearch -A -s bpftool_test_t -t unconfined_t -c bpf
> allow bpftool_test_t unconfined_t:bpf map_read;
>
> So, what I expect is that this type is not able to write to the map.
>
> Indeed, the current bpftool is not able to do it:
>
> # strace -f -etrace=bpf runcon -t bpftool_test_t bpftool iter pin
> writer.o /sys/fs/bpf/iter map pinned /sys/fs/bpf/map
> bpf(BPF_OBJ_GET, {pathname="/sys/fs/bpf/map", bpf_fd=0, file_flags=0},
> 144) = -1 EACCES (Permission denied)
> Error: bpf obj get (/sys/fs/bpf): Permission denied
>
> This happens because the current bpftool requests to access the map
> with read-write permission, and SELinux denies it:
>
> # cat /var/log/audit/audit.log|audit2allow
>
>
> #============= bpftool_test_t ==============
> allow bpftool_test_t unconfined_t:bpf map_write;
>
>
> The command failed, and the content of the map is still:
>
> # bpftool map dump pinned /sys/fs/bpf/map
> key: 00 00 00 00  value: 00
> Found 1 element
>
>
> Now, what I will do is to use a slightly modified version of bpftool
> which requests read-only access to the map instead:
>
> # strace -f -etrace=bpf runcon -t bpftool_test_t ./bpftool iter pin
> writer.o /sys/fs/bpf/iter map pinned /sys/fs/bpf/map
> bpf(BPF_OBJ_GET, {pathname="/sys/fs/bpf/map", bpf_fd=0,
> file_flags=BPF_F_RDONLY}, 16) = 3
> libbpf: elf: skipping unrecognized data section(5) .eh_frame
> libbpf: elf: skipping relo section(6) .rel.eh_frame for section(5)
> .eh_frame
>
> ...
>
> bpf(BPF_LINK_CREATE, {link_create={prog_fd=4, target_fd=0,
> attach_type=BPF_TRACE_ITER, flags=0}, ...}, 48) = 5
> bpf(BPF_OBJ_PIN, {pathname="/sys/fs/bpf/iter", bpf_fd=5, file_flags=0},
> 16) = 0
>
> That worked, because SELinux grants read-only permission to
> bpftool_test_t. However, the map iterator does not check how the fd was
> obtained, and thus allows the iterator to be created.
>
> At this point, we have write access, despite not having the right to do
> it:

That is a wrong assumption to begin with.
Having an fd to a bpf object (map, link, prog) allows access.
read/write sort-of applicable to maps, but not so much
to progs, links.
That file based read/write flag is only for user processes.
bpf progs always had separate flags for that.
See BPF_F_RDONLY vs BPF_F_RDONLY_PROG.
One doesn't imply the other.