[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <20210701234807.50453-1-alobakin@pm.me>
Date: Thu, 01 Jul 2021 23:55:14 +0000
From: Alexander Lobakin <alobakin@...me>
To: John Wood <john.wood@....com>
Cc: Alexander Lobakin <alobakin@...me>, 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>, Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>, x86@...nel.org, "H. Peter Anvin" <hpa@...or.com>, Arnd Bergmann <arnd@...db.de>, Andi Kleen <ak@...ux.intel.com>, valdis.kletnieks@...edu, Greg Kroah-Hartman <gregkh@...uxfoundation.org>, Randy Dunlap <rdunlap@...radead.org>, Andrew Morton <akpm@...ux-foundation.org>, linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org, linux-security-module@...r.kernel.org, linux-kselftest@...r.kernel.org, linux-arch@...r.kernel.org, linux-hardening@...r.kernel.org, kernel-hardening@...ts.openwall.com
Subject: Re: [PATCH v8 3/8] security/brute: Detect a brute force attack
Hi,
From: John Wood <john.wood@....com>
Date: Sat, 5 Jun 2021 17:04:00 +0200
> For a correct management of a fork brute force attack it is necessary to
> track all the information related to the application crashes. To do so,
> use the extended attributes (xattr) of the executable files and define a
> statistical data structure to hold all the necessary information shared
> by all the fork hierarchy processes. This info is the number of crashes,
> the last crash timestamp and the crash period's moving average.
>
> 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 this method makes not possible to
> manage this attack type that can be successfully treated using extended
> attributes.
>
> Also, to avoid false positives during the attack detection it is
> necessary to narrow the possible cases. So, only the following scenarios
> are taken into account:
>
> 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
>
> To mark that a privilege boundary has been crossed it is only necessary
> to create a new stats for the executable file via the extended attribute
> and only if it has no previous statistical data. This is done using four
> different LSM hooks, one per privilege boundary:
>
> setuid/setgid process --> bprm_creds_from_file hook (based on secureexec
> flag).
> network to local -------> socket_accept hook (taking into account only
> external connections).
> privilege changes ------> task_fix_setuid and task_fix_setgid hooks.
>
> To detect a brute force attack it is necessary that the executable file
> statistics be updated in every fatal crash and the most important data
> to update is the application crash period. To do so, use the new
> "task_fatal_signal" LSM hook added in a previous step.
>
> The application crash period must be a value that is not prone to change
> due to spurious data and follows the real crash period. So, to compute
> it, the exponential moving average (EMA) is used.
>
> Based on the updated statistics two different attacks can be handled. A
> slow brute force attack that is detected if the 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.
>
> Moreover, only the signals delivered by the kernel are taken into
> account with the exception of the SIGABRT signal since the latter is
> used by glibc for stack canary, malloc, etc failures, which may indicate
> that a mitigation has been triggered.
>
> Signed-off-by: John Wood <john.wood@....com>
>
> <snip>
>
> +static int brute_get_xattr_stats(struct dentry *dentry, struct inode *inode,
> + struct brute_stats *stats)
> +{
> + int rc;
> + struct brute_raw_stats raw_stats;
> +
> + rc = __vfs_getxattr(dentry, inode, XATTR_NAME_BRUTE, &raw_stats,
> + sizeof(raw_stats));
> + if (rc < 0)
> + return rc;
> +
> + stats->faults = le32_to_cpu(raw_stats.faults);
> + stats->nsecs = le64_to_cpu(raw_stats.nsecs);
> + stats->period = le64_to_cpu(raw_stats.period);
> + stats->flags = raw_stats.flags;
> + return 0;
> +}
>
> <snip>
>
> +static int brute_task_execve(struct linux_binprm *bprm, struct file *file)
> +{
> + struct dentry *dentry = file_dentry(bprm->file);
> + struct inode *inode = file_inode(bprm->file);
> + struct brute_stats stats;
> + int rc;
> +
> + inode_lock(inode);
> + rc = brute_get_xattr_stats(dentry, inode, &stats);
> + if (WARN_ON_ONCE(rc && rc != -ENODATA))
> + goto unlock;
I think I caught a problem here. Have you tested this with
initramfs?
According to init/do_mount.c's
init_rootfs()/rootfs_init_fs_context(), when `root=` cmdline
parameter is not empty, kernel creates rootfs of type ramfs
(tmpfs otherwise).
The thing about ramfs is that it doesn't support xattrs.
I'm running this v8 on a regular PC with initramfs and having
`root=` in cmdline, and Brute doesn't allow the kernel to run
any init processes (/init, /sbin/init, ...) with err == -95
(-EOPNOTSUPP) -- I'm getting a
WARNING: CPU: 0 PID: 173 at brute_task_execve+0x15d/0x200
<snip>
Failed to execute /init (error -95)
and so on (and a panic at the end).
If I omit `root=` from cmdline, then the kernel runs init process
just fine -- I guess because initramfs is then placed inside tmpfs
with xattr support.
As for me, this ramfs/tmpfs selection based on `root=` presence
is ridiculous and I don't see or know any reasons behind that.
But that's another story, and ramfs might be not the only one
system without xattr support.
I think Brute should have a fallback here, e.g. it could simply
ignore files from xattr-incapable filesystems instead of such
WARNING splats and stuff.
> +
> + if (rc == -ENODATA && bprm->secureexec) {
> + brute_reset_stats(&stats);
> + rc = brute_set_xattr_stats(dentry, inode, &stats);
> + if (WARN_ON_ONCE(rc))
> + goto unlock;
> + }
> +
> + rc = 0;
> +unlock:
> + inode_unlock(inode);
> + return rc;
> +}
> +
>
> <snip>
Thanks,
Al
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.
