lkml.org 
[lkml]   [2004]   [May]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] scaled-back caps, take 4 (was Re: [PATCH] capabilites, take 2)


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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2005-03-22 14:03    [W:0.055 / U:0.364 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site