Re: [PATCH v6 7/8] Documentation: Add documentation for the Brute LSM
On Sun, Mar 21, 2021 at 12:50:47PM -0600, Jonathan Corbet wrote:
> John Wood writes:
>
> > Add some info detailing what is the Brute LSM, its motivation, weak
> > points of existing implementations, proposed solutions, enabling,
> > disabling and self-tests.
> >
> > Signed-off-by: John Wood
> > ---
> > Documentation/admin-guide/LSM/Brute.rst | 278
> > Documentation/admin-guide/LSM/index.rst | 1 +
> > security/brute/Kconfig | 3 +-
> > 3 files changed, 281 insertions(+), 1 deletion(-)
> > create mode 100644 Documentation/admin-guide/LSM/Brute.rst
>
> Thanks for including documentation with the patch!
>
> As you get closer to merging this, though, you'll want to take a minute
> (OK, a few minutes) to build the docs and look at the result; there are
Thanks, I will do it.
> a number of places where you're not going to get what you expect. Just
> as an example:
>
> [...]
>
> > +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).
>
> Sphinx will try to recognize your enumerated list, but that may be a bit
> more punctuation than it is prepared to deal with; I'd take the hyphens
> out, if nothing else.
Thanks. I will fix this for the next version.
> > +These statistics are hold by the brute_stats struct.
> > +
> > +struct brute_cred {
> > + kuid_t uid;
> > + kgid_t gid;
> > + kuid_t suid;
> > + kgid_t sgid;
> > + kuid_t euid;
> > + kgid_t egid;
> > + kuid_t fsuid;
> > + kgid_t fsgid;
> > +};
>
> That will certainly not render the way you want. What you need here is
> a literal block:
>
> These statistics are hold by the brute_stats struct::
>
> struct brute_cred {
> kuid_t uid;
> kgid_t gid;
> kuid_t suid;
> kgid_t sgid;
> kuid_t euid;
> kgid_t egid;
> kuid_t fsuid;
> kgid_t fsgid;
> };
>
> The "::" causes all of the indented text following to be formatted
> literally.
Thanks a lot for your comments and guidance. I will build the docs and
check if the output is as I want.
> Thanks,
>
> jon
Regards,
John Wood
Re: [PATCH v6 7/8] Documentation: Add documentation for the Brute LSM
John Wood writes:
> Add some info detailing what is the Brute LSM, its motivation, weak
> points of existing implementations, proposed solutions, enabling,
> disabling and self-tests.
>
> Signed-off-by: John Wood
> ---
> Documentation/admin-guide/LSM/Brute.rst | 278
> Documentation/admin-guide/LSM/index.rst | 1 +
> security/brute/Kconfig | 3 +-
> 3 files changed, 281 insertions(+), 1 deletion(-)
> create mode 100644 Documentation/admin-guide/LSM/Brute.rst
Thanks for including documentation with the patch!
As you get closer to merging this, though, you'll want to take a minute
(OK, a few minutes) to build the docs and look at the result; there are
a number of places where you're not going to get what you expect. Just
as an example:
[...]
> +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).
Sphinx will try to recognize your enumerated list, but that may be a bit
more punctuation than it is prepared to deal with; I'd take the hyphens
out, if nothing else.
[...]
> +These statistics are hold by the brute_stats struct.
> +
> +struct brute_cred {
> + kuid_t uid;
> + kgid_t gid;
> + kuid_t suid;
> + kgid_t sgid;
> + kuid_t euid;
> + kgid_t egid;
> + kuid_t fsuid;
> + kgid_t fsgid;
> +};
That will certainly not render the way you want. What you need here is
a literal block:
These statistics are hold by the brute_stats struct::
struct brute_cred {
kuid_t uid;
kgid_t gid;
kuid_t suid;
kgid_t sgid;
kuid_t euid;
kgid_t egid;
kuid_t fsuid;
kgid_t fsgid;
};
The "::" causes all of the indented text following to be formatted
literally.
Thanks,
jon
Re: [PATCH v6 7/8] Documentation: Add documentation for the Brute LSM
On Wed, Mar 17, 2021 at 09:10:05PM -0700, Kees Cook wrote:
> On Sun, Mar 07, 2021 at 12:30:30PM +0100, John Wood wrote:
> > +These statistics are hold by the brute_stats struct.
> > +
> > +struct brute_cred {
> > + kuid_t uid;
> > + kgid_t gid;
> > + kuid_t suid;
> > + kgid_t sgid;
> > + kuid_t euid;
> > + kgid_t egid;
> > + kuid_t fsuid;
> > + kgid_t fsgid;
> > +};
> > +
> > +struct brute_stats {
> > + spinlock_t lock;
> > + refcount_t refc;
> > + unsigned char faults;
> > + u64 jiffies;
> > + u64 period;
> > + struct brute_cred saved_cred;
> > + unsigned char network : 1;
> > + unsigned char bounds_crossed : 1;
> > +};
>
> Instead of open-coding this, just use the kerndoc references you've
> already built in the .c files:
>
> .. kernel-doc:: security/brute/brute.c
>
Ok, thanks.
John Wood
Re: [PATCH v6 7/8] Documentation: Add documentation for the Brute LSM
On Sun, Mar 07, 2021 at 12:30:30PM +0100, John Wood wrote: > Add some info detailing what is the Brute LSM, its motivation, weak > points of existing implementations, proposed solutions, enabling, > disabling and self-tests. > > Signed-off-by: John Wood > --- > Documentation/admin-guide/LSM/Brute.rst | 278 > Documentation/admin-guide/LSM/index.rst | 1 + > security/brute/Kconfig | 3 +- > 3 files changed, 281 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 ..ca80aef9aa67 > --- /dev/null > +++ b/Documentation/admin-guide/LSM/Brute.rst > @@ -0,0 +1,278 @@ > +.. SPDX-License-Identifier: GPL-2.0 > +=== > +Brute: Fork brute force attack detection and mitigation LSM > +=== > + > +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 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
[PATCH v6 7/8] 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 and self-tests. Signed-off-by: John Wood --- Documentation/admin-guide/LSM/Brute.rst | 278 Documentation/admin-guide/LSM/index.rst | 1 + security/brute/Kconfig | 3 +- 3 files changed, 281 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 ..ca80aef9aa67 --- /dev/null +++ b/Documentation/admin-guide/LSM/Brute.rst @@ -0,0 +1,278 @@ +.. SPDX-License-Identifier: GPL-2.0 +=== +Brute: Fork brute force attack detection and mitigation LSM +=== + +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 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. This crash +period is the time between the execve system call and the first fault or the +time between two consecutive faults, but this has a drawback. If an application +crashes twice in a short period of time for some reason unrelated to a real +attack, a false
