public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Eric Paris <eparis@redhat.com>
To: "Serge E. Hallyn" <serge@canonical.com>
Cc: linux-kernel@vger.kernel.org,
	linux-security-module@vger.kernel.org, sgrubb@redhat.com,
	Andrew Morgan <morgan@kernel.org>
Subject: Re: [PATCH] System Wide Capability Bounding Set
Date: Fri, 14 Jan 2011 14:50:57 -0500	[thread overview]
Message-ID: <1295034658.2816.16.camel@localhost.localdomain> (raw)
In-Reply-To: <20110111220201.GA6446@localhost>

On Tue, 2011-01-11 at 16:02 -0600, Serge E. Hallyn wrote:
> Quoting Eric Paris (eparis@redhat.com):

> > @@ -305,6 +310,8 @@ static inline int bprm_caps_from_vfs_caps(struct cpu_vfs_cap_data *caps,
> >  		new->cap_permitted.cap[i] =
> >  			(new->cap_bset.cap[i] & permitted) |
> >  			(new->cap_inheritable.cap[i] & inheritable);
> > +		/* the global set is global damn it */
> > +		new->cap_permitted.cap[i] &= global_cap_bset.cap[i];
> 
> [ If I'm thinking right: ]
> 
> Global may be global, but you're changing the formula (here, for a
> non-root task executing a file with filecaps) from
> 
> 	pP' = (X & fP) | (pI & fI)
> 
> to
> 
> 	A  = (X & FP) | (pI & fI)
> 	pP'= Z & A                    // Z == global bounding set
> 
> In other words, you are not simply enforcing "the intersection of
> the global and per-process bounding sets".
> 
> Whereas,
> 
> >  		if (permitted & ~new->cap_permitted.cap[i])
> >  			/* insufficient to execute correctly */
> > @@ -438,6 +445,9 @@ int cap_bprm_set_creds(struct linux_binprm *bprm)
> >  		return ret;
> >  
> >  	if (!issecure(SECURE_NOROOT)) {
> > +		kernel_cap_t bset = cap_intersect(old->cap_bset,
> > +						  global_cap_bset);
> > +
> >  		/*
> >  		 * If the legacy file capability is set, then don't set privs
> >  		 * for a setuid root binary run by a non-root user.  Do set it
> > @@ -456,8 +466,7 @@ int cap_bprm_set_creds(struct linux_binprm *bprm)
> >  		 */
> >  		if (new->euid == 0 || new->uid == 0) {
> >  			/* pP' = (cap_bset & ~0) | (pI & ~0) */
> > -			new->cap_permitted = cap_combine(old->cap_bset,
> > -							 old->cap_inheritable);
> > +			new->cap_permitted = cap_combine(bset, old->cap_inheritable);
> 
> here (for a root task) you are using 
> 
> 	pP' = (Z & X) | pI
> 
> So the inheritable tasks get masked with the global bounding set for
> non-root tasks, but not for root tasks.

I believe you are thinking correctly and I am wrong.  Someone else has
some other issues with the patch but would prefer to keep that
conversation offline.  I will certainly be back with changes and
explanation of changes (hopefully shortly)

Thanks Serge!

-Eric


  parent reply	other threads:[~2011-01-14 19:51 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-01-05 22:25 [PATCH] System Wide Capability Bounding Set Eric Paris
2011-01-06 11:30 ` Tetsuo Handa
2011-01-06 16:44   ` Theodore Tso
2011-01-11 22:02 ` Serge E. Hallyn
2011-01-11 22:12   ` Serge E. Hallyn
2011-01-14 19:50   ` Eric Paris [this message]
2011-01-17  3:16     ` Andrew G. Morgan
2011-01-21 21:25       ` Eric Paris
2011-01-23  3:39         ` Andrew G. Morgan
2011-01-24 21:40           ` Serge Hallyn
2011-01-26 23:34             ` Eric Paris
2011-01-27 14:02               ` Serge E. Hallyn
2011-01-27 14:42                 ` Steve Grubb
2011-01-27 16:43                   ` Andrew G. Morgan
     [not found]                   ` <AANLkTi=k5QeE_-iNuW3-M5K3BnBtRxk-QYO5624HKrpE@mail.gmail.com>
2011-01-27 16:50                     ` Steve Grubb
2011-01-28 18:19                       ` Eric Paris
2011-01-28 18:49                   ` Serge E. Hallyn
2011-01-28 19:10                     ` Steve Grubb
2011-01-28 19:38                       ` Serge E. Hallyn
2011-01-28 22:24                         ` Eric Paris
2011-02-01 18:17                         ` Eric Paris
2011-02-01 21:26                           ` Serge E. Hallyn
2011-02-02  4:02                             ` Andrew G. Morgan
2011-02-08  2:55                               ` Eric Paris
2011-02-14 20:45                                 ` Eric Paris
2011-02-14 21:24                                   ` Serge E. Hallyn
2011-02-18  0:29                                 ` Serge E. Hallyn
2011-01-27 14:26               ` Andrew G. Morgan

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=1295034658.2816.16.camel@localhost.localdomain \
    --to=eparis@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=morgan@kernel.org \
    --cc=serge@canonical.com \
    --cc=sgrubb@redhat.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