From eea3ff0e1556ba69e120b6ac4ae57a57a050fbc1 Mon Sep 17 00:00:00 2001 From: Oliver Walsh Date: Wed, 25 Mar 2020 15:15:13 +0000 Subject: [PATCH] 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 (cherry picked from commit 45dd4e18a5e1609260236047fdf4dfb3d6858bd5) --- bindep.txt | 6 + .../nova_statedir_ownership.py | 48 +++++- .../tests/test_nova_statedir_ownership.py | 152 ++++++++++++------ .../nova/nova-compute-container-puppet.yaml | 42 ++--- .../nova/nova-ironic-container-puppet.yaml | 22 +-- .../nova/nova-libvirt-container-puppet.yaml | 16 +- ...ova-migration-target-container-puppet.yaml | 8 +- 7 files changed, 180 insertions(+), 114 deletions(-) diff --git a/bindep.txt b/bindep.txt index 2cfc7bb053..e379c56aab 100644 --- a/bindep.txt +++ b/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] diff --git a/container_config_scripts/nova_statedir_ownership.py b/container_config_scripts/nova_statedir_ownership.py index 177c48d13a..3c5719dbcd 100644 --- a/container_config_scripts/nova_statedir_ownership.py +++ b/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', diff --git a/container_config_scripts/tests/test_nova_statedir_ownership.py b/container_config_scripts/tests/test_nova_statedir_ownership.py index 7004c966af..39b4252227 100644 --- a/container_config_scripts/tests/test_nova_statedir_ownership.py +++ b/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 -from container_config_scripts.nova_statedir_ownership import PathManager + NovaStatedirOwnershipManager # noqa: E402 +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': - FakeStatInfo(st_mode=stat.S_IFDIR, + '/var/lib/nova': { + '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': - FakeStatInfo(st_mode=stat.S_IFDIR, - st_uid=nova_uid, - st_gid=nova_gid), - '/var/lib/nova/instances/foo': - FakeStatInfo(st_mode=stat.S_IFDIR, - st_uid=nova_uid, - st_gid=nova_gid), - '/var/lib/nova/instances/foo/bar': - FakeStatInfo(st_mode=stat.S_IFREG, - st_uid=0, - st_gid=0), - '/var/lib/nova/instances/foo/baz': - FakeStatInfo(st_mode=stat.S_IFREG, - st_uid=nova_uid, - st_gid=nova_gid), - '/var/lib/nova/instances/foo/abc': - FakeStatInfo(st_mode=stat.S_IFREG, - st_uid=0, - st_gid=nova_gid), - '/var/lib/nova/instances/foo/def': - FakeStatInfo(st_mode=stat.S_IFREG, - st_uid=nova_uid, - st_gid=0), + 'nfs': False, + }, + '/var/lib/nova/instances/foo': { + 'stat': FakeStatInfo(st_mode=stat.S_IFDIR, + st_uid=nova_uid, + st_gid=nova_gid), + 'nfs': True, + }, + '/var/lib/nova/instances/foo/bar': { + 'stat': FakeStatInfo(st_mode=stat.S_IFREG, + st_uid=0, + st_gid=0), + 'nfs': True, + }, + '/var/lib/nova/instances/foo/baz': { + 'stat': FakeStatInfo(st_mode=stat.S_IFREG, + st_uid=nova_uid, + st_gid=nova_gid), + 'nfs': True, + }, + '/var/lib/nova/instances/foo/abc': { + 'stat': FakeStatInfo(st_mode=stat.S_IFREG, + st_uid=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': - FakeStatInfo(st_mode=stat.S_IFREG, - st_uid=marker_uid, - st_gid=marker_gid) + '/var/lib/nova/upgrade_marker': { + 'stat': FakeStatInfo(st_mode=stat.S_IFREG, + st_uid=marker_uid, + 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, - fake_exists, - fake_listdir, - fake_stat, - fake_unlink) + with mock.patch( + 'selinux.lgetfilecon', + return_value=[10, 'newcontext'], + ) as fake_lgetfilecon: + 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]) diff --git a/deployment/nova/nova-compute-container-puppet.yaml b/deployment/nova/nova-compute-container-puppet.yaml index 3aa98fc01c..7bba1dafab 100644 --- a/deployment/nova/nova-compute-container-puppet.yaml +++ b/deployment/nova/nova-compute-container-puppet.yaml @@ -745,16 +745,9 @@ outputs: privileged: false detach: false volumes: - list_concat: - # 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 - - - - /var/lib/container-config-scripts/:/container-config-scripts/:z + - /var/lib/nova:/var/lib/nova:shared + - /var/lib/_nova_secontext:/var/lib/_nova_secontext: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 @@ -797,13 +790,7 @@ outputs: - /sys/class/net:/sys/class/net - /sys/bus/pci:/sys/bus/pci - /boot:/boot:ro - - - # 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 + - /var/lib/nova:/var/lib/nova:shared - if: - {equals: [{get_param: MultipathdEnable}, true]} @@ -842,7 +829,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': svirt_sandbox_file_t } + - { 'path': /var/lib/_nova_secontext, 'setype': svirt_sandbox_file_t } + - { 'path': /var/lib/nova/instances, 'setype': svirt_sandbox_file_t } + - { 'path': /var/lib/libvirt, 'setype': svirt_sandbox_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]} @@ -932,15 +929,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': svirt_sandbox_file_t } - - { 'path': /var/lib/nova/instances, 'setype': svirt_sandbox_file_t } - - { 'path': /var/lib/libvirt, 'setype': svirt_sandbox_file_t } - name: ensure ceph configurations exist file: path: /etc/ceph diff --git a/deployment/nova/nova-ironic-container-puppet.yaml b/deployment/nova/nova-ironic-container-puppet.yaml index 18147a9d2f..1f334eaa49 100644 --- a/deployment/nova/nova-ironic-container-puppet.yaml +++ b/deployment/nova/nova-ironic-container-puppet.yaml @@ -153,16 +153,9 @@ outputs: privileged: false detach: false volumes: - list_concat: - # 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 - - - - /var/lib/container-config-scripts/:/container-config-scripts/ + - /var/lib/nova:/var/lib/nova:shared + - /var/lib/_nova_secontext:/var/lib/_nova_secontext: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 - - - # 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 + - /var/lib/nova:/var/lib/nova:shared - if: - {equals: [{get_param: MultipathdEnable}, true]} @@ -223,6 +210,7 @@ outputs: with_items: - { 'path': /var/log/containers/nova, 'setype': svirt_sandbox_file_t, 'mode': '0750' } - { 'path': /var/lib/nova, 'setype': svirt_sandbox_file_t } + - { 'path': /var/lib/_nova_secontext, 'setype': svirt_sandbox_file_t} - name: enable virt_sandbox_use_netlink for healthcheck seboolean: name: virt_sandbox_use_netlink diff --git a/deployment/nova/nova-libvirt-container-puppet.yaml b/deployment/nova/nova-libvirt-container-puppet.yaml index 7611d8157d..4e02e66894 100644 --- a/deployment/nova/nova-libvirt-container-puppet.yaml +++ b/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 - # 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 + - /var/lib/nova:/var/lib/nova:shared environment: KOLLA_CONFIG_STRATEGY: COPY_ALWAYS nova_libvirt: @@ -710,13 +704,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 - # 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 + - /var/lib/nova:/var/lib/nova:shared - if: - docker_enabled diff --git a/deployment/nova/nova-migration-target-container-puppet.yaml b/deployment/nova/nova-migration-target-container-puppet.yaml index bab4311bba..709c07f617 100644 --- a/deployment/nova/nova-migration-target-container-puppet.yaml +++ b/deployment/nova/nova-migration-target-container-puppet.yaml @@ -162,13 +162,7 @@ outputs: - /var/lib/config-data/puppet-generated/nova_libvirt:/var/lib/kolla/config_files/src:ro - /etc/ssh/:/host-ssh/:ro - /run/libvirt:/run/libvirt - # 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 + - /var/lib/nova:/var/lib/nova:shared environment: KOLLA_CONFIG_STRATEGY: COPY_ALWAYS deploy_steps_tasks: