[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAHC9VhQg7mYnQw-o1TYon_bdtk_CMzJaf6u5FTPosniG-UXK1w@mail.gmail.com>
Date: Mon, 8 Jan 2024 11:45:17 -0500
From: Paul Moore <paul@...l-moore.com>
To: Linus Torvalds <torvalds@...uxfoundation.org>
Cc: Andrii Nakryiko <andrii@...nel.org>, bpf@...r.kernel.org, netdev@...r.kernel.org,
brauner@...nel.org, linux-fsdevel@...r.kernel.org,
linux-security-module@...r.kernel.org, kernel-team@...a.com
Subject: Re: [PATCH bpf-next 03/29] bpf: introduce BPF token object
On Fri, Jan 5, 2024 at 4:45 PM Linus Torvalds
<torvalds@...uxfoundation.org> wrote:
> On Wed, 3 Jan 2024 at 14:21, Andrii Nakryiko <andrii@...nel.org> wrote:
> >
> > +bool bpf_token_capable(const struct bpf_token *token, int cap)
> > +{
> > + /* BPF token allows ns_capable() level of capabilities, but only if
> > + * token's userns is *exactly* the same as current user's userns
> > + */
> > + if (token && current_user_ns() == token->userns) {
> > + if (ns_capable(token->userns, cap))
> > + return true;
> > + if (cap != CAP_SYS_ADMIN && ns_capable(token->userns, CAP_SYS_ADMIN))
> > + return true;
> > + }
> > + /* otherwise fallback to capable() checks */
> > + return capable(cap) || (cap != CAP_SYS_ADMIN && capable(CAP_SYS_ADMIN));
> > +}
>
> This *feels* like it should be written as
>
> bool bpf_token_capable(const struct bpf_token *token, int cap)
> {
> struct user_namespace *ns = &init_ns;
>
> /* BPF token allows ns_capable() level of capabilities, but only if
> * token's userns is *exactly* the same as current user's userns
> */
> if (token && current_user_ns() == token->userns)
> ns = token->userns;
> return ns_capable(ns, cap) ||
> (cap != CAP_SYS_ADMIN && capable(CAP_SYS_ADMIN));
> }
>
> And yes, I realize that the function will end up later growing a
>
> security_bpf_token_capable(token, cap)
>
> test inside that 'if (token ..)' statement, and this would change the
> order of that test so that the LSM hook would now be done before the
> capability checks are done, but that all still seems just more of an
> argument for the simplification.
I have no problem with rewriting things, my only ask is that we stick
with the idea of doing the capability checks before the LSM hook. The
DAC-before-MAC (capability-before-LSM) pattern is one we try to stick
to most everywhere in the kernel and deviating from it here could
potentially result in some odd/unexpected behavior from a user
perspective.
--
paul-moore.com
Powered by blists - more mailing lists