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.
next prev parent 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