diff --git a/rust/src/http2/http2.rs b/rust/src/http2/http2.rs index de51ff4aab..6490fb576e 100644 --- a/rust/src/http2/http2.rs +++ b/rust/src/http2/http2.rs @@ -883,10 +883,6 @@ 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; tx.tx_id = self.tx_id; tx.progress = HTTP2Progress::GLOBAL(HTTP2GlobalProgress::complete()); @@ -973,10 +969,6 @@ 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); return Some(self.transactions.back_mut().unwrap()); } @@ -1871,12 +1863,14 @@ pub unsafe extern "C" fn SCRegisterHttp2Parser() { HTTP2TxType::HTTP2TxTypeStream as u8, Some(HTTP2TxProgress::ffi_id_from_name), Some(HTTP2TxProgress::ffi_name_from_id), + HTTP2TxProgress::HTTP2ProgComplete as u8, ); SCAppLayerParserRegisterGetTxSubStateFuncs( ALPROTO_HTTP2, HTTP2TxType::HTTP2TxTypeGlobal as u8, Some(HTTP2TxGlobalProgress::ffi_id_from_name), Some(HTTP2TxGlobalProgress::ffi_name_from_id), + HTTP2TxGlobalProgress::HTTP2ProgGlobalComplete as u8, ); SCLogDebug!("Rust http2 parser registered."); diff --git a/rust/sys/src/sys.rs b/rust/sys/src/sys.rs index 67097e5edb..5945d001c4 100644 --- a/rust/sys/src/sys.rs +++ b/rust/sys/src/sys.rs @@ -1517,12 +1517,8 @@ pub struct AppLayerTxData { #[doc = " detection engine progress tracking for use by detection engine\n Reflects the \"progress\" of prefilter engines into this TX, where\n the value is offset by 1. So if for progress state 0 the engines\n are done, the value here will be 1. So a value of 0 means, no\n progress tracked yet.\n"] pub detect_progress_ts: u8, pub detect_progress_tc: u8, - #[doc = " Type of transaction. Meaning is defined by the parser. Used to\n select a state machine. 0 means it is not used."] + #[doc = " Type of transaction. Meaning is defined by the parser. Used to\n select a state machine. 0 means it is not used. A non-zero value is\n also the sub-state id registered by the parser, whose completion is\n the tx's end state."] pub tx_type: u8, - #[doc = " End of TX progress values\n\n toserver end of tx progress value"] - pub tx_type_eop_ts: u8, - #[doc = " toclient end of tx progress value"] - pub tx_type_eop_tc: u8, pub de_state: *mut DetectEngineState, pub events: *mut AppLayerDecoderEvents, pub txbits: *mut GenericVar, @@ -1570,7 +1566,7 @@ extern "C" { #[doc = " \\brief register state<>name funcs for a substate"] pub fn SCAppLayerParserRegisterGetTxSubStateFuncs( alproto: AppProto, sub_state: u8, GetIdByNameFunc: AppLayerParserGetStateIdByNameFn, - GetNameByIdFunc: AppLayerParserGetStateNameByIdFn, + GetNameByIdFunc: AppLayerParserGetStateNameByIdFn, completion: u8, ); } extern "C" { diff --git a/src/app-layer-parser.c b/src/app-layer-parser.c index dab0b17f53..014e85c872 100644 --- a/src/app-layer-parser.c +++ b/src/app-layer-parser.c @@ -150,6 +150,10 @@ typedef struct AppLayerParserProtoCtx_ /* max value of a sub state * only set for FLOW_PROTO_DEFAULT */ uint8_t max_sub_state; + /* progress value at which the sub-state's buffers are final, indexed by + * the sub state id + * only set for FLOW_PROTO_DEFAULT */ + uint8_t sub_state_completion[APP_LAYER_MAX_SUB_STATES]; } AppLayerParserProtoCtx; typedef struct AppLayerParserCtx_ { @@ -618,13 +622,19 @@ void AppLayerParserRegisterGetEventInfoById(uint8_t ipproto, AppProto alproto, void SCAppLayerParserRegisterGetTxSubStateFuncs(AppProto alproto, const uint8_t sub_state, AppLayerParserGetStateIdByNameFn GetIdByNameFunc, - AppLayerParserGetStateNameByIdFn GetNameByIdFunc) + AppLayerParserGetStateNameByIdFn GetNameByIdFunc, const uint8_t completion) { SCEnter(); /* validate input */ BUG_ON(sub_state == 0); BUG_ON(GetIdByNameFunc == NULL); BUG_ON(GetNameByIdFunc == NULL); + if (completion == 0 || sub_state >= APP_LAYER_MAX_SUB_STATES) { + /* 0 would make the first state look final, so it must not reach the + * end state helper */ + FatalError("invalid sub state %u (completion %u) for %s", sub_state, completion, + AppProtoToString(alproto)); + } AppLayerParserProtoCtx *p = &alp_ctx.ctxs[alproto][FLOW_PROTO_DEFAULT]; /* double registration not allowed */ @@ -640,6 +650,7 @@ void SCAppLayerParserRegisterGetTxSubStateFuncs(AppProto alproto, const uint8_t m->GetStateNameById = GetNameByIdFunc; m->next = p->sub_state_mappings; p->sub_state_mappings = m; + p->sub_state_completion[sub_state] = completion; p->max_sub_state = MAX(p->max_sub_state, sub_state); SCLogDebug("alproto %u:%s, sub_state:%u max:%u %p:%p", alproto, AppProtoToString(alproto), @@ -1180,15 +1191,16 @@ uint8_t AppLayerParserGetTxEndState(uint8_t ipproto, AppProto alproto, void *tx, if (txd == NULL || txd->tx_type == 0) { return (uint8_t)AppLayerParserGetStateProgressCompletionStatus(alproto, flags); } - const uint8_t eop = (flags & STREAM_TOSERVER) ? txd->tx_type_eop_ts : txd->tx_type_eop_tc; - if (unlikely(eop == 0)) { - /* a parser with tx types must fill both end-of-phase fields; without - * the fallback 0 would make the first state look final in release - * builds */ + /* the tx type is the sub-state the parser registered; its completion is + * the tx's end state */ + const uint8_t completion = AppLayerParserGetSubStateCompletion(alproto, txd->tx_type); + if (unlikely(completion == 0)) { + /* a parser with tx types must register the sub-state's completion: + * without the fallback 0 would make the first state look final */ DEBUG_VALIDATE_BUG_ON(1); return (uint8_t)AppLayerParserGetStateProgressCompletionStatus(alproto, flags); } - return eop; + return completion; } /** @@ -1308,18 +1320,16 @@ uint8_t AppLayerParserGetSubStateCompletion(const AppProto alproto, const uint8_ if (alproto == ALPROTO_DOH2) return AppLayerParserGetSubStateCompletion(ALPROTO_HTTP2, sub_state); - BUG_ON(alp_ctx.ctxs[alproto][FLOW_PROTO_DEFAULT].max_sub_state == 0); + const AppLayerParserProtoCtx *p = &alp_ctx.ctxs[alproto][FLOW_PROTO_DEFAULT]; - /* TODO hard coded for now */ - BUG_ON(alproto != ALPROTO_HTTP2); - - if (sub_state == 1) { - return 4; - } else if (sub_state == 2) { - return 1; - } else { - BUG_ON(1); + /* no max_sub_state guard here on purpose: an unregistered protocol lands on + * the fallback below, which is what a recovery path wants */ + if (likely(sub_state < APP_LAYER_MAX_SUB_STATES && p->sub_state_completion[sub_state] != 0)) { + return p->sub_state_completion[sub_state]; } + /* a tx type without a registered sub-state is a parser bug: the caller + * falls back to the protocol's completion status */ + DEBUG_VALIDATE_BUG_ON(1); return 0; } diff --git a/src/app-layer-parser.h b/src/app-layer-parser.h index 5e348e1eb0..42eb5d9ce7 100644 --- a/src/app-layer-parser.h +++ b/src/app-layer-parser.h @@ -80,6 +80,9 @@ uint8_t AppLayerParserGetTxEndState(uint8_t ipproto, AppProto alproto, void *tx, /** progress values need to stay under this. */ #define APP_LAYER_MAX_PROGRESS 48 +/** maximum number of sub-states a parser can register (ids 1 to N-1) */ +#define APP_LAYER_MAX_SUB_STATES 8 + /***** transaction handling *****/ int AppLayerParserSetup(void); @@ -211,14 +214,10 @@ typedef struct AppLayerTxData { uint8_t detect_progress_tc; /// Type of transaction. Meaning is defined by the parser. Used to - /// select a state machine. 0 means it is not used. + /// select a state machine. 0 means it is not used. A non-zero value is + /// also the sub-state id registered by the parser, whose completion is + /// the tx's end state. uint8_t tx_type; - /// End of TX progress values - /// - /// toserver end of tx progress value - uint8_t tx_type_eop_ts; - /// toclient end of tx progress value - uint8_t tx_type_eop_tc; DetectEngineState *de_state; AppLayerDecoderEvents *events; @@ -304,7 +303,7 @@ void AppLayerParserRegisterGetStateFuncs(uint8_t ipproto, AppProto alproto, /** \brief register state<>name funcs for a substate */ void SCAppLayerParserRegisterGetTxSubStateFuncs(AppProto alproto, const uint8_t sub_state, AppLayerParserGetStateIdByNameFn GetIdByNameFunc, - AppLayerParserGetStateNameByIdFn GetNameByIdFunc); + AppLayerParserGetStateNameByIdFn GetNameByIdFunc, uint8_t completion); void AppLayerParserRegisterTxDataFunc(uint8_t ipproto, AppProto alproto, AppLayerTxData *(*GetTxData)(void *tx));