bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Andrii Nakryiko <andrii.nakryiko@gmail.com>
To: Andy Lutomirski <luto@kernel.org>
Cc: Daniel Borkmann <daniel@iogearbox.net>,
	Maryam Tahhan <mtahhan@redhat.com>,
	 Andrii Nakryiko <andrii@kernel.org>,
	bpf@vger.kernel.org,  linux-security-module@vger.kernel.org,
	Kees Cook <keescook@chromium.org>,
	 Christian Brauner <brauner@kernel.org>,
	lennart@poettering.net, cyphar@cyphar.com,  kernel-team@meta.com
Subject: Re: [PATCH v2 bpf-next 00/18] BPF token
Date: Mon, 26 Jun 2023 15:31:30 -0700	[thread overview]
Message-ID: <CAEf4BzY5UWLCjDiQ_pfCeKMVJScdk7B4ZaKwi=yaf8ACnaOXLg@mail.gmail.com> (raw)
In-Reply-To: <fe47aeb6-dae8-43a6-bcb0-ada2ebf62e08@app.fastmail.com>

On Sat, Jun 24, 2023 at 7:00 AM Andy Lutomirski <luto@kernel.org> wrote:
>
>
>
> On Fri, Jun 23, 2023, at 4:23 PM, Daniel Borkmann wrote:
> > On 6/23/23 5:10 PM, Andy Lutomirski wrote:
> >> On Thu, Jun 22, 2023, at 6:02 PM, Andy Lutomirski wrote:
> >>> On Thu, Jun 22, 2023, at 11:40 AM, Andrii Nakryiko wrote:
> >>>
> >>>> Hopefully you can see where I'm going with this. And this is just one
> >>>> random tiny example. We can think up tons of other cases to prove BPF
> >>>> is not isolatable to any sort of "container".
> >>>
> >>> No.  You have not come up with an example of why BPF is not isolatable
> >>> to a container.  You have come up with an example of why binding to a
> >>> sched_switch raw tracepoint does not make sense in a container without
> >>> additional mechanisms to give it well defined functionality and
> >>> appropriate security.
> >
> > One big blocker for the case of BPF is not isolatable to a container are
> > CPU hardware bugs. There has been plenty of mitigation effort so that the
> > flexibility cannot be abused as a tool e.g. discussed in [0], but ultimately
> > it's a cat and mouse game and vendors are also not really transparent. So
> > actual reasonable discussion can be resumed once CPU vendors gets their
> > stuff fixed.
> >
> >    [0]
> > https://popl22.sigplan.org/details/prisc-2022-papers/11/BPF-and-Spectre-Mitigating-transient-execution-attacks
> >
>
> By this standard, shouldn’t we just give up?  Let everyone map /dev/mem readonly and stop pretending we can implement any form of access control.
>
> Of course, we don’t do this. We try pretty hard to squash bugs and keep programs from doing an end run around OS security.
>
> >> Thinking about this some more:
> >>
> >> Suppose the goal is to allow a workload in a container to monitor itself by attaching to a tracepoint (something in the scheduler, for example).  The workload is in the container.  The tracepoint is global.  Kernel memory is global unless something that is trusted and understands the containers is doing the reading.  And proxying BPF is a mess.
> >
> > Agree that proxy is a mess for various reasons stated earlier.
> >
> >> So here are a couple of possible solutions:
> >>
> >> (a) Improve BPF maps a bit so that BPF maps work well in containers.  It should be possible to create a map and share it (the file descriptor!) between the outside and the container without running into various snags.  (IIRC my patch series was a decent step in this direction,)  Now load the BPF program and attach it to the tracepoint outside the container but have it write its gathered data to the map that's in the container.  So you end up with a daemon outside the container that gets a request like "help me monitor such-and-such by running BPF program such-and-such (where the BPF program code presumably comes from a library outside the container", and the daemon arranges for the requesting container to have access to the map it needs to get the data.
> >
> > I don't think it's very practical, meaning the vast majority of applications
> > out there today are tightly coupled BPF code + user space application, and in
> > a lot of cases programs are dynamically created. This would require somehow
> > splitting up parts of your application to run outside the container in hostns
> > and other parts inside the container.. for the sake of the mentioned example
> > it's something fairly static, but real-world applications look different and
> > are much more complex.
> >
>
> It sounds like you are describing a situation where there is a workload in a container, where the *entire container* is part of the TCB, but the part of the workload that has the explicit right to read all of kernel memory (e.g. bpf_probe_read_kernel) is so tightly coupled to the container that no one outside the container wants to audit it.
>
> And yet someone still wants to run it in a userns.
>

Yes, to get all the other benefits of userns. Yes, BPF isolation
cannot be enforced and we rely on a human-driven process to decide
whether it's ok to run BPF inside each specific container. But why
can't we also get all the other benefits of userns outside of BPF
usage.

BPF parts are critical for such applications, but they also normally
have a huge user-space part, and use large common libraries, so there
is a lot of benefit to having as much userns-provided isolation as
possible.


> This is IMO a rather bizarre situation.
>
> If I were operating a large fleet, and I had teams developing software to run in a container, I would not want to grant those containers this right without strict controls, and I don’t mean on/off controls. I would want strict auditing of *what exact BPF code* (including source) was run, and why, and who wrote it, and what the intended results are, and what limits access to the results, etc.  After all, we’re talking about the right, BY DESIGN, to access PII, payment card information, medical information, information protected by any jurisdiction’s data control rights, etc. Literally everything.  This ability, as described, isn’t “the right to use BPF.”  It is the right to *read all secrets*, intentionally.  (And modify them, with bpf_probe_write_user, possibly subject to some constraints.)

What makes you think this is not how it's actually done in practice
already (except right now we don't have BPF token, so it's
all-or-nothin, userns or not, root or not, which is overall worse than
what we'll get with BPF token + userns)?

Audit, code review, proper development practices. Then discussions and
reviews between team running container manager and team with BPF-based
workload to make decisions whether it's safe to allow BPF access (and
to what degree) and how teams will maintain privacy and safety
obligations.


>
>
> If this series was about passing a “may load kernel modules” token around, I think it would get an extremely chilly reception, even though we have module signatures.  I don’t see anything about BPF that makes BPF tokens more reasonable unless a real security model is developed first.

If we had dozens of teams developing and loading/unloading their
custom kernel modules all the time, it might not have sounded so
ridiculous?

>
> >> (b) Make a way to pass a pre-approved program into a container.  So a daemon outside loads the program and does some new magic to say "make an fd that can beused to attach this particular program to this particular tracepoint" and pass that into the container.
> >
> > Same as above. Programs are in most cases very tightly coupled to the
> > application
> > itself. I'm not sure if the ask is to redesign/implement all the
> > existing user
> > space infra.
> >
> >> I think (a) is better.  In particular, if you have a workload with many containers, and they all want to monitor the same tracepoint as it relates to their container, you will get much better performance if a single BPF program does the monitoring and sends the data out to each container as needed instead of having one copy of the program per container.
> >>
> >> For what it's worth, BPF tokens seem like they'll have the same performance problem -- without coordination, you can end up with N containers generating N hooks all targeting the same global resource, resulting in overhead that scales linearly with the number of containers.
> >
> > Worst case, sure, but it's not the point. These containers which would
> > receive
> > the tokens are part of your trusted compute base.. so its up to the
> > specific
> > applications and their surrounding infrastructure with regards to what
> > problem
> > they solve where and approved by operators/platform engs to deploy in
> > your cluster.
> > I don't particularly see that there's a performance problem. Andrii
> > specifically
> > mentioned /trusted unprivileged applications/.

Yep, performance is not why this is being done.

> >
> >> And, again, I'm not an XDP expert, but if you have one NIC, and you attach N XDP programs to it, and each one is inspecting packets and sending some to one particular container's AF_XDP socket, you are not going to get good performance.  You want *one* XDP program fanning the packets out to the relevant containers.
> >>
> >> If this is hard right now, perhaps you could add new kernel mechanisms as needed to improve the situation.
> >>
> >> --Andy
> >>

  parent reply	other threads:[~2023-06-26 22:31 UTC|newest]

Thread overview: 72+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-07 23:53 [PATCH v2 bpf-next 00/18] BPF token Andrii Nakryiko
2023-06-07 23:53 ` [PATCH v2 bpf-next 01/18] bpf: introduce BPF token object Andrii Nakryiko
2023-06-07 23:53 ` [PATCH v2 bpf-next 02/18] libbpf: add bpf_token_create() API Andrii Nakryiko
2023-06-07 23:53 ` [PATCH v2 bpf-next 03/18] selftests/bpf: add BPF_TOKEN_CREATE test Andrii Nakryiko
2023-06-07 23:53 ` [PATCH v2 bpf-next 04/18] bpf: move unprivileged checks into map_create() and bpf_prog_load() Andrii Nakryiko
2023-06-07 23:53 ` [PATCH v2 bpf-next 05/18] bpf: inline map creation logic in map_create() function Andrii Nakryiko
2023-06-07 23:53 ` [PATCH v2 bpf-next 06/18] bpf: centralize permissions checks for all BPF map types Andrii Nakryiko
2023-06-07 23:53 ` [PATCH v2 bpf-next 07/18] bpf: add BPF token support to BPF_MAP_CREATE command Andrii Nakryiko
2023-06-07 23:53 ` [PATCH v2 bpf-next 08/18] libbpf: add BPF token support to bpf_map_create() API Andrii Nakryiko
2023-06-07 23:53 ` [PATCH v2 bpf-next 09/18] selftests/bpf: add BPF token-enabled test for BPF_MAP_CREATE command Andrii Nakryiko
2023-06-07 23:53 ` [PATCH v2 bpf-next 10/18] bpf: add BPF token support to BPF_BTF_LOAD command Andrii Nakryiko
2023-06-07 23:53 ` [PATCH v2 bpf-next 11/18] libbpf: add BPF token support to bpf_btf_load() API Andrii Nakryiko
2023-06-07 23:53 ` [PATCH v2 bpf-next 12/18] selftests/bpf: add BPF token-enabled BPF_BTF_LOAD selftest Andrii Nakryiko
2023-06-07 23:53 ` [PATCH v2 bpf-next 13/18] bpf: keep BPF_PROG_LOAD permission checks clear of validations Andrii Nakryiko
2023-06-07 23:53 ` [PATCH v2 bpf-next 14/18] bpf: add BPF token support to BPF_PROG_LOAD command Andrii Nakryiko
2023-06-07 23:53 ` [PATCH v2 bpf-next 15/18] bpf: take into account BPF token when fetching helper protos Andrii Nakryiko
2023-06-07 23:53 ` [PATCH v2 bpf-next 16/18] bpf: consistenly use BPF token throughout BPF verifier logic Andrii Nakryiko
2023-06-07 23:53 ` [PATCH v2 bpf-next 17/18] libbpf: add BPF token support to bpf_prog_load() API Andrii Nakryiko
2023-06-07 23:53 ` [PATCH v2 bpf-next 18/18] selftests/bpf: add BPF token-enabled BPF_PROG_LOAD tests Andrii Nakryiko
2023-06-08 18:49 ` [PATCH v2 bpf-next 00/18] BPF token Stanislav Fomichev
2023-06-08 22:17   ` Andrii Nakryiko
2023-06-09 11:17 ` Toke Høiland-Jørgensen
2023-06-09 18:21   ` Andrii Nakryiko
2023-06-09 21:21     ` Toke Høiland-Jørgensen
2023-06-09 22:03       ` Andrii Nakryiko
2023-06-12 10:49         ` Toke Høiland-Jørgensen
2023-06-12 22:08           ` Andrii Nakryiko
2023-06-13 21:48             ` Hao Luo
2023-06-14 12:06             ` Toke Høiland-Jørgensen
2023-06-15 22:55               ` Andrii Nakryiko
2023-06-09 18:32 ` Andy Lutomirski
2023-06-09 19:08   ` Andrii Nakryiko
2023-06-19 17:40     ` Andy Lutomirski
2023-06-21 23:48       ` Andrii Nakryiko
2023-06-22  8:22         ` Maryam Tahhan
2023-06-22 16:49           ` Andy Lutomirski
     [not found]             ` <5a75d1f0-4ed9-399c-4851-2df0755de9b5@redhat.com>
2023-06-22 18:40               ` Andrii Nakryiko
2023-06-22 21:04                 ` Maryam Tahhan
2023-06-22 23:35                   ` Andrii Nakryiko
2023-06-23  1:02                 ` Andy Lutomirski
2023-06-23 15:10                   ` Andy Lutomirski
2023-06-23 23:23                     ` Daniel Borkmann
2023-06-24 13:59                       ` Andy Lutomirski
2023-06-24 15:28                         ` Andy Lutomirski
2023-06-26 15:23                           ` Daniel Borkmann
2023-07-04 20:48                             ` Andy Lutomirski
2023-07-04 21:06                               ` Andy Lutomirski
2023-06-27 10:22                           ` Djalal Harouni
2023-06-26 22:31                         ` Andrii Nakryiko [this message]
2023-06-26 22:08                   ` Andrii Nakryiko
2023-06-22 19:05             ` Andrii Nakryiko
2023-06-23  3:28               ` Andy Lutomirski
2023-06-23 16:13                 ` Casey Schaufler
2023-06-26 22:08                 ` Andrii Nakryiko
2023-06-22 18:20           ` Andrii Nakryiko
2023-06-23 23:07             ` Toke Høiland-Jørgensen
2023-06-26 22:08               ` Andrii Nakryiko
2023-07-04 21:05                 ` Andy Lutomirski
2023-06-09 22:29 ` Djalal Harouni
2023-06-09 22:57   ` Andrii Nakryiko
2023-06-12 12:02     ` Djalal Harouni
2023-06-12 14:31       ` Djalal Harouni
2023-06-12 22:27       ` Andrii Nakryiko
2023-06-14  0:23         ` Djalal Harouni
2023-06-14  9:39           ` Christian Brauner
2023-06-15 22:48             ` Andrii Nakryiko
2023-06-23 22:18               ` Daniel Borkmann
2023-06-26 22:08                 ` Andrii Nakryiko
2023-06-15 22:47           ` Andrii Nakryiko
2023-06-12 12:44 ` Dave Tucker
2023-06-12 15:52   ` Djalal Harouni
2023-06-12 23:04   ` Andrii Nakryiko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAEf4BzY5UWLCjDiQ_pfCeKMVJScdk7B4ZaKwi=yaf8ACnaOXLg@mail.gmail.com' \
    --to=andrii.nakryiko@gmail.com \
    --cc=andrii@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=brauner@kernel.org \
    --cc=cyphar@cyphar.com \
    --cc=daniel@iogearbox.net \
    --cc=keescook@chromium.org \
    --cc=kernel-team@meta.com \
    --cc=lennart@poettering.net \
    --cc=linux-security-module@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=mtahhan@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).