On Fri, 04 Sep 2009, Daniel Veillard wrote: > On Fri, Sep 04, 2009 at 12:23:33PM -0500, 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. > > Could you triple check that the make syntax-check run clean with > your patches applied, because a very quick glimpse shows up a number > of malloc() which we don't allow for example. It may be because you > didn't enter the files in the SCM but the checks should be done anyway > on the new files please. > I noticed that syntax-check wouldn't scan the new files unless I did a git commit first. I did not run 'syntax-check' after adding the documention examples, and just found that examples/apparmor/libvirt-qemu had a trailing blank line (I was a bit surprised syntax-check checked these files (attached is an updated docs patch)). I can demonstrate that the files are being checked by removing the '#include <config.h>' line in security_apparmor.c and virt-aa-helper.c and syntax-check failing: $ make syntax-check .. require_config_h src/virt-aa-helper.c maint.mk: the above files do not include <config.h> make: *** [sc_require_config_h] Error 1 $ make syntax-check .. require_config_h src/security_apparmor.c maint.mk: the above files do not include <config.h> make: *** [sc_require_config_h] Error 1 I also knew the files were checked because, for example, I used str[n]cmp() in a couple places and prohibit_strcmp_and_strncmp caught them. I changed those to STR* before submission. I don't see that 'syntax-check' enforces not using *alloc, but I did read in HACKING that *alloc are deprecated, though I had already written the code before reading it. I do see other examples of *alloc in the codebase, and I do properly check the return code of all calls to *alloc. If you insist on those calls being replaced, I can do that, but I didn't before submitting because I knew the current calls were correct and didn't want to introduce bugs into the code. Here is what I do after a 'git pull': $ cd libvirt $ git checkout <apply patches> (using the attached patch_5_docs.patch) $ chmod 755 ./tests/virt-aa-helper-test $ git add . $ git commit ... [master 4b08401] add apparmor security driver 18 files changed, 1966 insertions(+), 3 deletions(-) create mode 100644 examples/apparmor/TEMPLATE create mode 100644 examples/apparmor/libvirt-qemu create mode 100644 examples/apparmor/usr.bin.virt-aa-helper create mode 100644 examples/apparmor/usr.sbin.libvirtd create mode 100644 src/security_apparmor.c create mode 100644 src/security_apparmor.h create mode 100644 src/virt-aa-helper.c create mode 100644 tests/secaatest.c create mode 100755 tests/virt-aa-helper-test $ ./autogen.sh $ ./configure --enable-compile-warnings=error $ make check .. CC security_apparmor.lo .. CC virt_aa_helper-virt-aa-helper.o CCLD virt-aa-helper .. CC secaatest.o CCLD secaatest .. PASS: virt-aa-helper-test .. PASS: secaatest .. (no errors, and I see my two added tests passed) $ make syntax-check void_if_before_free cast_of_argument_to_free cast_of_x_alloc_return_value prohibit_strcmp error_message_warn_fatal error_message_period prohibit_have_config_h require_config_h require_config_h_first prohibit_HAVE_MBRTOWC prohibit_assert_without_use prohibit_getopt_without_use prohibit_long_options_without_use prohibit_inttostr_without_use prohibit_error_without_use prohibit_safe_read_without_use prohibit_argmatch_without_use prohibit_root_dev_ino_without_use prohibit_c_ctype_without_use prohibit_signal_without_use changelog the_the trailing_blank unmarked_diagnostics prohibit_cvs_keyword proper_name_utf8_requires_ICONV redundant_const const_long_option makefile_TAB_only_indentation m4_quote_check po_check copyright_check avoid_write prohibit_strcmp_and_strncmp prohibit_asprintf prohibit_VIR_ERR_NO_MEMORY prohibit_nonreentrant prohibit_ctype_h TAB_in_indentation avoid_ctype_macros prohibit_virBufferAdd_with_string_literal prohibit_gethostby libvirt_unmarked_diagnostics prohibit_trailing_blank_lines Thanks! Jamie -- Jamie Strandboge | http://www.canonical.com
diff -Nurp ./libvirt.orig/docs/drvqemu.html.in ./libvirt/docs/drvqemu.html.in
--- ./libvirt.orig/docs/drvqemu.html.in 2009-09-02 14:34:08.000000000 -0500
+++ ./libvirt/docs/drvqemu.html.in 2009-09-04 15:42:13.000000000 -0500
@@ -296,6 +296,72 @@
file can be used to change the setting to <code>security_driver="none"</code>
</p>
+ <h3><a name="securitysvirtaa">AppArmor sVirt confinement</a></h3>
+
+ <p>
+ When using basic AppArmor protection for the libvirtd daemon and
+ QEMU virtual machines, the intention is to protect the host OS
+ from a compromised virtual machine process. There is no protection
+ between guests.
+ </p>
+
+ <p>
+ The AppArmor sVirt protection for QEMU virtual machines builds on
+ this basic level of protection, to also allow individual guests to
+ be protected from each other.
+ </p>
+
+ <p>
+ In the sVirt model, if a profile is loaded for the libvirtd daemon,
+ then each <code>qemu:///system</code> QEMU virtual machine will run
+ starts. This generated profile uses a profile name based on the UUID
+ of the QEMU virtual machine and contains rules allowing access to
+ only the files it needs to run, such as its disks, pid file and log
+ files. Just before the QEMU virtual machine is started, the libvirtd
+ daemon will change into this unique profile, preventing the QEMU
+ process from accessing any file resources that are present in another
+ QEMU process or the host machine.
+ </p>
+
+ <p>
+ The AppArmor sVirt implementation is flexible in that it allows an
+ administrator to customize the template file in
+ <code>/etc/apparmor.d/libvirt/TEMPLATE</code> for site-specific
+ access for all newly created QEMU virtual machines. Also, when a new
+ profile is generated, two files are created:
+ <code>/etc/apparmor.d/libvirt/libvirt-<uuid></code> and
+ <code>/etc/apparmor.d/libvirt/libvirt-<uuid>.files</code>. The
+ former can be fine-tuned by the administrator to allow custom access
+ for this particular QEMU virtual machine, and the latter will be
+ updated appropriately when required file access changes, such as when
+ a disk is added. This flexibility allows for situations such as
+ having one virtual machine in complain mode with all others in
+ enforce mode.
+ </p>
+
+ <p>
+ While users can define their own AppArmor profile scheme, a typical
+ configuration will include a profile for <code>/usr/sbin/libvirtd</code>,
+ <code>/usr/bin/virt-aa-helper</code> (a helper program which the
+ libvirtd daemon uses instead of manipulating AppArmor directly), and
+ an abstraction to be included by <code>/etc/apparmor.d/libvirt/TEMPLATE</code>
+ (typically <code>/etc/apparmor.d/abstractions/libvirt-qemu</code>).
+ An example profile scheme can be found in the examples/apparmor
+ directory of the source distribution.
+ </p>
+
+ <p>
+ If the sVirt security model is active, then the node capabilities
+ XML will include its details. If a virtual machine is currently
+ protected by the security model, then the guest XML will include
+ its assigned profile name. 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. To disable sVirt, and revert to the basic level of AppArmor
+ protection (host protection only), the <code>/etc/libvirt/qemu.conf</code>
+ file can be used to change the setting to <code>security_driver="none"</code>.
+ </p>
+
<h3><a name="securityacl">Cgroups device ACLs</a></h3>
diff -Nurp ./libvirt.orig/examples/apparmor/libvirt-qemu ./libvirt/examples/apparmor/libvirt-qemu
--- ./libvirt.orig/examples/apparmor/libvirt-qemu 1969-12-31 18:00:00.000000000 -0600
+++ ./libvirt/examples/apparmor/libvirt-qemu 2009-09-04 15:42:34.000000000 -0500
@@ -0,0 +1,70 @@
+# Last Modified: Wed Jul 8 09:57:41 2009
+
+ #include <abstractions/base>
+ #include <abstractions/consoles>
+ #include <abstractions/nameservice>
+
+ # required for reading disk images
+ capability dac_override,
+ capability dac_read_search,
+
+ network inet stream,
+ network inet6 stream,
+
+ /dev/net/tun rw,
+ /dev/kvm rw,
+ /dev/ptmx rw,
+ /dev/kqemu rw,
+
+ # WARNING: uncommenting these gives the guest direct access to host hardware.
+ # This is required for USB pass through but is a security risk. You have been
+ # warned.
+ #/sys/bus/usb/devices/ r,
+ #/sys/devices/*/*/usb[0-9]*/** r,
+ #/dev/bus/usb/*/[0-9]* rw,
+
+ /usr/share/kvm/** r,
+ /usr/share/qemu/** r,
+ /usr/share/bochs/** r,
+ /usr/share/openbios/** r,
+ /usr/share/openhackware/** r,
+ /usr/share/proll/** r,
+ /usr/share/vgabios/** r,
+
+ # the various binaries
+ /usr/bin/kvm rmix,
+ /usr/bin/qemu rmix,
+ /usr/bin/qemu-system-arm rmix,
+ /usr/bin/qemu-system-cris rmix,
+ /usr/bin/qemu-system-i386 rmix,
+ /usr/bin/qemu-system-m68k rmix,
+ /usr/bin/qemu-system-mips rmix,
+ /usr/bin/qemu-system-mips64 rmix,
+ /usr/bin/qemu-system-mips64el rmix,
+ /usr/bin/qemu-system-mipsel rmix,
+ /usr/bin/qemu-system-ppc rmix,
+ /usr/bin/qemu-system-ppc64 rmix,
+ /usr/bin/qemu-system-ppcemb rmix,
+ /usr/bin/qemu-system-sh4 rmix,
+ /usr/bin/qemu-system-sh4eb rmix,
+ /usr/bin/qemu-system-sparc rmix,
+ /usr/bin/qemu-system-sparc64 rmix,
+ /usr/bin/qemu-system-x86_64 rmix,
+ /usr/bin/qemu-alpha rmix,
+ /usr/bin/qemu-arm rmix,
+ /usr/bin/qemu-armeb rmix,
+ /usr/bin/qemu-cris rmix,
+ /usr/bin/qemu-i386 rmix,
+ /usr/bin/qemu-m68k rmix,
+ /usr/bin/qemu-mips rmix,
+ /usr/bin/qemu-mipsel rmix,
+ /usr/bin/qemu-ppc rmix,
+ /usr/bin/qemu-ppc64 rmix,
+ /usr/bin/qemu-ppc64abi32 rmix,
+ /usr/bin/qemu-sh4 rmix,
+ /usr/bin/qemu-sh4eb rmix,
+ /usr/bin/qemu-sparc rmix,
+ /usr/bin/qemu-sparc64 rmix,
+ /usr/bin/qemu-sparc32plus rmix,
+ /usr/bin/qemu-sparc64 rmix,
+ /usr/bin/qemu-x86_64 rmix,
diff -Nurp ./libvirt.orig/examples/apparmor/TEMPLATE ./libvirt/examples/apparmor/TEMPLATE
--- ./libvirt.orig/examples/apparmor/TEMPLATE 1969-12-31 18:00:00.000000000 -0600
+++ ./libvirt/examples/apparmor/TEMPLATE 2009-09-04 15:42:13.000000000 -0500
@@ -0,0 +1,9 @@
+#
+# This profile is for the domain whose UUID matches this file.
+#
+
+#include <tunables/global>
+
+profile LIBVIRT_TEMPLATE {
+ #include <abstractions/libvirt-qemu>
+}
diff -Nurp ./libvirt.orig/examples/apparmor/usr.bin.virt-aa-helper ./libvirt/examples/apparmor/usr.bin.virt-aa-helper
--- ./libvirt.orig/examples/apparmor/usr.bin.virt-aa-helper 1969-12-31 18:00:00.000000000 -0600
+++ ./libvirt/examples/apparmor/usr.bin.virt-aa-helper 2009-09-04 15:42:13.000000000 -0500
@@ -0,0 +1,22 @@
+# Last Modified: Mon Jul 06 17:22:37 2009
+#include <tunables/global>
+
+/usr/bin/virt-aa-helper {
+ #include <abstractions/base>
+
+ # needed for searching directories
+ capability dac_override,
+ capability dac_read_search,
+
+ # needed for when disk is on a network filesystem
+ network inet,
+
+ deny @{PROC}/[0-9]*/mounts r,
+ @{PROC}/filesystems r,
+
+ /usr/bin/virt-aa-helper mr,
+ /sbin/apparmor_parser Ux,
+
+ /etc/apparmor.d/libvirt/* r,
+ /etc/apparmor.d/libvirt/libvirt-[0-9a-f]*-[0-9a-f]*-[0-9a-f]*-[0-9a-f]*-[0-9a-f]* rw,
+}
diff -Nurp ./libvirt.orig/examples/apparmor/usr.sbin.libvirtd ./libvirt/examples/apparmor/usr.sbin.libvirtd
--- ./libvirt.orig/examples/apparmor/usr.sbin.libvirtd 1969-12-31 18:00:00.000000000 -0600
+++ ./libvirt/examples/apparmor/usr.sbin.libvirtd 2009-09-04 15:42:13.000000000 -0500
@@ -0,0 +1,39 @@
+# Last Modified: Mon Jul 6 17:23:58 2009
+#include <tunables/global>
+@{LIBVIRT}="libvirt"
+
+/usr/sbin/libvirtd {
+ #include <abstractions/base>
+
+ capability kill,
+ capability net_admin,
+ capability net_raw,
+ capability setgid,
+ capability sys_admin,
+ capability sys_module,
+ capability sys_ptrace,
+
+ network inet stream,
+
+ # Very lenient profile for libvirtd since we want to first focus on confining
+ # the guests. Guests will have a very restricted profile.
+ /** rwmkl,
+
+ /bin/* Ux,
+ /sbin/* Ux,
+ /usr/bin/* Ux,
+ /usr/sbin/* Ux,
+
+ # force the use of virt-aa-helper
+ audit deny /sbin/apparmor_parser rwxl,
+ audit deny /etc/apparmor.d/libvirt/** wxl,
+ audit deny /sys/kernel/security/apparmor/features rwxl,
+ audit deny /sys/kernel/security/apparmor/matching rwxl,
+ audit deny /sys/kernel/security/apparmor/.* rwxl,
+ /sys/kernel/security/apparmor/profiles r,
+ /usr/bin/virt-aa-helper Pxr,
+
+ # allow changing to our UUID-based named profiles
+ change_profile -> @{LIBVIRT}-[0-9a-f]*-[0-9a-f]*-[0-9a-f]*-[0-9a-f]*-[0-9a-f]*,
+
+}
Attachment:
signature.asc
Description: Digital signature
-- Libvir-list mailing list Libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list