diff --git a/glance/async/flows/base_import.py b/glance/async/flows/base_import.py index fb0c04502a..03c37cb7f9 100644 --- a/glance/async/flows/base_import.py +++ b/glance/async/flows/base_import.py @@ -152,13 +152,17 @@ class _ImportToFS(task.Task): path = self.store.add(image_id, data, 0, context=None)[0] + trycmd_kwargs = {} + if utils.QEMU_IMG_PROC_LIMITS is not None: + trycmd_kwargs['prlimit'] = utils.QEMU_IMG_PROC_LIMITS + try: # NOTE(flaper87): Consider moving this code to a common # 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) + log_errors=putils.LOG_ALL_ERRORS, + **trycmd_kwargs) except OSError as exc: with excutils.save_and_reraise_exception(): msg = (_LE('Failed to execute security checks on the image ' diff --git a/glance/async/flows/introspect.py b/glance/async/flows/introspect.py index af1be3b7ea..efc64bdb05 100644 --- a/glance/async/flows/introspect.py +++ b/glance/async/flows/introspect.py @@ -44,12 +44,15 @@ class _Introspect(utils.OptionalTask): :param image_id: Glance image ID :param file_path: Path to the file being introspected """ + trycmd_kwargs = {} + if utils.QEMU_IMG_PROC_LIMITS is not None: + trycmd_kwargs['prlimit'] = utils.QEMU_IMG_PROC_LIMITS try: stdout, stderr = putils.trycmd('qemu-img', 'info', '--output=json', file_path, - prlimit=utils.QEMU_IMG_PROC_LIMITS, - log_errors=putils.LOG_ALL_ERRORS) + log_errors=putils.LOG_ALL_ERRORS, + **trycmd_kwargs) except OSError as exc: # NOTE(flaper87): errno == 2 means the executable file # was not found. For now, log an error and move forward diff --git a/glance/async/utils.py b/glance/async/utils.py index 3a515dfe86..f99d233485 100644 --- a/glance/async/utils.py +++ b/glance/async/utils.py @@ -23,14 +23,22 @@ from glance import i18n LOG = logging.getLogger(__name__) _LW = i18n._LW +_LE = i18n._LE -# 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) +QEMU_IMG_PROC_LIMITS = None +try: + # 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) +except Exception: + LOG.error(_LE('Please upgrade to oslo.concurrency version 2.6.1 -- ' + 'the version presently installed prevents fixing the ' + 'vulnerability CVE-2015-5162.')) class OptionalTask(task.Task): diff --git a/glance/tests/unit/async/flows/test_import.py b/glance/tests/unit/async/flows/test_import.py index 1c8d6bcc08..48d67ef89c 100644 --- a/glance/tests/unit/async/flows/test_import.py +++ b/glance/tests/unit/async/flows/test_import.py @@ -408,6 +408,35 @@ class TestImportTask(test_utils.BaseTestCase): self.assertTrue(os.path.exists(tmp_image_path)) self._assert_qemu_process_limits(tmock) + def test_import_to_fs_ignores_process_limits(self): + import_fs = import_flow._ImportToFS(self.task.task_id, + self.task_type, + self.task_repo, + 'http://example.com/image.qcow2') + + with mock.patch.object(script_utils, 'get_image_data_iter') as dmock: + with mock.patch('glance.async.utils.QEMU_IMG_PROC_LIMITS', None): + dmock.return_value = "test" + + with mock.patch.object(putils, 'trycmd') as tmock: + tmock.return_value = (json.dumps({ + 'format': 'qcow2', + }), None) + + image_id = UUID1 + path = import_fs.execute(image_id) + reader, size = glance_store.get_from_backend(path) + self.assertEqual(4, size) + self.assertEqual(dmock.return_value, "".join(reader)) + + 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)) + # NOTE(sigmavirus234): Assert that process limits are + # being ignores for older oslo.concurrency versions. + kw_args = tmock.call_args[1] + self.assertNotIn('prlimit', kw_args) + def test_delete_from_fs(self): delete_fs = import_flow._DeleteFromFS(self.task.task_id, self.task_type) diff --git a/glance/tests/unit/async/flows/test_introspect.py b/glance/tests/unit/async/flows/test_introspect.py index 869f99de4f..1c0cb60055 100644 --- a/glance/tests/unit/async/flows/test_introspect.py +++ b/glance/tests/unit/async/flows/test_introspect.py @@ -97,6 +97,42 @@ class TestImportTask(test_utils.BaseTestCase): self.assertEqual(async_utils.QEMU_IMG_PROC_LIMITS, kw_args.get('prlimit')) + def test_introspect_quietly_refuses_to_apply_process_limits(self): + image_create = introspect._Introspect(self.task.task_id, + self.task_type, + self.img_repo) + + self.task_repo.get.return_value = self.task + image_id = mock.sentinel.image_id + image = mock.MagicMock(image_id=image_id) + self.img_repo.get.return_value = image + + with mock.patch.object(processutils, 'execute') as exc_mock: + with mock.patch('glance.async.utils.QEMU_IMG_PROC_LIMITS', None): + result = json.dumps({ + "virtual-size": 10737418240, + "filename": "/tmp/image.qcow2", + "cluster-size": 65536, + "format": "qcow2", + "actual-size": 373030912, + "format-specific": { + "type": "qcow2", + "data": { + "compat": "0.10" + } + }, + "dirty-flag": False + }) + + exc_mock.return_value = (result, None) + image_create.execute(image, '/test/path.qcow2') + self.assertEqual(10737418240, image.virtual_size) + + # NOTE(sigmavirus24): Assert that process limits are being + # ignored when oslo.concurrency is too old. + kw_args = exc_mock.call_args[1] + self.assertNotIn('prlimit', kw_args) + def test_introspect_no_image(self): image_create = introspect._Introspect(self.task.task_id, self.task_type, diff --git a/releasenotes/notes/add-processlimits-to-qemu-img-c215f5d90f741d8a.yaml b/releasenotes/notes/add-processlimits-to-qemu-img-c215f5d90f741d8a.yaml index 5c471d43dd..eaf8388d93 100644 --- a/releasenotes/notes/add-processlimits-to-qemu-img-c215f5d90f741d8a.yaml +++ b/releasenotes/notes/add-processlimits-to-qemu-img-c215f5d90f741d8a.yaml @@ -1,10 +1,11 @@ --- security: - - All ``qemu-img info`` calls are now run under resource + - All ``qemu-img info`` calls will be 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 + usage of the process if oslo.concurrency is at least + version 2.6.1. ``qemu-img info`` calls are now limited + 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. In the Mitaka release, tasks by default will only be