app-layer: add flag to skip detection on TX

Stamus team did discover a problem were a signature can shadow
other signatures.

For example, on a PCAP only containing Kerberos protocol and where the
following signature is matching:

alert krb5 $HOME_NET any -> any any (msg:"krb match"; krb5_cname; content:"marlo"; sid:3; rev:1;)

If we add the following signature to the list of signature

alert ssh $HOME_NET any -> any any (msg:"rr"; content:"rr"; flow:established,to_server; sid:4; rev:2;)

Then the Kerberos signature is not matching anymore.

To understand this case, we need some information:

- The krb5_cname is a to_client keyword
- The signal on ssh is to_server
- Kerberos has unidirectional transaction
- kerberos application state progress is a function always returning 1

As the two signatures are in opposite side, they end up in separate
sig group head.

Another fact is that, in the PCAP, the to_server side of the session
is sent first to the detection. It thus hit the sig group head of
the SSH signature. When Suricata runs detection in this direction
the Kerberos application layer send the transaction as it is existing
and because the alstate progress function just return 1 if the transaction
exists. So Suricata runs DetectRunTx() and stops when it sees that
sgh->tx_engines is NULL.

But the transaction is consumed by the engine as it has been evaluated
in one direction and the kerberos transaction are unidirectional so
there is no need to continue looking at it.

This results in no matching of the kerberos signature as the match
should occur in the evaluation of the other side but the transaction
with the data is already seen has been handled.

This problem was discovered on this Kerberos signature but all
the application layer with unidirectional transaction are impacted.

This patch introduces a flag that can be used by application layer
to signal that the TX should not be inspected. By using this flag
on the directional detect_flags_[ts|tc] the application layer can
prevent the TX to be consumed in the wrong direction.

Application layers with unidirectional TX will be updated
in separate commits to set the flag on the direction opposite
to the one they are.

Ticket: #5799

diff --git a/rust/src/applayer.rs b/rust/src/applayer.rs
index 795db989a48dd906fe95bf6fa8853b23a5465eb8..71d4e6e1b833111cc443ef029120ac13272f5610 100644 (file)
--- a/rust/src/applayer.rs
+++ b/rust/src/applayer.rs
@@ -18,7 +18,7 @@
 //! Parser registration functions and common interface
 
 use std;
-use crate::core::{DetectEngineState,Flow,AppLayerEventType,AppLayerDecoderEvents,AppProto};
+use crate::core::{DetectEngineState,Flow,AppLayerEventType,AppLayerDecoderEvents,AppProto, STREAM_TOCLIENT};
 use crate::filecontainer::FileContainer;
 use crate::applayer;
 use std::os::raw::{c_void,c_char,c_int};
@@ -85,6 +85,14 @@ impl AppLayerTxData {
     pub fn incr_files_opened(&mut self) {
         self.files_opened += 1;
     }
+
+    pub fn set_inspect_direction(&mut self, direction: u8) {
+        if direction == STREAM_TOCLIENT {
+            self.detect_flags_ts |= APP_LAYER_TX_SKIP_INSPECT_FLAG;
+        } else {
+            self.detect_flags_tc |= APP_LAYER_TX_SKIP_INSPECT_FLAG;
+        }
+    }
 }
 
 #[macro_export]
@@ -321,6 +329,7 @@ pub const APP_LAYER_PARSER_BYPASS_READY : u8 = BIT_U8!(4);
 
 pub const APP_LAYER_PARSER_OPT_ACCEPT_GAPS: u32 = BIT_U32!(0);
 pub const APP_LAYER_PARSER_OPT_UNIDIR_TXS: u32 = BIT_U32!(1);
+pub const APP_LAYER_TX_SKIP_INSPECT_FLAG: u64 = BIT_U64!(62);
 
 pub type AppLayerGetTxIteratorFn = extern "C" fn (ipproto: u8,
                                                   alproto: AppProto,

diff --git a/src/app-layer-parser.h b/src/app-layer-parser.h
index 86c3a3696feed77d70e2e38ba4a98f9655186e46..4a4def9b31ad45b8da90ab0c3f6a6bd284e935c4 100644 (file)
--- a/src/app-layer-parser.h
+++ b/src/app-layer-parser.h
@@ -49,6 +49,7 @@
 
 /* applies to DetectFlags uint64_t field */
 
+#define APP_LAYER_TX_SKIP_INSPECT_FLAG BIT_U64(62)
 /** is tx fully inspected? */
 #define APP_LAYER_TX_INSPECTED_FLAG             BIT_U64(63)
 /** other 63 bits are for tracking which prefilter engine is already

diff --git a/src/detect.c b/src/detect.c
index bd4f97604cb177a11b086485e4800fda990756fa..1aea748587eabb246a50dcb0b873c300f6fc92c8 100644 (file)
--- a/src/detect.c
+++ b/src/detect.c
@@ -1242,6 +1242,22 @@ static DetectTransaction GetDetectTx(const uint8_t ipproto, const AppProto alpro
         DetectTransaction no_tx = { NULL, 0, NULL, NULL, 0, 0, 0, 0, 0, };
         return no_tx;
     }
+    if (detect_flags & APP_LAYER_TX_SKIP_INSPECT_FLAG) {
+        SCLogDebug("%" PRIu64 " tx should not be inspected in direction %s. Flags %016" PRIx64,
+                tx_id, flow_flags & STREAM_TOSERVER ? "toserver" : "toclient", detect_flags);
+        DetectTransaction no_tx = {
+            NULL,
+            0,
+            NULL,
+            NULL,
+            0,
+            0,
+            0,
+            0,
+            0,
+        };
+        return no_tx;
+    }
 
     const int tx_progress = AppLayerParserGetStateProgress(ipproto, alproto, tx_ptr, flow_flags);
     const int dir_int = (flow_flags & STREAM_TOSERVER) ? 0 : 1;