linux-hardening - Re: [PATCH v7 0/7] Fork brute force attack mitigation

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20210523073124.GA3762@ubuntu>
Date:   Sun, 23 May 2021 09:31:24 +0200
From:   John Wood <john.wood@....com>
To:     Andi Kleen <ak@...ux.intel.com>
Cc:     John Wood <john.wood@....com>, Kees Cook <keescook@...omium.org>,
        Jann Horn <jannh@...gle.com>, Jonathan Corbet <corbet@....net>,
        James Morris <jmorris@...ei.org>,
        "Serge E. Hallyn" <serge@...lyn.com>,
        Shuah Khan <shuah@...nel.org>, valdis.kletnieks@...edu,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Randy Dunlap <rdunlap@...radead.org>,
        linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-security-module@...r.kernel.org,
        linux-kselftest@...r.kernel.org, linux-hardening@...r.kernel.org,
        kernel-hardening@...ts.openwall.com
Subject: Re: [PATCH v7 0/7] Fork brute force attack mitigation

Hi,

On Fri, May 21, 2021 at 11:02:14AM -0700, Andi Kleen wrote:
>
> > Moreover, I think this solves another problem pointed out by Andi Kleen
> > during the v5 review [2] related to the possibility that a supervisor
> > respawns processes killed by the Brute LSM. He suggested adding some way so
> > a supervisor can know that a process has been killed by Brute and then
> > decide to respawn or not. So, now, the supervisor can read the brute xattr
> > of one executable and know if it is blocked by Brute and why (using the
> > statistical data).
>
> It looks better now, Thank.
>
> One potential problem is that the supervisor might see the executable
> directly, but run it through some wrapper. In fact I suspect that will be
> fairly common with complex daemons. So it couldn't directly look at the
> xattr. Might be useful to also pass this information through the wait*
> chain, so that the supervisor can directly collect it. That would need some
> extension to these system calls.
>
Could something like this help? (not tested)

diff --git a/arch/x86/kernel/signal_compat.c b/arch/x86/kernel/signal_compat.c
index 0e5d0a7e203b..409c9c4c40c0 100644
--- a/arch/x86/kernel/signal_compat.c
+++ b/arch/x86/kernel/signal_compat.c
@@ -30,7 +30,7 @@ static inline void signal_compat_build_tests(void)
 	BUILD_BUG_ON(NSIGSEGV != 9);
 	BUILD_BUG_ON(NSIGBUS  != 5);
 	BUILD_BUG_ON(NSIGTRAP != 6);
-	BUILD_BUG_ON(NSIGCHLD != 6);
+	BUILD_BUG_ON(NSIGCHLD != 7);
 	BUILD_BUG_ON(NSIGSYS  != 2);

 	/* This is part of the ABI and can never change in size: */
diff --git a/include/brute/brute.h b/include/brute/brute.h
new file mode 100644
index 000000000000..1569bd495f94
--- /dev/null
+++ b/include/brute/brute.h
@@ -0,0 +1,13 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _BRUTE_H_
+#define _BRUTE_H_
+
+#include <linux/sched.h>
+
+#ifdef CONFIG_SECURITY_FORK_BRUTE
+bool brute_task_killed(struct task_struct *task);
+#else
+static inline bool brute_task_killed(struct task_struct *task) { return false; }
+#endif
+
+#endif /* _BRUTE_H_ */
diff --git a/include/uapi/asm-generic/siginfo.h b/include/uapi/asm-generic/siginfo.h
index 03d6f6d2c1fe..488abfdc7b0d 100644
--- a/include/uapi/asm-generic/siginfo.h
+++ b/include/uapi/asm-generic/siginfo.h
@@ -273,7 +273,8 @@ typedef struct siginfo {
 #define CLD_TRAPPED	4	/* traced child has trapped */
 #define CLD_STOPPED	5	/* child has stopped */
 #define CLD_CONTINUED	6	/* stopped child has continued */
-#define NSIGCHLD	6
+#define CLD_BRUTE	7	/* child was killed by brute LSM */
+#define NSIGCHLD	7

 /*
  * SIGPOLL (or any other signal without signal specific si_codes) si_codes
diff --git a/kernel/exit.c b/kernel/exit.c
index fd1c04193e18..69bcbd00d277 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -69,6 +69,8 @@
 #include <asm/unistd.h>
 #include <asm/mmu_context.h>

+#include <brute/brute.h>
+
 static void __unhash_process(struct task_struct *p, bool group_dead)
 {
 	nr_threads--;
@@ -1001,6 +1003,7 @@ static int wait_task_zombie(struct wait_opts *wo, struct task_struct *p)
 	pid_t pid = task_pid_vnr(p);
 	uid_t uid = from_kuid_munged(current_user_ns(), task_uid(p));
 	struct waitid_info *infop;
+	bool killed_by_brute = brute_task_killed(p);

 	if (!likely(wo->wo_flags & WEXITED))
 		return 0;
@@ -1114,7 +1117,8 @@ static int wait_task_zombie(struct wait_opts *wo, struct task_struct *p)
 			infop->cause = CLD_EXITED;
 			infop->status = status >> 8;
 		} else {
-			infop->cause = (status & 0x80) ? CLD_DUMPED : CLD_KILLED;
+			infop->cause = (status & 0x80) ? CLD_DUMPED :
+				killed_by_brute ? CLD_BRUTE : CLD_KILLED;
 			infop->status = status & 0x7f;
 		}
 		infop->pid = pid;
diff --git a/kernel/signal.c b/kernel/signal.c
index 62625ad98b14..f6c062b19563 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -55,6 +55,8 @@
 #include <asm/siginfo.h>
 #include <asm/cacheflush.h>

+#include <brute/brute.h>
+
 /*
  * SLAB caches for signal bits.
  */
@@ -1996,7 +1998,7 @@ bool do_notify_parent(struct task_struct *tsk, int sig)
 	if (tsk->exit_code & 0x80)
 		info.si_code = CLD_DUMPED;
 	else if (tsk->exit_code & 0x7f)
-		info.si_code = CLD_KILLED;
+		info.si_code = brute_task_killed(tsk) ? CLD_BRUTE : CLD_KILLED;
 	else {
 		info.si_code = CLD_EXITED;
 		info.si_status = tsk->exit_code >> 8;

Thanks,
John Wood