From dca3c8323cf8cf12aa8ce4ba21f647ce631e8153 Mon Sep 17 00:00:00 2001 From: Eric Harney Date: Tue, 9 Sep 2014 16:20:24 -0400 Subject: [PATCH] Refuse invalid qcow2 backing files Don't allow qcow2 files that are pointing to backing files outside of: volume- volume-. volume-.tmp-snap- (optionally prefixed with /mnt/path) Closes-Bug: #1350504 Change-Id: Ic89cffc93940b7b119cfcde3362f304c9f2875df --- cinder/tests/test_glusterfs.py | 13 ++++++------ cinder/tests/test_smbfs.py | 4 +++- cinder/volume/drivers/glusterfs.py | 13 ++++++++---- cinder/volume/drivers/remotefs.py | 34 ++++++++++++++++++++++++------ cinder/volume/drivers/smbfs.py | 21 ++++++++++++------ 5 files changed, 60 insertions(+), 25 deletions(-) diff --git a/cinder/tests/test_glusterfs.py b/cinder/tests/test_glusterfs.py index 9961408c9..269d71116 100644 --- a/cinder/tests/test_glusterfs.py +++ b/cinder/tests/test_glusterfs.py @@ -83,6 +83,7 @@ class GlusterFsDriverTestCase(test.TestCase): TEST_SHARES_CONFIG_FILE = '/etc/cinder/test-shares.conf' TEST_TMP_FILE = '/tmp/tempfile' VOLUME_UUID = 'abcdefab-cdef-abcd-efab-cdefabcdefab' + VOLUME_NAME = 'volume-%s' % VOLUME_UUID SNAP_UUID = 'bacadaca-baca-daca-baca-dacadacadaca' SNAP_UUID_2 = 'bebedede-bebe-dede-bebe-dedebebedede' @@ -1477,8 +1478,8 @@ class GlusterFsDriverTestCase(test.TestCase): volume = self._simple_volume() vol_filename = volume['name'] - vol_filename_2 = volume['name'] + '.asdfjkl' - vol_filename_3 = volume['name'] + 'qwertyuiop' + vol_filename_2 = volume['name'] + '.abcd' + vol_filename_3 = volume['name'] + '.efef' hashed = drv._get_hash_str(self.TEST_EXPORT1) vol_dir = '%s/%s' % (self.TEST_MNT_POINT_BASE, hashed) vol_path = '%s/%s' % (vol_dir, vol_filename) @@ -1687,7 +1688,7 @@ class GlusterFsDriverTestCase(test.TestCase): info = imageutils.QemuImgInfo() info.file_format = 'raw' - drv._qemu_img_info(IgnoreArg()).AndReturn(info) + drv._qemu_img_info(IgnoreArg(), IgnoreArg()).AndReturn(info) base_driver.VolumeDriver.backup_volume(IgnoreArg(), IgnoreArg(), @@ -1723,7 +1724,7 @@ class GlusterFsDriverTestCase(test.TestCase): info = imageutils.QemuImgInfo() info.file_format = 'raw' - drv._qemu_img_info(IgnoreArg()).AndReturn(info) + drv._qemu_img_info(IgnoreArg(), IgnoreArg()).AndReturn(info) base_driver.VolumeDriver.backup_volume(IgnoreArg(), IgnoreArg(), @@ -1777,7 +1778,7 @@ class GlusterFsDriverTestCase(test.TestCase): info.file_format = 'raw' info.backing_file = 'file1' - drv._qemu_img_info(IgnoreArg()).AndReturn(info) + drv._qemu_img_info(IgnoreArg(), IgnoreArg()).AndReturn(info) mox.ReplayAll() @@ -1806,7 +1807,7 @@ class GlusterFsDriverTestCase(test.TestCase): info.file_format = 'qcow2' drv.db.volume_get(ctxt, volume['id']).AndReturn(volume) - drv._qemu_img_info(IgnoreArg()).AndReturn(info) + drv._qemu_img_info(IgnoreArg(), IgnoreArg()).AndReturn(info) mox.ReplayAll() diff --git a/cinder/tests/test_smbfs.py b/cinder/tests/test_smbfs.py index 057b73d02..7f315412f 100644 --- a/cinder/tests/test_smbfs.py +++ b/cinder/tests/test_smbfs.py @@ -504,7 +504,9 @@ class SmbFsTestCase(test.TestCase): drv._DISK_FORMAT_VHDX, mock.sentinel.block_size) drv._do_extend_volume.assert_called_once_with( - self._FAKE_VOLUME_PATH, self._FAKE_VOLUME['size']) + self._FAKE_VOLUME_PATH, + self._FAKE_VOLUME['size'], + self._FAKE_VOLUME['name']) def test_copy_image_to_volume(self): self._test_copy_image_to_volume() diff --git a/cinder/volume/drivers/glusterfs.py b/cinder/volume/drivers/glusterfs.py index d494a823a..27aac48b9 100644 --- a/cinder/volume/drivers/glusterfs.py +++ b/cinder/volume/drivers/glusterfs.py @@ -153,6 +153,10 @@ class GlusterfsDriver(remotefs_drv.RemoteFSSnapDriver): self._ensure_shares_mounted() + def _qemu_img_info(self, path, volume_name): + return super(GlusterfsDriver, self)._qemu_img_info_base( + path, volume_name, self.configuration.glusterfs_mount_point_base) + def check_for_setup_error(self): """Just to override parent behavior.""" pass @@ -207,7 +211,8 @@ class GlusterfsDriver(remotefs_drv.RemoteFSSnapDriver): # Find the file which backs this file, which represents the point # when this snapshot was created. - img_info = self._qemu_img_info(forward_path) + img_info = self._qemu_img_info(forward_path, + snapshot['volume']['name']) path_to_snap_img = os.path.join(vol_path, img_info.backing_file) path_to_new_vol = self._local_path_volume(volume) @@ -392,7 +397,7 @@ class GlusterfsDriver(remotefs_drv.RemoteFSSnapDriver): data['options'] = self.shares[volume['provider_location']] # Test file for raw vs. qcow2 format - info = self._qemu_img_info(path) + info = self._qemu_img_info(path, volume['name']) data['format'] = info.file_format if data['format'] not in ['raw', 'qcow2']: msg = _('%s must be a valid raw or qcow2 image.') % path @@ -425,7 +430,7 @@ class GlusterfsDriver(remotefs_drv.RemoteFSSnapDriver): ' driver when no snapshots exist.') raise exception.InvalidVolume(msg) - info = self._qemu_img_info(volume_path) + info = self._qemu_img_info(volume_path, volume['name']) backing_fmt = info.file_format if backing_fmt not in ['raw', 'qcow2']: @@ -557,7 +562,7 @@ class GlusterfsDriver(remotefs_drv.RemoteFSSnapDriver): volume_dir, self.get_active_image_from_info(volume)) - info = self._qemu_img_info(active_file_path) + info = self._qemu_img_info(active_file_path, volume['name']) if info.backing_file is not None: msg = _('No snapshots found in database, but ' diff --git a/cinder/volume/drivers/remotefs.py b/cinder/volume/drivers/remotefs.py index 54904c167..abed9c806 100644 --- a/cinder/volume/drivers/remotefs.py +++ b/cinder/volume/drivers/remotefs.py @@ -425,7 +425,7 @@ class RemoteFSSnapDriver(RemoteFSDriver): with open(info_path, 'w') as f: json.dump(snap_info, f, indent=1, sort_keys=True) - def _qemu_img_info(self, path): + def _qemu_img_info_base(self, path, volume_name, basedir): """Sanitize image_utils' qemu_img_info. This code expects to deal only with relative filenames. @@ -435,10 +435,25 @@ class RemoteFSSnapDriver(RemoteFSDriver): if info.image: info.image = os.path.basename(info.image) if info.backing_file: + backing_file_template = \ + "(%(basedir)s/[0-9a-f]+/)?%" \ + "(volname)s(.(tmp-snap-)?[0-9a-f-]+)?$" % { + 'basedir': basedir, + 'volname': volume_name + } + if not re.match(backing_file_template, info.backing_file): + msg = _("File %(path)s has invalid backing file " + "%(bfile)s, aborting.") % {'path': path, + 'bfile': info.backing_file} + raise exception.RemoteFSException(msg) + info.backing_file = os.path.basename(info.backing_file) return info + def _qemu_img_info(self, path, volume_name): + raise NotImplementedError() + def _img_commit(self, path): self._execute('qemu-img', 'commit', path, run_as_root=True) self._delete(path) @@ -476,7 +491,7 @@ class RemoteFSSnapDriver(RemoteFSDriver): output = [] - info = self._qemu_img_info(path) + info = self._qemu_img_info(path, volume['name']) new_info = {} new_info['filename'] = os.path.basename(path) new_info['backing-filename'] = info.backing_file @@ -486,7 +501,7 @@ class RemoteFSSnapDriver(RemoteFSDriver): while new_info['backing-filename']: filename = new_info['backing-filename'] path = os.path.join(self._local_volume_dir(volume), filename) - info = self._qemu_img_info(path) + info = self._qemu_img_info(path, volume['name']) backing_filename = info.backing_file new_info = {} new_info['filename'] = filename @@ -559,7 +574,7 @@ class RemoteFSSnapDriver(RemoteFSDriver): active_file = self.get_active_image_from_info(volume) active_file_path = os.path.join(self._local_volume_dir(volume), active_file) - info = self._qemu_img_info(active_file_path) + info = self._qemu_img_info(active_file_path, volume['name']) backing_file = info.backing_file root_file_fmt = info.file_format @@ -692,7 +707,9 @@ class RemoteFSSnapDriver(RemoteFSDriver): self._local_volume_dir(snapshot['volume']), snapshot_file) - snapshot_path_img_info = self._qemu_img_info(snapshot_path) + snapshot_path_img_info = self._qemu_img_info( + snapshot_path, + snapshot['volume']['name']) vol_path = self._local_volume_dir(snapshot['volume']) @@ -718,7 +735,9 @@ class RemoteFSSnapDriver(RemoteFSDriver): base_path = os.path.join( self._local_volume_dir(snapshot['volume']), base_file) - base_file_img_info = self._qemu_img_info(base_path) + base_file_img_info = self._qemu_img_info( + base_path, + snapshot['volume']['name']) new_base_file = base_file_img_info.backing_file base_id = None @@ -868,7 +887,8 @@ class RemoteFSSnapDriver(RemoteFSDriver): 'backing_file=%s' % backing_path_full_path, new_snap_path] self._execute(*command, run_as_root=True) - info = self._qemu_img_info(backing_path_full_path) + info = self._qemu_img_info(backing_path_full_path, + snapshot['volume']['name']) backing_fmt = info.file_format command = ['qemu-img', 'rebase', '-u', diff --git a/cinder/volume/drivers/smbfs.py b/cinder/volume/drivers/smbfs.py index dab55fbf1..2a355a891 100644 --- a/cinder/volume/drivers/smbfs.py +++ b/cinder/volume/drivers/smbfs.py @@ -101,6 +101,10 @@ class SmbfsDriver(remotefs_drv.RemoteFSSnapDriver): smbfs_mount_options=opts) self.img_suffix = None + def _qemu_img_info(self, path, volume_name): + return super(SmbfsDriver, self)._qemu_img_info_base( + path, volume_name, self.configuration.smbfs_mount_point_base) + def initialize_connection(self, volume, connector): """Allow connection to connector and return connection info. @@ -111,7 +115,7 @@ class SmbfsDriver(remotefs_drv.RemoteFSSnapDriver): active_file = self.get_active_image_from_info(volume) active_file_path = os.path.join(self._local_volume_dir(volume), active_file) - info = self._qemu_img_info(active_file_path) + info = self._qemu_img_info(active_file_path, volume['name']) fmt = info.file_format data = {'export': volume['provider_location'], @@ -183,7 +187,7 @@ class SmbfsDriver(remotefs_drv.RemoteFSSnapDriver): volume_path = os.path.join(volume_dir, volume['name']) if os.path.exists(volume_path): - info = self._qemu_img_info(volume_path) + info = self._qemu_img_info(volume_path, volume['name']) volume_format = info.file_format else: volume_format = ( @@ -409,10 +413,10 @@ class SmbfsDriver(remotefs_drv.RemoteFSSnapDriver): self._check_extend_volume_support(volume, size_gb) LOG.info(_('Resizing file to %sG...') % size_gb) - self._do_extend_volume(volume_path, size_gb) + self._do_extend_volume(volume_path, size_gb, volume['name']) - def _do_extend_volume(self, volume_path, size_gb): - info = self._qemu_img_info(volume_path) + def _do_extend_volume(self, volume_path, size_gb, volume_name): + info = self._qemu_img_info(volume_path, volume_name) fmt = info.file_format # Note(lpetrut): as for version 2.0, qemu-img cannot resize @@ -481,7 +485,8 @@ class SmbfsDriver(remotefs_drv.RemoteFSSnapDriver): # Find the file which backs this file, which represents the point # when this snapshot was created. - img_info = self._qemu_img_info(forward_path) + img_info = self._qemu_img_info(forward_path, + snapshot['volume']['name']) path_to_snap_img = os.path.join(vol_dir, img_info.backing_file) LOG.debug("Will copy from snapshot at %s" % path_to_snap_img) @@ -512,7 +517,9 @@ class SmbfsDriver(remotefs_drv.RemoteFSSnapDriver): self.local_path(volume), volume_format, self.configuration.volume_dd_blocksize) - self._do_extend_volume(self.local_path(volume), volume['size']) + self._do_extend_volume(self.local_path(volume), + volume['size'], + volume['name']) data = image_utils.qemu_img_info(self.local_path(volume)) virt_size = data.virtual_size / units.Gi -- 2.45.2