search

USArmyResearchLab / Dshell

Dshell is a network forensic analysis framework.
Other
5.45k stars 1.14k forks source link

Ignore handshake #99

Closed amm3 closed 7 years ago

amm3 commented 7 years ago

With this approach, I propose two critical changes:

  1. Setting the default states (via Connection.__init__()) for nextoffset values to None instead of 0. Truthfully, zero doesn't make sense as zero has a legitimate next-expected value if/when sequence numbers loop past 2**32.

  2. Accounting for this condition when using ignore_handshake. Other states of operation, would/should throw errors when encountering nextoffset None values, if not properly implemented.

dev195 commented 7 years ago

Yes, this all matches what I was seeing. Thanks for the push.

While I was in there, I wanted to do a little clean up in the function, as well. Functionally, it should stay the same. I just wanted to move the if-statements around (it bugged me the number of times we checked self.ignore_handshake), and add some more clarification comments.

I also needed to update the UDPDecoder.UDP function, since UDP traffic was complaining about the None offsets.

It seems to hold up with my testing, but I'd like for you to give it a shot before I merged the pull request with the changes.

This is also the first time I tried editting a file directly from GitHub's interface. I apologize if I unintentionally messed something up.

amm3 commented 7 years ago

Works great in my tests. And good catch on the UDPDecoder! In my haste, I didn't check against any UDP packets. I also like the reorganized if/logic... definitely reads easier.