2021-05-21 20:26:27

by John Wood

[permalink] [raw]
Subject: [PATCH v7 0/7] Fork brute force attack mitigation

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/[email protected]/

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/[email protected]/

Version 2
https://lore.kernel.org/kernel-hardening/[email protected]/

Version 3
https://lore.kernel.org/lkml/[email protected]/

Version 4
https://lore.kernel.org/lkml/[email protected]/

Version 5
https://lore.kernel.org/kernel-hardening/[email protected]/

Version 6
https://lore.kernel.org/kernel-hardening/[email protected]/

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


2021-05-21 20:26:35

by John Wood

[permalink] [raw]
Subject: [PATCH v7 4/7] security/brute: Mitigate a brute force attack

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 are executing the same file that is running during
the brute force attack.

Also, to prevent the executable involved in the attack from being
respawned by a supervisor, and thus prevent a brute force attack from
being started again, test the "not_allowed" flag and avoid the file
execution based on this.

Signed-off-by: John Wood <[email protected]>
---
security/brute/brute.c | 113 +++++++++++++++++++++++++++++++++++++----
1 file changed, 102 insertions(+), 11 deletions(-)

diff --git a/security/brute/brute.c b/security/brute/brute.c
index 66db0dd0664d..7712dac888db 100644
--- a/security/brute/brute.c
+++ b/security/brute/brute.c
@@ -233,6 +233,88 @@ static inline void brute_print_attack_running(void)
current->comm);
}

+/**
+ * brute_print_file_not_allowed() - Warn about a file not allowed.
+ * @dentry: The dentry of the file not allowed.
+ */
+static void brute_print_file_not_allowed(struct dentry *dentry)
+{
+ char *buf, *path;
+
+ buf = __getname();
+ if (WARN_ON_ONCE(!buf))
+ return;
+
+ path = dentry_path_raw(dentry, buf, PATH_MAX);
+ if (WARN_ON_ONCE(IS_ERR(path)))
+ goto free;
+
+ pr_warn_ratelimited("%s not allowed\n", path);
+free:
+ __putname(buf);
+}
+
+/**
+ * brute_is_same_file() - Test if two files are the same.
+ * @file1: First file to compare. Cannot be NULL.
+ * @file2: Second file to compare. Cannot be NULL.
+ *
+ * Two files are the same if they have the same inode number and the same block
+ * device.
+ *
+ * Return: True if the two files are the same. False otherwise.
+ */
+static inline bool brute_is_same_file(const struct file *file1,
+ const struct file *file2)
+{
+ struct inode *inode1 = file_inode(file1);
+ struct inode *inode2 = file_inode(file2);
+
+ return inode1->i_ino == inode2->i_ino &&
+ inode1->i_sb->s_dev == inode2->i_sb->s_dev;
+}
+
+/**
+ * brute_kill_offending_tasks() - Kill the offending tasks.
+ * @file: The file executed during a brute force attack. Cannot be NULL.
+ *
+ * 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 are executing the same file that is running during the brute force
+ * attack. Moreover, the processes that have the same group_leader that the
+ * current task must be avoided since they are in the path to be killed.
+ *
+ * The for_each_process loop is protected by the tasklist_lock acquired in read
+ * mode instead of rcu_read_lock to avoid that the newly created processes
+ * escape this RCU read lock.
+ */
+static void brute_kill_offending_tasks(const struct file *file)
+{
+ struct task_struct *task;
+ struct file *exe_file;
+ bool is_same_file;
+
+ read_lock(&tasklist_lock);
+ for_each_process(task) {
+ if (task->group_leader == current->group_leader)
+ continue;
+
+ exe_file = get_task_exe_file(task);
+ if (!exe_file)
+ continue;
+
+ is_same_file = brute_is_same_file(exe_file, file);
+ fput(exe_file);
+ if (!is_same_file)
+ continue;
+
+ do_send_sig_info(SIGKILL, SEND_SIG_PRIV, task, PIDTYPE_PID);
+ pr_warn_ratelimited("Offending process %d [%s] killed\n",
+ task->pid, task->comm);
+ }
+ read_unlock(&tasklist_lock);
+}
+
/**
* brute_get_xattr_stats() - Get the stats from an extended attribute.
* @dentry: The dentry of the file to get the extended attribute.
@@ -295,6 +377,10 @@ static int brute_set_xattr_stats(struct dentry *dentry, struct inode *inode,
* created. This way, the scenario where an application has not crossed any
* privilege boundary is avoided since the existence of the extended attribute
* denotes the crossing of bounds.
+ *
+ * Also, do not update the statistics if the execution of the file is not
+ * allowed and kill all the offending tasks when a brute force attack is
+ * detected.
*/
static void brute_update_xattr_stats(const struct file *file)
{
@@ -306,7 +392,7 @@ static void brute_update_xattr_stats(const struct file *file)
inode_lock(inode);
rc = brute_get_xattr_stats(dentry, inode, &stats);
WARN_ON_ONCE(rc && rc != -ENODATA);
- if (rc) {
+ if (rc || (!rc && stats.not_allowed)) {
inode_unlock(inode);
return;
}
@@ -320,6 +406,9 @@ static void brute_update_xattr_stats(const struct file *file)
rc = brute_set_xattr_stats(dentry, inode, &stats);
WARN_ON_ONCE(rc);
inode_unlock(inode);
+
+ if (stats.not_allowed)
+ brute_kill_offending_tasks(file);
}

/**
@@ -433,21 +522,17 @@ static void brute_task_fatal_signal(const kernel_siginfo_t *siginfo)
* @bprm: Contains the linux_binprm structure.
* @file: Binary that will be executed without an interpreter.
*
- * This hook is useful to mark that a privilege boundary (setuid/setgid process)
- * has been crossed. This is done based on the "secureexec" flag.
+ * If there are statistics, test the "not_allowed" flag and avoid the file
+ * execution based on this. Also, this hook is useful to mark that a privilege
+ * boundary (setuid/setgid process) has been crossed. This is done based on the
+ * "secureexec" flag.
*
* To be defensive return an error code if it is not possible to get or set the
* stats using an extended attribute since this blocks the execution of the
* file. This scenario is treated as an attack.
*
- * It is important to note that here the brute_new_xattr_stats function could be
- * used with a previous test of the secureexec flag. However it is better to use
- * the basic xattr functions since in a future commit a test if the execution is
- * allowed (via the brute_stats::not_allowed flag) will be necessary. This way,
- * the stats of the file will be get only once.
- *
- * Return: An error code if it is not possible to get or set the statistical
- * data. Zero otherwise.
+ * Return: -EPERM if the execution of the file is not allowed. An error code if
+ * it is not possible to get or set the statistical data. Zero otherwise.
*/
static int brute_task_execve(struct linux_binprm *bprm, struct file *file)
{
@@ -461,6 +546,12 @@ static int brute_task_execve(struct linux_binprm *bprm, struct file *file)
if (WARN_ON_ONCE(rc && rc != -ENODATA))
goto unlock;

+ if (!rc && stats.not_allowed) {
+ brute_print_file_not_allowed(dentry);
+ rc = -EPERM;
+ goto unlock;
+ }
+
if (rc == -ENODATA && bprm->secureexec) {
brute_reset_stats(&stats);
rc = brute_set_xattr_stats(dentry, inode, &stats);
--
2.25.1

2021-05-21 20:40:51

by Andi Kleen

[permalink] [raw]
Subject: Re: [PATCH v7 0/7] Fork brute force attack mitigation


> 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/[email protected]/
>
> 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/[email protected]/
>
> Version 2
> https://lore.kernel.org/kernel-hardening/[email protected]/
>
> Version 3
> https://lore.kernel.org/lkml/[email protected]/
>
> Version 4
> https://lore.kernel.org/lkml/[email protected]/
>
> Version 5
> https://lore.kernel.org/kernel-hardening/[email protected]/
>
> Version 6
> https://lore.kernel.org/kernel-hardening/[email protected]/
>
> 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
>

2021-05-22 06:39:15

by John Wood

[permalink] [raw]
Subject: [PATCH v7 5/7] selftests/brute: Add tests for the Brute LSM

Add tests to check the brute LSM functionality and cover fork/exec brute
force attacks crossing the following privilege boundaries:

1.- setuid process
2.- privilege changes
3.- network to local

Also, as a first step check that fork/exec brute force attacks without
crossing any privilege boundary already commented doesn't trigger the
detection and mitigation stage.

Once a brute force attack is detected, the "test" executable is marked
as "not allowed". To start again a new test, use the "rmxattr" app to
revert this state. This way, all the tests can be run using the same
binary.

Signed-off-by: John Wood <[email protected]>
---
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 ++++++++++++
7 files changed, 806 insertions(+)
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

diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index bc3299a20338..5c413a010849 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -2,6 +2,7 @@
TARGETS = arm64
TARGETS += bpf
TARGETS += breakpoints
+TARGETS += brute
TARGETS += capabilities
TARGETS += cgroup
TARGETS += clone3
diff --git a/tools/testing/selftests/brute/.gitignore b/tools/testing/selftests/brute/.gitignore
new file mode 100644
index 000000000000..989894615766
--- /dev/null
+++ b/tools/testing/selftests/brute/.gitignore
@@ -0,0 +1,2 @@
+rmxattr
+test
diff --git a/tools/testing/selftests/brute/Makefile b/tools/testing/selftests/brute/Makefile
new file mode 100644
index 000000000000..c675d1df62ca
--- /dev/null
+++ b/tools/testing/selftests/brute/Makefile
@@ -0,0 +1,5 @@
+# SPDX-License-Identifier: GPL-2.0
+CFLAGS += -Wall -O2
+TEST_PROGS := test.sh
+TEST_GEN_FILES := rmxattr test
+include ../lib.mk
diff --git a/tools/testing/selftests/brute/config b/tools/testing/selftests/brute/config
new file mode 100644
index 000000000000..3587b7bf6c23
--- /dev/null
+++ b/tools/testing/selftests/brute/config
@@ -0,0 +1 @@
+CONFIG_SECURITY_FORK_BRUTE=y
diff --git a/tools/testing/selftests/brute/rmxattr.c b/tools/testing/selftests/brute/rmxattr.c
new file mode 100644
index 000000000000..9ed90409d337
--- /dev/null
+++ b/tools/testing/selftests/brute/rmxattr.c
@@ -0,0 +1,34 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <libgen.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <sys/xattr.h>
+
+static __attribute__((noreturn)) void error_failure(const char *message)
+{
+ perror(message);
+ exit(EXIT_FAILURE);
+}
+
+#define PROG_NAME basename(argv[0])
+
+#define XATTR_SECURITY_PREFIX "security."
+#define XATTR_BRUTE_SUFFIX "brute"
+#define XATTR_NAME_BRUTE XATTR_SECURITY_PREFIX XATTR_BRUTE_SUFFIX
+
+int main(int argc, char **argv)
+{
+ int rc;
+
+ if (argc < 2) {
+ printf("Usage: %s <FILE>\n", PROG_NAME);
+ exit(EXIT_FAILURE);
+ }
+
+ rc = removexattr(argv[1], XATTR_NAME_BRUTE);
+ if (rc)
+ error_failure("removexattr");
+
+ return EXIT_SUCCESS;
+}
diff --git a/tools/testing/selftests/brute/test.c b/tools/testing/selftests/brute/test.c
new file mode 100644
index 000000000000..44c32f446dca
--- /dev/null
+++ b/tools/testing/selftests/brute/test.c
@@ -0,0 +1,507 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <arpa/inet.h>
+#include <errno.h>
+#include <libgen.h>
+#include <pwd.h>
+#include <signal.h>
+#include <stdbool.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/socket.h>
+#include <sys/time.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+#include <unistd.h>
+
+static const char *message = "message";
+
+enum mode {
+ MODE_NONE,
+ MODE_CRASH,
+ MODE_SERVER_CRASH,
+ MODE_CLIENT,
+};
+
+enum crash_after {
+ CRASH_AFTER_NONE,
+ CRASH_AFTER_FORK,
+ CRASH_AFTER_EXEC,
+};
+
+enum signal_from {
+ SIGNAL_FROM_NONE,
+ SIGNAL_FROM_USER,
+ SIGNAL_FROM_KERNEL,
+};
+
+struct args {
+ uint32_t ip;
+ uint16_t port;
+ int counter;
+ long timeout;
+ enum mode mode;
+ enum crash_after crash_after;
+ enum signal_from signal_from;
+ unsigned char has_counter : 1;
+ unsigned char has_change_priv : 1;
+ unsigned char has_ip : 1;
+ unsigned char has_port : 1;
+ unsigned char has_timeout : 1;
+};
+
+#define OPT_STRING "hm:c:s:n:Ca:p:t:"
+
+static void usage(const char *prog)
+{
+ printf("Usage: %s <OPTIONS>\n", prog);
+ printf("OPTIONS:\n");
+ printf(" -h: Show this help and exit. Optional.\n");
+ printf(" -m (crash | server_crash | client): Mode. Required.\n");
+ printf("Options for crash mode:\n");
+ printf(" -c (fork | exec): Crash after. Optional.\n");
+ printf(" -s (user | kernel): Signal from. Required.\n");
+ printf(" -n counter: Number of crashes.\n");
+ printf(" Required if the option -c is used.\n");
+ printf(" Not used without the option -c.\n");
+ printf(" Range from 1 to INT_MAX.\n");
+ printf(" -C: Change privileges before crash. Optional.\n");
+ printf("Options for server_crash mode:\n");
+ printf(" -a ip: Ip v4 address to accept. Required.\n");
+ printf(" -p port: Port number. Required.\n");
+ printf(" Range from 1 to UINT16_MAX.\n");
+ printf(" -t secs: Accept timeout. Required.\n");
+ printf(" Range from 1 to LONG_MAX.\n");
+ printf(" -c (fork | exec): Crash after. Required.\n");
+ printf(" -s (user | kernel): Signal from. Required.\n");
+ printf(" -n counter: Number of crashes. Required.\n");
+ printf(" Range from 1 to INT_MAX.\n");
+ printf("Options for client mode:\n");
+ printf(" -a ip: Ip v4 address to connect. Required.\n");
+ printf(" -p port: Port number. Required.\n");
+ printf(" Range from 1 to UINT16_MAX.\n");
+ printf(" -t secs: Connect timeout. Required.\n");
+ printf(" Range from 1 to LONG_MAX.\n");
+}
+
+static __attribute__((noreturn)) void info_failure(const char *message,
+ const char *prog)
+{
+ printf("%s\n", message);
+ usage(prog);
+ exit(EXIT_FAILURE);
+}
+
+static enum mode get_mode(const char *text, const char *prog)
+{
+ if (!strcmp(text, "crash"))
+ return MODE_CRASH;
+
+ if (!strcmp(text, "server_crash"))
+ return MODE_SERVER_CRASH;
+
+ if (!strcmp(text, "client"))
+ return MODE_CLIENT;
+
+ info_failure("Invalid mode option [-m].", prog);
+}
+
+static enum crash_after get_crash_after(const char *text, const char *prog)
+{
+ if (!strcmp(text, "fork"))
+ return CRASH_AFTER_FORK;
+
+ if (!strcmp(text, "exec"))
+ return CRASH_AFTER_EXEC;
+
+ info_failure("Invalid crash after option [-c].", prog);
+}
+
+static enum signal_from get_signal_from(const char *text, const char *prog)
+{
+ if (!strcmp(text, "user"))
+ return SIGNAL_FROM_USER;
+
+ if (!strcmp(text, "kernel"))
+ return SIGNAL_FROM_KERNEL;
+
+ info_failure("Invalid signal from option [-s]", prog);
+}
+
+static int get_counter(const char *text, const char *prog)
+{
+ int counter;
+
+ counter = atoi(text);
+ if (counter > 0)
+ return counter;
+
+ info_failure("Invalid counter option [-n].", prog);
+}
+
+static __attribute__((noreturn)) void error_failure(const char *message)
+{
+ perror(message);
+ exit(EXIT_FAILURE);
+}
+
+static uint32_t get_ip(const char *text, const char *prog)
+{
+ int ret;
+ uint32_t ip;
+
+ ret = inet_pton(AF_INET, text, &ip);
+ if (!ret)
+ info_failure("Invalid ip option [-a].", prog);
+ else if (ret < 0)
+ error_failure("inet_pton");
+
+ return ip;
+}
+
+static uint16_t get_port(const char *text, const char *prog)
+{
+ long port;
+
+ port = atol(text);
+ if ((port > 0) && (port <= UINT16_MAX))
+ return htons(port);
+
+ info_failure("Invalid port option [-p].", prog);
+}
+
+static long get_timeout(const char *text, const char *prog)
+{
+ long timeout;
+
+ timeout = atol(text);
+ if (timeout > 0)
+ return timeout;
+
+ info_failure("Invalid timeout option [-t].", prog);
+}
+
+static void check_args(const struct args *args, const char *prog)
+{
+ if (args->mode == MODE_CRASH && args->crash_after != CRASH_AFTER_NONE &&
+ args->signal_from != SIGNAL_FROM_NONE && args->has_counter &&
+ !args->has_ip && !args->has_port && !args->has_timeout)
+ return;
+
+ if (args->mode == MODE_CRASH && args->signal_from != SIGNAL_FROM_NONE &&
+ args->crash_after == CRASH_AFTER_NONE && !args->has_counter &&
+ !args->has_ip && !args->has_port && !args->has_timeout)
+ return;
+
+ if (args->mode == MODE_SERVER_CRASH && args->has_ip && args->has_port &&
+ args->has_timeout && args->crash_after != CRASH_AFTER_NONE &&
+ args->signal_from != SIGNAL_FROM_NONE && args->has_counter &&
+ !args->has_change_priv)
+ return;
+
+ if (args->mode == MODE_CLIENT && args->has_ip && args->has_port &&
+ args->has_timeout && args->crash_after == CRASH_AFTER_NONE &&
+ args->signal_from == SIGNAL_FROM_NONE && !args->has_counter &&
+ !args->has_change_priv)
+ return;
+
+ info_failure("Invalid use of options.", prog);
+}
+
+static uid_t get_non_root_uid(void)
+{
+ struct passwd *pwent;
+ uid_t uid;
+
+ while (true) {
+ errno = 0;
+ pwent = getpwent();
+ if (!pwent) {
+ if (errno) {
+ perror("getpwent");
+ endpwent();
+ exit(EXIT_FAILURE);
+ }
+ break;
+ }
+
+ if (pwent->pw_uid) {
+ uid = pwent->pw_uid;
+ endpwent();
+ return uid;
+ }
+ }
+
+ endpwent();
+ printf("A user different of root is needed.\n");
+ exit(EXIT_FAILURE);
+}
+
+static inline void do_sigsegv(void)
+{
+ int *p = NULL;
+ *p = 0;
+}
+
+static void do_sigkill(void)
+{
+ int ret;
+
+ ret = kill(getpid(), SIGKILL);
+ if (ret)
+ error_failure("kill");
+}
+
+static void crash(enum signal_from signal_from, bool change_priv)
+{
+ int ret;
+
+ if (change_priv) {
+ ret = setuid(get_non_root_uid());
+ if (ret)
+ error_failure("setuid");
+ }
+
+ if (signal_from == SIGNAL_FROM_KERNEL)
+ do_sigsegv();
+
+ do_sigkill();
+}
+
+static void execve_crash(char *const argv[])
+{
+ execve(argv[0], argv, NULL);
+ error_failure("execve");
+}
+
+static void exec_crash_user(void)
+{
+ char *const argv[] = {
+ "./test", "-m", "crash", "-s", "user", NULL,
+ };
+
+ execve_crash(argv);
+}
+
+static void exec_crash_user_change_priv(void)
+{
+ char *const argv[] = {
+ "./test", "-m", "crash", "-s", "user", "-C", NULL,
+ };
+
+ execve_crash(argv);
+}
+
+static void exec_crash_kernel(void)
+{
+ char *const argv[] = {
+ "./test", "-m", "crash", "-s", "kernel", NULL,
+ };
+
+ execve_crash(argv);
+}
+
+static void exec_crash_kernel_change_priv(void)
+{
+ char *const argv[] = {
+ "./test", "-m", "crash", "-s", "kernel", "-C", NULL,
+ };
+
+ execve_crash(argv);
+}
+
+static void exec_crash(enum signal_from signal_from, bool change_priv)
+{
+ if (signal_from == SIGNAL_FROM_USER && !change_priv)
+ exec_crash_user();
+ if (signal_from == SIGNAL_FROM_USER && change_priv)
+ exec_crash_user_change_priv();
+ if (signal_from == SIGNAL_FROM_KERNEL && !change_priv)
+ exec_crash_kernel();
+ if (signal_from == SIGNAL_FROM_KERNEL && change_priv)
+ exec_crash_kernel_change_priv();
+}
+
+static void do_crash(enum crash_after crash_after, enum signal_from signal_from,
+ int counter, bool change_priv)
+{
+ pid_t pid;
+ int status;
+
+ if (crash_after == CRASH_AFTER_NONE)
+ crash(signal_from, change_priv);
+
+ while (counter > 0) {
+ pid = fork();
+ if (pid < 0)
+ error_failure("fork");
+
+ /* Child process */
+ if (!pid) {
+ if (crash_after == CRASH_AFTER_FORK)
+ crash(signal_from, change_priv);
+
+ exec_crash(signal_from, change_priv);
+ }
+
+ /* Parent process */
+ counter -= 1;
+ pid = waitpid(pid, &status, 0);
+ if (pid < 0)
+ error_failure("waitpid");
+ }
+}
+
+static __attribute__((noreturn)) void error_close_failure(const char *message,
+ int fd)
+{
+ perror(message);
+ close(fd);
+ exit(EXIT_FAILURE);
+}
+
+static void do_server(uint32_t ip, uint16_t port, long accept_timeout)
+{
+ int sockfd;
+ int ret;
+ struct sockaddr_in address;
+ struct timeval timeout;
+ int newsockfd;
+
+ sockfd = socket(AF_INET, SOCK_STREAM, 0);
+ if (sockfd < 0)
+ error_failure("socket");
+
+ address.sin_family = AF_INET;
+ address.sin_addr.s_addr = ip;
+ address.sin_port = port;
+
+ ret = bind(sockfd, (const struct sockaddr *)&address, sizeof(address));
+ if (ret)
+ error_close_failure("bind", sockfd);
+
+ ret = listen(sockfd, 1);
+ if (ret)
+ error_close_failure("listen", sockfd);
+
+ timeout.tv_sec = accept_timeout;
+ timeout.tv_usec = 0;
+ ret = setsockopt(sockfd, SOL_SOCKET, SO_RCVTIMEO,
+ (const struct timeval *)&timeout, sizeof(timeout));
+ if (ret)
+ error_close_failure("setsockopt", sockfd);
+
+ newsockfd = accept(sockfd, NULL, NULL);
+ if (newsockfd < 0)
+ error_close_failure("accept", sockfd);
+
+ close(sockfd);
+ close(newsockfd);
+}
+
+static void do_client(uint32_t ip, uint16_t port, long connect_timeout)
+{
+ int sockfd;
+ int ret;
+ struct timeval timeout;
+ struct sockaddr_in address;
+
+ sockfd = socket(AF_INET, SOCK_STREAM, 0);
+ if (sockfd < 0)
+ error_failure("socket");
+
+ timeout.tv_sec = connect_timeout;
+ timeout.tv_usec = 0;
+ ret = setsockopt(sockfd, SOL_SOCKET, SO_SNDTIMEO,
+ (const struct timeval *)&timeout, sizeof(timeout));
+ if (ret)
+ error_close_failure("setsockopt", sockfd);
+
+ address.sin_family = AF_INET;
+ address.sin_addr.s_addr = ip;
+ address.sin_port = port;
+
+ ret = connect(sockfd, (const struct sockaddr *)&address,
+ sizeof(address));
+ if (ret)
+ error_close_failure("connect", sockfd);
+
+ ret = write(sockfd, message, strlen(message));
+ if (ret < 0)
+ error_close_failure("write", sockfd);
+
+ close(sockfd);
+}
+
+#define PROG_NAME basename(argv[0])
+
+int main(int argc, char **argv)
+{
+ int opt;
+ struct args args = {
+ .mode = MODE_NONE,
+ .crash_after = CRASH_AFTER_NONE,
+ .signal_from = SIGNAL_FROM_NONE,
+ .has_counter = false,
+ .has_change_priv = false,
+ .has_ip = false,
+ .has_port = false,
+ .has_timeout = false,
+ };
+
+ while ((opt = getopt(argc, argv, OPT_STRING)) != -1) {
+ switch (opt) {
+ case 'h':
+ usage(PROG_NAME);
+ return EXIT_SUCCESS;
+ case 'm':
+ args.mode = get_mode(optarg, PROG_NAME);
+ break;
+ case 'c':
+ args.crash_after = get_crash_after(optarg, PROG_NAME);
+ break;
+ case 's':
+ args.signal_from = get_signal_from(optarg, PROG_NAME);
+ break;
+ case 'n':
+ args.counter = get_counter(optarg, PROG_NAME);
+ args.has_counter = true;
+ break;
+ case 'C':
+ args.has_change_priv = true;
+ break;
+ case 'a':
+ args.ip = get_ip(optarg, PROG_NAME);
+ args.has_ip = true;
+ break;
+ case 'p':
+ args.port = get_port(optarg, PROG_NAME);
+ args.has_port = true;
+ break;
+ case 't':
+ args.timeout = get_timeout(optarg, PROG_NAME);
+ args.has_timeout = true;
+ break;
+ default:
+ usage(PROG_NAME);
+ return EXIT_FAILURE;
+ }
+ }
+
+ check_args(&args, PROG_NAME);
+
+ if (args.mode == MODE_CRASH) {
+ do_crash(args.crash_after, args.signal_from, args.counter,
+ args.has_change_priv);
+ } else if (args.mode == MODE_SERVER_CRASH) {
+ do_server(args.ip, args.port, args.timeout);
+ do_crash(args.crash_after, args.signal_from, args.counter,
+ false);
+ } else if (args.mode == MODE_CLIENT) {
+ do_client(args.ip, args.port, args.timeout);
+ }
+
+ return EXIT_SUCCESS;
+}
diff --git a/tools/testing/selftests/brute/test.sh b/tools/testing/selftests/brute/test.sh
new file mode 100755
index 000000000000..47173f38a7c6
--- /dev/null
+++ b/tools/testing/selftests/brute/test.sh
@@ -0,0 +1,256 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0
+
+TCID="test.sh"
+
+KSFT_PASS=0
+KSFT_FAIL=1
+KSFT_SKIP=4
+
+errno=$KSFT_PASS
+
+check_root()
+{
+ local uid=$(id -u)
+ if [ $uid -ne 0 ]; then
+ echo $TCID: must be run as root >&2
+ exit $KSFT_SKIP
+ fi
+}
+
+tmp_files_setup()
+{
+ DMESG=$(mktemp --tmpdir -t brute-dmesg-XXXXXX)
+}
+
+tmp_files_cleanup()
+{
+ rm -f "$DMESG"
+}
+
+save_dmesg()
+{
+ dmesg > "$DMESG"
+}
+
+count_attack_matches()
+{
+ dmesg | comm --nocheck-order -13 "$DMESG" - | \
+ grep "brute: fork brute force attack detected" | wc -l
+}
+
+assert_equal()
+{
+ local val1=$1
+ local val2=$2
+
+ if [ $val1 -eq $val2 ]; then
+ echo "$TCID: $message [PASS]"
+ else
+ echo "$TCID: $message [FAIL]"
+ errno=$KSFT_FAIL
+ fi
+}
+
+test_fork_user()
+{
+ COUNTER=20
+
+ save_dmesg
+ ./test -m crash -c fork -s user -n $COUNTER
+ count=$(count_attack_matches)
+
+ message="Fork attack (user signals, no bounds crossed)"
+ assert_equal $count 0
+}
+
+test_fork_kernel()
+{
+ save_dmesg
+ ./test -m crash -c fork -s kernel -n $COUNTER
+ count=$(count_attack_matches)
+
+ message="Fork attack (kernel signals, no bounds crossed)"
+ assert_equal $count 0
+}
+
+test_exec_user()
+{
+ save_dmesg
+ ./test -m crash -c exec -s user -n $COUNTER
+ count=$(count_attack_matches)
+
+ message="Exec attack (user signals, no bounds crossed)"
+ assert_equal $count 0
+}
+
+test_exec_kernel()
+{
+ save_dmesg
+ ./test -m crash -c exec -s kernel -n $COUNTER
+ count=$(count_attack_matches)
+
+ message="Exec attack (kernel signals, no bounds crossed)"
+ assert_equal $count 0
+}
+
+assert_not_equal()
+{
+ local val1=$1
+ local val2=$2
+
+ if [ $val1 -ne $val2 ]; then
+ echo $TCID: $message [PASS]
+ else
+ echo $TCID: $message [FAIL]
+ errno=$KSFT_FAIL
+ fi
+}
+
+remove_xattr()
+{
+ ./rmxattr test >/dev/null 2>&1
+}
+
+test_fork_kernel_setuid()
+{
+ save_dmesg
+ chmod u+s test
+ ./test -m crash -c fork -s kernel -n $COUNTER
+ chmod u-s test
+ count=$(count_attack_matches)
+
+ message="Fork attack (kernel signals, setuid binary)"
+ assert_not_equal $count 0
+ remove_xattr
+}
+
+test_exec_kernel_setuid()
+{
+ save_dmesg
+ chmod u+s test
+ ./test -m crash -c exec -s kernel -n $COUNTER
+ chmod u-s test
+ count=$(count_attack_matches)
+
+ message="Exec attack (kernel signals, setuid binary)"
+ assert_not_equal $count 0
+ remove_xattr
+}
+
+test_fork_kernel_change_priv()
+{
+ save_dmesg
+ ./test -m crash -c fork -s kernel -n $COUNTER -C
+ count=$(count_attack_matches)
+
+ message="Fork attack (kernel signals, change privileges)"
+ assert_not_equal $count 0
+ remove_xattr
+}
+
+test_exec_kernel_change_priv()
+{
+ save_dmesg
+ ./test -m crash -c exec -s kernel -n $COUNTER -C
+ count=$(count_attack_matches)
+
+ message="Exec attack (kernel signals, change privileges)"
+ assert_not_equal $count 0
+ remove_xattr
+}
+
+network_ns_setup()
+{
+ local vnet_name=$1
+ local veth_name=$2
+ local ip_src=$3
+ local ip_dst=$4
+
+ ip netns add $vnet_name
+ ip link set $veth_name netns $vnet_name
+ ip -n $vnet_name addr add $ip_src/24 dev $veth_name
+ ip -n $vnet_name link set $veth_name up
+ ip -n $vnet_name route add $ip_dst/24 dev $veth_name
+}
+
+network_setup()
+{
+ VETH0_NAME=veth0
+ VNET0_NAME=vnet0
+ VNET0_IP=10.0.1.0
+ VETH1_NAME=veth1
+ VNET1_NAME=vnet1
+ VNET1_IP=10.0.2.0
+
+ ip link add $VETH0_NAME type veth peer name $VETH1_NAME
+ network_ns_setup $VNET0_NAME $VETH0_NAME $VNET0_IP $VNET1_IP
+ network_ns_setup $VNET1_NAME $VETH1_NAME $VNET1_IP $VNET0_IP
+}
+
+test_fork_kernel_network_to_local()
+{
+ INADDR_ANY=0.0.0.0
+ PORT=65535
+ TIMEOUT=5
+
+ save_dmesg
+ ip netns exec $VNET0_NAME ./test -m server_crash -a $INADDR_ANY \
+ -p $PORT -t $TIMEOUT -c fork -s kernel -n $COUNTER &
+ sleep 1
+ ip netns exec $VNET1_NAME ./test -m client -a $VNET0_IP -p $PORT \
+ -t $TIMEOUT
+ sleep 1
+ count=$(count_attack_matches)
+
+ message="Fork attack (kernel signals, network to local)"
+ assert_not_equal $count 0
+ remove_xattr
+}
+
+test_exec_kernel_network_to_local()
+{
+ save_dmesg
+ ip netns exec $VNET0_NAME ./test -m server_crash -a $INADDR_ANY \
+ -p $PORT -t $TIMEOUT -c exec -s kernel -n $COUNTER &
+ sleep 1
+ ip netns exec $VNET1_NAME ./test -m client -a $VNET0_IP -p $PORT \
+ -t $TIMEOUT
+ sleep 1
+ count=$(count_attack_matches)
+
+ message="Exec attack (kernel signals, network to local)"
+ assert_not_equal $count 0
+ remove_xattr
+}
+
+network_cleanup()
+{
+ ip netns del $VNET0_NAME >/dev/null 2>&1
+ ip netns del $VNET1_NAME >/dev/null 2>&1
+ ip link delete $VETH0_NAME >/dev/null 2>&1
+ ip link delete $VETH1_NAME >/dev/null 2>&1
+}
+
+cleanup()
+{
+ network_cleanup
+ tmp_files_cleanup
+ remove_xattr
+}
+trap cleanup EXIT
+
+check_root
+tmp_files_setup
+test_fork_user
+test_fork_kernel
+test_exec_user
+test_exec_kernel
+test_fork_kernel_setuid
+test_exec_kernel_setuid
+test_fork_kernel_change_priv
+test_exec_kernel_change_priv
+network_setup
+test_fork_kernel_network_to_local
+test_exec_kernel_network_to_local
+exit $errno
--
2.25.1

2021-05-22 06:42:30

by John Wood

[permalink] [raw]
Subject: [PATCH v7 6/7] Documentation: Add documentation for the Brute LSM

Add some info detailing what is the Brute LSM, its motivation, weak
points of existing implementations, proposed solutions, enabling,
disabling, configuration and self-tests.

Signed-off-by: John Wood <[email protected]>
---
Documentation/admin-guide/LSM/Brute.rst | 334 ++++++++++++++++++++++++
Documentation/admin-guide/LSM/index.rst | 1 +
security/brute/Kconfig | 3 +-
3 files changed, 337 insertions(+), 1 deletion(-)
create mode 100644 Documentation/admin-guide/LSM/Brute.rst

diff --git a/Documentation/admin-guide/LSM/Brute.rst b/Documentation/admin-guide/LSM/Brute.rst
new file mode 100644
index 000000000000..97c9aac33093
--- /dev/null
+++ b/Documentation/admin-guide/LSM/Brute.rst
@@ -0,0 +1,334 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+=====
+Brute
+=====
+
+Brute is a Linux Security Module that detects and mitigates fork brute force
+attacks against vulnerable userspace processes.
+
+
+Motivation
+==========
+
+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 implementation. 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 really needs to be detected are fork/exec brute force attacks that
+cross any of the commented bounds.
+
+
+Other implementations
+=====================
+
+The public version of grsecurity, as a summary, is based on the idea of delaying
+the fork system call if a child died due to some fatal signal (``SIGSEGV``,
+``SIGBUS``, ``SIGKILL`` or ``SIGILL``). This has some issues:
+
+Bad practices
+-------------
+
+Adding delays to the kernel is, in general, a bad idea.
+
+Scenarios not detected (false negatives)
+----------------------------------------
+
+This protection acts only when the fork system call is called after a child has
+crashed. So, it would still be possible for an attacker to fork a big amount of
+children (in the order of thousands), then probe all of them, and finally wait
+the protection time before repeating the steps.
+
+Moreover, this method is based on the idea that the protection doesn't act if
+the parent crashes. So, it would still be possible for an attacker to fork a
+process and probe itself. Then, fork the child process and probe itself again.
+This way, these steps can be repeated infinite times without any mitigation.
+
+Scenarios detected (false positives)
+------------------------------------
+
+Scenarios where an application rarely fails for reasons unrelated to a real
+attack.
+
+
+This implementation
+===================
+
+The main idea behind this implementation is to improve the existing ones
+focusing on the weak points annotated before. Basically, the adopted solution is
+to detect a fast crash rate instead of only one simple crash and to detect both
+the crash of parent and child processes. Also, fine tune the detection focusing
+on privilege boundary crossing. And finally, as a mitigation method, kill all
+the offending tasks involved in the attack and mark the executable as "not
+allowed" (to block the following executions) instead of using delays.
+
+To achieve this goal, and going into more details, this implementation is based
+on the use of some statistical data shared across all the processes that can
+have the same memory contents. Or in other words, a statistical data shared
+between all the fork hierarchy processes after an execve system call.
+
+The purpose of these statistics is, basically, collect all the necessary info to
+compute the application crash period in order to detect an attack. To track all
+this information, the extended attributes (xattr) of the executable files are
+used. More specifically, the name of the attribute is "brute" and uses the
+"security" namespace. So, the full xattr name for the Brute LSM is:
+
+ ``security.brute``
+
+The same can be achieved using a pointer to the fork hierarchy statistical data
+held by the ``task_struct`` structure, 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. Using
+the last method (pointer in the ``task_struct`` structure) makes not possible to
+manage this attack type that can be successfully treated using extended
+attributes.
+
+To detect a brute force attack it is necessary that the statistics shared by all
+the fork hierarchy processes be updated in every fatal crash and the most
+important data to update is the application crash period.
+
+The crash period is the time between two consecutive faults, but this also has a
+drawback: if an application crashes twice in a short period of time for some
+reason unrelated to a real attack, a false positive will be triggered. To avoid
+this scenario the exponential moving average (EMA) is used. This way, the
+application crash period will be a value that is not prone to change due to
+spurious data and follows the real crash period.
+
+These statistics are stored in the executables using the extended attributes
+feature. So, the detection and mitigation of brute force attacks using this LSM
+it is only feasible in filesystems that support xattr.
+
+.. kernel-doc:: security/brute/brute.c
+ :identifiers: brute_raw_stats
+
+This is a fixed sized struct with a very small footprint. So, in reference to
+memory usage, it is not expected to have problems storing it as an extended
+attribute.
+
+Concerning to access rights to this statistical data, as stated above, the
+"security" namespace is used. Since no custom policy, related to this extended
+attribute, has been implemented for the Brute LSM, all processes have read
+access to these statistics, and write access is limited to processes that have
+the ``CAP_SYS_ADMIN`` capability.
+
+Attack detection
+----------------
+
+There are two types of brute force attacks that need to be detected. The first
+one is an attack that happens through the fork system call and the second one is
+an attack that happens through the execve system call. Moreover, these two
+attack types have two variants. A slow brute force attack that is detected if a
+maximum number of faults per fork hierarchy is reached and a fast brute force
+attack that is detected if the application crash period falls below a certain
+threshold.
+
+Attack mitigation
+-----------------
+
+Once an attack has been detected, this is mitigated killing all the offending
+tasks involved. Or in other words, once an attack has been detected, this is
+mitigated killing all the processes that are executing the same file that is
+running during the brute force attack. Also, to prevent the executable involved
+in the attack from being respawned by a supervisor, and thus prevent a brute
+force attack from being started again, the file is marked as "not allowed" and
+the following executions are avoided based on this mark. This method allows
+supervisors to implement their own policy: they can read the statistics, know if
+the executable is blocked by the Brute LSM and why, and act based on this
+information. If they want to respawn the offending executable it is only
+necessary to remove the "``security.brute``" extended attribute and thus remove
+the statistical data.
+
+Fine tuning the attack detection
+--------------------------------
+
+To avoid false positives during the attack detection it is necessary to narrow
+the possible cases. To do so, and based on the threat scenarios that we want to
+detect, this implementation also focuses on the crossing of privilege bounds.
+
+To be precise, only the following privilege bounds are taken into account:
+
+ 1. setuid/setgid process
+ 2. network to local
+ 3. privilege changes
+
+Moreover, only the fatal signals delivered by the kernel are taken into account
+avoiding the fatal signals sent by userspace applications (with the exception of
+the ``SIGABRT`` user signal since this is used by glibc for stack canary,
+malloc, etc. failures, which may indicate that a mitigation has been triggered).
+
+Exponential moving average (EMA)
+--------------------------------
+
+This kind of average defines a weight (between 0 and 1) for the new value to add
+and applies the remainder of the weight to the current average value. This way,
+some spurious data will not excessively modify the average and only if the new
+values are persistent, the moving average will tend towards them.
+
+Mathematically the application crash period's EMA can be expressed as follows:
+
+ period_ema = period * weight + period_ema * (1 - weight)
+
+Related to the attack detection, the EMA must guarantee that not many crashes
+are needed. To demonstrate this, the scenario where an application has failed
+and then has been running without any crashes for a month, will be used.
+
+The period's EMA can be written now as:
+
+ period_ema[i] = period[i] * weight + period_ema[i - 1] * (1 - weight)
+
+If the new crash periods have insignificant values related to the first crash
+period (a month in this case), the formula can be rewritten as:
+
+ period_ema[i] = period_ema[i - 1] * (1 - weight)
+
+And by extension:
+
+ | period_ema[i - 1] = period_ema[i - 2] * (1 - weight)
+ | period_ema[i - 2] = period_ema[i - 3] * (1 - weight)
+ | period_ema[i - 3] = period_ema[i - 4] * (1 - weight)
+
+So, if the substitution is made:
+
+ | period_ema[i] = period_ema[i - 1] * (1 - weight)
+ | period_ema[i] = period_ema[i - 2] * (1 - weight)\ :sup:`2`
+ | period_ema[i] = period_ema[i - 3] * (1 - weight)\ :sup:`3`
+ | period_ema[i] = period_ema[i - 4] * (1 - weight)\ :sup:`4`
+
+And in a more generic form:
+
+ period_ema[i] = period_ema[i - n] * (1 - weight)\ :sup:`n`
+
+Where "n" represents the number of iterations to obtain an EMA value. Or in
+other words, the number of crashes to detect an attack.
+
+So, if we isolate the number of crashes:
+
+ | period_ema[i] / period_ema[i - n] = (1 - weight)\ :sup:`n`
+ | log(period_ema[i] / period_ema[i - n]) = log((1 - weight)\ :sup:`n`)
+ | log(period_ema[i] / period_ema[i - n]) = n * log(1 - weight)
+ | n = log(period_ema[i] / period_ema[i - n]) / log(1 - weight)
+
+Then, in the commented scenario (an application has failed and then has been
+running without any crashes for a month), the approximate number of crashes to
+detect an attack (using the default implementation values for the weight and the
+crash period threshold) is:
+
+ | weight = 7 / 10
+ | crash_period_threshold = 30 seconds
+
+ | n = log(crash_period_threshold / seconds_per_month) / log(1 - weight)
+ | n = log(30 / (30 * 24 * 3600)) / log(1 - 0.7)
+ | n = 9.44
+
+So, with 10 crashes for this scenario an attack will be detected. If these steps
+are repeated for different scenarios and the results are collected:
+
+ ======================== =====================================
+ time without any crashes number of crashes to detect an attack
+ ======================== =====================================
+ 1 month 9.44
+ 1 year 11.50
+ 10 years 13.42
+ ======================== =====================================
+
+However, this computation has a drawback. The first data added to the EMA not
+obtains a real average showing a trend. So the solution is simple, the EMA needs
+a minimum number of data to be able to be interpreted. This way, the case where
+a few first faults are fast enough followed by no crashes is avoided.
+
+Per system enabling/disabling
+-----------------------------
+
+This feature can be enabled at build time using the
+``CONFIG_SECURITY_FORK_BRUTE`` option or using the visual config application
+under the following menu:
+
+ Security options ``--->`` Fork brute force attack detection and mitigation
+
+Also, at boot time, this feature can be disable too, by changing the "``lsm=``"
+boot parameter.
+
+Per system configuration
+------------------------
+
+To customize the detection's sensibility there are five new sysctl attributes
+for the Brute LSM that are accessible through the following path:
+
+ ``/proc/sys/kernel/brute/``
+
+More specifically, the files and their description are:
+
+**ema_weight_numerator**
+
+ .. kernel-doc:: security/brute/brute.c
+ :doc: brute_ema_weight_numerator
+
+**ema_weight_denominator**
+
+ .. kernel-doc:: security/brute/brute.c
+ :doc: brute_ema_weight_denominator
+
+**max_faults**
+
+ .. kernel-doc:: security/brute/brute.c
+ :doc: brute_max_faults
+
+**min_faults**
+
+ .. kernel-doc:: security/brute/brute.c
+ :doc: brute_min_faults
+
+**crash_period_threshold**
+
+ .. kernel-doc:: security/brute/brute.c
+ :doc: brute_crash_period_threshold
+
+Kernel selftests
+----------------
+
+To validate all the expectations about this implementation, there is a set of
+selftests. This tests cover fork/exec brute force attacks crossing the following
+privilege boundaries:
+
+ 1. setuid process
+ 2. privilege changes
+ 3. network to local
+
+Also, there are some tests to check that fork/exec brute force attacks without
+crossing any privilege boundariy already commented doesn't trigger the detection
+and mitigation stage.
+
+To build the tests:
+ ``make -C tools/testing/selftests/ TARGETS=brute``
+
+To run the tests:
+ ``make -C tools/testing/selftests TARGETS=brute run_tests``
+
+To package the tests:
+ ``make -C tools/testing/selftests TARGETS=brute gen_tar``
diff --git a/Documentation/admin-guide/LSM/index.rst b/Documentation/admin-guide/LSM/index.rst
index a6ba95fbaa9f..1f68982bb330 100644
--- a/Documentation/admin-guide/LSM/index.rst
+++ b/Documentation/admin-guide/LSM/index.rst
@@ -41,6 +41,7 @@ subdirectories.
:maxdepth: 1

apparmor
+ Brute
LoadPin
SELinux
Smack
diff --git a/security/brute/Kconfig b/security/brute/Kconfig
index 5da314d221aa..d2dd33b08642 100644
--- a/security/brute/Kconfig
+++ b/security/brute/Kconfig
@@ -9,6 +9,7 @@ config SECURITY_FORK_BRUTE
offending tasks are killed. Also, the executable file involved in the
attack will be marked as "not allowed" and new execve system calls
using this file will fail. Like capabilities, this security module
- stacks with other LSMs.
+ stacks with other LSMs. Further information can be found in
+ Documentation/admin-guide/LSM/Brute.rst.

If you are unsure how to answer this question, answer N.
--
2.25.1

2021-05-22 06:49:29

by John Wood

[permalink] [raw]
Subject: [PATCH v7 7/7] MAINTAINERS: Add a new entry for the Brute LSM

In order to maintain the code for the Brute LSM add a new entry to the
maintainers list.

Signed-off-by: John Wood <[email protected]>
---
MAINTAINERS | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 008fcad7ac00..102eb3d7dcd6 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3847,6 +3847,13 @@ L: [email protected]
S: Supported
F: drivers/net/ethernet/brocade/bna/

+BRUTE SECURITY MODULE
+M: John Wood <[email protected]>
+S: Maintained
+F: Documentation/admin-guide/LSM/Brute.rst
+F: security/brute/
+F: tools/testing/selftests/brute/
+
BSG (block layer generic sg v4 driver)
M: FUJITA Tomonori <[email protected]>
L: [email protected]
--
2.25.1

2021-05-23 07:34:27

by John Wood

[permalink] [raw]
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

2021-05-23 14:46:20

by Andi Kleen

[permalink] [raw]
Subject: Re: [PATCH v7 0/7] Fork brute force attack mitigation


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


2021-05-23 15:53:31

by John Wood

[permalink] [raw]
Subject: Re: [PATCH v7 0/7] Fork brute force attack mitigation

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