Warn when the provided checksum algorithm does not match the detected

dtantsur · jayofdoom · commit 7e9dd7e9f140 · 2024-10-23T13:31:57.000Z
I have a case where a user provided the checksum URL with SHA256 checksums, while Metal3 defaulted os_hash_algo to "md5". We're going to change the Metal3 defaults in the next API version, but for now let us issue a clear warning in such case. Closes-Bug: #2085331 Change-Id: Ie4e62a378dc4a2089944f4302df3a8671b7c960f (cherry picked from commit d8d32d9) (cherry picked from commit aa01777)
diff --git a/ironic_python_agent/extensions/standby.py b/ironic_python_agent/extensions/standby.py
@@ -581,6 +581,25 @@ def __init__(self, image_info, time_obj=None):
                 self._expected_hash_value,
                 image_info)
 
+        # NOTE(dtantsur): verify that the user's input does not obviously
+        # contradict the actual value. It is especially easy to make such
+        # a mistake when providing a checksum URL.
+        if algo:
+            try:
+                detected_algo = _get_algorithm_by_length(
+                    self._expected_hash_value)
+            except ValueError:
+                pass  # an exotic algorithm?
+            else:
+                if detected_algo.name != algo:
+                    LOG.warning("Provided checksum algorithm %(provided)s "
+                                "does not match the detected algorithm "
+                                "%(detected)s. It may be a sign of a user "
+                                "error when providing the algorithm or the "
+                                "checksum URL.",
+                                {'provided': algo,
+                                 'detected': detected_algo.name})
+
         details = []
         for url in image_info['urls']:
             try:
diff --git a/ironic_python_agent/tests/unit/extensions/test_standby.py b/ironic_python_agent/tests/unit/extensions/test_standby.py
@@ -795,18 +795,21 @@ def test_verify_image_success_without_md5(self, requests_mock,
         image_download.verify_image(image_location)
         hashlib_mock.assert_called_with('sha512')
 
+    @mock.patch.object(standby.LOG, 'warning', 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, hash_mock):
+                                                    open_mock, hash_mock,
+                                                    warn_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
+        hash_mock.return_value.name = 'md5'
         hexdigest_mock = hash_mock.return_value.hexdigest
         hexdigest_mock.return_value = image_info['checksum']
         image_location = '/foo/bar'
@@ -818,7 +821,11 @@ def test_verify_image_success_with_md5_fallback(self, requests_mock,
         hash_mock.assert_has_calls([
             mock.call('md5'),
             mock.call().__bool__(),
+            mock.call('md5'),
             mock.call().hexdigest()])
+        warn_mock.assert_called_once_with(
+            mock.ANY,
+            {'provided': 'algo-beyond-milky-way', 'detected': 'md5'})
 
     @mock.patch('hashlib.new', autospec=True)
     @mock.patch('builtins.open', autospec=True)
@@ -1760,7 +1767,39 @@ def test_download_image_retries_success(self, sleep_mock, requests_mock,
         sleep_mock.assert_called_with(10)
         self.assertEqual(2, sleep_mock.call_count)
 
-    def test_download_image_and_checksum(self, requests_mock, hash_mock):
+    @mock.patch.object(standby.LOG, 'warning', autospec=True)
+    def test_download_image_and_checksum(self, warn_mock, 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
+        hash_mock.return_value.name = 'sha512'
+        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())
+        warn_mock.assert_not_called()
+
+    @mock.patch.object(standby.LOG, 'warning', autospec=True)
+    def test_download_image_and_checksum_warning_on_mismatch(
+            self, warn_mock, requests_mock, hash_mock):
         content = ['SpongeBob', 'SquarePants']
         fake_cs = "019fe036425da1c562f2e9f5299820bf"
         cs_response = mock.Mock()
@@ -1775,6 +1814,7 @@ def test_download_image_and_checksum(self, requests_mock, hash_mock):
         image_info['os_hash_algo'] = 'sha512'
         image_info['os_hash_value'] = 'http://example.com/checksum'
         hash_mock.return_value.hexdigest.return_value = fake_cs
+        hash_mock.return_value.name = 'md5'
         image_download = standby.ImageDownload(image_info)
 
         self.assertEqual(content, list(image_download))
@@ -1785,6 +1825,9 @@ def test_download_image_and_checksum(self, requests_mock, hash_mock):
                       stream=True, proxies={}, timeout=60),
         ])
         self.assertEqual(fake_cs, image_download._hash_algo.hexdigest())
+        warn_mock.assert_called_once_with(
+            mock.ANY,
+            {'provided': 'sha512', 'detected': 'md5'})
 
     def test_download_image_and_checksum_md5(self, requests_mock, hash_mock):
 
