[Reviews requested] kern/121073: chroot for non-root users

[Jille Timmermans]

Confirming Kostik,
using rfork(RFPROC) instead of rfork(RFPROC|RFFDG) and chrooting in the
child also chroot's the parent, without giving him the flag.

An option might be to store the P_NOSUGID flag somewhere in the desc table ?

Attached patch will show the difference between w/ and w/o RFFDG.

jille at elvis:~$ cc -o chroot-rfork chroot-rfork.c
jille at elvis:~$ sudo ./chroot-rfork
/COPYRIGHT does not exist (chrooted)
/COPYRIGHT does not exist (chrooted)
jille at elvis:~$ cc -o chroot-rfork chroot-rfork.c -DWITH_RFFDG_FLAG
jille at elvis:~$ sudo ./chroot-rfork
/COPYRIGHT does not exist (chrooted)
/COPYRIGHT exists (not chrooted)

-- Jille

Kostik Belousov wrote:
> On Sat, Aug 16, 2008 at 01:18:24PM +0200, Ed Schouten wrote:
>   
>> Hello everyone,
>>
>> When I visited FOSDEM back in February, I was talking with Jille
>> Timmermans about the chroot() call. After discussing that the problem
>> with chroot() is that it cannot be safely be executed by non-root users
>> w.r.t. setuid binaries*, we wrote this patchset for the kernel to add
>> something similar to `MNT_NOSUID' to the process flags. The result
>> being:
>>
>> 	http://bugs.FreeBSD.org/121073
>>
>> The patch even adds a small security improvement to the system. Say,
>> you'd change the typical chroot() + setuid() order the other way around,
>> you're guaranteed the chrooted process will never change users
>> afterwards, because it won't honour set[ug]id binaries anymore.
>>
>> Our security officer was wise enough to add the following to the PR:
>>
>>     +----------------------------------------------------------+
>>     |UNDER NO CONDITIONS SHOULD THIS PATCH BE COMMITTED WITHOUT|
>>     |EXPLICIT APPROVAL FROM THE FREEBSD SECURITY OFFICER.      |
>>     +----------------------------------------------------------+
>>
>> After having a discussion with Colin on IRC, there are a couple of
>> questions we would like to be answered (or discussed) before getting
>> this in the tree:
>>
>> - Are there any comments on the patch itself?
>>
>> - Colin was concerned if turned on, would it be possible for the user to
>>   do things which it normally couldn't and shouldn't?
>>
>> It would be great to get many reviews on this before we'd land it in the
>> source tree. I've attached the patch to this email as well. Thanks!
>>
>> -- 
>>  Ed Schouten <ed at 80386.nl>
>>  WWW: http://80386.nl/
>>
>> * Hardlink a setuid binary to a directory containing a fake C library
>>   and executing it.
>>     
>
> I think that the patch gives instant root. FreeBSD provides a rfork(2)
> system call. This call allows the processes to share filedesc table, that,
> among other information, contains the root of the filesystem namespace
> for the process.
>
> So, the scenario is to rfork() a process without RFFDG flag, and then
> for one of the resulting processes to perform a chroot. Now, second one
> has chrooted root, but no P_NOSUGID flag set.
>   

-------------- next part --------------
#include <err.h>
#include <unistd.h>
#include <sys/stat.h>
#include <sys/types.h>

#ifdef WITH_RFFDG_FLAG
#define RFORK_FLAGS RFPROC|RFFDG
#else
#define RFORK_FLAGS RFPROC
#endif

int
main(int argc, char **argv) {
	struct stat sb;
	switch(rfork(RFORK_FLAGS)) {
		case -1:
			err(1, "rfork()");
		case 0:
			if(chroot("/tmp")!=0)
				err(1, "chroot()");
			if(stat("/COPYRIGHT", &sb)==0)
				printf("/COPYRIGHT exists (not chrooted)\n");
			else
				printf("/COPYRIGHT does not exist (chrooted)\n");
			break;
		default:
			sleep(1);
			if(stat("/COPYRIGHT", &sb)==0)
				printf("/COPYRIGHT exists (not chrooted)\n");
			else
				printf("/COPYRIGHT does not exist (chrooted)\n");
	}
}