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
|
From 0e6b4a06ad72ac68ec41bab2063f8d167e8e277e Mon Sep 17 00:00:00 2001
From: Matthew Booth <mbooth@redhat.com>
Date: Thu, 10 Dec 2015 16:34:19 +0000
Subject: [PATCH 2/3] Fix format conversion in libvirt snapshot
The libvirt driver was calling images.convert_image during snapshot to
convert snapshots to the intended output format. However, this
function does not take the input format as an argument, meaning it
implicitly does format detection. This opened an exploit for setups
using raw storage on the backend, including raw on filesystem, LVM,
and RBD (Ceph). An authenticated user could write a qcow2 header to
their instance's disk which specified an arbitrary backing file on the
host. When convert_image ran during snapshot, this would then write
the contents of the backing file to glance, which is then available to
the user. If the setup uses an LVM backend this conversion runs as
root, meaning the user can exfiltrate any file on the host, including
raw disks.
This change adds an input format to convert_image.
Partial-Bug: #1524274
Change-Id: If73e73718ecd5db262ed9904091024238f98dbc0
(cherry picked from commit 840644d619e9560f205016eafc8799565ffd6d8c)
---
nova/tests/unit/virt/libvirt/test_driver.py | 5 +++--
nova/tests/unit/virt/libvirt/test_utils.py | 3 ++-
nova/virt/images.py | 26 ++++++++++++++++++++++++--
nova/virt/libvirt/imagebackend.py | 19 ++++++++++++++-----
4 files changed, 43 insertions(+), 10 deletions(-)
diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py
index 22ef56d..6fd8728 100644
--- a/nova/tests/unit/virt/libvirt/test_driver.py
+++ b/nova/tests/unit/virt/libvirt/test_driver.py
@@ -14985,7 +14985,7 @@ class LibvirtVolumeSnapshotTestCase(test.NoDBTestCase):
self.mox.VerifyAll()
-def _fake_convert_image(source, dest, out_format,
+def _fake_convert_image(source, dest, in_format, out_format,
run_as_root=True):
libvirt_driver.libvirt_utils.files[dest] = ''
@@ -15127,7 +15127,8 @@ class LVMSnapshotTests(_BaseSnapshotTests):
mock_volume_info.assert_has_calls([mock.call('/dev/nova-vg/lv')])
mock_convert_image.assert_called_once_with(
- '/dev/nova-vg/lv', mock.ANY, disk_format, run_as_root=True)
+ '/dev/nova-vg/lv', mock.ANY, 'raw', disk_format,
+ run_as_root=True)
def test_raw(self):
self._test_lvm_snapshot('raw')
diff --git a/nova/tests/unit/virt/libvirt/test_utils.py b/nova/tests/unit/virt/libvirt/test_utils.py
index 6773bea..6f75a92 100644
--- a/nova/tests/unit/virt/libvirt/test_utils.py
+++ b/nova/tests/unit/virt/libvirt/test_utils.py
@@ -594,7 +594,8 @@ disk size: 4.4M
target = 't.qcow2'
self.executes = []
expected_commands = [('qemu-img', 'convert', '-O', 'raw',
- 't.qcow2.part', 't.qcow2.converted'),
+ 't.qcow2.part', 't.qcow2.converted',
+ '-f', 'qcow2'),
('rm', 't.qcow2.part'),
('mv', 't.qcow2.converted', 't.qcow2')]
images.fetch_to_raw(context, image_id, target, user_id, project_id,
diff --git a/nova/virt/images.py b/nova/virt/images.py
index 5b9374b..e2b5b91 100644
--- a/nova/virt/images.py
+++ b/nova/virt/images.py
@@ -66,9 +66,31 @@ def qemu_img_info(path):
return imageutils.QemuImgInfo(out)
-def convert_image(source, dest, out_format, run_as_root=False):
+def convert_image(source, dest, in_format, out_format, run_as_root=False):
"""Convert image to other format."""
+ if in_format is None:
+ raise RuntimeError("convert_image without input format is a security"
+ "risk")
+ _convert_image(source, dest, in_format, out_format, run_as_root)
+
+
+def convert_image_unsafe(source, dest, out_format, run_as_root=False):
+ """Convert image to other format, doing unsafe automatic input format
+ detection. Do not call this function.
+ """
+
+ # NOTE: there is only 1 caller of this function:
+ # imagebackend.Lvm.create_image. It is not easy to fix that without a
+ # larger refactor, so for the moment it has been manually audited and
+ # allowed to continue. Remove this function when Lvm.create_image has
+ # been fixed.
+ _convert_image(source, dest, None, out_format, run_as_root)
+
+
+def _convert_image(source, dest, in_format, out_format, run_as_root):
cmd = ('qemu-img', 'convert', '-O', out_format, source, dest)
+ if in_format is not None:
+ cmd = cmd + ('-f', in_format)
utils.execute(*cmd, run_as_root=run_as_root)
@@ -123,7 +145,7 @@ def fetch_to_raw(context, image_href, path, user_id, project_id, max_size=0):
staged = "%s.converted" % path
LOG.debug("%s was %s, converting to raw" % (image_href, fmt))
with fileutils.remove_path_on_error(staged):
- convert_image(path_tmp, staged, 'raw')
+ convert_image(path_tmp, staged, fmt, 'raw')
os.unlink(path_tmp)
data = qemu_img_info(staged)
diff --git a/nova/virt/libvirt/imagebackend.py b/nova/virt/libvirt/imagebackend.py
index 5e14f61..151ebc4 100644
--- a/nova/virt/libvirt/imagebackend.py
+++ b/nova/virt/libvirt/imagebackend.py
@@ -477,7 +477,7 @@ class Raw(Image):
self.correct_format()
def snapshot_extract(self, target, out_format):
- images.convert_image(self.path, target, out_format)
+ images.convert_image(self.path, target, self.driver_format, out_format)
@staticmethod
def is_file_in_instance_path():
@@ -631,7 +631,16 @@ class Lvm(Image):
size, sparse=self.sparse)
if self.ephemeral_key_uuid is not None:
encrypt_lvm_image()
- images.convert_image(base, self.path, 'raw', run_as_root=True)
+ # NOTE: by calling convert_image_unsafe here we're
+ # telling qemu-img convert to do format detection on the input,
+ # because we don't know what the format is. For example,
+ # we might have downloaded a qcow2 image, or created an
+ # ephemeral filesystem locally, we just don't know here. Having
+ # audited this, all current sources have been sanity checked,
+ # either because they're locally generated, or because they have
+ # come from images.fetch_to_raw. However, this is major code smell.
+ images.convert_image_unsafe(base, self.path, self.driver_format,
+ run_as_root=True)
if resize:
disk.resize2fs(self.path, run_as_root=True)
@@ -678,8 +687,8 @@ class Lvm(Image):
lvm.remove_volumes([self.lv_path])
def snapshot_extract(self, target, out_format):
- images.convert_image(self.path, target, out_format,
- run_as_root=True)
+ images.convert_image(self.path, target, self.driver_format,
+ out_format, run_as_root=True)
def get_model(self, connection):
return imgmodel.LocalBlockImage(self.path)
@@ -786,7 +795,7 @@ class Rbd(Image):
self.driver.resize(self.rbd_name, size)
def snapshot_extract(self, target, out_format):
- images.convert_image(self.path, target, out_format)
+ images.convert_image(self.path, target, 'raw', out_format)
@staticmethod
def is_shared_block_storage():
--
2.5.0
|
GitWeb