summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorZuul <zuul@review.opendev.org>2023-05-02 06:41:25 +0000
committerGerrit Code Review <review@openstack.org>2023-05-02 06:41:25 +0000
commitf37ea85a2765a9fac580dc031bcf16b3a4e45d98 (patch)
treec768451306a8281e627ebce64a4d35e28062055c
parent3cd8c294fbad24b9cfd1d0b7ae5aeaf5629f5168 (diff)
parent32df26a22ad79fd9b69f6ae994a0d694ad03c5c0 (diff)
downloadironic-python-agent-f37ea85a2765a9fac580dc031bcf16b3a4e45d98.tar.gz
Merge "Disable MD5 image checksums"
-rw-r--r--doc/source/admin/how_it_works.rst33
-rw-r--r--ironic_python_agent/agent.py2
-rw-r--r--ironic_python_agent/config.py3
-rw-r--r--ironic_python_agent/extensions/standby.py102
-rw-r--r--ironic_python_agent/tests/unit/extensions/test_standby.py374
-rw-r--r--releasenotes/notes/disable-md5-image-checksum-7def176928d36e75.yaml19
6 files changed, 456 insertions, 77 deletions
diff --git a/doc/source/admin/how_it_works.rst b/doc/source/admin/how_it_works.rst
index 5f4a6773..20a5f477 100644
--- a/doc/source/admin/how_it_works.rst
+++ b/doc/source/admin/how_it_works.rst
@@ -223,3 +223,36 @@ fields:
.. note::
This is most likely to be set by the DHCP server. Could be localhost
if the DHCP server does not set it.
+
+Image Checksums
+---------------
+
+As part of the process of downloading images to be written to disk as part of
+image deployment, a series of fields are utilized to determine if the
+image which has been downloaded matches what the user stated as the expected
+image checksum utilizing the ``instance_info/image_checksum`` value.
+
+OpenStack, as a whole, has replaced the "legacy" ``checksum`` field with
+``os_hash_value`` and ``os_hash_algo`` fields, which allows for an image
+checksum and value to be asserted. An advantage of this is a variety of
+algorithms are available, if a user/operator is so-inclined.
+
+For the purposes of Ironic, we continue to support the pass-through checksum
+field as we support the checksum being retrieved via a URL.
+
+We also support determining the checksum by length.
+
+The field can be utilized to designate:
+
+* A URL to retreive a checksum from.
+* MD5 (Disabled by default, see ``[DEFAULT]md5_enabled`` in the agent
+ configuration file.)
+* SHA-2 based SHA256
+* SHA-2 based SHA512
+
+SHA-3 based checksums are not supported for auto-determination as they can
+have a variable length checksum result. At of when this documentation was
+added, SHA-2 based checksum algorithms have not been withdrawn from from
+approval. If you need to force use of SHA-3 based checksums, you *must*
+utilize the ``os_hash_algo`` setting along with the ``os_hash_value``
+setting.
diff --git a/ironic_python_agent/agent.py b/ironic_python_agent/agent.py
index c5b24560..3fad86f3 100644
--- a/ironic_python_agent/agent.py
+++ b/ironic_python_agent/agent.py
@@ -408,6 +408,8 @@ class IronicPythonAgent(base.ExecuteCommandMixin):
if config.get('metrics_statsd'):
for opt, val in config.items():
setattr(cfg.CONF.metrics_statsd, opt, val)
+ if config.get('agent_md5_checksum_enable'):
+ cfg.set_override('md5_enabled', True)
if config.get('agent_token_required'):
self.agent_token_required = True
token = config.get('agent_token')
diff --git a/ironic_python_agent/config.py b/ironic_python_agent/config.py
index 056cab85..5c5de305 100644
--- a/ironic_python_agent/config.py
+++ b/ironic_python_agent/config.py
@@ -328,6 +328,9 @@ cli_opts = [
'cleaning from inadvertently destroying a running '
'cluster which may be visible over a storage fabric '
'such as FibreChannel.'),
+ cfg.BoolOpt('md5_enabled',
+ default=False,
+ help='If the MD5 algorithm is enabled for file checksums.'),
]
CONF.register_cli_opts(cli_opts)
diff --git a/ironic_python_agent/extensions/standby.py b/ironic_python_agent/extensions/standby.py
index a78056cd..965ce1ef 100644
--- a/ironic_python_agent/extensions/standby.py
+++ b/ironic_python_agent/extensions/standby.py
@@ -99,9 +99,17 @@ def _download_with_proxy(image_info, url, image_id):
return resp
+def _is_checksum_url(checksum):
+ """Identify if checksum is not a url"""
+ if (checksum.startswith('http://') or checksum.startswith('https://')):
+ return True
+ else:
+ return False
+
+
def _fetch_checksum(checksum, image_info):
"""Fetch checksum from remote location, if needed."""
- if not (checksum.startswith('http://') or checksum.startswith('https://')):
+ if not _is_checksum_url(checksum):
# Not a remote checksum, return as it is.
return checksum
@@ -263,6 +271,47 @@ def _message_format(msg, image_info, device, partition_uuids):
return message
+def _get_algorithm_by_length(checksum):
+ """Determine the SHA-2 algorithm by checksum length.
+
+ :param checksum: The requested checksum.
+ :returns: A hashlib object based upon the checksum
+ or ValueError if the algorthm could not be
+ identified.
+ """
+ # NOTE(TheJulia): This is all based on SHA-2 lengths.
+ # SHA-3 would require a hint, thus ValueError because
+ # it may not be a fixed length. That said, SHA-2 is not
+ # as of this not being added, being withdrawn standards wise.
+ checksum_len = len(checksum)
+ if checksum_len == 128:
+ # Sha512 is 512 bits, or 128 characters
+ return hashlib.new('sha512')
+ elif checksum_len == 64:
+ # SHA256 is 256 bits, or 64 characters
+ return hashlib.new('sha256')
+ elif checksum_len == 32:
+ check_md5_enabled()
+ # This is not super great, but opt-in only.
+ return hashlib.new('md5') # nosec
+ else:
+ # Previously, we would have just assumed the value was
+ # md5 by default. This way we are a little smarter and
+ # gracefully handle things better when md5 is explicitly
+ # disabled.
+ raise ValueError('Unable to identify checksum algorithm '
+ 'used, and a value is not specified in '
+ 'the os_hash_algo setting.')
+
+
+def check_md5_enabled():
+ """Checks if md5 is permitted, otherwise raises ValueError."""
+ if not CONF.md5_enabled:
+ raise ValueError('MD5 support is disabled, and support '
+ 'will be removed in a 2024 version of '
+ 'Ironic.')
+
+
class ImageDownload(object):
"""Helper class that opens a HTTP connection to download an image.
@@ -292,6 +341,8 @@ class ImageDownload(object):
self._time = time_obj or time.time()
self._image_info = image_info
self._request = None
+ checksum = image_info.get('checksum')
+ retrieved_checksum = False
# Determine the hash algorithm and value will be used for calculation
# and verification, fallback to md5 if algorithm is not set or not
@@ -300,18 +351,37 @@ class ImageDownload(object):
if algo and algo in hashlib.algorithms_available:
self._hash_algo = hashlib.new(algo)
self._expected_hash_value = image_info.get('os_hash_value')
- elif image_info.get('checksum'):
+ elif checksum and _is_checksum_url(checksum):
+ # Treat checksum urls as first class request citizens, else
+ # fallback to legacy handling.
+ self._expected_hash_value = _fetch_checksum(
+ checksum,
+ image_info)
+ retrieved_checksum = True
+ if not algo:
+ # Override algorithm not suppied as os_hash_algo
+ self._hash_algo = _get_algorithm_by_length(
+ self._expected_hash_value)
+ elif checksum:
+ # Fallback to md5 path.
try:
- self._hash_algo = hashlib.md5()
+ new_algo = _get_algorithm_by_length(checksum)
+
+ if not new_algo:
+ # Realistically, this should never happen, but for
+ # compatability...
+ # TODO(TheJulia): Remove for a 2024 release.
+ self._hash_algo = hashlib.new('md5')
+ else:
+ self._hash_algo = new_algo
except ValueError as e:
- message = ('Unable to proceed with image {} as the legacy '
- 'checksum indicator has been used, which makes use '
- 'the MD5 algorithm. This algorithm failed to load '
- 'due to the underlying operating system. Error: '
+ message = ('Unable to proceed with image {} as the '
+ 'checksum indicator has been used but the '
+ 'algorithm could not be identified. Error: '
'{}').format(image_info['id'], str(e))
LOG.error(message)
raise errors.RESTError(details=message)
- self._expected_hash_value = image_info['checksum']
+ self._expected_hash_value = checksum
else:
message = ('Unable to verify image {} with available checksums. '
'Please make sure the specified \'os_hash_algo\' '
@@ -322,8 +392,12 @@ class ImageDownload(object):
LOG.error(message)
raise errors.RESTError(details=message)
- self._expected_hash_value = _fetch_checksum(self._expected_hash_value,
- image_info)
+ if not retrieved_checksum:
+ # Fallback to retrieve the checksum if we didn't retrieve it
+ # earlier on.
+ self._expected_hash_value = _fetch_checksum(
+ self._expected_hash_value,
+ image_info)
details = []
for url in image_info['urls']:
@@ -363,7 +437,10 @@ class ImageDownload(object):
# this code.
if chunk:
self._last_chunk_time = time.time()
- self._hash_algo.update(chunk)
+ if isinstance(chunk, str):
+ self._hash_algo.update(chunk.encode())
+ else:
+ self._hash_algo.update(chunk)
yield chunk
elif (time.time() - self._last_chunk_time
> CONF.image_download_connection_timeout):
@@ -476,7 +553,8 @@ def _validate_image_info(ext, image_info=None, **kwargs):
or not image_info['checksum']):
raise errors.InvalidCommandParamsError(
'Image \'checksum\' must be a non-empty string.')
- md5sum_avail = True
+ if CONF.md5_enabled:
+ md5sum_avail = True
os_hash_algo = image_info.get('os_hash_algo')
os_hash_value = image_info.get('os_hash_value')
diff --git a/ironic_python_agent/tests/unit/extensions/test_standby.py b/ironic_python_agent/tests/unit/extensions/test_standby.py
index ccfecc01..fee5ad30 100644
--- a/ironic_python_agent/tests/unit/extensions/test_standby.py
+++ b/ironic_python_agent/tests/unit/extensions/test_standby.py
@@ -19,6 +19,7 @@ from unittest import mock
from ironic_lib import exception
from oslo_concurrency import processutils
+from oslo_config import cfg
import requests
from ironic_python_agent import errors
@@ -29,13 +30,17 @@ from ironic_python_agent.tests.unit import base
from ironic_python_agent import utils
+CONF = cfg.CONF
+
+
def _build_fake_image_info(url='http://example.org'):
return {
'id': 'fake_id',
'node_uuid': '1be26c0b-03f2-4d2e-ae87-c02d7f33c123',
'urls': [url],
- 'checksum': 'abc123',
'image_type': 'whole-disk-image',
+ 'os_hash_algo': 'sha256',
+ 'os_hash_value': 'fake-checksum',
}
@@ -46,7 +51,6 @@ def _build_fake_partition_image_info():
'http://example.org',
],
'node_uuid': 'node_uuid',
- 'checksum': 'abc123',
'root_mb': '10',
'swap_mb': '10',
'ephemeral_mb': '10',
@@ -54,7 +58,9 @@ def _build_fake_partition_image_info():
'preserve_ephemeral': 'False',
'image_type': 'partition',
'disk_label': 'msdos',
- 'deploy_boot_mode': 'bios'}
+ 'deploy_boot_mode': 'bios',
+ 'os_hash_algo': 'sha256',
+ 'os_hash_value': 'fake-checksum'}
class TestStandbyExtension(base.IronicAgentTest):
@@ -83,7 +89,6 @@ class TestStandbyExtension(base.IronicAgentTest):
def test_validate_image_info_success_without_md5(self):
image_info = _build_fake_image_info()
- del image_info['checksum']
image_info['os_hash_algo'] = 'sha512'
image_info['os_hash_value'] = 'fake-checksum'
standby._validate_image_info(None, image_info)
@@ -95,8 +100,27 @@ class TestStandbyExtension(base.IronicAgentTest):
image_info['os_hash_value'] = 'fake-checksum'
standby._validate_image_info(None, image_info)
+ def test_validate_image_info_legacy_md5_checksum_enabled(self):
+ image_info = _build_fake_image_info()
+ CONF.set_override('md5_enabled', True)
+ image_info['checksum'] = 'fake-checksum'
+ del image_info['os_hash_algo']
+ del image_info['os_hash_value']
+ standby._validate_image_info(None, image_info)
+
+ def test_validate_image_info_legacy_md5_checksum(self):
+ image_info = _build_fake_image_info()
+ del image_info['os_hash_algo']
+ del image_info['os_hash_value']
+ image_info['checksum'] = 'fake-checksum'
+ self.assertRaisesRegex(errors.InvalidCommandParamsError,
+ 'Image checksum is not',
+ standby._validate_image_info,
+ None,
+ image_info)
+
def test_validate_image_info_missing_field(self):
- for field in ['id', 'urls', 'checksum']:
+ for field in ['id', 'urls', 'os_hash_value']:
invalid_info = _build_fake_image_info()
del invalid_info[field]
@@ -373,10 +397,10 @@ class TestStandbyExtension(base.IronicAgentTest):
self.assertEqual(expected_uuid, work_on_disk_mock.return_value)
- @mock.patch('hashlib.md5', autospec=True)
+ @mock.patch('hashlib.new', autospec=True)
@mock.patch('builtins.open', autospec=True)
@mock.patch('requests.get', autospec=True)
- def test_download_image(self, requests_mock, open_mock, md5_mock):
+ def test_download_image(self, requests_mock, open_mock, hash_mock):
image_info = _build_fake_image_info()
response = requests_mock.return_value
response.status_code = 200
@@ -384,8 +408,8 @@ class TestStandbyExtension(base.IronicAgentTest):
file_mock = mock.Mock()
open_mock.return_value.__enter__.return_value = file_mock
file_mock.read.return_value = None
- hexdigest_mock = md5_mock.return_value.hexdigest
- hexdigest_mock.return_value = image_info['checksum']
+ hexdigest_mock = hash_mock.return_value.hexdigest
+ hexdigest_mock.return_value = image_info['os_hash_value']
standby._download_image(image_info)
requests_mock.assert_called_once_with(image_info['urls'][0],
@@ -397,26 +421,27 @@ class TestStandbyExtension(base.IronicAgentTest):
write.assert_any_call('content')
self.assertEqual(2, write.call_count)
- @mock.patch('hashlib.md5', autospec=True)
+ @mock.patch('hashlib.new', autospec=True)
@mock.patch('builtins.open', autospec=True)
@mock.patch('requests.get', autospec=True)
@mock.patch.dict(os.environ, {})
def test_download_image_proxy(
- self, requests_mock, open_mock, md5_mock):
+ self, requests_mock, open_mock, hash_mock):
image_info = _build_fake_image_info()
proxies = {'http': 'http://a.b.com',
'https': 'https://secure.a.b.com'}
no_proxy = '.example.org,.b.com'
image_info['proxies'] = proxies
image_info['no_proxy'] = no_proxy
+ image_info['os_hash_value'] = 'fake-checksum'
response = requests_mock.return_value
response.status_code = 200
response.iter_content.return_value = ['some', 'content']
file_mock = mock.Mock()
open_mock.return_value.__enter__.return_value = file_mock
file_mock.read.return_value = None
- hexdigest_mock = md5_mock.return_value.hexdigest
- hexdigest_mock.return_value = image_info['checksum']
+ hexdigest_mock = hash_mock.return_value.hexdigest
+ hexdigest_mock.return_value = 'fake-checksum'
standby._download_image(image_info)
self.assertEqual(no_proxy, os.environ['no_proxy'])
@@ -428,6 +453,11 @@ class TestStandbyExtension(base.IronicAgentTest):
write.assert_any_call('some')
write.assert_any_call('content')
self.assertEqual(2, write.call_count)
+ hash_mock.assert_has_calls([
+ mock.call('sha256'),
+ mock.call().update(b'some'),
+ mock.call().update(b'content'),
+ mock.call().hexdigest()])
@mock.patch('requests.get', autospec=True)
def test_download_image_bad_status(self, requests_mock):
@@ -439,29 +469,29 @@ class TestStandbyExtension(base.IronicAgentTest):
standby._download_image,
image_info)
- @mock.patch('hashlib.md5', autospec=True)
+ @mock.patch('hashlib.new', autospec=True)
@mock.patch('builtins.open', autospec=True)
@mock.patch('requests.get', autospec=True)
def test_download_image_verify_fails(self, requests_mock, open_mock,
- md5_mock):
+ hash_mock):
image_info = _build_fake_image_info()
response = requests_mock.return_value
response.status_code = 200
- hexdigest_mock = md5_mock.return_value.hexdigest
+ hexdigest_mock = hash_mock.return_value.hexdigest
hexdigest_mock.return_value = 'invalid-checksum'
self.assertRaises(errors.ImageChecksumError,
standby._download_image,
image_info)
- @mock.patch('hashlib.md5', autospec=True)
+ @mock.patch('hashlib.new', autospec=True)
@mock.patch('builtins.open', autospec=True)
@mock.patch('requests.get', autospec=True)
- def test_verify_image_success(self, requests_mock, open_mock, md5_mock):
+ def test_verify_image_success(self, requests_mock, open_mock, hash_mock):
image_info = _build_fake_image_info()
response = requests_mock.return_value
response.status_code = 200
- hexdigest_mock = md5_mock.return_value.hexdigest
- hexdigest_mock.return_value = image_info['checksum']
+ hexdigest_mock = hash_mock.return_value.hexdigest
+ hexdigest_mock.return_value = image_info['os_hash_value']
image_location = '/foo/bar'
image_download = standby.ImageDownload(image_info)
image_download.verify_image(image_location)
@@ -490,7 +520,6 @@ class TestStandbyExtension(base.IronicAgentTest):
def test_verify_image_success_without_md5(self, requests_mock,
open_mock, hashlib_mock):
image_info = _build_fake_image_info()
- del image_info['checksum']
image_info['os_hash_algo'] = 'sha512'
image_info['os_hash_value'] = 'fake-sha512-value'
response = requests_mock.return_value
@@ -502,21 +531,46 @@ class TestStandbyExtension(base.IronicAgentTest):
image_download.verify_image(image_location)
hashlib_mock.assert_called_with('sha512')
- @mock.patch('hashlib.md5', autospec=True)
+ @mock.patch('hashlib.new', autospec=True)
@mock.patch('builtins.open', autospec=True)
@mock.patch('requests.get', autospec=True)
def test_verify_image_success_with_md5_fallback(self, requests_mock,
- open_mock, md5_mock):
+ open_mock, hash_mock):
+ CONF.set_override('md5_enabled', True)
image_info = _build_fake_image_info()
image_info['os_hash_algo'] = 'algo-beyond-milky-way'
image_info['os_hash_value'] = 'mysterious-alien-codes'
+ image_info['checksum'] = 'd41d8cd98f00b204e9800998ecf8427e'
response = requests_mock.return_value
response.status_code = 200
- hexdigest_mock = md5_mock.return_value.hexdigest
+ hexdigest_mock = hash_mock.return_value.hexdigest
hexdigest_mock.return_value = image_info['checksum']
image_location = '/foo/bar'
image_download = standby.ImageDownload(image_info)
image_download.verify_image(image_location)
+ # NOTE(TheJulia): This is the one test which falls all the
+ # way back to md5 as the default, legacy logic because it
+ # got bad input to start with.
+ hash_mock.assert_has_calls([
+ mock.call('md5'),
+ mock.call().__bool__(),
+ mock.call().hexdigest()])
+
+ @mock.patch('hashlib.new', autospec=True)
+ @mock.patch('builtins.open', autospec=True)
+ @mock.patch('requests.get', autospec=True)
+ def test_verify_image_fails_if_unknown_is_used(self, requests_mock,
+ open_mock, hash_mock):
+ image_info = _build_fake_image_info()
+ image_info['os_hash_algo'] = 'algo-beyond-milky-way'
+ image_info['os_hash_value'] = 'mysterious-alien-codes'
+ self.assertRaisesRegex(
+ errors.RESTError,
+ 'An error occurred: Unable to verify image fake_id with '
+ 'available checksums.',
+ standby.ImageDownload,
+ image_info)
+ hash_mock.assert_not_called()
@mock.patch('hashlib.new', autospec=True)
@mock.patch('builtins.open', autospec=True)
@@ -538,16 +592,16 @@ class TestStandbyExtension(base.IronicAgentTest):
image_location)
hashlib_mock.assert_called_with('sha512')
- @mock.patch('hashlib.md5', autospec=True)
+ @mock.patch('hashlib.new', autospec=True)
@mock.patch('builtins.open', autospec=True)
@mock.patch('requests.get', autospec=True)
- def test_verify_image_failure(self, requests_mock, open_mock, md5_mock):
+ def test_verify_image_failure(self, requests_mock, open_mock, hash_mock):
image_info = _build_fake_image_info()
response = requests_mock.return_value
response.status_code = 200
image_download = standby.ImageDownload(image_info)
image_location = '/foo/bar'
- hexdigest_mock = md5_mock.return_value.hexdigest
+ hexdigest_mock = hash_mock.return_value.hexdigest
hexdigest_mock.return_value = 'invalid-checksum'
self.assertRaises(errors.ImageChecksumError,
image_download.verify_image,
@@ -559,7 +613,6 @@ class TestStandbyExtension(base.IronicAgentTest):
def test_verify_image_failure_without_fallback(self, requests_mock,
open_mock, hashlib_mock):
image_info = _build_fake_image_info()
- del image_info['checksum']
image_info['os_hash_algo'] = 'unsupported-algorithm'
image_info['os_hash_value'] = 'fake-value'
response = requests_mock.return_value
@@ -1201,11 +1254,11 @@ class TestStandbyExtension(base.IronicAgentTest):
@mock.patch('ironic_lib.disk_utils.block_uuid', autospec=True)
@mock.patch('ironic_lib.disk_utils.fix_gpt_partition', autospec=True)
- @mock.patch('hashlib.md5', autospec=True)
+ @mock.patch('hashlib.new', autospec=True)
@mock.patch('builtins.open', autospec=True)
@mock.patch('requests.get', autospec=True)
def test_stream_raw_image_onto_device(self, requests_mock, open_mock,
- md5_mock, fix_gpt_mock,
+ hash_mock, fix_gpt_mock,
block_uuid_mock):
image_info = _build_fake_image_info()
response = requests_mock.return_value
@@ -1214,14 +1267,19 @@ class TestStandbyExtension(base.IronicAgentTest):
file_mock = mock.Mock()
open_mock.return_value.__enter__.return_value = file_mock
file_mock.read.return_value = None
- hexdigest_mock = md5_mock.return_value.hexdigest
- hexdigest_mock.return_value = image_info['checksum']
+ hexdigest_mock = hash_mock.return_value.hexdigest
+ hexdigest_mock.return_value = image_info['os_hash_value']
self.agent_extension.partition_uuids = {}
block_uuid_mock.return_value = 'aaaabbbb'
-
self.agent_extension._stream_raw_image_onto_device(image_info,
'/dev/foo')
+ hash_mock.assert_has_calls([
+ mock.call('sha256'),
+ mock.call().update(b'some'),
+ mock.call().update(b'content'),
+ mock.call().hexdigest()])
+
requests_mock.assert_called_once_with(image_info['urls'][0],
cert=None, verify=True,
stream=True, proxies={},
@@ -1235,22 +1293,22 @@ class TestStandbyExtension(base.IronicAgentTest):
self.agent_extension.partition_uuids['root uuid']
)
- @mock.patch('hashlib.md5', autospec=True)
+ @mock.patch('hashlib.new', autospec=True)
@mock.patch('builtins.open', autospec=True)
@mock.patch('requests.get', autospec=True)
def test_stream_raw_image_onto_device_write_error(self, requests_mock,
- open_mock, md5_mock):
+ open_mock, hash_mock):
self.config(image_download_connection_timeout=1)
self.config(image_download_connection_retry_interval=0)
image_info = _build_fake_image_info()
response = requests_mock.return_value
response.status_code = 200
- response.iter_content.return_value = ['some', 'content']
+ response.iter_content.return_value = [b'some', b'content']
file_mock = mock.Mock()
open_mock.return_value.__enter__.return_value = file_mock
file_mock.write.side_effect = Exception('Surprise!!!1!')
- hexdigest_mock = md5_mock.return_value.hexdigest
- hexdigest_mock.return_value = image_info['checksum']
+ hexdigest_mock = hash_mock.return_value.hexdigest
+ hexdigest_mock.return_value = image_info['os_hash_value']
self.assertRaises(errors.ImageDownloadError,
self.agent_extension._stream_raw_image_onto_device,
@@ -1265,17 +1323,17 @@ class TestStandbyExtension(base.IronicAgentTest):
stream=True, timeout=1, verify=True),
mock.call().iter_content(mock.ANY)]
requests_mock.assert_has_calls(calls)
- write_calls = [mock.call('some'),
- mock.call('some'),
- mock.call('some')]
+ write_calls = [mock.call(b'some'),
+ mock.call(b'some'),
+ mock.call(b'some')]
file_mock.write.assert_has_calls(write_calls)
@mock.patch('ironic_lib.disk_utils.fix_gpt_partition', autospec=True)
- @mock.patch('hashlib.md5', autospec=True)
+ @mock.patch('hashlib.new', autospec=True)
@mock.patch('builtins.open', autospec=True)
@mock.patch('requests.get', autospec=True)
def test_stream_raw_image_onto_device_socket_read_timeout(
- self, requests_mock, open_mock, md5_mock, fix_gpt_mock):
+ self, requests_mock, open_mock, hash_mock, fix_gpt_mock):
class create_timeout(object):
status_code = 200
@@ -1291,7 +1349,7 @@ class TestStandbyExtension(base.IronicAgentTest):
time.sleep(0.1)
return None
self.count += 1
- return "meow"
+ return b"meow"
def iter_content(self, chunk_size):
return self
@@ -1303,8 +1361,8 @@ class TestStandbyExtension(base.IronicAgentTest):
file_mock = mock.Mock()
open_mock.return_value.__enter__.return_value = file_mock
file_mock.read.return_value = None
- hexdigest_mock = md5_mock.return_value.hexdigest
- hexdigest_mock.return_value = image_info['checksum']
+ hexdigest_mock = hash_mock.return_value.hexdigest
+ hexdigest_mock.return_value = image_info['os_hash_value']
requests_mock.side_effect = create_timeout
self.assertRaisesRegex(
errors.ImageDownloadError,
@@ -1321,9 +1379,9 @@ class TestStandbyExtension(base.IronicAgentTest):
stream=True, proxies={}, timeout=1)]
requests_mock.assert_has_calls(calls)
- write_calls = [mock.call('meow'),
- mock.call('meow'),
- mock.call('meow')]
+ write_calls = [mock.call(b'meow'),
+ mock.call(b'meow'),
+ mock.call(b'meow')]
file_mock.write.assert_has_calls(write_calls)
fix_gpt_mock.assert_not_called()
@@ -1436,18 +1494,19 @@ class TestStandbyExtension(base.IronicAgentTest):
self.assertIsNone(node_uuid)
-@mock.patch('hashlib.md5', autospec=True)
+@mock.patch('hashlib.new', autospec=True)
@mock.patch('requests.get', autospec=True)
class TestImageDownload(base.IronicAgentTest):
- def test_download_image(self, requests_mock, md5_mock):
+ def test_download_image(self, requests_mock, hash_mock):
content = ['SpongeBob', 'SquarePants']
response = requests_mock.return_value
response.status_code = 200
response.iter_content.return_value = content
image_info = _build_fake_image_info()
- md5_mock.return_value.hexdigest.return_value = image_info['checksum']
+ hash_mock.return_value.hexdigest.return_value = image_info[
+ 'os_hash_value']
image_download = standby.ImageDownload(image_info)
self.assertEqual(content, list(image_download))
@@ -1455,7 +1514,7 @@ class TestImageDownload(base.IronicAgentTest):
cert=None, verify=True,
stream=True, proxies={},
timeout=60)
- self.assertEqual(image_info['checksum'],
+ self.assertEqual(image_info['os_hash_value'],
image_download._hash_algo.hexdigest())
@mock.patch('time.sleep', autospec=True)
@@ -1502,7 +1561,7 @@ class TestImageDownload(base.IronicAgentTest):
@mock.patch('time.sleep', autospec=True)
def test_download_image_retries_success(self, sleep_mock, requests_mock,
- md5_mock):
+ hash_mock):
content = ['SpongeBob', 'SquarePants']
fail_response = mock.Mock()
fail_response.status_code = 500
@@ -1513,7 +1572,8 @@ class TestImageDownload(base.IronicAgentTest):
requests_mock.side_effect = [requests.Timeout, fail_response, response]
image_info = _build_fake_image_info()
- md5_mock.return_value.hexdigest.return_value = image_info['checksum']
+ hash_mock.return_value.hexdigest.return_value = image_info[
+ 'os_hash_value']
image_download = standby.ImageDownload(image_info)
self.assertEqual(content, list(image_download))
@@ -1525,7 +1585,34 @@ class TestImageDownload(base.IronicAgentTest):
sleep_mock.assert_called_with(10)
self.assertEqual(2, sleep_mock.call_count)
- def test_download_image_and_checksum(self, requests_mock, md5_mock):
+ def test_download_image_and_checksum(self, requests_mock, hash_mock):
+ content = ['SpongeBob', 'SquarePants']
+ fake_cs = "019fe036425da1c562f2e9f5299820bf"
+ cs_response = mock.Mock()
+ cs_response.status_code = 200
+ cs_response.text = fake_cs + '\n'
+ response = mock.Mock()
+ response.status_code = 200
+ response.iter_content.return_value = content
+ requests_mock.side_effect = [cs_response, response]
+
+ image_info = _build_fake_image_info()
+ image_info['os_hash_algo'] = 'sha512'
+ image_info['os_hash_value'] = 'http://example.com/checksum'
+ hash_mock.return_value.hexdigest.return_value = fake_cs
+ image_download = standby.ImageDownload(image_info)
+
+ self.assertEqual(content, list(image_download))
+ requests_mock.assert_has_calls([
+ mock.call('http://example.com/checksum', cert=None, verify=True,
+ stream=True, proxies={}, timeout=60),
+ mock.call(image_info['urls'][0], cert=None, verify=True,
+ stream=True, proxies={}, timeout=60),
+ ])
+ self.assertEqual(fake_cs, image_download._hash_algo.hexdigest())
+
+ def test_download_image_and_checksum_md5(self, requests_mock, hash_mock):
+
content = ['SpongeBob', 'SquarePants']
fake_cs = "019fe036425da1c562f2e9f5299820bf"
cs_response = mock.Mock()
@@ -1537,8 +1624,11 @@ class TestImageDownload(base.IronicAgentTest):
requests_mock.side_effect = [cs_response, response]
image_info = _build_fake_image_info()
+ del image_info['os_hash_value']
+ del image_info['os_hash_algo']
+ CONF.set_override('md5_enabled', True)
image_info['checksum'] = 'http://example.com/checksum'
- md5_mock.return_value.hexdigest.return_value = fake_cs
+ hash_mock.return_value.hexdigest.return_value = fake_cs
image_download = standby.ImageDownload(image_info)
self.assertEqual(content, list(image_download))
@@ -1549,9 +1639,11 @@ class TestImageDownload(base.IronicAgentTest):
stream=True, proxies={}, timeout=60),
])
self.assertEqual(fake_cs, image_download._hash_algo.hexdigest())
+ hash_mock.assert_has_calls([
+ mock.call('md5')])
- def test_download_image_and_checksum_multiple(self, requests_mock,
- md5_mock):
+ def test_download_image_and_checksum_multiple_md5(self, requests_mock,
+ hash_mock):
content = ['SpongeBob', 'SquarePants']
fake_cs = "019fe036425da1c562f2e9f5299820bf"
cs_response = mock.Mock()
@@ -1568,7 +1660,43 @@ foobar irrelevant file.img
image_info = _build_fake_image_info(
'http://example.com/path/image.img')
image_info['checksum'] = 'http://example.com/checksum'
- md5_mock.return_value.hexdigest.return_value = fake_cs
+ del image_info['os_hash_algo']
+ del image_info['os_hash_value']
+ CONF.set_override('md5_enabled', True)
+ hash_mock.return_value.hexdigest.return_value = fake_cs
+ image_download = standby.ImageDownload(image_info)
+
+ self.assertEqual(content, list(image_download))
+ requests_mock.assert_has_calls([
+ mock.call('http://example.com/checksum', cert=None, verify=True,
+ stream=True, proxies={}, timeout=60),
+ mock.call(image_info['urls'][0], cert=None, verify=True,
+ stream=True, proxies={}, timeout=60),
+ ])
+ self.assertEqual(fake_cs, image_download._hash_algo.hexdigest())
+
+ def test_download_image_and_checksum_multiple_sha256(self, requests_mock,
+ hash_mock):
+ content = ['SpongeBob', 'SquarePants']
+ fake_cs = ('3b678e4fb651d450f4970e1647abc9b0a38bff3febd3d558753'
+ '623c66369a633')
+ cs_response = mock.Mock()
+ cs_response.status_code = 200
+ cs_response.text = """
+foobar irrelevant file.img
+%s image.img
+""" % fake_cs
+ response = mock.Mock()
+ response.status_code = 200
+ response.iter_content.return_value = iter(content)
+ requests_mock.side_effect = [cs_response, response]
+
+ image_info = _build_fake_image_info(
+ 'http://example.com/path/image.img')
+ image_info['checksum'] = 'http://example.com/checksum'
+ del image_info['os_hash_algo']
+ del image_info['os_hash_value']
+ hash_mock.return_value.hexdigest.return_value = fake_cs
image_download = standby.ImageDownload(image_info)
self.assertEqual(content, list(image_download))
@@ -1579,9 +1707,73 @@ foobar irrelevant file.img
stream=True, proxies={}, timeout=60),
])
self.assertEqual(fake_cs, image_download._hash_algo.hexdigest())
+ hash_mock.assert_has_calls([
+ mock.call('sha256')])
+
+ def test_download_image_and_checksum_multiple_sha512(self, requests_mock,
+ hash_mock):
+ content = ['SpongeBob', 'SquarePants']
+ fake_cs = ('3b678e4fb651d450f4970e1647abc9b0a38bff3febd3d558753'
+ '623c66369a6333b678e4fb651d450f4970e1647abc9b0a38b'
+ 'ff3febd3d558753623c66369a633')
+ cs_response = mock.Mock()
+ cs_response.status_code = 200
+ cs_response.text = """
+foobar irrelevant file.img
+%s image.img
+""" % fake_cs
+ response = mock.Mock()
+ response.status_code = 200
+ response.iter_content.return_value = iter(content)
+ requests_mock.side_effect = [cs_response, response]
+
+ image_info = _build_fake_image_info(
+ 'http://example.com/path/image.img')
+ image_info['checksum'] = 'http://example.com/checksum'
+ del image_info['os_hash_algo']
+ del image_info['os_hash_value']
+ hash_mock.return_value.hexdigest.return_value = fake_cs
+ image_download = standby.ImageDownload(image_info)
+
+ self.assertEqual(content, list(image_download))
+ requests_mock.assert_has_calls([
+ mock.call('http://example.com/checksum', cert=None, verify=True,
+ stream=True, proxies={}, timeout=60),
+ mock.call(image_info['urls'][0], cert=None, verify=True,
+ stream=True, proxies={}, timeout=60),
+ ])
+ self.assertEqual(fake_cs, image_download._hash_algo.hexdigest())
+ hash_mock.assert_has_calls([
+ mock.call('sha512')])
def test_download_image_and_checksum_unknown_file(self, requests_mock,
- md5_mock):
+ hash_mock):
+ content = ['SpongeBob', 'SquarePants']
+ fake_cs = "019fe036425da1c562f2e9f5299820bf"
+ cs_response = mock.Mock()
+ cs_response.status_code = 200
+ cs_response.text = """
+foobar irrelevant file.img
+%s not-my-image.img
+""" % fake_cs
+ response = mock.Mock()
+ response.status_code = 200
+ response.iter_content.return_value = content
+ requests_mock.side_effect = [cs_response, response]
+
+ image_info = _build_fake_image_info(
+ 'http://example.com/path/image.img')
+ image_info['os_hash_algo'] = 'sha512'
+ image_info['os_hash_value'] = 'http://example.com/checksum'
+ hash_mock.return_value.hexdigest.return_value = fake_cs
+ self.assertRaisesRegex(errors.ImageDownloadError,
+ 'Checksum file does not contain name image.img',
+ standby.ImageDownload, image_info)
+
+ def test_download_image_and_checksum_unknown_file_md5(self,
+ requests_mock,
+ hash_mock):
+ CONF.set_override('md5_enabled', True)
content = ['SpongeBob', 'SquarePants']
fake_cs = "019fe036425da1c562f2e9f5299820bf"
cs_response = mock.Mock()
@@ -1598,13 +1790,16 @@ foobar irrelevant file.img
image_info = _build_fake_image_info(
'http://example.com/path/image.img')
image_info['checksum'] = 'http://example.com/checksum'
- md5_mock.return_value.hexdigest.return_value = fake_cs
+ del image_info['os_hash_algo']
+ del image_info['os_hash_value']
+ hash_mock.return_value.hexdigest.return_value = fake_cs
self.assertRaisesRegex(errors.ImageDownloadError,
'Checksum file does not contain name image.img',
standby.ImageDownload, image_info)
- def test_download_image_and_checksum_empty_file(self, requests_mock,
- md5_mock):
+ def test_download_image_and_checksum_empty_file_md5(self, requests_mock,
+ hash_mock):
+ CONF.set_override('md5_enabled', True)
content = ['SpongeBob', 'SquarePants']
cs_response = mock.Mock()
cs_response.status_code = 200
@@ -1617,11 +1812,58 @@ foobar irrelevant file.img
image_info = _build_fake_image_info(
'http://example.com/path/image.img')
image_info['checksum'] = 'http://example.com/checksum'
+ del image_info['os_hash_algo']
+ del image_info['os_hash_value']
self.assertRaisesRegex(errors.ImageDownloadError,
'Empty checksum file',
standby.ImageDownload, image_info)
- def test_download_image_and_checksum_failed(self, requests_mock, md5_mock):
+ def test_download_image_and_checksum_empty_file(self, requests_mock,
+ hash_mock):
+ content = ['SpongeBob', 'SquarePants']
+ cs_response = mock.Mock()
+ cs_response.status_code = 200
+ cs_response.text = " "
+ response = mock.Mock()
+ response.status_code = 200
+ response.iter_content.return_value = content
+ requests_mock.side_effect = [cs_response, response]
+
+ image_info = _build_fake_image_info(
+ 'http://example.com/path/image.img')
+ image_info['os_hash_algo'] = 'sha512'
+ image_info['os_hash_value'] = 'http://example.com/checksum'
+ self.assertRaisesRegex(errors.ImageDownloadError,
+ 'Empty checksum file',
+ standby.ImageDownload, image_info)
+
+ def test_download_image_and_checksum_failed(self, requests_mock,
+ hash_mock):
+ self.config(image_download_connection_retry_interval=0)
+ content = ['SpongeBob', 'SquarePants']
+ cs_response = mock.Mock()
+ cs_response.status_code = 400
+ cs_response.text = " "
+ response = mock.Mock()
+ response.status_code = 200
+ response.iter_content.return_value = content
+ # 3 retries on status code
+ requests_mock.side_effect = [cs_response, cs_response, cs_response,
+ response]
+
+ image_info = _build_fake_image_info(
+ 'http://example.com/path/image.img')
+ image_info['os_hash_value'] = 'http://example.com/checksum'
+ image_info['os_hash_algo'] = 'sha512'
+ self.assertRaisesRegex(errors.ImageDownloadError,
+ 'Received status code 400 from '
+ 'http://example.com/checksum',
+ standby.ImageDownload, image_info)
+
+ def test_download_image_and_checksum_failed_md5(self,
+ requests_mock,
+ hash_mock):
+ CONF.set_override('md5_enabled', True)
self.config(image_download_connection_retry_interval=0)
content = ['SpongeBob', 'SquarePants']
cs_response = mock.Mock()
@@ -1637,6 +1879,8 @@ foobar irrelevant file.img
image_info = _build_fake_image_info(
'http://example.com/path/image.img')
image_info['checksum'] = 'http://example.com/checksum'
+ del image_info['os_hash_value']
+ del image_info['os_hash_algo']
self.assertRaisesRegex(errors.ImageDownloadError,
'Received status code 400 from '
'http://example.com/checksum',
diff --git a/releasenotes/notes/disable-md5-image-checksum-7def176928d36e75.yaml b/releasenotes/notes/disable-md5-image-checksum-7def176928d36e75.yaml
new file mode 100644
index 00000000..7fcacac4
--- /dev/null
+++ b/releasenotes/notes/disable-md5-image-checksum-7def176928d36e75.yaml
@@ -0,0 +1,19 @@
+---
+features:
+ - |
+ The ``ironic-python-agent`` will now attempt to determine a checksum type
+ by evaluating the length of the supplied checksum. This allows SHA512
+ (SHA-2) and SHA256 (SHA-2) checksums to be identified and utilized without
+ an explicit declaration of the checksum type utilizing the
+ ``os_hash_algo`` value.
+upgrade:
+ - |
+ MD5 support for checksums have been disabled by default. This may result
+ in rebulids or manual deploy attempts to fail if no updated checksum has
+ been supplied for the ``os_hash_value`` and ``os_hash_algo`` settings.
+ To re-enable MD5 support, you may utilize a the ``[DEFAULT]md5_enabled``
+ setting.
+deprecations:
+ - |
+ Support for MD5 checksums have been deprecated and disabled by default.
+ Support for MD5 checksums will be removed after the 2024 Release.