linux-security-module.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: serge@hallyn.com (Serge E. Hallyn)
To: linux-security-module@vger.kernel.org
Subject: [PATCH V3 02/10] capabilities: intuitive names for cap gain status
Date: Thu, 24 Aug 2017 11:03:44 -0500	[thread overview]
Message-ID: <20170824160344.GB10515@mail.hallyn.com> (raw)
In-Reply-To: <9e3f99d3c2cbfee24fb3af7ec9b4cf93e4430112.1503459890.git.rgb@redhat.com>

Quoting Richard Guy Briggs (rgb at redhat.com):
> Introduce macros cap_gained, cap_grew, cap_full to make the use of the
> negation of is_subset() easier to read and analyse.
> 
> Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> ---
>  security/commoncap.c |   16 ++++++++++------
>  1 files changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/security/commoncap.c b/security/commoncap.c
> index b7fbf77..6f05ec0 100644
> --- a/security/commoncap.c
> +++ b/security/commoncap.c
> @@ -513,6 +513,12 @@ void handle_privileged_root(struct linux_binprm *bprm, bool has_cap, bool *effec
>  		*effective = true;
>  }
>  

It's subjective and so might be just me, but I think I'd find it easier
to read if it was cap_gained(source, target, field) and cap_grew(cred, source, target)

This looks correct though, so either way

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

> +#define cap_gained(field, target, source) \
> +	!cap_issubset(target->cap_##field, source->cap_##field)
> +#define cap_grew(target, source, cred) \
> +	!cap_issubset(cred->cap_##target, cred->cap_##source)
> +#define cap_full(field, cred) \
> +	cap_issubset(CAP_FULL_SET, cred->cap_##field)
>  /**
>   * cap_bprm_set_creds - Set up the proposed credentials for execve().
>   * @bprm: The execution parameters, including the proposed creds
> @@ -541,10 +547,9 @@ int cap_bprm_set_creds(struct linux_binprm *bprm)
>  	handle_privileged_root(bprm, has_cap, &effective, root_uid);
>  
>  	/* if we have fs caps, clear dangerous personality flags */
> -	if (!cap_issubset(new->cap_permitted, old->cap_permitted))
> +	if (cap_gained(permitted, new, old))
>  		bprm->per_clear |= PER_CLEAR_ON_SETID;
>  
> -
>  	/* Don't let someone trace a set[ug]id/setpcap binary with the revised
>  	 * credentials unless they have the appropriate permit.
>  	 *
> @@ -552,8 +557,7 @@ int cap_bprm_set_creds(struct linux_binprm *bprm)
>  	 */
>  	is_setid = !uid_eq(new->euid, old->uid) || !gid_eq(new->egid, old->gid);
>  
> -	if ((is_setid ||
> -	     !cap_issubset(new->cap_permitted, old->cap_permitted)) &&
> +	if ((is_setid || cap_gained(permitted, new, old)) &&
>  	    ((bprm->unsafe & ~LSM_UNSAFE_PTRACE) ||
>  	     !ptracer_capable(current, new->user_ns))) {
>  		/* downgrade; they get no more than they had, and maybe less */
> @@ -605,8 +609,8 @@ int cap_bprm_set_creds(struct linux_binprm *bprm)
>  	 * Number 1 above might fail if you don't have a full bset, but I think
>  	 * that is interesting information to audit.
>  	 */
> -	if (!cap_issubset(new->cap_effective, new->cap_ambient)) {
> -		if (!cap_issubset(CAP_FULL_SET, new->cap_effective) ||
> +	if (cap_grew(effective, ambient, new)) {
> +		if (!cap_full(effective, new) ||
>  		    !uid_eq(new->euid, root_uid) || !uid_eq(new->uid, root_uid) ||
>  		    issecure(SECURE_NOROOT)) {
>  			ret = audit_log_bprm_fcaps(bprm, new, old);
> -- 
> 1.7.1
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

  reply	other threads:[~2017-08-24 16:03 UTC|newest]

Thread overview: 57+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-08-23 10:12 [PATCH V3 00/10] capabilities: do not audit log BPRM_FCAPS on set*id Richard Guy Briggs
2017-08-23 10:12 ` [PATCH V3 01/10] capabilities: factor out cap_bprm_set_creds privileged root Richard Guy Briggs
2017-08-24 15:42   ` Serge E. Hallyn
2017-08-25  5:55   ` James Morris
2017-08-25 10:49     ` Richard Guy Briggs
2017-08-23 10:12 ` [PATCH V3 02/10] capabilities: intuitive names for cap gain status Richard Guy Briggs
2017-08-24 16:03   ` Serge E. Hallyn [this message]
2017-08-24 16:19     ` Richard Guy Briggs
2017-08-24 16:37       ` Serge E. Hallyn
2017-08-24 19:06         ` Kees Cook
2017-08-24 21:17           ` Paul Moore
2017-08-28  9:19           ` Richard Guy Briggs
2017-08-28 11:08             ` Richard Guy Briggs
2017-09-01 10:18               ` Richard Guy Briggs
2017-09-02  5:37                 ` Serge E. Hallyn
2017-09-04  6:57                   ` Richard Guy Briggs
2017-09-05  6:45                     ` Richard Guy Briggs
2017-08-25  5:56   ` James Morris
2017-08-25 15:08   ` Andy Lutomirski
2017-08-25 18:47     ` Serge E. Hallyn
2017-08-23 10:12 ` [PATCH V3 03/10] capabilities: rename has_cap to has_fcap Richard Guy Briggs
2017-08-24 16:10   ` Serge E. Hallyn
2017-08-25  5:56   ` James Morris
2017-08-23 10:12 ` [PATCH V3 04/10] capabilities: use root_priveleged inline to clarify logic Richard Guy Briggs
2017-08-24 16:14   ` Serge E. Hallyn
2017-08-25  5:58   ` James Morris
2017-08-28 12:03     ` Richard Guy Briggs
2017-08-31 14:49       ` Serge E. Hallyn
2017-08-23 10:12 ` [PATCH V3 05/10] capabilities: use intuitive names for id changes Richard Guy Briggs
2017-08-24 16:17   ` Serge E. Hallyn
2017-08-25  5:59   ` James Morris
2017-08-25 15:06   ` Andy Lutomirski
2017-08-25 18:51     ` Serge E. Hallyn
2017-08-25 19:45       ` Andy Lutomirski
2017-08-25 20:06         ` Serge E. Hallyn
2017-08-28  1:32           ` James Morris
2017-08-28  9:12           ` Richard Guy Briggs
2017-08-28 20:12             ` Andy Lutomirski
2017-08-23 10:12 ` [PATCH V3 06/10] capabilities: move audit log decision to function Richard Guy Briggs
2017-08-24 16:18   ` Serge E. Hallyn
2017-08-25  6:01   ` James Morris
2017-08-23 10:12 ` [PATCH V3 07/10] capabilities: remove a layer of conditional logic Richard Guy Briggs
2017-08-24 16:20   ` Serge E. Hallyn
2017-08-25  5:47   ` James Morris
2017-08-25 15:11   ` Andy Lutomirski
2017-08-25 18:53     ` Serge E. Hallyn
2017-08-23 10:12 ` [PATCH V3 08/10] capabilities: invert logic for clarity Richard Guy Briggs
2017-08-24 16:23   ` Serge E. Hallyn
2017-08-25  5:47   ` James Morris
2017-08-23 10:13 ` [PATCH V3 09/10] capabilities: fix logic for effective root or real root Richard Guy Briggs
2017-08-24 16:29   ` Serge E. Hallyn
2017-08-24 16:44     ` Richard Guy Briggs
2017-08-24 16:47       ` Serge E. Hallyn
2017-08-25  5:48   ` James Morris
2017-08-23 10:13 ` [PATCH V3 10/10] capabilities: audit log other surprising conditions Richard Guy Briggs
2017-08-24 16:35   ` Serge E. Hallyn
2017-08-25  5:50   ` James Morris

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=20170824160344.GB10515@mail.hallyn.com \
    --to=serge@hallyn.com \
    --cc=linux-security-module@vger.kernel.org \
    /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 NNTP newsgroup(s).