From 06c59972674f063a84307b2d954f9a6013d66ccb Mon Sep 17 00:00:00 2001 From: Julia Kreger Date: Tue, 18 Feb 2020 11:56:10 -0800 Subject: [PATCH] Change force_raw_images to use sha256 if md5 is selected In order to enable ironic's conductor to execute on nodes set for FIPS 140-2 compliance, we need to not explicitly choose MD5. In the case of forcing images to raw, we were calculating the checksum at least once, if not twice. Now we will honor the original algorithm unless it is MD5, at which point we will default to SHA3-256, and only recalculate the checksum once. Change-Id: I408a2e461bebf1f6d9fa3e350eb7ab1a3544adad Story: 2007306 Task: 38791 --- ironic/drivers/modules/deploy_utils.py | 25 +++++++++-------- .../unit/drivers/modules/test_deploy_utils.py | 28 ++++--------------- ...recalculation-sha256-fd3d5b4b0b757e86.yaml | 10 +++++++ 3 files changed, 30 insertions(+), 33 deletions(-) create mode 100644 releasenotes/notes/image-checksum-recalculation-sha256-fd3d5b4b0b757e86.yaml diff --git a/ironic/drivers/modules/deploy_utils.py b/ironic/drivers/modules/deploy_utils.py index 9e26e0c4b0..aab91b58bd 100644 --- a/ironic/drivers/modules/deploy_utils.py +++ b/ironic/drivers/modules/deploy_utils.py @@ -1300,19 +1300,22 @@ def build_instance_info_for_deploy(task): force_raw=force_raw) if force_raw: instance_info['image_disk_format'] = 'raw' - + # Standard behavior is for image_checksum to be MD5, + # so if the hash algorithm is None, then we will use + # sha256. + os_hash_algo = image_info.get('os_hash_algo') + if os_hash_algo == 'md5': + LOG.debug('Checksum calculation for image %(image)s is ' + 'set to \'%(algo)s\', changing to \'sha256\'', + {'algo': os_hash_algo, + 'image': image_path}) + os_hash_algo = 'sha256' LOG.debug('Recalculating checksum for image %(image)s due to ' 'image conversion.', {'image': image_path}) - md5checksum = compute_image_checksum(image_path, 'md5') - instance_info['image_checksum'] = md5checksum - # Populate instance_info with os_hash_algo, os_hash_value - # if they exists and not md5 - os_hash_algo = image_info['os_hash_algo'] - if os_hash_algo and os_hash_algo != 'md5': - hash_value = compute_image_checksum(image_path, - os_hash_algo) - instance_info['image_os_hash_algo'] = os_hash_algo - instance_info['image_os_hash_value'] = hash_value + instance_info['image_checksum'] = 'md5-not-supported' + hash_value = compute_image_checksum(image_path, os_hash_algo) + instance_info['image_os_hash_algo'] = os_hash_algo + instance_info['image_os_hash_value'] = hash_value else: instance_info['image_checksum'] = image_info['checksum'] instance_info['image_disk_format'] = image_info['disk_format'] diff --git a/ironic/tests/unit/drivers/modules/test_deploy_utils.py b/ironic/tests/unit/drivers/modules/test_deploy_utils.py index 9624286b57..f07436b533 100644 --- a/ironic/tests/unit/drivers/modules/test_deploy_utils.py +++ b/ironic/tests/unit/drivers/modules/test_deploy_utils.py @@ -2641,37 +2641,21 @@ class TestBuildInstanceInfoForHttpProvisioning(db_base.DbTestCase): image_path, instance_info = self._test_build_instance_info( image_info=self.image_info, expect_raw=True) - self.assertEqual(instance_info['image_checksum'], 'fake-checksum') + self.assertEqual('md5-not-supported', instance_info['image_checksum']) self.assertEqual(instance_info['image_disk_format'], 'raw') - calls = [mock.call(image_path, algorithm='md5'), - mock.call(image_path, algorithm='sha512')] + calls = [mock.call(image_path, algorithm='sha512')] self.checksum_mock.assert_has_calls(calls) - def test_build_instance_info_force_raw_new_fields_none(self): - cfg.CONF.set_override('force_raw_images', True) - self.image_info['os_hash_algo'] = None - self.image_info['os_hash_value'] = None - image_path, instance_info = self._test_build_instance_info( - image_info=self.image_info, expect_raw=True) - - self.assertEqual(instance_info['image_checksum'], 'fake-checksum') - self.assertEqual(instance_info['image_disk_format'], 'raw') - self.assertNotIn('image_os_hash_algo', instance_info.keys()) - self.assertNotIn('image_os_hash_value', instance_info.keys()) - self.checksum_mock.assert_called_once_with(image_path, algorithm='md5') - - def test_build_instance_info_force_raw_new_fields_is_md5(self): + def test_build_instance_info_force_raw_drops_md5(self): cfg.CONF.set_override('force_raw_images', True) self.image_info['os_hash_algo'] = 'md5' - self.image_info['os_hash_value'] = 'fake-md5' image_path, instance_info = self._test_build_instance_info( image_info=self.image_info, expect_raw=True) - self.assertEqual(instance_info['image_checksum'], 'fake-checksum') + self.assertEqual('md5-not-supported', instance_info['image_checksum']) self.assertEqual(instance_info['image_disk_format'], 'raw') - self.assertNotIn('image_os_hash_algo', instance_info.keys()) - self.assertNotIn('image_os_hash_value', instance_info.keys()) - self.checksum_mock.assert_called_once_with(image_path, algorithm='md5') + calls = [mock.call(image_path, algorithm='sha256')] + self.checksum_mock.assert_has_calls(calls) class TestStorageInterfaceUtils(db_base.DbTestCase): diff --git a/releasenotes/notes/image-checksum-recalculation-sha256-fd3d5b4b0b757e86.yaml b/releasenotes/notes/image-checksum-recalculation-sha256-fd3d5b4b0b757e86.yaml new file mode 100644 index 0000000000..1edd2a4839 --- /dev/null +++ b/releasenotes/notes/image-checksum-recalculation-sha256-fd3d5b4b0b757e86.yaml @@ -0,0 +1,10 @@ +--- +upgrade: + - | + If ``[DEFAULT]force_raw_images`` is set to ``true``, then MD5 will not be + utilized to recalculate the image checksum. This requires the + ``ironic-python-agent`` ramdisk to be at least version 3.4.0. +security: + - | + Image checksum recalculation when images are forced to raw images, are now + calculated using SHA3-256 if MD5 was selected. This is now unconditional.