Insecure loads()

Bug #1006414 reported by Thierry Carrez
22
This bug affects 2 people
Affects Status Importance Assigned to Milestone
OpenStack Object Storage (swift)
Fix Released
Undecided
Vincent Untz

Bug Description

Split from bug 1005903, from Sebastian Krahmer:

swift uses pickle to store and load meta data. pickle is insecure
and allows to execute arbitrary code in loads().
[...]
BTW, you can read more on executing code via pickle or cPickle here:
http://nadiana.com/python-pickle-insecure

CVE References

Revision history for this message
Thierry Carrez (ttx) wrote :

Pickle is insecure in a model where an untrusted user can provide the pickled data. In the Swift model the data is pickled by Swift itself and stored in memcache, so the attack vector would suppose direct write access by an untrusted user to memcached data ?

Changed in swift:
status: New → Incomplete
Revision history for this message
Sebastian Krahmer (krahmer-p) wrote :

Ok, however the metadata is constructed with user data, e.g. 'self.name'. I hope its not possible
to do inlining tricks with that.

(Its also used directly to loads from the replicator, but I guess someone should not be able to
trigger such requests (but mind header injection attacks via proxy)).

Revision history for this message
Thierry Carrez (ttx) wrote :

Since we only unpickle data that was pickled by ourselves, I think we are safe from inlining. User-provided strings that get pickled+unpickled remain strings.

That said, I'd love to hear from Swift devs what type of complex data is actually stored pickled: depending on the answer we could use something a bit less powerful (like JSON), reducing the attack surface.

Revision history for this message
Chmouel Boudjnah (chmouel) wrote :

AFAIK most of the JSON usage in Swift are API interaction there is no objects marshalling via json in Swift.

Revision history for this message
Thomas Biege (thomas-suse-deactivatedaccount) wrote :

Regarding the attack vector via memcache mentioned in comment #1, AFAIK every local user has access to memcached because there is not authC in place to stop her/him.
I am not that familiar with the openstack architecture, so...
- Where does the memcached run exactly?
- And does it only listen to 127.0.0.1?

Revision history for this message
Chmouel Boudjnah (chmouel) wrote :

memcached on Swift runs on every proxy servers and shared a cache so it bind on the internal network ip. For swift we always assume that the internal network needs to be secure since Swift has been designed this way.

Revision history for this message
Mike Barton (redbo) wrote :

It's not an immediate security risk, since the pickle data can't be modified by external users. But it could let someone e.g. escalate a network compromise into arbitrary code execution.

I've looked at switching some of the pickle serialization to json, but it's pretty fiddly and will take time to get right.

Revision history for this message
Thierry Carrez (ttx) wrote :

So we all agree that this is not directly exploitable in Swift current security model, but it would be a very welcome improvement to serialize to something less potentially harmful ?

If yes, I'd suggest we open this bug to the public as a welcome security strengthening issue (rather than keep it as an embargoed exploitable vulnerability).

Thomas/Sebastian: do you agree ?

Revision history for this message
Thomas Biege (thomas-suse-deactivatedaccount) wrote :

I can agree with the reduced severity due to the attack vector and the additional steps needed.
I wish I could agree to the conclusion that this is not a vulnerability. It is a privilege escalation; an abuse is possible that crosses privilege boundaries to execute system commands.

The open question for me is only the level of impact. How likely is it to gain access to the internal network for an external attacker or an untrusted customer/user using a VM instance? What I heard from Sebastian is that it is really hard to separate the network if the attacker is an untrusted customer using a VM to execute network-level attacks.
Additionally Sebastian mentioned some ideas about alternative attack vectors in comment #2.
Sebastian, did you try to follow this ideas?
I also would not trust the authenticated user because OpenStack, by default, supports only password-based authentication and is at least vulnerable to brute force attacks.
So, for me it is a vulnerability with an yet unknown impact level.

The attack could be mitigated using authentication for memcached.

http://code.google.com/p/memcached/wiki/SASLAuthProtocol
http://dustin.github.com/2010/08/08/memcached-security.html
http://www.sensepost.com/blog/4873.html

Revision history for this message
Chmouel Boudjnah (chmouel) wrote : Re: [Bug 1006414] Re: Insecure loads()

On Fri, Jun 15, 2012 at 10:38 AM, Thomas Biege <email address hidden> wrote:
> The open question for me is only the level of impact. How likely is it to gain access to the internal network for an external attacker or an untrusted customer/user using a VM instance? What I heard from Sebastian is that it is really hard to separate the network if the attacker is an untrusted customer

Are you speaking from nova to swift ? Usually the private networks
would be on different networks between nova and swift only the proxy
will be exposed to the VM and the memcached services will be only
listening to the private interface on the Swift side and due not
exposed to the VMs.

Chmouel,

Revision history for this message
Sebastian Krahmer (krahmer-p) wrote :

Ok, so theres no immediate impact, yet its probably still worth considering to replace pickle
by something that cannot execute commands in de-serialization.

Revision history for this message
Sebastian Krahmer (krahmer-p) wrote :

Um, I just realized that we talked about 2 different attack vectors.

First, pickle is used in the backend to store and load data to/from extended attributes from
files inside read_metadata(). Thats not nice, but not a severe issue if nobody can mod the xattrs.

Then, I just realized that pickle is also used in the memcache part. As memcached is distributed
over a lot of nodes and has no authentication, except your mentioned separation into
a private LAN, this is worse than the above case.
The swift proxy runs as root (is that necessary anyways, after binding to a low port?) and potentially
unpickles anything that he gets. If just someone polluted the memcached with evil pickle data.
I see your private LAN argument, but my experience in network consulting unfortunally shows me that
there is often no such strict separation so that memcached cannot be accessed from the VM/cloud network.

However, I am not exactly sure how to handle that without too much efforts.

Revision history for this message
Thomas Biege (thomas-suse-deactivatedaccount) wrote :

I would propose to use SASL for memcached to mitigate the issue.

Revision history for this message
Sebastian Krahmer (krahmer-p) wrote :

E.g. an attacker from the VM or 'outside network' could send bogus ARP packets to the swift proxy,
polluting his ARP cache so that any connect() to an internal memcache IP will end at the attacker who can then
send any response to proxie's memcache.get() and trigger the pickle.

I think it will make a lot of headache to the network admin to nail down such things.

I am not really deep into the internals of WSGI, but would it be easily possible to drop
UID to a nova-user and/or chroot to /var/run/nova once swift proxy started? Or is that a problem
with python's importings?

Revision history for this message
gholt (gholt) wrote :

All the various Swift daemons support dropping privileges. For example, please see the "user" configuration variable at http://swift.openstack.org/deployment_guide.html#proxy-server-configuration

Revision history for this message
Vincent Untz (vuntz) wrote :

I've been looking at the use of pickle on the memcache side only, and it looks to me that moving to JSON there shouldn't be an issue -- I don't see any of the user calling it with really complex object types.

The only issue I can see with the default encoder/decoder is that some types will be changed after serialization/deserialization (for instance, tuples become lists, str becomes unicode, etc.). But I don't think it's something that will be a problem in the code.

Revision history for this message
Vincent Untz (vuntz) wrote :

As to the use of SASL for memcached: I'm not sure we want to implement that ourselves in our small memcache client : handling sasl, and moving to the binary protocal for memcache would have to be dealt with (not sure how easy it is to do sasl in python). We could instead use pylibmc (http://sendapatch.se/projects/pylibmc/) which wraps libmemcached and provides sasl support if the library was built with sasl.

What would be the preferred way here?

Revision history for this message
Vincent Untz (vuntz) wrote :

Here's a patch to use JSON instead of pickle for the memcache client. Moving to SASL will still be something worth the effort after that, but that's a first step, I guess.

This needs some more serious testing, as I'm unsure I correctly checked that all users of memcache are using simple-enough object types.

Also, as I wasn't sure that getting unicode when loading strings from json was good, I wrapped things to get str instead. I'm assuming we really always care only about utf8 (not sure this is a correct assumption, though).

Revision history for this message
Thierry Carrez (ttx) wrote :

Didn't get a clear answer to my proposal of opening this bug to public.

The fact that access to Swift "trusted" internal network allows you to fully compromise Swift is well-known. The "internal" network in Swift is much less vulnerable than the Nova internal network, where VM network containment vulnerabilities are much more likely (that's why Nova doesn't rely on a trusted network). And if you drop privileges to the swift user (as you should), there is no particular escalation due to use of pickle.

So we are left with the fact that breaking into Swift "trusted" internal network allows you to compromise Swift. I'd welcome any change that prevents that (or gives you at least the option to run on untrusted internal network), but we are a bit far away from that: all Swift internal RPC also rely on a trusted internal network model. So I see no reason to keep the discussion on that closed. On the contrary, discussing it in a small group probably slows us down in improving that well-known state.

So unless someone disagrees in the next 48 hours, I will open up this bug to the public.

Revision history for this message
Sebastian Krahmer (krahmer-p) wrote :

I see your point. But would be very nice to use Vincent's json patch (pickle seems really overload
for simple string dictionary storage anyway). For me its OK to
make this bug public.

Revision history for this message
Thierry Carrez (ttx) wrote :

Definitely in agreement that Vincent's patch should be included ASAP. Opening :)

security vulnerability: yes → no
visibility: private → public
tags: added: security
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to swift (master)

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

Changed in swift:
assignee: nobody → Vincent Untz (vuntz)
status: Incomplete → In Progress
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to swift (master)

Reviewed: https://review.openstack.org/9105
Committed: http://github.com/openstack/swift/commit/e1ff51c04554d51616d2845f92ab726cb0e5831a
Submitter: Jenkins
Branch: master

commit e1ff51c04554d51616d2845f92ab726cb0e5831a
Author: Vincent Untz <email address hidden>
Date: Thu Jun 21 14:37:41 2012 +0200

    Do not use pickle for serialization in memcache, but JSON

    We don't want to use pickle as it can execute arbitrary code. JSON is
    safer. However, note that it supports serialization for only some
    specific subset of object types; this should be enough for what we need,
    though.

    To avoid issues on upgrades (unability to read pickled values, and cache
    poisoning for old servers not understanding JSON), we add a
    memcache_serialization_support configuration option, with the following
    values:

     0 = older, insecure pickle serialization
     1 = json serialization but pickles can still be read (still insecure)
     2 = json serialization only (secure and the default)

    To avoid an instant full cache flush, existing installations should
    upgrade with 0, then set to 1 and reload, then after some time (24
    hours) set to 2 and reload. Support for 0 and 1 will be removed in
    future versions.

    Part of bug 1006414.

    Change-Id: Id7d6d547b103b4f23ebf5be98b88f09ec6027ce4

Changed in swift:
status: In Progress → Fix Committed
Thierry Carrez (ttx)
Changed in swift:
milestone: none → 1.7.0
status: Fix Committed → Fix Released
Ghe Rivero (ghe.rivero)
tags: added: essex-backport
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to swift (stable/essex)

Fix proposed to branch: stable/essex
Review: https://review.openstack.org/12492

Revision history for this message
Alan Pevec (apevec) wrote :

I don't have a permission to see the proposed essex backport but IMHO patch as-is is not appropriate for the stable branch.
It should at least be modified to have memcache_serialization_support=0 as default to avoid surprises on upgrade!

BTW does Swift team maintain stable branch at all? Currently stable/essex branch == 1.4.8 tag and last I remember was that stable for Swift is not needed, quoting PTL from one stable/essex proposal: "taking into account that swift releases are frequent, prod-ready, and there is a migration path for them" https://review.openstack.org/#/c/6619/

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.