diff --git a/doc/userguide/upgrade.rst b/doc/userguide/upgrade.rst index 67144e2127..4d382ce05f 100644 --- a/doc/userguide/upgrade.rst +++ b/doc/userguide/upgrade.rst @@ -71,6 +71,12 @@ Major Changes ``tx:request_headers()``, ``tx:request_headers_raw()`` and the response twins read the header tables and the raw header buffer, and that library has no trailer accessor. +- A HTTP/1 header block spanning several packets is no longer re-inspected while it is still + arriving: a rule on ``http.header`` or ``http.header.raw`` now alerts once per transaction + instead of once per inspection round. Which transactions alert is unchanged, so alert counts + drop on captures where such a block arrived in pieces. This is what makes a negated match or a + ``within:`` distance on a header buffer decide on the whole block. + - HTTP/2 gained a transaction state for the trailer block, ``http2:stream:request_trailers`` and ``http2:stream:response_trailers``, between ``..._data`` and ``..._closed``. Firewall rules with a state range spanning them (for diff --git a/rust/htp/src/c_api/config.rs b/rust/htp/src/c_api/config.rs index f56238b47a..e6f9e01838 100644 --- a/rust/htp/src/c_api/config.rs +++ b/rust/htp/src/c_api/config.rs @@ -112,6 +112,32 @@ pub unsafe extern "C" fn htp_config_register_request_start( } } +/// Registers a REQUEST_HEADERS callback, run when the request header block +/// is complete. +/// # Safety +/// When calling this method, you have to ensure that cfg is either properly initialized or NULL +#[no_mangle] +pub unsafe extern "C" fn htp_config_register_request_headers( + cfg: *mut Config, cbk_fn: TxExternalCallbackFn, +) { + if let Some(cfg) = cfg.as_mut() { + cfg.hook_request_headers.register_extern(cbk_fn) + } +} + +/// Registers a RESPONSE_HEADERS callback, run when the response header block +/// is complete. +/// # Safety +/// When calling this method, you have to ensure that cfg is either properly initialized or NULL +#[no_mangle] +pub unsafe extern "C" fn htp_config_register_response_headers( + cfg: *mut Config, cbk_fn: TxExternalCallbackFn, +) { + if let Some(cfg) = cfg.as_mut() { + cfg.hook_response_headers.register_extern(cbk_fn) + } +} + /// Registers a HTP_REQUEST_TRAILER callback. /// # Safety /// When calling this method, you have to ensure that cfg is either properly initialized or NULL diff --git a/rust/htp/src/config.rs b/rust/htp/src/config.rs index d165cb98d2..0c20207fd6 100644 --- a/rust/htp/src/config.rs +++ b/rust/htp/src/config.rs @@ -42,7 +42,6 @@ pub struct Config { /// they don't use headers). pub(crate) hook_request_header_data: DataHook, /// Request headers hook, invoked after all request headers are seen. - #[cfg(test)] pub(crate) hook_request_headers: TxHook, /// Request body data hook, invoked every time body data is available. Each /// invocation will provide a Data instance. Chunked data @@ -71,7 +70,6 @@ pub struct Config { /// they don't have response headers). pub(crate) hook_response_header_data: DataHook, /// Response headers book, invoked after all response headers have been seen. - #[cfg(test)] pub(crate) hook_response_headers: TxHook, /// Response body data hook, invoked every time body data is available. Each /// invocation will provide a Data instance. Chunked data @@ -119,7 +117,6 @@ impl Default for Config { hook_request_start: TxHook::default(), hook_request_line: TxHook::default(), hook_request_header_data: DataHook::default(), - #[cfg(test)] hook_request_headers: TxHook::default(), hook_request_body_data: DataHook::default(), hook_request_trailer_data: DataHook::default(), @@ -129,7 +126,6 @@ impl Default for Config { #[cfg(test)] hook_response_line: TxHook::default(), hook_response_header_data: DataHook::default(), - #[cfg(test)] hook_response_headers: TxHook::default(), hook_response_body_data: DataHook::default(), hook_response_trailer_data: DataHook::default(), diff --git a/rust/htp/src/connection_parser.rs b/rust/htp/src/connection_parser.rs index 6e9f6119f1..d60b944064 100644 --- a/rust/htp/src/connection_parser.rs +++ b/rust/htp/src/connection_parser.rs @@ -692,7 +692,6 @@ impl ConnectionParser { } req.process_request_headers()?; // Run hook REQUEST_HEADERS. - #[cfg(test)] self.cfg .hook_request_headers .clone() @@ -851,7 +850,6 @@ impl ConnectionParser { // Finalize sending raw header data. self.response_receiver_finalize_clear(input)?; // Run hook RESPONSE_HEADERS. - #[cfg(test)] self.cfg .hook_response_headers .clone() diff --git a/src/app-layer-htp.c b/src/app-layer-htp.c index 0c33375ea2..c18a84adbc 100644 --- a/src/app-layer-htp.c +++ b/src/app-layer-htp.c @@ -322,6 +322,7 @@ static const char *HtpStateGetStateNameById(const int id, const uint8_t directio static void *HTPStateGetTx(void *alstate, uint64_t tx_id); static int HTPStateGetAlstateProgress(void *tx, uint8_t direction); + static uint64_t HTPStateGetTxCnt(void *alstate); #ifdef UNITTESTS static void HTPParserRegisterTests(void); @@ -1832,6 +1833,53 @@ static int HTPCallbackRequestLine(const htp_connp_t *connp, htp_tx_t *tx) return HTP_STATUS_OK; } +/* The htp state completion hooks mark the block as finished. Progress is not delayed: + * the block buffers withhold their data until the bit is set, so detection never sees a + * header or trailer block that is still arriving. */ +static int HTPCallbackRequestHeaders(const htp_connp_t *connp, htp_tx_t *tx) +{ + SCEnter(); + if (tx == NULL) + SCReturnInt(HTP_STATUS_ERROR); + HtpTxUserData *tx_ud = (HtpTxUserData *)htp_tx_get_user_data(tx); + if (tx_ud != NULL) + tx_ud->ts_hooks_done |= HTP_HOOK_TS_HEADERS; + SCReturnInt(HTP_STATUS_OK); +} + +static int HTPCallbackRequestTrailer(const htp_connp_t *connp, htp_tx_t *tx) +{ + SCEnter(); + if (tx == NULL) + SCReturnInt(HTP_STATUS_ERROR); + HtpTxUserData *tx_ud = (HtpTxUserData *)htp_tx_get_user_data(tx); + if (tx_ud != NULL) + tx_ud->ts_hooks_done |= HTP_HOOK_TS_TRAILER; + SCReturnInt(HTP_STATUS_OK); +} + +static int HTPCallbackResponseHeaders(const htp_connp_t *connp, htp_tx_t *tx) +{ + SCEnter(); + if (tx == NULL) + SCReturnInt(HTP_STATUS_ERROR); + HtpTxUserData *tx_ud = (HtpTxUserData *)htp_tx_get_user_data(tx); + if (tx_ud != NULL) + tx_ud->tc_hooks_done |= HTP_HOOK_TC_HEADERS; + SCReturnInt(HTP_STATUS_OK); +} + +static int HTPCallbackResponseTrailer(const htp_connp_t *connp, htp_tx_t *tx) +{ + SCEnter(); + if (tx == NULL) + SCReturnInt(HTP_STATUS_ERROR); + HtpTxUserData *tx_ud = (HtpTxUserData *)htp_tx_get_user_data(tx); + if (tx_ud != NULL) + tx_ud->tc_hooks_done |= HTP_HOOK_TC_TRAILER; + SCReturnInt(HTP_STATUS_OK); +} + static int HTPCallbackRequestHeaderData(const htp_connp_t *connp, htp_tx_data_t *tx_data) { void *ptmp; @@ -1944,6 +1992,10 @@ static void HTPConfigSetDefaultsPhase1(HTPCfgRec *cfg_prec) htp_config_register_request_header_data(cfg_prec->cfg, HTPCallbackRequestHeaderData); htp_config_register_request_trailer_data(cfg_prec->cfg, HTPCallbackRequestTrailerData); + htp_config_register_request_headers(cfg_prec->cfg, HTPCallbackRequestHeaders); + htp_config_register_request_trailer(cfg_prec->cfg, HTPCallbackRequestTrailer); + htp_config_register_response_headers(cfg_prec->cfg, HTPCallbackResponseHeaders); + htp_config_register_response_trailer(cfg_prec->cfg, HTPCallbackResponseTrailer); htp_config_register_response_header_data(cfg_prec->cfg, HTPCallbackResponseHeaderData); htp_config_register_response_trailer_data(cfg_prec->cfg, HTPCallbackResponseTrailerData); @@ -5688,7 +5740,7 @@ static int HTPParserTest27(void) uint32_t len = 1000; - HtpTxUserData *tx_ud = SCMalloc(sizeof(HtpTxUserData)); + HtpTxUserData *tx_ud = SCCalloc(1, sizeof(HtpTxUserData)); FAIL_IF_NULL(tx_ud); tx_ud->tsflags |= HTP_STREAM_DEPTH_SET; diff --git a/src/app-layer-htp.h b/src/app-layer-htp.h index 05ef1391b7..d5486b6aa5 100644 --- a/src/app-layer-htp.h +++ b/src/app-layer-htp.h @@ -150,6 +150,11 @@ typedef struct HtpBody_ { /** Now the Body Chunks will be stored per transaction, at * the tx user data */ +#define HTP_HOOK_TS_HEADERS 0x01 +#define HTP_HOOK_TS_TRAILER 0x02 +#define HTP_HOOK_TC_HEADERS 0x01 +#define HTP_HOOK_TC_TRAILER 0x02 + typedef struct HtpTxUserData_ { /* Body of the request (if any) */ uint8_t request_body_init; @@ -158,6 +163,11 @@ typedef struct HtpTxUserData_ { uint8_t tsflags; uint8_t tcflags; + /* set as htp runs the state completion hooks; the block buffers check them + * before handing data out, progress itself is not delayed */ + uint8_t ts_hooks_done; + uint8_t tc_hooks_done; + uint8_t request_body_type; HtpBody request_body; @@ -184,6 +194,40 @@ typedef struct HtpTxUserData_ { FileContainer files_tc; } HtpTxUserData; +/* Block-shaped buffers must never be seen half-finished. htp reports a state as soon as + * its block starts, so completion comes from the state hook; on closure no header hook runs, + * and progress leaves the block state - request via request_headers()' CLOSED arm, response + * via state_response_complete() - which opens the gate. The third way out is the reassembly + * depth limit: htp is never closed and no hook ever runs there, so a depth-cut block must + * report what was parsed, which the flow flags say. */ +static inline bool HtpHeaderBlockIsComplete(htp_tx_t *tx, uint8_t flow_flags) +{ + HtpTxUserData *tx_ud = htp_tx_get_user_data(tx); + if (tx_ud == NULL) + return true; + if (flow_flags & STREAM_TOSERVER) + return (flow_flags & STREAM_DEPTH) != 0 || + htp_tx_request_progress(tx) != HTP_REQUEST_PROGRESS_HEADERS || + (tx_ud->ts_hooks_done & HTP_HOOK_TS_HEADERS) != 0; + return (flow_flags & STREAM_DEPTH) != 0 || + htp_tx_response_progress(tx) != HTP_RESPONSE_PROGRESS_HEADERS || + (tx_ud->tc_hooks_done & HTP_HOOK_TC_HEADERS) != 0; +} + +static inline bool HtpTrailerBlockIsComplete(htp_tx_t *tx, uint8_t flow_flags) +{ + HtpTxUserData *tx_ud = htp_tx_get_user_data(tx); + if (tx_ud == NULL) + return true; + if (flow_flags & STREAM_TOSERVER) + return (flow_flags & STREAM_DEPTH) != 0 || + htp_tx_request_progress(tx) != HTP_REQUEST_PROGRESS_TRAILER || + (tx_ud->ts_hooks_done & HTP_HOOK_TS_TRAILER) != 0; + return (flow_flags & STREAM_DEPTH) != 0 || + htp_tx_response_progress(tx) != HTP_RESPONSE_PROGRESS_TRAILER || + (tx_ud->tc_hooks_done & HTP_HOOK_TC_TRAILER) != 0; +} + typedef struct HtpState_ { /* Connection parser structure for each connection */ htp_connp_t *connp; diff --git a/src/detect-http-raw-header.c b/src/detect-http-raw-header.c index a4705687ba..3ac3bbbe13 100644 --- a/src/detect-http-raw-header.c +++ b/src/detect-http-raw-header.c @@ -96,19 +96,15 @@ void DetectHttpRawHeaderRegister(void) sigmatch_table[DETECT_HTTP_RAW_HEADER].Setup = DetectHttpRawHeaderSetupSticky; sigmatch_table[DETECT_HTTP_RAW_HEADER].flags |= SIGMATCH_NOOPT|SIGMATCH_INFO_STICKY_BUFFER; - /* one state past the headers, unlike the other header buffers: the raw bytes arrive - * through a data hook that htp only flushes as it leaves the header state, so a round at - * HEADERS would decide on a truncated block (negated content and within: would answer - * differently than on the complete block) */ DetectAppLayerInspectEngineRegister("http_raw_header", ALPROTO_HTTP1, SIG_FLAG_TOSERVER, - HTP_REQUEST_PROGRESS_HEADERS + 1, DetectEngineInspectBufferGeneric, GetData); + HTP_REQUEST_PROGRESS_HEADERS, DetectEngineInspectBufferGeneric, GetData); DetectAppLayerInspectEngineRegister("http_raw_header", ALPROTO_HTTP1, SIG_FLAG_TOCLIENT, - HTP_RESPONSE_PROGRESS_HEADERS + 1, DetectEngineInspectBufferGeneric, GetData); + HTP_RESPONSE_PROGRESS_HEADERS, DetectEngineInspectBufferGeneric, GetData); DetectAppLayerMpmRegister("http_raw_header", SIG_FLAG_TOSERVER, 2, PrefilterGenericMpmRegister, - GetData, ALPROTO_HTTP1, HTP_REQUEST_PROGRESS_HEADERS + 1); + GetData, ALPROTO_HTTP1, HTP_REQUEST_PROGRESS_HEADERS); DetectAppLayerMpmRegister("http_raw_header", SIG_FLAG_TOCLIENT, 2, PrefilterGenericMpmRegister, - GetData, ALPROTO_HTTP1, HTP_RESPONSE_PROGRESS_HEADERS + 1); + GetData, ALPROTO_HTTP1, HTP_RESPONSE_PROGRESS_HEADERS); DetectAppLayerInspectEngineRegisterSubState("http_raw_header", ALPROTO_HTTP2, SIG_FLAG_TOSERVER, HTTP2TxTypeStream, HTTP2ProgHeaders, DetectEngineInspectBufferGeneric, GetData2); @@ -196,6 +192,8 @@ static InspectionBuffer *GetData(DetectEngineThreadCtx *det_ctx, HtpTxUserData *tx_ud = htp_tx_get_user_data(tx); const bool ts = ((flow_flags & STREAM_TOSERVER) != 0); + if (!HtpHeaderBlockIsComplete(tx, flow_flags)) + return NULL; const uint8_t *data = ts ? tx_ud->request_headers_raw : tx_ud->response_headers_raw; if (data == NULL) diff --git a/src/detect-http-trailer-names.c b/src/detect-http-trailer-names.c index ed7c388854..3f7ed76d71 100644 --- a/src/detect-http-trailer-names.c +++ b/src/detect-http-trailer-names.c @@ -121,6 +121,9 @@ static uint8_t *GetBufferForTX( { *buffer_len = 0; + if (!HtpTrailerBlockIsComplete(tx, flags)) + return NULL; + HttpHeaderThreadData *hdr_td = NULL; HttpHeaderBuffer *buf = HttpHeaderGetBufferSpace(det_ctx, flags, g_keyword_thread_id, &hdr_td); if (unlikely(buf == NULL)) { diff --git a/src/detect-http-trailer-raw.c b/src/detect-http-trailer-raw.c index 7cc5f63a34..62626c75dd 100644 --- a/src/detect-http-trailer-raw.c +++ b/src/detect-http-trailer-raw.c @@ -117,6 +117,8 @@ static InspectionBuffer *GetData(DetectEngineThreadCtx *det_ctx, HtpTxUserData *tx_ud = htp_tx_get_user_data(tx); const bool ts = ((flow_flags & STREAM_TOSERVER) != 0); + if (!HtpTrailerBlockIsComplete(tx, flow_flags)) + return NULL; const uint8_t *data = ts ? tx_ud->request_trailers_raw : tx_ud->response_trailers_raw; if (data == NULL) return NULL; diff --git a/src/detect-http-trailer.c b/src/detect-http-trailer.c index d68eadc453..4dd83bfae0 100644 --- a/src/detect-http-trailer.c +++ b/src/detect-http-trailer.c @@ -132,6 +132,9 @@ static uint8_t *GetBufferForTX( { *buffer_len = 0; + if (!HtpTrailerBlockIsComplete(tx, flags)) + return NULL; + HttpHeaderThreadData *hdr_td = NULL; HttpHeaderBuffer *buf = HttpHeaderGetBufferSpace(det_ctx, flags, g_keyword_thread_id, &hdr_td); if (unlikely(buf == NULL)) {