References: bnc#823608 XSA-57

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

--- a/extras/mini-os/console/xencons_ring.c
+++ b/extras/mini-os/console/xencons_ring.c
@@ -264,12 +264,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) {
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1404,7 +1404,8 @@ int libxl_device_disk_add(struct libxl_c
 
     libxl_device_generic_add(ctx, &device,
                              libxl_xs_kvs_of_flexarray(ctx, back, boffset),
-                             libxl_xs_kvs_of_flexarray(ctx, front, foffset));
+                             libxl_xs_kvs_of_flexarray(ctx, front, foffset),
+                             NULL);
     flexarray_free(back);
     flexarray_free(front);
     return 0;
@@ -1499,7 +1500,8 @@ int libxl_device_nic_add(struct libxl_ct
 
     libxl_device_generic_add(ctx, &device,
                              libxl_xs_kvs_of_flexarray(ctx, back, boffset),
-                             libxl_xs_kvs_of_flexarray(ctx, front, foffset));
+                             libxl_xs_kvs_of_flexarray(ctx, front, foffset),
+                             NULL);
 
     /* FIXME: wait for plug */
     flexarray_free(back);
@@ -1585,10 +1587,10 @@ libxl_nicinfo *libxl_list_nics(struct li
 /******************************************************************************/
 int libxl_device_console_add(struct libxl_ctx *ctx, uint32_t domid, libxl_device_console *console)
 {
-    flexarray_t *front;
+    flexarray_t *front, *ro_front;
     flexarray_t *back;
     unsigned int boffset = 0;
-    unsigned int foffset = 0;
+    unsigned int foffset = 0, roffset = 0;
     libxl_device device;
 
     if (console->build_state) {
@@ -1616,6 +1618,9 @@ retry_transaction:
     front = flexarray_make(16, 1);
     if (!front)
         return ERROR_NOMEM;
+    ro_front = flexarray_make(16, 1);
+    if (!ro_front)
+        return ERROR_NOMEM;
     back = flexarray_make(16, 1);
     if (!back)
         return ERROR_NOMEM;
@@ -1642,20 +1647,22 @@ retry_transaction:
     flexarray_set(front, foffset++, libxl_sprintf(ctx, "%d", console->backend_domid));
     flexarray_set(front, foffset++, "state");
     flexarray_set(front, foffset++, libxl_sprintf(ctx, "%d", 1));
-    flexarray_set(front, foffset++, "limit");
-    flexarray_set(front, foffset++, libxl_sprintf(ctx, "%d", LIBXL_XENCONSOLE_LIMIT));
-    flexarray_set(front, foffset++, "protocol");
-    flexarray_set(front, foffset++, LIBXL_XENCONSOLE_PROTOCOL);
-    flexarray_set(front, foffset++, "type");
+    flexarray_set(ro_front, roffset++, "limit");
+    flexarray_set(ro_front, roffset++, libxl_sprintf(ctx, "%d", LIBXL_XENCONSOLE_LIMIT));
+    flexarray_set(ro_front, roffset++, "protocol");
+    flexarray_set(ro_front, roffset++, LIBXL_XENCONSOLE_PROTOCOL);
+    flexarray_set(ro_front, roffset++, "type");
     if (console->constype == CONSTYPE_XENCONSOLED)
-        flexarray_set(front, foffset++, "xenconsoled");
+        flexarray_set(ro_front, roffset++, "xenconsoled");
     else
-        flexarray_set(front, foffset++, "ioemu");
+        flexarray_set(ro_front, roffset++, "ioemu");
 
     libxl_device_generic_add(ctx, &device,
                              libxl_xs_kvs_of_flexarray(ctx, back, boffset),
-                             libxl_xs_kvs_of_flexarray(ctx, front, foffset));
+                             libxl_xs_kvs_of_flexarray(ctx, front, foffset),
+                             libxl_xs_kvs_of_flexarray(ctx, ro_front, roffset));
     flexarray_free(back);
+    flexarray_free(ro_front);
     flexarray_free(front);
 
     return 0;
@@ -1700,7 +1707,8 @@ int libxl_device_vkb_add(struct libxl_ct
 
     libxl_device_generic_add(ctx, &device,
                              libxl_xs_kvs_of_flexarray(ctx, back, boffset),
-                             libxl_xs_kvs_of_flexarray(ctx, front, foffset));
+                             libxl_xs_kvs_of_flexarray(ctx, front, foffset),
+                             NULL);
     flexarray_free(back);
     flexarray_free(front);
 
@@ -1963,7 +1971,8 @@ int libxl_device_vfb_add(struct libxl_ct
 
     libxl_device_generic_add(ctx, &device,
                              libxl_xs_kvs_of_flexarray(ctx, back, boffset),
-                             libxl_xs_kvs_of_flexarray(ctx, front, foffset));
+                             libxl_xs_kvs_of_flexarray(ctx, front, foffset),
+                             NULL);
     flexarray_free(front);
     flexarray_free(back);
 
@@ -2052,7 +2061,8 @@ static int libxl_create_pci_backend(stru
 
     libxl_device_generic_add(ctx, &device,
                              libxl_xs_kvs_of_flexarray(ctx, back, boffset),
-                             libxl_xs_kvs_of_flexarray(ctx, front, foffset));
+                             libxl_xs_kvs_of_flexarray(ctx, front, foffset),
+                             NULL);
 
     flexarray_free(back);
     flexarray_free(front);
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -38,11 +38,12 @@ static const char *string_of_kinds[] = {
 };
 
 int libxl_device_generic_add(struct libxl_ctx *ctx, libxl_device *device,
-                             char **bents, char **fents)
+                             char **bents, char **fents, char **ro_fents)
 {
     char *dom_path_backend, *dom_path, *frontend_path, *backend_path, *hotplug_path;
     xs_transaction_t t;
     struct xs_permissions frontend_perms[2];
+    struct xs_permissions ro_frontend_perms[2];
     struct xs_permissions backend_perms[2];
     struct xs_permissions hotplug_perms[1];
 
@@ -64,10 +65,10 @@ int libxl_device_generic_add(struct libx
     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;
 
     hotplug_perms[0].id = device->backend_domid;
     hotplug_perms[0].perms = XS_PERM_NONE;
@@ -80,7 +81,17 @@ retry_transaction:
     xs_rm(ctx->xsh, t, backend_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_mkdir(ctx->xsh, t, backend_path);
     xs_set_permissions(ctx->xsh, t, backend_path, backend_perms, ARRAY_SIZE(backend_perms));
@@ -93,7 +104,12 @@ retry_transaction:
 
     /* and write frontend kvs and backend kvs */
     libxl_xs_writev(ctx, t, backend_path, bents);
-    libxl_xs_writev(ctx, t, frontend_path, fents);
+    if (fents)
+        libxl_xs_writev_perms(ctx, t, frontend_path, fents,
+                              frontend_perms, ARRAY_SIZE(frontend_perms));
+    if (ro_fents)
+        libxl_xs_writev_perms(ctx, t, frontend_path, ro_fents,
+                              ro_frontend_perms, ARRAY_SIZE(ro_frontend_perms));
 
     if (!xs_transaction_end(ctx->xsh, t, 0)) {
         if (errno == EAGAIN)
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -104,6 +104,11 @@ char *libxl_dirname(struct libxl_ctx *ct
 char **libxl_xs_kvs_of_flexarray(struct libxl_ctx *ctx, flexarray_t *array, int length);
 int libxl_xs_writev(struct libxl_ctx *ctx, xs_transaction_t t,
                     char *dir, char **kvs);
+/* as writev but also sets the permissions on each path */
+int libxl_xs_writev_perms(struct libxl_ctx *ctx, xs_transaction_t t,
+                          char *dir, char *kvs[],
+                          struct xs_permissions *perms,
+                          unsigned int num_perms);
 int libxl_xs_write(struct libxl_ctx *ctx, xs_transaction_t t,
                    char *path, char *fmt, ...);
 char *libxl_xs_get_dompath(struct libxl_ctx *ctx, uint32_t domid); // logs errs
@@ -140,7 +145,7 @@ int device_physdisk_major_minor(char *ph
 int device_disk_dev_number(char *virtpath);
 
 int libxl_device_generic_add(struct libxl_ctx *ctx, libxl_device *device,
-                             char **bents, char **fents);
+                             char **bents, char **fents, char **ro_fents);
 int libxl_device_del(struct libxl_ctx *ctx, libxl_device *dev, int wait);
 int libxl_device_destroy(struct libxl_ctx *ctx, char *be_path, int force);
 int libxl_devices_destroy(struct libxl_ctx *ctx, uint32_t domid, int force);
--- a/tools/libxl/libxl_xshelp.c
+++ b/tools/libxl/libxl_xshelp.c
@@ -65,8 +65,10 @@ char **libxl_xs_kvs_of_flexarray(struct 
     return kvs;
 }
 
-int libxl_xs_writev(struct libxl_ctx *ctx, xs_transaction_t t,
-                    char *dir, char *kvs[])
+int libxl_xs_writev_perms(struct libxl_ctx *ctx, xs_transaction_t t,
+                          char *dir, char *kvs[],
+                          struct xs_permissions *perms,
+                          unsigned int num_perms)
 {
     char *path;
     int i;
@@ -79,12 +81,20 @@ int libxl_xs_writev(struct libxl_ctx *ct
         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);
         }
         libxl_free(ctx, path);
     }
     return 0;
 }
 
+int libxl_xs_writev(struct libxl_ctx *ctx, xs_transaction_t t,
+                    char *dir, char *kvs[])
+{
+    return libxl_xs_writev_perms(ctx, t, dir, kvs, NULL, 0);
+}
+
 int libxl_xs_write(struct libxl_ctx *ctx, xs_transaction_t t,
                    char *path, char *fmt, ...)
 {
