diff --git a/src/analyzer/protocol/tcp/TCP.cc b/src/analyzer/protocol/tcp/TCP.cc index 595fe8e6b6..1f5309a1b9 100644 --- a/src/analyzer/protocol/tcp/TCP.cc +++ b/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); diff --git a/src/analyzer/protocol/tcp/TCP_Endpoint.h b/src/analyzer/protocol/tcp/TCP_Endpoint.h index 4c1cf64d6c..b17cfef700 100644 --- a/src/analyzer/protocol/tcp/TCP_Endpoint.h +++ b/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 diff --git a/src/analyzer/protocol/tcp/TCP_Reassembler.cc b/src/analyzer/protocol/tcp/TCP_Reassembler.cc index 5a82197054..e91f400d76 100644 --- a/src/analyzer/protocol/tcp/TCP_Reassembler.cc +++ b/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 && - b->state == TCP_ENDPOINT_ESTABLISHED; + return a->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 - // states, these can arise falsely due to things + // are in a cleanly established or closing 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 ( established(endp, endp->peer) ) + if ( established_or_cleanly_closing(endp, endp->peer) ) endp->Gap(seq, len); if ( report_gap(endp, endp->peer) ) diff --git a/testing/btest/Baseline/core.tcp.large-file-reassembly/conn.log b/testing/btest/Baseline/core.tcp.large-file-reassembly/conn.log index 3a997687d1..fbb4a71369 100644 --- a/testing/btest/Baseline/core.tcp.large-file-reassembly/conn.log +++ b/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 diff --git a/testing/btest/Baseline/core.tcp.miss-end-data/conn.log b/testing/btest/Baseline/core.tcp.miss-end-data/conn.log index b33aec3366..e8d6102398 100644 --- a/testing/btest/Baseline/core.tcp.miss-end-data/conn.log +++ b/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 - -#close 2016-07-13-16-13-02 +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 2019-04-19-18-11-07 diff --git a/testing/btest/Baseline/core.tunnels.gtp.outer_ip_frag/conn.log b/testing/btest/Baseline/core.tunnels.gtp.outer_ip_frag/conn.log index 4c598b386d..dfa705f258 100644 --- a/testing/btest/Baseline/core.tunnels.gtp.outer_ip_frag/conn.log +++ b/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