From ace90e5e7834ba6eec1ae1538b7092d98b27e0b7 Mon Sep 17 00:00:00 2001 From: Grant Murphy Date: Wed, 6 Aug 2014 14:49:40 +1000 Subject: [PATCH] Store suds client cache at a secure location The default cache implementation stores pickled objects to a predictable path in /tmp. This can be used by a local attacker to redirect SOAP requests via symlinks or run a privilege escalation / code execution attack via a pickle exploit. A fix for this does not currently exist in upstream release of suds. As a temporary measure we should explictly create the suds cache directory in a safe manner. For more information refer to: CVE-2013-2217 Closes-bug: #1341954 Security-Impact --- oslo/vmware/api.py | 29 +++++++++++++++++++++++++---- oslo/vmware/pbm.py | 4 ++-- oslo/vmware/service.py | 10 ++++++++-- oslo/vmware/vim.py | 4 ++-- tests/test_api.py | 23 +++++++++++++++++++++-- 5 files changed, 58 insertions(+), 12 deletions(-) diff --git a/oslo/vmware/api.py b/oslo/vmware/api.py index a24aea5..e7749dc 100644 --- a/oslo/vmware/api.py +++ b/oslo/vmware/api.py @@ -20,8 +20,11 @@ This module contains classes to invoke VIM APIs. It supports automatic session re-establishment and retry of API invocations in case of connection problems or server API call overload. """ - +import atexit import logging +import os +import shutil +import tempfile import six @@ -37,6 +40,16 @@ from oslo.vmware import vim_util LOG = logging.getLogger(__name__) +def _delete_tempdir(dirname): + """Cleans up temp directory created by suds cache.""" + try: + if os.path.exists(dirname): + shutil.rmtree(dirname) + except OSError as e: + LOG.info("Failed to cleanup directory: %s", dirname) + LOG.debug(e) + + def _trunc_id(session_id): """Returns truncated session id which is suitable for logging.""" if session_id is not None: @@ -146,7 +159,7 @@ class VMwareAPISession(object): def __init__(self, host, server_username, server_password, api_retry_count, task_poll_interval, scheme='https', create_session=True, wsdl_loc=None, pbm_wsdl_loc=None, - port=443): + port=443, use_soap_cache=True): """Initializes the API session with given parameters. :param host: ESX/VC server IP address or host name @@ -162,6 +175,7 @@ class VMwareAPISession(object): instance creation :param wsdl_loc: VIM API WSDL file location :param pbm_wsdl_loc: PBM service WSDL file location + :param use_soap_cache: Cache SOAP client requests in tempdir :raises: VimException, VimFaultException, VimAttributeException, VimSessionOverLoadException """ @@ -178,6 +192,11 @@ class VMwareAPISession(object): self._session_username = None self._vim = None self._pbm = None + self._cache_dir = None + if use_soap_cache: + self._cache_dir = tempfile.mkdtemp(prefix="cache") + atexit.register(_delete_tempdir, self._cache_dir) + if create_session: self._create_session() @@ -187,7 +206,8 @@ class VMwareAPISession(object): self._vim = vim.Vim(protocol=self._scheme, host=self._host, port=self._port, - wsdl_url=self._vim_wsdl_loc) + wsdl_url=self._vim_wsdl_loc, + cache_dir=self._cache_dir) return self._vim @property @@ -196,7 +216,8 @@ class VMwareAPISession(object): self._pbm = pbm.Pbm(protocol=self._scheme, host=self._host, port=self._port, - wsdl_url=self._pbm_wsdl_loc) + wsdl_url=self._pbm_wsdl_loc, + cache_dir=self._cache_dir) if self._session_id: # To handle the case where pbm property is accessed after # session creation. If pbm property is accessed before session diff --git a/oslo/vmware/pbm.py b/oslo/vmware/pbm.py index 35da580..ae7f98c 100644 --- a/oslo/vmware/pbm.py +++ b/oslo/vmware/pbm.py @@ -41,7 +41,7 @@ class Pbm(service.Service): """Service class that provides access to the Storage Policy API.""" def __init__(self, protocol='https', host='localhost', port=443, - wsdl_url=None): + wsdl_url=None, cache_dir=None): """Constructs a PBM service client object. :param protocol: http or https @@ -51,7 +51,7 @@ class Pbm(service.Service): """ base_url = service.Service.build_base_url(protocol, host, port) soap_url = base_url + '/pbm' - super(Pbm, self).__init__(wsdl_url, soap_url) + super(Pbm, self).__init__(wsdl_url, soap_url, cache_dir) def set_soap_cookie(self, cookie): """Set the specified vCenter session cookie in the SOAP header diff --git a/oslo/vmware/service.py b/oslo/vmware/service.py index 92fadb8..1d22358 100644 --- a/oslo/vmware/service.py +++ b/oslo/vmware/service.py @@ -74,14 +74,20 @@ class Service(object): services """ - def __init__(self, wsdl_url=None, soap_url=None): + def __init__(self, wsdl_url=None, soap_url=None, cache_dir=None): self.wsdl_url = wsdl_url self.soap_url = soap_url LOG.debug("Creating suds client with soap_url='%s' and wsdl_url='%s'", self.soap_url, self.wsdl_url) + + self.cache = None + if cache_dir: + self.cache = suds.cache.FileCache(location=cache_dir) + self.client = suds.client.Client(self.wsdl_url, location=self.soap_url, - plugins=[ServiceMessagePlugin()]) + plugins=[ServiceMessagePlugin()], + cache=self.cache) self._service_content = None @staticmethod diff --git a/oslo/vmware/vim.py b/oslo/vmware/vim.py index a9074cf..172dfad 100644 --- a/oslo/vmware/vim.py +++ b/oslo/vmware/vim.py @@ -20,7 +20,7 @@ class Vim(service.Service): """Service class that provides access to the VIM API.""" def __init__(self, protocol='https', host='localhost', port=None, - wsdl_url=None): + wsdl_url=None, cache_dir=None): """Constructs a VIM service client object. :param protocol: http or https @@ -34,7 +34,7 @@ class Vim(service.Service): soap_url = base_url + '/sdk' if wsdl_url is None: wsdl_url = soap_url + '/vimService.wsdl' - super(Vim, self).__init__(wsdl_url, soap_url) + super(Vim, self).__init__(wsdl_url, soap_url, cache_dir) def retrieve_service_content(self): return self.RetrieveServiceContent(service.SERVICE_INSTANCE) diff --git a/tests/test_api.py b/tests/test_api.py index ec5a15c..b128764 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -17,6 +17,9 @@ Unit tests for session management and API invocation classes. """ +import atexit +import os + from eventlet import greenthread import mock @@ -116,7 +119,8 @@ class VMwareAPISessionTest(base.TestCase): task_poll_interval, 'https', _create_session, - port=VMwareAPISessionTest.PORT) + port=VMwareAPISessionTest.PORT, + use_soap_cache=False) def test_vim(self): api_session = self._create_api_session(False) @@ -124,7 +128,8 @@ class VMwareAPISessionTest(base.TestCase): self.VimMock.assert_called_with(protocol=api_session._scheme, host=VMwareAPISessionTest.SERVER_IP, port=VMwareAPISessionTest.PORT, - wsdl_url=api_session._vim_wsdl_loc) + wsdl_url=api_session._vim_wsdl_loc, + cache_dir=None) @mock.patch.object(pbm, 'Pbm') def test_pbm(self, pbm_mock): @@ -465,3 +470,17 @@ class VMwareAPISessionTest(base.TestCase): for k, v in _unknown_exceptions.iteritems(): self._poll_task_well_known_exceptions(k, v) + + def test_suds_cache(self): + path = None + try: + atexit.register = mock.Mock() + v = api.VMwareAPISession('', '', 1, 1, '', False) + path = v._cache_dir + assert(os.path.exists(path)) + atexit.register.assert_called_with(api._delete_tempdir, path) + v = None + + finally: + api._delete_tempdir(path) + assert(not os.path.exists(path)) -- 1.9.3