Skip to content
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

Feature request: Alias functionality (or generic: extensibility) for meterpreter shells. #5808

Open
henryk opened this issue Aug 4, 2015 · 18 comments

Comments

@henryk
Copy link

henryk commented Aug 4, 2015

In the current situation there are at least two distinct shells in metasploit: The metasploit shell and the meterpreter shell. The metasploit shell can load framework plugins (with load), the meterpreter shell can't. Well …

My end wish would be to have an alias command in both types of shells, but I'm completely unclear on how to achieve that and am looking for some guidance. These are the ways I could come up with:

  1. Easiest variant: Make the alias functionality a part of Rex::Ui::Text::DispatcherShell::CommandDispatcher because it's arguably as basic as help.
  2. Next easier variant: Give the meterpreter command dispatcher in Rex::Post::Meterpreter::Ui::Console::CommandDispatcher::Core the alias functionality (bonus question: extract Msf::Plugin::Alias::AliasCommandDispatcher so that both the original alias framework plugin and the new meterpreter alias command can use the same code).
  3. Slightly harder, more versatile: Add a generic meterpreter extension capability and provide an alias plugin for this. (Same bonus question as above.)
  4. Hardest, versatilest: Make it so that both the meterpreter and metasploit shell can use the same plugins, one of them being alias.

As a proof of concept and for internal use I have built a variant of method 3. The problem with the current meterpreter shell load command is that it will first try to load a dll into the target meterpreter before extending the shell. (Bonus: Discovering the code for this is somewhat hard, since the dlls for the existing meterpreter extensions are not in the metasploit git checkout.) My code is at https://github.com/henryk/metasploit-framework/compare/custom-commands but I have deliberately not opened a pull request. As is this code can't and shouldn't go upstream.
What I've done is:

  • Add an option -c to the meterpreter load command that will skip loading the meterpreter extension dll (and just load the associated command dispatcher), and
  • Add a meterpreter extension command dispatcher (sans dll) that is essentially a copy of the command dispatcher class from plugins/alias.rb, slightly edited to get it to work in the meterpreter environment. (I've tried to derive from AliasCommandDispatcher, but my ruby fu is very lacking and I couldn't get it to work.)

With these changes, it's possible to issue load -c alias in a meterpreter shell and behave as expected.
Cons: a) The separate -c option is very clumsy. b) Code duplication between AliasCommandDispatcher and my Alias extension.
Obviously I'd like this to be done right™, before proposing to pull something upstream.

@kernelsmith
Copy link
Contributor

@henryk. I wrote the original alias plugin so I'll take a look. I'm currently at DefCon/BlackHat so don't expect any real feedback (from me at least) until next week. I'm glad someone other than me uses it tho!

@OJ
Copy link
Contributor

OJ commented Aug 6, 2015

Hi @henryk,

The one thing I can comment on here is the suggestion for -c. The way the load functionality works in the Meterpreter shell is it first queries Meterpreter to determine if it already has the plugin DLL loaded. This was implemented recently to support both stageless sessions and transport resiliency. When sessions come in we don't know if they're new, old, or stageless. In the latter two cases, there might already be other plugins loaded, and so pushing them down to the target doesn't make sense.

If an existing session has, say, the extapi extension loaded and a transport switch happens, MSF closes the current session. The "new" connection that comes in is for a Meterpreter session that already has extapi loaded. When the use types use extapi, MSF asks Meterpreter for the functions that are already supported for that extension. If Meterpreter responds with a non-empty list, then MSF assumes that the plugin is already loaded and just wires in the client side.

So in short, the -c option is kind of already there!

Does that make sense?

Cheers!

@kernelsmith
Copy link
Contributor

@henryk one of the original reasons I made it into a console plugin was it's experimental & add-on nature at the time. It's definitely worth revisiting now.

other devs (@hmoore-r7, @wvu-r7, @todb-r7, @firefart, @zeroSteiner, @jlee-r7 et. al.) thoughts on moving alias functionality to lib/rex/ui/text/dispatcher_shell.rb? A quick investigation makes me think there are no technical barriers to doing so, however you guys may have objections/concerns. And, AFAIK, there's no good way to share functionality between msfconsole and the meterpreter console, tho I could definitely be wrong.

@kernelsmith
Copy link
Contributor

anyone? Bueller?

@hdm
Copy link
Contributor

hdm commented Aug 20, 2015

@kernelsmith How would saved aliases apply? There is no equivalent of the "save" command for the meterpreter shell.

@kernelsmith
Copy link
Contributor

aliases aren't really saved in the way that datastore options are saved when you hit save. I actually tried to add that one time, hit some snag, got distracted and never went back to it. Currently the only way to save aliases is in msfconsole.rc or similar.

@kernelsmith
Copy link
Contributor

for example, this is my stock msfconsole.rc: https://github.com/kernelsmith/env-customization/blob/master/msf/msfconsole.rc which I add to pretty much every metasploit install/dev environment that I have

@hdm
Copy link
Contributor

hdm commented Aug 20, 2015

Would having the same alias for both msfconsole and meterpreter sessions make sense? If not, implementing these in the Rex code may not make sense.

@henryk
Copy link
Author

henryk commented Aug 20, 2015

@hmoore-r7 In general, the same alias definition will not work in msfconsole and meterpreter, but doesn't need to be. The way I understand it, these are two separate objects, representing different classes, but derived from the same base class (where the alias code would go). That is, the alias definitions are instance variables and not shared between msfconsole and meterpreter. The alias definition syntax and associated code however could be shared without problem.

Like @kernelsmith said, the way to have the same aliases in all msfconsole sessions is with an rc file, and a similar thing can be done for meterpreter sessions with set AutoRunScript multi_console_command -rc ….

@kernelsmith
Copy link
Contributor

And I could have sworn that at one point there was a meterpreter.rc that would always get run on a new meterpreter session like msfconsole.rc does on console startup, but I must have dreamed that, tho it can still be accomplished w/AutoRunScript as @henryk points out

@kernelsmith
Copy link
Contributor

FYI, I removed the meterpreter label and added msfconsole as it doesn't really affect meterpreter directly.

@kernelsmith
Copy link
Contributor

@wvu-r7 @bcook-r7 Do you think this is still desired? I have some hospital time coming up in mid Feb and I might be able to take a look again

@wvu
Copy link
Contributor

wvu commented Jan 22, 2019

Go for it!

@ccondon-r7 ccondon-r7 added informative Info worthy usability Usability improvements labels Mar 4, 2020
@ccondon-r7
Copy link
Contributor

@kernelsmith I'm thisclose to adding the analysis paralysis! label, but I'm pumped you're looking at some of these super old but still good suggestions, so instead of adding another label I'm gonna tag in @zeroSteiner to give a definitive implementation opinion.

@zeroSteiner
Copy link
Contributor

zeroSteiner commented Mar 5, 2020

I like the option of moving it into Rex so the code base can be reused and shared between the two. I think the functionality is valuable enough that it makes sense to upgrade it's status from plugin to core functionality. That's # 2 with the bonus goal.

The load approach for making it an extension seems reasonable if we had a means to do it all in Ruby and mark it as not needing to load code meterpreter-side. However I think from a user perspective, they probably don't care about the shell (msfconsole vs meterpreter) they're in and if one has alias functionality they probably want the other too as well.

@kernelsmith
Copy link
Contributor

I forgot about this baby. @zeroSteiner yeah, not having it as an extension would reduce complexity. Tho, an ext would be client side only, but I don't remember if you can indicate that in the extension interface. I would have guessed you could. If it's in a lib, does Rex make sense? I dunno if Rex was ever unentangled, or is it still a hodgepodge and not really Ruby EXtensions. I'm assuming it was not unentangled so Rex is the only common lib location this thing could realistically reside.

@kernelsmith
Copy link
Contributor

Rex FTW! I take it Rex still hasn't been refactored to pull out non- "Ruby extension" code? Otherwise I would say console code probably shouldn't be in there. Thanks for weighing in @zeroSteiner . Taking design decisions from a Python guy just seems wrong ;)

@zeroSteiner
Copy link
Contributor

Yeah I'm pretty sure the state of Rex is still the same. Also wow... 😆

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants