Messages in this thread | | | Date | Tue, 18 May 2004 02:11:51 -0700 | From | Andy Lutomirski <> | Subject | Re: [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/
| |