Skip to content

Commit

Permalink
Validate provided checksum after successful import
Browse files Browse the repository at this point in the history 
Use the '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 osism#340

Signed-off-by: Gondermann <[email protected]>
  • Loading branch information
@gndrmnn
gndrmnn committed Aug 31, 2023
1 parent e0cdb3d commit 2be165f
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 9 deletions.
39 changes: 34 additions & 5 deletions openstack_image_manager/manage.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,8 @@ def process_images(self, images) -> set:
versions[version["version"]]["meta"][
"image_build_date"
] = version["build_date"]
if "checksum" in version:
versions[version["version"]]["checksum"] = version["checksum"]
if "id" in version:
versions[version["version"]]["id"] = version["id"]
except Exception:
Expand Down Expand Up @@ -627,11 +629,38 @@ 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 "checksum" in versions[version]:
hashAlgo, hashValue = versions[version]["checksum"].split(":", 2)
if hashAlgo != import_result.hash_algo:
logger.warning(
"Provided checksum algorithm '%s' does not equal the expected algorithm '%s'"
% (hashAlgo, import_result.hash_algo)
)
logger.warning(
"Checksum for '%s' will be ignored..."
% name
)
elif hashValue != import_result.hash_value:
logger.error(
"Provided checksum for '%s' does not match backend checksum!"
% name
)
hashCheckSuccess = False
else:
logger.info("Backend checksum matches expected value")

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"
Expand Down
37 changes: 33 additions & 4 deletions test/unit/test_manage.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
- version: '1'
build_date: '2021-01-21'
url: http://url.com
checksum: '1234'
checksum: 'sha512:1234'
'''

# sample image dict as generated from FAKE_YML
Expand All @@ -59,7 +59,7 @@
'build_date': '2021-01-21',
'version': '1',
'url': 'http://url.com',
'checksum': '1234'
'checksum': 'sha512:1234'
}
]
}
Expand Down Expand Up @@ -100,7 +100,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_checksum = 'sha512:1234'
self.versions = {'1': {'url': self.fake_url, 'meta': {'image_source': self.fake_url, 'image_build_date': '2021-01-21'}, 'checksum': self.fake_checksum}}
self.sorted_versions = ['2', '1']
self.previous_image = self.fake_image
self.imported_image = self.fake_image
Expand Down Expand Up @@ -232,14 +233,15 @@ 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')
@mock.patch('openstack_image_manager.manage.ImageManager.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
Expand All @@ -249,6 +251,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,
Expand All @@ -259,6 +262,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()

Expand All @@ -272,6 +298,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,
Expand All @@ -280,6 +307,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()
Expand All @@ -291,6 +319,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))
Expand Down

0 comments on commit 2be165f

Please sign in to comment.