From: John Wood john.wood@gmx.com Date: Fri, 2 Jul 2021 16:59:54 +0200
Hi,
On Thu, Jul 01, 2021 at 11:55:14PM +0000, Alexander Lobakin wrote:
Hi,
From: John Wood john.wood@gmx.com Date: Sat, 5 Jun 2021 17:04:00 +0200
+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?
No, it has not been tested 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.
It is a known issue that systems without xattr support are not suitable for Brute (there are a note in the documentation). However, the purpose is not to panic the system :(
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.
Ok, it seems reasonable to me: if the file system doesn't support xattr, but Brute is enabled, Brute will do nothing and the system will work normally.
On the other hand, it leaves a potentional window for attackers to perform brute force from xattr-incapable filesystems. So at the end of the day I think that the current implementation (a strong rejection of such filesystems) is way more secure than having a fallback I proposed.
I'm planning to make a patch which will eliminate such weird rootfs type selection and just always use more feature-rich tmpfs if it's compiled in. So, as an alternative, you could add it to your series as a preparatory change and just add a Kconfig dependency on CONFIG_TMPFS && CONFIG_TMPFS_XATTR to CONFIG_SECURITY_FORK_BRUTE without messing with any fallbacks at all. What do you think?
I will work on it for the next version. Thanks for the feedback.
John Wood
Thanks, Al