[libvirt] [PATCH 0/2] AppArmor updates
I'm working on getting AppArmor support enabled in the Debian libvirt package. As a result I've updated the profiles in example/ and added support for filesystem mounts. Felix Geyer (2): apparmor: Allow access to filesystem mounts apparmor: Improve profiles examples/apparmor/libvirt-qemu | 21 +++++++++++++++---- examples/apparmor/usr.lib.libvirt.virt-aa-helper | 10 +++++++++ examples/apparmor/usr.sbin.libvirtd | 16 +++++++++++---- src/security/virt-aa-helper.c | 26 ++++++++++++++++++------ 4 files changed, 59 insertions(+), 14 deletions(-) -- 1.8.5.3
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; -- 1.8.5.3
On 01/26/2014 03:47 PM, Felix Geyer wrote:
Make virt-aa-helper create rules to allow VMs access to filesystem mounts from the host.
Note that virt-aa-helper access to various parts of the filesystem is generally ok. However, can you be more specific about the problem you're trying to solve? Eg, is there a bug number?
--- 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;
-- Jamie Strandboge http://www.ubuntu.com/
Hi, On Sun, Jan 26, 2014 at 10:47:34PM +0100, Felix Geyer wrote: [..snip..] `
+ if (recursive) { + // allow reading (but not creating) the dir + virBufferAsprintf(buf, " \"%s/\" r,\n", tmp);
Just a minor nit. Libvirt prefers /* */ style comments (see HACKING). Cheers, -- Guido
On 29.01.2014 07:48, Guido Günther wrote:
Hi, On Sun, Jan 26, 2014 at 10:47:34PM +0100, Felix Geyer wrote: [..snip..] `
+ if (recursive) { + // allow reading (but not creating) the dir + virBufferAsprintf(buf, " \"%s/\" r,\n", tmp); Just a minor nit. Libvirt prefers /* */ style comments (see HACKING). Cheers, -- Guido
Oops, do you want me to resubmit the patch? Regards, Felix
Hi Felix, On Thu, Jan 30, 2014 at 06:22:51PM +0100, Felix Geyer wrote:
On 29.01.2014 07:48, Guido Günther wrote:
Hi, On Sun, Jan 26, 2014 at 10:47:34PM +0100, Felix Geyer wrote: [..snip..] `
+ if (recursive) { + // allow reading (but not creating) the dir + virBufferAsprintf(buf, " \"%s/\" r,\n", tmp); Just a minor nit. Libvirt prefers /* */ style comments (see HACKING). Cheers, -- Guido
Oops, do you want me to resubmit the patch?
Yes please but let's wait if other comments show up. Cheers, -- Guido
Quoting Guido Günther (gg@godiug.net):
Hi Felix, On Thu, Jan 30, 2014 at 06:22:51PM +0100, Felix Geyer wrote:
On 29.01.2014 07:48, Guido Günther wrote:
Hi, On Sun, Jan 26, 2014 at 10:47:34PM +0100, Felix Geyer wrote: [..snip..] `
+ if (recursive) { + // allow reading (but not creating) the dir + virBufferAsprintf(buf, " \"%s/\" r,\n", tmp); Just a minor nit. Libvirt prefers /* */ style comments (see HACKING). Cheers, -- Guido
Oops, do you want me to resubmit the patch?
Yes please but let's wait if other comments show up. Cheers, -- Guido
Hi, Just a ping, I don't see that this patch was ever applied. thanks, -serge
Hi, a separate patch was posted to a new launchpad bug which does a bit more sanity checking on the values passed in, so I went ahead and merged the two. I did however notice that there is no Signed-off-by for Felix. Felix, are you ok with this new version? Subject: [PATCH 1/1] virt-aa-helper: handle 9pfs Make virt-aa-helper create rules to allow VMs access to filesystem mounts from the host. Cc: Felix Geyer <debfx@fobos.de> Signed-off-by: Hiroshi Miura <miurahr@linux.com> Signed-off-by: Serge Hallyn <serge.hallyn@ubuntu.com> --- 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 b9282b4..8f6ce95 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,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; -- 1.9.0
Hi, Sorry for the delay. On 28.02.2014 21:36, Serge Hallyn wrote:
Hi,
a separate patch was posted to a new launchpad bug which does a bit more sanity checking on the values passed in, so I went ahead and merged the two. I did however notice that there is no Signed-off-by for Felix. Felix, are you ok with this new version?
Yes, your merged patch looks fine except it still uses // instead of /* ... */ for the comment. I have fixed that so hopefully this patch can be committed now. Cheers, Felix Felix Geyer (1): virt-aa-helper: handle 9pfs src/security/virt-aa-helper.c | 32 ++++++++++++++++++++++++++------ 1 file changed, 26 insertions(+), 6 deletions(-) -- 1.9.0
Make virt-aa-helper create rules to allow VMs access to filesystem mounts from the host. Signed-off-by: Felix Geyer <debfx@fobos.de> Signed-off-by: Hiroshi Miura <miurahr@linux.com> Signed-off-by: Serge Hallyn <serge.hallyn@ubuntu.com> --- 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; -- 1.9.0
Hi, On Sun, Mar 09, 2014 at 04:03:20PM +0100, Felix Geyer wrote:
Make virt-aa-helper create rules to allow VMs access to filesystem mounts from the host.
Signed-off-by: Felix Geyer <debfx@fobos.de> Signed-off-by: Hiroshi Miura <miurahr@linux.com> Signed-off-by: Serge Hallyn <serge.hallyn@ubuntu.com> --- 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;
Pushed now. Sorry for the delay, -- Guido
Hi, Thanks for merging the apparmor handle 9pfs patch! On 2014 03 10 00:03, Felix Geyer wrote:>
On 28.02.2014 21:36, Serge Hallyn wrote:
a separate patch was posted to a new launchpad bug which does a bit more sanity checking on the values passed in, so I went ahead and merged the two. I did however notice that there is no Signed-off-by for Felix. Felix, are you ok with this new version?
Yes, your merged patch looks fine except it still uses // instead of /* ... */ for the comment.
I have fixed that so hopefully this patch can be committed now.
Felix Geyer (1): virt-aa-helper: handle 9pfs
src/security/virt-aa-helper.c | 32 ++++++++++++++++++++++++++------ 1 file changed, 26 insertions(+), 6 deletions(-)
I also have an issue related that selinux don't allow filesystem mounts. There is a work around for it. When a host directory to be shared is `/share` , running following command on host makes things working. ```bash sudo semanage fcontext -a -t virt_content_t "/share(/.*)?" sudo restorecon -R /share ``` IMO it is neccesary to fix /* XXX fixme process def->fss if relabel == true */ part of src/security/security_selinux.c Hiroshi -- Hiroshi Miura OpenStreetMap Foundation Japan
Tested on Debian unstable. The profile updates are partly taken from the Ubuntu trusty libvirt package. --- 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, } 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, } 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]*, -- 1.8.5.3
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/
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
participants (6)
-
Felix Geyer -
Guido Günther -
Guido Günther -
Hiroshi Miura -
Jamie Strandboge -
Serge Hallyn