* [PATCH v5] run QEMU as non-root
@ 2015-06-30 13:55 Stefano Stabellini
2015-07-01 0:04 ` Jim Fehlig
0 siblings, 1 reply; 6+ messages in thread
From: Stefano Stabellini @ 2015-06-30 13:55 UTC (permalink / raw)
To: xen-devel; +Cc: wei.liu2, Ian.Jackson, ian.campbell, stefano.stabellini
Try to use "xen-qemudepriv-domid$domid" first, then
"xen-qemudepriv-shared" and root if everything else fails.
The uids need to be manually created by the user or, more likely, by the
xen package maintainer.
To actually secure QEMU when running in Dom0, we need at least to
deprivilege the privcmd and xenstore interfaces, this is just the first
step in that direction.
Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
Changes in v5:
- improve wording in doc
- fix wording in warning message
- fix example in doc
- drop xen-qemudepriv-$domname
Changes in v4:
- rename qemu-deprivilege to qemu-deprivilege.txt
- add a note about qemu-deprivilege.txt to INSTALL
- instead of xen-qemudepriv-base + $domid, try xen-qemudepriv-domid$domid
- introduce libxl__dm_runas_helper to make the code nicer
Changes in v3:
- clarify doc
- handle errno == ERANGE
---
INSTALL | 7 ++++++
docs/misc/qemu-deprivilege.txt | 26 +++++++++++++++++++++
tools/libxl/libxl_dm.c | 50 ++++++++++++++++++++++++++++++++++++++++
tools/libxl/libxl_internal.h | 4 ++++
4 files changed, 87 insertions(+)
create mode 100644 docs/misc/qemu-deprivilege.txt
diff --git a/INSTALL b/INSTALL
index a0f2e7b..fe83c20 100644
--- a/INSTALL
+++ b/INSTALL
@@ -297,6 +297,13 @@ systemctl enable xendomains.service
systemctl enable xen-watchdog.service
+QEMU Deprivilege
+================
+It is recommended to run QEMU as non-root.
+See docs/misc/qemu-deprivilege.txt for an explanation on what you need
+to do at installation time to run QEMU as a dedicated user.
+
+
History of options
==================
diff --git a/docs/misc/qemu-deprivilege.txt b/docs/misc/qemu-deprivilege.txt
new file mode 100644
index 0000000..783874b
--- /dev/null
+++ b/docs/misc/qemu-deprivilege.txt
@@ -0,0 +1,26 @@
+For security reasons, libxl tries to create QEMU as non-root.
+Libxl looks for the following users in this order:
+
+1) a user named "xen-qemuuser-domid$domid",
+Where $domid is the domid of the domain being created.
+This requires the reservation of 65535 uids from xen-qemuuser-domid1
+to xen-qemuuser-domid65535. To use this mechanism, you might want to
+create a large number of users at installation time. For example:
+
+for ((i=1; i<65536; i++))
+do
+ adduser --system xen-qemuuser-domid$i
+done
+
+
+2) a user named "xen-qemuuser-shared"
+As a fall back if both 1) and 2) fail, libxl will use a single user for
+all QEMU instances. The user is named xen-qemuuser-shared. This is
+less secure but still better than running QEMU as root. Using this is as
+simple as creating just one more user on your host:
+
+adduser --system xen-qemuuser-shared
+
+
+3) root
+As a last resort, libxl will start QEMU as root.
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 0c6408d..66d638d 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -19,6 +19,8 @@
#include "libxl_internal.h"
#include <xen/hvm/e820.h>
+#include <sys/types.h>
+#include <pwd.h>
static const char *libxl_tapif_script(libxl__gc *gc)
{
@@ -418,6 +420,33 @@ static char *dm_spice_options(libxl__gc *gc,
return opt;
}
+/* return 1 if the user was found, 0 if it was not, -1 on error */
+static int libxl__dm_runas_helper(libxl__gc *gc, char *username)
+{
+ struct passwd pwd, *user = NULL;
+ char *buf = NULL;
+ long buf_size;
+
+ buf_size = sysconf(_SC_GETPW_R_SIZE_MAX);
+ if (buf_size < 0) {
+ LOGE(ERROR, "sysconf(_SC_GETPW_R_SIZE_MAX) returned error %ld",
+ buf_size);
+ return -1;
+ }
+
+retry:
+ buf = libxl__realloc(gc, buf, buf_size);
+ errno = 0;
+ getpwnam_r(username, &pwd, buf, buf_size, &user);
+ if (user != NULL)
+ return 1;
+ if (errno == ERANGE) {
+ buf_size += 128;
+ goto retry;
+ }
+ return 0;
+}
+
static char ** libxl__build_device_model_args_new(libxl__gc *gc,
const char *dm, int guest_domid,
const libxl_domain_config *guest_config,
@@ -439,6 +468,7 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
int i, connection, devid;
uint64_t ram_size;
const char *path, *chardev;
+ char *user;
dm_args = flexarray_make(gc, 16, 1);
@@ -878,6 +908,26 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
default:
break;
}
+
+ user = libxl__sprintf(gc, "%s%d", LIBXL_QEMU_USER_BASE, guest_domid);
+ if (libxl__dm_runas_helper(gc, user) > 0)
+ goto end_search;
+
+ user = LIBXL_QEMU_USER_SHARED;
+ if (libxl__dm_runas_helper(gc, user) > 0) {
+ LOG(WARN, "Could not find user %s%d, falling back to %s",
+ LIBXL_QEMU_USER_BASE, guest_domid, LIBXL_QEMU_USER_SHARED);
+ goto end_search;
+ }
+
+ user = NULL;
+ LOG(WARN, "Could not find user %s, starting QEMU as root", LIBXL_QEMU_USER_SHARED);
+
+end_search:
+ if (user) {
+ flexarray_append(dm_args, "-runas");
+ flexarray_append(dm_args, user);
+ }
}
flexarray_append(dm_args, NULL);
return (char **) flexarray_contents(dm_args);
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 8eb38aa..7d0af40 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -3692,6 +3692,10 @@ static inline void libxl__update_config_vtpm(libxl__gc *gc,
*/
void libxl__bitmap_copy_best_effort(libxl__gc *gc, libxl_bitmap *dptr,
const libxl_bitmap *sptr);
+
+#define LIBXL_QEMU_USER_PREFIX "xen-qemuuser"
+#define LIBXL_QEMU_USER_BASE LIBXL_QEMU_USER_PREFIX"-domid"
+#define LIBXL_QEMU_USER_SHARED LIBXL_QEMU_USER_PREFIX"-shared"
#endif
/*
--
1.7.10.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v5] run QEMU as non-root
2015-06-30 13:55 [PATCH v5] run QEMU as non-root Stefano Stabellini
@ 2015-07-01 0:04 ` Jim Fehlig
2015-07-01 8:23 ` Fabio Fantoni
2015-07-01 10:42 ` Stefano Stabellini
0 siblings, 2 replies; 6+ messages in thread
From: Jim Fehlig @ 2015-07-01 0:04 UTC (permalink / raw)
To: Stefano Stabellini, xen-devel; +Cc: Ian.Jackson, wei.liu2, ian.campbell
On 06/30/2015 07:55 AM, Stefano Stabellini wrote:
> Try to use "xen-qemudepriv-domid$domid" first, then
> "xen-qemudepriv-shared" and root if everything else fails.
>
> The uids need to be manually created by the user or, more likely, by the
> xen package maintainer.
>
> To actually secure QEMU when running in Dom0, we need at least to
> deprivilege the privcmd and xenstore interfaces, this is just the first
> step in that direction.
>
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>
> ---
> Changes in v5:
> - improve wording in doc
> - fix wording in warning message
> - fix example in doc
> - drop xen-qemudepriv-$domname
>
> Changes in v4:
> - rename qemu-deprivilege to qemu-deprivilege.txt
> - add a note about qemu-deprivilege.txt to INSTALL
> - instead of xen-qemudepriv-base + $domid, try xen-qemudepriv-domid$domid
> - introduce libxl__dm_runas_helper to make the code nicer
>
> Changes in v3:
> - clarify doc
> - handle errno == ERANGE
> ---
> INSTALL | 7 ++++++
> docs/misc/qemu-deprivilege.txt | 26 +++++++++++++++++++++
> tools/libxl/libxl_dm.c | 50 ++++++++++++++++++++++++++++++++++++++++
> tools/libxl/libxl_internal.h | 4 ++++
> 4 files changed, 87 insertions(+)
> create mode 100644 docs/misc/qemu-deprivilege.txt
>
> diff --git a/INSTALL b/INSTALL
> index a0f2e7b..fe83c20 100644
> --- a/INSTALL
> +++ b/INSTALL
> @@ -297,6 +297,13 @@ systemctl enable xendomains.service
> systemctl enable xen-watchdog.service
>
>
> +QEMU Deprivilege
> +================
> +It is recommended to run QEMU as non-root.
> +See docs/misc/qemu-deprivilege.txt for an explanation on what you need
> +to do at installation time to run QEMU as a dedicated user.
> +
> +
> History of options
> ==================
>
> diff --git a/docs/misc/qemu-deprivilege.txt b/docs/misc/qemu-deprivilege.txt
> new file mode 100644
> index 0000000..783874b
> --- /dev/null
> +++ b/docs/misc/qemu-deprivilege.txt
> @@ -0,0 +1,26 @@
> +For security reasons, libxl tries to create QEMU as non-root.
> +Libxl looks for the following users in this order:
> +
> +1) a user named "xen-qemuuser-domid$domid",
> +Where $domid is the domid of the domain being created.
> +This requires the reservation of 65535 uids from xen-qemuuser-domid1
> +to xen-qemuuser-domid65535. To use this mechanism, you might want to
> +create a large number of users at installation time. For example:
> +
> +for ((i=1; i<65536; i++))
> +do
> + adduser --system xen-qemuuser-domid$i
> +done
> +
> +
> +2) a user named "xen-qemuuser-shared"
> +As a fall back if both 1) and 2) fail, libxl will use a single user for
> +all QEMU instances. The user is named xen-qemuuser-shared. This is
> +less secure but still better than running QEMU as root. Using this is as
> +simple as creating just one more user on your host:
> +
> +adduser --system xen-qemuuser-shared
> +
> +
> +3) root
> +As a last resort, libxl will start QEMU as root.
The more I think about it, the more I feel libxl is the wrong place for this
policy. As mentioned earlier [0], libvirt allows apps to control the user:group
policy. It is already supported by the qemu driver. It could be used by the
libxl driver to inform libxl that the emulator (and other binaries?) it spawns
should be in the context of the specified user:group.
Regards,
Jim
[0] http://lists.xenproject.org/archives/html/xen-devel/2015-05/msg02139.html
> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> index 0c6408d..66d638d 100644
> --- a/tools/libxl/libxl_dm.c
> +++ b/tools/libxl/libxl_dm.c
> @@ -19,6 +19,8 @@
>
> #include "libxl_internal.h"
> #include <xen/hvm/e820.h>
> +#include <sys/types.h>
> +#include <pwd.h>
>
> static const char *libxl_tapif_script(libxl__gc *gc)
> {
> @@ -418,6 +420,33 @@ static char *dm_spice_options(libxl__gc *gc,
> return opt;
> }
>
> +/* return 1 if the user was found, 0 if it was not, -1 on error */
> +static int libxl__dm_runas_helper(libxl__gc *gc, char *username)
> +{
> + struct passwd pwd, *user = NULL;
> + char *buf = NULL;
> + long buf_size;
> +
> + buf_size = sysconf(_SC_GETPW_R_SIZE_MAX);
> + if (buf_size < 0) {
> + LOGE(ERROR, "sysconf(_SC_GETPW_R_SIZE_MAX) returned error %ld",
> + buf_size);
> + return -1;
> + }
> +
> +retry:
> + buf = libxl__realloc(gc, buf, buf_size);
> + errno = 0;
> + getpwnam_r(username, &pwd, buf, buf_size, &user);
> + if (user != NULL)
> + return 1;
> + if (errno == ERANGE) {
> + buf_size += 128;
> + goto retry;
> + }
> + return 0;
> +}
> +
> static char ** libxl__build_device_model_args_new(libxl__gc *gc,
> const char *dm, int guest_domid,
> const libxl_domain_config *guest_config,
> @@ -439,6 +468,7 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
> int i, connection, devid;
> uint64_t ram_size;
> const char *path, *chardev;
> + char *user;
>
> dm_args = flexarray_make(gc, 16, 1);
>
> @@ -878,6 +908,26 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
> default:
> break;
> }
> +
> + user = libxl__sprintf(gc, "%s%d", LIBXL_QEMU_USER_BASE, guest_domid);
> + if (libxl__dm_runas_helper(gc, user) > 0)
> + goto end_search;
> +
> + user = LIBXL_QEMU_USER_SHARED;
> + if (libxl__dm_runas_helper(gc, user) > 0) {
> + LOG(WARN, "Could not find user %s%d, falling back to %s",
> + LIBXL_QEMU_USER_BASE, guest_domid, LIBXL_QEMU_USER_SHARED);
> + goto end_search;
> + }
> +
> + user = NULL;
> + LOG(WARN, "Could not find user %s, starting QEMU as root", LIBXL_QEMU_USER_SHARED);
> +
> +end_search:
> + if (user) {
> + flexarray_append(dm_args, "-runas");
> + flexarray_append(dm_args, user);
> + }
> }
> flexarray_append(dm_args, NULL);
> return (char **) flexarray_contents(dm_args);
> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> index 8eb38aa..7d0af40 100644
> --- a/tools/libxl/libxl_internal.h
> +++ b/tools/libxl/libxl_internal.h
> @@ -3692,6 +3692,10 @@ static inline void libxl__update_config_vtpm(libxl__gc *gc,
> */
> void libxl__bitmap_copy_best_effort(libxl__gc *gc, libxl_bitmap *dptr,
> const libxl_bitmap *sptr);
> +
> +#define LIBXL_QEMU_USER_PREFIX "xen-qemuuser"
> +#define LIBXL_QEMU_USER_BASE LIBXL_QEMU_USER_PREFIX"-domid"
> +#define LIBXL_QEMU_USER_SHARED LIBXL_QEMU_USER_PREFIX"-shared"
> #endif
>
> /*
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v5] run QEMU as non-root
2015-07-01 0:04 ` Jim Fehlig
@ 2015-07-01 8:23 ` Fabio Fantoni
2015-07-01 20:42 ` Jim Fehlig
2015-07-01 10:42 ` Stefano Stabellini
1 sibling, 1 reply; 6+ messages in thread
From: Fabio Fantoni @ 2015-07-01 8:23 UTC (permalink / raw)
To: Jim Fehlig, Stefano Stabellini, xen-devel
Cc: wei.liu2, Ian.Jackson, ian.campbell
Il 01/07/2015 02:04, Jim Fehlig ha scritto:
> On 06/30/2015 07:55 AM, Stefano Stabellini wrote:
>> Try to use "xen-qemudepriv-domid$domid" first, then
>> "xen-qemudepriv-shared" and root if everything else fails.
>>
>> The uids need to be manually created by the user or, more likely, by the
>> xen package maintainer.
>>
>> To actually secure QEMU when running in Dom0, we need at least to
>> deprivilege the privcmd and xenstore interfaces, this is just the first
>> step in that direction.
>>
>> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>>
>> ---
>> Changes in v5:
>> - improve wording in doc
>> - fix wording in warning message
>> - fix example in doc
>> - drop xen-qemudepriv-$domname
>>
>> Changes in v4:
>> - rename qemu-deprivilege to qemu-deprivilege.txt
>> - add a note about qemu-deprivilege.txt to INSTALL
>> - instead of xen-qemudepriv-base + $domid, try
>> xen-qemudepriv-domid$domid
>> - introduce libxl__dm_runas_helper to make the code nicer
>>
>> Changes in v3:
>> - clarify doc
>> - handle errno == ERANGE
>> ---
>> INSTALL | 7 ++++++
>> docs/misc/qemu-deprivilege.txt | 26 +++++++++++++++++++++
>> tools/libxl/libxl_dm.c | 50
>> ++++++++++++++++++++++++++++++++++++++++
>> tools/libxl/libxl_internal.h | 4 ++++
>> 4 files changed, 87 insertions(+)
>> create mode 100644 docs/misc/qemu-deprivilege.txt
>>
>> diff --git a/INSTALL b/INSTALL
>> index a0f2e7b..fe83c20 100644
>> --- a/INSTALL
>> +++ b/INSTALL
>> @@ -297,6 +297,13 @@ systemctl enable xendomains.service
>> systemctl enable xen-watchdog.service
>> +QEMU Deprivilege
>> +================
>> +It is recommended to run QEMU as non-root.
>> +See docs/misc/qemu-deprivilege.txt for an explanation on what you need
>> +to do at installation time to run QEMU as a dedicated user.
>> +
>> +
>> History of options
>> ==================
>> diff --git a/docs/misc/qemu-deprivilege.txt
>> b/docs/misc/qemu-deprivilege.txt
>> new file mode 100644
>> index 0000000..783874b
>> --- /dev/null
>> +++ b/docs/misc/qemu-deprivilege.txt
>> @@ -0,0 +1,26 @@
>> +For security reasons, libxl tries to create QEMU as non-root.
>> +Libxl looks for the following users in this order:
>> +
>> +1) a user named "xen-qemuuser-domid$domid",
>> +Where $domid is the domid of the domain being created.
>> +This requires the reservation of 65535 uids from xen-qemuuser-domid1
>> +to xen-qemuuser-domid65535. To use this mechanism, you might want to
>> +create a large number of users at installation time. For example:
>> +
>> +for ((i=1; i<65536; i++))
>> +do
>> + adduser --system xen-qemuuser-domid$i
>> +done
>> +
>> +
>> +2) a user named "xen-qemuuser-shared"
>> +As a fall back if both 1) and 2) fail, libxl will use a single user for
>> +all QEMU instances. The user is named xen-qemuuser-shared. This is
>> +less secure but still better than running QEMU as root. Using this
>> is as
>> +simple as creating just one more user on your host:
>> +
>> +adduser --system xen-qemuuser-shared
>> +
>> +
>> +3) root
>> +As a last resort, libxl will start QEMU as root.
>
> The more I think about it, the more I feel libxl is the wrong place
> for this policy. As mentioned earlier [0], libvirt allows apps to
> control the user:group policy. It is already supported by the qemu
> driver. It could be used by the libxl driver to inform libxl that the
> emulator (and other binaries?) it spawns should be in the context of
> the specified user:group.
>
> Regards,
> Jim
In the patch I saw this:
> + if (user) {
> + flexarray_append(dm_args, "-runas");
> + flexarray_append(dm_args, user);
> + }
So seems that already use qemu parameter for it.
What you mean is to add also the possibility to specify user for use it
from libvirt and similar instead doing a different thing to support the
xen specific one? If yes I also think can be a good idea. In that case
can you explain a better way to do it? Probably Stabellini didn't done
it because don't know other similar implementation already present in
other systems that use qemu and how are good and used.
>
> [0]
> http://lists.xenproject.org/archives/html/xen-devel/2015-05/msg02139.html
>
>> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
>> index 0c6408d..66d638d 100644
>> --- a/tools/libxl/libxl_dm.c
>> +++ b/tools/libxl/libxl_dm.c
>> @@ -19,6 +19,8 @@
>> #include "libxl_internal.h"
>> #include <xen/hvm/e820.h>
>> +#include <sys/types.h>
>> +#include <pwd.h>
>> static const char *libxl_tapif_script(libxl__gc *gc)
>> {
>> @@ -418,6 +420,33 @@ static char *dm_spice_options(libxl__gc *gc,
>> return opt;
>> }
>> +/* return 1 if the user was found, 0 if it was not, -1 on error */
>> +static int libxl__dm_runas_helper(libxl__gc *gc, char *username)
>> +{
>> + struct passwd pwd, *user = NULL;
>> + char *buf = NULL;
>> + long buf_size;
>> +
>> + buf_size = sysconf(_SC_GETPW_R_SIZE_MAX);
>> + if (buf_size < 0) {
>> + LOGE(ERROR, "sysconf(_SC_GETPW_R_SIZE_MAX) returned error %ld",
>> + buf_size);
>> + return -1;
>> + }
>> +
>> +retry:
>> + buf = libxl__realloc(gc, buf, buf_size);
>> + errno = 0;
>> + getpwnam_r(username, &pwd, buf, buf_size, &user);
>> + if (user != NULL)
>> + return 1;
>> + if (errno == ERANGE) {
>> + buf_size += 128;
>> + goto retry;
>> + }
>> + return 0;
>> +}
>> +
>> static char ** libxl__build_device_model_args_new(libxl__gc *gc,
>> const char *dm, int
>> guest_domid,
>> const libxl_domain_config
>> *guest_config,
>> @@ -439,6 +468,7 @@ static char **
>> libxl__build_device_model_args_new(libxl__gc *gc,
>> int i, connection, devid;
>> uint64_t ram_size;
>> const char *path, *chardev;
>> + char *user;
>> dm_args = flexarray_make(gc, 16, 1);
>> @@ -878,6 +908,26 @@ static char **
>> libxl__build_device_model_args_new(libxl__gc *gc,
>> default:
>> break;
>> }
>> +
>> + user = libxl__sprintf(gc, "%s%d", LIBXL_QEMU_USER_BASE,
>> guest_domid);
>> + if (libxl__dm_runas_helper(gc, user) > 0)
>> + goto end_search;
>> +
>> + user = LIBXL_QEMU_USER_SHARED;
>> + if (libxl__dm_runas_helper(gc, user) > 0) {
>> + LOG(WARN, "Could not find user %s%d, falling back to %s",
>> + LIBXL_QEMU_USER_BASE, guest_domid,
>> LIBXL_QEMU_USER_SHARED);
>> + goto end_search;
>> + }
>> +
>> + user = NULL;
>> + LOG(WARN, "Could not find user %s, starting QEMU as root",
>> LIBXL_QEMU_USER_SHARED);
>> +
>> +end_search:
>> + if (user) {
>> + flexarray_append(dm_args, "-runas");
>> + flexarray_append(dm_args, user);
>> + }
>> }
>> flexarray_append(dm_args, NULL);
>> return (char **) flexarray_contents(dm_args);
>> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
>> index 8eb38aa..7d0af40 100644
>> --- a/tools/libxl/libxl_internal.h
>> +++ b/tools/libxl/libxl_internal.h
>> @@ -3692,6 +3692,10 @@ static inline void
>> libxl__update_config_vtpm(libxl__gc *gc,
>> */
>> void libxl__bitmap_copy_best_effort(libxl__gc *gc, libxl_bitmap *dptr,
>> const libxl_bitmap *sptr);
>> +
>> +#define LIBXL_QEMU_USER_PREFIX "xen-qemuuser"
>> +#define LIBXL_QEMU_USER_BASE LIBXL_QEMU_USER_PREFIX"-domid"
>> +#define LIBXL_QEMU_USER_SHARED LIBXL_QEMU_USER_PREFIX"-shared"
>> #endif
>> /*
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v5] run QEMU as non-root
2015-07-01 8:23 ` Fabio Fantoni
@ 2015-07-01 20:42 ` Jim Fehlig
0 siblings, 0 replies; 6+ messages in thread
From: Jim Fehlig @ 2015-07-01 20:42 UTC (permalink / raw)
To: Fabio Fantoni, Stefano Stabellini, xen-devel
Cc: wei.liu2, Ian.Jackson, ian.campbell
On 07/01/2015 02:23 AM, Fabio Fantoni wrote:
> Il 01/07/2015 02:04, Jim Fehlig ha scritto:
>> On 06/30/2015 07:55 AM, Stefano Stabellini wrote:
>>> Try to use "xen-qemudepriv-domid$domid" first, then
>>> "xen-qemudepriv-shared" and root if everything else fails.
>>>
>>> The uids need to be manually created by the user or, more likely, by the
>>> xen package maintainer.
>>>
>>> To actually secure QEMU when running in Dom0, we need at least to
>>> deprivilege the privcmd and xenstore interfaces, this is just the first
>>> step in that direction.
>>>
>>> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>>>
>>> ---
>>> Changes in v5:
>>> - improve wording in doc
>>> - fix wording in warning message
>>> - fix example in doc
>>> - drop xen-qemudepriv-$domname
>>>
>>> Changes in v4:
>>> - rename qemu-deprivilege to qemu-deprivilege.txt
>>> - add a note about qemu-deprivilege.txt to INSTALL
>>> - instead of xen-qemudepriv-base + $domid, try xen-qemudepriv-domid$domid
>>> - introduce libxl__dm_runas_helper to make the code nicer
>>>
>>> Changes in v3:
>>> - clarify doc
>>> - handle errno == ERANGE
>>> ---
>>> INSTALL | 7 ++++++
>>> docs/misc/qemu-deprivilege.txt | 26 +++++++++++++++++++++
>>> tools/libxl/libxl_dm.c | 50
>>> ++++++++++++++++++++++++++++++++++++++++
>>> tools/libxl/libxl_internal.h | 4 ++++
>>> 4 files changed, 87 insertions(+)
>>> create mode 100644 docs/misc/qemu-deprivilege.txt
>>>
>>> diff --git a/INSTALL b/INSTALL
>>> index a0f2e7b..fe83c20 100644
>>> --- a/INSTALL
>>> +++ b/INSTALL
>>> @@ -297,6 +297,13 @@ systemctl enable xendomains.service
>>> systemctl enable xen-watchdog.service
>>> +QEMU Deprivilege
>>> +================
>>> +It is recommended to run QEMU as non-root.
>>> +See docs/misc/qemu-deprivilege.txt for an explanation on what you need
>>> +to do at installation time to run QEMU as a dedicated user.
>>> +
>>> +
>>> History of options
>>> ==================
>>> diff --git a/docs/misc/qemu-deprivilege.txt b/docs/misc/qemu-deprivilege.txt
>>> new file mode 100644
>>> index 0000000..783874b
>>> --- /dev/null
>>> +++ b/docs/misc/qemu-deprivilege.txt
>>> @@ -0,0 +1,26 @@
>>> +For security reasons, libxl tries to create QEMU as non-root.
>>> +Libxl looks for the following users in this order:
>>> +
>>> +1) a user named "xen-qemuuser-domid$domid",
>>> +Where $domid is the domid of the domain being created.
>>> +This requires the reservation of 65535 uids from xen-qemuuser-domid1
>>> +to xen-qemuuser-domid65535. To use this mechanism, you might want to
>>> +create a large number of users at installation time. For example:
>>> +
>>> +for ((i=1; i<65536; i++))
>>> +do
>>> + adduser --system xen-qemuuser-domid$i
>>> +done
>>> +
>>> +
>>> +2) a user named "xen-qemuuser-shared"
>>> +As a fall back if both 1) and 2) fail, libxl will use a single user for
>>> +all QEMU instances. The user is named xen-qemuuser-shared. This is
>>> +less secure but still better than running QEMU as root. Using this is as
>>> +simple as creating just one more user on your host:
>>> +
>>> +adduser --system xen-qemuuser-shared
>>> +
>>> +
>>> +3) root
>>> +As a last resort, libxl will start QEMU as root.
>>
>> The more I think about it, the more I feel libxl is the wrong place for this
>> policy. As mentioned earlier [0], libvirt allows apps to control the
>> user:group policy. It is already supported by the qemu driver. It could be
>> used by the libxl driver to inform libxl that the emulator (and other
>> binaries?) it spawns should be in the context of the specified user:group.
>>
>> Regards,
>> Jim
>
> In the patch I saw this:
>> + if (user) {
>> + flexarray_append(dm_args, "-runas");
>> + flexarray_append(dm_args, user);
>> + }
> So seems that already use qemu parameter for it.
>
> What you mean is to add also the possibility to specify user for use it from
> libvirt and similar instead doing a different thing to support the xen
> specific one?
Yes.
> If yes I also think can be a good idea. In that case can you explain a better
> way to do it? Probably Stabellini didn't done it because don't know other
> similar implementation already present in other systems that use qemu and how
> are good and used.
I replied to Stefano's mail with some additional thoughts and ideas, but I think
he understands.
Regards,
Jim
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v5] run QEMU as non-root
2015-07-01 0:04 ` Jim Fehlig
2015-07-01 8:23 ` Fabio Fantoni
@ 2015-07-01 10:42 ` Stefano Stabellini
2015-07-01 17:07 ` Jim Fehlig
1 sibling, 1 reply; 6+ messages in thread
From: Stefano Stabellini @ 2015-07-01 10:42 UTC (permalink / raw)
To: Jim Fehlig
Cc: Ian.Jackson, xen-devel, wei.liu2, ian.campbell,
Stefano Stabellini
On Tue, 30 Jun 2015, Jim Fehlig wrote:
> On 06/30/2015 07:55 AM, Stefano Stabellini wrote:
> > Try to use "xen-qemudepriv-domid$domid" first, then
> > "xen-qemudepriv-shared" and root if everything else fails.
> >
> > The uids need to be manually created by the user or, more likely, by the
> > xen package maintainer.
> >
> > To actually secure QEMU when running in Dom0, we need at least to
> > deprivilege the privcmd and xenstore interfaces, this is just the first
> > step in that direction.
> >
> > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> >
> > ---
> > Changes in v5:
> > - improve wording in doc
> > - fix wording in warning message
> > - fix example in doc
> > - drop xen-qemudepriv-$domname
> >
> > Changes in v4:
> > - rename qemu-deprivilege to qemu-deprivilege.txt
> > - add a note about qemu-deprivilege.txt to INSTALL
> > - instead of xen-qemudepriv-base + $domid, try xen-qemudepriv-domid$domid
> > - introduce libxl__dm_runas_helper to make the code nicer
> >
> > Changes in v3:
> > - clarify doc
> > - handle errno == ERANGE
> > ---
> > INSTALL | 7 ++++++
> > docs/misc/qemu-deprivilege.txt | 26 +++++++++++++++++++++
> > tools/libxl/libxl_dm.c | 50
> > ++++++++++++++++++++++++++++++++++++++++
> > tools/libxl/libxl_internal.h | 4 ++++
> > 4 files changed, 87 insertions(+)
> > create mode 100644 docs/misc/qemu-deprivilege.txt
> >
> > diff --git a/INSTALL b/INSTALL
> > index a0f2e7b..fe83c20 100644
> > --- a/INSTALL
> > +++ b/INSTALL
> > @@ -297,6 +297,13 @@ systemctl enable xendomains.service
> > systemctl enable xen-watchdog.service
> > +QEMU Deprivilege
> > +================
> > +It is recommended to run QEMU as non-root.
> > +See docs/misc/qemu-deprivilege.txt for an explanation on what you need
> > +to do at installation time to run QEMU as a dedicated user.
> > +
> > +
> > History of options
> > ==================
> > diff --git a/docs/misc/qemu-deprivilege.txt
> > b/docs/misc/qemu-deprivilege.txt
> > new file mode 100644
> > index 0000000..783874b
> > --- /dev/null
> > +++ b/docs/misc/qemu-deprivilege.txt
> > @@ -0,0 +1,26 @@
> > +For security reasons, libxl tries to create QEMU as non-root.
> > +Libxl looks for the following users in this order:
> > +
> > +1) a user named "xen-qemuuser-domid$domid",
> > +Where $domid is the domid of the domain being created.
> > +This requires the reservation of 65535 uids from xen-qemuuser-domid1
> > +to xen-qemuuser-domid65535. To use this mechanism, you might want to
> > +create a large number of users at installation time. For example:
> > +
> > +for ((i=1; i<65536; i++))
> > +do
> > + adduser --system xen-qemuuser-domid$i
> > +done
> > +
> > +
> > +2) a user named "xen-qemuuser-shared"
> > +As a fall back if both 1) and 2) fail, libxl will use a single user for
> > +all QEMU instances. The user is named xen-qemuuser-shared. This is
> > +less secure but still better than running QEMU as root. Using this is as
> > +simple as creating just one more user on your host:
> > +
> > +adduser --system xen-qemuuser-shared
> > +
> > +
> > +3) root
> > +As a last resort, libxl will start QEMU as root.
>
> The more I think about it, the more I feel libxl is the wrong place for this
> policy. As mentioned earlier [0], libvirt allows apps to control the
> user:group policy. It is already supported by the qemu driver. It could be
> used by the libxl driver to inform libxl that the emulator (and other
> binaries?) it spawns should be in the context of the specified user:group.
>
> Regards,
> Jim
>
> [0] http://lists.xenproject.org/archives/html/xen-devel/2015-05/msg02139.html
Are you suggesting to expose a per-domain user:group setting that can be
passed down by libvirt? Maybe something under
libxl_domain_build_info.u.hvm?
Then I could move the code below to xl_cmdimpl.c, making it xl specific
(rather than libxl).
> > diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> > index 0c6408d..66d638d 100644
> > --- a/tools/libxl/libxl_dm.c
> > +++ b/tools/libxl/libxl_dm.c
> > @@ -19,6 +19,8 @@
> > #include "libxl_internal.h"
> > #include <xen/hvm/e820.h>
> > +#include <sys/types.h>
> > +#include <pwd.h>
> > static const char *libxl_tapif_script(libxl__gc *gc)
> > {
> > @@ -418,6 +420,33 @@ static char *dm_spice_options(libxl__gc *gc,
> > return opt;
> > }
> > +/* return 1 if the user was found, 0 if it was not, -1 on error */
> > +static int libxl__dm_runas_helper(libxl__gc *gc, char *username)
> > +{
> > + struct passwd pwd, *user = NULL;
> > + char *buf = NULL;
> > + long buf_size;
> > +
> > + buf_size = sysconf(_SC_GETPW_R_SIZE_MAX);
> > + if (buf_size < 0) {
> > + LOGE(ERROR, "sysconf(_SC_GETPW_R_SIZE_MAX) returned error %ld",
> > + buf_size);
> > + return -1;
> > + }
> > +
> > +retry:
> > + buf = libxl__realloc(gc, buf, buf_size);
> > + errno = 0;
> > + getpwnam_r(username, &pwd, buf, buf_size, &user);
> > + if (user != NULL)
> > + return 1;
> > + if (errno == ERANGE) {
> > + buf_size += 128;
> > + goto retry;
> > + }
> > + return 0;
> > +}
> > +
> > static char ** libxl__build_device_model_args_new(libxl__gc *gc,
> > const char *dm, int guest_domid,
> > const libxl_domain_config
> > *guest_config,
> > @@ -439,6 +468,7 @@ static char **
> > libxl__build_device_model_args_new(libxl__gc *gc,
> > int i, connection, devid;
> > uint64_t ram_size;
> > const char *path, *chardev;
> > + char *user;
> > dm_args = flexarray_make(gc, 16, 1);
> > @@ -878,6 +908,26 @@ static char **
> > libxl__build_device_model_args_new(libxl__gc *gc,
> > default:
> > break;
> > }
> > +
> > + user = libxl__sprintf(gc, "%s%d", LIBXL_QEMU_USER_BASE,
> > guest_domid);
> > + if (libxl__dm_runas_helper(gc, user) > 0)
> > + goto end_search;
> > +
> > + user = LIBXL_QEMU_USER_SHARED;
> > + if (libxl__dm_runas_helper(gc, user) > 0) {
> > + LOG(WARN, "Could not find user %s%d, falling back to %s",
> > + LIBXL_QEMU_USER_BASE, guest_domid,
> > LIBXL_QEMU_USER_SHARED);
> > + goto end_search;
> > + }
> > +
> > + user = NULL;
> > + LOG(WARN, "Could not find user %s, starting QEMU as root",
> > LIBXL_QEMU_USER_SHARED);
> > +
> > +end_search:
> > + if (user) {
> > + flexarray_append(dm_args, "-runas");
> > + flexarray_append(dm_args, user);
> > + }
> > }
> > flexarray_append(dm_args, NULL);
> > return (char **) flexarray_contents(dm_args);
> > diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> > index 8eb38aa..7d0af40 100644
> > --- a/tools/libxl/libxl_internal.h
> > +++ b/tools/libxl/libxl_internal.h
> > @@ -3692,6 +3692,10 @@ static inline void
> > libxl__update_config_vtpm(libxl__gc *gc,
> > */
> > void libxl__bitmap_copy_best_effort(libxl__gc *gc, libxl_bitmap *dptr,
> > const libxl_bitmap *sptr);
> > +
> > +#define LIBXL_QEMU_USER_PREFIX "xen-qemuuser"
> > +#define LIBXL_QEMU_USER_BASE LIBXL_QEMU_USER_PREFIX"-domid"
> > +#define LIBXL_QEMU_USER_SHARED LIBXL_QEMU_USER_PREFIX"-shared"
> > #endif
> > /*
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v5] run QEMU as non-root
2015-07-01 10:42 ` Stefano Stabellini
@ 2015-07-01 17:07 ` Jim Fehlig
0 siblings, 0 replies; 6+ messages in thread
From: Jim Fehlig @ 2015-07-01 17:07 UTC (permalink / raw)
To: Stefano Stabellini; +Cc: Ian.Jackson, xen-devel, wei.liu2, ian.campbell
On 07/01/2015 04:42 AM, Stefano Stabellini wrote:
> On Tue, 30 Jun 2015, Jim Fehlig wrote:
>> On 06/30/2015 07:55 AM, Stefano Stabellini wrote:
[...]
>>> b/docs/misc/qemu-deprivilege.txt
>>> new file mode 100644
>>> index 0000000..783874b
>>> --- /dev/null
>>> +++ b/docs/misc/qemu-deprivilege.txt
>>> @@ -0,0 +1,26 @@
>>> +For security reasons, libxl tries to create QEMU as non-root.
>>> +Libxl looks for the following users in this order:
>>> +
>>> +1) a user named "xen-qemuuser-domid$domid",
>>> +Where $domid is the domid of the domain being created.
>>> +This requires the reservation of 65535 uids from xen-qemuuser-domid1
>>> +to xen-qemuuser-domid65535. To use this mechanism, you might want to
>>> +create a large number of users at installation time. For example:
>>> +
>>> +for ((i=1; i<65536; i++))
>>> +do
>>> + adduser --system xen-qemuuser-domid$i
>>> +done
>>> +
>>> +
>>> +2) a user named "xen-qemuuser-shared"
>>> +As a fall back if both 1) and 2) fail, libxl will use a single user for
>>> +all QEMU instances. The user is named xen-qemuuser-shared. This is
>>> +less secure but still better than running QEMU as root. Using this is as
>>> +simple as creating just one more user on your host:
>>> +
>>> +adduser --system xen-qemuuser-shared
>>> +
>>> +
>>> +3) root
>>> +As a last resort, libxl will start QEMU as root.
>> The more I think about it, the more I feel libxl is the wrong place for this
>> policy. As mentioned earlier [0], libvirt allows apps to control the
>> user:group policy. It is already supported by the qemu driver. It could be
>> used by the libxl driver to inform libxl that the emulator (and other
>> binaries?) it spawns should be in the context of the specified user:group.
>>
>> Regards,
>> Jim
>>
>> [0] http://lists.xenproject.org/archives/html/xen-devel/2015-05/msg02139.html
> Are you suggesting to expose a per-domain user:group setting that can be
> passed down by libvirt? Maybe something under
> libxl_domain_build_info.u.hvm?
Yes. But should it be restricted to .hvm? Maybe it will be useful now (or in the
future) for pv too. E.g. older pv bootloaders that run in dom0 could be executed
as the user:group.
> Then I could move the code below to xl_cmdimpl.c, making it xl specific
> (rather than libxl).
Right. Which means adding 'user=' and 'group=' to xl.cfg, correct? If so,
perhaps xl.conf should provide "xl-wide" defaults, applicable to all domains
created with xl that don't contain explicit 'user=' and 'group='.
Regards,
Jim
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-07-01 20:42 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-06-30 13:55 [PATCH v5] run QEMU as non-root Stefano Stabellini
2015-07-01 0:04 ` Jim Fehlig
2015-07-01 8:23 ` Fabio Fantoni
2015-07-01 20:42 ` Jim Fehlig
2015-07-01 10:42 ` Stefano Stabellini
2015-07-01 17:07 ` Jim Fehlig
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.