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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

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




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux