All of lore.kernel.org
 help / color / mirror / Atom feed
From: John Wood <john.wood@gmx.com>
To: Jann Horn <jannh@google.com>
Cc: Kees Cook <keescook@chromium.org>,
	Kernel Hardening <kernel-hardening@lists.openwall.com>,
	John Wood <john.wood@gmx.com>,
	Matthew Wilcox <willy@infradead.org>,
	Jonathan Corbet <corbet@lwn.net>,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	Ingo Molnar <mingo@redhat.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Juri Lelli <juri.lelli@redhat.com>,
	Vincent Guittot <vincent.guittot@linaro.org>,
	Dietmar Eggemann <dietmar.eggemann@arm.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	Ben Segall <bsegall@google.com>, Mel Gorman <mgorman@suse.de>,
	Luis Chamberlain <mcgrof@kernel.org>,
	Iurii Zaikin <yzaikin@google.com>,
	James Morris <jmorris@namei.org>,
	"Serge E. Hallyn" <serge@hallyn.com>,
	linux-doc@vger.kernel.org,
	kernel list <linux-kernel@vger.kernel.org>,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>,
	linux-security-module <linux-security-module@vger.kernel.org>
Subject: Re: [RFC PATCH 5/6] security/fbfam: Detect a fork brute force attack
Date: Sun, 13 Sep 2020 19:54:51 +0200	[thread overview]
Message-ID: <20200913172415.GA2880@ubuntu> (raw)
In-Reply-To: <CAG48ez1gbu+eBA_PthLemcVVR+AU7Xa1zzbJ8tLMLBDCe_a+fQ@mail.gmail.com>

Hi,

On Thu, Sep 10, 2020 at 11:10:38PM +0200, Jann Horn wrote:
> On Thu, Sep 10, 2020 at 10:22 PM Kees Cook <keescook@chromium.org> wrote:
> > To detect a fork brute force attack it is necessary to compute the
> > crashing rate of the application. This calculation is performed in each
> > fatal fail of a task, or in other words, when a core dump is triggered.
> > If this rate shows that the application is crashing quickly, there is a
> > clear signal that an attack is happening.
> >
> > Since the crashing rate is computed in milliseconds per fault, if this
> > rate goes under a certain threshold a warning is triggered.
> [...]
> > +/**
> > + * fbfam_handle_attack() - Fork brute force attack detection.
> > + * @signal: Signal number that causes the core dump.
> > + *
> > + * The crashing rate of an application is computed in milliseconds per fault in
> > + * each crash. So, if this rate goes under a certain threshold there is a clear
> > + * signal that the application is crashing quickly. At this moment, a fork brute
> > + * force attack is happening.
> > + *
> > + * Return: -EFAULT if the current task doesn't have statistical data. Zero
> > + *         otherwise.
> > + */
> > +int fbfam_handle_attack(int signal)
> > +{
> > +       struct fbfam_stats *stats = current->fbfam_stats;
> > +       u64 delta_jiffies, delta_time;
> > +       u64 crashing_rate;
> > +
> > +       if (!stats)
> > +               return -EFAULT;
> > +
> > +       if (!(signal == SIGILL || signal == SIGBUS || signal == SIGKILL ||
> > +             signal == SIGSEGV || signal == SIGSYS))
> > +               return 0;
>
> As far as I can tell, you can never get here with SIGKILL, since
> SIGKILL doesn't trigger core dumping and also isn't used by seccomp?

Understood.

> > +
> > +       stats->faults += 1;
>
> This is a data race. If you want to be able to increment a variable
> that may be concurrently incremented by other tasks, use either
> locking or the atomic_t helpers.

Ok, I will correct this for the next version. Thanks.

> > +       delta_jiffies = get_jiffies_64() - stats->jiffies;
> > +       delta_time = jiffies64_to_msecs(delta_jiffies);
> > +       crashing_rate = delta_time / (u64)stats->faults;
>
> Do I see this correctly, is this computing the total runtime of this
> process hierarchy divided by the total number of faults seen in this
> process hierarchy? If so, you may want to reconsider whether that's
> really the behavior you want. For example, if I configure the minimum
> period between crashes to be 30s (as is the default in the sysctl
> patch), and I try to attack a server that has been running without any
> crashes for a month, I'd instantly be able to crash around
> 30*24*60*60/30 = 86400 times before the detection kicks in. That seems
> suboptimal.

You are right. This is not the behaviour we want. So, for the next
version it would be better to compute the crashing period as the time
between two faults, or the time between the execve call and the first
fault (first fault case).

However, I am afraid of a premature detection if a child process fails
twice in a short period.

So, I think it would be a good idea add a new sysctl to setup a
minimum number of faults before the time between faults starts to be
computed. And so, the attack detection only will be triggered if the
application crashes quickly but after a number of crashes.

What do you think?

>
> (By the way, it kind of annoys me that you call it the "rate" when
> it's actually the inverse of the rate. "Period" might be more
> appropriate?)

Yes, "period" it's more appropiate. Thanks for the clarification.

> > +       if (crashing_rate < (u64)sysctl_crashing_rate_threshold)
> > +               pr_warn("fbfam: Fork brute force attack detected\n");
> > +
> > +       return 0;
> > +}
> > +
> > --
> > 2.25.1
> >

Regards,
John Wood


  reply	other threads:[~2020-09-13 17:56 UTC|newest]

Thread overview: 58+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-10 20:21 [RESEND][RFC PATCH 0/6] Fork brute force attack mitigation (fbfam) Kees Cook
2020-09-10 20:21 ` [RFC PATCH 1/6] security/fbfam: Add a Kconfig to enable the fbfam feature Kees Cook
2020-09-10 21:21   ` Jann Horn
2020-09-10 21:21     ` Jann Horn
2020-09-17 17:32     ` John Wood
2020-09-10 23:18   ` Kees Cook
2020-09-17 18:40     ` John Wood
2020-09-17 22:05       ` Kees Cook
2020-09-18 14:50         ` John Wood
2020-09-10 20:21 ` [RFC PATCH 2/6] security/fbfam: Add the api to manage statistics Kees Cook
2020-09-10 23:23   ` Kees Cook
2020-09-10 20:21 ` [RFC PATCH 3/6] security/fbfam: Use " Kees Cook
2020-09-10 20:27   ` Jann Horn
2020-09-10 20:27     ` Jann Horn
2020-09-10 23:33   ` Kees Cook
2020-09-29 23:47     ` Steven Rostedt
2020-09-29 23:49       ` Steven Rostedt
2020-10-03  9:52         ` John Wood
2020-09-10 20:21 ` [RFC PATCH 4/6] security/fbfam: Add a new sysctl to control the crashing rate threshold Kees Cook
2020-09-10 23:14   ` Kees Cook
2020-09-13 14:33     ` John Wood
2020-09-10 20:21 ` [RFC PATCH 5/6] security/fbfam: Detect a fork brute force attack Kees Cook
2020-09-10 21:10   ` Jann Horn
2020-09-10 21:10     ` Jann Horn
2020-09-13 17:54     ` John Wood [this message]
2020-09-14 19:42       ` Jann Horn
2020-09-14 19:42         ` Jann Horn
2020-09-15 18:44         ` John Wood
2020-09-10 23:49   ` Kees Cook
2020-09-11  0:01     ` Jann Horn
2020-09-11  0:01       ` Jann Horn
2020-09-13 16:56       ` John Wood
2020-09-14 19:39         ` Jann Horn
2020-09-14 19:39           ` Jann Horn
2020-09-15 17:36           ` John Wood
2020-09-10 20:21 ` [RFC PATCH 6/6] security/fbfam: Mitigate " Kees Cook
2020-09-10 20:55   ` Jann Horn
2020-09-10 20:55     ` Jann Horn
2020-09-10 23:56   ` Kees Cook
2020-09-11  0:20     ` Jann Horn
2020-09-11  0:20       ` Jann Horn
2020-09-18 16:02     ` John Wood
2020-09-18 21:35       ` Kees Cook
2020-09-19  8:01         ` John Wood
2020-09-10 20:39 ` [RESEND][RFC PATCH 0/6] Fork brute force attack mitigation (fbfam) Jann Horn
2020-09-10 20:39   ` Jann Horn
2020-09-10 23:58 ` Kees Cook
2020-09-11 14:48   ` John Wood
2020-09-12  7:55     ` Kees Cook
2020-09-12 12:24       ` John Wood
2020-09-12  0:03 ` James Morris
2020-09-12  7:56   ` Kees Cook
2020-09-12  9:36     ` John Wood
2020-09-12 14:47       ` Mel Gorman
2020-09-12 20:48         ` Ondrej Mosnacek
2020-09-12 20:48           ` Ondrej Mosnacek
2020-09-13  7:24           ` John Wood
2020-09-13  7:24             ` John Wood

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200913172415.GA2880@ubuntu \
    --to=john.wood@gmx.com \
    --cc=bsegall@google.com \
    --cc=corbet@lwn.net \
    --cc=dietmar.eggemann@arm.com \
    --cc=jannh@google.com \
    --cc=jmorris@namei.org \
    --cc=juri.lelli@redhat.com \
    --cc=keescook@chromium.org \
    --cc=kernel-hardening@lists.openwall.com \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=mcgrof@kernel.org \
    --cc=mgorman@suse.de \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=serge@hallyn.com \
    --cc=vincent.guittot@linaro.org \
    --cc=viro@zeniv.linux.org.uk \
    --cc=willy@infradead.org \
    --cc=yzaikin@google.com \
    --subject='Re: [RFC PATCH 5/6] security/fbfam: Detect a fork brute force attack' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.