detect: unify the transaction end state helper

AppLayerParserGetTxEndState() replaces the remaining copies of the
transaction end state computation in the firewall tx setup, the
app-layer-state and app-layer-event keywords and the completion check.
One tolerated a missing tx data pointer, the others asserted or
dereferenced it, so they could disagree.

Move DetectTxCompleted() above its first use while there, so it no
longer needs a forward declaration, and shorten its comment.

Ticket: #8944.

rust/src/http2/http2.rs

@@ -883,6 +883,8 @@ impl HTTP2State {
         let mut tx = HTTP2Transaction::new();
         tx.tx_data = AppLayerTxData::for_direction(dir);
         tx.tx_data.0.tx_type = HTTP2TxType::HTTP2TxTypeGlobal as u8;
+        // a parser with tx types must fill both eop fields: the end state
+        // helper asserts on 0
         tx.tx_data.0.tx_type_eop_ts = HTTP2TxGlobalProgress::HTTP2ProgGlobalComplete as u8;
         tx.tx_data.0.tx_type_eop_tc = HTTP2TxGlobalProgress::HTTP2ProgGlobalComplete as u8;
         self.tx_id += 1;
@@ -971,6 +973,8 @@ impl HTTP2State {
             tx.update_file_flags(tx.tx_data.0.file_flags);
             tx.tx_data.0.file_tx = STREAM_TOSERVER | STREAM_TOCLIENT; // might hold files in both directions
             tx.tx_data.0.tx_type = HTTP2TxType::HTTP2TxTypeStream as u8;
+            // a parser with tx types must fill both eop fields: the end state
+            // helper asserts on 0
             tx.tx_data.0.tx_type_eop_ts = HTTP2TxProgress::HTTP2ProgComplete as u8;
             tx.tx_data.0.tx_type_eop_tc = HTTP2TxProgress::HTTP2ProgComplete as u8;
             self.transactions.push_back(tx);

src/app-layer-parser.c

@@ -177,9 +177,6 @@ struct AppLayerParserState_ {
     FramesContainer *frames;
 };
 
-static inline uint8_t GetTxdEndProgress(uint8_t ipproto, AppProto alproto,
-        const AppLayerTxData *txd, uint8_t flags, uint8_t complete);
-
 enum ExceptionPolicy g_applayerparser_error_policy = EXCEPTION_POLICY_NOT_SET;
 
 static void AppLayerConfig(void)
@@ -853,7 +850,6 @@ void AppLayerParserSetTransactionInspectId(const Flow *f, AppLayerParserState *p
     const int direction = (flags & STREAM_TOSERVER) ? 0 : 1;
     const uint64_t total_txs = AppLayerParserGetTxCnt(f, alstate);
     uint64_t idx = AppLayerParserGetTransactionInspectId(pstate, flags);
-    const int state_done_progress = AppLayerParserGetStateProgressCompletionStatus(f->alproto, flags);
     const uint8_t ipproto = f->proto;
     const AppProto alproto = f->alproto;
 
@@ -874,8 +870,7 @@ void AppLayerParserSetTransactionInspectId(const Flow *f, AppLayerParserState *p
         idx = ires.tx_id;
 
         AppLayerTxData *txd = AppLayerParserGetTxData(ipproto, alproto, tx);
-        const int tx_end_state =
-                GetTxdEndProgress(ipproto, alproto, txd, flags, (uint8_t)state_done_progress);
+        const int tx_end_state = AppLayerParserGetTxEndState(ipproto, alproto, tx, flags);
         int state_progress = AppLayerParserGetStateProgress(ipproto, alproto, tx, flags);
         if (state_progress < tx_end_state)
             break;
@@ -915,8 +910,7 @@ void AppLayerParserSetTransactionInspectId(const Flow *f, AppLayerParserState *p
             idx = ires.tx_id;
 
             AppLayerTxData *txd = AppLayerParserGetTxData(ipproto, alproto, tx);
-            const int tx_end_state =
-                    GetTxdEndProgress(ipproto, alproto, txd, flags, (uint8_t)state_done_progress);
+            const int tx_end_state = AppLayerParserGetTxEndState(ipproto, alproto, tx, flags);
             const int state_progress = AppLayerParserGetStateProgress(ipproto, alproto, tx, flags);
             if (state_progress < tx_end_state)
                 break;
@@ -1017,8 +1011,6 @@ void AppLayerParserTransactionsCleanup(Flow *f, const uint8_t pkt_dir)
     const uint64_t min = alparser->min_id;
     const uint64_t total_txs = AppLayerParserGetTxCnt(f, alstate);
     const LoggerId logger_expectation = AppLayerParserProtocolGetLoggerBits(ipproto, alproto);
-    const int tx_end_state_ts = AppLayerParserGetStateProgressCompletionStatus(alproto, STREAM_TOSERVER);
-    const int tx_end_state_tc = AppLayerParserGetStateProgressCompletionStatus(alproto, STREAM_TOCLIENT);
     const uint8_t ts_disrupt_flags = FlowGetDisruptionFlags(f, STREAM_TOSERVER);
     const uint8_t tc_disrupt_flags = FlowGetDisruptionFlags(f, STREAM_TOCLIENT);
 
@@ -1058,15 +1050,13 @@ void AppLayerParserTransactionsCleanup(Flow *f, const uint8_t pkt_dir)
         }
         const int tx_progress_tc =
                 AppLayerParserGetStateProgress(ipproto, alproto, tx, tc_disrupt_flags);
-        const int end_state_tc =
-                GetTxdEndProgress(ipproto, alproto, txd, STREAM_TOCLIENT, (uint8_t)tx_end_state_tc);
+        const int end_state_tc = AppLayerParserGetTxEndState(ipproto, alproto, tx, STREAM_TOCLIENT);
         if (tx_progress_tc < end_state_tc) {
             SCLogDebug("%p/%"PRIu64" skipping: tc parser not done", tx, i);
             skipped = true;
             goto next;
         }
-        const int end_state_ts =
-                GetTxdEndProgress(ipproto, alproto, txd, STREAM_TOSERVER, (uint8_t)tx_end_state_ts);
+        const int end_state_ts = AppLayerParserGetTxEndState(ipproto, alproto, tx, STREAM_TOSERVER);
         const int tx_progress_ts =
                 AppLayerParserGetStateProgress(ipproto, alproto, tx, ts_disrupt_flags);
         if (tx_progress_ts < end_state_ts) {
@@ -1201,34 +1191,6 @@ uint8_t AppLayerParserGetTxEndState(uint8_t ipproto, AppProto alproto, void *tx,
     return eop;
 }
 
-/** \internal
- *  \brief get the end state (progress) for a TX(D)
- *  If the TX is supporting sub-states, return the value from the txd.
- *  \param txd pointer to the transactions txd
- *  \param complete optional final progress for the protocol
- *
- *  `complete` can be passed in as it is often already looked up
- *  outside of the tx loop.
- */
-static inline uint8_t GetTxdEndProgress(uint8_t ipproto, AppProto alproto,
-        const AppLayerTxData *txd, uint8_t flags, uint8_t complete)
-{
-    DEBUG_VALIDATE_BUG_ON(txd == NULL);
-    uint8_t tx_end_state;
-    if (txd->tx_type == 0) {
-        if (complete)
-            tx_end_state = complete;
-        else
-            tx_end_state = (uint8_t)AppLayerParserGetStateProgressCompletionStatus(alproto, flags);
-    } else {
-        if (flags & STREAM_TOSERVER)
-            tx_end_state = txd->tx_type_eop_ts;
-        else
-            tx_end_state = txd->tx_type_eop_tc;
-    }
-    return tx_end_state;
-}
-
 /**
  *  \brief get the progress value for a tx/protocol
  *

src/detect-app-layer-event.c

@@ -122,16 +122,7 @@ static uint8_t DetectEngineAptEventInspect(DetectEngineCtx *de_ctx, DetectEngine
     if (r == 1) {
         return DETECT_ENGINE_INSPECT_SIG_MATCH;
     } else {
-        AppLayerTxData *txd = AppLayerParserGetTxData(f->proto, alproto, tx);
-        uint8_t tx_end_state;
-        if (txd->tx_type == 0) {
-            tx_end_state = (uint8_t)AppLayerParserGetStateProgressCompletionStatus(alproto, flags);
-        } else {
-            if (flags & STREAM_TOSERVER)
-                tx_end_state = txd->tx_type_eop_ts;
-            else
-                tx_end_state = txd->tx_type_eop_tc;
-        }
+        const uint8_t tx_end_state = AppLayerParserGetTxEndState(f->proto, alproto, tx, flags);
         if (AppLayerParserGetStateProgress(f->proto, alproto, tx, flags) == tx_end_state) {
             return DETECT_ENGINE_INSPECT_SIG_CANT_MATCH;
         } else {

src/detect-app-layer-state.c

@@ -128,16 +128,7 @@ end:
         SCLogDebug("DETECT_ENGINE_INSPECT_SIG_MATCH");
         return DETECT_ENGINE_INSPECT_SIG_MATCH;
     } else {
-        AppLayerTxData *txd = AppLayerParserGetTxData(f->proto, alproto, tx);
-        uint8_t tx_end_state;
-        if (txd->tx_type == 0) {
-            tx_end_state = (uint8_t)AppLayerParserGetStateProgressCompletionStatus(alproto, flags);
-        } else {
-            if (flags & STREAM_TOSERVER)
-                tx_end_state = txd->tx_type_eop_ts;
-            else
-                tx_end_state = txd->tx_type_eop_tc;
-        }
+        const uint8_t tx_end_state = AppLayerParserGetTxEndState(f->proto, alproto, tx, flags);
         if (AppLayerParserGetStateProgress(f->proto, alproto, tx, flags) == tx_end_state) {
             SCLogDebug("DETECT_ENGINE_INSPECT_SIG_CANT_MATCH");
             return DETECT_ENGINE_INSPECT_SIG_CANT_MATCH;

src/detect-engine.c

@@ -2125,21 +2125,33 @@ int DetectEngineReloadIsIdle(void)
     return r;
 }
 
-/** \brief Do the content inspection & validation for a signature
+/** \internal
+ *  \brief is the engine's data final for this tx?
  *
- *  \param de_ctx Detection engine context
- *  \param det_ctx Detection engine thread context
- *  \param s Signature to inspect
- *  \param sm SigMatch to inspect
- *  \param f Flow
- *  \param flags app layer flags
- *  \param state App layer state
- *
- *  \retval 0 no match
- *  \retval 1 match
+ *  Past the engine's phase the answer is yes; at or beyond the tx end state it
+ *  is final too, even for engines registered at the completion state (no P+1).
+ *  AppLayerParserGetStateProgress() returns the end progress for disrupted
+ *  flows, so progress == end stays a valid finality signal there.
  */
 static bool DetectTxCompleted(
-        Flow *f, void *txv, uint8_t flags, const DetectEngineAppInspectionEngine *engine);
+        Flow *f, void *txv, uint8_t flags, const DetectEngineAppInspectionEngine *engine)
+{
+    if (f->alproto == ALPROTO_DOH2 && engine->alproto == ALPROTO_DOH2) {
+        // the DNS tx from DetectGetInnerTx is always complete
+        return true;
+    } // else
+    const int progress = AppLayerParserGetStateProgress(f->proto, f->alproto, txv, flags);
+    if (progress < 0) {
+        return false;
+    }
+    if (progress > engine->progress) {
+        return true;
+    }
+    if (progress < engine->progress) {
+        return false;
+    }
+    return progress == AppLayerParserGetTxEndState(f->proto, f->alproto, txv, flags);
+}
 
 uint8_t DetectEngineInspectGenericList(DetectEngineCtx *de_ctx, DetectEngineThreadCtx *det_ctx,
         const struct DetectEngineAppInspectionEngine_ *engine, const Signature *s, Flow *f,
@@ -2179,39 +2191,6 @@ uint8_t DetectEngineInspectGenericList(DetectEngineCtx *de_ctx, DetectEngineThre
     return DETECT_ENGINE_INSPECT_SIG_MATCH;
 }
 
-static bool DetectTxCompleted(
-        Flow *f, void *txv, uint8_t flags, const DetectEngineAppInspectionEngine *engine)
-{
-    if (f->alproto == ALPROTO_DOH2 && engine->alproto == ALPROTO_DOH2) {
-        // the DNS tx from DetectGetInnerTx is always complete
-        return true;
-    } // else
-    const int progress = AppLayerParserGetStateProgress(f->proto, f->alproto, txv, flags);
-    if (progress < 0) {
-        return false;
-    }
-    if (progress > engine->progress) {
-        return true;
-    }
-    if (progress < engine->progress) {
-        return false;
-    }
-    /* An engine registered at the completion state has no P + 1; its data is
-     * final once the tx reached that state. For the buffer engines this is the
-     * engine's eof, so absent/bsize are decided; for the firewall checks it is
-     * the point where the verdict can no longer change.
-     * AppLayerParserGetStateProgress() returns the end progress for disrupted
-     * flows, so progress == end stays a valid finality signal there. */
-    const AppLayerTxData *txd = AppLayerParserGetTxData(f->proto, f->alproto, txv);
-    uint8_t tx_end_state;
-    if (txd != NULL && txd->tx_type != 0) {
-        tx_end_state = (flags & STREAM_TOSERVER) ? txd->tx_type_eop_ts : txd->tx_type_eop_tc;
-    } else {
-        tx_end_state = AppLayerParserGetStateProgressCompletionStatus(f->alproto, flags);
-    }
-    return progress == tx_end_state;
-}
-
 /**
  * \brief Do the content inspection & validation for a signature
  *

src/detect.c

@@ -1572,18 +1572,16 @@ static int DetectRunTxInspectRule(ThreadVars *tv, DetectEngineCtx *de_ctx,
  *  \retval struct filled with relevant info or all nulls/0s
  */
 static DetectTransaction GetDetectTx(const uint8_t ipproto, const AppProto alproto,
-        const uint64_t tx_id, void *tx_ptr, const int tx_end_state, const uint8_t flow_flags)
+        const uint64_t tx_id, void *tx_ptr, const uint8_t flow_flags)
 {
-    DEBUG_VALIDATE_BUG_ON(tx_end_state >= APP_LAYER_MAX_PROGRESS);
-
     AppLayerTxData *txd = AppLayerParserGetTxData(ipproto, alproto, tx_ptr);
     const uint8_t tx_progress =
             (uint8_t)AppLayerParserGetStateProgress(ipproto, alproto, tx_ptr, flow_flags);
     DEBUG_VALIDATE_BUG_ON(tx_progress >= APP_LAYER_MAX_PROGRESS);
 
-    const uint8_t e_tx_end_state = txd->tx_type == 0                ? (uint8_t)tx_end_state
-                                   : (flow_flags & STREAM_TOSERVER) ? txd->tx_type_eop_ts
-                                                                    : txd->tx_type_eop_tc;
+    const uint8_t e_tx_end_state =
+            AppLayerParserGetTxEndState(ipproto, alproto, tx_ptr, flow_flags);
+    DEBUG_VALIDATE_BUG_ON(e_tx_end_state >= APP_LAYER_MAX_PROGRESS);
 
     bool updated = (flow_flags & STREAM_TOSERVER) ? txd->updated_ts : txd->updated_tc;
     if (!updated && tx_progress < e_tx_end_state && ((flow_flags & STREAM_EOF) == 0)) {
@@ -2569,8 +2567,6 @@ static void DetectRunTx(ThreadVars *tv,
 
     const uint64_t total_txs = AppLayerParserGetTxCnt(f, alstate);
     uint64_t tx_id_min = AppLayerParserGetTransactionInspectId(f->alparser, flow_flags);
-    const int tx_end_state = AppLayerParserGetStateProgressCompletionStatus(alproto, flow_flags);
-
     AppLayerGetTxIteratorFunc IterFunc = AppLayerGetTxIterator(ipproto, alproto);
     AppLayerGetTxIterState state = { 0 };
 
@@ -2590,8 +2586,7 @@ static void DetectRunTx(ThreadVars *tv,
             break;
         }
 
-        DetectTransaction tx =
-                GetDetectTx(ipproto, alproto, ires.tx_id, ires.tx_ptr, tx_end_state, flow_flags);
+        DetectTransaction tx = GetDetectTx(ipproto, alproto, ires.tx_id, ires.tx_ptr, flow_flags);
         if (tx.tx_ptr == NULL) {
             SCLogDebug("%p/%"PRIu64" no transaction to inspect",
                     tx.tx_ptr, tx_id_min);

src/output-tx.c

@@ -455,10 +455,6 @@ static TmEcode OutputTxLog(ThreadVars *tv, Packet *p, void *thread_data)
     AppLayerGetTxIterState state;
     memset(&state, 0, sizeof(state));
 
-    const int default_complete_ts =
-            AppLayerParserGetStateProgressCompletionStatus(alproto, STREAM_TOSERVER);
-    const int default_complete_tc =
-            AppLayerParserGetStateProgressCompletionStatus(alproto, STREAM_TOCLIENT);
     while (1) {
         AppLayerGetTxIterTuple ires = IterFunc(ipproto, alproto, alstate, tx_id, total_txs, &state);
         if (ires.tx_ptr == NULL)
@@ -468,14 +464,8 @@ static TmEcode OutputTxLog(ThreadVars *tv, Packet *p, void *thread_data)
         SCLogDebug("STARTING tx_id %" PRIu64 ", tx %p", tx_id, tx);
 
         AppLayerTxData *txd = AppLayerParserGetTxData(ipproto, alproto, tx);
-        int complete_ts, complete_tc;
-        if (txd->tx_type == 0) {
-            complete_ts = default_complete_ts;
-            complete_tc = default_complete_tc;
-        } else {
-            complete_ts = txd->tx_type_eop_ts;
-            complete_tc = txd->tx_type_eop_tc;
-        }
+        const int complete_ts = AppLayerParserGetTxEndState(ipproto, alproto, tx, STREAM_TOSERVER);
+        const int complete_tc = AppLayerParserGetTxEndState(ipproto, alproto, tx, STREAM_TOCLIENT);
 
         const int tx_progress_ts =
                 AppLayerParserGetStateProgress(ipproto, alproto, tx, ts_disrupt_flags);