Next/665/20241210/v4

doc/userguide/configuration/suricata-yaml.rst

@@ -695,7 +695,7 @@ meaning it will repeat its actions over and over again. With the
 option inspection-recursion-limit you can limit this action.
 
 The stream-tx-log-limit defines the maximum number of times a
-transaction will get logged for a stream-only rule match.
+transaction will get logged for rules without app-layer keywords.
 This is meant to avoid logging the same data an arbitrary number
 of times.
 

doc/userguide/output/eve/eve-json-output.rst

@@ -70,6 +70,21 @@ Alerts are event records for rule matches. They can be amended with
 metadata, such as the application layer record (HTTP, DNS, etc) an
 alert was generated for, and elements of the rule.
 
+The alert is amended with application layer metadata for signatures
+using application layer keywords. It is also the case for protocols
+over UDP as each single packet is expected to contain a PDU.
+
+For other signatures, the option ``guess-applayer-tx``
+can be used to force the detect engine to tie a transaction
+to an alert.
+This transaction is not guaranteed to be the relevant one,
+depending on your use case and how you define relevant here.
+If there are multiple live transactions, none will get
+picked up.
+The alert event will have ``"tx_guessed": true`` to recognize
+these alerts.
+
+
 Metadata::
 
         - alert:

doc/userguide/upgrade.rst

@@ -99,6 +99,10 @@ Logging changes
 ~~~~~~~~~~~~~~~
 - RFB security result is now consistently logged as ``security_result`` when it was
   sometimes logged with a dash instead of an underscore.
+- Application layer metadata is logged with alerts by default **only for rules that
+  use application layer keywords**. For other rules, the configuration parameter
+  ``detect.guess-applayer-tx`` can be used to force the detect engine to find a
+  transaction, which is not guaranteed to be the one you expect.
 
 Upgrading 6.0 to 7.0
 --------------------

etc/schema.json

@@ -115,6 +115,10 @@
         "tx_id": {
             "type": "integer"
         },
+        "tx_guessed": {
+            "description": "the signature that triggered this alert didn't tie to a transaction, so the transaction (and metadata) logged is a forced estimation and may not be the one you expect",
+            "type": "boolean"
+        },
         "files": {
             "type": "array",
             "minItems": 1,

rust/src/applayer.rs

@@ -98,6 +98,12 @@ pub struct AppLayerTxData {
     /// config: log flags
     pub config: AppLayerTxConfig,
 
+    /// The tx has been updated and needs to be processed : detection, logging, cleaning
+    /// It can then be skipped until new data arrives.
+    /// There is a boolean for both directions : to server and to client
+    pub updated_tc: bool,
+    pub updated_ts: bool,
+
     /// logger flags for tx logging api
     logged: LoggerFlags,
 
@@ -114,8 +120,9 @@ pub struct AppLayerTxData {
     /// STREAM_TOCLIENT: file tx , files only in toclient dir
     /// STREAM_TOSERVER|STREAM_TOCLIENT: files possible in both dirs
     pub file_tx: u8,
-    /// Number of times this tx data has already been logged for one stream match
-    pub stream_logged: u8,
+    /// Number of times this tx data has already been logged for signatures
+    /// not using application layer keywords
+    pub guessed_applayer_logged: u8,
 
     /// detection engine flags for use by detection engine
     detect_flags_ts: u64,
@@ -154,7 +161,9 @@ impl AppLayerTxData {
             files_stored: 0,
             file_flags: 0,
             file_tx: 0,
-            stream_logged: 0,
+            guessed_applayer_logged: 0,
+            updated_tc: true,
+            updated_ts: true,
             detect_flags_ts: 0,
             detect_flags_tc: 0,
             de_state: std::ptr::null_mut(),
@@ -165,9 +174,9 @@ impl AppLayerTxData {
     /// Create new AppLayerTxData for a transaction in a single
     /// direction.
     pub fn for_direction(direction: Direction) -> Self {
-        let (detect_flags_ts, detect_flags_tc) = match direction {
-            Direction::ToServer => (0, APP_LAYER_TX_SKIP_INSPECT_FLAG),
-            Direction::ToClient => (APP_LAYER_TX_SKIP_INSPECT_FLAG, 0),
+        let (detect_flags_ts, detect_flags_tc, updated_ts, updated_tc) = match direction {
+            Direction::ToServer => (0, APP_LAYER_TX_SKIP_INSPECT_FLAG, true, false),
+            Direction::ToClient => (APP_LAYER_TX_SKIP_INSPECT_FLAG, 0, false, true),
         };
         Self {
             config: AppLayerTxConfig::new(),
@@ -177,7 +186,9 @@ impl AppLayerTxData {
             files_stored: 0,
             file_flags: 0,
             file_tx: 0,
-            stream_logged: 0,
+            guessed_applayer_logged: 0,
+            updated_tc,
+            updated_ts,
             detect_flags_ts,
             detect_flags_tc,
             de_state: std::ptr::null_mut(),

rust/src/applayertemplate/template.rs

@@ -200,6 +200,7 @@ impl TemplateState {
                     start = rem;
 
                     if let Some(tx) =  self.find_request() {
+                        tx.tx_data.updated_tc = true;
                         tx.response = Some(response);
                         SCLogNotice!("Found response for request:");
                         SCLogNotice!("- Request: {:?}", tx.request);

rust/src/dcerpc/dcerpc.rs

@@ -357,6 +357,8 @@ impl DCERPCState {
             for tx_old in &mut self.transactions.range_mut(self.tx_index_completed..) {
                 index += 1;
                 if !tx_old.req_done || !tx_old.resp_done {
+                    tx_old.tx_data.updated_tc = true;
+                    tx_old.tx_data.updated_ts = true;
                     tx_old.req_done = true;
                     tx_old.resp_done = true;
                     break;
@@ -533,6 +535,8 @@ impl DCERPCState {
                         }
                     }
                 }
+                tx.tx_data.updated_tc = true;
+                tx.tx_data.updated_ts = true;
                 return Some(tx);
             }
         }

rust/src/dcerpc/dcerpc_udp.rs

@@ -88,6 +88,8 @@ impl DCERPCUDPState {
             for tx_old in &mut self.transactions.range_mut(self.tx_index_completed..) {
                 index += 1;
                 if !tx_old.req_done || !tx_old.resp_done {
+                    tx_old.tx_data.updated_tc = true;
+                    tx_old.tx_data.updated_ts = true;
                     tx_old.req_done = true;
                     tx_old.resp_done = true;
                     break;
@@ -164,6 +166,8 @@ impl DCERPCUDPState {
         }
 
         if let Some(tx) = otx {
+            tx.tx_data.updated_tc = true;
+            tx.tx_data.updated_ts = true;
             let done = (hdr.flags1 & PFCL1_FRAG) == 0 || (hdr.flags1 & PFCL1_LASTFRAG) != 0;
 
             match hdr.pkt_type {

rust/src/enip/enip.rs

@@ -203,6 +203,8 @@ impl EnipState {
     fn purge_tx_flood(&mut self) {
         let mut event_set = false;
         for tx in self.transactions.iter_mut() {
+            tx.tx_data.updated_tc = true;
+            tx.tx_data.updated_ts = true;
             tx.done = true;
             if !event_set {
                 tx.tx_data.set_event(EnipEvent::TooManyTransactions as u8);
@@ -216,6 +218,8 @@ impl EnipState {
             if let Some(req) = &tx.request {
                 if tx.response.is_none() {
                     tx.done = true;
+                    tx.tx_data.updated_tc = true;
+                    tx.tx_data.updated_ts = true;
                     if response_matches_request(req, pdu) {
                         return Some(tx);
                     }

rust/src/ftp/mod.rs

@@ -88,6 +88,9 @@ pub fn ftp_pasv_response(i: &[u8]) -> IResult<&[u8], u16> {
 
 #[no_mangle]
 pub unsafe extern "C" fn rs_ftp_active_port(input: *const u8, len: u32) -> u16 {
+    if input.is_null() {
+        return 0;
+    }
     let buf = build_slice!(input, len as usize);
     match ftp_active_port(buf) {
         Ok((_, dport)) => {
@@ -105,7 +108,10 @@ pub unsafe extern "C" fn rs_ftp_active_port(input: *const u8, len: u32) -> u16 {
 
 #[no_mangle]
 pub unsafe extern "C" fn rs_ftp_pasv_response(input: *const u8, len: u32) -> u16 {
-    let buf = std::slice::from_raw_parts(input, len as usize);
+    if input.is_null() {
+        return 0;
+    }
+    let buf =  build_slice!(input, len as usize);
     match ftp_pasv_response(buf) {
         Ok((_, dport)) => {
             return dport;
@@ -147,6 +153,9 @@ pub fn ftp_active_eprt(i: &[u8]) -> IResult<&[u8], u16> {
 
 #[no_mangle]
 pub unsafe extern "C" fn rs_ftp_active_eprt(input: *const u8, len: u32) -> u16 {
+    if input.is_null() {
+        return 0;
+    }
     let buf = build_slice!(input, len as usize);
     match ftp_active_eprt(buf) {
         Ok((_, dport)) => {
@@ -163,7 +172,10 @@ pub unsafe extern "C" fn rs_ftp_active_eprt(input: *const u8, len: u32) -> u16 {
 }
 #[no_mangle]
 pub unsafe extern "C" fn rs_ftp_epsv_response(input: *const u8, len: u32) -> u16 {
-    let buf = std::slice::from_raw_parts(input, len as usize);
+    if input.is_null() {
+        return 0;
+    }
+    let buf = build_slice!(input, len as usize);
     match ftp_epsv_response(buf) {
         Ok((_, dport)) => {
             return dport;

rust/src/http2/http2.rs

@@ -752,6 +752,8 @@ impl HTTP2State {
             let tx = &mut self.transactions[index - 1];
             tx.tx_data.update_file_flags(self.state_data.file_flags);
             tx.update_file_flags(tx.tx_data.file_flags);
+            tx.tx_data.updated_tc = true;
+            tx.tx_data.updated_ts = true;
             return Some(tx);
         } else {
             // do not use SETTINGS_MAX_CONCURRENT_STREAMS as it can grow too much
@@ -764,6 +766,8 @@ impl HTTP2State {
                     tx_old.set_event(HTTP2Event::TooManyStreams);
                     // use a distinct state, even if we do not log it
                     tx_old.state = HTTP2TransactionState::HTTP2StateTodrop;
+                    tx_old.tx_data.updated_tc = true;
+                    tx_old.tx_data.updated_ts = true;
                 }
                 return None;
             }

rust/src/ldap/ldap.rs

@@ -156,6 +156,8 @@ impl LdapState {
             for tx_old in &mut self.transactions.range_mut(self.tx_index_completed..) {
                 index += 1;
                 if !tx_old.complete {
+                    tx_old.tx_data.updated_tc = true;
+                    tx_old.tx_data.updated_ts = true;
                     tx_old.complete = true;
                     tx_old
                         .tx_data
@@ -308,6 +310,7 @@ impl LdapState {
                     if let Some(tx) = self.find_request(response.message_id) {
                         tx.complete = tx_is_complete(&response.protocol_op, Direction::ToClient);
                         let tx_id = tx.id();
+                        tx.tx_data.updated_tc = true;
                         tx.responses.push_back(response);
                         let consumed = start.len() - rem.len();
                         self.set_frame_tc(flow, tx_id, consumed as i64);

rust/src/modbus/modbus.rs

@@ -124,6 +124,8 @@ impl ModbusState {
         for tx in &mut self.transactions {
             if let Some(req) = &tx.request {
                 if tx.response.is_none() && resp.matches(req) {
+                    tx.tx_data.updated_tc = true;
+                    tx.tx_data.updated_ts = true;
                     return Some(tx);
                 }
             }
@@ -139,6 +141,8 @@ impl ModbusState {
         for tx in &mut self.transactions {
             if let Some(resp) = &tx.response {
                 if tx.request.is_none() && req.matches(resp) {
+                    tx.tx_data.updated_tc = true;
+                    tx.tx_data.updated_ts = true;
                     return Some(tx);
                 }
             }
@@ -184,6 +188,8 @@ impl ModbusState {
                             match self.find_response_and_validate(&mut msg) {
                                 Some(tx) => {
                                     tx.set_events_from_flags(&msg.error_flags);
+                                    tx.tx_data.updated_tc = true;
+                                    tx.tx_data.updated_ts = true;
                                     tx.request = Some(msg);
                                 }
                                 None => {
@@ -210,6 +216,8 @@ impl ModbusState {
                                 } else {
                                     tx.set_events_from_flags(&msg.error_flags);
                                 }
+                                tx.tx_data.updated_tc = true;
+                                tx.tx_data.updated_ts = true;
                                 tx.response = Some(msg);
                             }
                             None => {

rust/src/mqtt/mqtt.rs

@@ -174,6 +174,8 @@ impl MQTTState {
             if !tx.complete {
                 if let Some(mpktid) = tx.pkt_id {
                     if mpktid == pkt_id {
+                        tx.tx_data.updated_tc = true;
+                        tx.tx_data.updated_ts = true;
                         return Some(tx);
                     }
                 }
@@ -196,6 +198,8 @@ impl MQTTState {
             for tx_old in &mut self.transactions.range_mut(self.tx_index_completed..) {
                 index += 1;
                 if !tx_old.complete {
+                    tx_old.tx_data.updated_tc = true;
+                    tx_old.tx_data.updated_ts = true;
                     tx_old.complete = true;
                     MQTTState::set_event(tx_old, MQTTEvent::TooManyTransactions);
                     break;

rust/src/nfs/nfs.rs

@@ -431,6 +431,8 @@ impl NFSState {
             // set at least one another transaction to the drop state
             for tx_old in &mut self.transactions {
                 if !tx_old.request_done || !tx_old.response_done {
+                    tx_old.tx_data.updated_tc = true;
+                    tx_old.tx_data.updated_ts = true;
                     tx_old.request_done = true;
                     tx_old.response_done = true;
                     tx_old.is_file_closed = true;
@@ -484,6 +486,8 @@ impl NFSState {
     pub fn mark_response_tx_done(&mut self, xid: u32, rpc_status: u32, nfs_status: u32, resp_handle: &[u8])
     {
         if let Some(mytx) = self.get_tx_by_xid(xid) {
+            mytx.tx_data.updated_tc = true;
+            mytx.tx_data.updated_ts = true;
             mytx.response_done = true;
             mytx.rpc_response_status = rpc_status;
             mytx.nfs_response_status = nfs_status;
@@ -736,6 +740,8 @@ impl NFSState {
                     tx.tx_data.update_file_flags(self.state_data.file_flags);
                     d.update_file_flags(tx.tx_data.file_flags);
                     SCLogDebug!("Found NFS file TX with ID {} XID {:04X}", tx.id, tx.xid);
+                    tx.tx_data.updated_tc = true;
+                    tx.tx_data.updated_ts = true;
                     return Some(tx);
                 }
             }

rust/src/pgsql/pgsql.rs

@@ -208,6 +208,8 @@ impl PgsqlState {
             for tx_old in &mut self.transactions.range_mut(self.tx_index_completed..) {
                 index += 1;
                 if tx_old.tx_res_state < PgsqlTxProgress::TxDone {
+                    tx_old.tx_data.updated_tc = true;
+                    tx_old.tx_data.updated_ts = true;
                     // we don't check for TxReqDone for the majority of requests are basically completed
                     // when they're parsed, as of now
                     tx_old.tx_req_state = PgsqlTxProgress::TxFlushedOut;
@@ -360,6 +362,7 @@ impl PgsqlState {
                     // A simplified finite state machine for PostgreSQL v3 can be found at:
                     // https://samadhiweb.com/blog/2013.04.28.graphviz.postgresv3.html
                     if let Some(tx) = self.find_or_create_tx() {
+                        tx.tx_data.updated_ts = true;
                         tx.request = Some(request);
                         if let Some(state) = new_state {
                             if Self::request_is_complete(state) {
@@ -518,6 +521,7 @@ impl PgsqlState {
                         self.state_progress = state;
                     }
                     if let Some(tx) = self.find_or_create_tx() {
+                        tx.tx_data.updated_tc = true;
                         if tx.tx_res_state == PgsqlTxProgress::TxInit {
                             tx.tx_res_state = PgsqlTxProgress::TxReceived;
                         }

rust/src/rfb/rfb.rs

@@ -166,7 +166,13 @@ impl RFBState {
 
     fn get_current_tx(&mut self) -> Option<&mut RFBTransaction> {
         let tx_id = self.tx_id;
-        self.transactions.iter_mut().find(|tx| tx.tx_id == tx_id)
+        let r = self.transactions.iter_mut().find(|tx| tx.tx_id == tx_id);
+        if let Some(tx) = r {
+            tx.tx_data.updated_tc = true;
+            tx.tx_data.updated_ts = true;
+            return Some(tx);
+        }
+        return None;
     }
 
     fn parse_request(&mut self, flow: *const Flow, stream_slice: StreamSlice) -> AppLayerResult {

rust/src/smb/dcerpc.rs

@@ -168,6 +168,8 @@ impl SMBState {
                 _ => { false },
             };
             if found {
+                tx.tx_data.updated_tc = true;
+                tx.tx_data.updated_ts = true;
                 return Some(tx);
             }
         }