[openstack-glance] fix glance auth bypass for image deletion (CVE-2012-4573)
Pádraig Brady
pbrady at fedoraproject.org
Fri Nov 9 01:12:52 UTC 2012
commit e5a6f83713ed98454aad1f90bc00c9501974628c
Author: Pádraig Brady <P at draigBrady.com>
Date: Fri Nov 9 01:03:45 2012 +0000
fix glance auth bypass for image deletion (CVE-2012-4573)
0001-Bump-next-version-to-2012.2.1.patch | 24 ++++
...aultbranch-in-.gitreview-to-stable-folsom.patch | 14 +++
0003-Pass-empty-args-to-test-config-parser.patch | 41 +++++++
0004-pin-sqlalchemy-to-the-0.7.x-series.patch | 29 +++++
0005-FakeAuth-not-always-admin.patch | 42 +++++++
0006-Delete-from-store-after-registry-delete.patch | 122 ++++++++++++++++++++
...-authorization-before-deleting-from-store.patch | 79 +++++++++++++
...-Don-t-access-the-net-while-building-docs.patch | 2 +-
openstack-glance.spec | 23 +++-
9 files changed, 372 insertions(+), 4 deletions(-)
---
diff --git a/0001-Bump-next-version-to-2012.2.1.patch b/0001-Bump-next-version-to-2012.2.1.patch
new file mode 100644
index 0000000..ed2119f
--- /dev/null
+++ b/0001-Bump-next-version-to-2012.2.1.patch
@@ -0,0 +1,24 @@
+From 6740af2f8186aa903c45afa2c2329564fce90fdf Mon Sep 17 00:00:00 2001
+From: Thierry Carrez <thierry at openstack.org>
+Date: Fri, 28 Sep 2012 16:14:02 +0200
+Subject: [PATCH] Bump next version to 2012.2.1
+
+Open stable/folsom by bumping next version to 2012.2.1.
+
+Change-Id: Ia452eaf82c873a6b19f155f21a79116d32530b5b
+---
+ glance/version.py | 2 +-
+ 1 files changed, 1 insertions(+), 1 deletions(-)
+
+diff --git a/glance/version.py b/glance/version.py
+index 6e90b7e..4eef879 100644
+--- a/glance/version.py
++++ b/glance/version.py
+@@ -17,6 +17,6 @@
+
+ from glance.openstack.common import version as common_version
+
+-NEXT_VERSION = '2012.2'
++NEXT_VERSION = '2012.2.1'
+ version_info = common_version.VersionInfo('glance',
+ pre_version=NEXT_VERSION)
diff --git a/0002-Set-defaultbranch-in-.gitreview-to-stable-folsom.patch b/0002-Set-defaultbranch-in-.gitreview-to-stable-folsom.patch
new file mode 100644
index 0000000..075ba9d
--- /dev/null
+++ b/0002-Set-defaultbranch-in-.gitreview-to-stable-folsom.patch
@@ -0,0 +1,14 @@
+From ac223e243736ba98a0ec480d3f80e39a555d3343 Mon Sep 17 00:00:00 2001
+From: Mark McLoughlin <markmc at redhat.com>
+Date: Sat, 13 Oct 2012 22:44:12 +0100
+Subject: [PATCH] Set defaultbranch in .gitreview to stable/folsom
+
+This allows people run git-review without any arguments.
+
+Change-Id: I2b2488ffffd783b64e5c760324e7d4a00d5a39db
+---
+ .gitreview | 1 +
+ 1 files changed, 1 insertions(+), 0 deletions(-)
+
+diff --git a/.gitreview b/.gitreview
+index 31aa568..6d7d086 100644
diff --git a/0003-Pass-empty-args-to-test-config-parser.patch b/0003-Pass-empty-args-to-test-config-parser.patch
new file mode 100644
index 0000000..0239589
--- /dev/null
+++ b/0003-Pass-empty-args-to-test-config-parser.patch
@@ -0,0 +1,41 @@
+From 1d5c651321a87e218995821b0d148867db977988 Mon Sep 17 00:00:00 2001
+From: Mark Washenberger <mark.washenberger at rackspace.com>
+Date: Tue, 25 Sep 2012 19:57:10 +0000
+Subject: [PATCH] Pass empty args to test config parser.
+
+This fixes bug 1056420, which allows users to pass nosetests options
+through run_tests.sh again.
+
+Change-Id: I03e84488a24e2552b61aa0cab842d8325e8e856f
+(cherry picked from commit ef641597dbfc0fcb856c143b213be493ab44cc65)
+---
+ glance/tests/unit/test_clients.py | 2 +-
+ glance/tests/utils.py | 2 +-
+ 2 files changed, 2 insertions(+), 2 deletions(-)
+
+diff --git a/glance/tests/unit/test_clients.py b/glance/tests/unit/test_clients.py
+index 20200b3..5be190d 100644
+--- a/glance/tests/unit/test_clients.py
++++ b/glance/tests/unit/test_clients.py
+@@ -39,7 +39,7 @@ UUID1 = _gen_uuid()
+ UUID2 = _gen_uuid()
+
+ #NOTE(bcwaldon): needed to init config_dir cli opt
+-config.parse_args()
++config.parse_args(args=[])
+
+
+ class TestBadClients(test_utils.BaseTestCase):
+diff --git a/glance/tests/utils.py b/glance/tests/utils.py
+index 370fc8b..cb9278d 100644
+--- a/glance/tests/utils.py
++++ b/glance/tests/utils.py
+@@ -57,7 +57,7 @@ class BaseTestCase(unittest.TestCase):
+ #NOTE(bcwaldon): parse_args has to be called to register certain
+ # command-line options - specifically we need config_dir for
+ # the following policy tests
+- config.parse_args()
++ config.parse_args(args=[])
+
+ def tearDown(self):
+ super(BaseTestCase, self).tearDown()
diff --git a/0004-pin-sqlalchemy-to-the-0.7.x-series.patch b/0004-pin-sqlalchemy-to-the-0.7.x-series.patch
new file mode 100644
index 0000000..f8fd926
--- /dev/null
+++ b/0004-pin-sqlalchemy-to-the-0.7.x-series.patch
@@ -0,0 +1,29 @@
+From ddad2752b3afb480f5be1352cd2ef56739d3cdad Mon Sep 17 00:00:00 2001
+From: Sean Dague <sdague at linux.vnet.ibm.com>
+Date: Wed, 31 Oct 2012 11:38:47 -0400
+Subject: [PATCH] pin sqlalchemy to the 0.7.x series
+
+sqlalchemy 0.8beta is now out and has internal changes
+which mean it's not an in place seemless upgrade. This pins
+sqlalchemy to the 0.7.x series to avoid those breaks.
+
+Fixes bug #1073569
+
+Change-Id: Ia8976843a7e0a132fe38e80f2bcb01a16e12b8de
+---
+ tools/pip-requires | 2 +-
+ 1 files changed, 1 insertions(+), 1 deletions(-)
+
+diff --git a/tools/pip-requires b/tools/pip-requires
+index e873537..549f6e5 100644
+--- a/tools/pip-requires
++++ b/tools/pip-requires
+@@ -3,7 +3,7 @@
+ # package to get the right headers...
+ greenlet>=0.3.1
+
+-SQLAlchemy>=0.7
++SQLAlchemy>=0.7,<=0.7.9
+ anyjson
+ eventlet>=0.9.12
+ PasteDeploy
diff --git a/0005-FakeAuth-not-always-admin.patch b/0005-FakeAuth-not-always-admin.patch
new file mode 100644
index 0000000..999f1e4
--- /dev/null
+++ b/0005-FakeAuth-not-always-admin.patch
@@ -0,0 +1,42 @@
+From 7841cc93050fa48a1fbd8847cec887f0360be9a3 Mon Sep 17 00:00:00 2001
+From: Mark Washenberger <mark.washenberger at rackspace.com>
+Date: Sun, 7 Oct 2012 02:12:52 +0000
+Subject: [PATCH] FakeAuth not always admin
+
+The FakeAuthMiddleware was defaulting to admin=True for request
+contexts. This creates a situation where it is impossible to test
+non-admin requests in some situations.
+
+Change-Id: I949f708efed9f07a43b8506870c5b38fce4b3752
+---
+ glance/tests/unit/v1/test_api.py | 3 ++-
+ glance/tests/utils.py | 2 +-
+ 2 files changed, 3 insertions(+), 2 deletions(-)
+
+diff --git a/glance/tests/unit/v1/test_api.py b/glance/tests/unit/v1/test_api.py
+index dfb5e8a..de22657 100644
+--- a/glance/tests/unit/v1/test_api.py
++++ b/glance/tests/unit/v1/test_api.py
+@@ -103,7 +103,8 @@ class TestRegistryAPI(base.IsolatedUnitTest):
+ """Establish a clean test environment"""
+ super(TestRegistryAPI, self).setUp()
+ self.mapper = routes.Mapper()
+- self.api = test_utils.FakeAuthMiddleware(rserver.API(self.mapper))
++ self.api = test_utils.FakeAuthMiddleware(rserver.API(self.mapper),
++ is_admin=True)
+ self.FIXTURES = [
+ {'id': UUID1,
+ 'name': 'fake image #1',
+diff --git a/glance/tests/utils.py b/glance/tests/utils.py
+index cb9278d..92b6500 100644
+--- a/glance/tests/utils.py
++++ b/glance/tests/utils.py
+@@ -347,7 +347,7 @@ def minimal_add_command(port, name, suffix='', public=True):
+
+ class FakeAuthMiddleware(wsgi.Middleware):
+
+- def __init__(self, app, is_admin=True):
++ def __init__(self, app, is_admin=False):
+ super(FakeAuthMiddleware, self).__init__(app)
+ self.is_admin = is_admin
+
diff --git a/0006-Delete-from-store-after-registry-delete.patch b/0006-Delete-from-store-after-registry-delete.patch
new file mode 100644
index 0000000..fe92669
--- /dev/null
+++ b/0006-Delete-from-store-after-registry-delete.patch
@@ -0,0 +1,122 @@
+From 90bcdc5a89e350a358cf320a03f5afe99795f6f6 Mon Sep 17 00:00:00 2001
+From: Mark Washenberger <mark.washenberger at rackspace.com>
+Date: Wed, 7 Nov 2012 09:59:56 -0500
+Subject: [PATCH] Delete from store after registry delete.
+
+Because we rely on the registry to determine authorization in the glance
+v1 api, we must attempt a registry delete before deleting an image from
+the image store.
+
+This patch includes the test for the bug, which was posted separately
+on the bug.
+
+Fixes bug 1065187.
+
+Change-Id: I1a06b7c7421524066c684539e2f3516c4ed2c475
+---
+ glance/api/v1/images.py | 16 +++++++++++-----
+ glance/tests/stubs.py | 8 +++++++-
+ glance/tests/unit/v1/test_api.py | 20 ++++++++++++++++++++
+ glance/tests/utils.py | 1 +
+ 4 files changed, 39 insertions(+), 6 deletions(-)
+
+diff --git a/glance/api/v1/images.py b/glance/api/v1/images.py
+index b127c35..55d7cc2 100644
+--- a/glance/api/v1/images.py
++++ b/glance/api/v1/images.py
+@@ -821,22 +821,28 @@ class Controller(controller.BaseController):
+ request=req,
+ content_type="text/plain")
+
+- status = 'deleted'
++ if image['location'] and CONF.delayed_delete:
++ status = 'pending_delete'
++ else:
++ status = 'deleted'
++
+ try:
++ # Delete the image from the registry first, since we rely on it
++ # for authorization checks.
++ # See https://bugs.launchpad.net/glance/+bug/1065187
++ registry.update_image_metadata(req.context, id, {'status': status})
++ registry.delete_image_metadata(req.context, id)
++
+ # The image's location field may be None in the case
+ # of a saving or queued image, therefore don't ask a backend
+ # to delete the image if the backend doesn't yet store it.
+ # See https://bugs.launchpad.net/glance/+bug/747799
+ if image['location']:
+ if CONF.delayed_delete:
+- status = 'pending_delete'
+ schedule_delayed_delete_from_backend(image['location'], id)
+ else:
+ safe_delete_from_backend(image['location'],
+ req.context, id)
+-
+- registry.update_image_metadata(req.context, id, {'status': status})
+- registry.delete_image_metadata(req.context, id)
+ except exception.NotFound, e:
+ msg = ("Failed to find image to delete: %(e)s" % locals())
+ for line in msg.split('\n'):
+diff --git a/glance/tests/stubs.py b/glance/tests/stubs.py
+index 669c9a9..a1c6244 100644
+--- a/glance/tests/stubs.py
++++ b/glance/tests/stubs.py
+@@ -60,7 +60,13 @@ class FakeRegistryConnection(object):
+
+ def getresponse(self):
+ mapper = routes.Mapper()
+- api = context.UnauthenticatedContextMiddleware(rserver.API(mapper))
++ server = rserver.API(mapper)
++ # NOTE(markwash): we need to pass through context auth information if
++ # we have it.
++ if 'X-Auth-Token' in self.req.headers:
++ api = utils.FakeAuthMiddleware(server)
++ else:
++ api = context.UnauthenticatedContextMiddleware(server)
+ webob_res = self.req.get_response(api)
+
+ return utils.FakeHTTPResponse(status=webob_res.status_int,
+diff --git a/glance/tests/unit/v1/test_api.py b/glance/tests/unit/v1/test_api.py
+index de22657..1589b50 100644
+--- a/glance/tests/unit/v1/test_api.py
++++ b/glance/tests/unit/v1/test_api.py
+@@ -2930,6 +2930,26 @@ class TestGlanceAPI(base.IsolatedUnitTest):
+ res = req.get_response(self.api)
+ self.assertEquals(res.status_int, webob.exc.HTTPNotFound.code)
+
++ def test_delete_not_allowed(self):
++ # Verify we can get the image data
++ req = webob.Request.blank("/images/%s" % UUID2)
++ req.method = 'GET'
++ req.headers['X-Auth-Token'] = 'user:tenant:'
++ res = req.get_response(self.api)
++ self.assertEqual(res.status_int, 200)
++ self.assertEqual(len(res.body), 19)
++
++ # Verify we cannot delete the image
++ req.method = 'DELETE'
++ res = req.get_response(self.api)
++ self.assertEqual(res.status_int, 403)
++
++ # Verify the image data is still there
++ req.method = 'GET'
++ res = req.get_response(self.api)
++ self.assertEqual(res.status_int, 200)
++ self.assertEqual(len(res.body), 19)
++
+ def test_delete_queued_image(self):
+ """Delete an image in a queued state
+
+diff --git a/glance/tests/utils.py b/glance/tests/utils.py
+index 92b6500..514f90e 100644
+--- a/glance/tests/utils.py
++++ b/glance/tests/utils.py
+@@ -368,6 +368,7 @@ class FakeAuthMiddleware(wsgi.Middleware):
+ 'tenant': tenant,
+ 'roles': roles,
+ 'is_admin': self.is_admin,
++ 'auth_tok': auth_tok,
+ }
+
+ req.context = context.RequestContext(**kwargs)
diff --git a/0007-Ensure-authorization-before-deleting-from-store.patch b/0007-Ensure-authorization-before-deleting-from-store.patch
new file mode 100644
index 0000000..b26e271
--- /dev/null
+++ b/0007-Ensure-authorization-before-deleting-from-store.patch
@@ -0,0 +1,79 @@
+From fc0ee7623ec59c87ac6fc671e95a9798d6f2e2c3 Mon Sep 17 00:00:00 2001
+From: "Mark J. Washenberger" <mark.washenberger at markwash.net>
+Date: Thu, 8 Nov 2012 10:56:07 -0800
+Subject: [PATCH] Ensure authorization before deleting from store
+
+This fixes bug 1076506.
+
+Change-Id: I3794c14fe523a9a27e943d73dd0248489d2b91f6
+---
+ glance/api/v2/images.py | 21 ++++++++++++---------
+ glance/tests/functional/v2/test_images.py | 12 ++++++++++++
+ 2 files changed, 24 insertions(+), 9 deletions(-)
+
+diff --git a/glance/api/v2/images.py b/glance/api/v2/images.py
+index 0773ca5..8ecb4fe 100644
+--- a/glance/api/v2/images.py
++++ b/glance/api/v2/images.py
+@@ -268,19 +268,22 @@ class ImagesController(object):
+ % locals())
+ raise webob.exc.HTTPForbidden(explanation=msg)
+
+- status = 'deleted'
+- if image['location']:
+- if CONF.delayed_delete:
+- status = 'pending_delete'
+- self.store_api.schedule_delayed_delete_from_backend(
+- image['location'], id)
+- else:
+- self.store_api.safe_delete_from_backend(image['location'],
+- req.context, id)
++ if image['location'] and CONF.delayed_delete:
++ status = 'pending_delete'
++ else:
++ status = 'deleted'
+
+ try:
+ self.db_api.image_update(req.context, image_id, {'status': status})
+ self.db_api.image_destroy(req.context, image_id)
++
++ if image['location']:
++ if CONF.delayed_delete:
++ self.store_api.schedule_delayed_delete_from_backend(
++ image['location'], id)
++ else:
++ self.store_api.safe_delete_from_backend(image['location'],
++ req.context, id)
+ except (exception.NotFound, exception.Forbidden):
+ msg = ("Failed to find image %(image_id)s to delete" % locals())
+ LOG.info(msg)
+diff --git a/glance/tests/functional/v2/test_images.py b/glance/tests/functional/v2/test_images.py
+index ad079de..ac4a015 100644
+--- a/glance/tests/functional/v2/test_images.py
++++ b/glance/tests/functional/v2/test_images.py
+@@ -218,6 +218,12 @@ class TestImages(functional.FunctionalTest):
+ self.assertEqual(201, response.status_code)
+ image_id = json.loads(response.text)['id']
+
++ # Upload some image data
++ path = self._url('/v2/images/%s/file' % image_id)
++ headers = self._headers({'Content-Type': 'application/octet-stream'})
++ response = requests.put(path, headers=headers, data='ZZZZZ')
++ self.assertEqual(201, response.status_code)
++
+ # TENANT1 should see the image in their list
+ path = self._url('/v2/images')
+ response = requests.get(path, headers=self._headers())
+@@ -300,6 +306,12 @@ class TestImages(functional.FunctionalTest):
+ response = requests.delete(path, headers=headers)
+ self.assertEqual(404, response.status_code)
+
++ # Image data should still be present after the failed delete
++ path = self._url('/v2/images/%s/file' % image_id)
++ response = requests.get(path, headers=self._headers())
++ self.assertEqual(200, response.status_code)
++ self.assertEqual(response.text, 'ZZZZZ')
++
+ self.stop_servers()
+
+ def test_tag_lifecycle(self):
diff --git a/0001-Don-t-access-the-net-while-building-docs.patch b/0008-Don-t-access-the-net-while-building-docs.patch
similarity index 93%
rename from 0001-Don-t-access-the-net-while-building-docs.patch
rename to 0008-Don-t-access-the-net-while-building-docs.patch
index e2d589e..c552c99 100644
--- a/0001-Don-t-access-the-net-while-building-docs.patch
+++ b/0008-Don-t-access-the-net-while-building-docs.patch
@@ -1,4 +1,4 @@
-From 9bce91a06fcd87e33c64f46596e863f7078fdf56 Mon Sep 17 00:00:00 2001
+From 6d15431627326428e1f241c7e32d3384af626aee Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= <pbrady at redhat.com>
Date: Fri, 6 Jan 2012 17:12:54 +0000
Subject: [PATCH] Don't access the net while building docs
diff --git a/openstack-glance.spec b/openstack-glance.spec
index 6bf508f..817cdd4 100644
--- a/openstack-glance.spec
+++ b/openstack-glance.spec
@@ -3,7 +3,7 @@
#
Name: openstack-glance
Version: 2012.2
-Release: 2%{?dist}
+Release: 3%{?dist}
Summary: OpenStack Image Service
Group: Applications/System
@@ -17,7 +17,14 @@ Source3: openstack-glance.logrotate
#
# patches_base=2012.2
#
-Patch0001: 0001-Don-t-access-the-net-while-building-docs.patch
+Patch0001: 0001-Bump-next-version-to-2012.2.1.patch
+Patch0002: 0002-Set-defaultbranch-in-.gitreview-to-stable-folsom.patch
+Patch0003: 0003-Pass-empty-args-to-test-config-parser.patch
+Patch0004: 0004-pin-sqlalchemy-to-the-0.7.x-series.patch
+Patch0005: 0005-FakeAuth-not-always-admin.patch
+Patch0006: 0006-Delete-from-store-after-registry-delete.patch
+Patch0007: 0007-Ensure-authorization-before-deleting-from-store.patch
+Patch0008: 0008-Don-t-access-the-net-while-building-docs.patch
BuildArch: noarch
BuildRequires: python2-devel
@@ -100,7 +107,14 @@ This package contains documentation files for glance.
%prep
%setup -q -n glance-%{version}
-%patch0001 -p1
+#%patch0001 -p1
+#%patch0002 -p1
+#%patch0003 -p1
+#%patch0004 -p1
+#%patch0005 -p1
+%patch0006 -p1
+%patch0007 -p1
+%patch0008 -p1
# Remove bundled egg-info
rm -rf glance.egg-info
@@ -268,6 +282,9 @@ fi
%doc doc/build/html
%changelog
+* Fri Nov 9 2012 Pádraig Brady <P at draigBrady.com> 2012.2-3
+- Fix Glance Authentication bypass for image deletion (CVE-2012-4573)
+
* Thu Sep 27 2012 Alan Pevec <apevec at redhat.com> 2012.2-2
- Update to folsom final
More information about the scm-commits
mailing list