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

[Chris Wright <chrisw@osdl.org>]

* 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