The TcpReassembly class does not correctly handle the logic for closing connections with TCP RST commands.

[prudens]

Bug description

Describe the bug During testing, I found that if the client first sends a FIN command to the server and then immediately sends an RST command, the server will immediately close the TCP connection without responding.

However, in our code logic, since the sender send the FIN command, it will enter this code branch.

    // handle FIN/RST packets that don't contain additional TCP data
    if (isFinOrRst && tcpPayloadSize == 0)
    {
        PCPP_LOG_DEBUG("Got FIN or RST packet without data on side " << sideIndex);

        handleFinOrRst(tcpReassemblyData, sideIndex, flowKey, isRst);
        return FIN_RSTWithNoData;
    }

then handleFinOrRst function will set gotFinOrRst=true.When the RST command arrives, it simply returns without handling it.

    // if this side already got FIN or RST packet before, ignore this packet as this side is considered closed
    if (tcpReassemblyData->twoSides[sideIndex].gotFinOrRst)
    {
        PCPP_LOG_DEBUG("Got a packet after FIN or RST were already seen on this side (" << sideIndex << "). Ignoring this packet");
        return Ignore_PacketOfClosedFlow;
    }

As result, he other side will keep opened state forever.

Expected behavior I added some code to check for this, and it works correctly now.

    // if this side already got FIN or RST packet before, ignore this packet as this side is considered closed
    if (twoSides_[sideIndex].gotFinOrRst)
    {
        if (!twoSides_[1 - sideIndex].gotFinOrRst && isRst)
        {
            handleFinOrRst(1 - sideIndex, isRst);
            return FIN_RSTWithNoData;
        }
        LOG_DEBUG("[{}] Got a packet after FIN or RST were already seen on this side ({}). Ignoring this packet", flowKey_, sideIndex);
        return Ignore_PacketOfClosedFlow;
    }

PcapPlusPlus versions tested on

PcapPlusPlus master branch

Other PcapPlusPlus version (if applicable)

No response

Operating systems tested on

Linux

Other operation systems (if applicable)

No response

Compiler version

Clang 15

Packet capture backend (if applicable)

libpcap

[seladb]

@prudens the purpose of TCP reassmebly is not to decide which packet to send to each side. It simply tries to reconstruct the TCP messages from a given packet flow between 2 sides.

In the case you described - once TCP reassembly sees a FIN packet, it concludes the flow has been closed, no data packets should arrive on it, and it stops processing packets on this flow. When the RST packet comes after the FIN is ignores it because the flow has been closed.

Please let me know if it makes sense, or maybe I'm missing something?

[prudens]

@seladb Yes, the side that sends the FIN can close normally, but the close flag will not be automatically triggered. This is because it needs the other side to also set gotFinOrRst. However, due to the RST command, the server side will not send any TCP packets. Therefore, my solution is to directly close the other side when either side receives an RST command. This allows the entire TCP session to be received immediately and triggers the m_OnConnEnd callback function.

[seladb]

@prudens I think I understand now. I guess we can support this edge case. Would you consider opening a PR with a fix? We'd also want to write a test for it, and it seems you already have an example pcap file that we can use for these tests

[prudens]

I will open a PR to fix it this week. I found a new bug in this class and will fix it as well.