aiden
/
suricata
mirror of https://github.com/OISF/suricata
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

detect-state: handle duplicate inspect/match

Browse Source 
If for a packet we have a TX N that has detect state and a TX N+1 that
has no detect state, but does have 'progress', we have a corner case
in stateful detection.

ContinueDetection inspects TX N, but cannot flag the rule in the
de_state_sig_array as the next (TX N+1) has already started and needs
to be inspected. 'StartDetection' however, is then unaware of the fact
that ContinueDetection already inspected the rule. It uses the per
session 'inspect_id' that is only moved forward at the end of the
detection run.

This patch adds a workaround. It uses the DetectEngineThreadCtx::
de_state_sig_array to store an offset between the 'base' inspect_id
and the inspect_id that StartDetection should use. The data type is
limited, so if the offset would be too big, a search based fall back
is implemented as well.
pull/1479/head
Victor Julien 11 years ago
parent bc6e4140be
commit 37f0bd57b6
3 changed files with 67 additions and 23 deletions
Show Stats Download Patch File Download Diff File

80
src/detect-engine-state.c
Unescape Escape View File

@ -81,6 +81,18 @@
/** convert enum to string */
#define CASE_CODE(E) case E: return #E
/** The DetectEngineThreadCtx::de_state_sig_array contains 2 separate values:
* 1. the first bit tells the prefilter engine to bypass the rule (or not)
* 2. the other bits allow 'ContinueDetect' to specify an offset again the
* base tx id. This offset will then be used by 'StartDetect' to not
* inspect transactions again for the same signature.
*
* The offset in (2) has a max value due to the limited data type. If it is
* set to max the code will fall back to a slower path that validates that
* we're not adding duplicate rules to the detection state.
*/
#define MAX_STORED_TXID_OFFSET 127
/******** static internal helpers *********/
static inline int StateIsValid(uint16_t alproto, void *alstate)
@ -124,7 +136,6 @@ static DeStateStoreFlowRules *DeStateStoreFlowRulesAlloc(void)
return d;
}
#ifdef DEBUG_VALIDATION
static int DeStateSearchState(DetectEngineState *state, uint8_t direction, SigIntId num)
{
DetectEngineStateDirection *dir_state = &state->dir_state[direction & STREAM_TOSERVER ? 0 : 1];
@ -140,7 +151,7 @@ static int DeStateSearchState(DetectEngineState *state, uint8_t direction, SigIn
{
DeStateStoreItem *item = &tx_store->store[store_cnt];
if (item->sid == num) {
SCLogDebug("BUG! sid %u already in state: %p %p %p %u %u, direction %s",
SCLogDebug("sid %u already in state: %p %p %p %u %u, direction %s",
num, state, dir_state, tx_store, state_cnt,
store_cnt, direction & STREAM_TOSERVER ? "toserver" : "toclient");
return 1;
@ -149,7 +160,6 @@ static int DeStateSearchState(DetectEngineState *state, uint8_t direction, SigIn
}
return 0;
}
#endif
static void DeStateSignatureAppend(DetectEngineState *state, Signature *s, uint32_t inspect_flags, uint8_t direction)
{
@ -424,11 +434,14 @@ static void StoreStateTxFileOnly(DetectEngineThreadCtx *det_ctx,
}
}
/**
* \param check_before_add check for duplicates before adding the sig
*/
static void StoreStateTx(DetectEngineThreadCtx *det_ctx,
Flow *f, const uint8_t flags, const uint16_t alversion,
const uint64_t tx_id, void *tx,
Signature *s, SigMatch *sm,
const uint32_t inspect_flags, const uint16_t file_no_match)
const uint32_t inspect_flags, const uint16_t file_no_match, int check_before_add)
{
if (AppLayerParserSupportsTxDetectState(f->proto, f->alproto)) {
DetectEngineState *destate = AppLayerParserGetTxDetectState(f->proto, f->alproto, tx);
@ -444,7 +457,8 @@ static void StoreStateTx(DetectEngineThreadCtx *det_ctx,
SCLogDebug("destate created for %"PRIu64, tx_id);
}
DeStateSignatureAppend(destate, s, inspect_flags, flags);
if (check_before_add == 0 || DeStateSearchState(destate, flags, s->num) == 0)
DeStateSignatureAppend(destate, s, inspect_flags, flags);
DeStateStoreStateVersion(f, alversion, flags);
StoreStateTxHandleFiles(det_ctx, f, destate, flags, tx_id, file_no_match);
@ -462,6 +476,7 @@ int DeStateDetectStartDetection(ThreadVars *tv, DetectEngineCtx *de_ctx,
uint32_t inspect_flags = 0;
uint8_t direction = (flags & STREAM_TOSERVER) ? 0 : 1;
int alert_cnt = 0;
int check_before_add = 0;
FLOWLOCK_WRLOCK(f);
/* TX based matches (inspect engines) */
@ -474,7 +489,18 @@ int DeStateDetectStartDetection(ThreadVars *tv, DetectEngineCtx *de_ctx,
goto end;
}
/* if continue detection already inspected this rule for this tx,
* continue with the first not-inspected tx */
uint8_t offset = det_ctx->de_state_sig_array[s->num] & 0xef;
tx_id = AppLayerParserGetTransactionInspectId(f->alparser, flags);
if (offset > 0) {
SCLogDebug("using stored_tx_id %u instead of %u", (uint)tx_id+offset, (uint)tx_id);
tx_id += offset;
}
if (offset == MAX_STORED_TXID_OFFSET) {
check_before_add = 1;
}
total_txs = AppLayerParserGetTxCnt(f->proto, alproto, alstate);
SCLogDebug("total_txs %"PRIu64, total_txs);
@ -557,7 +583,7 @@ int DeStateDetectStartDetection(ThreadVars *tv, DetectEngineCtx *de_ctx,
/* store */
StoreStateTx(det_ctx, f, flags, alversion, tx_id, tx,
s, sm, inspect_flags, file_no_match);
s, sm, inspect_flags, file_no_match, check_before_add);
} else {
StoreStateTxFileOnly(det_ctx, f, flags, tx_id, tx, file_no_match);
}
@ -704,6 +730,16 @@ static int DoInspectItem(ThreadVars *tv,
SCLogDebug("skip and bypass: tx %u packet %u", (uint)inspect_tx_id, (uint)p->pcap_cnt);
} else {
SCLogDebug("just skip: tx %u packet %u", (uint)inspect_tx_id, (uint)p->pcap_cnt);
/* make sure that if we reinspect this right now from
* start detection, we skip this tx we just matched on */
uint64_t base_tx_id = AppLayerParserGetTransactionInspectId(f->alparser, flags);
uint64_t offset = (inspect_tx_id + 1) - base_tx_id;
if (offset > MAX_STORED_TXID_OFFSET)
offset = MAX_STORED_TXID_OFFSET;
det_ctx->de_state_sig_array[item->sid] = (uint8_t)offset;
BUG_ON(det_ctx->de_state_sig_array[item->sid] & DE_STATE_MATCH_NO_NEW_STATE); // check that we don't set the bit
SCLogDebug("storing tx_id %u for this sid", (uint)inspect_tx_id + 1);
}
return 0;
}
@ -730,6 +766,16 @@ static int DoInspectItem(ThreadVars *tv,
SCLogDebug("skip and bypass: tx %u packet %u", (uint)inspect_tx_id, (uint)p->pcap_cnt);
} else {
SCLogDebug("just skip: tx %u packet %u", (uint)inspect_tx_id, (uint)p->pcap_cnt);
/* make sure that if we reinspect this right now from
* start detection, we skip this tx we just matched on */
uint64_t base_tx_id = AppLayerParserGetTransactionInspectId(f->alparser, flags);
uint64_t offset = (inspect_tx_id + 1) - base_tx_id;
if (offset > MAX_STORED_TXID_OFFSET)
offset = MAX_STORED_TXID_OFFSET;
det_ctx->de_state_sig_array[item->sid] = (uint8_t)offset;
BUG_ON(det_ctx->de_state_sig_array[item->sid] & DE_STATE_MATCH_NO_NEW_STATE); // check that we don't set the bit
SCLogDebug("storing tx_id %u for this sid", (uint)inspect_tx_id + 1);
}
return 0;
}
@ -794,6 +840,17 @@ static int DoInspectItem(ThreadVars *tv,
if (TxIsLast(inspect_tx_id, total_txs) || inprogress || next_tx_no_progress) {
det_ctx->de_state_sig_array[item->sid] = DE_STATE_MATCH_NO_NEW_STATE;
SCLogDebug("inspected, now bypass: tx %u packet %u", (uint)inspect_tx_id, (uint)p->pcap_cnt);
} else {
/* make sure that if we reinspect this right now from
* start detection, we skip this tx we just matched on */
uint64_t base_tx_id = AppLayerParserGetTransactionInspectId(f->alparser, flags);
uint64_t offset = (inspect_tx_id + 1) - base_tx_id;
if (offset > MAX_STORED_TXID_OFFSET)
offset = MAX_STORED_TXID_OFFSET;
det_ctx->de_state_sig_array[item->sid] = (uint8_t)offset;
BUG_ON(det_ctx->de_state_sig_array[item->sid] & DE_STATE_MATCH_NO_NEW_STATE); // check that we don't set the bit
SCLogDebug("storing tx_id %u for this sid", (uint)inspect_tx_id + 1);
}
RULE_PROFILING_END(det_ctx, s, (alert == 1), p);
@ -1094,17 +1151,6 @@ void DetectEngineStateResetTxs(Flow *f)
}
}
/** \brief get string for match enum */
const char *DeStateMatchResultToString(DeStateMatchResult res)
{
switch (res) {
CASE_CODE (DE_STATE_MATCH_NO_NEW_STATE);
CASE_CODE (DE_STATE_MATCH_HAS_NEW_STATE);
}
return NULL;
}
/*********Unittests*********/
#ifdef UNITTESTS

6
src/detect-engine-state.h
Unescape Escape View File

@ -100,10 +100,8 @@
* the HAS_NEW_STATE flag, while if we don't have a new tx, we set
* NO_NEW_STATE, to avoid getting the sig reinspected for the already
* inspected tx. */
typedef enum {
DE_STATE_MATCH_HAS_NEW_STATE = 0,
DE_STATE_MATCH_NO_NEW_STATE,
} DeStateMatchResult;
#define DE_STATE_MATCH_HAS_NEW_STATE 0x00
#define DE_STATE_MATCH_NO_NEW_STATE 0x80
/* TX BASED (inspect engines) */

4
src/detect.c
Unescape Escape View File

@ -1374,10 +1374,10 @@ int SigMatchSignatures(ThreadVars *th_v, DetectEngineCtx *de_ctx, DetectEngineTh
PACKET_PROFILING_DETECT_START(p, PROF_DETECT_STATEFUL);
/* stateful app layer detection */
if ((p->flags & PKT_HAS_FLOW) && has_state) {
/* initialize to 0(DE_STATE_MATCH_HAS_NEW_STATE) */
memset(det_ctx->de_state_sig_array, 0x00, det_ctx->de_state_sig_array_len);
int has_inspectable_state = DeStateFlowHasInspectableState(pflow, alproto, alversion, flags);
if (has_inspectable_state == 1) {
/* initialize to 0(DE_STATE_MATCH_HAS_NEW_STATE) */
DeStateDetectContinueDetection(th_v, de_ctx, det_ctx, p, pflow,
flags, alproto, alversion);
} else if (has_inspectable_state == 2) {
@ -1500,7 +1500,7 @@ int SigMatchSignatures(ThreadVars *th_v, DetectEngineCtx *de_ctx, DetectEngineTh
}
}
if (sflags & SIG_FLAG_STATE_MATCH) {
if (det_ctx->de_state_sig_array[s->num] == DE_STATE_MATCH_NO_NEW_STATE)
if (det_ctx->de_state_sig_array[s->num] & DE_STATE_MATCH_NO_NEW_STATE)
goto next;
}

Write Preview
Loading…
Cancel
Save