[02/10] capability: add any wrappers to test for multiple caps with exactly one audit message

Commit Message

Christian Göttsche March 15, 2024, 11:37 a.m. UTC

Add the interfaces `capable_any()` and `ns_capable_any()` as an
alternative to multiple `capable()`/`ns_capable()` calls, like
`capable_any(CAP_SYS_NICE, CAP_SYS_ADMIN)` instead of
`capable(CAP_SYS_NICE) || capable(CAP_SYS_ADMIN)`.

`capable_any()`/`ns_capable_any()` will in particular generate exactly
one audit message, either for the left most capability in effect or, if
the task has none, the first one.

This is especially helpful with regard to SELinux, where each audit
message about a not allowed capability request will create a denial
message.  Using this new wrapper with the least invasive capability as
left most argument (e.g. CAP_SYS_NICE before CAP_SYS_ADMIN) enables
policy writers to only grant the least invasive one for the particular
subject instead of both.

CC: [e-mail]
Signed-off-by: Christian Göttsche <[e-mail]>
---
v5:
   - add check for identical passed capabilities
   - rename internal helper according to flag rename to
     ns_capable_noauditondeny()
v4:
   Use CAP_OPT_NODENYAUDIT via added ns_capable_nodenyaudit()
v3:
   - rename to capable_any()
   - fix typo in function documentation
   - add ns_capable_any()
v2:
   avoid varargs and fix to two capabilities; capable_or3() can be added
   later if needed
---
 include/linux/capability.h | 10 ++++++
 kernel/capability.c        | 73 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 83 insertions(+)

Comments

Andrii Nakryiko March 15, 2024, 4:45 p.m. UTC | #1

On Fri, Mar 15, 2024 at 4:39 AM Christian Göttsche
<[e-mail]> wrote:
>
> Add the interfaces `capable_any()` and `ns_capable_any()` as an
> alternative to multiple `capable()`/`ns_capable()` calls, like
> `capable_any(CAP_SYS_NICE, CAP_SYS_ADMIN)` instead of
> `capable(CAP_SYS_NICE) || capable(CAP_SYS_ADMIN)`.
>
> `capable_any()`/`ns_capable_any()` will in particular generate exactly
> one audit message, either for the left most capability in effect or, if
> the task has none, the first one.
>
> This is especially helpful with regard to SELinux, where each audit
> message about a not allowed capability request will create a denial
> message.  Using this new wrapper with the least invasive capability as
> left most argument (e.g. CAP_SYS_NICE before CAP_SYS_ADMIN) enables
> policy writers to only grant the least invasive one for the particular
> subject instead of both.
>
> CC: [e-mail]
> Signed-off-by: Christian Göttsche <[e-mail]>
> ---
> v5:
>    - add check for identical passed capabilities
>    - rename internal helper according to flag rename to
>      ns_capable_noauditondeny()
> v4:
>    Use CAP_OPT_NODENYAUDIT via added ns_capable_nodenyaudit()
> v3:
>    - rename to capable_any()
>    - fix typo in function documentation
>    - add ns_capable_any()
> v2:
>    avoid varargs and fix to two capabilities; capable_or3() can be added
>    later if needed
> ---
>  include/linux/capability.h | 10 ++++++
>  kernel/capability.c        | 73 ++++++++++++++++++++++++++++++++++++++
>  2 files changed, 83 insertions(+)
>

[...]

>
> +/**
> + * 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.

> +}
> +EXPORT_SYMBOL(ns_capable_any);
> +
> +/**
> + * capable_any - Determine if the current task has one of two superior capabilities in effect
> + * @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 capable_any(int cap1, int cap2)
> +{
> +       return ns_capable_any(&init_user_ns, cap1, cap2);
> +}
> +EXPORT_SYMBOL(capable_any);
> +
>  /**
>   * capable - Determine if the current task has a superior capability in effect
>   * @cap: The capability to be tested for
> --
> 2.43.0
>
>

Christian Göttsche March 15, 2024, 6:27 p.m. UTC | #2

On Fri, 15 Mar 2024 at 17:46, Andrii Nakryiko <[e-mail]> wrote:
>
> On Fri, Mar 15, 2024 at 4:39 AM Christian Göttsche
> <[e-mail]> wrote:
> >
> > Add the interfaces `capable_any()` and `ns_capable_any()` as an
> > alternative to multiple `capable()`/`ns_capable()` calls, like
> > `capable_any(CAP_SYS_NICE, CAP_SYS_ADMIN)` instead of
> > `capable(CAP_SYS_NICE) || capable(CAP_SYS_ADMIN)`.
> >
> > `capable_any()`/`ns_capable_any()` will in particular generate exactly
> > one audit message, either for the left most capability in effect or, if
> > the task has none, the first one.
> >
> > This is especially helpful with regard to SELinux, where each audit
> > message about a not allowed capability request will create a denial
> > message.  Using this new wrapper with the least invasive capability as
> > left most argument (e.g. CAP_SYS_NICE before CAP_SYS_ADMIN) enables
> > policy writers to only grant the least invasive one for the particular
> > subject instead of both.
> >
> > CC: [e-mail]
> > Signed-off-by: Christian Göttsche <[e-mail]>
> > ---
> > v5:
> >    - add check for identical passed capabilities
> >    - rename internal helper according to flag rename to
> >      ns_capable_noauditondeny()
> > v4:
> >    Use CAP_OPT_NODENYAUDIT via added ns_capable_nodenyaudit()
> > v3:
> >    - rename to capable_any()
> >    - fix typo in function documentation
> >    - add ns_capable_any()
> > v2:
> >    avoid varargs and fix to two capabilities; capable_or3() can be added
> >    later if needed
> > ---
> >  include/linux/capability.h | 10 ++++++
> >  kernel/capability.c        | 73 ++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 83 insertions(+)
> >
>
> [...]
>
> >
> > +/**
> > + * 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.

Logging the failure is quite different in AppArmor and SELinux, so
just log might not be so easy.
One option would be to change the entire LSM hook security_capable()
to take two capability arguments, and let the LSMs handle the any
logic.

> > +}
> > +EXPORT_SYMBOL(ns_capable_any);
> > +
> > +/**
> > + * capable_any - Determine if the current task has one of two superior capabilities in effect
> > + * @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 capable_any(int cap1, int cap2)
> > +{
> > +       return ns_capable_any(&init_user_ns, cap1, cap2);
> > +}
> > +EXPORT_SYMBOL(capable_any);
> > +
> >  /**
> >   * capable - Determine if the current task has a superior capability in effect
> >   * @cap: The capability to be tested for
> > --
> > 2.43.0
> >
> >

Andrii Nakryiko March 15, 2024, 6:30 p.m. UTC | #3

On Fri, Mar 15, 2024 at 11:27 AM Christian Göttsche
<[e-mail]> wrote:
>
> On Fri, 15 Mar 2024 at 17:46, Andrii Nakryiko <[e-mail]> wrote:
> >
> > On Fri, Mar 15, 2024 at 4:39 AM Christian Göttsche
> > <[e-mail]> wrote:
> > >
> > > Add the interfaces `capable_any()` and `ns_capable_any()` as an
> > > alternative to multiple `capable()`/`ns_capable()` calls, like
> > > `capable_any(CAP_SYS_NICE, CAP_SYS_ADMIN)` instead of
> > > `capable(CAP_SYS_NICE) || capable(CAP_SYS_ADMIN)`.
> > >
> > > `capable_any()`/`ns_capable_any()` will in particular generate exactly
> > > one audit message, either for the left most capability in effect or, if
> > > the task has none, the first one.
> > >
> > > This is especially helpful with regard to SELinux, where each audit
> > > message about a not allowed capability request will create a denial
> > > message.  Using this new wrapper with the least invasive capability as
> > > left most argument (e.g. CAP_SYS_NICE before CAP_SYS_ADMIN) enables
> > > policy writers to only grant the least invasive one for the particular
> > > subject instead of both.
> > >
> > > CC: [e-mail]
> > > Signed-off-by: Christian Göttsche <[e-mail]>
> > > ---
> > > v5:
> > >    - add check for identical passed capabilities
> > >    - rename internal helper according to flag rename to
> > >      ns_capable_noauditondeny()
> > > v4:
> > >    Use CAP_OPT_NODENYAUDIT via added ns_capable_nodenyaudit()
> > > v3:
> > >    - rename to capable_any()
> > >    - fix typo in function documentation
> > >    - add ns_capable_any()
> > > v2:
> > >    avoid varargs and fix to two capabilities; capable_or3() can be added
> > >    later if needed
> > > ---
> > >  include/linux/capability.h | 10 ++++++
> > >  kernel/capability.c        | 73 ++++++++++++++++++++++++++++++++++++++
> > >  2 files changed, 83 insertions(+)
> > >
> >
> > [...]
> >
> > >
> > > +/**
> > > + * 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.
>
> Logging the failure is quite different in AppArmor and SELinux, so
> just log might not be so easy.
> One option would be to change the entire LSM hook security_capable()
> to take two capability arguments, and let the LSMs handle the any
> logic.

that sounds like an even bigger overkill, probably not worth it

>
> > > +}
> > > +EXPORT_SYMBOL(ns_capable_any);
> > > +
> > > +/**
> > > + * capable_any - Determine if the current task has one of two superior capabilities in effect
> > > + * @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 capable_any(int cap1, int cap2)
> > > +{
> > > +       return ns_capable_any(&init_user_ns, cap1, cap2);
> > > +}
> > > +EXPORT_SYMBOL(capable_any);
> > > +
> > >  /**
> > >   * capable - Determine if the current task has a superior capability in effect
> > >   * @cap: The capability to be tested for
> > > --
> > > 2.43.0
> > >
> > >

Jens Axboe March 15, 2024, 6:41 p.m. UTC | #4

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?

I say this not knowing the full picture, as I unhelpfully was only CC'ed
on two of the patches... Please don't do that when sending patchsets.

Paul Moore March 15, 2024, 7:48 p.m. UTC | #5

On Fri, Mar 15, 2024 at 2:41 PM Jens Axboe <[e-mail]> 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?

Exactly.  We discussed this in an earlier patchset in 2022 (lore link below):

https://lore.kernel.org/all/CAHC9VhS8ASN+BB7adi=uoAj=LeNhiD4LEidbMc=[e-mail]

> I say this not knowing the full picture, as I unhelpfully was only CC'ed
> on two of the patches... Please don't do that when sending patchsets.

Agreed, if the patchset touches anything in the audit, LSM, or SELinux
code please send the full patchset to the related lists.  If I have to
dig the full patchset out of lore for review it makes me grumpy.
Don't resend the patchset for just this reason, but please keep it in
mind for future patchsets.

--
paul-moore.com

Serge E. Hallyn March 15, 2024, 8:19 p.m. UTC | #6

On Fri, Mar 15, 2024 at 12:37:23PM +0100, Christian Göttsche wrote:
> Add the interfaces `capable_any()` and `ns_capable_any()` as an
> alternative to multiple `capable()`/`ns_capable()` calls, like
> `capable_any(CAP_SYS_NICE, CAP_SYS_ADMIN)` instead of
> `capable(CAP_SYS_NICE) || capable(CAP_SYS_ADMIN)`.
> 
> `capable_any()`/`ns_capable_any()` will in particular generate exactly
> one audit message, either for the left most capability in effect or, if
> the task has none, the first one.
> 
> This is especially helpful with regard to SELinux, where each audit
> message about a not allowed capability request will create a denial
> message.  Using this new wrapper with the least invasive capability as
> left most argument (e.g. CAP_SYS_NICE before CAP_SYS_ADMIN) enables
> policy writers to only grant the least invasive one for the particular
> subject instead of both.
> 
> CC: [e-mail]
> Signed-off-by: Christian Göttsche <[e-mail]>

Reviewed-by: Serge Hallyn <[e-mail]>

> ---
> v5:
>    - add check for identical passed capabilities
>    - rename internal helper according to flag rename to
>      ns_capable_noauditondeny()
> v4:
>    Use CAP_OPT_NODENYAUDIT via added ns_capable_nodenyaudit()
> v3:
>    - rename to capable_any()
>    - fix typo in function documentation
>    - add ns_capable_any()
> v2:
>    avoid varargs and fix to two capabilities; capable_or3() can be added
>    later if needed
> ---
>  include/linux/capability.h | 10 ++++++
>  kernel/capability.c        | 73 ++++++++++++++++++++++++++++++++++++++
>  2 files changed, 83 insertions(+)
> 
> diff --git a/include/linux/capability.h b/include/linux/capability.h
> index 0c356a517991..eeb958440656 100644
> --- a/include/linux/capability.h
> +++ b/include/linux/capability.h
> @@ -146,7 +146,9 @@ extern bool has_capability_noaudit(struct task_struct *t, int cap);
>  extern bool has_ns_capability_noaudit(struct task_struct *t,
>  				      struct user_namespace *ns, int cap);
>  extern bool capable(int cap);
> +extern bool capable_any(int cap1, int cap2);
>  extern bool ns_capable(struct user_namespace *ns, int cap);
> +extern bool ns_capable_any(struct user_namespace *ns, int cap1, int cap2);
>  extern bool ns_capable_noaudit(struct user_namespace *ns, int cap);
>  extern bool ns_capable_setid(struct user_namespace *ns, int cap);
>  #else
> @@ -172,10 +174,18 @@ static inline bool capable(int cap)
>  {
>  	return true;
>  }
> +static inline bool capable_any(int cap1, int cap2)
> +{
> +	return true;
> +}
>  static inline bool ns_capable(struct user_namespace *ns, int cap)
>  {
>  	return true;
>  }
> +static inline bool ns_capable_any(struct user_namespace *ns, int cap1, int cap2)
> +{
> +	return true;
> +}
>  static inline bool ns_capable_noaudit(struct user_namespace *ns, int cap)
>  {
>  	return true;
> diff --git a/kernel/capability.c b/kernel/capability.c
> index dac4df77e376..73358abfe2e1 100644
> --- a/kernel/capability.c
> +++ b/kernel/capability.c
> @@ -402,6 +402,23 @@ bool ns_capable_noaudit(struct user_namespace *ns, int cap)
>  }
>  EXPORT_SYMBOL(ns_capable_noaudit);
>  
> +/**
> + * ns_capable_noauditondeny - Determine if the current task has a superior capability
> + * (unaudited when unauthorized) in effect
> + * @ns:  The usernamespace we want the capability in
> + * @cap: The capability to be tested for
> + *
> + * Return true if the current task has the given superior capability currently
> + * available for use, false if not.
> + *
> + * This sets PF_SUPERPRIV on the task if the capability is available on the
> + * assumption that it's about to be used.
> + */
> +static bool ns_capable_noauditondeny(struct user_namespace *ns, int cap)
> +{
> +	return ns_capable_common(ns, cap, CAP_OPT_NOAUDIT_ONDENY);
> +}
> +
>  /**
>   * ns_capable_setid - Determine if the current task has a superior capability
>   * in effect, while signalling that this check is being done from within a
> @@ -421,6 +438,62 @@ bool ns_capable_setid(struct user_namespace *ns, int cap)
>  }
>  EXPORT_SYMBOL(ns_capable_setid);
>  
> +/**
> + * 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);
> +}
> +EXPORT_SYMBOL(ns_capable_any);
> +
> +/**
> + * capable_any - Determine if the current task has one of two superior capabilities in effect
> + * @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 capable_any(int cap1, int cap2)
> +{
> +	return ns_capable_any(&init_user_ns, cap1, cap2);
> +}
> +EXPORT_SYMBOL(capable_any);
> +
>  /**
>   * capable - Determine if the current task has a superior capability in effect
>   * @cap: The capability to be tested for
> -- 
> 2.43.0
> 
>

Andrii Nakryiko March 15, 2024, 9:16 p.m. UTC | #7

On Fri, Mar 15, 2024 at 11:41 AM Jens Axboe <[e-mail]> 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.

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).

>
> I say this not knowing the full picture, as I unhelpfully was only CC'ed
> on two of the patches... Please don't do that when sending patchsets.
>
> --
> Jens Axboe
>

Jens Axboe March 16, 2024, 5:17 p.m. UTC | #8

On 3/15/24 3:16 PM, Andrii Nakryiko wrote:
> On Fri, Mar 15, 2024 at 11:41?AM Jens Axboe <[e-mail]> 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.

Paul Moore June 10, 2024, 8:58 p.m. UTC | #9

On Fri, Mar 15, 2024 at 4:19 PM Serge Hallyn <[e-mail]> wrote:
> On Fri, Mar 15, 2024 at 12:37:23PM +0100, Christian Göttsche wrote:
> > Add the interfaces `capable_any()` and `ns_capable_any()` as an
> > alternative to multiple `capable()`/`ns_capable()` calls, like
> > `capable_any(CAP_SYS_NICE, CAP_SYS_ADMIN)` instead of
> > `capable(CAP_SYS_NICE) || capable(CAP_SYS_ADMIN)`.
> >
> > `capable_any()`/`ns_capable_any()` will in particular generate exactly
> > one audit message, either for the left most capability in effect or, if
> > the task has none, the first one.
> >
> > This is especially helpful with regard to SELinux, where each audit
> > message about a not allowed capability request will create a denial
> > message.  Using this new wrapper with the least invasive capability as
> > left most argument (e.g. CAP_SYS_NICE before CAP_SYS_ADMIN) enables
> > policy writers to only grant the least invasive one for the particular
> > subject instead of both.
> >
> > CC: [e-mail]
> > Signed-off-by: Christian Göttsche <[e-mail]>
>
> Reviewed-by: Serge Hallyn <[e-mail]>

Looking over the patchset, I'm not seeing any strong objections from
anyone, and most of the patches have ACKs/Reviewed-by tags (exceptions
being the block and coccinelle patches), so I'm thinking we could
probably merge the first two patches that add the basic support along
with all of the subsystem patches except the two that haven't been
ACKd (we can push on those later).

Serge, as far as I'm concerned it's your call as this is largely a
capabilities patchset.  Assuming for a moment that you are still okay
with these patches, are you planning to pull them into the
capabilities tree and send them to Linus, or would you prefer I pull
it via the LSM tree?

--
paul-moore.com

Patch

diff --git a/include/linux/capability.h b/include/linux/capability.h
index 0c356a517991..eeb958440656 100644
--- a/include/linux/capability.h
+++ b/include/linux/capability.h
@@ -146,7 +146,9 @@  extern bool has_capability_noaudit(struct task_struct *t, int cap);
 extern bool has_ns_capability_noaudit(struct task_struct *t,
 				      struct user_namespace *ns, int cap);
 extern bool capable(int cap);
+extern bool capable_any(int cap1, int cap2);
 extern bool ns_capable(struct user_namespace *ns, int cap);
+extern bool ns_capable_any(struct user_namespace *ns, int cap1, int cap2);
 extern bool ns_capable_noaudit(struct user_namespace *ns, int cap);
 extern bool ns_capable_setid(struct user_namespace *ns, int cap);
 #else
@@ -172,10 +174,18 @@  static inline bool capable(int cap)
 {
 	return true;
 }
+static inline bool capable_any(int cap1, int cap2)
+{
+	return true;
+}
 static inline bool ns_capable(struct user_namespace *ns, int cap)
 {
 	return true;
 }
+static inline bool ns_capable_any(struct user_namespace *ns, int cap1, int cap2)
+{
+	return true;
+}
 static inline bool ns_capable_noaudit(struct user_namespace *ns, int cap)
 {
 	return true;
diff --git a/kernel/capability.c b/kernel/capability.c
index dac4df77e376..73358abfe2e1 100644
--- a/kernel/capability.c
+++ b/kernel/capability.c
@@ -402,6 +402,23 @@  bool ns_capable_noaudit(struct user_namespace *ns, int cap)
 }
 EXPORT_SYMBOL(ns_capable_noaudit);
 
+/**
+ * ns_capable_noauditondeny - Determine if the current task has a superior capability
+ * (unaudited when unauthorized) in effect
+ * @ns:  The usernamespace we want the capability in
+ * @cap: The capability to be tested for
+ *
+ * Return true if the current task has the given superior capability currently
+ * available for use, false if not.
+ *
+ * This sets PF_SUPERPRIV on the task if the capability is available on the
+ * assumption that it's about to be used.
+ */
+static bool ns_capable_noauditondeny(struct user_namespace *ns, int cap)
+{
+	return ns_capable_common(ns, cap, CAP_OPT_NOAUDIT_ONDENY);
+}
+
 /**
  * ns_capable_setid - Determine if the current task has a superior capability
  * in effect, while signalling that this check is being done from within a
@@ -421,6 +438,62 @@  bool ns_capable_setid(struct user_namespace *ns, int cap)
 }
 EXPORT_SYMBOL(ns_capable_setid);
 
+/**
+ * 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);
+}
+EXPORT_SYMBOL(ns_capable_any);
+
+/**
+ * capable_any - Determine if the current task has one of two superior capabilities in effect
+ * @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 capable_any(int cap1, int cap2)
+{
+	return ns_capable_any(&init_user_ns, cap1, cap2);
+}
+EXPORT_SYMBOL(capable_any);
+
 /**
  * capable - Determine if the current task has a superior capability in effect
  * @cap: The capability to be tested for