Potentially insecure dependency loading

[Thierry Carrez]

Grant Murphy and Dhiru Kholia from Red Hat Product Security Team reported the following potential issue. This is actually a setuptools issue but which we may be able to workaround, if we end up being affected:

---
A security flaw was found in the way Python Setuptools, a collection of enhancements to the Python distutils module, that allows more easily to build and distribute Python packages, performed integrity checks when loading external resources, previously extracted from zipped Python Egg archives(formerly if the timestamp and file size of a particular resource expanded from the archive matched the original values, the resource was successfully loaded). A local attacker, with write permission into the Python's EGG cache (directory) could use this flaw to provide a specially-crafted resource (in expanded form) that, when loaded in an application requiring that resource to (be able to) run, would lead to arbitrary code execution with the privileges of the user running the application.

It seems to be pretty common for Python applications to do something like os.evironment['PYTHON_EGG_CACHE'] = /tmp, prior to importing dependencies.

If the dependency contains a .so Python must unpack it into the cache directory to be able to load it. However if an attacker pre-emptively places a .so in the same location as long as the file has the same timestamp and file size it will be loaded.
---

Glance and Swift both set PYTHON_EGG_CACHE to '/tmp' :
./glance/glance/cmd/control.py: os.environ['PYTHON_EGG_CACHE'] = '/tmp'
./swift/swift/common/manager.py: os.environ['PYTHON_EGG_CACHE'] = '/tmp'

If we are immediately vulnerable to this (i.e. if stuff loaded from those commands contains an .so, if I understand correctly), we could workaround it by setting it to /tmp/secure-dir-XXXXXX/ until setuptools upstream fixes this.

[Chmouel Boudjnah (chmouel)]

We could just mkstemp that env variable right?

[Thierry Carrez (ttx)]

Yeah, that's a good way to work around it. But I'd first like to know if we are actually vulnerable to anything before keeping this private and start dancing the whole embargoed dance. If this is not exploitable we could just wait for the setuptools fix.

Those utilities seem pretty limited in their imports, and I wonder if any of them would actually be importing a .so-powered python dep.

[Chuck Thier (cthier)]

So if I understand this correctly, this is only possibly vulnerable if the dependencies are installed as zipped .egg files.

In general, most packages I have seen tend not to do this because it usually causes more issues than it solves. Distro (deb, rpm) packages don't do this. I did a quick look at my local installs, and none of the dependencies are stored as zipped .egg files. I will need to do a little more testing to validate this.

If I remember correctly, the only reason it was specifically set to /tmp was because certain setuptools functionality would bail out if it couldn't access the tmp egg dir (even if it is never used).

[Chuck Thier (cthier)]

It is unclear to me specifically when setuptools actually caches the zip contents (even after trying to read through some of the setuptools code). I intentionally built a zipped egg, and could not get it to show up as a cache on the filesystem anywhere. I'll try to poke at it some more tomorrow, but if anyone else can contribute some more clarity, I would appreciate it.

[Thierry Carrez (ttx)]

I looked into the dependencies for the Glance command and they all either end up in the stdlib or in dependencies which don't have .so files in them. Not sure that's enough to be immune from this though, given how much details we currently have.

In the case of Swift swift/common/manager.py tries to import simplejson which has a _speedups.so.

[Thierry Carrez (ttx)]

I still fail to see why those glance-manage / swift-manage commands would end up installing new dependencies (and then be abused by a writeable PYTHON_EGG_CACHE). These is not "pip" or "easy_install"... they don't even import setuptools.

[Thierry Carrez (ttx)]

Asking for more details from the reporter.

[Thierry Carrez (ttx)]

Got some more information from Grant Murphy.

If you put eggs early in PYTHONPATH satisfying part of your glance-manage or swift-manage dependencies, Python will unpack the egg in PYTHON_EGG_CACHE. Since setuptools will not overwrite anything that might already be there, if PYTHON_EGG_CACHE is set to a world-writeable directory we are vulnerable to code injection.

So the hypothetical way to exploit this would be to use some egg-based distro that would load dependencies from eggs, and have a local attacker pre-place some malicious /tmp/simplejson-*.egg-tmp/_speedups.so then when you run swift-manage that code gets executed.

Given nobody in their sane mind would use an eggs-based distro to deploy OpenStack (and no such distribution exists) and that all manual deploys leverage pip rather than eggs, I would consider fixing this a good strengthening measure rather than a vulnerability fix.

Question is... how to fix that. (Safely) creating a temporary directory looks like a safe workaround, but is setting PYTHON_EGG_CACHE actually needed ? There seems to be some cargo-culting involved in that setting. This is now just use by glance-manage and swift-manage, for which the default of ~/.python-eggs should cover any corner use case, and I somehow doubt we would ever use that directory anyway.

So I'd like to go deeper in WHY is this set in the first place. It's from the original version of swift/common/manager.py, then was cargo-culted in glance-manage. Chuck Thier said above: "If I remember correctly, the only reason it was specifically set to /tmp was because certain setuptools functionality would bail out if it couldn't access the tmp egg dir (even if it is never used)."

[Chuck Thier (cthier)]

I agree completely that nobody would run swift in such a way. I'll look to see if the PYTHON_EGG_CACHE is needed any more.

[Chuck Thier (cthier)]

So I took a peek at the setuptools code this morning and this function gets called every time it looks at the cached content:

    def _is_current(self, file_path, zip_path):
        """
        Return True if the file_path is current for this zip_path
        """
        timestamp, size = self._get_date_and_size(self.zipinfo[zip_path])
        if not os.path.isfile(file_path):
            return False
        stat = os.stat(file_path)
        if stat.st_size!=size or stat.st_mtime!=timestamp:
            return False
        # check that the contents match
        zip_contents = self.loader.get_data(zip_path)
        f = open(file_path, 'rb')
        file_contents = f.read()
        f.close()
        return zip_contents == file_contents

Notice that it checks the size and timestamp (as suggested by the bug report) but also checks the contents. So even if the file is modified, it will get replaced before being loaded.

Thoughts?

[Grant Murphy (gmurphy)]

@cthier I think that is actually part of the fix that python-setuptools has been adding to fix this issue... There is still a toctou race condition elsewhere that has not been correctly addressed by this code. In any case I would recommend not setting the PYTHON_EGG_CACHE to /tmp unless it is absolutely necessary. (Even if the exploit is unlikely).

[Thierry Carrez (ttx)]

If it's useless I agree we should get rid of it as a strengthening measure.
If it's useful then we could create a real tempdir in setup_env.

Most distributions are very fine without it. I guess you could set up your distribution of Swift or Glance in a way that relies on eggs for dep loading and requires the envvar being set. But then, THEY should set the envvar ?

In all cases, I think there is no reason to issue an advisory about that, since the base vulnerability is in setuptools and it's not really exploitable.

Grant: should we keep this embargoed so that it can be properly fixed in setuptools first ? Or can we open this bug publicly ?

[Grant Murphy (gmurphy)]

@ttx I think keep it embargoed until the python-setuptools CVE is released / fixed. I'll try to speed up that process in the meantime.

[Thierry Carrez (ttx)]

Not a problem -- should take us some time to determine if we actually need PYTHON_EGG_CACHE set or not, anyway.

[Thierry Carrez (ttx)]

@Grant: do you know if the python-setuptools CVE was released / fixed ?

[Grant Murphy (gmurphy)]

@Thierry Sorry this looks like it slipped through the cracks. It looks like it was decided not to assign this a CVE within setuptools for this. As such I think that this probably doesn't warrant a ossa either. I think it is probably safe to fix this one in the open if any code changes need to be made.

[Thierry Carrez (ttx)]

Yes, i'm also tempted to just open this one and consider it strengthening. More visibility will help getting the right people looking into it. Will open unless someone complains in the next days.

[Kurt Seifried (kseifried)]

Thierry can you post this to oss-security as well, you're probably not the only project affected by this class of flaw. Thanks!

[Thierry Carrez (ttx)]

@Kurt: Since it's not OpenStack-specific, I think it would be more appropriate for the original researchers (Grant and Dhiru) to post to oss-security (unless it's done already ?) We should probably wait for that before we make this bug public.

[Grant Murphy (gmurphy)]

@ttx I posted a write up of this vulnerability on oss-security today. You're probably ok to make this public now.

[Erno Kuvaja (jokke)]

This has been set in Glance since Jan 2012 commit 804396204e23ebb6c29c396396027bc3b09b0eab