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

Conditionally recalculate in ModuleSet #16368

Merged

Conversation

zeroSteiner
Copy link
Contributor

@zeroSteiner zeroSteiner commented Mar 21, 2022

This updates the Msf::ModuleSet#create method to skip calling recalculate when nothing was loaded from the module cache and the module set is populated. If nothing was loaded, then it doesn't seem necessary to perform a recalculation. Recalculating payloads is computationally expensive and thus time consuming. Since this code is executed when an unknown command is run from msfconsole, recalculating the payloads slows down the response. This leads to an unpleasant user experience. When a module was loaded however it makes sense to perform the recalculation. The call to load_cached_module will return false when the referenced name does not exist and nothing was loaded.

This regression was introduced in #16186 because of the additional loop required by the adapters.

Verification

List the steps needed to make sure this thing works

  • Start msfconsole
  • Run repeat -n 3 time missing_command
  • See much better times

Example Output

Before the patch

msf6 exploit(multi/http/apache_apisix_api_default_token_rce) > repeat -n 3 time missing_command
[-] Unknown command: missing_command
[+] Command "missing_command" completed in 1.0647388609977497 seconds
[-] Unknown command: missing_command
[+] Command "missing_command" completed in 1.0240395100008755 seconds
[-] Unknown command: missing_command
[+] Command "missing_command" completed in 1.2335878140002023 seconds
msf6 exploit(multi/http/apache_apisix_api_default_token_rce) >

After the patch

msf6 exploit(multi/http/apache_apisix_api_default_token_rce) > repeat -n 3 time missing_command
[-] Unknown command: missing_command
[+] Command "missing_command" completed in 0.006417524000426056 seconds
[-] Unknown command: missing_command
[+] Command "missing_command" completed in 0.006295037001109449 seconds
[-] Unknown command: missing_command
[+] Command "missing_command" completed in 0.004452250002941582 seconds
msf6 exploit(multi/http/apache_apisix_api_default_token_rce) >

Recalculations are time consuming, so only do it if necessary.
@zeroSteiner zeroSteiner force-pushed the fix/missing-command-performance branch from 28d3a5c to df04435 Compare March 21, 2022 21:01
@zeroSteiner zeroSteiner changed the title Only recalculate when something was loaded Conditionally recalculate in ModuleSet Mar 21, 2022
Copy link
Contributor

@jmartin-tech jmartin-tech left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes sense to me calling recalculate only if there is a possibility something is missing is a nice optimization!

Also verified this works with ./tools/automation/cache/update_module_cache.sh when automation needs to build the cache from scratch.

@jmartin-tech
Copy link
Contributor

Improvement verified:

msf6 > repeat -n 3 time missing_command
[-] Unknown command: missing_command
[+] Command "missing_command" completed in 1.10120000000461 seconds
[-] Unknown command: missing_command
[+] Command "missing_command" completed in 1.0377640000078827 seconds
[-] Unknown command: missing_command
[+] Command "missing_command" completed in 1.0391410000011092 seconds

Post merge:

msf6 > repeat -n 3 time missing_command
[-] Unknown command: missing_command
[+] Command "missing_command" completed in 0.0073450000054435804 seconds
[-] Unknown command: missing_command
[+] Command "missing_command" completed in 0.009362000011606142 seconds
[-] Unknown command: missing_command
[+] Command "missing_command" completed in 0.00660500000230968 seconds

@jmartin-tech jmartin-tech merged commit 122d380 into rapid7:master Mar 21, 2022
@jmartin-tech
Copy link
Contributor

Release notes

This improves response time when a cache miss occurs for commands not provided by msfconsole.

@@ -154,6 +154,7 @@ def load_and_create_module(options={})
)

module_set = module_set_for_type(module_type)
module_set.recalculate
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if there's a way of performing this action once at a higher level within the test setup? Otherwise, if I understand correctly, we'll be adding an extra 1-2 seconds per payload test which calls load_and_create_module? 👀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug library rn-fix release notes fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants