Re: [libvirt] [PATCH 02/12] Refactor setup & cleanup of security labels in security driver

On Wed, Jan 20, 2010 at 03:14:59PM +0000, Daniel P. Berrange wrote: > The current security driver architecture has the following > split of logic > > * domainGenSecurityLabel > > Allocate the unique label for the domain about to be started > > * domainGetSecurityLabel > > Retrieve the current live security label for a process > > * domainSetSecurityLabel > > Apply the previously allocated label to the current process > Setup all disk image / device labelling > > * domainRestoreSecurityLabel > > Restore the original disk image / device labelling. > Release the unique label for the domain > > The 'domainSetSecurityLabel' method is special because it runs > in the context of the child process between the fork + exec. > > This is require in order to set the process label. It is not > required in order to label disks/devices though. Having the > disk labelling code run in the child process limits what it > can do. > > In particularly libvirtd would like to remember the current > disk image label, and only change shared image labels for the > first VM to start. This requires use & update of global state > in the libvirtd daemon, and thus cannot run in the child > process context. > > The solution is to split domainSetSecurityLabel into two parts, > one applies process label, and the other handles disk image > labelling. At the same time domainRestoreSecurityLabel is > similarly split, just so that it matches the style. Thus the > previous 4 methods are replaced by the following 6 new methods > > * domainGenSecurityLabel > > Allocate the unique label for the domain about to be started > No actual change here. > > * domainReleaseSecurityLabel > > Release the unique label for the domain > > * domainGetSecurityProcessLabel > > Retrieve the current live security label for a process > Merely renamed for clarity. > > * domainSetSecurityProcessLabel > > Apply the previously allocated label to the current process > > * domainRestoreSecurityAllLabel > > Restore the original disk image / device labelling. > > * domainSetSecurityAllLabel > > Setup all disk image / device labelling > > The SELinux and AppArmour drivers are then updated to comply with > this new spec. Notice that the AppArmour driver was actually a > little different. It was creating its profile for the disk image > and device labels in the 'domainGenSecurityLabel' method, where as > the SELinux driver did it in 'domainSetSecurityLabel'. With the > new method split, we can have consistency, with both drivers doing > that in the domainSetSecurityAllLabel method. A bit complex, thanks for the explanations ! > NB, the AppArmour changes here haven't been compiled so may not > build. > --- > src/qemu/qemu_driver.c | 21 ++++++++--- > src/security/security_apparmor.c | 66 ++++++++++++++++++++++----------- > src/security/security_driver.h | 30 +++++++++------ > src/security/security_selinux.c | 75 +++++++++++++++++++++++++------------- > 4 files changed, 127 insertions(+), 65 deletions(-) [...] > +static int > +SELinuxSetSecurityAllLabel(virConnectPtr conn, > + virDomainObjPtr vm) > +{ > + const virSecurityLabelDefPtr secdef = &vm->def->seclabel; > + int i; > + > + if (secdef->type == VIR_DOMAIN_SECLABEL_STATIC) > + return 0; > + > + for (i = 0 ; i < vm->def->ndisks ; i++) { > + /* XXX fixme - we need to recursively label the entriy tree :-( */ while we're are there let's fix the entriy/entire typo