[kernel-hardening] rlimit_nproc check

[kernel-hardening] rlimit_nproc check

[Vasiliy Kulikov]

[-- Attachment #1: Type: text/plain, Size: 444 bytes --]

Solar, all -

I found 8-years old patch that enables RLIMIT_NPROC check at setuid (and
similar) calls:

http://lkml.org/lkml/2003/7/13/226

So, checking it on execve() is a bit redundant.  But it means that
setuid() may fail if it follows setrlimit() call and the target user
has already reached the limit (asserted on the test C program).  If the
limit is defined in pam_limit, the attack becomes real.


Thanks,

-- 
Vasiliy

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [kernel-hardening] rlimit_nproc check

[Solar Designer]

Vasiliy -

On Thu, Jun 09, 2011 at 06:17:45PM +0400, Vasiliy Kulikov wrote:
> I found 8-years old patch that enables RLIMIT_NPROC check at setuid (and
> similar) calls:
> 
> http://lkml.org/lkml/2003/7/13/226
> 
> So, checking it on execve() is a bit redundant.  But it means that
> setuid() may fail if it follows setrlimit() call and the target user
> has already reached the limit (asserted on the test C program).  If the
> limit is defined in pam_limit, the attack becomes real.

Right.  Dealing with setuid() failing to drop privs yet returning, which
many apps don't expect, is definitely something we (you) need to do
under this project.  In Linux 2.4.x-ow, I simply do:

--- linux-2.4.37.9.orig/kernel/sys.c	2010-02-01 21:04:46 +0000
+++ linux-2.4.37.9/kernel/sys.c	2010-02-18 14:04:42 +0000
@@ -514,8 +514,10 @@ static int set_user(uid_t new_ruid, int 
 	struct user_struct *new_user;
 
 	new_user = alloc_uid(new_ruid);
-	if (!new_user)
+	if (!new_user) {
+		force_sig(SIGSEGV, current);
 		return -EAGAIN;
+	}
 	switch_uid(new_user);
 
 	if(dumpclear)

As an option, you could propose to revert that 8-year old change and
introduce the check on execve().  Unrealistic?

The requirements are:

1. setuid(2) must not fail and return, when it was invoked with
"appropriate privileges".  If it fails, it must not return.

This suggests that it should not fail very often, so maybe the
RLIMIT_NPROC check does not belong there.

2. The setuid-execve sequence should not successfully start the new
program when that would exceed RLIMIT_NPROC for the target UID.

Oh, by the way, here's what I found:

Subject: [PATCH] sched: Don't allow setuid to succeed if the user does not have rt bandwidth
http://lists.openwall.net/linux-kernel/2009/02/27/177

Thanks,

Alexander

Re: [kernel-hardening] rlimit_nproc check

[Vasiliy Kulikov]

Solar,

On Sun, Jun 12, 2011 at 06:28 +0400, Solar Designer wrote:
> As an option, you could propose to revert that 8-year old change and
> introduce the check on execve().  Unrealistic?

I've started a separate thread on LKML for it, we'll see ;)

> Oh, by the way, here's what I found:
> 
> Subject: [PATCH] sched: Don't allow setuid to succeed if the user does not have rt bandwidth
> http://lists.openwall.net/linux-kernel/2009/02/27/177

Yeah, but it was removed in 7c9414385ebfdd87cc542d4e7e3bb0dbb2d3ce25 as
a "2.6.34 scheduled cleanup".


Thanks,

-- 
Vasiliy

Re: [kernel-hardening] rlimit_nproc check

[Vasiliy Kulikov]

On Sun, Jun 12, 2011 at 06:28 +0400, Solar Designer wrote:
> Right.  Dealing with setuid() failing to drop privs yet returning, which
> many apps don't expect, is definitely something we (you) need to do
> under this project.  In Linux 2.4.x-ow, I simply do:
> 
> --- linux-2.4.37.9.orig/kernel/sys.c	2010-02-01 21:04:46 +0000
> +++ linux-2.4.37.9/kernel/sys.c	2010-02-18 14:04:42 +0000
> @@ -514,8 +514,10 @@ static int set_user(uid_t new_ruid, int 
>  	struct user_struct *new_user;
>  
>  	new_user = alloc_uid(new_ruid);
> -	if (!new_user)
> +	if (!new_user) {
> +		force_sig(SIGSEGV, current);
>  		return -EAGAIN;
> +	}
>  	switch_uid(new_user);
>  
>  	if(dumpclear)
> 
> As an option, you could propose to revert that 8-year old change and
> introduce the check on execve().  Unrealistic?

I have slightly another idea.  Introduce mechanism (e.g. sysctl
variable) to make all capabilities dropping function cause SIGSEGV if
actual dropping process fails for any of several reasons.

The initial list should look like this:

    set*{u,g}id
    {set,init}groups
    unshare
    prctl(PR_CAPBSET_DROP, ...)
    prctl(PR_SET_SECCOMP, ...)
    capset

But any such list looks a bit not complete because there are many
different functions that might drop some capabilities in some
situations, like close(), *chdir(), rlimit(), nice(), fcntl(), ioctl(),
chroot(), which are, well, might fail in very common situations without
any actual secuity risk.

So, IMO this solution might not be enough consistent for upstream
inclusion.


Thanks,

-- 
Vasiliy

Re: [kernel-hardening] rlimit_nproc check

[Vasiliy Kulikov]

On Sun, Jun 19, 2011 at 17:34 +0400, Vasiliy Kulikov wrote:
> I have slightly another idea.  Introduce mechanism (e.g. sysctl
> variable) to make all capabilities dropping function cause SIGSEGV if
> actual dropping process fails for any of several reasons.
> 
> The initial list should look like this:
> 
>     set*{u,g}id
>     {set,init}groups
>     unshare
>     prctl(PR_CAPBSET_DROP, ...)
>     prctl(PR_SET_SECCOMP, ...)
>     capset
> [...]

The scheme actually is harmfull in some situations.  E.g. nfs daemon
with one process architecture switches to another user via setfsuid()
or similar, handles the request, switches back to the root.  If
setfsuid() fails (rlimit or some other reason) nfsd is DoS'ed by the
signal.  Any other daemon with similar architecture will be DoS'ed too.

So, the list of SIGKILL'able syscalls should be selected _very_
carefully (I don't know whether the safe list is nonempty).


Thanks,

-- 
Vasiliy Kulikov
http://www.openwall.com - bringing security into open computing environments