mirror of
https://github.com/zeek/zeek.git
synced 2025-10-10 02:28:21 +00:00
Fix a case where c$resp$size is misrepresented. Addresses #730.
That field is based on TCP sequence numbers and on seeing a SYN followed by a failed RST injection response, the initial sequence number tracked the value in the injection (most likely zero) instead of value in subsequent SYN response. This could make c$resp$size be set to large values when it's not really. Also removed some dead code paths.
This commit is contained in:
parent
6603b851fe
commit
81ae68be16
5 changed files with 44 additions and 18 deletions
40
src/TCP.cc
40
src/TCP.cc
|
@ -382,7 +382,7 @@ void TCP_Analyzer::ProcessFIN(double t, TCP_Endpoint* endpoint,
|
|||
endpoint->FIN_seq = base_seq - endpoint->StartSeq() + seq_len;
|
||||
}
|
||||
|
||||
bool TCP_Analyzer::ProcessRST(double t, TCP_Endpoint* endpoint,
|
||||
void TCP_Analyzer::ProcessRST(double t, TCP_Endpoint* endpoint,
|
||||
const IP_Hdr* ip, uint32 base_seq,
|
||||
int len, int& seq_len)
|
||||
{
|
||||
|
@ -406,11 +406,9 @@ bool TCP_Analyzer::ProcessRST(double t, TCP_Endpoint* endpoint,
|
|||
}
|
||||
|
||||
PacketWithRST();
|
||||
|
||||
return true;
|
||||
}
|
||||
|
||||
int TCP_Analyzer::ProcessFlags(double t,
|
||||
void TCP_Analyzer::ProcessFlags(double t,
|
||||
const IP_Hdr* ip, const struct tcphdr* tp,
|
||||
uint32 tcp_hdr_len, int len, int& seq_len,
|
||||
TCP_Endpoint* endpoint, TCP_Endpoint* peer,
|
||||
|
@ -425,14 +423,11 @@ int TCP_Analyzer::ProcessFlags(double t,
|
|||
if ( flags.FIN() )
|
||||
ProcessFIN(t, endpoint, seq_len, base_seq);
|
||||
|
||||
if ( flags.RST() &&
|
||||
! ProcessRST(t, endpoint, ip, base_seq, len, seq_len) )
|
||||
return 0;
|
||||
if ( flags.RST() )
|
||||
ProcessRST(t, endpoint, ip, base_seq, len, seq_len);
|
||||
|
||||
if ( flags.ACK() )
|
||||
ProcessACK(endpoint, peer, ack_seq, is_orig, flags);
|
||||
|
||||
return 1;
|
||||
}
|
||||
|
||||
void TCP_Analyzer::TransitionFromInactive(double t, TCP_Endpoint* endpoint,
|
||||
|
@ -825,10 +820,27 @@ void TCP_Analyzer::UpdateClosedState(double t, TCP_Endpoint* endpoint,
|
|||
}
|
||||
}
|
||||
|
||||
void TCP_Analyzer::UpdateResetState(int len, TCP_Flags flags)
|
||||
void TCP_Analyzer::UpdateResetState(int len, TCP_Flags flags,
|
||||
TCP_Endpoint* endpoint, uint32 base_seq,
|
||||
uint32 last_seq)
|
||||
{
|
||||
if ( flags.SYN() )
|
||||
{
|
||||
Weird("SYN_after_reset");
|
||||
|
||||
if ( endpoint->prev_state == TCP_ENDPOINT_INACTIVE )
|
||||
{
|
||||
// Seq. numbers were initialized by a RST packet from this endpoint,
|
||||
// but now that a SYN is seen from it, that could mean the earlier
|
||||
// RST was spoofed/injected, so re-initialize. This mostly just
|
||||
// helps prevent misrepresentations of payload sizes that are based
|
||||
// on bad initial sequence values.
|
||||
endpoint->InitStartSeq(base_seq);
|
||||
endpoint->InitAckSeq(base_seq);
|
||||
endpoint->InitLastSeq(last_seq);
|
||||
}
|
||||
}
|
||||
|
||||
if ( flags.FIN() )
|
||||
Weird("FIN_after_reset");
|
||||
|
||||
|
@ -871,7 +883,7 @@ void TCP_Analyzer::UpdateStateMachine(double t,
|
|||
break;
|
||||
|
||||
case TCP_ENDPOINT_RESET:
|
||||
UpdateResetState(len, flags);
|
||||
UpdateResetState(len, flags, endpoint, base_seq, last_seq);
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
@ -996,10 +1008,8 @@ void TCP_Analyzer::DeliverPacket(int len, const u_char* data, bool is_orig,
|
|||
|
||||
int seq_len = len; // length in terms of sequence space
|
||||
|
||||
if ( ! ProcessFlags(t, ip, tp, tcp_hdr_len, len, seq_len,
|
||||
endpoint, peer, base_seq, ack_seq,
|
||||
orig_addr, is_orig, flags) )
|
||||
return;
|
||||
ProcessFlags(t, ip, tp, tcp_hdr_len, len, seq_len, endpoint, peer, base_seq,
|
||||
ack_seq, orig_addr, is_orig, flags);
|
||||
|
||||
uint32 last_seq = base_seq + seq_len;
|
||||
|
||||
|
|
|
@ -135,13 +135,13 @@ protected:
|
|||
void ProcessFIN(double t, TCP_Endpoint* endpoint, int& seq_len,
|
||||
uint32 base_seq);
|
||||
|
||||
bool ProcessRST(double t, TCP_Endpoint* endpoint, const IP_Hdr* ip,
|
||||
void ProcessRST(double t, TCP_Endpoint* endpoint, const IP_Hdr* ip,
|
||||
uint32 base_seq, int len, int& seq_len);
|
||||
|
||||
void ProcessACK(TCP_Endpoint* endpoint, TCP_Endpoint* peer,
|
||||
uint32 ack_seq, int is_orig, TCP_Flags flags);
|
||||
|
||||
int ProcessFlags(double t, const IP_Hdr* ip, const struct tcphdr* tp,
|
||||
void ProcessFlags(double t, const IP_Hdr* ip, const struct tcphdr* tp,
|
||||
uint32 tcp_hdr_len, int len, int& seq_len,
|
||||
TCP_Endpoint* endpoint, TCP_Endpoint* peer,
|
||||
uint32 base_seq, uint32 ack_seq,
|
||||
|
@ -186,7 +186,8 @@ protected:
|
|||
int delta_last, TCP_Flags flags,
|
||||
int& do_close);
|
||||
|
||||
void UpdateResetState(int len, TCP_Flags flags);
|
||||
void UpdateResetState(int len, TCP_Flags flags, TCP_Endpoint* endpoint,
|
||||
uint32 base_seq, uint32 last_seq);
|
||||
|
||||
void GeneratePacketEvent(TCP_Endpoint* endpoint, TCP_Endpoint* peer,
|
||||
uint32 base_seq, uint32 ack_seq,
|
||||
|
|
3
testing/btest/Baseline/core.tcp.rst-after-syn/.stdout
Normal file
3
testing/btest/Baseline/core.tcp.rst-after-syn/.stdout
Normal file
|
@ -0,0 +1,3 @@
|
|||
[orig_h=1.2.0.2, orig_p=2527/tcp, resp_h=1.2.0.3, resp_p=6649/tcp]
|
||||
orig:, [size=175, state=1, num_pkts=4, num_bytes_ip=395, flow_label=0]
|
||||
resp:, [size=0, state=6, num_pkts=5, num_bytes_ip=236, flow_label=0]
|
BIN
testing/btest/Traces/tcp/rst-inject-rae.trace
Normal file
BIN
testing/btest/Traces/tcp/rst-inject-rae.trace
Normal file
Binary file not shown.
12
testing/btest/core/tcp/rst-after-syn.bro
Normal file
12
testing/btest/core/tcp/rst-after-syn.bro
Normal file
|
@ -0,0 +1,12 @@
|
|||
# @TEST-EXEC: bro -b -r $TRACES/tcp/rst-inject-rae.trace %INPUT
|
||||
# @TEST-EXEC: btest-diff .stdout
|
||||
|
||||
# Mostly just checking that c$resp$size isn't huge due to the injected
|
||||
# RST packet being used to initialize sequence number in TCP analyzer.
|
||||
|
||||
event connection_state_remove(c: connection)
|
||||
{
|
||||
print c$id;
|
||||
print "orig:", c$orig;
|
||||
print "resp:", c$resp;
|
||||
}
|
Loading…
Add table
Add a link
Reference in a new issue