diff --git a/src/app-layer-htp.c b/src/app-layer-htp.c index e8ab0e25573..3f3037ca30a 100644 --- a/src/app-layer-htp.c +++ b/src/app-layer-htp.c @@ -1172,8 +1172,6 @@ static void FlagDetectStateNewFile(HtpTxUserData *tx, int dir) SCLogDebug("DETECT_ENGINE_STATE_FLAG_FILE_NEW set"); tx->tx_data.de_state->dir_state[1].flags |= DETECT_ENGINE_STATE_FLAG_FILE_NEW; } - tx->tx_data.de_state->dir_state[DETECT_ENGINE_STATE_DIRECTION_BOTHDIR].flags |= - DETECT_ENGINE_STATE_FLAG_FILE_NEW; } } diff --git a/src/app-layer-smtp.c b/src/app-layer-smtp.c index 76ecd29864a..03260bfd3ae 100644 --- a/src/app-layer-smtp.c +++ b/src/app-layer-smtp.c @@ -495,8 +495,6 @@ static void FlagDetectStateNewFile(SMTPTransaction *tx) if (tx && tx->tx_data.de_state) { SCLogDebug("DETECT_ENGINE_STATE_FLAG_FILE_NEW set"); tx->tx_data.de_state->dir_state[0].flags |= DETECT_ENGINE_STATE_FLAG_FILE_NEW; - tx->tx_data.de_state->dir_state[DETECT_ENGINE_STATE_DIRECTION_BOTHDIR].flags |= - DETECT_ENGINE_STATE_FLAG_FILE_NEW; } else if (tx == NULL) { SCLogDebug("DETECT_ENGINE_STATE_FLAG_FILE_NEW NOT set, no TX"); } else if (tx->tx_data.de_state == NULL) { diff --git a/src/detect-engine-prefilter.h b/src/detect-engine-prefilter.h index 8a81aa170be..ec58594b9b0 100644 --- a/src/detect-engine-prefilter.h +++ b/src/detect-engine-prefilter.h @@ -33,8 +33,6 @@ typedef struct DetectTransaction_ { const uint64_t tx_id; struct AppLayerTxData *tx_data_ptr; DetectEngineStateDirection *de_state; - // state for bidirectional signatures - DetectEngineStateDirection *de_state_bidir; const uint64_t detect_flags; /* detect flags get/set from/to applayer */ uint64_t prefilter_flags; /* prefilter flags for direction, to be updated by prefilter code */ const uint64_t diff --git a/src/detect-engine-state.c b/src/detect-engine-state.c index 0cebf01c884..851f4ded6d6 100644 --- a/src/detect-engine-state.c +++ b/src/detect-engine-state.c @@ -89,9 +89,9 @@ static DeStateStore *DeStateStoreAlloc(void) } #ifdef DEBUG_VALIDATION -static int DeStateSearchState(DetectEngineState *state, uint8_t index, SigIntId num) +static int DeStateSearchState(DetectEngineState *state, uint8_t direction, SigIntId num) { - DetectEngineStateDirection *dir_state = &state->dir_state[index]; + DetectEngineStateDirection *dir_state = &state->dir_state[direction & STREAM_TOSERVER ? 0 : 1]; DeStateStore *tx_store = dir_state->head; SigIntId store_cnt; SigIntId state_cnt = 0; @@ -120,15 +120,11 @@ static void DeStateSignatureAppend(DetectEngineState *state, { SCEnter(); - uint8_t index = (direction & STREAM_TOSERVER) ? 0 : 1; - if (s->flags & SIG_FLAG_BOTHDIR) { - index = DETECT_ENGINE_STATE_DIRECTION_BOTHDIR; - } - - DetectEngineStateDirection *dir_state = &state->dir_state[index]; + DetectEngineStateDirection *dir_state = + &state->dir_state[(direction & STREAM_TOSERVER) ? 0 : 1]; #ifdef DEBUG_VALIDATION - BUG_ON(DeStateSearchState(state, index, s->num)); + BUG_ON(DeStateSearchState(state, direction, s->num)); #endif DeStateStore *store = dir_state->tail; if (store == NULL) { @@ -176,7 +172,7 @@ void DetectEngineStateFree(DetectEngineState *state) DeStateStore *store_next; int i = 0; - for (i = 0; i < DETECT_ENGINE_STATE_DIRECTIONS; i++) { + for (i = 0; i < 2; i++) { store = state->dir_state[i].head; while (store != NULL) { store_next = store->next; @@ -235,6 +231,11 @@ void DetectRunStoreStateTx( SCLogDebug("destate created for %"PRIu64, tx_id); } DeStateSignatureAppend(tx_data->de_state, s, inspect_flags, flow_flags); + if (s->flags & SIG_FLAG_BOTHDIR) { + // add also in the other DetectEngineStateDirection + DeStateSignatureAppend(tx_data->de_state, s, inspect_flags, + flow_flags ^ (STREAM_TOSERVER | STREAM_TOCLIENT)); + } StoreStateTxHandleFiles(sgh, f, tx_data->de_state, flow_flags, tx, tx_id, file_no_match); SCLogDebug("Stored for TX %"PRIu64, tx_id); @@ -243,13 +244,17 @@ void DetectRunStoreStateTx( static inline void ResetTxState(DetectEngineState *s) { if (s) { - for (int i = 0; i < DETECT_ENGINE_STATE_DIRECTIONS; i++) { - s->dir_state[i].cnt = 0; - s->dir_state[i].filestore_cnt = 0; - s->dir_state[i].flags = 0; - /* reset 'cur' back to the list head */ - s->dir_state[i].cur = s->dir_state[i].head; - } + s->dir_state[0].cnt = 0; + s->dir_state[0].filestore_cnt = 0; + s->dir_state[0].flags = 0; + /* reset 'cur' back to the list head */ + s->dir_state[0].cur = s->dir_state[0].head; + + s->dir_state[1].cnt = 0; + s->dir_state[1].filestore_cnt = 0; + s->dir_state[1].flags = 0; + /* reset 'cur' back to the list head */ + s->dir_state[1].cur = s->dir_state[1].head; } } diff --git a/src/detect-engine-state.h b/src/detect-engine-state.h index 34f78e89754..326b3bad4ec 100644 --- a/src/detect-engine-state.h +++ b/src/detect-engine-state.h @@ -89,17 +89,8 @@ typedef struct DetectEngineStateDirection_ { /* coccinelle: DetectEngineStateDirection:flags:DETECT_ENGINE_STATE_FLAG_ */ } DetectEngineStateDirection; -#define DETECT_ENGINE_STATE_DIRECTIONS 3 - -enum { - DETECT_ENGINE_STATE_DIRECTION_TOSERVER = 0, - DETECT_ENGINE_STATE_DIRECTION_TOCLIENT = 1, - DETECT_ENGINE_STATE_DIRECTION_BOTHDIR = 2, -}; - typedef struct DetectEngineState_ { - // to server, to client, and bidirectional - DetectEngineStateDirection dir_state[DETECT_ENGINE_STATE_DIRECTIONS]; + DetectEngineStateDirection dir_state[2]; } DetectEngineState; /** diff --git a/src/detect.c b/src/detect.c index 225494bc6c3..1948eddec7a 100644 --- a/src/detect.c +++ b/src/detect.c @@ -1296,7 +1296,7 @@ static bool DetectRunTxInspectRule(ThreadVars *tv, #define NO_TX \ { \ - NULL, 0, NULL, NULL, NULL, 0, 0, 0, 0, 0, \ + NULL, 0, NULL, NULL, 0, 0, 0, 0, 0, \ } /** \internal @@ -1336,18 +1336,16 @@ static DetectTransaction GetDetectTx(const uint8_t ipproto, const AppProto alpro DEBUG_VALIDATE_BUG_ON(prefilter_flags & APP_LAYER_TX_RESERVED_FLAGS); DetectTransaction tx = { - .tx_ptr = tx_ptr, - .tx_id = tx_id, - .tx_data_ptr = (struct AppLayerTxData *)txd, - .de_state = tx_dir_state, - .de_state_bidir = - tx_de_state ? &tx_de_state->dir_state[DETECT_ENGINE_STATE_DIRECTION_BOTHDIR] : NULL, - .detect_flags = detect_flags, - .prefilter_flags = prefilter_flags, - .prefilter_flags_orig = prefilter_flags, - .tx_progress = tx_progress, - .tx_end_state = tx_end_state, - }; + .tx_ptr = tx_ptr, + .tx_id = tx_id, + .tx_data_ptr = (struct AppLayerTxData *)txd, + .de_state = tx_dir_state, + .detect_flags = detect_flags, + .prefilter_flags = prefilter_flags, + .prefilter_flags_orig = prefilter_flags, + .tx_progress = tx_progress, + .tx_end_state = tx_end_state, + }; return tx; } @@ -1364,52 +1362,6 @@ static inline void StoreDetectFlags(DetectTransaction *tx, const uint8_t flow_fl } } -static bool RuleMatchCandidateMergeStoredState(DetectEngineCtx *de_ctx, - DetectEngineThreadCtx *det_ctx, DetectTransaction tx, DetectEngineStateDirection *de_state, - uint32_t *array_idx) -{ - if (de_state == NULL) { - return false; - } - const uint32_t old = *array_idx; - - /* if de_state->flags has 'new file' set and sig below has - * 'file inspected' flag, reset the file part of the state */ - const bool have_new_file = (de_state->flags & DETECT_ENGINE_STATE_FLAG_FILE_NEW); - if (have_new_file) { - SCLogDebug("%p/%" PRIu64 " destate: need to consider new file", tx.tx_ptr, tx.tx_id); - de_state->flags &= ~DETECT_ENGINE_STATE_FLAG_FILE_NEW; - } - - SigIntId state_cnt = 0; - DeStateStore *tx_store = de_state->head; - for (; tx_store != NULL; tx_store = tx_store->next) { - SCLogDebug("tx_store %p", tx_store); - - SigIntId store_cnt = 0; - for (store_cnt = 0; store_cnt < DE_STATE_CHUNK_SIZE && state_cnt < de_state->cnt; - store_cnt++, state_cnt++) { - DeStateStoreItem *item = &tx_store->store[store_cnt]; - SCLogDebug("rule id %u, inspect_flags %u", item->sid, item->flags); - if (have_new_file && (item->flags & DE_STATE_FLAG_FILE_INSPECT)) { - /* remove part of the state. File inspect engine will now - * be able to run again */ - item->flags &= ~(DE_STATE_FLAG_SIG_CANT_MATCH | DE_STATE_FLAG_FULL_INSPECT | - DE_STATE_FLAG_FILE_INSPECT); - SCLogDebug("rule id %u, post file reset inspect_flags %u", item->sid, item->flags); - } - det_ctx->tx_candidates[*array_idx].s = de_ctx->sig_array[item->sid]; - det_ctx->tx_candidates[*array_idx].id = item->sid; - det_ctx->tx_candidates[*array_idx].flags = &item->flags; - det_ctx->tx_candidates[*array_idx].stream_reset = 0; - (*array_idx)++; - } - } - SCLogDebug("%p/%" PRIu64 " rules added from 'continue' list: %u", tx.tx_ptr, tx.tx_id, - *array_idx - old); - return (old && old != *array_idx); // sort if continue list adds sids -} - // Merge 'state' rules from the regular prefilter // updates array_idx on the way static inline void RuleMatchCandidateMergeStateRules( @@ -1525,7 +1477,6 @@ static void DetectRunTx(ThreadVars *tv, uint32_t array_idx = 0; uint32_t total_rules = det_ctx->match_array_cnt; total_rules += (tx.de_state ? tx.de_state->cnt : 0); - total_rules += (tx.de_state_bidir ? tx.de_state_bidir->cnt : 0); /* run prefilter engines and merge results into a candidates array */ if (sgh->tx_engines) { @@ -1564,9 +1515,47 @@ static void DetectRunTx(ThreadVars *tv, RuleMatchCandidateMergeStateRules(det_ctx, &array_idx); /* merge stored state into results */ - do_sort |= RuleMatchCandidateMergeStoredState(de_ctx, det_ctx, tx, tx.de_state, &array_idx); - do_sort |= RuleMatchCandidateMergeStoredState( - de_ctx, det_ctx, tx, tx.de_state_bidir, &array_idx); + if (tx.de_state != NULL) { + const uint32_t old = array_idx; + + /* if tx.de_state->flags has 'new file' set and sig below has + * 'file inspected' flag, reset the file part of the state */ + const bool have_new_file = (tx.de_state->flags & DETECT_ENGINE_STATE_FLAG_FILE_NEW); + if (have_new_file) { + SCLogDebug("%p/%"PRIu64" destate: need to consider new file", + tx.tx_ptr, tx.tx_id); + tx.de_state->flags &= ~DETECT_ENGINE_STATE_FLAG_FILE_NEW; + } + + SigIntId state_cnt = 0; + DeStateStore *tx_store = tx.de_state->head; + for (; tx_store != NULL; tx_store = tx_store->next) { + SCLogDebug("tx_store %p", tx_store); + + SigIntId store_cnt = 0; + for (store_cnt = 0; + store_cnt < DE_STATE_CHUNK_SIZE && state_cnt < tx.de_state->cnt; + store_cnt++, state_cnt++) + { + DeStateStoreItem *item = &tx_store->store[store_cnt]; + SCLogDebug("rule id %u, inspect_flags %u", item->sid, item->flags); + if (have_new_file && (item->flags & DE_STATE_FLAG_FILE_INSPECT)) { + /* remove part of the state. File inspect engine will now + * be able to run again */ + item->flags &= ~(DE_STATE_FLAG_SIG_CANT_MATCH|DE_STATE_FLAG_FULL_INSPECT|DE_STATE_FLAG_FILE_INSPECT); + SCLogDebug("rule id %u, post file reset inspect_flags %u", item->sid, item->flags); + } + det_ctx->tx_candidates[array_idx].s = de_ctx->sig_array[item->sid]; + det_ctx->tx_candidates[array_idx].id = item->sid; + det_ctx->tx_candidates[array_idx].flags = &item->flags; + det_ctx->tx_candidates[array_idx].stream_reset = 0; + array_idx++; + } + } + do_sort |= (old && old != array_idx); // sort if continue list adds sids + SCLogDebug("%p/%" PRIu64 " rules added from 'continue' list: %u", tx.tx_ptr, tx.tx_id, + array_idx - old); + } if (do_sort) { qsort(det_ctx->tx_candidates, array_idx, sizeof(RuleMatchCandidateTx), DetectRunTxSortHelper);