|
1 Upstream patch to fix CVE-2014-5356. This fix will be included in |
|
2 future 2013.2.4 and 2014.1.3 releases. |
|
3 |
|
4 From 12f43cfed5a47cd16f08b7dad2424da0fc362e47 Mon Sep 17 00:00:00 2001 |
|
5 From: Tom Leaman <[email protected]> |
|
6 Date: Fri, 2 May 2014 10:09:20 +0000 |
|
7 Subject: [PATCH] Enforce image_size_cap on v2 upload |
|
8 |
|
9 image_size_cap should be checked and enforced on upload |
|
10 |
|
11 Enforcement is in two places: |
|
12 - on image metadata save |
|
13 - during image save to backend store |
|
14 |
|
15 (cherry picked from commit 92ab00fca6926eaf3f7f92a955a5e07140063718) |
|
16 Conflicts: |
|
17 glance/location.py |
|
18 glance/tests/functional/v2/test_images.py |
|
19 glance/tests/unit/test_store_image.py |
|
20 |
|
21 Closes-Bug: 1315321 |
|
22 Change-Id: I45bfb360703617bc394e9e27fe17adf43b09c0e1 |
|
23 Co-Author: Manuel Desbonnet <[email protected]> |
|
24 --- |
|
25 glance/db/__init__.py | 5 ++++ |
|
26 glance/store/__init__.py | 5 +++- |
|
27 glance/tests/functional/__init__.py | 2 ++ |
|
28 glance/tests/functional/v2/test_images.py | 42 +++++++++++++++++++++++++++++ |
|
29 glance/tests/unit/test_store_image.py | 6 +++-- |
|
30 glance/tests/unit/utils.py | 5 +++- |
|
31 6 files changed, 61 insertions(+), 4 deletions(-) |
|
32 |
|
33 diff --git a/glance/db/__init__.py b/glance/db/__init__.py |
|
34 index 56f4dac..8ac3606 100644 |
|
35 --- a/glance/db/__init__.py |
|
36 +++ b/glance/db/__init__.py |
|
37 @@ -32,6 +32,7 @@ db_opt = cfg.BoolOpt('use_tpool', |
|
38 'all DB API calls') |
|
39 |
|
40 CONF = cfg.CONF |
|
41 +CONF.import_opt('image_size_cap', 'glance.common.config') |
|
42 CONF.import_opt('metadata_encryption_key', 'glance.common.config') |
|
43 CONF.register_opt(db_opt) |
|
44 |
|
45 @@ -148,6 +149,8 @@ class ImageRepo(object): |
|
46 |
|
47 def add(self, image): |
|
48 image_values = self._format_image_to_db(image) |
|
49 + if image_values['size'] > CONF.image_size_cap: |
|
50 + raise exception.ImageSizeLimitExceeded |
|
51 # the updated_at value is not set in the _format_image_to_db |
|
52 # function since it is specific to image create |
|
53 image_values['updated_at'] = image.updated_at |
|
54 @@ -159,6 +162,8 @@ class ImageRepo(object): |
|
55 |
|
56 def save(self, image): |
|
57 image_values = self._format_image_to_db(image) |
|
58 + if image_values['size'] > CONF.image_size_cap: |
|
59 + raise exception.ImageSizeLimitExceeded |
|
60 try: |
|
61 new_values = self.db_api.image_update(self.context, |
|
62 image.image_id, |
|
63 diff --git a/glance/store/__init__.py b/glance/store/__init__.py |
|
64 index fa17f4b..fd25e27 100644 |
|
65 --- a/glance/store/__init__.py |
|
66 +++ b/glance/store/__init__.py |
|
67 @@ -646,7 +646,10 @@ class ImageProxy(glance.domain.proxy.Image): |
|
68 size = 0 # NOTE(markwash): zero -> unknown size |
|
69 location, size, checksum, loc_meta = self.store_api.add_to_backend( |
|
70 self.context, CONF.default_store, |
|
71 - self.image.image_id, utils.CooperativeReader(data), size) |
|
72 + self.image.image_id, |
|
73 + utils.LimitingReader(utils.CooperativeReader(data), |
|
74 + CONF.image_size_cap), |
|
75 + size) |
|
76 self.image.locations = [{'url': location, 'metadata': loc_meta}] |
|
77 self.image.size = size |
|
78 self.image.checksum = checksum |
|
79 diff --git a/glance/tests/functional/__init__.py b/glance/tests/functional/__init__.py |
|
80 index 1256133..18a1a7e 100644 |
|
81 --- a/glance/tests/functional/__init__.py |
|
82 +++ b/glance/tests/functional/__init__.py |
|
83 @@ -279,6 +279,7 @@ class ApiServer(Server): |
|
84 self.pid_file = pid_file or os.path.join(self.test_dir, "api.pid") |
|
85 self.scrubber_datadir = os.path.join(self.test_dir, "scrubber") |
|
86 self.log_file = os.path.join(self.test_dir, "api.log") |
|
87 + self.image_size_cap = 1099511627776 |
|
88 self.s3_store_host = "s3.amazonaws.com" |
|
89 self.s3_store_access_key = "" |
|
90 self.s3_store_secret_key = "" |
|
91 @@ -332,6 +333,7 @@ metadata_encryption_key = %(metadata_encryption_key)s |
|
92 registry_host = 127.0.0.1 |
|
93 registry_port = %(registry_port)s |
|
94 log_file = %(log_file)s |
|
95 +image_size_cap = %(image_size_cap)d |
|
96 s3_store_host = %(s3_store_host)s |
|
97 s3_store_access_key = %(s3_store_access_key)s |
|
98 s3_store_secret_key = %(s3_store_secret_key)s |
|
99 diff --git a/glance/tests/functional/v2/test_images.py b/glance/tests/functional/v2/test_images.py |
|
100 index a9f9147..c25d4e2 100644 |
|
101 --- a/glance/tests/functional/v2/test_images.py |
|
102 +++ b/glance/tests/functional/v2/test_images.py |
|
103 @@ -259,6 +259,48 @@ class TestImages(functional.FunctionalTest): |
|
104 |
|
105 self.stop_servers() |
|
106 |
|
107 + def test_image_size_cap(self): |
|
108 + self.api_server.image_size_cap = 128 |
|
109 + self.start_servers(**self.__dict__.copy()) |
|
110 + # create an image |
|
111 + path = self._url('/v2/images') |
|
112 + headers = self._headers({'content-type': 'application/json'}) |
|
113 + data = json.dumps({'name': 'image-size-cap-test-image', |
|
114 + 'type': 'kernel', 'disk_format': 'aki', |
|
115 + 'container_format': 'aki'}) |
|
116 + response = requests.post(path, headers=headers, data=data) |
|
117 + self.assertEqual(201, response.status_code) |
|
118 + |
|
119 + image = json.loads(response.text) |
|
120 + image_id = image['id'] |
|
121 + |
|
122 + #try to populate it with oversized data |
|
123 + path = self._url('/v2/images/%s/file' % image_id) |
|
124 + headers = self._headers({'Content-Type': 'application/octet-stream'}) |
|
125 + |
|
126 + class StreamSim(object): |
|
127 + # Using a one-shot iterator to force chunked transfer in the PUT |
|
128 + # request |
|
129 + def __init__(self, size): |
|
130 + self.size = size |
|
131 + |
|
132 + def __iter__(self): |
|
133 + yield 'Z' * self.size |
|
134 + |
|
135 + response = requests.put(path, headers=headers, data=StreamSim( |
|
136 + self.api_server.image_size_cap + 1)) |
|
137 + self.assertEqual(413, response.status_code) |
|
138 + |
|
139 + # hashlib.md5('Z'*129).hexdigest() |
|
140 + # == '76522d28cb4418f12704dfa7acd6e7ee' |
|
141 + # If the image has this checksum, it means that the whole stream was |
|
142 + # accepted and written to the store, which should not be the case. |
|
143 + path = self._url('/v2/images/{0}'.format(image_id)) |
|
144 + headers = self._headers({'content-type': 'application/json'}) |
|
145 + response = requests.get(path, headers=headers) |
|
146 + image_checksum = json.loads(response.text).get('checksum') |
|
147 + self.assertNotEqual(image_checksum, '76522d28cb4418f12704dfa7acd6e7ee') |
|
148 + |
|
149 def test_permissions(self): |
|
150 # Create an image that belongs to TENANT1 |
|
151 path = self._url('/v2/images') |
|
152 diff --git a/glance/tests/unit/test_store_image.py b/glance/tests/unit/test_store_image.py |
|
153 index f9f5d85..5bdd51e 100644 |
|
154 --- a/glance/tests/unit/test_store_image.py |
|
155 +++ b/glance/tests/unit/test_store_image.py |
|
156 @@ -126,8 +126,10 @@ class TestStoreImage(utils.BaseTestCase): |
|
157 |
|
158 self.stubs.Set(unit_test_utils.FakeStoreAPI, 'get_from_backend', |
|
159 fake_get_from_backend) |
|
160 - |
|
161 - self.assertEquals(image1.get_data().fd, 'ZZZ') |
|
162 + # This time, image1.get_data() returns the data wrapped in a |
|
163 + # LimitingReader|CooperativeReader pipeline, so peeking under |
|
164 + # the hood of those objects to get at the underlying string. |
|
165 + self.assertEquals(image1.get_data().data.fd, 'ZZZ') |
|
166 image1.locations.pop(0) |
|
167 self.assertEquals(len(image1.locations), 1) |
|
168 image2.delete() |
|
169 diff --git a/glance/tests/unit/utils.py b/glance/tests/unit/utils.py |
|
170 index dff87b1..ec62828 100644 |
|
171 --- a/glance/tests/unit/utils.py |
|
172 +++ b/glance/tests/unit/utils.py |
|
173 @@ -149,7 +149,10 @@ class FakeStoreAPI(object): |
|
174 if image_id in location: |
|
175 raise exception.Duplicate() |
|
176 if not size: |
|
177 - size = len(data.fd) |
|
178 + # 'data' is a string wrapped in a LimitingReader|CooperativeReader |
|
179 + # pipeline, so peek under the hood of those objects to get at the |
|
180 + # string itself. |
|
181 + size = len(data.data.fd) |
|
182 if (current_store_size + size) > store_max_size: |
|
183 raise exception.StorageFull() |
|
184 if context.user == USER2: |
|
185 -- |
|
186 1.7.9.5 |
|
187 |