virt: set address space & CPU time limits when running qemu-img

This uses the new 'prlimit' parameter for oslo.concurrency execute method, to set an address space limit of 1GB and CPU time limit of 2 seconds, when running qemu-img. This is a re-implementation of the previously reverted commit commit da217205f5 Author: Tristan Cacqueray <tdecacqu@redhat.com> Date: Wed Aug 5 17:17:04 2015 +0000 virt: Use preexec_fn to ulimit qemu-img info call NOTE(mriedem): The backport depends on raising the minimum required oslo.concurrency>=3.7.1 which contains the prlimits fix for this change, the code fails without it. Depends-on: I157a6fc88836da73814ae46c8991ddefc5066c96 Closes-Bug: #1449062 Change-Id: I135b5242af1bfdcb0ea09a6fcda21fc03a6fbe7d (cherry picked from commit 068d851561)
2016-04-18 16:32:19 +00:00 · 2016-04-18 16:32:19 +00:00 · c8ec9ebf37
parent 73820214bf
commit c8ec9ebf37
5 changed files with 37 additions and 13 deletions
--- a/nova/tests/unit/virt/libvirt/test_driver.py
+++ b/nova/tests/unit/virt/libvirt/test_driver.py
@ -88,6 +88,7 @@ from nova.virt import fake
 from nova.virt import firewall as base_firewall
 from nova.virt import hardware
 from nova.virt.image import model as imgmodel
+from nova.virt import images
 from nova.virt.libvirt import blockinfo
 from nova.virt.libvirt import config as vconfig
 from nova.virt.libvirt import driver as libvirt_driver
@ -8672,7 +8673,8 @@ class LibvirtConnTestCase(test.NoDBTestCase):

        self.mox.StubOutWithMock(utils, "execute")
        utils.execute('env', 'LC_ALL=C', 'LANG=C', 'qemu-img', 'info',
-                      '/test/disk.local').AndReturn((ret, ''))
+                      '/test/disk.local', prlimit = images.QEMU_IMG_LIMITS,
+                      ).AndReturn((ret, ''))

        self.mox.ReplayAll()
        drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
@ -8780,7 +8782,8 @@ class LibvirtConnTestCase(test.NoDBTestCase):

        self.mox.StubOutWithMock(utils, "execute")
        utils.execute('env', 'LC_ALL=C', 'LANG=C', 'qemu-img', 'info',
-                      '/test/disk.local').AndReturn((ret, ''))
+                      '/test/disk.local', prlimit = images.QEMU_IMG_LIMITS,
+                      ).AndReturn((ret, ''))

        self.mox.ReplayAll()
        conn_info = {'driver_volume_type': 'fake'}
--- a/nova/tests/unit/virt/libvirt/test_utils.py
+++ b/nova/tests/unit/virt/libvirt/test_utils.py
@ -101,7 +101,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',
-                                             'qemu-img', 'info', path)
+                                             'qemu-img', 'info', path,
+                                             prlimit=images.QEMU_IMG_LIMITS)
        mock_exists.assert_called_once_with(path)
        self.assertIsNone(d_backing)

@ -109,7 +110,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',
-                                             'qemu-img', 'info', path)
+                                             'qemu-img', 'info', path,
+                                             prlimit=images.QEMU_IMG_LIMITS)

    @mock.patch('os.path.exists', return_value=True)
    def test_disk_size(self, mock_exists):
@ -155,7 +157,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',
-                                             'qemu-img', 'info', path)
+                                             'qemu-img', 'info', path,
+                                             prlimit=images.QEMU_IMG_LIMITS)
        mock_exists.assert_called_once_with(path)
        self.assertEqual('disk.config', image_info.image)
        self.assertEqual('raw', image_info.file_format)
@ -177,7 +180,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',
-                                             'qemu-img', 'info', path)
+                                             'qemu-img', 'info', path,
+                                             prlimit=images.QEMU_IMG_LIMITS)
        mock_exists.assert_called_once_with(path)
        self.assertEqual('disk.config', image_info.image)
        self.assertEqual('qcow2', image_info.file_format)
@ -205,7 +209,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',
-                                             'qemu-img', 'info', path)
+                                             'qemu-img', 'info', path,
+                                             prlimit=images.QEMU_IMG_LIMITS)
        mock_exists.assert_called_once_with(path)
        self.assertEqual('disk.config', image_info.image)
        self.assertEqual('raw', image_info.file_format)
@ -233,7 +238,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',
-                                             'qemu-img', 'info', path)
+                                             'qemu-img', 'info', path,
+                                             prlimit=images.QEMU_IMG_LIMITS)
        mock_exists.assert_called_once_with(path)
        self.assertEqual('disk.config', image_info.image)
        self.assertEqual('raw', image_info.file_format)
@ -257,7 +263,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',
-                                             'qemu-img', 'info', path)
+                                             'qemu-img', 'info', path,
+                                             prlimit=images.QEMU_IMG_LIMITS)
        mock_exists.assert_called_once_with(path)
        self.assertEqual('disk.config', image_info.image)
        self.assertEqual('raw', image_info.file_format)
@ -293,7 +300,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',
-                           'qemu-img', 'info', '/some/path'),),
+                           'qemu-img', 'info', '/some/path'),
+                           {'prlimit': images.QEMU_IMG_LIMITS}),
                         (('qemu-img', 'create', '-f', 'qcow2',
                           '-o', 'backing_file=/some/path',
                           '/the/new/cow'),)]
@ -379,7 +387,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',
-                                             'qemu-img', 'info', path)
+                                             'qemu-img', 'info', path,
+                                             prlimit=images.QEMU_IMG_LIMITS)
        mock_exists.assert_called_once_with(path)

    def test_copy_image(self):
--- a/nova/virt/images.py
+++ b/nova/virt/images.py
@ -25,6 +25,7 @@ from oslo_concurrency import processutils
 from oslo_log import log as logging
 from oslo_utils import fileutils
 from oslo_utils import imageutils
+from oslo_utils import units

 import nova.conf
 from nova import exception
@ -37,6 +38,10 @@ LOG = logging.getLogger(__name__)
 CONF = nova.conf.CONF
 IMAGE_API = image.API()

+QEMU_IMG_LIMITS = processutils.ProcessLimits(
+    cpu_time=2,
+    address_space=1 * units.Gi)
+

 def qemu_img_info(path, format=None):
    """Return an object containing the parsed output from qemu-img info."""
@ -53,7 +58,7 @@ def qemu_img_info(path, format=None):
        cmd = ('env', 'LC_ALL=C', 'LANG=C', 'qemu-img', 'info', path)
        if format is not None:
            cmd = cmd + ('-f', format)
-        out, err = utils.execute(*cmd)
+        out, err = utils.execute(*cmd, prlimit=QEMU_IMG_LIMITS)
    except processutils.ProcessExecutionError as exp:
        msg = (_("qemu-img failed to execute on %(path)s : %(exp)s") %
                {'path': path, 'exp': exp})
--- a/releasenotes/notes/apply-limits-to-qemu-img-8813f7a333ebdf69.yaml
+++ b/releasenotes/notes/apply-limits-to-qemu-img-8813f7a333ebdf69.yaml
@ -0,0 +1,7 @@
+---
+security:
+  - The qemu-img tool now has resource limits applied
+    which prevent it from using more than 1GB of address
+    space or more than 2 seconds of CPU time. This provides
+    protection against denial of service attacks from
+    maliciously crafted or corrupted disk images.
--- a/requirements.txt
+++ b/requirements.txt
@ -36,7 +36,7 @@ stevedore>=1.5.0 # Apache-2.0
 setuptools>=16.0 # PSF/ZPL
 websockify>=0.6.1 # LGPLv3
 oslo.cache>=1.5.0 # Apache-2.0
-oslo.concurrency>=3.5.0 # Apache-2.0
+oslo.concurrency>=3.7.1 # Apache-2.0
 oslo.config>=3.7.0 # Apache-2.0
 oslo.context>=0.2.0 # Apache-2.0
 oslo.log>=1.14.0 # Apache-2.0