kernel-hardening.lists.openwall.com archive mirror
 help / color / mirror / Atom feed
From: peter enderborg <peter.enderborg@sony.com>
To: John Wood <john.wood@gmx.com>, Kees Cook <keescook@chromium.org>,
	Jann Horn <jannh@google.com>,
	Randy Dunlap <rdunlap@infradead.org>,
	Jonathan Corbet <corbet@lwn.net>,
	James Morris <jmorris@namei.org>, Shuah Khan <shuah@kernel.org>
Cc: "Serge E. Hallyn" <serge@hallyn.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	<linux-doc@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	<linux-security-module@vger.kernel.org>,
	<linux-kselftest@vger.kernel.org>,
	<kernel-hardening@lists.openwall.com>
Subject: Re: [PATCH v5 5/8] security/brute: Mitigate a brute force attack
Date: Thu, 11 Mar 2021 21:32:47 +0100	[thread overview]
Message-ID: <5419ebe6-bb82-9b66-052b-0eefff93e5ae@sony.com> (raw)
In-Reply-To: <20210227153013.6747-6-john.wood@gmx.com>

On 2/27/21 4:30 PM, John Wood wrote:
> In order to mitigate a brute force attack all the offending tasks involved
> in the attack must be killed. In other words, it is necessary to kill all
> the tasks that share the fork and/or exec statistical data related to the
> attack. Moreover, if the attack happens through the fork system call, the
> processes that have the same group_leader that the current task (the task
> that has crashed) must be avoided since they are in the path to be killed.
>
> When the SIGKILL signal is sent to the offending tasks, the function
> "brute_kill_offending_tasks" will be called in a recursive way from the
> task_fatal_signal LSM hook due to a small crash period. So, to avoid kill
> again the same tasks due to a recursive call of this function, it is
> necessary to disable the attack detection for the involved hierarchies.

Would it not be useful for forensic reasons to be able to send SIGABRT and get the a coredump?


> To disable the attack detection, set to zero the last crash timestamp and
> avoid to compute the application crash period in this case.
>
> Signed-off-by: John Wood <john.wood@gmx.com>
> ---
>  security/brute/brute.c | 141 ++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 132 insertions(+), 9 deletions(-)
>
> diff --git a/security/brute/brute.c b/security/brute/brute.c
> index 0a99cd4c3303..48b07d923ec7 100644
> --- a/security/brute/brute.c
> +++ b/security/brute/brute.c
> @@ -22,6 +22,7 @@
>  #include <linux/math64.h>
>  #include <linux/netdevice.h>
>  #include <linux/path.h>
> +#include <linux/pid.h>
>  #include <linux/printk.h>
>  #include <linux/refcount.h>
>  #include <linux/rwlock.h>
> @@ -64,7 +65,7 @@ struct brute_cred {
>   * @lock: Lock to protect the brute_stats structure.
>   * @refc: Reference counter.
>   * @faults: Number of crashes.
> - * @jiffies: Last crash timestamp.
> + * @jiffies: Last crash timestamp. If zero, the attack detection is disabled.
>   * @period: Crash period's moving average.
>   * @saved_cred: Saved credentials.
>   * @network: Network activity flag.
> @@ -566,6 +567,125 @@ static inline void print_fork_attack_running(void)
>  	pr_warn("Fork brute force attack detected [%s]\n", current->comm);
>  }
>
> +/**
> + * brute_disabled() - Test if the brute force attack detection is disabled.
> + * @stats: Statistical data shared by all the fork hierarchy processes.
> + *
> + * The brute force attack detection enabling/disabling is based on the last
> + * crash timestamp. A zero timestamp indicates that this feature is disabled. A
> + * timestamp greater than zero indicates that the attack detection is enabled.
> + *
> + * The statistical data shared by all the fork hierarchy processes cannot be
> + * NULL.
> + *
> + * It's mandatory to disable interrupts before acquiring the brute_stats::lock
> + * since the task_free hook can be called from an IRQ context during the
> + * execution of the task_fatal_signal hook.
> + *
> + * Context: Must be called with interrupts disabled and brute_stats_ptr_lock
> + *          held.
> + * Return: True if the brute force attack detection is disabled. False
> + *         otherwise.
> + */
> +static bool brute_disabled(struct brute_stats *stats)
> +{
> +	bool disabled;
> +
> +	spin_lock(&stats->lock);
> +	disabled = !stats->jiffies;
> +	spin_unlock(&stats->lock);
> +
> +	return disabled;
> +}
> +
> +/**
> + * brute_disable() - Disable the brute force attack detection.
> + * @stats: Statistical data shared by all the fork hierarchy processes.
> + *
> + * To disable the brute force attack detection it is only necessary to set the
> + * last crash timestamp to zero. A zero timestamp indicates that this feature is
> + * disabled. A timestamp greater than zero indicates that the attack detection
> + * is enabled.
> + *
> + * The statistical data shared by all the fork hierarchy processes cannot be
> + * NULL.
> + *
> + * Context: Must be called with interrupts disabled and brute_stats_ptr_lock
> + *          and brute_stats::lock held.
> + */
> +static inline void brute_disable(struct brute_stats *stats)
> +{
> +	stats->jiffies = 0;
> +}
> +
> +/**
> + * enum brute_attack_type - Brute force attack type.
> + * @BRUTE_ATTACK_TYPE_FORK: Attack that happens through the fork system call.
> + * @BRUTE_ATTACK_TYPE_EXEC: Attack that happens through the execve system call.
> + */
> +enum brute_attack_type {
> +	BRUTE_ATTACK_TYPE_FORK,
> +	BRUTE_ATTACK_TYPE_EXEC,
> +};
> +
> +/**
> + * brute_kill_offending_tasks() - Kill the offending tasks.
> + * @attack_type: Brute force attack type.
> + * @stats: Statistical data shared by all the fork hierarchy processes.
> + *
> + * When a brute force attack is detected all the offending tasks involved in the
> + * attack must be killed. In other words, it is necessary to kill all the tasks
> + * that share the same statistical data. Moreover, if the attack happens through
> + * the fork system call, the processes that have the same group_leader that the
> + * current task must be avoided since they are in the path to be killed.
> + *
> + * When the SIGKILL signal is sent to the offending tasks, this function will be
> + * called again from the task_fatal_signal hook due to a small crash period. So,
> + * to avoid kill again the same tasks due to a recursive call of this function,
> + * it is necessary to disable the attack detection for this fork hierarchy.
> + *
> + * The statistical data shared by all the fork hierarchy processes cannot be
> + * NULL.
> + *
> + * It's mandatory to disable interrupts before acquiring the brute_stats::lock
> + * since the task_free hook can be called from an IRQ context during the
> + * execution of the task_fatal_signal hook.
> + *
> + * Context: Must be called with interrupts disabled and tasklist_lock and
> + *          brute_stats_ptr_lock held.
> + */
> +static void brute_kill_offending_tasks(enum brute_attack_type attack_type,
> +				       struct brute_stats *stats)
> +{
> +	struct task_struct *p;
> +	struct brute_stats **p_stats;
> +
> +	spin_lock(&stats->lock);
> +
> +	if (attack_type == BRUTE_ATTACK_TYPE_FORK &&
> +	    refcount_read(&stats->refc) == 1) {
> +		spin_unlock(&stats->lock);
> +		return;
> +	}
> +
> +	brute_disable(stats);
> +	spin_unlock(&stats->lock);
> +
> +	for_each_process(p) {
> +		if (attack_type == BRUTE_ATTACK_TYPE_FORK &&
> +		    p->group_leader == current->group_leader)
> +			continue;
> +
> +		p_stats = brute_stats_ptr(p);
> +		if (*p_stats != stats)
> +			continue;
> +
> +		do_send_sig_info(SIGKILL, SEND_SIG_PRIV, p, PIDTYPE_PID);
> +		pr_warn_ratelimited("Offending process %d [%s] killed\n",
> +				    p->pid, p->comm);
> +	}
> +}
> +
>  /**
>   * brute_manage_fork_attack() - Manage a fork brute force attack.
>   * @stats: Statistical data shared by all the fork hierarchy processes.
> @@ -581,8 +701,8 @@ static inline void print_fork_attack_running(void)
>   * since the task_free hook can be called from an IRQ context during the
>   * execution of the task_fatal_signal hook.
>   *
> - * Context: Must be called with interrupts disabled and brute_stats_ptr_lock
> - *          held.
> + * Context: Must be called with interrupts disabled and tasklist_lock and
> + *          brute_stats_ptr_lock held.
>   * Return: The last crash timestamp before updating it.
>   */
>  static u64 brute_manage_fork_attack(struct brute_stats *stats, u64 now)
> @@ -590,8 +710,10 @@ static u64 brute_manage_fork_attack(struct brute_stats *stats, u64 now)
>  	u64 last_fork_crash;
>
>  	last_fork_crash = brute_update_crash_period(stats, now);
> -	if (brute_attack_running(stats))
> +	if (brute_attack_running(stats)) {
>  		print_fork_attack_running();
> +		brute_kill_offending_tasks(BRUTE_ATTACK_TYPE_FORK, stats);
> +	}
>
>  	return last_fork_crash;
>  }
> @@ -778,8 +900,10 @@ static void brute_manage_exec_attack(struct brute_stats *stats, u64 now,
>  	if (fork_period == exec_period)
>  		return;
>
> -	if (brute_attack_running(exec_stats))
> +	if (brute_attack_running(exec_stats)) {
>  		print_exec_attack_running(exec_stats);
> +		brute_kill_offending_tasks(BRUTE_ATTACK_TYPE_EXEC, exec_stats);
> +	}
>  }
>
>  /**
> @@ -895,10 +1019,9 @@ static void brute_task_fatal_signal(const kernel_siginfo_t *siginfo)
>  	read_lock(&tasklist_lock);
>  	read_lock_irqsave(&brute_stats_ptr_lock, flags);
>
> -	if (WARN(!*stats, "No statistical data\n"))
> -		goto unlock;
> -
> -	if (!brute_threat_model_supported(siginfo, *stats))
> +	if (WARN(!*stats, "No statistical data\n") ||
> +	    brute_disabled(*stats) ||
> +	    !brute_threat_model_supported(siginfo, *stats))
>  		goto unlock;
>
>  	last_fork_crash = brute_manage_fork_attack(*stats, now);
> --
> 2.25.1
>


  reply	other threads:[~2021-03-11 20:42 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-27 15:30 [PATCH v5 0/8] Fork brute force attack mitigation John Wood
2021-02-27 15:30 ` [PATCH v5 1/8] security: Add LSM hook at the point where a task gets a fatal signal John Wood
2021-02-27 15:30 ` [PATCH v5 2/8] security/brute: Define a LSM and manage statistical data John Wood
2021-03-02  5:49   ` [security/brute] cfe92ab6a3: WARNING:inconsistent_lock_state kernel test robot
2021-03-02 18:39     ` John Wood
2021-02-27 15:30 ` [PATCH v5 3/8] securtiy/brute: Detect a brute force attack John Wood
2021-02-27 15:30 ` [PATCH v5 4/8] security/brute: Fine tuning the attack detection John Wood
2021-02-27 15:30 ` [PATCH v5 5/8] security/brute: Mitigate a brute force attack John Wood
2021-03-11 20:32   ` peter enderborg [this message]
2021-03-12 16:19     ` John Wood
2021-02-27 15:30 ` [PATCH v5 6/8] selftests/brute: Add tests for the Brute LSM John Wood
2021-02-27 15:30 ` [PATCH v5 7/8] Documentation: Add documentation " John Wood
2021-02-28 18:56   ` Andi Kleen
2021-03-02 18:31     ` John Wood
2021-03-07 15:19       ` Andi Kleen
2021-03-07 16:45         ` John Wood
2021-03-07 17:25           ` Andi Kleen
2021-03-07 18:05             ` John Wood
2021-03-07 22:49               ` Andi Kleen
2021-03-09 18:40                 ` John Wood
2021-03-11 18:22                   ` John Wood
2021-03-11 20:08                     ` Andi Kleen
2021-03-12 17:47                       ` John Wood
2021-03-11 20:05                   ` Andi Kleen
2021-03-12 17:54                     ` John Wood
2021-02-27 15:30 ` [PATCH v5 8/8] MAINTAINERS: Add a new entry " John Wood
2021-02-27 18:44 ` [PATCH v5 0/8] Fork brute force attack mitigation 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=5419ebe6-bb82-9b66-052b-0eefff93e5ae@sony.com \
    --to=peter.enderborg@sony.com \
    --cc=corbet@lwn.net \
    --cc=gregkh@linuxfoundation.org \
    --cc=jannh@google.com \
    --cc=jmorris@namei.org \
    --cc=john.wood@gmx.com \
    --cc=keescook@chromium.org \
    --cc=kernel-hardening@lists.openwall.com \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=rdunlap@infradead.org \
    --cc=serge@hallyn.com \
    --cc=shuah@kernel.org \
    --subject='Re: [PATCH v5 5/8] security/brute: Mitigate a 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 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).