[PATCH v2 0/2] fuse: Send file/inode security context during creation

Hi,

This is V2 of patches. Posted V1 here.

https://lore.kernel.org/linux-fsdevel/20210924192442.916927-1-vgoyal@redhat.com/

Changes since v1:

- Added capability to send multiple security contexts in fuse protocol.
  Miklos suggestd this. So now protocol can easily carry multiple
  security labels. Just that right now we only send one. When a security
  hook becomes available which can handle multiple security labels,
  it should be easy to send those.

This patch series is dependent on following patch I have posted to
change signature of security_dentry_init_security().

https://lore.kernel.org/linux-fsdevel/YWWMO%2FZDrvDZ5X4c@redhat.com/

Description
-----------
When a file is created (create, mknod, mkdir, symlink), typically file
systems call  security_inode_init_security() to initialize security
context of an inode. But this does not very well with remote filesystems
as inode is not there yet. Client will send a creation request to
server and once server has created the file, client will instantiate
the inode.

So filesystems like nfs and ceph use security_dentry_init_security()
instead. This takes in a dentry and returns the security context of
file if any.

These patches call security_dentry_init_security() and send security
label of file along with creation request (FUSE_CREATE, FUSE_MKDIR,
FUSE_MKNOD, FUSE_SYMLINK). This will give server an opportunity
to create new file and also set security label (possibly atomically
where possible).

These patches are based on the work Chirantan Ekbote did some time
back but it never got upstreamed. So I have taken his patches,
and made modifications on top.

https://listman.redhat.com/archives/virtio-fs/2020-July/msg00014.html
https://listman.redhat.com/archives/virtio-fs/2020-July/msg00015.html

These patches will allow us to support SELinux on virtiofs.

Vivek Goyal (2):
  fuse: Add a flag FUSE_SECURITY_CTX
  fuse: Send security context of inode on file creation

 fs/fuse/dir.c             | 115 ++++++++++++++++++++++++++++++++++++--
 fs/fuse/fuse_i.h          |   3 +
 fs/fuse/inode.c           |   4 +-
 include/uapi/linux/fuse.h |  29 +++++++++-
 4 files changed, 144 insertions(+), 7 deletions(-)

-- 
2.31.1

Add the FUSE_SECURITY_CTX flag for the `flags` field of the
fuse_init_out struct.  When this flag is set the kernel will append the
security context for a newly created inode to the request (create,
mkdir, mknod, and symlink).  The server is responsible for ensuring that
the inode appears atomically (preferrably) with the requested security context.

For example, if the server is backed by a "real" linux file system then
it can write the security context value to
/proc/thread-self/attr/fscreate before making the syscall to create the
inode.

Vivek:
This patch is slightly modified version of patch from
Chirantan Ekbote [off-list ref]. I made changes so that this
patch applies to latest kernel.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
 include/uapi/linux/fuse.h | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h
index 36ed092227fa..2fe54c80051a 100644
--- a/include/uapi/linux/fuse.h
+++ b/include/uapi/linux/fuse.h

@@ -184,6 +184,10 @@
  *
  *  7.34
  *  - add FUSE_SYNCFS
+ *
+ *  7.35
+ *  - add FUSE_SECURITY_CTX flag for fuse_init_out
+ *  - add security context to create, mkdir, symlink, and mknod requests
  */
 
 #ifndef _LINUX_FUSE_H

@@ -219,7 +223,7 @@
 #define FUSE_KERNEL_VERSION 7
 
 /** Minor version number of this interface */
-#define FUSE_KERNEL_MINOR_VERSION 34
+#define FUSE_KERNEL_MINOR_VERSION 35
 
 /** The node ID of the root inode */
 #define FUSE_ROOT_ID 1

@@ -336,6 +340,8 @@ struct fuse_file_lock {
  *			write/truncate sgid is killed only if file has group
  *			execute permission. (Same as Linux VFS behavior).
  * FUSE_SETXATTR_EXT:	Server supports extended struct fuse_setxattr_in
+ * FUSE_SECURITY_CTX:	add security context to create, mkdir, symlink, and
+ * 			mknod
  */
 #define FUSE_ASYNC_READ		(1 << 0)
 #define FUSE_POSIX_LOCKS	(1 << 1)

@@ -367,6 +373,7 @@ struct fuse_file_lock {
 #define FUSE_SUBMOUNTS		(1 << 27)
 #define FUSE_HANDLE_KILLPRIV_V2	(1 << 28)
 #define FUSE_SETXATTR_EXT	(1 << 29)
+#define FUSE_SECURITY_CTX	(1 << 30)
 
 /**
  * CUSE INIT request/reply flags

-- 
2.31.1

When a new inode is created, send its security context to server along
with creation request (FUSE_CREAT, FUSE_MKNOD, FUSE_MKDIR and FUSE_SYMLINK).
This gives server an opportunity to create new file and set security
context (possibly atomically). In all the configurations it might not
be possible to set context atomically.

Like nfs and ceph, use security_dentry_init_security() to dermine security
context of inode and send it with create, mkdir, mknod, and symlink requests.

Following is the information sent to server.

- struct fuse_secctxs.
  This contains total number of security contexts being sent.

- struct fuse_secctx.
  This contains total size of security context which follows this structure.
  There is one fuse_secctx instance per security context.

- xattr name string.
  This string represents name of xattr which should be used while setting
  security context. As of now it is hardcoded to "security.selinux".

- security context.
  This is the actual security context whose size is specified in fuse_secctx
  struct.

This patch is modified version of patch from
Chirantan Ekbote [off-list ref]

v2:
- Added "fuse_secctxs" structure where one can specify how many security
  contexts are being sent. This can be useful down the line if we
  have more than one security contexts being set.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
 fs/fuse/dir.c             | 115 ++++++++++++++++++++++++++++++++++++--
 fs/fuse/fuse_i.h          |   3 +
 fs/fuse/inode.c           |   4 +-
 include/uapi/linux/fuse.h |  20 +++++++
 4 files changed, 136 insertions(+), 6 deletions(-)

diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index d9b977c0f38d..ce62593a61f9 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c

@@ -17,6 +17,9 @@
 #include <linux/xattr.h>
 #include <linux/iversion.h>
 #include <linux/posix_acl.h>
+#include <linux/security.h>
+#include <linux/types.h>
+#include <linux/kernel.h>
 
 static void fuse_advise_use_readdirplus(struct inode *dir)
 {

@@ -456,6 +459,66 @@ static struct dentry *fuse_lookup(struct inode *dir, struct dentry *entry,
 	return ERR_PTR(err);
 }
 
+static int get_security_context(struct dentry *entry, umode_t mode,
+				void **security_ctx, u32 *security_ctxlen)
+{
+	struct fuse_secctx *fsecctx;
+	struct fuse_secctxs *fsecctxs;
+	void *ctx, *full_ctx;
+	u32 ctxlen, full_ctxlen;
+	int err = 0;
+	const char *name;
+
+	err = security_dentry_init_security(entry, mode, &entry->d_name,
+					    &name, &ctx, &ctxlen);
+	if (err) {
+		if (err != -EOPNOTSUPP)
+			goto out_err;
+		/* No LSM is supporting this security hook. Ignore error */
+		err = 0;
+		ctxlen = 0;
+	}
+
+	if (ctxlen > 0) {
+		void *ptr;
+
+		full_ctxlen = sizeof(*fsecctxs) + sizeof(*fsecctx) +
+			      strlen(name) + ctxlen + 1;
+		full_ctx = kzalloc(full_ctxlen, GFP_KERNEL);
+		if (!full_ctx) {
+			err = -ENOMEM;
+			kfree(ctx);
+			goto out_err;
+		}
+
+		ptr = full_ctx;
+		fsecctxs = (struct fuse_secctxs*) ptr;
+		fsecctxs->nr_secctx = 1;
+		ptr += sizeof(*fsecctxs);
+
+		fsecctx = (struct fuse_secctx*) ptr;
+		fsecctx->size = ctxlen;
+		ptr += sizeof(*fsecctx);
+
+		strcpy(ptr, name);
+		ptr += strlen(name) + 1;
+		memcpy(ptr, ctx, ctxlen);
+		kfree(ctx);
+	} else {
+		full_ctxlen = sizeof(*fsecctxs);
+		full_ctx = kzalloc(full_ctxlen, GFP_KERNEL);
+		if (!full_ctx) {
+			err = -ENOMEM;
+			goto out_err;
+		}
+	}
+
+	*security_ctxlen = full_ctxlen;
+	*security_ctx = full_ctx;
+out_err:
+	return err;
+}
+
 /*
  * Atomic create+open operation
  *

@@ -476,6 +539,8 @@ static int fuse_create_open(struct inode *dir, struct dentry *entry,
 	struct fuse_entry_out outentry;
 	struct fuse_inode *fi;
 	struct fuse_file *ff;
+	void *security_ctx = NULL;
+	u32 security_ctxlen;
 
 	/* Userspace expects S_IFREG in create mode */
 	BUG_ON((mode & S_IFMT) != S_IFREG);

@@ -517,6 +582,18 @@ static int fuse_create_open(struct inode *dir, struct dentry *entry,
 	args.out_args[0].value = &outentry;
 	args.out_args[1].size = sizeof(outopen);
 	args.out_args[1].value = &outopen;
+
+	if (fm->fc->init_security) {
+		err = get_security_context(entry, mode, &security_ctx,
+					   &security_ctxlen);
+		if (err)
+			goto out_put_forget_req;
+
+		args.in_numargs = 3;
+		args.in_args[2].size = security_ctxlen;
+		args.in_args[2].value = security_ctx;
+	}
+
 	err = fuse_simple_request(fm, &args);
 	if (err)
 		goto out_free_ff;

@@ -554,6 +631,7 @@ static int fuse_create_open(struct inode *dir, struct dentry *entry,
 
 out_free_ff:
 	fuse_file_free(ff);
+	kfree(security_ctx);
 out_put_forget_req:
 	kfree(forget);
 out_err:

@@ -613,13 +691,15 @@ static int fuse_atomic_open(struct inode *dir, struct dentry *entry,
  */
 static int create_new_entry(struct fuse_mount *fm, struct fuse_args *args,
 			    struct inode *dir, struct dentry *entry,
-			    umode_t mode)
+			    umode_t mode, bool init_security)
 {
 	struct fuse_entry_out outarg;
 	struct inode *inode;
 	struct dentry *d;
 	int err;
 	struct fuse_forget_link *forget;
+	void *security_ctx = NULL;
+	u32 security_ctxlen = 0;
 
 	if (fuse_is_bad(dir))
 		return -EIO;

@@ -633,7 +713,29 @@ static int create_new_entry(struct fuse_mount *fm, struct fuse_args *args,
 	args->out_numargs = 1;
 	args->out_args[0].size = sizeof(outarg);
 	args->out_args[0].value = &outarg;
+
+	if (init_security) {
+		unsigned short idx = args->in_numargs;
+
+		if ((size_t)idx >= ARRAY_SIZE(args->in_args)) {
+			err = -ENOMEM;
+			goto out_put_forget_req;
+		}
+
+		err = get_security_context(entry, mode, &security_ctx,
+					   &security_ctxlen);
+		if (err)
+			goto out_put_forget_req;
+
+		if (security_ctxlen > 0) {
+			args->in_args[idx].size = security_ctxlen;
+			args->in_args[idx].value = security_ctx;
+			args->in_numargs++;
+		}
+	}
+
 	err = fuse_simple_request(fm, args);
+	kfree(security_ctx);
 	if (err)
 		goto out_put_forget_req;
 

@@ -691,7 +793,7 @@ static int fuse_mknod(struct user_namespace *mnt_userns, struct inode *dir,
 	args.in_args[0].value = &inarg;
 	args.in_args[1].size = entry->d_name.len + 1;
 	args.in_args[1].value = entry->d_name.name;
-	return create_new_entry(fm, &args, dir, entry, mode);
+	return create_new_entry(fm, &args, dir, entry, mode, fm->fc->init_security);
 }
 
 static int fuse_create(struct user_namespace *mnt_userns, struct inode *dir,

@@ -719,7 +821,8 @@ static int fuse_mkdir(struct user_namespace *mnt_userns, struct inode *dir,
 	args.in_args[0].value = &inarg;
 	args.in_args[1].size = entry->d_name.len + 1;
 	args.in_args[1].value = entry->d_name.name;
-	return create_new_entry(fm, &args, dir, entry, S_IFDIR);
+	return create_new_entry(fm, &args, dir, entry, S_IFDIR,
+				fm->fc->init_security);
 }
 
 static int fuse_symlink(struct user_namespace *mnt_userns, struct inode *dir,

@@ -735,7 +838,8 @@ static int fuse_symlink(struct user_namespace *mnt_userns, struct inode *dir,
 	args.in_args[0].value = entry->d_name.name;
 	args.in_args[1].size = len;
 	args.in_args[1].value = link;
-	return create_new_entry(fm, &args, dir, entry, S_IFLNK);
+	return create_new_entry(fm, &args, dir, entry, S_IFLNK,
+				fm->fc->init_security);
 }
 
 void fuse_update_ctime(struct inode *inode)

@@ -915,7 +1019,8 @@ static int fuse_link(struct dentry *entry, struct inode *newdir,
 	args.in_args[0].value = &inarg;
 	args.in_args[1].size = newent->d_name.len + 1;
 	args.in_args[1].value = newent->d_name.name;
-	err = create_new_entry(fm, &args, newdir, newent, inode->i_mode);
+	err = create_new_entry(fm, &args, newdir, newent, inode->i_mode,
+			       false);
 	/* Contrary to "normal" filesystems it can happen that link
 	   makes two "logical" inodes point to the same "physical"
 	   inode.  We invalidate the attributes of the old one, so it

diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index 319596df5dc6..885f34f9967f 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h

@@ -765,6 +765,9 @@ struct fuse_conn {
 	/* Propagate syncfs() to server */
 	unsigned int sync_fs:1;
 
+	/* Initialize security xattrs when creating a new inode */
+	unsigned int init_security:1;
+
 	/** The number of requests waiting for completion */
 	atomic_t num_waiting;
 

diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index 36cd03114b6d..343bc9cfbd92 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c

@@ -1152,6 +1152,8 @@ static void process_init_reply(struct fuse_mount *fm, struct fuse_args *args,
 			}
 			if (arg->flags & FUSE_SETXATTR_EXT)
 				fc->setxattr_ext = 1;
+			if (arg->flags & FUSE_SECURITY_CTX)
+				fc->init_security = 1;
 		} else {
 			ra_pages = fc->max_read / PAGE_SIZE;
 			fc->no_lock = 1;

@@ -1195,7 +1197,7 @@ void fuse_send_init(struct fuse_mount *fm)
 		FUSE_PARALLEL_DIROPS | FUSE_HANDLE_KILLPRIV | FUSE_POSIX_ACL |
 		FUSE_ABORT_ERROR | FUSE_MAX_PAGES | FUSE_CACHE_SYMLINKS |
 		FUSE_NO_OPENDIR_SUPPORT | FUSE_EXPLICIT_INVAL_DATA |
-		FUSE_HANDLE_KILLPRIV_V2 | FUSE_SETXATTR_EXT;
+		FUSE_HANDLE_KILLPRIV_V2 | FUSE_SETXATTR_EXT | FUSE_SECURITY_CTX;
 #ifdef CONFIG_FUSE_DAX
 	if (fm->fc->dax)
 		ia->in.flags |= FUSE_MAP_ALIGNMENT;

diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h
index 2fe54c80051a..b31a0f79fde8 100644
--- a/include/uapi/linux/fuse.h
+++ b/include/uapi/linux/fuse.h

@@ -986,4 +986,24 @@ struct fuse_syncfs_in {
 	uint64_t	padding;
 };
 
+/*
+ * For each security context, send fuse_secctx with size of security context
+ * fuse_secctx will be followed by security context name and this in turn
+ * will be followed by actual context label.
+ * fuse_secctx, name, context
+ * */
+struct fuse_secctx {
+	uint32_t	size;
+	uint32_t	padding;
+};
+
+/*
+ * Contains the information about how many fuse_secctx structures are being
+ * sent.
+ */
+struct fuse_secctxs {
+	uint32_t	nr_secctx;
+	uint32_t	padding;
+};
+
 #endif /* _LINUX_FUSE_H */

-- 
2.31.1

On 10/12/2021 11:06 AM, Vivek Goyal wrote:

When a new inode is created, send its security context to server along
with creation request (FUSE_CREAT, FUSE_MKNOD, FUSE_MKDIR and FUSE_SYMLINK).
This gives server an opportunity to create new file and set security
context (possibly atomically). In all the configurations it might not
be possible to set context atomically.

Like nfs and ceph, use security_dentry_init_security() to dermine security
context of inode and send it with create, mkdir, mknod, and symlink requests.

Following is the information sent to server.

- struct fuse_secctxs.
  This contains total number of security contexts being sent.

- struct fuse_secctx.
  This contains total size of security context which follows this structure.
  There is one fuse_secctx instance per security context.

- xattr name string.
  This string represents name of xattr which should be used while setting
  security context. As of now it is hardcoded to "security.selinux".

Where is the name hardcoded? I looks as if you're getting the attribute
name along with the value from security_dentry_init_security().

On Tue, Oct 12, 2021 at 11:24:23AM -0700, Casey Schaufler wrote:

On 10/12/2021 11:06 AM, Vivek Goyal wrote:

quoted

When a new inode is created, send its security context to server along
with creation request (FUSE_CREAT, FUSE_MKNOD, FUSE_MKDIR and FUSE_SYMLINK).
This gives server an opportunity to create new file and set security
context (possibly atomically). In all the configurations it might not
be possible to set context atomically.

Like nfs and ceph, use security_dentry_init_security() to dermine security
context of inode and send it with create, mkdir, mknod, and symlink requests.

Following is the information sent to server.

- struct fuse_secctxs.
  This contains total number of security contexts being sent.

- struct fuse_secctx.
  This contains total size of security context which follows this structure.
  There is one fuse_secctx instance per security context.

- xattr name string.
  This string represents name of xattr which should be used while setting
  security context. As of now it is hardcoded to "security.selinux".

Where is the name hardcoded? I looks as if you're getting the attribute
name along with the value from security_dentry_init_security().

Sorry, I copied pasted this description from V1 where I was hardcoding
the name to "security.selinux". But V2 got rid of that hardcoding and
that's why this patch series is dependent on the other patch which
modifies security_dentry_init_security() signature.

https://lore.kernel.org/linux-fsdevel/YWWMO%2FZDrvDZ5X4c@redhat.com/

Thanks
Vivek

quoted

- security context.
  This is the actual security context whose size is specified in fuse_secctx
  struct.

This patch is modified version of patch from
Chirantan Ekbote [off-list ref]

v2:
- Added "fuse_secctxs" structure where one can specify how many security
  contexts are being sent. This can be useful down the line if we
  have more than one security contexts being set.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
 fs/fuse/dir.c             | 115 ++++++++++++++++++++++++++++++++++++--
 fs/fuse/fuse_i.h          |   3 +
 fs/fuse/inode.c           |   4 +-
 include/uapi/linux/fuse.h |  20 +++++++
 4 files changed, 136 insertions(+), 6 deletions(-)

diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index d9b977c0f38d..ce62593a61f9 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c

@@ -17,6 +17,9 @@
 #include <linux/xattr.h>
 #include <linux/iversion.h>
 #include <linux/posix_acl.h>
+#include <linux/security.h>
+#include <linux/types.h>
+#include <linux/kernel.h>
 
 static void fuse_advise_use_readdirplus(struct inode *dir)
 {

@@ -456,6 +459,66 @@ static struct dentry *fuse_lookup(struct inode *dir, struct dentry *entry,
 	return ERR_PTR(err);
 }
 
+static int get_security_context(struct dentry *entry, umode_t mode,
+				void **security_ctx, u32 *security_ctxlen)
+{
+	struct fuse_secctx *fsecctx;
+	struct fuse_secctxs *fsecctxs;
+	void *ctx, *full_ctx;
+	u32 ctxlen, full_ctxlen;
+	int err = 0;
+	const char *name;
+
+	err = security_dentry_init_security(entry, mode, &entry->d_name,
+					    &name, &ctx, &ctxlen);
+	if (err) {
+		if (err != -EOPNOTSUPP)
+			goto out_err;
+		/* No LSM is supporting this security hook. Ignore error */
+		err = 0;
+		ctxlen = 0;
+	}
+
+	if (ctxlen > 0) {
+		void *ptr;
+
+		full_ctxlen = sizeof(*fsecctxs) + sizeof(*fsecctx) +
+			      strlen(name) + ctxlen + 1;
+		full_ctx = kzalloc(full_ctxlen, GFP_KERNEL);
+		if (!full_ctx) {
+			err = -ENOMEM;
+			kfree(ctx);
+			goto out_err;
+		}
+
+		ptr = full_ctx;
+		fsecctxs = (struct fuse_secctxs*) ptr;
+		fsecctxs->nr_secctx = 1;
+		ptr += sizeof(*fsecctxs);
+
+		fsecctx = (struct fuse_secctx*) ptr;
+		fsecctx->size = ctxlen;
+		ptr += sizeof(*fsecctx);
+
+		strcpy(ptr, name);
+		ptr += strlen(name) + 1;
+		memcpy(ptr, ctx, ctxlen);
+		kfree(ctx);
+	} else {
+		full_ctxlen = sizeof(*fsecctxs);
+		full_ctx = kzalloc(full_ctxlen, GFP_KERNEL);
+		if (!full_ctx) {
+			err = -ENOMEM;
+			goto out_err;
+		}
+	}
+
+	*security_ctxlen = full_ctxlen;
+	*security_ctx = full_ctx;
+out_err:
+	return err;
+}
+
 /*
  * Atomic create+open operation
  *

@@ -476,6 +539,8 @@ static int fuse_create_open(struct inode *dir, struct dentry *entry,
 	struct fuse_entry_out outentry;
 	struct fuse_inode *fi;
 	struct fuse_file *ff;
+	void *security_ctx = NULL;
+	u32 security_ctxlen;
 
 	/* Userspace expects S_IFREG in create mode */
 	BUG_ON((mode & S_IFMT) != S_IFREG);

@@ -517,6 +582,18 @@ static int fuse_create_open(struct inode *dir, struct dentry *entry,
 	args.out_args[0].value = &outentry;
 	args.out_args[1].size = sizeof(outopen);
 	args.out_args[1].value = &outopen;
+
+	if (fm->fc->init_security) {
+		err = get_security_context(entry, mode, &security_ctx,
+					   &security_ctxlen);
+		if (err)
+			goto out_put_forget_req;
+
+		args.in_numargs = 3;
+		args.in_args[2].size = security_ctxlen;
+		args.in_args[2].value = security_ctx;
+	}
+
 	err = fuse_simple_request(fm, &args);
 	if (err)
 		goto out_free_ff;

@@ -554,6 +631,7 @@ static int fuse_create_open(struct inode *dir, struct dentry *entry,
 
 out_free_ff:
 	fuse_file_free(ff);
+	kfree(security_ctx);
 out_put_forget_req:
 	kfree(forget);
 out_err:

@@ -613,13 +691,15 @@ static int fuse_atomic_open(struct inode *dir, struct dentry *entry,
  */
 static int create_new_entry(struct fuse_mount *fm, struct fuse_args *args,
 			    struct inode *dir, struct dentry *entry,
-			    umode_t mode)
+			    umode_t mode, bool init_security)
 {
 	struct fuse_entry_out outarg;
 	struct inode *inode;
 	struct dentry *d;
 	int err;
 	struct fuse_forget_link *forget;
+	void *security_ctx = NULL;
+	u32 security_ctxlen = 0;
 
 	if (fuse_is_bad(dir))
 		return -EIO;

@@ -633,7 +713,29 @@ static int create_new_entry(struct fuse_mount *fm, struct fuse_args *args,
 	args->out_numargs = 1;
 	args->out_args[0].size = sizeof(outarg);
 	args->out_args[0].value = &outarg;
+
+	if (init_security) {
+		unsigned short idx = args->in_numargs;
+
+		if ((size_t)idx >= ARRAY_SIZE(args->in_args)) {
+			err = -ENOMEM;
+			goto out_put_forget_req;
+		}
+
+		err = get_security_context(entry, mode, &security_ctx,
+					   &security_ctxlen);
+		if (err)
+			goto out_put_forget_req;
+
+		if (security_ctxlen > 0) {
+			args->in_args[idx].size = security_ctxlen;
+			args->in_args[idx].value = security_ctx;
+			args->in_numargs++;
+		}
+	}
+
 	err = fuse_simple_request(fm, args);
+	kfree(security_ctx);
 	if (err)
 		goto out_put_forget_req;
 

@@ -691,7 +793,7 @@ static int fuse_mknod(struct user_namespace *mnt_userns, struct inode *dir,
 	args.in_args[0].value = &inarg;
 	args.in_args[1].size = entry->d_name.len + 1;
 	args.in_args[1].value = entry->d_name.name;
-	return create_new_entry(fm, &args, dir, entry, mode);
+	return create_new_entry(fm, &args, dir, entry, mode, fm->fc->init_security);
 }
 
 static int fuse_create(struct user_namespace *mnt_userns, struct inode *dir,

@@ -719,7 +821,8 @@ static int fuse_mkdir(struct user_namespace *mnt_userns, struct inode *dir,
 	args.in_args[0].value = &inarg;
 	args.in_args[1].size = entry->d_name.len + 1;
 	args.in_args[1].value = entry->d_name.name;
-	return create_new_entry(fm, &args, dir, entry, S_IFDIR);
+	return create_new_entry(fm, &args, dir, entry, S_IFDIR,
+				fm->fc->init_security);
 }
 
 static int fuse_symlink(struct user_namespace *mnt_userns, struct inode *dir,

@@ -735,7 +838,8 @@ static int fuse_symlink(struct user_namespace *mnt_userns, struct inode *dir,
 	args.in_args[0].value = entry->d_name.name;
 	args.in_args[1].size = len;
 	args.in_args[1].value = link;
-	return create_new_entry(fm, &args, dir, entry, S_IFLNK);
+	return create_new_entry(fm, &args, dir, entry, S_IFLNK,
+				fm->fc->init_security);
 }
 
 void fuse_update_ctime(struct inode *inode)

@@ -915,7 +1019,8 @@ static int fuse_link(struct dentry *entry, struct inode *newdir,
 	args.in_args[0].value = &inarg;
 	args.in_args[1].size = newent->d_name.len + 1;
 	args.in_args[1].value = newent->d_name.name;
-	err = create_new_entry(fm, &args, newdir, newent, inode->i_mode);
+	err = create_new_entry(fm, &args, newdir, newent, inode->i_mode,
+			       false);
 	/* Contrary to "normal" filesystems it can happen that link
 	   makes two "logical" inodes point to the same "physical"
 	   inode.  We invalidate the attributes of the old one, so it

diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index 319596df5dc6..885f34f9967f 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h

@@ -765,6 +765,9 @@ struct fuse_conn {
 	/* Propagate syncfs() to server */
 	unsigned int sync_fs:1;
 
+	/* Initialize security xattrs when creating a new inode */
+	unsigned int init_security:1;
+
 	/** The number of requests waiting for completion */
 	atomic_t num_waiting;
 

diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index 36cd03114b6d..343bc9cfbd92 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c

@@ -1152,6 +1152,8 @@ static void process_init_reply(struct fuse_mount *fm, struct fuse_args *args,
 			}
 			if (arg->flags & FUSE_SETXATTR_EXT)
 				fc->setxattr_ext = 1;
+			if (arg->flags & FUSE_SECURITY_CTX)
+				fc->init_security = 1;
 		} else {
 			ra_pages = fc->max_read / PAGE_SIZE;
 			fc->no_lock = 1;

@@ -1195,7 +1197,7 @@ void fuse_send_init(struct fuse_mount *fm)
 		FUSE_PARALLEL_DIROPS | FUSE_HANDLE_KILLPRIV | FUSE_POSIX_ACL |
 		FUSE_ABORT_ERROR | FUSE_MAX_PAGES | FUSE_CACHE_SYMLINKS |
 		FUSE_NO_OPENDIR_SUPPORT | FUSE_EXPLICIT_INVAL_DATA |
-		FUSE_HANDLE_KILLPRIV_V2 | FUSE_SETXATTR_EXT;
+		FUSE_HANDLE_KILLPRIV_V2 | FUSE_SETXATTR_EXT | FUSE_SECURITY_CTX;
 #ifdef CONFIG_FUSE_DAX
 	if (fm->fc->dax)
 		ia->in.flags |= FUSE_MAP_ALIGNMENT;

diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h
index 2fe54c80051a..b31a0f79fde8 100644
--- a/include/uapi/linux/fuse.h
+++ b/include/uapi/linux/fuse.h

@@ -986,4 +986,24 @@ struct fuse_syncfs_in {
 	uint64_t	padding;
 };
 
+/*
+ * For each security context, send fuse_secctx with size of security context
+ * fuse_secctx will be followed by security context name and this in turn
+ * will be followed by actual context label.
+ * fuse_secctx, name, context
+ * */
+struct fuse_secctx {
+	uint32_t	size;
+	uint32_t	padding;
+};
+
+/*
+ * Contains the information about how many fuse_secctx structures are being
+ * sent.
+ */
+struct fuse_secctxs {
+	uint32_t	nr_secctx;
+	uint32_t	padding;
+};
+
 #endif /* _LINUX_FUSE_H */

On 10/12/2021 11:34 AM, Vivek Goyal wrote:

On Tue, Oct 12, 2021 at 11:24:23AM -0700, Casey Schaufler wrote:

quoted

On 10/12/2021 11:06 AM, Vivek Goyal wrote:

quoted

When a new inode is created, send its security context to server along
with creation request (FUSE_CREAT, FUSE_MKNOD, FUSE_MKDIR and FUSE_SYMLINK).
This gives server an opportunity to create new file and set security
context (possibly atomically). In all the configurations it might not
be possible to set context atomically.

Like nfs and ceph, use security_dentry_init_security() to dermine security
context of inode and send it with create, mkdir, mknod, and symlink requests.

Following is the information sent to server.

- struct fuse_secctxs.
  This contains total number of security contexts being sent.

- struct fuse_secctx.
  This contains total size of security context which follows this structure.
  There is one fuse_secctx instance per security context.

- xattr name string.
  This string represents name of xattr which should be used while setting
  security context. As of now it is hardcoded to "security.selinux".

Where is the name hardcoded? I looks as if you're getting the attribute
name along with the value from security_dentry_init_security().

Sorry, I copied pasted this description from V1 where I was hardcoding
the name to "security.selinux".

That's what I suspected. Thanks.

 But V2 got rid of that hardcoding and
that's why this patch series is dependent on the other patch which
modifies security_dentry_init_security() signature.

https://lore.kernel.org/linux-fsdevel/YWWMO%2FZDrvDZ5X4c@redhat.com/

Thanks
Vivek

quoted

quoted

- security context.
  This is the actual security context whose size is specified in fuse_secctx
  struct.

This patch is modified version of patch from
Chirantan Ekbote [off-list ref]

v2:
- Added "fuse_secctxs" structure where one can specify how many security
  contexts are being sent. This can be useful down the line if we
  have more than one security contexts being set.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
 fs/fuse/dir.c             | 115 ++++++++++++++++++++++++++++++++++++--
 fs/fuse/fuse_i.h          |   3 +
 fs/fuse/inode.c           |   4 +-
 include/uapi/linux/fuse.h |  20 +++++++
 4 files changed, 136 insertions(+), 6 deletions(-)

diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index d9b977c0f38d..ce62593a61f9 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c

@@ -17,6 +17,9 @@
 #include <linux/xattr.h>
 #include <linux/iversion.h>
 #include <linux/posix_acl.h>
+#include <linux/security.h>
+#include <linux/types.h>
+#include <linux/kernel.h>
 
 static void fuse_advise_use_readdirplus(struct inode *dir)
 {

@@ -456,6 +459,66 @@ static struct dentry *fuse_lookup(struct inode *dir, struct dentry *entry,
 	return ERR_PTR(err);
 }
 
+static int get_security_context(struct dentry *entry, umode_t mode,
+				void **security_ctx, u32 *security_ctxlen)
+{
+	struct fuse_secctx *fsecctx;
+	struct fuse_secctxs *fsecctxs;
+	void *ctx, *full_ctx;
+	u32 ctxlen, full_ctxlen;
+	int err = 0;
+	const char *name;
+
+	err = security_dentry_init_security(entry, mode, &entry->d_name,
+					    &name, &ctx, &ctxlen);
+	if (err) {
+		if (err != -EOPNOTSUPP)
+			goto out_err;
+		/* No LSM is supporting this security hook. Ignore error */
+		err = 0;
+		ctxlen = 0;
+	}
+
+	if (ctxlen > 0) {
+		void *ptr;
+
+		full_ctxlen = sizeof(*fsecctxs) + sizeof(*fsecctx) +
+			      strlen(name) + ctxlen + 1;
+		full_ctx = kzalloc(full_ctxlen, GFP_KERNEL);
+		if (!full_ctx) {
+			err = -ENOMEM;
+			kfree(ctx);
+			goto out_err;
+		}
+
+		ptr = full_ctx;
+		fsecctxs = (struct fuse_secctxs*) ptr;
+		fsecctxs->nr_secctx = 1;
+		ptr += sizeof(*fsecctxs);
+
+		fsecctx = (struct fuse_secctx*) ptr;
+		fsecctx->size = ctxlen;
+		ptr += sizeof(*fsecctx);
+
+		strcpy(ptr, name);
+		ptr += strlen(name) + 1;
+		memcpy(ptr, ctx, ctxlen);
+		kfree(ctx);
+	} else {
+		full_ctxlen = sizeof(*fsecctxs);
+		full_ctx = kzalloc(full_ctxlen, GFP_KERNEL);
+		if (!full_ctx) {
+			err = -ENOMEM;
+			goto out_err;
+		}
+	}
+
+	*security_ctxlen = full_ctxlen;
+	*security_ctx = full_ctx;
+out_err:
+	return err;
+}
+
 /*
  * Atomic create+open operation
  *

@@ -476,6 +539,8 @@ static int fuse_create_open(struct inode *dir, struct dentry *entry,
 	struct fuse_entry_out outentry;
 	struct fuse_inode *fi;
 	struct fuse_file *ff;
+	void *security_ctx = NULL;
+	u32 security_ctxlen;
 
 	/* Userspace expects S_IFREG in create mode */
 	BUG_ON((mode & S_IFMT) != S_IFREG);

@@ -517,6 +582,18 @@ static int fuse_create_open(struct inode *dir, struct dentry *entry,
 	args.out_args[0].value = &outentry;
 	args.out_args[1].size = sizeof(outopen);
 	args.out_args[1].value = &outopen;
+
+	if (fm->fc->init_security) {
+		err = get_security_context(entry, mode, &security_ctx,
+					   &security_ctxlen);
+		if (err)
+			goto out_put_forget_req;
+
+		args.in_numargs = 3;
+		args.in_args[2].size = security_ctxlen;
+		args.in_args[2].value = security_ctx;
+	}
+
 	err = fuse_simple_request(fm, &args);
 	if (err)
 		goto out_free_ff;

@@ -554,6 +631,7 @@ static int fuse_create_open(struct inode *dir, struct dentry *entry,
 
 out_free_ff:
 	fuse_file_free(ff);
+	kfree(security_ctx);
 out_put_forget_req:
 	kfree(forget);
 out_err:

@@ -613,13 +691,15 @@ static int fuse_atomic_open(struct inode *dir, struct dentry *entry,
  */
 static int create_new_entry(struct fuse_mount *fm, struct fuse_args *args,
 			    struct inode *dir, struct dentry *entry,
-			    umode_t mode)
+			    umode_t mode, bool init_security)
 {
 	struct fuse_entry_out outarg;
 	struct inode *inode;
 	struct dentry *d;
 	int err;
 	struct fuse_forget_link *forget;
+	void *security_ctx = NULL;
+	u32 security_ctxlen = 0;
 
 	if (fuse_is_bad(dir))
 		return -EIO;

@@ -633,7 +713,29 @@ static int create_new_entry(struct fuse_mount *fm, struct fuse_args *args,
 	args->out_numargs = 1;
 	args->out_args[0].size = sizeof(outarg);
 	args->out_args[0].value = &outarg;
+
+	if (init_security) {
+		unsigned short idx = args->in_numargs;
+
+		if ((size_t)idx >= ARRAY_SIZE(args->in_args)) {
+			err = -ENOMEM;
+			goto out_put_forget_req;
+		}
+
+		err = get_security_context(entry, mode, &security_ctx,
+					   &security_ctxlen);
+		if (err)
+			goto out_put_forget_req;
+
+		if (security_ctxlen > 0) {
+			args->in_args[idx].size = security_ctxlen;
+			args->in_args[idx].value = security_ctx;
+			args->in_numargs++;
+		}
+	}
+
 	err = fuse_simple_request(fm, args);
+	kfree(security_ctx);
 	if (err)
 		goto out_put_forget_req;
 

@@ -691,7 +793,7 @@ static int fuse_mknod(struct user_namespace *mnt_userns, struct inode *dir,
 	args.in_args[0].value = &inarg;
 	args.in_args[1].size = entry->d_name.len + 1;
 	args.in_args[1].value = entry->d_name.name;
-	return create_new_entry(fm, &args, dir, entry, mode);
+	return create_new_entry(fm, &args, dir, entry, mode, fm->fc->init_security);
 }
 
 static int fuse_create(struct user_namespace *mnt_userns, struct inode *dir,

@@ -719,7 +821,8 @@ static int fuse_mkdir(struct user_namespace *mnt_userns, struct inode *dir,
 	args.in_args[0].value = &inarg;
 	args.in_args[1].size = entry->d_name.len + 1;
 	args.in_args[1].value = entry->d_name.name;
-	return create_new_entry(fm, &args, dir, entry, S_IFDIR);
+	return create_new_entry(fm, &args, dir, entry, S_IFDIR,
+				fm->fc->init_security);
 }
 
 static int fuse_symlink(struct user_namespace *mnt_userns, struct inode *dir,

@@ -735,7 +838,8 @@ static int fuse_symlink(struct user_namespace *mnt_userns, struct inode *dir,
 	args.in_args[0].value = entry->d_name.name;
 	args.in_args[1].size = len;
 	args.in_args[1].value = link;
-	return create_new_entry(fm, &args, dir, entry, S_IFLNK);
+	return create_new_entry(fm, &args, dir, entry, S_IFLNK,
+				fm->fc->init_security);
 }
 
 void fuse_update_ctime(struct inode *inode)

@@ -915,7 +1019,8 @@ static int fuse_link(struct dentry *entry, struct inode *newdir,
 	args.in_args[0].value = &inarg;
 	args.in_args[1].size = newent->d_name.len + 1;
 	args.in_args[1].value = newent->d_name.name;
-	err = create_new_entry(fm, &args, newdir, newent, inode->i_mode);
+	err = create_new_entry(fm, &args, newdir, newent, inode->i_mode,
+			       false);
 	/* Contrary to "normal" filesystems it can happen that link
 	   makes two "logical" inodes point to the same "physical"
 	   inode.  We invalidate the attributes of the old one, so it

diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index 319596df5dc6..885f34f9967f 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h

@@ -765,6 +765,9 @@ struct fuse_conn {
 	/* Propagate syncfs() to server */
 	unsigned int sync_fs:1;
 
+	/* Initialize security xattrs when creating a new inode */
+	unsigned int init_security:1;
+
 	/** The number of requests waiting for completion */
 	atomic_t num_waiting;
 

diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index 36cd03114b6d..343bc9cfbd92 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c

@@ -1152,6 +1152,8 @@ static void process_init_reply(struct fuse_mount *fm, struct fuse_args *args,
 			}
 			if (arg->flags & FUSE_SETXATTR_EXT)
 				fc->setxattr_ext = 1;
+			if (arg->flags & FUSE_SECURITY_CTX)
+				fc->init_security = 1;
 		} else {
 			ra_pages = fc->max_read / PAGE_SIZE;
 			fc->no_lock = 1;

@@ -1195,7 +1197,7 @@ void fuse_send_init(struct fuse_mount *fm)
 		FUSE_PARALLEL_DIROPS | FUSE_HANDLE_KILLPRIV | FUSE_POSIX_ACL |
 		FUSE_ABORT_ERROR | FUSE_MAX_PAGES | FUSE_CACHE_SYMLINKS |
 		FUSE_NO_OPENDIR_SUPPORT | FUSE_EXPLICIT_INVAL_DATA |
-		FUSE_HANDLE_KILLPRIV_V2 | FUSE_SETXATTR_EXT;
+		FUSE_HANDLE_KILLPRIV_V2 | FUSE_SETXATTR_EXT | FUSE_SECURITY_CTX;
 #ifdef CONFIG_FUSE_DAX
 	if (fm->fc->dax)
 		ia->in.flags |= FUSE_MAP_ALIGNMENT;

diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h
index 2fe54c80051a..b31a0f79fde8 100644
--- a/include/uapi/linux/fuse.h
+++ b/include/uapi/linux/fuse.h

@@ -986,4 +986,24 @@ struct fuse_syncfs_in {
 	uint64_t	padding;
 };
 
+/*
+ * For each security context, send fuse_secctx with size of security context
+ * fuse_secctx will be followed by security context name and this in turn
+ * will be followed by actual context label.
+ * fuse_secctx, name, context
+ * */
+struct fuse_secctx {
+	uint32_t	size;
+	uint32_t	padding;
+};
+
+/*
+ * Contains the information about how many fuse_secctx structures are being
+ * sent.
+ */
+struct fuse_secctxs {
+	uint32_t	nr_secctx;
+	uint32_t	padding;
+};
+
 #endif /* _LINUX_FUSE_H */