From a9e38af4b871e6852ee41f6ee1cae965f3f4e123 Mon Sep 17 00:00:00 2001 From: "Daniel P. Berrange" Date: Tue, 16 Jun 2015 16:00:41 +0100 Subject: [PATCH] virt: restrict resource usage of qemu-img info The qemu-img info command may use an unreasonable amount of memory when asked to open various maliciously created disk images. Since the cloud tenant cannot be trusted, Nova must apply resource limits when invoking qemu-img info to avoid a potential denial-of-service on the compute node. This limits qemu-img info to 1 GB of address space, and 2 seconds of CPU time, which should be sufficient to allow "normal" disk images, while blocking malicious ones. Closes-bug: 1449062 Change-Id: I446aa3dcc78f2f4733bd4968d5b2883d98f775f1 --- nova/tests/unit/virt/libvirt/test_driver.py | 15 +++++-- nova/tests/unit/virt/libvirt/test_utils.py | 68 +++++++++++++++++++---------- nova/virt/images.py | 15 ++++++- 3 files changed, 69 insertions(+), 29 deletions(-) diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py index 8c922b4..3ada6e6 100644 --- a/nova/tests/unit/virt/libvirt/test_driver.py +++ b/nova/tests/unit/virt/libvirt/test_driver.py @@ -84,6 +84,7 @@ from nova.virt import driver from nova.virt import fake from nova.virt import firewall as base_firewall from nova.virt import hardware +from nova.virt import images from nova.virt.image import model as imgmodel from nova.virt.libvirt import blockinfo from nova.virt.libvirt import config as vconfig @@ -7641,8 +7642,11 @@ class LibvirtConnTestCase(test.NoDBTestCase): os.path.exists('/test/disk.local').AndReturn(True) self.mox.StubOutWithMock(utils, "execute") - utils.execute('env', 'LC_ALL=C', 'LANG=C', 'qemu-img', 'info', - '/test/disk.local').AndReturn((ret, '')) + utils.execute('env', 'LC_ALL=C', 'LANG=C', + 'qemu-img', 'info', + '/test/disk.local', + preexec_fn=images._limit_qemu_img_res).AndReturn( + (ret, '')) self.mox.ReplayAll() drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False) @@ -7749,8 +7753,11 @@ class LibvirtConnTestCase(test.NoDBTestCase): os.path.exists('/test/disk.local').AndReturn(True) self.mox.StubOutWithMock(utils, "execute") - utils.execute('env', 'LC_ALL=C', 'LANG=C', 'qemu-img', 'info', - '/test/disk.local').AndReturn((ret, '')) + utils.execute('env', 'LC_ALL=C', 'LANG=C', + 'qemu-img', 'info', + '/test/disk.local', + preexec_fn=images._limit_qemu_img_res).AndReturn( + (ret, '')) self.mox.ReplayAll() conn_info = {'driver_volume_type': 'fake'} diff --git a/nova/tests/unit/virt/libvirt/test_utils.py b/nova/tests/unit/virt/libvirt/test_utils.py index ac7ea8d..29412aa 100644 --- a/nova/tests/unit/virt/libvirt/test_utils.py +++ b/nova/tests/unit/virt/libvirt/test_utils.py @@ -52,8 +52,10 @@ 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_execute.assert_called_once_with( + 'env', 'LC_ALL=C', 'LANG=C', + 'qemu-img', 'info', path, + preexec_fn=images._limit_qemu_img_res) mock_exists.assert_called_once_with(path) self.assertEqual('raw', disk_type) @@ -106,7 +108,8 @@ disk size: 96K d_type = libvirt_utils.get_disk_type(path) mock_execute.assert_called_once_with( 'env', 'LC_ALL=C', 'LANG=C', - 'qemu-img', 'info', path) + 'qemu-img', 'info', path, + preexec_fn=images._limit_qemu_img_res) self.assertEqual(f, d_type) @mock.patch('os.path.exists', return_value=True) @@ -124,16 +127,20 @@ disk size: 96K }) mock_execute.return_value = (output, '') d_backing = libvirt_utils.get_disk_backing_file(path) - mock_execute.assert_called_once_with('env', 'LC_ALL=C', 'LANG=C', - 'qemu-img', 'info', path) + mock_execute.assert_called_once_with( + 'env', 'LC_ALL=C', 'LANG=C', + 'qemu-img', 'info', path, + preexec_fn=images._limit_qemu_img_res) mock_exists.assert_called_once_with(path) self.assertIsNone(d_backing) def _test_disk_size(self, mock_execute, path, expected_size): d_size = libvirt_utils.get_disk_size(path) self.assertEqual(expected_size, d_size) - mock_execute.assert_called_once_with('env', 'LC_ALL=C', 'LANG=C', - 'qemu-img', 'info', path) + mock_execute.assert_called_once_with( + 'env', 'LC_ALL=C', 'LANG=C', + 'qemu-img', 'info', path, + preexec_fn=images._limit_qemu_img_res) @mock.patch('os.path.exists', return_value=True) def test_disk_size(self, mock_exists): @@ -178,8 +185,10 @@ blah BLAH: bb """ mock_execute.return_value = (example_output, '') image_info = images.qemu_img_info(path) - mock_execute.assert_called_once_with('env', 'LC_ALL=C', 'LANG=C', - 'qemu-img', 'info', path) + mock_execute.assert_called_once_with( + 'env', 'LC_ALL=C', 'LANG=C', + 'qemu-img', 'info', path, + preexec_fn=images._limit_qemu_img_res) mock_exists.assert_called_once_with(path) self.assertEqual('disk.config', image_info.image) self.assertEqual('raw', image_info.file_format) @@ -200,8 +209,10 @@ backing file: /var/lib/nova/a328c7998805951a_2 """ mock_execute.return_value = (example_output, '') image_info = images.qemu_img_info(path) - mock_execute.assert_called_once_with('env', 'LC_ALL=C', 'LANG=C', - 'qemu-img', 'info', path) + mock_execute.assert_called_once_with( + 'env', 'LC_ALL=C', 'LANG=C', + 'qemu-img', 'info', path, + preexec_fn=images._limit_qemu_img_res) mock_exists.assert_called_once_with(path) self.assertEqual('disk.config', image_info.image) self.assertEqual('qcow2', image_info.file_format) @@ -228,8 +239,10 @@ backing file: /var/lib/nova/a328c7998805951a_2 (actual path: /b/3a988059e51a_2) """ mock_execute.return_value = (example_output, '') image_info = images.qemu_img_info(path) - mock_execute.assert_called_once_with('env', 'LC_ALL=C', 'LANG=C', - 'qemu-img', 'info', path) + mock_execute.assert_called_once_with( + 'env', 'LC_ALL=C', 'LANG=C', + 'qemu-img', 'info', path, + preexec_fn=images._limit_qemu_img_res) mock_exists.assert_called_once_with(path) self.assertEqual('disk.config', image_info.image) self.assertEqual('raw', image_info.file_format) @@ -256,8 +269,10 @@ junk stuff: bbb """ mock_execute.return_value = (example_output, '') image_info = images.qemu_img_info(path) - mock_execute.assert_called_once_with('env', 'LC_ALL=C', 'LANG=C', - 'qemu-img', 'info', path) + mock_execute.assert_called_once_with( + 'env', 'LC_ALL=C', 'LANG=C', + 'qemu-img', 'info', path, + preexec_fn=images._limit_qemu_img_res) mock_exists.assert_called_once_with(path) self.assertEqual('disk.config', image_info.image) self.assertEqual('raw', image_info.file_format) @@ -280,8 +295,10 @@ ID TAG VM SIZE DATE VM CLOCK """ mock_execute.return_value = (example_output, '') image_info = images.qemu_img_info(path) - mock_execute.assert_called_once_with('env', 'LC_ALL=C', 'LANG=C', - 'qemu-img', 'info', path) + mock_execute.assert_called_once_with( + 'env', 'LC_ALL=C', 'LANG=C', + 'qemu-img', 'info', path, + preexec_fn=images._limit_qemu_img_res) mock_exists.assert_called_once_with(path) self.assertEqual('disk.config', image_info.image) self.assertEqual('raw', image_info.file_format) @@ -316,11 +333,12 @@ ID TAG VM SIZE DATE VM CLOCK def test_create_cow_image(self, mock_execute, mock_exists): mock_execute.return_value = ('stdout', None) libvirt_utils.create_cow_image('/some/path', '/the/new/cow') - expected_args = [(('env', 'LC_ALL=C', 'LANG=C', - 'qemu-img', 'info', '/some/path'),), - (('qemu-img', 'create', '-f', 'qcow2', - '-o', 'backing_file=/some/path', - '/the/new/cow'),)] + expected_args = [mock.call('env', 'LC_ALL=C', 'LANG=C', + 'qemu-img', 'info', '/some/path', + preexec_fn=images._limit_qemu_img_res), + mock.call('qemu-img', 'create', '-f', 'qcow2', + '-o', 'backing_file=/some/path', + '/the/new/cow')] self.assertEqual(expected_args, mock_execute.call_args_list) def test_pick_disk_driver_name(self): @@ -402,8 +420,10 @@ disk size: 4.4M """ mock_execute.return_value = (example_output, '') self.assertEqual(4592640, disk.get_disk_size('/some/path')) - mock_execute.assert_called_once_with('env', 'LC_ALL=C', 'LANG=C', - 'qemu-img', 'info', path) + mock_execute.assert_called_once_with( + 'env', 'LC_ALL=C', 'LANG=C', + 'qemu-img', 'info', path, + preexec_fn=images._limit_qemu_img_res) mock_exists.assert_called_once_with(path) def test_copy_image(self): diff --git a/nova/virt/images.py b/nova/virt/images.py index 5b9374b..8119b06 100644 --- a/nova/virt/images.py +++ b/nova/virt/images.py @@ -20,10 +20,12 @@ Handling of VM disk images. """ import os +import resource from oslo_config import cfg from oslo_log import log as logging from oslo_utils import fileutils +from oslo_utils import units from nova import exception from nova.i18n import _, _LE @@ -44,6 +46,16 @@ CONF.register_opts(image_opts) IMAGE_API = image.API() +# qemu-img can consume considerable RAM & CPU time if fed a +# maliciously crafted disk image. Since cloud tenants are not +# to be trusted, ensure QEMU is limits to 1 GB address space +# and 2 seconds CPU time, which ought to be more than enough +# for real world disk images +def _limit_qemu_img_res(): + resource.setrlimit(resource.RLIMIT_CPU, 2, 2) + resource.setrlimit(resource.RLIMIT_AS, 1 * units.Gi, 1 * units.Gi) + + def qemu_img_info(path): """Return an object containing the parsed output from qemu-img info.""" # TODO(mikal): this code should not be referring to a libvirt specific @@ -57,7 +69,8 @@ def qemu_img_info(path): raise exception.InvalidDiskInfo(reason=msg) out, err = utils.execute('env', 'LC_ALL=C', 'LANG=C', - 'qemu-img', 'info', path) + 'qemu-img', 'info', path, + preexec_fn=_limit_qemu_img_res) if not out: msg = (_("Failed to run qemu-img info on %(path)s : %(error)s") % {'path': path, 'error': err}) -- 2.4.3