Use "multihash" for data download validation

When the Glance "multihash" is available on an image, the
glanceclient should use it instead of MD5 to validate data
downloads.  For cases in which the multihash specifies an
algorithm not available to the client, an option is added
to the image-download command that will allow fallback to
the legacy MD5 checksum verification.

Change-Id: I4ee6e5071eca08d3bbedceda2acc170e7ed21a6b
Closes-bug: #1788323

4 files changed, 263 insertions, 17 deletions

diff --git a/glanceclient/tests/unit/test_shell.py b/glanceclient/tests/unit/test_shell.py
index 0f15007..3027e46 100644
--- a/glanceclient/tests/unit/test_shell.py
+++ b/glanceclient/tests/unit/test_shell.py
@@ -965,7 +965,9 @@ class ShellTestRequests(testutils.TestCase):
             self.requests = self.useFixture(rm_fixture.Fixture())
             self.requests.get('http://example.com/v2/images/%s/file' % id,
                               headers=headers, raw=fake)
-
+            self.requests.get('http://example.com/v2/images/%s' % id,
+                              headers={'Content-type': 'application/json'},
+                              json=image_show_fixture)
             shell = openstack_shell.OpenStackImagesShell()
             argstr = ('--os-image-api-version 2 --os-auth-token faketoken '
                       '--os-image-url http://example.com '
diff --git a/glanceclient/tests/unit/v2/fixtures.py b/glanceclient/tests/unit/v2/fixtures.py
index 5a603c0..22e1ff7 100644
--- a/glanceclient/tests/unit/v2/fixtures.py
+++ b/glanceclient/tests/unit/v2/fixtures.py
@@ -14,6 +14,9 @@
 # License for the specific language governing permissions and limitations
 # under the License.
 
+import hashlib
+
+
 UUID = "3fc2ba62-9a02-433e-b565-d493ffc69034"
 
 image_list_fixture = {
@@ -65,7 +68,9 @@ image_show_fixture = {
     "tags": [],
     "updated_at": "2015-07-24T12:18:13Z",
     "virtual_size": "null",
-    "visibility": "shared"
+    "visibility": "shared",
+    "os_hash_algo": "sha384",
+    "os_hash_value": hashlib.sha384(b'DATA').hexdigest()
 }
 
 image_create_fixture = {
diff --git a/glanceclient/tests/unit/v2/test_images.py b/glanceclient/tests/unit/v2/test_images.py
index 23cbb43..753f3c4 100644
--- a/glanceclient/tests/unit/v2/test_images.py
+++ b/glanceclient/tests/unit/v2/test_images.py
@@ -14,6 +14,7 @@
 #    under the License.
 
 import errno
+import hashlib
 import mock
 import testtools
 
@@ -193,7 +194,43 @@ data_fixtures = {
             'A',
         ),
     },
-    '/v2/images/66fb18d6-db27-11e1-a1eb-080027cbe205/file': {
+    '/v2/images/5cc4bebc-db27-11e1-a1eb-080027cbe205': {
+        'GET': (
+            {},
+            {},
+        ),
+    },
+    '/v2/images/headeronly-db27-11e1-a1eb-080027cbe205/file': {
+        'GET': (
+            {
+                'content-md5': 'wrong'
+            },
+            'BB',
+        ),
+    },
+    '/v2/images/headeronly-db27-11e1-a1eb-080027cbe205': {
+        'GET': (
+            {},
+            {},
+        ),
+    },
+    '/v2/images/chkonly-db27-11e1-a1eb-080027cbe205/file': {
+        'GET': (
+            {
+                'content-md5': 'wrong'
+            },
+            'BB',
+        ),
+    },
+    '/v2/images/chkonly-db27-11e1-a1eb-080027cbe205': {
+        'GET': (
+            {},
+            {
+                'checksum': 'wrong',
+            },
+        ),
+    },
+    '/v2/images/multihash-db27-11e1-a1eb-080027cbe205/file': {
         'GET': (
             {
                 'content-md5': 'wrong'
@@ -201,7 +238,67 @@ data_fixtures = {
             'BB',
         ),
     },
-    '/v2/images/1b1c6366-dd57-11e1-af0f-02163e68b1d8/file': {
+    '/v2/images/multihash-db27-11e1-a1eb-080027cbe205': {
+        'GET': (
+            {},
+            {
+                'checksum': 'wrong',
+                'os_hash_algo': 'md5',
+                'os_hash_value': 'junk'
+            },
+        ),
+    },
+    '/v2/images/badalgo-db27-11e1-a1eb-080027cbe205/file': {
+        'GET': (
+            {
+                'content-md5': hashlib.md5(b'BB').hexdigest()
+            },
+            'BB',
+        ),
+    },
+    '/v2/images/badalgo-db27-11e1-a1eb-080027cbe205': {
+        'GET': (
+            {},
+            {
+                'checksum': hashlib.md5(b'BB').hexdigest(),
+                'os_hash_algo': 'not_an_algo',
+                'os_hash_value': 'whatever'
+            },
+        ),
+    },
+    '/v2/images/bad-multihash-value-good-checksum/file': {
+        'GET': (
+            {
+                'content-md5': hashlib.md5(b'GOODCHECKSUM').hexdigest()
+            },
+            'GOODCHECKSUM',
+        ),
+    },
+    '/v2/images/bad-multihash-value-good-checksum': {
+        'GET': (
+            {},
+            {
+                'checksum': hashlib.md5(b'GOODCHECKSUM').hexdigest(),
+                'os_hash_algo': 'sha512',
+                'os_hash_value': 'badmultihashvalue'
+            },
+        ),
+    },
+    '/v2/images/headeronly-dd57-11e1-af0f-02163e68b1d8/file': {
+        'GET': (
+            {
+                'content-md5': 'defb99e69a9f1f6e06f15006b1f166ae'
+            },
+            'CCC',
+        ),
+    },
+    '/v2/images/headeronly-dd57-11e1-af0f-02163e68b1d8': {
+        'GET': (
+            {},
+            {},
+        ),
+    },
+    '/v2/images/chkonly-dd57-11e1-af0f-02163e68b1d8/file': {
         'GET': (
             {
                 'content-md5': 'defb99e69a9f1f6e06f15006b1f166ae'
@@ -209,6 +306,32 @@ data_fixtures = {
             'CCC',
         ),
     },
+    '/v2/images/chkonly-dd57-11e1-af0f-02163e68b1d8': {
+        'GET': (
+            {},
+            {
+                'checksum': 'defb99e69a9f1f6e06f15006b1f166ae',
+            },
+        ),
+    },
+    '/v2/images/multihash-dd57-11e1-af0f-02163e68b1d8/file': {
+        'GET': (
+            {
+                'content-md5': 'defb99e69a9f1f6e06f15006b1f166ae'
+            },
+            'CCC',
+        ),
+    },
+    '/v2/images/multihash-dd57-11e1-af0f-02163e68b1d8': {
+        'GET': (
+            {},
+            {
+                'checksum': 'defb99e69a9f1f6e06f15006b1f166ae',
+                'os_hash_algo': 'sha384',
+                'os_hash_value': hashlib.sha384(b'CCC').hexdigest()
+            },
+        ),
+    },
     '/v2/images/87b634c1-f893-33c9-28a9-e5673c99239a/actions/reactivate': {
         'POST': ({}, None)
     },
@@ -846,12 +969,24 @@ class TestController(testtools.TestCase):
         self.assertEqual('A', body)
 
     def test_data_with_wrong_checksum(self):
-        body = self.controller.data('66fb18d6-db27-11e1-a1eb-080027cbe205',
+        body = self.controller.data('headeronly-db27-11e1-a1eb-080027cbe205',
                                     do_checksum=False)
         body = ''.join([b for b in body])
         self.assertEqual('BB', body)
+        body = self.controller.data('headeronly-db27-11e1-a1eb-080027cbe205')
+        try:
+            body = ''.join([b for b in body])
+            self.fail('data did not raise an error.')
+        except IOError as e:
+            self.assertEqual(errno.EPIPE, e.errno)
+            msg = 'was 9d3d9048db16a7eee539e93e3618cbe7 expected wrong'
+            self.assertIn(msg, str(e))
 
-        body = self.controller.data('66fb18d6-db27-11e1-a1eb-080027cbe205')
+        body = self.controller.data('chkonly-db27-11e1-a1eb-080027cbe205',
+                                    do_checksum=False)
+        body = ''.join([b for b in body])
+        self.assertEqual('BB', body)
+        body = self.controller.data('chkonly-db27-11e1-a1eb-080027cbe205')
         try:
             body = ''.join([b for b in body])
             self.fail('data did not raise an error.')
@@ -860,15 +995,103 @@ class TestController(testtools.TestCase):
             msg = 'was 9d3d9048db16a7eee539e93e3618cbe7 expected wrong'
             self.assertIn(msg, str(e))
 
-    def test_data_with_checksum(self):
-        body = self.controller.data('1b1c6366-dd57-11e1-af0f-02163e68b1d8',
+        body = self.controller.data('multihash-db27-11e1-a1eb-080027cbe205',
+                                    do_checksum=False)
+        body = ''.join([b for b in body])
+        self.assertEqual('BB', body)
+        body = self.controller.data('multihash-db27-11e1-a1eb-080027cbe205')
+        try:
+            body = ''.join([b for b in body])
+            self.fail('data did not raise an error.')
+        except IOError as e:
+            self.assertEqual(errno.EPIPE, e.errno)
+            msg = 'was 9d3d9048db16a7eee539e93e3618cbe7 expected junk'
+            self.assertIn(msg, str(e))
+
+        body = self.controller.data('badalgo-db27-11e1-a1eb-080027cbe205',
                                     do_checksum=False)
         body = ''.join([b for b in body])
-        self.assertEqual('CCC', body)
+        self.assertEqual('BB', body)
+        try:
+            body = self.controller.data('badalgo-db27-11e1-a1eb-080027cbe205')
+            self.fail('bad os_hash_algo did not raise an error.')
+        except ValueError as e:
+            msg = 'unsupported hash type not_an_algo'
+            self.assertIn(msg, str(e))
+
+    def test_data_with_checksum(self):
+        for prefix in ['headeronly', 'chkonly', 'multihash']:
+            body = self.controller.data(prefix +
+                                        '-dd57-11e1-af0f-02163e68b1d8',
+                                        do_checksum=False)
+            body = ''.join([b for b in body])
+            self.assertEqual('CCC', body)
+
+            body = self.controller.data(prefix +
+                                        '-dd57-11e1-af0f-02163e68b1d8')
+            body = ''.join([b for b in body])
+            self.assertEqual('CCC', body)
+
+    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
+        for prefix in ['headeronly', 'chkonly', 'multihash']:
+            body = self.controller.data(prefix +
+                                        '-dd57-11e1-af0f-02163e68b1d8',
+                                        do_checksum=False,
+                                        allow_md5_fallback=True)
+            body = ''.join([b for b in body])
+            self.assertEqual('CCC', body)
 
-        body = self.controller.data('1b1c6366-dd57-11e1-af0f-02163e68b1d8')
+            body = self.controller.data(prefix +
+                                        '-dd57-11e1-af0f-02163e68b1d8',
+                                        allow_md5_fallback=True)
+            body = ''.join([b for b in body])
+            self.assertEqual('CCC', body)
+
+    def test_data_with_bad_hash_algo_and_fallback(self):
+        # shouldn't matter when do_checksum is False
+        body = self.controller.data('badalgo-db27-11e1-a1eb-080027cbe205',
+                                    do_checksum=False,
+                                    allow_md5_fallback=True)
+        body = ''.join([b for b in body])
+        self.assertEqual('BB', body)
+
+        # default value for do_checksum is True
+        body = self.controller.data('badalgo-db27-11e1-a1eb-080027cbe205',
+                                    allow_md5_fallback=True)
+        body = ''.join([b for b in body])
+        self.assertEqual('BB', body)
+
+    def test_neg_data_with_bad_hash_value_and_fallback_enabled(self):
+        # make sure download fails when good hash_algo but bad hash_value
+        # even when correct checksum is present regardless of
+        # allow_md5_fallback setting
+        body = self.controller.data('bad-multihash-value-good-checksum',
+                                    allow_md5_fallback=False)
+        try:
+            body = ''.join([b for b in body])
+            self.fail('bad os_hash_value did not raise an error.')
+        except IOError as e:
+            self.assertEqual(errno.EPIPE, e.errno)
+            msg = 'expected badmultihashvalue'
+            self.assertIn(msg, str(e))
+
+        body = self.controller.data('bad-multihash-value-good-checksum',
+                                    allow_md5_fallback=True)
+        try:
+            body = ''.join([b for b in body])
+            self.fail('bad os_hash_value did not raise an error.')
+        except IOError as e:
+            self.assertEqual(errno.EPIPE, e.errno)
+            msg = 'expected badmultihashvalue'
+            self.assertIn(msg, str(e))
+
+        # download should succeed when do_checksum is off, though
+        body = self.controller.data('bad-multihash-value-good-checksum',
+                                    do_checksum=False)
         body = ''.join([b for b in body])
-        self.assertEqual('CCC', body)
+        self.assertEqual('GOODCHECKSUM', body)
 
     def test_image_import(self):
         uri = 'http://example.com/image.qcow'
@@ -883,7 +1106,7 @@ class TestController(testtools.TestCase):
     def test_download_no_data(self):
         resp = utils.FakeResponse(headers={}, status_code=204)
         self.controller.controller.http_client.get = mock.Mock(
-            return_value=(resp, None))
+            return_value=(resp, {}))
         self.controller.data('image_id')
 
     def test_download_forbidden(self):
diff --git a/glanceclient/tests/unit/v2/test_shell_v2.py b/glanceclient/tests/unit/v2/test_shell_v2.py
index 6eeca83..acf93bf 100644
--- a/glanceclient/tests/unit/v2/test_shell_v2.py
+++ b/glanceclient/tests/unit/v2/test_shell_v2.py
@@ -1729,7 +1729,8 @@ class ShellV2Test(testtools.TestCase):
 
     def test_image_download(self):
         args = self._make_args(
-            {'id': 'IMG-01', 'file': 'test', 'progress': True})
+            {'id': 'IMG-01', 'file': 'test', 'progress': True,
+             'allow_md5_fallback': False})
 
         with mock.patch.object(self.gc.images, 'data') as mocked_data, \
                 mock.patch.object(utils, '_extract_request_id'):
@@ -1737,14 +1738,27 @@ class ShellV2Test(testtools.TestCase):
                 [c for c in 'abcdef'])
 
             test_shell.do_image_download(self.gc, args)
-            mocked_data.assert_called_once_with('IMG-01')
+            mocked_data.assert_called_once_with('IMG-01',
+                                                allow_md5_fallback=False)
+
+        # check that non-default value is being passed correctly
+        args.allow_md5_fallback = True
+        with mock.patch.object(self.gc.images, 'data') as mocked_data, \
+                mock.patch.object(utils, '_extract_request_id'):
+            mocked_data.return_value = utils.RequestIdProxy(
+                [c for c in 'abcdef'])
+
+            test_shell.do_image_download(self.gc, args)
+            mocked_data.assert_called_once_with('IMG-01',
+                                                allow_md5_fallback=True)
 
     @mock.patch.object(utils, 'exit')
     @mock.patch('sys.stdout', autospec=True)
     def test_image_download_no_file_arg(self, mocked_stdout,
                                         mocked_utils_exit):
         # Indicate that no file name was given as command line argument
-        args = self._make_args({'id': '1234', 'file': None, 'progress': False})
+        args = self._make_args({'id': '1234', 'file': None, 'progress': False,
+                                'allow_md5_fallback': False})
         # Indicate that no file is specified for output redirection
         mocked_stdout.isatty = lambda: True
         test_shell.do_image_download(self.gc, args)
@@ -1835,7 +1849,8 @@ class ShellV2Test(testtools.TestCase):
     def test_do_image_download_with_forbidden_id(self, mocked_print_err,
                                                  mocked_stdout):
         args = self._make_args({'id': 'IMG-01', 'file': None,
-                                'progress': False})
+                                'progress': False,
+                                'allow_md5_fallback': False})
         mocked_stdout.isatty = lambda: False
         with mock.patch.object(self.gc.images, 'data') as mocked_data:
             mocked_data.side_effect = exc.HTTPForbidden
@@ -1852,7 +1867,8 @@ class ShellV2Test(testtools.TestCase):
     @mock.patch.object(utils, 'print_err')
     def test_do_image_download_with_500(self, mocked_print_err, mocked_stdout):
         args = self._make_args({'id': 'IMG-01', 'file': None,
-                                'progress': False})
+                                'progress': False,
+                                'allow_md5_fallback': False})
         mocked_stdout.isatty = lambda: False
         with mock.patch.object(self.gc.images, 'data') as mocked_data:
             mocked_data.side_effect = exc.HTTPInternalServerError