1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
267
268
269
270
271
272
273
274
275
276
277
278
279
280
281
282
283
284
285
286
287
288
289
290
291
292
293
294
295
296
297
298
299
300
301
302
303
304
305
306
307
308
309
310
311
312
313
314
315
316
317
318
319
320
321
322
323
324
325
326
327
328
329
330
331
332
333
334
335
336
337
338
339
340
341
342
343
344
345
346
347
348
349
350
351
352
353
354
355
356
357
358
359
360
361
362
363
364
365
366
367
368
369
370
371
372
373
374
375
376
377
378
379
380
381
382
383
384
385
386
387
388
389
390
391
392
393
394
395
396
397
398
399
400
401
402
403
404
405
406
407
408
409
410
411
412
413
414
415
416
417
418
419
420
421
422
423
424
425
426
427
428
429
430
431
432
433
434
435
436
437
438
439
440
441
442
443
444
445
446
447
448
449
450
451
452
|
From 3cdfe894ab58f7b91bf7fb690fc5bc724e44066f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= <pbrady@redhat.com>
Date: Fri, 27 Sep 2013 04:07:14 +0100
Subject: [PATCH] ensure we don't boot oversized images
Since we can't generally shrink incoming images, add extra checks
to ensure oversized images are not allowed through.
All cases when populating the libvirt image cache are now handled,
including the initial download from glance, where we avoid
converting to raw, as that could generate non sparse images
much larger than the downloaded image.
* nova/virt/libvirt/utils.py (fetch_image): Allow passing through
of the max_size parameter.
* nova/virt/images.py (fetch_to_raw): Accept the max_size parameter,
and use it to discard images with larger (virtual) sizes.
* nova/virt/libvirt/imagebackend.py (verify_base_size): A new
refactored function to identify and raise exception to oversized images.
(Raw.create_image): Pass the max_size to the fetch function.
Also enforce virtual image size checking for already fetched images,
as this class (despite the name) can be handling qcow files.
(Qcow2.create_image): Pass the max_size to the fetch function,
or verify the virtual size for the instance as done previously.
(Lvm.create_image): Pass the max_size to the fetch function.
Also check the size before transferring to the volume to improve
efficiency by not even attempting the transfer of oversized images.
(Rbd.create_image): Likewise.
* nova/tests/virt/libvirt/fake_libvirt_utils.py: Support max_size arg.
* nova/tests/virt/libvirt/test_libvirt.py (test_fetch_raw_image):
Add a case to check oversized images are discarded.
* nova/tests/virt/libvirt/test_imagebackend.py
(test_create_image_too_small): Adjust to avoid the fetch size check.
Fixes bug: 1177830
Fixes bug: 1206081
Change-Id: I3d47adaa2ad07434853f447feb27d7aae0e2e717
---
nova/tests/virt/libvirt/fake_libvirt_utils.py | 2 +-
nova/tests/virt/libvirt/test_imagebackend.py | 34 ++++++-----------
nova/tests/virt/libvirt/test_libvirt.py | 24 ++++++++++--
nova/virt/images.py | 24 ++++++++++--
nova/virt/libvirt/imagebackend.py | 55 +++++++++++++++++++--------
nova/virt/libvirt/utils.py | 5 ++-
6 files changed, 98 insertions(+), 46 deletions(-)
diff --git a/nova/tests/virt/libvirt/fake_libvirt_utils.py b/nova/tests/virt/libvirt/fake_libvirt_utils.py
index e18f9df..4799837 100644
--- a/nova/tests/virt/libvirt/fake_libvirt_utils.py
+++ b/nova/tests/virt/libvirt/fake_libvirt_utils.py
@@ -197,7 +197,7 @@ def get_fs_info(path):
'free': 84 * (1024 ** 3)}
-def fetch_image(context, target, image_id, user_id, project_id):
+def fetch_image(context, target, image_id, user_id, project_id, max_size=0):
pass
diff --git a/nova/tests/virt/libvirt/test_imagebackend.py b/nova/tests/virt/libvirt/test_imagebackend.py
index b862510..2455ec8 100644
--- a/nova/tests/virt/libvirt/test_imagebackend.py
+++ b/nova/tests/virt/libvirt/test_imagebackend.py
@@ -190,7 +190,7 @@ def prepare_mocks(self):
def test_create_image(self):
fn = self.prepare_mocks()
- fn(target=self.TEMPLATE_PATH, image_id=None)
+ fn(target=self.TEMPLATE_PATH, max_size=None, image_id=None)
imagebackend.libvirt_utils.copy_image(self.TEMPLATE_PATH, self.PATH)
self.mox.ReplayAll()
@@ -211,7 +211,7 @@ def test_create_image_generated(self):
def test_create_image_extend(self):
fn = self.prepare_mocks()
- fn(target=self.TEMPLATE_PATH, image_id=None)
+ fn(max_size=self.SIZE, target=self.TEMPLATE_PATH, image_id=None)
imagebackend.libvirt_utils.copy_image(self.TEMPLATE_PATH, self.PATH)
imagebackend.disk.extend(self.PATH, self.SIZE, use_cow=False)
self.mox.ReplayAll()
@@ -261,7 +261,7 @@ def prepare_mocks(self):
def test_create_image(self):
fn = self.prepare_mocks()
- fn(target=self.TEMPLATE_PATH)
+ fn(max_size=None, target=self.TEMPLATE_PATH)
imagebackend.libvirt_utils.create_cow_image(self.TEMPLATE_PATH,
self.PATH)
self.mox.ReplayAll()
@@ -273,15 +273,12 @@ def test_create_image(self):
def test_create_image_with_size(self):
fn = self.prepare_mocks()
- fn(target=self.TEMPLATE_PATH)
+ fn(max_size=self.SIZE, target=self.TEMPLATE_PATH)
self.mox.StubOutWithMock(os.path, 'exists')
- self.mox.StubOutWithMock(imagebackend.disk, 'get_disk_size')
if self.OLD_STYLE_INSTANCE_PATH:
os.path.exists(self.OLD_STYLE_INSTANCE_PATH).AndReturn(False)
os.path.exists(self.TEMPLATE_PATH).AndReturn(False)
os.path.exists(self.PATH).AndReturn(False)
- imagebackend.disk.get_disk_size(self.TEMPLATE_PATH
- ).AndReturn(self.SIZE)
os.path.exists(self.PATH).AndReturn(False)
imagebackend.libvirt_utils.create_cow_image(self.TEMPLATE_PATH,
self.PATH)
@@ -295,13 +292,11 @@ def test_create_image_with_size(self):
def test_create_image_too_small(self):
fn = self.prepare_mocks()
- fn(target=self.TEMPLATE_PATH)
self.mox.StubOutWithMock(os.path, 'exists')
self.mox.StubOutWithMock(imagebackend.disk, 'get_disk_size')
if self.OLD_STYLE_INSTANCE_PATH:
os.path.exists(self.OLD_STYLE_INSTANCE_PATH).AndReturn(False)
- os.path.exists(self.TEMPLATE_PATH).AndReturn(False)
- os.path.exists(self.PATH).AndReturn(False)
+ os.path.exists(self.TEMPLATE_PATH).AndReturn(True)
imagebackend.disk.get_disk_size(self.TEMPLATE_PATH
).AndReturn(self.SIZE)
self.mox.ReplayAll()
@@ -313,9 +308,8 @@ def test_create_image_too_small(self):
def test_generate_resized_backing_files(self):
fn = self.prepare_mocks()
- fn(target=self.TEMPLATE_PATH)
+ fn(max_size=self.SIZE, target=self.TEMPLATE_PATH)
self.mox.StubOutWithMock(os.path, 'exists')
- self.mox.StubOutWithMock(imagebackend.disk, 'get_disk_size')
self.mox.StubOutWithMock(imagebackend.libvirt_utils,
'get_disk_backing_file')
if self.OLD_STYLE_INSTANCE_PATH:
@@ -330,8 +324,6 @@ def test_generate_resized_backing_files(self):
self.QCOW2_BASE)
imagebackend.disk.extend(self.QCOW2_BASE, self.SIZE, use_cow=True)
- imagebackend.disk.get_disk_size(self.TEMPLATE_PATH
- ).AndReturn(self.SIZE)
os.path.exists(self.PATH).AndReturn(True)
self.mox.ReplayAll()
@@ -342,9 +334,8 @@ def test_generate_resized_backing_files(self):
def test_qcow2_exists_and_has_no_backing_file(self):
fn = self.prepare_mocks()
- fn(target=self.TEMPLATE_PATH)
+ fn(max_size=self.SIZE, target=self.TEMPLATE_PATH)
self.mox.StubOutWithMock(os.path, 'exists')
- self.mox.StubOutWithMock(imagebackend.disk, 'get_disk_size')
self.mox.StubOutWithMock(imagebackend.libvirt_utils,
'get_disk_backing_file')
if self.OLD_STYLE_INSTANCE_PATH:
@@ -354,8 +345,6 @@ def test_qcow2_exists_and_has_no_backing_file(self):
imagebackend.libvirt_utils.get_disk_backing_file(self.PATH)\
.AndReturn(None)
- imagebackend.disk.get_disk_size(self.TEMPLATE_PATH
- ).AndReturn(self.SIZE)
os.path.exists(self.PATH).AndReturn(True)
self.mox.ReplayAll()
@@ -392,7 +381,7 @@ def prepare_mocks(self):
def _create_image(self, sparse):
fn = self.prepare_mocks()
- fn(target=self.TEMPLATE_PATH)
+ fn(max_size=None, target=self.TEMPLATE_PATH)
self.libvirt_utils.create_lvm_image(self.VG,
self.LV,
self.TEMPLATE_SIZE,
@@ -424,7 +413,7 @@ def _create_image_generated(self, sparse):
def _create_image_resize(self, sparse):
fn = self.prepare_mocks()
- fn(target=self.TEMPLATE_PATH)
+ fn(max_size=self.SIZE, target=self.TEMPLATE_PATH)
self.libvirt_utils.create_lvm_image(self.VG, self.LV,
self.SIZE, sparse=sparse)
self.disk.get_disk_size(self.TEMPLATE_PATH
@@ -463,7 +452,7 @@ def test_create_image_resize_sparsed(self):
def test_create_image_negative(self):
fn = self.prepare_mocks()
- fn(target=self.TEMPLATE_PATH)
+ fn(max_size=self.SIZE, target=self.TEMPLATE_PATH)
self.libvirt_utils.create_lvm_image(self.VG,
self.LV,
self.SIZE,
@@ -607,7 +596,7 @@ def test_cache_template_exists(self):
def test_create_image(self):
fn = self.prepare_mocks()
- fn(rbd=self.rbd, target=self.TEMPLATE_PATH)
+ fn(max_size=None, rbd=self.rbd, target=self.TEMPLATE_PATH)
self.rbd.RBD_FEATURE_LAYERING = 1
@@ -635,6 +624,7 @@ def fake_fetch(target, *args, **kwargs):
return
self.stubs.Set(os.path, 'exists', lambda _: True)
+ self.stubs.Set(image, 'check_image_exists', lambda: True)
image.cache(fake_fetch, self.TEMPLATE_PATH, self.SIZE)
diff --git a/nova/tests/virt/libvirt/test_libvirt.py b/nova/tests/virt/libvirt/test_libvirt.py
index ac36be4..5d1361e 100644
--- a/nova/tests/virt/libvirt/test_libvirt.py
+++ b/nova/tests/virt/libvirt/test_libvirt.py
@@ -6308,7 +6308,8 @@ def test_fetch_image(self):
image_id = '4'
user_id = 'fake'
project_id = 'fake'
- images.fetch_to_raw(context, image_id, target, user_id, project_id)
+ images.fetch_to_raw(context, image_id, target, user_id, project_id,
+ max_size=0)
self.mox.ReplayAll()
libvirt_utils.fetch_image(context, target, image_id,
@@ -6338,20 +6339,27 @@ class FakeImgInfo(object):
file_format = path.split('.')[-2]
elif file_format == 'converted':
file_format = 'raw'
+
if 'backing' in path:
backing_file = 'backing'
else:
backing_file = None
+ if 'big' in path:
+ virtual_size = 2
+ else:
+ virtual_size = 1
+
FakeImgInfo.file_format = file_format
FakeImgInfo.backing_file = backing_file
+ FakeImgInfo.virtual_size = virtual_size
return FakeImgInfo()
self.stubs.Set(utils, 'execute', fake_execute)
self.stubs.Set(os, 'rename', fake_rename)
self.stubs.Set(os, 'unlink', fake_unlink)
- self.stubs.Set(images, 'fetch', lambda *_: None)
+ self.stubs.Set(images, 'fetch', lambda *_, **__: None)
self.stubs.Set(images, 'qemu_img_info', fake_qemu_img_info)
self.stubs.Set(fileutils, 'delete_if_exists', fake_rm_on_error)
@@ -6373,7 +6381,8 @@ class FakeImgInfo(object):
't.qcow2.part', 't.qcow2.converted'),
('rm', 't.qcow2.part'),
('mv', 't.qcow2.converted', 't.qcow2')]
- images.fetch_to_raw(context, image_id, target, user_id, project_id)
+ images.fetch_to_raw(context, image_id, target, user_id, project_id,
+ max_size=1)
self.assertEqual(self.executes, expected_commands)
target = 't.raw'
@@ -6390,6 +6399,15 @@ class FakeImgInfo(object):
context, image_id, target, user_id, project_id)
self.assertEqual(self.executes, expected_commands)
+ target = 'big.qcow2'
+ self.executes = []
+ expected_commands = [('rm', '-f', 'big.qcow2.part')]
+ self.assertRaises(exception.InstanceTypeDiskTooSmall,
+ images.fetch_to_raw,
+ context, image_id, target, user_id, project_id,
+ max_size=1)
+ self.assertEqual(self.executes, expected_commands)
+
del self.executes
def test_get_disk_backing_file(self):
diff --git a/nova/virt/images.py b/nova/virt/images.py
index 9c4c101..6d20e65 100644
--- a/nova/virt/images.py
+++ b/nova/virt/images.py
@@ -179,7 +179,7 @@ def convert_image(source, dest, out_format, run_as_root=False):
utils.execute(*cmd, run_as_root=run_as_root)
-def fetch(context, image_href, path, _user_id, _project_id):
+def fetch(context, image_href, path, _user_id, _project_id, max_size=0):
# TODO(vish): Improve context handling and add owner and auth data
# when it is added to glance. Right now there is no
# auth checking in glance, so we assume that access was
@@ -190,9 +190,10 @@ def fetch(context, image_href, path, _user_id, _project_id):
image_service.download(context, image_id, dst_path=path)
-def fetch_to_raw(context, image_href, path, user_id, project_id):
+def fetch_to_raw(context, image_href, path, user_id, project_id, max_size=0):
path_tmp = "%s.part" % path
- fetch(context, image_href, path_tmp, user_id, project_id)
+ fetch(context, image_href, path_tmp, user_id, project_id,
+ max_size=max_size)
with fileutils.remove_path_on_error(path_tmp):
data = qemu_img_info(path_tmp)
@@ -209,6 +210,23 @@ def fetch_to_raw(context, image_href, path, user_id, project_id):
reason=(_("fmt=%(fmt)s backed by: %(backing_file)s") %
{'fmt': fmt, 'backing_file': backing_file}))
+ # We can't generally shrink incoming images, so disallow
+ # images > size of the flavor we're booting. Checking here avoids
+ # an immediate DoS where we convert large qcow images to raw
+ # (which may compress well but not be sparse).
+ # TODO(p-draigbrady): loop through all flavor sizes, so that
+ # we might continue here and not discard the download.
+ # If we did that we'd have to do the higher level size checks
+ # irrespective of whether the base image was prepared or not.
+ disk_size = data.virtual_size
+ if max_size and max_size < disk_size:
+ msg = _('%(base)s virtual size %(disk_size)s '
+ 'larger than flavor root disk size %(size)s')
+ LOG.error(msg % {'base': path,
+ 'disk_size': disk_size,
+ 'size': max_size})
+ raise exception.InstanceTypeDiskTooSmall()
+
if fmt != "raw" and CONF.force_raw_images:
staged = "%s.converted" % path
LOG.debug("%s was %s, converting to raw" % (image_href, fmt))
diff --git a/nova/virt/libvirt/imagebackend.py b/nova/virt/libvirt/imagebackend.py
index 84c46e8..e900789 100644
--- a/nova/virt/libvirt/imagebackend.py
+++ b/nova/virt/libvirt/imagebackend.py
@@ -193,6 +193,36 @@ def _can_fallocate(self):
(CONF.preallocate_images, self.path))
return can_fallocate
+ @staticmethod
+ def verify_base_size(base, size, base_size=0):
+ """Check that the base image is not larger than size.
+ Since images can't be generally shrunk, enforce this
+ constraint taking account of virtual image size.
+ """
+
+ # Note(pbrady): The size and min_disk parameters of a glance
+ # image are checked against the instance size before the image
+ # is even downloaded from glance, but currently min_disk is
+ # adjustable and doesn't currently account for virtual disk size,
+ # so we need this extra check here.
+ # NOTE(cfb): Having a flavor that sets the root size to 0 and having
+ # nova effectively ignore that size and use the size of the
+ # image is considered a feature at this time, not a bug.
+
+ if size is None:
+ return
+
+ if size and not base_size:
+ base_size = disk.get_disk_size(base)
+
+ if size < base_size:
+ msg = _('%(base)s virtual size %(base_size)s '
+ 'larger than flavor root disk size %(size)s')
+ LOG.error(msg % {'base': base,
+ 'base_size': base_size,
+ 'size': size})
+ raise exception.InstanceTypeDiskTooSmall()
+
def snapshot_create(self):
raise NotImplementedError()
@@ -234,7 +264,8 @@ def copy_raw_image(base, target, size):
#Generating image in place
prepare_template(target=self.path, *args, **kwargs)
else:
- prepare_template(target=base, *args, **kwargs)
+ prepare_template(target=base, max_size=size, *args, **kwargs)
+ self.verify_base_size(base, size)
if not os.path.exists(self.path):
with fileutils.remove_path_on_error(self.path):
copy_raw_image(base, self.path, size)
@@ -273,7 +304,9 @@ def copy_qcow2_image(base, target, size):
# Download the unmodified base image unless we already have a copy.
if not os.path.exists(base):
- prepare_template(target=base, *args, **kwargs)
+ prepare_template(target=base, max_size=size, *args, **kwargs)
+ else:
+ self.verify_base_size(base, size)
legacy_backing_size = None
legacy_base = base
@@ -299,17 +332,6 @@ def copy_qcow2_image(base, target, size):
libvirt_utils.copy_image(base, legacy_base)
disk.extend(legacy_base, legacy_backing_size, use_cow=True)
- # NOTE(cfb): Having a flavor that sets the root size to 0 and having
- # nova effectively ignore that size and use the size of the
- # image is considered a feature at this time, not a bug.
- disk_size = disk.get_disk_size(base)
- if size and size < disk_size:
- msg = _('%(base)s virtual size %(disk_size)s'
- 'larger than flavor root disk size %(size)s')
- LOG.error(msg % {'base': base,
- 'disk_size': disk_size,
- 'size': size})
- raise exception.InstanceTypeDiskTooSmall()
if not os.path.exists(self.path):
with fileutils.remove_path_on_error(self.path):
copy_qcow2_image(base, self.path, size)
@@ -367,6 +389,7 @@ def create_image(self, prepare_template, base, size, *args, **kwargs):
@utils.synchronized(base, external=True, lock_path=self.lock_path)
def create_lvm_image(base, size):
base_size = disk.get_disk_size(base)
+ self.verify_base_size(base, size, base_size=base_size)
resize = size > base_size
size = size if resize else base_size
libvirt_utils.create_lvm_image(self.vg, self.lv,
@@ -384,7 +407,7 @@ def create_lvm_image(base, size):
with self.remove_volume_on_error(self.path):
prepare_template(target=self.path, *args, **kwargs)
else:
- prepare_template(target=base, *args, **kwargs)
+ prepare_template(target=base, max_size=size, *args, **kwargs)
with self.remove_volume_on_error(self.path):
create_lvm_image(base, size)
@@ -514,7 +537,9 @@ def create_image(self, prepare_template, base, size, *args, **kwargs):
features = self.rbd.RBD_FEATURE_LAYERING
if not os.path.exists(base):
- prepare_template(target=base, *args, **kwargs)
+ prepare_template(target=base, max_size=size, *args, **kwargs)
+ else:
+ self.verify_base_size(base, size)
# keep using the command line import instead of librbd since it
# detects zeroes to preserve sparseness in the image
diff --git a/nova/virt/libvirt/utils.py b/nova/virt/libvirt/utils.py
index 66ff83e..d7c92b7 100644
--- a/nova/virt/libvirt/utils.py
+++ b/nova/virt/libvirt/utils.py
@@ -639,9 +639,10 @@ def get_fs_info(path):
'used': used}
-def fetch_image(context, target, image_id, user_id, project_id):
+def fetch_image(context, target, image_id, user_id, project_id, max_size=0):
"""Grab image."""
- images.fetch_to_raw(context, image_id, target, user_id, project_id)
+ images.fetch_to_raw(context, image_id, target, user_id, project_id,
+ max_size=max_size)
def get_instance_path(instance, forceold=False, relative=False):
--
1.8.4
|