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

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



On Mon, Jan 27, 2014 at 11:49:45AM -0600, Jamie Strandboge wrote:
> 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>

I've pushed this patch now. Since there weren't any further comments to
the other patch in this series I think it's good to go as well with the
minor comment style nit fixed.
Cheers,
 -- Guido


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