diff options
author | John Thacker <johnthacker@gmail.com> | 2022-09-11 10:40:25 -0400 |
---|---|---|
committer | A Wireshark GitLab Utility <gerald+gitlab-utility@wireshark.org> | 2022-09-13 23:45:01 +0000 |
commit | 56ea9816d9bb439e62eb77fa20e7220783404b83 (patch) | |
tree | a71f1f9456c77a51141c16357d7a41295b08dba8 /epan | |
parent | 8724c249e1cd2a91e726a682d85924a58ae10b18 (diff) |
tcp: Create new conversations on a SYN after a RST or FIN
If we get a SYN packet with the same sequence number as the current
conversation, but after a RST (or FIN) segment, create a new
conversation.
In such a case, this is probably a peer using the same sequence
number to retry a handshake that failed with a RST due to a half-open
connection. The RST (or ACK that caused a RST) would have an
out-of-window sequence number (or inapplicable ACK for the rest of
the conversation), which can disrupt the follow info, sequence analysis,
desegmentation, etc. unless we create a new conversation.
It could also, less likely, be a new connection after a connection close
that happened to reuse the same sequence number, in which case we also
want to clear out our conversation state.
If we haven't received a RST (or FIN), then consider it a retransmission.
Fix #18333 (also handles the cases of #16944 and #17616 more smoothly).
Diffstat (limited to 'epan')
-rw-r--r-- | epan/dissectors/packet-tcp.c | 43 |
1 files changed, 34 insertions, 9 deletions
diff --git a/epan/dissectors/packet-tcp.c b/epan/dissectors/packet-tcp.c index 8333b2a99c..0ffa8f0491 100644 --- a/epan/dissectors/packet-tcp.c +++ b/epan/dissectors/packet-tcp.c @@ -7551,15 +7551,31 @@ dissect_tcp(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void* data _U_) /* If this is a SYN packet, then check if its seq-nr is different * from the base_seq of the retrieved conversation. If this is the * case, create a new conversation with the same addresses and ports - * and set the TA_PORTS_REUSED flag. If the seq-nr is the same as - * the base_seq, then restore some flow values to avoid buggy - * analysis. In the latter case, it will also be marked as a - * retransmission later. + * and set the TA_PORTS_REUSED flag. (XXX: There is a small chance + * that this is an old duplicate SYN received after the connection + * is ESTABLISHED on both sides, the other side will respond with + * an appropriate ACK, and this SYN ought to be ignored rather than + * create a new conversation.) + * + * If the seq-nr is the same as the base_seq, it might be a simple + * retransmission, reattempting a handshake that was reset (due + * to a half-open connection) with the same sequence number, or + * (unlikely) a new connection that happens to use the same sequence + * number as the previous one. + * + * If we have received a RST or FIN on the retrieved conversation, + * create a new conversation in order to clear out the follow info, + * sequence analysis, desegmentation, etc. + * If not, it's probably a retransmission, and will be marked + * as one later, but restore some flow values to reduce the + * sequence analysis warnings if our capture file is missing a RST + * or FIN segment that was present on the network. + * * XXX - Is this affected by MPTCP which can use multiple SYNs? */ if (tcpd != NULL && (tcph->th_flags & (TH_SYN|TH_ACK)) == TH_SYN) { if (tcpd->fwd->static_flags & TCP_S_BASE_SEQ_SET) { - if(tcph->th_seq!=tcpd->fwd->base_seq) { + if(tcph->th_seq!=tcpd->fwd->base_seq || (tcpd->conversation_completeness & TCP_COMPLETENESS_RST) || (tcpd->conversation_completeness & TCP_COMPLETENESS_FIN)) { if (!(pinfo->fd->visited)) { conv=conversation_new(pinfo->num, &pinfo->src, &pinfo->dst, CONVERSATION_TCP, pinfo->srcport, pinfo->destport, 0); @@ -7593,10 +7609,12 @@ dissect_tcp(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void* data _U_) /* If this is a SYN/ACK packet, then check if its seq-nr is different * from the base_seq of the retrieved conversation. If this is the - * case, try to find a conversation with the same addresses and ports - * and set the TA_PORTS_REUSED flag. If the seq-nr is the same as - * the base_seq, then do nothing so it will be marked as a retrans- - * mission later. + * case, set the TA_PORTS_REUSED flag and override the base seq. + * (XXX: Should this create a new conversation, as above with a + * SYN packet? We might have received the new connection's SYN/ACK before + * the SYN packet, or the SYN might be missing from the capture file.) + * If the seq-nr is the same as the base_seq, then do nothing so it + * will be marked as a retransmission later. * XXX - Is this affected by MPTCP which can use multiple SYNs? */ if (tcpd != NULL && (tcph->th_flags & (TH_SYN|TH_ACK)) == (TH_SYN|TH_ACK)) { @@ -7604,6 +7622,10 @@ dissect_tcp(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void* data _U_) (tcph->th_seq != tcpd->fwd->base_seq)) { /* the retrieved conversation might have a different base_seq (issue 16944) */ + /* XXX: Shouldn't this create a new conversation? Changing the + * base_seq will change how the previous packets in the conversation + * are processed in the second pass. + */ tcpd->fwd->base_seq = tcph->th_seq; if(!tcpd->ta) @@ -7816,6 +7838,9 @@ dissect_tcp(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void* data _U_) } /* RST */ + /* XXX: A RST segment should be validated (RFC 9293 3.5.3), + * and if not valid should not change the conversation state. + */ if(tcph->th_flags&(TH_RST)) { conversation_completeness |= TCP_COMPLETENESS_RST; } |