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

RFC: establish a separate module category for persistence modules #19592

Open
bcoles opened this issue Oct 27, 2024 · 8 comments · May be fixed by #19815
Open

RFC: establish a separate module category for persistence modules #19592

bcoles opened this issue Oct 27, 2024 · 8 comments · May be fixed by #19815
Labels
suggestion-feature New feature suggestions

Comments

@bcoles
Copy link
Contributor

bcoles commented Oct 27, 2024

Summary

We have a lot of modules to establish persistence (and will likely gain more: #16791 #19359 #19698).

# find modules/ -name *persist*
modules/exploits/unix/local/at_persistence.rb
modules/exploits/osx/local/persistence.rb
modules/exploits/windows/local/persistence.rb
modules/exploits/windows/local/persistence_image_exec_options.rb
modules/exploits/windows/local/registry_persistence.rb
modules/exploits/windows/local/s4u_persistence.rb
modules/exploits/windows/local/persistence_service.rb
modules/exploits/windows/local/ps_persist.rb
modules/exploits/windows/local/vss_persistence.rb
modules/exploits/windows/local/wmi_persistence.rb
modules/exploits/linux/local/apt_package_manager_persistence.rb
modules/exploits/linux/local/autostart_persistence.rb
modules/exploits/linux/local/rc_local_persistence.rb
modules/exploits/linux/local/service_persistence.rb
modules/exploits/linux/local/motd_persistence.rb
modules/exploits/linux/local/bash_profile_persistence.rb
modules/exploits/linux/local/cron_persistence.rb
modules/exploits/linux/local/yum_package_manager_persistence.rb
modules/exploits/multi/misc/persistent_hpca_radexec_exec.rb
modules/post/windows/manage/persistence_exe.rb
modules/post/windows/manage/sshkey_persistence.rb
modules/post/linux/manage/sshkey_persistence.rb

Although the modules are consistently named *_persistence*, the categorization is inconsistent - sometimes as post/<platform>/manage/, but usually within the exploit/<platform>/local/ directory.

Almost all of these modules establish persistence by creating a new session at an unknown time in the future. These are not exploits - they are post-exploitation modules more akin to management post modules.

Persistence modules were previously classified as local exploit modules as traditionally any module which could result in a new session was considered to be an "exploit" module; however, this old practice has been repeatedly undermined and the definition is no longer useful. For example:

  • Evasion modules return a session
    • these modules are not classified as exploits, but they effectively operate the same as file format exploits (like exploit/<platform>/file/ modules) and return a session
  • Management post modules return a session

Is it useful to separate persistence modules into a separate category/subdirectory?

Basic example

  • Move persistence modules to modules/post/<platform>/persistence/ ?
  • Move persistence modules to modules/exploits/<platform>/persistence/ ?
  • Move persistence modules to modules/persistence/<platform>/? (with a new Msf::Persistence class?)

Motivation

Although there is no real harm in leaving persistence modules in the local exploit category, the typical use case for persistence modules is sufficiently different to exploit modules that a separate category may be desirable to eliminate ambiguity.

Once segregated, exploit/<platform>/local exploits will be local exploits only:

  • Persistence modules can be more easily found should the operator wish to maintain persistence on a host.
  • Privesc modules can be more easily found should the operator wish to elevate privileges on a host.
  • Persistence modules will not be selected and tested using the Local Exploit Suggester module, which is intended for local exploitation (ie, privilege escalation).

Additionally, as most exploit modules within Framework return a session immediately, where as persistence modules return zero or more sessions at an unknown time in the future, segregation may be beneficial to facilitate intuitive handler workflows. For example, persistence modules could be configured with default module options to launch backgrounded long-running session handlers.

@bcoles bcoles added the suggestion-feature New feature suggestions label Oct 27, 2024
@h00die
Copy link
Contributor

h00die commented Oct 27, 2024

agree with this.

@jvoisin
Copy link
Contributor

jvoisin commented Nov 8, 2024

I think modules/persistence/<platform>/ makes the most sense.

@h00die
Copy link
Contributor

h00die commented Dec 29, 2024

Any comments from an R7 person about this? I love the idea, it always felt a little clunky having them mixed in with other modules. This should be pretty easy for someone to accomplish, from a base thought its just a bunch of git mv and adding moved_from. Would only take an hour or two (unless a new Msf::Persistence class is needed).

I like modules/persistence/<platform>/

@bcoles
Copy link
Contributor Author

bcoles commented Dec 29, 2024

I think modules/persistence// makes the most sense.
I like modules/persistence/<platform>/

I also like modules/persistence/<platform>/.

Would only take an hour or two (unless a new Msf::Persistence class is needed).

I think a new class would be necessary to handle new top-level directories within the modules directory. This is also the most Metasploit-y and future-proof approach. For an example class implementation, see the most recently added new category (for Evasion modules) in #10759.

@smcintyre-r7
Copy link
Contributor

I think I like the idea of restructuring modules in directories for organization vs creating a new module type altogether. We still occasionally find features that are missing or inconsistent between module types now (e.g. evasion) and creating yet another would contribute to the problem. I also think that long term, we'd be better off moving in the opposite direction and consolidating module classes and types in some case. Post, Auxiliary and Exploits sometimes get blurred now and it'd be nice if all three could for example have a check method, actions, and open sessions.

So to fix the problem of organizsation, I think moving them to a directory structure such as (exploit|post)/$platform/persistence would do the trick depending on if it's considered an "exploit" now due to opening a session.

@h00die
Copy link
Contributor

h00die commented Jan 16, 2025

I like not making a separate module type, it's a huge pain tbh.

I like organizationally putting these in their own root folder under modules as previously discussed. It helps set appropriate expectations as outlined by @bcoles (no new privs, shell at a potentially unpredictable future time, new shells are used).

I'll likely close the current PR and start a new one since there are a lot of commits and files that are no longer relevant in it

@h00die h00die linked a pull request Jan 18, 2025 that will close this issue
88 tasks
@h00die
Copy link
Contributor

h00die commented Jan 19, 2025

#19472

@h00die
Copy link
Contributor

h00die commented Jan 19, 2025

While I prefer the modules/persistence solution, the loaders aren't very happy with that. Here's the flow I'm seeing:

  1. https://github.com/rapid7/metasploit-framework/blob/master/lib/msf/core/modules/loader/directory.rb#L38 this line is failing to load from the persistence folder. with full_entry_path being modules/persistence and type persistence. Two solutions for that:

    • Looks like we need an override to set the type to exploit or
    • bypass that check for anything in path /persistent.
  2. However, the end yield then gives back something like /metasploit-framework/modules, exploit, linux/sshkey (going with type set instead of bypass strategy)

  3. This gets combined here.

  4. Eventually we crash because a look up of the module type (exploit) here causes the file not to load here

  5. With this in mind, we could once again make an override that if its not found in exploits we check persistence, or we have to make a stub module type.

I think this path is going to be a fools errand. These small overrides make the code ugly and harder to follow and much more patchwork than it is already. I doubt I've finished following the rabbit hole. Unfortunately I think @smcintyre-r7 's sage hint that this was not the way to go is right. I don't like exploit/$platform/persistence as a user, but for code quality and maintenance, I think this is the better way to go.

Unless someone else wants to take the lead on this, I'll pivot to that route.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
suggestion-feature New feature suggestions
Projects
None yet
4 participants