Bug #7238
openapplayer: protocol flows are miscounted in case of error
Description
The flow counter is not incremented if an applayer parser encountered an error. This needs to be fixed.
Updated by Philippe Antoine 4 months ago
Could you expand a bit more ?
Flow counters are incremented on final protocol detection.
Protocol detection has many (corner/edge) cases, and there are cases where the count is not right...
Which case are you talking about ?
First question : is this about UDP or TCP ?
Updated by Shivani Bhardwaj 4 months ago
Philippe Antoine wrote in #note-1:
Could you expand a bit more ?
First question : is this about UDP or TCP ?
It is about TCP.
Flow counters are incremented on final protocol detection.
Protocol detection has many (corner/edge) cases, and there are cases where the count is not right...
Which case are you talking about ?
I'm talking about the following case:
Let's assume that the RFC XXX of a ProtocolP
defines that its:
- header is 4 bytes in total
- byte 1 tells protocol version
- byte 2 tells the type of request/response
- bytes 3 and 4 of its header tell how big the entire fragment is
- rest of the bytes are the fragment data
Request 1 :
-------------------------- | Header (4B) | <- this tells that the total length of the fragment is 20 bytes -------------------------- | | | | | DATA | | | --------------------------
Observation: Request 1 is hence completed.
Request 2 :
-------------------------- | | | | | DATA | | | --------------------------Observation:
- The header of this request gives a protocol version and request type that are invalid so the protocol parser rejects it.
- As there are a lot of such invalid requests/responses received by
P
's parser, the error count ( app_layer.error.p.parser ) grows quite large. - Victor argues that if any data reached
P
's parser, it means that it was detected asP
so the flow count ofP
should also reflect that.[0] - However, in
master
we do not tend to update the flow counter if applayer parser errored out for some reason.[1]
[0] https://github.com/OISF/suricata/pull/11676#issuecomment-2324495507
[1] https://github.com/OISF/suricata/blob/master/src/app-layer.c#L660
Updated by Victor Julien 17 days ago
- Related to Bug #4917: tls: leading GAP in toserver direction leads to various issues added