Re: [libvirt] [PATCH 06/12] apparmor, libvirt-qemu: Allow access to hugepage mounts

On Tue, Dec 19, 2017 at 5:21 PM, Jamie Strandboge <jamie(a)canonical.com&gt; wrote: I agree - and in general it would also be nice to do a per guest rule with a full path to keep the guests away from each other. I see different ways to go thou: #1 - per security_apparmor.c The particular function you mention was generalized by [1] and further reused in [3]. The creation of the per guest paths is in [2]. So essentially it comes down to a call to domainSetPathLabel with the path as an argument. But then security_apparmor.c so far does not implement domainSetPathLabel at all. If we add a function for domainSetPathLabel it could call reload_profile with the path (as others in security_apparmor.c do). But this path is in use by other places as well: - qemuDomainWriteMasterKeyFile - qemuProcessMakeDir OTOH domainSetPathLabel is used in those places for the reson of it's meaning "add this to the scope of the security label right?" So we might after all want to add all these anyway? If that is so, an (untested) change could be like: --- a/src/security/security_apparmor.c +++ b/src/security/security_apparmor.c @@ -953,6 +953,13 @@ AppArmorSetSavedStateLabel(virSecurityManagerPtr mgr, return reload_profile(mgr, def, savefile, true); } +static int +AppArmorSetPathLabel(virSecurityManagerPtr mgr, + virDomainDefPtr def, + const char *path) +{ + return reload_profile(mgr, def, path, true); +} static int AppArmorRestoreSavedStateLabel(virSecurityManagerPtr mgr, @@ -1045,6 +1052,8 @@ virSecurityDriver virAppArmorSecurityDriver = { .domainSetSavedStateLabel = AppArmorSetSavedStateLabel, .domainRestoreSavedStateLabel = AppArmorRestoreSavedStateLabel, + .domainSetPathLabel = AppArmorSetPathLabel, + .domainSetSecurityImageFDLabel = AppArmorSetFDLabel, .domainSetSecurityTapFDLabel = AppArmorSetFDLabel, #2 in virt-aa-helper with XML parsing The next option would to try to detect if hugepages are used from the xml description in virt-aa-helper. #3 unconditional per domain rule And finally we could avoid too much (?over?)engineering and unconditionally add this in virt-aa-helper (at the cost of breaking if paths generated [2] ever change): owner "/dev/hugepages/libvirt/qemu/<domainName>" rw, IMHO - [1] or [3] - please let me know your opinions before I follow any of those paths. [1]: https://libvirt.org/git/?p=libvirt.git;a=commit;h=ce937d37105c2fc95638f4d... [2]: https://libvirt.org/git/?p=libvirt.git;a=commit;h=f55afd83b1338e17eae7ec8... [3]: https://libvirt.org/git/?p=libvirt.git;a=commit;h=eff2b2edb147572b6d8f701... [4]: https://libvirt.org/git/?p=libvirt.git;a=blob;f=src/security/virt-aa-help... -- Christian Ehrhardt Software Engineer, Ubuntu Server Canonical Ltd