Re: [PATCH v5 13/16] ima: Move some IMA policy and filesystem related variables into ima_namespace
From: Stefan Berger <stefanb@linux.ibm.com>
Date: 2021-12-13 15:34:03
Also in:
linux-security-module, lkml
On 12/11/21 04:50, Christian Brauner wrote:
quoted hunk ↗ jump to hunk
On Fri, Dec 10, 2021 at 08:57:11AM -0500, Stefan Berger wrote:quoted
there anything that would prevent us from setns()'ing to that target user namespace so that we would now see that of a user namespace that we are not allowed to see?If you're really worried about someone being able to access a securityfs instance whose userns doesn't match the userns the securityfs instance was mounted in there are multiple ways to fix it. The one that I tend to prefer is: From e0ff6a8dcc573763568e685dd70d1547efd68df9 Mon Sep 17 00:00:00 2001 From: Christian Brauner <redacted> Date: Fri, 10 Dec 2021 11:47:37 +0100 Subject: !!!! HERE BE DRAGONS - COMPLETELY UNTESTED !!!! securityfs: only allow access to securityfs from within same namespace Limit opening of securityfs files to callers located in the same namespace. --- security/inode.c | 33 +++++++++++++++++++++++++++++++-- 1 file changed, 31 insertions(+), 2 deletions(-)diff --git a/security/inode.c b/security/inode.c index eaccba7017d9..9eaf757c08cb 100644 --- a/security/inode.c +++ b/security/inode.c@@ -80,6 +80,35 @@ static struct file_system_type fs_type = { .fs_flags = FS_USERNS_MOUNT, }; +static int securityfs_permission(struct user_namespace *mnt_userns, + struct inode *inode, int mask) +{ + int err; + + err = generic_permission(&init_user_ns, inode, mask); + if (!err) { + if (inode->i_sb->s_user_ns != current_user_ns()) + err = -EACCES; + } + + return err; +} + +const struct inode_operations securityfs_dir_inode_operations = { + .permission = securityfs_permission, + .lookup = simple_lookup, +}; + +const struct file_operations securityfs_dir_operations = { + .permission = securityfs_permission,
This interface function on file operations doesn't exist. I'll use the inode_operations and also hook it to the root dentry of the super_block. Then there's no need to have this check on symlinks and files...
quoted hunk ↗ jump to hunk
+ .open = dcache_dir_open, + .release = dcache_dir_close, + .llseek = dcache_dir_lseek, + .read = generic_read_dir, + .iterate_shared = dcache_readdir, + .fsync = noop_fsync, +}; + /** * securityfs_create_dentry - create a dentry in the securityfs filesystem *@@ -167,8 +196,8 @@ static struct dentry *securityfs_create_dentry(const char *name, umode_t mode, inode->i_atime = inode->i_mtime = inode->i_ctime = current_time(inode); inode->i_private = data; if (S_ISDIR(mode)) { - inode->i_op = &simple_dir_inode_operations; - inode->i_fop = &simple_dir_operations; + inode->i_op = &securityfs_dir_inode_operations; + inode->i_fop = &securityfs_dir_operations; inc_nlink(inode); inc_nlink(dir); } else if (S_ISLNK(mode)) {