Merge remote-tracking branch 'origin/topic/jsiwek/tcp-improvements'

* origin/topic/jsiwek/tcp-improvements:
  Fix file_over_new_connection event to trigger when entire file is missed.
  Improve TCP connection size reporting for half-open connections.
  Improve gap reporting in TCP connections that never see data.
  Improve TCP FIN retransmission handling.

BIT-1119

scripts/base/init-bare.bro

@@ -2860,6 +2860,12 @@ global load_sample_freq = 20 &redef;
 ## .. bro:see:: gap_report
 const gap_report_freq = 1.0 sec &redef;
 
+## Whether to attempt to automatically detect SYN/FIN/RST-filtered trace
+## and not report missing segments for such connections.
+## If this is enabled, then missing data at the end of connections may not
+## be reported via :bro:see:`content_gap`.
+const detect_filtered_trace = F &redef;
+
 ## Whether we want :bro:see:`content_gap` and :bro:see:`gap_report` for partial
 ## connections. A connection is partial if it is missing a full handshake. Note
 ## that gap reports for partial connections might not be reliable.

src/analyzer/protocol/tcp/TCP.cc

@@ -373,14 +373,11 @@ void TCP_Analyzer::ProcessSYN(const IP_Hdr* ip, const struct tcphdr* tp,
 void TCP_Analyzer::ProcessFIN(double t, TCP_Endpoint* endpoint,
 				int& seq_len, uint32 base_seq)
 	{
-	if ( endpoint->FIN_cnt == 0 )
-		{
-		++seq_len;	// FIN consumes a byte of sequence space
-		++endpoint->FIN_cnt;	// remember that we've seen a FIN
-		}
+	++seq_len;  // FIN consumes a byte of sequence space.
+	++endpoint->FIN_cnt;  // remember that we've seen a FIN
 
-	else if ( t < endpoint->last_time + tcp_storm_interarrival_thresh &&
-		  ++endpoint->FIN_cnt == tcp_storm_thresh )
+	if ( t < endpoint->last_time + tcp_storm_interarrival_thresh &&
+	     endpoint->FIN_cnt == tcp_storm_thresh )
 		Weird("FIN_storm");
 
 	// Remember the relative seq in FIN_seq.

src/analyzer/protocol/tcp/TCP_Endpoint.cc

@@ -161,6 +161,13 @@ void TCP_Endpoint::SetState(EndpointState new_state)
 
 bro_int_t TCP_Endpoint::Size() const
 	{
+	if ( prev_state == TCP_ENDPOINT_SYN_SENT && state == TCP_ENDPOINT_RESET &&
+	     peer->state == TCP_ENDPOINT_INACTIVE && ! NoDataAcked() )
+		// This looks like a half-open connection was discovered and aborted.
+		// Sequence numbers could be misleading if used in context of data size
+		// and there was never a chance for this endpoint to send data anyway.
+		return 0;
+
 	bro_int_t size;
 
 	uint64 last_seq_64 = (uint64(last_seq_high) << 32) | last_seq;

src/analyzer/protocol/tcp/TCP_Reassembler.cc

@@ -178,7 +178,7 @@ void TCP_Reassembler::Undelivered(int up_to_seq)
 		// to this method and only if this condition is not true).
 		reporter->InternalError("Calling Undelivered for data that has already been delivered (or has already been marked as undelivered");
 
-	if ( last_reassem_seq == 1 &&
+	if ( BifConst::detect_filtered_trace && last_reassem_seq == 1 &&
 	     (endpoint->FIN_cnt > 0 || endpoint->RST_cnt > 0 ||
 	      peer->FIN_cnt > 0 || peer->RST_cnt > 0) )
 		{

src/const.bif

@@ -5,6 +5,7 @@
 const ignore_keep_alive_rexmit: bool;
 const skip_http_data: bool;
 const use_conn_size_analyzer: bool;
+const detect_filtered_trace: bool;
 const report_gaps_for_partial: bool;
 const exit_only_after_terminate: bool;
 

src/file_analysis/File.cc

@@ -103,7 +103,6 @@ File::~File()
 	DBG_LOG(DBG_FILE_ANALYSIS, "Destroying File object %s", id.c_str());
 	Unref(val);
 
-	// Queue may not be empty in the case where only content gaps were seen.
 	while ( ! fonc_queue.empty() )
 		{
 		delete_vals(fonc_queue.front().second);
@@ -460,20 +459,27 @@ void File::FileEvent(EventHandlerPtr h)
 	FileEvent(h, vl);
 	}
 
+static void flush_file_event_queue(queue<pair<EventHandlerPtr, val_list*> >& q)
+	{
+	while ( ! q.empty() )
+		{
+		pair<EventHandlerPtr, val_list*> p = q.front();
+		mgr.QueueEvent(p.first, p.second);
+		q.pop();
+		}
+	}
+
 void File::FileEvent(EventHandlerPtr h, val_list* vl)
 	{
+	if ( h == file_state_remove )
+		flush_file_event_queue(fonc_queue);
+
 	mgr.QueueEvent(h, vl);
 
 	if ( h == file_new )
 		{
 		did_file_new_event = true;
-
-		while ( ! fonc_queue.empty() )
-			{
-			pair<EventHandlerPtr, val_list*> p = fonc_queue.front();
-			mgr.QueueEvent(p.first, p.second);
-			fonc_queue.pop();
-			}
+		flush_file_event_queue(fonc_queue);
 		}
 
 	if ( h == file_new || h == file_timeout || h == file_extraction_limit )

testing/btest/Baseline/core.tcp.fin-retransmit/out

@@ -0,0 +1,2 @@
+[size=0, state=5, num_pkts=3, num_bytes_ip=156, flow_label=0]
+[size=0, state=6, num_pkts=2, num_bytes_ip=92, flow_label=0]

testing/btest/Baseline/core.tcp.miss-end-data/conn.log

@@ -0,0 +1,10 @@
+#separator \x09
+#set_separator	,
+#empty_field	(empty)
+#unset_field	-
+#path	conn
+#open	2014-01-24-22-19-38
+#fields	ts	uid	id.orig_h	id.orig_p	id.resp_h	id.resp_p	proto	service	duration	orig_bytes	resp_bytes	conn_state	local_orig	missed_bytes	history	orig_pkts	orig_ip_bytes	resp_pkts	resp_ip_bytes	tunnel_parents
+#types	time	string	addr	port	addr	port	enum	string	interval	count	count	string	bool	count	string	count	count	count	count	table[string]
+1331764471.664131	CXWv6p3arKYeMETxOg	192.168.122.230	60648	77.238.160.184	80	tcp	http	10.048360	538	2902	SF	-	2902	ShADafF	5	750	4	172	(empty)
+#close	2014-01-24-22-19-38

testing/btest/Baseline/core.tcp.miss-end-data/out

@@ -0,0 +1 @@
+content_gap, [orig_h=192.168.122.230, orig_p=60648/tcp, resp_h=77.238.160.184, resp_p=80/tcp], F, 1, 2902

testing/btest/core/tcp/fin-retransmit.bro

@@ -0,0 +1,8 @@
+# @TEST-EXEC: bro -b -r $TRACES/tcp/fin_retransmission.pcap %INPUT >out
+# @TEST-EXEC: btest-diff out
+
+event connection_state_remove(c: connection)
+    {
+    print c$orig;
+    print c$resp;
+    }

testing/btest/core/tcp/miss-end-data.bro

@@ -0,0 +1,10 @@
+# @TEST-EXEC: bro -r $TRACES/tcp/miss_end_data.pcap %INPUT >out
+# @TEST-EXEC: btest-diff out
+# @TEST-EXEC: btest-diff conn.log
+
+redef report_gaps_for_partial = T;
+
+event content_gap(c: connection, is_orig: bool, seq: count, length: count)
+	{
+	print "content_gap", c$id, is_orig, seq, length;
+	}