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

Refactor Msf::Exploit::Deprecated and assorted fixes #12223

Merged
merged 8 commits into from Aug 30, 2019

Conversation

@acammack-r7
Copy link
Contributor

commented Aug 22, 2019

Adds a generic way of adding error and warning messages to modules to be displayed to users in Msf::Module::Alert. Used twice in this PR, hopefully we can standardize some other error messages (like external module incompatibilities) to this mechanism.

Module reloads also now much faster as we avoid the slow module instance creation case of not providing a type to the module manager.

Fixes #12026
Supersedes #12027

Verification

New deprecation method:

  • ./msfconsole
  • use exploit/linux/http/ubiquiti_airos_file_upload
  • Verify the module loads without error
  • use exploit/linux/ssh/ubiquiti_airos_file_upload
  • Verify the module loads and is exploit/linux/http/ubiquiti_airos_file_upload (but is still called exploit/linux/ssh/ubiquiti_airos_file_upload in the prompt) and displays the deprecation message

Alias reload fix:

  • reload
  • Verify the module reloads as exploit/linux/ssh/ubiquiti_airos_file_upload
  • edit the module and remove the moved_from call
  • reload
  • Verify the module reloads as exploit/linux/http/ubiquiti_airos_file_upload and prints a warning
  • use exploit/linux/ssh/ubiquiti_airos_file_upload
  • Verify the alias no longer works
  • use exploit/linux/http/ubiquiti_airos_file_upload
  • Verify the module loads and does not display a warning
@acammack-r7

This comment has been minimized.

Copy link
Contributor Author

commented Aug 22, 2019

Looks like some of the tests relied on the setup jankiness, I'll address those.

@wvu-r7
wvu-r7 approved these changes Aug 22, 2019
Copy link
Contributor

left a comment

These look like good changes. I'll give it a test!

@wvu-r7 wvu-r7 self-assigned this Aug 22, 2019

@wvu-r7

This comment has been minimized.

Copy link
Contributor

commented Aug 22, 2019

Reminder to self: update #12133 when this lands.

@wvu-r7

This comment has been minimized.

Copy link
Contributor

commented Aug 22, 2019

@acammack-r7: Travis failures may be related. Already being handled.

@wvu-r7

This comment has been minimized.

Copy link
Contributor

commented Aug 22, 2019

Yeah, the setup/super stuff is a little janky. Let's make sure it's ironclad before this goes in.

@acammack-r7

This comment has been minimized.

Copy link
Contributor Author

commented Aug 22, 2019

If I can't get it figured out today I'll just revert it and tackle it later. From the standpoint of the PR it only means that some modules will have one less chance of displaying alerts to the user.

wvu-r7 and others added 6 commits Jun 28, 2019
Rewrite Msf::Module::Deprecated w/ alerts, aliases
Less janky, and allows module moves without copying!
Use alias when reloading aliased module
Also speeds up module reloads by avoiding module instance creating
without a type.

Fixes #12026

@acammack-r7 acammack-r7 force-pushed the acammack-r7:deprecated-refactor branch from 51a102f to 8aa00d9 Aug 22, 2019

@acammack-r7

This comment has been minimized.

Copy link
Contributor Author

commented Aug 22, 2019

@wvu-r7 The super change ended up being larger than what I wanted to take on so I left it out when I rebased to get some other specs playing nice.

@wvu-r7

This comment has been minimized.

Copy link
Contributor

commented Aug 22, 2019

All good, better we leave it out for now if it's causing issues.

@wvu-r7

This comment has been minimized.

Copy link
Contributor

commented Aug 23, 2019

@acammack-r7: Oh boy. Where I left off in the verification steps:

msf5 exploit(linux/http/ubiquiti_airos_file_upload) > use exploit/linux/ssh/ubiquiti_airos_file_upload
[-] Error while running command use: stack level too deep
msf5 exploit(linux/http/ubiquiti_airos_file_upload) >
@acammack-r7

This comment has been minimized.

Copy link
Contributor Author

commented Aug 23, 2019

Hmm, that would be possible if the alias table isn't getting reset right, though on my box I cannot reproduce:

msf5 exploit(linux/ssh/ubiquiti_airos_file_upload) > reload
[*] Reloading module...

[!] Alias exploit/linux/ssh/ubiquiti_airos_file_upload no longer available after reloading, using exploit/linux/http/ubiquiti_airos_file_upload
msf5 exploit(linux/http/ubiquiti_airos_file_upload) > use exploit/linux/ssh/ubiquiti_airos_file_upload
[-] No results from search
[-] Failed to load module: exploit/linux/ssh/ubiquiti_airos_file_upload
@wvu-r7

This comment has been minimized.

Copy link
Contributor

commented Aug 26, 2019

msf5 > use exploit/linux/http/ubiquiti_airos_file_upload
msf5 exploit(linux/http/ubiquiti_airos_file_upload) > use exploit/linux/ssh/ubiquiti_airos_file_upload

[!] *        The module exploit/linux/ssh/ubiquiti_airos_file_upload has been moved!         *
[!] *            You are now using exploit/linux/http/ubiquiti_airos_file_upload             *
msf5 exploit(linux/ssh/ubiquiti_airos_file_upload) > reload
[*] Reloading module...

[!] *        The module exploit/linux/ssh/ubiquiti_airos_file_upload has been moved!         *
[!] *            You are now using exploit/linux/http/ubiquiti_airos_file_upload             *
msf5 exploit(linux/ssh/ubiquiti_airos_file_upload) > edit
[*] Launching vim -i NONE /rapid7/metasploit-framework/modules/exploits/linux/http/ubiquiti_airos_file_upload.rb
msf5 exploit(linux/ssh/ubiquiti_airos_file_upload) > reload
[*] Reloading module...

[!] Alias exploit/linux/ssh/ubiquiti_airos_file_upload no longer available after reloading, using exploit/linux/http/ubiquiti_airos_file_upload
msf5 exploit(linux/http/ubiquiti_airos_file_upload) > use exploit/linux/ssh/ubiquiti_airos_file_upload
[-] Error while running command use: stack level too deep
msf5 exploit(linux/http/ubiquiti_airos_file_upload) > use exploit/linux/http/ubiquiti_airos_file_upload
msf5 exploit(linux/http/ubiquiti_airos_file_upload) >

/shrug

We have to go deeper.

@acammack-r7

This comment has been minimized.

Copy link
Contributor Author

commented Aug 26, 2019

Ah, with that exact sequence of commands I can get it. I can work to narrow this down now, thanks!

Teach module cache about `realname`
Also uses the class refname where appropriate since an instances refname
reflects the alias currently in use and if a module is reloaded while
using an alias the old behavior would generate spurious cache entries
that would not be cleaned up or modified. Specifically, this could
register a self-referential alias that would cause a stack overflow when
trying to `use` such an alias.

Additionally, some other `fullname`s were changed to `realname`s for
clarity.
@acammack-r7

This comment has been minimized.

Copy link
Contributor Author

commented Aug 26, 2019

@wvu-r7 should be fixed now. You will probably have to delete an entry for exploit_linux/ssh/ubiquiti_airos_file_upload out of ~/.msf4/store/modules_metadata.json for things to work.

Verification

  • After making sure the local metadata cache is free of exploit_linux/ssh/ubiquiti_airos_file_upload entries
  • The steps from #12223 (comment) should not add one
@wvu-r7

This comment has been minimized.

Copy link
Contributor

commented Aug 30, 2019

Much better.

msf5 exploit(linux/http/ubiquiti_airos_file_upload) > use exploit/linux/ssh/ubiquiti_airos_file_upload
[-] No results from search
[-] Failed to load module: exploit/linux/ssh/ubiquiti_airos_file_upload
msf5 exploit(linux/http/ubiquiti_airos_file_upload) >
wvu-r7 added a commit that referenced this pull request Aug 30, 2019

@wvu-r7 wvu-r7 merged commit bcccfd8 into rapid7:master Aug 30, 2019

1 of 3 checks passed

Metasploit Automation - Sanity Test Execution Running automation sanity tests. Details available on completion.
Details
Metasploit Automation - Test Execution Running automation framework tests. Details available on completion.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@wvu-r7

This comment has been minimized.

Copy link
Contributor

commented Aug 30, 2019

Release Notes

A new deprecation procedure allows deprecated module to be deleted, transparently redirecting the user to the new module. It uses module aliases, refactoring the deprecation mixin in the process.

@wvu-r7

This comment has been minimized.

Copy link
Contributor

commented Aug 30, 2019

Copied here for posterity:

wvu 1 minute ago
We may want to discuss a timeline for removing the moved_from line, but it's not essential
wvu 1 minute ago
Since the old code is gone already
wvu < 1 minute ago
There is no fear of users using the deprecated module and filing bugs against it

wvu 1 minute ago
It's purely a cleanliness/maintenance thing for me, but I'm really not bothered by it
wvu 1 minute ago
It's actually useful to track how a module has moved, since I can touch and git log

@acammack-r7 acammack-r7 deleted the acammack-r7:deprecated-refactor branch Aug 30, 2019

@acammack-r7 acammack-r7 added the msf5 label Aug 30, 2019

@acammack-r7

This comment has been minimized.

Copy link
Contributor Author

commented Aug 30, 2019

Whoops, some of the stuff here doesn't backport cleanly. I'll cherry-pick and open a PR to 4.x.

acammack-r7 added a commit to acammack-r7/metasploit-framework that referenced this pull request Aug 30, 2019
@wvu-r7

This comment has been minimized.

Copy link
Contributor

commented Aug 30, 2019

Ah, yeah, evasion.rb snuck in there.

@tdoan-r7 tdoan-r7 added rn-fix rn-enhancement and removed rn-fix labels Sep 5, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.