|
 |
902636 |
From 8727e4904e7a6588e39f231d837f4527f265e47e Mon Sep 17 00:00:00 2001
|
|
|
902636 |
From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
|
|
|
902636 |
Date: Tue, 5 May 2020 16:35:59 +0100
|
|
|
902636 |
Subject: [PATCH 8/9] virtiofsd: only retain file system capabilities
|
|
|
902636 |
|
|
|
902636 |
RH-Author: Dr. David Alan Gilbert <dgilbert@redhat.com>
|
|
|
902636 |
Message-id: <20200505163600.22956-7-dgilbert@redhat.com>
|
|
|
902636 |
Patchwork-id: 96272
|
|
|
902636 |
O-Subject: [RHEL-AV-8.2.1 qemu-kvm PATCH 6/7] virtiofsd: only retain file system capabilities
|
|
|
902636 |
Bugzilla: 1817445
|
|
|
902636 |
RH-Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
|
|
|
902636 |
RH-Acked-by: Max Reitz <mreitz@redhat.com>
|
|
|
902636 |
RH-Acked-by: Michael S. Tsirkin <mst@redhat.com>
|
|
|
902636 |
|
|
|
902636 |
From: Stefan Hajnoczi <stefanha@redhat.com>
|
|
|
902636 |
|
|
|
902636 |
virtiofsd runs as root but only needs a subset of root's Linux
|
|
|
902636 |
capabilities(7). As a file server its purpose is to create and access
|
|
|
902636 |
files on behalf of a client. It needs to be able to access files with
|
|
|
902636 |
arbitrary uid/gid owners. It also needs to be create device nodes.
|
|
|
902636 |
|
|
|
902636 |
Introduce a Linux capabilities(7) whitelist and drop all capabilities
|
|
|
902636 |
that we don't need, making the virtiofsd process less powerful than a
|
|
|
902636 |
regular uid root process.
|
|
|
902636 |
|
|
|
902636 |
# cat /proc/PID/status
|
|
|
902636 |
...
|
|
|
902636 |
Before After
|
|
|
902636 |
CapInh: 0000000000000000 0000000000000000
|
|
|
902636 |
CapPrm: 0000003fffffffff 00000000880000df
|
|
|
902636 |
CapEff: 0000003fffffffff 00000000880000df
|
|
|
902636 |
CapBnd: 0000003fffffffff 0000000000000000
|
|
|
902636 |
CapAmb: 0000000000000000 0000000000000000
|
|
|
902636 |
|
|
|
902636 |
Note that file capabilities cannot be used to achieve the same effect on
|
|
|
902636 |
the virtiofsd executable because mount is used during sandbox setup.
|
|
|
902636 |
Therefore we drop capabilities programmatically at the right point
|
|
|
902636 |
during startup.
|
|
|
902636 |
|
|
|
902636 |
This patch only affects the sandboxed child process. The parent process
|
|
|
902636 |
that sits in waitpid(2) still has full root capabilities and will be
|
|
|
902636 |
addressed in the next patch.
|
|
|
902636 |
|
|
|
902636 |
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
|
|
|
902636 |
Message-Id: <20200416164907.244868-2-stefanha@redhat.com>
|
|
|
902636 |
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
|
|
|
902636 |
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
|
|
|
902636 |
(cherry picked from commit a59feb483b8fae24d043569ccfcc97ea23d54a02)
|
|
|
902636 |
Signed-off-by: Danilo C. L. de Paula <ddepaula@redhat.com>
|
|
|
902636 |
---
|
|
|
902636 |
tools/virtiofsd/passthrough_ll.c | 38 ++++++++++++++++++++++++++++++++++++++
|
|
|
902636 |
1 file changed, 38 insertions(+)
|
|
|
902636 |
|
|
|
902636 |
diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c
|
|
|
902636 |
index 614ba55..6358874 100644
|
|
|
902636 |
--- a/tools/virtiofsd/passthrough_ll.c
|
|
|
902636 |
+++ b/tools/virtiofsd/passthrough_ll.c
|
|
|
902636 |
@@ -2723,6 +2723,43 @@ static void setup_mounts(const char *source)
|
|
|
902636 |
}
|
|
|
902636 |
|
|
|
902636 |
/*
|
|
|
902636 |
+ * Only keep whitelisted capabilities that are needed for file system operation
|
|
|
902636 |
+ */
|
|
|
902636 |
+static void setup_capabilities(void)
|
|
|
902636 |
+{
|
|
|
902636 |
+ pthread_mutex_lock(&cap.mutex);
|
|
|
902636 |
+ capng_restore_state(&cap.saved);
|
|
|
902636 |
+
|
|
|
902636 |
+ /*
|
|
|
902636 |
+ * Whitelist file system-related capabilities that are needed for a file
|
|
|
902636 |
+ * server to act like root. Drop everything else like networking and
|
|
|
902636 |
+ * sysadmin capabilities.
|
|
|
902636 |
+ *
|
|
|
902636 |
+ * Exclusions:
|
|
|
902636 |
+ * 1. CAP_LINUX_IMMUTABLE is not included because it's only used via ioctl
|
|
|
902636 |
+ * and we don't support that.
|
|
|
902636 |
+ * 2. CAP_MAC_OVERRIDE is not included because it only seems to be
|
|
|
902636 |
+ * used by the Smack LSM. Omit it until there is demand for it.
|
|
|
902636 |
+ */
|
|
|
902636 |
+ capng_setpid(syscall(SYS_gettid));
|
|
|
902636 |
+ capng_clear(CAPNG_SELECT_BOTH);
|
|
|
902636 |
+ capng_updatev(CAPNG_ADD, CAPNG_PERMITTED | CAPNG_EFFECTIVE,
|
|
|
902636 |
+ CAP_CHOWN,
|
|
|
902636 |
+ CAP_DAC_OVERRIDE,
|
|
|
902636 |
+ CAP_DAC_READ_SEARCH,
|
|
|
902636 |
+ CAP_FOWNER,
|
|
|
902636 |
+ CAP_FSETID,
|
|
|
902636 |
+ CAP_SETGID,
|
|
|
902636 |
+ CAP_SETUID,
|
|
|
902636 |
+ CAP_MKNOD,
|
|
|
902636 |
+ CAP_SETFCAP);
|
|
|
902636 |
+ capng_apply(CAPNG_SELECT_BOTH);
|
|
|
902636 |
+
|
|
|
902636 |
+ cap.saved = capng_save_state();
|
|
|
902636 |
+ pthread_mutex_unlock(&cap.mutex);
|
|
|
902636 |
+}
|
|
|
902636 |
+
|
|
|
902636 |
+/*
|
|
|
902636 |
* Lock down this process to prevent access to other processes or files outside
|
|
|
902636 |
* source directory. This reduces the impact of arbitrary code execution bugs.
|
|
|
902636 |
*/
|
|
|
902636 |
@@ -2732,6 +2769,7 @@ static void setup_sandbox(struct lo_data *lo, struct fuse_session *se,
|
|
|
902636 |
setup_namespaces(lo, se);
|
|
|
902636 |
setup_mounts(lo->source);
|
|
|
902636 |
setup_seccomp(enable_syslog);
|
|
|
902636 |
+ setup_capabilities();
|
|
|
902636 |
}
|
|
|
902636 |
|
|
|
902636 |
/* Set the maximum number of open file descriptors */
|
|
|
902636 |
--
|
|
|
902636 |
1.8.3.1
|
|
|
902636 |
|