On Wed, 2017-12-20 at 14:43 +0100, Christian Ehrhardt wrote: > On Tue, Dec 19, 2017 at 5:21 PM, Jamie Strandboge <jamie@xxxxxxxxxxxx > m> wrote: > > On Tue, 2017-12-19 at 16:03 +0100, Christian Ehrhardt wrote: > > > From: Serge Hallyn <serge.hallyn@xxxxxxxxxx> > > > > > > Allows owner access to hugepage mounts (both, the old and > > > new systemd variant). > > > > > > Bug-Ubuntu: https://bugs.launchpad.net/bugs/1250216 > > > Bug-Ubuntu: https://bugs.launchpad.net/bugs/1524737 > > > > > > Signed-off-by: Stefan Bader <stefan.bader@xxxxxxxxxxxxx> > > > --- > > > examples/apparmor/libvirt-qemu | 4 ++++ > > > 1 file changed, 4 insertions(+) > > > > > > diff --git a/examples/apparmor/libvirt-qemu > > > b/examples/apparmor/libvirt-qemu > > > index 912b4ac..bb30530 100644 > > > --- a/examples/apparmor/libvirt-qemu > > > +++ b/examples/apparmor/libvirt-qemu > > > @@ -184,6 +184,10 @@ > > > /sys/bus/ r, > > > /sys/class/ r, > > > > > > + # for access to hugepages (LP: #1250216 and LP: #1524737) > > > + owner "/run/hugepages/kvm/libvirt/qemu/**" rw, > > > + owner "/dev/hugepages/libvirt/qemu/**" rw, > > > + > > > > These rules are not vm-specific. I'm not familiar with the > > hugepages > > feature in libvirt, but the rule suggests that libvirtd will create > > files in these directories per-vm, and then the vm uses what > > libvirtd > > created for it. I see virSecurityManagerSetHugepages() in > > security_manager.c, > > > > is it possible that these rules can be removed and > > vm-specific ones added dynamically with virt-aa-helper? > > 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=ce937d37105c2f > Or a combination of 2 and 3. Ie, if hugepages is enabled, add the rule in 3. To me, 1 feels most correct cause while the other two fix hugepages, there seem to be lurking bugs since we aren't implementing domainSetPathLabel. -- Jamie Strandboge | http://www.canonical.com
Attachment:
signature.asc
Description: This is a digitally signed message part
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
