linux-security-module.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Andy Lutomirski <luto@kernel.org>
To: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Cc: Andy Lutomirski <luto@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Song Liu <songliubraving@fb.com>,
	Kees Cook <keescook@chromium.org>,
	Networking <netdev@vger.kernel.org>, bpf <bpf@vger.kernel.org>,
	Alexei Starovoitov <ast@kernel.org>,
	Kernel Team <Kernel-team@fb.com>,
	Lorenz Bauer <lmb@cloudflare.com>, Jann Horn <jannh@google.com>,
	Greg KH <gregkh@linuxfoundation.org>,
	Linux API <linux-api@vger.kernel.org>,
	LSM List <linux-security-module@vger.kernel.org>,
	Chenbo Feng <chenbofeng.kernel@gmail.com>
Subject: Re: RFC: very rough draft of a bpf permission model
Date: Mon, 26 Aug 2019 17:05:58 -0700	[thread overview]
Message-ID: <CALCETrUARqcn8EmjcgMc8KP=4O5nZDMh=tcruEYvUgSzMKJUBw@mail.gmail.com> (raw)
In-Reply-To: <20190826223558.6torq6keplniif6w@ast-mbp>

> On Aug 26, 2019, at 3:36 PM, Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
>
>> On Fri, Aug 23, 2019 at 04:09:11PM -0700, Andy Lutomirski wrote:
>> On Thu, Aug 22, 2019 at 4:26 PM Alexei Starovoitov
>> <alexei.starovoitov@gmail.com> wrote:
>>> You're proposing all of the above in addition to CAP_BPF, right?
>>> Otherwise I don't see how it addresses the use cases I kept
>>> explaining for the last few weeks.
>>
>> None of my proposal is intended to exclude changes like CAP_BPF to
>> make privileged bpf() operations need less privilege.  But I think
>> it's very hard to evaluate CAP_BPF without both a full description of
>> exactly what CAP_BPF would do and what at least one full example of a
>> user would look like.
>
> the example is previous email and systemd example was not "full" ?

Can you give an example of how a real user would want to configure
their system such that a non-root systemd instance has capabilities,
sets up a BPF firewall, and does something useful with it?  You
mentioned systemd, multiple people pointed out that, on a normal
system, systemd —user has no capabilities. That was the end of the
discussion.

A full example is one where peoples’ confusion as to what the example
is gets answered.

>
>> I also think that users who want CAP_BPF should look at manipulating
>> their effective capability set instead.  A daemon that wants to use
>> bpf() but otherwise minimize the chance of accidentally causing a
>> problem can use capset() to clear its effective and inheritable masks.
>> Then, each time it wants to call bpf(), it could re-add CAP_SYS_ADMIN
>> or CAP_NET_ADMIN to its effective set, call bpf(), and then clear its
>> effective set again.  This works in current kernels and is generally
>> good practice.
>
> Such logic means that CAP_NET_ADMIN is not necessary either.
> The process could re-add CAP_SYS_ADMIN when it needs to reconfigure
> network and then drop it.

This isn't really true. By giving a process CAP_NET_ADMIN and not
CAP_SYS_ADMIN, that process can configure the network but can’t load
kernel modules or reconfigure the machine deliberately or by accident.

But that's besides the point.  Can you give an example where this
approach doesn't help and CAP_BPF does?


>
>> Aside from this, and depending on exactly what CAP_BPF would be, I
>> have some further concerns.  Looking at your example in this email:
>>
>>> Here is another example of use case that CAP_BPF is solving:
>>> The daemon X is started by pid=1 and currently runs as root.
>>> It loads a bunch of tracing progs and attaches them to kprobes
>>> and tracepoints. It also loads cgroup-bpf progs and attaches them
>>> to cgroups. All progs are collecting data about the system and
>>> logging it for further analysis.
>>
>> This needs more than just bpf().  Creating a perf kprobe event
>> requires CAP_SYS_ADMIN, and without a perf kprobe event, you can't
>> attach a bpf program.
>
> that is already solved sysctl_perf_event_paranoid.
> CAP_BPF is about BPF part only.

Hence my point: I'd like to see a real example where CAP_BPF helps.
perf_event_paranoid does not appear to grant the ability to add
kprobes.  With perf_event_paranoid set to -1:

$ perf probe --add vfs_mknod
Failed to open kprobe_events: Permission denied
  Error: Failed to add events.
$ sudo perf probe --add vfs_mknod
Added new event:
  probe:vfs_mknod      (on vfs_mknod)

I suppose I could modify permissions on debugfs and set
perf_event_paranoid=-1, but at that point the overall security of the
system is so weak that talking about refining the bpf part seems
pointless.

>
>> And the privilege to attach bpf programs to
>> cgroups without any DAC or MAC checks (which is what the current API
>> does) is an extremely broad privilege that is not that much weaker
>> than CAP_SYS_ADMIN or CAP_NET_ADMIN.  Also:
>
> I don't think there is a hierarchy of CAP_SYS_ADMIN vs CAP_NET_ADMIN
> vs CAP_BPF.
> CAP_BPF and CAP_NET_ADMIN carve different areas of CAP_SYS_ADMIN.
> Just like all other caps.

The whole set of capabilities on Linux us a bit of a mess.  Their
features are mostly disjoint but, on a normal Linux machine, many of
the capabilities can be used to become root with full capabilities.

>
>>> This tracing bpf is looking into kernel memory
>>> and using bpf_probe_read. Clearly it's not _secure_. But it's _safe_.
>>> The system is not going to crash because of BPF,
>>> but it can easily crash because of simple coding bugs in the user
>>> space bits of that daemon.
>>
>> The BPF verifier and interpreter, taken in isolation, may be extremely
>> safe, but attaching BPF programs to various hooks can easily take down
>> the system, deliberately or by accident.  A handler, especially if it
>> can access user memory or otherwise fault, will explode if attached to
>> an inappropriate kprobe, hw_breakpoint, or function entry trace event.
>
> absolutely not true.

This is not a constructive way to have a conversation.  When you get
an email that contains a statement you disagree with, perhaps you
could try to give some argument as to why you disagree rather than
just saying "absolutely not true".  Especially when you are talking to
one of the maintainers of the affected system who has a
not-yet-finished branch that addresses some of the bugs that you claim
absolutely don't exist.  If it's really truly necessary, I can go and
write an example that will crash an x86 kernel, but I feel like it
would be a waste of everyone's time.

Right now, on all kernels, an hw_breakpoint on memory used in a
non-recursion-safe part of any of the x86 IST entry handlers will
corrupt the kernel no later than when the hw_breakpoint handler
returns.  It does not matter in the slightest what the BPF payload is.
The payload doesn't even have to be BPF for this to blow up.

Similarly, until very, very recently, a handler that pagefaulted (due
to generating a stack trace or failing a bpf_probe_read() in the
trace_hardirqs_on path would crash the system due to corrupting cr2 in
the x86 entry code.  PeterZ just fixed this bug recently.  I believe
that there are similar bugs relating to DR6, but they probably don't
kill the system as easily.  I wouldn't rule out a full system crash,
though.  Again, a not-really-done fix for this is part-way done in my
tree.

How confident are you that a BPF program that calls bpf_probe_read()
the maximum allowable number of times on the address
0xffffffffffffffff attached to, say, an network interrupt probe will
actually leave the system in a usable state?  Maybe it will, but I'd
be a bit surprised.

How confident are you that the BPF program that calls bpf_probe_read()
on an MMIO address has well-defined semantics?  How confident are you
that the system will still work if such a program runs?

>
>> (I and the other maintainers consider this to be a bug if it happens,
>> and we'll fix it, but these bugs definitely exist.)  A cgroup-bpf hook
>> that blocks all network traffic will effectively kill a machine,
>> especially if it's a server.
>
> this permission is granted by CAP_NET_ADMIN. Nothing changes here.
>
>> A bpf program that runs excessively
>> slowly attached to a high-frequency hook will kill the system, too.
>
> not true either.

What prevents this from happening?  Is there a specific mitigation in place?

My point here is that the bpf is 'safe' in isolation, but that bpf
tracing is only somewhat 'safe'.

>> A bpf firewall rule that's
>> wrong can cut a machine off from the network -- I've killed machines
>> using iptables more than once, and bpf isn't magically safer.
>
> this is CAP_NET_ADMIN permission. It's a different capability.

Since you haven't fully defined what CAP_BPF would do, I can only
assume that you intend for CAP_BPF to enable installation of a BPF
inet_ingress hook on the root cgroup.  A BPF program that rejects
everything will block all traffic.

>
>>
>> I'm wondering if something like CAP_TRACING would make sense.
>> CAP_TRACING would allow operations that can reveal kernel memory and
>> other secret kernel state but that do not, by design, allow modifying
>> system behavior.  So, for example, CAP_TRACING would allow privileged
>> perf_event_open() operations and privileged bpf verifier usage.  But
>> it would not allow cgroup-bpf unless further restrictions were added,
>> and it would not allow the *_BY_ID operations, as those can modify
>> other users' bpf programs' behavior.
>
> Makes little sense to me.
> I can imagine CAP_TRACING controlling kprobe/uprobe creation
> and probe_read() both from bpf side and from vanilla kprobe.
> That would be much nicer interface to use than existing
> sysctl_perf_event_paranoid, but that is orthogonal to CAP_BPF
> which is strictly about BPF.

I'm suggesting that CAP_TRACING would also enable most of all of the
things in the verifier that are currently CAP_SYS_ADMIN and would
enable loading and attaching BPF programs to perf events.  So it's not
orthogonal.

You're welcome to post CAP_BPF patches, but perhaps you could also
comment on CAP_TRACING and capset?

--Andy

  reply	other threads:[~2019-08-27  0:06 UTC|newest]

Thread overview: 61+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20190627201923.2589391-1-songliubraving@fb.com>
     [not found] ` <20190627201923.2589391-2-songliubraving@fb.com>
     [not found]   ` <21894f45-70d8-dfca-8c02-044f776c5e05@kernel.org>
     [not found]     ` <3C595328-3ABE-4421-9772-8D41094A4F57@fb.com>
     [not found]       ` <CALCETrWBnH4Q43POU8cQ7YMjb9LioK28FDEQf7aHZbdf1eBZWg@mail.gmail.com>
     [not found]         ` <0DE7F23E-9CD2-4F03-82B5-835506B59056@fb.com>
     [not found]           ` <CALCETrWBWbNFJvsTCeUchu3BZJ3SH3dvtXLUB2EhnPrzFfsLNA@mail.gmail.com>
     [not found]             ` <201907021115.DCD56BBABB@keescook>
     [not found]               ` <CALCETrXTta26CTtEDnzvtd03-WOGdXcnsAogP8JjLkcj4-mHvg@mail.gmail.com>
     [not found]                 ` <4A7A225A-6C23-4C0F-9A95-7C6C56B281ED@fb.com>
     [not found]                   ` <CALCETrX2bMnwC6_t4b_G-hzJSfMPrkK4YKs5ebcecv2LJ0rt3w@mail.gmail.com>
     [not found]                     ` <514D5453-0AEE-420F-AEB6-3F4F58C62E7E@fb.com>
     [not found]                       ` <1DE886F3-3982-45DE-B545-67AD6A4871AB@amacapital.net>
     [not found]                         ` <7F51F8B8-CF4C-4D82-AAE1-F0F28951DB7F@fb.com>
     [not found]                           ` <77354A95-4107-41A7-8936-D144F01C3CA4@fb.com>
     [not found]                             ` <369476A8-4CE1-43DA-9239-06437C0384C7@fb.com>
2019-07-30 20:24                               ` [PATCH v2 bpf-next 1/4] bpf: unprivileged BPF access via /dev/bpf Andy Lutomirski
2019-07-31  8:10                                 ` Song Liu
2019-07-31 19:09                                   ` Andy Lutomirski
2019-08-02  7:21                                     ` Song Liu
2019-08-04 22:16                                       ` Andy Lutomirski
2019-08-05  0:08                                         ` Andy Lutomirski
2019-08-05  5:47                                           ` Andy Lutomirski
2019-08-05  7:36                                             ` Song Liu
2019-08-05 17:23                                               ` Andy Lutomirski
2019-08-05 19:21                                                 ` Alexei Starovoitov
2019-08-05 21:25                                                   ` Andy Lutomirski
2019-08-05 22:21                                                     ` Andy Lutomirski
2019-08-06  1:11                                                     ` Alexei Starovoitov
2019-08-07  5:24                                                       ` Andy Lutomirski
2019-08-07  9:03                                                         ` Lorenz Bauer
2019-08-07 13:52                                                           ` Andy Lutomirski
2019-08-13 21:58                                                         ` Alexei Starovoitov
2019-08-13 22:26                                                           ` Daniel Colascione
2019-08-13 23:24                                                             ` Andy Lutomirski
2019-08-13 23:06                                                           ` Andy Lutomirski
2019-08-14  0:57                                                             ` Alexei Starovoitov
2019-08-14 17:51                                                               ` Andy Lutomirski
2019-08-14 22:05                                                                 ` Alexei Starovoitov
2019-08-14 22:30                                                                   ` Andy Lutomirski
2019-08-14 23:33                                                                     ` Alexei Starovoitov
2019-08-14 23:59                                                                       ` Andy Lutomirski
2019-08-15  0:36                                                                         ` Alexei Starovoitov
2019-08-15 11:24                                                                   ` Jordan Glover
2019-08-15 17:28                                                                     ` Alexei Starovoitov
2019-08-15 18:36                                                                       ` Andy Lutomirski
2019-08-15 23:08                                                                         ` Alexei Starovoitov
2019-08-16  9:34                                                                           ` Jordan Glover
2019-08-16  9:59                                                                             ` Thomas Gleixner
2019-08-16 11:33                                                                               ` Jordan Glover
2019-08-16 19:52                                                                                 ` Alexei Starovoitov
2019-08-16 20:28                                                                                   ` Thomas Gleixner
2019-08-17 15:02                                                                                     ` Alexei Starovoitov
2019-08-17 15:44                                                                                       ` Andy Lutomirski
2019-08-19  9:15                                                                                       ` Thomas Gleixner
2019-08-19 17:27                                                                                         ` Alexei Starovoitov
2019-08-19 17:38                                                                                           ` Andy Lutomirski
2019-08-15 18:43                                                                       ` Jordan Glover
2019-08-15 19:46                                                           ` Kees Cook
2019-08-15 23:46                                                             ` Alexei Starovoitov
2019-08-16  0:54                                                               ` Andy Lutomirski
2019-08-16  5:56                                                                 ` Song Liu
2019-08-16 21:45                                                                 ` Alexei Starovoitov
2019-08-16 22:22                                                                   ` Christian Brauner
2019-08-17 15:08                                                                     ` Alexei Starovoitov
2019-08-17 15:16                                                                       ` Christian Brauner
2019-08-17 15:36                                                                         ` Alexei Starovoitov
2019-08-17 15:42                                                                           ` Christian Brauner
2019-08-22 14:17                                                         ` Daniel Borkmann
2019-08-22 15:16                                                           ` Andy Lutomirski
2019-08-22 15:17                                                             ` RFC: very rough draft of a bpf permission model Andy Lutomirski
2019-08-22 23:26                                                               ` Alexei Starovoitov
2019-08-23 23:09                                                                 ` Andy Lutomirski
2019-08-26 22:36                                                                   ` Alexei Starovoitov
2019-08-27  0:05                                                                     ` Andy Lutomirski [this message]
2019-08-27  0:34                                                                       ` Alexei Starovoitov
2019-08-22 22:48                                                           ` [PATCH v2 bpf-next 1/4] bpf: unprivileged BPF access via /dev/bpf Alexei Starovoitov

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='CALCETrUARqcn8EmjcgMc8KP=4O5nZDMh=tcruEYvUgSzMKJUBw@mail.gmail.com' \
    --to=luto@kernel.org \
    --cc=Kernel-team@fb.com \
    --cc=alexei.starovoitov@gmail.com \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=chenbofeng.kernel@gmail.com \
    --cc=daniel@iogearbox.net \
    --cc=gregkh@linuxfoundation.org \
    --cc=jannh@google.com \
    --cc=keescook@chromium.org \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=lmb@cloudflare.com \
    --cc=netdev@vger.kernel.org \
    --cc=songliubraving@fb.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).