linux-kernel - Re: [PATCH v4 1/9] capability: introduce new capable flag NODENYAUDIT

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 31 May 2023 09:07:34 -0500
From:   "Serge E. Hallyn" <serge@...lyn.com>
To:     Christian Göttsche <cgzones@...glemail.com>
Cc:     selinux@...r.kernel.org, Paul Moore <paul@...l-moore.com>,
        John Johansen <john.johansen@...onical.com>,
        James Morris <jmorris@...ei.org>,
        "Serge E. Hallyn" <serge@...lyn.com>,
        Stephen Smalley <stephen.smalley.work@...il.com>,
        Eric Paris <eparis@...isplace.org>,
        Christian Brauner <brauner@...nel.org>,
        Casey Schaufler <casey@...aufler-ca.com>,
        Dave Chinner <dchinner@...hat.com>,
        Nathan Lynch <nathanl@...ux.ibm.com>,
        Al Viro <viro@...iv.linux.org.uk>,
        Roberto Sassu <roberto.sassu@...wei.com>,
        Micah Morton <mortonm@...omium.org>,
        Frederick Lawler <fred@...udflare.com>,
        Günther Noack <gnoack3000@...il.com>,
        linux-kernel@...r.kernel.org, apparmor@...ts.ubuntu.com,
        linux-security-module@...r.kernel.org, bpf@...r.kernel.org
Subject: Re: [PATCH v4 1/9] capability: introduce new capable flag NODENYAUDIT

On Thu, May 11, 2023 at 04:25:24PM +0200, Christian Göttsche wrote:
> Introduce a new capable flag, CAP_OPT_NODENYAUDIT, 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.
> 
> Suggested-by: Paul Moore <paul@...l-moore.com>
> Signed-off-by: Christian Göttsche <cgzones@...glemail.com>

Reviewed-by: Serge Hallyn <serge@...lyn.com>

> ---
>  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 e2734e9e44d5..629c775ec297 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -67,6 +67,8 @@ struct watch_notification;
>  #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_NODENYAUDIT 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 326a51838ef2..98120dd62ca7 100644
> --- a/security/apparmor/capability.c
> +++ b/security/apparmor/capability.c
> @@ -108,7 +108,8 @@ static int audit_caps(struct common_audit_data *sa, 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_NODENYAUDIT bit determines whether audit
> + *	record is generated
>   * @sa: 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_NODENYAUDIT) && error)) {
>  		if (!COMPLAIN_MODE(profile))
>  			return error;
>  		/* audit the cap request in complain mode but note that it
> @@ -142,7 +143,8 @@ static int profile_capable(struct aa_profile *profile, int cap,
>   * aa_capable - test permission to use capability
>   * @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_NODENYAUDIT 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 79b4890e9936..0730edf2f5f1 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -1571,7 +1571,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;
> @@ -1590,11 +1590,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_NODENYAUDIT) && rc))
> +		return rc;

Hm, if the caller passes only CAP_OPT_NODENYAUDIT, and rc == 0, then
you will audit the allow.  Is that what you want, or did you want, or
did you want CAP_OPT_NODENYAUDIT to imply CAP_OPT_NOAUDIT?

> +
> +	rc2 = avc_audit(sid, sid, sclass, av, &avd, rc, &ad);
> +	if (rc2)
> +		return rc2;
> +
>  	return rc;
>  }
>  
> -- 
> 2.40.1