PATCH: Actually make /local/domain/$DOMID readonly to the guest

["Daniel P. Berrange" <berrange@redhat.com>]

A few months back change 18564 fixed a permissions flaw in xenstore
which allowed guests to over-write critical data used by the host

    changeset:   18564:60937c4c5a67
    user:        Keir Fraser <keir.fraser@citrix.com>
    date:        Thu Oct 02 10:37:28 2008 +0100
    files:       tools/python/xen/xend/XendDomainInfo.py tools/python/xen/xend/image.py
    description:
    xend: Make only selected subdirs of /local/domain/<domid> writable by the guest.

    This protects critical data like
    /local/domain/<domid>/console/{tty,limit}. It also means we can trust
    .../vm, and hence do not need /vm_path. Various parts of the previous
    two changesets disappear.

    Signed-off-by: Keir Fraser <keir.fraser@citrix.com>


One of the parts in this patch was

@@ -1269,8 +1301,11 @@ class XendDomainInfo:
     def _recreateDomFunc(self, t):
         t.remove()
         t.mkdir()
-        t.set_permissions({'dom' : self.domid})
+        t.set_permissions({'dom' : self.domid, 'read' : True})
         t.write('vm', self.vmpath)
+        for i in [ 'device', 'control', 'error' ]:
+            t.mkdir(i)
+            t.set_permissions(i, {'dom' : self.domid})
 
Most people reading this chunk would assume that the call

     t.set_permissions({'dom' : self.domid, 'read' : True})

would make the xenstore path in question read only to the guest listed.
Unfortunately the 'set_permissions' call turns out to be bizarre

The first permission record listed does not set permissions at all. It
in fact sets the 'owner' of the path. The owner is given full access to
do whatever they like in that path, regardless of what permissions
bits are set.

To actually restrict the permissions on a path for a guest, you have
to first give a different guest permissions on the path so it becomes
the 'owner'.

The upshot is that changeset  18564:60937c4c5a67 didn't do anything to
protect xenstore data from the guest as we thought it did :-(

The increment fix is to explicitly make Dom0 the owner of all the 
/local/domain/$DOMID  paths, and then give the DomU read permission

So we need the following additional patch applied:



diff -r a76b4e00e186 tools/python/xen/xend/XendDomainInfo.py
--- a/tools/python/xen/xend/XendDomainInfo.py	Tue Dec 16 13:14:25 2008 +0000
+++ b/tools/python/xen/xend/XendDomainInfo.py	Thu Dec 18 10:20:42 2008 -0500
@@ -1359,7 +1359,18 @@ class XendDomainInfo:
     def _recreateDomFunc(self, t):
         t.remove()
         t.mkdir()
-        t.set_permissions({'dom' : self.domid, 'read' : True})
+        #
+        # The first permission record on any node indicates
+        # the domain owner. Thus any permissions associated
+        # with the first record are ignored, since the owner
+        # is allowed todo anything.
+        #
+        # Thus to make the node read-only to the guest
+        # we must explicitly give Dom0 permission making
+        # it the owner, even though Dom0 already has all the
+        # permissions inherited from the parent.
+        #
+        t.set_permissions({'dom' : 0 }, {'dom' : self.domid, 'read' : True})
         t.write('vm', self.vmpath)
         for i in [ 'device', 'control', 'error', 'memory' ]:
             t.mkdir(i)

Explicitly give Dom0 permissions on the /local/domain/$DOMID so it
becomes the owner of the path. The guest is then granted read perm
on the path.

   Signed-off-by: Daniel P. Berrange <berrange@redhat.com>

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|