Re: [libvirt] Resubmission: [PATCH 1/6] sVirt AppArmor security driver

On Tue, 08 Sep 2009, Jamie Strandboge wrote: > > [PATCH 1*] > > patch_1a_reenable-nonfile-labels.patch: > > When James Morris originally submitted his sVirt patches (as seen in > > libvirt 0.6.1), he did not require on disk labelling for > > virSecurityDomainRestoreImageLabel. A later commit[2] changed this > > behavior to assume on disk labelling, which halts implementations for > > path-based MAC systems such as AppArmor and TOMOYO where > > vm->def->seclabel is required to obtain the label. This patch simply > > adds the 'virDomainObjPtr vm' argument back to *RestoreImageLabel. > > > > patch_1b_optional.patch: > > Due to the above change, 'make syntax-check' fails because > > SELinuxRestoreSecurityImageLabel() does not use the 'virDomainObjPtr > > vm'. patch_1b_optional.patch is a simple patch to fix this by checking > > if vm->def->seclabel == NULL and returns with error if it does. I > > realize this may not be desired in the long term, but it should be > > harmless enough to include.

diff -Nurp ./libvirt.orig/src/qemu_driver.c ./libvirt/src/qemu_driver.c --- ./libvirt.orig/src/qemu_driver.c 2009-09-08 13:00:00.000000000 -0500 +++ ./libvirt/src/qemu_driver.c 2009-09-08 15:32:22.000000000 -0500 @@ -6144,7 +6144,7 @@ static int qemudDomainDetachDevice(virDo dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_VIRTIO)) { ret = qemudDomainDetachPciDiskDevice(dom->conn, vm, dev); if (driver->securityDriver) - driver->securityDriver->domainRestoreSecurityImageLabel(dom->conn, dev->data.disk); + driver->securityDriver->domainRestoreSecurityImageLabel(dom->conn, vm, dev->data.disk); if (qemuDomainSetDeviceOwnership(dom->conn, driver, dev, 1) < 0) VIR_WARN0("Fail to restore disk device ownership"); } else if (dev->type == VIR_DOMAIN_DEVICE_NET) { diff -Nurp ./libvirt.orig/src/security.h ./libvirt/src/security.h --- ./libvirt.orig/src/security.h 2009-08-17 11:00:40.000000000 -0500 +++ ./libvirt/src/security.h 2009-09-08 15:32:22.000000000 -0500 @@ -32,6 +32,7 @@ typedef virSecurityDriverStatus (*virSec typedef int (*virSecurityDriverOpen) (virConnectPtr conn, virSecurityDriverPtr drv); typedef int (*virSecurityDomainRestoreImageLabel) (virConnectPtr conn, + virDomainObjPtr vm, virDomainDiskDefPtr disk); typedef int (*virSecurityDomainSetImageLabel) (virConnectPtr conn, virDomainObjPtr vm, diff -Nurp ./libvirt.orig/src/security_selinux.c ./libvirt/src/security_selinux.c --- ./libvirt.orig/src/security_selinux.c 2009-09-02 14:34:08.000000000 -0500 +++ ./libvirt/src/security_selinux.c 2009-09-08 15:32:22.000000000 -0500 @@ -354,6 +354,7 @@ SELinuxSetFilecon(virConnectPtr conn, co static int SELinuxRestoreSecurityImageLabel(virConnectPtr conn, + virDomainObjPtr vm, virDomainDiskDefPtr disk) { struct stat buf; @@ -423,7 +429,8 @@ SELinuxRestoreSecurityLabel(virConnectPt int rc = 0; if (secdef->imagelabel) { for (i = 0 ; i < vm->def->ndisks ; i++) { - if (SELinuxRestoreSecurityImageLabel(conn, vm->def->disks[i]) < 0) + if (SELinuxRestoreSecurityImageLabel(conn, vm, + vm->def->disks[i]) < 0) rc = -1; } VIR_FREE(secdef->model);

diff -Nurp ./libvirt.orig/src/security_selinux.c ./libvirt/src/security_selinux.c --- ./libvirt.orig/src/security_selinux.c 2009-09-02 14:34:08.000000000 -0500 +++ ./libvirt/src/security_selinux.c 2009-09-08 15:32:22.000000000 -0500 @@ -364,6 +365,11 @@ SELinuxRestoreSecurityImageLabel(virConn char *newpath = NULL; const char *path = disk->src; + if (&vm->def->seclabel == NULL) { + virSecurityReportError(conn, VIR_ERR_ERROR, _("seclabel is NULL")); + return rc; + } + /* Don't restore labels on readoly/shared disks, because * other VMs may still be accessing these * Alternatively we could iterate over all running