Re: [libvirt PATCH 01/13] security: add SELinux policy for virt

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

>> +/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.

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.

Regards, Daniel