mbox series

[v7,0/7] Fork brute force attack mitigation

Message ID 20210521172414.69456-1-john.wood@gmx.com (mailing list archive)
Headers show
Series Fork brute force attack mitigation | expand

Message

John Wood May 21, 2021, 5:24 p.m. UTC
Attacks against vulnerable userspace applications with the purpose to break
ASLR or bypass canaries traditionally use some level of brute force with
the help of the fork system call. This is possible since when creating a
new process using fork its memory contents are the same as those of the
parent process (the process that called the fork system call). So, the
attacker can test the memory infinite times to find the correct memory
values or the correct memory addresses without worrying about crashing the
application.

Based on the above scenario it would be nice to have this detected and
mitigated, and this is the goal of this patch serie. Specifically the
following attacks are expected to be detected:

1.- Launching (fork()/exec()) a setuid/setgid process repeatedly until a
    desirable memory layout is got (e.g. Stack Clash).
2.- Connecting to an exec()ing network daemon (e.g. xinetd) repeatedly
    until a desirable memory layout is got (e.g. what CTFs do for simple
    network service).
3.- Launching processes without exec() (e.g. Android Zygote) and exposing
    state to attack a sibling.
4.- Connecting to a fork()ing network daemon (e.g. apache) repeatedly until
    the previously shared memory layout of all the other children is
    exposed (e.g. kind of related to HeartBleed).

In each case, a privilege boundary has been crossed:

Case 1: setuid/setgid process
Case 2: network to local
Case 3: privilege changes
Case 4: network to local

So, what will really be detected are fork/exec brute force attacks that
cross any of the commented bounds.

The implementation details and comparison against other existing
implementations can be found in the "Documentation" patch.

It is important to mention that this version has changed the method used to
track the information related to the application crashes. Prior this
version, a pointer per process (in the task_struct structure) held a
reference to the shared statistical data. Or in other words, these stats
were shared by all the fork hierarchy processes. But this has an important
drawback: a brute force attack that happens through the execve system call
losts the faults info since these statistics are freed when the fork
hierarchy disappears. So, the solution adopted in the v6 version was to use
an upper fork hierarchy to track the info for this attack type. But, as
Valdis Kletnieks pointed out during this discussion [1], this method can
be easily bypassed using a double exec (well, this was the method used in
the kselftest to avoid the detection ;) ). So, in this version, to track
all the statistical data (info related with application crashes), the
extended attributes feature for the executable files are used. The xattr is
also used to mark the executables as "not allowed" when an attack is
detected. Then, the execve system call rely on this flag to avoid following
executions of this file.

[1] https://lore.kernel.org/kernelnewbies/20210330173459.GA3163@ubuntu/

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

[2] https://lore.kernel.org/kernel-hardening/878s78dnrm.fsf@linux.intel.com/

Knowing all this information I will explain now the different patches:

The 1/7 patch defines a new LSM hook to get the fatal signal of a task.
This will be useful during the attack detection phase.

The 2/7 patch defines a new LSM and the necessary sysctl attributes to fine
tuning the attack detection.

The 3/7 patch detects a fork/exec brute force attack and narrows the
possible cases taken into account the privilege boundary crossing.

The 4/7 patch mitigates a brute force attack.

The 5/7 patch adds self-tests to validate the Brute LSM expectations.

The 6/7 patch adds the documentation to explain this implementation.

The 7/7 patch updates the maintainers file.

This patch serie is a task of the KSPP [3] and can also be accessed from my
github tree [4] in the "brute_v7" branch.

[3] https://github.com/KSPP/linux/issues/39
[4] https://github.com/johwood/linux/

When I ran the "checkpatch" script I got the following errors, but I think
they are false positives as I follow the same coding style for the others
extended attributes suffixes.

----------------------------------------------------------------------------
../patches/brute_v7/v7-0003-security-brute-Detect-a-brute-force-attack.patch
----------------------------------------------------------------------------
ERROR: Macros with complex values should be enclosed in parentheses
89: FILE: include/uapi/linux/xattr.h:80:
+#define XATTR_NAME_BRUTE XATTR_SECURITY_PREFIX XATTR_BRUTE_SUFFIX

-----------------------------------------------------------------------------
../patches/brute_v7/v7-0005-selftests-brute-Add-tests-for-the-Brute-LSM.patch
-----------------------------------------------------------------------------
ERROR: Macros with complex values should be enclosed in parentheses
100: FILE: tools/testing/selftests/brute/rmxattr.c:18:
+#define XATTR_NAME_BRUTE XATTR_SECURITY_PREFIX XATTR_BRUTE_SUFFIX

When I ran the "kernel-doc" script with the following parameters:

./scripts/kernel-doc --none -v security/brute/brute.c

I got the following warning:

security/brute/brute.c:65: warning: contents before sections

But I don't understand why it is complaining. Could it be a false positive?

The previous versions can be found in:

RFC
https://lore.kernel.org/kernel-hardening/20200910202107.3799376-1-keescook@chromium.org/

Version 2
https://lore.kernel.org/kernel-hardening/20201025134540.3770-1-john.wood@gmx.com/

Version 3
https://lore.kernel.org/lkml/20210221154919.68050-1-john.wood@gmx.com/

Version 4
https://lore.kernel.org/lkml/20210227150956.6022-1-john.wood@gmx.com/

Version 5
https://lore.kernel.org/kernel-hardening/20210227153013.6747-1-john.wood@gmx.com/

Version 6
https://lore.kernel.org/kernel-hardening/20210307113031.11671-1-john.wood@gmx.com/

Changelog RFC -> v2
-------------------
- Rename this feature with a more suitable name (Jann Horn, Kees Cook).
- Convert the code to an LSM (Kees Cook).
- Add locking  to avoid data races (Jann Horn).
- Add a new LSM hook to get the fatal signal of a task (Jann Horn, Kees
  Cook).
- Add the last crashes timestamps list to avoid false positives in the
  attack detection (Jann Horn).
- Use "period" instead of "rate" (Jann Horn).
- Other minor changes suggested (Jann Horn, Kees Cook).

Changelog v2 -> v3
------------------
- Compute the application crash period on an on-going basis (Kees Cook).
- Detect a brute force attack through the execve system call (Kees Cook).
- Detect an slow brute force attack (Randy Dunlap).
- Fine tuning the detection taken into account privilege boundary crossing
  (Kees Cook).
- Taken into account only fatal signals delivered by the kernel (Kees
  Cook).
- Remove the sysctl attributes to fine tuning the detection (Kees Cook).
- Remove the prctls to allow per process enabling/disabling (Kees Cook).
- Improve the documentation (Kees Cook).
- Fix some typos in the documentation (Randy Dunlap).
- Add self-test to validate the expectations (Kees Cook).

Changelog v3 -> v4
------------------
- Fix all the warnings shown by the tool "scripts/kernel-doc" (Randy
  Dunlap).

Changelog v4 -> v5
------------------
- Fix some typos (Randy Dunlap).

Changelog v5 -> v6
------------------
- Fix a reported deadlock (kernel test robot).
- Add high level details to the documentation (Andi Kleen).

Changelog v6 -> v7
------------------

- Add the "Reviewed-by:" tag to the first patch.
- Rearrange the brute LSM between lockdown and yama (Kees Cook).
- Split subdir and obj in security/Makefile (Kees Cook).
- Reduce the number of header files included (Kees Cook).
- Print the pid when an attack is detected (Kees Cook).
- Use the socket_accept LSM hook instead of socket_sock_rcv_skb hook to
  avoid running a hook on every incoming network packet (Kees Cook).
- Update the documentation and fix it to render it properly (Jonathan
  Corbet).
- Manage correctly an exec brute force attack avoiding the bypass (Valdis
  Kletnieks).
- Other minor changes and cleanups.

Any constructive comments are welcome.
Thanks in advance.

John Wood (7):
  security: Add LSM hook at the point where a task gets a fatal signal
  security/brute: Define a LSM and add sysctl attributes
  security/brute: Detect a brute force attack
  security/brute: Mitigate a brute force attack
  selftests/brute: Add tests for the Brute LSM
  Documentation: Add documentation for the Brute LSM
  MAINTAINERS: Add a new entry for the Brute LSM

 Documentation/admin-guide/LSM/Brute.rst  | 334 +++++++++++
 Documentation/admin-guide/LSM/index.rst  |   1 +
 MAINTAINERS                              |   7 +
 include/linux/lsm_hook_defs.h            |   1 +
 include/linux/lsm_hooks.h                |   4 +
 include/linux/security.h                 |   4 +
 include/uapi/linux/xattr.h               |   3 +
 kernel/signal.c                          |   1 +
 security/Kconfig                         |  11 +-
 security/Makefile                        |   2 +
 security/brute/Kconfig                   |  15 +
 security/brute/Makefile                  |   2 +
 security/brute/brute.c                   | 716 +++++++++++++++++++++++
 security/security.c                      |   5 +
 tools/testing/selftests/Makefile         |   1 +
 tools/testing/selftests/brute/.gitignore |   2 +
 tools/testing/selftests/brute/Makefile   |   5 +
 tools/testing/selftests/brute/config     |   1 +
 tools/testing/selftests/brute/rmxattr.c  |  34 ++
 tools/testing/selftests/brute/test.c     | 507 ++++++++++++++++
 tools/testing/selftests/brute/test.sh    | 256 ++++++++
 21 files changed, 1907 insertions(+), 5 deletions(-)
 create mode 100644 Documentation/admin-guide/LSM/Brute.rst
 create mode 100644 security/brute/Kconfig
 create mode 100644 security/brute/Makefile
 create mode 100644 security/brute/brute.c
 create mode 100644 tools/testing/selftests/brute/.gitignore
 create mode 100644 tools/testing/selftests/brute/Makefile
 create mode 100644 tools/testing/selftests/brute/config
 create mode 100644 tools/testing/selftests/brute/rmxattr.c
 create mode 100644 tools/testing/selftests/brute/test.c
 create mode 100755 tools/testing/selftests/brute/test.sh

--
2.25.1

Comments

Andi Kleen May 21, 2021, 6:02 p.m. UTC | #1
> 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.

-Andi


>
> [2] https://lore.kernel.org/kernel-hardening/878s78dnrm.fsf@linux.intel.com/
>
> Knowing all this information I will explain now the different patches:
>
> The 1/7 patch defines a new LSM hook to get the fatal signal of a task.
> This will be useful during the attack detection phase.
>
> The 2/7 patch defines a new LSM and the necessary sysctl attributes to fine
> tuning the attack detection.
>
> The 3/7 patch detects a fork/exec brute force attack and narrows the
> possible cases taken into account the privilege boundary crossing.
>
> The 4/7 patch mitigates a brute force attack.
>
> The 5/7 patch adds self-tests to validate the Brute LSM expectations.
>
> The 6/7 patch adds the documentation to explain this implementation.
>
> The 7/7 patch updates the maintainers file.
>
> This patch serie is a task of the KSPP [3] and can also be accessed from my
> github tree [4] in the "brute_v7" branch.
>
> [3] https://github.com/KSPP/linux/issues/39
> [4] https://github.com/johwood/linux/
>
> When I ran the "checkpatch" script I got the following errors, but I think
> they are false positives as I follow the same coding style for the others
> extended attributes suffixes.
>
> ----------------------------------------------------------------------------
> ../patches/brute_v7/v7-0003-security-brute-Detect-a-brute-force-attack.patch
> ----------------------------------------------------------------------------
> ERROR: Macros with complex values should be enclosed in parentheses
> 89: FILE: include/uapi/linux/xattr.h:80:
> +#define XATTR_NAME_BRUTE XATTR_SECURITY_PREFIX XATTR_BRUTE_SUFFIX
>
> -----------------------------------------------------------------------------
> ../patches/brute_v7/v7-0005-selftests-brute-Add-tests-for-the-Brute-LSM.patch
> -----------------------------------------------------------------------------
> ERROR: Macros with complex values should be enclosed in parentheses
> 100: FILE: tools/testing/selftests/brute/rmxattr.c:18:
> +#define XATTR_NAME_BRUTE XATTR_SECURITY_PREFIX XATTR_BRUTE_SUFFIX
>
> When I ran the "kernel-doc" script with the following parameters:
>
> ./scripts/kernel-doc --none -v security/brute/brute.c
>
> I got the following warning:
>
> security/brute/brute.c:65: warning: contents before sections
>
> But I don't understand why it is complaining. Could it be a false positive?
>
> The previous versions can be found in:
>
> RFC
> https://lore.kernel.org/kernel-hardening/20200910202107.3799376-1-keescook@chromium.org/
>
> Version 2
> https://lore.kernel.org/kernel-hardening/20201025134540.3770-1-john.wood@gmx.com/
>
> Version 3
> https://lore.kernel.org/lkml/20210221154919.68050-1-john.wood@gmx.com/
>
> Version 4
> https://lore.kernel.org/lkml/20210227150956.6022-1-john.wood@gmx.com/
>
> Version 5
> https://lore.kernel.org/kernel-hardening/20210227153013.6747-1-john.wood@gmx.com/
>
> Version 6
> https://lore.kernel.org/kernel-hardening/20210307113031.11671-1-john.wood@gmx.com/
>
> Changelog RFC -> v2
> -------------------
> - Rename this feature with a more suitable name (Jann Horn, Kees Cook).
> - Convert the code to an LSM (Kees Cook).
> - Add locking  to avoid data races (Jann Horn).
> - Add a new LSM hook to get the fatal signal of a task (Jann Horn, Kees
>    Cook).
> - Add the last crashes timestamps list to avoid false positives in the
>    attack detection (Jann Horn).
> - Use "period" instead of "rate" (Jann Horn).
> - Other minor changes suggested (Jann Horn, Kees Cook).
>
> Changelog v2 -> v3
> ------------------
> - Compute the application crash period on an on-going basis (Kees Cook).
> - Detect a brute force attack through the execve system call (Kees Cook).
> - Detect an slow brute force attack (Randy Dunlap).
> - Fine tuning the detection taken into account privilege boundary crossing
>    (Kees Cook).
> - Taken into account only fatal signals delivered by the kernel (Kees
>    Cook).
> - Remove the sysctl attributes to fine tuning the detection (Kees Cook).
> - Remove the prctls to allow per process enabling/disabling (Kees Cook).
> - Improve the documentation (Kees Cook).
> - Fix some typos in the documentation (Randy Dunlap).
> - Add self-test to validate the expectations (Kees Cook).
>
> Changelog v3 -> v4
> ------------------
> - Fix all the warnings shown by the tool "scripts/kernel-doc" (Randy
>    Dunlap).
>
> Changelog v4 -> v5
> ------------------
> - Fix some typos (Randy Dunlap).
>
> Changelog v5 -> v6
> ------------------
> - Fix a reported deadlock (kernel test robot).
> - Add high level details to the documentation (Andi Kleen).
>
> Changelog v6 -> v7
> ------------------
>
> - Add the "Reviewed-by:" tag to the first patch.
> - Rearrange the brute LSM between lockdown and yama (Kees Cook).
> - Split subdir and obj in security/Makefile (Kees Cook).
> - Reduce the number of header files included (Kees Cook).
> - Print the pid when an attack is detected (Kees Cook).
> - Use the socket_accept LSM hook instead of socket_sock_rcv_skb hook to
>    avoid running a hook on every incoming network packet (Kees Cook).
> - Update the documentation and fix it to render it properly (Jonathan
>    Corbet).
> - Manage correctly an exec brute force attack avoiding the bypass (Valdis
>    Kletnieks).
> - Other minor changes and cleanups.
>
> Any constructive comments are welcome.
> Thanks in advance.
>
> John Wood (7):
>    security: Add LSM hook at the point where a task gets a fatal signal
>    security/brute: Define a LSM and add sysctl attributes
>    security/brute: Detect a brute force attack
>    security/brute: Mitigate a brute force attack
>    selftests/brute: Add tests for the Brute LSM
>    Documentation: Add documentation for the Brute LSM
>    MAINTAINERS: Add a new entry for the Brute LSM
>
>   Documentation/admin-guide/LSM/Brute.rst  | 334 +++++++++++
>   Documentation/admin-guide/LSM/index.rst  |   1 +
>   MAINTAINERS                              |   7 +
>   include/linux/lsm_hook_defs.h            |   1 +
>   include/linux/lsm_hooks.h                |   4 +
>   include/linux/security.h                 |   4 +
>   include/uapi/linux/xattr.h               |   3 +
>   kernel/signal.c                          |   1 +
>   security/Kconfig                         |  11 +-
>   security/Makefile                        |   2 +
>   security/brute/Kconfig                   |  15 +
>   security/brute/Makefile                  |   2 +
>   security/brute/brute.c                   | 716 +++++++++++++++++++++++
>   security/security.c                      |   5 +
>   tools/testing/selftests/Makefile         |   1 +
>   tools/testing/selftests/brute/.gitignore |   2 +
>   tools/testing/selftests/brute/Makefile   |   5 +
>   tools/testing/selftests/brute/config     |   1 +
>   tools/testing/selftests/brute/rmxattr.c  |  34 ++
>   tools/testing/selftests/brute/test.c     | 507 ++++++++++++++++
>   tools/testing/selftests/brute/test.sh    | 256 ++++++++
>   21 files changed, 1907 insertions(+), 5 deletions(-)
>   create mode 100644 Documentation/admin-guide/LSM/Brute.rst
>   create mode 100644 security/brute/Kconfig
>   create mode 100644 security/brute/Makefile
>   create mode 100644 security/brute/brute.c
>   create mode 100644 tools/testing/selftests/brute/.gitignore
>   create mode 100644 tools/testing/selftests/brute/Makefile
>   create mode 100644 tools/testing/selftests/brute/config
>   create mode 100644 tools/testing/selftests/brute/rmxattr.c
>   create mode 100644 tools/testing/selftests/brute/test.c
>   create mode 100755 tools/testing/selftests/brute/test.sh
>
> --
> 2.25.1
>
John Wood May 23, 2021, 7:31 a.m. UTC | #2
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
Andi Kleen May 23, 2021, 2:43 p.m. UTC | #3
On 5/23/2021 12:31 AM, John Wood wrote:
> 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)

This works even when someone further down the chain died? Assuming it 
does, for SIGCHLD it seems reasonable.

I'm not fully sure how it will interact with cgroup release tracking 
though, that might need more research (my understanding is that modern 
supervisors often use cgroups)

-Andi
John Wood May 23, 2021, 3:47 p.m. UTC | #4
On Sun, May 23, 2021 at 07:43:16AM -0700, Andi Kleen wrote:
>
> On 5/23/2021 12:31 AM, John Wood wrote:
> > 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)
>
> This works even when someone further down the chain died?

Yes, this is the idea. (but now is a work in progress :) )

> Assuming it does, for SIGCHLD it seems reasonable.

So, if there are no objections I will work on it for the next version.

>
> I'm not fully sure how it will interact with cgroup release tracking though,
> that might need more research (my understanding is that modern supervisors
> often use cgroups)

Yeah, a new topic to learn: cgroups. I will try to work on this too if there are
no objections.

Thanks for the feedback.
John Wood