Re: [PATCH 01/10] capability: introduce new capable flag CAP_OPT_NOAUDIT_ONDENY

From: Serge Hallyn
Date: Fri Mar 15 2024 - 15:59:45 EST


On Fri, Mar 15, 2024 at 12:37:22PM +0100, Christian Göttsche wrote:
> Introduce a new capable flag, CAP_OPT_NOAUDIT_ONDENY, to not generate
> an audit event if the requested capability is not granted. This will be
> used in a new capable_any() functionality to reduce the number of
> necessary capable calls.
>
> Handle the flag accordingly in AppArmor and SELinux.
>
> CC: linux-block@xxxxxxxxxxxxxxx
> Suggested-by: Paul Moore <paul@xxxxxxxxxxxxxx>
> Signed-off-by: Christian Göttsche <cgzones@xxxxxxxxxxxxxx>

Thanks.

Reviewed-by: Serge Hallyn <serge@xxxxxxxxxx>

> ---
> v5:
> rename flag to CAP_OPT_NOAUDIT_ONDENY, suggested by Serge:
> https://lore.kernel.org/all/20230606190013.GA640488@xxxxxxxxxxxxxxx/
> ---
> include/linux/security.h | 2 ++
> security/apparmor/capability.c | 8 +++++---
> security/selinux/hooks.c | 14 ++++++++------
> 3 files changed, 15 insertions(+), 9 deletions(-)
>
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 41a8f667bdfa..c60cae78ff8b 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -70,6 +70,8 @@ struct lsm_ctx;
> #define CAP_OPT_NOAUDIT BIT(1)
> /* If capable is being called by a setid function */
> #define CAP_OPT_INSETID BIT(2)
> +/* If capable should audit the security request for authorized requests only */
> +#define CAP_OPT_NOAUDIT_ONDENY BIT(3)
>
> /* LSM Agnostic defines for security_sb_set_mnt_opts() flags */
> #define SECURITY_LSM_NATIVE_LABELS 1
> diff --git a/security/apparmor/capability.c b/security/apparmor/capability.c
> index 9934df16c843..08c9c9a0fc19 100644
> --- a/security/apparmor/capability.c
> +++ b/security/apparmor/capability.c
> @@ -108,7 +108,8 @@ static int audit_caps(struct apparmor_audit_data *ad, struct aa_profile *profile
> * profile_capable - test if profile allows use of capability @cap
> * @profile: profile being enforced (NOT NULL, NOT unconfined)
> * @cap: capability to test if allowed
> - * @opts: CAP_OPT_NOAUDIT bit determines whether audit record is generated
> + * @opts: CAP_OPT_NOAUDIT/CAP_OPT_NOAUDIT_ONDENY bit determines whether audit
> + * record is generated
> * @ad: audit data (MAY BE NULL indicating no auditing)
> *
> * Returns: 0 if allowed else -EPERM
> @@ -126,7 +127,7 @@ static int profile_capable(struct aa_profile *profile, int cap,
> else
> error = -EPERM;
>
> - if (opts & CAP_OPT_NOAUDIT) {
> + if ((opts & CAP_OPT_NOAUDIT) || ((opts & CAP_OPT_NOAUDIT_ONDENY) && error)) {
> if (!COMPLAIN_MODE(profile))
> return error;
> /* audit the cap request in complain mode but note that it
> @@ -143,7 +144,8 @@ static int profile_capable(struct aa_profile *profile, int cap,
> * @subj_cred: cred we are testing capability against
> * @label: label being tested for capability (NOT NULL)
> * @cap: capability to be tested
> - * @opts: CAP_OPT_NOAUDIT bit determines whether audit record is generated
> + * @opts: CAP_OPT_NOAUDIT/CAP_OPT_NOAUDIT_ONDENY bit determines whether audit
> + * record is generated
> *
> * Look up capability in profile capability set.
> *
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 3448454c82d0..1a2c7c1a89be 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -1624,7 +1624,7 @@ static int cred_has_capability(const struct cred *cred,
> u16 sclass;
> u32 sid = cred_sid(cred);
> u32 av = CAP_TO_MASK(cap);
> - int rc;
> + int rc, rc2;
>
> ad.type = LSM_AUDIT_DATA_CAP;
> ad.u.cap = cap;
> @@ -1643,11 +1643,13 @@ static int cred_has_capability(const struct cred *cred,
> }
>
> rc = avc_has_perm_noaudit(sid, sid, sclass, av, 0, &avd);
> - if (!(opts & CAP_OPT_NOAUDIT)) {
> - int rc2 = avc_audit(sid, sid, sclass, av, &avd, rc, &ad);
> - if (rc2)
> - return rc2;
> - }
> + if ((opts & CAP_OPT_NOAUDIT) || ((opts & CAP_OPT_NOAUDIT_ONDENY) && rc))
> + return rc;
> +
> + rc2 = avc_audit(sid, sid, sclass, av, &avd, rc, &ad);
> + if (rc2)
> + return rc2;
> +
> return rc;
> }
>
> --
> 2.43.0
>
>