From: Jens Axboe <axboe@kernel.dk>
To: Andrii Nakryiko <andrii.nakryiko@gmail.com>
Cc: "Christian Göttsche" <cgzones@googlemail.com>,
linux-security-module@vger.kernel.org,
linux-block@vger.kernel.org, "Serge Hallyn" <serge@hallyn.com>,
linux-kernel@vger.kernel.org, bpf@vger.kernel.org
Subject: Re: [PATCH 02/10] capability: add any wrappers to test for multiple caps with exactly one audit message
Date: Sat, 16 Mar 2024 11:17:26 -0600 [thread overview]
Message-ID: <4405edd4-f20f-4040-ad67-aca3fcd7eff3@kernel.dk> (raw)
In-Reply-To: <CAEf4BzbgQrYDMma=NbW6A-qikA693eSnz9-RwjkF3xPLRE8qqg@mail.gmail.com>
On 3/15/24 3:16 PM, Andrii Nakryiko wrote:
> On Fri, Mar 15, 2024 at 11:41?AM Jens Axboe <axboe@kernel.dk> wrote:
>>
>> On 3/15/24 10:45 AM, Andrii Nakryiko wrote:
>>>> +/**
>>>> + * ns_capable_any - Determine if the current task has one of two superior capabilities in effect
>>>> + * @ns: The usernamespace we want the capability in
>>>> + * @cap1: The capabilities to be tested for first
>>>> + * @cap2: The capabilities to be tested for secondly
>>>> + *
>>>> + * Return true if the current task has at least one of the two given superior
>>>> + * capabilities currently available for use, false if not.
>>>> + *
>>>> + * In contrast to or'ing capable() this call will create exactly one audit
>>>> + * message, either for @cap1, if it is granted or both are not permitted,
>>>> + * or @cap2, if it is granted while the other one is not.
>>>> + *
>>>> + * The capabilities should be ordered from least to most invasive, i.e. CAP_SYS_ADMIN last.
>>>> + *
>>>> + * This sets PF_SUPERPRIV on the task if the capability is available on the
>>>> + * assumption that it's about to be used.
>>>> + */
>>>> +bool ns_capable_any(struct user_namespace *ns, int cap1, int cap2)
>>>> +{
>>>> + if (cap1 == cap2)
>>>> + return ns_capable(ns, cap1);
>>>> +
>>>> + if (ns_capable_noauditondeny(ns, cap1))
>>>> + return true;
>>>> +
>>>> + if (ns_capable_noauditondeny(ns, cap2))
>>>> + return true;
>>>> +
>>>> + return ns_capable(ns, cap1);
>>>
>>> this will incur an extra capable() check (with all the LSMs involved,
>>> etc), and so for some cases where capability is expected to not be
>>> present, this will be a regression. Is there some way to not redo the
>>> check, but just audit the failure? At this point we do know that cap1
>>> failed before, so might as well just log that.
>>
>> Not sure why that's important - if it's a failure case, and any audit
>> failure should be, then why would we care if that's now doing a bit of
>> extra work?
>
> Lack of capability doesn't necessarily mean "failure". E.g., in FUSE
> there are at least few places where the code checks
> capable(CAP_SYS_ADMIN), and based on that decides on some limit values
> or extra checks. So if !capable(CAP_SYS_ADMIN), operation doesn't
> necessarily fail outright, it just has some more restricted resources
> or something.
>
> Luckily in FUSE's case it's singular capable() check, so capable_any()
> won't incur extra overhead. But I was just wondering if it would be
> possible to avoid this with capable_any() as well, so that no one has
> to do these trade-offs.
That's certainly a special and odd case, as most other cases really
would be of the:
if (capable(SOMETHING))
return -EFAUL;
Might make more sense to special case the FUSE thing then, or provide a
cheap way for it to do what it needs to do. I really don't think that
kind of:
if (capable(SOMETHING))
do something since I can
else
bummer, do something else then
is a common occurrence.
> We also had cases in production of some BPF applications tracing
> cap_capable() calls, so each extra triggering of it would be a bit of
> added overhead, as a general rule.
>
> Having said the above, I do like capable_any() changes (which is why I
> acked BPF side of things).
Yes, the BPF tracking capable in production is a pain in the butt, as it
slows down any valid fast path capable checking by a substantial amount.
We've had to work around that on the block side, unfortunately. These
are obviously cases where you expect success, and any failure is
permanent as far as that operation goes.
--
Jens Axboe
next prev parent reply other threads:[~2024-03-16 17:17 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-15 11:37 [PATCH 01/10] capability: introduce new capable flag CAP_OPT_NOAUDIT_ONDENY Christian Göttsche
2024-03-15 11:37 ` [PATCH 02/10] capability: add any wrappers to test for multiple caps with exactly one audit message Christian Göttsche
2024-03-15 16:45 ` Andrii Nakryiko
2024-03-15 18:27 ` Christian Göttsche
2024-03-15 18:30 ` Andrii Nakryiko
2024-03-15 18:41 ` Jens Axboe
2024-03-15 19:48 ` Paul Moore
2024-03-15 21:16 ` Andrii Nakryiko
2024-03-16 17:17 ` Jens Axboe [this message]
2024-03-15 20:19 ` Serge Hallyn
2024-03-15 11:37 ` [PATCH 03/10] capability: use new capable_any functionality Christian Göttsche
2024-03-15 16:46 ` Andrii Nakryiko
2024-03-15 11:37 ` [PATCH 04/10] block: " Christian Göttsche
2024-03-15 11:37 ` [PATCH 05/10] drivers: " Christian Göttsche
2024-03-15 15:03 ` Felix Kuehling
2024-03-15 11:37 ` [PATCH 06/10] fs: " Christian Göttsche
2024-03-15 11:37 ` [PATCH 07/10] kernel: " Christian Göttsche
2024-03-15 15:03 ` Tycho Andersen
2024-03-15 11:37 ` [PATCH 08/10] net: " Christian Göttsche
2024-03-15 23:11 ` Kuniyuki Iwashima
2024-03-15 11:37 ` [PATCH 09/10] bpf: " Christian Göttsche
2024-03-15 16:43 ` Andrii Nakryiko
2024-03-15 11:37 ` [PATCH 10/10] coccinelle: add script for capable_any() Christian Göttsche
2024-03-15 11:37 ` [PATCH 00/10] Introduce capable_any() Christian Göttsche
2024-03-15 19:59 ` [PATCH 01/10] capability: introduce new capable flag CAP_OPT_NOAUDIT_ONDENY Serge Hallyn
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=4405edd4-f20f-4040-ad67-aca3fcd7eff3@kernel.dk \
--to=axboe@kernel.dk \
--cc=andrii.nakryiko@gmail.com \
--cc=bpf@vger.kernel.org \
--cc=cgzones@googlemail.com \
--cc=linux-block@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-security-module@vger.kernel.org \
--cc=serge@hallyn.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 read-only IMAP folder(s) and NNTP newsgroup(s).