[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] Resubmission #2: [PATCH 0/3] sVirt AppArmor security driver



On Tue, 29 Sep 2009, Daniel P. Berrange wrote:

> On Fri, Sep 25, 2009 at 05:44:35PM -0500, Jamie Strandboge wrote:
> > 
> > This simplifies adding new files. Part of this process involved adding
> > support for kernel, initrd, loader, serial, and console. I have commented
> > out code for hostdev (and pci would be easy enough), but it requires a
> > patch to to move the _usbDevice struct fully into hostusb.h. I figure
> > once the AppArmor driver is in, I can submit a patch for that.
> 
> No, the usbDevice struct is intended to be private because we want the
> ability to easily change it without impacting callers. The idea is that
> there is not neccessarily just 1 single path associated with a USB
> device and thus 'path' should not be exposed to callers. Instead if you
> need process the 1-or-more paths associated with a USB device you should
> invoke the usbDeviceFileIterate() method and the callback you supply will
> be executing once for each path. The same pattern is there for PCI devices
> which have between 1 and 4 paths that need dealing with typically.
> 
Ok, I'll look at this once the patch lands.

> For virCapsPtr issues we should probably evaluate whether we can make
> the capabilities object passed to virDomainDefParse() optionally
> NULLable. The capabilities object is used to fill in missing pieces
> of metadata when getting an XML blob in from the client app. By the
> time we get into your virt-aa-helper, all this metadata ought to be
> present & correct, so in theory there should be no need for a virCapsPtr
> object. 
> 
I was hoping I could pass NULL, but as you said, as it's written, the
caps is needed to fill in a lot of blanks. I didn't want to try to
change that myself because I didn't have the overall picture and
couldn't forsee all the side-effects, especially (but certainly not
limited to ;) non-qemu domains.

> 
> Does the virt-aa-helper need the full domain XML when attaching or
> detaching a disk ?  If not, then you could try an approach where
> you pass the full domain XML when initially starting the guest,
> and then only pass the domain UUID + the device XML when attaching
> or detaching a particular device on the fly.
> 
virt-aa-helper will be called on startup via AppArmorGenSecurityLabel
and then also when things change via AppArmorRestoreSecurityImageLabel
and AppArmorSetSecurityImageLabel. For just attaching and detaching a
disk, I could probably do as you suggest (I had already considered
this), but the required virDomainDiskDefParseXML() is internal and thus
not available to virt-aa-helper. I then thought it would probably be
safest anyway to pass the complete XML since:

 a) the helper can always be sure it is in sync
 b) it makes it easier down the line if def or newDef is updated for
    some action. All I'd need to do is put a load_profile() call in the
    right place in security_apparmor.c rather than having to rewrite
    virt-aa-helper again to deal with a different XML situation.

I also wasn't sure if it was a design decision that the def or newDef
wasn't updated when attaching/detaching. I really wanted to just insert
the disk definition using virDomainDiskInsert, which would have solved
this issue completely, but I couldn't find a clean way to obtain a copy
of a domain definition that I could modify, send off to virt-aa-helper,
then discard.

Thanks for your feedback! :)

Jamie

-- 
Jamie Strandboge             | http://www.canonical.com

Attachment: signature.asc
Description: Digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]