[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 1/2] libxl: correctly list disks served by driver domains in block-list
On 10/09/13 14:54, Ian Campbell wrote: > 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. This is what xs_get_permissions reports, according to xenstore_lib.h: 0 = XS_PERM_NONE 1 = XS_PERM_READ Which doesn't make sense IMHO, because the guest has XS_PERM_NONE and is able to read/write on that path (I'm probably missing something here). > >> 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? That was my first thought, I think we can safely make this (/local/domain/<domid>/device/vbd/<devid>/backend) read-only for the guest, no well-behaved frontend should ever try to change that. libxl__devices_destroy logic already relies on /local/domain/<domid>/device/vbd/<devid>/backend pointing to the backend xenstore path. _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |