Re: [PATCH 2.5.30] Allow tasks to share credentials

Re: [PATCH 2.5.30] Allow tasks to share credentials

[Linus Torvalds]

On Fri, 2 Aug 2002, Dave McCracken wrote:
> 
> This patch allows tasks to optionally share credentials, selectable
> via a flag to clone().

This still has the "security hole you can run a slow-moving bight yellow
truck with flashing lights on through" problem..

		Linus

Re: [PATCH 2.5.30] Allow tasks to share credentials

[Dave McCracken]

--On Friday, August 02, 2002 01:10:48 PM -0700 Linus Torvalds
<torvalds@transmeta.com> wrote:

> This still has the "security hole you can run a slow-moving bight yellow
> truck with flashing lights on through" problem..

My apologies. I've been trying to get a copy of the patch that lkml would
accept, and had not seen your reply when I sent that one out.  I'll rework
the patch and resubmit.

Dave McCracken

======================================================================
Dave McCracken          IBM Linux Base Kernel Team      1-512-838-3059
dmccr@us.ibm.com                                        T/L   678-3059

Re: [PATCH 2.5.30] Allow tasks to share credentials

[Linus Torvalds]

On Fri, 2 Aug 2002, Dave McCracken wrote:
>
> This patch provides the ability to share credentials (uid, gid,
> capabilities) between tasks, using a clone() flag.

I worry about the lack of locking here.

Maybe it's the right thing to do, I don't really know.

But I _know_, for example, that this is just a horrid security hole the
way it is now - the execve() path doesn't create a unique "cred"
structure, so if you execve() a suid binary from a CLONE_CRED thread, the
other threads get the suid'ness and can do whatever they want.

At the very least, it should disallow suid exec's when

	atomic_read(&current->cred->count) > 1

which is the same approach we do wrt other shared state (ie disallow a
CLONE_FILES thing from doing a suid execve etc).

The alternative is to just allocate a new cred structure on execve.

As-is this patch is way way too dangerous. You can trivially create a root
hole by doing

	if (!clone(CLONE_CRED)) {
		execve("su");
		exit(1);
	}

	..this thread now also got root..

> There is no lock around the credential accesses, but from my analysis none
> is needed.

You may be right. I don't see any huge reason for it, but see above on
other fundamental problems.

		Linus

Re: [PATCH 2.5.30] Allow tasks to share credentials

[Dave McCracken]

--On Friday, August 02, 2002 10:22:54 AM -0700 Linus Torvalds
<torvalds@transmeta.com> wrote:

> But I _know_, for example, that this is just a horrid security hole the
> way it is now - the execve() path doesn't create a unique "cred"
> structure, so if you execve() a suid binary from a CLONE_CRED thread, the
> other threads get the suid'ness and can do whatever they want.

You are entirely correct.  It was an oversight on my part.  execve() should
always unshare the cred structure.  I'll work up a fixed version.

Dave McCracken

======================================================================
Dave McCracken          IBM Linux Base Kernel Team      1-512-838-3059
dmccr@us.ibm.com                                        T/L   678-3059