qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Daniel P. Berrangé" <berrange@redhat.com>
To: qemu-devel@nongnu.org
Cc: "Kevin Wolf" <kwolf@redhat.com>,
	"Daniel P. Berrangé" <berrange@redhat.com>,
	libvir-list@redhat.com, "Stefan Weil" <sw@weilnetz.de>,
	"Hanna Reitz" <hreitz@redhat.com>,
	"Gerd Hoffmann" <kraxel@redhat.com>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Eric Blake" <eblake@redhat.com>
Subject: [PATCH 2/4] os-posix: refactor code handling the -runas argument
Date: Fri,  4 Mar 2022 11:56:55 +0000	[thread overview]
Message-ID: <20220304115657.3177925-3-berrange@redhat.com> (raw)
In-Reply-To: <20220304115657.3177925-1-berrange@redhat.com>

Change the change_process_uid() function so that it takes its input as
parameters instead of relying on static global variables.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 os-posix.c | 83 +++++++++++++++++++++++++-----------------------------
 1 file changed, 39 insertions(+), 44 deletions(-)

diff --git a/os-posix.c b/os-posix.c
index 7cd662098e..5a127feee2 100644
--- a/os-posix.c
+++ b/os-posix.c
@@ -42,13 +42,9 @@
 #include <sys/prctl.h>
 #endif
 
-/*
- * Must set all three of these at once.
- * Legal combinations are              unset   by name   by uid
- */
-static struct passwd *user_pwd;    /*   NULL   non-NULL   NULL   */
-static uid_t user_uid = (uid_t)-1; /*   -1      -1        >=0    */
-static gid_t user_gid = (gid_t)-1; /*   -1      -1        >=0    */
+static char *user_name;
+static uid_t user_uid = (uid_t)-1;
+static gid_t user_gid = (gid_t)-1;
 
 static const char *chroot_dir;
 static int daemonize;
@@ -100,7 +96,8 @@ void os_set_proc_name(const char *s)
 }
 
 
-static bool os_parse_runas_uid_gid(const char *optarg)
+static bool os_parse_runas_uid_gid(const char *optarg,
+                                   uid_t *runas_uid, gid_t *runas_gid)
 {
     unsigned long lv;
     const char *ep;
@@ -120,9 +117,8 @@ static bool os_parse_runas_uid_gid(const char *optarg)
         return false;
     }
 
-    user_pwd = NULL;
-    user_uid = got_uid;
-    user_gid = got_gid;
+    *runas_uid = got_uid;
+    *runas_gid = got_gid;
     return true;
 }
 
@@ -132,13 +128,18 @@ static bool os_parse_runas_uid_gid(const char *optarg)
  */
 int os_parse_cmd_args(int index, const char *optarg)
 {
+    struct passwd *user_pwd;
+
     switch (index) {
     case QEMU_OPTION_runas:
         user_pwd = getpwnam(optarg);
         if (user_pwd) {
-            user_uid = -1;
-            user_gid = -1;
-        } else if (!os_parse_runas_uid_gid(optarg)) {
+            user_uid = user_pwd->pw_uid;
+            user_gid = user_pwd->pw_gid;
+            user_name = g_strdup(user_pwd->pw_name);
+        } else if (!os_parse_runas_uid_gid(optarg,
+                                           &user_uid,
+                                           &user_gid)) {
             error_report("User \"%s\" doesn't exist"
                          " (and is not <uid>:<gid>)",
                          optarg);
@@ -158,41 +159,33 @@ int os_parse_cmd_args(int index, const char *optarg)
     return 0;
 }
 
-static void change_process_uid(void)
+static void change_process_uid(uid_t uid, gid_t gid, const char *name)
 {
-    assert((user_uid == (uid_t)-1) || user_pwd == NULL);
-    assert((user_uid == (uid_t)-1) ==
-           (user_gid == (gid_t)-1));
-
-    if (user_pwd || user_uid != (uid_t)-1) {
-        gid_t intended_gid = user_pwd ? user_pwd->pw_gid : user_gid;
-        uid_t intended_uid = user_pwd ? user_pwd->pw_uid : user_uid;
-        if (setgid(intended_gid) < 0) {
-            error_report("Failed to setgid(%d)", intended_gid);
-            exit(1);
-        }
-        if (user_pwd) {
-            if (initgroups(user_pwd->pw_name, user_pwd->pw_gid) < 0) {
-                error_report("Failed to initgroups(\"%s\", %d)",
-                        user_pwd->pw_name, user_pwd->pw_gid);
-                exit(1);
-            }
-        } else {
-            if (setgroups(1, &user_gid) < 0) {
-                error_report("Failed to setgroups(1, [%d])",
-                        user_gid);
-                exit(1);
-            }
-        }
-        if (setuid(intended_uid) < 0) {
-            error_report("Failed to setuid(%d)", intended_uid);
+    if (setgid(gid) < 0) {
+        error_report("Failed to setgid(%d)", gid);
+        exit(1);
+    }
+    if (name) {
+        if (initgroups(name, gid) < 0) {
+            error_report("Failed to initgroups(\"%s\", %d)",
+                         name, gid);
             exit(1);
         }
-        if (setuid(0) != -1) {
-            error_report("Dropping privileges failed");
+    } else {
+        if (setgroups(1, &gid) < 0) {
+            error_report("Failed to setgroups(1, [%d])",
+                         gid);
             exit(1);
         }
     }
+    if (setuid(uid) < 0) {
+        error_report("Failed to setuid(%d)", uid);
+        exit(1);
+    }
+    if (setuid(0) != -1) {
+        error_report("Dropping privileges failed");
+        exit(1);
+    }
 }
 
 static void change_root(void)
@@ -275,7 +268,9 @@ void os_setup_post(void)
     }
 
     change_root();
-    change_process_uid();
+    if (user_uid != -1 && user_gid != -1) {
+        change_process_uid(user_uid, user_gid, user_name);
+    }
 
     if (daemonize) {
         uint8_t status = 0;
-- 
2.34.1



  parent reply	other threads:[~2022-03-04 12:00 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-04 11:56 [PATCH 0/4] softmmu: move and refactor -runas, -chroot and -daemonize Daniel P. Berrangé
2022-03-04 11:56 ` [PATCH 1/4] softmmu: remove deprecated --enable-fips option Daniel P. Berrangé
2022-03-04 13:55   ` Philippe Mathieu-Daudé
2022-03-04 17:14   ` Eric Blake
2022-03-04 11:56 ` Daniel P. Berrangé [this message]
2022-03-04 17:19   ` [PATCH 2/4] os-posix: refactor code handling the -runas argument Eric Blake
2022-03-04 11:56 ` [PATCH 3/4] os-posix: refactor code handling the -chroot argument Daniel P. Berrangé
2022-03-04 13:54   ` Philippe Mathieu-Daudé
2022-03-04 11:56 ` [PATCH 4/4] softmmu: move parsing of -runas, -chroot and -daemonize code Daniel P. Berrangé
2022-03-04 14:54   ` Daniel P. Berrangé
2022-03-04 17:21     ` Eric Blake

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220304115657.3177925-3-berrange@redhat.com \
    --to=berrange@redhat.com \
    --cc=eblake@redhat.com \
    --cc=hreitz@redhat.com \
    --cc=kraxel@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=libvir-list@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=sw@weilnetz.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).