Service manager fixes

[smashery]

This resolves several edge-case bugs in the service manager. I've grouped them together into one issue because the fixes are in some cases overlapping, and it seems better to test the entire feature at once given all the changes to it.

Bug 1: Communication channel not respected in service manager #16314

• Run an HTTP handler with the same settings (0.0.0.0 and the same port) locally, and on a remote session using ReverseListenerComm. Verify that listeners are started both locally, and via the remote session.
• Run an HTTP handler with the same settings (0.0.0.0 and the same port) on two separate remote sessions using ReverseListenerComm. Verify that listeners are started via both remote sessions.

Bug 2: Socket leak: HTTP service not dereferenced #16315

Verify the following:

• use exploit/multi/handler
• set payload linux/x64/meterpreter_reverse_http
• set lport 8080
• set lhost 0.0.0.0
• run -j
• Verify: netstat should show port 8080 open
• set luri abc
• run -j
• Verify: Should launch a new handler; should not complain about "the port is already open"

Bug 3: String key collision

The service manager attempts to find existing services based on a string-concatenated key of important properties (i.e. IP, port number). No delimiter was placed between the port and the host, so Port 123 on host 45.6.7.8 would be treated the same as port 1234 on host 5.6.7.8, and the service. I can't think of a situation that it's likely to happen (given that it actually needs to bind to that IP), but it's definitely a logic error. You can probably just verify this with a code review.

Bug 4: Modules don't use reference counting

The modules exploit/multi/misc/java_rmi_server and exploit/osx/browser/safari_file_policy call stop on the service directly, rather than respecting reference counting. As a result, any other services running at the same time as them on the same port will be killed by these modules.

[sempervictus]

@adfoster-r7 - i would venture those "additional issues" come from the circle of hell where we keep the handler/session logic (they made a new one, just for us - number 1337).
Overall, i think the deref approach is how Rex intended this to work - though i'm a bit behind at this point after the recent changes on whether these are refs right now or not (apologies, some messes being made requiring attention).
Personally, i think that all service instances should be refcounted with a final deref calling #close. I will try to carve some time in the next few days to dive the issues/PRs currently up for this stuff, but given everything going on i may not be able to put in the requisite time... I suppose its better to fix some stuff now and not let perfect be the enemy of good, i'm just not sure if all of the changes going in follow the same design/architecture and about related knock-on effects.

[smashery]

Yeah, I think some of these issues are related to how modules think stop_service works... maybe there's some vestigial stuff in there whereby it used to actually stop the service, but that isn't the case now - instead, it just does a dereference. I'll neaten it up to be clearer what the intent of each function is.

[smashery]

Okay, so looking more into this, it became apparent that this is an issue in a few other places.

exploit/multi/misc/java_rmi_server unilaterally shuts down the HTTP server once it's done; even if something else was using it. Here I start an HTTP_NTLM listener, and then the java_rmi_server, and then shut the latter down; the former is also killed in doing so:

msf6 > use auxiliary/server/capture/http_ntlm 
msf6 auxiliary(server/capture/http_ntlm) > run
[*] Auxiliary module running as background job 0.
msf6 auxiliary(server/capture/http_ntlm) > 
[*] Using URL: http://0.0.0.0:8080/aXuT3BU
[*] Local IP: http://192.168.20.206:8080/aXuT3BU
[*] Server started.

msf6 auxiliary(server/capture/http_ntlm) > use exploit/multi/misc/java_rmi_server 
[*] No payload configured, defaulting to java/meterpreter/reverse_tcp
msf6 exploit(multi/misc/java_rmi_server) > jobs

Jobs
====

  Id  Name                                 Payload  Payload opts
  --  ----                                 -------  ------------
  0   Auxiliary: server/capture/http_ntlm

msf6 exploit(multi/misc/java_rmi_server) > set rhosts 127.0.0.1
rhosts => 127.0.0.1
msf6 exploit(multi/misc/java_rmi_server) > run

[*] Started reverse TCP handler on 192.168.20.206:4444 
[*] 127.0.0.1:1099 - Using URL: http://0.0.0.0:8080/6dHKzvaROOlXeOQ
[*] 127.0.0.1:1099 - Local IP: http://192.168.20.206:8080/6dHKzvaROOlXeOQ
[*] 127.0.0.1:1099 - Server started.
[-] 127.0.0.1:1099 - Exploit failed [unreachable]: RuntimeError The connection was refused by the remote host (127.0.0.1:1099).
[*] Exploit completed, but no session was created.
msf6 exploit(multi/misc/java_rmi_server) > 
[*] Server stopped.

msf6 exploit(multi/misc/java_rmi_server) > jobs

Jobs
====

No active jobs.

The exploit/osx/browser/safari_file_policy module had the same problem too.

[jmartin-tech]

@msjenkins-r7 test this please.

[sempervictus]

I recently mucked with how stops work, because some modules run multiple services, so i split them out. We might be looking at the unintended effects of that. @smashery, if you have a sec, could you possibly trace the stop calls in there and see whether its an early call from the module or something tripping in the actual lib piece? I didn't audit all consumers, so this may well be on me.

[smashery]

@sempervictus I realise I've combined a few issues into this one PR here, which is muddying the waters a bit. I think what I've seen in general is just a bit of inconsistency in how different parts of the code interact with the service manager:

• The reverse HTTP handler didn't really use reference counting; rather, keeping track of how many sessions and resources existed... but those resources could have been from other modules, which themselves did respect the dereferencing. So it was a mix of conventions that were getting in each others' ways.
• When it did stop, the reverse HTTP handler would directly tell the service manager to stop, which would dereference it; but if other services were still using it, then the service manager would lose its own knowledge of the service, but the service wouldn't die, so the port would be open and the ServiceManager would be unable to find it for other modules wanting to use the service on that port, so the binding would fail.
• Two modules would just kill the service (Bug 4)

I think it's at a point where it's more consistent, and I can't see any issues (since fixing the one @adfoster-r7 raised).

Are you suggesting, though, that you have some outstanding work that might conflict with these changes?

[adfoster-r7]

@msjenkins-r7 test this please

[adfoster-r7]

Ran through a few scenarios, and this does a great job of cleaning up things as expected now. I imagine there might be gremlins still, but after the feedback is applied I think this should be good to ship ⛵

[adfoster-r7]

Release Notes

Ensures individual modules no longer accidentally shutdown joint services that are used across multiple modules/handlers etc, such as HTTP servers. Modules will now correctly unregister interest in the global service, and if there are no longer any interested modules in the running global service, it will be shut down correctly.