RFC: Improve Validation of Session Support

[smcintyre-r7]

Problem

Metasploit post and local exploit modules allow the selection of incompatible sessions. Since not all meterpreter sessions are created equal, when a module restricts to meterpreter sessions and the connection does not support the api features required to execute the user is not informed until actually attempting to execute the module.

Take for instance post/windows/gather/screen_spy, this module requires a meterpreter session however payload/python/meterpreter/bind_tcp would meet this requirement without actually being able to support the COMMAND_ID_START_ESPIA command in the targeted session.

Further, a core issue to address is that the handlers currently report any connection as a session. In this scenario, opening a handler on a port, any syn request to that listener will trigger a reported session even if no further interaction occurs on the port. This has lead to many incomplete or invalid sessions being reported as issues.

Solution

improve session determination

• When new connections are received by a hander vet the connection before reporting a session is available.
• Add more granular session enumeration:
  • Post module pre-validation allowing for metadata about what session support is required

  'SessionTypes'   => [ {'meterpreter', [ COMMAND_ID_START_ESPIA ]} ]
  

  • Expand initialization or session options set validation to determine whether the selected session actually supports the module.
• Improved error handling for matching TLV command id support to display more readable notifications when a command fails to run on a particular session.

[gwillcox-r7]

Great idea, I know I've run into a few of these issues in the past when testing things out like Python Meterpreter vs Windows Meterpreter and its not immediately apparent what the differences are. We might also want to consider some documentation page or chart which lists out the features each type supports to help end users when developing. Not not sure if that would grow unwieldy (there are a lot of items that might have to be listed). I imagine most of the info for this documentation would likely be generated whilst solving this issue but feel free to correct me if I am mistaken in this assumption.

[jmartin-tech]

As this feature gets implemented automation could leverage the detection code to generate and update documentation on what payloads provide support for each command.

[gwillcox-r7]

@jmartin-r7 Sounds like a good idea, this would prevent any confusion as well.

[smcintyre-r7]

As we continue to look into the compatibility of the feature set provided by Meterpreter, it may also be really beneficial to promote the system information out of the standard API and into the core so it can be available for extensions to determine whether or not they are compatible. At this time adding a normalized field to include the OS version number would be nice as well.

We could use this for example to pin the powershell extension to Windows 7 and greater.

[jmartin-tech]

Does adding this detail to core really improve the detection? Mechanisms for detection in the console could make requests of the session to determine if compatibility is reached. Thinking on this when an extension is requested to load may be the time to query this. Can the powershell extension deploy if standard API is not loaded? If the session has not loaded standard API is it really viable for using extensions at all?

As I understand it core is intentionally keep sparse to reduce the detection surface area.

[smcintyre-r7]

Moving the detection logic wouldn't change it, but rather make it more accessible. As far as I know, the individual extensions are independent of one another so you could load the core and the powershell extension without loading the standard API extension.

If we really don't want to move it into the core, I think we'd need to treat stdapi as a special case and load it when another non-stdapi extension needs to check the version information.

[bcoles]

As we continue to look into the compatibility of the feature set provided by Meterpreter, it may also be really beneficial to promote the system information out of the standard API and into the core so it can be available for extensions to determine whether or not they are compatible. At this time adding a normalized field to include the OS version number would be nice as well.

We could use this for example to pin the powershell extension to Windows 7 and greater.

Seeing as this conversation has tangented way off-topic towards storing system information ... here's some ramblings I've been meaning to dump somewhere.

tl/dr: The existing system information is far from normalized. There never was a standard and data was throw into the database haphazardly. This will cause problems if it is used as a basis to restrict access to functionality.

Currently, trying to assess system information programmatically is highly problematic, with perhaps the exception of Windows which has a fairly rigid versioning scheme (from Windows 10 onwards, and ignoring embedded / Window CE / Windows Mobile, and we pretend the 9.x/NT split was never a thing, and ...) and has no forks (ReactOS doesn't count). Ok so maybe Windows is highly problematic too.

Admittedly it's been a couple years since I looked into the way system info was stored but not much seems to have changed. Here's some old notes I've been meaning to post since 2018:

https://github.com/rapid7/metasploit-framework/blob/master/lib/msf/core/post/linux/system.rb
system_data[:kernel] = kernel_version
we grab the kernel version, then do nothing with it
it's not reported in the hosts table
the hosts table has a column for os_version and os_sp (service pack, which is only ever relevant for Windows)
should os_version be the kernel version, or kernel release, or the distro version (ie, 7 for CentOS 7)
the *nix world is also a mess due to distributions and forks
for solaris, is the OS "solaris" or "opensolaris" or "openindiana" or "sunos"
for windows, is the version the NT version or system version (ie, 7 for Windows 7) - is it "pro" or "enterprise" ?

insert obligatory xkcd about competing standards

Inconsistencies in software detection already cause issues.

• This code pattern is used (inconsistently) in a whole bunch of places and has to be updated every few years:
  • unless sysinfo_value =~ /(7|8|8\.1|10|2008|2012|2016|2019|1803|1903)/
• Trying to sessions -u on a cmd/unix shell usually fails, because upgrading "unix" sessions isn't supported
• probably more ...

Libraries fight each other to clobber the system info with no regard for whether the clobbering information is accurate or whether the clobbered information was more accurate.

Currently, this data cannot be easily and reliably operated on programmatically:

msf6 exploit(multi/handler) > sessions -i 1
[*] Starting interaction with 1...

meterpreter > sysinfo
Computer     : 172.16.191.170
OS           : LinuxMint 19 (Linux 4.15.0-20-generic)
Architecture : x64
BuildTuple   : x86_64-linux-musl
Meterpreter  : x64/linux
meterpreter > 
Background session 1? [y/N]  
msf6 exploit(multi/handler) > hosts

Hosts
=====

address         mac  name            os_name                                 os_flavor  os_sp  purpose  info  comments
-------         ---  ----            -------                                 ---------  -----  -------  ----  --------
172.16.191.170       172.16.191.170  LinuxMint 19 (Linux 4.15.0-20-generic)                    server         

msf6 exploit(multi/handler) > 

• os_flavor and os_sp are empty.
• A bunch of info is crammed into os_name
  • what is a LinuxMint ? (it's a flavor of Linux, one of the most popular forks of another flavor of Linux called Ubuntu, which is a fork of ...)
  • Am I looking at the kernel version or kernel release ? (it's the latter.)
  • Is it mainline kernel version or a different versioning scheme ? (it's Ubuntu's versioning)
• Is it really a server ? (No, it's a desktop distro running Cinnamon desktop environment with a user logged into console with a freedesktop "seat")

Using a Linux system is a bit of an unfair example but it demonstrates the issue well. The system info for Windows systems is generally more accurate.

If Metasploit is going to explicitly disable functionality because it thinks it knows what the target host is running/using, then there should be an option to allow the operator to bypass it. This is the same reason that the session is incompatible with this module message was originally downgraded from an error to a warning (in 2016). The check was way too inaccurate to be useful.

[smcintyre-r7]

Further a core issue to address is that handler currently reports any connection as a session. In this scenario, opening a handler on a port, any syn request to that listener will trigger a reported session even if no further interaction occurs on the port. This has lead to many incomplete or invalid sessions being reported as issues.

I think this was sufficiently addressed in #14789 which began to leverage the TLV encryption negotiation process to validate that a session is responsive within the allowed 30 second timeout period.

[jmartin-tech]

Possibly, the PR improves the handling only for meterpreter based sessions. A connection from a shell payload will still report immediately. There is some precedent for shell payloads to validate a functional communication channel when connecting, for instance the udp payload handler will do this already. Making this behaviour default, with an option to suppress if for users with a need avoid the behaviour as a detectable action could close the loop on this item.

[smcintyre-r7]

Good call, I often forget about non-meterpreter sessions.

[smcintyre-r7]

See #15051 which updates all of the command shell session types to use the same technique as the UDP payload handler you pointed out. Since post modules are only (as far as I'm aware) compatible with either meterpreter or shell sessions, there isn't an immediate need to perform validation on others such as VNC.

[smcintyre-r7]

Based on how the Meterpreter command ID strings are structured, I'm thinking that a wild card string could be the best option. This would get merged up through the module data. Then, for each requirement string, Metasploit would look up all commands it knows that matches the string, and verify that the sessions support all of those commands. For example, anything requiring railgun would specify stdapi_railgun_* which would expand to stdapi_railgun_api, stdapi_railgun_api_multi, stdapi_railgun_memread, and stdapi_railgun_memwrite.

This would allow module authors to be as general or granular as they'd like. Maintaining an explicit list of each command every module requires just doesn't seem feasible.

My concern that I haven't quite figured out how to address is mixins. If a mixin offers say 5 methods that require 5 different Meterpreter commands, but a module only calls one of those methods, should it only be compatible with sessions that support all 5 Meterpreter commands?

I'm thinking yes because:

1. We should be trying to close Meterpreter feature gaps anyways.
2. This would correctly abstract away the mixin's internal functionality so it can be updated without breaking modules in the future. For example, if the one mixin method the module calls is changed to require 2 Meterpreter commands, we wouldn't need to update each module with the second Meterpreter command.
3. This would be a more strict approach.

We could potentially add the ability to negate requirements, but that seems dangerous. In the previous example the module author, with a detailed understanding of the mixin's implementation would explicitly mark the 4 Meterpreter commands used by the 4 methods they're not using as not required.

The other concern is that this requires module authors that are using Meterpreter methods to be familiar with the commands that they are using. For example, any module author that calls session.core.migrate would just need to know that requires the core_migrate command. This case is pretty intuitive but they're not all so easy (session.powershell.import_file requires powershell_assembly_load not powershell_import_file).