rapid7 / metasploit-framework

Metasploit Framework
https://www.metasploit.com/
Other
33.78k stars 13.9k forks source link

RFC: Contributing Java Serialization Framework #11748

Open mb-syss opened 5 years ago

mb-syss commented 5 years ago

Hi,

Along with some new RMI/JMX scanner/exploitation modules we have built a general framework for exploiting Java serialization related issues. Both we would be happy to contribute. The serialization framework could be used to easily implement a wide variety of other exploits.

As this is a rather large chunk of code and I had to cut some corners to get what I wanted (which also might be because my lacking insight in metasploit internals) I'd like to start out with this "meta" issue so that we can decide whether you like it, how we can go about this whole process, how to split that up and maybe come up some design improvements. Let me know if there is a better platform to have these discussions.

Apart from the actual RMI/JMX modules most currently lives in a single ruby library:

What probably needs some discussion:

Moritz

timwr commented 5 years ago

Thanks for posting! Happy to discuss here on GitHub. Pinging @schierlm :)

schierlm commented 5 years ago

Not much discussion so far...

Anyway, my 2¢:

  1. I cannot see a link anywhere to the code you are planning to contribute (or where the project currently lives). It's hard to discuss about code you cannot see.

  2. You write that you need a "customized version of the Java meterpreter". I am pretty sure it should be doable without customizing Meterpreter (files below https://github.com/rapid7/metasploit-payloads/tree/master/java/meterpreter), but it should suffice to customize the payload loader or even wrap it in another classloader, similar to how it is done with the RMI loader.

    When looking at the current state of the payload loader, it got quite some features (compared to the version when I initially contributed it), so probably wrapping it is the easier approach. Still, the configuration (and probably some dependent classes) have to be somehow patched inside the wrapped classloader class if that classloader cannot load resource files from its parent classloader.

  3. Different Gadgets that support different exploitation methods (one that executes commands, and one that executes Java classes) should become separate Metasploit modules. If one gadget supports multiple of them, e.g. both loading classes and executing commands) you can use ARCH_CMD vs. ARCH_JAVA to differentiate (in the same module). I would try to avoid presenting more than one exploitation method that loads the same kind of payload via the same gadget to the user (e.g. upload of class files vs. download them from a httpserver). While your library classes should support it, a module should (in my opinion) choose the best way that matches the exploit (so, for example, if the exploit requires downloading files from a server, also download the payload classes from there).

mb-syss commented 5 years ago
  1. I was hoping to do some cleanup/splitting up (which it definitely still needs) based on the discussion first, but it is now at:
  1. Yes, the payload loader is what I meant. The config part should be reasonably simple, we can just try to load the resource or the generated config class and fallback to the other, or refactor that a wrapper can just pass in the Properties. What seems to be a bit more complex and possibly invasive are the features using writeEmbeddedFile, detaching via Spawn and dropping executables. I also have the feeling that relying on being able to load the classfiles as resources may also limit some other applications.This would require the relevant resources to be compiled into class files. As I haven't done that, to not block the calling thread I currently detach the staging code into a new thread, this might be a nice addition in general.

Another thing that I forgot to mention before is that due to subtle issues with the loader, I had to change the Stage interface into an abstract class (otherwise causes an exception). this should not make much of a functional difference but is a breaking API change.

Should we take these to metasploit-payloads? I could prepare some PRs and open issues for the things to discuss.

  1. With different metasploit modules you are talking about different exploit modules here, right? so that the payload module would then be /meterpreter, /exec and so forth?

For some very specific uses this could possibly work, but in many more generic, as the JMX stuff is, which gadgets can be exploited depends highly on the actual target application/codebase, so you really want to support as many gadgets as possible, preferrably (as the JMX module does) autodetecting which are available or the ability to try multiple. I don't think creating one module for each one and then having the user try works here.

schierlm commented 5 years ago

I only had a quick look at your code, so in case I misunderstood anything, sorry for that.

I see that you are injecting the config by building the actual config properties in dynamically generated code. I would have gone the way of just embedding "resources" as byte array (or as string which you getBytes() with an appropriate charset) in the class file (as I prefer not to generate code, I would have a huge byte[] or String containing all the resources and use a method to split them up), and then have the payload loader load these resources instead of your custom Map. So the payload loader will still construct a Properties, but not from the resource stream but from a ByteArrayInputStream that you provided.

Or (as I said) have a custom ClassLoader class template that overrides getResource to lookup your "embedded" resources, so you can use unmodified payload class (also embedded as a resource into that classloader and loaded by it).

An extreme way of that method is how Meterpreter's second level stager works by constructing an URLClassLoader around a JAR file in memory. In theory one could put that JAR file into a byte[] in a class, too.

In any cases, I would prefer if it is possible to use all existing ARCH_JAVA payload classes and stages (both in-tree and out-of-tree) unmodified with your library and/or exploits that use your library. Also to keep the on-wire staging protocol unchanged, so that you can point the reverse_tcp variant to external tools that stage completely different payloads than those included with Metasploit. But that would mean that you cannot "cherry-pick" features but rather have to try that the ClassLoader that ultimately loads your stage classes can load any classes (and interfaces), not just the ones you tested and patched.

I am kind of "out of the business" and did not really follow latest serialization exploits, so I am a bit surprised that it can be an obstacle from such an exploit to load (via an intermediate classloader) interfaces or resources. It may be that the exploited classloader is limited (but that is also true for other existing exploits that do not exploit serialization issues but e.g. Applet privilege elevation), but in that case using that limited classloader to load another classloader which in turn loads the rest would be preferrable from my point, over patching existing classes (those classes you know of) to work with the limitations of that particular class loader.

For maintainability of generated classes, I would always start with a class file generated by javac (loaded by your library), and then modify the constant pool or (if really required) the bytecode arrays of the method bodies. Generating the whole class "by hand" works too of course, but the number of people who are able to read and maintain that code are a lot fewer than the ones who can maintain code that "just patches existing classes".

Also, I would not try to introduce a new namespace java/classfile/ for the payloads, but use normal java/ payloads (not only meterpreter, but also shell and possibly out-of-tree ones) and have your library/mixin take care of mangling/wrapping the exploit.

Out of curiosity: What is the exact problem with loading Stage as an interface, which can be alleviated by loading it as an abstract class instead?

mb-syss commented 5 years ago

Thanks for having a look and the comments. I'll have a try at a more general wrapper, I think using a wrapper with a embedded JAR URLClassloader should avoid most of the issues there.
May take me a while though, a bit busy right now.

All these specifics are coming from the TemplatesImpl class, which is an "utility" (I think the only one) that allows inband loading/execution of arbitrary supplied bytecode starting out from a arbitrary method call/property access that can be achieved via a deserialization gadget. That class defines classes at runtime from an embedded bytecode array, which need have to have some specific properties (intended usage I think is XSLT transformations compiled to bytecode).

For maintainability of generated classes, I would always start with a class file generated by javac (loaded by your library), and then modify the constant pool or (if really required) the bytecode arrays of the method bodies. Generating the whole class "by hand" works too of course, but the number of people who are able to read and maintain that code are a lot fewer than the ones who can maintain code that "just patches existing classes".

I started out that way, but having the configuration strings in the class, I ended up fighting the optimizer quite a bit, so I settled for directly generating it. We'll probably still have to do some bytecode generation (to work around constant size limits we likely need array initialization), but I guess we can have most of the wrapper compiled from source.

Out of curiosity: What is the exact problem with loading Stage as an interface, which can be alleviated by loading it as an abstract class instead?

This is also coming from TemplatesImpl. You can't have custom interfaces there, as these have getSuperclass() == null, causing a NPE in TemplatesImpl.defineTransletClasses. This should no longer be an issue if we do the wrapping as these can then be loaded from the embedded JAR.

Also, I would not try to introduce a new namespace java/classfile/ for the payloads, but use normal java/ payloads (not only meterpreter, but also shell and possibly out-of-tree ones) and have your library/mixin take care of mangling/wrapping the exploit.

Sure, that was just to avoid messing with the "normal" meterpreter payload right now.

mb-syss commented 5 years ago

Some progress to show finally. Now we have a much cleaner generic loader for the Translet stuff which can invoke code from arbitrary JARs so we can inject the stock Java meterpreter (or something else): Payload at https://github.com/mb-syss/metasploit-payloads/blob/master/java/javapayload/src/main/java/metasploit/TransletPayload.java and generator code https://github.com/mb-syss/ruby-serialize/blob/master/loader.rb

I haven't adjusted the remaining code to use that yet, but it should show the concept.

I believe this now addresses the meterpreter side nicely. Also I think this implicitly puts the gadget generation / payload generation boundary naturally between the exploit module and the payload module as we now can now simply use the JAR produced by the payload module. This means that the exploit module will be responsible for creating the gadget object graph and setting up possibly required services. I'll have another go to see how this could be designed so that it is reusable.

mb-syss commented 5 years ago

One more question, as I finally may have a chance to continue working on this some time soon: I think we should keep much of the functionality in a reusable library, any suggestion how to go about this? Target rex-java? There is a bit of code that can be shared I guess, however we will end up with some duplication I suppose, as parts perform the same tasks, only on a much higher level abstraction (I don't remember the details right now, but I think I could not use the rex-java serialization API as a basis because of some issues regarding references). Or go for a separate library?

mb-syss commented 5 years ago

So I went ahead and did quite a bit of cleanup and refactoring, we now have:

A bunch of questions/remarks:

There are still a bunch of open tasks and some improvements I'd like to make. Would you care to have another look at the design? What is the next move from here?

mb-syss commented 4 years ago

Did anyone have a chance to have a look yet?

timwr commented 4 years ago

So I went ahead and did quite a bit of cleanup and refactoring, we now have:

It might be worth opening a pull request with the changes rather than just keeping it on a branch. If it's not ready you can mark it as a draft.

A bunch of questions/remarks:

  • I chose to target rex-java for a lot of code and also included the model and probing definitions there. I guess these are bound to be changed/fixed/upgraded somewhat regularly so I'm not sure whether it's acceptable to include this in library from a maintenance perspective. It can be a little more fiddly to update a library rather than the main repo but I suggest you keep there code there if that's where it belongs.

  • Is there a preferred way of logging inside msf lib/ utility classes? I see a bunch of modules using print_status from the module, but that does not seem to work for me when using classes (maybe based on my lacking insight into ruby scoping rules).

  • It's very common that the gadgets will fire multiple times and therefor multiple sessions will be established, it would seem to me that UUID tracking would be the preferred way to get rid of these, however I'm not sure how this needs to be set up in the module to work properly.

  • The Translet loader patching could probably go into the metasploit-payloads gem, would that be preferred?

There are still a bunch of open tasks and some improvements I'd like to make. Would you care to have another look at the design? What is the next move from here?

mb-syss commented 4 years ago

Opened PRs: