Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20230326092208.13613-1-laoar.shao@gmail.com> <ZCHSOxto9DlEzVy9@google.com>
 <CALOAHbBJEXdR4Myk1zrgFDf9UJYu2-3tbjz0ETNvK3WamD5sFg@mail.gmail.com>
 <ZCMgpRtT6ywmtALi@google.com> <CALOAHbCc843AGvVftCumnNgM87NzqAYpPcPEj0i+aQ5QEOUW7Q@mail.gmail.com>
 <CAKH8qBuRS5mp5akkT-sQHHdnnjWZYKehppgO5D1vE5rhYPuo8Q@mail.gmail.com>
In-Reply-To: <CAKH8qBuRS5mp5akkT-sQHHdnnjWZYKehppgO5D1vE5rhYPuo8Q@mail.gmail.com>
From:   Yafang Shao <laoar.shao@gmail.com>
Date:   Thu, 30 Mar 2023 10:40:37 +0800
Message-ID: <CALOAHbDpv6AEOJQBP+unNHTOkuNtZFhp5OnOy7xWnsZWFqGGGg@mail.gmail.com>
Subject: Re: [RFC PATCH bpf-next 00/13] bpf: Introduce BPF namespace
To:     Stanislav Fomichev <sdf@google.com>
Cc:     ast@kernel.org, daniel@iogearbox.net, andrii@kernel.org,
        kafai@fb.com, songliubraving@fb.com, yhs@fb.com,
        john.fastabend@gmail.com, kpsingh@kernel.org, haoluo@google.com,
        jolsa@kernel.org, bpf@vger.kernel.org, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Thu, Mar 30, 2023 at 4:50=E2=80=AFAM Stanislav Fomichev <sdf@google.com>=
 wrote:
>
> On Tue, Mar 28, 2023 at 8:03=E2=80=AFPM Yafang Shao <laoar.shao@gmail.com=
> wrote:
> >
> > On Wed, Mar 29, 2023 at 1:15=E2=80=AFAM Stanislav Fomichev <sdf@google.=
com> wrote:
> > >
> > > On 03/28, Yafang Shao wrote:
> > > > On Tue, Mar 28, 2023 at 1:28=E2=80=AFAM Stanislav Fomichev <sdf@goo=
gle.com> wrote:
> > > > >
> > > > > On 03/26, Yafang Shao wrote:
> > > > > > Currently only CAP_SYS_ADMIN can iterate BPF object IDs and con=
vert
> > > > IDs
> > > > > > to FDs, that's intended for BPF's security model[1]. Not only d=
oes it
> > > > > > prevent non-privilidged users from getting other users' bpf pro=
gram,
> > > > but
> > > > > > also it prevents the user from iterating his own bpf objects.
> > > > >
> > > > > > In container environment, some users want to run bpf programs i=
n their
> > > > > > containers. These users can run their bpf programs under CAP_BP=
F and
> > > > > > some other specific CAPs, but they can't inspect their bpf prog=
rams
> > > > in a
> > > > > > generic way. For example, the bpftool can't be used as it requi=
res
> > > > > > CAP_SYS_ADMIN. That is very inconvenient.
> > > > >
> > > > > > Without CAP_SYS_ADMIN, the only way to get the information of a=
 bpf
> > > > object
> > > > > > which is not created by the process itself is with SCM_RIGHTS, =
that
> > > > > > requires each processes which created bpf object has to impleme=
nt a
> > > > unix
> > > > > > domain socket to share the fd of a bpf object between different
> > > > > > processes, that is really trivial and troublesome.
> > > > >
> > > > > > Hence we need a better mechanism to get bpf object info without
> > > > > > CAP_SYS_ADMIN.
> > > > >
> > > > > [..]
> > > > >
> > > > > > BPF namespace is introduced in this patchset with an attempt to=
 remove
> > > > > > the CAP_SYS_ADMIN requirement. The user can create bpf map, pro=
g and
> > > > > > link in a specific bpf namespace, then these bpf objects will n=
ot be
> > > > > > visible to the users in a different bpf namespace. But these bp=
f
> > > > > > objects are visible to its parent bpf namespace, so the sys adm=
in can
> > > > > > still iterate and inspect them.
> > > > >
> > > > > Does it essentially mean unpriv bpf?
> > >
> > > > Right. With CAP_BPF and some other CAPs enabled.
> > >
> > > > > Can I, as a non-root, create
> > > > > a new bpf namespace and start loading/attaching progs?
> > >
> > > > No, you can't create a new bpf namespace as a non-root, see also
> > > > copy_namespaces().
> > > > In the container environment, new namespaces are always created by
> > > > containered, which is started by root.
> > >
> > > Are you talking about "if (!ns_capable(user_ns, CAP_SYS_ADMIN))" part
> > > from copy_namespaces? Isn't it trivially bypassed with a new user
> > > namespace?
> > >
> > > IIUC, I can create a new user namespace which gives me CAP_SYS_ADMIN
> > > in this particular user-ns. Then I can go on and create a new bpf
> > > namespace (with CAP_BPF) and go wild? I won't see anything from the
> > > other namespaces, but I'll be able to load/attach bpf programs?
> > >
> >
> > I don't think so. If you create a new userspace, and give the process
> > the CAP_BPF or CAP_SYS_ADMIN in this new user namespace but not the
> > initial namespace, you can't do that. Because currently only CAP_BPF
> > or CAP_SYS_ADMIN in the init user namespace can load/attach bpf
> > programs.
> >
> > > > > Maybe add a paragraph about now vs whatever you're proposing.
> > >
> > > > What I'm proposing in this patchset is to put bpf objects (map, pro=
g,
> > > > link, and btf) into the bpf namespace. Next step I will put bpffs i=
nto
> > > > the bpf namespace as well.
> > > > That said, I'm trying to put  all the objects created in bpf into t=
he
> > > > bpf namespace. Below is a simple paragraph to illustrate it.
> > >
> > > > Regarding the unpriv user with CAP_BPF enabled,
> > > >                                                               Now |=
 Future
> > > > -------------------------------------------------------------------=
-----
> > > > Iterate his BPF IDs                                | N   |  Y  |
> > > > Iterate others' BPF IDs                          | N   |  N  |
> > > > Convert his BPF IDs to FDs                  | N   |  Y  |
> > > > Convert others' BPF IDs to FDs            | N   |  N  |
> > > > Get others' object info from pinned file  | Y(*) | N  |
> > > > -------------------------------------------------------------------=
-----
> > >
> > > > (*) It can be improved by,
> > > >       1). Different containers has different bpffs
> > > >       2). Setting file permission
> > > >       That's not perfect, for example, if one single user has two b=
pf
> > > > instances, but we don't want them to inspect each other.
> > >
> > > I think the question here is what happens to the existing
> > > capable(CAP_BPF) checks? Do they become ns_capable(CAP_BPF) eventuall=
y?
> > >
> >
> > They won't become ns_capable(CAP_BPF). If it becomes
> > ns_capable(CAP_BPF), it will really go wild then.
> >
> > > And if not, I don't think it integrates well with the user namespaces=
?
> > >
> >
> > IIUC, it is the CAP_BPF which doesn't integrate with the user
> > namespaces, right?
>
> Yeah. And it's probably fine if we don't, I just wanted to see some
> explanation on the long-term plan.
> If the purpose is to have a bpf namespace and use it for pure
> isolation purposes, let's state it clearly in the cover letter.

Will add it.

> Otherwise it's not clear whether it's only about isolation or
> potentially allowing CAP_BPF in user namespaces.
> Maybe clone(CLONE_NEWBPF|CLONE_NEWUSER) should return an explicit
> error? (or maybe it already does, haven't looked at the patches)
>

Good suggestion. It should return an error.  I will change it in the
next version.

> One other question I have is: should init bpf namespace see
> everything? Otherwise it might be hard to chase down the namespaces
> that loaded some BPF program...

Right, the init bpf namespace will see everything.

--=20
Regards
Yafang