Hi Lorenz, Thanks for your nice summary. On Thu, Sep 22, 2022 at 9:29 AM Lorenz Bauer <oss@xxxxxx> wrote: > > On Thu, 15 Sep 2022, at 11:30, Lorenz Bauer wrote: > > Hi list, > > > > Here is a summary of the talk I gave at LPC '22 titled "Closing the BPF > > map permission loophole", with slides at [0]. [...] > > Problem #1: Read-only fds can be modified via a BPF program > > Up until 591fe9888d78 BPF_F_RDONLY_PROG we'd probably be OK with just refusing !rw fds. The only kernel.org version that this applies to is 4.19. 5.4, 5.10, 5.15 need the fix Roberto suggested. For this problem, I'm thinking of a fix purely in the verifier: passing along the file permission up to the site where the map is used. For maps that are not passed from outside, this permission is rw by default. For maps that are obtained from fdget(), we can do the following: - associate the env->used_maps with an array of permissions. - then extend bpf_reg_state and bpf_call_arg_meta to include the permission information. - then in record_func_map(), we can reject the program if the permission doesn't allow the map operation. Not the simplest solution, but this is the first solution that comes up in my head. > > Problem #2: Read-only fds can be "transmuted" into read-write fds via map in map > > This is not as big a problem as I initially thought. First, lookup in a nested map from userspace returns an ID, not a fd. Going from ID to fd requires CAP_SYS_ADMIN. On the program side, it's possible to retrieve the inner map and modify it. > > I think it's possible to fix this by refusing to insert !rw inner maps, by patching bpf_map_fd_get_ptr(). That function hasn't changed in a long time either. 4.19, 5.4, 5.10, 5.15. With the permission info associated with map_ptr in verifier that is established for program #1, a fix seems relatively easy. > > Problem #3: Read-only fds can be transmuted into read-write fds via > > object pinning > > > > 1. BPF_OBJ_PIN(read-only fd, /sys/fs/bpf/foo) > > 2. BPF_OBJ_GET(/sys/fs/bpf/foo) = read-write fd > > bpf_map doesn't have a concept of an owning user, so the only solution I can come up with is to refuse OBJ_PIN if !rw and !capable(CAP_DAC_OVERRIDE). Replace CAP_DAC_OVERRIDE with another capability of your choice, the idea being that this allows programs that run as root (probably a lot?) to continue functioning. For an idea mentioned in the summary, > In OBJ_GET, refuse a read-write fd if the fd passed to OBJ_PIN wasn't read-write. This sounds reasonable to me. Can we extend the object type referenced by inode to include the permission? > Again 4.19, 5.4, 5.10, 5.15. > > Daniel, Alexei, Andrii: any thoughts? This is a pretty deep rabbit hole, and I don't want to waste my time on the wrong approach. > > 0: https://github.com/willfindlay/bpfcontain-rs/blob/eb2cd826b609e165d63d784c0f562b7a278171d2/src/bpf/include/allocator.h#L16
