From: Alexei Starovoitov <alexei.starovoitov@gmail.com>
To: YiFei Zhu <zhuyifei1999@gmail.com>
Cc: containers@lists.linux.dev, bpf <bpf@vger.kernel.org>,
YiFei Zhu <yifeifz2@illinois.edu>,
LSM List <linux-security-module@vger.kernel.org>,
Alexei Starovoitov <ast@kernel.org>,
Andrea Arcangeli <aarcange@redhat.com>,
Andy Lutomirski <luto@amacapital.net>,
Austin Kuo <hckuo2@illinois.edu>,
Claudio Canella <claudio.canella@iaik.tugraz.at>,
Daniel Borkmann <daniel@iogearbox.net>,
Daniel Gruss <daniel.gruss@iaik.tugraz.at>,
Dimitrios Skarlatos <dskarlat@cs.cmu.edu>,
Giuseppe Scrivano <gscrivan@redhat.com>,
Hubertus Franke <frankeh@us.ibm.com>,
Jann Horn <jannh@google.com>, Jinghao Jia <jinghao7@illinois.edu>,
Josep Torrellas <torrella@illinois.edu>,
Kees Cook <keescook@chromium.org>,
Sargun Dhillon <sargun@sargun.me>, Tianyin Xu <tyxu@illinois.edu>,
Tobin Feldman-Fitzthum <tobin@ibm.com>,
Tom Hromatka <tom.hromatka@oracle.com>,
Will Drewry <wad@chromium.org>
Subject: Re: [RFC PATCH bpf-next seccomp 10/12] seccomp-ebpf: Add ability to read user memory
Date: Wed, 12 May 2021 15:36:26 -0700 [thread overview]
Message-ID: <20210512223626.olex7ewf6xd6m2c4@ast-mbp.dhcp.thefacebook.com> (raw)
In-Reply-To: <CABqSeAT8iz-VhWjWqABqGbF7ydkoT7LmzJ5Do8K1ANQvQK=FJQ@mail.gmail.com>
On Tue, May 11, 2021 at 02:14:01AM -0500, YiFei Zhu wrote:
> On Mon, May 10, 2021 at 9:04 PM Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
> >
> > On Mon, May 10, 2021 at 12:22:47PM -0500, YiFei Zhu wrote:
> > >
> > > +BPF_CALL_3(bpf_probe_read_user_dumpable, void *, dst, u32, size,
> > > + const void __user *, unsafe_ptr)
> > > +{
> > > + int ret = -EPERM;
> > > +
> > > + if (get_dumpable(current->mm))
> > > + ret = copy_from_user_nofault(dst, unsafe_ptr, size);
> >
> > Could you explain a bit more how dumpable flag makes it safe for unpriv?
> > The unpriv prog is attached to the children tasks only, right?
> > and dumpable gets cleared if euid changes?
>
> This is the "reduction to ptrace". The model here is that the eBPF
> seccomp filter is doing the equivalent of ptracing the user process
> using the privileges of the task at the time of loading the seccomp
> filter.
>
> ptrace access control is governed by ptrace.c:__ptrace_may_access. The
> requirements are:
> * always allow thread group introspection -- assume false so we are
> more restrictive than ptrace.
> * tracer has CAP_PTRACE in the target user namespace or tracer
> r/fsu/gidid equal target resu/gid -- discuss below
> * tracer has CAP_PTRACE in the target user namespace or target is
> SUID_DUMP_USER (I realized I should probably change the condition to
> == SUID_DUMP_USER).
> * passes LSM checks (eg yama ptrace_scope) -- we expose a hook to LSM
> but it's more of a "disable all advanced seccomp-eBPF features". How
> would a better interface to LSM look like?
>
> The dumpable check handles the "target is SUID_DUMP_USER" condition,
> in the circumstance that the loader does not have CAP_PTRACE in its
> namespace at the time of load. Why would this imply its CAP_PTRACE
> capability in target namespace? This is based on my understanding on
> how capabilities and user namespaces interact:
> For the sake of simplicity, let's first assume that loader is the same
> task as the task that attaches the filter (via prctl or seccomp
> syscall).
> * Case 1: target and loader are the same user namespace. Trivial case,
> the two operations are the same.
> * Case 2: target is loader's parent namespace. Can't happen under
> assumption. Seccomp affects itself and children only, and it is only
> possible to join a descendant user ns.
> * Case 3: target is loader's descendant namespace. Loader would have
> full CAP_PTRACE on target. We are more restrictive than ptrace.
> * Case 4: target and loader are on unrelated namespace branches. Can't
> happen under assumption. Same as case 2.
>
> Let's break this assumption and see what happens if the loader and
> attacher are in different contexts:
> * Case 1: attacher is less capable (as a general term of "what it can
> do") than loader then all of the above applies, since the model
> concerns and checks the capabilities of the loader.
> * Case 2: attacher is more capable than loader. The attacher would
> need an fd to the prog to attach it:
> * subcase 1: attacher inherited the fd after an exec and became more
> capable. uh... why is it trusting fds from a less capable context?
> * subcase 2: attacher has CAP_SYS_ADMIN and gets the fd via
> BPF_PROG_GET_FD_BY_ID. uh... why is it trusting random fds and
> attaching it?
> * subcase 3: attacher received the fd via a domain socket from a
> process which may be in a different user namespace. On my first
> thought, I thought, why is it trusting random fds from a less capable
> context? Except I just thought of an adversary could:
> * Clone into new userns,
> * Load filter in child, which has CAP_PTRACE in new userns
> * Send filter to the parent which doesn't have CAP_PTRACE in its userns
> * It's broken :(
> We'll think more about this case. One way is to check against init
> namespace, which means unpriv container runtimes won't have the
> non-dumpable override. Though, it shouldn't be affecting most of the
> use cases. Alternatively we can store which userns it was loaded from
> and reject attaching from a different userns.
Typically the verifier does all the checks at load time to avoid
run-time overhead during program execution. Then at attach time we
check that attach parameters provided at load time match exactly
to those at attach time. ifindex, attach_btf_id, etc fall into this category.
Doing something similar it should be possible to avoid
doing get_dumpable() at run-time.
next prev parent reply other threads:[~2021-05-12 23:11 UTC|newest]
Thread overview: 39+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-05-10 17:22 [RFC PATCH bpf-next seccomp 00/12] eBPF seccomp filters YiFei Zhu
2021-05-10 17:22 ` [RFC PATCH bpf-next seccomp 01/12] seccomp: Move no_new_privs check to after prepare_filter YiFei Zhu
2021-05-10 17:22 ` [RFC PATCH bpf-next seccomp 02/12] bpf, seccomp: Add eBPF filter capabilities YiFei Zhu
2021-05-10 17:22 ` [RFC PATCH bpf-next seccomp 03/12] seccomp, ptrace: Add a mechanism to retrieve attached eBPF seccomp filters YiFei Zhu
2021-05-10 17:22 ` [RFC PATCH bpf-next seccomp 04/12] libbpf: recognize section "seccomp" YiFei Zhu
2021-05-10 17:22 ` [RFC PATCH bpf-next seccomp 05/12] samples/bpf: Add eBPF seccomp sample programs YiFei Zhu
2021-05-10 17:22 ` [RFC PATCH bpf-next seccomp 06/12] lsm: New hook seccomp_extended YiFei Zhu
2021-05-10 17:22 ` [RFC PATCH bpf-next seccomp 07/12] bpf/verifier: allow restricting direct map access YiFei Zhu
2021-05-10 17:22 ` [RFC PATCH bpf-next seccomp 08/12] seccomp-ebpf: restrict filter to almost cBPF if LSM request such YiFei Zhu
2021-05-10 17:22 ` [RFC PATCH bpf-next seccomp 09/12] yama: (concept) restrict seccomp-eBPF with ptrace_scope YiFei Zhu
2021-05-10 17:22 ` [RFC PATCH bpf-next seccomp 10/12] seccomp-ebpf: Add ability to read user memory YiFei Zhu
2021-05-11 2:04 ` Alexei Starovoitov
2021-05-11 7:14 ` YiFei Zhu
2021-05-12 22:36 ` Alexei Starovoitov [this message]
2021-05-13 5:26 ` YiFei Zhu
2021-05-13 14:53 ` Andy Lutomirski
2021-05-13 17:12 ` YiFei Zhu
2021-05-13 17:15 ` Andy Lutomirski
2021-05-10 17:22 ` [RFC PATCH bpf-next seccomp 11/12] bpf/verifier: support NULL-able ptr to BTF ID as helper argument YiFei Zhu
2021-05-10 17:22 ` [RFC PATCH bpf-next seccomp 12/12] seccomp-ebpf: support task storage from BPF-LSM, defaulting to group leader YiFei Zhu
2021-05-11 1:58 ` Alexei Starovoitov
2021-05-11 5:44 ` YiFei Zhu
2021-05-12 21:56 ` Alexei Starovoitov
2021-05-10 17:47 ` [RFC PATCH bpf-next seccomp 00/12] eBPF seccomp filters Andy Lutomirski
2021-05-11 5:21 ` YiFei Zhu
2021-05-15 15:49 ` Andy Lutomirski
2021-05-20 9:05 ` Christian Brauner
[not found] ` <fffbea8189794a8da539f6082af3de8e@DM5PR11MB1692.namprd11.prod.outlook.com>
2021-05-16 8:38 ` Tianyin Xu
2021-05-17 15:40 ` Tycho Andersen
2021-05-17 17:07 ` Sargun Dhillon
[not found] ` <108b4b9c2daa4123805d2b92cf51374b@DM5PR11MB1692.namprd11.prod.outlook.com>
2021-05-20 8:16 ` Tianyin Xu
2021-05-20 8:56 ` Christian Brauner
2021-05-20 9:37 ` Christian Brauner
2021-06-01 19:55 ` Kees Cook
2021-06-09 6:32 ` Jinghao Jia
2021-06-09 6:27 ` Jinghao Jia
[not found] ` <00fe481c572d486289bc88780f48e88f@DM5PR11MB1692.namprd11.prod.outlook.com>
2021-05-20 22:13 ` Tianyin Xu
[not found] ` <eae2a0e5038b41c4af87edcb3d4cdc13@DM5PR11MB1692.namprd11.prod.outlook.com>
2021-05-20 8:22 ` Tianyin Xu
2021-05-24 18:55 ` Sargun Dhillon
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=20210512223626.olex7ewf6xd6m2c4@ast-mbp.dhcp.thefacebook.com \
--to=alexei.starovoitov@gmail.com \
--cc=aarcange@redhat.com \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=claudio.canella@iaik.tugraz.at \
--cc=containers@lists.linux.dev \
--cc=daniel.gruss@iaik.tugraz.at \
--cc=daniel@iogearbox.net \
--cc=dskarlat@cs.cmu.edu \
--cc=frankeh@us.ibm.com \
--cc=gscrivan@redhat.com \
--cc=hckuo2@illinois.edu \
--cc=jannh@google.com \
--cc=jinghao7@illinois.edu \
--cc=keescook@chromium.org \
--cc=linux-security-module@vger.kernel.org \
--cc=luto@amacapital.net \
--cc=sargun@sargun.me \
--cc=tobin@ibm.com \
--cc=tom.hromatka@oracle.com \
--cc=torrella@illinois.edu \
--cc=tyxu@illinois.edu \
--cc=wad@chromium.org \
--cc=yifeifz2@illinois.edu \
--cc=zhuyifei1999@gmail.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).