suds client subject to cache poisoning by local attacker

This sounds OSSA worthy to me...

@ttx, I agree, this can lead to local code execution by local user in services context.

I think we can reuse the existing CVE for this issue and issue an advisory. I've created the draft impact description below:

Title: oslo.vmware uses a version of the suds soap client with known vulnerabilities.
Reporter: Grant Murphy (Red Hat)
Products: oslo.vmware, Nova, Cinder
Versions: from 2013.2 to 2013.2.3, and 2014.1 versions up to 2014.1.1

Description:
Grant Murphy from Red Hat found that oslo.vmware uses a vulnerable dependency.
The suds soap client cache stores pickled objects at a predictable path in /tmp.
A local attacker could pre-emptively create poisoned cache entries to execute
arbitrary code via pickle deserialization. The oslo.vmware code can be found
in the Nova and Cinder projects.

References:
http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2013-2217

@Grant, Small nits:

Products: Nova, Cinder (we don't support oslo.vmware iirc)

Versions: up to 2013.2.3, 2014.1 versions up to 2014.1.1 (grizzly at least looks affected too, see:
$ git show grizzly-2:tools/pip-requires | grep suds
suds==0.4
)

I would merge the two first sentences -> "oslo.vmware uses a vulnerable version of the suds soap client that cache stores pickled objects at a predictable path in /tmp."

"A local attacker" -> "A local attacker with shell access"

"The oslo.vmware code can be found in the Nova and Cinder projects." -> "All Nova and Cinder setups are affected."

Attaching proposed fix for oslo.vmware. This is probably not the cleanest solution. It uses mkdtemp to create a secure temporary directory to store cached objects. This directory will be cleaned up when object is out of scope. It may be a better to use a contextmanager for this however I was unsure of the typical usage. Another alternative would be to register a cleanup function via atexit.

How do you guys think we should patch this?

 @Tristan - Thanks for the feedback. I think I was looking at cinder which tags which start at 2013.2. You're right though. Nova is affected from 2013.1. How's this one sound:

Title: oslo.vmware uses a version of the suds soap client with known vulnerabilities.
Reporter: Grant Murphy (Red Hat)
Products: Nova, Cinder
Versions: from 2013.1 to 2013.2.3, and 2014.1 versions up to 2014.1.1

Description:
Grant Murphy from Red Hat found that oslo.vmware uses a vulnerable version of the suds soap client that stores pickled objects at a predictable path in /tmp for caching purposes. A local attacker with shell access could pre-emptively create poisoned cache entries to execute arbitrary code when cached objects are deserialized. All Nova and Cinder setups are affected.

References:
http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2013-2217

@Grant, the patch is looking good to me, though using __del__ for tmp clean can go wrong (as you don't control destruction order), maybe we could put the cleaning code into a function and call it in __del__ and as an atexit handler just to be on the safe side ?

Also, here (Ubuntu 13.10) suds does not install correctly with py33: http://paste.openstack.org/show/OebG2GJnDRTgm9JpzdMM/

+1 on impact description in comment #6.

@Tristan I don't like the __del__ much either. I might rewrite it to do the atexit cleanup and define the cache directory at the oslo/vmware/api.py level and pass it in as a configuration parameter.

The incompatibility with Python 3.3. is also concerning with this library. Even the more up to date fork [1] has problems when you try to import the module in Python 3.3. Unfortunately there doesn't seem to be many other contenders in this space either [2].

[1] https://bitbucket.org/jurko/suds/overview
[2] http://stackoverflow.com/questions/206154/what-soap-client-libraries-exist-for-python-and-where-is-the-documentation-for

Maybe shorter title: "oslo.vmware uses a vulnerable version of the suds soap client"

This patch removes the use of __del__() in favour of a call out to atexit. The caching can now also explicitly be disabled.

@oslo-core please review proposed patch (or point to oslo.vmware people we should invite to this bug)

I'm a little confused here - from what I see suds defaults to no caching (https://jortel.fedorapeople.org/suds/doc/suds.options.Options-class.html), so since oslo.vmware wasn't providing any cache setting, suds wouldn't have been writing files to disk, would it? This appears to only be a problem if you use the FileCache without providing a location, which we weren't doing in the first place.

That said, the change generally looks fine to me. A few minor comments, but nothing that would change how it functions:

* If the directory cleanup fails, I would prefer to log the exception at the same level as the message. It's not something that should happen, so we want to provide information on why it did. Requiring debug logging to figure out problems in OpenStack is something we really need to get away from.

* self.cache should be either self._cache or just cache, since it doesn't appear to need to be a member variable.

* The tests should use the unit test assert methods - self.assertTrue/False(os.path.exists...)

If we can confirm that we're not using suds in a vulnerable manner in either of Havana or Icehouse, then we shouldn't need to publish a security advisory for this and can work it in the open as a normal bug.

I think the documentation does not actually represent what is happening in the code.

[gm@lappy suds_test]$ virtualenv venv
New python executable in venv/bin/python
Installing setuptools, pip...done.
[gm@lappy suds_test]$ source venv/bin/activate
(venv)[gm@lappy suds_test]$ pip install suds==0.4
Downloading/unpacking suds==0.4
  Downloading suds-0.4.tar.gz (104kB): 104kB downloaded
  Running setup.py (path:/home/gm/suds_test/venv/build/suds/setup.py) egg_info for package suds

Installing collected packages: suds
  Running setup.py install for suds
    /home/gm/suds_test/venv/bin/python -O /tmp/tmpqRpfM4.py
    removing /tmp/tmpqRpfM4.py

Successfully installed suds
Cleaning up...
(venv)[gm@lappy suds_test]$ python
Python 2.7.5 (default, Jun 25 2014, 10:19:55)
[GCC 4.8.2 20131212 (Red Hat 4.8.2-7)] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import suds
>>> wsdl = "http://wsf.cdyne.com/WeatherWS/Weather.asmx?WSDL"
>>> c = suds.client.Client(wsdl)
>>> c.options.cache.location
'/tmp/suds'
>>> c.options.cache.__class__
<class suds.cache.ObjectCache at 0x7fe90c9aba78>

See also -
https://github.com/infirmary/suds/blob/master/suds/client.py#L109
https://github.com/infirmary/suds/blob/master/suds/cache.py#L314

Agreed--I tried the same and also confirmed that the /tmp/suds directory is indeed created and used to store pickled data. It does indeed appear that suds has caching on by default after all.

I tried with the latest version of oslo.vmware and it created the directory "tmpoKkE2U" with files of the format suds-*-document.px. There was no /tmp/suds directory. The same was observed while running the vmware cinder driver.

@vbala

I think the dependency might be getting picked up via your vendors packaging system it could potentially be patched against this flaw. For example this package is patched: python-suds_0.4.1-2ubuntu1.1_all.deb for ubuntu.

Ah, they default the Client class differently from the Options class. In that case I agree there would be an issue in any unpatched versions of suds, so we should definitely fix this.

So... this could easily be considered a vulnerability in suds rather than in OpenStack, in which case it would not be OSSA territory. In particular, if most distros consider it a vulnerability in suds, we can workaround the issue in master (and even backport the fix) to care for vulnerable suds, but would not issue an OSSA about it. We could issue an OSSN about it though.

Also it would be well-known, so this could be made public.

Sure. That makes sense. I think we need to look at reviving upstream or finding an alternative too. There are python 3 incompatibilities IIUC.

I'll cancel the OSSA task, add OSSN task bug, and switch to public if nobody screams about it soon.

Sounds fine to me.

Fix proposed to branch: master
Review: https://review.openstack.org/116270

Fixed in oslo.vmware - Ieec9d99aa674adf5cbc9be924fef3856cf4e5d66

Reviewed: https://review.openstack.org/116270
Committed: https://git.openstack.org/cgit/openstack/cinder/commit/?id=6a41fe9c5c98a14a355fa81b41aae2c4b0ce0a7b
Submitter: Jenkins
Branch: master

commit 6a41fe9c5c98a14a355fa81b41aae2c4b0ce0a7b
Author: Vipin Balachandran <email address hidden>
Date: Fri Aug 22 10:19:29 2014 +0530

    VMware: Disable suds caching

    The default cache implementation in suds store pickled objects in a
    predictable path in /tmp which could lead to attacks. This patch
    turns off suds caching to address this security issue.

    Change-Id: I7daaa25a0677004e03896298e9c3026d5c33c6ac
    Closes-Bug: #1341954

Fix proposed to branch: stable/icehouse
Review: https://review.openstack.org/118560

Fix proposed to branch: stable/havana
Review: https://review.openstack.org/118565

Here's a oslo.vmware review that adds back a memory based cache - https://review.openstack.org/#/c/116297/

Reviewed: https://review.openstack.org/118560
Committed: https://git.openstack.org/cgit/openstack/cinder/commit/?id=03573a02a3235db2bf65d48aceb92a6e81850dd9
Submitter: Jenkins
Branch: stable/icehouse

commit 03573a02a3235db2bf65d48aceb92a6e81850dd9
Author: Vipin Balachandran <email address hidden>
Date: Fri Aug 22 10:19:29 2014 +0530

    VMware: Disable suds caching

    The default cache implementation in suds store pickled objects in a
    predictable path in /tmp which could lead to attacks. This patch
    turns off suds caching to address this security issue.

    Change-Id: I7daaa25a0677004e03896298e9c3026d5c33c6ac
    Closes-Bug: #1341954
    (cherry picked from commit 6a41fe9c5c98a14a355fa81b41aae2c4b0ce0a7b)

This has been fixed in oslo.vmware which Nova now depends on, we just need a fresh release of oslo.vmware to fix this in Nova

Reviewed: https://review.openstack.org/118565
Committed: https://git.openstack.org/cgit/openstack/cinder/commit/?id=3c3530db2445eca9be583715ff689bd746e6d387
Submitter: Jenkins
Branch: stable/havana

commit 3c3530db2445eca9be583715ff689bd746e6d387
Author: Vipin Balachandran <email address hidden>
Date: Fri Aug 22 10:19:29 2014 +0530

    VMware: Disable suds caching

    The default cache implementation in suds store pickled objects in a
    predictable path in /tmp which could lead to attacks. This patch
    turns off suds caching to address this security issue.

    Conflicts:
            cinder/volume/drivers/vmware/pbm.py

    Change-Id: I7daaa25a0677004e03896298e9c3026d5c33c6ac
    Closes-Bug: #1341954
    (cherry picked from commit 6a41fe9c5c98a14a355fa81b41aae2c4b0ce0a7b)

This should be addressed by the oslo.vmware bump in requirements

Fix proposed to branch: master
Review: https://review.openstack.org/121614

Change abandoned by Davanum Srinivas (dims) (<email address hidden>) on branch: master
Review: https://review.openstack.org/121614

Tim, are you still working on this?

This has been published as OSSN-0038 to the openstack and openstack-dev mailing lists as well as the wiki:

  https://wiki.openstack.org/wiki/OSSN/OSSN-0038