]> git.ipfire.org Git - thirdparty/suricata.git/commitdiff
app-layer-parser: give direction to progress func
authorEric Leblond <el@stamus-networks.com>
Fri, 20 Jan 2023 09:35:59 +0000 (10:35 +0100)
committerVictor Julien <vjulien@oisf.net>
Sat, 1 Apr 2023 05:07:33 +0000 (07:07 +0200)
The tx progress functions are expecting a direction and were given
a flow flags. As a result, they were not reporting correctly the
status if a DetectRunScratchPad flow_flags was containing some other
bits in the flag.

One case was when a signature was alterating the stream analysis
and triggering the addition of the STREAM_FLUSH flags.

The consequences are quite severe as the transactions are pilling
up waiting to be inspected causing sometimes a 10x performance hit
on pcap parsing. Also as the inspection was not done, Suricata is
missing a part of the alerts.

This was discovered when working on the following set of signatures:

alert ssh $HOME_NET any -> any any (msg:"pcre without content"; pcre:"/rabbit/"; sid:1; rev:1;)
alert smb $HOME_NET any -> any any (msg:"smb share content"; smb.share; content:"C"; sid:2; rev:1;)

When the first one is present the second is not triggering even
though the pcap file had no ssh inside. This is due to the fact
that the ssh signature was triggering the STREAM_FLUSH flag to
be set on the flowflags of the packet. But the application
layer will ask the smb state progress via

r = alp_ctx.ctxs[FlowGetProtoMapping(ipproto)][alproto].
        StateGetProgress(alstate, flags);

passing it the flow flags but the smb function is expecting
a direction so we end up in a unplanned case

pub unsafe extern "C" fn rs_smb_tx_get_alstate_progress(tx: *mut ffi::c_void,
                                                  direction: u8)
...
if direction == Direction::ToServer as u8 && tx.request_done {

This leads the signature to not be evaluated correctly.

Ticket: #5799

src/app-layer-parser.c

index e5732cb260133738074ee7b542a3715ae2c72370..473ce277d3a4709360a24dd06ae12cfb0bd5f7d2 100644 (file)
@@ -1073,8 +1073,9 @@ int AppLayerParserGetStateProgress(uint8_t ipproto, AppProto alproto,
         r = alp_ctx.ctxs[FLOW_PROTO_DEFAULT][alproto].
             StateGetProgressCompletionStatus(flags);
     } else {
-        r = alp_ctx.ctxs[FlowGetProtoMapping(ipproto)][alproto].
-            StateGetProgress(alstate, flags);
+        uint8_t direction = flags & (STREAM_TOCLIENT | STREAM_TOSERVER);
+        r = alp_ctx.ctxs[FlowGetProtoMapping(ipproto)][alproto].StateGetProgress(
+                alstate, direction);
     }
     SCReturnInt(r);
 }