linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] [RFC] Deter exploit bruteforcing
@ 2014-12-24 21:39 Richard Weinberger
  2014-12-30 18:40 ` Kees Cook
  0 siblings, 1 reply; 24+ messages in thread
From: Richard Weinberger @ 2014-12-24 21:39 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-fsdevel, rientjes, atomlin, keescook, daeseok.youn, tglx,
	vdavydov, riel, oleg, akpm, peterz, mingo, viro, luto,
	Richard Weinberger

While exploring the offset2lib attack I remembered that
grsecurity has an interesting feature to make such attacks
much harder. Exploits can brute stack canaries often very easily
if the target is a forking server like sshd or Apache httpd.
The problem is that after fork() the child has by definition
exactly the same memory as the parent and therefore also the same
stack canaries.
The attacker can guess the stack canaries byte by byte.
After 256 times 7 forks() a good exploit can find the correct
canary value.

The basic idea behind this patch is to delay fork() if a child died
due to a fatal error.
Currently it delays fork() by 30 seconds if the parent tries to fork()
within 60 seconds after a child died due to a fatal error.

I'm sure you'll hate this patch but I want to find out how much you hate it
and whether there is a little chance to get it mainline in a modified form.
Later I'd make it depend on a new Kconfig option and off by default
and the timing constants changeable via sysctl.

Credit for the concept goes to grsecurity folks, I'll take the flames.

Signed-off-by: Richard Weinberger <richard@nod.at>
---
 fs/coredump.c         | 10 ++++++++++
 include/linux/sched.h |  1 +
 kernel/fork.c         |  7 +++++++
 3 files changed, 18 insertions(+)

diff --git a/fs/coredump.c b/fs/coredump.c
index b5c86ff..d7730c8 100644
--- a/fs/coredump.c
+++ b/fs/coredump.c
@@ -512,6 +512,7 @@ void do_coredump(const siginfo_t *siginfo)
 	bool need_nonrelative = false;
 	bool core_dumped = false;
 	static atomic_t core_dump_count = ATOMIC_INIT(0);
+	int sig = siginfo->si_signo;
 	struct coredump_params cprm = {
 		.siginfo = siginfo,
 		.regs = signal_pt_regs(),
@@ -526,6 +527,15 @@ void do_coredump(const siginfo_t *siginfo)
 
 	audit_core_dumps(siginfo->si_signo);
 
+	if (sig == SIGSEGV || sig == SIGBUS || sig == SIGKILL || sig == SIGILL) {
+		rcu_read_lock();
+		read_lock(&tasklist_lock);
+		if (current->real_parent && (current->flags & PF_FORKNOEXEC))
+			current->real_parent->brute_expires = get_seconds() + (30 * 60);
+		read_unlock(&tasklist_lock);
+		rcu_read_unlock();
+	}
+
 	binfmt = mm->binfmt;
 	if (!binfmt || !binfmt->core_dump)
 		goto fail;
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 8db31ef..c616735 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1701,6 +1701,7 @@ struct task_struct {
 #ifdef CONFIG_DEBUG_ATOMIC_SLEEP
 	unsigned long	task_state_change;
 #endif
+	unsigned long brute_expires;
 };
 
 /* Future-safe accessor for struct task_struct's cpus_allowed. */
diff --git a/kernel/fork.c b/kernel/fork.c
index 4dc2dda..178c80e 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -74,6 +74,7 @@
 #include <linux/uprobes.h>
 #include <linux/aio.h>
 #include <linux/compiler.h>
+#include <linux/delay.h>
 
 #include <asm/pgtable.h>
 #include <asm/pgalloc.h>
@@ -352,6 +353,8 @@ static struct task_struct *dup_task_struct(struct task_struct *orig)
 	tsk->splice_pipe = NULL;
 	tsk->task_frag.page = NULL;
 
+	tsk->brute_expires = 0;
+
 	account_kernel_stack(ti, 1);
 
 	return tsk;
@@ -1669,6 +1672,10 @@ long do_fork(unsigned long clone_flags,
 		if (clone_flags & CLONE_PARENT_SETTID)
 			put_user(nr, parent_tidptr);
 
+		if (unlikely(current->brute_expires) && time_before(get_seconds(),
+		    current->brute_expires))
+			msleep(30 * 1000);
+
 		if (clone_flags & CLONE_VFORK) {
 			p->vfork_done = &vfork;
 			init_completion(&vfork);
-- 
2.1.0

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

* Re: [PATCH] [RFC] Deter exploit bruteforcing
  2014-12-24 21:39 [PATCH] [RFC] Deter exploit bruteforcing Richard Weinberger
@ 2014-12-30 18:40 ` Kees Cook
  2014-12-30 18:49   ` Andy Lutomirski
                     ` (2 more replies)
  0 siblings, 3 replies; 24+ messages in thread
From: Kees Cook @ 2014-12-30 18:40 UTC (permalink / raw)
  To: Richard Weinberger
  Cc: LKML, linux-fsdevel, David Rientjes, Aaron Tomlin, DaeSeok Youn,
	Thomas Gleixner, vdavydov, Rik van Riel, Oleg Nesterov,
	Andrew Morton, Peter Zijlstra, Ingo Molnar, Al Viro,
	Andy Lutomirski, Brad Spengler

On Wed, Dec 24, 2014 at 1:39 PM, Richard Weinberger <richard@nod.at> wrote:
> While exploring the offset2lib attack I remembered that
> grsecurity has an interesting feature to make such attacks
> much harder. Exploits can brute stack canaries often very easily
> if the target is a forking server like sshd or Apache httpd.
> The problem is that after fork() the child has by definition
> exactly the same memory as the parent and therefore also the same
> stack canaries.
> The attacker can guess the stack canaries byte by byte.
> After 256 times 7 forks() a good exploit can find the correct
> canary value.
>
> The basic idea behind this patch is to delay fork() if a child died
> due to a fatal error.
> Currently it delays fork() by 30 seconds if the parent tries to fork()
> within 60 seconds after a child died due to a fatal error.
>
> I'm sure you'll hate this patch but I want to find out how much you hate it
> and whether there is a little chance to get it mainline in a modified form.
> Later I'd make it depend on a new Kconfig option and off by default
> and the timing constants changeable via sysctl.
>
> Credit for the concept goes to grsecurity folks, I'll take the flames.
>
> Signed-off-by: Richard Weinberger <richard@nod.at>
> ---
>  fs/coredump.c         | 10 ++++++++++
>  include/linux/sched.h |  1 +
>  kernel/fork.c         |  7 +++++++
>  3 files changed, 18 insertions(+)
>
> diff --git a/fs/coredump.c b/fs/coredump.c
> index b5c86ff..d7730c8 100644
> --- a/fs/coredump.c
> +++ b/fs/coredump.c
> @@ -512,6 +512,7 @@ void do_coredump(const siginfo_t *siginfo)
>         bool need_nonrelative = false;
>         bool core_dumped = false;
>         static atomic_t core_dump_count = ATOMIC_INIT(0);
> +       int sig = siginfo->si_signo;
>         struct coredump_params cprm = {
>                 .siginfo = siginfo,
>                 .regs = signal_pt_regs(),
> @@ -526,6 +527,15 @@ void do_coredump(const siginfo_t *siginfo)
>
>         audit_core_dumps(siginfo->si_signo);
>
> +       if (sig == SIGSEGV || sig == SIGBUS || sig == SIGKILL || sig == SIGILL) {

I think we should add SIGSYS to this list.

> +               rcu_read_lock();
> +               read_lock(&tasklist_lock);
> +               if (current->real_parent && (current->flags & PF_FORKNOEXEC))
> +                       current->real_parent->brute_expires = get_seconds() + (30 * 60);
> +               read_unlock(&tasklist_lock);
> +               rcu_read_unlock();
> +       }
> +
>         binfmt = mm->binfmt;
>         if (!binfmt || !binfmt->core_dump)
>                 goto fail;
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index 8db31ef..c616735 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -1701,6 +1701,7 @@ struct task_struct {
>  #ifdef CONFIG_DEBUG_ATOMIC_SLEEP
>         unsigned long   task_state_change;
>  #endif
> +       unsigned long brute_expires;
>  };
>
>  /* Future-safe accessor for struct task_struct's cpus_allowed. */
> diff --git a/kernel/fork.c b/kernel/fork.c
> index 4dc2dda..178c80e 100644
> --- a/kernel/fork.c
> +++ b/kernel/fork.c
> @@ -74,6 +74,7 @@
>  #include <linux/uprobes.h>
>  #include <linux/aio.h>
>  #include <linux/compiler.h>
> +#include <linux/delay.h>
>
>  #include <asm/pgtable.h>
>  #include <asm/pgalloc.h>
> @@ -352,6 +353,8 @@ static struct task_struct *dup_task_struct(struct task_struct *orig)
>         tsk->splice_pipe = NULL;
>         tsk->task_frag.page = NULL;
>
> +       tsk->brute_expires = 0;
> +
>         account_kernel_stack(ti, 1);
>
>         return tsk;
> @@ -1669,6 +1672,10 @@ long do_fork(unsigned long clone_flags,
>                 if (clone_flags & CLONE_PARENT_SETTID)
>                         put_user(nr, parent_tidptr);
>
> +               if (unlikely(current->brute_expires) && time_before(get_seconds(),
> +                   current->brute_expires))
> +                       msleep(30 * 1000);
> +
>                 if (clone_flags & CLONE_VFORK) {
>                         p->vfork_done = &vfork;
>                         init_completion(&vfork);
> --
> 2.1.0
>

Instead of open-coding the checks here, maybe it'd make sense to
extract the "attach" and "check" logic into explicit functions that
can be CONFIG stubbed out? This is how grsec handles it via their
gr_handle_brute_* functions.

Regardless, I'm for adding this feature to mainline. :)

-Kees

-- 
Kees Cook
Chrome OS Security

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

* Re: [PATCH] [RFC] Deter exploit bruteforcing
  2014-12-30 18:40 ` Kees Cook
@ 2014-12-30 18:49   ` Andy Lutomirski
  2014-12-30 18:50   ` Richard Weinberger
  2015-01-02  5:11   ` Pavel Machek
  2 siblings, 0 replies; 24+ messages in thread
From: Andy Lutomirski @ 2014-12-30 18:49 UTC (permalink / raw)
  To: Kees Cook
  Cc: Richard Weinberger, LKML, linux-fsdevel, David Rientjes,
	Aaron Tomlin, DaeSeok Youn, Thomas Gleixner, vdavydov,
	Rik van Riel, Oleg Nesterov, Andrew Morton, Peter Zijlstra,
	Ingo Molnar, Al Viro, Brad Spengler

On Tue, Dec 30, 2014 at 10:40 AM, Kees Cook <keescook@chromium.org> wrote:
> On Wed, Dec 24, 2014 at 1:39 PM, Richard Weinberger <richard@nod.at> wrote:
>> While exploring the offset2lib attack I remembered that
>> grsecurity has an interesting feature to make such attacks
>> much harder. Exploits can brute stack canaries often very easily
>> if the target is a forking server like sshd or Apache httpd.
>> The problem is that after fork() the child has by definition
>> exactly the same memory as the parent and therefore also the same
>> stack canaries.
>> The attacker can guess the stack canaries byte by byte.
>> After 256 times 7 forks() a good exploit can find the correct
>> canary value.
>>
>> The basic idea behind this patch is to delay fork() if a child died
>> due to a fatal error.
>> Currently it delays fork() by 30 seconds if the parent tries to fork()
>> within 60 seconds after a child died due to a fatal error.
>>
>> I'm sure you'll hate this patch but I want to find out how much you hate it
>> and whether there is a little chance to get it mainline in a modified form.
>> Later I'd make it depend on a new Kconfig option and off by default
>> and the timing constants changeable via sysctl.
>>
>> Credit for the concept goes to grsecurity folks, I'll take the flames.
>>
>> Signed-off-by: Richard Weinberger <richard@nod.at>
>> ---
>>  fs/coredump.c         | 10 ++++++++++
>>  include/linux/sched.h |  1 +
>>  kernel/fork.c         |  7 +++++++
>>  3 files changed, 18 insertions(+)
>>
>> diff --git a/fs/coredump.c b/fs/coredump.c
>> index b5c86ff..d7730c8 100644
>> --- a/fs/coredump.c
>> +++ b/fs/coredump.c
>> @@ -512,6 +512,7 @@ void do_coredump(const siginfo_t *siginfo)
>>         bool need_nonrelative = false;
>>         bool core_dumped = false;
>>         static atomic_t core_dump_count = ATOMIC_INIT(0);
>> +       int sig = siginfo->si_signo;
>>         struct coredump_params cprm = {
>>                 .siginfo = siginfo,
>>                 .regs = signal_pt_regs(),
>> @@ -526,6 +527,15 @@ void do_coredump(const siginfo_t *siginfo)
>>
>>         audit_core_dumps(siginfo->si_signo);
>>
>> +       if (sig == SIGSEGV || sig == SIGBUS || sig == SIGKILL || sig == SIGILL) {
>
> I think we should add SIGSYS to this list.
>
>> +               rcu_read_lock();
>> +               read_lock(&tasklist_lock);
>> +               if (current->real_parent && (current->flags & PF_FORKNOEXEC))
>> +                       current->real_parent->brute_expires = get_seconds() + (30 * 60);
>> +               read_unlock(&tasklist_lock);
>> +               rcu_read_unlock();
>> +       }
>> +
>>         binfmt = mm->binfmt;
>>         if (!binfmt || !binfmt->core_dump)
>>                 goto fail;
>> diff --git a/include/linux/sched.h b/include/linux/sched.h
>> index 8db31ef..c616735 100644
>> --- a/include/linux/sched.h
>> +++ b/include/linux/sched.h
>> @@ -1701,6 +1701,7 @@ struct task_struct {
>>  #ifdef CONFIG_DEBUG_ATOMIC_SLEEP
>>         unsigned long   task_state_change;
>>  #endif
>> +       unsigned long brute_expires;
>>  };
>>
>>  /* Future-safe accessor for struct task_struct's cpus_allowed. */
>> diff --git a/kernel/fork.c b/kernel/fork.c
>> index 4dc2dda..178c80e 100644
>> --- a/kernel/fork.c
>> +++ b/kernel/fork.c
>> @@ -74,6 +74,7 @@
>>  #include <linux/uprobes.h>
>>  #include <linux/aio.h>
>>  #include <linux/compiler.h>
>> +#include <linux/delay.h>
>>
>>  #include <asm/pgtable.h>
>>  #include <asm/pgalloc.h>
>> @@ -352,6 +353,8 @@ static struct task_struct *dup_task_struct(struct task_struct *orig)
>>         tsk->splice_pipe = NULL;
>>         tsk->task_frag.page = NULL;
>>
>> +       tsk->brute_expires = 0;
>> +
>>         account_kernel_stack(ti, 1);
>>
>>         return tsk;
>> @@ -1669,6 +1672,10 @@ long do_fork(unsigned long clone_flags,
>>                 if (clone_flags & CLONE_PARENT_SETTID)
>>                         put_user(nr, parent_tidptr);
>>
>> +               if (unlikely(current->brute_expires) && time_before(get_seconds(),
>> +                   current->brute_expires))
>> +                       msleep(30 * 1000);
>> +
>>                 if (clone_flags & CLONE_VFORK) {
>>                         p->vfork_done = &vfork;
>>                         init_completion(&vfork);
>> --
>> 2.1.0
>>
>
> Instead of open-coding the checks here, maybe it'd make sense to
> extract the "attach" and "check" logic into explicit functions that
> can be CONFIG stubbed out? This is how grsec handles it via their
> gr_handle_brute_* functions.
>
> Regardless, I'm for adding this feature to mainline. :)
>

If this is going to go into mainline, I think it needs better
configurability (the timeouts, etc, should be settable by sysctl and
possibly prctl), and I expect we'll need a prctl to turn it off.  I
can imagine programs (e.g. anything Zygote-like) that will want to
turn it off because it'll do more harm than good.

But I like the concept, too.

--Andy

> -Kees
>
> --
> Kees Cook
> Chrome OS Security



-- 
Andy Lutomirski
AMA Capital Management, LLC

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

* Re: [PATCH] [RFC] Deter exploit bruteforcing
  2014-12-30 18:40 ` Kees Cook
  2014-12-30 18:49   ` Andy Lutomirski
@ 2014-12-30 18:50   ` Richard Weinberger
  2015-01-02  5:11   ` Pavel Machek
  2 siblings, 0 replies; 24+ messages in thread
From: Richard Weinberger @ 2014-12-30 18:50 UTC (permalink / raw)
  To: Kees Cook
  Cc: LKML, linux-fsdevel, David Rientjes, Aaron Tomlin, DaeSeok Youn,
	Thomas Gleixner, vdavydov, Rik van Riel, Oleg Nesterov,
	Andrew Morton, Peter Zijlstra, Ingo Molnar, Al Viro,
	Andy Lutomirski, Brad Spengler

Am 30.12.2014 um 19:40 schrieb Kees Cook:
> On Wed, Dec 24, 2014 at 1:39 PM, Richard Weinberger <richard@nod.at> wrote:
>> While exploring the offset2lib attack I remembered that
>> grsecurity has an interesting feature to make such attacks
>> much harder. Exploits can brute stack canaries often very easily
>> if the target is a forking server like sshd or Apache httpd.
>> The problem is that after fork() the child has by definition
>> exactly the same memory as the parent and therefore also the same
>> stack canaries.
>> The attacker can guess the stack canaries byte by byte.
>> After 256 times 7 forks() a good exploit can find the correct
>> canary value.
>>
>> The basic idea behind this patch is to delay fork() if a child died
>> due to a fatal error.
>> Currently it delays fork() by 30 seconds if the parent tries to fork()
>> within 60 seconds after a child died due to a fatal error.
>>
>> I'm sure you'll hate this patch but I want to find out how much you hate it
>> and whether there is a little chance to get it mainline in a modified form.
>> Later I'd make it depend on a new Kconfig option and off by default
>> and the timing constants changeable via sysctl.
>>
>> Credit for the concept goes to grsecurity folks, I'll take the flames.
>>
>> Signed-off-by: Richard Weinberger <richard@nod.at>
>> ---
>>  fs/coredump.c         | 10 ++++++++++
>>  include/linux/sched.h |  1 +
>>  kernel/fork.c         |  7 +++++++
>>  3 files changed, 18 insertions(+)
>>
>> diff --git a/fs/coredump.c b/fs/coredump.c
>> index b5c86ff..d7730c8 100644
>> --- a/fs/coredump.c
>> +++ b/fs/coredump.c
>> @@ -512,6 +512,7 @@ void do_coredump(const siginfo_t *siginfo)
>>         bool need_nonrelative = false;
>>         bool core_dumped = false;
>>         static atomic_t core_dump_count = ATOMIC_INIT(0);
>> +       int sig = siginfo->si_signo;
>>         struct coredump_params cprm = {
>>                 .siginfo = siginfo,
>>                 .regs = signal_pt_regs(),
>> @@ -526,6 +527,15 @@ void do_coredump(const siginfo_t *siginfo)
>>
>>         audit_core_dumps(siginfo->si_signo);
>>
>> +       if (sig == SIGSEGV || sig == SIGBUS || sig == SIGKILL || sig == SIGILL) {
> 
> I think we should add SIGSYS to this list.
> 
>> +               rcu_read_lock();
>> +               read_lock(&tasklist_lock);
>> +               if (current->real_parent && (current->flags & PF_FORKNOEXEC))
>> +                       current->real_parent->brute_expires = get_seconds() + (30 * 60);
>> +               read_unlock(&tasklist_lock);
>> +               rcu_read_unlock();
>> +       }
>> +
>>         binfmt = mm->binfmt;
>>         if (!binfmt || !binfmt->core_dump)
>>                 goto fail;
>> diff --git a/include/linux/sched.h b/include/linux/sched.h
>> index 8db31ef..c616735 100644
>> --- a/include/linux/sched.h
>> +++ b/include/linux/sched.h
>> @@ -1701,6 +1701,7 @@ struct task_struct {
>>  #ifdef CONFIG_DEBUG_ATOMIC_SLEEP
>>         unsigned long   task_state_change;
>>  #endif
>> +       unsigned long brute_expires;
>>  };
>>
>>  /* Future-safe accessor for struct task_struct's cpus_allowed. */
>> diff --git a/kernel/fork.c b/kernel/fork.c
>> index 4dc2dda..178c80e 100644
>> --- a/kernel/fork.c
>> +++ b/kernel/fork.c
>> @@ -74,6 +74,7 @@
>>  #include <linux/uprobes.h>
>>  #include <linux/aio.h>
>>  #include <linux/compiler.h>
>> +#include <linux/delay.h>
>>
>>  #include <asm/pgtable.h>
>>  #include <asm/pgalloc.h>
>> @@ -352,6 +353,8 @@ static struct task_struct *dup_task_struct(struct task_struct *orig)
>>         tsk->splice_pipe = NULL;
>>         tsk->task_frag.page = NULL;
>>
>> +       tsk->brute_expires = 0;
>> +
>>         account_kernel_stack(ti, 1);
>>
>>         return tsk;
>> @@ -1669,6 +1672,10 @@ long do_fork(unsigned long clone_flags,
>>                 if (clone_flags & CLONE_PARENT_SETTID)
>>                         put_user(nr, parent_tidptr);
>>
>> +               if (unlikely(current->brute_expires) && time_before(get_seconds(),
>> +                   current->brute_expires))
>> +                       msleep(30 * 1000);
>> +
>>                 if (clone_flags & CLONE_VFORK) {
>>                         p->vfork_done = &vfork;
>>                         init_completion(&vfork);
>> --
>> 2.1.0
>>
> 
> Instead of open-coding the checks here, maybe it'd make sense to
> extract the "attach" and "check" logic into explicit functions that
> can be CONFIG stubbed out? This is how grsec handles it via their
> gr_handle_brute_* functions.

Of course. I've stripped down the patch as much as possible to outline the
concept/idea. It is not intended to get merged as is.
Beside of having a new Kconfig options it also needs some logging.
Maybe "WARNING: forking very fast, broken daemon or exploit attempt?!"

> Regardless, I'm for adding this feature to mainline. :)

Thanks,
//richard

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

* Re: [PATCH] [RFC] Deter exploit bruteforcing
  2014-12-30 18:40 ` Kees Cook
  2014-12-30 18:49   ` Andy Lutomirski
  2014-12-30 18:50   ` Richard Weinberger
@ 2015-01-02  5:11   ` Pavel Machek
  2015-01-02 11:00     ` Richard Weinberger
  2 siblings, 1 reply; 24+ messages in thread
From: Pavel Machek @ 2015-01-02  5:11 UTC (permalink / raw)
  To: Kees Cook
  Cc: Richard Weinberger, LKML, linux-fsdevel, David Rientjes,
	Aaron Tomlin, DaeSeok Youn, Thomas Gleixner, vdavydov,
	Rik van Riel, Oleg Nesterov, Andrew Morton, Peter Zijlstra,
	Ingo Molnar, Al Viro, Andy Lutomirski, Brad Spengler

On Tue 2014-12-30 10:40:15, Kees Cook wrote:
> On Wed, Dec 24, 2014 at 1:39 PM, Richard Weinberger <richard@nod.at> wrote:
> > While exploring the offset2lib attack I remembered that
> > grsecurity has an interesting feature to make such attacks
> > much harder. Exploits can brute stack canaries often very easily
> > if the target is a forking server like sshd or Apache httpd.
> > The problem is that after fork() the child has by definition
> > exactly the same memory as the parent and therefore also the same
> > stack canaries.
> > The attacker can guess the stack canaries byte by byte.
> > After 256 times 7 forks() a good exploit can find the correct
> > canary value.
> >
> > The basic idea behind this patch is to delay fork() if a child died
> > due to a fatal error.
> > Currently it delays fork() by 30 seconds if the parent tries to fork()
> > within 60 seconds after a child died due to a fatal error.
> >
> > I'm sure you'll hate this patch but I want to find out how much you hate it
> > and whether there is a little chance to get it mainline in a modified form.
> > Later I'd make it depend on a new Kconfig option and off by default
> > and the timing constants changeable via sysctl.

Does this break trinity, crashme, and similar programs?

Can you detect it died due to the stack canary? Then, the patch might
be actually acceptable.

								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

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

* Re: [PATCH] [RFC] Deter exploit bruteforcing
  2015-01-02  5:11   ` Pavel Machek
@ 2015-01-02 11:00     ` Richard Weinberger
  2015-01-02 19:46       ` Pavel Machek
  0 siblings, 1 reply; 24+ messages in thread
From: Richard Weinberger @ 2015-01-02 11:00 UTC (permalink / raw)
  To: Pavel Machek, Kees Cook
  Cc: LKML, linux-fsdevel, David Rientjes, Aaron Tomlin, DaeSeok Youn,
	Thomas Gleixner, vdavydov, Rik van Riel, Oleg Nesterov,
	Andrew Morton, Peter Zijlstra, Ingo Molnar, Al Viro,
	Andy Lutomirski, Brad Spengler

Am 02.01.2015 um 06:11 schrieb Pavel Machek:
> On Tue 2014-12-30 10:40:15, Kees Cook wrote:
>> On Wed, Dec 24, 2014 at 1:39 PM, Richard Weinberger <richard@nod.at> wrote:
>>> While exploring the offset2lib attack I remembered that
>>> grsecurity has an interesting feature to make such attacks
>>> much harder. Exploits can brute stack canaries often very easily
>>> if the target is a forking server like sshd or Apache httpd.
>>> The problem is that after fork() the child has by definition
>>> exactly the same memory as the parent and therefore also the same
>>> stack canaries.
>>> The attacker can guess the stack canaries byte by byte.
>>> After 256 times 7 forks() a good exploit can find the correct
>>> canary value.
>>>
>>> The basic idea behind this patch is to delay fork() if a child died
>>> due to a fatal error.
>>> Currently it delays fork() by 30 seconds if the parent tries to fork()
>>> within 60 seconds after a child died due to a fatal error.
>>>
>>> I'm sure you'll hate this patch but I want to find out how much you hate it
>>> and whether there is a little chance to get it mainline in a modified form.
>>> Later I'd make it depend on a new Kconfig option and off by default
>>> and the timing constants changeable via sysctl.
> 
> Does this break trinity, crashme, and similar programs?

If they fork() without execve() and a child dies very fast the next fork()
will be throttled.
This is why I'd like to make this feature disabled by default.

> Can you detect it died due to the stack canary? Then, the patch might
> be actually acceptable.

I don't think so as this is glibc specific.

Thanks,
//richard

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

* Re: [PATCH] [RFC] Deter exploit bruteforcing
  2015-01-02 11:00     ` Richard Weinberger
@ 2015-01-02 19:46       ` Pavel Machek
  2015-01-02 21:40         ` Richard Weinberger
  0 siblings, 1 reply; 24+ messages in thread
From: Pavel Machek @ 2015-01-02 19:46 UTC (permalink / raw)
  To: Richard Weinberger
  Cc: Kees Cook, LKML, linux-fsdevel, David Rientjes, Aaron Tomlin,
	DaeSeok Youn, Thomas Gleixner, vdavydov, Rik van Riel,
	Oleg Nesterov, Andrew Morton, Peter Zijlstra, Ingo Molnar,
	Al Viro, Andy Lutomirski, Brad Spengler

On Fri 2015-01-02 12:00:08, Richard Weinberger wrote:
> Am 02.01.2015 um 06:11 schrieb Pavel Machek:
> > On Tue 2014-12-30 10:40:15, Kees Cook wrote:
> >> On Wed, Dec 24, 2014 at 1:39 PM, Richard Weinberger <richard@nod.at> wrote:
> >>> While exploring the offset2lib attack I remembered that
> >>> grsecurity has an interesting feature to make such attacks
> >>> much harder. Exploits can brute stack canaries often very easily
> >>> if the target is a forking server like sshd or Apache httpd.
> >>> The problem is that after fork() the child has by definition
> >>> exactly the same memory as the parent and therefore also the same
> >>> stack canaries.
> >>> The attacker can guess the stack canaries byte by byte.
> >>> After 256 times 7 forks() a good exploit can find the correct
> >>> canary value.
> >>>
> >>> The basic idea behind this patch is to delay fork() if a child died
> >>> due to a fatal error.
> >>> Currently it delays fork() by 30 seconds if the parent tries to fork()
> >>> within 60 seconds after a child died due to a fatal error.
> >>>
> >>> I'm sure you'll hate this patch but I want to find out how much you hate it
> >>> and whether there is a little chance to get it mainline in a modified form.
> >>> Later I'd make it depend on a new Kconfig option and off by default
> >>> and the timing constants changeable via sysctl.
> > 
> > Does this break trinity, crashme, and similar programs?
> 
> If they fork() without execve() and a child dies very fast the next fork()
> will be throttled.
> This is why I'd like to make this feature disabled by default.
> 
> > Can you detect it died due to the stack canary? Then, the patch might
> > be actually acceptable.
> 
> I don't think so as this is glibc specific.

Can the slowdown be impelmented in glibc, then?

If not, can glibc provide enough information to the kernel to allow us
to do the right thing?
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

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

* Re: [PATCH] [RFC] Deter exploit bruteforcing
  2015-01-02 19:46       ` Pavel Machek
@ 2015-01-02 21:40         ` Richard Weinberger
  2015-01-02 22:29           ` Pavel Machek
  0 siblings, 1 reply; 24+ messages in thread
From: Richard Weinberger @ 2015-01-02 21:40 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Kees Cook, LKML, linux-fsdevel, David Rientjes, Aaron Tomlin,
	DaeSeok Youn, Thomas Gleixner, vdavydov, Rik van Riel,
	Oleg Nesterov, Andrew Morton, Peter Zijlstra, Ingo Molnar,
	Al Viro, Andy Lutomirski, Brad Spengler

Am 02.01.2015 um 20:46 schrieb Pavel Machek:
>>> Does this break trinity, crashme, and similar programs?
>>
>> If they fork() without execve() and a child dies very fast the next fork()
>> will be throttled.
>> This is why I'd like to make this feature disabled by default.
>>
>>> Can you detect it died due to the stack canary? Then, the patch might
>>> be actually acceptable.
>>
>> I don't think so as this is glibc specific.
> 
> Can the slowdown be impelmented in glibc, then?

glibc has a lot of asserts where it can detect stack smashing and kills the
current process using abort(). Here it could of course also call sleep().

> If not, can glibc provide enough information to the kernel to allow us
> to do the right thing?

IMHO we should not strictly focus on the stack canary.
If an attacker can kind of control the attacked child and it segfaults the generic
in-kernel bruteforce detection will still work.
Many exploits use the fact that after fork() the child has the same memory as before
and brute force is possible. A user space solution won't help here.

Thanks,
//richard

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

* Re: [PATCH] [RFC] Deter exploit bruteforcing
  2015-01-02 21:40         ` Richard Weinberger
@ 2015-01-02 22:29           ` Pavel Machek
  2015-01-02 22:32             ` Jiri Kosina
  0 siblings, 1 reply; 24+ messages in thread
From: Pavel Machek @ 2015-01-02 22:29 UTC (permalink / raw)
  To: Richard Weinberger
  Cc: Kees Cook, LKML, linux-fsdevel, David Rientjes, Aaron Tomlin,
	DaeSeok Youn, Thomas Gleixner, vdavydov, Rik van Riel,
	Oleg Nesterov, Andrew Morton, Peter Zijlstra, Ingo Molnar,
	Al Viro, Andy Lutomirski, Brad Spengler

On Fri 2015-01-02 22:40:14, Richard Weinberger wrote:
> Am 02.01.2015 um 20:46 schrieb Pavel Machek:
> >>> Does this break trinity, crashme, and similar programs?
> >>
> >> If they fork() without execve() and a child dies very fast the next fork()
> >> will be throttled.
> >> This is why I'd like to make this feature disabled by default.
> >>
> >>> Can you detect it died due to the stack canary? Then, the patch might
> >>> be actually acceptable.
> >>
> >> I don't think so as this is glibc specific.
> > 
> > Can the slowdown be impelmented in glibc, then?
> 
> glibc has a lot of asserts where it can detect stack smashing and kills the
> current process using abort(). Here it could of course also call
> sleep().

Please do it in glibc, then.

> > If not, can glibc provide enough information to the kernel to allow us
> > to do the right thing?
> 
> IMHO we should not strictly focus on the stack canary.

IMO we should. We want it enabled by default.
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

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

* Re: [PATCH] [RFC] Deter exploit bruteforcing
  2015-01-02 22:29           ` Pavel Machek
@ 2015-01-02 22:32             ` Jiri Kosina
  2015-01-02 22:46               ` Pavel Machek
  0 siblings, 1 reply; 24+ messages in thread
From: Jiri Kosina @ 2015-01-02 22:32 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Richard Weinberger, Kees Cook, LKML, linux-fsdevel,
	David Rientjes, Aaron Tomlin, DaeSeok Youn, Thomas Gleixner,
	vdavydov, Rik van Riel, Oleg Nesterov, Andrew Morton,
	Peter Zijlstra, Ingo Molnar, Al Viro, Andy Lutomirski,
	Brad Spengler

On Fri, 2 Jan 2015, Pavel Machek wrote:

> > > Can the slowdown be impelmented in glibc, then?
> > 
> > glibc has a lot of asserts where it can detect stack smashing and kills the
> > current process using abort(). Here it could of course also call
> > sleep().
> 
> Please do it in glibc, then.

You also want to protect against binaries that are evil on purpose, right?

There is no guarantee that those will calling the kernel through glibc at 
all.

-- 
Jiri Kosina
SUSE Labs

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

* Re: [PATCH] [RFC] Deter exploit bruteforcing
  2015-01-02 22:32             ` Jiri Kosina
@ 2015-01-02 22:46               ` Pavel Machek
  2015-01-02 22:49                 ` Jiri Kosina
  0 siblings, 1 reply; 24+ messages in thread
From: Pavel Machek @ 2015-01-02 22:46 UTC (permalink / raw)
  To: Jiri Kosina
  Cc: Richard Weinberger, Kees Cook, LKML, linux-fsdevel,
	David Rientjes, Aaron Tomlin, DaeSeok Youn, Thomas Gleixner,
	vdavydov, Rik van Riel, Oleg Nesterov, Andrew Morton,
	Peter Zijlstra, Ingo Molnar, Al Viro, Andy Lutomirski,
	Brad Spengler

On Fri 2015-01-02 23:32:35, Jiri Kosina wrote:
> On Fri, 2 Jan 2015, Pavel Machek wrote:
> 
> > > > Can the slowdown be impelmented in glibc, then?
> > > 
> > > glibc has a lot of asserts where it can detect stack smashing and kills the
> > > current process using abort(). Here it could of course also call
> > > sleep().
> > 
> > Please do it in glibc, then.
> 
> You also want to protect against binaries that are evil on purpose,
> right?

Umm. No. Not by default. We don't want to break crashme or trinity by
default.

We want to delay people trying to bruteforce glibc canaries. We want
to do it by default.

								Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

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

* Re: [PATCH] [RFC] Deter exploit bruteforcing
  2015-01-02 22:46               ` Pavel Machek
@ 2015-01-02 22:49                 ` Jiri Kosina
  2015-01-02 22:53                   ` Jiri Kosina
  2015-01-02 22:54                   ` Pavel Machek
  0 siblings, 2 replies; 24+ messages in thread
From: Jiri Kosina @ 2015-01-02 22:49 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Richard Weinberger, Kees Cook, LKML, linux-fsdevel,
	David Rientjes, Aaron Tomlin, DaeSeok Youn, Thomas Gleixner,
	vdavydov, Rik van Riel, Oleg Nesterov, Andrew Morton,
	Peter Zijlstra, Ingo Molnar, Al Viro, Andy Lutomirski,
	Brad Spengler

On Fri, 2 Jan 2015, Pavel Machek wrote:

> > You also want to protect against binaries that are evil on purpose,
> > right?
> 
> Umm. No. Not by default. We don't want to break crashme or trinity by
> default.

I thought trinity is issuing syscalls directly (would make more sense than 
going through glibc, wouldn't it?) ... haven't checked the source though.

-- 
Jiri Kosina
SUSE Labs

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

* Re: [PATCH] [RFC] Deter exploit bruteforcing
  2015-01-02 22:49                 ` Jiri Kosina
@ 2015-01-02 22:53                   ` Jiri Kosina
  2015-01-02 22:54                   ` Pavel Machek
  1 sibling, 0 replies; 24+ messages in thread
From: Jiri Kosina @ 2015-01-02 22:53 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Richard Weinberger, Kees Cook, LKML, linux-fsdevel,
	David Rientjes, Aaron Tomlin, DaeSeok Youn, Thomas Gleixner,
	vdavydov, Rik van Riel, Oleg Nesterov, Andrew Morton,
	Peter Zijlstra, Ingo Molnar, Al Viro, Andy Lutomirski,
	Brad Spengler

On Fri, 2 Jan 2015, Jiri Kosina wrote:

> > > You also want to protect against binaries that are evil on purpose,
> > > right?
> > 
> > Umm. No. Not by default. We don't want to break crashme or trinity by
> > default.
> 
> I thought trinity is issuing syscalls directly (would make more sense than 
> going through glibc, wouldn't it?) ... haven't checked the source though.

Okay, I checked, it is. Now I get your point. Seems like "too much pain 
for little gain" though. So it really should be optional, so that 
potentially exposed systems (such as hosting servers, where things like 
trinity are not expected to be run) could turn it on voluntarily.

-- 
Jiri Kosina
SUSE Labs

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

* Re: [PATCH] [RFC] Deter exploit bruteforcing
  2015-01-02 22:49                 ` Jiri Kosina
  2015-01-02 22:53                   ` Jiri Kosina
@ 2015-01-02 22:54                   ` Pavel Machek
  2015-01-02 23:00                     ` Richard Weinberger
  1 sibling, 1 reply; 24+ messages in thread
From: Pavel Machek @ 2015-01-02 22:54 UTC (permalink / raw)
  To: Jiri Kosina
  Cc: Richard Weinberger, Kees Cook, LKML, linux-fsdevel,
	David Rientjes, Aaron Tomlin, DaeSeok Youn, Thomas Gleixner,
	vdavydov, Rik van Riel, Oleg Nesterov, Andrew Morton,
	Peter Zijlstra, Ingo Molnar, Al Viro, Andy Lutomirski,
	Brad Spengler

On Fri 2015-01-02 23:49:52, Jiri Kosina wrote:
> On Fri, 2 Jan 2015, Pavel Machek wrote:
> 
> > > You also want to protect against binaries that are evil on purpose,
> > > right?
> > 
> > Umm. No. Not by default. We don't want to break crashme or trinity by
> > default.
> 
> I thought trinity is issuing syscalls directly (would make more sense than 
> going through glibc, wouldn't it?) ... haven't checked the source though.

Patch in this thread wanted to insert delays into kernel on SIGSEGV
processing. That's bad idea by default.

But changing glibc to do sleep(30); abort(); instead of abort(); to
slow down bruteforcing of canaries makes some kind of sense... and
should be ok by default.
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

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

* Re: [PATCH] [RFC] Deter exploit bruteforcing
  2015-01-02 22:54                   ` Pavel Machek
@ 2015-01-02 23:00                     ` Richard Weinberger
  2015-01-02 23:08                       ` Pavel Machek
  0 siblings, 1 reply; 24+ messages in thread
From: Richard Weinberger @ 2015-01-02 23:00 UTC (permalink / raw)
  To: Pavel Machek, Jiri Kosina
  Cc: Kees Cook, LKML, linux-fsdevel, David Rientjes, Aaron Tomlin,
	DaeSeok Youn, Thomas Gleixner, vdavydov, Rik van Riel,
	Oleg Nesterov, Andrew Morton, Peter Zijlstra, Ingo Molnar,
	Al Viro, Andy Lutomirski, Brad Spengler

Am 02.01.2015 um 23:54 schrieb Pavel Machek:
> On Fri 2015-01-02 23:49:52, Jiri Kosina wrote:
>> On Fri, 2 Jan 2015, Pavel Machek wrote:
>>
>>>> You also want to protect against binaries that are evil on purpose,
>>>> right?
>>>
>>> Umm. No. Not by default. We don't want to break crashme or trinity by
>>> default.
>>
>> I thought trinity is issuing syscalls directly (would make more sense than 
>> going through glibc, wouldn't it?) ... haven't checked the source though.
> 
> Patch in this thread wanted to insert delays into kernel on SIGSEGV
> processing. That's bad idea by default.

No. This is not what this patch does.

> But changing glibc to do sleep(30); abort(); instead of abort(); to
> slow down bruteforcing of canaries makes some kind of sense... and
> should be ok by default.

As I saidn only focusing one the specific stack canary case is not enough.

Thanks,
//richard

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

* Re: [PATCH] [RFC] Deter exploit bruteforcing
  2015-01-02 23:00                     ` Richard Weinberger
@ 2015-01-02 23:08                       ` Pavel Machek
  2015-01-03  9:45                         ` Richard Weinberger
  0 siblings, 1 reply; 24+ messages in thread
From: Pavel Machek @ 2015-01-02 23:08 UTC (permalink / raw)
  To: Richard Weinberger
  Cc: Jiri Kosina, Kees Cook, LKML, linux-fsdevel, David Rientjes,
	Aaron Tomlin, DaeSeok Youn, Thomas Gleixner, vdavydov,
	Rik van Riel, Oleg Nesterov, Andrew Morton, Peter Zijlstra,
	Ingo Molnar, Al Viro, Andy Lutomirski, Brad Spengler

On Sat 2015-01-03 00:00:22, Richard Weinberger wrote:
> Am 02.01.2015 um 23:54 schrieb Pavel Machek:
> > On Fri 2015-01-02 23:49:52, Jiri Kosina wrote:
> >> On Fri, 2 Jan 2015, Pavel Machek wrote:
> >>
> >>>> You also want to protect against binaries that are evil on purpose,
> >>>> right?
> >>>
> >>> Umm. No. Not by default. We don't want to break crashme or trinity by
> >>> default.
> >>
> >> I thought trinity is issuing syscalls directly (would make more sense than 
> >> going through glibc, wouldn't it?) ... haven't checked the source though.
> > 
> > Patch in this thread wanted to insert delays into kernel on SIGSEGV
> > processing. That's bad idea by default.
> 
> No. This is not what this patch does.
> 
> > But changing glibc to do sleep(30); abort(); instead of abort(); to
> > slow down bruteforcing of canaries makes some kind of sense... and
> > should be ok by default.
> 
> As I saidn only focusing one the specific stack canary case is not enough.

Ok, so I am now saying "adding random delays to the kernel, hoping
they slow attacker down" is bad idea. Feel free to add my NAK to the
patch.

If really neccessary, "kill_me_slowly()" syscall would be acceptable,
but it seems just sleep(); abort(); combination is enough.

glibc should cover 99% cases where this matters, please just fix glibc,
others will follow.
								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

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

* Re: [PATCH] [RFC] Deter exploit bruteforcing
  2015-01-02 23:08                       ` Pavel Machek
@ 2015-01-03  9:45                         ` Richard Weinberger
  2015-01-03 22:36                           ` Pavel Machek
  0 siblings, 1 reply; 24+ messages in thread
From: Richard Weinberger @ 2015-01-03  9:45 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Jiri Kosina, Kees Cook, LKML, linux-fsdevel, David Rientjes,
	Aaron Tomlin, DaeSeok Youn, Thomas Gleixner, vdavydov,
	Rik van Riel, Oleg Nesterov, Andrew Morton, Peter Zijlstra,
	Ingo Molnar, Al Viro, Andy Lutomirski, Brad Spengler

Am 03.01.2015 um 00:08 schrieb Pavel Machek:
> On Sat 2015-01-03 00:00:22, Richard Weinberger wrote:
>> Am 02.01.2015 um 23:54 schrieb Pavel Machek:
>>> On Fri 2015-01-02 23:49:52, Jiri Kosina wrote:
>>>> On Fri, 2 Jan 2015, Pavel Machek wrote:
>>>>
>>>>>> You also want to protect against binaries that are evil on purpose,
>>>>>> right?
>>>>>
>>>>> Umm. No. Not by default. We don't want to break crashme or trinity by
>>>>> default.
>>>>
>>>> I thought trinity is issuing syscalls directly (would make more sense than 
>>>> going through glibc, wouldn't it?) ... haven't checked the source though.
>>>
>>> Patch in this thread wanted to insert delays into kernel on SIGSEGV
>>> processing. That's bad idea by default.
>>
>> No. This is not what this patch does.
>>
>>> But changing glibc to do sleep(30); abort(); instead of abort(); to
>>> slow down bruteforcing of canaries makes some kind of sense... and
>>> should be ok by default.
>>
>> As I saidn only focusing one the specific stack canary case is not enough.
> 
> Ok, so I am now saying "adding random delays to the kernel, hoping
> they slow attacker down" is bad idea. Feel free to add my NAK to the
> patch.

The patch does not add random delays nor is hope involved.

It has a very clear purpose, it makes brute force attacks to forking
services unattractive.
Exploits often use the fact that after fork() the child has the same memory
as the parent and therefore an attacker can start fruitful brute force attacks
to brute stack canaries, offsets, etc. as the new child will always have mostly
the same memory layout as before.

But I'll happily add your NAK to this series.

> If really neccessary, "kill_me_slowly()" syscall would be acceptable,
> but it seems just sleep(); abort(); combination is enough.

The goal of the patch is not to protect only against brute forcing the stack canary.
It should protect against all kind of brute forcing using forking services.

> glibc should cover 99% cases where this matters, please just fix glibc,
> others will follow.

There are a lot of systems out there without glibc.
And many applications make system calls without going though any libc wrapper.
Hey, we want also protect esoteric distros like http://sta.li. :-)

I'm all for implementing as much in user space as possible but in this case the kernel
is the only sane way to offer the protection to _all_ kind of applications.

Thanks,
//richard

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

* Re: [PATCH] [RFC] Deter exploit bruteforcing
  2015-01-03  9:45                         ` Richard Weinberger
@ 2015-01-03 22:36                           ` Pavel Machek
  2015-01-03 22:44                             ` Richard Weinberger
  2015-01-03 23:06                             ` Andy Lutomirski
  0 siblings, 2 replies; 24+ messages in thread
From: Pavel Machek @ 2015-01-03 22:36 UTC (permalink / raw)
  To: Richard Weinberger
  Cc: Jiri Kosina, Kees Cook, LKML, linux-fsdevel, David Rientjes,
	Aaron Tomlin, DaeSeok Youn, Thomas Gleixner, vdavydov,
	Rik van Riel, Oleg Nesterov, Andrew Morton, Peter Zijlstra,
	Ingo Molnar, Al Viro, Andy Lutomirski, Brad Spengler


> >> No. This is not what this patch does.
> >>
> >>> But changing glibc to do sleep(30); abort(); instead of abort(); to
> >>> slow down bruteforcing of canaries makes some kind of sense... and
> >>> should be ok by default.
> >>
> >> As I saidn only focusing one the specific stack canary case is not enough.
> > 
> > Ok, so I am now saying "adding random delays to the kernel, hoping
> > they slow attacker down" is bad idea. Feel free to add my NAK to the
> > patch.
> 
> The patch does not add random delays nor is hope involved.
> 
> It has a very clear purpose, it makes brute force attacks to forking
> services unattractive.
> Exploits often use the fact that after fork() the child has the same memory
> as the parent and therefore an attacker can start fruitful brute force attacks
> to brute stack canaries, offsets, etc. as the new child will always have mostly
> the same memory layout as before.
> 
> But I'll happily add your NAK to this series.

Please do.

> > If really neccessary, "kill_me_slowly()" syscall would be acceptable,
> > but it seems just sleep(); abort(); combination is enough.
> 
> The goal of the patch is not to protect only against brute forcing the stack canary.
> It should protect against all kind of brute forcing using forking services.
> 
> > glibc should cover 99% cases where this matters, please just fix glibc,
> > others will follow.
> 
> There are a lot of systems out there without glibc.

Only "interesting" systems that are without glibc are androids, and
they usually run very old kernels.

If you implement sleep() in glibc, distros will enable it and you'll
protect all the desktop users.

If you implement it in kernel, it will not be compatible-enough to be
enabled by default, and you'll be protecting special "high security"
distros at most.

> And many applications make system calls without going though any libc wrapper.
> Hey, we want also protect esoteric distros like http://sta.li. :-)

No, we don't. We want to maximize number of protected users. And
patching glibc does that. (And then you can patch bionic. And then the
small players will follow).

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

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

* Re: [PATCH] [RFC] Deter exploit bruteforcing
  2015-01-03 22:36                           ` Pavel Machek
@ 2015-01-03 22:44                             ` Richard Weinberger
  2015-01-03 23:01                               ` Pavel Machek
  2015-01-03 23:06                             ` Andy Lutomirski
  1 sibling, 1 reply; 24+ messages in thread
From: Richard Weinberger @ 2015-01-03 22:44 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Jiri Kosina, Kees Cook, LKML, linux-fsdevel, David Rientjes,
	Aaron Tomlin, DaeSeok Youn, Thomas Gleixner, vdavydov,
	Rik van Riel, Oleg Nesterov, Andrew Morton, Peter Zijlstra,
	Ingo Molnar, Al Viro, Andy Lutomirski, Brad Spengler

Am 03.01.2015 um 23:36 schrieb Pavel Machek:
> 
>>>> No. This is not what this patch does.
>>>>
>>>>> But changing glibc to do sleep(30); abort(); instead of abort(); to
>>>>> slow down bruteforcing of canaries makes some kind of sense... and
>>>>> should be ok by default.
>>>>
>>>> As I saidn only focusing one the specific stack canary case is not enough.
>>>
>>> Ok, so I am now saying "adding random delays to the kernel, hoping
>>> they slow attacker down" is bad idea. Feel free to add my NAK to the
>>> patch.
>>
>> The patch does not add random delays nor is hope involved.
>>
>> It has a very clear purpose, it makes brute force attacks to forking
>> services unattractive.
>> Exploits often use the fact that after fork() the child has the same memory
>> as the parent and therefore an attacker can start fruitful brute force attacks
>> to brute stack canaries, offsets, etc. as the new child will always have mostly
>> the same memory layout as before.
>>
>> But I'll happily add your NAK to this series.
> 
> Please do.
> 
>>> If really neccessary, "kill_me_slowly()" syscall would be acceptable,
>>> but it seems just sleep(); abort(); combination is enough.
>>
>> The goal of the patch is not to protect only against brute forcing the stack canary.
>> It should protect against all kind of brute forcing using forking services.
>>
>>> glibc should cover 99% cases where this matters, please just fix glibc,
>>> others will follow.
>>
>> There are a lot of systems out there without glibc.
> 
> Only "interesting" systems that are without glibc are androids, and
> they usually run very old kernels.
> 
> If you implement sleep() in glibc, distros will enable it and you'll
> protect all the desktop users.
> 
> If you implement it in kernel, it will not be compatible-enough to be
> enabled by default, and you'll be protecting special "high security"
> distros at most.
> 
>> And many applications make system calls without going though any libc wrapper.
>> Hey, we want also protect esoteric distros like http://sta.li. :-)
> 
> No, we don't. We want to maximize number of protected users. And
> patching glibc does that. (And then you can patch bionic. And then the
> small players will follow).

And what about static linked programs or programs which do not use a libc wrapper
for system calls?
Say, any program written in go?

Thanks,
//richard

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

* Re: [PATCH] [RFC] Deter exploit bruteforcing
  2015-01-03 22:44                             ` Richard Weinberger
@ 2015-01-03 23:01                               ` Pavel Machek
  2015-01-03 23:07                                 ` Richard Weinberger
  0 siblings, 1 reply; 24+ messages in thread
From: Pavel Machek @ 2015-01-03 23:01 UTC (permalink / raw)
  To: Richard Weinberger
  Cc: Jiri Kosina, Kees Cook, LKML, linux-fsdevel, David Rientjes,
	Aaron Tomlin, DaeSeok Youn, Thomas Gleixner, vdavydov,
	Rik van Riel, Oleg Nesterov, Andrew Morton, Peter Zijlstra,
	Ingo Molnar, Al Viro, Andy Lutomirski, Brad Spengler

On Sat 2015-01-03 23:44:18, Richard Weinberger wrote:
> Am 03.01.2015 um 23:36 schrieb Pavel Machek:
> > 
> >>>> No. This is not what this patch does.
> >>>>
> >>>>> But changing glibc to do sleep(30); abort(); instead of abort(); to
> >>>>> slow down bruteforcing of canaries makes some kind of sense... and
> >>>>> should be ok by default.
> >>>>
> >>>> As I saidn only focusing one the specific stack canary case is not enough.
> >>>
> >>> Ok, so I am now saying "adding random delays to the kernel, hoping
> >>> they slow attacker down" is bad idea. Feel free to add my NAK to the
> >>> patch.
> >>
> >> The patch does not add random delays nor is hope involved.
> >>
> >> It has a very clear purpose, it makes brute force attacks to forking
> >> services unattractive.
> >> Exploits often use the fact that after fork() the child has the same memory
> >> as the parent and therefore an attacker can start fruitful brute force attacks
> >> to brute stack canaries, offsets, etc. as the new child will always have mostly
> >> the same memory layout as before.
> >>
> >> But I'll happily add your NAK to this series.
> > 
> > Please do.
> > 
> >>> If really neccessary, "kill_me_slowly()" syscall would be acceptable,
> >>> but it seems just sleep(); abort(); combination is enough.
> >>
> >> The goal of the patch is not to protect only against brute forcing the stack canary.
> >> It should protect against all kind of brute forcing using forking services.
> >>
> >>> glibc should cover 99% cases where this matters, please just fix glibc,
> >>> others will follow.
> >>
> >> There are a lot of systems out there without glibc.
> > 
> > Only "interesting" systems that are without glibc are androids, and
> > they usually run very old kernels.
> > 
> > If you implement sleep() in glibc, distros will enable it and you'll
> > protect all the desktop users.
> > 
> > If you implement it in kernel, it will not be compatible-enough to be
> > enabled by default, and you'll be protecting special "high security"
> > distros at most.
> > 
> >> And many applications make system calls without going though any libc wrapper.
> >> Hey, we want also protect esoteric distros like http://sta.li. :-)
> > 
> > No, we don't. We want to maximize number of protected users. And
> > patching glibc does that. (And then you can patch bionic. And then the
> > small players will follow).
> 
> And what about static linked programs or programs which do not use a libc wrapper
> for system calls?
> Say, any program written in go?

And what about my Atari 800XL?

How many such programs are on common distributions? <1%

How many systems will your kernel hack leave unprotected? >70%

(Plus, reasonable languages like go should not really allow classical
buffer overflows, and yes, you'll get protection if you statically
link against glibc. And AFAICT this has nothing to do with syscalls,
and everything to do with abort() implementation.).

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

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

* Re: [PATCH] [RFC] Deter exploit bruteforcing
  2015-01-03 22:36                           ` Pavel Machek
  2015-01-03 22:44                             ` Richard Weinberger
@ 2015-01-03 23:06                             ` Andy Lutomirski
  2015-01-03 23:19                               ` Richard Weinberger
  1 sibling, 1 reply; 24+ messages in thread
From: Andy Lutomirski @ 2015-01-03 23:06 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Richard Weinberger, Jiri Kosina, Kees Cook, LKML, linux-fsdevel,
	David Rientjes, Aaron Tomlin, DaeSeok Youn, Thomas Gleixner,
	vdavydov, Rik van Riel, Oleg Nesterov, Andrew Morton,
	Peter Zijlstra, Ingo Molnar, Al Viro, Brad Spengler

On Sat, Jan 3, 2015 at 2:36 PM, Pavel Machek <pavel@ucw.cz> wrote:
>
>> >> No. This is not what this patch does.
>> >>
>> >>> But changing glibc to do sleep(30); abort(); instead of abort(); to
>> >>> slow down bruteforcing of canaries makes some kind of sense... and
>> >>> should be ok by default.
>> >>
>> >> As I saidn only focusing one the specific stack canary case is not enough.
>> >
>> > Ok, so I am now saying "adding random delays to the kernel, hoping
>> > they slow attacker down" is bad idea. Feel free to add my NAK to the
>> > patch.
>>
>> The patch does not add random delays nor is hope involved.
>>
>> It has a very clear purpose, it makes brute force attacks to forking
>> services unattractive.
>> Exploits often use the fact that after fork() the child has the same memory
>> as the parent and therefore an attacker can start fruitful brute force attacks
>> to brute stack canaries, offsets, etc. as the new child will always have mostly
>> the same memory layout as before.
>>
>> But I'll happily add your NAK to this series.
>
> Please do.
>
>> > If really neccessary, "kill_me_slowly()" syscall would be acceptable,
>> > but it seems just sleep(); abort(); combination is enough.
>>
>> The goal of the patch is not to protect only against brute forcing the stack canary.
>> It should protect against all kind of brute forcing using forking services.
>>
>> > glibc should cover 99% cases where this matters, please just fix glibc,
>> > others will follow.
>>
>> There are a lot of systems out there without glibc.
>
> Only "interesting" systems that are without glibc are androids, and
> they usually run very old kernels.
>
> If you implement sleep() in glibc, distros will enable it and you'll
> protect all the desktop users.

As an attempt to help end this particular line of debate: putting the
sleep in glibc won't work.  The point isn't to make the crashed
process crash more slowly; it's to limit the rate at which *new*
siblings can be forked and crashed as a canary or ASLR brute-force
probe.  IOW, adding a sleep call to glibc slows down the wrong thing.
Also, trying to get libc to take action on a plain old segfault is a
giant mess, because it involves mucking with signal handling, which
glibc really has no business doing by default.

Also, this patch is missing a bit, I think.  We really want to control
the total rate of crashes.  This patch imposes a delay per crash, but
AFAICS it would still be possible for an attacker to coerce a forking
server to fork, say, 10k children, then probe all of them, then wait
30 seconds and repeat.

--Andy

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

* Re: [PATCH] [RFC] Deter exploit bruteforcing
  2015-01-03 23:01                               ` Pavel Machek
@ 2015-01-03 23:07                                 ` Richard Weinberger
  0 siblings, 0 replies; 24+ messages in thread
From: Richard Weinberger @ 2015-01-03 23:07 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Jiri Kosina, Kees Cook, LKML, linux-fsdevel, David Rientjes,
	Aaron Tomlin, DaeSeok Youn, Thomas Gleixner, vdavydov,
	Rik van Riel, Oleg Nesterov, Andrew Morton, Peter Zijlstra,
	Ingo Molnar, Al Viro, Andy Lutomirski, Brad Spengler

Am 04.01.2015 um 00:01 schrieb Pavel Machek:
> On Sat 2015-01-03 23:44:18, Richard Weinberger wrote:
>> Am 03.01.2015 um 23:36 schrieb Pavel Machek:
>>>
>>>>>> No. This is not what this patch does.
>>>>>>
>>>>>>> But changing glibc to do sleep(30); abort(); instead of abort(); to
>>>>>>> slow down bruteforcing of canaries makes some kind of sense... and
>>>>>>> should be ok by default.
>>>>>>
>>>>>> As I saidn only focusing one the specific stack canary case is not enough.
>>>>>
>>>>> Ok, so I am now saying "adding random delays to the kernel, hoping
>>>>> they slow attacker down" is bad idea. Feel free to add my NAK to the
>>>>> patch.
>>>>
>>>> The patch does not add random delays nor is hope involved.
>>>>
>>>> It has a very clear purpose, it makes brute force attacks to forking
>>>> services unattractive.
>>>> Exploits often use the fact that after fork() the child has the same memory
>>>> as the parent and therefore an attacker can start fruitful brute force attacks
>>>> to brute stack canaries, offsets, etc. as the new child will always have mostly
>>>> the same memory layout as before.
>>>>
>>>> But I'll happily add your NAK to this series.
>>>
>>> Please do.
>>>
>>>>> If really neccessary, "kill_me_slowly()" syscall would be acceptable,
>>>>> but it seems just sleep(); abort(); combination is enough.
>>>>
>>>> The goal of the patch is not to protect only against brute forcing the stack canary.
>>>> It should protect against all kind of brute forcing using forking services.
>>>>
>>>>> glibc should cover 99% cases where this matters, please just fix glibc,
>>>>> others will follow.
>>>>
>>>> There are a lot of systems out there without glibc.
>>>
>>> Only "interesting" systems that are without glibc are androids, and
>>> they usually run very old kernels.
>>>
>>> If you implement sleep() in glibc, distros will enable it and you'll
>>> protect all the desktop users.
>>>
>>> If you implement it in kernel, it will not be compatible-enough to be
>>> enabled by default, and you'll be protecting special "high security"
>>> distros at most.
>>>
>>>> And many applications make system calls without going though any libc wrapper.
>>>> Hey, we want also protect esoteric distros like http://sta.li. :-)
>>>
>>> No, we don't. We want to maximize number of protected users. And
>>> patching glibc does that. (And then you can patch bionic. And then the
>>> small players will follow).
>>
>> And what about static linked programs or programs which do not use a libc wrapper
>> for system calls?
>> Say, any program written in go?
> 
> And what about my Atari 800XL?

If it runs Linux it can be protected.

> How many such programs are on common distributions? <1%
> 
> How many systems will your kernel hack leave unprotected? >70%
> 
> (Plus, reasonable languages like go should not really allow classical
> buffer overflows, and yes, you'll get protection if you statically
> link against glibc. And AFAICT this has nothing to do with syscalls,
> and everything to do with abort() implementation.).

Go does not use libc at all.

Anyway, you've stated your point.
I'm for a generic solution and not for one which works only for some systems.

Thanks,
//richard

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

* Re: [PATCH] [RFC] Deter exploit bruteforcing
  2015-01-03 23:06                             ` Andy Lutomirski
@ 2015-01-03 23:19                               ` Richard Weinberger
  2015-01-05 22:56                                 ` Kees Cook
  0 siblings, 1 reply; 24+ messages in thread
From: Richard Weinberger @ 2015-01-03 23:19 UTC (permalink / raw)
  To: Andy Lutomirski, Pavel Machek
  Cc: Jiri Kosina, Kees Cook, LKML, linux-fsdevel, David Rientjes,
	Aaron Tomlin, DaeSeok Youn, Thomas Gleixner, vdavydov,
	Rik van Riel, Oleg Nesterov, Andrew Morton, Peter Zijlstra,
	Ingo Molnar, Al Viro, Brad Spengler

Am 04.01.2015 um 00:06 schrieb Andy Lutomirski:
> As an attempt to help end this particular line of debate: putting the
> sleep in glibc won't work.  The point isn't to make the crashed
> process crash more slowly; it's to limit the rate at which *new*
> siblings can be forked and crashed as a canary or ASLR brute-force
> probe.  IOW, adding a sleep call to glibc slows down the wrong thing.
> Also, trying to get libc to take action on a plain old segfault is a
> giant mess, because it involves mucking with signal handling, which
> glibc really has no business doing by default.

Thanks for pointing this out!

> Also, this patch is missing a bit, I think.  We really want to control
> the total rate of crashes.  This patch imposes a delay per crash, but
> AFAICS it would still be possible for an attacker to coerce a forking
> server to fork, say, 10k children, then probe all of them, then wait
> 30 seconds and repeat.

Sounds reasonable. This is exactly why I've extracted that feature from
grsecurity and posted it here on LKML.
Now we have the chance to make the feature better and can identify weak points.

Thanks,
//richard

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

* Re: [PATCH] [RFC] Deter exploit bruteforcing
  2015-01-03 23:19                               ` Richard Weinberger
@ 2015-01-05 22:56                                 ` Kees Cook
  0 siblings, 0 replies; 24+ messages in thread
From: Kees Cook @ 2015-01-05 22:56 UTC (permalink / raw)
  To: Richard Weinberger
  Cc: Andy Lutomirski, Pavel Machek, Jiri Kosina, LKML, linux-fsdevel,
	David Rientjes, Aaron Tomlin, DaeSeok Youn, Thomas Gleixner,
	vdavydov, Rik van Riel, Oleg Nesterov, Andrew Morton,
	Peter Zijlstra, Ingo Molnar, Al Viro, Brad Spengler, PaX Team

On Sat, Jan 3, 2015 at 3:19 PM, Richard Weinberger <richard@nod.at> wrote:
> Am 04.01.2015 um 00:06 schrieb Andy Lutomirski:
>> As an attempt to help end this particular line of debate: putting the
>> sleep in glibc won't work.  The point isn't to make the crashed
>> process crash more slowly; it's to limit the rate at which *new*
>> siblings can be forked and crashed as a canary or ASLR brute-force
>> probe.  IOW, adding a sleep call to glibc slows down the wrong thing.
>> Also, trying to get libc to take action on a plain old segfault is a
>> giant mess, because it involves mucking with signal handling, which
>> glibc really has no business doing by default.
>
> Thanks for pointing this out!

Yeah, this doesn't belong in glibc. The support for fork-speed-control
must be in the kernel. Improving glibc is an entirely separate idea.

For glibc, it would be nice to create a "halp, I'm being attacked"
syscall that like abort(), but used in cases of memory corruption,
etc. This would provide a much cleaner way for programs to communicate
to the kernel that something malicious is happening. Neither SIGABRT
nor SIGSYS are quite right for this, but SIGSYS (as used by seccomp)
is likely closer.

>> Also, this patch is missing a bit, I think.  We really want to control
>> the total rate of crashes.  This patch imposes a delay per crash, but
>> AFAICS it would still be possible for an attacker to coerce a forking
>> server to fork, say, 10k children, then probe all of them, then wait
>> 30 seconds and repeat.

I general, while I would expect this to be limited by the default
RLIMIT_NOFILE of 1024, it's still a valid observation, though the
scope is smaller. Regardless, it would still be a defensive benefit to
slow down forks.

Brad, PaXTeam: have you considered limiting open connections from the
same IP address or anything like that for defending against this kind
"wide prefork" attack?

> Sounds reasonable. This is exactly why I've extracted that feature from
> grsecurity and posted it here on LKML.
> Now we have the chance to make the feature better and can identify weak points.

The grsecurity version also handles set[ug]id binaries more aggressively.
https://en.wikibooks.org/wiki/Grsecurity/Appendix/Grsecurity_and_PaX_Configuration_Options#Deter_exploit_bruteforcing

Speaking to "default or not", it needs to be off by default in the
kernel configs. It's easy to imagine things that will trip on false
positives, so we must first get the system in place in the kernel and
improve it until we get to the point where we can set it on by default
(this may take years, after all the distros are using it, etc etc).
Security feature addition requires a fair bit of patience. :)

If you can spin up a version with sysctls and Kconfig stuff, I think
that'd be the best next step.

-Kees

-- 
Kees Cook
Chrome OS Security

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

end of thread, other threads:[~2015-01-05 22:56 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-12-24 21:39 [PATCH] [RFC] Deter exploit bruteforcing Richard Weinberger
2014-12-30 18:40 ` Kees Cook
2014-12-30 18:49   ` Andy Lutomirski
2014-12-30 18:50   ` Richard Weinberger
2015-01-02  5:11   ` Pavel Machek
2015-01-02 11:00     ` Richard Weinberger
2015-01-02 19:46       ` Pavel Machek
2015-01-02 21:40         ` Richard Weinberger
2015-01-02 22:29           ` Pavel Machek
2015-01-02 22:32             ` Jiri Kosina
2015-01-02 22:46               ` Pavel Machek
2015-01-02 22:49                 ` Jiri Kosina
2015-01-02 22:53                   ` Jiri Kosina
2015-01-02 22:54                   ` Pavel Machek
2015-01-02 23:00                     ` Richard Weinberger
2015-01-02 23:08                       ` Pavel Machek
2015-01-03  9:45                         ` Richard Weinberger
2015-01-03 22:36                           ` Pavel Machek
2015-01-03 22:44                             ` Richard Weinberger
2015-01-03 23:01                               ` Pavel Machek
2015-01-03 23:07                                 ` Richard Weinberger
2015-01-03 23:06                             ` Andy Lutomirski
2015-01-03 23:19                               ` Richard Weinberger
2015-01-05 22:56                                 ` Kees Cook

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).