Insecure temporary file usage in Cassandra datastore

[Travis McPeak]

This code in the Cassandra datastore for Trove uses temporary files insecurely here:

https://github.com/openstack/trove/blob/master/trove/guestagent/datastore/cassandra/service.py#L130

Around the above line, config is written to a hardcoded file in /tmp and then copied (with sudo) to another hardcoded path. Since this path is hardcoded and in tmp (with no special permissions set on it), an attacker may be able to tamper with the contents of the file, or beat the legitimate code to the creation of the file. In either case, the attacker controlled config will be copied to the protected (as evidenced by the need for sudo) Cassandra config file path.

If temporary files are required, secure libraries/functions should be used instead. Potentially sensitive config data should be protected with adequate access controls at all times.

[Jeremy Stanley (fungi)]

I agree this sounds like a classic "predictable temporary file" vulnerability. Risk is likely low since the trove guest agent runs on trove instances which typically don't have untrusted local users in a typical deployment, though I suppose there might be some accessible database features capable of coaxing symlink creation to perform an overwrite attack on systems lacking sufficient in-kernel safeguards against that.

I've subscribed Trove core security reviewers to weigh in on the bug report and potential fixes.

[Amrith Kumar (amrith)]

See https://review.openstack.org/#/c/138719/

[Jeremy Stanley (fungi)]

I was hoping to at least get some exploitability discussion out of the way before this became public, but since it's now referenced in a public code review there's no longer any reason to maintain an embargo.

In the future, fixes for private security bugs should be proposed as attachments so that the discussion around them can remain private while a coordinated disclosure is being prepared.

[Travis McPeak (travis-mcpeak)]

Is this the right CVE? It looks like it refers to POODLE.

[Jeremy Stanley (fungi)]

The OpenStack vulnerability management team hasn't requested a CVE yet since we're still waiting on one of the Trove core security reviewers to weigh in on impact and exploitability first.

[Thierry Carrez (ttx)]

OK, let's wait for Trove folks. I think it's highly unexploitable in Trove normal mode of operation, but better safe than sorry.

[Nikhil Manchanda (slicknik)]

The impact of this is pretty minimal. From a deployment perspective, datastores are deployed so that file access is not allowed. Coupling that with the fact that SSH access to the Trove instance is also restricted, this vulnerability seems very hard to exploit. However, regardless of these mitigations, we're planning on having a fix for this in Trove during kilo.

[Jeremy Stanley (fungi)]

Due to the need for access to the instance filesystem and the limited exposure (basically anyone with shell access to a Trove instance is going to be the administrator of the infrastructure on which it's running) along with the fact that it's only slated to be fixed in the master branch for inclusion in the upcoming Kilo release, the VMT will not be publishing a security advisory nor requesting a CVE for this bug.

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/138719
Committed: https://git.openstack.org/cgit/openstack/trove/commit/?id=61774984aa2bacfe89867fc39a402a6a4cfb8f33
Submitter: Jenkins
Branch: master

commit 61774984aa2bacfe89867fc39a402a6a4cfb8f33
Author: Amrith Kumar <email address hidden>
Date: Thu Dec 4 10:26:24 2014 +0200

    Address predictable temp file vulnerability

    This change addresses a predictable temporary file vulnerability in
    the code path that writes the cassandra configuration. Unit tests have
    been added.

    Since there is a problem with Mock()'ing a shared entry point (like
    os.unlink or utils.execute_with_timeout) there is an inherent
    instability in these tests. The safe way around this is to make
    write_config() accept arguments that can be Mock()'ed for the purpose
    of testing.

    A lengthy explanation of the rationale for this change is on the
    openstack-trove IRC channel at about 10:40AM on 12/29/2014.

    Also, there was a redundant test that has been eliminated.

    Change-Id: I760b937b6714b2b2b366cd8bdb700bece6055fba
    Closes-Bug: #1398195
    Partial-Bug: #1398966