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
Attachments:
- signature.asc (application/pgp-signature — 197 bytes)