[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[PATCH v5 5/5] virtiofsd: Switch creds, drop FSETID for system.posix_acl
From: |
Vivek Goyal |
Subject: |
[PATCH v5 5/5] virtiofsd: Switch creds, drop FSETID for system.posix_acl_access xattr |
Date: |
Thu, 25 Mar 2021 11:38:52 -0400 |
When posix access acls are set on a file, it can lead to adjusting file
permissions (mode) as well. If caller does not have CAP_FSETID and it
also does not have membership of owner group, this will lead to clearing
SGID bit in mode.
Current fuse code is written in such a way that it expects file server
to take care of chaning file mode (permission), if there is a need.
Right now, host kernel does not clear SGID bit because virtiofsd is
running as root and has CAP_FSETID. For host kernel to clear SGID,
virtiofsd need to switch to gid of caller in guest and also drop
CAP_FSETID (if caller did not have it to begin with).
If SGID needs to be cleared, client will set the flag
FUSE_SETXATTR_ACL_KILL_SGID in setxattr request. In that case server
should kill sgid.
Currently just switch to uid/gid of the caller and drop CAP_FSETID
and that should do it.
This should fix the xfstest generic/375 test case.
We don't have to switch uid for this to work. That could be one optimization
that pass a parameter to lo_change_cred() to only switch gid and not uid.
Also this will not work whenever (if ever) we support idmapped mounts. In
that case it is possible that uid/gid in request are 0/0 but still we
need to clear SGID. So we will have to pick a non-root sgid and switch
to that instead. That's an TODO item for future when idmapped mount
support is introduced.
Reported-by: Luis Henriques <lhenriques@suse.de>
Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
include/standard-headers/linux/fuse.h | 7 +++++
tools/virtiofsd/passthrough_ll.c | 42 +++++++++++++++++++++++++--
2 files changed, 47 insertions(+), 2 deletions(-)
diff --git a/include/standard-headers/linux/fuse.h
b/include/standard-headers/linux/fuse.h
index cc87ff27d0..4eb79399d4 100644
--- a/include/standard-headers/linux/fuse.h
+++ b/include/standard-headers/linux/fuse.h
@@ -180,6 +180,7 @@
* - add FUSE_HANDLE_KILLPRIV_V2, FUSE_WRITE_KILL_SUIDGID, FATTR_KILL_SUIDGID
* - add FUSE_OPEN_KILL_SUIDGID
* - add FUSE_SETXATTR_V2
+ * - add FUSE_SETXATTR_ACL_KILL_SGID
*/
#ifndef _LINUX_FUSE_H
@@ -450,6 +451,12 @@ struct fuse_file_lock {
*/
#define FUSE_OPEN_KILL_SUIDGID (1 << 0)
+/**
+ * setxattr flags
+ * FUSE_SETXATTR_ACL_KILL_SGID: Clear SGID when system.posix_acl_access is set
+ */
+#define FUSE_SETXATTR_ACL_KILL_SGID (1 << 0)
+
enum fuse_opcode {
FUSE_LOOKUP = 1,
FUSE_FORGET = 2, /* no reply */
diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c
index 3f5c267604..8a48071d0b 100644
--- a/tools/virtiofsd/passthrough_ll.c
+++ b/tools/virtiofsd/passthrough_ll.c
@@ -175,7 +175,7 @@ struct lo_data {
int user_killpriv_v2, killpriv_v2;
/* If set, virtiofsd is responsible for setting umask during creation */
bool change_umask;
- int user_posix_acl;
+ int user_posix_acl, posix_acl;
};
static const struct fuse_opt lo_opts[] = {
@@ -716,8 +716,10 @@ static void lo_init(void *userdata, struct fuse_conn_info
*conn)
* in fuse_lowlevel.c
*/
fuse_log(FUSE_LOG_DEBUG, "lo_init: enabling posix acl\n");
- conn->want |= FUSE_CAP_POSIX_ACL | FUSE_CAP_DONT_MASK;
+ conn->want |= FUSE_CAP_POSIX_ACL | FUSE_CAP_DONT_MASK |
+ FUSE_CAP_SETXATTR_V2;
lo->change_umask = true;
+ lo->posix_acl = true;
} else {
/* User either did not specify anything or wants it disabled */
fuse_log(FUSE_LOG_DEBUG, "lo_init: disabling posix_acl\n");
@@ -3092,12 +3094,48 @@ static void lo_setxattr(fuse_req_t req, fuse_ino_t ino,
const char *in_name,
sprintf(procname, "%i", inode->fd);
if (S_ISREG(inode->filetype) || S_ISDIR(inode->filetype)) {
+ bool switched_creds = false;
+ struct lo_cred old = {};
+
fd = openat(lo->proc_self_fd, procname, O_RDONLY);
if (fd < 0) {
saverr = errno;
goto out;
}
+
+ /*
+ * If we are setting posix access acl and if SGID needs to be
+ * cleared, then switch to caller's gid and drop CAP_FSETID
+ * and that should make sure host kernel clears SGID.
+ *
+ * This probably will not work when we support idmapped mounts.
+ * In that case we will need to find a non-root gid and switch
+ * to it. (Instead of gid in request). Fix it when we support
+ * idmapped mounts.
+ */
+ if (lo->posix_acl && !strcmp(name, "system.posix_acl_access")
+ && (extra_flags & FUSE_SETXATTR_ACL_KILL_SGID)) {
+ ret = lo_change_cred(req, &old, false);
+ if (ret) {
+ saverr = ret;
+ goto out;
+ }
+ ret = drop_effective_cap("FSETID", NULL);
+ if (ret != 0) {
+ lo_restore_cred(&old, false);
+ saverr = ret;
+ goto out;
+ }
+ switched_creds = true;
+ }
+
ret = fsetxattr(fd, name, value, size, flags);
+
+ if (switched_creds) {
+ if (gain_effective_cap("FSETID"))
+ fuse_log(FUSE_LOG_ERR, "Failed to gain CAP_FSETID\n");
+ lo_restore_cred(&old, false);
+ }
} else {
/* fchdir should not fail here */
assert(fchdir(lo->proc_self_fd) == 0);
--
2.25.4
- [PATCH v5 0/5] virtiofsd: Add support to enable/disable posix acls, Vivek Goyal, 2021/03/25
- [PATCH v5 2/5] virtiofsd: Add capability to change/restore umask, Vivek Goyal, 2021/03/25
- [PATCH v5 4/5] virtiofsd: Add support for setxattr_v2, Vivek Goyal, 2021/03/25
- [PATCH v5 1/5] virtiofsd: Add umask to seccom allow list, Vivek Goyal, 2021/03/25
- [PATCH v5 3/5] virtiofsd: Add an option to enable/disable posix acls, Vivek Goyal, 2021/03/25
- [PATCH v5 5/5] virtiofsd: Switch creds, drop FSETID for system.posix_acl_access xattr,
Vivek Goyal <=
- Re: [PATCH v5 0/5] virtiofsd: Add support to enable/disable posix acls, no-reply, 2021/03/25