linux-security-module.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Andi Kleen <ak@linux.intel.com>
To: John Wood <john.wood@gmx.com>, Kees Cook <keescook@chromium.org>,
	Jann Horn <jannh@google.com>, Jonathan Corbet <corbet@lwn.net>,
	James Morris <jmorris@namei.org>,
	"Serge E. Hallyn" <serge@hallyn.com>,
	Shuah Khan <shuah@kernel.org>
Cc: valdis.kletnieks@vt.edu,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Randy Dunlap <rdunlap@infradead.org>,
	linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-security-module@vger.kernel.org,
	linux-kselftest@vger.kernel.org, linux-hardening@vger.kernel.org,
	kernel-hardening@lists.openwall.com
Subject: Re: [PATCH v7 0/7] Fork brute force attack mitigation
Date: Fri, 21 May 2021 11:02:14 -0700	[thread overview]
Message-ID: <19903478-52e0-3829-0515-3e17669108f7@linux.intel.com> (raw)
In-Reply-To: <20210521172414.69456-1-john.wood@gmx.com>


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

  parent reply	other threads:[~2021-05-21 18:02 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-21 17:24 [PATCH v7 0/7] Fork brute force attack mitigation John Wood
2021-05-21 17:24 ` [PATCH v7 1/7] security: Add LSM hook at the point where a task gets a fatal signal John Wood
2021-05-21 17:24 ` [PATCH v7 2/7] security/brute: Define a LSM and add sysctl attributes John Wood
2021-05-21 17:24 ` [PATCH v7 3/7] security/brute: Detect a brute force attack John Wood
2021-05-21 17:24 ` [PATCH v7 4/7] security/brute: Mitigate " John Wood
2021-05-21 18:02 ` Andi Kleen [this message]
2021-05-23  7:31   ` [PATCH v7 0/7] Fork brute force attack mitigation John Wood
2021-05-23 14:43     ` Andi Kleen
2021-05-23 15:47       ` John Wood
2021-05-22  6:35 ` [PATCH v7 5/7] selftests/brute: Add tests for the Brute LSM John Wood
2021-05-22  6:39 ` [PATCH v7 6/7] Documentation: Add documentation " John Wood
2021-05-22  6:44 ` [PATCH v7 7/7] MAINTAINERS: Add a new entry " John Wood

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=19903478-52e0-3829-0515-3e17669108f7@linux.intel.com \
    --to=ak@linux.intel.com \
    --cc=corbet@lwn.net \
    --cc=gregkh@linuxfoundation.org \
    --cc=jannh@google.com \
    --cc=jmorris@namei.org \
    --cc=john.wood@gmx.com \
    --cc=keescook@chromium.org \
    --cc=kernel-hardening@lists.openwall.com \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-hardening@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=rdunlap@infradead.org \
    --cc=serge@hallyn.com \
    --cc=shuah@kernel.org \
    --cc=valdis.kletnieks@vt.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).