Alive Connections Being Dropped

[jcyprus]

Summary

Connections are being prematurely severed based on occasionally seen TCP communication gaps (typically [TCP ACKed unseen segment] or [TCP Retransmission] packets). While the devices autonegotiate a fix to these gaps and continue with their communication, Zeek has measures in place to alert parsers of these gaps and leave it up to them on how to handle it. The current implementation is more in-line with how other Zeek parsers handle these types of packet sequences, however it is resulting in a high volume of packet loss in this instance.

Motivation and Context

ENIP communications may occasionally send [TCP ACKed unseen segment] or [TCP Retransmission] packets, which results in a TCP gap being formed. While this doesn’t impact the communication between devices, Zeek sees this as a failed connection and sets the had_gap variable to true so that parsers can know that this communication is faulty. The variable will be reset for that communication on a per parser and per connection basis, meaning that it is up to the parsers to handle the gap and tell Zeek what to do next.

Lines 37 and 38, if (had_gap) return;, in ENIP.cc handle the error. These lines result in all future events, for a given TCP connection, being skipped if the parser detected a gap. However, since this flag is being set during the normal communications of healthy and ongoing connections, a high volume of unrecoverable packet loss occurs.

As currently implemented, the large amount of packet loss in the final parser output leads to inconsistent and inaccurate results when compared with communication over the wire. If this condition is realized, Wireshark will show significantly more traffic than Zeek, as Zeek has stopped logging all traffic from that connection. This fix would allow for reestablished/retransmitted connections to continue to be logged. Therefore, anyone who is viewing traffic will have an accurate understanding of the relationship between assets and the true volume of traffic being sent.

Potential Fixes and Implementation Notes

Comment out or remove the if (had_gap) return; line in ENIP.cc.

Code Comparison Between Current Implementation and Proposed Fixes

Comparing files ENIP.cc and ENIP-WITH-FIXES.CC
***** ENIP.cc
36: assert(TCP());
37: if(had_gap)
38: return;
39:
***** ENIP-WITH-FIXES.CC
36: assert(TCP());
37: //if(had_gap)
38: // return;
39:
*****

[Kleinspider]

Code changes as requested have been tested and pushed into main