Unify clean shutdown and update C2 channel with session tracking #358
Conversation
|
This also needs a lot more hands on testing for each of the C2 types. |
|
Until the C2 tests get automated I'm going to manually test each C2 and record the results based on the above changes, with the following test plan:
SSLShellServer
SimpleShellServer
HTTPServeShell / HTTPServeFile
ShellTunnel
Things fixed from this:
template
|
- Ordering of session catching to kill off all `net.Conn` instances - Fixed doScan and doVerify holding C2 open until timeout, this appears to be a regression and will now instantly close the C2 if these fail. - Ensured that `-o` was supported - Clarified phrasing of shutdown belongs to C2 - `HTTPServeShell` emits 2 shutdown messages right now since it starts up 2 instances. Might be worth adding context to the channel for C2Type and printing that. Just juggled waitgroup properly and moved the Drop to inside the core check. - `ShellTunnel` required some coercing of Session logic in order to track connections for client and server upstream. There's a few hacks to deal with shutdown logic and currently can't handle a queue of sessions if we ever add support for that, it'll need to revisit some of that logic if we do (but that applies to everything).
|
I think it just needs docs and I will undraft. |
| @@ -314,6 +331,8 @@ func startC2Server(conf *config.Config) bool { | |||
| } | |||
|
|
|||
| // execute verify, version check, and exploit. Return false if an unrecoverable error occurred. | |||
| // | |||
| //nolint:gocognit | |||
There was a problem hiding this comment.
The linter hates thinking.
| } | ||
| // Handle the signal interrupt channel. If the signal is triggered, then trigger the done | ||
| // channel which will clean up the server and close cleanly. | ||
| go func(sigint <-chan os.Signal, channel *channel.Channel) { |
There was a problem hiding this comment.
This is something that has been brought up but maybe not thoroughly discussed.
Do we make this opt-in or opt-out?
With this in place as-is you can opt-out using something like:
if sigint: os.Exit(0)
But will this become boilerplate in everything or will the alternative (letting this code do the shutdown) be the default case?
Seems like a 'gallery' discussion.
There was a problem hiding this comment.
Why would you opt out? I don't think I see that behavior ever being set up as a flag. if there was one thing I'd like is to maybe split it into an OS specific build gate so we could support non-unix-y stuff
|
Working on patching the HTTPShellServer, then this will be g2g |
default vs error.
| if httpServer.Channel().Shutdown.Load() { | ||
| server.Close() | ||
| httpServer.Shutdown() | ||
| wg.Done() |
There was a problem hiding this comment.
should we not defer this on line 267 instead?
There was a problem hiding this comment.
My theory with this was that if an error occured that was fatal to the http server that close would still be called and defer would be triggered, while the channel load checks are just for any other situation that occurs: timeout, manual exit, other c2 trigger, demons, etc.
This unifies the work from @lobsterjerusalem in #340 and #268 and supersedes it with a different path. Instead of handling signal shutdowns and cleanup per-C2, instead this updates the C2 interface and
channel.Channelto have:channel.Channel.Shutdown *atomic.Bool- A C2 specific atomic to track if the C2 needs to shutdown for any reason.c2.Shutdown() bool- A uniform shutdown interface for calling shutdown, manually or triggered from the shutdown tracking atomicc2.Channel() *channel.Channel- Allows access to the underlying C2 channel structchannel.Channel.Sessions map[string]Session- Basic "session" tracking. This allows a map of random session IDs of any string type to a basic session struct that contains all the current connected clients.channel.Input io.Reader- Allow for the channel input to be user controlled. This is in preparation for adding end-to-end tests for C2s asos.Stdinneeds to be controlled.(c *Channel) HasSessions() bool- The long awaited way to check if a C2 is connected or not. Instead of accessing a simple boolean, this can handle callback lifecycle future needs in exchange for a bit of clarity and some reference juggling. Can be invoked with:(c *Channel) AddSession(conn *net.Conn, addr string) bool- Adds a session to track. At the moment theconnis optional and if it is set, thenClose()will be called on cleanup, but if it isnilit will be skipped. My current version uses a 16bit base32 string for unique identifiers in order to stay within stdlib and because I think it's easier to suss out than UUIDs, but I suspect some people will have thoughts.(c *Channel) RemoveSession(id string) bool- Remove the session from the tracking map and if it has a non-nilnet.Connit will close the connection.(c *Channel) RemoveSessions() bool- Remove all sessions and close all opennet.Conns.This allows us to put the shutdown logic for OS signals in the cli/framework, as well as from inside of the C2 itself. This allows uniform clean shutdown for each of the C2s, including (finally) fixing the shutdown of open shell sessions.
There's some documentation and better comments I want to add to make sure to clarify some components such as how to track the atomic for new C2s. It will also require a change to our experimental C2s for cleanup and channel tracking. This should be considered to be an API break for those C2 users (of which I don't see any public ones at the moment besides us, but it should be noted on tag).