+ + + + + +
+
+

ExMachina Code Audit

+ + +
+ +
+ +
+

Introduction

+
+

I’ve been poking around Ex Machina + (EM) for a bit. While doing my poking, I tried to see if I could find +any ways to break it. I’m writing this as a running log of my +investigation, so that other folks can see bits of the investigative +process I followed to complete a layman’s security audit.

+

I’ll make the relevant changes to ExMachina soon. Till then, it shouldn’t be used in production.

+
+
+
+

Code Snippets

+

For each of these snippets, I’ll first display the code involved, +then state the problems it contains, and finally post the solutions I’ve + been able to find for those problems.

+
+

Authentication

+
+

For reference, I’m talking about the following block of code. Don’t +worry if you don’t understand it, I’ll break it down the best that I can + understand:

+
class ExMachinaHandler(bjsonrpc.handlers.BaseHandler):
+
+    # authentication state variable. If not None, still need to authenticate;
+    # if None then authentication not require or was already successful for
+    # this instantiation of the Handler. This class variable gets optionally
+    # overridden on a per-process basis
+    secret_key = None
+
+    # ... 
+    def authenticate(self, secret_key):
+        if not self.secret_key:
+            log.warn("Unecessary authentication attempt")
+            return
+        if not self.hash(secret_key.strip()) == self.secret_key:
+            # fail hard 
+            log.error("Authentication failed!")
+            sys.exit()
+        self.secret_key = None
+

This is a small piece of code from the class that defines an RPC +server, the bit of code that acts on system maintenence requests from +other programs. This is the workhorse program that actually tries to +change the underlying system based on requests it receives. Some program + uses the exmachina client to send a request to the exmachina server +which in turn changes the underlying system. If you’re a visual learner, + it looks like this:

+
"some program" -> "exmachina client" -> "exmachina server" -> "system"
+

Since any old “some program” could send data to the EM client, which +passes it along to the EM server, that server had better make sure that +only clients it trusts can send data, otherwise anybody could try to +send destructive commands, like sudo rm -rf /, which is the + equivalent of detonating a small bomb on your computer’s harddrive: +after that command finishes, everything you care about on your system +will be gone.

+

To use the authentication feature, the client does the following:

+
class ExMachinaClient():
+    def __init__(self, socket_path="/tmp/exmachina.sock", secret_key=None):
+        # ... some initialization ... 
+        if secret_key:
+            self.conn.call.authenticate(secret_key)
+        # ... more initialization ... 
+

These code snippets create a few different problems.

+
+
+

Authentication is Optional

+
+

The client has to willingly use the authentication feature. Nothing +bad happens to a client that doesn’t authenticate. The reference code +doesn’t even handle an error condition (“if authentication fails, then +…”), it just keeps initializing.

+

Servers with keys don’t require clients to have or use those keys:

+
$ echo "12345" | ./exmachina.py -vks /tmp/exmachina.sock &
+
+2012-10-20 15:06:09,562 DEBUG Waiting for secret key on stdin...
+2012-10-20 15:06:09,562 DEBUG Got it!
+2012-10-20 15:06:09,562 WARNING Expected to be running as root!
+
+$ ./test_exmachina.py
+
+========= Testing JSON-RPC connection
+/*: None
+/augeas/*: None
+/etc/* files:
+...
+

It should’ve bailed instead of returning results.

+
+
+

Non-Optional Authentication

+
+

The EM server should verify the client once per connection, or every +call, if it’s impossible to determine (or possible to fake) a call’s +source. This brings up the whole ordeal of authentication tokens, which +is a hairy mess I’m not looking forward to.

+

The best I can do, I think, is to just decorate every function call with an authenticator. I’m not sure how to best do this.

+
+
+
+
+

Authentication Failures Kill the Server

+
+

Incorrectly authenticating the client kills the server. This is like +taking Google offline because somebody mistyped their Gmail passsword.

+
$ echo "12345" | ./exmachina.py -vks /tmp/exmachina.sock &
+
+2012-10-20 15:06:09,562 DEBUG Waiting for secret key on stdin...
+2012-10-20 15:06:09,562 DEBUG Got it!
+2012-10-20 15:06:09,562 WARNING Expected to be running as root!
+
+$ echo "the kind of thing an idiot would have on his luggage" | ./test_exmachina.py -k &
+
+========= Testing JSON-RPC connection
+2012-10-20 15:05:50,453 ERROR Authentication failed!
+
+$ jobs
+
+[1]-  Done                    echo "12345" | ./exmachina.py -vks /tmp/exmachina.sock
+[2]+  Exit 1                  echo "the kind of thing an idiot would have on his luggage" | ./test_exmachina.py -k
+
+$
+

The server should’ve rejected the client, the server shouldn’t've died.

+

Looking through the code, it seems like the author might’ve forgot +they were working in the server and actually meant to put that code in +the client.

+
+
+
+

Plain Text Passwords

+
+

Next, the password’s stored in plain text. An attacker who wanted to +read the password from the server should be able to… Well, no, the +process separation (via the socket) should prevent that, sort of. As +long as the process separation only exposes entry points into the +server’s functions and not the server object itself, the server’s data +should be protected. This comes with one very obvious caveat:

+

Anything exported by the host object can be called by the client +object, so the client might be able to set the secret key. We should +make sure that the secret key can only be set by a trusted user.

+
+
+

Don’t Store the Password in Plain Text

+
+

Hashing in Python is simple:

+
def hash(self, secret_key, salt):
+    """Hash the secret key with the salt."""
+
+    if not salt:
+        salt = os.urandom(128)
+    return hashlib.sha256(secret_key.strip() + salt)
+

Set the secret key to that value and it’s generally available. +Salting with a 128 bit salt, while not strictly necessary if no client +has access to the secret key, is nonetheless preventative, in case +somebody does figure out how to read the key. This way, each ExMachina +appears to have a different password (even those with the same password) + and those beautiful SHA256 rainbow tables are mostly irrelevant: nobody + creates 340282366920938463463374607431768211456 tables just in case.

+
+
+
+
+

Untrusted Client Password Setting

+
+

Unless bjsonrpc offers sufficient process separation between the server and client, this is introspective Python + so any program could simply reach far enough up into the client to find + the server and its password. Thus, even if the authentication code +worked, the idea is still completely screwed at a fundamental level.

+
+
+

Don’t Set the Password from an Untrusted Client

+
+

So, this isn’t actually a problem, just something I wanted to point out. The process separation seems sufficient, given how bjsonrpc is built, that no client should + be able to reach into the server to pull out the password. The server +exposes functions that the client can call, as filtered by bjsonrpc. If +the server doesn’t expose it as a function, the client can’t ask for it. + Thus, the client can’t simply ask for non-functional data: that’s +effectively private, thanks to the process separation.

+

Even if clients could read the hashed password, as long as they can’t + set data directly on the server, we can safely allow clients to set a +custom password. First, we’d need to force the server to reject any +subsequent attempts to set the password, if it’s already set, and then +we’d need to prevent untrusted users from writing to the socket until +the password is successfully set. We can prevent users from writing data + to the socket by setting the file permissions on the socket correctly +(0×0600, preventing other users from writing to it), before we start +using it for communication. Preventing the server from accepting a new +password involves adding only a few lines of code:

+
def set_password(self, password):
+    if self.secret_key:
+        return
+    else:
+        self.secret_key = self.hash(password, self.salt)
+

Even if client functions can be re-bound, the server’s process separation still protects us here. So, a client could change self.server.call.set_password + to another function, but then it’s no longer a parameter-passing shim +between the client and server process (the part that does the real +work): the client can change its functions, but then it’s no longer +calling the server. This is just message passing, so as long as bjsonrpc + itself doesn’t support clients modifying the server (which I didn’t see + evidence of when I looked, but might’ve missed, but would be shameful +if it did exist).

+
+
+
+
+
+

Socket Binding

+
+

This might be the most important problem, but is likely the one that +requires the most code changes and strictest set of fixes. See if you +can’t figure out what’s up with this code:

+
def run_server(socket_path, secret_key=None, socket_group=None):
+    # ... 
+    if os.path.exists(socket_path):
+        os.unlink(socket_path)
+    sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
+    sock.bind(socket_path)
+    sock.listen(1)
+
+    if socket_group is not None:
+        socket_uid = os.stat(socket_path).st_uid
+        socket_gid = grp.getgrnam(socket_group).gr_gid
+        os.chmod(socket_path, 0660)
+        os.chown(socket_path, socket_uid, socket_gid)
+    else:
+        os.chmod(socket_path, 0666)
+    # ... 
+
+
+

There are two problems.  Did you catch them both?

+

A Self DOSing Platform

+
+

If you create multiple ExMachinae each one takes over the default socket after it initializes, DOSing the real EM.

+
+
+

Don’t Self DOS

+
+

Unix file permissions to the rescue again! Set the directory that contains the socket files to forbid writes from any but the owning user. That’ll prevent anyone else from overwriting the existing sockets.

+

If we do that, each socket that replaces the previous one will have +the same permissions the previous one did. We’ll lose communication with + running processes, but that’s about it.

+
+
+
+
+

Problematically Trustworthy

+
+

We accept input from any user on the system. As mentioned above, +that’s not a great idea, because it makes the process vulnerable to a +wider selection of attackers than necessary.

+
+
+

Be Less Trusting

+
+

Simple. Don’t give any groups access to the socket unless specifically requested. Never give the other user access to the socket.

+
def run_server(socket_path, secret_key=None, socket_group=None):
+    # ... 
+    if os.path.exists(socket_path):
+        os.unlink(socket_path)
+
+    sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
+    sock.bind(socket_path)
+
+    os.chmod(socket_path, 0600)
+
+    if socket_group:
+        os.chmod(socket_path, 0660)
+        os.chown(socket_path, gid=grp.getgrnam(socket_group).gr_gid)
+    # ... 
+
+
+
+
+
+
+

Conclusion

+
+

ExMachina is a neat idea but it shouldn’t be used till these issues +are fixed. Then, it should be considered in a testing phase, undergo +peer review, and folks should try to break it for a while before it’s +considered solid.

+
+
+
is a…

+
+
+
+ + +
+ +
+ + + +
+

Leave a Reply

+
+

Your email address will not be published. Required fields are marked *

*

+ +

+

You may use these HTML tags and attributes: <a + href="" title=""> <abbr title=""> <acronym title=""> +<b> <blockquote cite=""> <cite> <code> <del +datetime=""> <em> <i> <q cite=""> <strike> +<strong>

+ + + +

+
+
+ +
+ + +