diff --git a/rust/sys/src/sys.rs b/rust/sys/src/sys.rs index 0f0e7d3d65..566c570d05 100644 --- a/rust/sys/src/sys.rs +++ b/rust/sys/src/sys.rs @@ -21,6 +21,7 @@ pub const SIGMATCH_INFO_UINT32: u32 = 65536; pub const SIGMATCH_INFO_UINT64: u32 = 131072; pub const SIGMATCH_INFO_MULTI_UINT: u32 = 262144; pub const SIGMATCH_INFO_ENUM_UINT: u32 = 524288; +pub const SIGMATCH_STATEFUL: u32 = 8388608; pub const SIGMATCH_INFO_BITFLAGS_UINT: u32 = 1048576; pub const SIGMATCH_BAN_FIREWALL_RULE: u32 = 2097152; pub const SIGMATCH_BAN_FIREWALL_MODE: u32 = 4194304; diff --git a/src/app-layer-parser.c b/src/app-layer-parser.c index be7d9394fe..41fd413789 100644 --- a/src/app-layer-parser.c +++ b/src/app-layer-parser.c @@ -177,7 +177,6 @@ struct AppLayerParserState_ { FramesContainer *frames; }; -static inline uint8_t GetTxEndProgress(uint8_t ipproto, AppProto alproto, void *tx, uint8_t flags); static inline uint8_t GetTxdEndProgress(uint8_t ipproto, AppProto alproto, const AppLayerTxData *txd, uint8_t flags, uint8_t complete); @@ -1185,20 +1184,21 @@ static inline int StateGetProgressCompletionStatus(const AppProto alproto, const * If the TX is supporting sub-states, return the value from the txd. * \param tx pointer to the transaction */ -static inline uint8_t GetTxEndProgress(uint8_t ipproto, AppProto alproto, void *tx, uint8_t flags) +uint8_t AppLayerParserGetTxEndState(uint8_t ipproto, AppProto alproto, void *tx, uint8_t flags) { const AppLayerTxData *txd = AppLayerParserGetTxData(ipproto, alproto, tx); - DEBUG_VALIDATE_BUG_ON(txd == NULL); - 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; + 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 */ + DEBUG_VALIDATE_BUG_ON(1); + return (uint8_t)AppLayerParserGetStateProgressCompletionStatus(alproto, flags); } - return tx_end_state; + return eop; } /** \internal @@ -1239,7 +1239,7 @@ int AppLayerParserGetStateProgress(uint8_t ipproto, AppProto alproto, void *tx, SCEnter(); int r; if (unlikely(IS_DISRUPTED(flags))) { - r = (int)GetTxEndProgress(ipproto, alproto, tx, flags); + r = (int)AppLayerParserGetTxEndState(ipproto, alproto, tx, flags); } else { const uint8_t direction = flags & (STREAM_TOCLIENT | STREAM_TOSERVER); r = alp_ctx.ctxs[alproto][FlowGetProtoMapping(ipproto)].StateGetProgress(tx, direction); diff --git a/src/app-layer-parser.h b/src/app-layer-parser.h index ebddac421c..5e348e1eb0 100644 --- a/src/app-layer-parser.h +++ b/src/app-layer-parser.h @@ -71,6 +71,12 @@ typedef struct AppLayerGetFileState AppLayerGetFileState; int AppLayerParserProtoIsRegistered(uint8_t ipproto, AppProto alproto); +/** \brief get the end state (progress) for a transaction. + * + * Uses the transaction type specific end state when the parser provides one, + * else the protocol completion status. Tolerates a NULL tx data pointer. */ +uint8_t AppLayerParserGetTxEndState(uint8_t ipproto, AppProto alproto, void *tx, uint8_t flags); + /** progress values need to stay under this. */ #define APP_LAYER_MAX_PROGRESS 48 diff --git a/src/app-layer-ssl.c b/src/app-layer-ssl.c index 80640eae6e..e98a00793f 100644 --- a/src/app-layer-ssl.c +++ b/src/app-layer-ssl.c @@ -1624,6 +1624,9 @@ static int TLSDecodeHandshakeHello(SSLState *ssl_state, if (SC_ATOMIC_GET(ssl_config.enable_ja3) && ssl_state->curr_connp->ja3_hash == NULL) { ssl_state->curr_connp->ja3_hash = Ja3GenerateHash(ssl_state->curr_connp->ja3_str); } + /* the hello, supported_versions included, decoded cleanly: its version + * is final for this connp */ + ssl_state->curr_connp->hello_decoded = true; return 0; fail: diff --git a/src/app-layer-ssl.h b/src/app-layer-ssl.h index 6f51368f9a..b8da3c6173 100644 --- a/src/app-layer-ssl.h +++ b/src/app-layer-ssl.h @@ -217,6 +217,11 @@ typedef struct SSLStateConnp_ { * ServerHello completion or a certificate header) with an * incomplete chain */ bool cert_chain_final; + /* a hello (including its supported_versions extension) decoded cleanly: + * this connp's version is final from here on. A failed hello sets the + * legacy version but leaves this clear, so a later record must not turn + * a partially decoded hello into a version match */ + bool hello_decoded; int64_t cert0_not_before; int64_t cert0_not_after; char *cert0_fingerprint; diff --git a/src/detect-engine-prefilter.c b/src/detect-engine-prefilter.c index cdef64ceda..6368382a00 100644 --- a/src/detect-engine-prefilter.c +++ b/src/detect-engine-prefilter.c @@ -148,8 +148,9 @@ void DetectRunPrefilterTx(DetectEngineThreadCtx *det_ctx, tx->tx_progress, engine->ctx.app.tx_min_progress); /* if state value is at or beyond engine state, we can skip it. It means we ran at - * least once already. */ - if (tx->detect_progress > engine->ctx.app.tx_min_progress) { + * least once already. A run-always engine (stateful keyword) is revisited on + * every update, so detect_progress must not skip it. */ + if (!engine->run_always && tx->detect_progress > engine->ctx.app.tx_min_progress) { SCLogDebug("tx already marked progress as beyond engine: %u > %u", tx->detect_progress, engine->ctx.app.tx_min_progress); goto next; @@ -177,7 +178,13 @@ void DetectRunPrefilterTx(DetectEngineThreadCtx *det_ctx, * though a parser may use 0 as a valid value. */ // tx->tx_ptr == tx_ptr ensures we do not use a dns engine progress // to update a HTTP2 tx detect_progress in case of DOH2 - tx->detect_progress = engine->ctx.app.tx_min_progress + 1; + // + // monotonic: a run-always engine at a lower progress is revisited + // after the tx moved on, and must not re-enable the engines it + // already completed + if (tx->detect_progress < engine->ctx.app.tx_min_progress + 1) { + tx->detect_progress = engine->ctx.app.tx_min_progress + 1; + } SCLogDebug("tx->tx_progress %d engine->ctx.app.tx_min_progress %d " "engine->is_last_for_progress %d => tx->detect_progress updated to %02x", tx->tx_progress, engine->ctx.app.tx_min_progress, @@ -739,6 +746,10 @@ struct TxNonPFData { uint8_t progress; /**< progress state value to register at */ int sig_list; /**< special handling: normally 0, but for special cases (app-layer-state, app-layer-event) use the list id to create separate engines */ + /** the buffer holds a stateful keyword: evaluate its engine on every tx + * update (engine progress -1) so a provisional miss can be revisited as + * the transaction advances. */ + bool run_always; uint32_t sigs_cnt; struct PrefilterNonPFDataSig *sigs; const char *engine_name; /**< pointer to name owned by DetectEngineCtx::non_pf_engine_names */ @@ -747,7 +758,8 @@ struct TxNonPFData { static uint32_t TxNonPFHash(HashListTable *h, void *data, uint16_t _len) { struct TxNonPFData *d = data; - return (d->alproto + d->sub_state + d->progress + d->dir + d->sig_list) % h->array_size; + return (d->alproto + d->sub_state + d->progress + d->dir + d->sig_list + d->run_always) % + h->array_size; } static char TxNonPFCompare(void *data1, uint16_t _len1, void *data2, uint16_t len2) @@ -755,7 +767,8 @@ static char TxNonPFCompare(void *data1, uint16_t _len1, void *data2, uint16_t le struct TxNonPFData *d1 = data1; struct TxNonPFData *d2 = data2; return d1->alproto == d2->alproto && d1->sub_state == d2->sub_state && - d1->progress == d2->progress && d1->dir == d2->dir && d1->sig_list == d2->sig_list; + d1->progress == d2->progress && d1->dir == d2->dir && d1->sig_list == d2->sig_list && + d1->run_always == d2->run_always; } static void TxNonPFFree(void *data) @@ -767,7 +780,7 @@ static void TxNonPFFree(void *data) static int TxNonPFAddSig(DetectEngineCtx *de_ctx, HashListTable *tx_engines_hash, const AppProto alproto, const uint8_t sub_state, const int dir, const uint8_t progress, - const int sig_list, const char *name, const Signature *s) + const int sig_list, const char *name, const Signature *s, const bool run_always) { const uint32_t max_sids = DetectEngineGetMaxSigId(de_ctx); @@ -777,6 +790,7 @@ static int TxNonPFAddSig(DetectEngineCtx *de_ctx, HashListTable *tx_engines_hash .dir = dir, .progress = progress, .sig_list = sig_list, + .run_always = run_always, .sigs_cnt = 0, .sigs = NULL, .engine_name = NULL, @@ -809,6 +823,7 @@ static int TxNonPFAddSig(DetectEngineCtx *de_ctx, HashListTable *tx_engines_hash add->alproto = alproto; add->progress = progress; add->sig_list = sig_list; + add->run_always = run_always; add->sigs = SCCalloc(max_sids, sizeof(struct PrefilterNonPFDataSig)); if (add->sigs == NULL) { SCFree(add); @@ -992,7 +1007,7 @@ static int SetupNonPrefilter(DetectEngineCtx *de_ctx, SigGroupHead *sgh) const int sm_list = DetectEngineAppHookToSmlist(s->alproto, sub_state, state, direction); if (TxNonPFAddSig(de_ctx, tx_engines_hash, s->alproto, sub_state, dir, state, - sm_list, pname, s) != 0) { + sm_list, pname, s, false) != 0) { goto error; } tx_non_pf = true; @@ -1062,9 +1077,12 @@ static int SetupNonPrefilter(DetectEngineCtx *de_ctx, SigGroupHead *sgh) int sig_list = 0; if (list_id == app_state_list_id) sig_list = app_state_list_id; + /* buffers marked run-always (stateful keywords) must be + * revisited as the tx advances. */ + const bool run_always = buf != NULL && buf->run_always; const uint8_t sub_state = app->sub_state; if (TxNonPFAddSig(de_ctx, tx_engines_hash, app->alproto, sub_state, app->dir, - app->progress, sig_list, buf->name, s) != 0) { + app->progress, sig_list, buf->name, s, run_always) != 0) { goto error; } tx_non_pf = true; @@ -1086,8 +1104,8 @@ static int SetupNonPrefilter(DetectEngineCtx *de_ctx, SigGroupHead *sgh) uint8_t sub_state = s->init_data->hook.t.app.sub_state; if (TxNonPFAddSig(de_ctx, tx_engines_hash, s->alproto, sub_state, dir, - s->init_data->hook.t.app.app_progress, s->init_data->hook.sm_list, pname, - s) != 0) { + s->init_data->hook.t.app.app_progress, s->init_data->hook.sm_list, pname, s, + false) != 0) { goto error; } tx_non_pf = true; @@ -1163,11 +1181,13 @@ static int SetupNonPrefilter(DetectEngineCtx *de_ctx, SigGroupHead *sgh) continue; } - /* register special progress value to indicate we need to run it all the time */ DEBUG_VALIDATE_BUG_ON(t->progress > INT8_MAX); int8_t engine_progress = (int8_t)t->progress; if (t->sig_list == app_state_list_id) { - SCLogDebug("engine %s for state list", t->engine_name); + /* app-layer-state engines use the -1 "run every update" sentinel; + * they are proto-agnostic (ALPROTO_UNKNOWN), which that path + * requires. */ + SCLogDebug("engine %s for state list, run always", t->engine_name); engine_progress = -1; } @@ -1184,6 +1204,15 @@ static int SetupNonPrefilter(DetectEngineCtx *de_ctx, SigGroupHead *sgh) SCFree(data); goto error; } + if (t->run_always) { + /* A stateful keyword must be revisited as the tx advances. Keep + * its real progress (so the progress bookkeeping runs and the + * proto-agnostic -1 invariant holds) and carry an explicit flag. */ + PrefilterEngineList *tail = sgh->init->tx_engines; + while (tail->next != NULL) + tail = tail->next; + tail->run_always = true; + } } HashListTableFree(tx_engines_hash); tx_engines_hash = NULL; @@ -1360,6 +1389,7 @@ int PrefilterSetupRuleGroup(DetectEngineCtx *de_ctx, SigGroupHead *sgh) e->alproto = el->alproto; e->ctx.app.tx_min_progress = el->tx_min_progress; e->ctx.app.sub_state = el->sub_state; + e->run_always = el->run_always; e->cb.PrefilterTx = el->PrefilterTx; e->pectx = el->pectx; el->pectx = NULL; // e now owns the ctx diff --git a/src/detect-engine-register.h b/src/detect-engine-register.h index 221b25ef69..c02e93b337 100644 --- a/src/detect-engine-register.h +++ b/src/detect-engine-register.h @@ -350,6 +350,10 @@ extern int DETECT_TBLSIZE_IDX; #define SIGMATCH_INFO_MULTI_UINT (1UL << (18)) /** keyword is an uint with enumeration stringer */ #define SIGMATCH_INFO_ENUM_UINT (1UL << (19)) +/** keyword matches mutable state that can change as the transaction advances + * (e.g. ssl_state): a no match must stay revisitable instead of becoming + * final at P + 1. */ +#define SIGMATCH_STATEFUL (1UL << (23)) /** keyword is an uint with bitflags */ #define SIGMATCH_INFO_BITFLAGS_UINT (1UL << (20)) /** keyword cannot be used in firewall rules */ diff --git a/src/detect-engine.c b/src/detect-engine.c index 6dd5d2b1f2..39715e8119 100644 --- a/src/detect-engine.c +++ b/src/detect-engine.c @@ -1558,6 +1558,26 @@ void DetectBufferTypeSetDescriptionByName(const char *name, const char *desc) strlcpy(exists->description, desc, sizeof(exists->description)); } +void DetectBufferTypeSetRunAlways(const char *name) +{ + DetectBufferType *exists = DetectBufferTypeLookupByName(name); + if (!exists) { + return; + } + exists->run_always = true; +} + +void DetectEngineBufferTypeSetRunAlways(DetectEngineCtx *de_ctx, const int id) +{ + const char *name = DetectEngineBufferTypeGetNameById(de_ctx, id); + if (name == NULL) { + return; + } + DetectBufferType *exists = DetectEngineBufferTypeLookupByName(de_ctx, name); + BUG_ON(!exists); + exists->run_always = true; +} + const char *DetectEngineBufferTypeGetDescriptionById(const DetectEngineCtx *de_ctx, const int id) { const DetectBufferType *exists = DetectEngineBufferTypeGetById(de_ctx, id); @@ -2118,6 +2138,9 @@ int DetectEngineReloadIsIdle(void) * \retval 0 no match * \retval 1 match */ +static bool DetectTxCompleted( + Flow *f, void *txv, uint8_t flags, const DetectEngineAppInspectionEngine *engine); + uint8_t DetectEngineInspectGenericList(DetectEngineCtx *de_ctx, DetectEngineThreadCtx *det_ctx, const struct DetectEngineAppInspectionEngine_ *engine, const Signature *s, Flow *f, uint8_t flags, void *alstate, void *txv, uint64_t tx_id) @@ -2131,8 +2154,18 @@ uint8_t DetectEngineInspectGenericList(DetectEngineCtx *de_ctx, DetectEngineThre match = sigmatch_table[smd->type]. AppLayerTxMatch(det_ctx, f, flags, alstate, txv, s, smd->ctx); KEYWORD_PROFILING_END(det_ctx, smd->type, (match == 1)); - if (match == 0) - return DETECT_ENGINE_INSPECT_SIG_NO_MATCH; + if (match == 0) { + /* like the buffer engines, a no match is final once the tx + * moved past the phase the engine is registered at (P + 1). + * stateful keywords read live tx state, so they stay + * revisitable. */ + if ((sigmatch_table[smd->type].flags & SIGMATCH_STATEFUL) != 0) { + return DETECT_ENGINE_INSPECT_SIG_NO_MATCH; + } + return DetectTxCompleted(f, txv, flags, engine) + ? DETECT_ENGINE_INSPECT_SIG_CANT_MATCH + : DETECT_ENGINE_INSPECT_SIG_NO_MATCH; + } if (match == 2) { return DETECT_ENGINE_INSPECT_SIG_CANT_MATCH; } @@ -5653,9 +5686,25 @@ static int DetectEngineThreadCtxInitKeywordFailTest(void) #endif +#ifdef UNITTESTS +/** \test SIGMATCH_STATEFUL uses a unique bit and only stateful keywords set it */ +static int DetectEngineStatefulFlagTest01(void) +{ + FAIL_IF((SIGMATCH_STATEFUL & SIGMATCH_INFO_BITFLAGS_UINT) != 0); + FAIL_IF((SIGMATCH_STATEFUL & SIGMATCH_BAN_FIREWALL_RULE) != 0); + FAIL_IF((SIGMATCH_STATEFUL & SIGMATCH_BAN_FIREWALL_MODE) != 0); + + FAIL_IF((sigmatch_table[DETECT_SSL_STATE].flags & SIGMATCH_STATEFUL) == 0); + FAIL_IF((sigmatch_table[DETECT_DNP3IND].flags & SIGMATCH_STATEFUL) != 0); + + PASS; +} +#endif + void DetectEngineRegisterTests(void) { #ifdef UNITTESTS + UtRegisterTest("DetectEngineStatefulFlagTest01", DetectEngineStatefulFlagTest01); UtRegisterTest("DetectEngineTest01", DetectEngineTest01); UtRegisterTest("DetectEngineTest02", DetectEngineTest02); UtRegisterTest("DetectEngineTest03", DetectEngineTest03); diff --git a/src/detect-engine.h b/src/detect-engine.h index 8367248c23..ff788ef7cb 100644 --- a/src/detect-engine.h +++ b/src/detect-engine.h @@ -41,6 +41,8 @@ void DetectBufferTypeSupportsMultiInstance(const char *name); int DetectBufferTypeMaxId(void); void DetectBufferTypeCloseRegistration(void); void DetectBufferTypeSetDescriptionByName(const char *name, const char *desc); +void DetectBufferTypeSetRunAlways(const char *name); +void DetectEngineBufferTypeSetRunAlways(DetectEngineCtx *de_ctx, const int id); void DetectBufferTypeRegisterSetupCallback(const char *name, void (*Callback)(const DetectEngineCtx *, Signature *, const DetectBufferType *)); void DetectBufferTypeRegisterValidateCallback( diff --git a/src/detect-ssl-state.c b/src/detect-ssl-state.c index fa89135439..aea347b93a 100644 --- a/src/detect-ssl-state.c +++ b/src/detect-ssl-state.c @@ -76,6 +76,8 @@ void DetectSslStateRegister(void) sigmatch_table[DETECT_SSL_STATE].desc = "match the state of the SSL connection"; sigmatch_table[DETECT_SSL_STATE].url = "/rules/tls-keywords.html#ssl-state"; sigmatch_table[DETECT_SSL_STATE].AppLayerTxMatch = DetectSslStateMatch; + /* the value follows the handshake state */ + sigmatch_table[DETECT_SSL_STATE].flags |= SIGMATCH_STATEFUL; sigmatch_table[DETECT_SSL_STATE].Setup = DetectSslStateSetup; sigmatch_table[DETECT_SSL_STATE].Free = DetectSslStateFree; #ifdef UNITTESTS @@ -89,6 +91,11 @@ void DetectSslStateRegister(void) DetectBufferTypeSetDescriptionByName("tls_generic", "generic ssl/tls inspection"); + /* ssl_state reads live handshake state, so its engine must be revisited + * as the transaction advances (a miss at the started state must not be + * final): run it on every update. */ + DetectBufferTypeSetRunAlways("tls_generic"); + DetectAppLayerInspectEngineRegister( "tls_generic", ALPROTO_TLS, SIG_FLAG_TOSERVER, 0, DetectEngineInspectGenericList, NULL); DetectAppLayerInspectEngineRegister( diff --git a/src/detect-ssl-version.c b/src/detect-ssl-version.c index d32dd0e662..751777d6e1 100644 --- a/src/detect-ssl-version.c +++ b/src/detect-ssl-version.c @@ -71,6 +71,9 @@ void DetectSslVersionRegister(void) sigmatch_table[DETECT_SSL_VERSION].AppLayerTxMatch = DetectSslVersionMatch; sigmatch_table[DETECT_SSL_VERSION].Setup = DetectSslVersionSetup; sigmatch_table[DETECT_SSL_VERSION].Free = DetectSslVersionFree; + /* ssl_version matches the record version, which can be seen before the + * hello is decoded: a miss must stay revisitable across records. */ + sigmatch_table[DETECT_SSL_VERSION].flags |= SIGMATCH_STATEFUL; #ifdef UNITTESTS sigmatch_table[DETECT_SSL_VERSION].RegisterTests = DetectSslVersionRegisterTests; #endif diff --git a/src/detect-tls-version.c b/src/detect-tls-version.c index b0f99a7503..1bd4754270 100644 --- a/src/detect-tls-version.c +++ b/src/detect-tls-version.c @@ -65,7 +65,11 @@ static int DetectTlsVersionSetup (DetectEngineCtx *, Signature *, const char *); static void DetectTlsVersionRegisterTests(void); #endif static void DetectTlsVersionFree(DetectEngineCtx *, void *); -static int g_tls_generic_list_id = 0; + +/** buffer for the version keywords, registered at the hello states: the + * version is only final once the hello has been decoded (a hello can span + * several records, so the record layer version may be seen first). */ +static int g_tls_version_list_id = 0; /** * \brief Registration function for keyword: tls.version @@ -78,14 +82,25 @@ void DetectTlsVersionRegister (void) sigmatch_table[DETECT_TLS_VERSION].AppLayerTxMatch = DetectTlsVersionMatch; sigmatch_table[DETECT_TLS_VERSION].Setup = DetectTlsVersionSetup; sigmatch_table[DETECT_TLS_VERSION].Free = DetectTlsVersionFree; - sigmatch_table[DETECT_TLS_VERSION].flags = SIGMATCH_SUPPORT_FIREWALL; + /* the negotiated version is not final until the hello is decoded (a hello + * can span records, and TLS 1.3 reveals the version in supported_versions), + * so a no match must stay revisitable until then. */ + sigmatch_table[DETECT_TLS_VERSION].flags = SIGMATCH_SUPPORT_FIREWALL | SIGMATCH_STATEFUL; #ifdef UNITTESTS sigmatch_table[DETECT_TLS_VERSION].RegisterTests = DetectTlsVersionRegisterTests; #endif DetectSetupParseRegexes(PARSE_REGEX, &parse_regex); - g_tls_generic_list_id = DetectBufferTypeRegister("tls_generic"); + g_tls_version_list_id = DetectBufferTypeRegister("tls_version"); + DetectBufferTypeSetDescriptionByName("tls_version", "generic tls version inspection"); + /* the negotiated version is only final once the hello decoded, so the + * keyword's engine must be revisited as the tx advances. */ + DetectBufferTypeSetRunAlways("tls_version"); + DetectAppLayerInspectEngineRegister("tls_version", ALPROTO_TLS, SIG_FLAG_TOSERVER, + TLS_STATE_CLIENT_HELLO, DetectEngineInspectGenericList, NULL); + DetectAppLayerInspectEngineRegister("tls_version", ALPROTO_TLS, SIG_FLAG_TOCLIENT, + TLS_STATE_SERVER_HELLO, DetectEngineInspectGenericList, NULL); } /** @@ -96,8 +111,9 @@ void DetectTlsVersionRegister (void) * \param p pointer to the current packet * \param m pointer to the sigmatch that we will cast into DetectTlsVersionData * - * \retval 0 no match + * \retval 0 no match, version not decoded yet (revisitable) * \retval 1 match + * \retval 2 no match, version decoded and different (final) */ static int DetectTlsVersionMatch (DetectEngineThreadCtx *det_ctx, Flow *f, uint8_t flags, void *state, void *txv, @@ -113,16 +129,43 @@ static int DetectTlsVersionMatch (DetectEngineThreadCtx *det_ctx, } uint16_t version = 0; + bool decoded = false; SCLogDebug("looking for tls_data->ver 0x%02X (flags 0x%02X)", tls_data->ver, flags); if (flags & STREAM_TOCLIENT) { version = ssl_state->server_connp.version; - SCLogDebug("server (toclient) version is 0x%02X", version); + /* the version is final only once the hello (including + * supported_versions) decoded: the phase can advance on a later + * app-data record or EOF even when the hello never decoded */ + decoded = ssl_state->server_connp.hello_decoded; + SCLogDebug("server (toclient) version is 0x%02X decoded %s", version, BOOL2STR(decoded)); } else if (flags & STREAM_TOSERVER) { - version = ssl_state->client_connp.version; - SCLogDebug("client (toserver) version is 0x%02X", version); + version = ssl_state->client_connp.version; + decoded = ssl_state->client_connp.hello_decoded; + SCLogDebug("client (toserver) version is 0x%02X decoded %s", version, BOOL2STR(decoded)); } + if (!decoded) { + /* the hello (or its supported_versions) is not decoded yet: the + * version seen so far (record layer or legacy hello field) is not + * final, so the miss must stay revisitable. */ + SCReturnInt(0); + } + + /* The rule's phase: hook rules use the hook's progress, non-hook rules + * inspect the direction's hello state. A rule must not match before its + * phase. */ + const uint8_t engine_progress = + (tls_data->hook_progress >= 0) + ? (uint8_t)tls_data->hook_progress + : ((flags & STREAM_TOCLIENT) ? (uint8_t)TLS_STATE_SERVER_HELLO + : (uint8_t)TLS_STATE_CLIENT_HELLO); + const int progress = AppLayerParserGetStateProgress(f->proto, f->alproto, txv, flags); + if (progress < 0) + SCReturnInt(0); + if (progress < engine_progress) + SCReturnInt(0); + if ((tls_data->flags & DETECT_TLS_VERSION_FLAG_RAW) == 0) { /* Match all TLSv1.3 drafts as TLSv1.3 */ if (((version >> 8) & 0xff) == 0x7f) { @@ -130,7 +173,17 @@ static int DetectTlsVersionMatch (DetectEngineThreadCtx *det_ctx, } } - SCReturnInt((tls_data->ver == version)); + if (tls_data->ver == version) + SCReturnInt(1); + + /* Decoded but a different version: the mismatch is final only once the + * transaction moved past the rule's phase (or reached its end state), so + * a rule hooked at a state keeps the same decision point it had before. */ + if (progress > engine_progress || + progress == AppLayerParserGetTxEndState(f->proto, f->alproto, txv, flags)) { + SCReturnInt(2); + } + SCReturnInt(0); } /** @@ -235,11 +288,19 @@ static int DetectTlsVersionSetup (DetectEngineCtx *de_ctx, Signature *s, const c return -1; /* keyword supports multiple hooks, so attach to the hook specified in the rule. */ - int list = g_tls_generic_list_id; + int list = g_tls_version_list_id; + tls->hook_progress = -1; /* Okay so far so good, lets get this into a SigMatch * and put it in the Signature. */ if (s->init_data->hook.type == SIGNATURE_HOOK_TYPE_APP) { list = s->init_data->hook.sm_list; + tls->hook_progress = (int8_t)s->init_data->hook.t.app.app_progress; + /* A hook at the first state (e.g. tls:client_started) is evaluated + * before the hello decoded and would not be revisited once the tx + * advances: run its engine on every update. Later hooks are revisited + * by the normal P+1 evaluation. */ + if (tls->hook_progress == 0) + DetectEngineBufferTypeSetRunAlways(de_ctx, list); } if (SCSigMatchAppendSMToList(de_ctx, s, DETECT_TLS_VERSION, (SigMatchCtx *)tls, list) == NULL) { diff --git a/src/detect-tls-version.h b/src/detect-tls-version.h index eabb6b57ae..8831b347b4 100644 --- a/src/detect-tls-version.h +++ b/src/detect-tls-version.h @@ -29,6 +29,9 @@ typedef struct DetectTlsVersionData_ { uint16_t ver; /** tls version to match */ uint8_t flags; + /** for hook rules: progress of the hook the rule is attached to. -1 for + * non-hook rules, which inspect the hello state. */ + int8_t hook_progress; } DetectTlsVersionData; /* prototypes */ diff --git a/src/detect.c b/src/detect.c index 0e4f3cd970..e442791bfd 100644 --- a/src/detect.c +++ b/src/detect.c @@ -1302,6 +1302,10 @@ void *DetectGetInnerTx(void *tx_ptr, AppProto alproto, AppProto engine_alproto, * \retval 1 sig matched * \retval 0 partial incomplete match * \retval -1 failed to match + * \retval -2 no match yet, but not final: the MPM engine will revisit the + * tx as it progresses. The caller must not apply a hook default + * policy for it: that could decide the packet before the revisit + * can turn it into a match. */ static int DetectRunTxInspectRule(ThreadVars *tv, DetectEngineCtx *de_ctx, DetectEngineThreadCtx *det_ctx, Packet *p, Flow *f, diff --git a/src/detect.h b/src/detect.h index bc679e6755..075ce7564c 100644 --- a/src/detect.h +++ b/src/detect.h @@ -459,6 +459,9 @@ typedef struct DetectBufferType_ { bool mpm; bool packet; /**< compat to packet matches */ bool frame; /**< is about Frame inspection */ + /** the buffer's keywords can change as the transaction advances: run + * the inspect engine on every tx update instead of only at its progress. */ + bool run_always; bool supports_transforms; bool multi_instance; /**< buffer supports multiple buffer instances per tx */ void (*SetupCallback)( @@ -1631,6 +1634,11 @@ typedef struct PrefilterEngineList_ { /** Free function for pectx data. If NULL the memory is not freed. */ void (*Free)(void *pectx); + /** Run this tx engine on every tx update, regardless of tx progress. + * Only used with Tx Engines. The engine keeps its real tx_min_progress, + * so it still takes part in the progress bookkeeping. */ + bool run_always; + const char *name; /* global id for this prefilter */ uint32_t gid; @@ -1659,6 +1667,11 @@ typedef struct PrefilterEngine_ { bool is_last; bool is_last_for_progress; + /** Tx engine must run on every tx update even once the tx progressed past + * its tx_min_progress. Kept separate from tx_min_progress (-1 is the + * proto-agnostic packet-style sentinel). */ + bool run_always; + /** Context for matching. Might be MpmCtx for MPM engines, other ctx' * for other engines. */ void *pectx;