[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PATCH v2 19/20] 9p: darwin: virtfs-proxy: Implement setuid
From: |
Keno Fischer |
Subject: |
[Qemu-devel] [PATCH v2 19/20] 9p: darwin: virtfs-proxy: Implement setuid code for darwin |
Date: |
Thu, 31 May 2018 21:26:14 -0400 |
Darwin does not have linux capabilities, so make that code linux-only.
Darwin also does not have setresuid/gid. The correct way to temporarily
drop capabilities is to call seteuid/gid.
Also factor out the code that acquires acquire_dac_override into a separate
function in the linux implementation. I had originally done this when
I thought it made sense to have only one `setugid` function, but I retained
this because it seems clearer this way.
Signed-off-by: Keno Fischer <address@hidden>
---
Changes from v1: New patch.
fsdev/virtfs-proxy-helper.c | 200 +++++++++++++++++++++++++++-----------------
1 file changed, 125 insertions(+), 75 deletions(-)
diff --git a/fsdev/virtfs-proxy-helper.c b/fsdev/virtfs-proxy-helper.c
index d8dd3f5..6baf2a6 100644
--- a/fsdev/virtfs-proxy-helper.c
+++ b/fsdev/virtfs-proxy-helper.c
@@ -82,6 +82,7 @@ static void do_perror(const char *string)
}
}
+#ifdef CONFIG_LINUX
static int do_cap_set(cap_value_t *cap_value, int size, int reset)
{
cap_t caps;
@@ -121,6 +122,85 @@ error:
return -1;
}
+static int acquire_dac_override(void)
+{
+ cap_value_t cap_list[] = {
+ CAP_DAC_OVERRIDE,
+ };
+ return do_cap_set(cap_list, ARRAY_SIZE(cap_list), 0);
+}
+
+/*
+ * from man 7 capabilities, section
+ * Effect of User ID Changes on Capabilities:
+ * If the effective user ID is changed from nonzero to 0, then the permitted
+ * set is copied to the effective set. If the effective user ID is changed
+ * from 0 to nonzero, then all capabilities are are cleared from the effective
+ * set.
+ *
+ * The setfsuid/setfsgid man pages warn that changing the effective user ID may
+ * expose the program to unwanted signals, but this is not true anymore: for an
+ * unprivileged (without CAP_KILL) program to send a signal, the real or
+ * effective user ID of the sending process must equal the real or saved user
+ * ID of the target process. Even when dropping privileges, it is enough to
+ * keep the saved UID to a "privileged" value and virtfs-proxy-helper won't
+ * be exposed to signals. So just use setresuid/setresgid.
+ */
+static int setugid(int uid, int gid, int *suid, int *sgid)
+{
+ int retval;
+
+ *suid = geteuid();
+ *sgid = getegid();
+
+ if (setresgid(-1, gid, *sgid) == -1) {
+ retval = -errno;
+ goto err_out;
+ }
+
+ if (setresuid(-1, uid, *suid) == -1) {
+ retval = -errno;
+ goto err_sgid;
+ }
+
+ if (uid != 0 || gid != 0) {
+ /*
+ * We still need DAC_OVERRIDE because we don't change
+ * supplementary group ids, and hence may be subjected DAC rules
+ */
+ if (acquire_dac_override() < 0) {
+ retval = -errno;
+ goto err_suid;
+ }
+ }
+ return 0;
+
+err_suid:
+ if (setresuid(-1, *suid, *suid) == -1) {
+ abort();
+ }
+err_sgid:
+ if (setresgid(-1, *sgid, *sgid) == -1) {
+ abort();
+ }
+err_out:
+ return retval;
+}
+
+/*
+ * This is used to reset the ugid back with the saved values
+ * There is nothing much we can do checking error values here.
+ */
+static void resetugid(int suid, int sgid)
+{
+ if (setresgid(-1, sgid, sgid) == -1) {
+ abort();
+ }
+ if (setresuid(-1, suid, suid) == -1) {
+ abort();
+ }
+}
+
static int init_capabilities(void)
{
/* helper needs following capabilities only */
@@ -135,6 +215,51 @@ static int init_capabilities(void)
};
return do_cap_set(cap_list, ARRAY_SIZE(cap_list), 1);
}
+#else
+static int setugid(int uid, int gid, int *suid, int *sgid)
+{
+ int retval;
+
+ *suid = geteuid();
+ *sgid = getegid();
+
+ if (setegid(gid) == -1) {
+ retval = -errno;
+ goto err_out;
+ }
+
+ if (seteuid(uid) == -1) {
+ retval = -errno;
+ goto err_sgid;
+ }
+
+err_sgid:
+ if (setgid(*sgid) == -1) {
+ abort();
+ }
+err_out:
+ return retval;
+}
+
+/*
+ * This is used to reset the ugid back with the saved values
+ * There is nothing much we can do checking error values here.
+ */
+static void resetugid(int suid, int sgid)
+{
+ if (setegid(sgid) == -1) {
+ abort();
+ }
+ if (seteuid(suid) == -1) {
+ abort();
+ }
+}
+
+static int init_capabilities(void)
+{
+ return 0;
+}
+#endif
static int socket_read(int sockfd, void *buff, ssize_t size)
{
@@ -279,81 +404,6 @@ static int send_status(int sockfd, struct iovec *iovec,
int status)
}
/*
- * from man 7 capabilities, section
- * Effect of User ID Changes on Capabilities:
- * If the effective user ID is changed from nonzero to 0, then the permitted
- * set is copied to the effective set. If the effective user ID is changed
- * from 0 to nonzero, then all capabilities are are cleared from the effective
- * set.
- *
- * The setfsuid/setfsgid man pages warn that changing the effective user ID may
- * expose the program to unwanted signals, but this is not true anymore: for an
- * unprivileged (without CAP_KILL) program to send a signal, the real or
- * effective user ID of the sending process must equal the real or saved user
- * ID of the target process. Even when dropping privileges, it is enough to
- * keep the saved UID to a "privileged" value and virtfs-proxy-helper won't
- * be exposed to signals. So just use setresuid/setresgid.
- */
-static int setugid(int uid, int gid, int *suid, int *sgid)
-{
- int retval;
-
- /*
- * We still need DAC_OVERRIDE because we don't change
- * supplementary group ids, and hence may be subjected DAC rules
- */
- cap_value_t cap_list[] = {
- CAP_DAC_OVERRIDE,
- };
-
- *suid = geteuid();
- *sgid = getegid();
-
- if (setresgid(-1, gid, *sgid) == -1) {
- retval = -errno;
- goto err_out;
- }
-
- if (setresuid(-1, uid, *suid) == -1) {
- retval = -errno;
- goto err_sgid;
- }
-
- if (uid != 0 || gid != 0) {
- if (do_cap_set(cap_list, ARRAY_SIZE(cap_list), 0) < 0) {
- retval = -errno;
- goto err_suid;
- }
- }
- return 0;
-
-err_suid:
- if (setresuid(-1, *suid, *suid) == -1) {
- abort();
- }
-err_sgid:
- if (setresgid(-1, *sgid, *sgid) == -1) {
- abort();
- }
-err_out:
- return retval;
-}
-
-/*
- * This is used to reset the ugid back with the saved values
- * There is nothing much we can do checking error values here.
- */
-static void resetugid(int suid, int sgid)
-{
- if (setresgid(-1, sgid, sgid) == -1) {
- abort();
- }
- if (setresuid(-1, suid, suid) == -1) {
- abort();
- }
-}
-
-/*
* send response in two parts
* 1) ProxyHeader
* 2) Response or error status
--
2.8.1
- [Qemu-devel] [PATCH v2 04/20] 9p: linux: Fix a couple Linux assumptions, (continued)
- [Qemu-devel] [PATCH v2 04/20] 9p: linux: Fix a couple Linux assumptions, Keno Fischer, 2018/05/31
- [Qemu-devel] [PATCH v2 07/20] 9p: Move a couple xattr functions to 9p-util, Keno Fischer, 2018/05/31
- [Qemu-devel] [PATCH v2 11/20] 9p: darwin: Handle struct dirent differences, Keno Fischer, 2018/05/31
- [Qemu-devel] [PATCH v2 08/20] 9p: Rename 9p-util -> 9p-util-linux, Keno Fischer, 2018/05/31
- [Qemu-devel] [PATCH v2 09/20] 9p: Properly check/translate flags in unlinkat, Keno Fischer, 2018/05/31
- [Qemu-devel] [PATCH v2 10/20] 9p: darwin: Handle struct stat(fs) differences, Keno Fischer, 2018/05/31
- [Qemu-devel] [PATCH v2 12/20] 9p: darwin: Explicitly cast comparisons of mode_t with -1, Keno Fischer, 2018/05/31
- [Qemu-devel] [PATCH v2 14/20] 9p: darwin: Provide a compatibility definition for XATTR_SIZE_MAX, Keno Fischer, 2018/05/31
- [Qemu-devel] [PATCH v2 13/20] 9p: darwin: Ignore O_{NOATIME, DIRECT}, Keno Fischer, 2018/05/31
- [Qemu-devel] [PATCH v2 18/20] 9p: darwin: Implement compatibility for mknodat, Keno Fischer, 2018/05/31
- [Qemu-devel] [PATCH v2 19/20] 9p: darwin: virtfs-proxy: Implement setuid code for darwin,
Keno Fischer <=
- [Qemu-devel] [PATCH v2 15/20] 9p: darwin: *xattr_nofollow implementations, Keno Fischer, 2018/05/31
- [Qemu-devel] [PATCH v2 20/20] 9p: darwin: configure: Allow VirtFS on Darwin, Keno Fischer, 2018/05/31
- [Qemu-devel] [PATCH v2 16/20] 9p: darwin: Compatibility for f/l*xattr, Keno Fischer, 2018/05/31
- [Qemu-devel] [PATCH v2 17/20] 9p: darwin: Provide a fallback implementation for utimensat, Keno Fischer, 2018/05/31