Re: [PATCH] capabilites, take 2

Re: [PATCH] capabilites, take 2

[Andy Lutomirski]

Stephen Smalley wrote:

> On Fri, 2004-05-14 at 08:03, Albert Cahalan wrote:
> 
>>This would be an excellent time to reconsider how capabilities
>>are assigned to bits. You're breaking things anyway; you might
>>as well do all the breaking at once. I want local-use bits so
>>that the print queue management access isn't by magic UID/GID.
>>We haven't escaped UID-as-priv if server apps and setuid apps
>>are still making UID-based access control decisions.
> 
> 
> Capabilities are a broken model for privilege management; try RBAC/TE
> instead.  http://www.securecomputing.com/pdf/secureos.pdf has notes
> about the history and comparison of capabilities vs. TE.
> 
> Instead of adding new capability bits, replace capable() calls with LSM
> hook calls that offer you finer granularity both in operation and in
> object-based decisions, and then use a security module like SELinux to
> map that to actual permission checks.  SELinux provides a framework for
> defining security classes and permissions, including both definitions
> used by the kernel and definitions used by userspace policy enforcers
> (ala security-enhanced X).
> 

Thanks -- turning brain back on, SELinux is obviously better than any
fine-grained capability scheme I can imagine.

So unless anyone convinces me you're wrong, I'll stick with just
fixing up capabilities to work without making them finer-grained.

--Andy

Re: [PATCH] capabilites, take 2

[Stephen Smalley]

On Fri, 2004-05-14 at 11:57, Andy Lutomirski wrote:
> Thanks -- turning brain back on, SELinux is obviously better than any
> fine-grained capability scheme I can imagine.
> 
> So unless anyone convinces me you're wrong, I'll stick with just
> fixing up capabilities to work without making them finer-grained.

Great, thanks.  Fixing capabilities to work is definitely useful and
desirable.  Significantly expanding them in any manner is a poor use of
limited resources, IMHO; I'd much rather see people work on applying
SELinux to the problem and solving it more effectively for the future.

-- 
Stephen Smalley <sds@epoch.ncsc.mil>
National Security Agency

Re: [PATCH] capabilites, take 2

[Andy Lutomirski]

Stephen Smalley wrote:

> On Fri, 2004-05-14 at 11:57, Andy Lutomirski wrote:
> 
>>Thanks -- turning brain back on, SELinux is obviously better than any
>>fine-grained capability scheme I can imagine.
>>
>>So unless anyone convinces me you're wrong, I'll stick with just
>>fixing up capabilities to work without making them finer-grained.
> 
> 
> Great, thanks.  Fixing capabilities to work is definitely useful and
> desirable.  Significantly expanding them in any manner is a poor use of
> limited resources, IMHO; I'd much rather see people work on applying
> SELinux to the problem and solving it more effectively for the future.
> 

Does this mean I should trash my 'maximum' mask?

(I like 'cap -c = sftp-server' so it can't try to run setuid/fP apps.)
OTOH, since SELinux accomplishes this better, it may not be worth the
effort.

--Andy

Re: [PATCH] capabilites, take 2

[Stephen Smalley]

On Fri, 2004-05-14 at 12:18, Andy Lutomirski wrote:
> Does this mean I should trash my 'maximum' mask?
> 
> (I like 'cap -c = sftp-server' so it can't try to run setuid/fP apps.)
> OTOH, since SELinux accomplishes this better, it may not be worth the
> effort.

Not my call to make, but I'm not sure it is worthwhile.  Even filesystem
capabilities are questionable IMHO, as you can already authorize
capabilities based on program and call chain (typically just the
immediate caller's domain, but potentially encoding the entire call
chain in the domain transition rules to track any untrusted components
in the call chain) today using SELinux TE.  Today, the program still has
to be setuid as well, as we currently check both ordinary Linux
capability and SELinux permission for each operation to be conservative,
but eventually we would hope to drop the use of the capability module as
a secondary module and let SELinux manage the privileges entirely (once
the TE policy configuration is fully locked down and userland is fully
aware of SELinux).

-- 
Stephen Smalley <sds@epoch.ncsc.mil>
National Security Agency

Re: [PATCH] capabilites, take 2

[Chris Wright]

* Andy Lutomirski (luto@stanford.edu) wrote:
> Stephen Smalley wrote:
> > On Fri, 2004-05-14 at 11:57, Andy Lutomirski wrote:
> > > Thanks -- turning brain back on, SELinux is obviously better than any
> > > fine-grained capability scheme I can imagine.
> > > 
> > > So unless anyone convinces me you're wrong, I'll stick with just
> > > fixing up capabilities to work without making them finer-grained.
> > 
> > Great, thanks.  Fixing capabilities to work is definitely useful and
> > desirable.  Significantly expanding them in any manner is a poor use of
> > limited resources, IMHO; I'd much rather see people work on applying
> > SELinux to the problem and solving it more effectively for the future.
> 
> Does this mean I should trash my 'maximum' mask?
> 
> (I like 'cap -c = sftp-server' so it can't try to run setuid/fP apps.)
> OTOH, since SELinux accomplishes this better, it may not be worth the
> effort.

Let's just get back to the simplest task.  Allow execve() to do smth.
reasonable with capabilities.

thanks,
-chris
-- 
Linux Security Modules     http://lsm.immunix.org     http://lsm.bkbits.net

[PATCH] scaled-back caps, take 4 (was Re: [PATCH] capabilites, take 2)

[Andy Lutomirski]

On Friday 14 May 2004 11:07, Chris Wright wrote:
> * Andy Lutomirski (luto@stanford.edu) wrote:
> > Stephen Smalley wrote:
> > > On Fri, 2004-05-14 at 11:57, Andy Lutomirski wrote:
> > > > Thanks -- turning brain back on, SELinux is obviously better than any
> > > > fine-grained capability scheme I can imagine.
> > > > 
> > > > So unless anyone convinces me you're wrong, I'll stick with just
> > > > fixing up capabilities to work without making them finer-grained.
> > > 
> > > Great, thanks.  Fixing capabilities to work is definitely useful and
> > > desirable.  Significantly expanding them in any manner is a poor use of
> > > limited resources, IMHO; I'd much rather see people work on applying
> > > SELinux to the problem and solving it more effectively for the future.
> > 
> > Does this mean I should trash my 'maximum' mask?
> > 
> > (I like 'cap -c = sftp-server' so it can't try to run setuid/fP apps.)
> > OTOH, since SELinux accomplishes this better, it may not be worth the
> > effort.
> 
> Let's just get back to the simplest task.  Allow execve() to do smth.
> reasonable with capabilities.

Sold.

This version throws out the inheritable mask.  There is no change in
behavior with newcaps=0.

All that's left in newcaps=1 mode is:

fP := permitted (app gains these)
pP := permitted (app can make them effective)
pE := effective

There is no inheritable mask :)  So we can't argue about what it's
supposed to / not supposed to.

It's not too well tested yet.  Enjoy.

--Andy

cap_2.6.6-mm2_4.patch: New stripped-back capabilities.

 fs/exec.c               |   15 ++++-
 include/linux/binfmts.h |    9 ++-
 security/commoncap.c    |  130 ++++++++++++++++++++++++++++++++++++++++++------
 3 files changed, 136 insertions(+), 18 deletions(-)

--- linux-2.6.6-mm2/fs/exec.c~caps	2004-05-13 11:42:26.000000000 -0700
+++ linux-2.6.6-mm2/fs/exec.c	2004-05-14 12:36:17.000000000 -0700
@@ -882,8 +882,10 @@
 
 	if(!(bprm->file->f_vfsmnt->mnt_flags & MNT_NOSUID)) {
 		/* Set-uid? */
-		if (mode & S_ISUID)
+		if (mode & S_ISUID) {
 			bprm->e_uid = inode->i_uid;
+			bprm->secflags |= BINPRM_SEC_SETUID;
+		}
 
 		/* Set-gid? */
 		/*
@@ -891,10 +893,18 @@
 		 * is a candidate for mandatory locking, not a setgid
 		 * executable.
 		 */
-		if ((mode & (S_ISGID | S_IXGRP)) == (S_ISGID | S_IXGRP))
+		if ((mode & (S_ISGID | S_IXGRP)) == (S_ISGID | S_IXGRP)) {
 			bprm->e_gid = inode->i_gid;
+			bprm->secflags |= BINPRM_SEC_SETGID;
+		}
 	}
 
+	/* Pretend we have VFS capabilities */
+	if ((bprm->secflags & BINPRM_SEC_SETUID) && bprm->e_uid == 0)
+		cap_set_full(bprm->cap_permitted);
+	else
+		cap_clear(bprm->cap_permitted);
+
 	/* fill in binprm security blob */
 	retval = security_bprm_set(bprm);
 	if (retval)
@@ -1089,6 +1099,7 @@
 	bprm.loader = 0;
 	bprm.exec = 0;
 	bprm.security = NULL;
+	bprm.secflags = 0;
 	bprm.mm = mm_alloc();
 	retval = -ENOMEM;
 	if (!bprm.mm)
--- linux-2.6.6-mm2/security/commoncap.c~caps	2004-05-13 11:42:26.000000000 -0700
+++ linux-2.6.6-mm2/security/commoncap.c	2004-05-14 13:24:45.000000000 -0700
@@ -24,6 +24,11 @@
 #include <linux/xattr.h>
 #include <linux/hugetlb.h>
 
+static int newcaps = 0;
+
+module_param(newcaps, int, 444);
+MODULE_PARM_DESC(newcaps, "Set newcaps=1 to enable experimental capabilities");
+
 int cap_capable (struct task_struct *tsk, int cap)
 {
 	/* Derived from include/linux/sched.h:capable. */
@@ -48,17 +53,19 @@
 {
 	/* Derived from kernel/capability.c:sys_capget. */
 	*effective = cap_t (target->cap_effective);
-	*inheritable = cap_t (target->cap_inheritable);
 	*permitted = cap_t (target->cap_permitted);
+	if (newcaps)
+		*inheritable = CAP_EMPTY_SET;
+	else
+		*inheritable = cap_t (target->cap_inheritable);
 	return 0;
 }
 
 int cap_capset_check (struct task_struct *target, kernel_cap_t *effective,
 		      kernel_cap_t *inheritable, kernel_cap_t *permitted)
 {
-	/* Derived from kernel/capability.c:sys_capset. */
 	/* verify restrictions on target's new Inheritable set */
-	if (!cap_issubset (*inheritable,
+	if (!newcaps && !cap_issubset (*inheritable,
 			   cap_combine (target->cap_inheritable,
 					current->cap_permitted))) {
 		return -EPERM;
@@ -83,12 +90,16 @@
 		     kernel_cap_t *inheritable, kernel_cap_t *permitted)
 {
 	target->cap_effective = *effective;
-	target->cap_inheritable = *inheritable;
 	target->cap_permitted = *permitted;
+	if (!newcaps)
+		target->cap_inheritable = *inheritable;
 }
 
 int cap_bprm_set_security (struct linux_binprm *bprm)
 {
+	if (newcaps)
+		return 0;
+
 	/* Copied from fs/exec.c:prepare_binprm. */
 
 	/* We don't have VFS support for capabilities yet */
@@ -115,9 +126,9 @@
 	return 0;
 }
 
-void cap_bprm_apply_creds (struct linux_binprm *bprm, int unsafe)
+static void cap_bprm_apply_creds_compat (struct linux_binprm *bprm, int unsafe)
 {
-	/* Derived from fs/exec.c:compute_creds. */
+	/* This function will hopefully die in 2.7. */
 	kernel_cap_t new_permitted, working;
 
 	new_permitted = cap_intersect (bprm->cap_permitted, cap_bset);
@@ -158,15 +169,88 @@
 	current->keep_capabilities = 0;
 }
 
+/*
+ * The rules of Linux capabilities (not POSIX!)
+ *
+ * What the masks mean:
+ *  pP = capabilities that this process has
+ *  pE = capabilities that this process has and are enabled
+ * (so pE <= pP)
+ *
+ * The capability evolution rules are:
+ *
+ *  pP' = ((fP & cap_bset) | pP) & Y
+ *  pE' = (pE | fP) & pP'
+ *
+ *  X = cap_bset
+ *  Y is zero if uid!=0, euid==0, and setuid non-root
+ *
+ *  Exception: if setuid-nonroot, then pE' is reset to 0.
+ *
+ * If file capabilities are introduced, programs that are granted
+ * fP > 0 need to be aware of how to deal with it.
+ * Because the effective set is left alone on non-setuid fP>0,
+ * such a program should drop capabilities that were not in its initial
+ * effective set before running untrusted code.
+ *
+ */
+void cap_bprm_apply_creds (struct linux_binprm *bprm, int unsafe)
+{
+	kernel_cap_t new_pP, new_pE, fP;
+	int is_setuid, is_setgid;
+
+	if(!newcaps) {
+		cap_bprm_apply_creds_compat(bprm, unsafe);
+		return;
+	}
+
+	fP = bprm->cap_permitted;
+	is_setuid = (bprm->secflags & BINPRM_SEC_SETUID);
+	is_setgid = (bprm->secflags & BINPRM_SEC_SETGID);
+
+	/* Check that it's safe to elevate privileges */
+	if (unsafe & ~LSM_UNSAFE_PTRACE_CAP)
+		bprm->secflags |= BINPRM_SEC_NOELEVATE;
+
+	if (bprm->secflags & BINPRM_SEC_NOELEVATE) {
+		is_setuid = is_setgid = 0;
+		cap_clear(fP);
+	}
+
+	new_pP = cap_intersect(fP, cap_bset);
+	new_pP = cap_combine(new_pP, current->cap_permitted);
+
+	/* setuid-nonroot is special. */
+	if (is_setuid && bprm->e_uid != 0 && current->uid != 0 &&
+	    current->euid == 0)
+		cap_clear(new_pP);
+
+	if (!cap_issubset(new_pP, current->cap_permitted))
+		bprm->secflags |= BINPRM_SEC_SECUREEXEC;
+
+	new_pE = cap_combine(current->cap_effective, fP);
+	cap_mask(new_pE, new_pP);
+	if (is_setuid && bprm->e_uid != 0)
+		cap_clear(new_pE);
+
+	/* Apply new security state */
+	if (is_setuid) {
+	        current->suid = current->euid = current->fsuid = bprm->e_uid;
+	}
+	if (is_setgid)
+	        current->sgid = current->egid = current->fsgid = bprm->e_gid;
+
+	current->cap_permitted = new_pP;
+	current->cap_effective = new_pE;
+
+	current->keep_capabilities = 0;
+}
+
 int cap_bprm_secureexec (struct linux_binprm *bprm)
 {
-	/* If/when this module is enhanced to incorporate capability
-	   bits on files, the test below should be extended to also perform a 
-	   test between the old and new capability sets.  For now,
-	   it simply preserves the legacy decision algorithm used by
-	   the old userland. */
 	return (current->euid != current->uid ||
-		current->egid != current->gid);
+		current->egid != current->gid ||
+		(bprm->secflags & BINPRM_SEC_SECUREEXEC));
 }
 
 int cap_inode_setxattr(struct dentry *dentry, char *name, void *value,
@@ -280,9 +364,14 @@
 
 void cap_task_reparent_to_init (struct task_struct *p)
 {
-	p->cap_effective = CAP_INIT_EFF_SET;
-	p->cap_inheritable = CAP_INIT_INH_SET;
-	p->cap_permitted = CAP_FULL_SET;
+	if (newcaps) {
+		cap_set_full(p->cap_permitted);
+		cap_set_full(p->cap_effective);
+	} else {
+		p->cap_effective = CAP_INIT_EFF_SET;
+		p->cap_inheritable = CAP_INIT_INH_SET;
+		p->cap_permitted = CAP_FULL_SET;
+	}
 	p->keep_capabilities = 0;
 	return;
 }
@@ -367,6 +456,15 @@
 	return -ENOMEM;
 }
 
+static int __init commoncap_init (void)
+{
+	if (newcaps) {
+		printk(KERN_NOTICE "Experimental capability support is on\n");
+	}
+
+	return 0;
+}
+
 EXPORT_SYMBOL(cap_capable);
 EXPORT_SYMBOL(cap_ptrace);
 EXPORT_SYMBOL(cap_capget);
@@ -382,5 +480,7 @@
 EXPORT_SYMBOL(cap_syslog);
 EXPORT_SYMBOL(cap_vm_enough_memory);
 
+module_init(commoncap_init);
+
 MODULE_DESCRIPTION("Standard Linux Common Capabilities Security Module");
 MODULE_LICENSE("GPL");
--- linux-2.6.6-mm2/include/linux/binfmts.h~caps	2004-05-13 11:42:26.000000000 -0700
+++ linux-2.6.6-mm2/include/linux/binfmts.h	2004-05-14 10:28:05.000000000 -0700
@@ -20,6 +20,10 @@
 /*
  * This structure is used to hold the arguments that are used when loading binaries.
  */
+#define BINPRM_SEC_SETUID	1
+#define BINPRM_SEC_SETGID	2
+#define BINPRM_SEC_SECUREEXEC	4
+#define BINPRM_SEC_NOELEVATE	8
 struct linux_binprm{
 	char buf[BINPRM_BUF_SIZE];
 	struct page *page[MAX_ARG_PAGES];
@@ -28,7 +32,10 @@
 	int sh_bang;
 	struct file * file;
 	int e_uid, e_gid;
-	kernel_cap_t cap_inheritable, cap_permitted, cap_effective;
+	int secflags;
+	kernel_cap_t cap_permitted;
+	/* old caps -- do NOT use in new code */
+	kernel_cap_t cap_inheritable, cap_effective;
 	void *security;
 	int argc, envc;
 	char * filename;	/* Name of binary as seen by procps */

Re: [PATCH] scaled-back caps, take 4

[Olaf Dietsche]

Andy Lutomirski <luto@myrealbox.com> writes:

> cap_2.6.6-mm2_4.patch: New stripped-back capabilities.
>
>  fs/exec.c               |   15 ++++-
>  include/linux/binfmts.h |    9 ++-
>  security/commoncap.c    |  130 ++++++++++++++++++++++++++++++++++++++++++------
>  3 files changed, 136 insertions(+), 18 deletions(-)
[patch]

Why don't you provide this as a configurable andycap.c module?
I think, this is the whole point of LSM.

Regards, Olaf.

Re: [PATCH] scaled-back caps, take 4

[Albert Cahalan]

On Fri, 2004-05-14 at 20:06, Olaf Dietsche wrote:
> Andy Lutomirski <luto@myrealbox.com> writes:
> 
> > cap_2.6.6-mm2_4.patch: New stripped-back capabilities.
> >
> >  fs/exec.c               |   15 ++++-
> >  include/linux/binfmts.h |    9 ++-
> >  security/commoncap.c    |  130 ++++++++++++++++++++++++++++++++++++++++++------
> >  3 files changed, 136 insertions(+), 18 deletions(-)
> [patch]
> 
> Why don't you provide this as a configurable andycap.c module?

**GROAN**

For those that don't know, this is Andy Cap:
http://picosel.free.fr/andycap/andy_billard.jpg

Re: [PATCH] scaled-back caps, take 4

[Chris Wright]

* Olaf Dietsche (olaf+list.linux-kernel@olafdietsche.de) wrote:
> Andy Lutomirski <luto@myrealbox.com> writes:
> 
> > cap_2.6.6-mm2_4.patch: New stripped-back capabilities.
> >
> >  fs/exec.c               |   15 ++++-
> >  include/linux/binfmts.h |    9 ++-
> >  security/commoncap.c    |  130 ++++++++++++++++++++++++++++++++++++++++++------
> >  3 files changed, 136 insertions(+), 18 deletions(-)
> [patch]
> 
> Why don't you provide this as a configurable andycap.c module?
> I think, this is the whole point of LSM.

I agree, if we can't find a clean way to do it.  However, note this
includes changes to core.  And it's nice to fix this for the base case.

thanks,
-chris
-- 
Linux Security Modules     http://lsm.immunix.org     http://lsm.bkbits.net

Re: [PATCH] scaled-back caps, take 4 (was Re: [PATCH] capabilites, take 2)

[Andy Lutomirski]

Chris Wright wrote:
> * Andy Lutomirski (luto@myrealbox.com) wrote:
> 
>>This version throws out the inheritable mask.  There is no change in
>>behavior with newcaps=0.
> 
> 
> Alright, I tried to write up my expectations for all the various modes
> w.r.t dropping privs, keeping them, setuid apps, etc.  I then wrote some
> tests to get a baseline, and well as a way to compare results.  Finally
> I wrote a patch that meets the requirements I laid out, and compared it
> against yours.  With one minor exception, the capabilities bits match
> up.  You drop effective caps when a non-root users execs a non-root
> setuid app, and I the caps alone.  ...

Paranoia.  There are legacy setuid programs out there (presumably even 
setuid-nonroot).  Let's make them behave as closely to the way they 
currently do as possible.


 > ... One side note, you're changes effect
> the user/group saved ids unexpectedly.

Oops.  That's a trivial fix.

> 
> Here's a bunch of test cases:

> # Still w/out changing inheritable and with KEEPCAPS set
> # 10 Root process does setuid(!0), effective caps are dropped
> # 11 Root process does seteuid(!0), effective caps are dropped
> # 12 Nonroot process restores uid 0, effective restored to permitted

I still want some variant on KEEPCAPS that causes setxuid to leave caps 
completely alone.  Oh, well.

> # 18 Non-root execs setuid-nonroot process, new caps bound by inheritable

What new caps?

>>cap_2.6.6-mm2_4.patch: New stripped-back capabilities.
>>
>> fs/exec.c               |   15 ++++-
>> include/linux/binfmts.h |    9 ++-
>> security/commoncap.c    |  130 ++++++++++++++++++++++++++++++++++++++++++------
>> 3 files changed, 136 insertions(+), 18 deletions(-)
>>
>>--- linux-2.6.6-mm2/fs/exec.c~caps	2004-05-13 11:42:26.000000000 -0700
>>+++ linux-2.6.6-mm2/fs/exec.c	2004-05-14 12:36:17.000000000 -0700
>>@@ -882,8 +882,10 @@
>> 
>> 	if(!(bprm->file->f_vfsmnt->mnt_flags & MNT_NOSUID)) {
>> 		/* Set-uid? */
>>-		if (mode & S_ISUID)
>>+		if (mode & S_ISUID) {
>> 			bprm->e_uid = inode->i_uid;
>>+			bprm->secflags |= BINPRM_SEC_SETUID;
>>+		}
> 
> 
> No strong objection, but I don't think it's necessary.

I wanted to distinguish between setuid-me and non-setuid in apply_creds. 
That one doesn't matter much, though.

> 
> 
>> 
>> 		/* Set-gid? */
>> 		/*
>>@@ -891,10 +893,18 @@
>> 		 * is a candidate for mandatory locking, not a setgid
>> 		 * executable.
>> 		 */
>>-		if ((mode & (S_ISGID | S_IXGRP)) == (S_ISGID | S_IXGRP))
>>+		if ((mode & (S_ISGID | S_IXGRP)) == (S_ISGID | S_IXGRP)) {
>> 			bprm->e_gid = inode->i_gid;
>>+			bprm->secflags |= BINPRM_SEC_SETGID;
>>+		}
>> 	}
>> 
>>+	/* Pretend we have VFS capabilities */
>>+	if ((bprm->secflags & BINPRM_SEC_SETUID) && bprm->e_uid == 0)
>>+		cap_set_full(bprm->cap_permitted);
>>+	else
>>+		cap_clear(bprm->cap_permitted);
>>+
> 
> 
> Thus far we've kept all this stuff out of core.  I believe we should
> keep it that way.  This could easily be left in bprm_set().

True.  But as long as linux_binprm has fields for this stuff, intuitively 
it should always be filled in (i.e. not just in commoncap).  That way we 
can say that commoncap doesn't get special treatment :)

Also, this seems like the right place to check for VFS caps.  This way we can.

> 
> 
>>--- linux-2.6.6-mm2/security/commoncap.c~caps	2004-05-13 11:42:26.000000000 -0700
>>+++ linux-2.6.6-mm2/security/commoncap.c	2004-05-14 13:24:45.000000000 -0700
>>@@ -24,6 +24,11 @@
>> #include <linux/xattr.h>
>> #include <linux/hugetlb.h>
>> 
>>+static int newcaps = 0;
>>+
>>+module_param(newcaps, int, 444);
>>+MODULE_PARM_DESC(newcaps, "Set newcaps=1 to enable experimental capabilities");
> 
> 
> It would be really nice to have a one size fits all solution.  Esp.
> since the legacy mode is what one gets when leaving inheritable mask
> untouched.

I agree.  Andrew specifically asked for this, though, at least until this 
stuff is well-tested and everyone likes it.

> 
> 
>> int cap_bprm_set_security (struct linux_binprm *bprm)
>> {
>>+	if (newcaps)
>>+		return 0;
>>+
> 
> 
> That setup could go here instead of in core.
> 
> 

[snip]
>> 
>>+/*
>>+ * The rules of Linux capabilities (not POSIX!)
>>+ *
>>+ * What the masks mean:
>>+ *  pP = capabilities that this process has
>>+ *  pE = capabilities that this process has and are enabled
>>+ * (so pE <= pP)
>>+ *
>>+ * The capability evolution rules are:
>>+ *
>>+ *  pP' = ((fP & cap_bset) | pP) & Y
>>+ *  pE' = (pE | fP) & pP'
>>+ *
>>+ *  X = cap_bset
>>+ *  Y is zero if uid!=0, euid==0, and setuid non-root
>>+ *
>>+ *  Exception: if setuid-nonroot, then pE' is reset to 0.
> 
> 
> While this works fine, I was interested to see what we could do to
> leave the old algorithm.  Seems both work out fine.
> 
> 
>>+	/* setuid-nonroot is special. */
>>+	if (is_setuid && bprm->e_uid != 0 && current->uid != 0 &&
>>+	    current->euid == 0)
>>+		cap_clear(new_pP);
> 
> 
> setuid-nonroot from a non-root user needs to clear effective?

Yes.  Say user 500 runs a setuid-root program, which goes back and runs a 
setuid-500 program.  uid==euid==500 now, so the program expects to be 
unprivileged.  This makes that work.  (I'm assuming you meant permitted. 
Effective gets cleared in cap_mask(new_pE, new_pP)).


The reason I killed the old algorithm is because it's scary (in the sense 
of being complicated and subtle for no good reason) and because I don't see 
the point of inheritable caps.  I doubt anything uses them currently on a 
vanilla kernel because they don't _do_ anything.  So I killed them.


--Andy

Re: [PATCH] scaled-back caps, take 4 (was Re: [PATCH] capabilites, take 2)

[Chris Wright]

* Andy Lutomirski (luto@stanford.edu) wrote:
> Chris Wright wrote:
> > Alright, I tried to write up my expectations for all the various modes
> > w.r.t dropping privs, keeping them, setuid apps, etc.  I then wrote some
> > tests to get a baseline, and well as a way to compare results.  Finally
> > I wrote a patch that meets the requirements I laid out, and compared it
> > against yours.  With one minor exception, the capabilities bits match
> > up.  You drop effective caps when a non-root users execs a non-root
> > setuid app, and I the caps alone.  ...
> 
> Paranoia.  There are legacy setuid programs out there (presumably even 
> setuid-nonroot).  Let's make them behave as closely to the way they 
> currently do as possible.

Yes, that's the goal I have.  Although, the above scenario, they've
already been limited (IOW, if nothing's been touched, all behaves the
same).  Starting with limited inheritable (as say uid 500), execute
a non-root, setuid (say uid 99) program, is this expected to change
effective set?  Currently it's a transition for 0 caps to 0 caps, not
very interesting.  Given they are both unprivelged uids, I kept
the (limited) effective.

> > # Still w/out changing inheritable and with KEEPCAPS set
> > # 10 Root process does setuid(!0), effective caps are dropped
> > # 11 Root process does seteuid(!0), effective caps are dropped
> > # 12 Nonroot process restores uid 0, effective restored to permitted
> 
> I still want some variant on KEEPCAPS that causes setxuid to leave caps 
> completely alone.  Oh, well.

Yeah, digs hole deeper though.

> > # 18 Non-root execs setuid-nonroot process, new caps bound by inheritable
> 
> What new caps?

The caps in the newly exec'd process.  IOW, effective aren't dropped,
but as you'd expect inheritable provides limit.

> >>+	/* Pretend we have VFS capabilities */
> >>+	if ((bprm->secflags & BINPRM_SEC_SETUID) && bprm->e_uid == 0)
> >>+		cap_set_full(bprm->cap_permitted);
> >>+	else
> >>+		cap_clear(bprm->cap_permitted);
> >>+
> > 
> > 
> > Thus far we've kept all this stuff out of core.  I believe we should
> > keep it that way.  This could easily be left in bprm_set().
> 
> True.  But as long as linux_binprm has fields for this stuff, intuitively 
> it should always be filled in (i.e. not just in commoncap).  That way we 
> can say that commoncap doesn't get special treatment :)
> 
> Also, this seems like the right place to check for VFS caps.  This way we can.

This does change the current notion of layering.  I see your point though, 
likening it to say reading inode and finding S_ISUID bit.

> >>+ * The rules of Linux capabilities (not POSIX!)
> >>+ *
> >>+ * What the masks mean:
> >>+ *  pP = capabilities that this process has
> >>+ *  pE = capabilities that this process has and are enabled
> >>+ * (so pE <= pP)
> >>+ *
> >>+ * The capability evolution rules are:
> >>+ *
> >>+ *  pP' = ((fP & cap_bset) | pP) & Y
> >>+ *  pE' = (pE | fP) & pP'
> >>+ *
> >>+ *  X = cap_bset
> >>+ *  Y is zero if uid!=0, euid==0, and setuid non-root
> >>+ *
> >>+ *  Exception: if setuid-nonroot, then pE' is reset to 0.
> > 
> > While this works fine, I was interested to see what we could do to
> > leave the old algorithm.  Seems both work out fine.
> > 
> >>+	/* setuid-nonroot is special. */
> >>+	if (is_setuid && bprm->e_uid != 0 && current->uid != 0 &&
> >>+	    current->euid == 0)
> >>+		cap_clear(new_pP);
> > 
> > 
> > setuid-nonroot from a non-root user needs to clear effective?
> 
> Yes.  Say user 500 runs a setuid-root program, which goes back and runs a 
> setuid-500 program.  uid==euid==500 now, so the program expects to be 
> unprivileged.  This makes that work.  (I'm assuming you meant permitted. 
> Effective gets cleared in cap_mask(new_pE, new_pP)).

Yup, I see.  This works in my patch as well.  I'll add this test.  Also
added test to check disabling priv escalation during ptrace of setuid
app.

> The reason I killed the old algorithm is because it's scary (in the sense 
> of being complicated and subtle for no good reason) and because I don't see 
> the point of inheritable caps.  I doubt anything uses them currently on a 
> vanilla kernel because they don't _do_ anything.  So I killed them.

This does break all those caps aware apps (yeah, tongue-in-cheek ;-)
that actually have the idea to widen the effective set, yet limit the
inheriable set.  Seriously, I don't know how much this matters.

thanks,
-chris
-- 
Linux Security Modules     http://lsm.immunix.org     http://lsm.bkbits.net

Re: [PATCH] scaled-back caps, take 4

[Andy Lutomirski]

Chris Wright wrote:

> * Andy Lutomirski (luto@stanford.edu) wrote:
> 
>>Chris Wright wrote:
>>
>>>
>>>
>>>Thus far we've kept all this stuff out of core.  I believe we should
>>>keep it that way.  This could easily be left in bprm_set().
>>
>>True.  But as long as linux_binprm has fields for this stuff, intuitively 
>>it should always be filled in (i.e. not just in commoncap).  That way we 
>>can say that commoncap doesn't get special treatment :)
>>
>>Also, this seems like the right place to check for VFS caps.  This way we can.
> 
> 
> This does change the current notion of layering.  I see your point though, 
> likening it to say reading inode and finding S_ISUID bit.

On the other hand, no one would put reading of a SELinux security label 
here.  But we already have fields in binprm specifically for commoncap.  I 
have no strong preference.


>>The reason I killed the old algorithm is because it's scary (in the sense 
>>of being complicated and subtle for no good reason) and because I don't see 
>>the point of inheritable caps.  I doubt anything uses them currently on a 
>>vanilla kernel because they don't _do_ anything.  So I killed them.
> 
> 
> This does break all those caps aware apps (yeah, tongue-in-cheek ;-)
> that actually have the idea to widen the effective set, yet limit the
> inheriable set.  Seriously, I don't know how much this matters.

Yah, they're broken anyway right now if that's what they're doing.

The reason I didn't go for something like your approach (other than not 
thinking of it) was that, as long as we're changing the semantics, we might 
as well make them as clean as possible.  I also didn't mind ripping out 
lots of old code :).  If the inheritable mask stays, then programs need to 
be audited for what happens if they are run with different inheritable 
masks.  I'd rather just eliminate that complication and the corresponding 
blob of commoncap code.  Obviously my patch fails a lot of your tests as a 
result.

So do we arm-wrestle over whose implementation wins? :)  I'd say mine wins 
on readability (not your fault -- the old code was pretty bad to begin 
with) and some simplicity, but yours has the benefit of being less intrusive.

--Andy

Re: [PATCH] scaled-back caps, take 4

[Chris Wright]

* Andy Lutomirski (luto@myrealbox.com) wrote:
> Chris Wright wrote:
> > This does change the current notion of layering.  I see your point though, 
> > likening it to say reading inode and finding S_ISUID bit.
> 
> On the other hand, no one would put reading of a SELinux security label 
> here.  But we already have fields in binprm specifically for commoncap.  I 
> have no strong preference.

Yes, I stopped short of that argument only because capabilities fall
into a bit more of a gray zone than other modules.  But I do prefer
leaving it in the module.

> >>The reason I killed the old algorithm is because it's scary (in the sense 
> >>of being complicated and subtle for no good reason) and because I don't see 
> >>the point of inheritable caps.  I doubt anything uses them currently on a 
> >>vanilla kernel because they don't _do_ anything.  So I killed them.
> > 
> > This does break all those caps aware apps (yeah, tongue-in-cheek ;-)
> > that actually have the idea to widen the effective set, yet limit the
> > inheriable set.  Seriously, I don't know how much this matters.
> 
> Yah, they're broken anyway right now if that's what they're doing.

On Linux they are.  On IRIX they aren't.  This is part of the issue as I
see it.

> The reason I didn't go for something like your approach (other than not 
> thinking of it) was that, as long as we're changing the semantics, we might 
> as well make them as clean as possible.  I also didn't mind ripping out 
> lots of old code :).  If the inheritable mask stays, then programs need to 
> be audited for what happens if they are run with different inheritable 
> masks.  I'd rather just eliminate that complication and the corresponding 
> blob of commoncap code.  Obviously my patch fails a lot of your tests as a 
> result.

Actually the only glaring difference (aside from the uid/suid and non-root
execs nonroot-yet-diff-id-setuid-app issue I mentioned earlier) is in
something like "=ep cap_setpcap-ep cap_ipc_lock+i" IIRC.

I have the feeling we both are after the same thing, which is introducing
the ability to keep some caps through exec and still being able to sleep
at night w/ confidence that there isn't some subtle new hole lurking.
This is why I aimed to change as little code as possible.

> So do we arm-wrestle over whose implementation wins? :)  I'd say mine wins 
> on readability (not your fault -- the old code was pretty bad to begin 
> with) and some simplicity, but yours has the benefit of being less intrusive.

Hehe, arm wrestling could be entertaining ;-)  I'm in favor of the most
conservative change, which I feel is in my patch.  But I'm game to
continue to pick on each.

thanks,
-chris
-- 
Linux Security Modules     http://lsm.immunix.org     http://lsm.bkbits.net

Re: [PATCH] scaled-back caps, take 4

[Andy Lutomirski]

On Wednesday 19 May 2004 00:30, Chris Wright wrote:
> * Andy Lutomirski (luto@myrealbox.com) wrote:
> > Chris Wright wrote:

> > The reason I didn't go for something like your approach (other than not 
> > thinking of it) was that, as long as we're changing the semantics, we might 
> > as well make them as clean as possible.  I also didn't mind ripping out 
> > lots of old code :).  If the inheritable mask stays, then programs need to 
> > be audited for what happens if they are run with different inheritable 
> > masks.  I'd rather just eliminate that complication and the corresponding 
> > blob of commoncap code.  Obviously my patch fails a lot of your tests as a 
> > result.
> 
> Actually the only glaring difference (aside from the uid/suid and non-root
> execs nonroot-yet-diff-id-setuid-app issue I mentioned earlier) is in
> something like "=ep cap_setpcap-ep cap_ipc_lock+i" IIRC.
> 
> I have the feeling we both are after the same thing, which is introducing
> the ability to keep some caps through exec and still being able to sleep
> at night w/ confidence that there isn't some subtle new hole lurking.
> This is why I aimed to change as little code as possible.
> 
> > So do we arm-wrestle over whose implementation wins? :)  I'd say mine wins 
> > on readability (not your fault -- the old code was pretty bad to begin 
> > with) and some simplicity, but yours has the benefit of being less intrusive.
> 
> Hehe, arm wrestling could be entertaining ;-)  I'm in favor of the most
> conservative change, which I feel is in my patch.  But I'm game to
> continue to pick on each.
> 

You don't like my patch because it rips out a bunch of code and it's
not clear it won't break stuff.

I don't like your patch because it takes a bunch of incomprehensible
code that does almost nothing and tweaks it slightly to do something
useful.  (That's not to say it's does the wrong thing; I just don't
think the code is clear.)

So I decided to figure out what was going on with the original code.

First, CAP_SETPCAP is never obtainable (by anything).
Since cap_bset never has this bit set, nothing can inherit it
from fP.  capset_check prevents it from getting set in pI.

Second, cap_bset is broken.  For one thing, there's no way
to remove the caps you want to restrict from already-running
tasks.  So I don't think it matters if we break/change it.


cap_bprm_set_security does:
fP = fI = (new_uid == 0 || new_euid == 0)
fE = (new_euid == 0)

cap_bprm_apply_creds then does:
1. pP' = (fP & X) | (fI & pI)
2. unsafeness stuff
3. fix up uids
4. pE' = fE & pP'

Now for the fun part:

Since fP == fI, pP' = fP & (X | pI)
But X | pI == X (since pI can't have CAP_SETPCAP and X==~CAP_SETPCAP)
So pP' = fP & X

pE' == fE & pP' == fE & fP & X
But fE & fP == fE, so:
pE' = fE & X

The whole result is just
pP' = (uid == 0 || euid==0) & X
pE' = (euid == 0) & X


This patch implements this.  It should be invisible to userspace
(unless userspace (ab)uses cap_bset).  It also adds a secureexec
flag, which we both need.

First, did I get this right?  It seems to work :)

Second, do you have any objection to both of us redoing our
patches against this one?  It should make them nicer-looking
at least.

--Andy

--- 2.6.6-mm4/fs/exec.c~cap_cleanup	2004-05-19 10:37:25.000000000 -0700
+++ 2.6.6-mm4/fs/exec.c	2004-05-21 19:03:16.000000000 -0700
@@ -1093,6 +1093,7 @@
 	bprm.loader = 0;
 	bprm.exec = 0;
 	bprm.security = NULL;
+	bprm.secflags = 0;
 	bprm.mm = mm_alloc();
 	retval = -ENOMEM;
 	if (!bprm.mm)
--- 2.6.6-mm4/security/commoncap.c~cap_cleanup	2004-05-21 18:51:42.000000000 -0700
+++ 2.6.6-mm4/security/commoncap.c	2004-05-23 01:42:51.000000000 -0700
@@ -89,44 +89,20 @@
 
 int cap_bprm_set_security (struct linux_binprm *bprm)
 {
-	/* Copied from fs/exec.c:prepare_binprm. */
-
-	/* We don't have VFS support for capabilities yet */
-	cap_clear (bprm->cap_inheritable);
-	cap_clear (bprm->cap_permitted);
-	cap_clear (bprm->cap_effective);
-
-	/*  To support inheritance of root-permissions and suid-root
-	 *  executables under compatibility mode, we raise all three
-	 *  capability sets for the file.
-	 *
-	 *  If only the real uid is 0, we only raise the inheritable
-	 *  and permitted sets of the executable file.
-	 */
-
-	if (!issecure (SECURE_NOROOT)) {
-		if (bprm->e_uid == 0 || current->uid == 0) {
-			cap_set_full (bprm->cap_inheritable);
-			cap_set_full (bprm->cap_permitted);
-		}
-		if (bprm->e_uid == 0)
-			cap_set_full (bprm->cap_effective);
-	}
 	return 0;
 }
 
 void cap_bprm_apply_creds (struct linux_binprm *bprm, int unsafe)
 {
-	/* Derived from fs/exec.c:compute_creds. */
-	kernel_cap_t new_permitted, working;
+	kernel_cap_t new_pP, new_pE;
 
-	new_permitted = cap_intersect (bprm->cap_permitted, cap_bset);
-	working = cap_intersect (bprm->cap_inheritable,
-				 current->cap_inheritable);
-	new_permitted = cap_combine (new_permitted, working);
+	if (bprm->e_uid == 0 || current->uid == 0)
+		new_pP = cap_bset;
+	else
+		cap_clear(new_pP);
 
 	if (bprm->e_uid != current->uid || bprm->e_gid != current->gid ||
-	    !cap_issubset (new_permitted, current->cap_permitted)) {
+	    !cap_issubset (new_pP, current->cap_permitted)) {
 		current->mm->dumpable = 0;
 
 		if (unsafe & ~LSM_UNSAFE_PTRACE_CAP) {
@@ -134,13 +110,16 @@
 				bprm->e_uid = current->uid;
 				bprm->e_gid = current->gid;
 			}
-			if (!capable (CAP_SETPCAP)) {
-				new_permitted = cap_intersect (new_permitted,
-							current->cap_permitted);
-			}
+			new_pP = cap_intersect (new_pP,
+						current->cap_permitted);
 		}
 	}
 
+	if (bprm->e_uid == 0)
+		new_pE = new_pP;
+	else
+		cap_clear(new_pE);
+
 	current->suid = current->euid = current->fsuid = bprm->e_uid;
 	current->sgid = current->egid = current->fsgid = bprm->e_gid;
 
@@ -148,9 +127,8 @@
 	 * in the init_task struct. Thus we skip the usual
 	 * capability rules */
 	if (current->pid != 1) {
-		current->cap_permitted = new_permitted;
-		current->cap_effective =
-		    cap_intersect (new_permitted, bprm->cap_effective);
+		current->cap_permitted = new_pP;
+		current->cap_effective = new_pE;
 	}
 
 	/* AUD: Audit candidate if current->cap_effective is set */
@@ -160,13 +138,9 @@
 
 int cap_bprm_secureexec (struct linux_binprm *bprm)
 {
-	/* If/when this module is enhanced to incorporate capability
-	   bits on files, the test below should be extended to also perform a 
-	   test between the old and new capability sets.  For now,
-	   it simply preserves the legacy decision algorithm used by
-	   the old userland. */
 	return (current->euid != current->uid ||
-		current->egid != current->gid);
+		current->egid != current->gid ||
+		(bprm->secflags & BINPRM_SEC_SECUREEXEC));
 }
 
 int cap_inode_setxattr(struct dentry *dentry, char *name, void *value,
--- 2.6.6-mm4/include/linux/binfmts.h~cap_cleanup	2004-05-19 10:37:25.000000000 -0700
+++ 2.6.6-mm4/include/linux/binfmts.h	2004-05-23 02:24:31.034876220 -0700
@@ -20,6 +20,7 @@
 /*
  * This structure is used to hold the arguments that are used when loading binaries.
  */
+#define BINPRM_SEC_SECUREEXEC	1
 struct linux_binprm{
 	char buf[BINPRM_BUF_SIZE];
 	struct page *page[MAX_ARG_PAGES];
@@ -28,6 +29,7 @@
 	int sh_bang;
 	struct file * file;
 	int e_uid, e_gid;
+	int secflags;
 	kernel_cap_t cap_inheritable, cap_permitted, cap_effective;
 	void *security;
 	int argc, envc;

Re: [PATCH] scaled-back caps, take 4

[Olaf Dietsche]

Andy Lutomirski <luto@myrealbox.com> writes:

> You don't like my patch because it rips out a bunch of code and it's
> not clear it won't break stuff.
>
> I don't like your patch because it takes a bunch of incomprehensible
> code that does almost nothing and tweaks it slightly to do something
> useful.  (That's not to say it's does the wrong thing; I just don't
> think the code is clear.)
>
> So I decided to figure out what was going on with the original code.
>
> First, CAP_SETPCAP is never obtainable (by anything).
> Since cap_bset never has this bit set, nothing can inherit it
> from fP.  capset_check prevents it from getting set in pI.

# mv /sbin/init /sbin/init.bin
# cat >/sbin/init
#! /bin/sh

if test $$ -eq 1; then
        mount /proc
        echo -1 >/proc/sys/kernel/cap-bound
fi

exec /sbin/init.bin "$@"
^D
# chmod 755 /sbin/init
# reboot

> Second, cap_bset is broken.  For one thing, there's no way
> to remove the caps you want to restrict from already-running
> tasks.  So I don't think it matters if we break/change it.

Maybe I don't understand this, but I think this is what sys_capset()
is for.

> cap_bprm_set_security does:
> fP = fI = (new_uid == 0 || new_euid == 0)
> fE = (new_euid == 0)

Only if (!issecure (SECURE_NOROOT))

[...]
> The whole result is just
> pP' = (uid == 0 || euid==0) & X
> pE' = (euid == 0) & X
>
> This patch implements this.  It should be invisible to userspace
> (unless userspace (ab)uses cap_bset).  It also adds a secureexec
> flag, which we both need.
>
> First, did I get this right?  It seems to work :)

With this patch you effectively revert all capable() calls back to
suser() tests. If this is what you intended, your patch looks fine.

> Second, do you have any objection to both of us redoing our
> patches against this one?  It should make them nicer-looking
> at least.

I didn't scrutinize capabilities as thoroughly as you did, but I still
don't see why your patch is necessary, besides the changes in
fs/exec.c and include/binfmts.h, maybe.

$ cp commoncap.c lutocap.c
modify it to your liking
# insmod lutocap

same goes for chriscap.c

Please, don't get me wrong. For me, it's just a matter of maintaining
a slightly bigger fscaps patch. But I don't think capabilities in
Linux are really broken, only because some proponents of SELinux claim
so.

If you want a simpler - setuid like - capabilities model, throw out
the inheritable _and_ permitted set and use the effective set alone.

Regards, Olaf.

[PATCH] caps, compromise version (was Re: [PATCH] scaled-back caps, take 4)

[Andy Lutomirski]

> 
> Hehe, arm wrestling could be entertaining ;-)  I'm in favor of the most
> conservative change, which I feel is in my patch.  But I'm game to
> continue to pick on each.


I like your legacy mode.  I don't like making processes inherit
non-legacyness.  (With your patch, some daemon might be secure
when started from initscripts but insecure when started from the
command line, if root ended up in non-legacy mode.)

You've also convinced me that an inheritable mask is good, because
it may make some IRIX apps work.  It's also necessary for this patch
to be safe.

I don't like touching the inode in the security module (you
forgot to check nosuid, for one thing).

So here's another shot at it:

"Legacy mode" is controlled by a new bit in task_struct called
keep_all_caps (controlled by PRCTL_SET_KEEPALLCAPS).  This bit turns
off setuid emulation completely (except for setfsuid).

The evolution rules are:
pP' = (fP & X) | (pI & pP) [with the setuid-nonroot fix]
pE' = (pE | fP) & pP'
pI' = full

This time around, I haven't touched the unsafeness rules.

The magic is in the setuid emulation:
	if (current->uid == 0 || current->euid == 0)
		cap_set_full(current->cap_inheritable);
	else
		cap_clear(current->cap_inheritable);

So, unless a program plays with it's inheritable mask,
root will not pick up caps on exec (which is good -- it
means it's safe to chroot somewhere, disable all caps
except CAP_SETUID, and let untrusted code play around.)
But, if you start as root and setuid away, _even with
keepcaps_, you lose the caps on exec.  Which is the broken
behavior we want to preserve.

So, to avoid this, new code can either set keep_all_caps
or just explicitly enable inheritance after setuid, in
which case it just works.

I have pI' = full because otherwise it's just one more
(partially) user-controlled variable that programs need
to worry about.  (And because anything else would break
root.)

As for the rest of the changes:

The code no longer assumes that pI<pP, so I yanked all checks
on the inheritable mask.  On the other hand, it makes no
sense to me for capset when changing lots of processes'
masks to affect the inheritable mask.  So I made it leave
it alone, except when changing current.

keep_all_caps is clearly not entirely necessary.  I can take
it out if anyone objects.

I yanked all capset sanity checks from kernel/capability.c --
they were duplicates anyway.

And I left the old (IMHO pointless) behavior that one needs to hack
init in order to use CAP_SETPCAP.

[Side note: for cap_bset to be useful, I think there needs to be
an operation "atomically remove these caps from all tasks."  I
don't see one.]

This patch also should work fine if VFS capabilities are
introduced (there's an fP mask which defaults to (setuid-
root ? full : 0).

Patch against 2.6.6-mm4 (-mm5 didn't like my filesystem...).
It's not as well tested as it should be.  The old cap.cc
tool still works (but remember to set inheritable).  I
don't have a tool yet to play with keep_all_caps.

This depends on my cleanup patch from Saturday.

--Andy

Signed-off-by: Andy Lutomirski (luto@myrealbox.com)

 fs/exec.c                  |    8 +++++-
 include/linux/binfmts.h    |    2 +
 include/linux/capability.h |    2 -
 include/linux/init_task.h  |    1 
 include/linux/prctl.h      |    4 +++
 include/linux/sched.h      |    1 
 kernel/capability.c        |   13 ----------
 kernel/sys.c               |   11 +++++++++
 security/commoncap.c       |   54 +++++++++++++++++++++++++++++++--------------
 9 files changed, 64 insertions(+), 32 deletions(-)

--- 2.6.6-mm4/fs/exec.c~cap_2	2004-05-23 22:22:42.000000000 -0700
+++ 2.6.6-mm4/fs/exec.c	2004-05-23 22:43:22.000000000 -0700
@@ -886,8 +886,10 @@
 
 	if(!(bprm->file->f_vfsmnt->mnt_flags & MNT_NOSUID)) {
 		/* Set-uid? */
-		if (mode & S_ISUID)
+		if (mode & S_ISUID) {
 			bprm->e_uid = inode->i_uid;
+			bprm->secflags |= BINPRM_SEC_SETUID;
+		}
 
 		/* Set-gid? */
 		/*
@@ -895,8 +897,10 @@
 		 * is a candidate for mandatory locking, not a setgid
 		 * executable.
 		 */
-		if ((mode & (S_ISGID | S_IXGRP)) == (S_ISGID | S_IXGRP))
+		if ((mode & (S_ISGID | S_IXGRP)) == (S_ISGID | S_IXGRP)) {
 			bprm->e_gid = inode->i_gid;
+			bprm->secflags |= BINPRM_SEC_SETGID;
+		}
 	}
 
 	/* fill in binprm security blob */
--- 2.6.6-mm4/security/commoncap.c~cap_2	2004-05-23 22:15:39.000000000 -0700
+++ 2.6.6-mm4/security/commoncap.c	2004-05-24 14:32:43.210834050 -0700
@@ -57,13 +57,6 @@
 		      kernel_cap_t *inheritable, kernel_cap_t *permitted)
 {
 	/* Derived from kernel/capability.c:sys_capset. */
-	/* verify restrictions on target's new Inheritable set */
-	if (!cap_issubset (*inheritable,
-			   cap_combine (target->cap_inheritable,
-					current->cap_permitted))) {
-		return -EPERM;
-	}
-
 	/* verify restrictions on target's new Permitted set */
 	if (!cap_issubset (*permitted,
 			   cap_combine (target->cap_permitted,
@@ -83,12 +76,19 @@
 		     kernel_cap_t *inheritable, kernel_cap_t *permitted)
 {
 	target->cap_effective = *effective;
-	target->cap_inheritable = *inheritable;
 	target->cap_permitted = *permitted;
+	if (current == target)
+		target->cap_inheritable = *inheritable;
 }
 
 int cap_bprm_set_security (struct linux_binprm *bprm)
 {
+	/* Pretend we have VFS capabilities */
+	if ((bprm->secflags & BINPRM_SEC_SETUID) && bprm->e_uid == 0)
+		cap_set_full(bprm->cap_permitted);
+	else
+		cap_clear(bprm->cap_permitted);		
+
 	return 0;
 }
 
@@ -96,10 +96,9 @@
 {
 	kernel_cap_t new_pP, new_pE;
 
-	if (bprm->e_uid == 0 || current->uid == 0)
-		new_pP = cap_bset;
-	else
-		cap_clear(new_pP);
+	new_pP = cap_combine(cap_intersect(bprm->cap_permitted, cap_bset),
+			     cap_intersect(current->cap_permitted,
+					   current->cap_inheritable));
 
 	if (bprm->e_uid != current->uid || bprm->e_gid != current->gid ||
 	    !cap_issubset (new_pP, current->cap_permitted)) {
@@ -115,10 +114,15 @@
 		}
 	}
 
-	if (bprm->e_uid == 0)
-		new_pE = new_pP;
-	else
-		cap_clear(new_pE);
+	/* setuid-nonroot is special. */
+	if (bprm->e_uid != 0 && current->uid != 0 && current->euid == 0)
+		cap_clear(new_pP);
+
+	new_pE = cap_combine(current->cap_effective, bprm->cap_permitted);
+	cap_mask(new_pE, new_pP);
+
+	if (!cap_issubset(new_pP, current->cap_permitted))
+		bprm->secflags |= BINPRM_SEC_SECUREEXEC;
 
 	current->suid = current->euid = current->fsuid = bprm->e_uid;
 	current->sgid = current->egid = current->fsgid = bprm->e_gid;
@@ -129,11 +133,13 @@
 	if (current->pid != 1) {
 		current->cap_permitted = new_pP;
 		current->cap_effective = new_pE;
+		cap_set_full(current->cap_inheritable);
 	}
 
 	/* AUD: Audit candidate if current->cap_effective is set */
 
 	current->keep_capabilities = 0;
+	current->keep_all_caps = 0;
 }
 
 int cap_bprm_secureexec (struct linux_binprm *bprm)
@@ -191,10 +197,18 @@
  * Keeping uid 0 is not an option because uid 0 owns too many vital
  * files..
  * Thanks to Olaf Kirch and Peter Benie for spotting this.
+ *
+ * luto - New behavior, May '04
+ * To emulate old evolution rules, inheritable tracks uid and euid.
+ * prctl(PRCTL_SET_KEEPALLCAPS) disables this.  In fact, keepallcaps
+ * disables this whole function.  BE CAREFUL ABOUT THE EFFECTIVE MASK!!!
  */
 static inline void cap_emulate_setxuid (int old_ruid, int old_euid,
 					int old_suid)
 {
+	if (current->keep_all_caps)
+		return;
+
 	if ((old_ruid == 0 || old_euid == 0 || old_suid == 0) &&
 	    (current->uid != 0 && current->euid != 0 && current->suid != 0) &&
 	    !current->keep_capabilities) {
@@ -207,6 +221,11 @@
 	if (old_euid != 0 && current->euid == 0) {
 		current->cap_effective = current->cap_permitted;
 	}
+
+	if (current->uid == 0 || current->euid == 0)
+		cap_set_full(current->cap_inheritable);
+	else
+		cap_clear(current->cap_inheritable);
 }
 
 int cap_task_post_setuid (uid_t old_ruid, uid_t old_euid, uid_t old_suid,
@@ -230,6 +249,9 @@
 			/*
 			 * FIXME - is fsuser used for all CAP_FS_MASK capabilities?
 			 *          if not, we might be a bit too harsh here.
+			 *
+			 * This explicitly ignores keep_all_caps.  The
+			 * potential for error is just too large.
 			 */
 
 			if (!issecure (SECURE_NO_SETUID_FIXUP)) {
--- 2.6.6-mm4/kernel/sys.c~cap_2	2004-05-23 22:15:24.000000000 -0700
+++ 2.6.6-mm4/kernel/sys.c	2004-05-23 22:57:40.000000000 -0700
@@ -1646,6 +1646,17 @@
 			}
 			current->keep_capabilities = arg2;
 			break;
+		case PR_GET_KEEPALLCAPS:
+			if (current->keep_all_caps)
+				error = 1;
+			break;
+		case PR_SET_KEEPALLCAPS:
+			if (arg2 != 0 && arg2 != 1) {
+				error = -EINVAL;
+				break;
+			}
+			current->keep_all_caps = arg2;
+			break;
 		default:
 			error = -EINVAL;
 			break;
--- 2.6.6-mm4/kernel/capability.c~cap_2	2004-05-23 23:11:49.000000000 -0700
+++ 2.6.6-mm4/kernel/capability.c	2004-05-23 23:13:14.000000000 -0700
@@ -174,19 +174,6 @@
      if (security_capset_check(target, &effective, &inheritable, &permitted))
 	     goto out;
 
-     if (!cap_issubset(inheritable, cap_combine(target->cap_inheritable,
-                       current->cap_permitted)))
-             goto out;
-
-     /* verify restrictions on target's new Permitted set */
-     if (!cap_issubset(permitted, cap_combine(target->cap_permitted,
-                       current->cap_permitted)))
-             goto out;
-
-     /* verify the _new_Effective_ is a subset of the _new_Permitted_ */
-     if (!cap_issubset(effective, permitted))
-             goto out;
-
      ret = 0;
 
      /* having verified that the proposed changes are legal,
--- 2.6.6-mm4/include/linux/capability.h~cap_2	2004-05-23 22:42:22.000000000 -0700
+++ 2.6.6-mm4/include/linux/capability.h	2004-05-23 22:42:34.000000000 -0700
@@ -309,7 +309,7 @@
 #define CAP_EMPTY_SET       to_cap_t(0)
 #define CAP_FULL_SET        to_cap_t(~0)
 #define CAP_INIT_EFF_SET    to_cap_t(~0 & ~CAP_TO_MASK(CAP_SETPCAP))
-#define CAP_INIT_INH_SET    to_cap_t(0)
+#define CAP_INIT_INH_SET    CAP_FULL_SET
 
 #define CAP_TO_MASK(x) (1 << (x))
 #define cap_raise(c, flag)   (cap_t(c) |=  CAP_TO_MASK(flag))
--- 2.6.6-mm4/include/linux/sched.h~cap_2	2004-05-23 22:04:50.000000000 -0700
+++ 2.6.6-mm4/include/linux/sched.h	2004-05-23 22:11:15.000000000 -0700
@@ -462,6 +462,7 @@
 	struct group_info *group_info;
 	kernel_cap_t   cap_effective, cap_inheritable, cap_permitted;
 	int keep_capabilities:1;
+	int keep_all_caps:1;
 	struct user_struct *user;
 /* limits */
 	struct rlimit rlim[RLIM_NLIMITS];
--- 2.6.6-mm4/include/linux/prctl.h~cap_2	2004-05-23 22:12:18.000000000 -0700
+++ 2.6.6-mm4/include/linux/prctl.h	2004-05-23 22:13:35.000000000 -0700
@@ -20,6 +20,10 @@
 #define PR_GET_KEEPCAPS   7
 #define PR_SET_KEEPCAPS   8
 
+/* Get/set whether to emulate old capability behavior */
+#define PR_GET_KEEPALLCAPS 15
+#define PR_SET_KEEPALLCAPS 16
+
 /* Get/set floating-point emulation control bits (if meaningful) */
 #define PR_GET_FPEMU  9
 #define PR_SET_FPEMU 10
--- 2.6.6-mm4/include/linux/binfmts.h~cap_2	2004-05-23 22:27:43.000000000 -0700
+++ 2.6.6-mm4/include/linux/binfmts.h	2004-05-23 22:27:58.000000000 -0700
@@ -21,6 +21,8 @@
  * This structure is used to hold the arguments that are used when loading binaries.
  */
 #define BINPRM_SEC_SECUREEXEC	1
+#define BINPRM_SEC_SETUID	2
+#define BINPRM_SEC_SETGID	4
 struct linux_binprm{
 	char buf[BINPRM_BUF_SIZE];
 	struct page *page[MAX_ARG_PAGES];
--- 2.6.6-mm4/include/linux/init_task.h~cap_2	2004-05-23 22:11:27.000000000 -0700
+++ 2.6.6-mm4/include/linux/init_task.h	2004-05-23 22:11:44.000000000 -0700
@@ -96,6 +96,7 @@
 	.cap_inheritable = CAP_INIT_INH_SET,				\
 	.cap_permitted	= CAP_FULL_SET,					\
 	.keep_capabilities = 0,						\
+	.keep_all_caps  = 0,						\
 	.rlim		= INIT_RLIMITS,					\
 	.user		= INIT_USER,					\
 	.comm		= "swapper",					\

Re: [PATCH] caps, compromise version (was Re: [PATCH] scaled-back caps, take 4)

[Chris Wright]

* Andy Lutomirski (luto@myrealbox.com) wrote:
> > 
> > Hehe, arm wrestling could be entertaining ;-)  I'm in favor of the most
> > conservative change, which I feel is in my patch.  But I'm game to
> > continue to pick on each.
> 
> 
> I like your legacy mode.  I don't like making processes inherit
> non-legacyness.  (With your patch, some daemon might be secure
> when started from initscripts but insecure when started from the
> command line, if root ended up in non-legacy mode.)

Hmm, that was intentional (my very first cut at this thing cleared it,
but that patch had many other broken behaviours).  Specifically because
it goes through pI, which POSIX draft says is untouched through exec.

> You've also convinced me that an inheritable mask is good, because
> it may make some IRIX apps work.  It's also necessary for this patch
> to be safe.
> 
> I don't like touching the inode in the security module (you
> forgot to check nosuid, for one thing).

Yup, I changed that since then, using the secflags approach you did.

> So here's another shot at it:
> 
> "Legacy mode" is controlled by a new bit in task_struct called
> keep_all_caps (controlled by PRCTL_SET_KEEPALLCAPS).  This bit turns
> off setuid emulation completely (except for setfsuid).

I had same idea.  I wished we could hijack keep_capabilities as a
bit vector.

> The evolution rules are:
> pP' = (fP & X) | (pI & pP) [with the setuid-nonroot fix]
> pE' = (pE | fP) & pP'
> pI' = full
> 
> This time around, I haven't touched the unsafeness rules.
> 
> The magic is in the setuid emulation:
> 	if (current->uid == 0 || current->euid == 0)
> 		cap_set_full(current->cap_inheritable);
> 	else
> 		cap_clear(current->cap_inheritable);
> 
> So, unless a program plays with it's inheritable mask,
> root will not pick up caps on exec (which is good -- it
> means it's safe to chroot somewhere, disable all caps
> except CAP_SETUID, and let untrusted code play around.)
> But, if you start as root and setuid away, _even with
> keepcaps_, you lose the caps on exec.  Which is the broken
> behavior we want to preserve.
> 
> So, to avoid this, new code can either set keep_all_caps
> or just explicitly enable inheritance after setuid, in
> which case it just works.
> 
> I have pI' = full because otherwise it's just one more
> (partially) user-controlled variable that programs need
> to worry about.  (And because anything else would break
> root.)

How do you keep passing down the same caps through multiple execs?

> As for the rest of the changes:
> 
> The code no longer assumes that pI<pP, so I yanked all checks
> on the inheritable mask.  On the other hand, it makes no
> sense to me for capset when changing lots of processes'
> masks to affect the inheritable mask.  So I made it leave
> it alone, except when changing current.
> 
> keep_all_caps is clearly not entirely necessary.  I can take
> it out if anyone objects.
> 
> I yanked all capset sanity checks from kernel/capability.c --
> they were duplicates anyway.
> 
> And I left the old (IMHO pointless) behavior that one needs to hack
> init in order to use CAP_SETPCAP.
> 
> [Side note: for cap_bset to be useful, I think there needs to be
> an operation "atomically remove these caps from all tasks."  I
> don't see one.]

Yeah.  It depends on the definition of useful.  Get a couple privileged
tasks running (which may fork/exec from time to time), then clamp down
the machine is one form of useful.  In general, I don't cap_bset is that
useful though.

> This patch also should work fine if VFS capabilities are
> introduced (there's an fP mask which defaults to (setuid-
> root ? full : 0).
> 
> Patch against 2.6.6-mm4 (-mm5 didn't like my filesystem...).
> It's not as well tested as it should be.  The old cap.cc
> tool still works (but remember to set inheritable).  I
> don't have a tool yet to play with keep_all_caps.

I can add this to the test stuff to play with it.

thanks,
-chris
-- 
Linux Security Modules     http://lsm.immunix.org     http://lsm.bkbits.net

Re: [PATCH] caps, compromise version (was Re: [PATCH] scaled-back caps, take 4)

[Andy Lutomirski]

Chris Wright wrote:

> * Andy Lutomirski (luto@myrealbox.com) wrote:
> 
>>>Hehe, arm wrestling could be entertaining ;-)  I'm in favor of the most
>>>conservative change, which I feel is in my patch.  But I'm game to
>>>continue to pick on each.
>>
>>
>>I like your legacy mode.  I don't like making processes inherit
>>non-legacyness.  (With your patch, some daemon might be secure
>>when started from initscripts but insecure when started from the
>>command line, if root ended up in non-legacy mode.)
> 
> 
> Hmm, that was intentional (my very first cut at this thing cleared it,
> but that patch had many other broken behaviours).  Specifically because
> it goes through pI, which POSIX draft says is untouched through exec.

Not in IRIX, though.  And I'm afraid of:

cap -c all-i <some setuid binary>
versus
cap -c all+i <some setuid binary>

Suddently the binary's behavior might be different.  This isn't 
inheritantly bad, but it seems like a pointless gotcha.

I like my version of using inheritable for legaciness, but only because my 
inheritable semantics make sense.  Your version would worry me a lot less 
if you just added a new field.  But mine doesn't actually need the new field ;)

>>
>>"Legacy mode" is controlled by a new bit in task_struct called
>>keep_all_caps (controlled by PRCTL_SET_KEEPALLCAPS).  This bit turns
>>off setuid emulation completely (except for setfsuid).
> 
> 
> I had same idea.  I wished we could hijack keep_capabilities as a
> bit vector.

It's a bitfield.  Just add fields -- no cost in memory.  Fairly large cost 
in compile time, though...

> 
> 
>>The evolution rules are:
>>pP' = (fP & X) | (pI & pP) [with the setuid-nonroot fix]
>>pE' = (pE | fP) & pP'
>>pI' = full
>>
>>This time around, I haven't touched the unsafeness rules.
>>
>>The magic is in the setuid emulation:
>>	if (current->uid == 0 || current->euid == 0)
>>		cap_set_full(current->cap_inheritable);
>>	else
>>		cap_clear(current->cap_inheritable);
>>
>>So, unless a program plays with it's inheritable mask,
>>root will not pick up caps on exec (which is good -- it
>>means it's safe to chroot somewhere, disable all caps
>>except CAP_SETUID, and let untrusted code play around.)
>>But, if you start as root and setuid away, _even with
>>keepcaps_, you lose the caps on exec.  Which is the broken
>>behavior we want to preserve.
>>
>>So, to avoid this, new code can either set keep_all_caps
>>or just explicitly enable inheritance after setuid, in
>>which case it just works.
>>
>>I have pI' = full because otherwise it's just one more
>>(partially) user-controlled variable that programs need
>>to worry about.  (And because anything else would break
>>root.)
> 
> 
> How do you keep passing down the same caps through multiple execs?

This only takes effect when set*uid is called successfully.  It bites 
programs that start as non-root with CAP_SETUID and change their uid, but 
these programs either don't exist or don't work at all right now.

[root@luto andy]# cap -c all+i -u andy bash
[andy@luto andy]$ dumpcap [note second exec]
         Real        Eff
User    500         500
Group   500         500

Caps: =ip cap_setpcap-p

> 
> 
>>As for the rest of the changes:
>>
>>The code no longer assumes that pI<pP, so I yanked all checks
>>on the inheritable mask.  On the other hand, it makes no
>>sense to me for capset when changing lots of processes'
>>masks to affect the inheritable mask.  So I made it leave
>>it alone, except when changing current.
>>
>>keep_all_caps is clearly not entirely necessary.  I can take
>>it out if anyone objects.
>>
>>I yanked all capset sanity checks from kernel/capability.c --
>>they were duplicates anyway.
>>
>>And I left the old (IMHO pointless) behavior that one needs to hack
>>init in order to use CAP_SETPCAP.
>>
>>[Side note: for cap_bset to be useful, I think there needs to be
>>an operation "atomically remove these caps from all tasks."  I
>>don't see one.]
> 
> 
> Yeah.  It depends on the definition of useful.  Get a couple privileged
> tasks running (which may fork/exec from time to time), then clamp down
> the machine is one form of useful.  In general, I don't cap_bset is that
> useful though.

Especially with CAP_SYS_ADMIN...  SELinux is clearly the way to go here.

I just discovered a patch 
(http://www.kernel.org/pub/linux/libs/security/linux-privs/kernel-2.4-fcap/README) 
that claims to implement per-process-tree maximum cap masks (like I did for 
  awhile).  It hasn't been maintained, though.

If one of our patches hits -mm or -linus, I may try and add a feature like 
that.  It'll (rightly) annoy the SELinux folks, though.

> 
> 
>>This patch also should work fine if VFS capabilities are
>>introduced (there's an fP mask which defaults to (setuid-
>>root ? full : 0).
>>
>>Patch against 2.6.6-mm4 (-mm5 didn't like my filesystem...).
>>It's not as well tested as it should be.  The old cap.cc
>>tool still works (but remember to set inheritable).  I
>>don't have a tool yet to play with keep_all_caps.
> 
> 
> I can add this to the test stuff to play with it.

Except that I fail a lot of your tests because of inheritable mask 
differences.  Oh, well.

I may revive my ext3 caps patch sometime.  Is there a way to make that work 
with your patch?

--Andy

Re: [PATCH] support cap inheritable (Re: [PATCH] scaled-back caps, take 4 (was Re: [PATCH] capabilites, take 2)

[Andy Lutomirski]

Chris Wright wrote:

> * Chris Wright (chrisw@osdl.org) wrote:
> 
>>* Andy Lutomirski (luto@myrealbox.com) wrote:
>>
>>>This version throws out the inheritable mask.  There is no change in
>>>behavior with newcaps=0.
>>
>>Alright, I tried to write up my expectations for all the various modes
>>w.r.t dropping privs, keeping them, setuid apps, etc.  I then wrote some
>>tests to get a baseline, and well as a way to compare results.  Finally
>>I wrote a patch that meets the requirements I laid out, and compared it
>>against yours.  With one minor exception, the capabilities bits match
>>up.  You drop effective caps when a non-root users execs a non-root
>>setuid app, and I the caps alone.  One side note, you're changes effect
>>the user/group saved ids unexpectedly.

Disclaimer: I haven't had a chance to boot this version.

> 
> 
> The tests are available at:
>   http://developer.osdl.org/~chrisw/capabilities/testcap.tar.bz2
> 
> patch is there too and is also inline below:
>   http://developer.osdl.org/~chrisw/capabilities/2.6.6-mm2/chris_cap.patch
> 
> --- linux-2.6.6-mm2-cap/include/linux/capability.h.orig	2004-05-09 19:32:26.000000000 -0700
> +++ linux-2.6.6-mm2-cap/include/linux/capability.h	2004-05-17 23:24:08.143860096 -0700
> @@ -309,7 +309,9 @@
>  #define CAP_EMPTY_SET       to_cap_t(0)
>  #define CAP_FULL_SET        to_cap_t(~0)
>  #define CAP_INIT_EFF_SET    to_cap_t(~0 & ~CAP_TO_MASK(CAP_SETPCAP))
> -#define CAP_INIT_INH_SET    to_cap_t(0)
> +#define CAP_INIT_INH_SET    to_cap_t(~0)
> +/* ~0 is legacy inheritable mode and can never be capset by user */
> +#define cap_orig_inh(_cap) (cap_t((_cap)) == ~0)

So how do you say "inherit all caps"?

[...]

> @@ -56,10 +59,13 @@
>  int cap_capset_check (struct task_struct *target, kernel_cap_t *effective,
>  		      kernel_cap_t *inheritable, kernel_cap_t *permitted)
>  {
> +	kernel_cap_t target_inheritable = target->cap_inheritable;
> +	if (cap_orig_inh(target_inheritable))
> +		target_inheritable = 0;
>  	/* Derived from kernel/capability.c:sys_capset. */
>  	/* verify restrictions on target's new Inheritable set */
>  	if (!cap_issubset (*inheritable,
> -			   cap_combine (target->cap_inheritable,
> +			   cap_combine (target_inheritable,
>  					current->cap_permitted))) {
>  		return -EPERM;
>  	}

What stops legacy mode from being reenabled?

[...]

I think you missed the case when root-but-no-caps execs setuid root -- I 
don't see anything that would enable secureexec.

--Andy

Re: [PATCH] support cap inheritable (Re: [PATCH] scaled-back caps, take 4 (was Re: [PATCH] capabilites, take 2)

[Chris Wright]

* Andy Lutomirski (luto@myrealbox.com) wrote:
> Chris Wright wrote:
> 
> > -#define CAP_INIT_INH_SET    to_cap_t(0)
> > +#define CAP_INIT_INH_SET    to_cap_t(~0)
> > +/* ~0 is legacy inheritable mode and can never be capset by user */
> > +#define cap_orig_inh(_cap) (cap_t((_cap)) == ~0)
> 
> So how do you say "inherit all caps"?

Legacy mode requires effectively reserving a bit, which works in this case
since all 32 aren't used.  So inherit all caps is all valid caps (which,
btw,  still exludes CAP_SETPCAP).  Something like "=eip cap_setpcap-eip"
would inherit all valid caps.

> > @@ -56,10 +59,13 @@
> >  int cap_capset_check (struct task_struct *target, kernel_cap_t *effective,
> >  		      kernel_cap_t *inheritable, kernel_cap_t *permitted)
> >  {
> > +	kernel_cap_t target_inheritable = target->cap_inheritable;
> > +	if (cap_orig_inh(target_inheritable))
> > +		target_inheritable = 0;
> >  	/* Derived from kernel/capability.c:sys_capset. */
> >  	/* verify restrictions on target's new Inheritable set */
> >  	if (!cap_issubset (*inheritable,
> > -			   cap_combine (target->cap_inheritable,
> > +			   cap_combine (target_inheritable,
> >  					current->cap_permitted))) {
> >  		return -EPERM;
> >  	}
> 
> What stops legacy mode from being reenabled?

I believe only a kernel thread could do this (and init) since they are the
only ones that could get CAP_SETPCAP.  Same threat as it is currently.

> I think you missed the case when root-but-no-caps execs setuid root -- I 
> don't see anything that would enable secureexec.

Yup you're right.  I liked how you did that in your patch and was just going
to steal that bit ;-)

thanks,
-chris
-- 
Linux Security Modules     http://lsm.immunix.org     http://lsm.bkbits.net