Suricata

I do not think this bug is bad.

I think it only happens in midstream with fin+syn packets after some first midstream packet

Impact would be some structures kept in memory a bit longer than needed...

@Shivani Bhardwaj you added the assertion in https://github.com/OISF/suricata/commit/d096b989c813dcc71154a9080cadaee927e45d24

It is not true if ssn->state TCP_CLOSING

What should we do ?
- remove the assertion
- use stream_flags | STREAM_EOF in AppLayerHandleTCPData
- have StreamGetAppLayerFlags consider ssn->state >= TCP_CLOSING. instead of ssn->state TCP_CLOSED

Philippe Antoine wrote in #note-6:

@Shivani Bhardwaj you added the assertion in https://github.com/OISF/suricata/commit/d096b989c813dcc71154a9080cadaee927e45d24

It is not true if ssn->state TCP_CLOSING

Thank you, Philippe! Indeed your analysis is correct.

What should we do ?
- remove the assertion
- use stream_flags | STREAM_EOF in AppLayerHandleTCPData
- have StreamGetAppLayerFlags consider ssn->state >= TCP_CLOSING. instead of ssn->state TCP_CLOSED

TCP_CLOSING seems to not be treated as EOF ref: in StreamTcpPacketSetState, flow's state is updated to "established" if TCP state was "closing" so perhaps removal of assertion is the best course of action. Lmk wdyt?

Philippe Antoine wrote in #note-8:

Ok, should you the PR, or should I ?

Please check: https://github.com/OISF/suricata/pull/13762