| 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 |
| |