[gentoo-commits] gentoo-x86 commit in app-admin/glance/files: glance-CVE-2014-9623.patch

[Matt Thode (prometheanfire)]

prometheanfire    15/01/29 05:39:38

  Added:                glance-CVE-2014-9623.patch
  Log:
  bumping glance for CVE-2014-9623
  
  (Portage version: 2.2.14/cvs/Linux x86_64, signed Manifest commit with key 0x33ED3FD25AFC78BA)

Revision  Changes    Path
1.1                  app-admin/glance/files/glance-CVE-2014-9623.patch

file : http://sources.gentoo.org/viewvc.cgi/gentoo-x86/app-admin/glance/files/glance-CVE-2014-9623.patch?rev=1.1&view=markup
plain: http://sources.gentoo.org/viewvc.cgi/gentoo-x86/app-admin/glance/files/glance-CVE-2014-9623.patch?rev=1.1&content-type=text/plain

Index: glance-CVE-2014-9623.patch
===================================================================
From 7d5d8657fd70b20518610b3c6f8e41e16c72fa31 Mon Sep 17 00:00:00 2001
From: Zhi Yan Liu <zhiyanl@××××××.com>
Date: Tue, 30 Dec 2014 22:25:50 +0800
Subject: [PATCH] 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

SecurityImpact
UpgradeImpact
APIImpact

Closes-Bug: 1383973
Closes-Bug: 1398830
Closes-Bug: 1188532

Conflicts:
	glance/api/v1/upload_utils.py
	glance/api/v2/image_data.py
	glance/tests/unit/test_domain_proxy.py
	glance/tests/unit/v1/test_api.py

Change-Id: I47229b366c25367ec1bd48aec684e0880f3dfe60
Signed-off-by: Zhi Yan Liu <zhiyanl@××××××.com>
(cherry picked from commit 0dc8fbb3479a53c5bba8475d14f4c7206904c5ea)
---
 glance/api/authorization.py                      |  4 +-
 glance/api/policy.py                             |  8 ++--
 glance/api/v1/upload_utils.py                    | 19 +++++---
 glance/api/v2/image_data.py                      |  8 +++-
 glance/db/__init__.py                            |  7 +--
 glance/domain/proxy.py                           |  4 +-
 glance/location.py                               |  4 +-
 glance/notifier.py                               |  4 +-
 glance/quota/__init__.py                         |  4 +-
 glance/tests/unit/test_domain_proxy.py           | 14 +++---
 glance/tests/unit/test_policy.py                 |  2 +-
 glance/tests/unit/test_quota.py                  | 17 ++++---
 glance/tests/unit/test_store_image.py            |  2 +-
 glance/tests/unit/v1/test_api.py                 | 57 +++++++++++-------------
 glance/tests/unit/v2/test_image_data_resource.py | 24 +++++-----
 15 files changed, 98 insertions(+), 80 deletions(-)

diff --git a/glance/api/authorization.py b/glance/api/authorization.py
index 149ff55..77f7ed3 100644
--- a/glance/api/authorization.py
+++ b/glance/api/authorization.py
@@ -158,10 +158,10 @@ class ImageMemberRepoProxy(glance.domain.proxy.Repo):
             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)
diff --git a/glance/api/policy.py b/glance/api/policy.py
index 0bc8d56..e395876 100644
--- a/glance/api/policy.py
+++ b/glance/api/policy.py
@@ -182,9 +182,9 @@ class ImageRepoProxy(glance.domain.proxy.Repo):
         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', {})
@@ -285,9 +285,9 @@ class ImageMemberRepoProxy(glance.domain.proxy.Repo):
         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', {})
diff --git a/glance/api/v1/upload_utils.py b/glance/api/v1/upload_utils.py
index 8a190fc..60c3d3d 100644
--- a/glance/api/v1/upload_utils.py
+++ b/glance/api/v1/upload_utils.py
@@ -153,12 +153,19 @@ def upload_data_to_store(req, image_meta, image_data, store, notifier):
         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:
+            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 = _LI("Image %s could not be found after upload. The image may"
                       " have been deleted during the upload.") % image_id
             LOG.info(msg)
diff --git a/glance/api/v2/image_data.py b/glance/api/v2/image_data.py
index 430ffc5..cdfa34b 100644
--- a/glance/api/v2/image_data.py
+++ b/glance/api/v2/image_data.py
@@ -73,8 +73,8 @@ class ImageDataController(object):
             try:
                 image_repo.save(image)
                 image.set_data(data, size)
-                image_repo.save(image)
-            except exception.NotFound as e:
+                image_repo.save(image, from_state='saving')
+            except (exception.NotFound, exception.Conflict) 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") %
@@ -152,6 +152,10 @@ class ImageDataController(object):
             raise webob.exc.HTTPServiceUnavailable(explanation=msg,
                                                    request=req)
 
+        except webob.exc.HTTPGone as e:
+            with excutils.save_and_reraise_exception():
+                LOG.error(_LE("Failed to upload image data due to HTTP error"))
+
         except webob.exc.HTTPError as e:
             with excutils.save_and_reraise_exception():
                 LOG.error(_LE("Failed to upload image data due to HTTP error"))
diff --git a/glance/db/__init__.py b/glance/db/__init__.py
index 05db7f8..483ba21 100644
--- a/glance/db/__init__.py
+++ b/glance/db/__init__.py
@@ -164,7 +164,7 @@ class ImageRepo(object):
         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
@@ -172,7 +172,8 @@ class ImageRepo(object):
             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)
@@ -265,7 +266,7 @@ class ImageMemberRepo(object):
             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,
diff --git a/glance/domain/proxy.py b/glance/domain/proxy.py
index 5a91d34..09c0fb2 100644
--- a/glance/domain/proxy.py
+++ b/glance/domain/proxy.py
@@ -94,9 +94,9 @@ class Repo(object):
         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):
diff --git a/glance/location.py b/glance/location.py
index f83fa7a..b49546d 100644
--- a/glance/location.py
+++ b/glance/location.py
@@ -60,8 +60,8 @@ class ImageRepoProxy(glance.domain.proxy.Repo):
         self._set_acls(image)
         return result
 
-    def save(self, image):
-        result = super(ImageRepoProxy, self).save(image)
+    def save(self, image, from_state=None):
+        result = super(ImageRepoProxy, self).save(image, from_state=from_state)
         self._set_acls(image)
         return result
 
diff --git a/glance/notifier.py b/glance/notifier.py
index 5ec0854..21223da 100644
--- a/glance/notifier.py
+++ b/glance/notifier.py
@@ -122,8 +122,8 @@ class ImageRepoProxy(glance.domain.proxy.Repo):
                                              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))
 
diff --git a/glance/quota/__init__.py b/glance/quota/__init__.py
index 4051992..d628a8c 100644
--- a/glance/quota/__init__.py
+++ b/glance/quota/__init__.py
@@ -104,10 +104,10 @@ class ImageRepoProxy(glance.domain.proxy.Repo):
             LOG.debug(six.text_type(exc))
             raise exc
 
-    def save(self, image):
+    def save(self, image, from_state=None):
         if image.added_new_properties():
             self._enforce_image_property_quota(len(image.extra_properties))
-        return super(ImageRepoProxy, self).save(image)
+        return super(ImageRepoProxy, self).save(image, from_state=from_state)
 
     def add(self, image):
         self._enforce_image_property_quota(len(image.extra_properties))
diff --git a/glance/tests/unit/test_domain_proxy.py b/glance/tests/unit/test_domain_proxy.py
index 1bb6863..2b8f792 100644
--- a/glance/tests/unit/test_domain_proxy.py
+++ b/glance/tests/unit/test_domain_proxy.py
@@ -74,7 +74,7 @@ class TestProxyRepoPlain(test_utils.BaseTestCase):
         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 @@ class TestProxyRepoWrapping(test_utils.BaseTestCase):
             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.assertIsNone(proxy_result)
@@ -145,10 +145,12 @@ class TestProxyRepoWrapping(test_utils.BaseTestCase):
         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')
diff --git a/glance/tests/unit/test_policy.py b/glance/tests/unit/test_policy.py
index 5b5e870..44546a0 100644
--- a/glance/tests/unit/test_policy.py
+++ b/glance/tests/unit/test_policy.py
@@ -78,7 +78,7 @@ class MemberRepoStub(object):
     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):
diff --git a/glance/tests/unit/test_quota.py b/glance/tests/unit/test_quota.py
index c12eda2..1c40fb4 100644
--- a/glance/tests/unit/test_quota.py
+++ b/glance/tests/unit/test_quota.py
@@ -367,7 +367,8 @@ class TestImagePropertyQuotas(test_utils.BaseTestCase):
         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)
@@ -383,7 +384,8 @@ class TestImagePropertyQuotas(test_utils.BaseTestCase):
         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)
@@ -422,7 +424,8 @@ class TestImagePropertyQuotas(test_utils.BaseTestCase):
         self.config(image_property_quota=1)
         self.image.extra_properties = {'foo': 'frob', 'spam': 'eggs'}
         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)
         self.assertEqual('frob', self.base_image.extra_properties['foo'])
         self.assertEqual('eggs', self.base_image.extra_properties['spam'])
 
@@ -431,7 +434,8 @@ class TestImagePropertyQuotas(test_utils.BaseTestCase):
         self.config(image_property_quota=1)
         del self.image.extra_properties['foo']
         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)
         self.assertNotIn('foo', self.base_image.extra_properties)
         self.assertEqual('ham', self.base_image.extra_properties['spam'])
 
@@ -447,7 +451,7 @@ class TestImagePropertyQuotas(test_utils.BaseTestCase):
         del self.image.extra_properties['frob']
         del self.image.extra_properties['lorem']
         self.image_repo_proxy.save(self.image)
-        call_args = mock.call(self.base_image)
+        call_args = mock.call(self.base_image, from_state=None)
         self.assertEqual(call_args, self.image_repo_mock.save.call_args)
         self.assertEqual('bar', self.base_image.extra_properties['foo'])
         self.assertEqual('ham', self.base_image.extra_properties['spam'])
@@ -466,7 +470,8 @@ class TestImagePropertyQuotas(test_utils.BaseTestCase):
         self.config(image_property_quota=1)
         del self.image.extra_properties['foo']
         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)
         self.assertNotIn('foo', self.base_image.extra_properties)
         self.assertEqual('ham', self.base_image.extra_properties['spam'])
         self.assertEqual('baz', self.base_image.extra_properties['frob'])
diff --git a/glance/tests/unit/test_store_image.py b/glance/tests/unit/test_store_image.py
index 8b334ab..b656454 100644
--- a/glance/tests/unit/test_store_image.py
+++ b/glance/tests/unit/test_store_image.py
@@ -36,7 +36,7 @@ class ImageRepoStub(object):
     def add(self, image):
         return image
 
-    def save(self, image):
+    def save(self, image, from_state=None):
         return image
 
 
diff --git a/glance/tests/unit/v1/test_api.py b/glance/tests/unit/v1/test_api.py
index 39e9a44..7dc0737 100644
--- a/glance/tests/unit/v1/test_api.py
+++ b/glance/tests/unit/v1/test_api.py
@@ -39,7 +39,6 @@ from glance.db.sqlalchemy import api as db_api
 from glance.db.sqlalchemy import models as db_models
 from glance.openstack.common import jsonutils
 from glance.openstack.common import timeutils
-
 import glance.registry.client.v1.api as registry
 from glance.tests.unit import base
 import glance.tests.unit.utils as unit_test_utils
@@ -1735,8 +1734,7 @@ class TestGlanceAPI(base.IsolatedUnitTest):
 
         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',
@@ -1744,8 +1742,8 @@ class TestGlanceAPI(base.IsolatedUnitTest):
                            'x-image-meta-name': 'fake image #3',
                            'x-image-meta-property-key1': 'value1'}
 
-        req = webob.Request.blank("/images")
-        req.method = 'POST'
+        req = unit_test_utils.get_fake_request(path="/images",
+                                               is_admin=is_admin)
         for k, v in six.iteritems(fixture_headers):
             req.headers[k] = v
 
@@ -1770,31 +1768,18 @@ class TestGlanceAPI(base.IsolatedUnitTest):
                        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):
 
-            if args[2].get('status', None) == 'deleted':
+        data = "somedata"
 
-                # 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)
+        def mock_update_image_metadata(*args, **kwargs):
 
-                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.assertFalse(res.location)
+                self.assertEqual(200, res.status_int)
 
                 self.stubs.Set(registry, 'update_image_metadata',
                                orig_update_image_metadata)
@@ -1804,20 +1789,30 @@ class TestGlanceAPI(base.IsolatedUnitTest):
         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
diff --git a/glance/tests/unit/v2/test_image_data_resource.py b/glance/tests/unit/v2/test_image_data_resource.py
index cc8148a..a121e82 100644
--- a/glance/tests/unit/v2/test_image_data_resource.py
+++ b/glance/tests/unit/v2/test_image_data_resource.py
@@ -81,7 +81,7 @@ class FakeImageRepo(object):
         else:
             return self.result
 
-    def save(self, image):
+    def save(self, image, from_state=None):
         self.saved_image = image
 
 
@@ -184,17 +184,21 @@ class TestImagesController(base.StoreClearingUnitTest):
                           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_raises_not_found_exception(self):
         def fake_save(self):
-- 
2.0.5