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

Thu Aug 26 11:22:23 UTC 2021

On 20. 08. 21 13:33, Daniel P. Berrangé wrote:
> 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 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
> 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.
>
Yes, it's a fork.

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

Yes, we need to deploy it on Fedora first.

>
> 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 ?

We'll deploy it in rawhide as soon as we get the MLS part sorted.
No ETA at this point.

Vit

>
>
> Regards,
> Daniel