-
Notifications
You must be signed in to change notification settings - Fork 13.7k
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
Capture plugin #16298
Capture plugin #16298
Conversation
Use normal capitalisation when showing service names to users.
Known issue: SMB and HTTP servers do not support different Comms channels (e.g. forwarding via Meterpreter or SSH), so these do not work; but this should be resolved in #16250 |
Open to the possibility that we set the |
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 left some comments with some suggestions based on my testing so far.
Co-authored-by: Spencer McIntyre <58950994+smcintyre-r7@users.noreply.github.com>
plugins/capture.rb
Outdated
|
||
mod = framework.modules.create(module_name) | ||
# Bail if we couldn't | ||
unless mod |
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.
There's a pattern for extracting module load errors that we follow over here
metasploit-framework/lib/msf/ui/console/command_dispatcher/modules.rb
Lines 718 to 740 in d6ff12f
unless mod | |
# Checks to see if we have any load_errors for the current module. | |
# and if so, returns them to the user. | |
load_error = framework.modules.load_error_by_name(mod_name) | |
if load_error | |
print_error("Failed to load module: #{load_error}") | |
return false | |
end | |
unless mod_resolved | |
elog("Module #{mod_name} not found, and no loading errors found. If you're using a custom module" \ | |
' refer to our wiki: https://github.com/rapid7/metasploit-framework/wiki/Running-Private-Modules') | |
# Avoid trying to use the search result if it exactly matches | |
# the module we were trying to load. The module cannot be | |
# loaded and searching isn't going to change that. | |
mods_found = cmd_search('-I', '-u', *args) | |
end | |
unless mods_found | |
print_error("Failed to load module: #{mod_name}") | |
return false | |
end | |
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.
Thanks. I guess in that case, it's doing some searching to try to figure out what the user wants; in this case, we're explicitly specifying the exact module, and don't want it to go searching. So I'd imagine that most of that we don't want to use; but I can use the load_error_by_name
method to give a better error?
Thanks for your pull request! Before this pull request can be merged, it must pass the checks of our automated linting tools. We use Rubocop and msftidy to ensure the quality of our code. This can be ran from the root directory of Metasploit:
You can automate most of these changes with the
Please update your branch after these have been made, and reach out if you have any problems. |
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.
Some thoughts from a group review.
… exist. Fixes some bugs relating to pivoting through a session
ntlm_challenge: "1122334455667788" | ||
ntlm_domain: anonymous | ||
http_basic: no | ||
ssl_cert: null |
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.
Does it makes sense to make these values belong to the HTTP
or HTTPS
services? Is there a reason they should be global per config file?
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.
If moved into per service consider all hardcoded defaults could move into config and the configure_*
methods could be consolidated to iterate the proposed datastore
map in a meta-programing format similar to:
def configure_module(datastore, config)
config[:datastore].each do |option|
datastore[option.to_s] = config[:datastore][option]
end
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.
There's no particular reason they should be global; just an artifact of the earlier request to have the command line options defaults be pulled from a file. That said, even if we did it per-service, I think having an overall default makes sense, to provide a default in its absence.
I can see how this suggested approach would be much more flexible; but I'm weighing up flexibility vs usability in my mind. Given a fixed NTLM challenge is used for rainbow tables, would anyone want to provide a different value for each different service? Would there be an opsec use case for different NTLM domains for each service (I'd think someone would probably want the same for all of them)? Is providing a different TLS cert for each service a valuable thing? (I'd think that 99% of the time, we just want them to be the same).
To me, this feature is most useful because it packages everything up neatly for the common use case, and it's easy for someone to understand how to use it. I should clarify: I'm happy to make that change if you think it's important; just presenting my view as an operator.
If we did want to go ahead, though, the question to answer is: how do these settings interact with the individual command line options? Would providing a command line option for SSLCert override all of the individual datastore settings in the config file?
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.
This is sound reasoning to me. I think having the a default in place is good, customizing the services down the line can be another iteration.
The ntlm_challenge
value seems like a fingerprintable
item that may be worth thinking more on in the future. As things iterate I could see http_basic
for a configured service being something a user might want to very and fallback to the global default for some services.
Shifting hardcoded
port values and such currently in the configure_*
methods may help users understand what is running at a glance in the config instead of having to view the plugin source. That also can be deferred as an enhancement.
Multiple modules provide a "Capture" action that would collide with this name. Rename it to `captureg` for Capture-Global.
Release NotesThis adds the new "capture" plugin which can be used to easily start and stop credential-capturing services. |
This introduces a new plugin,
capture
, which activates a range of spoof and capture modules, in an attempt to capture credentials from other systems on the network. It provides no new modules in and of itself, but rather a way to orchestrate them together in a more cohesive way.This can be used to run modules on remote sessions, although currently UDP services (including all the active spoofing ones) are not supported remotely.
To use this feature, load the plugin with
load capture
and then run it withcapture start
, and the settings below:To stop a capture, run
capture stop
Other settings are controlled by a config file; a sample one (the default one used) is found in the metasploit directory, in the
data/capture_config.yaml
file. This includes the ability to disable certain services, set a specific NTLM challenge and domain name.