CVE-2015-5162

@@ -30,6 +30,7 @@
 from taskflow import task
 from taskflow.types import failure
 
+from glance.async import utils
 from glance.common import exception
 from glance.common.scripts.image_import import main as image_import
 from glance.common.scripts import utils as script_utils
@@ -154,6 +155,7 @@ def execute(self, image_id):
             # place that other tasks can consume as well.
             stdout, stderr = putils.trycmd('qemu-img', 'info',
                                            '--output=json', path,
+                                           prlimit=utils.QEMU_IMG_PROC_LIMITS,
                                            log_errors=putils.LOG_ALL_ERRORS)
         except OSError as exc:
             with excutils.save_and_reraise_exception():

@@ -101,12 +101,23 @@ def execute(self, image_id, file_path):
                 _Convert.conversion_missing_warned = True
             return
 
+        image_obj = self.image_repo.get(image_id)
+        src_format = image_obj.disk_format
+
         # TODO(flaper87): Check whether the image is in the desired
         # format already. Probably using `qemu-img` just like the
         # `Introspection` task.
+
+        # NOTE(hemanthm): We add '-f' parameter to the convert command here so
+        # that the image format need not be inferred by qemu utils. This
+        # shields us from being vulnerable to an attack vector described here
+        # https://bugs.launchpad.net/glance/+bug/1449062
+
         dest_path = os.path.join(CONF.task.work_dir, "%s.converted" % image_id)
-        stdout, stderr = putils.trycmd('qemu-img', 'convert', '-O',
-                                       conversion_format, file_path, dest_path,
+        stdout, stderr = putils.trycmd('qemu-img', 'convert',
+                                       '-f', src_format,
+                                       '-O', conversion_format,
+                                       file_path, dest_path,
                                        log_errors=putils.LOG_ALL_ERRORS)
 
         if stderr:

@@ -48,6 +48,7 @@ def execute(self, image_id, file_path):
         try:
             stdout, stderr = putils.trycmd('qemu-img', 'info',
                                            '--output=json', file_path,
+                                           prlimit=utils.QEMU_IMG_PROC_LIMITS,
                                            log_errors=putils.LOG_ALL_ERRORS)
         except OSError as exc:
             # NOTE(flaper87): errno == 2 means the executable file

@@ -13,15 +13,25 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+from oslo_concurrency import processutils as putils
 from oslo_log import log as logging
 from oslo_utils import encodeutils
+from oslo_utils import units
 from taskflow import task
 
 from glance.i18n import _LW
 
 
 LOG = logging.getLogger(__name__)
 
+# NOTE(hemanthm): As reported in the bug #1449062, "qemu-img info" calls can
+# be exploited to craft DoS attacks by providing malicious input. The process
+# limits defined here are protections against such attacks. This essentially
+# limits the CPU time and address space used by the process that executes
+# "qemu-img info" command to 2 seconds and 1 GB respectively.
+QEMU_IMG_PROC_LIMITS = putils.ProcessLimits(cpu_time=2,
+                                            address_space=1 * units.Gi)
+
 
 class OptionalTask(task.Task):
 

@@ -94,6 +94,12 @@ def test_convert_success(self):
                 rm_mock.return_value = None
                 image_convert.execute(image, 'file:///test/path.raw')
 
+                # NOTE(hemanthm): Asserting that the source format is passed
+                # to qemu-utis to avoid inferring the image format. This
+                # shields us from an attack vector described at
+                # https://bugs.launchpad.net/glance/+bug/1449062/comments/72
+                self.assertIn('-f', exc_mock.call_args[0])
+
     def test_convert_revert_success(self):
         image_convert = convert._Convert(self.task.task_id,
                                          self.task_type,
@@ -178,3 +184,15 @@ def fake_execute(*args, **kwargs):
                 self.assertEqual([], os.listdir(self.work_dir))
                 self.assertEqual('qcow2', image.disk_format)
                 self.assertEqual(10737418240, image.virtual_size)
+
+                # NOTE(hemanthm): Asserting that the source format is passed
+                # to qemu-utis to avoid inferring the image format when
+                # converting. This shields us from an attack vector described
+                # at https://bugs.launchpad.net/glance/+bug/1449062/comments/72
+                #
+                # A total of three calls will be made to 'execute': 'info',
+                # 'convert' and 'info' towards introspection, conversion and
+                # OVF packaging respectively. We care about the 'convert' call
+                # here, hence we fetch the 2nd set of args from the args list.
+                convert_call_args, _ = exc_mock.call_args_list[1]
+                self.assertIn('-f', convert_call_args)

@@ -27,6 +27,7 @@
 
 import glance.async.flows.base_import as import_flow
 from glance.async import taskflow_executor
+from glance.async import utils as async_utils
 from glance.common.scripts.image_import import main as image_import
 from glance.common.scripts import utils as script_utils
 from glance.common import utils
@@ -86,6 +87,14 @@ def setUp(self):
                                                task_time_to_live=task_ttl,
                                                task_input=task_input)
 
+    def _assert_qemu_process_limits(self, exec_mock):
+        # NOTE(hemanthm): Assert that process limits are being applied
+        # on "qemu-img info" calls. See bug #1449062 for more details.
+        kw_args = exec_mock.call_args[1]
+        self.assertIn('prlimit', kw_args)
+        self.assertEqual(async_utils.QEMU_IMG_PROC_LIMITS,
+                         kw_args.get('prlimit'))
+
     def test_import_flow(self):
         self.config(engine_mode='serial',
                     group='taskflow_executor')
@@ -127,6 +136,8 @@ def create_image(*args, **kwargs):
                                                    self.image.image_id),
                                  self.image.locations[0]['url'])
 
+                self._assert_qemu_process_limits(tmock)
+
     def test_import_flow_missing_work_dir(self):
         self.config(engine_mode='serial', group='taskflow_executor')
         self.config(work_dir=None, group='task')
@@ -235,6 +246,7 @@ def create_image(*args, **kwargs):
                     self.assertTrue(rmock.called)
                     self.assertIsInstance(rmock.call_args[1]['result'],
                                           failure.Failure)
+                    self._assert_qemu_process_limits(tmock)
 
                     image_path = os.path.join(self.test_dir,
                                               self.image.image_id)
@@ -283,6 +295,8 @@ def create_image(*args, **kwargs):
                                       executor.begin_processing,
                                       self.task.task_id)
 
+                    self._assert_qemu_process_limits(tmock)
+
                     image_path = os.path.join(self.test_dir,
                                               self.image.image_id)
                     tmp_image_path = os.path.join(self.work_dir,
@@ -393,6 +407,7 @@ def test_import_to_fs(self):
                 image_path = os.path.join(self.work_dir, image_id)
                 tmp_image_path = os.path.join(self.work_dir, image_path)
                 self.assertTrue(os.path.exists(tmp_image_path))
+                self._assert_qemu_process_limits(tmock)
 
     def test_delete_from_fs(self):
         delete_fs = import_flow._DeleteFromFS(self.task.task_id,

@@ -21,6 +21,7 @@
 from oslo_config import cfg
 
 from glance.async.flows import introspect
+from glance.async import utils as async_utils
 from glance import domain
 import glance.tests.utils as test_utils
 
@@ -89,6 +90,13 @@ def test_introspect_success(self):
             image_create.execute(image, '/test/path.qcow2')
             self.assertEqual(10737418240, image.virtual_size)
 
+            # NOTE(hemanthm): Assert that process limits are being applied on
+            # "qemu-img info" calls. See bug #1449062 for more details.
+            kw_args = exc_mock.call_args[1]
+            self.assertIn('prlimit', kw_args)
+            self.assertEqual(async_utils.QEMU_IMG_PROC_LIMITS,
+                             kw_args.get('prlimit'))
+
     def test_introspect_no_image(self):
         image_create = introspect._Introspect(self.task.task_id,
                                               self.task_type,

@@ -0,0 +1,12 @@
+---
+security:
+  - All ``qemu-img info`` calls are now run under resource
+    limitations that limit the CPU time and address space
+    usage of the process running the command to 2 seconds
+    and 1 GB respectively. This addresses the bug
+    https://bugs.launchpad.net/glance/+bug/1449062
+
+    Current usage of "qemu-img" is limited to Glance tasks,
+    which by default (since the Liberty release) are only
+    available to admin users. We continue to recommend that
+    tasks only be exposed to trusted users

@@ -54,13 +54,18 @@
 CONF = cfg.CONF
 CONF.register_opts(image_helper_opts)
 
+QEMU_IMG_LIMITS = processutils.ProcessLimits(
+    cpu_time=2,
+    address_space=1 * units.Gi)
+
 
 def qemu_img_info(path, run_as_root=True):
     """Return a object containing the parsed output from qemu-img info."""
     cmd = ('env', 'LC_ALL=C', 'qemu-img', 'info', path)
     if os.name == 'nt':
         cmd = cmd[2:]
-    out, _err = utils.execute(*cmd, run_as_root=run_as_root)
+    out, _err = utils.execute(*cmd, run_as_root=run_as_root,
+                              prlimit=QEMU_IMG_LIMITS)
     return imageutils.QemuImgInfo(out)
 
 

@@ -38,7 +38,8 @@ def test_qemu_img_info(self, mock_exec, mock_info):
 
         output = image_utils.qemu_img_info(test_path)
         mock_exec.assert_called_once_with('env', 'LC_ALL=C', 'qemu-img',
-                                          'info', test_path, run_as_root=True)
+                                          'info', test_path, run_as_root=True,
+                                          prlimit=image_utils.QEMU_IMG_LIMITS)
         self.assertEqual(mock_info.return_value, output)
 
     @mock.patch('cinder.openstack.common.imageutils.QemuImgInfo')
@@ -51,7 +52,8 @@ def test_qemu_img_info_not_root(self, mock_exec, mock_info):
 
         output = image_utils.qemu_img_info(test_path, run_as_root=False)
         mock_exec.assert_called_once_with('env', 'LC_ALL=C', 'qemu-img',
-                                          'info', test_path, run_as_root=False)
+                                          'info', test_path, run_as_root=False,
+                                          prlimit=image_utils.QEMU_IMG_LIMITS)
         self.assertEqual(mock_info.return_value, output)
 
     @mock.patch('cinder.image.image_utils.os')
@@ -66,7 +68,8 @@ def test_qemu_img_info_on_nt(self, mock_exec, mock_info, mock_os):
 
         output = image_utils.qemu_img_info(test_path)
         mock_exec.assert_called_once_with('qemu-img', 'info', test_path,
-                                          run_as_root=True)
+                                          run_as_root=True,
+                                          prlimit=image_utils.QEMU_IMG_LIMITS)
         self.assertEqual(mock_info.return_value, output)
 
     @mock.patch('cinder.utils.execute')

@@ -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.

@@ -85,6 +85,7 @@
 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
@@ -7704,7 +7705,8 @@ def fake_lookup(instance_name):
 
         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)
@@ -7812,7 +7814,8 @@ def fake_lookup(instance_name):
 
         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'}

@@ -91,15 +91,17 @@ def test_disk_backing(self, mock_execute, mock_exists):
         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)
 
     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)
+                                             'qemu-img', 'info', path,
+                                             prlimit=images.QEMU_IMG_LIMITS)
 
     @mock.patch('os.path.exists', return_value=True)
     def test_disk_size(self, mock_exists):
@@ -145,7 +147,8 @@ def test_qemu_info_canon(self, mock_execute, mock_exists):
         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)
@@ -167,7 +170,8 @@ def test_qemu_info_canon2(self, mock_execute, mock_exists):
         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)
@@ -195,7 +199,8 @@ def test_qemu_backing_file_actual(self,
         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)
@@ -223,7 +228,8 @@ def test_qemu_info_convert(self, mock_execute, mock_exists):
         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)
@@ -247,7 +253,8 @@ def test_qemu_info_snaps(self, mock_execute, mock_exists):
         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)
@@ -283,7 +290,8 @@ 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', 'info', '/some/path'),
+                           {'prlimit': images.QEMU_IMG_LIMITS}),
                          (('qemu-img', 'create', '-f', 'qcow2',
                            '-o', 'backing_file=/some/path',
                            '/the/new/cow'),)]
@@ -369,7 +377,8 @@ def test_get_disk_size(self, mock_execute, mock_exists):
         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):

@@ -25,6 +25,7 @@
 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
@@ -43,6 +44,16 @@
 CONF = cfg.CONF
 CONF.register_opts(image_opts)
 IMAGE_API = image.API()
+QEMU_IMG_LIMITS = None
+
+try:
+    QEMU_IMG_LIMITS = processutils.ProcessLimits(
+        cpu_time=2,
+        address_space=1 * units.Gi)
+except Exception:
+    LOG.error(_LE('Please upgrade to oslo.concurrency version '
+                  '2.6.1 -- this version has fixes for the '
+                  'vulnerability CVE-2015-5162.'))
 
 
 def qemu_img_info(path, format=None):
@@ -61,7 +72,10 @@ def qemu_img_info(path, format=None):
     if format is not None:
         cmd = cmd + ('-f', format)
     try:
-        out, err = utils.execute(*cmd)
+        if QEMU_IMG_LIMITS is not None:
+            out, err = utils.execute(*cmd, prlimit=QEMU_IMG_LIMITS)
+        else:
+            out, err = utils.execute(*cmd)
     except processutils.ProcessExecutionError as exp:
         msg = (_("qemu-img failed to execute on %(path)s : %(exp)s") %
                 {'path': path, 'exp': exp})

@@ -0,0 +1,8 @@
+---
+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.
+    oslo.concurrency>=2.6.1 is required for this fix.