kernel-hardening.lists.openwall.com archive mirror
 help / color / mirror / Atom feed
* [kernel-hardening] RLIMIT_NPROC DoS fix
@ 2012-06-12  9:38 Vasily Kulikov
  2012-06-13  0:22 ` [kernel-hardening] Re: [owl-dev] " Pavel Kankovsky
  0 siblings, 1 reply; 3+ messages in thread
From: Vasily Kulikov @ 2012-06-12  9:38 UTC (permalink / raw)
  To: owl-dev, kernel-hardening

Hi,

There is a problem with RLIMIT_NPROC patch:
http://www.openwall.com/lists/kernel-hardening/2011/06/12/9
http://www.openwall.com/lists/kernel-hardening/2011/08/08/2

As planned, it moves RLIMIT check from set_user() (set*uid()) stage to
fork()/execve() stage.  But the drawback is that now there is no RLIMIT
check at set_user().  It means that there are two bad cases:

1) a root process sometimes does setrlimit()+setuid(U)+execve()/fork().
In this case U can do kill(pid, SIGSTOP) each time just after setuid(U).
U may store unlimited number of processes this way.
(Spender has learned this way.)

2) there are nonroot users A and B.
A makes a binary ./loop which does setuid(A)+for(;;) {}
A does chmod u+s ./loop
B does 'while :; do ./loop &; done'


IOW, if there is a way one can produce unlimited number of processes at
atfer-setuid before-execve stage, one may DoS the system.

I don't see any fair way to fix that.  The way I see is using additional
limit which is used to kill overmultiplied processes.  E.g.:

set_user()
{
    ...
	if (atomic_read(&new_user->processes) >= rlimit(RLIMIT_NPROC) &&
			new_user != INIT_USER) {
		if (atomic_read(&new_user->processes) >= 2*rlimit(RLIMIT_NPROC) + 10) {
		    ...some ratelimited printk warning...
		    force_sig(SIGKILL, current);
		}
		current->flags |= PF_NPROC_EXCEEDED;
	} else {
		current->flags &= ~PF_NPROC_EXCEEDED;
    }
        
    ...
}

Yes, this is dirty.  Any thoughts about less dirty way?

-- 
Vasily

^ permalink raw reply	[flat|nested] 3+ messages in thread

* [kernel-hardening] Re: [owl-dev] RLIMIT_NPROC DoS fix
  2012-06-12  9:38 [kernel-hardening] RLIMIT_NPROC DoS fix Vasily Kulikov
@ 2012-06-13  0:22 ` Pavel Kankovsky
  2012-06-21  0:55   ` [kernel-hardening] " Solar Designer
  0 siblings, 1 reply; 3+ messages in thread
From: Pavel Kankovsky @ 2012-06-13  0:22 UTC (permalink / raw)
  To: owl-dev; +Cc: kernel-hardening

On Tue, 12 Jun 2012, Vasily Kulikov wrote:

> There is a problem with RLIMIT_NPROC patch:
> http://www.openwall.com/lists/kernel-hardening/2011/06/12/9

I'd like to point out there are at least two other ways setuid() et alii 
can fail in recent kernels: LSM and memory allocation.

> 1) a root process sometimes does setrlimit()+setuid(U)+execve()/fork().
> In this case U can do kill(pid, SIGSTOP) each time just after setuid(U).
> U may store unlimited number of processes this way.
> (Spender has learned this way.)

You could probably refuse to handle SIGSTOP sent by a nonprivileged user 
when PF_NPROC_EXCEEDED is set. I do not thing it would break anything.
But it would make things even dirtier.

> 2) there are nonroot users A and B.
> A makes a binary ./loop which does setuid(A)+for(;;) {}
> A does chmod u+s ./loop
> B does 'while :; do ./loop &; done'

I think processes should count towards RLIMIT_NPROC of the user who forked 
them (B in this example) for the rest of their CPU cycles. Even after they 
exec a setuid program.

> Yes, this is dirty.  Any thoughts about less dirty way?

Create a cgroup controller limiting the number of tasks (and perhaps the 
number of other kernel objects) and pretend RLIMIT_NPROC has never 
existed?

-- 
Pavel Kankovsky aka Peak                          / Jeremiah 9:21        \
"For death is come up into our MS Windows(tm)..." \ 21st century edition /

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [kernel-hardening] RLIMIT_NPROC DoS fix
  2012-06-13  0:22 ` [kernel-hardening] Re: [owl-dev] " Pavel Kankovsky
@ 2012-06-21  0:55   ` Solar Designer
  0 siblings, 0 replies; 3+ messages in thread
From: Solar Designer @ 2012-06-21  0:55 UTC (permalink / raw)
  To: kernel-hardening; +Cc: Pavel Kankovsky

Hi,

Let's keep this on kernel-hardening only (dropping owl-dev, adding Pavel).

FWIW, in pre-2.6 kernels there was no RLIMIT_NPROC enforcement on
setuid() at all, so adding this check on execve() (like I did in -ow
patches for 2.0-2.4 kernels) made things strictly more restricted.
Now when we applied the same approach to mainline kernels, after
RLIMIT_NPROC enforcement had been added to setuid(), we made things more
relaxed in the way described by Vasily as compared to what they have
been in 2.6 kernels for a long while.  Yet I continue to think that the
RLIMIT_NPROC enforcement on setuid() that existed in 2.6 kernels was
wrong, and overall we're in a better state now.

On Wed, Jun 13, 2012 at 02:22:41AM +0200, Pavel Kankovsky wrote:
> On Tue, 12 Jun 2012, Vasily Kulikov wrote:
> 
> >There is a problem with RLIMIT_NPROC patch:
> >http://www.openwall.com/lists/kernel-hardening/2011/06/12/9
> 
> I'd like to point out there are at least two other ways setuid() et alii 
> can fail in recent kernels: LSM and memory allocation.

LSM can break lots of things. ;-)

As to memory allocation, we previously determined (in last year's
discussion on kernel-hardening) that it currently can't fail - yet I
proposed (and I continue to propose) that we make the code fail-close in
this respect (have the process killed if this currently impossible
condition does occur, such as in a future code revision or if we made an
error in our analysis).  (We already have it that way - with process
killing - in the patch applied on top of RHEL5/OpenVZ kernels in Owl.)

> >1) a root process sometimes does setrlimit()+setuid(U)+execve()/fork().
> >In this case U can do kill(pid, SIGSTOP) each time just after setuid(U).
> >U may store unlimited number of processes this way.
> >(Spender has learned this way.)
> 
> You could probably refuse to handle SIGSTOP sent by a nonprivileged user 
> when PF_NPROC_EXCEEDED is set. I do not thing it would break anything.
> But it would make things even dirtier.

Yes, and we'd need to disallow setpriority() as well.

> >2) there are nonroot users A and B.
> >A makes a binary ./loop which does setuid(A)+for(;;) {}
> >A does chmod u+s ./loop
> >B does 'while :; do ./loop &; done'
> 
> I think processes should count towards RLIMIT_NPROC of the user who forked 
> them (B in this example) for the rest of their CPU cycles. Even after they 
> exec a setuid program.

I think this is already the case, but the crucial detail about Vasily's
example above is that the process itself fully switches to the target
user with a setuid() call - and indeed we perform set_user() on setuid().
Should we start to treat setuid() from non-root real UID specially for
the purpose of RLIMIT_NPROC?  I think not.

Maybe we could postpone set_user() until execve() (which would never
happen in the example above, thereby leaving the process B's for the
purpose of RLIMIT_NPROC).  This would deal with the SIGSTOP attack too.

This needs further consideration.

Alexander

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2012-06-21  0:55 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-06-12  9:38 [kernel-hardening] RLIMIT_NPROC DoS fix Vasily Kulikov
2012-06-13  0:22 ` [kernel-hardening] Re: [owl-dev] " Pavel Kankovsky
2012-06-21  0:55   ` [kernel-hardening] " Solar Designer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).