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

Re: [libvirt] [PATCH 2/2] apparmor: Improve profiles



On 01/26/2014 03:47 PM, Felix Geyer wrote:
> Tested on Debian unstable.
> The profile updates are partly taken from the Ubuntu trusty libvirt package.

Thanks for these updates! :) Comments inline.

> ---
>  examples/apparmor/libvirt-qemu                   | 21 +++++++++++++++++----
>  examples/apparmor/usr.lib.libvirt.virt-aa-helper | 10 ++++++++++
>  examples/apparmor/usr.sbin.libvirtd              | 16 ++++++++++++----
>  3 files changed, 39 insertions(+), 8 deletions(-)
> 
> diff --git a/examples/apparmor/libvirt-qemu b/examples/apparmor/libvirt-qemu
> index 766a334..e1980b7 100644
> --- a/examples/apparmor/libvirt-qemu
> +++ b/examples/apparmor/libvirt-qemu
> @@ -9,6 +9,10 @@
>    capability dac_read_search,
>    capability chown,
>  
> +  # needed to drop privileges
> +  capability setgid,
> +  capability setuid,
> +
>    network inet stream,
>    network inet6 stream,
>  
> @@ -20,7 +24,7 @@
>  
>    # For hostdev access. The actual devices will be added dynamically
>    /sys/bus/usb/devices/ r,
> -  /sys/devices/*/*/usb[0-9]*/** r,
> +  /sys/devices/**/usb[0-9]*/** r,
>  
>    # WARNING: this gives the guest direct access to host hardware and specific
>    # portions of shared memory. This is required for sound using ALSA with kvm,
> @@ -32,6 +36,8 @@
>    /{dev,run}/shmpulse-shm* rwk,
>    /dev/snd/* rw,
>    capability ipc_lock,
> +  # spice
> +  owner /{dev,run}/shm/spice.* rw,
>    # 'kill' is not required for sound and is a security risk. Do not enable
>    # unless you absolutely need it.
>    deny capability kill,
> @@ -58,6 +64,7 @@
>    /usr/share/proll/** r,
>    /usr/share/vgabios/** r,
>    /usr/share/seabios/** r,
> +  /usr/share/ovmf/** r,
>  
>    # access PKI infrastructure
>    /etc/pki/libvirt-vnc/** r,
> @@ -109,9 +116,15 @@
>    /bin/dd rmix,
>    /bin/cat rmix,
>  
> -  /usr/libexec/qemu-bridge-helper Cx,
> +  # for usb access
> +  /dev/bus/usb/ r,
> +  /etc/udev/udev.conf r,
> +  /sys/bus/ r,
> +  /sys/class/ r,
> +
> +  /usr/{lib,libexec}/qemu-bridge-helper Cx -> qemu_bridge_helper,
>    # child profile for bridge helper process
> -  profile /usr/libexec/qemu-bridge-helper {
> +  profile qemu_bridge_helper {
>     #include <abstractions/base>
>  
>     capability setuid,
> @@ -125,5 +138,5 @@
>     /etc/qemu/** r,
>     owner @{PROC}/*/status r,
>  
> -   /usr/libexec/qemu-bridge-helper rmix,
> +   /usr/{lib,libexec}/qemu-bridge-helper rmix,
>    }

I think you could actually deny the access to /etc/udev/udev.conf, but the
access is harmless.

Acked-By: Jamie Strandboge <jamie canonical com>


> diff --git a/examples/apparmor/usr.lib.libvirt.virt-aa-helper b/examples/apparmor/usr.lib.libvirt.virt-aa-helper
> index 94bf359..bceaaff 100644
> --- a/examples/apparmor/usr.lib.libvirt.virt-aa-helper
> +++ b/examples/apparmor/usr.lib.libvirt.virt-aa-helper
> @@ -12,6 +12,8 @@
>    network inet,
>  
>    deny @{PROC}/[0-9]*/mounts r,
> +  @{PROC}/[0-9]*/net/psched r,
> +  owner @{PROC}/[0-9]*/status r,
>    @{PROC}/filesystems r,
>  
>    # for hostdev
> @@ -35,4 +37,12 @@
>    @{HOME}/** r,
>    /var/lib/libvirt/images/ r,
>    /var/lib/libvirt/images/** r,
> +  /{media,mnt,opt,srv}/** r,
> +
> +  /**.img r,
> +  /**.qcow{,2} r,
> +  /**.qed r,
> +  /**.vmdk r,
> +  /**.[iI][sS][oO] r,
> +  /**/disk{,.*} r,
>  }

Acked-By: Jamie Strandboge <jamie canonical com>


> diff --git a/examples/apparmor/usr.sbin.libvirtd b/examples/apparmor/usr.sbin.libvirtd
> index 1b24835..fd6def1 100644
> --- a/examples/apparmor/usr.sbin.libvirtd
> +++ b/examples/apparmor/usr.sbin.libvirtd
> @@ -4,6 +4,7 @@
>  
>  /usr/sbin/libvirtd {
>    #include <abstractions/base>
> +  #include <abstractions/dbus>
>  
>    capability kill,
>    capability net_admin,
> @@ -22,20 +23,25 @@
>    capability setpcap,
>    capability mknod,
>    capability fsetid,
> +  capability audit_write,
>  
>    network inet stream,
>    network inet dgram,
>    network inet6 stream,
>    network inet6 dgram,
> +  network packet dgram,
>  
>    # Very lenient profile for libvirtd since we want to first focus on confining
>    # the guests. Guests will have a very restricted profile.
> +  / r,
>    /** rwmkl,
>  
> -  /bin/* Ux,
> -  /sbin/* Ux,
> -  /usr/bin/* Ux,
> -  /usr/sbin/* Ux,
> +  /bin/* PUx,
> +  /sbin/* PUx,
> +  /usr/bin/* PUx,
> +  /usr/sbin/* PUx,
> +  /lib/udev/scsi_id PUx,
> +  /usr/lib/xen-common/bin/xen-toolstack PUx,
>  
>    # force the use of virt-aa-helper
>    audit deny /sbin/apparmor_parser rwxl,
> @@ -45,6 +51,8 @@
>    audit deny /sys/kernel/security/apparmor/.* rwxl,
>    /sys/kernel/security/apparmor/profiles r,
>    /usr/lib/libvirt/* PUxr,
> +  /etc/libvirt/hooks/** rmix,
> +  /etc/xen/scripts/** rmix,
>  
>    # 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]*,
> 

Acked-By: Jamie Strandboge <jamie canonical com>

-- 
Jamie Strandboge                 http://www.ubuntu.com/

Attachment: signature.asc
Description: OpenPGP digital signature


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