rapid7/meterpreter — pull request #34 by OJ (2013-10-17 12:41:21+00:00)
Title: Command refactor for clean shutdown + inline calls

This work contains a bunch of changes around command dispatching. The
goals for this bit of work were to:

* Provide the ability for commands to be executed on the same thread as the server rather than always creating new threads and executing them on those threads.
* Have the means for _special_ commands, such as `exit` and `migrate` to shut down the server cleanly without having to rely on signalling across threads or by doing brutal thread termination via shared global handles. This should not only fix the dirty shutdown problem on Windows which leaves tasks dangling (or based on the prior attempt at fixing, crashing stuff as well), it should also help clean up the shutdown process in POSIX.

These changes hit a very important part of Meterpreter and so should be reviewed with intense scrutnity. I expect this PR to garner a log of critique and most likely a number of changes before being included in the main line.

The `PacketDispatcher` was modified to include a new function pointer called an `inline_handler`. This new member indicates that there's a handler which should be invoked inline. While this sits alongside the existing `handler`, they are actually mutually exclusive. If an `inline_handler` is specified then the `handler` is ignored and it is assumed that the command is intended to be handled inline. The signature of the inline handler is different to the standard handler, and this is why a new function pointer was added rather than a simple flag. Addition of this parameter meant that the basic command structure changed, and that obviously affects all of the extensions and their respective commands. This changeset therefore updates each of those command declarations so that they use the new macros that hide this detail.

Other things to be mindful of:

* This version of the code reads the command's `method` prior to invoking any other function, and after that the command itself is passed around to the threaded or non-threaded routes for invocation. An extra thread parameter was included as as result, and an overload for the `thread_create` function was added which supported this new parameter. This was named `thread_create3` because `thread_create_with_another_paramter` sounded a bit crap.
* The migration code, which originally had a `thread_kill` and an event wait once the new meterpreter session had been created, has been modified to not do any waiting at all. Instead it finishes execution as fast as possible and returns control to the server which should respond by shutting down in a clean way.
* Originally the code always attempted to call a command handler in the base command list and then, if found, would also call an "overload" in the extension commands list. From the investigation that I did, it appears that the overloaded methods did nothing in the base (they'd early out during invocation). As a result, the new way of doing things acts like a 'true' overload in that the extension commands are searched first, and if one is found this is the command that is executed. Any base commands with the same method name will not get executed. In the case where there is no extension command found, the base command list is then queried. If a command is found that command is instead invoked.
* The POSIX version still compiles cleanly, but I've never been able to build a version that runs on my machines. I'm not sure if there's a trick to getting POSIX builds to run, and if there is I don't know it. Whoever scrutinises this build should make sure that the POSIX version that they build can still run and (hopefully) exit cleanly.

I've added lots of documentation, but there's always room for improvement.

Hopefully this will fix the `*_tcp` side of [Redmine 8438](https://dev.metasploit.com/redmine/issues/8438).

Bring on the feedback!