21936036 problem in SERVICE/GLANCE
authorDanek Duvall <danek.duvall@oracle.com>
Mon, 19 Oct 2015 13:13:17 -0700
changeset 4990 ce7a7efc042b
parent 4989 26e5e37ce46e
child 4991 dba45c643059
21936036 problem in SERVICE/GLANCE
components/openstack/glance/patches/08-CVE-2015-5286.patch
components/openstack/glance/patches/09-CVE-2015-5286.patch
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/components/openstack/glance/patches/08-CVE-2015-5286.patch	Mon Oct 19 13:13:17 2015 -0700
@@ -0,0 +1,147 @@
+From 5447e8419d92f0cbb14de53b207e772ce9067933 Mon Sep 17 00:00:00 2001
+From: Mike Fedosin <[email protected]>
+Date: Sun, 20 Sep 2015 17:01:22 +0300
+Subject: [PATCH] Cleanup chunks for deleted image if token expired
+
+In patch I47229b366c25367ec1bd48aec684e0880f3dfe60 it was
+introduced the logic that if image was deleted during file
+upload when we want to update image status from 'saving'
+to 'active' it's expected to get Duplicate error and delete
+stale chunks after that. But if user's token is expired
+there will be Unathorized exception and chunks will stay
+in store and clog it.
+And when, the upload operation for such an image is
+completed the operator configured quota can be exceeded.
+
+This patch fixes the issue of left over chunks for an image
+which was deleted from saving status, by correcly handle
+auth exceptions from registry server.
+
+This patch fixes the issue of left over chunks for an image
+which was deleted from saving status, by correctly handle
+auth exceptions from registry server.
+
+Partial-bug: #1498163
+
+Conflicts:
+	glance/api/v1/upload_utils.py
+        (Kilo catches NotFound instead of ImagenotFound)
+
+Change-Id: I17a66eca55bfb83107046910e69c4da01415deec
+(cherry picked from commit 50e3a7c58a9862206d92fef577540c5b144ecbf0)
+---
+ glance/api/v1/upload_utils.py                    |  8 ++++++++
+ glance/api/v2/image_data.py                      | 14 ++++++++++++-
+ glance/tests/unit/v1/test_upload_utils.py        | 26 ++++++++++++++++++++++++
+ glance/tests/unit/v2/test_image_data_resource.py | 17 ++++++++++++++++
+ 4 files changed, 64 insertions(+), 1 deletion(-)
+
+diff --git a/glance/api/v1/upload_utils.py b/glance/api/v1/upload_utils.py
+index e587319..f9bdc29 100644
+--- a/glance/api/v1/upload_utils.py
++++ b/glance/api/v1/upload_utils.py
[email protected]@ -171,6 +171,14 @@ def upload_data_to_store(req, image_meta, image_data, store, notifier):
+                     raise exception.NotFound()
+                 else:
+                     raise
++
++        except exception.NotAuthenticated as e:
++            # Delete image data due to possible token expiration.
++            LOG.debug("Authentication error - the token may have "
++                      "expired during file upload. Deleting image data for "
++                      " %s " % image_id)
++            initiate_deletion(req, location_data, image_id)
++            raise webob.exc.HTTPUnauthorized(explanation=e.msg, request=req)
+         except exception.NotFound:
+             msg = _LI("Image %s could not be found after upload. The image may"
+                       " have been deleted during the upload.") % image_id
+diff --git a/glance/api/v2/image_data.py b/glance/api/v2/image_data.py
+index cdfa34b..ee9d0ba 100644
+--- a/glance/api/v2/image_data.py
++++ b/glance/api/v2/image_data.py
[email protected]@ -90,7 +90,19 @@ class ImageDataController(object):
+                 raise webob.exc.HTTPGone(explanation=msg,
+                                          request=req,
+                                          content_type='text/plain')
+-
++            except exception.NotAuthenticated:
++                msg = (_("Authentication error - the token may have "
++                         "expired during file upload. Deleting image data for "
++                         "%s.") % image_id)
++                LOG.debug(msg)
++                try:
++                    image.delete()
++                except exception.NotAuthenticated:
++                    # NOTE: Ignore this exception
++                    pass
++                raise webob.exc.HTTPUnauthorized(explanation=msg,
++                                                 request=req,
++                                                 content_type='text/plain')
+         except ValueError as e:
+             LOG.debug("Cannot save data for image %(id)s: %(e)s",
+                       {'id': image_id, 'e': utils.exception_to_str(e)})
+diff --git a/glance/tests/unit/v1/test_upload_utils.py b/glance/tests/unit/v1/test_upload_utils.py
+index 083cda3..f561dbe 100644
+--- a/glance/tests/unit/v1/test_upload_utils.py
++++ b/glance/tests/unit/v1/test_upload_utils.py
[email protected]@ -323,3 +323,29 @@ class TestUploadUtils(base.StoreClearingUnitTest):
+                                   'metadata': {}}, image_meta['id'])
+                         mock_safe_kill.assert_called_once_with(
+                             req, image_meta['id'], 'saving')
++
++    @mock.patch.object(registry, 'update_image_metadata',
++                       side_effect=exception.NotAuthenticated)
++    @mock.patch.object(upload_utils, 'initiate_deletion')
++    def test_activate_image_with_expired_token(
++            self, mocked_delete, mocked_update):
++        """Test token expiration during image upload.
++
++        If users token expired before image was uploaded then if auth error
++        was caught from registry during changing image status from 'saving'
++        to 'active' then it's required to delete all image data.
++        """
++        context = mock.Mock()
++        req = mock.Mock()
++        req.context = context
++        with self._get_store_and_notifier() as (location, checksum, image_meta,
++                                                image_data, store, notifier,
++                                                update_data):
++            self.assertRaises(webob.exc.HTTPUnauthorized,
++                              upload_utils.upload_data_to_store,
++                              req, image_meta, image_data, store, notifier)
++            self.assertEqual(2, mocked_update.call_count)
++            mocked_delete.assert_called_once_with(
++                req,
++                {'url': 'file://foo/bar', 'status': 'active', 'metadata': {}},
++                'c80a1a6c-bd1f-41c5-90ee-81afedb1d58d')
+diff --git a/glance/tests/unit/v2/test_image_data_resource.py b/glance/tests/unit/v2/test_image_data_resource.py
+index a121e82..b7bacab 100644
+--- a/glance/tests/unit/v2/test_image_data_resource.py
++++ b/glance/tests/unit/v2/test_image_data_resource.py
[email protected]@ -183,6 +183,23 @@ class TestImagesController(base.StoreClearingUnitTest):
+         self.assertRaises(webob.exc.HTTPBadRequest, self.controller.upload,
+                           request, unit_test_utils.UUID1, 'YYYY', 4)
+ 
++    def test_upload_with_expired_token(self):
++        def side_effect(image, from_state=None):
++            if from_state == 'saving':
++                raise exception.NotAuthenticated()
++
++        mocked_save = mock.Mock(side_effect=side_effect)
++        mocked_delete = mock.Mock()
++        request = unit_test_utils.get_fake_request()
++        image = FakeImage('abcd')
++        image.delete = mocked_delete
++        self.image_repo.result = image
++        self.image_repo.save = mocked_save
++        self.assertRaises(webob.exc.HTTPUnauthorized, self.controller.upload,
++                          request, unit_test_utils.UUID1, 'YYYY', 4)
++        self.assertEqual(3, mocked_save.call_count)
++        mocked_delete.assert_called_once_with()
++
+     def test_upload_non_existent_image_during_save_initiates_deletion(self):
+         def fake_save_not_found(self):
+             raise exception.NotFound()
+-- 
+1.9.1
+
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/components/openstack/glance/patches/09-CVE-2015-5286.patch	Mon Oct 19 13:13:17 2015 -0700
@@ -0,0 +1,80 @@
+From 868d9161c60f839cbf53393b804d7c0c41338e5b Mon Sep 17 00:00:00 2001
+From: Mike Fedosin <[email protected]>
+Date: Thu, 1 Oct 2015 18:28:48 +0300
+Subject: [PATCH] Catch NotAuthenticated exception in import task
+
+If glance uses registry as data_api then it's possible
+that token may expire during image import task and Glance
+will have NotUauthenticated exception.
+
+This code adds a correct handling of this exception and
+allows Glance to remove stale chunks from store.
+
+Partial-Bug: #1498163
+
+Change-Id: Ia6e1fe0d27b13b920ee7e728feb5305dec77e066
+(cherry picked from ebdf076cc9bd5d9239cdc96c6e7cecc72f852bbb)
+---
+ glance/common/scripts/image_import/main.py         |  3 ++-
+ .../unit/common/scripts/image_import/test_main.py  | 26 ++++++++++++++++++++++
+ 2 files changed, 28 insertions(+), 1 deletion(-)
+
+diff --git a/glance/common/scripts/image_import/main.py b/glance/common/scripts/image_import/main.py
+index ced93c9..5e9dc5c 100644
+--- a/glance/common/scripts/image_import/main.py
++++ b/glance/common/scripts/image_import/main.py
[email protected]@ -109,7 +109,8 @@ def import_image(image_repo, image_factory, task_input, task_id, uri):
+                     "processing.") % {"image_id": image_id,
+                                       "task_id": task_id}
+             raise exception.Conflict(msg)
+-    except (exception.Conflict, exception.NotFound):
++    except (exception.Conflict, exception.NotFound,
++            exception.NotAuthenticated):
+         with excutils.save_and_reraise_exception():
+             if new_image.locations:
+                 for location in new_image.locations:
+diff --git a/glance/tests/unit/common/scripts/image_import/test_main.py b/glance/tests/unit/common/scripts/image_import/test_main.py
+index a81a66c..78ba8a2 100644
+--- a/glance/tests/unit/common/scripts/image_import/test_main.py
++++ b/glance/tests/unit/common/scripts/image_import/test_main.py
[email protected]@ -16,7 +16,10 @@
+ import mock
+ import urllib2
+ 
++import glance.common.exception as exception
+ from glance.common.scripts.image_import import main as image_import_script
++from glance.common import store_utils
++
+ import glance.tests.utils as test_utils
+ 
+ 
[email protected]@ -91,3 +94,26 @@ class TestImageImport(test_utils.BaseTestCase):
+         image = mock.Mock()
+         self.assertRaises(urllib2.URLError,
+                           image_import_script.set_image_data, image, uri, None)
++
++    @mock.patch.object(image_import_script, 'create_image')
++    @mock.patch.object(image_import_script, 'set_image_data')
++    @mock.patch.object(store_utils, 'delete_image_location_from_backend')
++    def test_import_image_failed_with_expired_token(
++            self, mock_delete_data, mock_set_img_data, mock_create_image):
++        image_id = mock.ANY
++        locations = ['location']
++        image = mock.Mock(image_id=image_id, locations=locations)
++        image_repo = mock.Mock()
++        image_repo.get.side_effect = [image, exception.NotAuthenticated]
++        image_factory = mock.ANY
++        task_input = mock.Mock(image_properties=mock.ANY)
++        uri = mock.ANY
++
++        mock_create_image.return_value = image
++        self.assertRaises(exception.NotAuthenticated,
++                          image_import_script.import_image,
++                          image_repo, image_factory,
++                          task_input, None, uri)
++        self.assertEqual(1, mock_set_img_data.call_count)
++        mock_delete_data.assert_called_once_with(
++            mock_create_image().context, image_id, 'location')
+-- 
+1.9.1
+