-
Notifications
You must be signed in to change notification settings - Fork 14.4k
Initial support for Malleable C2 Profiles in HTTP Meterpreter #20419
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: 6.5
Are you sure you want to change the base?
Conversation
Munged a few commits into this one. But we have basic support for TLV-based configuration blocks instead of hard-coded block sizes. Initial support for the MC2 stuff is in as well, but more to come.
Still don't have all the fields implemented, but this at least supports the notion of having different URIs for GET and POST. The approach taken, to reduce the impact on how much code has to be changed, is to extract the UUID for the connection and use that as a resource identifier. This UUID doesn't have any slashes in it, and hence will not collide with any URI. This means we can use the UUID as a key in the same hash as the resource URIs knowing that a direct lookup will find the right session, even if by some miracle the UUID collides with a chosen/generated URI. Any URI in the resource list will be prefixed with a forward slash. The listener will listen on all URIs that exist for the Meterp configuration, including LURI setting, and the `uri` values in all three areas that it might be specified in the C2 profile.
* Supporting "wrapping" and "unwrapping" of payloads based on the C2 profile, which means that suffixes and prefixes are used based on what the configuration indicates. * Made sure taht the debug_build flag is passed through on HTTP/S payloads. * push details of the C2 profile into the meterp client so that required details can be easily accessed.
Stageless payloads start with an :init_connect which needs special consideration given that it's just redirected. There's no client instance at that point, so there's no C2 associated with it, so we have to just manually wrap the outbound packet so that things work correctly.
Includes removal of the referrer and accept types specific TLV values, because they can be treated like any other header, despite what the MSDN documentation says about the HTTP APIs. Moved packet wrapping to somewhere reusable. Added support for binary-escaped strings in C2 profile values (eg. "\x00").
Interim commit, contains code persists a C2 profile instance for reuse rather than having many being parsed all the time. Also begins work handling UUIDs outside of the URI.
The `Http::Request` class had an overload for the `body` accessor that returned the query string parameters in the case that the body was empty. This is not only logically bizzarre, but functionally insane. The query string is not part of the body. If you want the query string, go get it. An interesting side effect of this craziness, along with the way the body is constructed, is that if you send a POST request to the server with a body AND a query string, MSF is kind enough to give you both together. Crazy right? Well, this is because the class uses the `body` accessor as an internal buffer, but that getter is overloaded. So if the `body` is blank, and the `+=` operator is used (which, it is!) then you end up with the query string being prepended to any actual body content. Insane. Also, from an API point of view, it looks just as crazy. Observe: ``` >> r = Rex::Proto::Http::Request::Post.new('/foo?lol=wtf') => ... >> r.body = '' => "" >> r.body => "lol=wtf" ``` No. This is a complete violation of logic. This commit removes this "feature" and not only fixes the bugs that I was fighting against, but restores some semblance of reason.
NOTE: This change does remove the trailing "/" from URIs registered.. which implies that things might not match. So more to do here. Connection IDs are stored in the request now, so that they can be referenced by clients if and when required. IDs are pulled from various locations in the request.
Logic for finding connection UUIDs has been pushed into reverse_http so that it's not part of the Http::Server any more. It's a little bit of a leaky abstraction, but at least the logic is in the one place now. Support added and tweaked for including the UUID in an HTTP header or in a GET param. Currently don't have support for it in the BODY as as param, not sure if that's a requirement yet or not. Same goes for cookies.
I hate this craziness, but I have no idea what I'll break if I don't leave this in.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome!
lib/msf/core/handler/reverse_http.rb
Outdated
while base_uri[-1, 1] == '/' | ||
base_uri = base_uri[0...-1] | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
while base_uri[-1, 1] == '/' | |
base_uri = base_uri[0...-1] | |
end | |
while base_uri.ends_with?('/') | |
base_uri.chop! | |
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd prefer avoiding chop!
as we should avoid mutating strings we don't own as it introduces bugs - and in the future there will be more frozen strings about in the future:
Ruby is implementing frozen string literals gradually over three releases:
Ruby 3.4 (Now): Opt-in warnings when you enable deprecation warnings
Ruby 3.7 (Future): Warnings enabled by default
Ruby 4.0 (Future): Frozen string literals become the default
https://www.prateekcodes.dev/ruby-34-frozen-string-literals-rails-upgrade-guide/
Which will cause issues:
3.3.0 :002 > x.chop!
(irb):2:in `chop!': can't modify frozen String: "a" (FrozenError)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
base_uri = base_uri.chop
then?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No preference, the code change is just renaming an existing var so I don't mind leaving the code as-is
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FWIW, this isn't code that I wrote as part of this PR, it was me moving some code around that was already part of the codebase. Happy to tweak it though! :)
Would it not be better to use base_uri[-1]
instead of .ends_with?()
given it's just a single index and character comparison rather than a tail-end string comparison? Forgive my ignorance here :)
elsif scanner.scan(/^\s*#.*$/) | ||
# comment | ||
next | ||
elsif scanner.scan(/\"(\\.|[^"])*\"/) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is ""
an acceptable token?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm.. I guess so? though I can't see where we would want to specify that somewhere and it still have meaning.
Thanks for the comments folks, I will validate in the morning. Getting close to having this out of draft at least. I appreciate the effort so far :) |
end | ||
|
||
def is_block_keyword?(word) | ||
BLOCK_KEYWORDS.include?(word) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not a blocker: Introducing constants for the regex union like BLOCK_KEYWORDS = /#Regexp.union(BLOCK_KEYWORDS.sort)}\b/
as well as for OTHER_KEYWORDS
might be nice to help align the approach taken for finding these lexemes (i.e. since there's is_block_keyword?
here, but below you directly use type = BLOCK_KEYWORDS.union(OTHER_KEYWORDS).include?(word) ? :keyword : :identifier
too)
def expect(types) | ||
token = current_token | ||
types = [types] unless types.kind_of?(Array) | ||
raise "Expected #{types.inspect}, got #{token&.type}=#{token&.value}" unless token && types.include?(token.type) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we'll need to spit out line number/column here if possible from the lexer, as most people will likely get their profiles wrong and it'd be hard to debug things as user when this happens:
generate malleablec2=profilelhost=192.168.123.1
[-] Payload generation failed: Expected [:identifier, :keyword], got =
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah. Likely going to need to stash that information alongside the token along the way. That shouldn't be too hard though.
I think I'm at the point where the only things are non-blockers and it's probably worth testing a LOT, and for everyone to decide on how to change any crappy design decisions, before implementing this in the other meterps. |
I'll remove the |
👑 |
IGNORE - WORK IN PROGRESS -- Also waiting for @smcintyre-r7 to fire up a staging branch to PR this against (it's not going to
master
)Initial support for Malleable C2 profiles (HTTP(S) only).
NOTE: Associated payloads PR is over here, and has a couple of things still to do before ready.
This PR contains changes to provide initial support for malleable C2 features stored in a
.profile
file (such as this).Given the nature of how Meterpreter works, and how payloads are generated, there are a lot of C2 profile features that can't be supported without drastic changes. The intent here is to focus on the transport-specific details around HTTP payloads.
These changes are BREAKING changes, for a few reasons, and hence will not be backwards-compatible with payloads from older framework versions. Whatever you do, do not try to upgrade to this version if you're mid-engagement.
Summary of changes
C2
to fit with the new vernacular. These IDs are different to the old ones.MALLEABLEC2
has been added to thereverse_http(s)
listener which allows the user to specify a path to the.profile
. This profile is loaded and, where relevant, use to configure the HTTP parameters.Detailed discussion
Configuration Block
The config block isn't a block any more. It's a TLV packet with it's own type. It's treated the same as any other packet, but doesn't get encrypted (only XOR-ed). Configuration can contain these TLVs:
These new C2 TLVs are used for all transports, not just the HTTP ones. From the list above it should be obvious which ones are used by HTTP(S) only.
A
TLV_TYPE_C2
is a single instance of a transport, that could be TCP, or it could be malleable HTTP. There can be many of them in the configuration.What's important to note for HTTP(S) is that there are 3 sets of options. One is default, or top-level, and those values appear outside of the
http-(get|post)
sections of the profile. These are also used by the standard Meterp payloads that don't use a C2 profile. Then there are GET- and POST- specific blocks that override those default values. They are specified in theTLV_TYPE_C2_GET
andTLV_TYPE_C2_POST
blocks. The values that can be set/overridden are:The configuration block can have multiple instances of the
TLV_TYPE_EXTENSION
as well, each of which has a raw data block, size and initialisation script.Config blocks should have been TLVs from the outset. The person who came up with the original design should be ashamed of themselves :)
Binary Modifications
Meterpreter doesn't, by nature, provide the means to support most binary-specific things that something like Cobalt Strike does. Implementing those features was not part of the scope of this bit of work, instead the focus was on the HTTP transport, in an effort to make it easier to configure Metepreter's network behaviour. This means we can look less like Meterpreter on the wire.
UUID handling
One of the limitations that we have is that we rely on the transport-specific UUID to identify the incoming session. This UUID needs to be maintained across the requests, and we used to just rely on this being in the URI. Malleable C2 profiles allow us to specify different locations for the
id
(theUUID
in our case). This is made a little more complex because of the way that stageless payloads work.Stageless payloads have a UUID baked into them. That UUID contains a
mode
value that is set to:init_connect
. When a stageless payload is invoked, and the first request is made to MSF, the UUID is extracted and MSF sees the mode has come from the stageless payload because of this:init_connect
mode. It immediately generates a new UUID with a mode set to:connect
. This new UUID is returned to the Meterpeter instance, and from there the new UUID is used for every future request. This means that we have the ability to know when difference instances of the stageless payload are invoked and hence multiple runs of the same payload don't tread on each other's toes.In the past, ever single Meterpreter session would call back on a URI with the format
/LURI/UUID
. After a payload was staged and a session established, each Meterp session would register a passive packet handler with the server/listener with the/LURI/UUID
, which made it easy for incoming requests to be linked with the appropriate Meterpreter session, as it was a simple key-value lookup based on the full URI.With malleable C2 profiles comes the ability to GET and POST to different URIs, as well as put the UUID in different locations (URI, query string, custom header, etc). This means that we can't rely on
/LURI/UUID
as an identifier any more. Therefore this PR contains some code that changes the way that the incoming requests are mapped to Meterpreter sessions.Note that any Meterpreter HTTP payloads that do not make use of Malleable C2 profiles work as they used to, with the usual parameters. These will continue to use
/LURI/UUID
on incoming requests as they always have done.Keyword support
As mentioned, not all keywords are supported because they don't match with the way Meterpreter works. Here's an example of a profile that covers what's currently supported:
Encoding stuff will be done shortly.
It's important to note that a few parameters are "global" and are overridden in POST and GET specific blocks. Those parameters are:
uri
useragent
parameter
header
HTTP Handler modifications
As discussed, we can't rely on the UUID being in the URI any more so we needed a smarter way to associate sessions with incoming requests.
Any entity registering itself with the http server to handle requests specifies which URI it wants to handle. This is used as a lookup to match requests with handlers. Each of those URIs starts with the
/
. Using this same feature, any Metepreter session that has been established, registers itself with the listener with the transport's UUID, without using the/
prefix. Therefore the handler has a key-value lookup that contains a mixture of:/SOME/LURI
=> HTTP Request HandlerUUID
=> Meterpreter Client Passive HandlerWhen a request comes in, the handler will look to see if there's any associated UUID extraction function attached to the object that's stored in the
MsfExploit
parameter. This doesn't exist for standard HTTP listeners, and as a result, it falls back to the URI and matches the handler based on that. For Metepreter-specific HTTP listeners, a function exists that looks in the associated request for the connection ID (ie. the UUID) that is based on the configuration of that listener. When invoked, the function extracts the UUID from the incoming request and returns it. The server can then use that UUID as a key to lookup the associated client passive handler that is handling that particular session.This means that UUID extraction is specific to the listener's configuration, and specific to the meterpreter session.
This method is up for discussion, but it seemed like the functionality to extract the UUID should be configured based rather than an attempt to find the value anywhere in the request every time one came in.
The HTTP Packet/Request bug
Check out link incoming for discussion on this issue.
Verification
The best way to make sure that things work as expected is to watch the traffic over Wireshark or via a proxy of some kind. It's hard to see it otherwise!
msfconsole
windows/(x64/)meterpreter_reverse_http(s)
and make sure that theMALLEABLEC2
option is set to/path/to/test.profile
I would recommend sticking with
windows/meterpreter_reverse_http
to start with so that you can use Wireshark to look at the traffic and make sure that the requests and responses contain all the things they should contain.There are lots of things that need to be tested in various combinations. Bearing in mind that there are a set of keywords that don't make a difference to Meterp, and hence are ignored. Worth testing are:
base64
andbase64url
in thehttp-post/client/output
andhttp-get/server/output
sections).prepend
andappend
in thehttp-post/client/output
andhttp-get/server/output
sections).header
andparameter
options in thehttp-post/client/id
andhttp-get/client/metadata
sections). Make sure that when you useheader "MY-CUSTOM-HEADER"
that the UUID appears in the request asMY-CUSTOM-HEADER: <uuid>
and when you useparameter "mygetparam"
that the request contains a query string parameter that looks like...&mygetparam=<uuid>
.header
andparameter
values in thehttp-(get|post)/(client|server)
sections.uri
anduseragent
and overriding them in thehttp-(get|post)
sections.Also bear in mind that the following should be tested:
EXTENSIONS
andEXTINIT
options still work.MALLEABLEC2
should work as they did before.Sample Runs
Sample generation of payload:
Handler setup and interaction:
TODO
transport add
command