* [PATCH 1/4] softmmu: remove deprecated --enable-fips option
2022-03-04 11:56 [PATCH 0/4] softmmu: move and refactor -runas, -chroot and -daemonize Daniel P. Berrangé
@ 2022-03-04 11:56 ` Daniel P. Berrangé
2022-03-04 13:55 ` Philippe Mathieu-Daudé
2022-03-04 17:14 ` Eric Blake
2022-03-04 11:56 ` [PATCH 2/4] os-posix: refactor code handling the -runas argument Daniel P. Berrangé
` (2 subsequent siblings)
3 siblings, 2 replies; 11+ messages in thread
From: Daniel P. Berrangé @ 2022-03-04 11:56 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Daniel P. Berrangé, libvir-list, Stefan Weil,
Hanna Reitz, Gerd Hoffmann, Paolo Bonzini, Eric Blake
Users requiring FIPS support must build QEMU with either the libgcrypt
or gnutls libraries for as the crytography backend.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
docs/about/deprecated.rst | 12 ------------
docs/about/removed-features.rst | 11 +++++++++++
include/qemu/osdep.h | 3 ---
os-posix.c | 8 --------
qemu-options.hx | 10 ----------
ui/vnc.c | 7 -------
util/osdep.c | 28 ----------------------------
7 files changed, 11 insertions(+), 68 deletions(-)
diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 26d00812ba..a458dd453c 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -67,18 +67,6 @@ and will cause a warning.
The replacement for the ``nodelay`` short-form boolean option is ``nodelay=on``
rather than ``delay=off``.
-``--enable-fips`` (since 6.0)
-'''''''''''''''''''''''''''''
-
-This option restricts usage of certain cryptographic algorithms when
-the host is operating in FIPS mode.
-
-If FIPS compliance is required, QEMU should be built with the ``libgcrypt``
-library enabled as a cryptography provider.
-
-Neither the ``nettle`` library, or the built-in cryptography provider are
-supported on FIPS enabled hosts.
-
``-writeconfig`` (since 6.0)
'''''''''''''''''''''''''''''
diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst
index cb0575fd49..6ca66f658d 100644
--- a/docs/about/removed-features.rst
+++ b/docs/about/removed-features.rst
@@ -336,6 +336,17 @@ for the RISC-V ``virt`` machine and ``sifive_u`` machine.
The ``-no-quit`` was a synonym for ``-display ...,window-close=off`` which
should be used instead.
+``--enable-fips`` (removed in 7.0)
+''''''''''''''''''''''''''''''''''
+
+This option restricted usage of certain cryptographic algorithms when
+the host is operating in FIPS mode.
+
+If FIPS compliance is required, QEMU should be built with the ``libgcrypt``
+or ``gnutls`` library enabled as a cryptography provider.
+
+Neither the ``nettle`` library, or the built-in cryptography provider are
+supported on FIPS enabled hosts.
QEMU Machine Protocol (QMP) commands
------------------------------------
diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index 7bcce3bceb..66e70e24ff 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -534,9 +534,6 @@ static inline void qemu_timersub(const struct timeval *val1,
void qemu_set_cloexec(int fd);
-void fips_set_state(bool requested);
-bool fips_get_state(void);
-
/* Return a dynamically allocated pathname denoting a file or directory that is
* appropriate for storing local state.
*
diff --git a/os-posix.c b/os-posix.c
index ae6c9f2a5e..7cd662098e 100644
--- a/os-posix.c
+++ b/os-posix.c
@@ -151,14 +151,6 @@ int os_parse_cmd_args(int index, const char *optarg)
case QEMU_OPTION_daemonize:
daemonize = 1;
break;
-#if defined(CONFIG_LINUX)
- case QEMU_OPTION_enablefips:
- warn_report("-enable-fips is deprecated, please build QEMU with "
- "the `libgcrypt` library as the cryptography provider "
- "to enable FIPS compliance");
- fips_set_state(true);
- break;
-#endif
default:
return -1;
}
diff --git a/qemu-options.hx b/qemu-options.hx
index 094a6c1d7c..cb0c58904b 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -4655,16 +4655,6 @@ HXCOMM Internal use
DEF("qtest", HAS_ARG, QEMU_OPTION_qtest, "", QEMU_ARCH_ALL)
DEF("qtest-log", HAS_ARG, QEMU_OPTION_qtest_log, "", QEMU_ARCH_ALL)
-#ifdef __linux__
-DEF("enable-fips", 0, QEMU_OPTION_enablefips,
- "-enable-fips enable FIPS 140-2 compliance\n",
- QEMU_ARCH_ALL)
-#endif
-SRST
-``-enable-fips``
- Enable FIPS 140-2 compliance mode.
-ERST
-
DEF("msg", HAS_ARG, QEMU_OPTION_msg,
"-msg [timestamp[=on|off]][,guest-name=[on|off]]\n"
" control error message format\n"
diff --git a/ui/vnc.c b/ui/vnc.c
index 3ccd33dedc..82b28aec95 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -4051,13 +4051,6 @@ void vnc_display_open(const char *id, Error **errp)
password = qemu_opt_get_bool(opts, "password", false);
}
if (password) {
- if (fips_get_state()) {
- error_setg(errp,
- "VNC password auth disabled due to FIPS mode, "
- "consider using the VeNCrypt or SASL authentication "
- "methods as an alternative");
- goto fail;
- }
if (!qcrypto_cipher_supports(
QCRYPTO_CIPHER_ALG_DES, QCRYPTO_CIPHER_MODE_ECB)) {
error_setg(errp,
diff --git a/util/osdep.c b/util/osdep.c
index 723cdcb004..456df9e81a 100644
--- a/util/osdep.c
+++ b/util/osdep.c
@@ -43,8 +43,6 @@ extern int madvise(char *, size_t, int);
#include "qemu/hw-version.h"
#include "monitor/monitor.h"
-static bool fips_enabled = false;
-
static const char *hw_version = QEMU_HW_VERSION;
int socket_set_cork(int fd, int v)
@@ -526,32 +524,6 @@ const char *qemu_hw_version(void)
return hw_version;
}
-void fips_set_state(bool requested)
-{
-#ifdef __linux__
- if (requested) {
- FILE *fds = fopen("/proc/sys/crypto/fips_enabled", "r");
- if (fds != NULL) {
- fips_enabled = (fgetc(fds) == '1');
- fclose(fds);
- }
- }
-#else
- fips_enabled = false;
-#endif /* __linux__ */
-
-#ifdef _FIPS_DEBUG
- fprintf(stderr, "FIPS mode %s (requested %s)\n",
- (fips_enabled ? "enabled" : "disabled"),
- (requested ? "enabled" : "disabled"));
-#endif
-}
-
-bool fips_get_state(void)
-{
- return fips_enabled;
-}
-
#ifdef _WIN32
static void socket_cleanup(void)
{
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 1/4] softmmu: remove deprecated --enable-fips option
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
1 sibling, 0 replies; 11+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-03-04 13:55 UTC (permalink / raw)
To: Daniel P. Berrangé, qemu-devel
Cc: Kevin Wolf, libvir-list, Stefan Weil, Hanna Reitz, Gerd Hoffmann,
Paolo Bonzini, Eric Blake
On 4/3/22 12:56, Daniel P. Berrangé wrote:
> Users requiring FIPS support must build QEMU with either the libgcrypt
> or gnutls libraries for as the crytography backend.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> docs/about/deprecated.rst | 12 ------------
> docs/about/removed-features.rst | 11 +++++++++++
> include/qemu/osdep.h | 3 ---
> os-posix.c | 8 --------
> qemu-options.hx | 10 ----------
> ui/vnc.c | 7 -------
> util/osdep.c | 28 ----------------------------
> 7 files changed, 11 insertions(+), 68 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/4] softmmu: remove deprecated --enable-fips option
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
1 sibling, 0 replies; 11+ messages in thread
From: Eric Blake @ 2022-03-04 17:14 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: Kevin Wolf, libvir-list, Stefan Weil, qemu-devel, Hanna Reitz,
Gerd Hoffmann, Paolo Bonzini
On Fri, Mar 04, 2022 at 11:56:54AM +0000, Daniel P. Berrangé wrote:
> Users requiring FIPS support must build QEMU with either the libgcrypt
> or gnutls libraries for as the crytography backend.
s/for //
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> docs/about/deprecated.rst | 12 ------------
> docs/about/removed-features.rst | 11 +++++++++++
> include/qemu/osdep.h | 3 ---
> os-posix.c | 8 --------
> qemu-options.hx | 10 ----------
> ui/vnc.c | 7 -------
> util/osdep.c | 28 ----------------------------
> 7 files changed, 11 insertions(+), 68 deletions(-)
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/4] os-posix: refactor code handling the -runas argument
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 11:56 ` Daniel P. Berrangé
2022-03-04 17:19 ` Eric Blake
2022-03-04 11:56 ` [PATCH 3/4] os-posix: refactor code handling the -chroot argument Daniel P. Berrangé
2022-03-04 11:56 ` [PATCH 4/4] softmmu: move parsing of -runas, -chroot and -daemonize code Daniel P. Berrangé
3 siblings, 1 reply; 11+ messages in thread
From: Daniel P. Berrangé @ 2022-03-04 11:56 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Daniel P. Berrangé, libvir-list, Stefan Weil,
Hanna Reitz, Gerd Hoffmann, Paolo Bonzini, Eric Blake
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
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 2/4] os-posix: refactor code handling the -runas argument
2022-03-04 11:56 ` [PATCH 2/4] os-posix: refactor code handling the -runas argument Daniel P. Berrangé
@ 2022-03-04 17:19 ` Eric Blake
0 siblings, 0 replies; 11+ messages in thread
From: Eric Blake @ 2022-03-04 17:19 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: Kevin Wolf, libvir-list, Stefan Weil, qemu-devel, Hanna Reitz,
Gerd Hoffmann, Paolo Bonzini
On Fri, Mar 04, 2022 at 11:56:55AM +0000, Daniel P. Berrangé wrote:
> 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(-)
>
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 3/4] os-posix: refactor code handling the -chroot argument
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 11:56 ` [PATCH 2/4] os-posix: refactor code handling the -runas argument Daniel P. Berrangé
@ 2022-03-04 11:56 ` 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é
3 siblings, 1 reply; 11+ messages in thread
From: Daniel P. Berrangé @ 2022-03-04 11:56 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Daniel P. Berrangé, libvir-list, Stefan Weil,
Hanna Reitz, Gerd Hoffmann, Paolo Bonzini, Eric Blake
Change the change_root() 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 | 23 +++++++++++------------
1 file changed, 11 insertions(+), 12 deletions(-)
diff --git a/os-posix.c b/os-posix.c
index 5a127feee2..30da1a1491 100644
--- a/os-posix.c
+++ b/os-posix.c
@@ -188,19 +188,16 @@ static void change_process_uid(uid_t uid, gid_t gid, const char *name)
}
}
-static void change_root(void)
+static void change_root(const char *root)
{
- if (chroot_dir) {
- if (chroot(chroot_dir) < 0) {
- error_report("chroot failed");
- exit(1);
- }
- if (chdir("/")) {
- error_report("not able to chdir to /: %s", strerror(errno));
- exit(1);
- }
+ if (chroot(root) < 0) {
+ error_report("chroot failed");
+ exit(1);
+ }
+ if (chdir("/")) {
+ error_report("not able to chdir to /: %s", strerror(errno));
+ exit(1);
}
-
}
void os_daemonize(void)
@@ -267,7 +264,9 @@ void os_setup_post(void)
}
}
- change_root();
+ if (chroot_dir) {
+ change_root(chroot_dir);
+ }
if (user_uid != -1 && user_gid != -1) {
change_process_uid(user_uid, user_gid, user_name);
}
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 3/4] os-posix: refactor code handling the -chroot argument
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é
0 siblings, 0 replies; 11+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-03-04 13:54 UTC (permalink / raw)
To: Daniel P. Berrangé, qemu-devel
Cc: Kevin Wolf, libvir-list, Stefan Weil, Hanna Reitz, Gerd Hoffmann,
Paolo Bonzini, Eric Blake
On 4/3/22 12:56, Daniel P. Berrangé wrote:
> Change the change_root() 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 | 23 +++++++++++------------
> 1 file changed, 11 insertions(+), 12 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 4/4] softmmu: move parsing of -runas, -chroot and -daemonize code
2022-03-04 11:56 [PATCH 0/4] softmmu: move and refactor -runas, -chroot and -daemonize Daniel P. Berrangé
` (2 preceding siblings ...)
2022-03-04 11:56 ` [PATCH 3/4] os-posix: refactor code handling the -chroot argument Daniel P. Berrangé
@ 2022-03-04 11:56 ` Daniel P. Berrangé
2022-03-04 14:54 ` Daniel P. Berrangé
3 siblings, 1 reply; 11+ messages in thread
From: Daniel P. Berrangé @ 2022-03-04 11:56 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Daniel P. Berrangé, libvir-list, Stefan Weil,
Hanna Reitz, Gerd Hoffmann, Paolo Bonzini, Eric Blake
With the future intent to try to move to a fully QAPI driven
configuration system, we want to have any current command
parsing well isolated from logic that applies the resulting
configuration.
We also don't want os-posix.c to contain code that is specific
to the system emulators, as this file is linked to other binaries
too.
To satisfy these goals, we move parsing of the -runas, -chroot and
-daemonize code out of the os-posix.c helper code, and pass the
parsed data into APIs in os-posix.c.
As a side benefit the 'os_daemonize()' function now lives upto to
its name and will always daemonize, instead of using global state
to decide to be a no-op sometimes.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
include/sysemu/os-posix.h | 4 +-
include/sysemu/os-win32.h | 1 -
os-posix.c | 148 +++++++++++---------------------------
os-win32.c | 9 ---
softmmu/vl.c | 76 ++++++++++++++++++--
5 files changed, 113 insertions(+), 125 deletions(-)
diff --git a/include/sysemu/os-posix.h b/include/sysemu/os-posix.h
index 2edf33658a..390f3f8396 100644
--- a/include/sysemu/os-posix.h
+++ b/include/sysemu/os-posix.h
@@ -46,7 +46,9 @@ void os_set_line_buffering(void);
void os_set_proc_name(const char *s);
void os_setup_signal_handling(void);
void os_daemonize(void);
-void os_setup_post(void);
+void os_setup_post(const char *chroot_dir,
+ uid_t uid, gid_t gid,
+ const char *username);
int os_mlock(void);
#define closesocket(s) close(s)
diff --git a/include/sysemu/os-win32.h b/include/sysemu/os-win32.h
index 43f569b5c2..4879f8731d 100644
--- a/include/sysemu/os-win32.h
+++ b/include/sysemu/os-win32.h
@@ -61,7 +61,6 @@ struct tm *localtime_r(const time_t *timep, struct tm *result);
static inline void os_setup_signal_handling(void) {}
static inline void os_daemonize(void) {}
-static inline void os_setup_post(void) {}
void os_set_line_buffering(void);
static inline void os_set_proc_name(const char *dummy) {}
diff --git a/os-posix.c b/os-posix.c
index 30da1a1491..f598a52a4f 100644
--- a/os-posix.c
+++ b/os-posix.c
@@ -42,11 +42,6 @@
#include <sys/prctl.h>
#endif
-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;
static int daemon_pipe;
@@ -96,69 +91,6 @@ void os_set_proc_name(const char *s)
}
-static bool os_parse_runas_uid_gid(const char *optarg,
- uid_t *runas_uid, gid_t *runas_gid)
-{
- unsigned long lv;
- const char *ep;
- uid_t got_uid;
- gid_t got_gid;
- int rc;
-
- rc = qemu_strtoul(optarg, &ep, 0, &lv);
- got_uid = lv; /* overflow here is ID in C99 */
- if (rc || *ep != ':' || got_uid != lv || got_uid == (uid_t)-1) {
- return false;
- }
-
- rc = qemu_strtoul(ep + 1, 0, 0, &lv);
- got_gid = lv; /* overflow here is ID in C99 */
- if (rc || got_gid != lv || got_gid == (gid_t)-1) {
- return false;
- }
-
- *runas_uid = got_uid;
- *runas_gid = got_gid;
- return true;
-}
-
-/*
- * Parse OS specific command line options.
- * return 0 if option handled, -1 otherwise
- */
-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 = 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);
- exit(1);
- }
- break;
- case QEMU_OPTION_chroot:
- chroot_dir = optarg;
- break;
- case QEMU_OPTION_daemonize:
- daemonize = 1;
- break;
- default:
- return -1;
- }
-
- return 0;
-}
-
static void change_process_uid(uid_t uid, gid_t gid, const char *name)
{
if (setgid(gid) < 0) {
@@ -202,54 +134,56 @@ static void change_root(const char *root)
void os_daemonize(void)
{
- if (daemonize) {
- pid_t pid;
- int fds[2];
+ pid_t pid;
+ int fds[2];
- if (pipe(fds) == -1) {
- exit(1);
- }
+ if (pipe(fds) == -1) {
+ exit(1);
+ }
- pid = fork();
- if (pid > 0) {
- uint8_t status;
- ssize_t len;
+ pid = fork();
+ if (pid > 0) {
+ uint8_t status;
+ ssize_t len;
- close(fds[1]);
+ close(fds[1]);
- do {
- len = read(fds[0], &status, 1);
- } while (len < 0 && errno == EINTR);
+ do {
+ len = read(fds[0], &status, 1);
+ } while (len < 0 && errno == EINTR);
- /* only exit successfully if our child actually wrote
- * a one-byte zero to our pipe, upon successful init */
- exit(len == 1 && status == 0 ? 0 : 1);
+ /* only exit successfully if our child actually wrote
+ * a one-byte zero to our pipe, upon successful init */
+ exit(len == 1 && status == 0 ? 0 : 1);
- } else if (pid < 0) {
- exit(1);
- }
+ } else if (pid < 0) {
+ exit(1);
+ }
- close(fds[0]);
- daemon_pipe = fds[1];
- qemu_set_cloexec(daemon_pipe);
+ close(fds[0]);
+ daemon_pipe = fds[1];
+ qemu_set_cloexec(daemon_pipe);
- setsid();
+ setsid();
- pid = fork();
- if (pid > 0) {
+ pid = fork();
+ if (pid > 0) {
exit(0);
- } else if (pid < 0) {
- exit(1);
- }
- umask(027);
-
- signal(SIGTSTP, SIG_IGN);
- signal(SIGTTOU, SIG_IGN);
- signal(SIGTTIN, SIG_IGN);
+ } else if (pid < 0) {
+ exit(1);
}
+ umask(027);
+
+ signal(SIGTSTP, SIG_IGN);
+ signal(SIGTTOU, SIG_IGN);
+ signal(SIGTTIN, SIG_IGN);
+
+ daemonize = true;
}
-void os_setup_post(void)
+void os_setup_post(const char *root_dir,
+ uid_t runas_uid, gid_t runas_gid,
+ const char *runas_name)
{
int fd = 0;
@@ -264,11 +198,11 @@ void os_setup_post(void)
}
}
- if (chroot_dir) {
- change_root(chroot_dir);
+ if (root_dir != NULL) {
+ change_root(root_dir);
}
- if (user_uid != -1 && user_gid != -1) {
- change_process_uid(user_uid, user_gid, user_name);
+ if (runas_uid != -1 && runas_gid != -1) {
+ change_process_uid(runas_uid, runas_gid, runas_name);
}
if (daemonize) {
diff --git a/os-win32.c b/os-win32.c
index e31c921983..6f21b57841 100644
--- a/os-win32.c
+++ b/os-win32.c
@@ -61,12 +61,3 @@ void os_set_line_buffering(void)
setbuf(stdout, NULL);
setbuf(stderr, NULL);
}
-
-/*
- * Parse OS specific command line options.
- * return 0 if option handled, -1 otherwise
- */
-int os_parse_cmd_args(int index, const char *optarg)
-{
- return -1;
-}
diff --git a/softmmu/vl.c b/softmmu/vl.c
index 1fe028800f..cdf27b6027 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -2602,11 +2602,13 @@ static void qemu_process_help_options(void)
}
}
-static void qemu_maybe_daemonize(const char *pid_file)
+static void qemu_maybe_daemonize(bool daemonize, const char *pid_file)
{
Error *err = NULL;
- os_daemonize();
+ if (daemonize) {
+ os_daemonize();
+ }
rcu_disable_atfork();
if (pid_file && !qemu_write_pidfile(pid_file, &err)) {
@@ -2768,6 +2770,35 @@ void qmp_x_exit_preconfig(Error **errp)
}
}
+#ifndef WIN32
+static bool os_parse_runas_uid_gid(const char *optarg,
+ uid_t *runas_uid,
+ gid_t *runas_gid)
+{
+ unsigned long lv;
+ const char *ep;
+ uid_t got_uid;
+ gid_t got_gid;
+ int rc;
+
+ rc = qemu_strtoul(optarg, &ep, 0, &lv);
+ got_uid = lv; /* overflow here is ID in C99 */
+ if (rc || *ep != ':' || got_uid != lv || got_uid == (uid_t)-1) {
+ return false;
+ }
+
+ rc = qemu_strtoul(ep + 1, 0, 0, &lv);
+ got_gid = lv; /* overflow here is ID in C99 */
+ if (rc || got_gid != lv || got_gid == (gid_t)-1) {
+ return false;
+ }
+
+ *runas_gid = got_gid;
+ *runas_uid = got_uid;
+ return true;
+}
+#endif /* !WIN32 */
+
void qemu_init(int argc, char **argv, char **envp)
{
QemuOpts *opts;
@@ -2778,6 +2809,14 @@ void qemu_init(int argc, char **argv, char **envp)
MachineClass *machine_class;
bool userconfig = true;
FILE *vmstate_dump_file = NULL;
+ bool daemonize = false;
+#ifndef WIN32
+ struct passwd *pwd;
+ uid_t runas_uid = -1;
+ gid_t runas_gid = -1;
+ g_autofree char *runas_name = NULL;
+ const char *chroot_dir = NULL;
+#endif /* !WIN32 */
qemu_add_opts(&qemu_drive_opts);
qemu_add_drive_opts(&qemu_legacy_drive_opts);
@@ -3659,11 +3698,32 @@ void qemu_init(int argc, char **argv, char **envp)
case QEMU_OPTION_nouserconfig:
/* Nothing to be parsed here. Especially, do not error out below. */
break;
- default:
- if (os_parse_cmd_args(popt->index, optarg)) {
- error_report("Option not supported in this build");
+#ifndef WIN32
+ case QEMU_OPTION_runas:
+ pwd = getpwnam(optarg);
+ if (pwd) {
+ runas_uid = pwd->pw_uid;
+ runas_gid = pwd->pw_gid;
+ runas_name = g_strdup(pwd->pw_name);
+ } else if (!os_parse_runas_uid_gid(optarg,
+ &runas_uid,
+ &runas_gid)) {
+ error_report("User \"%s\" doesn't exist"
+ " (and is not <uid>:<gid>)",
+ optarg);
exit(1);
}
+ break;
+ case QEMU_OPTION_chroot:
+ chroot_dir = optarg;
+ break;
+ case QEMU_OPTION_daemonize:
+ daemonize = 1;
+ break;
+#endif /* !WIN32 */
+ default:
+ error_report("Option not supported in this build");
+ exit(1);
}
}
}
@@ -3683,7 +3743,7 @@ void qemu_init(int argc, char **argv, char **envp)
qemu_process_early_options();
qemu_process_help_options();
- qemu_maybe_daemonize(pid_file);
+ qemu_maybe_daemonize(daemonize, pid_file);
/*
* The trace backend must be initialized after daemonizing.
@@ -3778,6 +3838,8 @@ void qemu_init(int argc, char **argv, char **envp)
}
qemu_init_displays();
accel_setup_post(current_machine);
- os_setup_post();
+#ifndef WIN32
+ os_setup_post(chroot_dir, runas_uid, runas_gid, runas_name);
+#endif /* !WIN32 */
resume_mux_open();
}
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 4/4] softmmu: move parsing of -runas, -chroot and -daemonize code
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
0 siblings, 1 reply; 11+ messages in thread
From: Daniel P. Berrangé @ 2022-03-04 14:54 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, libvir-list, Stefan Weil, Hanna Reitz, Gerd Hoffmann,
Paolo Bonzini, Eric Blake
On Fri, Mar 04, 2022 at 11:56:57AM +0000, Daniel P. Berrangé wrote:
> With the future intent to try to move to a fully QAPI driven
> configuration system, we want to have any current command
> parsing well isolated from logic that applies the resulting
> configuration.
>
> We also don't want os-posix.c to contain code that is specific
> to the system emulators, as this file is linked to other binaries
> too.
>
> To satisfy these goals, we move parsing of the -runas, -chroot and
> -daemonize code out of the os-posix.c helper code, and pass the
> parsed data into APIs in os-posix.c.
>
> As a side benefit the 'os_daemonize()' function now lives upto to
> its name and will always daemonize, instead of using global state
> to decide to be a no-op sometimes.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> include/sysemu/os-posix.h | 4 +-
> include/sysemu/os-win32.h | 1 -
> os-posix.c | 148 +++++++++++---------------------------
> os-win32.c | 9 ---
> softmmu/vl.c | 76 ++++++++++++++++++--
> 5 files changed, 113 insertions(+), 125 deletions(-)
> diff --git a/softmmu/vl.c b/softmmu/vl.c
> index 1fe028800f..cdf27b6027 100644
> --- a/softmmu/vl.c
> +++ b/softmmu/vl.c
> @@ -2602,11 +2602,13 @@ static void qemu_process_help_options(void)
> }
> }
>
> -static void qemu_maybe_daemonize(const char *pid_file)
> +static void qemu_maybe_daemonize(bool daemonize, const char *pid_file)
> {
> Error *err = NULL;
>
> - os_daemonize();
> + if (daemonize) {
> + os_daemonize();
> + }
> rcu_disable_atfork();
>
> if (pid_file && !qemu_write_pidfile(pid_file, &err)) {
> @@ -3683,7 +3743,7 @@ void qemu_init(int argc, char **argv, char **envp)
> qemu_process_early_options();
>
> qemu_process_help_options();
> - qemu_maybe_daemonize(pid_file);
> + qemu_maybe_daemonize(daemonize, pid_file);
This commit is a bit flawed, because we're until we call the
os_daemonize() method, the is_daemonized() method won't return
true. Unfortunately some callers rely is_daemonized() returning
true merely for the request, even though we've not yet put it
into action. ie the method would have been better called
is_daemonize_requested()
The upshot is that we fail to properly close stderr.
I'll send a v2 that handles this by fully removing the
is_daemonize() method.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH 4/4] softmmu: move parsing of -runas, -chroot and -daemonize code
2022-03-04 14:54 ` Daniel P. Berrangé
@ 2022-03-04 17:21 ` Eric Blake
0 siblings, 0 replies; 11+ messages in thread
From: Eric Blake @ 2022-03-04 17:21 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: Kevin Wolf, libvir-list, Stefan Weil, qemu-devel, Hanna Reitz,
Gerd Hoffmann, Paolo Bonzini
On Fri, Mar 04, 2022 at 02:54:54PM +0000, Daniel P. Berrangé wrote:
> On Fri, Mar 04, 2022 at 11:56:57AM +0000, Daniel P. Berrangé wrote:
> > With the future intent to try to move to a fully QAPI driven
> > configuration system, we want to have any current command
> > parsing well isolated from logic that applies the resulting
> > configuration.
> >
> > We also don't want os-posix.c to contain code that is specific
> > to the system emulators, as this file is linked to other binaries
> > too.
> >
> > To satisfy these goals, we move parsing of the -runas, -chroot and
> > -daemonize code out of the os-posix.c helper code, and pass the
> > parsed data into APIs in os-posix.c.
> >
> > As a side benefit the 'os_daemonize()' function now lives upto to
up to
> > its name and will always daemonize, instead of using global state
> > to decide to be a no-op sometimes.
Yay.
> > @@ -3683,7 +3743,7 @@ void qemu_init(int argc, char **argv, char **envp)
> > qemu_process_early_options();
> >
> > qemu_process_help_options();
> > - qemu_maybe_daemonize(pid_file);
> > + qemu_maybe_daemonize(daemonize, pid_file);
>
> This commit is a bit flawed, because we're until we call the
> os_daemonize() method, the is_daemonized() method won't return
> true. Unfortunately some callers rely is_daemonized() returning
> true merely for the request, even though we've not yet put it
> into action. ie the method would have been better called
> is_daemonize_requested()
Eww, indeed.
>
> The upshot is that we fail to properly close stderr.
>
> I'll send a v2 that handles this by fully removing the
> is_daemonize() method.
Looking forward to it.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 11+ messages in thread