Fix issue w/ TCP reassembler not delivering some segments.

For example, if we have a connection between TCP "A" and TCP "B" and "A"
sends segments "1" and "2", but we don't see the first and then the next
acknowledgement from "B" is for everything up to, and including, "2",
the gap would be reported to include both segments instead of just the
first and then delivering the second.  Put generally: any segments that
weren't yet delivered because they're waiting for an earlier gap to be
filled would be dropped when an ACK comes in that includes the gap as
well as those pending segments.  (If a distinct ACK was seen for just
the gap, that situation would have worked).

Addresses BIT-1246.
This commit is contained in:
Jon Siwek 2014-09-11 10:47:56 -05:00
parent f97f58e9db
commit f1cef9d2a9
7 changed files with 96 additions and 39 deletions

View file

@ -117,6 +117,45 @@ void TCP_Reassembler::SetContentsFile(BroFile* f)
record_contents_file = f;
}
static inline bool established(const TCP_Endpoint* a, const TCP_Endpoint* b)
{
return a->state == TCP_ENDPOINT_ESTABLISHED &&
b->state == TCP_ENDPOINT_ESTABLISHED;
}
static inline bool report_gap(const TCP_Endpoint* a, const TCP_Endpoint* b)
{
return content_gap &&
( BifConst::report_gaps_for_partial || established(a, b) );
}
void TCP_Reassembler::Gap(uint64 seq, uint64 len)
{
// Only report on content gaps for connections that
// are in a cleanly established state. In other
// states, these can arise falsely due to things
// like sequence number mismatches in RSTs, or
// unseen previous packets in partial connections.
// The one opportunity we lose here is on clean FIN
// handshakes, but Oh Well.
if ( report_gap(endp, endp->peer) )
{
val_list* vl = new val_list;
vl->append(dst_analyzer->BuildConnVal());
vl->append(new Val(IsOrig(), TYPE_BOOL));
vl->append(new Val(seq, TYPE_COUNT));
vl->append(new Val(len, TYPE_COUNT));
dst_analyzer->ConnectionEvent(content_gap, vl);
}
if ( type == Direct )
dst_analyzer->NextUndelivered(seq, len, IsOrig());
else
dst_analyzer->ForwardUndelivered(seq, len, IsOrig());
had_gap = true;
}
void TCP_Reassembler::Undelivered(uint64 up_to_seq)
{
@ -189,48 +228,33 @@ void TCP_Reassembler::Undelivered(uint64 up_to_seq)
if ( ! skip_deliveries )
{
// This can happen because we're processing a trace
// that's been filtered. For example, if it's just
// SYN/FIN data, then there can be data in the FIN
// packet, but it's undelievered because it's out of
// sequence.
uint64 seq = last_reassem_seq;
uint64 len = up_to_seq - last_reassem_seq;
// Only report on content gaps for connections that
// are in a cleanly established state. In other
// states, these can arise falsely due to things
// like sequence number mismatches in RSTs, or
// unseen previous packets in partial connections.
// The one opportunity we lose here is on clean FIN
// handshakes, but Oh Well.
if ( content_gap &&
(BifConst::report_gaps_for_partial ||
(endpoint->state == TCP_ENDPOINT_ESTABLISHED &&
peer->state == TCP_ENDPOINT_ESTABLISHED ) ) )
DataBlock* b = blocks;
// If we have blocks that begin below up_to_seq, deliver them.
while ( b )
{
val_list* vl = new val_list;
vl->append(dst_analyzer->BuildConnVal());
vl->append(new Val(IsOrig(), TYPE_BOOL));
vl->append(new Val(seq, TYPE_COUNT));
vl->append(new Val(len, TYPE_COUNT));
if ( b->seq < last_reassem_seq )
{
// Already delivered this block.
b = b->next;
continue;
}
dst_analyzer->ConnectionEvent(content_gap, vl);
if ( b->seq >= up_to_seq )
// Block is beyond what we need to process at this point.
break;
uint64 gap_at_seq = last_reassem_seq;
uint64 gap_len = b->seq - last_reassem_seq;
Gap(gap_at_seq, gap_len);
last_reassem_seq += gap_len;
BlockInserted(b);
b = b->next;
}
if ( type == Direct )
dst_analyzer->NextUndelivered(last_reassem_seq,
len, IsOrig());
else
{
dst_analyzer->ForwardUndelivered(last_reassem_seq,
len, IsOrig());
}
if ( up_to_seq > last_reassem_seq )
Gap(last_reassem_seq, up_to_seq - last_reassem_seq);
}
had_gap = true;
}
// We should record and match undelivered even if we are skipping

View file

@ -94,6 +94,7 @@ private:
DECLARE_SERIAL(TCP_Reassembler);
void Undelivered(uint64 up_to_seq);
void Gap(uint64 seq, uint64 len);
void RecordToSeq(uint64 start_seq, uint64 stop_seq, BroFile* f);
void RecordBlock(DataBlock* b, BroFile* f);

View file

@ -0,0 +1,4 @@
^J0.26 | 2012-08-24 15:10:04 -0700^J^J * Fixing update-changes, which could pick the wrong control file. (Robin Sommer)^J^J * Fixing GPG signing script. (Robin Sommer)^J^J0.25 | 2012-08-01 13:55:46 -0500^J^J * Fix configure script to exit with non-zero status on error (Jon Siwek)^J^J0.24 | 2012-07-05 12:50:43 -0700^J^J * Raise minimum required CMake version to 2.6.3 (Jon Siwek)^J^J * Adding script to delete old fully-merged branches. (Robin Sommer)^J^J0.23-2 | 2012-01-25 13:24:01 -0800^J^J * Fix a bro-cut error message. (Daniel Thayer)^J^J0.23 | 2012-01-11 12:16:11 -0800^J^J * Tweaks to release scripts, plus a new one for signing files.^J (Robin Sommer)^J^J0.22 | 2012-01-10 16:45:19 -0800^J^J * Tweaks for OpenBSD support. (Jon Siwek)^J^J * bro-cut extensions and fixes. (Robin Sommer)^J ^J - If no field names are given on the command line, we now pass through^J all fields. Adresses #657.^J^J - Removing some GNUism from awk script. Addresses #653.^J^J - Added option for time output in UTC. Addresses #668.^J^J - Added output field separator option -F. Addresses #649.^J^J - Fixing option -c: only some header lines were passed through^J
rather than all. (Robin Sommer)^J^J * Fix parallel make portability. (Jon Siwek)^J^J0.21-9 | 2011-11-07 05:44:14 -0800^J^J * Fixing compiler warnings. Addresses #388. (Jon Siwek)^J^J0.21-2 | 2011-11-02 18:12:13 -0700^J^J * Fix for misnaming temp file in update-changes script. (Robin Sommer)^J^J0.21-1 | 2011-11-02 18:10:39 -0700^J^J * Little fix for make-release script, which could pick out the wrong^J tag. (Robin Sommer)^J^J0.21 | 2011-10-27 17:40:45 -0700^J^J * Fixing bro-cut's usage message and argument error handling. (Robin Sommer)^J^J * Bugfix in update-changes script. (Robin Sommer)^J^J * update-changes now ignores commits it did itself. (Robin Sommer)^J^J * Fix a bug in the update-changes script. (Robin Sommer)^J^J * bro-cut now always installs to $prefix/bin by `make install`. (Jon Siwek)^J^J * Options to adjust time format for bro-cut. (Robin Sommer)^J^J The default with -d is now ISO format. The new option "-D <fmt>"^J specifies a custom strftime()-style format string. Alternatively,^J the environment variable BRO_CUT_TIMEFMT can set the format as^J well.^J^J * bro-cut now understands the field separator header. (Robin Sommer)^J^J * Renaming options -h/-H -> -c/-C, and doing some general cleanup.^J^J0.2 | 2011-10-25 19:53:57 -0700^J^J * Adding support for replacing version string in a setup.py. (Robin^J Sommer)^J^J * Change generated root cert DN indices format for RFC2253^J compliance. (Jon Siwek)^J^J * New tool devel-tool
<1448 byte gap>
thread library when necessary (e.g.^J PF_RING's libpcap) (Jon Siwek)^J^J * Install binaries with an RPATH (Jon Siwek)^J^J * Workaround for FreeBSD CMake port missing debug flags (Jon Siwek)^J^J * Rewrite of the update-changes script. (Robin Sommer)^J^J0.1-1 | 2011-06-14 21:12:41 -0700^J^J * Add a script for generating Mozilla's CA list for the SSL analyzer.^J (Seth Hall)^J^J0.1 | 2011-04-01 16:28:22 -0700^J^J * Converting build process to CMake. (Jon Siwek)^J^J * Removing cf/hf/ca-* from distribution. The README has a note where^J to find them now. (Robin Sommer)^J^J * General cleanup. (Robin Sommer)^J^J * Initial import of bro/aux from SVN r7088. (Jon Siwek)^J

View file

@ -3,9 +3,13 @@
#empty_field (empty)
#unset_field -
#path modbus
#open 2013-08-26-19-04-11
#open 2014-09-11-15-00-05
#fields ts uid id.orig_h id.orig_p id.resp_h id.resp_p func exception
#types time string addr port addr port string string
1153491909.414125 CXWv6p3arKYeMETxOg 192.168.66.235 2582 166.161.16.230 502 unknown-156 -
1153491911.997264 CXWv6p3arKYeMETxOg 192.168.66.235 2582 166.161.16.230 502 unknown-160 -
1153491913.013726 CXWv6p3arKYeMETxOg 192.168.66.235 2582 166.161.16.230 502 unknown-162 -
#close 2013-08-26-19-04-11
1153491923.091742 CXWv6p3arKYeMETxOg 192.168.66.235 2582 166.161.16.230 502 unknown-175 -
1153491923.091742 CXWv6p3arKYeMETxOg 192.168.66.235 2582 166.161.16.230 502 unknown-179 -
1153491923.623460 CXWv6p3arKYeMETxOg 192.168.66.235 2582 166.161.16.230 502 unknown-165 -
#close 2014-09-11-15-00-05

Binary file not shown.

View file

@ -0,0 +1,24 @@
# @TEST-EXEC: bro -r $TRACES/http/entity_gap2.trace %INPUT
# @TEST-EXEC: btest-diff entity_data
# @TEST-EXEC: btest-diff extract_files/file0
global f = open("entity_data");
global fn = 0;
event http_entity_data(c: connection, is_orig: bool, length: count,
data: string)
{
print f, data;
}
event content_gap(c: connection, is_orig: bool, seq: count, length: count)
{
print f, fmt("<%d byte gap>", length);
}
event file_new(f: fa_file)
{
Files::add_analyzer(f, Files::ANALYZER_EXTRACT,
[$extract_filename=fmt("file%d", fn)]);
++fn;
}