summaryrefslogtreecommitdiff
blob: 9f2429df1abca201c37a6c3d78407482abb383e8 (plain)
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
From f41488f828fda1370e1b017503711248a810d432 Mon Sep 17 00:00:00 2001
From: Matthew Booth <mbooth@redhat.com>
Date: Wed, 9 Dec 2015 15:36:32 +0000
Subject: [PATCH 1/3] Fix format detection in libvirt snapshot

The libvirt driver was using automatic format detection during
snapshot for disks stored on the local filesystem. This opened an
exploit if nova was configured to use local file storage, and
additionally to store those files in raw format by specifying
use_cow_images = False in nova.conf. An authenticated user could write
a qcow2 header to their guest image with a backing file on the host.
libvirt.utils.get_disk_type() would then misdetect the type of this
image as qcow2 and pass this to the Qcow2 image backend, whose
snapshot_extract method interprets the image as qcow2 and writes the
backing file to glance. The authenticated user can then download the
host file from glance.

This patch makes 2 principal changes. libvirt.utils.get_disk_type,
which ought to be removed entirely as soon as possible, is updated to
no longer do format detection if the format can't be determined from
the path. Its name is changed to get_disk_type_from_path to reflect
its actual function.

libvirt.utils.find_disk is updated to return both the path and format
of the root disk, rather than just the path. This is the most reliable
source of this information, as it reflects the actual format in use.
The previous format detection function of get_disk_type is replaced by
the format taken from libvirt.

We replace a call to get_disk_type in _rebase_with_qemu_img with an
explicit call to qemu_img_info, as the other behaviour of
get_disk_type was not relevant in this context. qemu_img_info is safe
from the backing file exploit when called on a file known to be a
qcow2 image. As the file in this context is a volume snapshot, this is
a safe use.

(cherry picked from commit c69fbad4860a1ce931d80f3f0ce0f90da29e8e5f)

 Conflicts:
	nova/tests/unit/virt/libvirt/test_driver.py
	nova/tests/unit/virt/libvirt/test_utils.py
	nova/virt/libvirt/driver.py
	nova/virt/libvirt/utils.py

    Most about method _rebase_with_qemu_img which does not exist.

Partial-Bug: #1524274
Change-Id: I94c1c0d26215c061f71c3f95e1a6bf3a58fa19ea
---
 nova/tests/unit/virt/libvirt/fake_libvirt_utils.py | 10 +++--
 nova/tests/unit/virt/libvirt/test_utils.py         | 44 +++-------------------
 nova/virt/libvirt/driver.py                        | 25 +++++++++---
 nova/virt/libvirt/utils.py                         | 26 ++++++++++---
 4 files changed, 51 insertions(+), 54 deletions(-)

diff --git a/nova/tests/unit/virt/libvirt/fake_libvirt_utils.py b/nova/tests/unit/virt/libvirt/fake_libvirt_utils.py
index 302ccee..52d1e85 100644
--- a/nova/tests/unit/virt/libvirt/fake_libvirt_utils.py
+++ b/nova/tests/unit/virt/libvirt/fake_libvirt_utils.py
@@ -40,7 +40,9 @@ def get_disk_backing_file(path):
     return disk_backing_files.get(path, None)
 
 
-def get_disk_type(path):
+def get_disk_type_from_path(path):
+    if disk_type in ('raw', 'qcow2'):
+        return None
     return disk_type
 
 
@@ -99,11 +101,11 @@ def file_open(path, mode=None):
 
 def find_disk(virt_dom):
     if disk_type == 'lvm':
-        return "/dev/nova-vg/lv"
+        return ("/dev/nova-vg/lv", "raw")
     elif disk_type in ['raw', 'qcow2']:
-        return "filename"
+        return ("filename", disk_type)
     else:
-        return "unknown_type_disk"
+        return ("unknown_type_disk", None)
 
 
 def load_file(path):
diff --git a/nova/tests/unit/virt/libvirt/test_utils.py b/nova/tests/unit/virt/libvirt/test_utils.py
index ac7ea8d..6773bea 100644
--- a/nova/tests/unit/virt/libvirt/test_utils.py
+++ b/nova/tests/unit/virt/libvirt/test_utils.py
@@ -39,24 +39,6 @@ CONF = cfg.CONF
 
 class LibvirtUtilsTestCase(test.NoDBTestCase):
 
-    @mock.patch('os.path.exists', return_value=True)
-    @mock.patch('nova.utils.execute')
-    def test_get_disk_type(self, mock_execute, mock_exists):
-        path = "disk.config"
-        example_output = """image: disk.config
-file format: raw
-virtual size: 64M (67108864 bytes)
-cluster_size: 65536
-disk size: 96K
-blah BLAH: bb
-"""
-        mock_execute.return_value = (example_output, '')
-        disk_type = libvirt_utils.get_disk_type(path)
-        mock_execute.assert_called_once_with('env', 'LC_ALL=C', 'LANG=C',
-                                             'qemu-img', 'info', path)
-        mock_exists.assert_called_once_with(path)
-        self.assertEqual('raw', disk_type)
-
     @mock.patch('nova.utils.execute')
     def test_copy_image_local(self, mock_execute):
         libvirt_utils.copy_image('src', 'dest')
@@ -77,37 +59,21 @@ blah BLAH: bb
             on_completion=None, on_execute=None, compression=True)
 
     @mock.patch('os.path.exists', return_value=True)
-    def test_disk_type(self, mock_exists):
+    def test_disk_type_from_path(self, mock_exists):
         # Seems like lvm detection
         # if its in /dev ??
         for p in ['/dev/b', '/dev/blah/blah']:
-            d_type = libvirt_utils.get_disk_type(p)
+            d_type = libvirt_utils.get_disk_type_from_path(p)
             self.assertEqual('lvm', d_type)
 
         # Try rbd detection
-        d_type = libvirt_utils.get_disk_type('rbd:pool/instance')
+        d_type = libvirt_utils.get_disk_type_from_path('rbd:pool/instance')
         self.assertEqual('rbd', d_type)
 
         # Try the other types
-        template_output = """image: %(path)s
-file format: %(format)s
-virtual size: 64M (67108864 bytes)
-cluster_size: 65536
-disk size: 96K
-"""
         path = '/myhome/disk.config'
-        for f in ['raw', 'qcow2']:
-            output = template_output % ({
-                'format': f,
-                'path': path,
-            })
-            with mock.patch('nova.utils.execute',
-                return_value=(output, '')) as mock_execute:
-                d_type = libvirt_utils.get_disk_type(path)
-                mock_execute.assert_called_once_with(
-                    'env', 'LC_ALL=C', 'LANG=C',
-                    'qemu-img', 'info', path)
-                self.assertEqual(f, d_type)
+        d_type = libvirt_utils.get_disk_type_from_path(path)
+        self.assertIsNone(d_type)
 
     @mock.patch('os.path.exists', return_value=True)
     @mock.patch('nova.utils.execute')
diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py
index fc1c909..51b1e4b 100644
--- a/nova/virt/libvirt/driver.py
+++ b/nova/virt/libvirt/driver.py
@@ -1338,10 +1338,23 @@ class LibvirtDriver(driver.ComputeDriver):
 
         snapshot = self._image_api.get(context, image_id)
 
-        disk_path = libvirt_utils.find_disk(virt_dom)
-        source_format = libvirt_utils.get_disk_type(disk_path)
-
-        image_format = CONF.libvirt.snapshot_image_format or source_format
+        # source_format is an on-disk format
+        # source_type is a backend type
+        disk_path, source_format = libvirt_utils.find_disk(virt_dom)
+        source_type = libvirt_utils.get_disk_type_from_path(disk_path)
+
+        # We won't have source_type for raw or qcow2 disks, because we can't
+        # determine that from the path. We should have it from the libvirt
+        # xml, though.
+        if source_type is None:
+            source_type = source_format
+        # For lxc instances we won't have it either from libvirt xml
+        # (because we just gave libvirt the mounted filesystem), or the path,
+        # so source_type is still going to be None. In this case,
+        # snapshot_backend is going to default to CONF.libvirt.images_type
+        # below, which is still safe.
+
+        image_format = CONF.libvirt.snapshot_image_format or source_type
 
         # NOTE(bfilippov): save lvm and rbd as raw
         if image_format == 'lvm' or image_format == 'rbd':
@@ -1367,7 +1380,7 @@ class LibvirtDriver(driver.ComputeDriver):
         if (self._host.has_min_version(MIN_LIBVIRT_LIVESNAPSHOT_VERSION,
                                        MIN_QEMU_LIVESNAPSHOT_VERSION,
                                        host.HV_DRIVER_QEMU)
-             and source_format not in ('lvm', 'rbd')
+             and source_type not in ('lvm', 'rbd')
              and not CONF.ephemeral_storage_encryption.enabled
              and not CONF.workarounds.disable_libvirt_livesnapshot):
             live_snapshot = True
@@ -1402,7 +1415,7 @@ class LibvirtDriver(driver.ComputeDriver):
 
         snapshot_backend = self.image_backend.snapshot(instance,
                 disk_path,
-                image_type=source_format)
+                image_type=source_type)
 
         if live_snapshot:
             LOG.info(_LI("Beginning live snapshot process"),
diff --git a/nova/virt/libvirt/utils.py b/nova/virt/libvirt/utils.py
index 5573927..062b2fb 100644
--- a/nova/virt/libvirt/utils.py
+++ b/nova/virt/libvirt/utils.py
@@ -334,13 +334,20 @@ def find_disk(virt_dom):
     """
     xml_desc = virt_dom.XMLDesc(0)
     domain = etree.fromstring(xml_desc)
+    driver = None
     if CONF.libvirt.virt_type == 'lxc':
-        source = domain.find('devices/filesystem/source')
+        filesystem = domain.find('devices/filesystem')
+        driver = filesystem.find('driver')
+
+        source = filesystem.find('source')
         disk_path = source.get('dir')
         disk_path = disk_path[0:disk_path.rfind('rootfs')]
         disk_path = os.path.join(disk_path, 'disk')
     else:
-        source = domain.find('devices/disk/source')
+        disk = domain.find('devices/disk')
+        driver = disk.find('driver')
+
+        source = disk.find('source')
         disk_path = source.get('file') or source.get('dev')
         if not disk_path and CONF.libvirt.images_type == 'rbd':
             disk_path = source.get('name')
@@ -351,17 +358,26 @@ def find_disk(virt_dom):
         raise RuntimeError(_("Can't retrieve root device path "
                              "from instance libvirt configuration"))
 
-    return disk_path
+    if driver is not None:
+        format = driver.get('type')
+        # This is a legacy quirk of libvirt/xen. Everything else should
+        # report the on-disk format in type.
+        if format == 'aio':
+            format = 'raw'
+    else:
+        format = None
+    return (disk_path, format)
 
 
-def get_disk_type(path):
+def get_disk_type_from_path(path):
     """Retrieve disk type (raw, qcow2, lvm) for given file."""
     if path.startswith('/dev'):
         return 'lvm'
     elif path.startswith('rbd:'):
         return 'rbd'
 
-    return images.qemu_img_info(path).file_format
+    # We can't reliably determine the type from this path
+    return None
 
 
 def get_fs_info(path):
-- 
2.5.0