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

Thu Aug 19 15:23:48 UTC 2021

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 at redhat.com>
>>>
>>> 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 at redhat.com>
>>> ---
>>>   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

>
> 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).

We may have overlooked some bits in the plit, so feel free to point them 
out.

Regards,

Vit

>
> Regards,
> Daniel