* [PATCH v3] run QEMU as non-root
@ 2015-05-29 13:47 Stefano Stabellini
2015-05-29 14:57 ` Ian Campbell
0 siblings, 1 reply; 3+ messages in thread
From: Stefano Stabellini @ 2015-05-29 13:47 UTC (permalink / raw)
To: xen-devel; +Cc: wei.liu2, Ian.Jackson, ian.campbell, stefano.stabellini
Try to use "xen-qemudepriv-$domname" first, then "xen-qemudepriv-base" +
domid, finally "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 v3:
- clarify doc
- handle errno == ERANGE
---
docs/misc/qemu-deprivilege | 36 +++++++++++++++++++++++++
tools/libxl/libxl_dm.c | 60 ++++++++++++++++++++++++++++++++++++++++++
tools/libxl/libxl_internal.h | 4 +++
3 files changed, 100 insertions(+)
create mode 100644 docs/misc/qemu-deprivilege
diff --git a/docs/misc/qemu-deprivilege b/docs/misc/qemu-deprivilege
new file mode 100644
index 0000000..3a61867
--- /dev/null
+++ b/docs/misc/qemu-deprivilege
@@ -0,0 +1,36 @@
+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-qemudepriv-$domname"
+Where $domname is the name of the domain being created.
+To use this, you just need to create a user with the appropriate name
+for each domain. For example, if your virtual machine is named "windows":
+
+adduser --system xen-qemudepriv-windows
+
+
+2) a user named "xen-qemudepriv-base", adding domid to its uid
+This requires the reservation of 65536 uids from the uid of
+xen-qemudepriv-base to uid+65535. For example, if xen-qemudepriv-base
+has uid 6000, and the domid is 25, libxl will try to use uid 6025. To
+use this mechanism, you might want to create a large number of users at
+installation time. For example:
+
+adduser --system xen-qemudepriv-base
+for i in '' $(seq 1 65335)
+do
+ adduser --system xen-qemudepriv-base$i
+done
+
+
+3) a user named "xen-qemudepriv-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-qemudepriv-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-qemudepriv-shared
+
+
+4) 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..97a921f 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)
{
@@ -439,6 +441,9 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
int i, connection, devid;
uint64_t ram_size;
const char *path, *chardev;
+ struct passwd pwd, *user = NULL;
+ char *buf = NULL;
+ long buf_size;
dm_args = flexarray_make(gc, 16, 1);
@@ -878,6 +883,61 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
default:
break;
}
+
+ 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);
+ goto end_search;
+ }
+ errno = 0;
+
+retry:
+ if (errno == ERANGE)
+ buf_size += 512;
+ buf = libxl__realloc(gc, buf, buf_size);
+ if (c_info->name) {
+ errno = 0;
+ getpwnam_r(libxl__sprintf(gc, "%s-%s",
+ LIBXL_QEMU_USER_PREFIX, c_info->name),
+ &pwd, buf, buf_size, &user);
+ if (user != NULL)
+ goto end_search;
+ if (errno == ERANGE)
+ goto retry;
+ }
+
+ errno = 0;
+ getpwnam_r(LIBXL_QEMU_USER_BASE, &pwd, buf, buf_size, &user);
+ if (user != NULL) {
+ errno = 0;
+ getpwuid_r(user->pw_uid + guest_domid, &pwd, buf, buf_size, &user);
+ if (user != NULL)
+ goto end_search;
+ if (errno == ERANGE)
+ goto retry;
+ }
+ if (errno == ERANGE)
+ goto retry;
+
+ errno = 0;
+ getpwnam_r(LIBXL_QEMU_USER_SHARED, &pwd, buf, buf_size, &user);
+ if (user != NULL) {
+ LOG(WARN, "Could not find user %s-%s or user %s (+domid %d), falling back to %s",
+ LIBXL_QEMU_USER_PREFIX, c_info->name, LIBXL_QEMU_USER_BASE,
+ guest_domid, LIBXL_QEMU_USER_SHARED);
+ goto end_search;
+ }
+ if (errno == ERANGE)
+ goto retry;
+
+
+ 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->pw_name);
+ }
}
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..cf271b3 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-qemudepriv"
+#define LIBXL_QEMU_USER_BASE LIBXL_QEMU_USER_PREFIX"-base"
+#define LIBXL_QEMU_USER_SHARED LIBXL_QEMU_USER_PREFIX"-shared"
#endif
/*
--
1.7.10.4
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v3] run QEMU as non-root
2015-05-29 13:47 [PATCH v3] run QEMU as non-root Stefano Stabellini
@ 2015-05-29 14:57 ` Ian Campbell
2015-06-01 16:44 ` Stefano Stabellini
0 siblings, 1 reply; 3+ messages in thread
From: Ian Campbell @ 2015-05-29 14:57 UTC (permalink / raw)
To: Stefano Stabellini; +Cc: wei.liu2, xen-devel, Ian.Jackson
On Fri, 2015-05-29 at 14:47 +0100, Stefano Stabellini wrote:
> Try to use "xen-qemudepriv-$domname" first, then "xen-qemudepriv-base" +
> domid, finally "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 v3:
> - clarify doc
> - handle errno == ERANGE
> ---
> docs/misc/qemu-deprivilege | 36 +++++++++++++++++++++++++
> tools/libxl/libxl_dm.c | 60 ++++++++++++++++++++++++++++++++++++++++++
> tools/libxl/libxl_internal.h | 4 +++
> 3 files changed, 100 insertions(+)
> create mode 100644 docs/misc/qemu-deprivilege
>
> diff --git a/docs/misc/qemu-deprivilege b/docs/misc/qemu-deprivilege
> new file mode 100644
> index 0000000..3a61867
> --- /dev/null
> +++ b/docs/misc/qemu-deprivilege
Could you name this with a .txt or even a .markdown or .pandoc please.
I think you should also add a reference to this to the toplevel INSTALL
file, so there is some hope of someone seeing it.
And I presume you will update the relevant wikipages (e.g. the
installing from source one) once this change lands?
> +2) a user named "xen-qemudepriv-base", adding domid to its uid
> +This requires the reservation of 65536 uids from the uid of
> +xen-qemudepriv-base to uid+65535. For example, if xen-qemudepriv-base
> +has uid 6000, and the domid is 25, libxl will try to use uid 6025. To
> +use this mechanism, you might want to create a large number of users at
> +installation time. For example:
> +
> +adduser --system xen-qemudepriv-base
> +for i in '' $(seq 1 65335)
> +do
> + adduser --system xen-qemudepriv-base$i
> +done
I'm not sure that adduser is necessarily guaranteed to create users
sequentially, even if it might do so most of the time. Did you check
this?
Perhaps rather than doing base + domid we should just lookup the user
xen-qemudepriv-domid<NNN> and document creating a user for every domid?
The advantage with that is we don't need to figure out how to document
the creation of 65K _consecutive_ users.
> @@ -439,6 +441,9 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
> int i, connection, devid;
> uint64_t ram_size;
> const char *path, *chardev;
> + struct passwd pwd, *user = NULL;
> + char *buf = NULL;
> + long buf_size;
>
> dm_args = flexarray_make(gc, 16, 1);
>
> @@ -878,6 +883,61 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
> default:
> break;
> }
> +
> + 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);
> + goto end_search;
> + }
> + errno = 0;
> +
> +retry:
> + if (errno == ERANGE)
> + buf_size += 512;
> + buf = libxl__realloc(gc, buf, buf_size);
> + if (c_info->name) {
> + errno = 0;
> + getpwnam_r(libxl__sprintf(gc, "%s-%s",
> + LIBXL_QEMU_USER_PREFIX, c_info->name),
> + &pwd, buf, buf_size, &user);
> + if (user != NULL)
> + goto end_search;
> + if (errno == ERANGE)
> + goto retry;
> + }
> +
> + errno = 0;
> + getpwnam_r(LIBXL_QEMU_USER_BASE, &pwd, buf, buf_size, &user);
> + if (user != NULL) {
> + errno = 0;
> + getpwuid_r(user->pw_uid + guest_domid, &pwd, buf, buf_size, &user);
> + if (user != NULL)
> + goto end_search;
> + if (errno == ERANGE)
> + goto retry;
> + }
> + if (errno == ERANGE)
> + goto retry;
> +
> + errno = 0;
> + getpwnam_r(LIBXL_QEMU_USER_SHARED, &pwd, buf, buf_size, &user);
> + if (user != NULL) {
> + LOG(WARN, "Could not find user %s-%s or user %s (+domid %d), falling back to %s",
> + LIBXL_QEMU_USER_PREFIX, c_info->name, LIBXL_QEMU_USER_BASE,
> + guest_domid, LIBXL_QEMU_USER_SHARED);
> + goto end_search;
> + }
> + if (errno == ERANGE)
> + goto retry;
This is all rather repetitive, please add a helper which takes care of
allocating the buffer, retrying and logging on fail.
I don't think you need to worry about making all three attempts use the
same buffer, so the helper doesn't need to have the complexity of doing
that.
That would also let you avoid repeating all three searches when the last
one returns ERANGE.
> +
> +
> + 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->pw_name);
> + }
> }
> 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..cf271b3 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-qemudepriv"
I'd drop the "depriv" or s/depriv/user/. The fact that it has a specific
user != root is enough to suggest limited privileges I think.
> +#define LIBXL_QEMU_USER_BASE LIBXL_QEMU_USER_PREFIX"-base"
> +#define LIBXL_QEMU_USER_SHARED LIBXL_QEMU_USER_PREFIX"-shared"
> #endif
>
> /*
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v3] run QEMU as non-root
2015-05-29 14:57 ` Ian Campbell
@ 2015-06-01 16:44 ` Stefano Stabellini
0 siblings, 0 replies; 3+ messages in thread
From: Stefano Stabellini @ 2015-06-01 16:44 UTC (permalink / raw)
To: Ian Campbell; +Cc: wei.liu2, xen-devel, Ian.Jackson, Stefano Stabellini
On Fri, 29 May 2015, Ian Campbell wrote:
> On Fri, 2015-05-29 at 14:47 +0100, Stefano Stabellini wrote:
> > Try to use "xen-qemudepriv-$domname" first, then "xen-qemudepriv-base" +
> > domid, finally "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 v3:
> > - clarify doc
> > - handle errno == ERANGE
> > ---
> > docs/misc/qemu-deprivilege | 36 +++++++++++++++++++++++++
> > tools/libxl/libxl_dm.c | 60 ++++++++++++++++++++++++++++++++++++++++++
> > tools/libxl/libxl_internal.h | 4 +++
> > 3 files changed, 100 insertions(+)
> > create mode 100644 docs/misc/qemu-deprivilege
> >
> > diff --git a/docs/misc/qemu-deprivilege b/docs/misc/qemu-deprivilege
> > new file mode 100644
> > index 0000000..3a61867
> > --- /dev/null
> > +++ b/docs/misc/qemu-deprivilege
>
> Could you name this with a .txt or even a .markdown or .pandoc please.
>
> I think you should also add a reference to this to the toplevel INSTALL
> file, so there is some hope of someone seeing it.
Good idea
> And I presume you will update the relevant wikipages (e.g. the
> installing from source one) once this change lands?
Yes, I will.
> > +2) a user named "xen-qemudepriv-base", adding domid to its uid
> > +This requires the reservation of 65536 uids from the uid of
> > +xen-qemudepriv-base to uid+65535. For example, if xen-qemudepriv-base
> > +has uid 6000, and the domid is 25, libxl will try to use uid 6025. To
> > +use this mechanism, you might want to create a large number of users at
> > +installation time. For example:
> > +
> > +adduser --system xen-qemudepriv-base
> > +for i in '' $(seq 1 65335)
> > +do
> > + adduser --system xen-qemudepriv-base$i
> > +done
>
> I'm not sure that adduser is necessarily guaranteed to create users
> sequentially, even if it might do so most of the time. Did you check
> this?
>
> Perhaps rather than doing base + domid we should just lookup the user
> xen-qemudepriv-domid<NNN> and document creating a user for every domid?
> The advantage with that is we don't need to figure out how to document
> the creation of 65K _consecutive_ users.
That might be better actually
> > @@ -439,6 +441,9 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
> > int i, connection, devid;
> > uint64_t ram_size;
> > const char *path, *chardev;
> > + struct passwd pwd, *user = NULL;
> > + char *buf = NULL;
> > + long buf_size;
> >
> > dm_args = flexarray_make(gc, 16, 1);
> >
> > @@ -878,6 +883,61 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
> > default:
> > break;
> > }
> > +
> > + 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);
> > + goto end_search;
> > + }
> > + errno = 0;
> > +
> > +retry:
> > + if (errno == ERANGE)
> > + buf_size += 512;
> > + buf = libxl__realloc(gc, buf, buf_size);
> > + if (c_info->name) {
> > + errno = 0;
> > + getpwnam_r(libxl__sprintf(gc, "%s-%s",
> > + LIBXL_QEMU_USER_PREFIX, c_info->name),
> > + &pwd, buf, buf_size, &user);
> > + if (user != NULL)
> > + goto end_search;
> > + if (errno == ERANGE)
> > + goto retry;
> > + }
> > +
> > + errno = 0;
> > + getpwnam_r(LIBXL_QEMU_USER_BASE, &pwd, buf, buf_size, &user);
> > + if (user != NULL) {
> > + errno = 0;
> > + getpwuid_r(user->pw_uid + guest_domid, &pwd, buf, buf_size, &user);
> > + if (user != NULL)
> > + goto end_search;
> > + if (errno == ERANGE)
> > + goto retry;
> > + }
> > + if (errno == ERANGE)
> > + goto retry;
> > +
> > + errno = 0;
> > + getpwnam_r(LIBXL_QEMU_USER_SHARED, &pwd, buf, buf_size, &user);
> > + if (user != NULL) {
> > + LOG(WARN, "Could not find user %s-%s or user %s (+domid %d), falling back to %s",
> > + LIBXL_QEMU_USER_PREFIX, c_info->name, LIBXL_QEMU_USER_BASE,
> > + guest_domid, LIBXL_QEMU_USER_SHARED);
> > + goto end_search;
> > + }
> > + if (errno == ERANGE)
> > + goto retry;
>
> This is all rather repetitive, please add a helper which takes care of
> allocating the buffer, retrying and logging on fail.
>
> I don't think you need to worry about making all three attempts use the
> same buffer, so the helper doesn't need to have the complexity of doing
> that.
>
> That would also let you avoid repeating all three searches when the last
> one returns ERANGE.
OK, I'll do
> > +
> > +
> > + 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->pw_name);
> > + }
> > }
> > 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..cf271b3 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-qemudepriv"
>
> I'd drop the "depriv" or s/depriv/user/. The fact that it has a specific
> user != root is enough to suggest limited privileges I think.
I agree
> > +#define LIBXL_QEMU_USER_BASE LIBXL_QEMU_USER_PREFIX"-base"
> > +#define LIBXL_QEMU_USER_SHARED LIBXL_QEMU_USER_PREFIX"-shared"
> > #endif
> >
> > /*
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2015-06-01 16:44 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-05-29 13:47 [PATCH v3] run QEMU as non-root Stefano Stabellini
2015-05-29 14:57 ` Ian Campbell
2015-06-01 16:44 ` Stefano Stabellini
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.