public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Andrew Morgan <morgan@kernel.org>
To: "Serge E. Hallyn" <serue@us.ibm.com>
Cc: Chris Friedhoff <chris@friedhoff.org>,
	Serge E Hallyn <sergeh@us.ibm.com>,
	linux-security-module@vger.kernel.org,
	Stephen Smalley <sds@epoch.ncsc.mil>,
	Chris Wright <chrisw@sous-sol.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: Posix file capabilities in 2.6.24rc2; now 2.6.24-rc3
Date: Wed, 21 Nov 2007 23:42:31 -0800	[thread overview]
Message-ID: <474532E7.6010406@kernel.org> (raw)
In-Reply-To: <20071120145106.GA6641@sergelap.austin.ibm.com>

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Serge E. Hallyn wrote:
> The problem is that when you run a setuid binary, its pP and pE are
> fully raised.  The following patch fixes it for me.  Chris, does it fix
> your problem?  Andrew, am I again confusing myself and doing something
> unsafe?

I think this is yet another example of the fragile mess that is UID
emulation with capabilities. Your patch is an example of privilege
escalation - luser can kill a more-capable process. In the kill CONT
case we reached the opposite conclusion to this one. As was the case
then, I didn't disagree then :*). If it meets folk's expectations, then
this is probably a good patch...

> --- a/security/commoncap.c
> +++ b/security/commoncap.c
> @@ -543,6 +543,9 @@ int cap_task_kill(struct task_struct *p, struct siginfo *info,
>  	if (capable(CAP_KILL))
>  		return 0;
>  
> +	if (p->euid==0 && p->uid==current->uid)
> +		return 0;
> +

Its late and I'm obviously tired, but is there any reason not to simply use:

 if (p->uid == current->uid)
	 return 0;

Whatever the case, could you put the new code closer to the sig ==
SIGCONT test? The capability tests are at the end of cap_task_kill() and
this new check breaks that pattern.

Cheers

Andrew

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.6 (GNU/Linux)

iD8DBQFHRTLlQheEq9QabfIRAt/hAKCJgj2kbuyAWI486LOwwDLdkbcpoQCfQdrQ
J+bcvi+9pGTodFn42PsHJHA=
=cXaG
-----END PGP SIGNATURE-----

      parent reply	other threads:[~2007-11-22  7:42 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-11-13 22:07 Posix file capabilities in 2.6.24rc2 Chris Friedhoff
2007-11-13 23:53 ` Serge E. Hallyn
2007-11-14  9:12   ` Chris Friedhoff
2007-11-14 18:02     ` Serge E. Hallyn
2007-11-15 22:02       ` Chris Friedhoff
2007-11-19 13:39         ` Posix file capabilities in 2.6.24rc2; now 2.6.24-rc3 Chris Friedhoff
2007-11-19 23:16           ` Serge E. Hallyn
2007-11-20  9:46             ` Chris Friedhoff
2007-11-20 14:51               ` Serge E. Hallyn
2007-11-20 22:29                 ` Chris Friedhoff
2007-11-20 22:51                   ` Serge E. Hallyn
2007-11-21  0:50                     ` Chris Friedhoff
2007-11-22  7:42                 ` Andrew Morgan [this message]

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=474532E7.6010406@kernel.org \
    --to=morgan@kernel.org \
    --cc=chris@friedhoff.org \
    --cc=chrisw@sous-sol.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=sds@epoch.ncsc.mil \
    --cc=sergeh@us.ibm.com \
    --cc=serue@us.ibm.com \
    /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