Fail gracefully when MD5 is unavailable

The glanceclient currently assumes that MD5 will always be available.
This is not the case, however, in a FIPS-compliant environment.  This
patch enables the glanceclient to fail gracefully in such a case.

Closes-bug: #1871675
Change-Id: Ibd89989e06cc5be7da71f5f21561d73b5abc4104

glanceclient/common/utils.py

@@ -436,7 +436,14 @@ def integrity_iter(iter, checksum):
 
     :raises: IOError
     """
-    md5sum = hashlib.md5()
+    try:
+        md5sum = hashlib.new('md5')
+    except ValueError:
+        raise IOError(errno.EPIPE,
+                      'Corrupt image download. Expected checksum is %s '
+                      'but md5 algorithm is not available on the client' %
+                      checksum)
+
     for chunk in iter:
         yield chunk
         if isinstance(chunk, six.string_types):

glanceclient/tests/unit/v2/test_images.py

@@ -18,6 +18,8 @@ import hashlib
 import testtools
 from unittest import mock
 
+import ddt
+
 from glanceclient import exc
 from glanceclient.tests.unit.v2 import base
 from glanceclient.tests import utils
@@ -704,6 +706,7 @@ schema_fixtures = {
 }
 
 
+@ddt.ddt
 class TestController(testtools.TestCase):
     def setUp(self):
         super(TestController, self).setUp()
@@ -1092,6 +1095,21 @@ class TestController(testtools.TestCase):
             body = ''.join([b for b in body])
             self.assertEqual('CCC', body)
 
+    @ddt.data('headeronly', 'chkonly', 'multihash')
+    def test_data_with_checksum_but_no_md5_algo(self, prefix):
+        with mock.patch('hashlib.new', mock.MagicMock(
+                side_effect=ValueError('unsupported hash type'))):
+            body = self.controller.data(prefix +
+                                        '-dd57-11e1-af0f-02163e68b1d8',
+                                        allow_md5_fallback=True)
+            try:
+                body = ''.join([b for b in body])
+                self.fail('missing md5 algo did not raise an error')
+            except IOError as e:
+                self.assertEqual(errno.EPIPE, e.errno)
+                msg = 'md5 algorithm is not available on the client'
+                self.assertIn(msg, str(e))
+
     def test_data_with_checksum_and_fallback(self):
         # make sure the allow_md5_fallback option does not cause any
         # incorrect behavior when fallback is not needed

glanceclient/v2/images.py

@@ -209,9 +209,11 @@ class Controller(object):
            specified hash algorithm is not available AND allow_md5_fallback
            is True, then continue to step #2
         2. else if the image has a checksum property, MD5 is used to
-           validate against the 'checksum' value
+           validate against the 'checksum' value.  (If MD5 is not available
+           to the client, the download fails.)
         3. else if the download response has a 'content-md5' header, MD5
-           is used to validate against the header value
+           is used to validate against the header value.  (If MD5 is not
+           available to the client, the download fails.)
         4. if none of 1-3 obtain, the data is **not validated** (this is
            compatible with legacy behavior)
 

lower-constraints.txt

@@ -7,6 +7,7 @@ cliff==2.8.0
 cmd2==0.8.0
 coverage==4.0
 cryptography==2.1
+ddt==1.2.1
 debtcollector==1.2.0
 docutils==0.11
 dulwich==0.15.0

releasenotes/notes/check-for-md5-59db8fd67870b214.yaml

@@ -0,0 +1,13 @@
+---
+other:
+  -|
+   For legacy (pre-Rocky) images that do not contain "multihash" metadata,
+   or when the ``--allow-md5-fallback`` option is used in cases where the
+   multihash metadata is present but the specified algorithm is not available
+   to the glanceclient, the glanceclient uses an MD5 checksum to validate
+   the download.  When operating in a FIPS-compliant environment, however,
+   the MD5 algorithm may be unavailable to the glanceclient. In such a case,
+   (that is, when the MD5 checksum information is available to the glanceclient
+   but the MD5 algorithm is not), the glanceclient will fail the download as
+   corrupt because it cannot prove otherwise.  This is consistent with
+   current behavior.

test-requirements.txt

@@ -9,6 +9,7 @@ os-client-config>=1.28.0 # Apache-2.0
 stestr>=2.0.0 # Apache-2.0
 testtools>=2.2.0 # MIT
 testscenarios>=0.4 # Apache-2.0/BSD
+ddt>=1.2.1 # MIT
 fixtures>=3.0.0 # Apache-2.0/BSD
 requests-mock>=1.2.0 # Apache-2.0
 tempest>=17.1.0 # Apache-2.0