Re: [Xen-devel] [PATCH 1/2] libxl: correctly list disks served by driver domains in block-list

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Tue, 2013-09-10 at 14:06 +0200, Roger Pau Monnà wrote:
> On 10/09/13 12:11, Ian Campbell wrote:
> > On Fri, 2013-09-06 at 12:36 +0200, Roger Pau Monne wrote:
> >> The block-list command was not able to lists disks with backends
> >> running on domains different than Dom0, because it was only looking on
> >> the backend xenstore path of Dom0. Fix this by instead fetching the
> >> disks from the DomU xenstore entries.
> > 
> > Need to be a bit careful here about reading from potentially guest
> > controllable keys. This should mostly be a question of permissions.
> > 
> >> +    fe_path = libxl__sprintf(gc, "/local/domain/%d/device/vbd", domid);
> > 
> > Are guests able to create new subdirectories under here?
> 
> Yes
> 
> > 
> >> +    devs = libxl__xs_directory(gc, XBT_NULL, fe_path, &xs_num);
> >> +    if (!devs)
> >> +        /* Domain has no disks */
> >> +        goto out;
> >> +    disks = libxl__calloc(NOGC, xs_num, sizeof(*disks));
> >> +    if (!disks)
> >> +        goto out_err;
> >> +    for (i = 0; i < xs_num; i++) {
> >> +        fe_path = GCSPRINTF("/local/domain/%d/device/vbd/%s/backend",
> >> +                            domid, devs[i]);
> > 
> > Is this path writeable by the guest? The containing directory is I
> > think, so this needs to include delete and recreate type situations
> > (although ISTR xenstore not having the posix like semantics here).
> 
> Yes, the whole directory including the backend entry is writeable by the
> guest.
> 
> > 
> > If the guest can remove and recreate then we should check the current
> > owner of the key is e.g. the toolstack domain or whoever should be
> > trusted to won the key, within the same transaction as the read itself.
> 
> I'm sorry but I'm not following you here, I assume you are speaking
> about the frontend node that points to the backend ie:
> 
> /local/domain/<domid>/device/vbd/<devid>/backend
> 
> Permissions on this node are:
> 
> domid: <domid> perms: 0
> domid: 0 perms: 1

What are perms 0 and 1? We normally use r/w/b in xenstore speak.

> If the guest changes this node, or recreates it permissions will still
> be the same.

I was hoping we might be able to spot this case because the permissions
would have changed.

So the backend path in the frontend dir might point somewhere which is
maliciously controlled by the guest, or by another guest, something
which I don't think libxl_foo_from_xs_be is prepared to deal with. We
need to mitigate this somehow. Can we tighten the permissions
on /local/domain/<domid>/device/vbd/<devid>/backend such that only the
toolstack domain can fiddle with it?

Ian



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