Thread (68 messages) 68 messages, 7 authors, 2021-12-13

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)) {
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help