Tolerate NFS exports in /var/lib/nova when selinux relabelling

When the :z bind mount option is used, podman peforms a recursive relabel of
the mount point which is failing with "Operation not supported" if there are
any NFS exports mounted within. While it's possible for NFS to support true
selinux labelling, in practice is rarely does.

As we are already walking the tree to set ownership/permission, take ownership
of the relabelling logic too and skip relabelling on subtrees where we hit this
error.

Change-Id: Id5503ed274bd5dc0c5365cc994de7e5cdcbc2fb6
Closes-bug: #1869020

bindep.txt

@@ -2,3 +2,9 @@
 # see https://docs.openstack.org/infra/bindep/ for additional information.
 libssl-dev [platform:dpkg]
 openssl-devel [platform:rpm]
+
+# For SELinux
+libselinux-python  [platform:rpm !platform:rhel-8 !platform:centos-8]
+libsemanage-python [platform:redhat !platform:rhel-8 !platform:centos-8]
+libselinux-python3  [platform:rpm !platform:rhel-7 !platform:centos-7]
+libsemanage-python3 [platform:redhat !platform:rhel-7 !platform:centos-7]

container_config_scripts/nova_statedir_ownership.py

@@ -17,6 +17,7 @@ from __future__ import print_function
 import logging
 import os
 import pwd
+import selinux
 import stat
 import sys
 
@@ -42,6 +43,7 @@ class PathManager(object):
         self.is_dir = stat.S_ISDIR(statinfo.st_mode)
         self.uid = statinfo.st_uid
         self.gid = statinfo.st_gid
+        self.secontext = selinux.lgetfilecon(self.path)[1]
 
     def __str__(self):
         return "uid: {} gid: {} path: {}{}".format(
@@ -83,6 +85,27 @@ class PathManager(object):
                      uid,
                      gid)
 
+    def chcon(self, context):
+        # If dir returns whether to recusively set context
+        try:
+            try:
+                selinux.lsetfilecon(self.path, context)
+                LOG.info('Setting selinux context of %s to %s',
+                     self.path, context)
+                return True
+            except OSError as e:
+                if self.is_dir and e.errno == 95:
+                    # Operation not supported, assume NFS mount and skip
+                    LOG.info('Setting selinux context not supported for %s',
+                             self.path)
+                    return False
+                else:
+                    raise
+        except Exception:
+            LOG.error('Could not set selinux context of %s to %s:',
+                          self.path, context)
+            raise
+
 
 class NovaStatedirOwnershipManager(object):
     """Class to manipulate the ownership of the nova statedir (/var/lib/nova).
@@ -106,17 +129,19 @@ class NovaStatedirOwnershipManager(object):
        docker nova uid/gid is not known in this context).
     """
     def __init__(self, statedir, upgrade_marker='upgrade_marker',
-                 nova_user='nova'):
+                 nova_user='nova', secontext_marker='../_nova_secontext'):
         self.statedir = statedir
         self.nova_user = nova_user
 
         self.upgrade_marker_path = os.path.join(statedir, upgrade_marker)
+        self.secontext_marker_path = os.path.normpath(os.path.join(statedir, secontext_marker))
         self.upgrade = os.path.exists(self.upgrade_marker_path)
 
         self.target_uid, self.target_gid = self._get_nova_ids()
         self.previous_uid, self.previous_gid = self._get_previous_nova_ids()
         self.id_change = (self.target_uid, self.target_gid) != \
             (self.previous_uid, self.previous_gid)
+        self.target_secontext = self._get_secontext()
 
     def _get_nova_ids(self):
         nova_uid, nova_gid = pwd.getpwnam(self.nova_user)[2:4]
@@ -129,7 +154,13 @@ class NovaStatedirOwnershipManager(object):
         else:
             return self._get_nova_ids()
 
-    def _walk(self, top):
+    def _get_secontext(self):
+        if os.path.exists(self.secontext_marker_path):
+            return selinux.lgetfilecon(self.secontext_marker_path)[1]
+        else:
+            return None
+
+    def _walk(self, top, chcon=True):
         for f in os.listdir(top):
             pathname = os.path.join(top, f)
 
@@ -141,7 +172,10 @@ class NovaStatedirOwnershipManager(object):
             if pathinfo.is_dir:
                 # Always chown the directories
                 pathinfo.chown(self.target_uid, self.target_gid)
-                self._walk(pathname)
+                chcon_r = chcon
+                if chcon:
+                    chcon_r = pathinfo.chcon(self.target_secontext)
+                self._walk(pathname, chcon_r)
             elif self.id_change:
                 # Only chown files if it's an upgrade and the file is owned by
                 # the host nova uid/gid
@@ -151,6 +185,8 @@ class NovaStatedirOwnershipManager(object):
                     self.target_gid if pathinfo.gid == self.previous_gid
                     else pathinfo.gid
                 )
+                if chcon:
+                    pathinfo.chcon(self.target_secontext)
 
     def run(self):
         LOG.info('Applying nova statedir ownership')
@@ -162,8 +198,12 @@ class NovaStatedirOwnershipManager(object):
         pathinfo = PathManager(self.statedir)
         LOG.info("Checking %s", pathinfo)
         pathinfo.chown(self.target_uid, self.target_gid)
+        chcon = self.target_secontext is not None
 
-        self._walk(self.statedir)
+        if chcon:
+            pathinfo.chcon(self.target_secontext)
+
+        self._walk(self.statedir, chcon)
 
         if self.upgrade:
             LOG.info('Removing upgrade_marker %s',

container_config_scripts/tests/test_nova_statedir_ownership.py

@@ -17,12 +17,24 @@ import contextlib
 import mock
 import six
 import stat
+import sys
 
 from oslotest import base
 
+
+class FakeSelinux(object):
+    @staticmethod
+    def lgetfilecon(path):
+        pass
+
+    @staticmethod
+    def lsetfilecon(path, context):
+        pass
+sys.modules["selinux"] = FakeSelinux
+
 from container_config_scripts.nova_statedir_ownership import \
-    NovaStatedirOwnershipManager
+    NovaStatedirOwnershipManager  # noqa: E402
-from container_config_scripts.nova_statedir_ownership import PathManager
+from container_config_scripts.nova_statedir_ownership import PathManager  # noqa: E402
 
 # Real chown would require root, so in order to test this we need to fake
 # all of the methods that interact with the filesystem
@@ -43,60 +55,83 @@ class FakeStatInfo(object):
 
 def generate_testtree1(nova_uid, nova_gid):
     return {
-        '/var/lib/nova':
+        '/var/lib/nova': {
-            FakeStatInfo(st_mode=stat.S_IFDIR,
+            'stat': FakeStatInfo(st_mode=stat.S_IFDIR,
+                                 st_uid=nova_uid,
+                                 st_gid=nova_gid),
+            'nfs': False,
+        },
+        '/var/lib/_nova_secontext': {
+            'stat': FakeStatInfo(st_mode=stat.S_IFDIR,
+                                 st_uid=nova_uid,
+                                 st_gid=nova_gid),
+            'nfs': False,
+        },
+
+        '/var/lib/nova/instances': {
+            'stat': FakeStatInfo(st_mode=stat.S_IFDIR,
                          st_uid=nova_uid,
                          st_gid=nova_gid),
-        '/var/lib/nova/instances':
+            'nfs': False,
-            FakeStatInfo(st_mode=stat.S_IFDIR,
+        },
-                         st_uid=nova_uid,
+        '/var/lib/nova/instances/foo': {
-                         st_gid=nova_gid),
+            'stat': FakeStatInfo(st_mode=stat.S_IFDIR,
-        '/var/lib/nova/instances/foo':
+                                 st_uid=nova_uid,
-            FakeStatInfo(st_mode=stat.S_IFDIR,
+                                 st_gid=nova_gid),
-                         st_uid=nova_uid,
+            'nfs': True,
-                         st_gid=nova_gid),
+        },
-        '/var/lib/nova/instances/foo/bar':
+        '/var/lib/nova/instances/foo/bar': {
-            FakeStatInfo(st_mode=stat.S_IFREG,
+            'stat': FakeStatInfo(st_mode=stat.S_IFREG,
-                         st_uid=0,
+                                 st_uid=0,
-                         st_gid=0),
+                                 st_gid=0),
-        '/var/lib/nova/instances/foo/baz':
+            'nfs': True,
-            FakeStatInfo(st_mode=stat.S_IFREG,
+        },
-                         st_uid=nova_uid,
+        '/var/lib/nova/instances/foo/baz': {
-                         st_gid=nova_gid),
+            'stat': FakeStatInfo(st_mode=stat.S_IFREG,
-        '/var/lib/nova/instances/foo/abc':
+                                 st_uid=nova_uid,
-            FakeStatInfo(st_mode=stat.S_IFREG,
+                                 st_gid=nova_gid),
-                         st_uid=0,
+            'nfs': True,
-                         st_gid=nova_gid),
+        },
-        '/var/lib/nova/instances/foo/def':
+        '/var/lib/nova/instances/foo/abc': {
-            FakeStatInfo(st_mode=stat.S_IFREG,
+            'stat': FakeStatInfo(st_mode=stat.S_IFREG,
-                         st_uid=nova_uid,
+                                 st_uid=0,
-                         st_gid=0),
+                                 st_gid=nova_gid),
+            'nfs': True,
+        },
+        '/var/lib/nova/instances/foo/def': {
+            'stat': FakeStatInfo(st_mode=stat.S_IFREG,
+                                 st_uid=nova_uid,
+                                 st_gid=0),
+            'nfs': True,
+        },
     }
 
 
 def generate_testtree2(marker_uid, marker_gid, *args, **kwargs):
     tree = generate_testtree1(*args, **kwargs)
     tree.update({
-        '/var/lib/nova/upgrade_marker':
+        '/var/lib/nova/upgrade_marker': {
-            FakeStatInfo(st_mode=stat.S_IFREG,
+            'stat': FakeStatInfo(st_mode=stat.S_IFREG,
-                         st_uid=marker_uid,
+                                 st_uid=marker_uid,
-                         st_gid=marker_gid)
+                                 st_gid=marker_gid),
+            'nfs': False,
+        }
     })
     return tree
 
 
 def generate_fake_stat(testtree):
     def fake_stat(path):
-        return testtree.get(path)
+        return testtree.get(path, {}).get('stat')
     return fake_stat
 
 
 def generate_fake_chown(testtree):
     def fake_chown(path, uid, gid):
         if uid != -1:
-            testtree[path].st_uid = uid
+            testtree[path]['stat'].st_uid = uid
         if gid != -1:
-            testtree[path].st_gid = gid
+            testtree[path]['stat'].st_gid = gid
     return fake_chown
 
 
@@ -123,6 +158,14 @@ def generate_fake_unlink(testtree):
     return fake_unlink
 
 
+def generate_fake_lsetfilecon(testtree):
+    def fake_lsetfilecon(path, context):
+        if testtree[path]['nfs']:
+            e = OSError('Operation not supported')
+            e.errno = 95
+            raise e
+
+
 @contextlib.contextmanager
 def fake_testtree(testtree):
     fake_stat = generate_fake_stat(testtree)
@@ -130,6 +173,7 @@ def fake_testtree(testtree):
     fake_exists = generate_fake_exists(testtree)
     fake_listdir = generate_fake_listdir(testtree)
     fake_unlink = generate_fake_unlink(testtree)
+    fake_lsetfilecon = generate_fake_lsetfilecon(testtree)
     with mock.patch('os.chown',
                     side_effect=fake_chown) as fake_chown:
         with mock.patch('os.path.exists',
@@ -144,17 +188,27 @@ def fake_testtree(testtree):
                                 'os.unlink',
                                 side_effect=fake_unlink
                                 ) as fake_unlink:
-                            yield (fake_chown,
+                            with mock.patch(
-                                   fake_exists,
+                                    'selinux.lgetfilecon',
-                                   fake_listdir,
+                                    return_value=[10, 'newcontext'],
-                                   fake_stat,
+                                    ) as fake_lgetfilecon:
-                                   fake_unlink)
+                                with mock.patch(
+                                        'selinux.lsetfilecon',
+                                        side_effect=fake_lsetfilecon,
+                                        ) as fake_lsetfilecon:
+                                    yield (fake_chown,
+                                           fake_exists,
+                                           fake_listdir,
+                                           fake_stat,
+                                           fake_unlink,
+                                           fake_lgetfilecon,
+                                           fake_lsetfilecon)
 
 
 def assert_ids(testtree, path, uid, gid):
-    statinfo = testtree[path]
+    statinfo = testtree[path]['stat']
     assert (uid, gid) == (statinfo.st_uid, statinfo.st_gid), \
-        "{}: expected {}:{} actual {}:{}".format(
+        "{}: expected ownership {}:{} actual {}:{}".format(
             path, uid, gid, statinfo.st_uid, statinfo.st_gid
         )
 
@@ -219,9 +273,13 @@ class NovaStatedirOwnershipManagerTestCase(base.BaseTestCase):
     def test_no_upgrade_marker(self):
         testtree = generate_testtree1(current_uid, current_gid)
 
-        with fake_testtree(testtree) as (fake_chown, _, _, _, _):
+        with fake_testtree(testtree) as (fake_chown, _, _, _, _, _, fake_lsetfilecon):
             NovaStatedirOwnershipManager('/var/lib/nova').run()
             fake_chown.assert_not_called()
+            fake_lsetfilecon.assert_any_call('/var/lib/nova', 'newcontext')
+            fake_lsetfilecon.assert_any_call('/var/lib/nova/instances/foo', 'newcontext')
+            chcon_paths = [x[0][0] for x in fake_lsetfilecon.call_args_list]
+            self.assertNotIn('/var/lib/nova/instances/foo/bar', chcon_paths)
 
     def test_upgrade_marker_no_id_change(self):
         testtree = generate_testtree2(current_uid,
@@ -229,7 +287,7 @@ class NovaStatedirOwnershipManagerTestCase(base.BaseTestCase):
                                       current_uid,
                                       current_gid)
 
-        with fake_testtree(testtree) as (fake_chown, _, _, _, fake_unlink):
+        with fake_testtree(testtree) as (fake_chown, _, _, _, fake_unlink, _, _):
             NovaStatedirOwnershipManager('/var/lib/nova').run()
             fake_chown.assert_not_called()
             fake_unlink.assert_called_with('/var/lib/nova/upgrade_marker')
@@ -248,13 +306,17 @@ class NovaStatedirOwnershipManagerTestCase(base.BaseTestCase):
             if k == '/var/lib/nova/upgrade_marker':
                 # Ignore the marker, it should be deleted
                 continue
+            if k == '/var/lib/_nova_secontext':
+                # Ignore, outside tree
+                continue
+            v = v['stat']
             if v.st_uid == other_uid or v.st_gid == other_gid:
                 expected_changes[k] = (
                     current_uid if v.st_uid == other_uid else v.st_uid,
                     current_gid if v.st_gid == other_gid else v.st_gid
                 )
 
-        with fake_testtree(testtree) as (_, _, _, _, fake_unlink):
+        with fake_testtree(testtree) as (_, _, _, _, fake_unlink, _, _):
             NovaStatedirOwnershipManager('/var/lib/nova').run()
             for fn, expected in six.iteritems(expected_changes):
                 assert_ids(testtree, fn, expected[0], expected[1])

deployment/nova/nova-compute-container-puppet.yaml

@@ -779,16 +779,9 @@ outputs:
             privileged: false
             detach: false
             volumes:
-              list_concat:
+              - /var/lib/nova:/var/lib/nova:shared
-                # podman fails to relable if nova_nfs_enabled where we have
+              - /var/lib/_nova_secontext:/var/lib/_nova_secontext:shared,z
-                # the nfs share mounted to /var/lib/nova/instances
+              - /var/lib/container-config-scripts/:/container-config-scripts/:z
-                -
-                  if:
-                  - nova_nfs_enabled
-                  - - /var/lib/nova:/var/lib/nova:shared
-                  - - /var/lib/nova:/var/lib/nova:shared,z
-                -
-                  - /var/lib/container-config-scripts/:/container-config-scripts/:z
             command: "/container-config-scripts/pyshim.sh /container-config-scripts/nova_statedir_ownership.py"
             environment:
               # NOTE: this should force this container to re-run on each
@@ -831,13 +824,7 @@ outputs:
                       - /sys/class/net:/sys/class/net
                       - /sys/bus/pci:/sys/bus/pci
                       - /boot:/boot:ro
-                    -
+                      - /var/lib/nova:/var/lib/nova:shared
-                      # podman fails to relable if nova_nfs_enabled where we have
-                      # the nfs share mounted to /var/lib/nova/instances
-                      if:
-                      - nova_nfs_enabled
-                      - - /var/lib/nova:/var/lib/nova:shared
-                      - - /var/lib/nova:/var/lib/nova:shared,z
                     -
                       if:
                       - {equals: [{get_param: MultipathdEnable}, true]}
@@ -876,7 +863,17 @@ outputs:
       host_prep_tasks:
         list_concat:
         - {get_attr: [NovaLogging, host_prep_tasks]}
-        - - name: Mount Nova NFS Share
+        - - name: create persistent directories
+            file:
+              path: "{{ item.path }}"
+              state: directory
+              setype: "{{ item.setype }}"
+            with_items:
+              - { 'path': /var/lib/nova, 'setype': container_file_t }
+              - { 'path': /var/lib/_nova_secontext, 'setype': container_file_t}
+              - { 'path': /var/lib/nova/instances, 'setype': container_file_t }
+              - { 'path': /var/lib/libvirt, 'setype': container_file_t }
+          - name: Mount Nova NFS Share
             vars:
               nfs_backend_enable: {get_attr: [RoleParametersValue, value, nfs_backend_enable]}
               nfs_share: {get_attr: [RoleParametersValue, value, nfs_share]}
@@ -967,15 +964,6 @@ outputs:
                   name: tripleo_nova_libvirt_guests
                   enabled: yes
                   daemon_reload: yes
-          - name: create persistent directories
-            file:
-              path: "{{ item.path }}"
-              state: directory
-              setype: "{{ item.setype }}"
-            with_items:
-              - { 'path': /var/lib/nova, 'setype': container_file_t }
-              - { 'path': /var/lib/nova/instances, 'setype': container_file_t }
-              - { 'path': /var/lib/libvirt, 'setype': container_file_t }
           - name: ensure ceph configurations exist
             file:
               path: /etc/ceph

deployment/nova/nova-ironic-container-puppet.yaml

@@ -153,16 +153,9 @@ outputs:
             privileged: false
             detach: false
             volumes:
-              list_concat:
+              - /var/lib/nova:/var/lib/nova:shared
-                # podman fails to relable if nova_nfs_enabled where we have
+              - /var/lib/_nova_secontext:/var/lib/_nova_secontext:shared,z
-                # the nfs share mounted to /var/lib/nova/instances
+              - /var/lib/container-config-scripts/:/container-config-scripts/
-                -
-                  if:
-                  - nova_nfs_enabled
-                  - - /var/lib/nova:/var/lib/nova:shared
-                  - - /var/lib/nova:/var/lib/nova:shared,z
-                -
-                  - /var/lib/container-config-scripts/:/container-config-scripts/
             command: "/container-config-scripts/pyshim.sh /container-config-scripts/nova_statedir_ownership.py"
         step_5:
           nova_compute:
@@ -184,13 +177,7 @@ outputs:
                   - /dev:/dev
                   - /var/lib/iscsi:/var/lib/iscsi:z
                   - /var/log/containers/nova:/var/log/nova:z
-                -
+                  - /var/lib/nova:/var/lib/nova:shared
-                  # podman fails to relable if nova_nfs_enabled where we have
-                  # the nfs share mounted to /var/lib/nova/instances
-                  if:
-                  - nova_nfs_enabled
-                  - - /var/lib/nova:/var/lib/nova:shared
-                  - - /var/lib/nova:/var/lib/nova:shared,z
                 -
                   if:
                   - {equals: [{get_param: MultipathdEnable}, true]}
@@ -223,6 +210,7 @@ outputs:
           with_items:
             - { 'path': /var/log/containers/nova, 'setype': container_file_t, 'mode': '0750' }
             - { 'path': /var/lib/nova, 'setype': container_file_t }
+            - { 'path': /var/lib/_nova_secontext, 'setype': container_file_t}
         - name: enable virt_sandbox_use_netlink for healthcheck
           seboolean:
             name: virt_sandbox_use_netlink

deployment/nova/nova-libvirt-container-puppet.yaml

@@ -670,13 +670,7 @@ outputs:
                   - /var/lib/libvirt:/var/lib/libvirt
                   - /etc/libvirt/qemu:/etc/libvirt/qemu:ro
                   - /var/log/libvirt/qemu:/var/log/libvirt/qemu
-                # podman fails to relable if nova_nfs_enabled where we have
+                  - /var/lib/nova:/var/lib/nova:shared
-                # the nfs share mounted to /var/lib/nova/instances
-                -
-                  if:
-                  - nova_nfs_enabled
-                  - - /var/lib/nova:/var/lib/nova:shared
-                  - - /var/lib/nova:/var/lib/nova:shared,z
             environment:
               KOLLA_CONFIG_STRATEGY: COPY_ALWAYS
           nova_libvirt:
@@ -709,13 +703,7 @@ outputs:
                   - /var/lib/libvirt:/var/lib/libvirt:shared,z
                   - /var/log/libvirt/qemu:/var/log/libvirt/qemu:ro
                   - /var/lib/vhost_sockets:/var/lib/vhost_sockets:z
-                # podman fails to relable if nova_nfs_enabled where we have
+                  - /var/lib/nova:/var/lib/nova:shared
-                # the nfs share mounted to /var/lib/nova/instances
-                -
-                  if:
-                  - nova_nfs_enabled
-                  - - /var/lib/nova:/var/lib/nova:shared
-                  - - /var/lib/nova:/var/lib/nova:shared,z
                 -
                   if:
                     - docker_enabled

deployment/nova/nova-migration-target-container-puppet.yaml

@@ -177,13 +177,7 @@ outputs:
                   - /var/lib/config-data/puppet-generated/nova_libvirt:/var/lib/kolla/config_files/src:ro
                   - /etc/ssh/:/host-ssh/:ro
                   - /var/run/libvirt:/var/run/libvirt:shared,z
-                # podman fails to relable if nova_nfs_enabled where we have
+                  - /var/lib/nova:/var/lib/nova:shared
-                # the nfs share mounted to /var/lib/nova/instances
-                -
-                  if:
-                  - nova_nfs_enabled
-                  - - /var/lib/nova:/var/lib/nova:shared
-                  - - /var/lib/nova:/var/lib/nova:shared,z
             environment:
               KOLLA_CONFIG_STRATEGY: COPY_ALWAYS
       deploy_steps_tasks: