From 63087d59499cb10ab87651c9cb249d42d207e6c1 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. The 'prlimit' command is found in the widely available util-limit package in Linux distros. Closes-bug: 1449062 Change-Id: I446aa3dcc78f2f4733bd4968d5b2883d98f775f1 --- nova/tests/unit/virt/libvirt/test_driver.py | 10 ++++++++-- nova/tests/unit/virt/libvirt/test_utils.py | 23 +++++++++++++++++++++++ nova/virt/images.py | 8 ++++++++ 3 files changed, 39 insertions(+), 2 deletions(-) diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py index ab0f04d..3a2d0e7 100644 --- a/nova/tests/unit/virt/libvirt/test_driver.py +++ b/nova/tests/unit/virt/libvirt/test_driver.py @@ -6836,7 +6836,10 @@ 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', + utils.execute('env', 'LC_ALL=C', 'LANG=C', + 'prlimit', '--cpu=2', + '--as=' + str(1 * units.Gi), + 'qemu-img', 'info', '/test/disk.local').AndReturn((ret, '')) self.mox.ReplayAll() @@ -6926,7 +6929,10 @@ 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', + utils.execute('env', 'LC_ALL=C', 'LANG=C', + 'prlimit', '--cpu=2', + '--as=' + str(1 * units.Gi), + 'qemu-img', 'info', '/test/disk.local').AndReturn((ret, '')) self.mox.ReplayAll() diff --git a/nova/tests/unit/virt/libvirt/test_utils.py b/nova/tests/unit/virt/libvirt/test_utils.py index 9d00b8a..14422eb 100644 --- a/nova/tests/unit/virt/libvirt/test_utils.py +++ b/nova/tests/unit/virt/libvirt/test_utils.py @@ -20,6 +20,7 @@ import tempfile import mock from oslo_concurrency import processutils from oslo_config import cfg +from oslo_utils import units import six from nova.compute import arch @@ -53,6 +54,8 @@ 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', + 'prlimit', '--cpu=2', + '--as=' + str(1 * units.Gi), 'qemu-img', 'info', path) mock_exists.assert_called_once_with(path) self.assertEqual('raw', disk_type) @@ -145,6 +148,8 @@ disk size: 96K d_type = libvirt_utils.get_disk_type(path) mock_execute.assert_called_once_with( 'env', 'LC_ALL=C', 'LANG=C', + 'prlimit', '--cpu=2', + '--as=' + str(1 * units.Gi), 'qemu-img', 'info', path) self.assertEqual(f, d_type) @@ -164,6 +169,8 @@ 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', + 'prlimit', '--cpu=2', + '--as=' + str(1 * units.Gi), 'qemu-img', 'info', path) mock_exists.assert_called_once_with(path) self.assertIsNone(d_backing) @@ -172,6 +179,8 @@ disk size: 96K 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', + 'prlimit', '--cpu=2', + '--as=' + str(1 * units.Gi), 'qemu-img', 'info', path) @mock.patch('os.path.exists', return_value=True) @@ -218,6 +227,8 @@ 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', + 'prlimit', '--cpu=2', + '--as=' + str(1 * units.Gi), 'qemu-img', 'info', path) mock_exists.assert_called_once_with(path) self.assertEqual('disk.config', image_info.image) @@ -240,6 +251,8 @@ 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', + 'prlimit', '--cpu=2', + '--as=' + str(1 * units.Gi), 'qemu-img', 'info', path) mock_exists.assert_called_once_with(path) self.assertEqual('disk.config', image_info.image) @@ -268,6 +281,8 @@ 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', + 'prlimit', '--cpu=2', + '--as=' + str(1 * units.Gi), 'qemu-img', 'info', path) mock_exists.assert_called_once_with(path) self.assertEqual('disk.config', image_info.image) @@ -296,6 +311,8 @@ 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', + 'prlimit', '--cpu=2', + '--as=' + str(1 * units.Gi), 'qemu-img', 'info', path) mock_exists.assert_called_once_with(path) self.assertEqual('disk.config', image_info.image) @@ -320,6 +337,8 @@ 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', + 'prlimit', '--cpu=2', + '--as=' + str(1 * units.Gi), 'qemu-img', 'info', path) mock_exists.assert_called_once_with(path) self.assertEqual('disk.config', image_info.image) @@ -356,6 +375,8 @@ ID TAG VM SIZE DATE VM CLOCK mock_execute.return_value = ('stdout', None) libvirt_utils.create_cow_image('/some/path', '/the/new/cow') expected_args = [(('env', 'LC_ALL=C', 'LANG=C', + 'prlimit', '--cpu=2', + '--as=' + str(1 * units.Gi), 'qemu-img', 'info', '/some/path'),), (('qemu-img', 'create', '-f', 'qcow2', '-o', 'backing_file=/some/path', @@ -442,6 +463,8 @@ 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', + 'prlimit', '--cpu=2', + '--as=' + str(1 * units.Gi), 'qemu-img', 'info', path) mock_exists.assert_called_once_with(path) diff --git a/nova/virt/images.py b/nova/virt/images.py index 57d27aa..fad664b 100644 --- a/nova/virt/images.py +++ b/nova/virt/images.py @@ -23,6 +23,7 @@ import os from oslo_config import cfg from oslo_log import log as logging +from oslo_utils import units from nova import exception from nova.i18n import _, _LE @@ -56,7 +57,14 @@ def qemu_img_info(path): msg = (_("Path does not exist %(path)s") % {'path': path}) raise exception.InvalidDiskInfo(reason=msg) + # 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 out, err = utils.execute('env', 'LC_ALL=C', 'LANG=C', + 'prlimit', '--cpu=2', + '--as=' + str(1 * units.Gi), 'qemu-img', 'info', path) if not out: msg = (_("Failed to run qemu-img info on %(path)s : %(error)s") % -- 2.4.3