|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v3] run QEMU as non-root
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@xxxxxxxxxxxxx>
> >
> > ---
> > 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
> >
> > /*
>
>
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |