From 728301fc2faeaaef737885966552c9139e2628d8 Mon Sep 17 00:00:00 2001 From: Riccardo Pittau Date: Sat, 24 Dec 2022 11:49:23 +0100 Subject: Fix selinux context of published image hardlink If the published image is a hardlink, the source selinux context is preserved. This could cause access denied when retrieving the image using its URL. Change-Id: I550dac9d055ec30ec11530f18a675cf9e16063b5 (cherry picked from commit c05c09fd3ac7ed2c3a5dd13a602e3ae70dfb8734) --- ironic/drivers/modules/image_utils.py | 10 +++++ .../tests/unit/drivers/modules/test_image_utils.py | 47 ++++++++++++++++------ ...ix-context-image-hardlink-16f452974abc7327.yaml | 7 ++++ 3 files changed, 52 insertions(+), 12 deletions(-) create mode 100644 releasenotes/notes/fix-context-image-hardlink-16f452974abc7327.yaml diff --git a/ironic/drivers/modules/image_utils.py b/ironic/drivers/modules/image_utils.py index 304c199bf..86607ee25 100644 --- a/ironic/drivers/modules/image_utils.py +++ b/ironic/drivers/modules/image_utils.py @@ -211,6 +211,16 @@ class ImageHandler(object): try: os.link(image_file, published_file) os.chmod(image_file, self._file_permission) + try: + utils.execute( + '/usr/sbin/restorecon', '-i', '-R', 'v', public_dir) + except FileNotFoundError as exc: + LOG.debug( + "Could not restore SELinux context on " + "%(public_dir)s, restorecon command not found.\n" + "Error: %(error)s", + {'public_dir': public_dir, + 'error': exc}) except OSError as exc: LOG.debug( diff --git a/ironic/tests/unit/drivers/modules/test_image_utils.py b/ironic/tests/unit/drivers/modules/test_image_utils.py index 753452f5d..b6c572125 100644 --- a/ironic/tests/unit/drivers/modules/test_image_utils.py +++ b/ironic/tests/unit/drivers/modules/test_image_utils.py @@ -105,73 +105,96 @@ class RedfishImageHandlerTestCase(db_base.DbTestCase): mock_swift_api.delete_object.assert_called_once_with( 'ironic_redfish_container', object_name) + @mock.patch.object(utils, 'execute', autospec=True) @mock.patch.object(os, 'chmod', autospec=True) @mock.patch.object(image_utils, 'shutil', autospec=True) @mock.patch.object(os, 'link', autospec=True) @mock.patch.object(os, 'mkdir', autospec=True) def test_publish_image_local_link( - self, mock_mkdir, mock_link, mock_shutil, mock_chmod): + self, mock_mkdir, mock_link, mock_shutil, mock_chmod, + mock_execute): self.config(use_swift=False, group='redfish') self.config(http_url='http://localhost', group='deploy') img_handler_obj = image_utils.ImageHandler(self.node.driver) - url = img_handler_obj.publish_image('file.iso', 'boot.iso') - self.assertEqual( 'http://localhost/redfish/boot.iso', url) + mock_mkdir.assert_called_once_with('/httpboot/redfish', 0o755) + mock_link.assert_called_once_with( + 'file.iso', '/httpboot/redfish/boot.iso') + mock_chmod.assert_called_once_with('file.iso', 0o644) + mock_execute.assert_called_once_with( + '/usr/sbin/restorecon', '-i', '-R', 'v', '/httpboot/redfish') + @mock.patch.object(utils, 'execute', autospec=True) + @mock.patch.object(os, 'chmod', autospec=True) + @mock.patch.object(image_utils, 'shutil', autospec=True) + @mock.patch.object(os, 'link', autospec=True) + @mock.patch.object(os, 'mkdir', autospec=True) + def test_publish_image_local_link_no_restorecon( + self, mock_mkdir, mock_link, mock_shutil, mock_chmod, + mock_execute): + self.config(use_swift=False, group='redfish') + self.config(http_url='http://localhost', group='deploy') + img_handler_obj = image_utils.ImageHandler(self.node.driver) + url = img_handler_obj.publish_image('file.iso', 'boot.iso') + self.assertEqual( + 'http://localhost/redfish/boot.iso', url) mock_mkdir.assert_called_once_with('/httpboot/redfish', 0o755) mock_link.assert_called_once_with( 'file.iso', '/httpboot/redfish/boot.iso') mock_chmod.assert_called_once_with('file.iso', 0o644) + mock_execute.return_value = FileNotFoundError + mock_shutil.assert_not_called() + @mock.patch.object(utils, 'execute', autospec=True) @mock.patch.object(os, 'chmod', autospec=True) @mock.patch.object(image_utils, 'shutil', autospec=True) @mock.patch.object(os, 'link', autospec=True) @mock.patch.object(os, 'mkdir', autospec=True) def test_publish_image_external_ip( - self, mock_mkdir, mock_link, mock_shutil, mock_chmod): + self, mock_mkdir, mock_link, mock_shutil, mock_chmod, + mock_execute): self.config(use_swift=False, group='redfish') self.config(http_url='http://localhost', external_http_url='http://non-local.host', group='deploy') img_handler_obj = image_utils.ImageHandler(self.node.driver) - url = img_handler_obj.publish_image('file.iso', 'boot.iso') - self.assertEqual( 'http://non-local.host/redfish/boot.iso', url) - mock_mkdir.assert_called_once_with('/httpboot/redfish', 0o755) mock_link.assert_called_once_with( 'file.iso', '/httpboot/redfish/boot.iso') mock_chmod.assert_called_once_with('file.iso', 0o644) + mock_execute.assert_called_once_with( + '/usr/sbin/restorecon', '-i', '-R', 'v', '/httpboot/redfish') + @mock.patch.object(utils, 'execute', autospec=True) @mock.patch.object(os, 'chmod', autospec=True) @mock.patch.object(image_utils, 'shutil', autospec=True) @mock.patch.object(os, 'link', autospec=True) @mock.patch.object(os, 'mkdir', autospec=True) def test_publish_image_external_ip_node_override( - self, mock_mkdir, mock_link, mock_shutil, mock_chmod): + self, mock_mkdir, mock_link, mock_shutil, mock_chmod, + mock_execute): self.config(use_swift=False, group='redfish') self.config(http_url='http://localhost', external_http_url='http://non-local.host', group='deploy') img_handler_obj = image_utils.ImageHandler(self.node.driver) self.node.driver_info["external_http_url"] = "http://node.override.url" - override_url = self.node.driver_info.get("external_http_url") - url = img_handler_obj.publish_image('file.iso', 'boot.iso', override_url) - self.assertEqual( 'http://node.override.url/redfish/boot.iso', url) - mock_mkdir.assert_called_once_with('/httpboot/redfish', 0o755) mock_link.assert_called_once_with( 'file.iso', '/httpboot/redfish/boot.iso') mock_chmod.assert_called_once_with('file.iso', 0o644) + mock_execute.assert_called_once_with( + '/usr/sbin/restorecon', '-i', '-R', 'v', '/httpboot/redfish') @mock.patch.object(os, 'chmod', autospec=True) @mock.patch.object(image_utils, 'shutil', autospec=True) diff --git a/releasenotes/notes/fix-context-image-hardlink-16f452974abc7327.yaml b/releasenotes/notes/fix-context-image-hardlink-16f452974abc7327.yaml new file mode 100644 index 000000000..90d38d5cc --- /dev/null +++ b/releasenotes/notes/fix-context-image-hardlink-16f452974abc7327.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + Fixes an issue where if selinux is enabled and enforcing, and + the published image is a hardlink, the source selinux context + is preserved, causing access denied when retrieving the image + using hardlink URL. -- cgit v1.2.1