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

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

Resubmitting again based on feedback from this list. Notably, the driver
has been reworked to use:

  xml = virDomainDefFormat(conn, vm->def, ...

Then the helper program uses:

  ctl->def = virDomainDefParseString(... xmlStr ...);

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.

This iteration also properly works with attach/detach. While I use
virDomainDefFormat() primarily, this does not work with attach because
the definition is not updated with the information yet (neither def nor
newDef), so I pass the disk file on the command line. For now this is ok
since SetSecurityImageLabel currently only deals with a single file, but
this will need to be adjusted depending on how the storage backend patch
works out. I thought about creating a new def and inserting the disk into
it and sending it off to virt-aa-helper, but I couldn't find an easy
way to get an accurate virCapsPtr in security_apparmor.c. I had a
similar problem with virt-aa-helper, but there I just created a fake
virCapsPtr (which defaults to 'hvm', which was a compromise I was
willing to make at this time since sVirt only supports qemu right now).
The issues with virCapsPtr aside, the code is overall cleaner and much
more easily extendable, so thanks for the excellent feedback. :)

The patches are against up to date trunk, compiles with no warnings,
'make check' passes for the tests I added, and the code passes
syntax-check. With the exception of the three calls to strncpy I had to
convert to virStr*, this is the code that is currently in Ubuntu 9.10
(0.7.0-1ubuntu8). Among other things, that means 9.10 now has your
preferred licensing.

patch_1_reenable-nonfile-labels.patch (Updated based on prior feedback):
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_2_apparmor_driver.patch (updated and merged into one patch based
on prior feedback):
- Updates src/security.c for AppArmor
- Adds security_apparmor.c, security_apparmor.h, virt-aa-helper.c and
  updates po/POTFILES.in. virt-aa-helper.c is a new binary which is used
  exclusively by the AppArmor security driver to manipulate AppArmor.
- Adds tests for virt-aa-helper and the security driver. secaatest.c is
  identical to seclabeltest.c except it initializes the 'apparmor' driver
  instead of 'selinux'. These tests are integrated into 'make check' and
- Updates configure.in, Makefile.am, src/Makefile.am, tests/Makefile.am,
  and tools/Makefile.am for AppArmor. It is based on and should operate
  the same as the SELinux configuration.

patch_3_docs.patch (Updated based on prior feedback):
Updates docs/drvqemu.html.in for AppArmor and adds profile examples to
examples/apparmor. Updated based on prior feedback.


On Fri, 04 Sep 2009, Jamie Strandboge wrote:

> This patch series implements the AppArmor security driver for sVirt.
> This implementation was developed for the Ubuntu AppArmorLibvirtProfile
> specification[1], but is general enough for any AppArmor deployment
> (such as Ubuntu, *SUSE and Mandriva).
> This patch has seen quite a bit of real world testing in Ubuntu 9.10
> (our development release) in our 0.7.0-1ubuntu3 package. I did make a
> few small changes after going through HACKING, but mostly I got the
> tests going and added documentation.
> ------
> When a virtual machine is started, determine if a profile is currently
> defined for the machine, and use it if available. If not, generate a new
> profile for the machine based on a template, which is by default a very
> restrictive profile allowing access to disk files, and anything else
> needed to run, such as the pid, monitor and log files.
> Virtual machines should have a unique profile specific to that machine.
> To ensure uniqueness, the profile name will be derived from the UUID of
> the virtual machine. These profiles should be configurable, either by
> adjusting the profile template for new machines, creating/modifying the
> VM profile directly or through the use of AppArmor abstractions. This
> will allow for administrators to fine-tune confinement for individual
> machines if desired.
> If enabled at compile time, the sVirt security model will be activated
> if AppArmor is available on the host OS and a profile for the libvirtd
> daemon is loaded when libvirtd is started.
> libvirtd should not be allowed to create arbitrary profiles or modify
> profiles directly, so as to not allow libvirtd to potentially (ie via a
> security bug in libvirtd itself) bootstrap out of AppArmor confinement.
> Because root privileges are needed to manipulate AppArmor profiles,
> qemu:///session will not be supported at this time, but the
> implementation must allow for a confined libvirtd with qemu:///session
> guests running unconfined. This can be revisited when AppArmor supports
> per-user profiles.
> Please see the specification[1] for more details.

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]