From ef1ef55dd2147a6121539e305f3b5e9339b8fdbe Mon Sep 17 00:00:00 2001 From: Gondermann Date: Wed, 9 Aug 2023 16:00:28 +0200 Subject: [PATCH] Validate provided checksum after successful import Use the 'verify_checksum' hash value in the yaml files to verify the image integrity after it has been successfully imported. Show a warning, if either the hash algorithm or the hash value does not match the expected fields. Fixes #340 Signed-off-by: Gondermann --- openstack_image_manager/manage.py | 45 +++++++++++++++++++++++++++---- test/unit/test_manage.py | 37 ++++++++++++++++++++++--- 2 files changed, 74 insertions(+), 8 deletions(-) diff --git a/openstack_image_manager/manage.py b/openstack_image_manager/manage.py index 81a2a2b4..e6151419 100644 --- a/openstack_image_manager/manage.py +++ b/openstack_image_manager/manage.py @@ -325,6 +325,9 @@ def process_images(self, images) -> set: if "build_date" in version: versions[version["version"]]["meta"]["image_build_date"] = date.isoformat(version["build_date"]) + if "verify_checksum" in version: + versions[version["version"]]["verify_checksum"] = version["verify_checksum"] + if "id" in version: versions[version["version"]]["id"] = version["id"] except ValueError as e: @@ -626,11 +629,43 @@ def process_image( if not self.CONF.dry_run: import_result = self.import_image(image, name, url, versions, version) if import_result: - logger.info( - "Import of '%s' successfully completed, reloading images" % name - ) - cloud_images = self.get_images() - imported_image = cloud_images.get(name, None) + + hashCheckSuccess = True + if "verify_checksum" in versions[version]: + hashAlgo, hashValue = versions[version]["verify_checksum"].split(":", 2) + if hashAlgo != import_result.hash_algo: + logger.warning( + "Provided verify_checksum algorithm '%s' does not equal the expected algorithm '%s'" + % (hashAlgo, import_result.hash_algo) + ) + logger.warning( + "Verification checksum for '%s' will be ignored..." + % name + ) + elif hashValue != import_result.hash_value: + logger.error( + "Provided verify_checksum for '%s' does not match backend checksum!" + % name + ) + hashCheckSuccess = False + else: + logger.info("Backend checksum matches expected value") + else: + logger.warning( + "No verification checksum for '%s'. Ignoring..." + % name + ) + + if hashCheckSuccess: + logger.info( + "Import of '%s' successfully completed, reloading images" % name + ) + cloud_images = self.get_images() + imported_image = cloud_images.get(name, None) + else: + logger.info("Deleting possibly corrupt image %s" % import_result.id) + self.conn.image.delete_image(import_result.id) + continue else: logger.info( f"Skipping required import of image '{name}', running in dry-run mode" diff --git a/test/unit/test_manage.py b/test/unit/test_manage.py index 4836fb74..fbe810cd 100644 --- a/test/unit/test_manage.py +++ b/test/unit/test_manage.py @@ -34,6 +34,7 @@ build_date: 2021-01-21 url: http://url.com checksum: '1234' + verify_checksum: 'sha512:abcd' ''' # sample image dict as generated from FAKE_YML @@ -59,7 +60,8 @@ 'build_date': date.fromisoformat("2021-01-21"), 'version': '1', 'url': 'http://url.com', - 'checksum': '1234' + 'checksum': '1234', + 'verify_checksum': 'sha512:abcd' } ] } @@ -100,7 +102,8 @@ def setUp(self): self.fake_image = Image(**FAKE_IMAGE_DATA) self.fake_name = '%s (%s)' % (self.fake_image_dict['name'], '1') self.fake_url = 'http://url.com' - self.versions = {'1': {'url': self.fake_url, 'meta': {'image_source': self.fake_url, 'image_build_date': '2021-01-21'}}} + self.fake_verify_checksum = 'sha512:abcd' + self.versions = {'1': {'url': self.fake_url, 'meta': {'image_source': self.fake_url, 'image_build_date': '2021-01-21'}, 'verify_checksum': self.fake_verify_checksum}} self.sorted_versions = ['2', '1'] self.previous_image = self.fake_image self.imported_image = self.fake_image @@ -232,6 +235,7 @@ def test_check_image_age(self, mock_read_image_files, mock_get_images): too_old_images = self.sot.check_image_age() self.assertIn(self.fake_name, too_old_images) + @mock.patch('openstack_image_manager.manage.openstack.image.v2._proxy.Proxy.delete_image') @mock.patch('openstack_image_manager.manage.ImageManager.set_properties') @mock.patch('openstack_image_manager.manage.ImageManager.import_image') @mock.patch('openstack_image_manager.manage.requests.head') @@ -239,7 +243,7 @@ def test_check_image_age(self, mock_read_image_files, mock_get_images): @mock.patch('os.path.isfile') @mock.patch('os.path.exists') def test_process_image(self, mock_path_exists, mock_path_isfile, mock_get_images, mock_requests, - mock_import_image, mock_set_properties): + mock_import_image, mock_set_properties, mock_delete_image): ''' test manage.ImageManager.process_image() ''' mock_requests.return_value.status_code = 200 @@ -249,6 +253,7 @@ def test_process_image(self, mock_path_exists, mock_path_isfile, mock_get_images self.assertEqual(mock_get_images.call_count, 2) mock_requests.assert_called_once_with(self.fake_url) + mock_delete_image.assert_not_called() mock_import_image.assert_called_once_with(self.fake_image_dict, self.fake_name, self.fake_url, @@ -259,6 +264,29 @@ def test_process_image(self, mock_path_exists, mock_path_isfile, mock_get_images mock_get_images.reset_mock() mock_requests.reset_mock() + mock_delete_image.reset_mock() + mock_import_image.reset_mock() + mock_set_properties.reset_mock() + + # test wrong checksum + mock_import_image.return_value.hash_algo = "sha512" + mock_import_image.return_value.hash_value = "wrong-checksum" + result = self.sot.process_image(self.fake_image_dict, self.versions, self.sorted_versions, meta) + + self.assertEqual(mock_get_images.call_count, 1) + mock_requests.assert_called_once_with(self.fake_url) + mock_delete_image.assert_called_once() + mock_import_image.assert_called_once_with(self.fake_image_dict, + self.fake_name, + self.fake_url, + self.versions, + '1') + mock_set_properties.assert_not_called() + self.assertEqual(result, ({self.fake_image_dict['name']}, None, None)) + + mock_get_images.reset_mock() + mock_requests.reset_mock() + mock_delete_image.reset_mock() mock_import_image.reset_mock() mock_set_properties.reset_mock() @@ -272,6 +300,7 @@ def test_process_image(self, mock_path_exists, mock_path_isfile, mock_get_images self.assertEqual(mock_get_images.call_count, 2) mock_requests.assert_not_called() + mock_delete_image.assert_not_called() mock_import_image.assert_called_once_with(self.file_image_dict, self.fake_name, self.file_url, @@ -280,6 +309,7 @@ def test_process_image(self, mock_path_exists, mock_path_isfile, mock_get_images mock_get_images.reset_mock() mock_requests.reset_mock() + mock_delete_image.reset_mock() mock_import_image.reset_mock() mock_set_properties.reset_mock() mock_path_exists.reset_mock() @@ -291,6 +321,7 @@ def test_process_image(self, mock_path_exists, mock_path_isfile, mock_get_images mock_get_images.assert_called_once() mock_requests.assert_called_once_with(self.fake_url) + mock_delete_image.assert_not_called() mock_import_image.assert_not_called() mock_set_properties.assert_not_called() self.assertEqual(result, ({self.fake_image_dict['name']}, None, None))