diff options
Diffstat (limited to 'app-admin/glance/files/cve-2016-0757-stable-liberty.patch')
| -rw-r--r-- | app-admin/glance/files/cve-2016-0757-stable-liberty.patch | 332 |
1 files changed, 332 insertions, 0 deletions
diff --git a/app-admin/glance/files/cve-2016-0757-stable-liberty.patch b/app-admin/glance/files/cve-2016-0757-stable-liberty.patch new file mode 100644 index 000000000000..19c8365eb018 --- /dev/null +++ b/app-admin/glance/files/cve-2016-0757-stable-liberty.patch @@ -0,0 +1,332 @@ +From c6021e9b3642340036347026a3f251e066e53094 Mon Sep 17 00:00:00 2001 +From: Erno Kuvaja <jokke@usr.fi> +Date: Tue, 19 Jan 2016 13:37:05 +0000 +Subject: [PATCH] Prevent user to remove last location of the image + +If the last location of the image is removed, image transitions back to queued. +This allows user to upload new data into the existing image record. By +preventing removal of the last location we prevent the image transition back to +queued. + +This change also prevents doing the same operation via replacing the locations +with empty list. + +SecurityImpact +DocImpact +APIImpact + +Conflicts: + glance/tests/unit/v2/test_images_resource.py + +Change-Id: Ieb03aaba887492819f9c58aa67f7acfcea81720e +Closes-Bug: #1525915 +(cherry picked from commit 2f4504da2149697bcdb93ed855e15025d2a08f8c) +--- + glance/api/v2/images.py | 19 +++- + glance/tests/functional/v2/test_images.py | 14 --- + glance/tests/unit/v2/test_images_resource.py | 122 ++++----------------- + ...oving-last-image-location-d5ee3e00efe14f34.yaml | 10 ++ + 4 files changed, 44 insertions(+), 121 deletions(-) + create mode 100644 releasenotes/notes/Prevent-removing-last-image-location-d5ee3e00efe14f34.yaml + +diff --git a/glance/api/v2/images.py b/glance/api/v2/images.py +index 17678f2..cf667bf 100644 +--- a/glance/api/v2/images.py ++++ b/glance/api/v2/images.py +@@ -181,7 +181,10 @@ class ImagesController(object): + path = change['path'] + path_root = path[0] + value = change['value'] +- if path_root == 'locations': ++ if path_root == 'locations' and value == []: ++ msg = _("Cannot set locations to empty list.") ++ raise webob.exc.HTTPForbidden(message=msg) ++ elif path_root == 'locations' and value != []: + self._do_replace_locations(image, value) + elif path_root == 'owner' and req.context.is_admin == False: + msg = _("Owner can't be updated by non admin.") +@@ -217,7 +220,10 @@ class ImagesController(object): + path = change['path'] + path_root = path[0] + if path_root == 'locations': +- self._do_remove_locations(image, path[1]) ++ try: ++ self._do_remove_locations(image, path[1]) ++ except exception.Forbidden as e: ++ raise webob.exc.HTTPForbidden(e.msg) + else: + if hasattr(image, path_root): + msg = _("Property %s may not be removed.") +@@ -306,6 +312,11 @@ class ImagesController(object): + explanation=encodeutils.exception_to_unicode(ve)) + + def _do_remove_locations(self, image, path_pos): ++ if len(image.locations) == 1: ++ LOG.debug("User forbidden to remove last location of image %s", ++ image.image_id) ++ msg = _("Cannot remove last location in the image.") ++ raise exception.Forbidden(message=msg) + pos = self._get_locations_op_pos(path_pos, + len(image.locations), False) + if pos is None: +@@ -315,11 +326,11 @@ class ImagesController(object): + # NOTE(zhiyan): this actually deletes the location + # from the backend store. + image.locations.pop(pos) ++ # TODO(jokke): Fix this, we should catch what store throws and ++ # provide definitely something else than IternalServerError to user. + except Exception as e: + raise webob.exc.HTTPInternalServerError( + explanation=encodeutils.exception_to_unicode(e)) +- if len(image.locations) == 0 and image.status == 'active': +- image.status = 'queued' + + + class RequestDeserializer(wsgi.JSONRequestDeserializer): +diff --git a/glance/tests/functional/v2/test_images.py b/glance/tests/functional/v2/test_images.py +index aabc567..f199787 100644 +--- a/glance/tests/functional/v2/test_images.py ++++ b/glance/tests/functional/v2/test_images.py +@@ -522,20 +522,6 @@ class TestImages(functional.FunctionalTest): + response = requests.patch(path, headers=headers, data=data) + self.assertEqual(200, response.status_code, response.text) + +- # Remove all locations of the image then the image size shouldn't be +- # able to access +- path = self._url('/v2/images/%s' % image2_id) +- media_type = 'application/openstack-images-v2.1-json-patch' +- headers = self._headers({'content-type': media_type}) +- doc = [{'op': 'replace', 'path': '/locations', 'value': []}] +- data = jsonutils.dumps(doc) +- response = requests.patch(path, headers=headers, data=data) +- self.assertEqual(200, response.status_code, response.text) +- image = jsonutils.loads(response.text) +- self.assertIsNone(image['size']) +- self.assertIsNone(image['virtual_size']) +- self.assertEqual('queued', image['status']) +- + # Deletion should work. Deleting image-1 + path = self._url('/v2/images/%s' % image_id) + response = requests.delete(path, headers=self._headers()) +diff --git a/glance/tests/unit/v2/test_images_resource.py b/glance/tests/unit/v2/test_images_resource.py +index 698c284..ee09ee7 100644 +--- a/glance/tests/unit/v2/test_images_resource.py ++++ b/glance/tests/unit/v2/test_images_resource.py +@@ -1417,26 +1417,6 @@ class TestImagesController(base.IsolatedUnitTest): + self.assertRaises(webob.exc.HTTPConflict, self.controller.update, + another_request, created_image.image_id, changes) + +- def test_update_replace_locations(self): +- self.stubs.Set(store, 'get_size_from_backend', +- unit_test_utils.fake_get_size_from_backend) +- request = unit_test_utils.get_fake_request() +- changes = [{'op': 'replace', 'path': ['locations'], 'value': []}] +- output = self.controller.update(request, UUID1, changes) +- self.assertEqual(UUID1, output.image_id) +- self.assertEqual(0, len(output.locations)) +- self.assertEqual('queued', output.status) +- self.assertIsNone(output.size) +- +- new_location = {'url': '%s/fake_location' % BASE_URI, 'metadata': {}} +- changes = [{'op': 'replace', 'path': ['locations'], +- 'value': [new_location]}] +- output = self.controller.update(request, UUID1, changes) +- self.assertEqual(UUID1, output.image_id) +- self.assertEqual(1, len(output.locations)) +- self.assertEqual(new_location, output.locations[0]) +- self.assertEqual('active', output.status) +- + def test_update_replace_locations_non_empty(self): + new_location = {'url': '%s/fake_location' % BASE_URI, 'metadata': {}} + request = unit_test_utils.get_fake_request() +@@ -1448,35 +1428,9 @@ class TestImagesController(base.IsolatedUnitTest): + def test_update_replace_locations_invalid(self): + request = unit_test_utils.get_fake_request() + changes = [{'op': 'replace', 'path': ['locations'], 'value': []}] +- output = self.controller.update(request, UUID1, changes) +- self.assertEqual(UUID1, output.image_id) +- self.assertEqual(0, len(output.locations)) +- self.assertEqual('queued', output.status) +- +- request = unit_test_utils.get_fake_request() +- changes = [{'op': 'replace', 'path': ['locations'], +- 'value': [{'url': 'unknow://foo', 'metadata': {}}]}] +- self.assertRaises(webob.exc.HTTPBadRequest, self.controller.update, ++ self.assertRaises(webob.exc.HTTPForbidden, self.controller.update, + request, UUID1, changes) + +- def test_update_replace_locations_status_exception(self): +- self.stubs.Set(store, 'get_size_from_backend', +- unit_test_utils.fake_get_size_from_backend) +- request = unit_test_utils.get_fake_request() +- changes = [{'op': 'replace', 'path': ['locations'], 'value': []}] +- output = self.controller.update(request, UUID2, changes) +- self.assertEqual(UUID2, output.image_id) +- self.assertEqual(0, len(output.locations)) +- self.assertEqual('queued', output.status) +- +- self.db.image_update(None, UUID2, {'disk_format': None}) +- +- new_location = {'url': '%s/fake_location' % BASE_URI, 'metadata': {}} +- changes = [{'op': 'replace', 'path': ['locations'], +- 'value': [new_location]}] +- self.assertRaises(webob.exc.HTTPBadRequest, self.controller.update, +- request, UUID2, changes) +- + def test_update_add_property(self): + request = unit_test_utils.get_fake_request() + +@@ -1600,24 +1554,6 @@ class TestImagesController(base.IsolatedUnitTest): + self.assertRaises(webob.exc.HTTPBadRequest, self.controller.update, + request, UUID1, changes) + +- def test_update_add_locations_status_exception(self): +- self.stubs.Set(store, 'get_size_from_backend', +- unit_test_utils.fake_get_size_from_backend) +- request = unit_test_utils.get_fake_request() +- changes = [{'op': 'replace', 'path': ['locations'], 'value': []}] +- output = self.controller.update(request, UUID2, changes) +- self.assertEqual(UUID2, output.image_id) +- self.assertEqual(0, len(output.locations)) +- self.assertEqual('queued', output.status) +- +- self.db.image_update(None, UUID2, {'disk_format': None}) +- +- new_location = {'url': '%s/fake_location' % BASE_URI, 'metadata': {}} +- changes = [{'op': 'add', 'path': ['locations', '-'], +- 'value': new_location}] +- self.assertRaises(webob.exc.HTTPBadRequest, self.controller.update, +- request, UUID2, changes) +- + def test_update_add_duplicate_locations(self): + new_location = {'url': '%s/fake_location' % BASE_URI, 'metadata': {}} + request = unit_test_utils.get_fake_request() +@@ -1631,23 +1567,6 @@ class TestImagesController(base.IsolatedUnitTest): + self.assertRaises(webob.exc.HTTPBadRequest, self.controller.update, + request, UUID1, changes) + +- def test_update_replace_duplicate_locations(self): +- self.stubs.Set(store, 'get_size_from_backend', +- unit_test_utils.fake_get_size_from_backend) +- request = unit_test_utils.get_fake_request() +- changes = [{'op': 'replace', 'path': ['locations'], 'value': []}] +- output = self.controller.update(request, UUID1, changes) +- self.assertEqual(UUID1, output.image_id) +- self.assertEqual(0, len(output.locations)) +- self.assertEqual('queued', output.status) +- +- new_location = {'url': '%s/fake_location' % BASE_URI, 'metadata': {}} +- changes = [{'op': 'replace', 'path': ['locations'], +- 'value': [new_location, new_location]}] +- +- self.assertRaises(webob.exc.HTTPBadRequest, self.controller.update, +- request, UUID1, changes) +- + def test_update_add_too_many_locations(self): + self.config(image_location_quota=1) + request = unit_test_utils.get_fake_request() +@@ -1748,9 +1667,12 @@ class TestImagesController(base.IsolatedUnitTest): + {'op': 'add', 'path': ['locations', '-'], + 'value': {'url': '%s/fake_location_1' % BASE_URI, + 'metadata': {}}}, ++ {'op': 'add', 'path': ['locations', '-'], ++ 'value': {'url': '%s/fake_location_2' % BASE_URI, ++ 'metadata': {}}}, + ] + self.controller.update(request, UUID1, changes) +- self.config(image_location_quota=1) ++ self.config(image_location_quota=2) + + # We must remove two properties to avoid being + # over the limit of 1 property +@@ -1763,8 +1685,8 @@ class TestImagesController(base.IsolatedUnitTest): + ] + output = self.controller.update(request, UUID1, changes) + self.assertEqual(UUID1, output.image_id) +- self.assertEqual(1, len(output.locations)) +- self.assertIn('fake_location_3', output.locations[0]['url']) ++ self.assertEqual(2, len(output.locations)) ++ self.assertIn('fake_location_3', output.locations[1]['url']) + self.assertNotEqual(output.created_at, output.updated_at) + + def test_update_remove_base_property(self): +@@ -1805,24 +1727,23 @@ class TestImagesController(base.IsolatedUnitTest): + unit_test_utils.fake_get_size_from_backend) + + request = unit_test_utils.get_fake_request() +- changes = [{'op': 'remove', 'path': ['locations', '0']}] +- output = self.controller.update(request, UUID1, changes) +- self.assertEqual(output.image_id, UUID1) +- self.assertEqual(0, len(output.locations)) +- self.assertEqual('queued', output.status) +- self.assertIsNone(output.size) +- + new_location = {'url': '%s/fake_location' % BASE_URI, 'metadata': {}} + changes = [{'op': 'add', 'path': ['locations', '-'], + 'value': new_location}] ++ self.controller.update(request, UUID1, changes) ++ changes = [{'op': 'remove', 'path': ['locations', '0']}] + output = self.controller.update(request, UUID1, changes) + self.assertEqual(UUID1, output.image_id) + self.assertEqual(1, len(output.locations)) +- self.assertEqual(new_location, output.locations[0]) + self.assertEqual('active', output.status) + + def test_update_remove_location_invalid_pos(self): + request = unit_test_utils.get_fake_request() ++ changes = [ ++ {'op': 'add', 'path': ['locations', '-'], ++ 'value': {'url': '%s/fake_location' % BASE_URI, ++ 'metadata': {}}}] ++ self.controller.update(request, UUID1, changes) + changes = [{'op': 'remove', 'path': ['locations', None]}] + self.assertRaises(webob.exc.HTTPBadRequest, self.controller.update, + request, UUID1, changes) +@@ -1844,6 +1765,11 @@ class TestImagesController(base.IsolatedUnitTest): + fake_delete_image_location_from_backend) + + request = unit_test_utils.get_fake_request() ++ changes = [ ++ {'op': 'add', 'path': ['locations', '-'], ++ 'value': {'url': '%s/fake_location' % BASE_URI, ++ 'metadata': {}}}] ++ self.controller.update(request, UUID1, changes) + changes = [{'op': 'remove', 'path': ['locations', '0']}] + self.assertRaises(webob.exc.HTTPInternalServerError, + self.controller.update, request, UUID1, changes) +@@ -2137,16 +2063,6 @@ class TestImagesControllerPolicies(base.IsolatedUnitTest): + self.assertRaises(webob.exc.HTTPForbidden, self.controller.update, + request, UUID1, changes) + +- self.stubs.Set(self.store_utils, 'delete_image_location_from_backend', +- fake_delete_image_location_from_backend) +- +- changes = [{'op': 'replace', 'path': ['locations'], 'value': []}] +- self.controller.update(request, UUID1, changes) +- changes = [{'op': 'replace', 'path': ['locations'], +- 'value': [new_location]}] +- self.assertRaises(webob.exc.HTTPForbidden, self.controller.update, +- request, UUID1, changes) +- + def test_update_delete_image_location_unauthorized(self): + rules = {"delete_image_location": False} + self.policy.set_rules(rules) +diff --git a/releasenotes/notes/Prevent-removing-last-image-location-d5ee3e00efe14f34.yaml b/releasenotes/notes/Prevent-removing-last-image-location-d5ee3e00efe14f34.yaml +new file mode 100644 +index 0000000..344e6e5 +--- /dev/null ++++ b/releasenotes/notes/Prevent-removing-last-image-location-d5ee3e00efe14f34.yaml +@@ -0,0 +1,10 @@ ++--- ++security: ++ - Fixing bug 1525915; image might be transitioning ++ from active to queued by regular user by removing ++ last location of image (or replacing locations ++ with empty list). This allows user to re-upload ++ data to the image breaking Glance's promise of ++ image data immutability. From now on, last ++ location cannot be removed and locations cannot ++ be replaced with empty list. +-- +1.9.1 + |