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 d7dbfe6ab90e50cd2f356df16e4bc22344a288ca..fd189c68656ff920d24bfe932d978cb27c968f84 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::{self,DetectEngineState,Flow,AppLayerEventType,AppProto};
+use crate::core::{self,DetectEngineState,Flow,AppLayerEventType,AppProto,Direction};
 use crate::filecontainer::FileContainer;
 use crate::applayer;
 use std::os::raw::{c_void,c_char,c_int};
@@ -180,6 +180,14 @@ impl AppLayerTxData {
             self.file_flags |= state_flags;
         }
     }
+
+    pub fn set_inspect_direction(&mut self, direction: Direction) {
+        if direction == Direction::ToClient {
+            self.detect_flags_ts |= APP_LAYER_TX_SKIP_INSPECT_FLAG;
+        } else {
+            self.detect_flags_tc |= APP_LAYER_TX_SKIP_INSPECT_FLAG;
+        }
+    }
 }
 
 #[macro_export]
@@ -474,6 +482,8 @@ pub const APP_LAYER_PARSER_TRUNC_TC : u16 = BIT_U16!(8);
 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 = unsafe extern "C" fn (ipproto: u8,
                                                   alproto: AppProto,
                                                   alstate: *mut c_void,

diff --git a/src/app-layer-parser.h b/src/app-layer-parser.h
index ead069184c91745f567242c6b34300000c62f7ae..4836ab06be25664c982487e0e3a8e90dbefdfd96 100644 (file)
--- a/src/app-layer-parser.h
+++ b/src/app-layer-parser.h
@@ -66,7 +66,6 @@
 #define APP_LAYER_TX_RESERVED12_FLAG BIT_U64(59)
 #define APP_LAYER_TX_RESERVED13_FLAG BIT_U64(60)
 #define APP_LAYER_TX_RESERVED14_FLAG BIT_U64(61)
-#define APP_LAYER_TX_RESERVED15_FLAG BIT_U64(62)
 
 #define APP_LAYER_TX_RESERVED_FLAGS                                                                \
     (APP_LAYER_TX_RESERVED1_FLAG | APP_LAYER_TX_RESERVED2_FLAG | APP_LAYER_TX_RESERVED3_FLAG |     \
@@ -75,8 +74,10 @@
             APP_LAYER_TX_RESERVED8_FLAG | APP_LAYER_TX_RESERVED9_FLAG |                            \
             APP_LAYER_TX_RESERVED10_FLAG | APP_LAYER_TX_RESERVED11_FLAG |                          \
             APP_LAYER_TX_RESERVED12_FLAG | APP_LAYER_TX_RESERVED13_FLAG |                          \
-            APP_LAYER_TX_RESERVED14_FLAG | APP_LAYER_TX_RESERVED15_FLAG)
+            APP_LAYER_TX_RESERVED14_FLAG)
 
+/** should inspection be skipped in that direction */
+#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 2dbbdc0c45742a9b356395c253c6831a91605add..65ca4ebfbbef10d12f77bf536f9bf70d5434ca61 100644 (file)
--- a/src/detect.c
+++ b/src/detect.c
@@ -1256,6 +1256,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;