[prev in list] [next in list] [prev in thread] [next in thread] 

List:       xen-users
Subject:    [Xen-users] Xen Security Advisory 57 (CVE-2013-2211) - libxl allows guest write access to sensitive
From:       Xen.org security team <security () xen ! org>
Date:       2013-06-26 10:38:39
Message-ID: E1Urn7X-00056F-Ad () xenbits ! xen ! org
[Download RAW message or body]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

               Xen Security Advisory CVE-2013-2211 / XSA-57
                               version 4

 libxl allows guest write access to sensitive console related xenstore keys

UPDATES IN VERSION 4
====================

This issue has been assigned CVE-2013-2211.

ISSUE DESCRIPTION
=================

The libxenlight (libxl) toolstack library does not correctly set
permissions on xenstore keys relating to paravirtualised and emulated
serial console devices. This could allow a malicious guest
administrator to change values in xenstore which the host later relies
on being implicitly trusted.

This vulnerability has not yet been assigned a CVE Candidate number by
MITRE.  We will issue an updated version of XSA-57 when this is
available.

IMPACT
======

A malicious guest administrator can read and write any files in the
host filesystem which are accessible to the user id running the
xenconsole client binary. This may be the user id of a host
administrator who connects to the guest's console or the user id of
any self service mechanism provided to guest administrators by the
host provider.

As well as reading and writing files an attacker with access to an HVM
guest can cause any PV or serial consoles to be connected to a variety
of network resources (sockets, udp connections) or other end points
(fifo, pipes) in the host file filesystem according to the privileges
granted to the qemu device model for that guest.

A malicious guest administrator can also redirect the VNC console
port of the guest to another port on the host. This may expose the VNC
port of other guests or of other firewalled services to an attack.

VULNERABLE SYSTEMS
==================

All systems which use libxl as part of the toolstack are vulnerable.

libxl is present in Xen versions 4.0 onwards.

The major consumer of libxl functionality is the xl toolstack which
became the default in Xen 4.2.

In addition to this libvirt can optionally make use of libxl. This can
be queried with
        # virsh version

Which will report "xenlight" if libxl is in use. libvirt currently
prefers the xend backend if xend is running.

The xend and xapi toolstacks do not currently use libxl.

MITIGATION
==========

Host administrators can start a domain paused and manually correct the
xenstore permissions of the relevant nodes.

A domain can be started in the paused state with xl by using
    # xl create -p <cfg>

A domain's domid can then be determined with:
    # xl domid <name>

If using libvirt then virsh can be used instead:
    # virsh start --paused <name>
    # virsh domid <name>

For a domain $DOMID the following command will recursively correct the
permissions for the primary PV console:

    # xenstore-chmod -r /local/domain/$DOMID/console n0 r$DOMID

If the domain uses a device model stubdomain then it will also be
necessary to fix the permissions for the stubdomain. The stubdomain is
named "<name>-dm". Assuming its domain ID is $DMDOM:

    # xenstore-chmod -r /local/domain/$DMDOM/console n0 r$DMDOM

In addition a stub domain has three secondary PV consoles which must be
fixed, however in this case the "state" and "protocol" nodes along
with the device node itself should not be restricted. For each device
$D in [1,2,3]:

    # xenstore-chmod -r /local/domain/$DMDOM/device/console/$N n0 r$DMDOM
    # xenstore-chmod /local/domain/$DMDOM/device/console/$N/state n$DMDOM r0
    # xenstore-chmod /local/domain/$DMDOM/device/console/$N/protocol n$DMDOM r0
    # xenstore-chmod /local/domain/$DMDOM/device/console/$N n$DMDOM r0

The current permissions can be listed with
    # xenstore-ls -fp <PATH>

Once the permissions are fixed you may unpause the domain with
    # xl unpause <domain>
or with virsh:
    # virsh resume <domain>

The permissions can also be corrected on a live system if they are
then manually validated to be non-malicious.

See http://wiki.xen.org/wiki/XenBus#Permissions for information on the
permissions syntax.

RESOLUTION
==========

Applying the appropriate attached patch resolves this issue.

xsa57-4.2.patch             Xen 4.2.x
xsa57-4.1.patch             Xen 4.1.x
xsa57-unstable.patch        xen-unstable

$ sha256sum xsa57-*.patch
428a1d42f4314404cde339a78a59422bf4f0590c4d16ea8adc83425fe5eede3d  xsa57-4.1.patch
b6a5106848541972519cc529859d9ff3083c79367276c7031560fa4ce6f9f770  xsa57-4.2.patch
d329f56c30f7a4f91906658ea661234d2ca31b74ee68257bf009072999b3d3ef  xsa57-unstable.patch
$
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)

iQEcBAEBAgAGBQJRysSAAAoJEIP+FMlX6CvZ+9cH/R1sMTy9m9Vg7dopyMcSgtFz
1VatpxBUE0ldwv40t4kfMiKjocW/VUKV2j0HIOFCNh/XUTxtdO8SdVOsrQgfady2
IUGzRPIjnL82fRHcN1BNc81bViikDQ6R9cypA+R0V4X5sj8lwTtz5G73yoKnqWfb
2X57m0HT4pwySSTnhHyMyBdbBix8EdtjpyW3gzcrF1SmvQSIozz5NV80EpIWEnvY
x6uoVhCI6HD+JwH5xqn/E0oWvrc9v2+c300YIsTiXZcm7S19c+mphWO8o+wupwaX
xI1YfBO/YdxBlT5awFYtYLKwe6ld11K+AeonVapwRMiwyqMXRIiCSAmnjtSy7lg=
=vA8Q
-----END PGP SIGNATURE-----

["xsa57-4.1.patch" (application/octet-stream)]

libxl: Restrict permissions on PV console device xenstore nodes

Matthew Daley has observed that the PV console protocol places sensitive host
state into a guest writeable xenstore locations, this includes:

 - The pty used to communicate between the console backend daemon and its
   client, allowing the guest administrator to read and write arbitrary host
   files.
 - The output file, allowing the guest administrator to write arbitrary host
   files or to target arbitrary qemu chardevs which include sockets, udp, ptr,
   pipes etc (see -chardev in qemu(1) for a more complete list).
 - The maximum buffer size, allowing the guest administrator to consume more
   resources than the host administrator has configured.
 - The backend to use (qemu vs xenconsoled), potentially allowing the guest
   administrator to confuse host software.

So we arrange to make the sensitive keys in the xenstore frontend directory
read only for the guest. This is safe since the xenstore permissions model,
unlike POSIX directory permissions, does not allow the guest to remove and
recreate a node if it has write access to the containing directory.

There are a few associated wrinkles:

 - The primary PV console is "special". It's xenstore node is not under the
   usual /devices/ subtree and it does not use the customary xenstore state
   machine protocol. Unfortunately its directory is used for other things,
   including the vnc-port node, which we do not want the guest to be able to
   write to. Rather than trying to track down all the possible secondary uses
   of this directory just make it r/o to the guest. All newly created
   subdirectories inherit these permissions and so are now safe by default.

 - The other serial consoles do use the customary xenstore state machine and
   therefore need write access to at least the "protocol" and "state" nodes,
   however they may also want to use arbitrary "feature-foo" nodes (although
   I'm not aware of any) and therefore we cannot simply lock down the entire
   frontend directory. Instead we add support to libxl__device_generic_add for
   frontend keys which are explicitly read only and use that to lock down the
   sensitive keys.

 - Minios' console frontend wants to write the "type" node, which it has no
   business doing since this is a host/toolstack level decision. This fails
   now that the node has become read only to the PV guest. Since the toolstack
   already writes this node just remove the attempt to set it.

This is CVE-XXXX-XXX / XSA-57

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>

Conflicts (4.2 backport):
	tools/libxl/libxl.c (no vtpm, free front_ro on error in
	                     libxl__device_console_add)

Conflicts (4.1 backport):
	extras/mini-os/console/xenbus.c
	tools/libxl/libxl.c
	tools/libxl/libxl_device.c
	tools/libxl/libxl_internal.h
	tools/libxl/libxl_pci.c
	tools/libxl/libxl_xshelp.c

  - minios code was in xencons_ring.c
  - many places need &gc not just gc
  - libxl__xs_writev path is not const
  - varios minor context fixups

diff --git a/extras/mini-os/console/xencons_ring.c \
b/extras/mini-os/console/xencons_ring.c index 9ed3756..286c650 100644
--- a/extras/mini-os/console/xencons_ring.c
+++ b/extras/mini-os/console/xencons_ring.c
@@ -291,12 +291,6 @@ again:
         goto abort_transaction;
     }
 
-    err = xenbus_printf(xbt, nodename, "type", "%s", "ioemu");
-    if (err) {
-        message = "writing type";
-        goto abort_transaction;
-    }
-
     snprintf(path, sizeof(path), "%s/state", nodename);
     err = xenbus_switch_state(xbt, path, XenbusStateConnected);
     if (err) {
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 3c2e1b2..54f440c 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1036,8 +1036,9 @@ int libxl_device_disk_add(libxl_ctx *ctx, uint32_t domid, \
libxl_device_disk *dis  }
 
     libxl__device_generic_add(ctx, &device,
-                             libxl__xs_kvs_of_flexarray(&gc, back, back->count),
-                             libxl__xs_kvs_of_flexarray(&gc, front, front->count));
+                              libxl__xs_kvs_of_flexarray(&gc, back, back->count),
+                              libxl__xs_kvs_of_flexarray(&gc, front, front->count),
+                              NULL);
 
     rc = 0;
 
@@ -1266,8 +1267,9 @@ int libxl_device_nic_add(libxl_ctx *ctx, uint32_t domid, \
libxl_device_nic *nic)  }
 
     libxl__device_generic_add(ctx, &device,
-                             libxl__xs_kvs_of_flexarray(&gc, back, back->count),
-                             libxl__xs_kvs_of_flexarray(&gc, front, front->count));
+                              libxl__xs_kvs_of_flexarray(&gc, back, back->count),
+                              libxl__xs_kvs_of_flexarray(&gc, front, front->count),
+                              NULL);
 
     /* FIXME: wait for plug */
     rc = 0;
@@ -1478,8 +1480,9 @@ int libxl_device_net2_add(libxl_ctx *ctx, uint32_t domid, \
libxl_device_net2 *net  flexarray_append(front, "1");
 
     libxl__device_generic_add(ctx, &device,
-                             libxl__xs_kvs_of_flexarray(&gc, back, back->count),
-                             libxl__xs_kvs_of_flexarray(&gc, front, front->count));
+                              libxl__xs_kvs_of_flexarray(&gc, back, back->count),
+                              libxl__xs_kvs_of_flexarray(&gc, front, front->count),
+                              NULL);
 
     /* FIXME: wait for plug */
     rc = 0;
@@ -1571,7 +1574,7 @@ int libxl_device_net2_del(libxl_ctx *ctx, libxl_device_net2 \
*net2, int wait)  int libxl_device_console_add(libxl_ctx *ctx, uint32_t domid, \
libxl_device_console *console)  {
     libxl__gc gc = LIBXL_INIT_GC(ctx);
-    flexarray_t *front;
+    flexarray_t *front, *ro_front;
     flexarray_t *back;
     libxl__device device;
     int rc;
@@ -1581,6 +1584,11 @@ int libxl_device_console_add(libxl_ctx *ctx, uint32_t domid, \
libxl_device_consol  rc = ERROR_NOMEM;
         goto out;
     }
+    ro_front = flexarray_make(16, 1);
+    if (!ro_front) {
+        rc = ERROR_NOMEM;
+        goto out;
+    }
     back = flexarray_make(16, 1);
     if (!back) {
         rc = ERROR_NOMEM;
@@ -1607,25 +1615,27 @@ int libxl_device_console_add(libxl_ctx *ctx, uint32_t domid, \
libxl_device_consol  
     flexarray_append(front, "backend-id");
     flexarray_append(front, libxl__sprintf(&gc, "%d", console->backend_domid));
-    flexarray_append(front, "limit");
-    flexarray_append(front, libxl__sprintf(&gc, "%d", LIBXL_XENCONSOLE_LIMIT));
-    flexarray_append(front, "type");
+    flexarray_append(ro_front, "limit");
+    flexarray_append(ro_front, libxl__sprintf(&gc, "%d", LIBXL_XENCONSOLE_LIMIT));
+    flexarray_append(ro_front, "type");
     if (console->consback == LIBXL_CONSBACK_XENCONSOLED)
-        flexarray_append(front, "xenconsoled");
+        flexarray_append(ro_front, "xenconsoled");
     else
-        flexarray_append(front, "ioemu");
-    flexarray_append(front, "output");
-    flexarray_append(front, console->output);
+        flexarray_append(ro_front, "ioemu");
+    flexarray_append(ro_front, "output");
+    flexarray_append(ro_front, console->output);
+    flexarray_append(ro_front, "tty");
+    flexarray_append(ro_front, "");
 
     if (device.devid == 0) {
         if (console->build_state == NULL) {
             rc = ERROR_INVAL;
             goto out_free;
         }
-        flexarray_append(front, "port");
-        flexarray_append(front, libxl__sprintf(&gc, "%"PRIu32, \
                console->build_state->console_port));
-        flexarray_append(front, "ring-ref");
-        flexarray_append(front, libxl__sprintf(&gc, "%lu", \
console->build_state->console_mfn)); +        flexarray_append(ro_front, "port");
+        flexarray_append(ro_front, libxl__sprintf(&gc, "%"PRIu32, \
console->build_state->console_port)); +        flexarray_append(ro_front, \
"ring-ref"); +        flexarray_append(ro_front, libxl__sprintf(&gc, "%lu", \
console->build_state->console_mfn));  } else {
         flexarray_append(front, "state");
         flexarray_append(front, libxl__sprintf(&gc, "%d", 1));
@@ -1634,11 +1644,13 @@ int libxl_device_console_add(libxl_ctx *ctx, uint32_t domid, \
libxl_device_consol  }
 
     libxl__device_generic_add(ctx, &device,
-                             libxl__xs_kvs_of_flexarray(&gc, back, back->count),
-                             libxl__xs_kvs_of_flexarray(&gc, front, front->count));
+                              libxl__xs_kvs_of_flexarray(&gc, back, back->count),
+                              libxl__xs_kvs_of_flexarray(&gc, front, front->count),
+                              libxl__xs_kvs_of_flexarray(&gc, ro_front, \
ro_front->count));  rc = 0;
 out_free:
     flexarray_free(back);
+    flexarray_free(ro_front);
     flexarray_free(front);
 out:
     libxl__free_all(&gc);
@@ -1693,8 +1705,9 @@ int libxl_device_vkb_add(libxl_ctx *ctx, uint32_t domid, \
libxl_device_vkb *vkb)  flexarray_append(front, libxl__sprintf(&gc, "%d", 1));
 
     libxl__device_generic_add(ctx, &device,
-                             libxl__xs_kvs_of_flexarray(&gc, back, back->count),
-                             libxl__xs_kvs_of_flexarray(&gc, front, front->count));
+                              libxl__xs_kvs_of_flexarray(&gc, back, back->count),
+                              libxl__xs_kvs_of_flexarray(&gc, front, front->count),
+                              NULL);
     rc = 0;
 out_free:
     flexarray_free(back);
@@ -1921,8 +1934,9 @@ int libxl_device_vfb_add(libxl_ctx *ctx, uint32_t domid, \
libxl_device_vfb *vfb)  flexarray_append_pair(front, "state", libxl__sprintf(&gc, \
"%d", 1));  
     libxl__device_generic_add(ctx, &device,
-                             libxl__xs_kvs_of_flexarray(&gc, back, back->count),
-                             libxl__xs_kvs_of_flexarray(&gc, front, front->count));
+                              libxl__xs_kvs_of_flexarray(&gc, back, back->count),
+                              libxl__xs_kvs_of_flexarray(&gc, front, front->count),
+                              NULL);
     rc = 0;
 out_free:
     flexarray_free(front);
diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index 7e8fcef..0628840 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -62,12 +62,13 @@ char *libxl__device_backend_path(libxl__gc *gc, libxl__device \
*device)  }
 
 int libxl__device_generic_add(libxl_ctx *ctx, libxl__device *device,
-                             char **bents, char **fents)
+                              char **bents, char **fents, char **ro_fents)
 {
     libxl__gc gc = LIBXL_INIT_GC(ctx);
     char *frontend_path, *backend_path;
     xs_transaction_t t;
     struct xs_permissions frontend_perms[2];
+    struct xs_permissions ro_frontend_perms[2];
     struct xs_permissions backend_perms[2];
     int rc;
 
@@ -84,21 +85,36 @@ int libxl__device_generic_add(libxl_ctx *ctx, libxl__device \
*device,  frontend_perms[1].id = device->backend_domid;
     frontend_perms[1].perms = XS_PERM_READ;
 
-    backend_perms[0].id = device->backend_domid;
-    backend_perms[0].perms = XS_PERM_NONE;
-    backend_perms[1].id = device->domid;
-    backend_perms[1].perms = XS_PERM_READ;
+    ro_frontend_perms[0].id = backend_perms[0].id = device->backend_domid;
+    ro_frontend_perms[0].perms = backend_perms[0].perms = XS_PERM_NONE;
+    ro_frontend_perms[1].id = backend_perms[1].id = device->domid;
+    ro_frontend_perms[1].perms = backend_perms[1].perms = XS_PERM_READ;
 
 retry_transaction:
     t = xs_transaction_start(ctx->xsh);
     /* FIXME: read frontend_path and check state before removing stuff */
 
-    if (fents) {
+    if (fents || ro_fents) {
         xs_rm(ctx->xsh, t, frontend_path);
         xs_mkdir(ctx->xsh, t, frontend_path);
-        xs_set_permissions(ctx->xsh, t, frontend_path, frontend_perms, \
ARRAY_SIZE(frontend_perms)); +        /* Console 0 is a special case. It doesn't use \
the regular PV +         * state machine but also the frontend directory has
+         * historically contained other information, such as the
+         * vnc-port, which we don't want the guest fiddling with.
+         */
+        if (device->kind == DEVICE_CONSOLE && device->devid == 0)
+            xs_set_permissions(ctx->xsh, t, frontend_path,
+                               ro_frontend_perms, ARRAY_SIZE(ro_frontend_perms));
+        else
+            xs_set_permissions(ctx->xsh, t, frontend_path,
+                               frontend_perms, ARRAY_SIZE(frontend_perms));
         xs_write(ctx->xsh, t, libxl__sprintf(&gc, "%s/backend", frontend_path), \
                backend_path, strlen(backend_path));
-        libxl__xs_writev(&gc, t, frontend_path, fents);
+        if (fents)
+            libxl__xs_writev_perms(&gc, t, frontend_path, fents,
+                                   frontend_perms, ARRAY_SIZE(frontend_perms));
+        if (ro_fents)
+            libxl__xs_writev_perms(&gc, t, frontend_path, ro_fents,
+                                   ro_frontend_perms, \
ARRAY_SIZE(ro_frontend_perms));  }
 
     if (bents) {
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 9cf503f..5ddd27b 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -143,6 +143,11 @@ _hidden char **libxl__xs_kvs_of_flexarray(libxl__gc *gc, \
flexarray_t *array, int  
 _hidden int libxl__xs_writev(libxl__gc *gc, xs_transaction_t t,
                     char *dir, char **kvs);
+/* as writev but also sets the permissions on each path */
+_hidden int libxl__xs_writev_perms(libxl__gc *gc, xs_transaction_t t,
+                                   char *dir, char *kvs[],
+                                   struct xs_permissions *perms,
+                                   unsigned int num_perms);
 _hidden int libxl__xs_write(libxl__gc *gc, xs_transaction_t t,
                    char *path, char *fmt, ...) PRINTF_ATTRIBUTE(4, 5);
    /* Each fn returns 0 on success.
@@ -185,7 +190,7 @@ _hidden int libxl__device_physdisk_major_minor(const char \
*physpath, int *major,  _hidden int libxl__device_disk_dev_number(const char \
*virtpath);  
 _hidden int libxl__device_generic_add(libxl_ctx *ctx, libxl__device *device,
-                             char **bents, char **fents);
+                                      char **bents, char **fents, char **ro_fents);
 _hidden char *libxl__device_backend_path(libxl__gc *gc, libxl__device *device);
 _hidden char *libxl__device_frontend_path(libxl__gc *gc, libxl__device *device);
 _hidden int libxl__device_del(libxl_ctx *ctx, libxl__device *dev, int wait);
diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
index b1d05d9..9c76bce 100644
--- a/tools/libxl/libxl_pci.c
+++ b/tools/libxl/libxl_pci.c
@@ -274,8 +274,9 @@ static int libxl_create_pci_backend(libxl__gc *gc, uint32_t \
domid, libxl_device_  flexarray_append_pair(front, "state", libxl__sprintf(gc, "%d", \
1));  
     libxl__device_generic_add(ctx, &device,
-                             libxl__xs_kvs_of_flexarray(gc, back, back->count),
-                             libxl__xs_kvs_of_flexarray(gc, front, front->count));
+                              libxl__xs_kvs_of_flexarray(gc, back, back->count),
+                              libxl__xs_kvs_of_flexarray(gc, front, front->count),
+                              NULL);
 
 out:
     if (back)
diff --git a/tools/libxl/libxl_xshelp.c b/tools/libxl/libxl_xshelp.c
index 3dc9239..06b95e0 100644
--- a/tools/libxl/libxl_xshelp.c
+++ b/tools/libxl/libxl_xshelp.c
@@ -48,8 +48,10 @@ char **libxl__xs_kvs_of_flexarray(libxl__gc *gc, flexarray_t \
*array, int length)  return kvs;
 }
 
-int libxl__xs_writev(libxl__gc *gc, xs_transaction_t t,
-                    char *dir, char *kvs[])
+int libxl__xs_writev_perms(libxl__gc *gc, xs_transaction_t t,
+                           char *dir, char *kvs[],
+                           struct xs_permissions *perms,
+                           unsigned int num_perms)
 {
     libxl_ctx *ctx = libxl__gc_owner(gc);
     char *path;
@@ -63,11 +65,19 @@ int libxl__xs_writev(libxl__gc *gc, xs_transaction_t t,
         if (path && kvs[i + 1]) {
             int length = strlen(kvs[i + 1]);
             xs_write(ctx->xsh, t, path, kvs[i + 1], length);
+            if (perms)
+                xs_set_permissions(ctx->xsh, t, path, perms, num_perms);
         }
     }
     return 0;
 }
 
+int libxl__xs_writev(libxl__gc *gc, xs_transaction_t t,
+                    char *dir, char *kvs[])
+{
+    return libxl__xs_writev_perms(gc, t, dir, kvs, NULL, 0);
+}
+
 int libxl__xs_write(libxl__gc *gc, xs_transaction_t t,
                    char *path, char *fmt, ...)
 {


["xsa57-4.2.patch" (application/octet-stream)]

libxl: Restrict permissions on PV console device xenstore nodes

Matthew Daley has observed that the PV console protocol places sensitive host
state into a guest writeable xenstore locations, this includes:

 - The pty used to communicate between the console backend daemon and its
   client, allowing the guest administrator to read and write arbitrary host
   files.
 - The output file, allowing the guest administrator to write arbitrary host
   files or to target arbitrary qemu chardevs which include sockets, udp, ptr,
   pipes etc (see -chardev in qemu(1) for a more complete list).
 - The maximum buffer size, allowing the guest administrator to consume more
   resources than the host administrator has configured.
 - The backend to use (qemu vs xenconsoled), potentially allowing the guest
   administrator to confuse host software.

So we arrange to make the sensitive keys in the xenstore frontend directory
read only for the guest. This is safe since the xenstore permissions model,
unlike POSIX directory permissions, does not allow the guest to remove and
recreate a node if it has write access to the containing directory.

There are a few associated wrinkles:

 - The primary PV console is "special". It's xenstore node is not under the
   usual /devices/ subtree and it does not use the customary xenstore state
   machine protocol. Unfortunately its directory is used for other things,
   including the vnc-port node, which we do not want the guest to be able to
   write to. Rather than trying to track down all the possible secondary uses
   of this directory just make it r/o to the guest. All newly created
   subdirectories inherit these permissions and so are now safe by default.

 - The other serial consoles do use the customary xenstore state machine and
   therefore need write access to at least the "protocol" and "state" nodes,
   however they may also want to use arbitrary "feature-foo" nodes (although
   I'm not aware of any) and therefore we cannot simply lock down the entire
   frontend directory. Instead we add support to libxl__device_generic_add for
   frontend keys which are explicitly read only and use that to lock down the
   sensitive keys.

 - Minios' console frontend wants to write the "type" node, which it has no
   business doing since this is a host/toolstack level decision. This fails
   now that the node has become read only to the PV guest. Since the toolstack
   already writes this node just remove the attempt to set it.

This is CVE-XXXX-XXX / XSA-57

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>

Conflicts:
	tools/libxl/libxl.c (no vtpm, free front_ro on error in
	                     libxl__device_console_add)

diff --git a/extras/mini-os/console/xenbus.c b/extras/mini-os/console/xenbus.c
index 77de82a..e65baf7 100644
--- a/extras/mini-os/console/xenbus.c
+++ b/extras/mini-os/console/xenbus.c
@@ -122,12 +122,6 @@ again:
         goto abort_transaction;
     }
 
-    err = xenbus_printf(xbt, nodename, "type", "%s", "ioemu");
-    if (err) {
-        message = "writing type";
-        goto abort_transaction;
-    }
-
     snprintf(path, sizeof(path), "%s/state", nodename);
     err = xenbus_switch_state(xbt, path, XenbusStateConnected);
     if (err) {
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index a6e9601..32d788a 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1920,8 +1920,9 @@ static void device_disk_add(libxl__egc *egc, uint32_t domid,
         flexarray_append(front, disk->is_cdrom ? "cdrom" : "disk");
 
         libxl__device_generic_add(gc, t, device,
-                            libxl__xs_kvs_of_flexarray(gc, back, back->count),
-                            libxl__xs_kvs_of_flexarray(gc, front, front->count));
+                                  libxl__xs_kvs_of_flexarray(gc, back, back->count),
+                                  libxl__xs_kvs_of_flexarray(gc, front, \
front->count), +                                  NULL);
 
         rc = libxl__xs_transaction_commit(gc, &t);
         if (!rc) break;
@@ -2633,8 +2634,9 @@ void libxl__device_nic_add(libxl__egc *egc, uint32_t domid,
     flexarray_append(front, libxl__sprintf(gc,
                                     LIBXL_MAC_FMT, LIBXL_MAC_BYTES(nic->mac)));
     libxl__device_generic_add(gc, XBT_NULL, device,
-                             libxl__xs_kvs_of_flexarray(gc, back, back->count),
-                             libxl__xs_kvs_of_flexarray(gc, front, front->count));
+                              libxl__xs_kvs_of_flexarray(gc, back, back->count),
+                              libxl__xs_kvs_of_flexarray(gc, front, front->count),
+                              NULL);
 
     aodev->dev = device;
     aodev->action = DEVICE_CONNECT;
@@ -2830,7 +2832,7 @@ int libxl__device_console_add(libxl__gc *gc, uint32_t domid,
                               libxl__device_console *console,
                               libxl__domain_build_state *state)
 {
-    flexarray_t *front;
+    flexarray_t *front, *ro_front;
     flexarray_t *back;
     libxl__device device;
     int rc;
@@ -2845,6 +2847,11 @@ int libxl__device_console_add(libxl__gc *gc, uint32_t domid,
         rc = ERROR_NOMEM;
         goto out;
     }
+    ro_front = flexarray_make(16, 1);
+    if (!ro_front) {
+        rc = ERROR_NOMEM;
+        goto out;
+    }
     back = flexarray_make(16, 1);
     if (!back) {
         rc = ERROR_NOMEM;
@@ -2871,21 +2878,24 @@ int libxl__device_console_add(libxl__gc *gc, uint32_t domid,
 
     flexarray_append(front, "backend-id");
     flexarray_append(front, libxl__sprintf(gc, "%d", console->backend_domid));
-    flexarray_append(front, "limit");
-    flexarray_append(front, libxl__sprintf(gc, "%d", LIBXL_XENCONSOLE_LIMIT));
-    flexarray_append(front, "type");
+
+    flexarray_append(ro_front, "limit");
+    flexarray_append(ro_front, libxl__sprintf(gc, "%d", LIBXL_XENCONSOLE_LIMIT));
+    flexarray_append(ro_front, "type");
     if (console->consback == LIBXL__CONSOLE_BACKEND_XENCONSOLED)
-        flexarray_append(front, "xenconsoled");
+        flexarray_append(ro_front, "xenconsoled");
     else
-        flexarray_append(front, "ioemu");
-    flexarray_append(front, "output");
-    flexarray_append(front, console->output);
+        flexarray_append(ro_front, "ioemu");
+    flexarray_append(ro_front, "output");
+    flexarray_append(ro_front, console->output);
+    flexarray_append(ro_front, "tty");
+    flexarray_append(ro_front, "");
 
     if (state) {
-        flexarray_append(front, "port");
-        flexarray_append(front, libxl__sprintf(gc, "%"PRIu32, state->console_port));
-        flexarray_append(front, "ring-ref");
-        flexarray_append(front, libxl__sprintf(gc, "%lu", state->console_mfn));
+        flexarray_append(ro_front, "port");
+        flexarray_append(ro_front, libxl__sprintf(gc, "%"PRIu32, \
state->console_port)); +        flexarray_append(ro_front, "ring-ref");
+        flexarray_append(ro_front, libxl__sprintf(gc, "%lu", state->console_mfn));
     } else {
         flexarray_append(front, "state");
         flexarray_append(front, libxl__sprintf(gc, "%d", 1));
@@ -2894,11 +2904,13 @@ int libxl__device_console_add(libxl__gc *gc, uint32_t domid,
     }
 
     libxl__device_generic_add(gc, XBT_NULL, &device,
-                             libxl__xs_kvs_of_flexarray(gc, back, back->count),
-                             libxl__xs_kvs_of_flexarray(gc, front, front->count));
+                              libxl__xs_kvs_of_flexarray(gc, back, back->count),
+                              libxl__xs_kvs_of_flexarray(gc, front, front->count),
+                              libxl__xs_kvs_of_flexarray(gc, ro_front, \
ro_front->count));  rc = 0;
 out_free:
     flexarray_free(back);
+    flexarray_free(ro_front);
     flexarray_free(front);
 out:
     return rc;
@@ -2982,8 +2994,9 @@ int libxl__device_vkb_add(libxl__gc *gc, uint32_t domid,
     flexarray_append(front, libxl__sprintf(gc, "%d", 1));
 
     libxl__device_generic_add(gc, XBT_NULL, &device,
-                             libxl__xs_kvs_of_flexarray(gc, back, back->count),
-                             libxl__xs_kvs_of_flexarray(gc, front, front->count));
+                              libxl__xs_kvs_of_flexarray(gc, back, back->count),
+                              libxl__xs_kvs_of_flexarray(gc, front, front->count),
+                              NULL);
     rc = 0;
 out_free:
     flexarray_free(back);
@@ -3096,8 +3109,9 @@ int libxl__device_vfb_add(libxl__gc *gc, uint32_t domid, \
libxl_device_vfb *vfb)  flexarray_append_pair(front, "state", libxl__sprintf(gc, \
"%d", 1));  
     libxl__device_generic_add(gc, XBT_NULL, &device,
-                             libxl__xs_kvs_of_flexarray(gc, back, back->count),
-                             libxl__xs_kvs_of_flexarray(gc, front, front->count));
+                              libxl__xs_kvs_of_flexarray(gc, back, back->count),
+                              libxl__xs_kvs_of_flexarray(gc, front, front->count),
+                              NULL);
     rc = 0;
 out_free:
     flexarray_free(front);
diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index c3283f1..1c04a21 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -84,11 +84,12 @@ out:
 }
 
 int libxl__device_generic_add(libxl__gc *gc, xs_transaction_t t,
-        libxl__device *device, char **bents, char **fents)
+        libxl__device *device, char **bents, char **fents, char **ro_fents)
 {
     libxl_ctx *ctx = libxl__gc_owner(gc);
     char *frontend_path, *backend_path;
     struct xs_permissions frontend_perms[2];
+    struct xs_permissions ro_frontend_perms[2];
     struct xs_permissions backend_perms[2];
     int create_transaction = t == XBT_NULL;
 
@@ -100,22 +101,37 @@ int libxl__device_generic_add(libxl__gc *gc, xs_transaction_t \
t,  frontend_perms[1].id = device->backend_domid;
     frontend_perms[1].perms = XS_PERM_READ;
 
-    backend_perms[0].id = device->backend_domid;
-    backend_perms[0].perms = XS_PERM_NONE;
-    backend_perms[1].id = device->domid;
-    backend_perms[1].perms = XS_PERM_READ;
+    ro_frontend_perms[0].id = backend_perms[0].id = device->backend_domid;
+    ro_frontend_perms[0].perms = backend_perms[0].perms = XS_PERM_NONE;
+    ro_frontend_perms[1].id = backend_perms[1].id = device->domid;
+    ro_frontend_perms[1].perms = backend_perms[1].perms = XS_PERM_READ;
 
 retry_transaction:
     if (create_transaction)
         t = xs_transaction_start(ctx->xsh);
     /* FIXME: read frontend_path and check state before removing stuff */
 
-    if (fents) {
+    if (fents || ro_fents) {
         xs_rm(ctx->xsh, t, frontend_path);
         xs_mkdir(ctx->xsh, t, frontend_path);
-        xs_set_permissions(ctx->xsh, t, frontend_path, frontend_perms, \
ARRAY_SIZE(frontend_perms)); +        /* Console 0 is a special case. It doesn't use \
the regular PV +         * state machine but also the frontend directory has
+         * historically contained other information, such as the
+         * vnc-port, which we don't want the guest fiddling with.
+         */
+        if (device->kind == LIBXL__DEVICE_KIND_CONSOLE && device->devid == 0)
+            xs_set_permissions(ctx->xsh, t, frontend_path,
+                               ro_frontend_perms, ARRAY_SIZE(ro_frontend_perms));
+        else
+            xs_set_permissions(ctx->xsh, t, frontend_path,
+                               frontend_perms, ARRAY_SIZE(frontend_perms));
         xs_write(ctx->xsh, t, libxl__sprintf(gc, "%s/backend", frontend_path), \
                backend_path, strlen(backend_path));
-        libxl__xs_writev(gc, t, frontend_path, fents);
+        if (fents)
+            libxl__xs_writev_perms(gc, t, frontend_path, fents,
+                                   frontend_perms, ARRAY_SIZE(frontend_perms));
+        if (ro_fents)
+            libxl__xs_writev_perms(gc, t, frontend_path, ro_fents,
+                                   ro_frontend_perms, \
ARRAY_SIZE(ro_frontend_perms));  }
 
     if (bents) {
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 13fa509..ae96a74 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -516,6 +516,11 @@ _hidden char **libxl__xs_kvs_of_flexarray(libxl__gc *gc, \
flexarray_t *array, int  /* treats kvs as pairs of keys and values and writes each to \
dir. */  _hidden int libxl__xs_writev(libxl__gc *gc, xs_transaction_t t,
                              const char *dir, char **kvs);
+/* as writev but also sets the permissions on each path */
+_hidden int libxl__xs_writev_perms(libxl__gc *gc, xs_transaction_t t,
+                                   const char *dir, char *kvs[],
+                                   struct xs_permissions *perms,
+                                   unsigned int num_perms);
 /* _atonce creates a transaction and writes all keys at once */
 _hidden int libxl__xs_writev_atonce(libxl__gc *gc,
                              const char *dir, char **kvs);
@@ -930,7 +935,7 @@ _hidden int libxl__device_console_add(libxl__gc *gc, uint32_t \
domid,  libxl__domain_build_state *state);
 
 _hidden int libxl__device_generic_add(libxl__gc *gc, xs_transaction_t t,
-        libxl__device *device, char **bents, char **fents);
+        libxl__device *device, char **bents, char **fents, char **ro_fents);
 _hidden char *libxl__device_backend_path(libxl__gc *gc, libxl__device *device);
 _hidden char *libxl__device_frontend_path(libxl__gc *gc, libxl__device *device);
 _hidden int libxl__parse_backend_path(libxl__gc *gc, const char *path,
diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
index 48986f3..d373b4d 100644
--- a/tools/libxl/libxl_pci.c
+++ b/tools/libxl/libxl_pci.c
@@ -106,7 +106,8 @@ int libxl__create_pci_backend(libxl__gc *gc, uint32_t domid,
 
     libxl__device_generic_add(gc, XBT_NULL, &device,
                               libxl__xs_kvs_of_flexarray(gc, back, back->count),
-                              libxl__xs_kvs_of_flexarray(gc, front, front->count));
+                              libxl__xs_kvs_of_flexarray(gc, front, front->count),
+                              NULL);
 
 out:
     if (back)
diff --git a/tools/libxl/libxl_xshelp.c b/tools/libxl/libxl_xshelp.c
index 52af484..d7eaa66 100644
--- a/tools/libxl/libxl_xshelp.c
+++ b/tools/libxl/libxl_xshelp.c
@@ -41,8 +41,10 @@ char **libxl__xs_kvs_of_flexarray(libxl__gc *gc, flexarray_t \
*array, int length)  return kvs;
 }
 
-int libxl__xs_writev(libxl__gc *gc, xs_transaction_t t,
-                     const char *dir, char *kvs[])
+int libxl__xs_writev_perms(libxl__gc *gc, xs_transaction_t t,
+                           const char *dir, char *kvs[],
+                           struct xs_permissions *perms,
+                           unsigned int num_perms)
 {
     libxl_ctx *ctx = libxl__gc_owner(gc);
     char *path;
@@ -56,11 +58,19 @@ int libxl__xs_writev(libxl__gc *gc, xs_transaction_t t,
         if (path && kvs[i + 1]) {
             int length = strlen(kvs[i + 1]);
             xs_write(ctx->xsh, t, path, kvs[i + 1], length);
+            if (perms)
+                xs_set_permissions(ctx->xsh, t, path, perms, num_perms);
         }
     }
     return 0;
 }
 
+int libxl__xs_writev(libxl__gc *gc, xs_transaction_t t,
+                     const char *dir, char *kvs[])
+{
+    return libxl__xs_writev_perms(gc, t, dir, kvs, NULL, 0);
+}
+
 int libxl__xs_writev_atonce(libxl__gc *gc,
                             const char *dir, char *kvs[])
 {


["xsa57-unstable.patch" (application/octet-stream)]

libxl: Restrict permissions on PV console device xenstore nodes

Matthew Daley has observed that the PV console protocol places sensitive host
state into a guest writeable xenstore locations, this includes:

 - The pty used to communicate between the console backend daemon and its
   client, allowing the guest administrator to read and write arbitrary host
   files.
 - The output file, allowing the guest administrator to write arbitrary host
   files or to target arbitrary qemu chardevs which include sockets, udp, ptr,
   pipes etc (see -chardev in qemu(1) for a more complete list).
 - The maximum buffer size, allowing the guest administrator to consume more
   resources than the host administrator has configured.
 - The backend to use (qemu vs xenconsoled), potentially allowing the guest
   administrator to confuse host software.

So we arrange to make the sensitive keys in the xenstore frontend directory
read only for the guest. This is safe since the xenstore permissions model,
unlike POSIX directory permissions, does not allow the guest to remove and
recreate a node if it has write access to the containing directory.

There are a few associated wrinkles:

 - The primary PV console is "special". It's xenstore node is not under the
   usual /devices/ subtree and it does not use the customary xenstore state
   machine protocol. Unfortunately its directory is used for other things,
   including the vnc-port node, which we do not want the guest to be able to
   write to. Rather than trying to track down all the possible secondary uses
   of this directory just make it r/o to the guest. All newly created
   subdirectories inherit these permissions and so are now safe by default.

 - The other serial consoles do use the customary xenstore state machine and
   therefore need write access to at least the "protocol" and "state" nodes,
   however they may also want to use arbitrary "feature-foo" nodes (although
   I'm not aware of any) and therefore we cannot simply lock down the entire
   frontend directory. Instead we add support to libxl__device_generic_add for
   frontend keys which are explicitly read only and use that to lock down the
   sensitive keys.

 - Minios' console frontend wants to write the "type" node, which it has no
   business doing since this is a host/toolstack level decision. This fails
   now that the node has become read only to the PV guest. Since the toolstack
   already writes this node just remove the attempt to set it.

This is CVE-XXXX-XXX / XSA-57

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>

diff --git a/extras/mini-os/console/xenbus.c b/extras/mini-os/console/xenbus.c
index 77de82a..e65baf7 100644
--- a/extras/mini-os/console/xenbus.c
+++ b/extras/mini-os/console/xenbus.c
@@ -122,12 +122,6 @@ again:
         goto abort_transaction;
     }
 
-    err = xenbus_printf(xbt, nodename, "type", "%s", "ioemu");
-    if (err) {
-        message = "writing type";
-        goto abort_transaction;
-    }
-
     snprintf(path, sizeof(path), "%s/state", nodename);
     err = xenbus_switch_state(xbt, path, XenbusStateConnected);
     if (err) {
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index ee1fa9c..0612d85 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1813,8 +1813,9 @@ void libxl__device_vtpm_add(libxl__egc *egc, uint32_t domid,
     flexarray_append(front, GCSPRINTF("%d", vtpm->devid));
 
     libxl__device_generic_add(gc, XBT_NULL, device,
-                             libxl__xs_kvs_of_flexarray(gc, back, back->count),
-                             libxl__xs_kvs_of_flexarray(gc, front, front->count));
+                              libxl__xs_kvs_of_flexarray(gc, back, back->count),
+                              libxl__xs_kvs_of_flexarray(gc, front, front->count),
+                              NULL);
 
     aodev->dev = device;
     aodev->action = LIBXL__DEVICE_ACTION_ADD;
@@ -2195,8 +2196,9 @@ static void device_disk_add(libxl__egc *egc, uint32_t domid,
         }
 
         libxl__device_generic_add(gc, t, device,
-                            libxl__xs_kvs_of_flexarray(gc, back, back->count),
-                            libxl__xs_kvs_of_flexarray(gc, front, front->count));
+                                  libxl__xs_kvs_of_flexarray(gc, back, back->count),
+                                  libxl__xs_kvs_of_flexarray(gc, front, \
front->count), +                                  NULL);
 
         rc = libxl__xs_transaction_commit(gc, &t);
         if (!rc) break;
@@ -2938,8 +2940,9 @@ void libxl__device_nic_add(libxl__egc *egc, uint32_t domid,
     flexarray_append(front, libxl__sprintf(gc,
                                     LIBXL_MAC_FMT, LIBXL_MAC_BYTES(nic->mac)));
     libxl__device_generic_add(gc, XBT_NULL, device,
-                             libxl__xs_kvs_of_flexarray(gc, back, back->count),
-                             libxl__xs_kvs_of_flexarray(gc, front, front->count));
+                              libxl__xs_kvs_of_flexarray(gc, back, back->count),
+                              libxl__xs_kvs_of_flexarray(gc, front, front->count),
+                              NULL);
 
     aodev->dev = device;
     aodev->action = LIBXL__DEVICE_ACTION_ADD;
@@ -3132,7 +3135,7 @@ int libxl__device_console_add(libxl__gc *gc, uint32_t domid,
                               libxl__device_console *console,
                               libxl__domain_build_state *state)
 {
-    flexarray_t *front;
+    flexarray_t *front, *ro_front;
     flexarray_t *back;
     libxl__device device;
     int rc;
@@ -3143,6 +3146,7 @@ int libxl__device_console_add(libxl__gc *gc, uint32_t domid,
     }
 
     front = flexarray_make(gc, 16, 1);
+    ro_front = flexarray_make(gc, 16, 1);
     back = flexarray_make(gc, 16, 1);
 
     device.backend_devid = console->devid;
@@ -3165,21 +3169,24 @@ int libxl__device_console_add(libxl__gc *gc, uint32_t domid,
 
     flexarray_append(front, "backend-id");
     flexarray_append(front, libxl__sprintf(gc, "%d", console->backend_domid));
-    flexarray_append(front, "limit");
-    flexarray_append(front, libxl__sprintf(gc, "%d", LIBXL_XENCONSOLE_LIMIT));
-    flexarray_append(front, "type");
+
+    flexarray_append(ro_front, "limit");
+    flexarray_append(ro_front, libxl__sprintf(gc, "%d", LIBXL_XENCONSOLE_LIMIT));
+    flexarray_append(ro_front, "type");
     if (console->consback == LIBXL__CONSOLE_BACKEND_XENCONSOLED)
-        flexarray_append(front, "xenconsoled");
+        flexarray_append(ro_front, "xenconsoled");
     else
-        flexarray_append(front, "ioemu");
-    flexarray_append(front, "output");
-    flexarray_append(front, console->output);
+        flexarray_append(ro_front, "ioemu");
+    flexarray_append(ro_front, "output");
+    flexarray_append(ro_front, console->output);
+    flexarray_append(ro_front, "tty");
+    flexarray_append(ro_front, "");
 
     if (state) {
-        flexarray_append(front, "port");
-        flexarray_append(front, libxl__sprintf(gc, "%"PRIu32, state->console_port));
-        flexarray_append(front, "ring-ref");
-        flexarray_append(front, libxl__sprintf(gc, "%lu", state->console_mfn));
+        flexarray_append(ro_front, "port");
+        flexarray_append(ro_front, libxl__sprintf(gc, "%"PRIu32, \
state->console_port)); +        flexarray_append(ro_front, "ring-ref");
+        flexarray_append(ro_front, libxl__sprintf(gc, "%lu", state->console_mfn));
     } else {
         flexarray_append(front, "state");
         flexarray_append(front, libxl__sprintf(gc, "%d", 1));
@@ -3188,8 +3195,9 @@ int libxl__device_console_add(libxl__gc *gc, uint32_t domid,
     }
 
     libxl__device_generic_add(gc, XBT_NULL, &device,
-                             libxl__xs_kvs_of_flexarray(gc, back, back->count),
-                             libxl__xs_kvs_of_flexarray(gc, front, front->count));
+                              libxl__xs_kvs_of_flexarray(gc, back, back->count),
+                              libxl__xs_kvs_of_flexarray(gc, front, front->count),
+                              libxl__xs_kvs_of_flexarray(gc, ro_front, \
ro_front->count));  rc = 0;
 out:
     return rc;
@@ -3274,8 +3282,9 @@ int libxl__device_vkb_add(libxl__gc *gc, uint32_t domid,
     flexarray_append(front, libxl__sprintf(gc, "%d", 1));
 
     libxl__device_generic_add(gc, XBT_NULL, &device,
-                             libxl__xs_kvs_of_flexarray(gc, back, back->count),
-                             libxl__xs_kvs_of_flexarray(gc, front, front->count));
+                              libxl__xs_kvs_of_flexarray(gc, back, back->count),
+                              libxl__xs_kvs_of_flexarray(gc, front, front->count),
+                              NULL);
     rc = 0;
 out:
     return rc;
@@ -3387,8 +3396,9 @@ int libxl__device_vfb_add(libxl__gc *gc, uint32_t domid, \
libxl_device_vfb *vfb)  flexarray_append_pair(front, "state", libxl__sprintf(gc, \
"%d", 1));  
     libxl__device_generic_add(gc, XBT_NULL, &device,
-                             libxl__xs_kvs_of_flexarray(gc, back, back->count),
-                             libxl__xs_kvs_of_flexarray(gc, front, front->count));
+                              libxl__xs_kvs_of_flexarray(gc, back, back->count),
+                              libxl__xs_kvs_of_flexarray(gc, front, front->count),
+                              NULL);
     rc = 0;
 out:
     return rc;
diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index bc86648..ea845b7 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -84,11 +84,12 @@ out:
 }
 
 int libxl__device_generic_add(libxl__gc *gc, xs_transaction_t t,
-        libxl__device *device, char **bents, char **fents)
+        libxl__device *device, char **bents, char **fents, char **ro_fents)
 {
     libxl_ctx *ctx = libxl__gc_owner(gc);
     char *frontend_path, *backend_path;
     struct xs_permissions frontend_perms[2];
+    struct xs_permissions ro_frontend_perms[2];
     struct xs_permissions backend_perms[2];
     int create_transaction = t == XBT_NULL;
 
@@ -100,22 +101,37 @@ int libxl__device_generic_add(libxl__gc *gc, xs_transaction_t \
t,  frontend_perms[1].id = device->backend_domid;
     frontend_perms[1].perms = XS_PERM_READ;
 
-    backend_perms[0].id = device->backend_domid;
-    backend_perms[0].perms = XS_PERM_NONE;
-    backend_perms[1].id = device->domid;
-    backend_perms[1].perms = XS_PERM_READ;
+    ro_frontend_perms[0].id = backend_perms[0].id = device->backend_domid;
+    ro_frontend_perms[0].perms = backend_perms[0].perms = XS_PERM_NONE;
+    ro_frontend_perms[1].id = backend_perms[1].id = device->domid;
+    ro_frontend_perms[1].perms = backend_perms[1].perms = XS_PERM_READ;
 
 retry_transaction:
     if (create_transaction)
         t = xs_transaction_start(ctx->xsh);
     /* FIXME: read frontend_path and check state before removing stuff */
 
-    if (fents) {
+    if (fents || ro_fents) {
         xs_rm(ctx->xsh, t, frontend_path);
         xs_mkdir(ctx->xsh, t, frontend_path);
-        xs_set_permissions(ctx->xsh, t, frontend_path, frontend_perms, \
ARRAY_SIZE(frontend_perms)); +        /* Console 0 is a special case. It doesn't use \
the regular PV +         * state machine but also the frontend directory has
+         * historically contained other information, such as the
+         * vnc-port, which we don't want the guest fiddling with.
+         */
+        if (device->kind == LIBXL__DEVICE_KIND_CONSOLE && device->devid == 0)
+            xs_set_permissions(ctx->xsh, t, frontend_path,
+                               ro_frontend_perms, ARRAY_SIZE(ro_frontend_perms));
+        else
+            xs_set_permissions(ctx->xsh, t, frontend_path,
+                               frontend_perms, ARRAY_SIZE(frontend_perms));
         xs_write(ctx->xsh, t, libxl__sprintf(gc, "%s/backend", frontend_path), \
                backend_path, strlen(backend_path));
-        libxl__xs_writev(gc, t, frontend_path, fents);
+        if (fents)
+            libxl__xs_writev_perms(gc, t, frontend_path, fents,
+                                   frontend_perms, ARRAY_SIZE(frontend_perms));
+        if (ro_fents)
+            libxl__xs_writev_perms(gc, t, frontend_path, ro_fents,
+                                   ro_frontend_perms, \
ARRAY_SIZE(ro_frontend_perms));  }
 
     if (bents) {
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 3ba3a21..00ff6b9 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -519,6 +519,11 @@ _hidden char **libxl__xs_kvs_of_flexarray(libxl__gc *gc, \
flexarray_t *array, int  /* treats kvs as pairs of keys and values and writes each to \
dir. */  _hidden int libxl__xs_writev(libxl__gc *gc, xs_transaction_t t,
                              const char *dir, char **kvs);
+/* as writev but also sets the permissions on each path */
+_hidden int libxl__xs_writev_perms(libxl__gc *gc, xs_transaction_t t,
+                                   const char *dir, char *kvs[],
+                                   struct xs_permissions *perms,
+                                   unsigned int num_perms);
 /* _atonce creates a transaction and writes all keys at once */
 _hidden int libxl__xs_writev_atonce(libxl__gc *gc,
                              const char *dir, char **kvs);
@@ -933,7 +938,7 @@ _hidden int libxl__device_console_add(libxl__gc *gc, uint32_t \
domid,  libxl__domain_build_state *state);
 
 _hidden int libxl__device_generic_add(libxl__gc *gc, xs_transaction_t t,
-        libxl__device *device, char **bents, char **fents);
+        libxl__device *device, char **bents, char **fents, char **ro_fents);
 _hidden char *libxl__device_backend_path(libxl__gc *gc, libxl__device *device);
 _hidden char *libxl__device_frontend_path(libxl__gc *gc, libxl__device *device);
 _hidden int libxl__parse_backend_path(libxl__gc *gc, const char *path,
diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
index eac35c1..2f9f010 100644
--- a/tools/libxl/libxl_pci.c
+++ b/tools/libxl/libxl_pci.c
@@ -102,7 +102,8 @@ int libxl__create_pci_backend(libxl__gc *gc, uint32_t domid,
 
     libxl__device_generic_add(gc, XBT_NULL, &device,
                               libxl__xs_kvs_of_flexarray(gc, back, back->count),
-                              libxl__xs_kvs_of_flexarray(gc, front, front->count));
+                              libxl__xs_kvs_of_flexarray(gc, front, front->count),
+                              NULL);
 
     return 0;
 }
diff --git a/tools/libxl/libxl_xshelp.c b/tools/libxl/libxl_xshelp.c
index 52af484..d7eaa66 100644
--- a/tools/libxl/libxl_xshelp.c
+++ b/tools/libxl/libxl_xshelp.c
@@ -41,8 +41,10 @@ char **libxl__xs_kvs_of_flexarray(libxl__gc *gc, flexarray_t \
*array, int length)  return kvs;
 }
 
-int libxl__xs_writev(libxl__gc *gc, xs_transaction_t t,
-                     const char *dir, char *kvs[])
+int libxl__xs_writev_perms(libxl__gc *gc, xs_transaction_t t,
+                           const char *dir, char *kvs[],
+                           struct xs_permissions *perms,
+                           unsigned int num_perms)
 {
     libxl_ctx *ctx = libxl__gc_owner(gc);
     char *path;
@@ -56,11 +58,19 @@ int libxl__xs_writev(libxl__gc *gc, xs_transaction_t t,
         if (path && kvs[i + 1]) {
             int length = strlen(kvs[i + 1]);
             xs_write(ctx->xsh, t, path, kvs[i + 1], length);
+            if (perms)
+                xs_set_permissions(ctx->xsh, t, path, perms, num_perms);
         }
     }
     return 0;
 }
 
+int libxl__xs_writev(libxl__gc *gc, xs_transaction_t t,
+                     const char *dir, char *kvs[])
+{
+    return libxl__xs_writev_perms(gc, t, dir, kvs, NULL, 0);
+}
+
 int libxl__xs_writev_atonce(libxl__gc *gc,
                             const char *dir, char *kvs[])
 {



_______________________________________________
Xen-users mailing list
Xen-users@lists.xen.org
http://lists.xen.org/xen-users

[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic