Reassembler integer overflow issues. Data not delivered after 2GB

[gregor]

The TCP Reassembler does not deliver any data to analyzers after the first 2GB due to signed integer overflow (Actually it will deliver again between 4--6GB, etc.) This happens silently, i.e., without content_gap events or Undelivered calls.

This report superseded #315, #137

The TCP Reassembler (and Reassem) base class use int to keep track of sequence numbers and seq_delta to check for differences. If a connection exceeds 2GB, the relative sequence numbers (int) used by the Reassembler become negative. While many parts of the Reassembler still work (because seq_delta still reports the correct difference) some parts do not. In particular seq_to_skip is broken (and fails silently). There might well be other parts of the Reassembler that fail silently as well, that I haven't found yet.

See Comments in TCP_Reassembler.cc for more details.

The Reassembler should use int64. However this will require deep changes to the Reassembler and the TCP Analyzer and TCP_Endpoint classes (since we also store sequence numbers there). Also, the analyzer framework will need tweaks as well (e.g., Undelivered uses int for sequence numbers, also has to go to 64 bit)

As a hotfix that seems to work I disabled the seq_to_skip features. It wasn't used by any analyzer or policy script (Note, that seq_to_skip is different from skip_deliveries). Hotfix is in topic/gregor/reassembler-hotfix

[gregor]

Filed a merge request for the hotfix (#404). This ticket should remain open though, since it flags that more work is needed to really fix the problem (instead of avoiding it as my hotfix does).

[robin]

Moving some notes out of the source and in here:

// The Reassembler uses 32 bit ints for keeping track of sequence                                                                                                           
// numbers. This means that the seq numbers will become negative once we                                                                                                    
// exceed 2 GB of data. The Reassembler seems to mostly work despite negative                                                                                               
// sequence numbers, since seq_delta() will handle them gracefully. However,                                                                                                
// there are a couple of issues. E.g., seq_to_skip doesn't work (which is now                                                                                               
// disabled with an ifdef, since it wasn't used) Also, a check in                                                                                                           
// Undelivered() had a problem with negative sequence numbers.                                                                                                              
//                                                                                                                                                                          
// There are numerous counters (e.g., number of total bytes, etc.) that are                                                                                                 
// incorrect due to overflow too. However, these seem to be for informative                                                                                                 
// purposes only, so we currently ignore them.                                                                                                                              
//                                                                                                                                                                          
// There might be other problems hidden somewhere, that I haven't discovered                                                                                                
// yet......                                                                                                                                                                
//                                                                                                                                                                          
// Reassem.{cc|h} and other "Reassemblers" that inherit from it (e.g., Frag)                                                                                                
// need to be updated too.               

[seth]

Is this something that we should address for 2.1?