| 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
| ||
|
Date: Thu, 9 Jun 2022 12:55:15 +0000
From: Roberto Sassu <roberto.sassu@...wei.com>
To: Andrii Nakryiko <andrii.nakryiko@...il.com>
CC: Alexei Starovoitov <ast@...nel.org>,
Daniel Borkmann <daniel@...earbox.net>,
Andrii Nakryiko <andrii@...nel.org>,
KP Singh <kpsingh@...nel.org>, bpf <bpf@...r.kernel.org>,
Networking <netdev@...r.kernel.org>,
"open list:KERNEL SELFTEST FRAMEWORK"
<linux-kselftest@...r.kernel.org>,
open list <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH v2 0/9] bpf: Per-operation map permissions
> From: Andrii Nakryiko [mailto:andrii.nakryiko@...il.com]
> Sent: Friday, June 3, 2022 11:00 PM
> On Thu, Jun 2, 2022 at 7:38 AM Roberto Sassu <roberto.sassu@...wei.com>
> wrote:
> >
> > With the bpf_map security hook, an eBPF program is able to restrict access
> > to a map. For example, it might allow only read accesses and deny write
> > accesses.
> >
> > Unfortunately, permissions are not accurately specified by libbpf and
> > bpftool. As a consequence, even if they are requested to perform a
> > read-like operation, such as a map lookup, that operation fails even if the
> > caller has the right to do so.
> >
> > Even worse, the iteration over existing maps stops as soon as a
> > write-protected one is encountered. Maps after the write-protected one are
> > not accessible, even if the user has the right to perform operations on
> > them.
> >
> > At low level, the problem is that open_flags and file_flags, respectively
> > in the bpf_map_get_fd_by_id() and bpf_obj_get(), are set to zero. The
> > kernel interprets this as a request to obtain a file descriptor with full
> > permissions.
> >
> > For some operations, like show or dump, a read file descriptor is enough.
> > Those operations could be still performed even in a write-protected map.
> >
> > Also for searching a map by name, which requires getting the map info, a
> > read file descriptor is enough. If an operation requires more permissions,
> > they could still be requested later, after the search.
> >
> > First, solve both problems by extending libbpf with two new functions,
> > bpf_map_get_fd_by_id_flags() and bpf_obj_get_flags(), which unlike their
> > counterparts bpf_map_get_fd_by_id() and bpf_obj_get(), have the additional
> > parameter flags to specify the needed permissions for an operation.
> >
> > Then, propagate the flags in bpftool from the functions implementing the
> > subcommands down to the functions calling bpf_map_get_fd_by_id() and
> > bpf_obj_get(), and replace the latter functions with their new variant.
> > Initially, set the flags to zero, so that the current behavior does not
> > change.
> >
> > The only exception is for map search by name, where a read-only permission
> > is requested, regardless of the operation, to get the map info. In this
> > case, request a new file descriptor if a write-like operation needs to be
> > performed after the search.
> >
> > Finally, identify other read-like operations in bpftool and for those
> > replace the zero value for flags with BPF_F_RDONLY.
> >
> > The patch set is organized as follows.
> >
> > Patches 1-2 introduce the two new variants of bpf_map_get_fd_by_id() and
> > bpf_obj_get() in libbpf, named respectively bpf_map_get_fd_by_id_flags()
> > and bpf_obj_get_flags().
> >
> > Patches 3-7 propagate the flags in bpftool from the functions implementing
> > the subcommands to the two new libbpf functions, and always set flags to
> > BPF_F_RDONLY for the map search operation.
> >
> > Patch 8 adjusts permissions depending on the map operation performed.
> >
> > Patch 9 ensures that read-only accesses to a write-protected map succeed
> > and write accesses still fail. Also ensure that map search is always
> > successful even if there are write-protected maps.
> >
> > Changelog
> >
> > v1:
> > - Define per-operation permissions rather than retrying access with
> > read-only permission (suggested by Daniel)
> > https://lore.kernel.org/bpf/20220530084514.10170-1-
> roberto.sassu@...wei.com/
> >
> > Roberto Sassu (9):
> > libbpf: Introduce bpf_map_get_fd_by_id_flags()
> > libbpf: Introduce bpf_obj_get_flags()
> > bpftool: Add flags parameter to open_obj_pinned_any() and
> > open_obj_pinned()
> > bpftool: Add flags parameter to *_parse_fd() functions
> > bpftool: Add flags parameter to map_parse_fds()
> > bpftool: Add flags parameter to map_parse_fd_and_info()
> > bpftool: Add flags parameter in struct_ops functions
> > bpftool: Adjust map permissions
> > selftests/bpf: Add map access tests
> >
> > tools/bpf/bpftool/btf.c | 11 +-
> > tools/bpf/bpftool/cgroup.c | 4 +-
> > tools/bpf/bpftool/common.c | 52 ++--
> > tools/bpf/bpftool/iter.c | 2 +-
> > tools/bpf/bpftool/link.c | 9 +-
> > tools/bpf/bpftool/main.h | 17 +-
> > tools/bpf/bpftool/map.c | 24 +-
> > tools/bpf/bpftool/map_perf_ring.c | 3 +-
> > tools/bpf/bpftool/net.c | 2 +-
> > tools/bpf/bpftool/prog.c | 12 +-
> > tools/bpf/bpftool/struct_ops.c | 39 ++-
> > tools/lib/bpf/bpf.c | 16 +-
> > tools/lib/bpf/bpf.h | 2 +
> > tools/lib/bpf/libbpf.map | 2 +
> > .../bpf/prog_tests/test_map_check_access.c | 264 ++++++++++++++++++
> > .../selftests/bpf/progs/map_check_access.c | 65 +++++
> > 16 files changed, 452 insertions(+), 72 deletions(-)
> > create mode 100644
> tools/testing/selftests/bpf/prog_tests/test_map_check_access.c
> > create mode 100644 tools/testing/selftests/bpf/progs/map_check_access.c
> >
> > --
> > 2.25.1
> >
>
> Also check CI failures ([0]).
Thanks for the review Andrii. Will send a new version of the
patch set soon.
Roberto
HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Li Peng, Yang Xi, Li He
> test_test_map_check_access:PASS:skel 0 nsec
> test_test_map_check_access:PASS:skel 0 nsec
> test_test_map_check_access:PASS:bpf_object__find_map_by_name 0 nsec
> test_test_map_check_access:PASS:bpf_obj_get_info_by_fd 0 nsec
> test_test_map_check_access:PASS:bpf_map_get_fd_by_id 0 nsec
> test_test_map_check_access:PASS:bpf_map_get_fd_by_id_flags 0 nsec
> test_test_map_check_access:PASS:bpf_map_get_fd_by_id_flags 0 nsec
> test_test_map_check_access:PASS:bpf_map_lookup_elem 0 nsec
> test_test_map_check_access:PASS:bpf_map_update_elem 0 nsec
> test_test_map_check_access:PASS:bpf_map_update_elem 0 nsec
> test_test_map_check_access:PASS:bpf_map__pin 0 nsec
> test_test_map_check_access:PASS:bpf_obj_get_flags 0 nsec
> test_test_map_check_access:PASS:bpf_obj_get_flags 0 nsec
> test_test_map_check_access:FAIL:bpftool map list - error: 127
> #189 test_map_check_access:FAIL
>
> [0] https://github.com/kernel-
> patches/bpf/runs/6730796689?check_suite_focus=true
Powered by blists - more mailing lists