|
1 From 45be8e1c620c50f3cbca76f561945200a8843bc8 Mon Sep 17 00:00:00 2001 |
|
2 From: Stuart McLaren <[email protected]> |
|
3 Date: Tue, 11 Aug 2015 10:37:09 +0000 |
|
4 Subject: [PATCH] Prevent image status being directly modified via v1 |
|
5 |
|
6 Users shouldn't be able to change an image's status directly via the |
|
7 v1 API. |
|
8 |
|
9 Some existing consumers of Glance set the x-image-meta-status header in |
|
10 requests to the Glance API, eg: |
|
11 |
|
12 https://github.com/openstack/nova/blob/master/plugins/xenserver/xenapi/etc/xapi.d/plugins/glance#L184 |
|
13 |
|
14 We should try to prevent users setting 'status' via v1, but without breaking |
|
15 existing benign API calls such as these. |
|
16 |
|
17 I've adopted the following approach (which has some prior art in 'protected properties'). |
|
18 |
|
19 If a PUT request is received which contains an x-image-meta-status header: |
|
20 |
|
21 * The user provided status is ignored if it matches the current image |
|
22 status (this prevents benign calls such as the nova one above from |
|
23 breaking). The usual code (eg 200) will be returned. |
|
24 |
|
25 * If the user provided status doesn't match the current image status (ie |
|
26 there is a real attempt to change the value) 403 will be returned. This |
|
27 will break any calls which currently intentionally change the status. |
|
28 |
|
29 APIImpact |
|
30 |
|
31 Closes-bug: 1482371 |
|
32 |
|
33 Change-Id: I44fadf32abb57c962b67467091c3f51c1ccc25e6 |
|
34 (cherry picked from commit 4d08db5b6d42323ac1958ef3b7417d875e7bea8c) |
|
35 (cherry picked from commit 9beca533f42ae1fc87418de0c360e19bc59b24b5) |
|
36 --- |
|
37 glance/api/v1/__init__.py | 3 + |
|
38 glance/api/v1/images.py | 9 +++ |
|
39 glance/tests/functional/v1/test_api.py | 89 ++++++++++++++++++++++ |
|
40 .../integration/legacy_functional/test_v1_api.py | 2 + |
|
41 test-requirements.txt | 5 ++ |
|
42 5 files changed, 108 insertions(+) |
|
43 |
|
44 diff --git a/glance/api/v1/__init__.py b/glance/api/v1/__init__.py |
|
45 index 74de9aa..9306bbb 100644 |
|
46 --- a/glance/api/v1/__init__.py |
|
47 +++ b/glance/api/v1/__init__.py |
|
48 @@ -21,3 +21,6 @@ SUPPORTED_PARAMS = ('limit', 'marker', 'sort_key', 'sort_dir') |
|
49 |
|
50 # Metadata which only an admin can change once the image is active |
|
51 ACTIVE_IMMUTABLE = ('size', 'checksum') |
|
52 + |
|
53 +# Metadata which cannot be changed (irrespective of the current image state) |
|
54 +IMMUTABLE = ('status',) |
|
55 diff --git a/glance/api/v1/images.py b/glance/api/v1/images.py |
|
56 index 746f8cd..f976f9d 100644 |
|
57 --- a/glance/api/v1/images.py |
|
58 +++ b/glance/api/v1/images.py |
|
59 @@ -56,6 +56,7 @@ _LW = gettextutils._LW |
|
60 SUPPORTED_PARAMS = glance.api.v1.SUPPORTED_PARAMS |
|
61 SUPPORTED_FILTERS = glance.api.v1.SUPPORTED_FILTERS |
|
62 ACTIVE_IMMUTABLE = glance.api.v1.ACTIVE_IMMUTABLE |
|
63 +IMMUTABLE = glance.api.v1.IMMUTABLE |
|
64 |
|
65 CONF = cfg.CONF |
|
66 CONF.import_opt('disk_formats', 'glance.common.config', group='image_format') |
|
67 @@ -895,6 +896,14 @@ class Controller(controller.BaseController): |
|
68 request=req, |
|
69 content_type="text/plain") |
|
70 |
|
71 + for key in IMMUTABLE: |
|
72 + if (image_meta.get(key) is not None and |
|
73 + image_meta.get(key) != orig_image_meta.get(key)): |
|
74 + msg = _("Forbidden to modify '%s' of image.") % key |
|
75 + raise HTTPForbidden(explanation=msg, |
|
76 + request=req, |
|
77 + content_type="text/plain") |
|
78 + |
|
79 # The default behaviour for a PUT /images/<IMAGE_ID> is to |
|
80 # override any properties that were previously set. This, however, |
|
81 # leads to a number of issues for the common use case where a caller |
|
82 diff --git a/glance/tests/functional/v1/test_api.py b/glance/tests/functional/v1/test_api.py |
|
83 index 1486fb3..ad54005 100644 |
|
84 --- a/glance/tests/functional/v1/test_api.py |
|
85 +++ b/glance/tests/functional/v1/test_api.py |
|
86 @@ -765,3 +765,92 @@ class TestApi(functional.FunctionalTest): |
|
87 self.assertEqual(200, response.status) |
|
88 |
|
89 self.stop_servers() |
|
90 + |
|
91 + def test_status_cannot_be_manipulated_directly(self): |
|
92 + self.cleanup() |
|
93 + self.start_servers(**self.__dict__.copy()) |
|
94 + headers = minimal_headers('Image1') |
|
95 + |
|
96 + # Create a 'queued' image |
|
97 + http = httplib2.Http() |
|
98 + headers = {'Content-Type': 'application/octet-stream', |
|
99 + 'X-Image-Meta-Disk-Format': 'raw', |
|
100 + 'X-Image-Meta-Container-Format': 'bare'} |
|
101 + path = "http://%s:%d/v1/images" % ("127.0.0.1", self.api_port) |
|
102 + response, content = http.request(path, 'POST', headers=headers, |
|
103 + body=None) |
|
104 + self.assertEqual(201, response.status) |
|
105 + image = jsonutils.loads(content)['image'] |
|
106 + self.assertEqual('queued', image['status']) |
|
107 + |
|
108 + # Ensure status of 'queued' image can't be changed |
|
109 + path = "http://%s:%d/v1/images/%s" % ("127.0.0.1", self.api_port, |
|
110 + image['id']) |
|
111 + http = httplib2.Http() |
|
112 + headers = {'X-Image-Meta-Status': 'active'} |
|
113 + response, content = http.request(path, 'PUT', headers=headers) |
|
114 + self.assertEqual(403, response.status) |
|
115 + response, content = http.request(path, 'HEAD') |
|
116 + self.assertEqual(200, response.status) |
|
117 + self.assertEqual('queued', response['x-image-meta-status']) |
|
118 + |
|
119 + # We allow 'setting' to the same status |
|
120 + http = httplib2.Http() |
|
121 + headers = {'X-Image-Meta-Status': 'queued'} |
|
122 + response, content = http.request(path, 'PUT', headers=headers) |
|
123 + self.assertEqual(200, response.status) |
|
124 + response, content = http.request(path, 'HEAD') |
|
125 + self.assertEqual(200, response.status) |
|
126 + self.assertEqual('queued', response['x-image-meta-status']) |
|
127 + |
|
128 + # Make image active |
|
129 + http = httplib2.Http() |
|
130 + headers = {'Content-Type': 'application/octet-stream'} |
|
131 + response, content = http.request(path, 'PUT', headers=headers, |
|
132 + body='data') |
|
133 + self.assertEqual(200, response.status) |
|
134 + image = jsonutils.loads(content)['image'] |
|
135 + self.assertEqual('active', image['status']) |
|
136 + |
|
137 + # Ensure status of 'active' image can't be changed |
|
138 + http = httplib2.Http() |
|
139 + headers = {'X-Image-Meta-Status': 'queued'} |
|
140 + response, content = http.request(path, 'PUT', headers=headers) |
|
141 + self.assertEqual(403, response.status) |
|
142 + response, content = http.request(path, 'HEAD') |
|
143 + self.assertEqual(200, response.status) |
|
144 + self.assertEqual('active', response['x-image-meta-status']) |
|
145 + |
|
146 + # We allow 'setting' to the same status |
|
147 + http = httplib2.Http() |
|
148 + headers = {'X-Image-Meta-Status': 'active'} |
|
149 + response, content = http.request(path, 'PUT', headers=headers) |
|
150 + self.assertEqual(200, response.status) |
|
151 + response, content = http.request(path, 'HEAD') |
|
152 + self.assertEqual(200, response.status) |
|
153 + self.assertEqual('active', response['x-image-meta-status']) |
|
154 + |
|
155 + # Create a 'queued' image, ensure 'status' header is ignored |
|
156 + http = httplib2.Http() |
|
157 + path = "http://%s:%d/v1/images" % ("127.0.0.1", self.api_port) |
|
158 + headers = {'Content-Type': 'application/octet-stream', |
|
159 + 'X-Image-Meta-Status': 'active'} |
|
160 + response, content = http.request(path, 'POST', headers=headers, |
|
161 + body=None) |
|
162 + self.assertEqual(201, response.status) |
|
163 + image = jsonutils.loads(content)['image'] |
|
164 + self.assertEqual('queued', image['status']) |
|
165 + |
|
166 + # Create an 'active' image, ensure 'status' header is ignored |
|
167 + http = httplib2.Http() |
|
168 + path = "http://%s:%d/v1/images" % ("127.0.0.1", self.api_port) |
|
169 + headers = {'Content-Type': 'application/octet-stream', |
|
170 + 'X-Image-Meta-Disk-Format': 'raw', |
|
171 + 'X-Image-Meta-Status': 'queued', |
|
172 + 'X-Image-Meta-Container-Format': 'bare'} |
|
173 + response, content = http.request(path, 'POST', headers=headers, |
|
174 + body='data') |
|
175 + self.assertEqual(201, response.status) |
|
176 + image = jsonutils.loads(content)['image'] |
|
177 + self.assertEqual('active', image['status']) |
|
178 + self.stop_servers() |
|
179 diff --git a/glance/tests/integration/legacy_functional/test_v1_api.py b/glance/tests/integration/legacy_functional/test_v1_api.py |
|
180 index 66455a2..0e5b339 100644 |
|
181 --- a/glance/tests/integration/legacy_functional/test_v1_api.py |
|
182 +++ b/glance/tests/integration/legacy_functional/test_v1_api.py |
|
183 @@ -357,6 +357,8 @@ class TestApi(base.ApiTest): |
|
184 path = "/v1/images" |
|
185 response, content = self.http.request(path, 'POST', headers=headers) |
|
186 self.assertEqual(response.status, 201) |
|
187 + image = jsonutils.loads(content)['image'] |
|
188 + self.assertEqual('active', image['status']) |
|
189 |
|
190 # 2. HEAD image-location |
|
191 # Verify image size is zero and the status is active |
|
192 diff --git a/test-requirements.txt b/test-requirements.txt |
|
193 index 6d435f2..97affae 100644 |
|
194 --- a/test-requirements.txt |
|
195 +++ b/test-requirements.txt |
|
196 @@ -29,3 +29,8 @@ xattr>=0.4 |
|
197 |
|
198 # Documentation |
|
199 oslosphinx>=2.2.0 # Apache-2.0 |
|
200 + |
|
201 +# Gate is failing because of an older version of oslo.vmware is installing |
|
202 +# PyYAML 3.11. Adding this line here will help moving this patch forward and |
|
203 +# fixing Glance's stable/juno gate |
|
204 +PyYAML<=3.10,>=3.1.0 |
|
205 -- |
|
206 1.9.1 |