No sVirt protection for VMs due to disabled SELinux in 'nova_libvirt' container.

Bug #1880947 reported by Kashyap Chamarthy
266
This bug affects 1 person
Affects Status Importance Assigned to Milestone
tripleo
Fix Released
Critical
Cédric Jeanneret

Bug Description

[Heads-Up: I first reported this to Red Hat's Product Security Team, and they have analyzed the issue and got this CVE number assigned to the problem: CVE-2020-10731; still embargoed, as of 27-May-2020.]

Summary
-------

No sVirt protection for VMs due to disabled SELinux in 'nova_libvirt' container.

Problem Description
-------------------

The 'nova_libvirt' container that TripleO generates has SELinux disabled. This is the container where the libvirt daemon (`libvirtd`) runs; which in turn means, all OSP-16 VMs will have _no_ sVirt protection.

On IRC, Oliver Walsh <email address hidden> pointed out a patch to 'openstack/tripleo-heat-templates' that is linked to this problem; OSP-16 also has this patch:

https://review.opendev.org/#/c/631235/ – nova-libvirt: conditionalize selinux bind-mount

Steps to Reproduce
------------------

(0) Setup a TripleO-based deployment, and ensure at least one VM is launched on the Compute node. (Please refer to the Red Hat documentation about this; it's too long to write it down here.)

(1) Log into the OSP-16 Compute node; and check the SELinux status
_outside_ the container: check SELinux status:

[root@compute-1 ~]# getenforce
Enforcing

(2) However, SELinux is *disabled* in the 'nova_libvirt' container, which spawns the virutal machines VMs:

    [root@compute-1 ~]# podman exec -u root -it nova_libvirt bash

    ()[root@compute-1 /]# getenforce
    Disabled

Actual Results
--------------

Consequently, any VM will now have no SELinux-based sVirt protection. This can be observed by the following. Log into the 'nova_libvirt' container:

    [root@Compute_1 /]# podman exec -u root -it nova_libvirt bash

Then check the "Security model" status of a running (or offline) Nova
instance via:

    ()[root@Compute_1 /]# virsh dominfo instance-00000014 | grep Security
    Security model: none
    Security DOI: 0

Notice the "Security model: none" — this must be (but it's not):

    "Security model: selinux"

Expected Results
----------------

The 'nova_libvirt' container must have SELinux enabled and Triple)-based Nova VMs to have sVirt protection.

    * * *

Thanks
------

Lukas Bezdicka <email address hidden>
Daniel Berrangé <email address hidden>

Lukas ran into this error in a downstream (OSP) environment. And Daniel helped with libvirt expertise and root cause analysis.

CVE References

Revision history for this message
Kashyap Chamarthy (kashyapc) wrote :

Update:

Piotr Kopec <email address hidden> from Red Hat today has confirmed by reverting the above mentioned patch[*], SELinux is correctly enabled inside the 'nova_libvirt" container. And I've also verified it in the environment:

    [heat-admin@compute-1 ~]$ sudo podman exec -it nova_libvirt bash
    ()[root@compute-1 /]#

    ()[root@compute-0 /]# sestatus
    SELinux status: enabled
    SELinuxfs mount: /sys/fs/selinux
    SELinux root directory: /etc/selinux
    Loaded policy name: targeted
    Current mode: enforcing
    Mode from config file: enforcing
    Policy MLS status: enabled
    Policy deny_unknown status: allowed
    Memory protection checking: actual (secure)
    Max kernel policy version: 31

However, Piotr informs on IRC that just simply removing the option in TripleO might not be a good idea, "as it will change the API".

I'll let Piotr chime in here.

[*] https://review.opendev.org/#/c/631235/–nova-libvirt: conditionalize selinux bind-mount"
-------------------------------------------------------------------------------
[stack@undercloud-0 ~]$ cd /usr/share/openstack-tripleo-heat-templates/
[stack@undercloud-0 openstack-tripleo-heat-templates]$ git diff
diff --git a/deployment/nova/nova-libvirt-container-puppet.yaml b/deployment/nova/nova-libvirt-container-puppet.yaml
index 4e02e66..4dea2fa 100644
--- a/deployment/nova/nova-libvirt-container-puppet.yaml
+++ b/deployment/nova/nova-libvirt-container-puppet.yaml
@@ -705,12 +705,7 @@ outputs:
                   - /var/log/libvirt/qemu:/var/log/libvirt/qemu:ro
                   - /var/lib/vhost_sockets:/var/lib/vhost_sockets:z
                   - /var/lib/nova:/var/lib/nova:shared
- -
- if:
- - docker_enabled
- -
- - /sys/fs/selinux:/sys/fs/selinux
- - null
+ - /sys/fs/selinux:/sys/fs/selinux
                 -
                   if:
                     - use_tls_for_live_migration
-------------------------------------------------------------------------------

Changed in tripleo:
milestone: none → victoria-1
importance: Undecided → Critical
status: New → Triaged
Revision history for this message
Alex Schultz (alex-schultz) wrote :

This is probably left overs from early podman implementations when it didn't have proper selinux support and needs to be updated now that selinux does work it correctly with podman

Revision history for this message
Emilien Macchi (emilienm) wrote :

Can we make the bug public and submit the patch upstream?

Revision history for this message
Emilien Macchi (emilienm) wrote :

Reverting that patch won't have any impact on API or UX.

Changed in tripleo:
assignee: nobody → Piotr Kopec (pkopec)
status: Triaged → In Progress
Revision history for this message
Kashyap Chamarthy (kashyapc) wrote :

Emilien, I don't think this can be made public yet; since the embargo is set on the CVE number.

tl;dr: Post the revert as AN ATTACHMENT here; review and get it pre-approve here. Then, once the embargo is lifted (need to coordinate with by Red Hat's Product Security team), it can be fast-tracked through Gerrit.

Per the process here (https://security.openstack.org/vmt-process.html):

On patch development:

[quote]
For a private report, the reporter (automatic if reported directly as a bug) and the affected projects’ core security review teams plus anyone they deem necessary to develop and validate a fix are added to the bug’s subscription list. A fix is proposed as a patch to the current master branch (as well as any affected supported branches) and attached to the private bug report, *not sent to the public code review system.*
[/quote]

And, on patch review:

[quote]
For a private report once the initial patch has been attached to the bug, core reviewers on the subscription list from the project in question should review it and suggest updates or pre-approve it for merging. Privately-developed patches need to be pre-approved so that they can be fast-tracked through public code review later at disclosure time.
[/quote]

Revision history for this message
Jeremy Stanley (fungi) wrote :

Note that since vulnerability reports for TripleO deliverables are not overseen by the OpenStack VMT, they are not obligated to follow that process (but it's a good process nonetheless, or at least I'm supposed to think so as I helped write it).

If the risk for this bug is low, then I would still recommend asking Red Hat's security team to agree to end the imposed embargo as soon as they feel is appropriate. For reports falling under OpenStack VMT oversight we require embargoes not last longer than 90 days except under unusual circumstances, regardless of whether there is a fix available. By comparison, bugs reported to the private linux-distros mailing list (no idea if this has been mentioned there yet) are only allowed to remain private for at most two weeks, reflecting consensus in the broader F/LOSS information security community that embargoes are best kept short if applied at all. Fixing bugs under the shadow of an embargo is a lot of extra work for everyone, and lingering embargoes only make that worse. Kurt Seifried's article here (no idea why his author credit was stripped, that's disconcerting) is still relevant even five years later: https://access.redhat.com/blogs/766093/posts/1976653

If this can be discussed and fixed safely in public, then lifting the embargo as soon as possible is best for everyone involved. It gets information into the hands of users at the earliest opportunity so they can make informed decisions, and also significantly speeds up development of any fixes. In my work on the OpenStack VMT, I try to lift or eliminate embargoes as quickly as possible for these reasons.

Revision history for this message
Nick Tait (nickthetait) wrote :

RH Product Security here, I would like to propose we go public (remove the embargo) tomorrow 5/29 @ 17:00 UTC.

Revision history for this message
Nick Tait (nickthetait) wrote :

I imagine upstream are busy this week with virtual PTG. Will accept whatever the soonest (with at least 24hrs notice) go public date that fungi picks.

Revision history for this message
Cédric Jeanneret (cjeanner) wrote :

Hello,

so we might get something - after digging in the history, labels and all, it appears that:

- prior to osp-15 (aka stable/queens and earlier - hence running docker) docker daemon didn't have the "--enable-selinux" option
- this lead the files within the container to get a certain label (container_share_t)
- there's a specific SELinux policy allowing svirt_t to actually access container_share_t here[1]

Starting OSP-15 (aka stable/stein), we've moved to podman, and there, we actually enforce SELinux all the way down.
This leads to a content change in the container (and this is NOT due to podman - docker has the same), where the files label changes to container_file_t - hence, this escapes the existing SELinux rule listed in [1].
Apparently, it was decided sometime to just disable SELinux for the nova_libvirt container, creating the CVE.
The revert of this deactivation puts back the need to correct/update the SELinux policy, probably duplicating the line pointed in [1] in order to allow svirt_t to {entrypoing execute} on container_file_t:file.

We're currently running some tests with this policy update, and hopefully we'll be able to come back with a working solution.

Cheers,

C.

[1] https://github.com/redhat-openstack/openstack-selinux/blob/master/os-nova.te#L138

Revision history for this message
Jeremy Stanley (fungi) wrote :

It's not really up to me, vulnerability reports for TripleO deliverables are not overseen by the OpenStack VMT, I was just trying to remind folks (in light of Emilien's initial query) that working under an embargo is a lot of extra effort, and so keeping bugs private should only be done if the risk is severe enough to warrant that additional work. I was also trying to counter the impression in Kashyap's reply that the OpenStack VMT's process document encourages keeping vulnerability reports embargoed until there are fixes available (it isn't meant to encourage embargoes, merely to describe how to utilize them when they're warranted). If the TripleO security reviewers subscribed to this bug are comfortable making it public now, then they should go ahead.

Revision history for this message
Kashyap Chamarthy (kashyapc) wrote :

Jeremey,

First of all, I agree with almost everything you said in comment#6 — majorly, keeping embargo dates as short as possible, for all the good reasons you outlined. I was quoting the VMT merely as a guideline, and not as a strict doctrine.

And as discussed on IRC, RHT Product Security (thanks for chiming in here, Nick) is also in broadly in agreement with your thoughts in comment#6—to keep the embargo time frame as reasonably short as possible. So we're all on the same page.

          - - -

Current status (as an addendum to the great summary by Cédric in comment#9):

While testing the fix (i.e. revert of this commit, https://review.opendev.org/#/c/631235/ – "nova-libvirt: conditionalize selinux bind-mount"), we ran into the other SELinux labelling issue that Cedric mentions above. I've described the issue in this publicly-viewable SELinux bug here (https://bugzilla.redhat.com/show_bug.cgi?id=1841822 — "SELinux blocks 'qemu-kvm' running in a container (running in a VM)").

I'm working with Cédric (and also the Red Hat SELinux team) to work out the SELinux policy quirks. I spent a good deal of time yesterday on this, and doing the tests with a fixed SELinux policy that Cédric alluded to in comment#9 above.

Revision history for this message
Nick Tait (nickthetait) wrote :

Fixes have landed and are currently going through QA. Our go-public date has been set to Tuesday (2020-07-28). No specific release time but I am tracking that down...

Changed in tripleo:
milestone: victoria-1 → victoria-3
Revision history for this message
Nick Tait (nickthetait) wrote :

Our embargo has now been lifted, please make this bug public ASAP.

Revision history for this message
Kashyap Chamarthy (kashyapc) wrote :
Download full text (3.5 KiB)

With the fix, here is the correct and desired behaviour:

Validate the SELinux labels
---------------------------

On the Compute node, exec into the 'nova_libvirt' container:

    [root@overcloud-0-novacompute-0 ~]# podman exec -it nova_libvirt /bin/bash

SELinux info:

    ()[root@overcloud-0-novacompute-0 /]# getenforce
    Enforcing

    ()[root@overcloud-0-novacompute-0 /]# sestatus
    SELinux status: enabled
    SELinuxfs mount: /sys/fs/selinux
    SELinux root directory: /etc/selinux
    Loaded policy name: targeted
    Current mode: enforcing
    Mode from config file: enforcing
    Policy MLS status: enabled
    Policy deny_unknown status: allowed
    Memory protection checking: actual (secure)
    Max kernel policy version: 31

Run `virsh dominfo` for the guest:

    ()[root@overcloud-0-novacompute-0 /]# virsh dominfo instance-00000001
    Id: 1
    Name: instance-00000001
    UUID: 9aa82fee-ee30-44d1-a547-cb283f1edf8c
    OS Type: hvm
    State: running
    CPU(s): 1
    CPU time: 18.9s
    Max memory: 524288 KiB
    Used memory: 524288 KiB
    Persistent: yes
    Autostart: disable
    Managed save: no
    Security model: selinux
    Security DOI: 0
    Security label: system_u:system_r:svirt_t:s0:c643,c979 (enforcing)

SELinux label of the running QEMU processes:

    ()[root@overcloud-0-novacompute-0 /]# ps -eZ | grep qemu
    system_u:system_r:svirt_t:s0:c496,c549 216371 ? 00:01:30 qemu-kvm

SELinux label for the QEMU binary file:

    ()[root@overcloud-0-novacompute-0 /]# ls -lZ /usr/libexec/qemu-kvm
    -rwxr-xr-x. 1 root root system_u:object_r:container_ro_file_t:s0 16356584 Apr 6 20:47 /usr/libexec/qemu-kvm

SELinux label for the libvirtd process:

    ()[root@overcloud-0-novacompute-0 /]# ps -eZ | grep libvirtd
    system_u:system_r:spc_t:s0 209874 ? 00:00:01 libvirtd

SELinux label for the libvirtd binary file:

    ()[root@overcloud-0-novacompute-0 /]# ls -lZ /usr/sbin/libvirtd
    -rwxr-xr-x. 1 root root system_u:object_r:container_ro_file_t:s0 618304 Dec 20 01:11 /usr/sbin/libvirtd

                * * *

Capture evidence that sVirt in effect
-------------------------------------

Enumerate the running guests:

    ()[root@overcloud-0-novacompute-0 /]# virsh list
     Id Name State
    -----------------------------------
     1 instance-00000001 running

Enumerate the disk images the above guests are using:

    ()[root@overcloud-0-novacompute-0 /]# virsh domblklist 1
     Target Source
    -----------------------------------------------------------------------------
     vda /var/lib/nova/instances/dfad20d8-8152-46ec-bdd9-b9be2dd0c538/disk

Check the SELinux labels for the above disk image:

    ()[root@overcloud-0-novacompute-0 /]# ls -lZ /var/lib/nova/instances/dfad20d8-8152-46ec-bdd9-b9be2dd0c538/disk
    -rw-r--r--. 1 qemu qemu system_u:object_r:svirt_image_t:s0:c496,c549 59113472 Jun 17 11:46 /var/lib/nova/instances/dfad20d8-8152-46ec-bdd9-b9be2dd0c538/disk

They match the labels for...

Read more...

information type: Private Security → Public
information type: Public → Public Security
Changed in tripleo:
milestone: victoria-3 → wallaby-1
Changed in tripleo:
milestone: wallaby-1 → wallaby-2
Changed in tripleo:
milestone: wallaby-2 → wallaby-3
Changed in tripleo:
milestone: wallaby-3 → wallaby-rc1
Changed in tripleo:
milestone: wallaby-rc1 → xena-1
Changed in tripleo:
milestone: xena-1 → xena-2
Revision history for this message
Marios Andreou (marios-b) wrote :

This is an automated action. Bug status has been set to 'Incomplete' and target milestone has been removed due to inactivity. If you disagree please re-set these values and reach out to us on freenode #tripleo

Changed in tripleo:
milestone: xena-2 → none
status: In Progress → Incomplete
Revision history for this message
Cédric Jeanneret (cjeanner) wrote :

The following patch has been pushed in the relevant releases:
https://review.opendev.org/q/I9e0da2a48c23c35e084bea831fc744b9f053508b

It has been merged long time ago - the "stein" version won't make it due to podman version - in case you're affected, please consider an upgrade to newer releases, it will also provide a newer podman with some more stability.

Changed in tripleo:
status: Incomplete → Fix Released
assignee: Piotr Kopec (pkopec) → Cédric Jeanneret (cjeanner)
To post a comment you must log in.
This report contains Public Security information  
Everyone can see this security related information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.