In certain cases, packets would arrive before their handlers were ready
to handle them. This resulted in packets going into the incomplete list
and being re-queued into the packet input queue. This is a problem when
MORE packets arrive while processing because the older packets end up at
the end of the queue instead of the start of the queue. This means newer
packets are processed FIRST, and hence we have an out-of-order
sequencing problem.
This commit adds an "incomplete queue" which gets prioritised over new
packets. If packets are incomplete at any point, they are added to this
queue, and are dequeued prior to the new packet queue. This results in
packet sequences being maintained.
This was causing issues with things like port forwards. BUT NOT ANY
MORE!
Rex::Proto::DNS::Resolver inherits from Net::DNS::Resolver however it
changes the signature of the send_tcp and send_udp methods, making it
break when a method from the parent class (such as #axfr) is called.
A long time ago prior to supporting both encrypted packets and packet
pivots, a bit of code existing in the packet dispatcher that reordered
packets before passing them on to the internal workings. This reordering
would prioritise responses first, it would put "channel close" messages
at the end, and the rest would go in between. It's a bit gross, but it
is what it is.
The key here is to note that for this ordering to happen, the code needs
to be able to access the packet header (to determine if it's request or
response), and to access the packet body (to get access to the method
and check if it's a channel close message).
When packet encryption came in this wasn't too much of a concern because
the packet decryption could happen as soon as the packet came off the
wire. This meant that both the header and the body were available for
consumption and everything sunshine, daisies and unicorn farts.
ENTER PACKET PIVOTING TO MESS THIS ALL UP!
As we're all fully aware (right?) encryption keys are per-session. So
this means that every session has its own set of keys, and hence to
decrypt a packet we need to make sure we've got the right session. This
was a no brainer before, because sessions read their own packets off
their own transports. But with pivots, that changed because packets
could appear on the transport that were intended for other sessions.
It appeared that the solution here was simple. When a packet is read off
the wire, just read the body in full without decrypting. Check the
session GUID to see if it matches the current session, or to see if it's
inteded for a pivoted session. If it's the latter, then use the pivot
session decryption key, if the former, use the current session's key.
Too easy, right?
Right?
There was an internal function that was invoked to dispatch packets
after the came off the wire, called `dispatch_inbound_packet`. It seemed
to make sense to decrypt the packet here because that was invoked across
the various transports. So code was added at this point to decrypt the
packets based on the appropriate session. Testing was done, things
seemed to work.
Fast forward to last night, where I lost a bunch of hours while working
on something that shouldn't be related. I have been changing the
mechanism used for methods so that we don't use strings, we instead use
identifiers (makes the noise on the wire smaller/less obvious, and
allows us to remove method strings from our payloads). Rather than
attempt to locate all the spots where the method IDs are either
hard-coded or generated, it made more sense to start with functionality
in the `Packet` class that would map between method strings and command
identifiers. In order to catch the case where we had a method string
that we didn't expect, I raise an exception when the method string
doesn't exist in the map of known strings.
This exception was a blessing and a curse. To cut this story a little
shorter, we ended up with the following situation:
* Packets would start coming in and the reader would read the header and
then decode it so that we could find the size of the packet and read
the packet body.
* The packet header was then in the clear, but the packet body was yet
to be decrypted.
* The "prioritisation" hack would run, checking the packet type (which
is fine because it's in the clear), then the method (which is not
fine, because it hasn't been decrypted).
Prior to the work I was doing, the method id check would _always fail_
because the method string would come out blank.
After including my work, the exception literally killed the packet
dispatching, resulting in all kinds of horrid woes (such as having all
channels failing).
What this means is that since packet pivots came about, we have not been
correctly pushing channel close messages to the back of the queue before
processing. The result? I don't know! I know that we've had issues
raised against the code saying that packets are coming out of order in
certain cases when channels are in use, but I don't think that's
related. What's clear is that I broke it when I did the packet pivots,
and I've only just realised it now.
So this code is intended to fix the problem and make sure that channel
close messages are pushed to the back like they were before.
At this point, people should be well aware of how easy it is for me to
break things, and therefore revoke my access to anything with
a keyboard.
This adds and uses the functionality to leverage the Windows API for
managing users and groups via meterpreter sessions. This replaces
relevant functionality in a few modules which previously relied on shell
commands.Merge branch 'pr/12988' into upstream-master
Spencer McIntyre