From 5b3573394edbcf6c8926e84c21d84c13faa412e7 Mon Sep 17 00:00:00 2001 From: Jon Siwek Date: Fri, 24 Jan 2014 15:51:58 -0600 Subject: [PATCH] Improve TCP FIN retransmission handling. In the case multiple FIN packets are seen from a TCP endpoint (e.g. when one is retransmitted), only the first counted towards a byte in the sequence space. This could cause a subsequent FIN packet to induce an incorrect wrap around in the sequence numbers (e.g. the retransmitted FIN packet now is one sequence number behind the the first) and misleadingly large connection sizes. The change is to always treat a FIN packet as counting one byte in to the sequence space. --- src/analyzer/protocol/tcp/TCP.cc | 11 ++++------- .../btest/Baseline/core.tcp.fin-retransmit/out | 2 ++ testing/btest/Traces/tcp/fin_retransmission.pcap | Bin 0 -> 434 bytes testing/btest/core/tcp/fin-retransmit.bro | 8 ++++++++ 4 files changed, 14 insertions(+), 7 deletions(-) create mode 100644 testing/btest/Baseline/core.tcp.fin-retransmit/out create mode 100644 testing/btest/Traces/tcp/fin_retransmission.pcap create mode 100644 testing/btest/core/tcp/fin-retransmit.bro diff --git a/src/analyzer/protocol/tcp/TCP.cc b/src/analyzer/protocol/tcp/TCP.cc index aefc5a1808..57c4ebef18 100644 --- a/src/analyzer/protocol/tcp/TCP.cc +++ b/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. diff --git a/testing/btest/Baseline/core.tcp.fin-retransmit/out b/testing/btest/Baseline/core.tcp.fin-retransmit/out new file mode 100644 index 0000000000..8afb8222c9 --- /dev/null +++ b/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] diff --git a/testing/btest/Traces/tcp/fin_retransmission.pcap b/testing/btest/Traces/tcp/fin_retransmission.pcap new file mode 100644 index 0000000000000000000000000000000000000000..1e17844af55bcf6ccb65c637d5f4f9f3f1173321 GIT binary patch literal 434 zcmca|c+)~A1{MYw`2U}Qff2|lH#!zHv5}R*56A}LBZdF0Vh=^guQHL?!@=Onz~Ery z<-p*;#w*OlC|4@D{Gim6=W*o?i&Wthj-opy7^s$ko-S+Td;crAns*h0-5Mz$i-j-G#!L7 z+^EsH59G$J+tA$zH1%eHpaX;bGLXwa0A$v$i+pgiFkGjR&jxaxMjo2$ZuY$_{20KZ Mz_~;MViE%b0C*yJumAu6 literal 0 HcmV?d00001 diff --git a/testing/btest/core/tcp/fin-retransmit.bro b/testing/btest/core/tcp/fin-retransmit.bro new file mode 100644 index 0000000000..42bf062f5a --- /dev/null +++ b/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; + }