Delineate Rex::Proto::*::Server mixins for Msf

[sempervictus]

trafficstars

Msf::Exploit::Remote::*::*Server mixins are implemented either as descendants of the SocketServer mixin providing ports as service objects and handling protocol interactions using handlers on those, or as actual *Server instances out of Rex::Proto. When creating MetasploitModules (or other namespaces) which mix two or more of these Msf::Exploit::Remote::*::*Server mixins together, a namespace collision occurs on the :service object of the namespace mixing in the two setters for it and creating a last-write-wins condition on self.server which causes the first server instantiated to be lost into ObjectSpace - no rational reference to it, and no GC on it as it has its own references to unmanaged memory (file descriptors).

Address the problem by delineating each of the Rex::Proto servers with named prefixes such as http_service and dns_service in order to permit mixing them all together along with the socket services which can run arbitrary protocol handlers from the Msf namespace. Leave backward compatibility via :service decorator methods in the updated Msf Server mixins and method aliases to their start/stop methods for access when overriding names are mixed in after these.

[sempervictus]

The 2.7 fail is... weird? Not sure how it got to calling deref like that. To make this "worthwhile" suggest we hold merge until #15969 lands and then i can rewrite that real quick to make use of this code as an example for how other similar modules can be refactored.

[sempervictus]

@smcintyre-r7 - this is relevant to the discussion in #15972, and probably needs eyes-on from a few people. Also, could you please whisper the magic go-word to the bots? They failed, and seemingly not on my code (again).

[gwillcox-r7]

@msjenkins-r7 Test this please

[gwillcox-r7]

Looks like one of the RSpec checks is failing:

rspec ./spec/lib/msf/core/exploit/http/server_spec.rb:55 # Msf::Exploit::Remote::HttpServer#cleanup should remove successfully-added resources

Odd that its only failing for Ruby 2.6 though. We dropped support for Ruby 2.5 so in theory those checks shouldn't matter any more.

[gwillcox-r7]

@msjenkins-r7 Test this please

[gwillcox-r7]

@msjenkins-r7 Test this please

[timwr]

@sempervictus this looks reasonable. Can you fix the conflicts and also provide verification steps please?

[sempervictus]

oh bollocks, didn't realize this wasn't merged. Thanks @timwr - will revisit the branch today

[timwr]

@sempervictus can you provide verification steps please?

[sempervictus]

@timwr thanks - off the top of my head:

• Run the log4j exploit a few times and ensure that there are no longering services after each run
• Try using any modules that serve multiple protocols at the same time and verify that their service stop calls work consistently as well
• If you're an author, spin up a test module or two which serve on multiple protocols and verify correct start/stop functions

[smcintyre-r7]

I'm going to attic this. We should absolutely work on refactoring the protocol services to deal with the name space issues but we haven't been able to allocate the time to complete and test this sufficiently.

[github-actions[bot]]

Thanks for your contribution to Metasploit Framework! We've looked at this pull request, and we agree that it seems like a good addition to Metasploit, but it looks like it is not quite ready to land. We've labeled it attic and closed it for now.

What does this generally mean? It could be one or more of several things:

• It doesn't look like there has been any activity on this pull request in a while
• We may not have the proper access or equipment to test this pull request, or the contributor doesn't have time to work on it right now.
• Sometimes the implementation isn't quite right and a different approach is necessary.

We would love to land this pull request when it's ready. If you have a chance to address all comments, we would be happy to reopen and discuss how to merge this!

[sempervictus]

FYI i am running with this in my fork, so code's live and in-use for whenever upstream is ready.