openstack/glance
Code Issues Proposed changes

Adding constraints around qemu-img calls

Browse Source 
* All "qemu-img info" calls are now run under resource limitations
  that limit CPU time to 2 seconds and address space usage to 1 GB.
  This helps avoid any DoS attacks via malicious images.
* All "qemu-img convert" calls now specify the import format so that
  it does not have to be inferred by qemu-img.

SecurityImpact

Change-Id: Ib900bbc05cb9ccd90c6f56ccb4bf2006e30cdc80
Closes-Bug: #1449062
This commit is contained in:
Hemanth Makkapati 2016-09-23 09:29:12 -05:00
parent b9237e33f7
commit 69a9b659fd
8 changed files with 79 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
glance/async/flows/base_import.py
View File

@ -30,6 +30,7 @@ from taskflow import retry
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 @@ class _ImportToFS(task.Task):
# 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():

15
glance/async/flows/convert.py
View File

@ -101,12 +101,23 @@ class _Convert(task.Task):
_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:

1
glance/async/flows/introspect.py
View File

@ -48,6 +48,7 @@ class _Introspect(utils.OptionalTask):
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

10
glance/async/utils.py
View File

@ -13,8 +13,10 @@
# 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
@ -22,6 +24,14 @@ 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):

18
glance/tests/unit/async/flows/test_convert.py
View File

@ -94,6 +94,12 @@ class TestImportTask(test_utils.BaseTestCase):
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 @@ class TestImportTask(test_utils.BaseTestCase):
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)

15
glance/tests/unit/async/flows/test_import.py
View File

@ -27,6 +27,7 @@ from taskflow.types import failure
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 @@ class TestImportTask(test_utils.BaseTestCase):
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 @@ class TestImportTask(test_utils.BaseTestCase):
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 @@ class TestImportTask(test_utils.BaseTestCase):
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 @@ class TestImportTask(test_utils.BaseTestCase):
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 @@ class TestImportTask(test_utils.BaseTestCase):
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,

8
glance/tests/unit/async/flows/test_introspect.py
View File

@ -21,6 +21,7 @@ from oslo_concurrency import processutils
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 @@ class TestImportTask(test_utils.BaseTestCase):
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,

12
releasenotes/notes/add-processlimits-to-qemu-img-c215f5d90f741d8a.yaml Normal file
View File

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