Patches UA and Proxy Settings in metsrv when running migrate #3632
Conversation
|
Get the same behaviour here, but we should refactor the code to a common class in lib/rex/somewhere to prevent the code deviating in future. lib/rex/post/meterpreter isn't appropriate as the code is for payloads and post, but not sure where to put it at the moment. Poke @jlee-r7 ? |
|
Maybe something like lib/rex/payloads/meterpreter/patch.rb or something? |
|
@Meatballs1 - For sure. Does it make sense to build something that would handle all of the original payload's advanced options? For example, I don't believe that EnableStageEncoding or StageEncoder get applied during migrate. |
|
Yes, if stage encoding etc dont get applied on migrate that's not good! |
|
UA and Proxy are definitely oversights. The stage used for migration should be compressed and encrypted anyway, so stage encoding doesn't matter. |
|
After a bit of a look, I think this belongs in a method on the payload objects. Similar code (though not identical! copy-pasta for the lose) is in |
|
I'll start working on a proper solution. Will probably be a little while before I have something ready though. |
|
Does this seem reasonable? Want to make sure this passes the sniff test before moving forward with applying these methods to reverse_http and reverse_hop_http. |
|
Looks reasonable to me, the code isn't quite correct (patch_transport isn't in a method). Not sure if @jlee-r7 wanted it to sit on lib/msf/core/handler/reverse_http.rb but shouldn't require 'msf' from 'rex' etc. |
|
Doh! Thanks, Meatballs1. |
|
bump |
|
@Meatballs1 stage encoding is not needed, because the stage is transferred over the existing Meterpreter SSL channel (it isn't clear-text on the wire unless you use http). |
|
Thank you for the feedback, everyone! @Meatballs1 - Didn't see what you meant about the missing patch_transport method until I looked at the file on github, patch_transport was unclear because of indents; hence the "fixed indents" commit. Please let me know if there's something else I'm missing. Understood on not crossing the streams (including 'rex' on 'msf') so I'll hold off on that. Though I'm not sure how else metsrv patching could get refactored to accomodate client_core, reverse_http, and reverse_hop_http without crossing the streams. ...which gets me to thinking, does it make sense to refactor patching methods (as opposed to copy-paste) when they're only going to be included in a single file (client_core)? @hmoore-r7 - I was thinking of a decrypting web gateway which would still be in the middle of Meterpreter's SSL channel. As jlee-r7 pointed out, since stage is encrypted and compressed, no worries there. User-Agent with METERPRETER _UA would still be low hanging fruit for web traffic inspection though. |
|
@veritysr Absolutely, any other options we need to patch in should be preserved as well (timeouts, expiration, etc). There is a secondary effort to encode the contents of http stagers, beyond the transport level, and emulate actual web applications. |
|
As far as I can tell, all of the metsrv patching for rex/post/meterpreter/client_core.rb is refactored into rex/payloads/meterpreter/patch.rb with e55dab3. Can line 238 in rex/post/meterpreter/client_core.rb be removed? I see "conn_id" getting initialized but I don't see where "conn_id" is used. |
|
FYI, this may end up colliding with the proxy setting PR (#4295). Not a problem, but something to take into account. |
|
I think I ended up using extend on blob because I couldn't get include to work...tried including Rex::Payloads::Meterpreter::Patch on blob, migrate_stager, and c. I'll take another look at it and figure out what I was doing wrong. I took a look at #4295 - will also try to keep an eye out for when it gets landed to make sure this is compatible. |
|
Looks good! We might want to rename this to something other than Patch at some point depending on what else gets added (proxy host brackets for ipv6, etc), but this should be good to test/merge here. |
|
Looks like the following two stagers need to be updated to use this method as well:
Can you update these two stagers to require this mixin and use the patch methods? This should centralize all of the logic around metsrv patching once and for all. |
|
In the case of IPv6 addresses, the following logic would need to get implemented somewhere (both for the LHOST and the PROXY_HOST): |
|
Will do. Thanks
|
|
In the case of reverse_http.rb, Rex::Payloads_Meterpreter::Patch.patch_passive_service relies on the caller to send a properly formatted IPv6 host (payload_uri method in reverse_http.rb): Is this reasonable? And for reverse_hop_http.rb and IPv6, is it reasonable to expect that the user is specifying HOPURL with a correctly formatted host when using IPv6? |
|
The expectation is that the input is a IPv6 address, but since some applications and interfaces require brackets, we have to manually add them when appropriate (in this case, WinInet requires it for IPv6 addresses). The LHOST code you mentioned is an example of what we need to handle for PROXY_HOST as well within the patching code. |
|
@veritysr In retrospect, don't worry about IPv6, I can take a pass once this is landed and normalize it all then. |
|
@veritysr Thank you very much for this patch and working with us on the feedback and merge process! |
|
Always a pleasure working with the msf-dev community! :) Thanks!
|
Earlier today, I saw that my User-Agent was changing to "METERPRETER_UA" after running "migrate" with windows/meterpreter/reverse_http. It appeared that metsrv was not getting patched with MeterpreterUserAgent so I pulled the pertinent code (added the proxy stuff for completeness) from msf/core/handler/reverse_http.rb and dumped it here.