|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v4] run QEMU as non-root
On Thu, 25 Jun 2015, Ian Campbell wrote:
> On Mon, 2015-06-01 at 17:46 +0100, Stefano Stabellini wrote:
> > Try to use "xen-qemudepriv-$domname" first, then
> > "xen-qemudepriv-domid$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@xxxxxxxxxxxxx>
> >
> > ---
> > 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 | 34 +++++++++++++++++++++++++
> > tools/libxl/libxl_dm.c | 54
> > ++++++++++++++++++++++++++++++++++++++++
> > tools/libxl/libxl_internal.h | 4 +++
> > 4 files changed, 99 insertions(+)
> > create mode 100644 docs/misc/qemu-deprivilege.txt
> >
> > diff --git a/INSTALL b/INSTALL
> > index a0f2e7b..3286a5f 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 regular user.
>
> I'd be tempted to say something like "as a dedicated user" or something,
> "regular user" sounds like I should be running it as my normal login
> user or something.
OK
> > +
> > +
> > History of options
> > ==================
> >
> > diff --git a/docs/misc/qemu-deprivilege.txt b/docs/misc/qemu-deprivilege.txt
> > new file mode 100644
> > index 0000000..db61aaf
> > --- /dev/null
> > +++ b/docs/misc/qemu-deprivilege.txt
> > @@ -0,0 +1,34 @@
> > +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-$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-qemuuser-windows
>
> I just had an annoying thought: On migration (with xl at least) the
> receive side appends "-incoming" to the domain name for the duration and
> then renames ones everything is done, which is suddenly not going to
> match here :-(
>
> Since this is managed at the xl level I don't see any way libxl could
> cope without being told separately the domain's "real" name.
>
> I think this reduces the utility of this option quite substantially (or
> at least presents an enormous caveat), perhaps to the point where we may
> as well not bother to offer it?
I think we can detect the "--incoming" and remove it. The only problem
is that "--incoming" is actually set by xl (xl_cmdimpl.c), not libxl.
Could we break the abstraction layer just this time and in libxl_dm.c
have something like:
if ((strlen(c_info->name) > strlen("--incoming")) &&
(!strcmp(c_info->name + strlen(c_info->name) - strlen("--incoming"),
"--incoming"))) {
name = libxl__strdup(gc, c_info->name);
name[strlen(c_info->name) - strlen("--incoming")] = '\0';
} else {
name = c_info->name;
}
user = libxl__sprintf(gc, "%s-%s", LIBXL_QEMU_USER_PREFIX, name);
This is ugly but it was only meant to demonstrate the idea.
> > +
> > +
> > +2) a user named "xen-qemuuser-domid$domid",
> > +Where $domid is the domid of the domain being created.
> > +This requires the reservation of 65536 uids from xen-qemuuser-domid0
>
> Is xen-qemuuser-domid0 actually useful?
All right, it can be 1-65535 inclusive.
> > +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 in '' $(seq 0 65335)
>
> This creates a user with no $i on it too, which I think is a left over
> from the previous scheme and no longer useful.
You are right. I'll turn it into the more bash-like:
for ((i=1; i<65336; i++))
do
> > +do
> > + adduser --system xen-qemuuser-domid$i
> > +done
> > +
> > +
> > +3) 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
> > +
> > +
> > +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..79a9a22 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,32 @@ 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);
>
> This line is slightly too long. Wrapping buf_size would suffice.
OK
> > + 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 +467,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 +907,31 @@ static char **
> > libxl__build_device_model_args_new(libxl__gc *gc,
> > default:
> > break;
> > }
> > +
> > + user = libxl__sprintf(gc, "%s-%s", LIBXL_QEMU_USER_PREFIX,
> > c_info->name);
> > + if (libxl__dm_runas_helper(gc, user) > 0)
> > + goto end_search;
> > +
> > + 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-%s or user %s (+domid %d),
> > falling back to %s",
>
> This message ("+domid %d)" seems to refer to the old way of doing
> things.
OK
> > + LIBXL_QEMU_USER_PREFIX, c_info->name,
> > 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@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |