Bug 2122656 - Inconsistent selinux type of /var/run/libvirt
Summary: Inconsistent selinux type of /var/run/libvirt
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Red Hat OpenStack
Classification: Red Hat
Component: openstack-tripleo-heat-templates
Version: 16.2 (Train)
Hardware: Unspecified
OS: Unspecified
high
high
Target Milestone: z4
: 16.2 (Train on RHEL 8.4)
Assignee: Cédric Jeanneret
QA Contact: James Parker
URL:
Whiteboard:
Depends On:
Blocks: 2103969 2103970 2103971
TreeView+ depends on / blocked
 
Reported: 2022-08-30 14:27 UTC by Takashi Kajinami
Modified: 2024-12-11 12:29 UTC (History)
10 users (show)

Fixed In Version: openstack-tripleo-heat-templates-11.6.1-2.20221010235131.e0d438c.el8ost
Doc Type: No Doc Update
Doc Text:
Clone Of:
Environment:
Last Closed: 2022-12-07 19:24:09 UTC
Target Upstream Version:
Embargoed:


Attachments (Terms of Use)


Links
System ID Private Priority Status Summary Last Updated
OpenStack gerrit 856535 0 None MERGED Correct label for /run/libvirt 2022-09-14 13:44:23 UTC
OpenStack gerrit 858616 0 None MERGED Correct label for /var/run/libvirt 2022-09-23 13:06:03 UTC
Red Hat Issue Tracker OSP-18465 0 None None None 2022-08-30 15:00:47 UTC
Red Hat Product Errata RHBA-2022:8794 0 None None None 2022-12-07 19:24:50 UTC

Description Takashi Kajinami 2022-08-30 14:27:27 UTC
Description of problem:

The following change introduced the z flag to /var/run/libvirt bind mount in nova_migration_target.
Because of this the directory is labeled with container_file_t when the service is started.

https://review.opendev.org/c/openstack/tripleo-heat-templates/+/741403
https://github.com/openstack/tripleo-heat-templates/blob/621425b2551891b1c68e42507dce4096a8e10174/deployment/nova/nova-migration-target-container-puppet.yaml#L183

However this is inconsistent with the tasks to create the directory.
The directory is initially created with virt_var_run_t.

https://github.com/openstack/tripleo-heat-templates/blob/621425b2551891b1c68e42507dce4096a8e10174/deployment/nova/nova-migration-target-container-puppet.yaml#L160
https://github.com/openstack/tripleo-heat-templates/blob/621425b2551891b1c68e42507dce4096a8e10174/deployment/nova/nova-libvirt-container-puppet.yaml#L861

Because of this we observe "flapping" of selinux type assigned to this directory.

1. When the deployment is initially deployed, the directory is initially created with virt_var_run_t
   (which is var_run_t it seems), but later relabeled by container_file_t when migration target is started.

2. Then if a user runs overcloud deploy again without any change, the directory is labeled by
   virt_var_run_t.

I could not find the exact selinux denial fixed by the above change, but if container_file_t
is really required then we should update the directory creation task to use container_file_t.

Version-Release number of selected component (if applicable):


How reproducible:


Steps to Reproduce:
1.
2.
3.

Actual results:


Expected results:


Additional info:

Comment 1 Kashyap Chamarthy 2022-09-07 14:54:09 UTC
(In reply to Takashi Kajinami from comment #0)
> Description of problem:
> 
> The following change introduced the z flag to /var/run/libvirt bind mount in
> nova_migration_target.
> Because of this the directory is labeled with container_file_t when the
> service is started.
> 
> https://review.opendev.org/c/openstack/tripleo-heat-templates/+/741403
> https://github.com/openstack/tripleo-heat-templates/blob/
> 621425b2551891b1c68e42507dce4096a8e10174/deployment/nova/nova-migration-
> target-container-puppet.yaml#L183
> 
> However this is inconsistent with the tasks to create the directory.
> The directory is initially created with virt_var_run_t.
> 
> https://github.com/openstack/tripleo-heat-templates/blob/
> 621425b2551891b1c68e42507dce4096a8e10174/deployment/nova/nova-migration-
> target-container-puppet.yaml#L160
> https://github.com/openstack/tripleo-heat-templates/blob/
> 621425b2551891b1c68e42507dce4096a8e10174/deployment/nova/nova-libvirt-
> container-puppet.yaml#L861
> 
> Because of this we observe "flapping" of selinux type assigned to this
> directory.
> 
> 1. When the deployment is initially deployed, the directory is initially
> created with virt_var_run_t
>    (which is var_run_t it seems), but later relabeled by container_file_t
> when migration target is started.
> 
> 2. Then if a user runs overcloud deploy again without any change, the
> directory is labeled by
>    virt_var_run_t.
> 
> I could not find the exact selinux denial fixed by the above change, but if
> container_file_t
> is really required then we should update the directory creation task to use
> container_file_t.

@Cedric: What do you think of the above, on consistently using "container_file_t"?  I recall you discussing this on IRC and the above-mentioned patch

Comment 2 Cédric Jeanneret 2022-09-08 07:06:38 UTC
Hello,

IMHO we should stick to the virt_var_run_t. It makes more sense, since it IS a var_run mounted in a container, hence the virt_ prefix (remember the old virt_sandbox_file_t thing [or close to that] before container_file_t appears).

Also, I'd rather avoid playing too much with relabeling on that location, since we spent a fair amount of time ensuring sVirt was working [again] about 2y ago....

So, IMHO, we should just remove all of the "z" flag for that mount point, and check the potential denials raised by actual usage and fix the denials in the openstack-selinux...

@jpichon any thoughts? @kchamart wdyt? I don't remember who helped us so much on that sVirt thing, but maybe they have an opinion as well?

Cheers,

C.

Comment 3 Cédric Jeanneret 2022-09-08 14:52:33 UTC
Hello,

So, some news, good mostly:

History:
virt_var_run_t is the default type for /run/libvirt:
/var/run/libvirt(/.*)?                             all files          system_u:object_r:virt_var_run_t:s0 
(from semanage fcontext -l)

I therefore set that value when creating the location - it was probably dumb, but that was 4 years ago(!)[1]

Now, that label is probably wrong, and, frankly, I have the feeling the whole thing is working just by chance, thanks to the migration-target container having the ":z" flag[2]...

Proposal:
1. we removed the setype for the creation of the directory here[3]
2. we ensure all of the bind-mount of /run/libvirt has the :z flag (mostly, it will be :shared,z)

Impact for update: none (ephemeral directory recreated at boot time, and contains a couple of files, and it's already container_file_t at runtime)
Impact for upgrade: none (same reason - ephemeral, and already relabeled)

Guess that's the safest, fastest, less invasive way to solve the issue.

I'll propose a patch shortly.

Cheers,

C.


[1] https://review.opendev.org/c/openstack/tripleo-heat-templates/+/615956/7/docker/services/nova-libvirt.yaml
[2] https://opendev.org/openstack/tripleo-heat-templates/src/branch/master/deployment/nova/nova-migration-target-container-puppet.yaml#L208
[3] https://opendev.org/openstack/tripleo-heat-templates/src/branch/master/deployment/nova/nova-modular-libvirt-container-puppet.yaml#L938

Comment 4 Bogdan Dobrelya 2022-09-08 14:56:12 UTC
Thank you, Cedric, I'll take it over

Comment 5 Bogdan Dobrelya 2022-09-08 15:04:17 UTC
Sorry, that was a misunderstanding. It's all yours, Cedric, please go for it

Comment 6 Leif Madsen 2022-09-14 13:45:40 UTC
I think this can be closed now that the changes are merged upstream to master. We can use the blocker issues to track the changes into downstream (backports) as ultimately this fix is the underlying resolution for those tracked issues in CloudOps.

Comment 7 Kashyap Chamarthy 2022-09-21 14:01:39 UTC
@(In reply to Cédric Jeanneret from comment #3)
> Hello,
> 
> So, some news, good mostly:
> 
> History:
> virt_var_run_t is the default type for /run/libvirt:
> /var/run/libvirt(/.*)?                             all files         
> system_u:object_r:virt_var_run_t:s0 
> (from semanage fcontext -l)
> 
> I therefore set that value when creating the location - it was probably
> dumb, but that was 4 years ago(!)[1]
> 
> Now, that label is probably wrong, and, frankly, I have the feeling the
> whole thing is working just by chance, thanks to the migration-target
> container having the ":z" flag[2]...
> 
> Proposal:
> 1. we removed the setype for the creation of the directory here[3]
> 2. we ensure all of the bind-mount of /run/libvirt has the :z flag (mostly,
> it will be :shared,z)
> 
> Impact for update: none (ephemeral directory recreated at boot time, and
> contains a couple of files, and it's already container_file_t at runtime)
> Impact for upgrade: none (same reason - ephemeral, and already relabeled)
> 
> Guess that's the safest, fastest, less invasive way to solve the issue.
> 
> I'll propose a patch shortly.

Great sleuthing, Cédric.  This makes sense to me; we should indeed avoid too much messing around with relabeling, as it involves too much testing[1]

And the person that helped us with sVirt was Daniel Berrangé :-) (He wrote the sVirt part in libvirt, IIRC.)

[1] https://kashyapc.fedorapeople.org/SELinux_libvirt_and_QEMU_in_a_container.html
 
    - - -

When you get around to it, can you please add the "Fixed in Version" field above, and then close it out?

I see the patch has merged: https://review.opendev.org/c/openstack/tripleo-heat-templates/+/856535/ (Correct label for /run/libvirt)


[...]

Comment 8 Cédric Jeanneret 2022-09-22 13:27:38 UTC
Hey Kashyap,

Can't set any FIV for now, it's not downstream - patch is in merge-conflict for 17.0, and not yet in stable/train, not yet proposed to 16.2 either..

This should follow shortly hopefully.

I'll check with Leif about the 17.0, in order to correct the current merge conflict. Shouldn't be that hard to squash.

Cheers,

C.

Comment 21 errata-xmlrpc 2022-12-07 19:24:09 UTC
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory (Release of components for Red Hat OpenStack Platform 16.2.4), and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHBA-2022:8794

Comment 22 Bogdan Dobrelya 2024-12-11 12:29:24 UTC
*** Bug 2331316 has been marked as a duplicate of this bug. ***


Note You need to log in before you can comment on or make changes to this bug.