* Re: [PATCH bpf-next] bpf, capabilities: introduce CAP_BPF
From: Alexei Starovoitov @ 2019-08-28 22:55 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Alexei Starovoitov, Kees Cook, LSM List, James Morris, Jann Horn,
Peter Zijlstra, Masami Hiramatsu, Steven Rostedt, David S. Miller,
Daniel Borkmann, Network Development, bpf, kernel-team, Linux API
In-Reply-To: <CALCETrW1o+Lazi2Ng6b9JN6jeJffgdW9f3HvqYhNo4TpHRXW=g@mail.gmail.com>
On Tue, Aug 27, 2019 at 11:12:29PM -0700, Andy Lutomirski wrote:
> >
> > >
> > > From the previous discussion, you want to make progress toward solving
> > > a lot of problems with CAP_BPF. One of them was making BPF
> > > firewalling more generally useful. By making CAP_BPF grant the ability
> > > to read kernel memory, you will make administrators much more nervous
> > > to grant CAP_BPF.
> >
> > Andy, were your email hacked?
> > I explained several times that in this proposal
> > CAP_BPF _and_ CAP_TRACING _both_ are necessary to read kernel memory.
> > CAP_BPF alone is _not enough_.
>
> You have indeed said this many times. You've stated it as a matter of
> fact as though it cannot possibly discussed. I'm asking you to
> justify it.
That's not how I see it.
I kept stating that both CAP_BPF and CAP_TRACING are necessary to read
kernel memory whereas you kept distorting my statement by dropping second
part and then making claims that "CAP_BPF grant the ability to read
kernel memory, you will make administrators much more nervous".
Just s/CAP_BPF/CAP_BPF and CAP_TRACING/ in this above sentence.
See that meaning suddenly changes?
Now administrators would be worried about tasks that have both at once.
They also would be worried about tasks that have CAP_TRACING alone,
because that's what allows probe_kernel_read().
> It seems like you are specifically trying to add a new switch to turn
> as much of BPF as possible on and off. Why?
Didn't I explain it several times already with multiple examples
from systemd, daemons, bpftrace ?
Let's try again.
Take your laptop with linux distro.
You're the only user there. I'm assuming you're not sharing it with
partner and kids. This is my definition of 'single user system'.
You can sudo on it at any time, but obviously prefer to run as many
apps as possible without cap_sys_admin.
Now you found some awesome open source app on the web that monitors
the health of the kernel and will pop a nice message on a screen if
something is wrong. Currently this app needs root. You hesitate,
but the apps is so useful and it has strong upstream code review process
that you keep running it 24/7.
This is open source app. New versions come. You upgrade.
You have enough trust in that app that you keep running it as root.
But there is always a chance that new version doing accidentaly
something stupid as 'kill -9 -1'. It's an open source app at the end.
Now I come with this CAP* proposal to make this app safer.
I'm not making your system more secure and not making this app
more secure. I can only make your laptop safer for day to day work
by limiting the operations this app can do.
This particular app monitros the kernel via bpf and tracing.
Hence you can give it CAP_TRACING and CAP_BPF and drop the rest.
> > speaking of MDS... I already asked you to help investigate its
> > applicability with existing bpf exposure. Are you going to do that?
>
> I am blissfully uninvolved in MDS, and I don't know all that much more
> about the overall mechanism than a random reader of tech news :) ISTM
> there are two meaningful ways that BPF could be involved: a BPF
> program could leak info into the state exposed by MDS, or a BPF
> program could try to read that state. From what little I understand,
> it's essentially inevitable that BPF leaks information into MDS state,
> and this is probably even controllable by an attacker that understands
> MDS in enough detail. So the interesting questions are: can BPF be
> used to read MDS state and can BPF be used to leak information in a
> more useful way than the rest of the kernel to an attacker.
agree. that's exactly the question to ask.
> Keeping in mind that the kernel will flush MDS state on every exit to
> usermode, I think the most likely attack is to try to read MDS state
> with BPF. This could happen, I suppose -- BPF programs can easily
> contain the usual speculation gadgets of "do something and read an
> address that depends on the outcome". Fortunately, outside of
> bpf_probe_read(), AFAIK BPF programs can't directly touch user memory,
> and an attacker that is allowed to use bpf_probe_read() doesn't need
> MDS to read things.
true as well.
So what do we do with that sentence in Documentation/x86/mds.rst?
Nothing?
New hw bugs will keep coming.
All of them should get similar wording?
Your understanding of MDS and BPF is way above the average.
What other users suppose to do when they read such sentence?
I think they have no choice but to do kernel.unprivileged_bpf_disabled=1.
We, as a kernel community, are forcing the users into it.
Hence I really do not see a value in any proposal today that expands
unprivileged bpf usage.
Since kernel.unprivileged_bpf_disabled=1 all bpf is under cap_sys_admin.
It's not great from security and safety pov. Hence this CAP* proposal.
> > Please take a look at Jann's var1 exploit. Was it hard to run bpf prog
> > in controlled environment without test_run command ?
> >
>
> Can you send me a link?
https://bugs.chromium.org/p/project-zero/issues/detail?id=1272
writeup_files.tar:kernel_leak_exploit_amd_pro_a8_9600_r7/bpf_stuff.c
Execution is as trivial as write(sockfd, "X", 1) line 405.
^ permalink raw reply
* Re: [PATCH bpf-next] bpf, capabilities: introduce CAP_BPF
From: Alexei Starovoitov @ 2019-08-28 22:08 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Andy Lutomirski, Alexei Starovoitov, Kees Cook, LSM List,
James Morris, Jann Horn, Masami Hiramatsu, Steven Rostedt,
David S. Miller, Daniel Borkmann, Network Development, bpf,
kernel-team, Linux API
In-Reply-To: <20190828071421.GK2332@hirez.programming.kicks-ass.net>
On Wed, Aug 28, 2019 at 09:14:21AM +0200, Peter Zijlstra wrote:
> On Tue, Aug 27, 2019 at 04:01:08PM -0700, Andy Lutomirski wrote:
>
> > > Tracing:
> > >
> > > CAP_BPF and perf_paranoid_tracepoint_raw() (which is kernel.perf_event_paranoid == -1)
> > > are necessary to:
>
> That's not tracing, that's perf.
>
> > > +bool cap_bpf_tracing(void)
> > > +{
> > > + return capable(CAP_SYS_ADMIN) ||
> > > + (capable(CAP_BPF) && !perf_paranoid_tracepoint_raw());
> > > +}
>
> A whole long time ago, I proposed we introduce CAP_PERF or something
> along those lines; as a replacement for that horrible crap Android and
> Debian ship. But nobody was ever interested enough.
>
> The nice thing about that is that you can then disallow perf/tracing in
> general, but tag the perf executable (and similar tools) with the
> capability so that unpriv users can still use it, but only limited
> through the tool, not the syscalls directly.
Exactly.
Similar motivation for CAP_BPF as well.
re: your first comment above.
I'm not sure what difference you see in words 'tracing' and 'perf'.
I really hope we don't partition the overall tracing category
into CAP_PERF and CAP_FTRACE only because these pieces are maintained
by different people.
On one side perf_event_open() isn't really doing tracing (as step by
step ftracing of function sequences), but perf_event_open() opens
an event and the sequence of events (may include IP) becomes a trace.
imo CAP_TRACING is the best name to descibe the privileged space
of operations possible via perf_event_open, ftrace, kprobe, stack traces, etc.
Another reason are kuprobes. They can be crated via perf_event_open
and via tracefs. Are they in CAP_PERF or in CAP_FTRACE ? In both, right?
Should then CAP_KPROBE be used ? that would be an overkill.
It would partition the space even further without obvious need.
Looking from BPF angle... BPF doesn't have integration with ftrace yet.
bpf_trace_printk is using ftrace mechanism, but that's 1% of ftrace.
In the long run I really like to see bpf using all of ftrace.
Whereas bpf is using a lot of 'perf'.
And extending some perf things in bpf specific way.
Take a look at how BPF_F_STACK_BUILD_ID. It's clearly perf/stack_tracing
feature that generic perf can use one day.
Currently it sits in bpf land and accessible via bpf only.
Though its bpf only today I categorize it under CAP_TRACING.
I think CAP_TRACING privilege should allow task to do all of perf_event_open,
kuprobe, stack trace, ftrace, and kallsyms.
We can think of some exceptions that should stay under CAP_SYS_ADMIN,
but most of the functionality available by 'perf' binary should be
usable with CAP_TRACING. 'perf' can do bpf too.
With CAP_BPF it would be all set.
^ permalink raw reply
* Re: [PATCH] ima: use struct_size() in kzalloc()
From: Gustavo A. R. Silva @ 2019-08-28 18:51 UTC (permalink / raw)
To: Mimi Zohar, Dmitry Kasatkin, James Morris, Serge E. Hallyn
Cc: linux-integrity, linux-security-module, linux-kernel
In-Reply-To: <1567018017.6115.61.camel@linux.ibm.com>
On 8/28/19 1:46 PM, Mimi Zohar wrote:
> On Wed, 2019-08-28 at 13:29 -0500, Gustavo A. R. Silva wrote:
>> Hi all,
>>
>> Friendly ping:
>>
>> Who can take this, please?
>
> Thank you for the reminder. I'm just getting back from LSS and a very
> short vacation. I'll look at it shortly.
>
Thanks, Mimi.
--
Gustavo
^ permalink raw reply
* Re: [PATCH][next] ima: ima_modsig: Fix use-after-free bug in ima_read_modsig
From: Gustavo A. R. Silva @ 2019-08-28 18:55 UTC (permalink / raw)
To: Mimi Zohar, Dmitry Kasatkin, James Morris, Serge E. Hallyn,
Thiago Jung Bauermann
Cc: linux-integrity, linux-security-module, linux-kernel
In-Reply-To: <1567000215.6115.19.camel@linux.ibm.com>
On 8/28/19 8:50 AM, Mimi Zohar wrote:
> Hi Gustavo,
>
> On Sun, 2019-08-11 at 18:55 -0500, Gustavo A. R. Silva wrote:
>> hdr is being freed and then dereferenced by accessing hdr->pkcs7_msg
>>
>> Fix this by copying the value returned by PTR_ERR(hdr->pkcs7_msg) into
>> automatic variable err for its safe use after freeing hdr.
>>
>> Addresses-Coverity-ID: 1485813 ("Read from pointer after free")
>> Fixes: 39b07096364a ("ima: Implement support for module-style appended signatures")
>> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
>
> This bug was reported Julia and addressed by Thiago on 8/7. If you
> would like to add your Review/Tested-by, the patch can be found in the
> linux-integrity next-queued-testing branch.
>
I'm glad this is fixed now. :)
Yeah, you can add my:
Reviewed-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
Thanks
--
Gustavo
^ permalink raw reply
* Re: [PATCH] ima: use struct_size() in kzalloc()
From: Gustavo A. R. Silva @ 2019-08-28 18:29 UTC (permalink / raw)
To: Mimi Zohar, Dmitry Kasatkin, James Morris, Serge E. Hallyn
Cc: linux-integrity, linux-security-module, linux-kernel
In-Reply-To: <20190529165343.GA2584@embeddedor>
Hi all,
Friendly ping:
Who can take this, please?
Thanks
--
Gustavo
On 5/29/19 11:53 AM, Gustavo A. R. Silva wrote:
> One of the more common cases of allocation size calculations is finding
> the size of a structure that has a zero-sized array at the end, along
> with memory for some number of elements for that array. For example:
>
> struct foo {
> int stuff;
> struct boo entry[];
> };
>
> instance = kzalloc(sizeof(struct foo) + count * sizeof(struct boo), GFP_KERNEL);
>
> Instead of leaving these open-coded and prone to type mistakes, we can
> now use the new struct_size() helper:
>
> instance = kzalloc(struct_size(instance, entry, count), GFP_KERNEL);
>
> This code was detected with the help of Coccinelle.
>
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
> ---
> security/integrity/ima/ima_template.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c
> index b631b8bc7624..b945dff2ed14 100644
> --- a/security/integrity/ima/ima_template.c
> +++ b/security/integrity/ima/ima_template.c
> @@ -281,9 +281,8 @@ static int ima_restore_template_data(struct ima_template_desc *template_desc,
> int ret = 0;
> int i;
>
> - *entry = kzalloc(sizeof(**entry) +
> - template_desc->num_fields * sizeof(struct ima_field_data),
> - GFP_NOFS);
> + *entry = kzalloc(struct_size(*entry, template_data,
> + template_desc->num_fields), GFP_NOFS);
> if (!*entry)
> return -ENOMEM;
>
>
^ permalink raw reply
* Re: [PATCH] ima: use struct_size() in kzalloc()
From: Mimi Zohar @ 2019-08-28 18:46 UTC (permalink / raw)
To: Gustavo A. R. Silva, Dmitry Kasatkin, James Morris,
Serge E. Hallyn
Cc: linux-integrity, linux-security-module, linux-kernel
In-Reply-To: <671185b9-5c91-5235-b5ea-96d3449bf716@embeddedor.com>
On Wed, 2019-08-28 at 13:29 -0500, Gustavo A. R. Silva wrote:
> Hi all,
>
> Friendly ping:
>
> Who can take this, please?
Thank you for the reminder. I'm just getting back from LSS and a very
short vacation. I'll look at it shortly.
Mimi
^ permalink raw reply
* Re: [PATCH][next] ima: ima_modsig: Fix use-after-free bug in ima_read_modsig
From: Mimi Zohar @ 2019-08-28 13:50 UTC (permalink / raw)
To: Gustavo A. R. Silva, Dmitry Kasatkin, James Morris,
Serge E. Hallyn, Thiago Jung Bauermann
Cc: linux-integrity, linux-security-module, linux-kernel
In-Reply-To: <20190811235507.GA9587@embeddedor>
Hi Gustavo,
On Sun, 2019-08-11 at 18:55 -0500, Gustavo A. R. Silva wrote:
> hdr is being freed and then dereferenced by accessing hdr->pkcs7_msg
>
> Fix this by copying the value returned by PTR_ERR(hdr->pkcs7_msg) into
> automatic variable err for its safe use after freeing hdr.
>
> Addresses-Coverity-ID: 1485813 ("Read from pointer after free")
> Fixes: 39b07096364a ("ima: Implement support for module-style appended signatures")
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
This bug was reported Julia and addressed by Thiago on 8/7. If you
would like to add your Review/Tested-by, the patch can be found in the
linux-integrity next-queued-testing branch.
thanks,
Mimi
^ permalink raw reply
* Re: [PATCH v12 00/11] Appended signatures support for IMA appraisal
From: Mimi Zohar @ 2019-08-28 13:43 UTC (permalink / raw)
To: Jordan Hand, Thiago Jung Bauermann, linux-integrity
Cc: linux-security-module, keyrings, linux-crypto, linuxppc-dev,
linux-doc, linux-kernel, Dmitry Kasatkin, James Morris,
Serge E. Hallyn, David Howells, David Woodhouse, Jessica Yu,
Herbert Xu, David S. Miller, Jonathan Corbet, AKASHI, Takahiro
In-Reply-To: <9682b5d0-1634-2dd0-2cbb-eb1fa8ba7423@linux.microsoft.com>
Hi Jordan,
On Mon, 2019-08-26 at 15:46 -0700, Jordan Hand wrote:
> On 6/27/19 7:19 PM, Thiago Jung Bauermann wrote:
> > On the OpenPOWER platform, secure boot and trusted boot are being
> > implemented using IMA for taking measurements and verifying signatures.
> > Since the kernel image on Power servers is an ELF binary, kernels are
> > signed using the scripts/sign-file tool and thus use the same signature
> > format as signed kernel modules.
> >
> > This patch series adds support in IMA for verifying those signatures.
> > It adds flexibility to OpenPOWER secure boot, because it allows it to boot
> > kernels with the signature appended to them as well as kernels where the
> > signature is stored in the IMA extended attribute.
>
> I know this is pretty late, but I just wanted to let you know that I
> tested this patch set on x86_64 with QEMU.
>
> That is, I enrolled a key to _ima keyring, signed my kernel and modules
> with appended signatures (with scripts/sign-file), set the IMA policy to
> appraise and measure my kernel and modules. Also tested kexec appraisal.
>
> You can add my tested-by if you'd like.
I really appreciate your testing. Based on the recent
Documentation/maintainer/rebasing-and-merging.rst, I'm trying not to
rebase patches already staged in linux-next. Patches are first being
staged in the next-queued-testing branch.
FYI, I just posted a patch that adds IMA appended signature support to
test_kexec_file_load.sh.
thanks,
Mimi
^ permalink raw reply
* Re: [PATCH bpf-next] bpf, capabilities: introduce CAP_BPF
From: Peter Zijlstra @ 2019-08-28 7:14 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Alexei Starovoitov, Kees Cook, LSM List, James Morris, Jann Horn,
Masami Hiramatsu, Steven Rostedt, David S. Miller,
Daniel Borkmann, Network Development, bpf, kernel-team, Linux API
In-Reply-To: <CALCETrV8iJv9+Ai11_1_r6MapPhhwt9hjxi=6EoixytabTScqg@mail.gmail.com>
On Tue, Aug 27, 2019 at 04:01:08PM -0700, Andy Lutomirski wrote:
> > Tracing:
> >
> > CAP_BPF and perf_paranoid_tracepoint_raw() (which is kernel.perf_event_paranoid == -1)
> > are necessary to:
That's not tracing, that's perf.
> > +bool cap_bpf_tracing(void)
> > +{
> > + return capable(CAP_SYS_ADMIN) ||
> > + (capable(CAP_BPF) && !perf_paranoid_tracepoint_raw());
> > +}
A whole long time ago, I proposed we introduce CAP_PERF or something
along those lines; as a replacement for that horrible crap Android and
Debian ship. But nobody was ever interested enough.
The nice thing about that is that you can then disallow perf/tracing in
general, but tag the perf executable (and similar tools) with the
capability so that unpriv users can still use it, but only limited
through the tool, not the syscalls directly.
^ permalink raw reply
* Re: [PATCH bpf-next] bpf, capabilities: introduce CAP_BPF
From: Andy Lutomirski @ 2019-08-28 6:20 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Andy Lutomirski, Alexei Starovoitov, Kees Cook, LSM List,
James Morris, Jann Horn, Peter Zijlstra, Masami Hiramatsu,
Steven Rostedt, David S. Miller, Daniel Borkmann,
Network Development, bpf, kernel-team, Linux API
In-Reply-To: <20190828044903.nv3hvinkkolnnxtv@ast-mbp.dhcp.thefacebook.com>
On Tue, Aug 27, 2019 at 9:49 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Tue, Aug 27, 2019 at 07:00:40PM -0700, Andy Lutomirski wrote:
> >
> > Let me put this a bit differently. Part of the point is that
> > CAP_TRACING should allow a user or program to trace without being able
> > to corrupt the system. CAP_BPF as you’ve proposed it *can* likely
> > crash the system.
>
> Really? I'm still waiting for your example where bpf+kprobe crashes the system...
>
That's not what I meant. bpf+kprobe causing a crash is a bug. I'm
referring to a totally different issue. On my laptop:
$ sudo bpftool map
48: hash name foobar flags 0x0
key 8B value 8B max_entries 64 memlock 8192B
181: lpm_trie flags 0x1
key 8B value 8B max_entries 1 memlock 4096B
182: lpm_trie flags 0x1
key 20B value 8B max_entries 1 memlock 4096B
183: lpm_trie flags 0x1
key 8B value 8B max_entries 1 memlock 4096B
184: lpm_trie flags 0x1
key 20B value 8B max_entries 1 memlock 4096B
185: lpm_trie flags 0x1
key 8B value 8B max_entries 1 memlock 4096B
186: lpm_trie flags 0x1
key 20B value 8B max_entries 1 memlock 4096B
187: lpm_trie flags 0x1
key 8B value 8B max_entries 1 memlock 4096B
188: lpm_trie flags 0x1
key 20B value 8B max_entries 1 memlock 4096B
$ sudo bpftool map dump id 186
key:
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00
value:
02 00 00 00 00 00 00 00
Found 1 element
$ sudo bpftool map delete id 186 key hex 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00
[this worked]
I don't know what my laptop was doing with map id 186 in particular,
but, whatever it was, I definitely broke it. If a BPF firewall is in
use on something important enough, this could easily remove
connectivity from part or all of the system. Right now, this needs
CAP_SYS_ADMIN. With your patch, CAP_BPF is sufficient to do this, but
you *also* need CAP_BPF to trace the system using BPF. Tracing with
BPF is 'safe' in the absence of bugs. Modifying other peoples' maps
is not.
One possible answer to this would be to limit CAP_BPF to the subset of
BPF that is totaly safe in the absence of bugs (e.g. loading most
program types if they don't have dangerous BPF_CALL instructions but
not *_BY_ID). Another answer would be to say that CAP_BPF will not be
needed by future unprivileged bpf mechanisms, and that CAP_TRACING
plus unprivileged bpf will be enough to do most or all interesting BPF
tracing operations.
If the answer is the latter, then maybe it would make sense to try to
implement some of the unprivileged bpf stuff and then to see whether
CAP_BPF is still needed.
^ permalink raw reply
* Re: [PATCH bpf-next] bpf, capabilities: introduce CAP_BPF
From: Andy Lutomirski @ 2019-08-28 6:12 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Andy Lutomirski, Alexei Starovoitov, Kees Cook, LSM List,
James Morris, Jann Horn, Peter Zijlstra, Masami Hiramatsu,
Steven Rostedt, David S. Miller, Daniel Borkmann,
Network Development, bpf, kernel-team, Linux API
In-Reply-To: <20190828044340.zeha3k3cmmxgfqj7@ast-mbp.dhcp.thefacebook.com>
On Tue, Aug 27, 2019 at 9:43 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Tue, Aug 27, 2019 at 05:55:41PM -0700, Andy Lutomirski wrote:
> >
> > I was hoping for something in Documentation/admin-guide, not in a
> > changelog that's hard to find.
>
> eventually yes.
>
> > >
> > > > Changing the capability that some existing operation requires could
> > > > break existing programs. The old capability may need to be accepted
> > > > as well.
> > >
> > > As far as I can see there is no ABI breakage. Please point out
> > > which line of the patch may break it.
> >
> > As a more or less arbitrary selection:
> >
> > void bpf_prog_kallsyms_add(struct bpf_prog *fp)
> > {
> > if (!bpf_prog_kallsyms_candidate(fp) ||
> > - !capable(CAP_SYS_ADMIN))
> > + !capable(CAP_BPF))
> > return;
> >
> > Before your patch, a task with CAP_SYS_ADMIN could do this. Now it
> > can't. Per the usual Linux definition of "ABI break", this is an ABI
> > break if and only if someone actually did this in a context where they
> > have CAP_SYS_ADMIN but not all capabilities. How confident are you
> > that no one does things like this?
> > void bpf_prog_kallsyms_add(struct bpf_prog *fp)
> > {
> > if (!bpf_prog_kallsyms_candidate(fp) ||
> > - !capable(CAP_SYS_ADMIN))
> > + !capable(CAP_BPF))
> > return;
>
> Yes. I'm confident that apps don't drop everything and
> leave cap_sys_admin only before doing bpf() syscall, since it would
> break their own use of networking.
> Hence I'm not going to do the cap_syslog-like "deprecated" message mess
> because of this unfounded concern.
> If I turn out to be wrong we will add this "deprecated mess" later.
>
> >
> > From the previous discussion, you want to make progress toward solving
> > a lot of problems with CAP_BPF. One of them was making BPF
> > firewalling more generally useful. By making CAP_BPF grant the ability
> > to read kernel memory, you will make administrators much more nervous
> > to grant CAP_BPF.
>
> Andy, were your email hacked?
> I explained several times that in this proposal
> CAP_BPF _and_ CAP_TRACING _both_ are necessary to read kernel memory.
> CAP_BPF alone is _not enough_.
You have indeed said this many times. You've stated it as a matter of
fact as though it cannot possibly discussed. I'm asking you to
justify it.
> > Similarly, and correct me if I'm wrong, most of
> > these capabilities are primarily or only useful for tracing, so I
> > don't see why users without CAP_TRACING should get them.
> > bpf_trace_printk(), in particular, even has "trace" in its name :)
> >
> > Also, if a task has CAP_TRACING, it's expected to be able to trace the
> > system -- that's the whole point. Why shouldn't it be able to use BPF
> > to trace the system better?
>
> CAP_TRACING shouldn't be able to do BPF because BPF is not tracing only.
What does "do BPF" even mean? seccomp() does BPF. SO_ATTACH_FILTER
does BPF. Saying that using BPF should require a specific capability
seems kind of like saying that using the network should require a
specific capability. Linux (and Unixy systems in general) distinguish
between binding low-number ports, binding high-number ports, using raw
sockets, and changing the system's IP address. These have different
implications and require different capabilities.
It seems like you are specifically trying to add a new switch to turn
as much of BPF as possible on and off. Why?
> >
> > test_run allows fully controlled inputs, in a context where a program
> > can trivially flush caches, mistrain branch predictors, etc first. It
> > seems to me that, if a JITted bpf program contains an exploitable
> > speculation gadget (MDS, Spectre v1, RSB, or anything else),
>
> speaking of MDS... I already asked you to help investigate its
> applicability with existing bpf exposure. Are you going to do that?
I am blissfully uninvolved in MDS, and I don't know all that much more
about the overall mechanism than a random reader of tech news :) ISTM
there are two meaningful ways that BPF could be involved: a BPF
program could leak info into the state exposed by MDS, or a BPF
program could try to read that state. From what little I understand,
it's essentially inevitable that BPF leaks information into MDS state,
and this is probably even controllable by an attacker that understands
MDS in enough detail. So the interesting questions are: can BPF be
used to read MDS state and can BPF be used to leak information in a
more useful way than the rest of the kernel to an attacker.
Keeping in mind that the kernel will flush MDS state on every exit to
usermode, I think the most likely attack is to try to read MDS state
with BPF. This could happen, I suppose -- BPF programs can easily
contain the usual speculation gadgets of "do something and read an
address that depends on the outcome". Fortunately, outside of
bpf_probe_read(), AFAIK BPF programs can't directly touch user memory,
and an attacker that is allowed to use bpf_probe_read() doesn't need
MDS to read things.
So it's not entirely obvious to me how an attack would be mounted.
test_run would make it a lot easier, I think.
>
> > it will
> > be *much* easier to exploit it using test_run than using normal
> > network traffic. Similarly, normal network traffic will have network
> > headers that are valid enough to have caused the BPF program to be
> > invoked in the first place. test_run can inject arbitrary garbage.
>
> Please take a look at Jann's var1 exploit. Was it hard to run bpf prog
> in controlled environment without test_run command ?
>
Can you send me a link?
^ permalink raw reply
* Re: [PATCH v5 4/4] KEYS: trusted: move tpm2 trusted keys code
From: Sumit Garg @ 2019-08-28 5:28 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: keyrings, linux-integrity,
open list:HARDWARE RANDOM NUMBER GENERATOR CORE,
linux-security-module, dhowells, Herbert Xu, davem, peterhuewe,
jgg, jejb, Arnd Bergmann, Greg Kroah-Hartman, Mimi Zohar,
James Morris, Serge E. Hallyn, Casey Schaufler, Ard Biesheuvel,
Daniel Thompson, Linux Kernel Mailing List,
tee-dev @ lists . linaro . org
In-Reply-To: <20190827141742.6qxowsigqolxaod4@linux.intel.com>
On Tue, 27 Aug 2019 at 19:47, Jarkko Sakkinen
<jarkko.sakkinen@linux.intel.com> wrote:
>
> On Wed, Aug 21, 2019 at 06:29:05PM +0530, Sumit Garg wrote:
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Copyright (C) 2004 IBM Corporation
> > + * Copyright (C) 2014 Intel Corporation
>
> Everything below can be dropped from this new file. Git has the most
> accurate authority information.
>
> I'm not sure why I added the authors-list in the first place to the
> header when I implemented these functions as none of those folks have
> contributed to this particular piece of work.
>
> > + * Authors:
> > + * Leendert van Doorn <leendert@watson.ibm.com>
> > + * Dave Safford <safford@watson.ibm.com>
> > + * Reiner Sailer <sailer@watson.ibm.com>
> > + * Kylene Hall <kjhall@us.ibm.com>
> > + *
> > + * Maintained by: <tpmdd-devel@lists.sourceforge.net>
> > + *
> > + * Trusted Keys code for TCG/TCPA TPM2 (trusted platform module).
> > + */
>
> To summarize, I think this would be sufficient:
>
> // SPDX-License-Identifier: GPL-2.0-only
> /*
> * Copyright (C) 2004 IBM Corporation
> * Copyright (C) 2014 Intel Corporation
> */
Sounds good to me.
>
> I think there should never be such a rush that acronym could not be
> written with the correct spelling. I'm referring to 'tpm2' in the short
> summary.
So you mean to say we should use upper-case letters for 'TPM2' acronym?
> I'm sorry, I had to say it, just can't help myself with those
> kind of details :-) I can take care of fixing those once I apply these
> patches.
>
> You've done an awesome job. Thank you.
>
You are welcome.
> Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
>
Thanks for your review.
-Sumit
> Unfortunately I'm not yet sure if I have time to test these before going
> to Linux Plumbers but these would be anyway too close to the next merge
> window to be added to the v5.4 PR.
>
> /Jarkko
^ permalink raw reply
* Re: [PATCH bpf-next] bpf, capabilities: introduce CAP_BPF
From: Alexei Starovoitov @ 2019-08-28 4:49 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Alexei Starovoitov, Kees Cook, LSM List, James Morris, Jann Horn,
Peter Zijlstra, Masami Hiramatsu, Steven Rostedt, David S. Miller,
Daniel Borkmann, Network Development, bpf, kernel-team, Linux API
In-Reply-To: <CALCETrVVQs1s27y8fB17JtQi-VzTq1YZPTPy3k=fKhQB1X-KKA@mail.gmail.com>
On Tue, Aug 27, 2019 at 07:00:40PM -0700, Andy Lutomirski wrote:
>
> Let me put this a bit differently. Part of the point is that
> CAP_TRACING should allow a user or program to trace without being able
> to corrupt the system. CAP_BPF as you’ve proposed it *can* likely
> crash the system.
Really? I'm still waiting for your example where bpf+kprobe crashes the system...
^ permalink raw reply
* Re: [PATCH bpf-next] bpf, capabilities: introduce CAP_BPF
From: Alexei Starovoitov @ 2019-08-28 4:47 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: Steven Rostedt, Andy Lutomirski, Alexei Starovoitov, Kees Cook,
LSM List, James Morris, Jann Horn, Peter Zijlstra,
David S. Miller, Daniel Borkmann, Network Development, bpf,
kernel-team, Linux API
In-Reply-To: <20190828123041.c0c90c15865897461ee819a2@kernel.org>
On Wed, Aug 28, 2019 at 12:30:41PM +0900, Masami Hiramatsu wrote:
> > kprobes can be created in the tracefs filesystem (which is separate from
> > debugfs, tracefs just gets automatically mounted
> > in /sys/kernel/debug/tracing when debugfs is mounted) from the
> > kprobe_events file. /sys/kernel/tracing is just the tracefs
> > directory without debugfs, and was created specifically to allow
> > tracing to be access without opening up the can of worms in debugfs.
>
> I like the CAP_TRACING for tracefs. Can we make the tracefs itself
> check the CAP_TRACING and call file_ops? or each tracefs file-ops
> handlers must check it?
Thanks for the feedback.
I'll hack a prototype of CAP_TRACING for perf bits that I understand
and you folks will be able to use it in ftrace when initial support lands.
imo the question above is an implementation detail that you can resolve later.
I see it as a followup to initial CAP_TRACING drop.
^ permalink raw reply
* Re: [PATCH bpf-next] bpf, capabilities: introduce CAP_BPF
From: Alexei Starovoitov @ 2019-08-28 4:43 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Alexei Starovoitov, Kees Cook, LSM List, James Morris, Jann Horn,
Peter Zijlstra, Masami Hiramatsu, Steven Rostedt, David S. Miller,
Daniel Borkmann, Network Development, bpf, kernel-team, Linux API
In-Reply-To: <CALCETrVbPPPr=BdPAx=tJKxD3oLXP4OVSgCYrB_E4vb6idELow@mail.gmail.com>
On Tue, Aug 27, 2019 at 05:55:41PM -0700, Andy Lutomirski wrote:
>
> I was hoping for something in Documentation/admin-guide, not in a
> changelog that's hard to find.
eventually yes.
> >
> > > Changing the capability that some existing operation requires could
> > > break existing programs. The old capability may need to be accepted
> > > as well.
> >
> > As far as I can see there is no ABI breakage. Please point out
> > which line of the patch may break it.
>
> As a more or less arbitrary selection:
>
> void bpf_prog_kallsyms_add(struct bpf_prog *fp)
> {
> if (!bpf_prog_kallsyms_candidate(fp) ||
> - !capable(CAP_SYS_ADMIN))
> + !capable(CAP_BPF))
> return;
>
> Before your patch, a task with CAP_SYS_ADMIN could do this. Now it
> can't. Per the usual Linux definition of "ABI break", this is an ABI
> break if and only if someone actually did this in a context where they
> have CAP_SYS_ADMIN but not all capabilities. How confident are you
> that no one does things like this?
> void bpf_prog_kallsyms_add(struct bpf_prog *fp)
> {
> if (!bpf_prog_kallsyms_candidate(fp) ||
> - !capable(CAP_SYS_ADMIN))
> + !capable(CAP_BPF))
> return;
Yes. I'm confident that apps don't drop everything and
leave cap_sys_admin only before doing bpf() syscall, since it would
break their own use of networking.
Hence I'm not going to do the cap_syslog-like "deprecated" message mess
because of this unfounded concern.
If I turn out to be wrong we will add this "deprecated mess" later.
>
> From the previous discussion, you want to make progress toward solving
> a lot of problems with CAP_BPF. One of them was making BPF
> firewalling more generally useful. By making CAP_BPF grant the ability
> to read kernel memory, you will make administrators much more nervous
> to grant CAP_BPF.
Andy, were your email hacked?
I explained several times that in this proposal
CAP_BPF _and_ CAP_TRACING _both_ are necessary to read kernel memory.
CAP_BPF alone is _not enough_.
> Similarly, and correct me if I'm wrong, most of
> these capabilities are primarily or only useful for tracing, so I
> don't see why users without CAP_TRACING should get them.
> bpf_trace_printk(), in particular, even has "trace" in its name :)
>
> Also, if a task has CAP_TRACING, it's expected to be able to trace the
> system -- that's the whole point. Why shouldn't it be able to use BPF
> to trace the system better?
CAP_TRACING shouldn't be able to do BPF because BPF is not tracing only.
> > For example:
> > BPF_CALL_3(bpf_probe_read, void *, dst, u32, size, const void *, unsafe_ptr)
> > {
> > int ret;
> >
> > ret = probe_kernel_read(dst, unsafe_ptr, size);
> > if (unlikely(ret < 0))
> > memset(dst, 0, size);
> >
> > return ret;
> > }
> >
> > All of BPF (including prototype of bpf_probe_read) is controlled by CAP_BPF.
> > But the kernel primitives its using (probe_kernel_read) is controlled by CAP_TRACING.
> > Hence a task needs _both_ CAP_BPF and CAP_TRACING to attach and run bpf program
> > that uses bpf_probe_read.
> >
> > Similar with all other kernel code that BPF helpers may call directly or indirectly.
> > If there is a way for bpf program to call into piece of code controlled by CAP_TRACING
> > such helper would need CAP_BPF and CAP_TRACING.
> > If bpf helper calls into something that may mangle networking packet
> > such helper would need both CAP_BPF and CAP_NET_ADMIN to execute.
>
> Why do you want to require CAP_BPF to call into functions like
> bpf_probe_read()? I understand why you want to limit access to bpf,
> but I think that CAP_TRACING should be sufficient to allow the tracing
> parts of BPF. After all, a lot of your concerns, especially the ones
> involving speculation, don't really apply to users with CAP_TRACING --
> users with CAP_TRACING can read kernel memory with or without bpf.
Let me try again to explain the concept...
Imagine AUDI logo with 4 circles.
They partially intersect.
The first circle is CAP_TRACING. Second is CAP_BPF. Third is CAP_NET_ADMIN.
Fourth - up to your imagination :)
These capabilities subdivide different parts of root privileges.
CAP_NET_ADMIN is useful on its own.
Just as CAP_TRACING that is useful for perf, ftrace, and probably
other tracing things that don't need bpf.
'bpftrace' is using a lot of tracing and a lot of bpf features,
but not all of bpf and not all tracing.
It falls into intersection of CAP_BPF and CAP_TRACING.
probe_kernel_read is a tracing mechanism.
perf can use it without bpf.
Hence it should be controlled by CAP_TRACING.
bpf_probe_read is a wrapper of that mechanism.
It's a place where BPF and TRACING circles intersect.
A task needs to have both CAP_BPF (to load the program)
and CAP_TRACING (to read kernel memory) to execute bpf_probe_read() helper.
> > > > @@ -2080,7 +2083,10 @@ static int bpf_prog_test_run(const union bpf_attr *attr,
> > > > struct bpf_prog *prog;
> > > > int ret = -ENOTSUPP;
> > > >
> > > > - if (!capable(CAP_SYS_ADMIN))
> > > > + if (!capable(CAP_NET_ADMIN) || !capable(CAP_BPF))
> > > > + /* test_run callback is available for networking progs only.
> > > > + * Add cap_bpf_tracing() above when tracing progs become runable.
> > > > + */
> > >
> > > I think test_run should probably be CAP_SYS_ADMIN forever. test_run
> > > is the only way that one can run a bpf program and call helper
> > > functions via the program if one doesn't have permission to attach the
> > > program.
> >
> > Since CAP_BPF + CAP_NET_ADMIN allow attach. It means that a task
> > with these two permissions will have programs running anyway.
> > (traffic will flow through netdev, socket events will happen, etc)
> > Hence no reason to disallow running program via test_run.
> >
>
> test_run allows fully controlled inputs, in a context where a program
> can trivially flush caches, mistrain branch predictors, etc first. It
> seems to me that, if a JITted bpf program contains an exploitable
> speculation gadget (MDS, Spectre v1, RSB, or anything else),
speaking of MDS... I already asked you to help investigate its
applicability with existing bpf exposure. Are you going to do that?
> it will
> be *much* easier to exploit it using test_run than using normal
> network traffic. Similarly, normal network traffic will have network
> headers that are valid enough to have caused the BPF program to be
> invoked in the first place. test_run can inject arbitrary garbage.
Please take a look at Jann's var1 exploit. Was it hard to run bpf prog
in controlled environment without test_run command ?
^ permalink raw reply
* Re: [PATCH bpf-next] bpf, capabilities: introduce CAP_BPF
From: Masami Hiramatsu @ 2019-08-28 3:30 UTC (permalink / raw)
To: Steven Rostedt
Cc: Andy Lutomirski, Alexei Starovoitov, Kees Cook, LSM List,
James Morris, Jann Horn, Peter Zijlstra, Masami Hiramatsu,
David S. Miller, Daniel Borkmann, Network Development, bpf,
kernel-team, Linux API
In-Reply-To: <20190827192144.3b38b25a@gandalf.local.home>
On Tue, 27 Aug 2019 19:21:44 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:
> > Here's my proposal for CAP_TRACING, documentation-style:
> >
> > --- begin ---
> >
> > CAP_TRACING enables a task to use various kernel features to trace
> > running user programs and the kernel itself. CAP_TRACING also enables
> > a task to bypass some speculation attack countermeasures. A task in
> > the init user namespace with CAP_TRACING will be able to tell exactly
> > what kernel code is executed and when, and will be able to read kernel
> > registers and kernel memory. It will, similarly, be able to read the
> > state of other user tasks.
> >
> > Specifically, CAP_TRACING allows the following operations. It may
> > allow more operations in the future:
> >
> > - Full use of perf_event_open(), similarly to the effect of
> > kernel.perf_event_paranoid == -1.
> >
> > - Loading and attaching tracing BPF programs, including use of BPF
> > raw tracepoints.
> >
> > - Use of BPF stack maps.
> >
> > - Use of bpf_probe_read() and bpf_trace_printk().
> >
> > - Use of unsafe pointer-to-integer conversions in BPF.
> >
> > - Bypassing of BPF's speculation attack hardening measures and
> > constant blinding. (Note: other mechanisms might also allow this.)
> >
> > CAP_TRACING does not override normal permissions on sysfs or debugfs.
> > This means that, unless a new interface for programming kprobes and
> > such is added, it does not directly allow use of kprobes.
>
> kprobes can be created in the tracefs filesystem (which is separate from
> debugfs, tracefs just gets automatically mounted
> in /sys/kernel/debug/tracing when debugfs is mounted) from the
> kprobe_events file. /sys/kernel/tracing is just the tracefs
> directory without debugfs, and was created specifically to allow
> tracing to be access without opening up the can of worms in debugfs.
I like the CAP_TRACING for tracefs. Can we make the tracefs itself
check the CAP_TRACING and call file_ops? or each tracefs file-ops
handlers must check it?
> Should we allow CAP_TRACING access to /proc/kallsyms? as it is helpful
> to convert perf and trace-cmd's function pointers into names. Once you
> allow tracing of the kernel, hiding /proc/kallsyms is pretty useless.
Also, there is a blacklist of kprobes under debugfs. If CAP_TRACING
introduced and it allows to access kallsyms, I would like to move the
blacklist under tracefs, or make an alias of blacklist entry on tracefs.
Thank you,
--
Masami Hiramatsu <mhiramat@kernel.org>
^ permalink raw reply
* Re: [PATCH bpf-next] bpf, capabilities: introduce CAP_BPF
From: Steven Rostedt @ 2019-08-28 2:22 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Andy Lutomirski, Alexei Starovoitov, Kees Cook, LSM List,
James Morris, Jann Horn, Peter Zijlstra, Masami Hiramatsu,
David S. Miller, Daniel Borkmann, Network Development, bpf,
kernel-team, Linux API
In-Reply-To: <A95DA1BC-E2A1-4CC3-B17F-36C494FB7540@amacapital.net>
On Tue, 27 Aug 2019 18:12:59 -0700
Andy Lutomirski <luto@amacapital.net> wrote:
> Too many slashes :/
>
> A group could work for v1. Maybe all the tools should get updated to use this path?
trace-cmd now does. In fact, if run as root, it will first check if
tracefs is mounted, and if not, it will try to mount it at this
location.
-- Steve
^ permalink raw reply
* Re: [PATCH bpf-next] bpf, capabilities: introduce CAP_BPF
From: Andy Lutomirski @ 2019-08-28 2:00 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Alexei Starovoitov, Alexei Starovoitov, Kees Cook, LSM List,
James Morris, Jann Horn, Peter Zijlstra, Masami Hiramatsu,
Steven Rostedt, David S. Miller, Daniel Borkmann,
Network Development, bpf, kernel-team, Linux API
In-Reply-To: <CALCETrVbPPPr=BdPAx=tJKxD3oLXP4OVSgCYrB_E4vb6idELow@mail.gmail.com>
> On Aug 27, 2019, at 5:55 PM, Andy Lutomirski <luto@kernel.org> wrote:
>
> On Tue, Aug 27, 2019 at 5:34 PM Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
>>
> From the previous discussion, you want to make progress toward solving
> a lot of problems with CAP_BPF. One of them was making BPF
> firewalling more generally useful. By making CAP_BPF grant the ability
> to read kernel memory, you will make administrators much more nervous
> to grant CAP_BPF. Similarly, and correct me if I'm wrong, most of
> these capabilities are primarily or only useful for tracing, so I
> don't see why users without CAP_TRACING should get them.
> bpf_trace_printk(), in particular, even has "trace" in its name :)
>
> Also, if a task has CAP_TRACING, it's expected to be able to trace the
> system -- that's the whole point. Why shouldn't it be able to use BPF
> to trace the system better?
Let me put this a bit differently. Part of the point is that
CAP_TRACING should allow a user or program to trace without being able
to corrupt the system. CAP_BPF as you’ve proposed it *can* likely
crash the system. For example, CAP_BPF allows bpf_map_get_fd_by_id()
in your patch. If the system uses a BPF firewall that stores some of
its rules in maps, then bpf_map_get_fd_by_id() can be used to get a
writable fd to the map, which can be used to change the map, thus
preventing network access. This means that no combination of
CAP_TRACING and CAP_BPF ends up allowing tracing without granting the
ability to reconfigure or otherwise corrupt the system.
^ permalink raw reply
* Re: [PATCH bpf-next] bpf, capabilities: introduce CAP_BPF
From: Andy Lutomirski @ 2019-08-28 1:12 UTC (permalink / raw)
To: Steven Rostedt
Cc: Andy Lutomirski, Alexei Starovoitov, Kees Cook, LSM List,
James Morris, Jann Horn, Peter Zijlstra, Masami Hiramatsu,
David S. Miller, Daniel Borkmann, Network Development, bpf,
kernel-team, Linux API
In-Reply-To: <20190827204433.3af91faf@gandalf.local.home>
> On Aug 27, 2019, at 5:44 PM, Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Tue, 27 Aug 2019 16:34:47 -0700
> Andy Lutomirski <luto@kernel.org> wrote:
>
>>>> CAP_TRACING does not override normal permissions on sysfs or debugfs.
>>>> This means that, unless a new interface for programming kprobes and
>>>> such is added, it does not directly allow use of kprobes.
>>>
>>> kprobes can be created in the tracefs filesystem (which is separate from
>>> debugfs, tracefs just gets automatically mounted
>>> in /sys/kernel/debug/tracing when debugfs is mounted) from the
>>> kprobe_events file. /sys/kernel/tracing is just the tracefs
>>> directory without debugfs, and was created specifically to allow
>>> tracing to be access without opening up the can of worms in debugfs.
>>
>> I think that, in principle, CAP_TRACING should allow this, but I'm not
>> sure how to achieve that. I suppose we could set up
>> inode_operations.permission on tracefs, but what exactly would it do?
>> Would it be just like generic_permission() except that it would look
>> at CAP_TRACING instead of CAP_DAC_OVERRIDE? That is, you can use
>> tracefs if you have CAP_TRACING *or* acl access? Or would it be:
>>
>> int tracing_permission(struct inode *inode, int mask)
>> {
>> if (!capable(CAP_TRACING))
>> return -EPERM;
>>
>> return generic_permission(inode, mask);
>> }
>
> Perhaps we should make a group for it?
>
Hmm. That means that you’d need CAP_TRACING and a group. That’s probably not terrible, but it could be annoying.
>>
>> Which would mean that you need ACL *and* CAP_TRACING, so
>> administrators would change the mode to 777. That's a bit scary.
>>
>> And this still doesn't let people even *find* tracefs, since it's
>> hidden in debugfs.
>>
>> So maybe make CAP_TRACING override ACLs but also add /sys/fs/tracing
>> and mount tracefs there, too, so that regular users can at least find
>> the mountpoint.
>
> I think you missed what I said. It's not hidden in /sys/kernel/debug.
> If you enable tracefs, you have /sys/kernel/tracing created, and is
> completely separate from debugfs. I only have it *also* automatically
> mounted to /sys/kernel/debug/tracing for backward compatibility
> reasons, as older versions of trace-cmd will only mount debugfs (as
> root), and expect to find it there.
>
> mount -t tracefs nodev /sys/kernel/tracing
Too many slashes :/
A group could work for v1. Maybe all the tools should get updated to use this path?
>
> -- Steve
>
>>
>>>
>>> Should we allow CAP_TRACING access to /proc/kallsyms? as it is helpful
>>> to convert perf and trace-cmd's function pointers into names. Once you
>>> allow tracing of the kernel, hiding /proc/kallsyms is pretty useless.
>>
>> I think we should.
>
^ permalink raw reply
* Re: [PATCH bpf-next] bpf, capabilities: introduce CAP_BPF
From: Andy Lutomirski @ 2019-08-28 0:55 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Andy Lutomirski, Alexei Starovoitov, Kees Cook, LSM List,
James Morris, Jann Horn, Peter Zijlstra, Masami Hiramatsu,
Steven Rostedt, David S. Miller, Daniel Borkmann,
Network Development, bpf, kernel-team, Linux API
In-Reply-To: <20190828003447.htgzsxs5oevn3eys@ast-mbp.dhcp.thefacebook.com>
On Tue, Aug 27, 2019 at 5:34 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Tue, Aug 27, 2019 at 04:01:08PM -0700, Andy Lutomirski wrote:
> > [adding some security and tracing folks to cc]
> >
> > On Tue, Aug 27, 2019 at 1:52 PM Alexei Starovoitov <ast@kernel.org> wrote:
> > >
> > > Introduce CAP_BPF that allows loading all types of BPF programs,
> > > create most map types, load BTF, iterate programs and maps.
> > > CAP_BPF alone is not enough to attach or run programs.
> > >
> > > Networking:
> > >
> > > CAP_BPF and CAP_NET_ADMIN are necessary to:
> > > - attach to cgroup-bpf hooks like INET_INGRESS, INET_SOCK_CREATE, INET4_CONNECT
> > > - run networking bpf programs (like xdp, skb, flow_dissector)
> > >
> > > Tracing:
> > >
> > > CAP_BPF and perf_paranoid_tracepoint_raw() (which is kernel.perf_event_paranoid == -1)
> > > are necessary to:
> > > - attach bpf program to raw tracepoint
> > > - use bpf_trace_printk() in all program types (not only tracing programs)
> > > - create bpf stackmap
> > >
> > > To attach bpf to perf_events perf_event_open() needs to succeed as usual.
> > >
> > > CAP_BPF controls BPF side.
> > > CAP_NET_ADMIN controls intersection where BPF calls into networking.
> > > perf_paranoid_tracepoint_raw controls intersection where BPF calls into tracing.
> > >
> > > In the future CAP_TRACING could be introduced to control
> > > creation of kprobe/uprobe and attaching bpf to perf_events.
> > > In such case bpf_probe_read() thin wrapper would be controlled by CAP_BPF.
> > > Whereas probe_read() would be controlled by CAP_TRACING.
> > > CAP_TRACING would also control generic kprobe+probe_read.
> > > CAP_BPF and CAP_TRACING would be necessary for tracing bpf programs
> > > that want to use bpf_probe_read.
> >
> > First, some high-level review:
> >
> > Can you write up some clear documentation aimed at administrators that
> > says what CAP_BPF does? For example, is it expected that CAP_BPF by
> > itself permits reading all kernel memory?
>
> hmm. the answer is in the sentence you quoted right above.
I was hoping for something in Documentation/admin-guide, not in a
changelog that's hard to find.
>
> > Can you give at least one fully described use case where CAP_BPF
> > solves a real-world problem that is not solved by existing mechanisms?
>
> bpftrace binary would be installed with CAP_BPF and CAP_TRACING.
> bcc tools would be installed with CAP_BPF and CAP_TRACING.
> perf binary would be installed with CAP_TRACING only.
> XDP networking daemon would be installed with CAP_BPF and CAP_NET_ADMIN.
> None of them would need full root.
As in just setting these bits in fscaps? What does this achieve
beyond just installing them setuid-root or with CAP_SYS_ADMIN and
judicious use of capset internally? For that matter, what prevents
unauthorized users from tracing the system if you do this? This just
lets anyone trace the system, which seems like a mistake.
Can you clarify your example or give another one?
>
> > Changing the capability that some existing operation requires could
> > break existing programs. The old capability may need to be accepted
> > as well.
>
> As far as I can see there is no ABI breakage. Please point out
> which line of the patch may break it.
As a more or less arbitrary selection:
void bpf_prog_kallsyms_add(struct bpf_prog *fp)
{
if (!bpf_prog_kallsyms_candidate(fp) ||
- !capable(CAP_SYS_ADMIN))
+ !capable(CAP_BPF))
return;
Before your patch, a task with CAP_SYS_ADMIN could do this. Now it
can't. Per the usual Linux definition of "ABI break", this is an ABI
break if and only if someone actually did this in a context where they
have CAP_SYS_ADMIN but not all capabilities. How confident are you
that no one does things like this?
void bpf_prog_kallsyms_add(struct bpf_prog *fp)
{
if (!bpf_prog_kallsyms_candidate(fp) ||
- !capable(CAP_SYS_ADMIN))
+ !capable(CAP_BPF))
return;
> > > ---
> > > I would prefer to introduce CAP_TRACING soon, since it
> > > will make tracing and networking permission model symmetrical.
> > >
> >
> > Here's my proposal for CAP_TRACING, documentation-style:
> >
> > --- begin ---
> >
> > CAP_TRACING enables a task to use various kernel features to trace
> > running user programs and the kernel itself. CAP_TRACING also enables
> > a task to bypass some speculation attack countermeasures. A task in
> > the init user namespace with CAP_TRACING will be able to tell exactly
> > what kernel code is executed and when, and will be able to read kernel
> > registers and kernel memory. It will, similarly, be able to read the
> > state of other user tasks.
> >
> > Specifically, CAP_TRACING allows the following operations. It may
> > allow more operations in the future:
> >
> > - Full use of perf_event_open(), similarly to the effect of
> > kernel.perf_event_paranoid == -1.
>
> +1
>
> > - Loading and attaching tracing BPF programs, including use of BPF
> > raw tracepoints.
>
> -1
>
> > - Use of BPF stack maps.
>
> -1
>
> > - Use of bpf_probe_read() and bpf_trace_printk().
>
> -1
>
> > - Use of unsafe pointer-to-integer conversions in BPF.
>
> -1
>
> > - Bypassing of BPF's speculation attack hardening measures and
> > constant blinding. (Note: other mechanisms might also allow this.)
>
> -1
> All of the above are allowed by CAP_BPF.
> They are not allowed by CAP_TRACING.
Why? I don't mean to discount your -1, and you may well have a
compelling reason. If so, I'll change my proposal.
From the previous discussion, you want to make progress toward solving
a lot of problems with CAP_BPF. One of them was making BPF
firewalling more generally useful. By making CAP_BPF grant the ability
to read kernel memory, you will make administrators much more nervous
to grant CAP_BPF. Similarly, and correct me if I'm wrong, most of
these capabilities are primarily or only useful for tracing, so I
don't see why users without CAP_TRACING should get them.
bpf_trace_printk(), in particular, even has "trace" in its name :)
Also, if a task has CAP_TRACING, it's expected to be able to trace the
system -- that's the whole point. Why shouldn't it be able to use BPF
to trace the system better?
>
> > CAP_TRACING does not override normal permissions on sysfs or debugfs.
> > This means that, unless a new interface for programming kprobes and
> > such is added, it does not directly allow use of kprobes.
>
> kprobes can be created via perf_event_open already.
> So above statement contradicts your first statement.
Hmm. The way of using perf with kprobes that I'm familiar with is:
# perf probe --add func_name
And this uses "/sys/kernel/debug/tracing//kprobe_events" (with the
double slash!). Is there indeed another way to do this?
Anyway, I didn't mean to exclude any existing perf_event_open()
mechanism -- what I meant was that, without some extension to my
proposal, /sys/kernel/debug/tracing wouldn't magically become
accessible due to CAP_TRACING.
>
> > If CAP_TRACING, by itself, enables a task to crash or otherwise
> > corrupt the kernel or other tasks, this will be considered a kernel
> > bug.
>
> +1
>
> > CAP_TRACING in a non-init user namespace may, in the future, allow
> > tracing of other tasks in that user namespace or its descendants. It
> > will not enable kernel tracing or tracing of tasks outside the user
> > namespace in question.
>
> I would avoid describing user ns for now.
> There is enough confusion without it.
>
> > --- end ---
> >
> > Does this sound good? The idea here is that CAP_TRACING should be
> > very useful even without CAP_BPF, which allows CAP_BPF to be less
> > powerful.
>
> As proposed CAP_BPF does not allow tracing or networking on its own.
> CAP_BPF only controls BPF side.
>
> For example:
> BPF_CALL_3(bpf_probe_read, void *, dst, u32, size, const void *, unsafe_ptr)
> {
> int ret;
>
> ret = probe_kernel_read(dst, unsafe_ptr, size);
> if (unlikely(ret < 0))
> memset(dst, 0, size);
>
> return ret;
> }
>
> All of BPF (including prototype of bpf_probe_read) is controlled by CAP_BPF.
> But the kernel primitives its using (probe_kernel_read) is controlled by CAP_TRACING.
> Hence a task needs _both_ CAP_BPF and CAP_TRACING to attach and run bpf program
> that uses bpf_probe_read.
>
> Similar with all other kernel code that BPF helpers may call directly or indirectly.
> If there is a way for bpf program to call into piece of code controlled by CAP_TRACING
> such helper would need CAP_BPF and CAP_TRACING.
> If bpf helper calls into something that may mangle networking packet
> such helper would need both CAP_BPF and CAP_NET_ADMIN to execute.
Why do you want to require CAP_BPF to call into functions like
bpf_probe_read()? I understand why you want to limit access to bpf,
but I think that CAP_TRACING should be sufficient to allow the tracing
parts of BPF. After all, a lot of your concerns, especially the ones
involving speculation, don't really apply to users with CAP_TRACING --
users with CAP_TRACING can read kernel memory with or without bpf.
>
> > > @@ -2080,7 +2083,10 @@ static int bpf_prog_test_run(const union bpf_attr *attr,
> > > struct bpf_prog *prog;
> > > int ret = -ENOTSUPP;
> > >
> > > - if (!capable(CAP_SYS_ADMIN))
> > > + if (!capable(CAP_NET_ADMIN) || !capable(CAP_BPF))
> > > + /* test_run callback is available for networking progs only.
> > > + * Add cap_bpf_tracing() above when tracing progs become runable.
> > > + */
> >
> > I think test_run should probably be CAP_SYS_ADMIN forever. test_run
> > is the only way that one can run a bpf program and call helper
> > functions via the program if one doesn't have permission to attach the
> > program.
>
> Since CAP_BPF + CAP_NET_ADMIN allow attach. It means that a task
> with these two permissions will have programs running anyway.
> (traffic will flow through netdev, socket events will happen, etc)
> Hence no reason to disallow running program via test_run.
>
test_run allows fully controlled inputs, in a context where a program
can trivially flush caches, mistrain branch predictors, etc first. It
seems to me that, if a JITted bpf program contains an exploitable
speculation gadget (MDS, Spectre v1, RSB, or anything else), it will
be *much* easier to exploit it using test_run than using normal
network traffic. Similarly, normal network traffic will have network
headers that are valid enough to have caused the BPF program to be
invoked in the first place. test_run can inject arbitrary garbage.
^ permalink raw reply
* Re: [PATCH bpf-next] bpf, capabilities: introduce CAP_BPF
From: Steven Rostedt @ 2019-08-28 0:44 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Alexei Starovoitov, Kees Cook, LSM List, James Morris, Jann Horn,
Peter Zijlstra, Masami Hiramatsu, David S. Miller,
Daniel Borkmann, Network Development, bpf, kernel-team, Linux API
In-Reply-To: <CALCETrUOHRMkBRJi_s30CjZdOLDGtdMOEgqfgPf+q0x+Fw7LtQ@mail.gmail.com>
On Tue, 27 Aug 2019 16:34:47 -0700
Andy Lutomirski <luto@kernel.org> wrote:
> > > CAP_TRACING does not override normal permissions on sysfs or debugfs.
> > > This means that, unless a new interface for programming kprobes and
> > > such is added, it does not directly allow use of kprobes.
> >
> > kprobes can be created in the tracefs filesystem (which is separate from
> > debugfs, tracefs just gets automatically mounted
> > in /sys/kernel/debug/tracing when debugfs is mounted) from the
> > kprobe_events file. /sys/kernel/tracing is just the tracefs
> > directory without debugfs, and was created specifically to allow
> > tracing to be access without opening up the can of worms in debugfs.
>
> I think that, in principle, CAP_TRACING should allow this, but I'm not
> sure how to achieve that. I suppose we could set up
> inode_operations.permission on tracefs, but what exactly would it do?
> Would it be just like generic_permission() except that it would look
> at CAP_TRACING instead of CAP_DAC_OVERRIDE? That is, you can use
> tracefs if you have CAP_TRACING *or* acl access? Or would it be:
>
> int tracing_permission(struct inode *inode, int mask)
> {
> if (!capable(CAP_TRACING))
> return -EPERM;
>
> return generic_permission(inode, mask);
> }
Perhaps we should make a group for it?
>
> Which would mean that you need ACL *and* CAP_TRACING, so
> administrators would change the mode to 777. That's a bit scary.
>
> And this still doesn't let people even *find* tracefs, since it's
> hidden in debugfs.
>
> So maybe make CAP_TRACING override ACLs but also add /sys/fs/tracing
> and mount tracefs there, too, so that regular users can at least find
> the mountpoint.
I think you missed what I said. It's not hidden in /sys/kernel/debug.
If you enable tracefs, you have /sys/kernel/tracing created, and is
completely separate from debugfs. I only have it *also* automatically
mounted to /sys/kernel/debug/tracing for backward compatibility
reasons, as older versions of trace-cmd will only mount debugfs (as
root), and expect to find it there.
mount -t tracefs nodev /sys/kernel/tracing
-- Steve
>
> >
> > Should we allow CAP_TRACING access to /proc/kallsyms? as it is helpful
> > to convert perf and trace-cmd's function pointers into names. Once you
> > allow tracing of the kernel, hiding /proc/kallsyms is pretty useless.
>
> I think we should.
^ permalink raw reply
* Re: [PATCH bpf-next] bpf, capabilities: introduce CAP_BPF
From: Alexei Starovoitov @ 2019-08-28 0:38 UTC (permalink / raw)
To: Steven Rostedt
Cc: Andy Lutomirski, Alexei Starovoitov, Kees Cook, LSM List,
James Morris, Jann Horn, Peter Zijlstra, Masami Hiramatsu,
David S. Miller, Daniel Borkmann, Network Development, bpf,
kernel-team, Linux API
In-Reply-To: <20190827192144.3b38b25a@gandalf.local.home>
On Tue, Aug 27, 2019 at 07:21:44PM -0400, Steven Rostedt wrote:
>
> At least for CAP_TRACING (if it were to allow read/write access
> to /sys/kernel/tracing), that would be very useful. It would be useful
> to those that basically own their machines, and want to trace their
> applications all the way into the kernel without having to run as full
> root.
+1
The proposal is to have CAP_TRACING to control perf and ftrace.
perf and trace-cmd binaries could be installed with CAP_TRACING and that's
all they need to do full tracing.
I can craft a patch for perf_event_open side and demo CAP_TRACING.
Once that cap bit is ready you can use it on ftrace side?
> Should we allow CAP_TRACING access to /proc/kallsyms? as it is helpful
> to convert perf and trace-cmd's function pointers into names. Once you
> allow tracing of the kernel, hiding /proc/kallsyms is pretty useless.
yep.
^ permalink raw reply
* Re: [PATCH bpf-next] bpf, capabilities: introduce CAP_BPF
From: Alexei Starovoitov @ 2019-08-28 0:34 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Alexei Starovoitov, Kees Cook, LSM List, James Morris, Jann Horn,
Peter Zijlstra, Masami Hiramatsu, Steven Rostedt, David S. Miller,
Daniel Borkmann, Network Development, bpf, kernel-team, Linux API
In-Reply-To: <CALCETrV8iJv9+Ai11_1_r6MapPhhwt9hjxi=6EoixytabTScqg@mail.gmail.com>
On Tue, Aug 27, 2019 at 04:01:08PM -0700, Andy Lutomirski wrote:
> [adding some security and tracing folks to cc]
>
> On Tue, Aug 27, 2019 at 1:52 PM Alexei Starovoitov <ast@kernel.org> wrote:
> >
> > Introduce CAP_BPF that allows loading all types of BPF programs,
> > create most map types, load BTF, iterate programs and maps.
> > CAP_BPF alone is not enough to attach or run programs.
> >
> > Networking:
> >
> > CAP_BPF and CAP_NET_ADMIN are necessary to:
> > - attach to cgroup-bpf hooks like INET_INGRESS, INET_SOCK_CREATE, INET4_CONNECT
> > - run networking bpf programs (like xdp, skb, flow_dissector)
> >
> > Tracing:
> >
> > CAP_BPF and perf_paranoid_tracepoint_raw() (which is kernel.perf_event_paranoid == -1)
> > are necessary to:
> > - attach bpf program to raw tracepoint
> > - use bpf_trace_printk() in all program types (not only tracing programs)
> > - create bpf stackmap
> >
> > To attach bpf to perf_events perf_event_open() needs to succeed as usual.
> >
> > CAP_BPF controls BPF side.
> > CAP_NET_ADMIN controls intersection where BPF calls into networking.
> > perf_paranoid_tracepoint_raw controls intersection where BPF calls into tracing.
> >
> > In the future CAP_TRACING could be introduced to control
> > creation of kprobe/uprobe and attaching bpf to perf_events.
> > In such case bpf_probe_read() thin wrapper would be controlled by CAP_BPF.
> > Whereas probe_read() would be controlled by CAP_TRACING.
> > CAP_TRACING would also control generic kprobe+probe_read.
> > CAP_BPF and CAP_TRACING would be necessary for tracing bpf programs
> > that want to use bpf_probe_read.
>
> First, some high-level review:
>
> Can you write up some clear documentation aimed at administrators that
> says what CAP_BPF does? For example, is it expected that CAP_BPF by
> itself permits reading all kernel memory?
hmm. the answer is in the sentence you quoted right above.
> Can you give at least one fully described use case where CAP_BPF
> solves a real-world problem that is not solved by existing mechanisms?
bpftrace binary would be installed with CAP_BPF and CAP_TRACING.
bcc tools would be installed with CAP_BPF and CAP_TRACING.
perf binary would be installed with CAP_TRACING only.
XDP networking daemon would be installed with CAP_BPF and CAP_NET_ADMIN.
None of them would need full root.
> Changing the capability that some existing operation requires could
> break existing programs. The old capability may need to be accepted
> as well.
As far as I can see there is no ABI breakage. Please point out
which line of the patch may break it.
> I'm inclined to suggest that CAP_TRACING be figured out or rejected
> before something like this gets applied.
that's fair.
>
> >
> > Signed-off-by: Alexei Starovoitov <ast@kernel.org>
> > ---
> > I would prefer to introduce CAP_TRACING soon, since it
> > will make tracing and networking permission model symmetrical.
> >
>
> Here's my proposal for CAP_TRACING, documentation-style:
>
> --- begin ---
>
> CAP_TRACING enables a task to use various kernel features to trace
> running user programs and the kernel itself. CAP_TRACING also enables
> a task to bypass some speculation attack countermeasures. A task in
> the init user namespace with CAP_TRACING will be able to tell exactly
> what kernel code is executed and when, and will be able to read kernel
> registers and kernel memory. It will, similarly, be able to read the
> state of other user tasks.
>
> Specifically, CAP_TRACING allows the following operations. It may
> allow more operations in the future:
>
> - Full use of perf_event_open(), similarly to the effect of
> kernel.perf_event_paranoid == -1.
+1
> - Loading and attaching tracing BPF programs, including use of BPF
> raw tracepoints.
-1
> - Use of BPF stack maps.
-1
> - Use of bpf_probe_read() and bpf_trace_printk().
-1
> - Use of unsafe pointer-to-integer conversions in BPF.
-1
> - Bypassing of BPF's speculation attack hardening measures and
> constant blinding. (Note: other mechanisms might also allow this.)
-1
All of the above are allowed by CAP_BPF.
They are not allowed by CAP_TRACING.
> CAP_TRACING does not override normal permissions on sysfs or debugfs.
> This means that, unless a new interface for programming kprobes and
> such is added, it does not directly allow use of kprobes.
kprobes can be created via perf_event_open already.
So above statement contradicts your first statement.
> If CAP_TRACING, by itself, enables a task to crash or otherwise
> corrupt the kernel or other tasks, this will be considered a kernel
> bug.
+1
> CAP_TRACING in a non-init user namespace may, in the future, allow
> tracing of other tasks in that user namespace or its descendants. It
> will not enable kernel tracing or tracing of tasks outside the user
> namespace in question.
I would avoid describing user ns for now.
There is enough confusion without it.
> --- end ---
>
> Does this sound good? The idea here is that CAP_TRACING should be
> very useful even without CAP_BPF, which allows CAP_BPF to be less
> powerful.
As proposed CAP_BPF does not allow tracing or networking on its own.
CAP_BPF only controls BPF side.
For example:
BPF_CALL_3(bpf_probe_read, void *, dst, u32, size, const void *, unsafe_ptr)
{
int ret;
ret = probe_kernel_read(dst, unsafe_ptr, size);
if (unlikely(ret < 0))
memset(dst, 0, size);
return ret;
}
All of BPF (including prototype of bpf_probe_read) is controlled by CAP_BPF.
But the kernel primitives its using (probe_kernel_read) is controlled by CAP_TRACING.
Hence a task needs _both_ CAP_BPF and CAP_TRACING to attach and run bpf program
that uses bpf_probe_read.
Similar with all other kernel code that BPF helpers may call directly or indirectly.
If there is a way for bpf program to call into piece of code controlled by CAP_TRACING
such helper would need CAP_BPF and CAP_TRACING.
If bpf helper calls into something that may mangle networking packet
such helper would need both CAP_BPF and CAP_NET_ADMIN to execute.
> > @@ -2080,7 +2083,10 @@ static int bpf_prog_test_run(const union bpf_attr *attr,
> > struct bpf_prog *prog;
> > int ret = -ENOTSUPP;
> >
> > - if (!capable(CAP_SYS_ADMIN))
> > + if (!capable(CAP_NET_ADMIN) || !capable(CAP_BPF))
> > + /* test_run callback is available for networking progs only.
> > + * Add cap_bpf_tracing() above when tracing progs become runable.
> > + */
>
> I think test_run should probably be CAP_SYS_ADMIN forever. test_run
> is the only way that one can run a bpf program and call helper
> functions via the program if one doesn't have permission to attach the
> program.
Since CAP_BPF + CAP_NET_ADMIN allow attach. It means that a task
with these two permissions will have programs running anyway.
(traffic will flow through netdev, socket events will happen, etc)
Hence no reason to disallow running program via test_run.
^ permalink raw reply
* Re: [PATCH bpf-next] bpf, capabilities: introduce CAP_BPF
From: Andy Lutomirski @ 2019-08-27 23:34 UTC (permalink / raw)
To: Steven Rostedt
Cc: Andy Lutomirski, Alexei Starovoitov, Kees Cook, LSM List,
James Morris, Jann Horn, Peter Zijlstra, Masami Hiramatsu,
David S. Miller, Daniel Borkmann, Network Development, bpf,
kernel-team, Linux API
In-Reply-To: <20190827192144.3b38b25a@gandalf.local.home>
On Tue, Aug 27, 2019 at 4:21 PM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Tue, 27 Aug 2019 16:01:08 -0700
> Andy Lutomirski <luto@kernel.org> wrote:
>
> > [adding some security and tracing folks to cc]
> >
> > On Tue, Aug 27, 2019 at 1:52 PM Alexei Starovoitov <ast@kernel.org> wrote:
> > >
> > > Introduce CAP_BPF that allows loading all types of BPF programs,
> > > create most map types, load BTF, iterate programs and maps.
> > > CAP_BPF alone is not enough to attach or run programs.
> > >
> > > Networking:
> > >
> > > CAP_BPF and CAP_NET_ADMIN are necessary to:
> > > - attach to cgroup-bpf hooks like INET_INGRESS, INET_SOCK_CREATE, INET4_CONNECT
> > > - run networking bpf programs (like xdp, skb, flow_dissector)
> > >
> > > Tracing:
> > >
> > > CAP_BPF and perf_paranoid_tracepoint_raw() (which is kernel.perf_event_paranoid == -1)
> > > are necessary to:
> > > - attach bpf program to raw tracepoint
> > > - use bpf_trace_printk() in all program types (not only tracing programs)
> > > - create bpf stackmap
> > >
> > > To attach bpf to perf_events perf_event_open() needs to succeed as usual.
> > >
> > > CAP_BPF controls BPF side.
> > > CAP_NET_ADMIN controls intersection where BPF calls into networking.
> > > perf_paranoid_tracepoint_raw controls intersection where BPF calls into tracing.
> > >
> > > In the future CAP_TRACING could be introduced to control
> > > creation of kprobe/uprobe and attaching bpf to perf_events.
> > > In such case bpf_probe_read() thin wrapper would be controlled by CAP_BPF.
> > > Whereas probe_read() would be controlled by CAP_TRACING.
> > > CAP_TRACING would also control generic kprobe+probe_read.
> > > CAP_BPF and CAP_TRACING would be necessary for tracing bpf programs
> > > that want to use bpf_probe_read.
>
> No mention of the tracefs (/sys/kernel/tracing) file?
See below. Also, I am embarrassed to admit that I just assumed that
/sys/kernel/debug/tracing was just like any other debugfs directory.
>
>
> >
> > Changing the capability that some existing operation requires could
> > break existing programs. The old capability may need to be accepted
> > as well.
> >
> > I'm inclined to suggest that CAP_TRACING be figured out or rejected
> > before something like this gets applied.
> >
> >
> > >
> > > Signed-off-by: Alexei Starovoitov <ast@kernel.org>
> > > ---
> > > I would prefer to introduce CAP_TRACING soon, since it
> > > will make tracing and networking permission model symmetrical.
> > >
> >
> > Here's my proposal for CAP_TRACING, documentation-style:
> >
> > --- begin ---
> >
> > CAP_TRACING enables a task to use various kernel features to trace
> > running user programs and the kernel itself. CAP_TRACING also enables
> > a task to bypass some speculation attack countermeasures. A task in
> > the init user namespace with CAP_TRACING will be able to tell exactly
> > what kernel code is executed and when, and will be able to read kernel
> > registers and kernel memory. It will, similarly, be able to read the
> > state of other user tasks.
> >
> > Specifically, CAP_TRACING allows the following operations. It may
> > allow more operations in the future:
> >
> > - Full use of perf_event_open(), similarly to the effect of
> > kernel.perf_event_paranoid == -1.
> >
> > - Loading and attaching tracing BPF programs, including use of BPF
> > raw tracepoints.
> >
> > - Use of BPF stack maps.
> >
> > - Use of bpf_probe_read() and bpf_trace_printk().
> >
> > - Use of unsafe pointer-to-integer conversions in BPF.
> >
> > - Bypassing of BPF's speculation attack hardening measures and
> > constant blinding. (Note: other mechanisms might also allow this.)
> >
> > CAP_TRACING does not override normal permissions on sysfs or debugfs.
> > This means that, unless a new interface for programming kprobes and
> > such is added, it does not directly allow use of kprobes.
>
> kprobes can be created in the tracefs filesystem (which is separate from
> debugfs, tracefs just gets automatically mounted
> in /sys/kernel/debug/tracing when debugfs is mounted) from the
> kprobe_events file. /sys/kernel/tracing is just the tracefs
> directory without debugfs, and was created specifically to allow
> tracing to be access without opening up the can of worms in debugfs.
I think that, in principle, CAP_TRACING should allow this, but I'm not
sure how to achieve that. I suppose we could set up
inode_operations.permission on tracefs, but what exactly would it do?
Would it be just like generic_permission() except that it would look
at CAP_TRACING instead of CAP_DAC_OVERRIDE? That is, you can use
tracefs if you have CAP_TRACING *or* acl access? Or would it be:
int tracing_permission(struct inode *inode, int mask)
{
if (!capable(CAP_TRACING))
return -EPERM;
return generic_permission(inode, mask);
}
Which would mean that you need ACL *and* CAP_TRACING, so
administrators would change the mode to 777. That's a bit scary.
And this still doesn't let people even *find* tracefs, since it's
hidden in debugfs.
So maybe make CAP_TRACING override ACLs but also add /sys/fs/tracing
and mount tracefs there, too, so that regular users can at least find
the mountpoint.
>
> Should we allow CAP_TRACING access to /proc/kallsyms? as it is helpful
> to convert perf and trace-cmd's function pointers into names. Once you
> allow tracing of the kernel, hiding /proc/kallsyms is pretty useless.
I think we should.
^ permalink raw reply
* Re: [PATCH bpf-next] bpf, capabilities: introduce CAP_BPF
From: Steven Rostedt @ 2019-08-27 23:21 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Alexei Starovoitov, Kees Cook, LSM List, James Morris, Jann Horn,
Peter Zijlstra, Masami Hiramatsu, David S. Miller,
Daniel Borkmann, Network Development, bpf, kernel-team, Linux API
In-Reply-To: <CALCETrV8iJv9+Ai11_1_r6MapPhhwt9hjxi=6EoixytabTScqg@mail.gmail.com>
On Tue, 27 Aug 2019 16:01:08 -0700
Andy Lutomirski <luto@kernel.org> wrote:
> [adding some security and tracing folks to cc]
>
> On Tue, Aug 27, 2019 at 1:52 PM Alexei Starovoitov <ast@kernel.org> wrote:
> >
> > Introduce CAP_BPF that allows loading all types of BPF programs,
> > create most map types, load BTF, iterate programs and maps.
> > CAP_BPF alone is not enough to attach or run programs.
> >
> > Networking:
> >
> > CAP_BPF and CAP_NET_ADMIN are necessary to:
> > - attach to cgroup-bpf hooks like INET_INGRESS, INET_SOCK_CREATE, INET4_CONNECT
> > - run networking bpf programs (like xdp, skb, flow_dissector)
> >
> > Tracing:
> >
> > CAP_BPF and perf_paranoid_tracepoint_raw() (which is kernel.perf_event_paranoid == -1)
> > are necessary to:
> > - attach bpf program to raw tracepoint
> > - use bpf_trace_printk() in all program types (not only tracing programs)
> > - create bpf stackmap
> >
> > To attach bpf to perf_events perf_event_open() needs to succeed as usual.
> >
> > CAP_BPF controls BPF side.
> > CAP_NET_ADMIN controls intersection where BPF calls into networking.
> > perf_paranoid_tracepoint_raw controls intersection where BPF calls into tracing.
> >
> > In the future CAP_TRACING could be introduced to control
> > creation of kprobe/uprobe and attaching bpf to perf_events.
> > In such case bpf_probe_read() thin wrapper would be controlled by CAP_BPF.
> > Whereas probe_read() would be controlled by CAP_TRACING.
> > CAP_TRACING would also control generic kprobe+probe_read.
> > CAP_BPF and CAP_TRACING would be necessary for tracing bpf programs
> > that want to use bpf_probe_read.
No mention of the tracefs (/sys/kernel/tracing) file?
>
> First, some high-level review:
>
> Can you write up some clear documentation aimed at administrators that
> says what CAP_BPF does? For example, is it expected that CAP_BPF by
> itself permits reading all kernel memory? Why might one grant it?
>
> Can you give at least one fully described use case where CAP_BPF
> solves a real-world problem that is not solved by existing mechanisms?
At least for CAP_TRACING (if it were to allow read/write access
to /sys/kernel/tracing), that would be very useful. It would be useful
to those that basically own their machines, and want to trace their
applications all the way into the kernel without having to run as full
root.
>
> Changing the capability that some existing operation requires could
> break existing programs. The old capability may need to be accepted
> as well.
>
> I'm inclined to suggest that CAP_TRACING be figured out or rejected
> before something like this gets applied.
>
>
> >
> > Signed-off-by: Alexei Starovoitov <ast@kernel.org>
> > ---
> > I would prefer to introduce CAP_TRACING soon, since it
> > will make tracing and networking permission model symmetrical.
> >
>
> Here's my proposal for CAP_TRACING, documentation-style:
>
> --- begin ---
>
> CAP_TRACING enables a task to use various kernel features to trace
> running user programs and the kernel itself. CAP_TRACING also enables
> a task to bypass some speculation attack countermeasures. A task in
> the init user namespace with CAP_TRACING will be able to tell exactly
> what kernel code is executed and when, and will be able to read kernel
> registers and kernel memory. It will, similarly, be able to read the
> state of other user tasks.
>
> Specifically, CAP_TRACING allows the following operations. It may
> allow more operations in the future:
>
> - Full use of perf_event_open(), similarly to the effect of
> kernel.perf_event_paranoid == -1.
>
> - Loading and attaching tracing BPF programs, including use of BPF
> raw tracepoints.
>
> - Use of BPF stack maps.
>
> - Use of bpf_probe_read() and bpf_trace_printk().
>
> - Use of unsafe pointer-to-integer conversions in BPF.
>
> - Bypassing of BPF's speculation attack hardening measures and
> constant blinding. (Note: other mechanisms might also allow this.)
>
> CAP_TRACING does not override normal permissions on sysfs or debugfs.
> This means that, unless a new interface for programming kprobes and
> such is added, it does not directly allow use of kprobes.
kprobes can be created in the tracefs filesystem (which is separate from
debugfs, tracefs just gets automatically mounted
in /sys/kernel/debug/tracing when debugfs is mounted) from the
kprobe_events file. /sys/kernel/tracing is just the tracefs
directory without debugfs, and was created specifically to allow
tracing to be access without opening up the can of worms in debugfs.
Should we allow CAP_TRACING access to /proc/kallsyms? as it is helpful
to convert perf and trace-cmd's function pointers into names. Once you
allow tracing of the kernel, hiding /proc/kallsyms is pretty useless.
-- Steve
>
> If CAP_TRACING, by itself, enables a task to crash or otherwise
> corrupt the kernel or other tasks, this will be considered a kernel
> bug.
>
> CAP_TRACING in a non-init user namespace may, in the future, allow
> tracing of other tasks in that user namespace or its descendants. It
> will not enable kernel tracing or tracing of tasks outside the user
> namespace in question.
>
> --- end ---
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox