Replace METHOD string with COMMAND_ID integer (to remove obvious strings) #13395
Conversation
|
Flattery will get you everywhere @sempervictus ! |
|
BTW I will fix up the payload cache sizes when we've got the payloads PRs landed. |
|
Ah need to fix up the specs. |
|
Technically, that's where it started. This was a complete rewrite though 🚑 |
|
Everyone stop pretending like they know what's going on in this PR and just merge it. Worst case we revert it and send OJ a glitter bomb |
|
I've not looked too deeply, but it seems like sanity tests should fail because we've changed the dispatching system, but sanity tests have passed? |
|
Local verification run of sanity test shows the expected failures due to these transport changes. I am investigating why automation did not flag as expected. Original It would be nice to at least allow This PR may reflect a significant enough change in the how Metasploit communicates to call for a version bump to |
|
I do remember that discussion. Keeping both wasn't totally out of the question, but boy was the code messy. It just didn't feel like the right thing to do. Thing is, as far as breaking back compat goes this is no worse than the refactor/removal of delay loading, though I'll admit that was just on the Meterp side. Obviously it's not my call, but I think this has to be done and we just need to go through the pain of dealing with the changes and making sure that people don't upgrade mid-engagement. |
|
@msjenkins-r7 test this please. Sanity test should correctly report failure now, once dependent gems are released and spec/lock are bumped in this PR they should pass again. |
|
Thanks @jmartin-r7 ! |
|
Picked up on a few issues so please refrain from landing at this point :) FIXIFICATING! |
|
OK back on track. |
|
@msjenkins-r7 test this please. |
1 similar comment
|
@msjenkins-r7 test this please. |
|
Hooray! It is failing as expected, now. |
lib/rex/post/meterpreter/ui/console/command_dispatcher/android.rb
Outdated
Show resolved
Hide resolved
Not sure why, but this is causing issues. Gross.
This was not originally ported to an int when it should have been.
OJ
force-pushed
the
remove-tlv-command-strings
branch
from
ada53bc to
5b69fe9
Compare
|
Sometimes that's how the ball bounces @kernelsmith! We'll get this licked yet. |
|
Automation result here is failing reverse connections, running tests in another lab to validate results. I suspect gremlins I need to clear out of the infrastructure. Separate lab is passing tests: This represents passing sanity test. I will investigate automation side. |
|
Thank you @jmartin-r7 ! |
|
Agreed @busterb. To be clear, I was only referring to OJ's last commit as being anticlimactic, not the precess. The process is awesome |
|
@msjenkins-r7 test this please. |
There was a problem hiding this comment.
Tested this with the Python, PHP, Java, Linux x86 and Windows x64 Meterpreters. While doing so, I was specifically attentive towards channels by verifying TCP client and file channels. TCP client channels were verified using using a few techniques like the connect command, pivoting modules and the SOCKS5 server. File channels were verified using the post/test/file module and checking for errors.
I'll have this merged in soon.
|
Landed after a minor bug fix for the
|
|
Thanks everyone!
I'll make sure the other PRs are lined up and ready to go.
…On Thu, 11 Jun 2020, 06:43 Spencer McIntyre, ***@***.***> wrote:
Landed after a minor bug fix for the powershell_session_remove command in
commit 6ca3368
<6ca3368>
.
powershell_session_remove test results:
[*] Meterpreter session 1 opened (192.168.159.128:4444 -> 192.168.159.31:49172) at 2020-06-10 16:37:57 -0400
meterpreter > getuid
Server username: NT AUTHORITY\SYSTEM
meterpreter > sysinfo
Computer : WIN-9NSI4A6AIHJ
OS : Windows 7 (6.1 Build 7601, Service Pack 1).
Architecture : x64
System Language : en_US
Domain : WORKGROUP
Logged On Users : 1
Meterpreter : x64/windows
meterpreter > load powershell
Loading extension powershell...Success.
meterpreter > powershell_shell -s test
PS > $msg='Hello World'
PS > ^Z
Background channel 1? [y/N] y
meterpreter > powershell_shell -s test
PS > Write-Host $msg
Hello World
PS > ^Z
Background channel 2? [y/N] y
meterpreter > powershell_session_remove -h
Usage: powershell_session_remove -s session-id
Removes a named session from the powershell instance.
OPTIONS:
-h Help banner
-s <opt> Specify the id/name of the Powershell session to interact with (cannot be "default").
meterpreter > powershell_session_remove -s test
[+] Session 'test' removed.
meterpreter > powershell_shell -s test
PS > Write-Host $msg
PS >
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#13395 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAHBYFE7TVHRWM4IZD43ULRV7V5NANCNFSM4MZFYOVA>
.
|
Release NotesUpdated the TLV protocol used by Meterpreter to use numeric identifiers for commands instead of string values. This makes the traffic (and certain generated binaries) less conspicuous by removing strings that are obviously associated with Meterpreter such as |
|
|
Intro
Associated PRs
Description
This may be another longer story. I'm sorry, but detail is important.
Among the many goals we have right now, one of them is to make Meterp less obvious. That is, as a binary on disk/in memory, and with behaviour both on and off the wire.
Meterp is full of "commands". When operators tell
meterpreter >to do something, behind the scenes TLV packets are created that contain a "method", which basically represents the identifier of the command the packet represents. That identifier was a string, and not just any string, a string that stands out from the crowd.For those that haven't seen them, examples include
stdapi_fs_file_copy,core_migrateandmimikatz_custom_command.While these don't matter so much on the wire once TLV packets are encrypted, they do matter in other scenarios, such as strings embedded in binaries. The presence of anything
stdapi_as a string tends to indicate bad things as far as AV is concerned.The primary goal of this PR is to change
TLV_TYPE_METHOD(a string) toTLV_TYPE_COMMAND_ID(an integer). This means that the obvious strings are removed from binaries and aren't present on the wire any more. This should have some impact on the obviousness of the binaries that we're producing.This does mean that the ABI has been broken again, sorry about that. While doing this work, I was forced to make a bunch of other changes and did some other tidying up as well. But on the whole this PR is summed up as "convert a string to an int".
Changes
Hopefully I haven't missed anything in this list:
TLV_TYPE_METHODno longer exists, and has been replaced byTLV_TYPE_COMMAND_IDwhich is an integer.EXTENSION_ID_*. This serves a few purposes:$REASONSmust be a multiple of1000.1000number range (as no extension will have more than1000commands).ExtensionMapperthat is used by the configuration generator and the extension loader. This thing has the responsibility of getting hold of extension classes, as well as being able to provide the identity of an extension based on its name. You can give it"stdapi"and it'll give you1000for example.EXTINITfunctionality. Given that we don't have string identifiers for the extensions any more, we now us the ID of the extension as an integer. This means that stageless payloads look a little different too.networkpugwas still a thing. I removed it after talking to @bcook-r7 about it, because the old linux Meterpreter (which no longer exists) was the only thing that used it. Bye bye networkpug.Verification
This may be a little painful, and for that I am sorry. ALL of the Meterps have changed, so full regression testing of stuff will be required. I'm sure I have covered most of the cases, but there's a good chance I've missed something. Please help me make sure I haven't made any mistakes.
mettleandmetasploit-paylodas, compile/build/etc and have them all stashed in the appropriate folders under$MSF/data.Things left to do
Thanks to Channels having a string-based identifier, there's still some evidence of stuff like
stdapi_in the code. Once this PR has been landed, I'll look to fix that up as well. I didn't want it to include that in this PR given it's already so big.Thanks
Cheers as always to Brent and Caitlin for the support. Thanks to Tim for his help getting more going with the Android stuff. Thanks to Spencer for his original work on the Python side that I stole while doing this.
Attention
Given the depth/breadth of these changes, I'm going to need lots of eyes. There are many of you out there that can help but specifically I need the following folks for their expertise in certain areas: @bcook-r7 @smcintyre-r7 @jmartin-r7 @timwr - changes in MSF might not be that interesting to all of you, but reviews of the stuff I've done in the payloads/mettle PRs would be greatly appreciated. DO YOUR WORST!