public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Serge Hallyn <serge.hallyn@ubuntu.com>
To: Andrew Vagin <avagin@parallels.com>
Cc: Eric Paris <eparis@redhat.com>,
	linux-kernel@vger.kernel.org,
	linux-security-module@vger.kernel.org,
	Andrew Vagin <avagin@openvz.org>,
	"Andrew G. Morgan" <morgan@kernel.org>,
	"Serge E. Hallyn" <serge.hallyn@canonical.com>,
	Kees Cook <keescook@chromium.org>,
	Steve Grubb <sgrubb@redhat.com>, Dan Walsh <dwalsh@redhat.com>,
	stable@kernel.org
Subject: Re: [PATCH] CAPABILITIES: remove undefined caps from all processes
Date: Tue, 22 Jul 2014 16:25:56 +0000	[thread overview]
Message-ID: <20140722162556.GU21815@ubuntumail> (raw)
In-Reply-To: <20140722153937.GA26122@paralelels.com>

Quoting Andrew Vagin (avagin@parallels.com):
> On Mon, Jul 21, 2014 at 04:59:01PM -0400, Eric Paris wrote:
> > This is effectively a revert of 7b9a7ec565505699f503b4fcf61500dceb36e744
> > plus fixing it a different way...
> > 
> > We found, when trying to run an application from an application which
> > had dropped privs that the kernel does security checks on undefined
> > capability bits.  This was ESPECIALLY difficult to debug as those
> > undefined bits are hidden from /proc/$PID/status.
> > 
> > Consider a root application which drops all capabilities from ALL 4
> > capability sets.  We assume, since the application is going to set
> > eff/perm/inh from an array that it will clear not only the defined caps
> > less than CAP_LAST_CAP, but also the higher 28ish bits which are
> > undefined future capabilities.
> > 
> > The BSET gets cleared differently.  Instead it is cleared one bit at a
> > time.  The problem here is that in security/commoncap.c::cap_task_prctl()
> > we actually check the validity of a capability being read.  So any task
> > which attempts to 'read all things set in bset' followed by 'unset all
> > things set in bset' will not even attempt to unset the undefined bits
> > higher than CAP_LAST_CAP.
> > 
> > So the 'parent' will look something like:
> > CapInh:	0000000000000000
> > CapPrm:	0000000000000000
> > CapEff:	0000000000000000
> > CapBnd:	ffffffc000000000
> > 
> > All of this 'should' be fine.  Given that these are undefined bits that
> > aren't supposed to have anything to do with permissions.  But they do...
> > 
> > So lets now consider a task which cleared the eff/perm/inh completely
> > and cleared all of the valid caps in the bset (but not the invalid caps
> > it couldn't read out of the kernel).  We know that this is exactly what
> > the libcap-ng library does and what the go capabilities library does.
> > They both leave you in that above situation if you try to clear all of
> > you capapabilities from all 4 sets.  If that root task calls execve()
> > the child task will pick up all caps not blocked by the bset.  The bset
> > however does not block bits higher than CAP_LAST_CAP.  So now the child
> > task has bits in eff which are not in the parent.  These are
> > 'meaningless' undefined bits, but still bits which the parent doesn't
> > have.
> > 
> > The problem is now in cred_cap_issubset() (or any operation which does a
> > subset test) as the child, while a subset for valid cap bits, is not a
> > subset for invalid cap bits!  So now we set durring commit creds that
> > the child is not dumpable.  Given it is 'more priv' than its parent.  It
> > also means the parent cannot ptrace the child and other stupidity.
> > 
> > The solution here is 2 things.
> > 1) stop hiding capability bits in status
> > 	we hide those upper bits which meant I couldn't spot this issue
> > 2) stop giving any task undefined capability bits.  it's simple, it you
> > don't put those invalid bits in CAP_FULL_SET you won't get them in init
> > and you won't get them in any other task either.
> 
> Pls, look at the comment for my first patch: https://lkml.org/lkml/2012/10/5/374
> 
> The following command fails with this patch (and succeeds without).
> 
> [root@avagin-fc19-cr ~]# capsh --caps="all=eip" -- -c /bin/bash
> Unable to set capabilities [--caps=all=eip]

Thanks - so at least capset will need to mask what is passed in
by the user with CAP_FULL_SET.

  reply	other threads:[~2014-07-22 16:26 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-07-21 20:59 [PATCH] CAPABILITIES: remove undefined caps from all processes Eric Paris
2014-07-21 22:27 ` Andy Lutomirski
2014-07-22  2:24 ` Serge E. Hallyn
2014-07-22 15:39 ` Andrew Vagin
2014-07-22 16:25   ` Serge Hallyn [this message]
  -- strict thread matches above, loose matches on Subject: below --
2014-07-23 19:36 Eric Paris
2014-07-23 20:46 ` Andy Lutomirski
2014-07-23 20:49   ` Eric Paris
2014-07-23 21:00     ` Kees Cook
2014-07-23 21:32       ` Serge E. Hallyn
2014-07-24 12:06 ` James Morris

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=20140722162556.GU21815@ubuntumail \
    --to=serge.hallyn@ubuntu.com \
    --cc=avagin@openvz.org \
    --cc=avagin@parallels.com \
    --cc=dwalsh@redhat.com \
    --cc=eparis@redhat.com \
    --cc=keescook@chromium.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=morgan@kernel.org \
    --cc=serge.hallyn@canonical.com \
    --cc=sgrubb@redhat.com \
    --cc=stable@kernel.org \
    /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