[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#776585: unblock: glance/2014.1.3-12 (CVE-2014-9623: Glance user storage quota bypass)



Package: release.debian.org
Severity: normal
User: release.debian.org@packages.debian.org
Usertags: unblock

Dear release team,

Please unblock glance/2014.1.3-12. It contains a fix for quota bypass. Debdiff
attached (full explanation of the issue in the patch).

Cheers,

Thomas Goirand (zigo)
diff -Nru glance-2014.1.3/debian/changelog glance-2014.1.3/debian/changelog
--- glance-2014.1.3/debian/changelog	2015-01-21 16:24:22.000000000 +0000
+++ glance-2014.1.3/debian/changelog	2015-01-29 15:22:59.000000000 +0000
@@ -1,3 +1,11 @@
+glance (2014.1.3-12) unstable; urgency=high
+
+  * CVE-2014-9623: Glance user storage quota bypass. Applied upstream patch:
+    Cleanup_chunks_for_deleted_image_that_was_saving_icehouse.patch
+    (Closes: #776580).
+
+ -- Thomas Goirand <zigo@debian.org>  Thu, 29 Jan 2015 16:21:39 +0100
+
 glance (2014.1.3-11) unstable; urgency=high
 
   * CVE-2015-1195: fixes "Glance still allows users to download and delete any
diff -Nru glance-2014.1.3/debian/patches/CVE-2014-9623_Cleanup_chunks_for_deleted_image_that_was_saving_icehouse.patch glance-2014.1.3/debian/patches/CVE-2014-9623_Cleanup_chunks_for_deleted_image_that_was_saving_icehouse.patch
--- glance-2014.1.3/debian/patches/CVE-2014-9623_Cleanup_chunks_for_deleted_image_that_was_saving_icehouse.patch	1970-01-01 00:00:00.000000000 +0000
+++ glance-2014.1.3/debian/patches/CVE-2014-9623_Cleanup_chunks_for_deleted_image_that_was_saving_icehouse.patch	2015-01-29 15:22:59.000000000 +0000
@@ -0,0 +1,466 @@
+Subject: Cleanup chunks for deleted image that was 'saving'
+ Currently image data cannot be removed synchronously for an image that
+ is in saving state. 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. However, by the limitation of the design we
+ cannot enforce a global quota check for the image in saving status.
+ This change introduces a inconsonance between http response codes of
+ v1 and v2 APIs. The status codes which we will now see after the upload
+ process completes on an image which was deleted mid way are:
+ .
+  v1: 412 Precondition Failed
+  v2: 410 Gone
+Author: Zhi Yan Liu <zhiyanl@cn.ibm.com>
+Origin: upstream, https://review.openstack.org/#/c/149646/
+Date: Tue, 30 Dec 2014 14:25:50 +0000 (+0800)
+X-Git-Url: https://review.openstack.org/gitweb?p=openstack%2Fglance.git;a=commitdiff_plain;h=f1260cc771ee068651aa62b972bef49d9af81eb0
+Bug-Ubuntu: https://launchpad.net/bugs/1383973
+Bug-Ubuntu: https://launchpad.net/bugs/1398830
+Bug-Ubuntu: https://launchpad.net/bugs/1188532
+Bug-Debian: https://bugs.debian.org/776580
+Last-Update: 2015-01-29
+
+Index: glance/glance/api/authorization.py
+===================================================================
+--- glance.orig/glance/api/authorization.py	2015-01-29 16:05:14.000000000 +0100
++++ glance/glance/api/authorization.py	2015-01-29 16:21:26.000000000 +0100
+@@ -147,10 +147,10 @@
+             raise exception.Forbidden(message
+                                       % self.image.image_id)
+ 
+-    def save(self, image_member):
++    def save(self, image_member, from_state=None):
+         if (self.context.is_admin or
+                 self.context.owner == image_member.member_id):
+-            self.member_repo.save(image_member)
++            self.member_repo.save(image_member, from_state=from_state)
+         else:
+             message = _("You cannot update image member %s")
+             raise exception.Forbidden(message % image_member.member_id)
+Index: glance/glance/api/policy.py
+===================================================================
+--- glance.orig/glance/api/policy.py	2015-01-29 16:05:14.000000000 +0100
++++ glance/glance/api/policy.py	2015-01-29 16:21:26.000000000 +0100
+@@ -182,9 +182,9 @@
+         self.policy.enforce(self.context, 'get_images', {})
+         return super(ImageRepoProxy, self).list(*args, **kwargs)
+ 
+-    def save(self, image):
++    def save(self, image, from_state=None):
+         self.policy.enforce(self.context, 'modify_image', {})
+-        return super(ImageRepoProxy, self).save(image)
++        return super(ImageRepoProxy, self).save(image, from_state=from_state)
+ 
+     def add(self, image):
+         self.policy.enforce(self.context, 'add_image', {})
+@@ -283,9 +283,9 @@
+         self.policy.enforce(self.context, 'get_member', {})
+         return self.member_repo.get(member_id)
+ 
+-    def save(self, member):
++    def save(self, member, from_state=None):
+         self.policy.enforce(self.context, 'modify_member', {})
+-        self.member_repo.save(member)
++        self.member_repo.save(member, from_state=from_state)
+ 
+     def list(self, *args, **kwargs):
+         self.policy.enforce(self.context, 'get_members', {})
+Index: glance/glance/api/v1/upload_utils.py
+===================================================================
+--- glance.orig/glance/api/v1/upload_utils.py	2015-01-29 16:05:14.000000000 +0100
++++ glance/glance/api/v1/upload_utils.py	2015-01-29 16:21:26.000000000 +0100
+@@ -146,14 +146,21 @@
+         update_data = {'checksum': checksum,
+                        'size': size}
+         try:
+-            image_meta = registry.update_image_metadata(req.context,
+-                                                        image_id,
+-                                                        update_data,
+-                                                        from_state='saving')
+-
+-        except exception.NotFound as e:
+-            msg = _("Image %s could not be found after upload. The image may "
+-                    "have been deleted during the upload.") % image_id
++            try:
++                state = 'saving'
++                image_meta = registry.update_image_metadata(req.context,
++                                                            image_id,
++                                                            update_data,
++                                                            from_state=state)
++            except exception.Duplicate:
++                image = registry.get_image_metadata(req.context, image_id)
++                if image['status'] == 'deleted':
++                    raise exception.NotFound()
++                else:
++                    raise
++        except exception.NotFound:
++            msg = _("Image %s could not be found after upload. The image may"
++                    " have been deleted during the upload.") % image_id
+             LOG.info(msg)
+ 
+             # NOTE(jculp): we need to clean up the datastore if an image
+Index: glance/glance/api/v2/image_data.py
+===================================================================
+--- glance.orig/glance/api/v2/image_data.py	2015-01-29 16:05:14.000000000 +0100
++++ glance/glance/api/v2/image_data.py	2015-01-29 16:21:26.000000000 +0100
+@@ -22,6 +22,7 @@
+ import glance.db
+ import glance.gateway
+ import glance.notifier
++from glance.openstack.common import excutils
+ import glance.openstack.common.log as logging
+ import glance.store
+ 
+@@ -66,13 +67,12 @@
+             try:
+                 image_repo.save(image)
+                 image.set_data(data, size)
+-                image_repo.save(image)
+-            except exception.NotFound as e:
+-                msg = (_("Image %(id)s could not be found after upload."
+-                         "The image may have been deleted during the upload: "
+-                         "%(error)s Cleaning up the chunks uploaded") %
+-                       {'id': image_id,
+-                        'error': e})
++                image_repo.save(image, from_state='saving')
++            except (exception.NotFound, exception.Conflict):
++                msg = (_("Image %s could not be found after upload. "
++                         "The image may have been deleted during the "
++                         "upload, cleaning up the chunks uploaded.") %
++                       image_id)
+                 LOG.warn(msg)
+                 # NOTE(sridevi): Cleaning up the uploaded chunks.
+                 try:
+@@ -131,6 +131,10 @@
+             raise webob.exc.HTTPServiceUnavailable(explanation=msg,
+                                                    request=req)
+ 
++        except webob.exc.HTTPGone as e:
++            with excutils.save_and_reraise_exception():
++                LOG.error(_("Failed to upload image data due to HTTP error"))
++
+         except webob.exc.HTTPError as e:
+             LOG.error(_("Failed to upload image data due to HTTP error"))
+             self._restore(image_repo, image)
+Index: glance/glance/db/__init__.py
+===================================================================
+--- glance.orig/glance/db/__init__.py	2015-01-29 16:05:14.000000000 +0100
++++ glance/glance/db/__init__.py	2015-01-29 16:21:26.000000000 +0100
+@@ -162,7 +162,7 @@
+         image.created_at = new_values['created_at']
+         image.updated_at = new_values['updated_at']
+ 
+-    def save(self, image):
++    def save(self, image, from_state=None):
+         image_values = self._format_image_to_db(image)
+         if image_values['size'] > CONF.image_size_cap:
+             raise exception.ImageSizeLimitExceeded
+@@ -170,7 +170,8 @@
+             new_values = self.db_api.image_update(self.context,
+                                                   image.image_id,
+                                                   image_values,
+-                                                  purge_props=True)
++                                                  purge_props=True,
++                                                  from_state=from_state)
+         except (exception.NotFound, exception.Forbidden):
+             msg = _("No image found with ID %s") % image.image_id
+             raise exception.NotFound(msg)
+@@ -263,7 +264,7 @@
+             msg = _("The specified member %s could not be found")
+             raise exception.NotFound(msg % image_member.id)
+ 
+-    def save(self, image_member):
++    def save(self, image_member, from_state=None):
+         image_member_values = self._format_image_member_to_db(image_member)
+         try:
+             new_values = self.db_api.image_member_update(self.context,
+Index: glance/glance/domain/proxy.py
+===================================================================
+--- glance.orig/glance/domain/proxy.py	2015-01-29 16:05:14.000000000 +0100
++++ glance/glance/domain/proxy.py	2015-01-29 16:21:26.000000000 +0100
+@@ -94,9 +94,9 @@
+         result = self.base.add(base_item)
+         return self.helper.proxy(result)
+ 
+-    def save(self, item):
++    def save(self, item, from_state=None):
+         base_item = self.helper.unproxy(item)
+-        result = self.base.save(base_item)
++        result = self.base.save(base_item, from_state=from_state)
+         return self.helper.proxy(result)
+ 
+     def remove(self, item):
+Index: glance/glance/notifier.py
+===================================================================
+--- glance.orig/glance/notifier.py	2015-01-29 16:05:14.000000000 +0100
++++ glance/glance/notifier.py	2015-01-29 16:21:26.000000000 +0100
+@@ -178,8 +178,8 @@
+                                              item_proxy_class=ImageProxy,
+                                              item_proxy_kwargs=proxy_kwargs)
+ 
+-    def save(self, image):
+-        super(ImageRepoProxy, self).save(image)
++    def save(self, image, from_state=None):
++        super(ImageRepoProxy, self).save(image, from_state=from_state)
+         self.notifier.info('image.update',
+                            format_image_notification(image))
+ 
+Index: glance/glance/quota/__init__.py
+===================================================================
+--- glance.orig/glance/quota/__init__.py	2015-01-29 16:05:14.000000000 +0100
++++ glance/glance/quota/__init__.py	2015-01-29 16:21:26.000000000 +0100
+@@ -96,9 +96,9 @@
+             raise exception.ImagePropertyLimitExceeded(attempted=attempted,
+                                                        maximum=maximum)
+ 
+-    def save(self, image):
++    def save(self, image, from_state=None):
+         self._enforce_image_property_quota(image)
+-        super(ImageRepoProxy, self).save(image)
++        return super(ImageRepoProxy, self).save(image, from_state=from_state)
+ 
+     def add(self, image):
+         self._enforce_image_property_quota(image)
+Index: glance/glance/store/__init__.py
+===================================================================
+--- glance.orig/glance/store/__init__.py	2015-01-29 16:20:22.000000000 +0100
++++ glance/glance/store/__init__.py	2015-01-29 16:21:26.000000000 +0100
+@@ -468,7 +468,7 @@
+         self._set_acls(image)
+         return result
+ 
+-    def save(self, image):
++    def save(self, image, from_state=None):
+         result = super(ImageRepoProxy, self).save(image)
+         self._set_acls(image)
+         return result
+Index: glance/glance/tests/unit/test_domain_proxy.py
+===================================================================
+--- glance.orig/glance/tests/unit/test_domain_proxy.py	2015-01-29 16:05:14.000000000 +0100
++++ glance/glance/tests/unit/test_domain_proxy.py	2015-01-29 16:21:26.000000000 +0100
+@@ -74,7 +74,7 @@
+         self._test_method('add', 'snuff', 'enough')
+ 
+     def test_save(self):
+-        self._test_method('save', 'snuff', 'enough')
++        self._test_method('save', 'snuff', 'enough', from_state=None)
+ 
+     def test_remove(self):
+         self._test_method('add', None, 'flying')
+@@ -121,14 +121,14 @@
+             self.assertEqual(results[i].args, tuple())
+             self.assertEqual(results[i].kwargs, {'a': 1})
+ 
+-    def _test_method_with_proxied_argument(self, name, result):
++    def _test_method_with_proxied_argument(self, name, result, **kwargs):
+         self.fake_repo.result = result
+         item = FakeProxy('snoop')
+         method = getattr(self.proxy_repo, name)
+         proxy_result = method(item)
+ 
+-        self.assertEqual(self.fake_repo.args, ('snoop',))
+-        self.assertEqual(self.fake_repo.kwargs, {})
++        self.assertEqual(('snoop',), self.fake_repo.args)
++        self.assertEqual(kwargs, self.fake_repo.kwargs)
+ 
+         if result is None:
+             self.assertTrue(proxy_result is None)
+@@ -145,10 +145,12 @@
+         self._test_method_with_proxied_argument('add', None)
+ 
+     def test_save(self):
+-        self._test_method_with_proxied_argument('save', 'dog')
++        self._test_method_with_proxied_argument('save', 'dog',
++                                                from_state=None)
+ 
+     def test_save_with_no_result(self):
+-        self._test_method_with_proxied_argument('save', None)
++        self._test_method_with_proxied_argument('save', None,
++                                                from_state=None)
+ 
+     def test_remove(self):
+         self._test_method_with_proxied_argument('remove', 'dog')
+Index: glance/glance/tests/unit/test_policy.py
+===================================================================
+--- glance.orig/glance/tests/unit/test_policy.py	2015-01-29 16:05:14.000000000 +0100
++++ glance/glance/tests/unit/test_policy.py	2015-01-29 16:21:26.000000000 +0100
+@@ -69,7 +69,7 @@
+     def get(self, *args, **kwargs):
+         return 'member_repo_get'
+ 
+-    def save(self, image_member):
++    def save(self, image_member, from_state=None):
+         image_member.output = 'member_repo_save'
+ 
+     def list(self, *args, **kwargs):
+Index: glance/glance/tests/unit/test_quota.py
+===================================================================
+--- glance.orig/glance/tests/unit/test_quota.py	2015-01-29 16:05:14.000000000 +0100
++++ glance/glance/tests/unit/test_quota.py	2015-01-29 16:21:26.000000000 +0100
+@@ -290,7 +290,8 @@
+         self.image.extra_properties = {'foo': 'bar'}
+         self.image_repo_proxy.save(self.image)
+ 
+-        self.image_repo_mock.save.assert_called_once_with(self.base_image)
++        self.image_repo_mock.save.assert_called_once_with(self.base_image,
++                                                          from_state=None)
+ 
+     def test_save_image_too_many_image_properties(self):
+         self.config(image_property_quota=1)
+@@ -306,7 +307,8 @@
+         self.image.extra_properties = {'foo': 'bar'}
+         self.image_repo_proxy.save(self.image)
+ 
+-        self.image_repo_mock.save.assert_called_once_with(self.base_image)
++        self.image_repo_mock.save.assert_called_once_with(self.base_image,
++                                                          from_state=None)
+ 
+     def test_add_image_with_image_property(self):
+         self.config(image_property_quota=1)
+Index: glance/glance/tests/unit/test_store_image.py
+===================================================================
+--- glance.orig/glance/tests/unit/test_store_image.py	2015-01-29 16:20:22.000000000 +0100
++++ glance/glance/tests/unit/test_store_image.py	2015-01-29 16:21:26.000000000 +0100
+@@ -34,7 +34,7 @@
+     def add(self, image):
+         return image
+ 
+-    def save(self, image):
++    def save(self, image, from_state=None):
+         return image
+ 
+ 
+Index: glance/glance/tests/unit/v1/test_api.py
+===================================================================
+--- glance.orig/glance/tests/unit/v1/test_api.py	2015-01-29 16:20:22.000000000 +0100
++++ glance/glance/tests/unit/v1/test_api.py	2015-01-29 16:21:26.000000000 +0100
+@@ -1633,8 +1633,7 @@
+ 
+         self.assertEqual(1, mock_store_add_to_backend.call_count)
+ 
+-    def test_delete_during_image_upload(self):
+-        req = unit_test_utils.get_fake_request()
++    def _check_delete_during_image_upload(self, is_admin=False):
+ 
+         fixture_headers = {'x-image-meta-store': 'file',
+                            'x-image-meta-disk-format': 'vhd',
+@@ -1668,30 +1667,18 @@
+                        mock_initiate_deletion)
+ 
+         orig_update_image_metadata = registry.update_image_metadata
+-        ctlr = glance.api.v1.controller.BaseController
+-        orig_get_image_meta_or_404 = ctlr.get_image_meta_or_404
+ 
+-        def mock_update_image_metadata(*args, **kwargs):
++        data = "somedata"
+ 
+-            if args[2].get('status', None) == 'deleted':
++        def mock_update_image_metadata(*args, **kwargs):
+ 
+-                # One shot.
+-                def mock_get_image_meta_or_404(*args, **kwargs):
+-                    ret = orig_get_image_meta_or_404(*args, **kwargs)
+-                    ret['status'] = 'queued'
+-                    self.stubs.Set(ctlr, 'get_image_meta_or_404',
+-                                   orig_get_image_meta_or_404)
+-                    return ret
+-
+-                self.stubs.Set(ctlr, 'get_image_meta_or_404',
+-                               mock_get_image_meta_or_404)
+-
+-                req = webob.Request.blank("/images/%s" % image_id)
+-                req.method = 'PUT'
+-                req.headers['Content-Type'] = 'application/octet-stream'
+-                req.body = "somedata"
++            if args[2].get('size', None) == len(data):
++                path = "/images/%s" % image_id
++                req = unit_test_utils.get_fake_request(path=path,
++                                                       method='DELETE',
++                                                       is_admin=is_admin)
+                 res = req.get_response(self.api)
+-                self.assertEqual(res.status_int, 200)
++                self.assertEqual(200, res.status_int)
+ 
+                 self.stubs.Set(registry, 'update_image_metadata',
+                                orig_update_image_metadata)
+@@ -1701,20 +1688,30 @@
+         self.stubs.Set(registry, 'update_image_metadata',
+                        mock_update_image_metadata)
+ 
+-        req = webob.Request.blank("/images/%s" % image_id)
+-        req.method = 'DELETE'
++        req = unit_test_utils.get_fake_request(path="/images/%s" % image_id,
++                                               method='PUT')
++        req.headers['Content-Type'] = 'application/octet-stream'
++        req.body = data
+         res = req.get_response(self.api)
+-        self.assertEqual(res.status_int, 200)
++        self.assertEqual(412, res.status_int)
++        self.assertFalse(res.location)
+ 
+         self.assertTrue(called['initiate_deletion'])
+ 
+-        req = webob.Request.blank("/images/%s" % image_id)
+-        req.method = 'HEAD'
++        req = unit_test_utils.get_fake_request(path="/images/%s" % image_id,
++                                               method='HEAD',
++                                               is_admin=True)
+         res = req.get_response(self.api)
+         self.assertEqual(res.status_int, 200)
+         self.assertEqual(res.headers['x-image-meta-deleted'], 'True')
+         self.assertEqual(res.headers['x-image-meta-status'], 'deleted')
+ 
++    def test_delete_during_image_upload_by_normal_user(self):
++        self._check_delete_during_image_upload(is_admin=False)
++
++    def test_delete_during_image_upload_by_admin(self):
++        self._check_delete_during_image_upload(is_admin=True)
++
+     def test_disable_purge_props(self):
+         """
+         Test the special x-glance-registry-purge-props header controls
+Index: glance/glance/tests/unit/v2/test_image_data_resource.py
+===================================================================
+--- glance.orig/glance/tests/unit/v2/test_image_data_resource.py	2015-01-29 16:05:14.000000000 +0100
++++ glance/glance/tests/unit/v2/test_image_data_resource.py	2015-01-29 16:21:26.000000000 +0100
+@@ -79,7 +79,7 @@
+         else:
+             return self.result
+ 
+-    def save(self, image):
++    def save(self, image, from_state=None):
+         self.saved_image = image
+ 
+ 
+@@ -180,17 +180,21 @@
+                           request, unit_test_utils.UUID1, 'YYYY', 4)
+ 
+     def test_upload_non_existent_image_during_save_initiates_deletion(self):
+-        def fake_save(self):
++        def fake_save_not_found(self):
+             raise exception.NotFound()
+ 
+-        request = unit_test_utils.get_fake_request()
+-        image = FakeImage('abcd', locations=['http://example.com/image'])
+-        self.image_repo.result = image
+-        self.image_repo.save = fake_save
+-        image.delete = mock.Mock()
+-        self.assertRaises(webob.exc.HTTPGone, self.controller.upload,
+-                          request, str(uuid.uuid4()), 'ABC', 3)
+-        self.assertTrue(image.delete.called)
++        def fake_save_conflict(self):
++            raise exception.Conflict()
++
++        for fun in [fake_save_not_found, fake_save_conflict]:
++            request = unit_test_utils.get_fake_request()
++            image = FakeImage('abcd', locations=['http://example.com/image'])
++            self.image_repo.result = image
++            self.image_repo.save = fun
++            image.delete = mock.Mock()
++            self.assertRaises(webob.exc.HTTPGone, self.controller.upload,
++                              request, str(uuid.uuid4()), 'ABC', 3)
++            self.assertTrue(image.delete.called)
+ 
+     def test_upload_non_existent_image_before_save(self):
+         request = unit_test_utils.get_fake_request()
diff -Nru glance-2014.1.3/debian/patches/series glance-2014.1.3/debian/patches/series
--- glance-2014.1.3/debian/patches/series	2015-01-21 16:24:22.000000000 +0000
+++ glance-2014.1.3/debian/patches/series	2015-01-29 15:22:59.000000000 +0000
@@ -3,3 +3,4 @@
 sql_conn-registry.patch
 restrict_client_download_and_delete_files_in_glance-api.patch
 CVE-2015-1195_Prevent_file_swift_config_and_filesystem_schemes.patch
+CVE-2014-9623_Cleanup_chunks_for_deleted_image_that_was_saving_icehouse.patch

Reply to: