On Thu, Aug 19, 2021 at 05:23:48PM +0200, Vit Mojzis wrote: > > On 10. 08. 21 18:35, Daniel P. Berrangé wrote: > > On Tue, Aug 10, 2021 at 10:39:23AM +0200, Pavel Hrdina wrote: > > > On Fri, Aug 06, 2021 at 06:47:58PM +0100, Daniel P. Berrangé wrote: > > > > From: Nikola Knazekova <nknazeko@xxxxxxxxxx> > > > > > > > > SELinux policy was created for: > > > > > > > > Hypervisor drivers: > > > > - virtqemud (QEMU/KVM) > > > > - virtlxcd (LXC) > > > > - virtvboxd (VirtualBox) > > > > > > > > Secondary drivers: > > > > - virtstoraged (host storage mgmt) > > > > - virtnetworkd (virtual network mgmt) > > > > - virtinterface (network interface mgmt) > > > > - virtnodedevd (physical device mgmt) > > > > - virtsecretd (security credential mgmt) > > > > - virtnwfilterd (ip[6]tables/ebtables mgmt) > > > > - virtproxyd (proxy daemon) > > > > > > > > SELinux policy for virtvxz and virtxend has not been created yet, > > > > because I wasn't able to reproduce AVC messages. These drivers > > > > run in unconfined_domain until the AVC messages are reproduced > > > > internally and policy for these drivers is made. > > > > > > > > Signed-off-by: Nikola Knazekova <nknazeko@xxxxxxxxxx> > > > > --- > > > > src/security/selinux/virt.fc | 111 ++ > > > > src/security/selinux/virt.if | 1984 ++++++++++++++++++++++++++++++++ > > > > src/security/selinux/virt.te | 2078 ++++++++++++++++++++++++++++++++++ > > > > 3 files changed, 4173 insertions(+) > > > > create mode 100644 src/security/selinux/virt.fc > > > > create mode 100644 src/security/selinux/virt.if > > > > create mode 100644 src/security/selinux/virt.te > > > > > > > > diff --git a/src/security/selinux/virt.fc b/src/security/selinux/virt.fc > > > > new file mode 100644 > > > > index 0000000000..554e1094d9 > > > > --- /dev/null > > > > +++ b/src/security/selinux/virt.fc > > > > @@ -0,0 +1,111 @@ > > > > +HOME_DIR/\.libvirt(/.*)? gen_context(system_u:object_r:virt_home_t,s0) > > > > +HOME_DIR/\.libvirt/qemu(/.*)? gen_context(system_u:object_r:svirt_home_t,s0) > > > > +HOME_DIR/\.cache/libvirt(/.*)? gen_context(system_u:object_r:virt_home_t,s0) > > > > +HOME_DIR/\.cache/libvirt/qemu(/.*)? gen_context(system_u:object_r:svirt_home_t,s0) > > > > +HOME_DIR/\.config/libvirt(/.*)? gen_context(system_u:object_r:virt_home_t,s0) > > > > +HOME_DIR/\.config/libvirt/qemu(/.*)? gen_context(system_u:object_r:svirt_home_t,s0) > > > > +HOME_DIR/VirtualMachines(/.*)? gen_context(system_u:object_r:virt_home_t,s0) > > > > +HOME_DIR/VirtualMachines/isos(/.*)? gen_context(system_u:object_r:virt_content_t,s0) > > > These two doesn't look like libvirt selinux bits, more like virt-manager > > > or some other tool. > > Rationale is largely lost in the mists of time to be honest. $HOME/VirtualMachines > > does make sense for desktop virt use case I think, while the below rules make > > sense as a direct translation of libvirt's system paths. > > > > I think its ok to have both really > > > > > > +HOME_DIR/\.local/share/libvirt/images(/.*)? gen_context(system_u:object_r:svirt_home_t,s0) > > > > +HOME_DIR/\.local/share/libvirt/boot(/.*)? gen_context(system_u:object_r:svirt_home_t,s0) > > > > +/var/lib/libvirt(/.*)? gen_context(system_u:object_r:virt_var_lib_t,s0) > > > > +/var/lib/libvirt/boot(/.*)? gen_context(system_u:object_r:virt_content_t,s0) > > > > +/var/lib/libvirt/images(/.*)? gen_context(system_u:object_r:virt_image_t,s0) > > > > +/var/lib/libvirt/isos(/.*)? gen_context(system_u:object_r:virt_content_t,s0) > > > > +/var/lib/libvirt/lockd(/.*)? gen_context(system_u:object_r:virt_var_lockd_t,s0) > > > > +/var/lib/libvirt/qemu(/.*)? gen_context(system_u:object_r:qemu_var_run_t,s0-mls_systemhigh) > > > > + > > > > +/var/log/log(/.*)? gen_context(system_u:object_r:virt_log_t,s0) > > > Based on commit from selinux-policy 63ead48cf8 this seems vdsm related. > > > I don't think that we use this directory in libvirt. > > Yeah, that's dubious. > Good point, we'll move it out of virt policy. > > > > > > +/var/log/libvirt(/.*)? gen_context(system_u:object_r:virt_log_t,s0) > > > > +/var/run/libvirtd\.pid -- gen_context(system_u:object_r:virt_var_run_t,s0) > > > > +# Avoid calling m4's "interface" by using en empty string > > > > +/var/run/libvirt/interfac(e)(/.*)? gen_context(system_u:object_r:virtinterfaced_var_run_t,s0) > > > > +/var/run/libvirt/nodedev(/.*)? gen_context(system_u:object_r:virtnodedevd_var_run_t,s0) > > > > +/var/run/libvirt/nwfilter(/.*)? gen_context(system_u:object_r:virtnwfilterd_var_run_t,s0) > > > > +/var/run/libvirt/secrets(/.*)? gen_context(system_u:object_r:virtsecretd_var_run_t,s0) > > > > +/var/run/libvirt/storage(/.*)? gen_context(system_u:object_r:virtstoraged_var_run_t,s0) > > > > + > > > > +/var/run/virtlogd\.pid -- gen_context(system_u:object_r:virtlogd_var_run_t,s0) > > > > +/var/run/virtlxcd\.pid -- gen_context(system_u:object_r:virt_lxc_var_run_t,s0) > > > > +/var/run/virtqemud\.pid -- gen_context(system_u:object_r:virtqemud_var_run_t,s0) > > > > +/var/run/virtvboxd\.pid -- gen_context(system_u:object_r:virtvboxd_var_run_t,s0) > > > > +/var/run/virtproxyd\.pid -- gen_context(system_u:object_r:virtproxyd_var_run_t,s0) > > > > +/var/run/virtinterfaced\.pid -- gen_context(system_u:object_r:virtinterfaced_var_run_t,s0) > > > > +/var/run/virtnetworkd\.pid -- gen_context(system_u:object_r:virtnetworkd_var_run_t,s0) > > > > +/var/run/virtnodedevd\.pid -- gen_context(system_u:object_r:virtnodedevd_var_run_t,s0) > > > > +/var/run/virtnwfilterd\.pid -- gen_context(system_u:object_r:virtnwfilterd_var_run_t,s0) > > > > +/var/run/virtnwfilterd-binding\.pid -- gen_context(system_u:object_r:virtnwfilterd_var_run_t,s0) > > > > +/var/run/virtsecretd\.pid -- gen_context(system_u:object_r:virtsecretd_var_run_t,s0) > > > > +/var/run/virtstoraged\.pid -- gen_context(system_u:object_r:virtstoraged_var_run_t,s0) > > > [...] > > > > > > I was not able to figure out on which selinux policy is this one based > > > on as the upstream for rawhide from <https://github.com/fedora-selinux/selinux-policy.git> > > > is a bit different. There are some cosmetics changes but I see two major > > > differences: > > > > > > - the upstream policy doesn't have split-daemon bits compared to > > > this one, I checked it and it looks reasonable but I'm not that > > > familiar with selinux policy > > Now I compare the two, I see there's a bunch of stuff in the current > > fedora virt.te that doesn't exist in this virt.te. > > This policy has been mostly rewritten by Nikola Knazekova to work with the > split daemon configuration. > > It's only been tested using libvirt-tck and by running some VM by hand so we > could use your help running other tests you have available. > > There has actually been some progress on the policy since I last updated the > PR. The latest version is available here: > https://github.com/5umm3r15/selinux-policy/blob/libvirt-selinux/policy/modules/contrib/virt.te > > https://github.com/5umm3r15/selinux-policy/blob/libvirt-selinux/policy/modules/contrib/virt.fc > > https://github.com/5umm3r15/selinux-policy/blob/libvirt-selinux/policy/modules/contrib/virt.if > > The MLS parts of the policy are still not 100% since we are not sure about > some access that is taking place during testing with libvirt-tck. > > Please see: > https://gitlab.com/libvirt/libvirt-tck/-/merge_requests/8#note_601463944 Doh I missed those questions. > > > > So if we deploy this, then its likely to break stuff that's in the > > current virt.te that we've omitted. > > > > I should have checked this more closely before re-sendig, as I just > > blindly assumed that the differences to fedora-selinux had been > > eliminated after my original review comments :-( > > > > > - the upstream policy has important `system.token` issue fix that > > > we've seen recently introduced by upstream commit <1f761d0bbd> > > > > My view for pulling any SElinux policy into libvirt.git is that we need > > to untangle the current fedora selinux virt policy first to remove all > > the non-libvirt pieces. It should then be a direct copy into libvirt.git > > with no modifications. > > > > So I don't think this is mergable as it exists now. > > The policy has been split to virt and virt_supplementary (https://github.com/5umm3r15/selinux-policy/blob/libvirt-selinux/policy/modules/contrib/virt_supplementary.te), > where virt_supplementary has the bits that are non-libvirt (this part will > stay in selinux-policy repo). IIUC, the repo 5umm3r15/selinux-policy is not the main repo used for Fedora policy. If I merged this policy in libvirt now, and we deployed it on Fedora we would regress becaue virt_supplementary doesn't exist in any current Fedora / rawhide IIUC. Is there an ETA for merging the virt/virt_supplementary stuff into the official Fedora policy, and updating rawhide / RHEL-9, so that we can in turn merge & release the libvirt parts ? Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
