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

Support RHOSTS for exploit modules #9246

Merged
merged 11 commits into from May 16, 2018

Conversation

Projects
None yet
10 participants
@busterb
Copy link
Contributor

busterb commented Nov 28, 2017

This adds initial support for RHOSTS with exploit modules. What this does is:

  • Make RHOST and RHOSTS aliases for each other, with RHOSTS being the preferred spelling (both work still)
  • Factor out exploit handling from command-line dispatch
  • Add a rangewalker to modulate RHOST on the module datastore
  • Add a '-J' option which is the inverse of '-j', that is it makes something not run as a job
  • Work just like that resource script you probably have for doing this already

This doesn't:

  • Try to handle multiple architectures, targets, or payloads
  • Try to handle multiple listeners on a single port (though as long as your exploits aren't passive, it seems to work out anyway)
  • Anything other than make RHOSTS iterate over a range and plug it into an exploit module, running it once for every host in RHOSTS.

There are lots of things we can do on top of this to make it even better, so looking forward to feedback and improvements..

msf exploit(ms08_067_netapi) > set RHOSTS 127.0.0.0/24
RHOSTS => 127.0.0.0/24
msf exploit(ms08_067_netapi) > run

[*] Started reverse TCP handler on 192.168.56.1:4444 
[-] 127.0.0.0:445 - Exploit failed [unreachable]: Rex::HostUnreachable The host (127.0.0.0:445) was unreachable.
[*] Started reverse TCP handler on 192.168.56.1:4444 
[-] 127.0.0.1:445 - Exploit failed [unreachable]: Rex::ConnectionRefused The connection was refused by the remote host (127.0.0.1:445).
[*] Started reverse TCP handler on 192.168.56.1:4444 
[-] 127.0.0.2:445 - Exploit failed [unreachable]: Rex::ConnectionRefused The connection was refused by the remote host (127.0.0.2:445).
[*] Started reverse TCP handler on 192.168.56.1:4444 
[-] 127.0.0.3:445 - Exploit failed [unreachable]: Rex::ConnectionRefused The connection was refused by the remote host (127.0.0.3:445).
[*] Started reverse TCP handler on 192.168.56.1:4444 
[-] 127.0.0.4:445 - Exploit failed [unreachable]: Rex::ConnectionRefused The connection was refused by the remote host (127.0.0.4:445).
[*] Started reverse TCP handler on 192.168.56.1:4444 
[-] 127.0.0.5:445 - Exploit failed [unreachable]: Rex::ConnectionRefused The connection was refused by the remote host (127.0.0.5:445).
[*] Started reverse TCP handler on 192.168.56.1:4444
...
@sempervictus

This comment has been minimized.

Copy link
Contributor

sempervictus commented Nov 29, 2017

I will get this tested. My first instinct is to say that there be dragons here as we have tried this before and if memory serves things went south. That said, will review and do all I can to help it land, this has been much needed for a long time (PSH wmi for one already does this sort of madness).
Ping @kernelsmith and @egypt.

@busterb

This comment has been minimized.

Copy link
Contributor

busterb commented Nov 29, 2017

I've had a positive reactions from a few pen testers who are already using this. I've also been looking closely at how 'check' does this and will be following this up with some refactors so they both work in a similar way.

@vipzen

This comment has been minimized.

Copy link
Contributor

vipzen commented Nov 30, 2017

Very handy! Reading RHOSTS from a file does not work yet, right?

@egypt

This comment has been minimized.

Copy link
Contributor

egypt commented Nov 30, 2017

I'm not fundamentally opposed to the idea of this, but as @sempervictus mentioned there are dragons. I've discussed this with @hdm, @busterb, @wvu, and many others over the years. The biggest difficulty is the difference in victims as regards to payload compatibility. Theoretically, different Targets can have different size constraints but in practice there are zero exploits that actually do.

Exploits targeting multiple operating systems and multiple architectures are a bigger concern. In practice, it's less of an issue because when you find a group of vulnerable boxes, they're quite often all the same, but we still need to be careful about e.g. sending x64 shellcode to an x86 process.

@jhart-r7 jhart-r7 referenced this pull request Dec 20, 2017

Merged

Cambium modules #9316

@sempervictus

This comment has been minimized.

Copy link
Contributor

sempervictus commented Dec 29, 2017

So, about them dragons - this breaks Aux scanners pretty badly. If you pry into http/version scanner and run_host(ip) you're ok, but hitting cmd_run doesnt seem to get there. Think i'mma need to revert this till i can devote some time to digging around in the subtleties of how Aux and Exp execute their runners.

@busterb

This comment has been minimized.

Copy link
Contributor

busterb commented Jan 9, 2018

Yes, aux scanner modules use the same trick as this PR. To complete this, we need to move the differentiated logic from the different module types and have only one cook in the kitchen slinging the RHOSTS values one by one into the module datastores.

@@ -42,7 +42,7 @@ def self.Proxies(default=nil, required=false, desc="A proxy chain of format type

# @return [OptAddress]
def self.RHOST(default=nil, required=true, desc="The target address")
Msf::OptAddress.new(__method__.to_s, [ required, desc, default ])
Msf::OptAddressRange.new('RHOSTS', [ required, desc, default ], aliases: [ 'RHOST' ])

This comment has been minimized.

@bwatters-r7

bwatters-r7 Jan 12, 2018

Contributor

FYI, this line is what is killing scanners, I believe. Specifically, the RHOST alias. If you remove the alias, then aux scanners appear to work (or at least have RHOSTS values). My best guess right now is that when we added redundant code to tear out RHOST in AUX to prevent confusion, this alias causes the logic that tore out RHOST to tear out RHOSTS, too.

This comment has been minimized.

@bwatters-r7

bwatters-r7 Jan 12, 2018

Contributor

Gah....... I saw this earlier and thought I checked it. Yeah; the changes in #7127 likely delete the RHOSTS from the datastore since it is aliased. Removing the setup/cleanup functions makes the smb1 scanner and 08_067 appear to work like a champ.

This comment has been minimized.

@busterb

busterb Jan 12, 2018

Contributor

I thought I had mentioned this earlier. We can't do this though, because I want to kill RHOST as a user-visible option.

What needs to happen is the code in the 'run' method in lib/msf/core/auxiliary/scanner.rb needs to mostly be merged with the code in at this PR adds, or at least modified so it's using the datastore clone trick that we're using here.

This comment has been minimized.

@busterb

busterb Jan 12, 2018

Contributor

Whatever the common parent of Msf::Exploit and Msf::Auxiliary is, I think we can make a single run method that knows how to run them all in this mode. The main difference I think would be around whether parallelism is allowed or not, with the different module types.

This comment has been minimized.

@busterb

busterb Jan 12, 2018

Contributor
  # RHOST should not be used in scanner modules, only RHOSTS
  deregister_options('RHOST')

this is only one place where we do it (then it proceeds to use it as a temp variable)

rhosts = mod.datastore['RHOSTS']
if rhosts
Rex::Socket::RangeWalker.new(rhosts).each do |rhost|
nmod = mod.replicant

This comment has been minimized.

@busterb

busterb Jan 12, 2018

Contributor

the way this patch works and avoids clobbering the global RHOSTS value when it sets RHOSTS per-run to a single value is to call 'replicant', which makes a fully copy. This is opposed to aux scanner which assumes that RHOST and RHOSTS coexist. But to hide that sausage-making in the background, we have dozens of unregister RHOST calls in aux scanners, that we could remove after this.

@bwatters-r7 bwatters-r7 force-pushed the busterb:rhosts branch from 93ad572 to f5d1dd3 Jan 12, 2018

@bwatters-r7

This comment has been minimized.

Copy link
Contributor

bwatters-r7 commented Jan 18, 2018

The above push fixes scanners (to my knowledge). It turns out that we 'fixed' problems we had with RHOST earlier by simply deleting it in the setup function for aux/scanner. This worked great, but to avoid confusion and support legacy stuff, this PR aliased RHOST and RHOSTS. Unfortunately while that was an awesome idea for exploits, it meant that when you launched a scanner module, the first step in the setup process was deleting all your targets. 😢 I removed the setup/cleanup functions and some other stuff related to RHOST as an independent concept, and it seems to work (at least with SMBv1 scanner.

@bwatters-r7

This comment has been minimized.

Copy link
Contributor

bwatters-r7 commented Jan 24, 2018

Are there any more dragons on this PR? Should we look to merge this now?

@hdm

This comment has been minimized.

Copy link
Contributor

hdm commented Jan 24, 2018

Definitely gremlins in here, for example: obj = self.clone replacing obj = obj.class.new is NOT the same thing and will have disastrous downstream effects on all auxiliary scanners (although subtle). Clone only clones the top-level object, it copies references below it IIRC. This seems like a dangerous PR to merge without extensive testing.

@hdm

This comment has been minimized.

Copy link
Contributor

hdm commented Jan 24, 2018

I am pretty sure replicant is used in other contexts where this can go wrong (maybe MSPro?).

@sempervictus

This comment has been minimized.

Copy link
Contributor

sempervictus commented Jan 24, 2018

My vote is "do not merge, will eat your pets." In testing it breaks aux all over the place.

loop do
if @range_count > 0

This comment has been minimized.

@acammack-r7

acammack-r7 Mar 12, 2018

Contributor

nop?

@busterb busterb force-pushed the busterb:rhosts branch from f5d1dd3 to 0d51ba8 Mar 12, 2018

@busterb busterb added the msf5 label Mar 12, 2018

@busterb

This comment has been minimized.

Copy link
Contributor

busterb commented Mar 12, 2018

@sempervictus did you test 0d51ba8 or was that prior?

@busterb

This comment has been minimized.

Copy link
Contributor

busterb commented Mar 12, 2018

We are not concerned about preserving MSPro compatibility with this change, by the way. I haven't stumbled into a way to break an aux module yet, but will keep trying. In the mean time, I'll look into cleaning up some aux code so it's actually shared here.

@sempervictus

This comment has been minimized.

Copy link
Contributor

sempervictus commented Mar 12, 2018

Prior, I had to back it out a while ago. Will re-merge on the next window.

@jmartin-r7

This comment has been minimized.

Copy link
Contributor

jmartin-r7 commented Mar 13, 2018

Jenkins test this please.

@sky305 sky305 referenced this pull request Mar 19, 2018

Closed

RHOST Solution #99

@busterb busterb referenced this pull request Mar 19, 2018

Closed

RHOST and RHOSTS #9724

@sky305

This comment has been minimized.

Copy link

sky305 commented Mar 19, 2018

Do we have any status if this code is up to par ?

@sempervictus

This comment has been minimized.

Copy link
Contributor

sempervictus commented Mar 19, 2018

Do we have any status if this code is up to par ?

Up to par with what? If you're asking whether or not it works without breaking everything in sight, it seems to, on my end anyway (though merge conflicting a decent chunk of my altered command_dispatcher/exploit, which i imagine wont be a problem for everyone else given Jenkins' approval).

@busterb: it looks like its working so far, aux scanners and exploits both execute, threading appears to work. Will keep using it during sniffer & dns testing.

@bwatters-r7 bwatters-r7 self-assigned this May 7, 2018

@bwatters-r7

This comment has been minimized.

Copy link
Contributor

bwatters-r7 commented May 7, 2018

Alright; I'm going to restart testing this and land this. If there are any last concerns, voice them.

@bwatters-r7

This comment has been minimized.

Copy link
Contributor

bwatters-r7 commented May 16, 2018

Well, this is a bit of an irritant, because as it is, with most of our exploits, this stops after the first successful exploit. I wish it were a simple matter to enable something like ExitOnSession, though that's a lower-level exploit option. I think that the ability to automatically background should be available on the payload/handler side, and I explored that and got a little close before metasploit threading monsters started launching about 40 handlers for every exploit. Honestly, while irritating, there's no reason not to land this, even if it not as usable as I'd like. I'd welcome other ideas for making this more useful to automate large IP ranges, but that's not something that I think should limit its landing.
Some possible solutions while I'm thinking on this
Optimal: allow the payloads and handlers to become a bit more independent. The multi/handler can now do a lot of negotiating itself because of UUIDs, and supporting the exploitation component somewhat asynchronously would be hella' useful for hackers. Start a handler, then just fire off some exploits. See what comes back. I tried to do this using browser autopwn2 as a guide. I'm not sold that it is impossible, but it took longer than 2 days so I moved to something else. If nothing comes, I'll likely return to it.
Alternative: Enable auto-back-grounding of sessions immediately, possibly leveraging on_connect or something similar.

Regardless, this is still useful, and it is increasingly falling behind master, so I'm just going to merge it and wait for the sound of shattering glass. We can address increasing usefulness of the feature later.

@acammack-r7

This comment has been minimized.

Copy link
Contributor

acammack-r7 commented May 16, 2018

Each one of those handler threads has a database connection (thanks, ActiveRecord!), so that might lead to some problems with resource starvation if you can't control how many spin up. May not impact if we want to land it, but we should be aware.

@bwatters-r7

This comment has been minimized.

Copy link
Contributor

bwatters-r7 commented May 16, 2018

I had not planned on adding the changes to the handlers until I can make them not explode, and I might as well do that in another PR, anyway.

@kernelsmith

This comment has been minimized.

Copy link
Contributor

kernelsmith commented May 16, 2018

@bwatters-r7 isn't there a callback when a session is created. Could that be leveraged? The callback could be modified to send the uuid (if it doesn't already). I'm not sure that is enough, along with the session information, to make useful determinations in this situation, but thought I would throw it out there. Admittedly, I don't %100 grok the full situation here.

On a related note, I've always dreamed of the ability to have a callback from multi/handler that could identify the exploit used to get the session. I realize that's unrealistic as you'd somehow have to relay uniquely identifiable info, but man it'd be sweet for tracking.

@bwatters-r7 bwatters-r7 merged commit 588993f into rapid7:master May 16, 2018

3 checks passed

Metasploit Automation - Sanity Test Execution Successfully ran sanity checks.
Details
Metasploit Automation - Test Execution Successfully ran `autoPayloadTest.py`.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

bwatters-r7 added a commit that referenced this pull request May 16, 2018

@bwatters-r7

This comment has been minimized.

Copy link
Contributor

bwatters-r7 commented May 16, 2018

@kernelsmith I do not entirely grok it myself, so absolutely happy to take advice!
If I understand it, many (maybe all?) of the UUID payloads already send that UUID in the first stage (if staged) so that the multi/handler can tell what stage needs to be sent to complete the session. Not all payloads support that, which was the catch for trying to add an option to use the multi/handler as a universal handler. It was a nice tie-in here, as the multi/handler already supported the 'ExitOnSession' setting, so it would have added features and solved a problem simultaneously.

I'm not sure that leveraging the callback would work, though I've been embarrassingly wrong before on this stuff, so I' not going to say no. Honestly, in the darkest night, I even thought about making a silly, kludgey script that just ran 'background' as the autorun script. Likely, that would work, though I really like the idea of supporting a universal handler, as I can see some benefits as the payloads get smarter, but at least my attempts over the last 2 days suggested it is non-trivial (for me, at least).

@bwatters-r7

This comment has been minimized.

Copy link
Contributor

bwatters-r7 commented May 16, 2018

Release Notes

This PR allows the user to specify an IP range when using remote exploits.

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