Refined state machine update placement to (1) properly deal with gaps capped

by clean FIN handshakes, and (1) fix failure to detect split routing.

Fixed typo flagged by Pierre Lalet.

src/analyzer/protocol/tcp/TCP.cc

@@ -1321,14 +1321,6 @@ void TCP_Analyzer::DeliverPacket(int len, const u_char* data, bool is_orig,
 		PacketWithRST();
 		}
 
-	int32 delta_last = update_last_seq(endpoint, seq_one_past_segment, flags, len);
-	endpoint->last_time = current_timestamp;
-
-	int do_close;
-	int gen_event;
-	UpdateStateMachine(current_timestamp, endpoint, peer, base_seq, ack_seq,
-	                   len, delta_last, is_orig, flags, do_close, gen_event);
-
 	uint64 rel_ack = 0;
 
 	if ( flags.ACK() )
@@ -1361,10 +1353,25 @@ void TCP_Analyzer::DeliverPacket(int len, const u_char* data, bool is_orig,
 				// Don't trust ack's in RST packets.
 				update_ack_seq(peer, ack_seq);
 			}
-
-		peer->AckReceived(rel_ack);
 		}
 
+	int32 delta_last = update_last_seq(endpoint, seq_one_past_segment, flags, len);
+	endpoint->last_time = current_timestamp;
+
+	int do_close;
+	int gen_event;
+	UpdateStateMachine(current_timestamp, endpoint, peer, base_seq, ack_seq,
+	                   len, delta_last, is_orig, flags, do_close, gen_event);
+
+	if ( flags.ACK() )
+		// We wait on doing this until we've updated the state
+		// machine so that if the ack reveals a content gap,
+		// we can tell whether it came at the very end of the
+		// connection (in a FIN or RST).  Those gaps aren't
+		// reliable - especially those for RSTs - and we refrain
+		// from flagging them in the connection history.
+		peer->AckReceived(rel_ack);
+
 	if ( tcp_packet )
 		GeneratePacketEvent(rel_seq, rel_ack, data, len, caplen, is_orig,
 		                    flags);

src/analyzer/protocol/tcp/TCP_Endpoint.h

@@ -175,7 +175,7 @@ public:
 	// Called to inform endpoint that it has offered a zero window.
 	void ZeroWindow();
 
-	// Called to inform endpoint that it a gap occurred.
+	// Called to inform endpoint that a gap occurred.
 	void Gap(uint64 seq, uint64 len);
 
 	// Returns true if the data was used (and hence should be recorded

src/analyzer/protocol/tcp/TCP_Reassembler.cc

@@ -112,29 +112,35 @@ void TCP_Reassembler::SetContentsFile(BroFile* f)
 	record_contents_file = f;
 	}
 
-static inline bool established(const TCP_Endpoint* a, const TCP_Endpoint* b)
+static inline bool is_clean(const TCP_Endpoint* a)
 	{
-	return a->state == TCP_ENDPOINT_ESTABLISHED &&
+	return a->state == TCP_ENDPOINT_ESTABLISHED ||
-	       b->state == TCP_ENDPOINT_ESTABLISHED;
+		(a->state == TCP_ENDPOINT_CLOSED &&
+		 a->prev_state == TCP_ENDPOINT_ESTABLISHED);
+	}
+
+static inline bool established_or_cleanly_closing(const TCP_Endpoint* a,
+							const TCP_Endpoint* b)
+	{
+	return is_clean(a) && is_clean(b);
 	}
 
 static inline bool report_gap(const TCP_Endpoint* a, const TCP_Endpoint* b)
 	{
 	return content_gap &&
-	       ( BifConst::report_gaps_for_partial || established(a, b) );
+	       ( BifConst::report_gaps_for_partial ||
+	         established_or_cleanly_closing(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
+	// are in a cleanly established or closing  state. In
-	// states, these can arise falsely due to things
+	// 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 ( established(endp, endp->peer) )
+	if ( established_or_cleanly_closing(endp, endp->peer) )
 		endp->Gap(seq, len);
 
 	if ( report_gap(endp, endp->peer) )

testing/btest/Baseline/core.tcp.large-file-reassembly/conn.log

@@ -3,10 +3,10 @@
 #empty_field	(empty)
 #unset_field	-
 #path	conn
-#open	2019-04-17-20-41-29
+#open	2019-04-19-18-10-57
 #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	local_resp	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	bool	count	string	count	count	count	count	set[string]
 1395939406.175845	ClEkJM2Vm5giqnMf4h	192.168.56.1	59763	192.168.56.101	63988	tcp	ftp-data	0.001676	0	270	SF	-	-	0	ShAdfFa	5	272	4	486	-
-1395939411.361078	C4J4Th3PJpwUYZZ6gc	192.168.56.1	59764	192.168.56.101	37150	tcp	ftp-data	150.496065	0	5416666670	SF	-	-	4675708816	ShAdgfFa	13	688	12	24454	-
+1395939411.361078	C4J4Th3PJpwUYZZ6gc	192.168.56.1	59764	192.168.56.101	37150	tcp	ftp-data	150.496065	0	5416666670	SF	-	-	5416642848	ShAdgfFa	13	688	12	24454	-
 1395939399.984671	CHhAvVGS1DHFjwGM9	192.168.56.1	59762	192.168.56.101	21	tcp	ftp	169.634297	104	1041	SF	-	-	0	ShAdDaFf	31	1728	18	1985	-
-#close	2019-04-17-20-41-29
+#close	2019-04-19-18-10-57

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

@@ -3,8 +3,8 @@
 #empty_field	(empty)
 #unset_field	-
 #path	conn
-#open	2016-07-13-16-13-02
+#open	2019-04-19-18-11-06
 #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	local_resp	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	bool	count	string	count	count	count	count	set[string]
-1331764471.664131	CHhAvVGS1DHFjwGM9	192.168.122.230	60648	77.238.160.184	80	tcp	http	10.048360	538	2902	SF	-	-	2902	ShADafF	5	750	4	172	-
+1331764471.664131	CHhAvVGS1DHFjwGM9	192.168.122.230	60648	77.238.160.184	80	tcp	http	10.048360	538	2902	SF	-	-	2902	ShADafgF	5	750	4	172	-
-#close	2016-07-13-16-13-02
+#close	2019-04-19-18-11-07

testing/btest/Baseline/core.tunnels.gtp.outer_ip_frag/conn.log

@@ -3,9 +3,9 @@
 #empty_field	(empty)
 #unset_field	-
 #path	conn
-#open	2016-07-13-16-13-10
+#open	2019-04-19-18-10-49
 #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	local_resp	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	bool	count	string	count	count	count	count	set[string]
-1333458850.364667	ClEkJM2Vm5giqnMf4h	10.131.47.185	1923	79.101.110.141	80	tcp	http	0.069783	2100	56702	SF	-	-	0	ShADadfF	27	3204	41	52594	CHhAvVGS1DHFjwGM9
+1333458850.364667	ClEkJM2Vm5giqnMf4h	10.131.47.185	1923	79.101.110.141	80	tcp	http	0.069783	2100	56702	SF	-	-	5760	ShADadfgF	27	3204	41	52594	CHhAvVGS1DHFjwGM9
 1333458850.364667	CHhAvVGS1DHFjwGM9	239.114.155.111	2152	63.94.149.181	2152	udp	gtpv1	0.069813	3420	52922	SF	-	-	0	Dd	27	4176	41	54070	-
-#close	2016-07-13-16-13-10
+#close	2019-04-19-18-10-49