Make virt-aa-helper create rules to allow VMs access to filesystem mounts from the host.

--- src/security/virt-aa-helper.c | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index b9282b4..e1f7848 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -578,9 +578,6 @@ valid_path(const char *path, const bool readonly) return -1; switch (sb.st_mode & S_IFMT) { - case S_IFDIR: - return 1; - break; case S_IFSOCK: return 1; break; @@ -747,7 +744,7 @@ get_definition(vahControl * ctl, const char *xmlStr) } static int -vah_add_file(virBufferPtr buf, const char *path, const char *perms) +vah_add_path(virBufferPtr buf, const char *path, const char *perms, bool recursive) { char *tmp = NULL; int rc = -1; @@ -788,10 +785,14 @@ vah_add_file(virBufferPtr buf, const char *path, const char *perms) goto cleanup; } - virBufferAsprintf(buf, " \"%s\" %s,\n", tmp, perms); + virBufferAsprintf(buf, " \"%s%s\" %s,\n", tmp, recursive ? "/**" : "", perms); if (readonly) { virBufferAddLit(buf, " # don't audit writes to readonly files\n"); - virBufferAsprintf(buf, " deny \"%s\" w,\n", tmp); + virBufferAsprintf(buf, " deny \"%s%s\" w,\n", tmp, recursive ? "/**" : ""); + } + if (recursive) { + // allow reading (but not creating) the dir + virBufferAsprintf(buf, " \"%s/\" r,\n", tmp); } cleanup: @@ -801,6 +802,12 @@ vah_add_file(virBufferPtr buf, const char *path, const char *perms) } static int +vah_add_file(virBufferPtr buf, const char *path, const char *perms) +{ + return vah_add_path(buf, path, perms, false); +} + +static int vah_add_file_chardev(virBufferPtr buf, const char *path, const char *perms, @@ -1049,6 +1056,13 @@ get_files(vahControl * ctl) } /* switch */ } + for (i = 0; i < ctl->def->nfss; i++) { + virDomainFSDefPtr fs = ctl->def->fss[i]; + + if (vah_add_path(&buf, fs->src, fs->readonly ? "r" : "rw", true) != 0) + goto cleanup; + } + if (ctl->newfile) if (vah_add_file(&buf, ctl->newfile, "rw") != 0) goto cleanup;

Make virt-aa-helper create rules to allow VMs access to filesystem mounts from the host. Signed-off-by: Felix Geyer <debfx(a)fobos.de&gt; Signed-off-by: Hiroshi Miura <miurahr(a)linux.com&gt; Signed-off-by: Serge Hallyn <serge.hallyn(a)ubuntu.com&gt; --- src/security/virt-aa-helper.c | 32 ++++++++++++++++++++++++++------ 1 file changed, 26 insertions(+), 6 deletions(-) diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index e7f1359..2396715 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -606,9 +606,6 @@ valid_path(const char *path, const bool readonly) return -1; switch (sb.st_mode & S_IFMT) { - case S_IFDIR: - return 1; - break; case S_IFSOCK: return 1; break; @@ -775,7 +772,7 @@ get_definition(vahControl * ctl, const char *xmlStr) } static int -vah_add_file(virBufferPtr buf, const char *path, const char *perms) +vah_add_path(virBufferPtr buf, const char *path, const char *perms, bool recursive) { char *tmp = NULL; int rc = -1; @@ -816,10 +813,14 @@ vah_add_file(virBufferPtr buf, const char *path, const char *perms) goto cleanup; } - virBufferAsprintf(buf, " \"%s\" %s,\n", tmp, perms); + virBufferAsprintf(buf, " \"%s%s\" %s,\n", tmp, recursive ? "/**" : "", perms); if (readonly) { virBufferAddLit(buf, " # don't audit writes to readonly files\n"); - virBufferAsprintf(buf, " deny \"%s\" w,\n", tmp); + virBufferAsprintf(buf, " deny \"%s%s\" w,\n", tmp, recursive ? "/**" : ""); + } + if (recursive) { + /* allow reading (but not creating) the dir */ + virBufferAsprintf(buf, " \"%s/\" r,\n", tmp); } cleanup: @@ -829,6 +830,12 @@ vah_add_file(virBufferPtr buf, const char *path, const char *perms) } static int +vah_add_file(virBufferPtr buf, const char *path, const char *perms) +{ + return vah_add_path(buf, path, perms, false); +} + +static int vah_add_file_chardev(virBufferPtr buf, const char *path, const char *perms, @@ -1077,6 +1084,19 @@ get_files(vahControl * ctl) } /* switch */ } + for (i = 0; i < ctl->def->nfss; i++) { + if (ctl->def->fss[i] && + ctl->def->fss[i]->type == VIR_DOMAIN_FS_TYPE_MOUNT && + (ctl->def->fss[i]->fsdriver == VIR_DOMAIN_FS_DRIVER_TYPE_PATH || + ctl->def->fss[i]->fsdriver == VIR_DOMAIN_FS_DRIVER_TYPE_DEFAULT) && + ctl->def->fss[i]->src){ + virDomainFSDefPtr fs = ctl->def->fss[i]; + + if (vah_add_path(&buf, fs->src, fs->readonly ? "r" : "rw", true) != 0) + goto cleanup; + } + } + if (ctl->newfile) if (vah_add_file(&buf, ctl->newfile, "rw") != 0) goto cleanup;

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(a)canonical.com&gt; > 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(a)canonical.com&gt; > 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(a)canonical.com&gt;