calculate
/
gentoo-overlay
6
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity
You can not select more than 25 topics Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.
Branches Tags
${ item.name }
Create tag ${ searchTerm }
Create branch ${ searchTerm }
from 'e336b3e477'
${ noResults }
gentoo-overlay/app-admin/glance/files/cve-2015-5251-stable-kilo.p...

193 lines
8.9 KiB
Raw Blame History

From 9beca533f42ae1fc87418de0c360e19bc59b24b5 Mon Sep 17 00:00:00 2001
From: Stuart McLaren <stuart.mclaren@hp.com>
Date: Tue, 11 Aug 2015 10:37:09 +0000
Subject: [PATCH] Prevent image status being directly modified via v1
Users shouldn't be able to change an image's status directly via the
v1 API.
Some existing consumers of Glance set the x-image-meta-status header in
requests to the Glance API, eg:
https://github.com/openstack/nova/blob/master/plugins/xenserver/xenapi/etc/xapi.d/plugins/glance#L184
We should try to prevent users setting 'status' via v1, but without breaking
existing benign API calls such as these.
I've adopted the following approach (which has some prior art in 'protected properties').
If a PUT request is received which contains an x-image-meta-status header:
* The user provided status is ignored if it matches the current image
status (this prevents benign calls such as the nova one above from
breaking). The usual code (eg 200) will be returned.
* If the user provided status doesn't match the current image status (ie
there is a real attempt to change the value) 403 will be returned. This
will break any calls which currently intentionally change the status.
APIImpact
Closes-bug: 1482371
Change-Id: I44fadf32abb57c962b67467091c3f51c1ccc25e6
(cherry picked from commit 4d08db5b6d42323ac1958ef3b7417d875e7bea8c)
---
glance/api/v1/__init__.py | 3 +
glance/api/v1/images.py | 9 +++
glance/tests/functional/v1/test_api.py | 89 ++++++++++++++++++++++
.../integration/legacy_functional/test_v1_api.py | 2 +
4 files changed, 103 insertions(+)
diff --git a/glance/api/v1/__init__.py b/glance/api/v1/__init__.py
index 74de9aa1411d8e926770b67f7d851cf14e794414..9306bbb4fe78f77a26bb539c717fdfd2b38767c8 100644
--- a/glance/api/v1/__init__.py
+++ b/glance/api/v1/__init__.py
@@ -21,3 +21,6 @@ SUPPORTED_PARAMS = ('limit', 'marker', 'sort_key', 'sort_dir')
# Metadata which only an admin can change once the image is active
ACTIVE_IMMUTABLE = ('size', 'checksum')
+
+# Metadata which cannot be changed (irrespective of the current image state)
+IMMUTABLE = ('status',)
diff --git a/glance/api/v1/images.py b/glance/api/v1/images.py
index e33b91fbca79377e78ccfd329fa542ad422f5ffc..95e32949d958d0f57a3b60c141b91784a5801f5a 100644
--- a/glance/api/v1/images.py
+++ b/glance/api/v1/images.py
@@ -57,6 +57,7 @@ _LW = i18n._LW
SUPPORTED_PARAMS = glance.api.v1.SUPPORTED_PARAMS
SUPPORTED_FILTERS = glance.api.v1.SUPPORTED_FILTERS
ACTIVE_IMMUTABLE = glance.api.v1.ACTIVE_IMMUTABLE
+IMMUTABLE = glance.api.v1.IMMUTABLE
CONF = cfg.CONF
CONF.import_opt('disk_formats', 'glance.common.config', group='image_format')
@@ -912,6 +913,14 @@ class Controller(controller.BaseController):
request=req,
content_type="text/plain")
+ for key in IMMUTABLE:
+ if (image_meta.get(key) is not None and
+ image_meta.get(key) != orig_image_meta.get(key)):
+ msg = _("Forbidden to modify '%s' of image.") % key
+ raise HTTPForbidden(explanation=msg,
+ request=req,
+ content_type="text/plain")
+
# The default behaviour for a PUT /images/<IMAGE_ID> is to
# override any properties that were previously set. This, however,
# leads to a number of issues for the common use case where a caller
diff --git a/glance/tests/functional/v1/test_api.py b/glance/tests/functional/v1/test_api.py
index 9fba3bb5e40c8742530691228c7b436b385fc2ca..6b3bfbb4270f1eb0f50418504e65be30ea23d10b 100644
--- a/glance/tests/functional/v1/test_api.py
+++ b/glance/tests/functional/v1/test_api.py
@@ -715,3 +715,92 @@ class TestApi(functional.FunctionalTest):
self.assertEqual(404, response.status)
self.stop_servers()
+
+ def test_status_cannot_be_manipulated_directly(self):
+ self.cleanup()
+ self.start_servers(**self.__dict__.copy())
+ headers = minimal_headers('Image1')
+
+ # Create a 'queued' image
+ http = httplib2.Http()
+ headers = {'Content-Type': 'application/octet-stream',
+ 'X-Image-Meta-Disk-Format': 'raw',
+ 'X-Image-Meta-Container-Format': 'bare'}
+ path = "http://%s:%d/v1/images" % ("127.0.0.1", self.api_port)
+ response, content = http.request(path, 'POST', headers=headers,
+ body=None)
+ self.assertEqual(201, response.status)
+ image = jsonutils.loads(content)['image']
+ self.assertEqual('queued', image['status'])
+
+ # Ensure status of 'queued' image can't be changed
+ path = "http://%s:%d/v1/images/%s" % ("127.0.0.1", self.api_port,
+ image['id'])
+ http = httplib2.Http()
+ headers = {'X-Image-Meta-Status': 'active'}
+ response, content = http.request(path, 'PUT', headers=headers)
+ self.assertEqual(403, response.status)
+ response, content = http.request(path, 'HEAD')
+ self.assertEqual(200, response.status)
+ self.assertEqual('queued', response['x-image-meta-status'])
+
+ # We allow 'setting' to the same status
+ http = httplib2.Http()
+ headers = {'X-Image-Meta-Status': 'queued'}
+ response, content = http.request(path, 'PUT', headers=headers)
+ self.assertEqual(200, response.status)
+ response, content = http.request(path, 'HEAD')
+ self.assertEqual(200, response.status)
+ self.assertEqual('queued', response['x-image-meta-status'])
+
+ # Make image active
+ http = httplib2.Http()
+ headers = {'Content-Type': 'application/octet-stream'}
+ response, content = http.request(path, 'PUT', headers=headers,
+ body='data')
+ self.assertEqual(200, response.status)
+ image = jsonutils.loads(content)['image']
+ self.assertEqual('active', image['status'])
+
+ # Ensure status of 'active' image can't be changed
+ http = httplib2.Http()
+ headers = {'X-Image-Meta-Status': 'queued'}
+ response, content = http.request(path, 'PUT', headers=headers)
+ self.assertEqual(403, response.status)
+ response, content = http.request(path, 'HEAD')
+ self.assertEqual(200, response.status)
+ self.assertEqual('active', response['x-image-meta-status'])
+
+ # We allow 'setting' to the same status
+ http = httplib2.Http()
+ headers = {'X-Image-Meta-Status': 'active'}
+ response, content = http.request(path, 'PUT', headers=headers)
+ self.assertEqual(200, response.status)
+ response, content = http.request(path, 'HEAD')
+ self.assertEqual(200, response.status)
+ self.assertEqual('active', response['x-image-meta-status'])
+
+ # Create a 'queued' image, ensure 'status' header is ignored
+ http = httplib2.Http()
+ path = "http://%s:%d/v1/images" % ("127.0.0.1", self.api_port)
+ headers = {'Content-Type': 'application/octet-stream',
+ 'X-Image-Meta-Status': 'active'}
+ response, content = http.request(path, 'POST', headers=headers,
+ body=None)
+ self.assertEqual(201, response.status)
+ image = jsonutils.loads(content)['image']
+ self.assertEqual('queued', image['status'])
+
+ # Create an 'active' image, ensure 'status' header is ignored
+ http = httplib2.Http()
+ path = "http://%s:%d/v1/images" % ("127.0.0.1", self.api_port)
+ headers = {'Content-Type': 'application/octet-stream',
+ 'X-Image-Meta-Disk-Format': 'raw',
+ 'X-Image-Meta-Status': 'queued',
+ 'X-Image-Meta-Container-Format': 'bare'}
+ response, content = http.request(path, 'POST', headers=headers,
+ body='data')
+ self.assertEqual(201, response.status)
+ image = jsonutils.loads(content)['image']
+ self.assertEqual('active', image['status'])
+ self.stop_servers()
diff --git a/glance/tests/integration/legacy_functional/test_v1_api.py b/glance/tests/integration/legacy_functional/test_v1_api.py
index dff436465919569480bdbac537d20a6d61c98f46..511d46dfe18028bb430504784cc9d24c58736c3b 100644
--- a/glance/tests/integration/legacy_functional/test_v1_api.py
+++ b/glance/tests/integration/legacy_functional/test_v1_api.py
@@ -358,6 +358,8 @@ class TestApi(base.ApiTest):
path = "/v1/images"
response, content = self.http.request(path, 'POST', headers=headers)
self.assertEqual(201, response.status)
+ image = jsonutils.loads(content)['image']
+ self.assertEqual('active', image['status'])
# 2. HEAD image-location
# Verify image size is zero and the status is active
--
2.5.0
Reference in new issue View Git Blame Copy Permalink