|
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
Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.