[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <19903478-52e0-3829-0515-3e17669108f7@linux.intel.com>
Date: Fri, 21 May 2021 11:02:14 -0700
From: Andi Kleen <ak@...ux.intel.com>
To: John Wood <john.wood@....com>, Kees Cook <keescook@...omium.org>,
Jann Horn <jannh@...gle.com>, Jonathan Corbet <corbet@....net>,
James Morris <jmorris@...ei.org>,
"Serge E. Hallyn" <serge@...lyn.com>, Shuah Khan <shuah@...nel.org>
Cc: valdis.kletnieks@...edu,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Randy Dunlap <rdunlap@...radead.org>,
linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-security-module@...r.kernel.org,
linux-kselftest@...r.kernel.org, linux-hardening@...r.kernel.org,
kernel-hardening@...ts.openwall.com
Subject: Re: [PATCH v7 0/7] Fork brute force attack mitigation
> 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
>
Powered by blists - more mailing lists