next/682/20250108/v1

.github/workflows/builds.yml

@@ -656,10 +656,10 @@ jobs:
       - run: make clean
       - run: make -j ${{ env.CPUS }}
 
-  fedora-39-sv-codecov:
-    name: Fedora 39 (Suricata Verify codecov)
+  fedora-41-sv-codecov:
+    name: Fedora 41 (Suricata Verify codecov)
     runs-on: ubuntu-latest
-    container: fedora:39
+    container: fedora:41
     needs: [prepare-deps, prepare-cbindgen]
     steps:
 
@@ -718,7 +718,7 @@ jobs:
                 zlib-devel
       # packaged Rust version has no profiler support built in, so get from rustup
       - name: Install Rust
-        run: curl https://sh.rustup.rs -sSf | sh -s -- --default-toolchain 1.67.1 -y
+        run: curl https://sh.rustup.rs -sSf | sh -s -- --default-toolchain 1.83 -y
       - run: echo "$HOME/.cargo/bin" >> $GITHUB_PATH
       - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683
       - uses: ./.github/actions/install-cbindgen
@@ -752,11 +752,11 @@ jobs:
           fail_ci_if_error: false
           flags: suricata-verify
 
-  # Fedora 39 build using Clang.
-  fedora-39-clang:
-    name: Fedora 39 (clang, debug, asan, wshadow, rust-strict, systemd)
+  # Fedora 41 build using Clang.
+  fedora-41-clang:
+    name: Fedora 41 (clang, debug, asan, wshadow, rust-strict, systemd)
     runs-on: ubuntu-latest
-    container: fedora:39
+    container: fedora:41
     needs: [prepare-deps, prepare-cbindgen]
     steps:
 
@@ -859,10 +859,10 @@ jobs:
       - run: src/suricata --build-info | grep -E "Systemd support:\s+yes" &> /dev/null
 
   # Fedora 39 build using GCC.
-  fedora-39-gcc:
-    name: Fedora 39 (gcc, debug, asan, wshadow, rust-strict)
+  fedora-41-gcc:
+    name: Fedora 41 (gcc, debug, asan, wshadow, rust-strict)
     runs-on: ubuntu-latest
-    container: fedora:39
+    container: fedora:41
     needs: [prepare-deps, prepare-cbindgen]
     steps:
 

doc/userguide/configuration/suricata-yaml.rst

@@ -660,6 +660,7 @@ The detection-engine builds internal groups of signatures. Suricata loads signat
     sgh-mpm-context: auto
     inspection-recursion-limit: 3000
     stream-tx-log-limit: 4
+    guess-applayer-tx: no
 
 At all of these options, you can add (or change) a value. Most
 signatures have the adjustment to focus on one direction, meaning
@@ -694,11 +695,17 @@ complicated issues. It could end up in an 'endless loop' due to a bug,
 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
+The ``stream-tx-log-limit`` defines the maximum number of times a
 transaction will get logged for rules without app-layer keywords.
 This is meant to avoid logging the same data an arbitrary number
 of times.
 
+The ``guess-applayer-tx`` option controls whether the engine will try to guess
+and tie a transaction to a given alert if the matching signature doesn't have
+app-layer keywords. If enabled, AND ONLY ONE LIVE TRANSACTION EXISTS, that
+transaction's data will be added to the alert metadata. Note that this may not
+be the expected data, from an analyst's perspective.
+
 *Example 4	Detection-engine grouping tree*
 
 .. image:: suricata-yaml/grouping_tree.png

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

@@ -79,11 +79,11 @@ 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.
-
+**WARNING: If there are multiple live transactions, none will get
+picked up.** This is to reduce the chances of logging unrelated data, and may
+lead to alerts being logged without metadata, in some cases.
+The alert event will have ``tx_guessed: true`` to recognize
+such alerts.
 
 Metadata::
 

doc/userguide/upgrade.rst

@@ -101,8 +101,11 @@ Logging changes
   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.
+  ``detect.guess-applayer-tx`` can be used to force the detect engine to guess a
+  transaction, which is not guaranteed to be the one you expect. **In this case,
+  the engine will NOT log any transaction metadata if there is more than one
+  live transaction, to reduce the chances of logging unrelated data.** This may
+  lead to what looks like a regression in behavior, but it is a considered choice.
 
 Upgrading 6.0 to 7.0
 --------------------

etc/schema.json

@@ -6509,9 +6509,6 @@
                         "pseudo": {
                             "type": "integer"
                         },
-                        "pseudo_failed": {
-                            "type": "integer"
-                        },
                         "reassembly_exception_policy": {
                             "description":
                                     "How many times reassembly memcap exception policy was applied, and which one",

rust/src/ldap/ldap.rs

@@ -89,7 +89,6 @@ pub struct LdapState {
     state_data: AppLayerStateData,
     tx_id: u64,
     transactions: VecDeque<LdapTransaction>,
-    tx_index_completed: usize,
     request_frame: Option<Frame>,
     response_frame: Option<Frame>,
     request_gap: bool,
@@ -114,7 +113,6 @@ impl LdapState {
             state_data: AppLayerStateData::new(),
             tx_id: 0,
             transactions: VecDeque::new(),
-            tx_index_completed: 0,
             request_frame: None,
             response_frame: None,
             request_gap: false,
@@ -138,7 +136,6 @@ impl LdapState {
             }
         }
         if found {
-            self.tx_index_completed = 0;
             self.transactions.remove(index);
         }
     }
@@ -147,27 +144,24 @@ impl LdapState {
         self.transactions.iter().find(|tx| tx.tx_id == tx_id + 1)
     }
 
-    pub fn new_tx(&mut self) -> LdapTransaction {
-        let mut tx = LdapTransaction::new();
-        self.tx_id += 1;
-        tx.tx_id = self.tx_id;
+    pub fn new_tx(&mut self) -> Option<LdapTransaction> {
         if self.transactions.len() > unsafe { LDAP_MAX_TX } {
-            let mut index = self.tx_index_completed;
-            for tx_old in &mut self.transactions.range_mut(self.tx_index_completed..) {
-                index += 1;
+            for tx_old in &mut self.transactions {
                 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
                         .set_event(LdapEvent::TooManyTransactions as u8);
-                    break;
                 }
             }
-            self.tx_index_completed = index;
+            return None;
         }
-        return tx;
+        let mut tx = LdapTransaction::new();
+        self.tx_id += 1;
+        tx.tx_id = self.tx_id;
+        return Some(tx);
     }
 
     fn set_event(&mut self, e: LdapEvent) {
@@ -228,7 +222,11 @@ impl LdapState {
             }
             match ldap_parse_msg(start) {
                 Ok((rem, msg)) => {
-                    let mut tx = self.new_tx();
+                    let tx = self.new_tx();
+                    if tx.is_none() {
+                        return AppLayerResult::err();
+                    }
+                    let mut tx = tx.unwrap();
                     let tx_id = tx.id();
                     let request = LdapMessage::from(msg);
                     // check if STARTTLS was requested
@@ -237,7 +235,7 @@ impl LdapState {
                             self.request_tls = true;
                         }
                     }
-                    tx.complete = tx_is_complete(&request.protocol_op, Direction::ToServer);
+                    tx.complete |= tx_is_complete(&request.protocol_op, Direction::ToServer);
                     tx.request = Some(request);
                     self.transactions.push_back(tx);
                     let consumed = start.len() - rem.len();
@@ -298,8 +296,7 @@ impl LdapState {
                     let response = LdapMessage::from(msg);
                     // check if STARTTLS was requested
                     if self.request_tls {
-                        if let ProtocolOp::ExtendedResponse(response) = &response.protocol_op
-                        {
+                        if let ProtocolOp::ExtendedResponse(response) = &response.protocol_op {
                             if response.result.result_code == ResultCode(0) {
                                 SCLogDebug!("LDAP: STARTTLS detected");
                                 self.has_starttls = true;
@@ -308,7 +305,7 @@ impl LdapState {
                         }
                     }
                     if let Some(tx) = self.find_request(response.message_id) {
-                        tx.complete = tx_is_complete(&response.protocol_op, Direction::ToClient);
+                        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);
@@ -317,15 +314,23 @@ impl LdapState {
                     } else if let ProtocolOp::ExtendedResponse(_) = response.protocol_op {
                         // this is an unsolicited notification, which means
                         // there is no request
-                        let mut tx = self.new_tx();
+                        let tx = self.new_tx();
+                        if tx.is_none() {
+                            return AppLayerResult::err();
+                        }
+                        let mut tx = tx.unwrap();
                         let tx_id = tx.id();
                         tx.complete = true;
                         tx.responses.push_back(response);
                         self.transactions.push_back(tx);
                         let consumed = start.len() - rem.len();
                         self.set_frame_tc(flow, tx_id, consumed as i64);
                     } else {
-                        let mut tx = self.new_tx();
+                        let tx = self.new_tx();
+                        if tx.is_none() {
+                            return AppLayerResult::err();
+                        }
+                        let mut tx = tx.unwrap();
                         tx.complete = true;
                         let tx_id = tx.id();
                         tx.responses.push_back(response);
@@ -367,9 +372,13 @@ impl LdapState {
 
         match ldap_parse_msg(input) {
             Ok((_, msg)) => {
-                let mut tx = self.new_tx();
+                let tx = self.new_tx();
+                if tx.is_none() {
+                    return AppLayerResult::err();
+                }
+                let mut tx = tx.unwrap();
                 let request = LdapMessage::from(msg);
-                tx.complete = tx_is_complete(&request.protocol_op, Direction::ToServer);
+                tx.complete |= tx_is_complete(&request.protocol_op, Direction::ToServer);
                 tx.request = Some(request);
                 self.transactions.push_back(tx);
             }
@@ -411,23 +420,31 @@ impl LdapState {
                 Ok((rem, msg)) => {
                     let response = LdapMessage::from(msg);
                     if let Some(tx) = self.find_request(response.message_id) {
-                        tx.complete = tx_is_complete(&response.protocol_op, Direction::ToClient);
+                        tx.complete |= tx_is_complete(&response.protocol_op, Direction::ToClient);
                         let tx_id = tx.id();
                         tx.responses.push_back(response);
                         let consumed = start.len() - rem.len();
                         self.set_frame_tc(flow, tx_id, consumed as i64);
                     } else if let ProtocolOp::ExtendedResponse(_) = response.protocol_op {
                         // this is an unsolicited notification, which means
                         // there is no request
-                        let mut tx = self.new_tx();
+                        let tx = self.new_tx();
+                        if tx.is_none() {
+                            return AppLayerResult::err();
+                        }
+                        let mut tx = tx.unwrap();
                         tx.complete = true;
                         let tx_id = tx.id();
                         tx.responses.push_back(response);
                         self.transactions.push_back(tx);
                         let consumed = start.len() - rem.len();
                         self.set_frame_tc(flow, tx_id, consumed as i64);
                     } else {
-                        let mut tx = self.new_tx();
+                        let tx = self.new_tx();
+                        if tx.is_none() {
+                            return AppLayerResult::err();
+                        }
+                        let mut tx = tx.unwrap();
                         tx.complete = true;
                         let tx_id = tx.id();
                         tx.responses.push_back(response);

src/app-layer-detect-proto.c

@@ -158,8 +158,15 @@ typedef struct AppLayerProtoDetectCtx_ {
 
     /* Indicates the protocols that have registered themselves
      * for protocol detection.  This table is independent of the
-     * ipproto. */
-    const char *alproto_names[ALPROTO_MAX];
+     * ipproto. It should be allocated to contain ALPROTO_MAX
+     * protocols. */
+    const char **alproto_names;
+
+    /* Protocol expectations, like ftp-data on tcp.
+     * It should be allocated to contain ALPROTO_MAX
+     * app-layer protocols. For each protocol, an iptype
+     * is referenced (or 0 if there is no expectation). */
+    uint8_t *expectation_proto;
 } AppLayerProtoDetectCtx;
 
 typedef struct AppLayerProtoDetectAliases_ {
@@ -1718,6 +1725,15 @@ int AppLayerProtoDetectSetup(void)
         }
     }
 
+    alpd_ctx.alproto_names = SCCalloc(ALPROTO_MAX, sizeof(char *));
+    if (unlikely(alpd_ctx.alproto_names == NULL)) {
+        FatalError("Unable to alloc alproto_names.");
+    }
+    // to realloc when dynamic protos are added
+    alpd_ctx.expectation_proto = SCCalloc(ALPROTO_MAX, sizeof(uint8_t));
+    if (unlikely(alpd_ctx.expectation_proto == NULL)) {
+        FatalError("Unable to alloc expectation_proto.");
+    }
     AppLayerExpectationSetup();
 
     SCReturnInt(0);
@@ -1749,6 +1765,11 @@ int AppLayerProtoDetectDeSetup(void)
         }
     }
 
+    SCFree(alpd_ctx.alproto_names);
+    alpd_ctx.alproto_names = NULL;
+    SCFree(alpd_ctx.expectation_proto);
+    alpd_ctx.expectation_proto = NULL;
+
     SpmDestroyGlobalThreadCtx(alpd_ctx.spm_global_thread_ctx);
 
     AppLayerProtoDetectFreeAliases();
@@ -1762,6 +1783,7 @@ void AppLayerProtoDetectRegisterProtocol(AppProto alproto, const char *alproto_n
 {
     SCEnter();
 
+    // should have just been realloced when dynamic protos is added
     if (alpd_ctx.alproto_names[alproto] == NULL)
         alpd_ctx.alproto_names[alproto] = alproto_name;
 
@@ -2111,27 +2133,25 @@ void AppLayerProtoDetectSupportedAppProtocols(AppProto *alprotos)
     SCReturn;
 }
 
-uint8_t expectation_proto[ALPROTO_MAX];
-
 static void AppLayerProtoDetectPEGetIpprotos(AppProto alproto,
                                              uint8_t *ipprotos)
 {
-    if (expectation_proto[alproto] == IPPROTO_TCP) {
+    if (alpd_ctx.expectation_proto[alproto] == IPPROTO_TCP) {
         ipprotos[IPPROTO_TCP / 8] |= 1 << (IPPROTO_TCP % 8);
     }
-    if (expectation_proto[alproto] == IPPROTO_UDP) {
+    if (alpd_ctx.expectation_proto[alproto] == IPPROTO_UDP) {
         ipprotos[IPPROTO_UDP / 8] |= 1 << (IPPROTO_UDP % 8);
     }
 }
 
 void AppLayerRegisterExpectationProto(uint8_t proto, AppProto alproto)
 {
-    if (expectation_proto[alproto]) {
-        if (proto != expectation_proto[alproto]) {
+    if (alpd_ctx.expectation_proto[alproto]) {
+        if (proto != alpd_ctx.expectation_proto[alproto]) {
             SCLogError("Expectation on 2 IP protocols are not supported");
         }
     }
-    expectation_proto[alproto] = proto;
+    alpd_ctx.expectation_proto[alproto] = proto;
 }
 
 /***** Unittests *****/