From 4ceb0e7a528b3769751089a7b048b6c61e5a0144 Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Thu, 8 Oct 2026 22:45:26 +0000 Subject: [PATCH 1/9] detect: allow state hooked rules to use buffers that complete earlier A rule with an app-layer state hook required every buffer engine to sit at exactly the hook state, which makes sensible combinations impossible to load: a rule hooked at the trailer state cannot use the header buffer, although that data is complete well before the trailer. Relax the check to an upper bound on the buffer state. Engines stay at the state their buffer registers: an engine runs as soon as the tx reaches its own state, so the hook bounds which buffers a rule may use, it does not delay evaluation. A rule hooked at the trailer state on a header buffer is decided where the header data is complete, which is also what the prefilter leg queueing its fast pattern relies on. Raising an engine to the hook state instead moves matching off that leg, so the buffer is never inspected. Buffers completing after the hook stay rejected: their data falls outside the state the hook opens, and for a firewall rule that is past the point its policy lets the flow reach. Protocol agnostic engines (app-layer-event, app-layer-state) register at progress 0 in every protocol, so an upper bound would admit them under any hook and the hook would stop gating them. They keep the exact state requirement they had before. Rules that loaded before are unaffected: for them the two states were already equal. A buffer that keeps growing after the hooked state gets the rule revisited at those later states, so such a rule can alert more than once per transaction. Documented with the hooks. Ticket: #8256. --- src/detect-parse.c | 52 ++++++++++++++++++++++++++++---------- src/tests/detect.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 102 insertions(+), 13 deletions(-) diff --git a/src/detect-parse.c b/src/detect-parse.c index 045f660f04..2598e89648 100644 --- a/src/detect-parse.c +++ b/src/detect-parse.c @@ -3038,9 +3038,8 @@ static int SigValidateCheckBuffers( app_buffers_evaluated++; if (s->init_data->hook.type == SIGNATURE_HOOK_TYPE_APP) { - /* only allow rules to use the hook for engines at that - * exact progress for now. We make an exception for generic - * engines like app-layer-event. */ + /* the hook selects protocol and sub-state; the buffer state is + * checked against the hook state below */ if (!(AppProtoEqualsStrict(s->alproto, app->alproto) || app->alproto == ALPROTO_UNKNOWN)) { SCLogDebug("%u:%s: for buffer %s skip engine %s alproto %s", s->id, @@ -3084,16 +3083,43 @@ static int SigValidateCheckBuffers( } if (s->init_data->hook.type == SIGNATURE_HOOK_TYPE_APP) { - if ((s->flags & SIG_FLAG_TOSERVER) && (app->dir == 0) && - app->progress != s->init_data->hook.t.app.app_progress) { - SCLogError("engine progress value %d doesn't match hook %u", app->progress, - s->init_data->hook.t.app.app_progress); - SCReturnInt(0); - } - if ((s->flags & SIG_FLAG_TOCLIENT) && (app->dir == 1) && - app->progress != s->init_data->hook.t.app.app_progress) { - SCLogError("engine progress value doesn't match hook"); - SCReturnInt(0); + const uint8_t hook_progress = s->init_data->hook.t.app.app_progress; + const bool dir_match = (app->dir == 0 && (s->flags & SIG_FLAG_TOSERVER)) || + (app->dir == 1 && (s->flags & SIG_FLAG_TOCLIENT)); + if (dir_match) { + /* Engines run from their own state on, so a buffer finishing before the + * hook is decided there: the hook bounds the buffer state, it does not + * delay evaluation. After the hook the match could land outside the hook + * window, so that stays rejected. Generic engines (app-layer-event, + * app-layer-state) sit at progress 0 in every protocol, so an upper bound + * would admit them under any hook and the hook would gate nothing: they + * keep the exact state requirement. + * A firewall rule is only a candidate at its hook, so an exact hook below + * its buffer's completion state can never be decided: refuse it instead of + * loading a rule that does nothing. Threat rules keep working in either + * mode, they are inspected from the buffer's state on. The '<' form is held + * to the same bound: its top state is the hook checked here, so a buffer + * completing below the bound is refused as well. */ + if (app->alproto == ALPROTO_UNKNOWN) { + if (app->progress != hook_progress) { + SCLogError("buffer %s is not protocol specific, needs the hook at " + "state %u", + bt->name, app->progress); + SCReturnInt(0); + } + } else if (app->progress > hook_progress) { + SCLogError("buffer %s completes in state %u, after the hooked state %u", + bt->name, app->progress, hook_progress); + SCReturnInt(0); + } else if (app->progress < hook_progress && (s->flags & SIG_FLAG_FIREWALL)) { + SCLogError("buffer %s completes in state %u, before the hooked state %u: " + "a firewall rule is only evaluated at its hook, so it could " + "never match. Hook at state %u, or use a buffer that completes " + "at state %u.", + bt->name, app->progress, hook_progress, app->progress, + hook_progress); + SCReturnInt(0); + } } } diff --git a/src/tests/detect.c b/src/tests/detect.c index f0d87f4770..43a1cef2b0 100644 --- a/src/tests/detect.c +++ b/src/tests/detect.c @@ -4383,6 +4383,68 @@ static int SigTestDropFlow03(void) PASS; } +/** \test a state hooked rule may use a buffer that completes before the hooked + * state, but not one that completes after it */ +static int SigTestStateHookBuffer01(void) +{ + DetectEngineCtx *de_ctx = DetectEngineCtxInit(); + FAIL_IF_NULL(de_ctx); + de_ctx->flags |= DE_QUIET; + + /* Header data completes well before the trailer state, so the rule is decidable where + * the headers end. Inspection follows the buffer, not the hook: at runtime the alert + * fires where an unhooked rule would, even on a tx that never reaches the trailer + * state (SV rule-hooks/http1-state-hook-buffer-timing). On main the same rule fails + * to load: engine progress for http.header sits at the header state, the hook at the + * trailer state, and the two had to be equal. */ + Signature *s = DetectEngineAppendSig(de_ctx, + "alert http1:request_trailer any any -> any any (flow:to_server; http.header; " + "content:\"a\"; sid:1;)"); + FAIL_IF_NULL(s); + + /* body data only completes after the header state, so inspection would have to run + * past the hooked state, which stays disallowed. */ + s = DetectEngineAppendSig(de_ctx, + "alert http1:request_headers any any -> any any (flow:to_server; http.request_body; " + "content:\"a\"; sid:2;)"); + FAIL_IF_NOT_NULL(s); + + /* The ticket case. Its fast pattern is queued by the leg for the buffer it came from, + * so both contents run at the buffer's own state and the hook gates nothing at + * runtime. The load-time check is the only hook/buffer tie. Rejecting this would + * leave the reported rule unloadable. */ + s = DetectEngineAppendSig(de_ctx, + "alert http1:request_trailer any any -> any any (flow:to_server; http.header; " + "content:\"a\"; fast_pattern; content:\"b\"; sid:3;)"); + FAIL_IF_NULL(s); + + /* likewise to the client */ + s = DetectEngineAppendSig(de_ctx, + "alert http1:response_trailer any any -> any any (flow:to_client; http.header; " + "content:\"a\"; sid:4;)"); + FAIL_IF_NULL(s); + + /* Firewall rules with a hook or LTE bound above the state where their buffer completes are + * refused at load: they are only evaluated at their hook, so they could never match. That + * path needs a firewall rule file, which DetectEngineAppendSig does not parse, so it is not + * covered here. */ + + /* generic engines are at progress 0 whatever the protocol, so they keep needing the + * hook at their own state: only the state 0 hooks can carry them, as before. */ + s = DetectEngineAppendSig(de_ctx, + "alert http1:request_trailer any any -> any any (flow:to_server; " + "app-layer-event:http.request_header_host_invalid; sid:5;)"); + FAIL_IF_NOT_NULL(s); + + s = DetectEngineAppendSig(de_ctx, + "alert http1:request_started any any -> any any (flow:to_server; " + "app-layer-event:http.request_header_host_invalid; sid:6;)"); + FAIL_IF_NULL(s); + + DetectEngineCtxFree(de_ctx); + PASS; +} + /** \test ICMP packet shouldn't be matching port based sig * Bug #611 */ static int SigTestPorts01(void) @@ -4912,6 +4974,7 @@ void SigRegisterTests(void) UtRegisterTest("DetectAddressYamlParsing03", DetectAddressYamlParsing03); UtRegisterTest("DetectAddressYamlParsing04", DetectAddressYamlParsing04); + UtRegisterTest("SigTestStateHookBuffer01", SigTestStateHookBuffer01); UtRegisterTest("SigTestPorts01", SigTestPorts01); UtRegisterTest("SigTestBug01", SigTestBug01); From a0b518b9d61ac596d2c56105aabd4fa02347ac2c Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Fri, 9 Oct 2026 21:26:44 +0000 Subject: [PATCH 2/9] htp: keep trailer fields out of the header table Trailer fields of a chunked message are parsed by the same code as header fields and were appended to the header table, where a name already used in the header section is combined into the existing value. So a header keyword silently gained trailer data, and a chunked request ending in "User-Agent: x" reported its user agent as "Mozilla, x". Store them in a separate table per direction instead. Repeated names are still combined and the field count limit still applies across both sections, but the Content-Length bookkeeping is skipped: trailer fields take no part in framing. The C API gains htp_tx_request_trailers() and htp_tx_response_trailers(); every existing getter keeps returning the header section only. A few HTTP unit tests assert the old guarantee that trailer content is reachable through http_header, so they fail from here until the commit that adds the trailer buffers converts them. Every commit in the series still builds. Ticket: #8256. --- rust/htp/src/c_api/transaction.rs | 28 ++++++ rust/htp/src/request.rs | 48 +++++++++- rust/htp/src/response.rs | 52 ++++++++++- .../test/files/30-request-trailer-repeats.t | 84 +++++++++++++++++ .../test/files/31-response-trailer-repeats.t | 84 +++++++++++++++++ rust/htp/src/test/main.rs | 90 +++++++++++++++++++ rust/htp/src/transaction.rs | 14 +++ src/output-json-http.c | 35 ++++++-- 8 files changed, 423 insertions(+), 12 deletions(-) create mode 100644 rust/htp/src/test/files/30-request-trailer-repeats.t create mode 100644 rust/htp/src/test/files/31-response-trailer-repeats.t diff --git a/rust/htp/src/c_api/transaction.rs b/rust/htp/src/c_api/transaction.rs index 3647849ed0..7653807e7c 100644 --- a/rust/htp/src/c_api/transaction.rs +++ b/rust/htp/src/c_api/transaction.rs @@ -189,6 +189,20 @@ pub unsafe extern "C" fn htp_tx_request_headers_size(tx: *const Transaction) -> .unwrap_or(-1) } +/// Get a transaction's request trailer fields, which are kept apart from its headers. +/// +/// tx: Transaction pointer. +/// +/// Returns the trailer table or NULL on error. +/// # Safety +/// When calling this method, you have to ensure that tx is either properly initialized or NULL +#[no_mangle] +pub unsafe extern "C" fn htp_tx_request_trailers(tx: *const Transaction) -> *const Headers { + tx.as_ref() + .map(|tx| &tx.request_trailers as *const Headers) + .unwrap_or(std::ptr::null()) +} + /// Get the first request header value matching the key from a transaction. /// /// tx: Transaction pointer. @@ -393,6 +407,20 @@ pub unsafe extern "C" fn htp_tx_response_headers(tx: *const Transaction) -> *con .unwrap_or(std::ptr::null()) } +/// Get a transaction's response trailer fields, which are kept apart from its headers. +/// +/// tx: Transaction pointer. +/// +/// Returns the trailer table or NULL on error. +/// # Safety +/// When calling this method, you have to ensure that tx is either properly initialized or NULL +#[no_mangle] +pub unsafe extern "C" fn htp_tx_response_trailers(tx: *const Transaction) -> *const Headers { + tx.as_ref() + .map(|tx| &tx.response_trailers as *const Headers) + .unwrap_or(std::ptr::null()) +} + /// Get the first response header value matching the key from a transaction. /// /// tx: Transaction pointer. diff --git a/rust/htp/src/request.rs b/rust/htp/src/request.rs index 73c2ce51d5..073e2aeccb 100644 --- a/rust/htp/src/request.rs +++ b/rust/htp/src/request.rs @@ -423,7 +423,9 @@ impl ConnectionParser { Ordering::Equal => { // End of data self.request_state = State::Headers; - self.request_mut().unwrap().request_progress = HtpRequestProgress::TRAILER + let req = self.request_mut().unwrap(); + req.request_progress = HtpRequestProgress::TRAILER; + req.request_in_trailers = true; } Ordering::Greater => { // More data available. @@ -714,12 +716,56 @@ impl ConnectionParser { } } + /// Extract one request trailer field. Kept out of the header table, but otherwise treated + /// like a header field: repeated names are combined under the same repetition cap, and the + /// total field count is shared with the header section. + fn process_request_trailer(&mut self, header: Header, hl: usize) -> Result<()> { + let req = self.request_mut().unwrap(); + if req.request_trailers.elements.len() + req.request_headers.elements.len() > hl { + if !req.flags.is_set(HtpFlags::HEADERS_TOO_MANY) { + htp_warn!( + self.logger, + HtpLogCode::REQUEST_TOO_MANY_HEADERS, + "Too many request headers" + ); + let req = self.request_mut().unwrap(); + req.flags.set(HtpFlags::HEADERS_TOO_MANY); + } + return Err(HtpStatus::ERROR); + } + let req = self.request_mut().unwrap(); + let mut update_reps = false; + if let Some(existing) = req.request_trailers.get_nocase_mut(header.name.as_slice()) { + // Same cap as the header path: past it, repeated names stop being merged, so a + // peer cannot grow one value without bound. + if existing.flags.is_set(HeaderFlags::FIELD_REPEATED) { + if req.request_header_repetitions >= 64 { + return Ok(()); + } + update_reps = true; + } + existing.flags.set(HeaderFlags::FIELD_REPEATED); + existing.value.extend_from_slice(b", "); + existing.value.extend_from_slice(header.value.as_slice()); + } else { + req.request_trailers.elements.push(header); + } + if update_reps { + let req = self.request_mut().unwrap(); + req.request_header_repetitions = req.request_header_repetitions.wrapping_add(1); + } + Ok(()) + } + /// Extract one request header. A header can span multiple lines, in /// which case they will be folded into one before parsing is attempted. fn process_request_header(&mut self, header: Header) -> Result<()> { // Try to parse the header. // ensured by caller let hl = self.cfg.number_headers_limit as usize; + if self.request_mut().unwrap().request_in_trailers { + return self.process_request_trailer(header, hl); + } let req = self.request_mut().unwrap(); let mut repeated = false; let reps = req.request_header_repetitions; diff --git a/rust/htp/src/response.rs b/rust/htp/src/response.rs index 64ea16cd4e..375de74983 100644 --- a/rust/htp/src/response.rs +++ b/rust/htp/src/response.rs @@ -238,8 +238,9 @@ impl ConnectionParser { Ordering::Equal => { // End of data self.response_state = State::Headers; - self.response_mut().unwrap().response_progress = - HtpResponseProgress::TRAILER + let resp = self.response_mut().unwrap(); + resp.response_progress = HtpResponseProgress::TRAILER; + resp.response_in_trailers = true; } Ordering::Greater => { // More data available. @@ -882,9 +883,56 @@ impl ConnectionParser { } } + /// Extract one response trailer field. Kept out of the header table, but otherwise treated + /// like a header field: repeated names are combined, and the total field count is shared + /// with the header section. + fn process_response_trailer(&mut self, header: Header, hl: usize) -> Result<()> { + let resp = self.response_mut().unwrap(); + if resp.response_trailers.elements.len() + resp.response_headers.elements.len() > hl { + if !resp.flags.is_set(HtpFlags::HEADERS_TOO_MANY) { + htp_warn!( + self.logger, + HtpLogCode::RESPONSE_TOO_MANY_HEADERS, + "Too many response headers" + ); + let resp = self.response_mut().unwrap(); + resp.flags.set(HtpFlags::HEADERS_TOO_MANY); + } + return Err(HtpStatus::ERROR); + } + let resp = self.response_mut().unwrap(); + let mut update_reps = false; + if let Some(existing) = resp + .response_trailers + .get_nocase_mut(header.name.as_slice()) + { + // Same cap as the header path: past it, repeated names stop being merged, so a + // peer cannot grow one value without bound. + if existing.flags.is_set(HeaderFlags::FIELD_REPEATED) { + if resp.response_header_repetitions >= 64 { + return Ok(()); + } + update_reps = true; + } + existing.flags.set(HeaderFlags::FIELD_REPEATED); + existing.value.extend_from_slice(b", "); + existing.value.extend_from_slice(header.value.as_slice()); + } else { + resp.response_trailers.elements.push(header); + } + if update_reps { + let resp = self.response_mut().unwrap(); + resp.response_header_repetitions = resp.response_header_repetitions.wrapping_add(1); + } + Ok(()) + } + /// Response header line(s) processor, which assembles folded lines /// into a single buffer before invoking the parsing function. fn process_response_header(&mut self, header: Header) -> Result<()> { + if self.response_mut().unwrap().response_in_trailers { + return self.process_response_trailer(header, self.cfg.number_headers_limit as usize); + } let mut repeated = false; let hl = self.cfg.number_headers_limit as usize; let resp = self.response_mut(); diff --git a/rust/htp/src/test/files/30-request-trailer-repeats.t b/rust/htp/src/test/files/30-request-trailer-repeats.t new file mode 100644 index 0000000000..824dead78b --- /dev/null +++ b/rust/htp/src/test/files/30-request-trailer-repeats.t @@ -0,0 +1,84 @@ +>>> +POST / HTTP/1.1 +Host: x +Transfer-Encoding: chunked + +1 +x +0 +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v + + +<<< +HTTP/1.1 200 OK +Content-Length: 0 + diff --git a/rust/htp/src/test/files/31-response-trailer-repeats.t b/rust/htp/src/test/files/31-response-trailer-repeats.t new file mode 100644 index 0000000000..cfe8975b6d --- /dev/null +++ b/rust/htp/src/test/files/31-response-trailer-repeats.t @@ -0,0 +1,84 @@ +>>> +GET / HTTP/1.1 +Host: x + + +<<< +HTTP/1.1 200 OK +Transfer-Encoding: chunked + +1 +x +0 +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v +A: v + + diff --git a/rust/htp/src/test/main.rs b/rust/htp/src/test/main.rs index 5e6b9d3582..ced2ddb860 100644 --- a/rust/htp/src/test/main.rs +++ b/rust/htp/src/test/main.rs @@ -771,6 +771,96 @@ fn ResponseTrailerData() { assert_eq!(4, *tx.user_data::().unwrap()); } +/// Trailer fields must stay out of the header table: a name that appears in both sections +/// would otherwise be combined into one header value. +#[test] +fn RequestTrailersKeptOutOfHeaders() { + let cfg = TestConfig(); + let mut t = Test::new(cfg); + assert!(t.run_file("27-request-trailer-raw.t").is_ok()); + let tx = t.connp.tx(0).unwrap(); + assert_eq!( + b"1", + tx.request_headers + .get_nocase("Cookie") + .unwrap() + .value + .as_slice() + ); + assert_eq!(1, tx.request_trailers.size()); + assert_eq!( + b"2", + tx.request_trailers + .get_nocase("Cookie") + .unwrap() + .value + .as_slice() + ); +} + +/// The header repetition cap must also bound trailer values: a peer that repeats one +/// trailer name forever would otherwise grow a single value without limit. +#[test] +fn RequestTrailerRepeatsCapped() { + let cfg = TestConfig(); + let mut t = Test::new(cfg); + assert!(t.run_file("30-request-trailer-repeats.t").is_ok()); + let tx = t.connp.tx(0).unwrap(); + let v = tx + .request_trailers + .get_nocase("A") + .unwrap() + .value + .as_slice(); + assert_eq!( + 1, + v.iter() + .take(1) + .filter(|&&c| c == b'A' || c == b'v') + .count() + ); + assert_eq!(66, bytecount(v, b'v')); + assert!(tx.request_trailers.size() == 1); +} + +#[test] +fn ResponseTrailerRepeatsCapped() { + let cfg = TestConfig(); + let mut t = Test::new(cfg); + assert!(t.run_file("31-response-trailer-repeats.t").is_ok()); + let tx = t.connp.tx(0).unwrap(); + let v = tx + .response_trailers + .get_nocase("A") + .unwrap() + .value + .as_slice(); + assert_eq!(66, bytecount(v, b'v')); + assert!(tx.response_trailers.size() == 1); +} + +fn bytecount(hay: &[u8], needle: u8) -> usize { + hay.iter().filter(|&&c| c == needle).count() +} + +#[test] +fn ResponseTrailersKeptOutOfHeaders() { + let cfg = TestConfig(); + let mut t = Test::new(cfg); + assert!(t.run_file("29-response-trailer-raw.t").is_ok()); + let tx = t.connp.tx(0).unwrap(); + assert!(tx.response_headers.get_nocase("Set-Cookie").is_none()); + assert_eq!(2, tx.response_trailers.size()); + assert_eq!( + b"name=value", + tx.response_trailers + .get_nocase("Set-Cookie") + .unwrap() + .value + .as_slice() + ); +} + #[test] fn GetIPv6() { let mut t = Test::new(TestConfig()); diff --git a/rust/htp/src/transaction.rs b/rust/htp/src/transaction.rs index 0a37573b8e..e3467d5461 100644 --- a/rust/htp/src/transaction.rs +++ b/rust/htp/src/transaction.rs @@ -378,6 +378,11 @@ pub struct Transaction { pub(crate) request_entity_len: u64, /// Parsed request headers. pub(crate) request_headers: Headers, + /// Trailer fields of a chunked request. Kept out of ``request_headers`` so that a + /// header keyword never silently gains trailer data. + pub(crate) request_trailers: Headers, + /// Set once the last chunk is parsed; the fields that follow go to ``request_trailers``. + pub(crate) request_in_trailers: bool, /// Request transfer coding. Can be one of UNKNOWN (body presence not /// determined yet), IDENTITY, CHUNKED, NO_BODY, /// and UNRECOGNIZED. @@ -451,6 +456,10 @@ pub struct Transaction { pub(crate) seen_100continue: bool, /// Parsed response headers. Contains instances of Header. pub(crate) response_headers: Headers, + /// Trailer fields of a chunked response, kept apart from ``response_headers``. + pub(crate) response_trailers: Headers, + /// Set once the last chunk is parsed; the fields that follow go to ``response_trailers``. + pub(crate) response_in_trailers: bool, /// Is this a response a HTTP/2.0 upgrade? pub(crate) is_http_2_upgrade: bool, @@ -545,6 +554,7 @@ impl std::fmt::Debug for Transaction { .field("request_message_len", &self.request_message_len) .field("request_entity_len", &self.request_entity_len) .field("request_headers", &self.request_headers) + .field("request_trailers", &self.request_trailers) .field("request_transfer_coding", &self.request_transfer_coding) .field("request_content_encoding", &self.request_content_encoding) .field( @@ -635,6 +645,8 @@ impl Transaction { request_message_len: 0, request_entity_len: 0, request_headers: Headers::with_capacity(32), + request_trailers: Headers::with_capacity(8), + request_in_trailers: false, request_transfer_coding: HtpTransferCoding::Unknown, request_content_encoding: HtpContentEncoding::None, request_content_encoding_processing: HtpContentEncoding::None, @@ -659,6 +671,8 @@ impl Transaction { response_message: None, seen_100continue: false, response_headers: Headers::with_capacity(32), + response_trailers: Headers::with_capacity(8), + response_in_trailers: false, is_http_2_upgrade: false, response_message_len: 0, response_entity_len: 0, diff --git a/src/output-json-http.c b/src/output-json-http.c index 20888902a9..e2346d2147 100644 --- a/src/output-json-http.c +++ b/src/output-json-http.c @@ -305,18 +305,13 @@ static void EveHttpLogJSONExtended(SCJsonBuilder *js, htp_tx_t *tx) SCJbSetUint(js, "length", htp_tx_response_message_len(tx)); } -static void EveHttpLogJSONHeaders( - SCJsonBuilder *js, uint32_t direction, htp_tx_t *tx, LogHttpFileCtx *http_ctx) +static bool EveHttpLogJSONHeaderTable(SCJsonBuilder *js, const htp_headers_t *headers, + uint32_t direction, LogHttpFileCtx *http_ctx) { - const htp_headers_t *headers = direction & LOG_HTTP_REQ_HEADERS ? htp_tx_request_headers(tx) - : htp_tx_response_headers(tx); char name[MAX_SIZE_HEADER_NAME] = {0}; char value[MAX_SIZE_HEADER_VALUE] = {0}; + bool wrote = false; size_t n = htp_headers_size(headers); - SCJsonBuilderMark mark = { 0, 0, 0 }; - SCJbGetMark(js, &mark); - bool array_empty = true; - SCJbOpenArray(js, direction & LOG_HTTP_REQ_HEADERS ? "request_headers" : "response_headers"); for (size_t i = 0; i < n; i++) { const htp_header_t *h = htp_headers_get_index(headers, i); if ((http_ctx->flags & direction) == 0 && http_ctx->fields != 0) { @@ -339,7 +334,7 @@ static void EveHttpLogJSONHeaders( continue; } } - array_empty = false; + wrote = true; SCJbStartObject(js); size_t size_name = htp_header_name_len(h) < MAX_SIZE_HEADER_NAME - 1 ? htp_header_name_len(h) @@ -355,6 +350,28 @@ static void EveHttpLogJSONHeaders( SCJbSetString(js, "value", value); SCJbClose(js); } + return wrote; +} + +static void EveHttpLogJSONHeaders( + SCJsonBuilder *js, uint32_t direction, htp_tx_t *tx, LogHttpFileCtx *http_ctx) +{ + const htp_headers_t *headers = direction & LOG_HTTP_REQ_HEADERS ? htp_tx_request_headers(tx) + : htp_tx_response_headers(tx); + const htp_headers_t *trailers = direction & LOG_HTTP_REQ_HEADERS ? htp_tx_request_trailers(tx) + : htp_tx_response_trailers(tx); + SCJsonBuilderMark mark = { 0, 0, 0 }; + SCJbGetMark(js, &mark); + SCJbOpenArray(js, direction & LOG_HTTP_REQ_HEADERS ? "request_headers" : "response_headers"); + bool array_empty = true; + if (headers != NULL) { + array_empty = !EveHttpLogJSONHeaderTable(js, headers, direction, http_ctx); + } + /* Trailer fields are no longer merged into the header table, but they must stay in the + * dump: a field only present in the trailer block would otherwise vanish from EVE. */ + if (trailers != NULL) { + array_empty &= !EveHttpLogJSONHeaderTable(js, trailers, direction, http_ctx); + } if (array_empty) { SCJbRestoreMark(js, &mark); } else { From cb3dbe9f33c5eb6c4295e9bba06d87c7b5528ab1 Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Fri, 9 Oct 2026 21:29:14 +0000 Subject: [PATCH 3/9] app-layer/htp1: keep trailer bytes out of the raw header buffer The trailer data hooks were pointed at the header data callbacks, so the bytes of a chunked message's trailer section were appended to the same buffer the header block goes to. http.header.raw therefore matched trailer content, which rules written against the header section cannot expect and cannot avoid. Accumulate the trailer bytes per direction in a buffer of their own instead. Nothing reads them yet. Ticket: #8256. --- src/app-layer-htp.c | 48 +++++++++++++++++++++++++++++++++++++++++++-- src/app-layer-htp.h | 7 +++++++ 2 files changed, 53 insertions(+), 2 deletions(-) diff --git a/src/app-layer-htp.c b/src/app-layer-htp.c index 7c9102758e..afb6c66541 100644 --- a/src/app-layer-htp.c +++ b/src/app-layer-htp.c @@ -476,6 +476,10 @@ static void HtpTxUserDataFree(void *txud) HTPFree(htud->request_headers_raw, htud->request_headers_raw_len); if (htud->response_headers_raw) HTPFree(htud->response_headers_raw, htud->response_headers_raw_len); + if (htud->request_trailers_raw) + HTPFree(htud->request_trailers_raw, htud->request_trailers_raw_len); + if (htud->response_trailers_raw) + HTPFree(htud->response_trailers_raw, htud->response_trailers_raw_len); if (htud->mime_state) SCMimeStateFree(htud->mime_state); SCAppLayerTxDataCleanup(&htud->tx_data); @@ -1894,6 +1898,46 @@ static int HTPCallbackResponseHeaderData(const htp_connp_t *connp, htp_tx_data_t return HTP_STATUS_OK; } +static void HTPAppendTrailerData( + HtpTxUserData *tx_ud, bool to_server, const uint8_t *data, size_t len) +{ + uint8_t **buf = to_server ? &tx_ud->request_trailers_raw : &tx_ud->response_trailers_raw; + uint32_t *buf_len = + to_server ? &tx_ud->request_trailers_raw_len : &tx_ud->response_trailers_raw_len; + + void *ptmp = HTPRealloc(*buf, *buf_len, (size_t)*buf_len + len); + if (ptmp == NULL) { + return; + } + *buf = ptmp; + memcpy(*buf + *buf_len, data, len); + *buf_len += len; +} + +static int HTPCallbackRequestTrailerData(const htp_connp_t *connp, htp_tx_data_t *tx_data) +{ + const htp_tx_t *tx = htp_tx_data_tx(tx_data); + if (htp_tx_data_is_empty(tx_data) || tx == NULL) + return HTP_STATUS_OK; + + HtpTxUserData *tx_ud = htp_tx_get_user_data(tx); + tx_ud->tx_data.updated_ts = true; + HTPAppendTrailerData(tx_ud, true, htp_tx_data_data(tx_data), htp_tx_data_len(tx_data)); + return HTP_STATUS_OK; +} + +static int HTPCallbackResponseTrailerData(const htp_connp_t *connp, htp_tx_data_t *tx_data) +{ + const htp_tx_t *tx = htp_tx_data_tx(tx_data); + if (htp_tx_data_is_empty(tx_data) || tx == NULL) + return HTP_STATUS_OK; + + HtpTxUserData *tx_ud = htp_tx_get_user_data(tx); + tx_ud->tx_data.updated_tc = true; + HTPAppendTrailerData(tx_ud, false, htp_tx_data_data(tx_data), htp_tx_data_len(tx_data)); + return HTP_STATUS_OK; +} + /* * We have a similar set function called HTPConfigSetDefaultsPhase1. */ @@ -1915,9 +1959,9 @@ static void HTPConfigSetDefaultsPhase1(HTPCfgRec *cfg_prec) cfg_prec->randomize_range = HTP_CONFIG_DEFAULT_RANDOMIZE_RANGE; htp_config_register_request_header_data(cfg_prec->cfg, HTPCallbackRequestHeaderData); - htp_config_register_request_trailer_data(cfg_prec->cfg, HTPCallbackRequestHeaderData); + htp_config_register_request_trailer_data(cfg_prec->cfg, HTPCallbackRequestTrailerData); htp_config_register_response_header_data(cfg_prec->cfg, HTPCallbackResponseHeaderData); - htp_config_register_response_trailer_data(cfg_prec->cfg, HTPCallbackResponseHeaderData); + htp_config_register_response_trailer_data(cfg_prec->cfg, HTPCallbackResponseTrailerData); htp_config_register_request_trailer(cfg_prec->cfg, HTPCallbackRequestHasTrailer); htp_config_register_response_trailer(cfg_prec->cfg, HTPCallbackResponseHasTrailer); diff --git a/src/app-layer-htp.h b/src/app-layer-htp.h index bad642d691..959d75bf58 100644 --- a/src/app-layer-htp.h +++ b/src/app-layer-htp.h @@ -171,6 +171,13 @@ typedef struct HtpTxUserData_ { uint32_t request_headers_raw_len; uint32_t response_headers_raw_len; + /* raw bytes of the trailer section of a chunked message, kept apart from the + * header block so that http.header.raw does not gain trailer data */ + uint8_t *request_trailers_raw; + uint8_t *response_trailers_raw; + uint32_t request_trailers_raw_len; + uint32_t response_trailers_raw_len; + MimeStateHTTP *mime_state; HttpRangeContainerBlock *file_range; /**< used to assign track ids to range file */ From 3ecba26fb2cacb066e960d92c2007a59805294c4 Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Fri, 9 Oct 2026 21:41:29 +0000 Subject: [PATCH 4/9] detect/http1: give trailers buffers of their own Trailer fields were merged into the header fields by the parser, so http.header, http.header.raw, http.header_names and every field keyword could match trailer data, and whether they did was a matter of packet timing: the header buffers are complete at the headers state, while a trailer arrives after the body. Add three buffers for the trailer section, registered at the state that section completes in: http.trailer trailer fields, normalized as "Name: value" http.trailer_names trailer field names http.trailer.raw the trailer section as seen on the wire Like the other HTTP/1 buffers they become final one state later, so inspection rounds while a trailer is still arriving get revisited rather than retiring rules early. That also removes the need for the trailer legs of the header prefilters, which existed only to revisit a buffer that could still grow. With their last reader gone, the request_has_trailers and response_has_trailers flags and the htp callbacks setting them are dropped as well; the revisit trigger they also provided is set by the trailer data callbacks. http.header.raw keeps its engines at one state past the headers, unlike its siblings: its data comes from the raw header data hook, which htp finalizes as the parser leaves the header state, so a round at the headers state can still see a short block. HTTP/2 keeps putting trailer blocks in the same list as regular headers and is handled separately. Ticket: #8256. --- src/Makefile.am | 5 + src/app-layer-htp.c | 19 --- src/app-layer-htp.h | 3 - src/detect-engine-register.c | 4 + src/detect-engine-register.h | 3 + src/detect-http-header.c | 152 +--------------------- src/detect-http-raw-header.c | 155 ++--------------------- src/detect-http-trailer-names.c | 169 +++++++++++++++++++++++++ src/detect-http-trailer-raw.c | 118 +++++++++++++++++ src/detect-http-trailer.c | 171 +++++++++++++++++++++++++ src/detect-http-trailer.h | 31 +++++ src/tests/detect-http-header.c | 36 +++--- src/tests/detect-http-raw-header.c | 4 +- src/tests/detect-http-trailer.c | 195 +++++++++++++++++++++++++++++ src/tests/detect.c | 6 + 15 files changed, 735 insertions(+), 336 deletions(-) create mode 100644 src/detect-http-trailer-names.c create mode 100644 src/detect-http-trailer-raw.c create mode 100644 src/detect-http-trailer.c create mode 100644 src/detect-http-trailer.h create mode 100644 src/tests/detect-http-trailer.c diff --git a/src/Makefile.am b/src/Makefile.am index 7e27764a93..030e9d1c23 100755 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -216,6 +216,8 @@ noinst_HEADERS = \ detect-http-start.h \ detect-http-stat-code.h \ detect-http-stat-msg.h \ + detect-http-trailer.h \ + tests/detect-http-trailer.c \ detect-http-ua.h \ detect-http-uri.h \ detect-http2.h \ @@ -791,6 +793,9 @@ libsuricata_c_a_SOURCES = \ detect-http-start.c \ detect-http-stat-code.c \ detect-http-stat-msg.c \ + detect-http-trailer-names.c \ + detect-http-trailer-raw.c \ + detect-http-trailer.c \ detect-http-ua.c \ detect-http-uri.c \ detect-http2.c \ diff --git a/src/app-layer-htp.c b/src/app-layer-htp.c index afb6c66541..0c33375ea2 100644 --- a/src/app-layer-htp.c +++ b/src/app-layer-htp.c @@ -1609,22 +1609,6 @@ void HTPFreeConfig(void) SCReturn; } -static int HTPCallbackRequestHasTrailer(const htp_connp_t *connp, htp_tx_t *tx) -{ - HtpTxUserData *htud = (HtpTxUserData *)htp_tx_get_user_data(tx); - htud->tx_data.updated_ts = true; - htud->request_has_trailers = 1; - return HTP_STATUS_OK; -} - -static int HTPCallbackResponseHasTrailer(const htp_connp_t *connp, htp_tx_t *tx) -{ - HtpTxUserData *htud = (HtpTxUserData *)htp_tx_get_user_data(tx); - htud->tx_data.updated_tc = true; - htud->response_has_trailers = 1; - return HTP_STATUS_OK; -} - static void *HTPCallbackTxCreate(bool request) { HtpTxUserData *tx_ud = HTPCalloc(1, sizeof(HtpTxUserData)); @@ -1963,9 +1947,6 @@ static void HTPConfigSetDefaultsPhase1(HTPCfgRec *cfg_prec) htp_config_register_response_header_data(cfg_prec->cfg, HTPCallbackResponseHeaderData); htp_config_register_response_trailer_data(cfg_prec->cfg, HTPCallbackResponseTrailerData); - htp_config_register_request_trailer(cfg_prec->cfg, HTPCallbackRequestHasTrailer); - htp_config_register_response_trailer(cfg_prec->cfg, HTPCallbackResponseHasTrailer); - htp_config_register_request_body_data(cfg_prec->cfg, HTPCallbackRequestBodyData); htp_config_register_response_body_data(cfg_prec->cfg, HTPCallbackResponseBodyData); diff --git a/src/app-layer-htp.h b/src/app-layer-htp.h index 959d75bf58..05ef1391b7 100644 --- a/src/app-layer-htp.h +++ b/src/app-layer-htp.h @@ -155,9 +155,6 @@ typedef struct HtpTxUserData_ { uint8_t request_body_init; uint8_t response_body_init; - uint8_t request_has_trailers; - uint8_t response_has_trailers; - uint8_t tsflags; uint8_t tcflags; diff --git a/src/detect-engine-register.c b/src/detect-engine-register.c index 0c94152a10..9af89e0bcf 100644 --- a/src/detect-engine-register.c +++ b/src/detect-engine-register.c @@ -169,6 +169,7 @@ #include "detect-http-header-names.h" #include "detect-http-headers.h" #include "detect-http-raw-header.h" +#include "detect-http-trailer.h" #include "detect-http-uri.h" #include "detect-http-protocol.h" #include "detect-http-start.h" @@ -671,6 +672,9 @@ void SigTableSetup(void) DetectHttpProtocolRegister(); DetectHttpStartRegister(); DetectHttpRawHeaderRegister(); + DetectHttpTrailerRegister(); + DetectHttpTrailerNamesRegister(); + DetectHttpTrailerRawRegister(); DetectHttpMethodRegister(); DetectHttpCookieRegister(); diff --git a/src/detect-engine-register.h b/src/detect-engine-register.h index 8a247f3600..9b378adcda 100644 --- a/src/detect-engine-register.h +++ b/src/detect-engine-register.h @@ -171,6 +171,9 @@ enum DetectKeywordId { DETECT_HTTP_HEADER_CM, DETECT_HTTP_HEADER, DETECT_HTTP_HEADER_NAMES, + DETECT_HTTP_TRAILER, + DETECT_HTTP_TRAILER_NAMES, + DETECT_HTTP_TRAILER_RAW, DETECT_HTTP_HEADER_ACCEPT, DETECT_HTTP_HEADER_ACCEPT_LANG, DETECT_HTTP_HEADER_ACCEPT_ENC, diff --git a/src/detect-http-header.c b/src/detect-http-header.c index 89ea1d199e..1fb0a507da 100644 --- a/src/detect-http-header.c +++ b/src/detect-http-header.c @@ -181,148 +181,6 @@ static InspectionBuffer *GetData1(DetectEngineThreadCtx *det_ctx, return buffer; } -typedef struct PrefilterMpmHttpHeaderCtx { - int list_id; - const MpmCtx *mpm_ctx; - const DetectEngineTransforms *transforms; -} PrefilterMpmHttpHeaderCtx; - -/** \brief Generic Mpm prefilter callback - * - * \param det_ctx detection engine thread ctx - * \param p packet to inspect - * \param f flow to inspect - * \param txv tx to inspect - * \param pectx inspection context - */ -static void PrefilterMpmHttpHeader(DetectEngineThreadCtx *det_ctx, const void *pectx, Packet *p, - Flow *f, void *txv, const uint64_t idx, const AppLayerTxData *_txd, const uint8_t flags) -{ - SCEnter(); - - const PrefilterMpmHttpHeaderCtx *ctx = pectx; - const MpmCtx *mpm_ctx = ctx->mpm_ctx; - SCLogDebug("running on list %d", ctx->list_id); - - const int list_id = ctx->list_id; - InspectionBuffer *buffer = SCInspectionBufferGet(det_ctx, list_id); - if (buffer->inspect == NULL) { - uint32_t rawdata_len = 0; - uint8_t *rawdata = GetBufferForTX(txv, det_ctx, flags, &rawdata_len); - if (rawdata_len == 0) - return; - - /* setup buffer and apply transforms */ - SCInspectionBufferSetupAndApplyTransforms( - det_ctx, list_id, buffer, rawdata, rawdata_len, ctx->transforms); - } - - const uint32_t data_len = buffer->inspect_len; - const uint8_t *data = buffer->inspect; - - SCLogDebug("mpm'ing buffer:"); - //PrintRawDataFp(stdout, data, data_len); - - if (data != NULL && data_len >= mpm_ctx->minlen) { - (void)mpm_table[mpm_ctx->mpm_type].Search( - mpm_ctx, &det_ctx->mtc, &det_ctx->pmq, data, data_len); - PREFILTER_PROFILING_ADD_BYTES(det_ctx, data_len); - } -} - -static void PrefilterMpmHttpTrailer(DetectEngineThreadCtx *det_ctx, const void *pectx, Packet *p, - Flow *f, void *txv, const uint64_t idx, const AppLayerTxData *_txd, const uint8_t flags) -{ - SCEnter(); - - htp_tx_t *tx = txv; - const HtpTxUserData *htud = (const HtpTxUserData *)htp_tx_get_user_data(tx); - /* if the request wasn't flagged as having a trailer, we skip */ - if (((flags & STREAM_TOSERVER) && !htud->request_has_trailers) || - ((flags & STREAM_TOCLIENT) && !htud->response_has_trailers)) { - SCReturn; - } - PrefilterMpmHttpHeader(det_ctx, pectx, p, f, txv, idx, _txd, flags); - SCReturn; -} - -static void PrefilterMpmHttpHeaderFree(void *ptr) -{ - SCFree(ptr); -} - -static int PrefilterMpmHttpHeaderRequestRegister(DetectEngineCtx *de_ctx, SigGroupHead *sgh, - MpmCtx *mpm_ctx, const DetectBufferMpmRegistry *mpm_reg, int list_id) -{ - SCEnter(); - - /* header */ - PrefilterMpmHttpHeaderCtx *pectx = SCCalloc(1, sizeof(*pectx)); - if (pectx == NULL) - return -1; - pectx->list_id = list_id; - pectx->mpm_ctx = mpm_ctx; - pectx->transforms = &mpm_reg->transforms; - - int r = PrefilterAppendTxEngine(de_ctx, sgh, PrefilterMpmHttpHeader, mpm_reg->app_v2.alproto, - HTP_REQUEST_PROGRESS_HEADERS, pectx, PrefilterMpmHttpHeaderFree, mpm_reg->pname); - if (r != 0) { - SCFree(pectx); - return r; - } - - /* trailer */ - pectx = SCCalloc(1, sizeof(*pectx)); - if (pectx == NULL) - return -1; - pectx->list_id = list_id; - pectx->mpm_ctx = mpm_ctx; - pectx->transforms = &mpm_reg->transforms; - - r = PrefilterAppendTxEngine(de_ctx, sgh, PrefilterMpmHttpTrailer, mpm_reg->app_v2.alproto, - HTP_REQUEST_PROGRESS_TRAILER, pectx, PrefilterMpmHttpHeaderFree, mpm_reg->pname); - if (r != 0) { - SCFree(pectx); - } - return r; -} - -static int PrefilterMpmHttpHeaderResponseRegister(DetectEngineCtx *de_ctx, SigGroupHead *sgh, - MpmCtx *mpm_ctx, const DetectBufferMpmRegistry *mpm_reg, int list_id) -{ - SCEnter(); - - /* header */ - PrefilterMpmHttpHeaderCtx *pectx = SCCalloc(1, sizeof(*pectx)); - if (pectx == NULL) - return -1; - pectx->list_id = list_id; - pectx->mpm_ctx = mpm_ctx; - pectx->transforms = &mpm_reg->transforms; - - int r = PrefilterAppendTxEngine(de_ctx, sgh, PrefilterMpmHttpHeader, mpm_reg->app_v2.alproto, - HTP_RESPONSE_PROGRESS_HEADERS, pectx, PrefilterMpmHttpHeaderFree, mpm_reg->pname); - if (r != 0) { - SCFree(pectx); - return r; - } - - /* trailer */ - pectx = SCCalloc(1, sizeof(*pectx)); - if (pectx == NULL) - return -1; - pectx->list_id = list_id; - pectx->mpm_ctx = mpm_ctx; - pectx->transforms = &mpm_reg->transforms; - - r = PrefilterAppendTxEngine(de_ctx, sgh, PrefilterMpmHttpTrailer, mpm_reg->app_v2.alproto, - HTP_RESPONSE_PROGRESS_TRAILER, pectx, PrefilterMpmHttpHeaderFree, mpm_reg->pname); - if (r != 0) { - SCFree(pectx); - } - return r; -} - /** * \brief The setup function for the http_header keyword for a signature. * @@ -388,15 +246,13 @@ void DetectHttpHeaderRegister(void) DetectAppLayerInspectEngineRegister("http_header", ALPROTO_HTTP1, SIG_FLAG_TOSERVER, HTP_REQUEST_PROGRESS_HEADERS, DetectEngineInspectBufferGeneric, GetData1); - DetectAppLayerMpmRegister("http_header", SIG_FLAG_TOSERVER, 2, - PrefilterMpmHttpHeaderRequestRegister, NULL, ALPROTO_HTTP1, - 0); /* not used, registered twice: HEADERS/TRAILER */ + DetectAppLayerMpmRegister("http_header", SIG_FLAG_TOSERVER, 2, PrefilterGenericMpmRegister, + GetData1, ALPROTO_HTTP1, HTP_REQUEST_PROGRESS_HEADERS); DetectAppLayerInspectEngineRegister("http_header", ALPROTO_HTTP1, SIG_FLAG_TOCLIENT, HTP_RESPONSE_PROGRESS_HEADERS, DetectEngineInspectBufferGeneric, GetData1); - DetectAppLayerMpmRegister("http_header", SIG_FLAG_TOCLIENT, 2, - PrefilterMpmHttpHeaderResponseRegister, NULL, ALPROTO_HTTP1, - 0); /* not used, registered twice: HEADERS/TRAILER */ + DetectAppLayerMpmRegister("http_header", SIG_FLAG_TOCLIENT, 2, PrefilterGenericMpmRegister, + GetData1, ALPROTO_HTTP1, HTP_RESPONSE_PROGRESS_HEADERS); /* header is for stream TX type */ DetectAppLayerInspectEngineRegisterSubState("http_header", ALPROTO_HTTP2, SIG_FLAG_TOSERVER, diff --git a/src/detect-http-raw-header.c b/src/detect-http-raw-header.c index 3bb50cd1fc..a4705687ba 100644 --- a/src/detect-http-raw-header.c +++ b/src/detect-http-raw-header.c @@ -71,11 +71,6 @@ static InspectionBuffer *GetData2(DetectEngineThreadCtx *det_ctx, const DetectEngineTransforms *transforms, Flow *_f, const uint8_t flow_flags, void *txv, const int list_id); -static int PrefilterMpmHttpHeaderRawRequestRegister(DetectEngineCtx *de_ctx, SigGroupHead *sgh, - MpmCtx *mpm_ctx, const DetectBufferMpmRegistry *mpm_reg, int list_id); -static int PrefilterMpmHttpHeaderRawResponseRegister(DetectEngineCtx *de_ctx, SigGroupHead *sgh, - MpmCtx *mpm_ctx, const DetectBufferMpmRegistry *mpm_reg, int list_id); - /** * \brief Registers the keyword handlers for the "http_raw_header" keyword. */ @@ -101,17 +96,19 @@ 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); DetectAppLayerInspectEngineRegister("http_raw_header", ALPROTO_HTTP1, SIG_FLAG_TOCLIENT, HTP_RESPONSE_PROGRESS_HEADERS + 1, DetectEngineInspectBufferGeneric, GetData); - DetectAppLayerMpmRegister("http_raw_header", SIG_FLAG_TOSERVER, 2, - PrefilterMpmHttpHeaderRawRequestRegister, NULL, ALPROTO_HTTP1, - 0); /* progress handled in register */ - DetectAppLayerMpmRegister("http_raw_header", SIG_FLAG_TOCLIENT, 2, - PrefilterMpmHttpHeaderRawResponseRegister, NULL, ALPROTO_HTTP1, - 0); /* progress handled in register */ + DetectAppLayerMpmRegister("http_raw_header", SIG_FLAG_TOSERVER, 2, PrefilterGenericMpmRegister, + GetData, ALPROTO_HTTP1, HTP_REQUEST_PROGRESS_HEADERS + 1); + DetectAppLayerMpmRegister("http_raw_header", SIG_FLAG_TOCLIENT, 2, PrefilterGenericMpmRegister, + GetData, ALPROTO_HTTP1, HTP_RESPONSE_PROGRESS_HEADERS + 1); DetectAppLayerInspectEngineRegisterSubState("http_raw_header", ALPROTO_HTTP2, SIG_FLAG_TOSERVER, HTTP2TxTypeStream, HTTP2ProgHeaders, DetectEngineInspectBufferGeneric, GetData2); @@ -236,142 +233,6 @@ static InspectionBuffer *GetData2(DetectEngineThreadCtx *det_ctx, return buffer; } -typedef struct PrefilterMpmHttpHeaderRawCtx { - int list_id; - const MpmCtx *mpm_ctx; - const DetectEngineTransforms *transforms; -} PrefilterMpmHttpHeaderRawCtx; - -/** \brief Generic Mpm prefilter callback - * - * \param det_ctx detection engine thread ctx - * \param p packet to inspect - * \param f flow to inspect - * \param txv tx to inspect - * \param pectx inspection context - */ -static void PrefilterMpmHttpHeaderRaw(DetectEngineThreadCtx *det_ctx, const void *pectx, Packet *p, - Flow *f, void *txv, const uint64_t idx, const AppLayerTxData *_txd, const uint8_t flags) -{ - SCEnter(); - - const PrefilterMpmHttpHeaderRawCtx *ctx = pectx; - const MpmCtx *mpm_ctx = ctx->mpm_ctx; - SCLogDebug("running on list %d", ctx->list_id); - - const int list_id = ctx->list_id; - - InspectionBuffer *buffer = GetData(det_ctx, ctx->transforms, f, - flags, txv, list_id); - if (buffer == NULL) - return; - - const uint32_t data_len = buffer->inspect_len; - const uint8_t *data = buffer->inspect; - - SCLogDebug("mpm'ing buffer:"); - //PrintRawDataFp(stdout, data, data_len); - - if (data != NULL && data_len >= mpm_ctx->minlen) { - (void)mpm_table[mpm_ctx->mpm_type].Search( - mpm_ctx, &det_ctx->mtc, &det_ctx->pmq, data, data_len); - PREFILTER_PROFILING_ADD_BYTES(det_ctx, data_len); - } -} - -static void PrefilterMpmHttpTrailerRaw(DetectEngineThreadCtx *det_ctx, const void *pectx, Packet *p, - Flow *f, void *txv, const uint64_t idx, const AppLayerTxData *_txd, const uint8_t flags) -{ - SCEnter(); - - htp_tx_t *tx = txv; - const HtpTxUserData *htud = (const HtpTxUserData *)htp_tx_get_user_data(tx); - /* if the request wasn't flagged as having a trailer, we skip */ - if (((flags & STREAM_TOSERVER) && !htud->request_has_trailers) || - ((flags & STREAM_TOCLIENT) && !htud->response_has_trailers)) { - SCReturn; - } - PrefilterMpmHttpHeaderRaw(det_ctx, pectx, p, f, txv, idx, _txd, flags); - SCReturn; -} - -static void PrefilterMpmHttpHeaderRawFree(void *ptr) -{ - SCFree(ptr); -} - -static int PrefilterMpmHttpHeaderRawRequestRegister(DetectEngineCtx *de_ctx, SigGroupHead *sgh, - MpmCtx *mpm_ctx, const DetectBufferMpmRegistry *mpm_reg, int list_id) -{ - SCEnter(); - - /* header */ - PrefilterMpmHttpHeaderRawCtx *pectx = SCCalloc(1, sizeof(*pectx)); - if (pectx == NULL) - return -1; - pectx->list_id = list_id; - pectx->mpm_ctx = mpm_ctx; - pectx->transforms = &mpm_reg->transforms; - - int r = PrefilterAppendTxEngine(de_ctx, sgh, PrefilterMpmHttpHeaderRaw, mpm_reg->app_v2.alproto, - HTP_REQUEST_PROGRESS_HEADERS + 1, pectx, PrefilterMpmHttpHeaderRawFree, mpm_reg->pname); - if (r != 0) { - SCFree(pectx); - return r; - } - - /* trailer */ - pectx = SCCalloc(1, sizeof(*pectx)); - if (pectx == NULL) - return -1; - pectx->list_id = list_id; - pectx->mpm_ctx = mpm_ctx; - pectx->transforms = &mpm_reg->transforms; - - r = PrefilterAppendTxEngine(de_ctx, sgh, PrefilterMpmHttpTrailerRaw, mpm_reg->app_v2.alproto, - HTP_REQUEST_PROGRESS_TRAILER + 1, pectx, PrefilterMpmHttpHeaderRawFree, mpm_reg->pname); - if (r != 0) { - SCFree(pectx); - } - return r; -} - -static int PrefilterMpmHttpHeaderRawResponseRegister(DetectEngineCtx *de_ctx, SigGroupHead *sgh, - MpmCtx *mpm_ctx, const DetectBufferMpmRegistry *mpm_reg, int list_id) -{ - SCEnter(); - - /* header */ - PrefilterMpmHttpHeaderRawCtx *pectx = SCCalloc(1, sizeof(*pectx)); - if (pectx == NULL) - return -1; - pectx->list_id = list_id; - pectx->mpm_ctx = mpm_ctx; - pectx->transforms = &mpm_reg->transforms; - - int r = PrefilterAppendTxEngine(de_ctx, sgh, PrefilterMpmHttpHeaderRaw, mpm_reg->app_v2.alproto, - HTP_RESPONSE_PROGRESS_HEADERS, pectx, PrefilterMpmHttpHeaderRawFree, mpm_reg->pname); - if (r != 0) { - SCFree(pectx); - return r; - } - - /* trailer */ - pectx = SCCalloc(1, sizeof(*pectx)); - if (pectx == NULL) - return -1; - pectx->list_id = list_id; - pectx->mpm_ctx = mpm_ctx; - pectx->transforms = &mpm_reg->transforms; - - r = PrefilterAppendTxEngine(de_ctx, sgh, PrefilterMpmHttpTrailerRaw, mpm_reg->app_v2.alproto, - HTP_RESPONSE_PROGRESS_TRAILER, pectx, PrefilterMpmHttpHeaderRawFree, mpm_reg->pname); - if (r != 0) { - SCFree(pectx); - } - return r; -} - /************************************Unittests*********************************/ #ifdef UNITTESTS diff --git a/src/detect-http-trailer-names.c b/src/detect-http-trailer-names.c new file mode 100644 index 0000000000..bf4fd2b23c --- /dev/null +++ b/src/detect-http-trailer-names.c @@ -0,0 +1,169 @@ +/* Copyright (C) 2026 Open Information Security Foundation + * + * You can copy, redistribute or modify this Program under the terms of + * the GNU General Public License version 2 as published by the Free + * Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * version 2 along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA + * 02110-1301, USA. + */ + +/** + * \file + * + * \author Victor Julien + * + * Names of the trailer fields of an HTTP/1 chunked message. + */ + +#include "suricata-common.h" + +#include "detect.h" +#include "detect-parse.h" +#include "detect-engine.h" +#include "detect-engine-buffer.h" +#include "detect-engine-mpm.h" +#include "detect-engine-prefilter.h" +#include "detect-engine-state.h" +#include "detect-content.h" +#include "detect-http-trailer.h" +#include "detect-http-header-common.h" + +#include "app-layer.h" +#include "app-layer-parser.h" +#include "app-layer-htp.h" + +#include "flow.h" +#include "util-debug.h" +#include "util-print.h" + +#define KEYWORD_NAME "http.trailer_names" +#define KEYWORD_NAME_LEGACY "http_trailer_names" +#define KEYWORD_DOC "http-keywords.html#http-trailer-names" +#define BUFFER_NAME "http_trailer_names" +#define BUFFER_DESC "http trailer field names" +static int g_buffer_id = 0; +static int g_keyword_thread_id = 0; + +#define BUFFER_SIZE_STEP 256 +static HttpHeaderThreadDataConfig g_td_config = { BUFFER_SIZE_STEP }; + +static InspectionBuffer *GetBuffer1ForTX(DetectEngineThreadCtx *det_ctx, + const DetectEngineTransforms *transforms, Flow *_f, const uint8_t flow_flags, void *txv, + const int list_id); + +static int DetectHttpTrailerNamesSetup(DetectEngineCtx *de_ctx, Signature *s, const char *str) +{ + if (SCDetectBufferSetActiveList(de_ctx, s, g_buffer_id) < 0) + return -1; + if (SCDetectSignatureSetAppProto(s, ALPROTO_HTTP) < 0) + return -1; + return 0; +} + +void DetectHttpTrailerNamesRegister(void) +{ + sigmatch_table[DETECT_HTTP_TRAILER_NAMES].name = KEYWORD_NAME; + sigmatch_table[DETECT_HTTP_TRAILER_NAMES].alias = KEYWORD_NAME_LEGACY; + sigmatch_table[DETECT_HTTP_TRAILER_NAMES].desc = BUFFER_NAME " sticky buffer"; + sigmatch_table[DETECT_HTTP_TRAILER_NAMES].url = "/rules/" KEYWORD_DOC; + sigmatch_table[DETECT_HTTP_TRAILER_NAMES].Setup = DetectHttpTrailerNamesSetup; + sigmatch_table[DETECT_HTTP_TRAILER_NAMES].flags |= SIGMATCH_NOOPT | SIGMATCH_INFO_STICKY_BUFFER; + + DetectAppLayerInspectEngineRegister(BUFFER_NAME, ALPROTO_HTTP1, SIG_FLAG_TOSERVER, + HTP_REQUEST_PROGRESS_TRAILER, DetectEngineInspectBufferGeneric, GetBuffer1ForTX); + DetectAppLayerInspectEngineRegister(BUFFER_NAME, ALPROTO_HTTP1, SIG_FLAG_TOCLIENT, + HTP_RESPONSE_PROGRESS_TRAILER, DetectEngineInspectBufferGeneric, GetBuffer1ForTX); + + DetectAppLayerMpmRegister(BUFFER_NAME, SIG_FLAG_TOSERVER, 2, PrefilterGenericMpmRegister, + GetBuffer1ForTX, ALPROTO_HTTP1, HTP_REQUEST_PROGRESS_TRAILER); + DetectAppLayerMpmRegister(BUFFER_NAME, SIG_FLAG_TOCLIENT, 2, PrefilterGenericMpmRegister, + GetBuffer1ForTX, ALPROTO_HTTP1, HTP_RESPONSE_PROGRESS_TRAILER); + + DetectBufferTypeSetDescriptionByName(BUFFER_NAME, BUFFER_DESC); + + g_buffer_id = DetectBufferTypeGetByName(BUFFER_NAME); + + g_keyword_thread_id = SCDetectRegisterThreadCtxGlobalFuncs( + KEYWORD_NAME, HttpHeaderThreadDataInit, &g_td_config, HttpHeaderThreadDataFree); +} + +static uint8_t *GetBufferForTX( + htp_tx_t *tx, DetectEngineThreadCtx *det_ctx, uint8_t flags, uint32_t *buffer_len) +{ + *buffer_len = 0; + + HttpHeaderThreadData *hdr_td = NULL; + HttpHeaderBuffer *buf = HttpHeaderGetBufferSpace(det_ctx, flags, g_keyword_thread_id, &hdr_td); + if (unlikely(buf == NULL)) { + return NULL; + } + + const htp_headers_t *trailers = + (flags & STREAM_TOSERVER) ? htp_tx_request_trailers(tx) : htp_tx_response_trailers(tx); + if (trailers == NULL) + return NULL; + + /* fill the buffer. \r\nName1\r\nName2\r\n\r\n */ + size_t i = 0; + size_t no_of_trailers = htp_headers_size(trailers); + for (; i < no_of_trailers; i++) { + const htp_header_t *h = htp_headers_get_index(trailers, i); + size_t size = htp_header_name_len(h) + 2; // for \r\n + if (i == 0) + size += 2; + if (i + 1 == no_of_trailers) + size += 2; + + if (size + buf->len > buf->size) { + if (HttpHeaderExpandBuffer(hdr_td, buf, size) != 0) { + return NULL; + } + } + + /* start with a \r\n */ + if (i == 0) { + buf->buffer[buf->len++] = '\r'; + buf->buffer[buf->len++] = '\n'; + } + + memcpy(buf->buffer + buf->len, htp_header_name_ptr(h), htp_header_name_len(h)); + buf->len += htp_header_name_len(h); + buf->buffer[buf->len++] = '\r'; + buf->buffer[buf->len++] = '\n'; + + /* end with an extra \r\n */ + if (i + 1 == no_of_trailers) { + buf->buffer[buf->len++] = '\r'; + buf->buffer[buf->len++] = '\n'; + } + } + + *buffer_len = buf->len; + return buf->buffer; +} + +static InspectionBuffer *GetBuffer1ForTX(DetectEngineThreadCtx *det_ctx, + const DetectEngineTransforms *transforms, Flow *_f, const uint8_t flow_flags, void *txv, + const int list_id) +{ + InspectionBuffer *buffer = SCInspectionBufferGet(det_ctx, list_id); + if (buffer->inspect == NULL) { + uint32_t rawdata_len = 0; + uint8_t *rawdata = GetBufferForTX(txv, det_ctx, flow_flags, &rawdata_len); + if (rawdata_len == 0) + return NULL; + + SCInspectionBufferSetupAndApplyTransforms( + det_ctx, list_id, buffer, rawdata, rawdata_len, transforms); + } + + return buffer; +} diff --git a/src/detect-http-trailer-raw.c b/src/detect-http-trailer-raw.c new file mode 100644 index 0000000000..d529bc4e6e --- /dev/null +++ b/src/detect-http-trailer-raw.c @@ -0,0 +1,118 @@ +/* Copyright (C) 2026 Open Information Security Foundation + * + * You can copy, redistribute or modify this Program under the terms of + * the GNU General Public License version 2 as published by the Free + * Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * version 2 along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA + * 02110-1301, USA. + */ + +/** + * \file + * + * \author Victor Julien + * + * Raw bytes of the trailer section of an HTTP/1 chunked message. + */ + +#include "suricata-common.h" + +#include "detect.h" +#include "detect-parse.h" +#include "detect-engine.h" +#include "detect-engine-buffer.h" +#include "detect-engine-mpm.h" +#include "detect-engine-prefilter.h" +#include "detect-engine-state.h" +#include "detect-content.h" +#include "detect-http-trailer.h" + +#include "app-layer.h" +#include "app-layer-parser.h" +#include "app-layer-htp.h" + +#include "flow.h" +#include "util-debug.h" +#include "util-print.h" + +#define KEYWORD_NAME "http.trailer.raw" +#define KEYWORD_NAME_LEGACY "http_trailer_raw" +#define KEYWORD_DOC "http-keywords.html#http-trailer-raw" +#define BUFFER_NAME "http_trailer_raw" +#define BUFFER_DESC "raw http trailer data" +static int g_buffer_id = 0; + +static InspectionBuffer *GetData(DetectEngineThreadCtx *det_ctx, + const DetectEngineTransforms *transforms, Flow *_f, const uint8_t flow_flags, void *txv, + const int list_id); + +static int DetectHttpTrailerRawSetup(DetectEngineCtx *de_ctx, Signature *s, const char *str) +{ + if (SCDetectBufferSetActiveList(de_ctx, s, g_buffer_id) < 0) + return -1; + if (SCDetectSignatureSetAppProto(s, ALPROTO_HTTP) < 0) + return -1; + return 0; +} + +void DetectHttpTrailerRawRegister(void) +{ + sigmatch_table[DETECT_HTTP_TRAILER_RAW].name = KEYWORD_NAME; + sigmatch_table[DETECT_HTTP_TRAILER_RAW].alias = KEYWORD_NAME_LEGACY; + sigmatch_table[DETECT_HTTP_TRAILER_RAW].desc = BUFFER_NAME " sticky buffer"; + sigmatch_table[DETECT_HTTP_TRAILER_RAW].url = "/rules/" KEYWORD_DOC; + sigmatch_table[DETECT_HTTP_TRAILER_RAW].Setup = DetectHttpTrailerRawSetup; + sigmatch_table[DETECT_HTTP_TRAILER_RAW].flags |= SIGMATCH_NOOPT | SIGMATCH_INFO_STICKY_BUFFER; + + DetectAppLayerInspectEngineRegister(BUFFER_NAME, ALPROTO_HTTP1, SIG_FLAG_TOSERVER, + HTP_REQUEST_PROGRESS_TRAILER, DetectEngineInspectBufferGeneric, GetData); + DetectAppLayerInspectEngineRegister(BUFFER_NAME, ALPROTO_HTTP1, SIG_FLAG_TOCLIENT, + HTP_RESPONSE_PROGRESS_TRAILER, DetectEngineInspectBufferGeneric, GetData); + + DetectAppLayerMpmRegister(BUFFER_NAME, SIG_FLAG_TOSERVER, 2, PrefilterGenericMpmRegister, + GetData, ALPROTO_HTTP1, HTP_REQUEST_PROGRESS_TRAILER); + DetectAppLayerMpmRegister(BUFFER_NAME, SIG_FLAG_TOCLIENT, 2, PrefilterGenericMpmRegister, + GetData, ALPROTO_HTTP1, HTP_RESPONSE_PROGRESS_TRAILER); + + DetectBufferTypeSetDescriptionByName(BUFFER_NAME, BUFFER_DESC); + + g_buffer_id = DetectBufferTypeGetByName(BUFFER_NAME); +} + +static InspectionBuffer *GetData(DetectEngineThreadCtx *det_ctx, + const DetectEngineTransforms *transforms, Flow *_f, const uint8_t flow_flags, void *txv, + const int list_id) +{ + InspectionBuffer *buffer = SCInspectionBufferGet(det_ctx, list_id); + if (buffer->inspect == NULL) { + htp_tx_t *tx = (htp_tx_t *)txv; + + HtpTxUserData *tx_ud = htp_tx_get_user_data(tx); + + const bool ts = ((flow_flags & STREAM_TOSERVER) != 0); + const uint8_t *data = ts ? tx_ud->request_trailers_raw : tx_ud->response_trailers_raw; + if (data == NULL) + return NULL; + const uint32_t data_len = + ts ? tx_ud->request_trailers_raw_len : tx_ud->response_trailers_raw_len; + /* A section with no trailer fields holds only the framing that ends the chunked + * body - the empty line, possibly with stray whitespace. Not buffer content. */ + const htp_headers_t *trailers = + ts ? htp_tx_request_trailers(tx) : htp_tx_response_trailers(tx); + if (trailers == NULL || htp_headers_size(trailers) == 0) + return NULL; + + SCInspectionBufferSetupAndApplyTransforms( + det_ctx, list_id, buffer, data, data_len, transforms); + } + + return buffer; +} diff --git a/src/detect-http-trailer.c b/src/detect-http-trailer.c new file mode 100644 index 0000000000..f691a03b08 --- /dev/null +++ b/src/detect-http-trailer.c @@ -0,0 +1,171 @@ +/* Copyright (C) 2026 Open Information Security Foundation + * + * You can copy, redistribute or modify this Program under the terms of + * the GNU General Public License version 2 as published by the Free + * Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * version 2 along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA + * 02110-1301, USA. + */ + +/** + * \file + * + * \author Victor Julien + * + * Trailer fields of an HTTP/1 chunked message, as "Name: value\r\n" per field. + * The header section is a separate buffer: see detect-http-header.c. + */ + +#include "suricata-common.h" + +#include "detect.h" +#include "detect-parse.h" +#include "detect-engine.h" +#include "detect-engine-buffer.h" +#include "detect-engine-mpm.h" +#include "detect-engine-prefilter.h" +#include "detect-engine-state.h" +#include "detect-content.h" +#include "detect-http-trailer.h" +#include "detect-http-header-common.h" + +#include "app-layer.h" +#include "app-layer-parser.h" +#include "app-layer-htp.h" + +#include "flow.h" +#include "util-debug.h" +#include "util-print.h" + +#define KEYWORD_NAME "http.trailer" +#define KEYWORD_NAME_LEGACY "http_trailer" +#define KEYWORD_DOC "http-keywords.html#http-trailer" +#define BUFFER_NAME "http_trailer" +#define BUFFER_DESC "http trailer fields" +static int g_buffer_id = 0; +static int g_keyword_thread_id = 0; + +#ifdef UNITTESTS +void DetectHttpTrailerRegisterTests(void); +#endif + +#define BUFFER_SIZE_STEP 512 +static HttpHeaderThreadDataConfig g_td_config = { BUFFER_SIZE_STEP }; + +static InspectionBuffer *GetBuffer1ForTX(DetectEngineThreadCtx *det_ctx, + const DetectEngineTransforms *transforms, Flow *_f, const uint8_t flow_flags, void *txv, + const int list_id); + +static int DetectHttpTrailerSetup(DetectEngineCtx *de_ctx, Signature *s, const char *str) +{ + if (SCDetectBufferSetActiveList(de_ctx, s, g_buffer_id) < 0) + return -1; + if (SCDetectSignatureSetAppProto(s, ALPROTO_HTTP) < 0) + return -1; + return 0; +} + +void DetectHttpTrailerRegister(void) +{ + sigmatch_table[DETECT_HTTP_TRAILER].name = KEYWORD_NAME; + sigmatch_table[DETECT_HTTP_TRAILER].alias = KEYWORD_NAME_LEGACY; + sigmatch_table[DETECT_HTTP_TRAILER].desc = BUFFER_NAME " sticky buffer"; + sigmatch_table[DETECT_HTTP_TRAILER].url = "/rules/" KEYWORD_DOC; + sigmatch_table[DETECT_HTTP_TRAILER].Setup = DetectHttpTrailerSetup; + sigmatch_table[DETECT_HTTP_TRAILER].flags |= SIGMATCH_NOOPT | SIGMATCH_INFO_STICKY_BUFFER; + +#ifdef UNITTESTS + sigmatch_table[DETECT_HTTP_TRAILER].RegisterTests = DetectHttpTrailerRegisterTests; +#endif + + /* Inspect at the trailer state, the state the trailer section completes in. Rounds + * while the section is still arriving are not final, the one at COMPLETE is. */ + DetectAppLayerInspectEngineRegister(BUFFER_NAME, ALPROTO_HTTP1, SIG_FLAG_TOSERVER, + HTP_REQUEST_PROGRESS_TRAILER, DetectEngineInspectBufferGeneric, GetBuffer1ForTX); + DetectAppLayerInspectEngineRegister(BUFFER_NAME, ALPROTO_HTTP1, SIG_FLAG_TOCLIENT, + HTP_RESPONSE_PROGRESS_TRAILER, DetectEngineInspectBufferGeneric, GetBuffer1ForTX); + + DetectAppLayerMpmRegister(BUFFER_NAME, SIG_FLAG_TOSERVER, 2, PrefilterGenericMpmRegister, + GetBuffer1ForTX, ALPROTO_HTTP1, HTP_REQUEST_PROGRESS_TRAILER); + DetectAppLayerMpmRegister(BUFFER_NAME, SIG_FLAG_TOCLIENT, 2, PrefilterGenericMpmRegister, + GetBuffer1ForTX, ALPROTO_HTTP1, HTP_RESPONSE_PROGRESS_TRAILER); + + DetectBufferTypeSetDescriptionByName(BUFFER_NAME, BUFFER_DESC); + + g_buffer_id = DetectBufferTypeGetByName(BUFFER_NAME); + + g_keyword_thread_id = SCDetectRegisterThreadCtxGlobalFuncs( + KEYWORD_NAME, HttpHeaderThreadDataInit, &g_td_config, HttpHeaderThreadDataFree); +} + +static uint8_t *GetBufferForTX( + htp_tx_t *tx, DetectEngineThreadCtx *det_ctx, uint8_t flags, uint32_t *buffer_len) +{ + *buffer_len = 0; + + HttpHeaderThreadData *hdr_td = NULL; + HttpHeaderBuffer *buf = HttpHeaderGetBufferSpace(det_ctx, flags, g_keyword_thread_id, &hdr_td); + if (unlikely(buf == NULL)) { + return NULL; + } + + const htp_headers_t *trailers = + (flags & STREAM_TOSERVER) ? htp_tx_request_trailers(tx) : htp_tx_response_trailers(tx); + if (trailers == NULL) + return NULL; + + size_t i = 0; + size_t no_of_trailers = htp_headers_size(trailers); + for (; i < no_of_trailers; i++) { + const htp_header_t *h = htp_headers_get_index(trailers, i); + size_t size = htp_header_name_len(h) + htp_header_value_len(h) + 4; + + if (size + buf->len > buf->size) { + if (HttpHeaderExpandBuffer(hdr_td, buf, size) != 0) { + return NULL; + } + } + + memcpy(buf->buffer + buf->len, htp_header_name_ptr(h), htp_header_name_len(h)); + buf->len += htp_header_name_len(h); + buf->buffer[buf->len++] = ':'; + buf->buffer[buf->len++] = ' '; + memcpy(buf->buffer + buf->len, htp_header_value_ptr(h), htp_header_value_len(h)); + buf->len += htp_header_value_len(h); + buf->buffer[buf->len++] = '\r'; + buf->buffer[buf->len++] = '\n'; + } + + *buffer_len = buf->len; + return buf->buffer; +} + +static InspectionBuffer *GetBuffer1ForTX(DetectEngineThreadCtx *det_ctx, + const DetectEngineTransforms *transforms, Flow *_f, const uint8_t flow_flags, void *txv, + const int list_id) +{ + InspectionBuffer *buffer = SCInspectionBufferGet(det_ctx, list_id); + if (buffer->inspect == NULL) { + uint32_t rawdata_len = 0; + uint8_t *rawdata = GetBufferForTX(txv, det_ctx, flow_flags, &rawdata_len); + if (rawdata_len == 0) + return NULL; + + SCInspectionBufferSetupAndApplyTransforms( + det_ctx, list_id, buffer, rawdata, rawdata_len, transforms); + } + + return buffer; +} + +#ifdef UNITTESTS +#include "tests/detect-http-trailer.c" +#endif diff --git a/src/detect-http-trailer.h b/src/detect-http-trailer.h new file mode 100644 index 0000000000..30921cd869 --- /dev/null +++ b/src/detect-http-trailer.h @@ -0,0 +1,31 @@ +/* Copyright (C) 2026 Open Information Security Foundation + * + * You can copy, redistribute or modify this Program under the terms of + * the GNU General Public License version 2 as published by the Free + * Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * version 2 along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA + * 02110-1301, USA. + */ + +/** + * \file + * + * \author Victor Julien + */ + +#ifndef SURICATA_DETECT_HTTP_TRAILER_H +#define SURICATA_DETECT_HTTP_TRAILER_H + +void DetectHttpTrailerRegister(void); +void DetectHttpTrailerNamesRegister(void); +void DetectHttpTrailerRawRegister(void); + +#endif /* SURICATA_DETECT_HTTP_TRAILER_H */ diff --git a/src/tests/detect-http-header.c b/src/tests/detect-http-header.c index 7aa6ddae98..e9169eef21 100644 --- a/src/tests/detect-http-header.c +++ b/src/tests/detect-http-header.c @@ -4384,9 +4384,9 @@ static int DetectEngineHttpHeaderTest32(void) de_ctx->flags |= DE_QUIET; - de_ctx->sig_list = SigInit(de_ctx,"alert http any any -> any any " - "(content:\"Dummy\"; http_header; " - "sid:1;)"); + /* "Dummy" is a trailer field: the header buffer must not see it. */ + de_ctx->sig_list = SigInit(de_ctx, "alert http any any -> any any " + "(content:\"Dummy\"; http_header; sid:1;)"); if (de_ctx->sig_list == NULL) goto end; @@ -4411,8 +4411,8 @@ static int DetectEngineHttpHeaderTest32(void) /* do detect */ SigMatchSignatures(&th_v, de_ctx, det_ctx, p1); - if (!(PacketAlertCheck(p1, 1))) { - printf("sid 1 didn't match but should have: "); + if (PacketAlertCheck(p1, 1)) { + printf("sid 1 matched but should not have: "); goto end; } @@ -4493,9 +4493,9 @@ static int DetectEngineHttpHeaderTest33(void) FAIL_IF(de_ctx == NULL); de_ctx->flags |= DE_QUIET; - de_ctx->sig_list = SigInit(de_ctx,"alert http any any -> any any " - "(content:\"Dummy\"; http_header; " - "sid:1;)"); + /* same with the trailer arriving in a second packet */ + de_ctx->sig_list = SigInit(de_ctx, "alert http any any -> any any " + "(content:\"Dummy\"; http_header; sid:1;)"); FAIL_IF(de_ctx->sig_list == NULL); SigGroupBuild(de_ctx); @@ -4520,7 +4520,7 @@ static int DetectEngineHttpHeaderTest33(void) /* do detect */ SigMatchSignatures(&th_v, de_ctx, det_ctx, p2); - FAIL_IF(!PacketAlertCheck(p2, 1)); + FAIL_IF(PacketAlertCheck(p2, 1)); /* trailer bytes are not header bytes */ AppLayerParserThreadCtxFree(alp_tctx); DetectEngineThreadCtxDeinit(&th_v, (void *)det_ctx); @@ -4602,9 +4602,10 @@ static int DetectEngineHttpHeaderTest34(void) FAIL_IF(de_ctx == NULL); de_ctx->flags |= DE_QUIET; - de_ctx->sig_list = SigInit(de_ctx,"alert http any any -> any any " - "(content:\"Dummy\"; http_header; content:\"Header2\"; http_header; within:8; " - "sid:1;)"); + de_ctx->sig_list = SigInit(de_ctx, + "alert http any any -> any any " + "(content:\"Dummy\"; http_header; content:\"Header2\"; http_header; within:8; " + "sid:1;)"); FAIL_IF(de_ctx->sig_list == NULL); SigGroupBuild(de_ctx); @@ -4635,7 +4636,7 @@ static int DetectEngineHttpHeaderTest34(void) /* do detect */ SigMatchSignatures(&th_v, de_ctx, det_ctx, p3); - FAIL_IF(!PacketAlertCheck(p3, 1)); /* should match in trailer */ + FAIL_IF(PacketAlertCheck(p3, 1)); /* neither trailer field is header data */ AppLayerParserThreadCtxFree(alp_tctx); DetectEngineThreadCtxDeinit(&th_v, (void *)det_ctx); @@ -4718,9 +4719,10 @@ static int DetectEngineHttpHeaderTest35(void) FAIL_IF(de_ctx == NULL); de_ctx->flags |= DE_QUIET; - de_ctx->sig_list = SigInit(de_ctx,"alert http any any -> any any " - "(content:\"Dummy\"; http_header; fast_pattern; content:\"Header2\"; http_header; within:8; " - "sid:1;)"); + de_ctx->sig_list = SigInit(de_ctx, + "alert http any any -> any any " + "(content:\"Dummy\"; http_header; fast_pattern; content:\"Header2\"; http_header; " + "within:8; sid:1;)"); FAIL_IF(de_ctx->sig_list == NULL); SigGroupBuild(de_ctx); @@ -4751,7 +4753,7 @@ static int DetectEngineHttpHeaderTest35(void) /* do detect */ SigMatchSignatures(&th_v, de_ctx, det_ctx, p3); - FAIL_IF(!PacketAlertCheck(p3, 1)); /* should match in trailer */ + FAIL_IF(PacketAlertCheck(p3, 1)); /* same with a fast pattern queued */ AppLayerParserThreadCtxFree(alp_tctx); DetectEngineThreadCtxDeinit(&th_v, (void *)det_ctx); diff --git a/src/tests/detect-http-raw-header.c b/src/tests/detect-http-raw-header.c index 13e35d1c94..7c856cdddd 100644 --- a/src/tests/detect-http-raw-header.c +++ b/src/tests/detect-http-raw-header.c @@ -2188,7 +2188,7 @@ static int DetectEngineHttpRawHeaderTest31(void) FAIL_IF_NULL(http_state); SigMatchSignatures(&th_v, de_ctx, det_ctx, p1); - FAIL_IF(!(PacketAlertCheck(p1, 1))); + FAIL_IF(PacketAlertCheck(p1, 1)); /* trailer bytes are not in the raw header block */ UTHFreePackets(&p1, 1); FLOW_DESTROY(&f); @@ -2278,7 +2278,7 @@ static int DetectEngineHttpRawHeaderTest32(void) FAIL_IF_NOT(r == 0); SigMatchSignatures(&th_v, de_ctx, det_ctx, p2); - FAIL_IF(!PacketAlertCheck(p2, 1)); + FAIL_IF(PacketAlertCheck(p2, 1)); /* and not when they arrive in a later packet */ UTHFreePackets(&p1, 1); UTHFreePackets(&p2, 1); diff --git a/src/tests/detect-http-trailer.c b/src/tests/detect-http-trailer.c new file mode 100644 index 0000000000..256d753d69 --- /dev/null +++ b/src/tests/detect-http-trailer.c @@ -0,0 +1,195 @@ +/* Copyright (C) 2025 Open Information Security Foundation + * + * You can copy, redistribute or modify this Program under the terms of + * the GNU General Public License version 2 as published by the Free + * Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +/** + * \file + * + * Tests for the http.trailer buffers. The peer asserts, that header buffers do not see + * this data, are in tests/detect-http-header.c and tests/detect-http-raw-header.c. + */ + +#include "../suricata-common.h" +#include "../flow.h" +#include "../flow-util.h" +#include "../app-layer.h" +#include "../app-layer-parser.h" +#include "../app-layer-htp.h" +#include "../detect.h" +#include "../detect-parse.h" +#include "../detect-engine.h" +#include "../detect-engine-alert.h" +#include "../detect-engine-build.h" +#include "../stream-tcp.h" +#include "../counters.h" +#include "../util-unittest.h" +#include "../util-unittest-helper.h" + +/* chunked request, two trailer fields */ +static uint8_t trailer_test_buf[] = "GET /index.html HTTP/1.0\r\n" + "host: boom\r\n" + "Transfer-Encoding: chunked\r\n" + "\r\n" + "13\r\n" + "This is dummy body1\r\n" + "0\r\n" + "Dummy-Header: kaboom\r\n" + "Header2: value2\r\n" + "\r\n"; + +/* same message with an empty trailer section */ +static uint8_t trailer_test_buf_nofields[] = "GET /index.html HTTP/1.0\r\n" + "host: boom\r\n" + "Transfer-Encoding: chunked\r\n" + "\r\n" + "13\r\n" + "This is dummy body1\r\n" + "0\r\n" + "\r\n"; + +/** + * \brief Run one rule over a to-server stream and check whether it alerted. + * + * The whole transaction is handed to the parser in one go, so trailer state is + * reached during the parse and detection runs on the following packet. + */ +static int DetectHttpTrailerRun(const char *sig, uint8_t *buf, uint32_t buf_len, bool expect) +{ + TcpSession ssn; + Packet *p = NULL; + ThreadVars th_v; + DetectEngineCtx *de_ctx = NULL; + DetectEngineThreadCtx *det_ctx = NULL; + Flow f; + int result = 0; + AppLayerParserThreadCtx *alp_tctx = AppLayerParserThreadCtxAlloc(); + + memset(&th_v, 0, sizeof(th_v)); + StatsThreadInit(&th_v.stats); + memset(&f, 0, sizeof(f)); + memset(&ssn, 0, sizeof(ssn)); + + p = UTHBuildPacket(NULL, 0, IPPROTO_TCP); + if (p == NULL) + goto end; + if (alp_tctx == NULL) + goto end; + + FLOW_INITIALIZE(&f); + f.protoctx = (void *)&ssn; + f.proto = IPPROTO_TCP; + f.flags |= FLOW_IPV4; + p->flow = &f; + p->flowflags |= FLOW_PKT_TOSERVER; + p->flowflags |= FLOW_PKT_ESTABLISHED; + p->flags |= PKT_HAS_FLOW | PKT_STREAM_EST; + f.alproto = ALPROTO_HTTP1; + + StreamTcpInitConfig(true); + + de_ctx = DetectEngineCtxInit(); + FAIL_IF_NULL(de_ctx); + de_ctx->flags |= DE_QUIET; + de_ctx->sig_list = SigInit(de_ctx, sig); + if (de_ctx->sig_list == NULL) + goto end; + + SigGroupBuild(de_ctx); + DetectEngineThreadCtxInit(&th_v, (void *)de_ctx, (void *)&det_ctx); + + FAIL_IF(AppLayerParserParse(NULL, alp_tctx, &f, ALPROTO_HTTP1, STREAM_TOSERVER, buf, buf_len) != + 0); + FAIL_IF_NULL(f.alstate); + + SigMatchSignatures(&th_v, de_ctx, det_ctx, p); + if (expect) + FAIL_IF(!PacketAlertCheck(p, 1)); + else + FAIL_IF(PacketAlertCheck(p, 1)); + + result = 1; +end: + if (alp_tctx != NULL) + AppLayerParserThreadCtxFree(alp_tctx); + if (det_ctx != NULL) + DetectEngineThreadCtxDeinit(&th_v, (void *)det_ctx); + if (de_ctx != NULL) + DetectEngineCtxFree(de_ctx); + StreamTcpFreeConfig(true); + FLOW_DESTROY(&f); + UTHFreePackets(&p, 1); + StatsThreadCleanup(&th_v.stats); + return result; +} + +/** \test a trailer field matches on http.trailer */ +static int DetectHttpTrailerTest01(void) +{ + return DetectHttpTrailerRun("alert http any any -> any any (flow:to_server; http.trailer; " + "content:\"Dummy-Header\"; sid:1;)", + trailer_test_buf, sizeof(trailer_test_buf) - 1, true); +} + +/** \test the raw buffer holds the field block as sent: name, value, order */ +static int DetectHttpTrailerTest02(void) +{ + return DetectHttpTrailerRun("alert http any any -> any any (flow:to_server; http.trailer.raw; " + "content:\"Dummy-Header: kaboom|0d 0a|Header2\"; sid:1;)", + trailer_test_buf, sizeof(trailer_test_buf) - 1, true); +} + +/** \test http.trailer_names carries the field names */ +static int DetectHttpTrailerTest03(void) +{ + return DetectHttpTrailerRun( + "alert http any any -> any any (flow:to_server; http.trailer_names; " + "content:\"Header2\"; sid:1;)", + trailer_test_buf, sizeof(trailer_test_buf) - 1, true); +} + +/** \test ... and not the values */ +static int DetectHttpTrailerTest04(void) +{ + return DetectHttpTrailerRun( + "alert http any any -> any any (flow:to_server; http.trailer_names; " + "content:\"kaboom\"; sid:1;)", + trailer_test_buf, sizeof(trailer_test_buf) - 1, false); +} + +/** \test one rule can use a header buffer and a trailer buffer */ +static int DetectHttpTrailerTest05(void) +{ + return DetectHttpTrailerRun("alert http any any -> any any (flow:to_server; http.header; " + "content:\"host: boom\"; http.trailer; content:\"Header2\"; " + "sid:1;)", + trailer_test_buf, sizeof(trailer_test_buf) - 1, true); +} + +/** \test a trailer section without fields has nothing to match */ +static int DetectHttpTrailerTest06(void) +{ + return DetectHttpTrailerRun("alert http any any -> any any (flow:to_server; http.trailer; " + "content:\"Dummy-Header\"; sid:1;)", + trailer_test_buf_nofields, sizeof(trailer_test_buf_nofields) - 1, false); +} + +void DetectHttpTrailerRegisterTests(void) +{ + UtRegisterTest("DetectHttpTrailerTest01", DetectHttpTrailerTest01); + UtRegisterTest("DetectHttpTrailerTest02", DetectHttpTrailerTest02); + UtRegisterTest("DetectHttpTrailerTest03", DetectHttpTrailerTest03); + UtRegisterTest("DetectHttpTrailerTest04", DetectHttpTrailerTest04); + UtRegisterTest("DetectHttpTrailerTest05", DetectHttpTrailerTest05); + UtRegisterTest("DetectHttpTrailerTest06", DetectHttpTrailerTest06); +} diff --git a/src/tests/detect.c b/src/tests/detect.c index 43a1cef2b0..317eb43613 100644 --- a/src/tests/detect.c +++ b/src/tests/detect.c @@ -4441,6 +4441,12 @@ static int SigTestStateHookBuffer01(void) "app-layer-event:http.request_header_host_invalid; sid:6;)"); FAIL_IF_NULL(s); + /* a buffer completing in the hooked state itself needs no relaxation */ + s = DetectEngineAppendSig(de_ctx, + "alert http1:request_trailer any any -> any any (flow:to_server; http.trailer; " + "content:\"X-Trailed\"; sid:7;)"); + FAIL_IF_NULL(s); + DetectEngineCtxFree(de_ctx); PASS; } From 4bd1ffd3aedbd4b73112744a26a119ed32e1fb5a Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Fri, 9 Oct 2026 21:55:09 +0000 Subject: [PATCH 5/9] doc: describe the trailer buffers And say on the header buffers that the trailer section is not part of them. That is a change in behaviour for chunked traffic carrying trailers: a rule that happened to match trailer content through http.header, http.header.raw, http.header_names or a field keyword now needs the matching trailer buffer. Ticket: #8256. --- doc/userguide/lua/libs/http.rst | 5 ++ doc/userguide/rules/http-keywords.rst | 83 +++++++++++++++++++++++++++ doc/userguide/upgrade.rst | 25 ++++++++ 3 files changed, 113 insertions(+) diff --git a/doc/userguide/lua/libs/http.rst b/doc/userguide/lua/libs/http.rst index e73822bcdf..169020f77a 100644 --- a/doc/userguide/lua/libs/http.rst +++ b/doc/userguide/lua/libs/http.rst @@ -43,6 +43,11 @@ HTTP is transaction based, and the current transaction must be obtained before u All other functions are methods on the transaction table. +.. note:: + + The header functions below cover the header section only. HTTP trailer fields are not + reachable from Lua; match them with the ``http.trailer`` buffers in a rule instead. + Transaction Methods ~~~~~~~~~~~~~~~~~~~ diff --git a/doc/userguide/rules/http-keywords.rst b/doc/userguide/rules/http-keywords.rst index 1dd0a1c91c..63e296b536 100644 --- a/doc/userguide/rules/http-keywords.rst +++ b/doc/userguide/rules/http-keywords.rst @@ -1088,6 +1088,9 @@ removed. To match on non-normalized header data, use the :ref:`http.header.raw` keyword. +The buffer covers the header section of the message. When a chunked body ends with +trailer fields, those are not part of ``http.header``; use :ref:`http.trailer` for them. + Normalization Example:: GET /index.html HTTP/1.1 @@ -1138,6 +1141,9 @@ http.header.raw The ``http.header.raw`` buffer matches on HTTP header content but does not have any normalization performed on the buffer contents (see :ref:`http.header`) +Like ``http.header`` it covers the header section only; the trailer section of a +chunked body is available through :ref:`http.trailer.raw`. + Abnormal HTTP Header Example:: GET /index.html HTTP/1.1 @@ -1239,6 +1245,83 @@ after ``User-Agent`` but not necessarily directly after. .. note:: ``http.header_names`` can have additional formatting/normalization applied to buffer contents, see :ref:`http.normalization` for additional details. +.. note:: Names used in the trailer section of a chunked body are not part of this + buffer, see :ref:`http.trailer_names`. + +.. _http.trailer: + +http.trailer +------------ + +The ``http.trailer`` keyword matches on the trailer fields of an HTTP/1 message. A +chunked body may end with a second block of fields, the trailers, which are parsed +once the body is complete. They are deliberately kept out of :ref:`http.header`, +:ref:`http.header.raw` and :ref:`http.header_names` as well as out of the field +keywords such as ``http.user_agent``. + +Fields are rendered as ``Name: value``, separated by \r\n, like ``http.header`` does. +In HTTP/1 repeated names are combined with a comma, as in the header section. In HTTP/2 +the trailer block is rendered as one line per entry, so a repeated name appears twice. + +Example HTTP Request:: + + POST /upload HTTP/1.1 + Host: suricata.io + Trailer: Server-Timing + Content-Type: application/x-www-form-urlencoded + Transfer-Encoding: chunked + + 5 + hello + 0 + Server-Timing: total;dur=120 + +.. container:: example-rule + + alert http $HOME_NET any -> $EXTERNAL_NET any (msg:"HTTP Trailer Example"; \ + flow:established,to_server; :example-rule-options:`http.trailer; content:"Server-Timing|3a 20|"; \ + content:"dur=120|0d 0a|";` classtype:bad-unknown; sid:117; rev:1;) + +.. note:: Since the trailer section follows the body, a rule that only matches here + alerts when the last chunk is parsed, not when the headers are. To run a rule at + that point regardless of what it matches, use the ``http1:request_trailer`` or + ``http1:response_trailer`` state hook. + +.. note:: HTTP/2 does not have a trailer section; its trailing HEADERS frames are + currently part of :ref:`http.header`. + +.. _http.trailer_names: + +http.trailer_names +------------------ + +The ``http.trailer_names`` keyword matches on the names of the trailer fields of an +HTTP/1 message. The buffer has the same shape as :ref:`http.header_names`: it starts +with a \r\n, lists one name per line and ends with an extra \r\n. + +.. container:: example-rule + + alert http $HOME_NET any -> $EXTERNAL_NET any (msg:"HTTP Trailer Names Example"; \ + flow:established,to_server; :example-rule-options:`http.trailer_names; \ + content:"|0d 0a|Server-Timing|0d 0a 0d 0a|";` classtype:bad-unknown; sid:118; rev:1;) + +.. _http.trailer.raw: + +http.trailer.raw +---------------- + +The ``http.trailer.raw`` buffer matches on the trailer section as it appears in the +stream, without the normalization ``http.trailer`` applies. Folded field values and +repeated names are therefore not combined. A chunked message whose +trailer section carries no fields holds only the framing that ends the body, and is not a +buffer: a rule on ``http.trailer.raw`` does not match it. + +.. container:: example-rule + + alert http $HOME_NET any -> $EXTERNAL_NET any (msg:"HTTP Trailer Raw Example"; \ + flow:established,to_server; :example-rule-options:`http.trailer.raw; \ + content:"Server-Timing|3a |";` classtype:bad-unknown; sid:119; rev:1;) + .. _http.protocol: http.protocol diff --git a/doc/userguide/upgrade.rst b/doc/userguide/upgrade.rst index 1673d0db8a..0757ce0e08 100644 --- a/doc/userguide/upgrade.rst +++ b/doc/userguide/upgrade.rst @@ -46,6 +46,31 @@ Major Changes - Default value for ``stream.reassembly.depth`` when the value is not specified in suricata.yaml is now 1 MiB instead of 0/unlimited. - LLMNR protocol parser, logger and sticky buffers are implemented. +- A rule with an app-layer state hook can now use buffers that complete in an earlier + state: a rule hooked at ``http1:request_trailer`` matching on ``http.header`` loads, where + it was rejected before with "engine progress value doesn't match hook". The buffer is + inspected from its own state on, so the hook limits which buffers a rule may use rather + than when the data is looked at. Buffers that complete after the hooked state are still + rejected. A firewall rule - one loaded through ``firewall.rule-files`` - whose exact state + hook sits above the state where its buffer completes is refused at load: it is only + evaluated at its hook, so it could never match. The `` Date: Sat, 10 Oct 2026 04:42:29 +0000 Subject: [PATCH 6/9] app-layer/http2: add a transaction state for the trailer block An HTTP/2 stream puts its trailers in a HEADERS block after the body. There was no state for it, so detection had nothing to hook: the block was merged into the header buffers, where the header state had already been declared complete. gRPC streams, which always end with such a block, could not be matched at all. Add HTTP2ProgTrailers between the data and the closed state, and mark the frames of a trailer block. Marking keys on the header section having completed in that direction, not on the progress value and not on body data: RFC 9113 permits a trailer section behind a message that sent no DATA frames at all. A second block that only follows an informational response is not a trailer, so a to-client block completes the header section only with a non-1xx :status, and a CONTINUATION takes the classification of the block it continues, not the state the direction reached since. A state inserted into a shipped protocol defaults to drop:flow in firewall mode, which would block every stream that has trailers the moment this builds. So a parser can now describe its sub-states fully - completion state and the built-in policy of each state - instead of AppLayerParserGetSubStateCompletion guessing from hard coded numbers. The trailer states register accept:hook, so behaviour only changes for deployments that opt in; the generic drop:flow still applies to the states the protocol has always had. Ticket: #8256. --- doc/userguide/firewall/firewall-design.rst | 4 +- doc/userguide/upgrade.rst | 6 + rust/src/http2/detect.rs | 3 + rust/src/http2/http2.rs | 497 ++++++++++++++++++++- rust/sys/src/sys.rs | 9 +- src/app-layer-parser.c | 45 +- src/app-layer-parser.h | 14 +- src/detect-parse.c | 22 +- 8 files changed, 581 insertions(+), 19 deletions(-) diff --git a/doc/userguide/firewall/firewall-design.rst b/doc/userguide/firewall/firewall-design.rst index 681c51e57b..5c2d78bef7 100644 --- a/doc/userguide/firewall/firewall-design.rst +++ b/doc/userguide/firewall/firewall-design.rst @@ -374,7 +374,9 @@ Default policies Each hook has a default policy applied to traffic that no firewall rule handled. By default ``packet.filter`` enforces ``drop:packet``, ``packet.pre-flow`` and ``packet.pre-stream`` enforce ``accept:hook``, and every ``app`` hook enforces -``drop:flow``. +``drop:flow``. A parser can ask for a different built-in for one of its states, which +is what the HTTP/2 trailer states do (``accept:hook``): a state added after the +protocol shipped must not start dropping flows just because it is reached. Defaults are configured in the ``firewall.policies`` block. A ``default-policy`` for any hook may be given at several levels and the most specific present diff --git a/doc/userguide/upgrade.rst b/doc/userguide/upgrade.rst index 0757ce0e08..3b1850297f 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. +- 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 + example ``http2:stream:request_complete<``) now cover one more state. The new states + start out with the built-in default policy ``accept:hook`` instead of ``drop:flow``, + so streams carrying trailers are not blocked before a policy is chosen. - ``pgsql`` is no longer enabled by default. Previously, if the ``app-layer.protocols.pgsql`` section was absent from suricata.yaml, the parser would be enabled. It's now disabled by default. Simply enabling its EVE diff --git a/rust/src/http2/detect.rs b/rust/src/http2/detect.rs index 689d2144c5..b99ac91bd2 100644 --- a/rust/src/http2/detect.rs +++ b/rust/src/http2/detect.rs @@ -1003,6 +1003,7 @@ fn http2_tx_set_header(state: &mut HTTP2State, name: &[u8], input: &[u8]) { .find_or_create_tx(&head, &txdata, Direction::ToServer) .unwrap(); tx.frames_ts.push(HTTP2Frame { + is_trailer: false, header: head, data: txdata, }); @@ -1050,6 +1051,7 @@ fn http2_tx_set_settings(state: &mut HTTP2State, input: &[u8]) { .find_or_create_tx(&head, &txdata, Direction::ToServer) .unwrap(); tx.frames_ts.push(HTTP2Frame { + is_trailer: false, header: head, data: txdata, }); @@ -1166,6 +1168,7 @@ mod tests { }; let txdata = HTTP2FrameTypeData::HEADERS(hs); tx.frames_ts.push(HTTP2Frame { + is_trailer: false, header: head, data: txdata, }); diff --git a/rust/src/http2/http2.rs b/rust/src/http2/http2.rs index de51ff4aab..2ec9c65fe3 100644 --- a/rust/src/http2/http2.rs +++ b/rust/src/http2/http2.rs @@ -140,8 +140,9 @@ pub enum HTTP2TxProgress { HTTP2ProgStarted = 0, HTTP2ProgHeaders = 1, HTTP2ProgData = 2, - HTTP2ProgClosed = 3, - HTTP2ProgComplete = 4, // complete is a pseudo state set only when both sides are closed + HTTP2ProgTrailers = 3, + HTTP2ProgClosed = 4, + HTTP2ProgComplete = 5, // complete is a pseudo state set only when both sides are closed } #[repr(u8)] @@ -156,6 +157,8 @@ pub enum HTTP2TxGlobalProgress { pub struct HTTP2Frame { pub header: parser::HTTP2FrameHeader, pub data: HTTP2FrameTypeData, + /// the blocks of this frame belong to a trailer section, not to the header section + pub is_trailer: bool, } #[derive(Debug, Default)] @@ -292,6 +295,14 @@ pub struct HTTP2Transaction { pub frames_tc: Vec, pub frames_ts: Vec, + /// header section complete per direction: END_HEADERS seen, for a response only past a + /// non-1xx status; a later header block in that direction is a trailer section + headers_done_ts: bool, + headers_done_tc: bool, + final_status_tc: bool, + // whether the header block currently open in each direction is a trailer block + open_block_trailer_ts: Option, + open_block_trailer_tc: Option, decoder: decompression::HTTP2Decoder, pub file_range: *mut HttpRangeContainerBlock, @@ -328,6 +339,11 @@ impl HTTP2Transaction { to_drop: false, frames_tc: Vec::new(), frames_ts: Vec::new(), + headers_done_ts: false, + headers_done_tc: false, + final_status_tc: false, + open_block_trailer_ts: None, + open_block_trailer_tc: None, decoder: decompression::HTTP2Decoder::new(), file_range: std::ptr::null_mut(), tx_data: AppLayerTxData::new(), @@ -519,11 +535,90 @@ impl HTTP2Transaction { return Ok(()); } + /// A header block is trailers once the header section of the message has completed in + /// that direction, which is sequence, not body: RFC 9113 allows a trailer section after a + /// message with no DATA frames at all. A stream that opens with DATA still has no completed + /// header section, and an informational response does not complete one either, so neither + /// can turn a following block into trailers. + fn frame_is_trailer(&self, data: &HTTP2FrameTypeData, dir: Direction) -> bool { + match data { + HTTP2FrameTypeData::HEADERS(_) => { + if dir == Direction::ToServer { + self.headers_done_ts + } else { + self.headers_done_tc + } + } + // A CONTINUATION belongs to the block that opened it, so it inherits that + // block's classification instead of the state the direction reached since. + HTTP2FrameTypeData::CONTINUATION(_) => { + if dir == Direction::ToServer { + self.open_block_trailer_ts.unwrap_or(false) + } else { + self.open_block_trailer_tc.unwrap_or(false) + } + } + _ => false, + } + } + + /// True once a response block carries a non-1xx :status, i.e. the header section of the + /// message proper has been seen. + fn blocks_have_final_status(blocks: &[parser::HTTP2FrameHeaderBlock]) -> bool { + blocks + .iter() + .any(|b| b.name.as_slice() == b":status" && b.value.first() != Some(&b'1')) + } + fn handle_frame( &mut self, header: &parser::HTTP2FrameHeader, data: &HTTP2FrameTypeData, dir: Direction, + is_trailer: bool, ) -> Option> { //handle child_stream_id changes let mut r = None; + let end_headers = header.flags & parser::HTTP2_FLAG_HEADER_END_HEADERS != 0; + match data { + HTTP2FrameTypeData::HEADERS(_) | HTTP2FrameTypeData::PUSHPROMISE(_) => { + let open = if dir == Direction::ToServer { + &mut self.open_block_trailer_ts + } else { + &mut self.open_block_trailer_tc + }; + *open = if end_headers { None } else { Some(is_trailer) }; + } + HTTP2FrameTypeData::CONTINUATION(_) if end_headers => { + let open = if dir == Direction::ToServer { + &mut self.open_block_trailer_ts + } else { + &mut self.open_block_trailer_tc + }; + *open = None; + } + _ => {} + } + if !is_trailer + && matches!( + data, + HTTP2FrameTypeData::HEADERS(_) | HTTP2FrameTypeData::CONTINUATION(_) + ) + { + let blocks = match data { + HTTP2FrameTypeData::HEADERS(hs) => &hs.blocks, + HTTP2FrameTypeData::CONTINUATION(hs) => &hs.blocks, + _ => unreachable!(), + }; + if dir == Direction::ToClient && Self::blocks_have_final_status(blocks) { + self.final_status_tc = true; + } + if header.flags & parser::HTTP2_FLAG_HEADER_END_HEADERS != 0 { + if dir == Direction::ToServer { + self.headers_done_ts = true; + } else { + // an informational response has not finished the header section yet + self.headers_done_tc = self.final_status_tc; + } + } + } match data { HTTP2FrameTypeData::PUSHPROMISE(hs) => { if dir == Direction::ToClient { @@ -559,6 +654,20 @@ impl HTTP2Transaction { _ => {} } if let HTTP2Progress::STREAM(ref mut stream_tx) = self.progress { + // a trailer section is complete once its last block frame is in; the end of stream + // handling below takes precedence, it moves further. Use the caller's value: the + // arming above has already set headers_done for this frame, so re-evaluating here + // would turn the block that opens a stream into trailers. + if is_trailer && header.flags & parser::HTTP2_FLAG_HEADER_END_HEADERS != 0 { + let st = if dir == Direction::ToServer { + &mut stream_tx.progress_ts + } else { + &mut stream_tx.progress_tc + }; + if *st < HTTP2TxProgress::HTTP2ProgTrailers { + *st = HTTP2TxProgress::HTTP2ProgTrailers; + } + } //handle closing state changes let state = if dir == Direction::ToServer { &mut stream_tx.progress_ts @@ -1487,7 +1596,10 @@ impl HTTP2State { continue; // skip this frame, continue parsing the next } - if let Some(doh_req_buf) = tx.handle_frame(&head, &txdata, dir) { + // classify before handling: handling moves the state this frame is + // judged against + let is_trailer = tx.frame_is_trailer(&txdata, dir); + if let Some(doh_req_buf) = tx.handle_frame(&head, &txdata, dir, is_trailer) { if let Ok(mut dtx) = dns_parse_request(&doh_req_buf, &DnsVariant::Dns) { dtx.id = 1; unsafe { @@ -1521,6 +1633,7 @@ impl HTTP2State { h2frames.push(HTTP2Frame { header: head, data: txdata, + is_trailer, }); } else { tx.tx_data.set_event(HTTP2Event::TooManyFrames as u8); @@ -1752,6 +1865,19 @@ unsafe extern "C" fn http2_state_get_tx_count(state: *mut std::os::raw::c_void) return state.tx_id; } +// the trailer state was added after HTTP/2 support shipped: default to accept for the hook +// so flows with trailers are not dropped on upgrade, until a policy is configured explicitly +const HTTP2_TRAILERS_POLICY: &[u8] = b"accept:hook\0"; + +unsafe extern "C" fn http2_tx_state_get_policy( + state: std::os::raw::c_int, _direction: u8, +) -> *const std::os::raw::c_char { + if HTTP2TxProgress::from_u8(state as u8) == Some(HTTP2TxProgress::HTTP2ProgTrailers) { + return HTTP2_TRAILERS_POLICY.as_ptr() as *const std::os::raw::c_char; + } + std::ptr::null() +} + unsafe extern "C" fn http2_tx_get_alstate_progress( tx: *mut std::os::raw::c_void, direction: u8, ) -> std::os::raw::c_int { @@ -1871,12 +1997,16 @@ 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, + Some(http2_tx_state_get_policy), ); SCAppLayerParserRegisterGetTxSubStateFuncs( ALPROTO_HTTP2, HTTP2TxType::HTTP2TxTypeGlobal as u8, Some(HTTP2TxGlobalProgress::ffi_id_from_name), Some(HTTP2TxGlobalProgress::ffi_name_from_id), + HTTP2TxGlobalProgress::HTTP2ProgGlobalComplete as u8, + None, ); SCLogDebug!("Rust http2 parser registered."); @@ -1906,3 +2036,364 @@ pub unsafe extern "C" fn SCRegisterHttp2Parser() { SCLogNotice!("Protocol detector and parser disabled for DOH2."); } } + +#[cfg(test)] +mod tests { + use super::*; + + fn hdr(ftype: u8, flags: u8) -> parser::HTTP2FrameHeader { + parser::HTTP2FrameHeader { + length: 0, + ftype, + flags, + reserved: 0, + stream_id: 1, + } + } + + fn blocks(pairs: &[(&[u8], &[u8])]) -> Vec { + pairs + .iter() + .map(|(n, v)| parser::HTTP2FrameHeaderBlock { + name: std::rc::Rc::new(n.to_vec()), + value: std::rc::Rc::new(v.to_vec()), + error: parser::HTTP2HeaderDecodeStatus::HTTP2HeaderDecodeSuccess, + sizeupdate: 0, + }) + .collect() + } + + fn headers(pairs: &[(&[u8], &[u8])]) -> HTTP2FrameTypeData { + HTTP2FrameTypeData::HEADERS(parser::HTTP2FrameHeaders { + padlength: None, + priority: None, + blocks: blocks(pairs), + }) + } + + fn progress_tc(tx: &HTTP2Transaction) -> HTTP2TxProgress { + match &tx.progress { + HTTP2Progress::STREAM(s) => s.progress_tc, + _ => panic!("not a stream tx"), + } + } + + fn progress_ts(tx: &HTTP2Transaction) -> HTTP2TxProgress { + match &tx.progress { + HTTP2Progress::STREAM(s) => s.progress_ts, + _ => panic!("not a stream tx"), + } + } + + /// A header block after the body is a trailer section: it is marked as such and it + /// moves the transaction to the trailer state, where the trailer buffers complete. + #[test] + fn http2_trailers_get_their_own_state() { + let mut tx = HTTP2Transaction::new(); + let end = parser::HTTP2_FLAG_HEADER_END_HEADERS; + let hs = headers(&[ + (&b":method"[..], &b"POST"[..]), + (&b"host"[..], &b"a.example"[..]), + ]); + let is_trailer = tx.frame_is_trailer(&hs, Direction::ToServer); + tx.handle_frame( + &hdr(parser::HTTP2FrameType::Headers as u8, end), + &hs, + Direction::ToServer, + is_trailer, + ); + // a further block is trailers already, body or no body + assert!(tx.frame_is_trailer(&hs, Direction::ToServer)); + assert_eq!(progress_ts(&tx), HTTP2TxProgress::HTTP2ProgHeaders); + + let is_trailer = tx.frame_is_trailer(&HTTP2FrameTypeData::DATA, Direction::ToServer); + tx.handle_frame( + &hdr(parser::HTTP2FrameType::Data as u8, 0), + &HTTP2FrameTypeData::DATA, + Direction::ToServer, + is_trailer, + ); + assert_eq!(progress_ts(&tx), HTTP2TxProgress::HTTP2ProgData); + + let tr = headers(&[(&b"x-checksum"[..], &b"deadbeef"[..])]); + let is_trailer = tx.frame_is_trailer(&tr, Direction::ToServer); + assert!(is_trailer); + tx.handle_frame( + &hdr( + parser::HTTP2FrameType::Headers as u8, + end | parser::HTTP2_FLAG_HEADER_EOS, + ), + &tr, + Direction::ToServer, + is_trailer, + ); + // end of stream wins over the trailer bump, the section is complete either way + assert_eq!(progress_ts(&tx), HTTP2TxProgress::HTTP2ProgClosed); + } + + /// RFC 9113 allows a trailer section behind a message that sent no DATA frames, so + /// classification must key on block sequence. Both directions are covered; an + /// informational response must not complete the header section, which the next case + /// in http2_intermediate_headers_are_not_trailers pins from the other side. + #[test] + fn http2_bodyless_trailers_are_trailers() { + let mut tx = HTTP2Transaction::new(); + let end = parser::HTTP2_FLAG_HEADER_END_HEADERS; + let eos = parser::HTTP2_FLAG_HEADER_EOS; + let hs = headers(&[ + (&b":method"[..], &b"POST"[..]), + (&b"host"[..], &b"a.example"[..]), + ]); + tx.handle_frame( + &hdr(parser::HTTP2FrameType::Headers as u8, end), + &hs, + Direction::ToServer, + false, + ); + let tr = headers(&[(&b"x-checksum"[..], &b"deadbeef"[..])]); + assert!(tx.frame_is_trailer(&tr, Direction::ToServer)); + tx.handle_frame( + &hdr(parser::HTTP2FrameType::Headers as u8, end | eos), + &tr, + Direction::ToServer, + true, + ); + + let mut tx = HTTP2Transaction::new(); + let hs = headers(&[(&b":status"[..], &b"200"[..])]); + tx.handle_frame( + &hdr(parser::HTTP2FrameType::Headers as u8, end), + &hs, + Direction::ToClient, + false, + ); + let tr = headers(&[(&b"x-checksum"[..], &b"deadbeef"[..])]); + assert!(tx.frame_is_trailer(&tr, Direction::ToClient)); + tx.handle_frame( + &hdr(parser::HTTP2FrameType::Headers as u8, end | eos), + &tr, + Direction::ToClient, + true, + ); + match &tx.progress { + HTTP2Progress::STREAM(s) => { + // end of stream takes precedence over the trailer bump, as elsewhere + assert!(matches!( + s.progress_tc, + HTTP2TxProgress::HTTP2ProgTrailers | HTTP2TxProgress::HTTP2ProgClosed + )); + } + _ => panic!("not a stream tx"), + } + } + + /// A 1xx block must not arm the trailer classifier for the final block that follows it + /// when no body came in between, which is the case DATA used to stand in for. + #[test] + fn http2_interim_then_final_blocks_are_not_trailers() { + let mut tx = HTTP2Transaction::new(); + let end = parser::HTTP2_FLAG_HEADER_END_HEADERS; + let i = headers(&[(&b":status"[..], &b"103"[..])]); + tx.handle_frame( + &hdr(parser::HTTP2FrameType::Headers as u8, end), + &i, + Direction::ToClient, + false, + ); + assert!(!tx.frame_is_trailer(&i, Direction::ToClient)); + let f = headers(&[(&b":status"[..], &b"200"[..])]); + assert!(!tx.frame_is_trailer(&f, Direction::ToClient)); + } + + /// A stream that opens with a DATA frame keeps its header block in the header section. + #[test] + fn http2_data_before_headers_is_not_a_trailer() { + let mut tx = HTTP2Transaction::new(); + let end = parser::HTTP2_FLAG_HEADER_END_HEADERS; + let is_trailer = tx.frame_is_trailer(&HTTP2FrameTypeData::DATA, Direction::ToServer); + tx.handle_frame( + &hdr(parser::HTTP2FrameType::Data as u8, 0), + &HTTP2FrameTypeData::DATA, + Direction::ToServer, + is_trailer, + ); + let hs = headers(&[ + (&b":method"[..], &b"POST"[..]), + (&b"host"[..], &b"a.example"[..]), + ]); + assert!(!tx.frame_is_trailer(&hs, Direction::ToServer)); + let is_trailer = tx.frame_is_trailer(&hs, Direction::ToServer); + tx.handle_frame( + &hdr(parser::HTTP2FrameType::Headers as u8, end), + &hs, + Direction::ToServer, + is_trailer, + ); + // the stray DATA frame ahead of it must not push this block into the trailer state + assert_ne!(progress_ts(&tx), HTTP2TxProgress::HTTP2ProgTrailers); + // body data behind the header section, then a block: that one is trailers + let is_trailer = tx.frame_is_trailer(&HTTP2FrameTypeData::DATA, Direction::ToServer); + tx.handle_frame( + &hdr(parser::HTTP2FrameType::Data as u8, 0), + &HTTP2FrameTypeData::DATA, + Direction::ToServer, + is_trailer, + ); + let tr = headers(&[(&b"x-checksum"[..], &b"deadbeef"[..])]); + let is_trailer = tx.frame_is_trailer(&tr, Direction::ToServer); + assert!(is_trailer); + tx.handle_frame( + &hdr(parser::HTTP2FrameType::Headers as u8, end), + &tr, + Direction::ToServer, + is_trailer, + ); + assert_eq!(progress_ts(&tx), HTTP2TxProgress::HTTP2ProgTrailers); + } + + /// Trailers without end-stream stay in the trailer state until the stream closes. + #[test] + fn http2_trailers_without_end_stream() { + let mut tx = HTTP2Transaction::new(); + let end = parser::HTTP2_FLAG_HEADER_END_HEADERS; + let hs = headers(&[(&b":method"[..], &b"POST"[..])]); + let is_trailer = tx.frame_is_trailer(&hs, Direction::ToServer); + tx.handle_frame( + &hdr(parser::HTTP2FrameType::Headers as u8, end), + &hs, + Direction::ToServer, + is_trailer, + ); + let is_trailer = tx.frame_is_trailer(&HTTP2FrameTypeData::DATA, Direction::ToServer); + tx.handle_frame( + &hdr(parser::HTTP2FrameType::Data as u8, 0), + &HTTP2FrameTypeData::DATA, + Direction::ToServer, + is_trailer, + ); + let tr = headers(&[(&b"x-checksum"[..], &b"1"[..])]); + let is_trailer = tx.frame_is_trailer(&tr, Direction::ToServer); + tx.handle_frame( + &hdr(parser::HTTP2FrameType::Headers as u8, end), + &tr, + Direction::ToServer, + is_trailer, + ); + assert_eq!(progress_ts(&tx), HTTP2TxProgress::HTTP2ProgTrailers); + } + + /// A CONTINUATION continues the block that opened it. A PUSH_PROMISE issued while the + /// response body is in flight must not have its fields read as the parent stream's + /// trailers, even though the parent has completed its header section. + #[test] + fn http2_push_promise_continuation_is_not_a_trailer() { + let mut tx = HTTP2Transaction::new(); + let end = parser::HTTP2_FLAG_HEADER_END_HEADERS; + tx.handle_frame( + &hdr(parser::HTTP2FrameType::Headers as u8, end), + &headers(&[(&b":status"[..], &b"200"[..])]), + Direction::ToClient, + false, + ); + tx.handle_frame( + &hdr(parser::HTTP2FrameType::Data as u8, 0), + &HTTP2FrameTypeData::DATA, + Direction::ToClient, + false, + ); + let pp = HTTP2FrameTypeData::PUSHPROMISE(parser::HTTP2FramePushPromise { + padlength: None, + reserved: 0, + stream_id: 2, + blocks: vec![], + }); + assert!(!tx.frame_is_trailer(&pp, Direction::ToClient)); + tx.handle_frame( + &hdr(parser::HTTP2FrameType::PushPromise as u8, 0), + &pp, + Direction::ToClient, + false, + ); + let c = HTTP2FrameTypeData::CONTINUATION(parser::HTTP2FrameContinuation { blocks: vec![] }); + assert!(!tx.frame_is_trailer(&c, Direction::ToClient)); + tx.handle_frame( + &hdr(parser::HTTP2FrameType::Continuation as u8, end), + &c, + Direction::ToClient, + false, + ); + assert_ne!(progress_tc(&tx), HTTP2TxProgress::HTTP2ProgTrailers); + } + + /// A trailer block spread over HEADERS plus CONTINUATION keeps its classification for + /// the whole block, and completes in the trailer state. + #[test] + fn http2_trailer_block_spanning_continuation() { + let mut tx = HTTP2Transaction::new(); + let end = parser::HTTP2_FLAG_HEADER_END_HEADERS; + tx.handle_frame( + &hdr(parser::HTTP2FrameType::Headers as u8, end), + &headers(&[(&b":method"[..], &b"POST"[..])]), + Direction::ToServer, + false, + ); + tx.handle_frame( + &hdr(parser::HTTP2FrameType::Data as u8, 0), + &HTTP2FrameTypeData::DATA, + Direction::ToServer, + false, + ); + let hs = headers(&[(&b"x-checksum"[..], &b"deadbeef"[..])]); + assert!(tx.frame_is_trailer(&hs, Direction::ToServer)); + tx.handle_frame( + &hdr(parser::HTTP2FrameType::Headers as u8, 0), + &hs, + Direction::ToServer, + true, + ); + let c = HTTP2FrameTypeData::CONTINUATION(parser::HTTP2FrameContinuation { blocks: vec![] }); + assert!(tx.frame_is_trailer(&c, Direction::ToServer)); + tx.handle_frame( + &hdr( + parser::HTTP2FrameType::Continuation as u8, + end | parser::HTTP2_FLAG_HEADER_EOS, + ), + &c, + Direction::ToServer, + true, + ); + } + + /// Repeated header blocks are not trailers when no body came in between, which is how + /// an HTTP/2 response sends 1xx before its final block. + #[test] + fn http2_intermediate_headers_are_not_trailers() { + let mut tx = HTTP2Transaction::new(); + let end = parser::HTTP2_FLAG_HEADER_END_HEADERS; + let h1 = headers(&[(&b":status"[..], &b"100"[..])]); + let is_trailer = tx.frame_is_trailer(&h1, Direction::ToClient); + tx.handle_frame( + &hdr(parser::HTTP2FrameType::Headers as u8, end), + &h1, + Direction::ToClient, + is_trailer, + ); + let h2 = headers(&[(&b":status"[..], &b"200"[..])]); + assert!(!tx.frame_is_trailer(&h2, Direction::ToClient)); + let is_trailer = tx.frame_is_trailer(&h2, Direction::ToClient); + tx.handle_frame( + &hdr( + parser::HTTP2FrameType::Headers as u8, + end | parser::HTTP2_FLAG_HEADER_EOS, + ), + &h2, + Direction::ToClient, + is_trailer, + ); + assert_eq!( + progress_ts(&tx), + HTTP2TxProgress::HTTP2ProgStarted, + "wrong direction must not move" + ); + } +} diff --git a/rust/sys/src/sys.rs b/rust/sys/src/sys.rs index 67097e5edb..a184852e36 100644 --- a/rust/sys/src/sys.rs +++ b/rust/sys/src/sys.rs @@ -1550,6 +1550,10 @@ pub type AppLayerParserGetStateIdByNameFn = ::std::option::Option< pub type AppLayerParserGetStateNameByIdFn = ::std::option::Option< unsafe extern "C" fn(id: ::std::os::raw::c_int, direction: u8) -> *const ::std::os::raw::c_char, >; +#[doc = " \\param id progress value id to get the default firewall policy for\n \\param direction STREAM_TOSERVER/STREAM_TOCLIENT\n \\retval policy string such as \"accept:hook\", NULL to use the generic default"] +pub type AppLayerParserGetStatePolicyByIdFn = ::std::option::Option< + unsafe extern "C" fn(id: ::std::os::raw::c_int, direction: u8) -> *const ::std::os::raw::c_char, +>; pub type AppLayerParserGetFrameIdByNameFn = ::std::option::Option< unsafe extern "C" fn(frame_name: *const ::std::os::raw::c_char) -> ::std::os::raw::c_int, >; @@ -1567,10 +1571,11 @@ extern "C" { pub fn SCAppLayerParserRegisterLogger(ipproto: u8, alproto: AppProto); } extern "C" { - #[doc = " \\brief register state<>name funcs for a substate"] + #[doc = " \\brief register state<>name funcs, the completion state and an optional\n per-state default firewall policy for a substate"] pub fn SCAppLayerParserRegisterGetTxSubStateFuncs( alproto: AppProto, sub_state: u8, GetIdByNameFunc: AppLayerParserGetStateIdByNameFn, - GetNameByIdFunc: AppLayerParserGetStateNameByIdFn, + GetNameByIdFunc: AppLayerParserGetStateNameByIdFn, completion: u8, + GetPolicyFunc: AppLayerParserGetStatePolicyByIdFn, ); } extern "C" { diff --git a/src/app-layer-parser.c b/src/app-layer-parser.c index dab0b17f53..d52513bc02 100644 --- a/src/app-layer-parser.c +++ b/src/app-layer-parser.c @@ -77,6 +77,8 @@ struct AppLayerParserSubStateMapping { uint8_t sub_state; AppLayerParserGetStateIdByNameFn GetStateIdByName; AppLayerParserGetStateNameByIdFn GetStateNameById; + AppLayerParserGetStatePolicyByIdFn GetStatePolicy; + uint8_t completion; struct AppLayerParserSubStateMapping *next; }; @@ -618,7 +620,8 @@ 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, + AppLayerParserGetStatePolicyByIdFn GetPolicyFunc) { SCEnter(); /* validate input */ @@ -638,6 +641,8 @@ void SCAppLayerParserRegisterGetTxSubStateFuncs(AppProto alproto, const uint8_t m->sub_state = sub_state; m->GetStateIdByName = GetIdByNameFunc; m->GetStateNameById = GetNameByIdFunc; + m->GetStatePolicy = GetPolicyFunc; + m->completion = completion; m->next = p->sub_state_mappings; p->sub_state_mappings = m; @@ -1310,19 +1315,39 @@ uint8_t AppLayerParserGetSubStateCompletion(const AppProto alproto, const uint8_ BUG_ON(alp_ctx.ctxs[alproto][FLOW_PROTO_DEFAULT].max_sub_state == 0); - /* 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); + for (struct AppLayerParserSubStateMapping *m = + alp_ctx.ctxs[alproto][FLOW_PROTO_DEFAULT].sub_state_mappings; + m != NULL; m = m->next) { + if (m->sub_state == sub_state) { + return m->completion; + } } + BUG_ON(1); return 0; } +const char *AppLayerParserGetSubStateProgressPolicy(const AppProto alproto, const uint8_t sub_state, + const uint8_t state, const uint8_t dir_flag) +{ + if (!AppLayerParserIsEnabled(alproto)) + return NULL; + + if (alproto == ALPROTO_DOH2) + return AppLayerParserGetSubStateProgressPolicy(ALPROTO_HTTP2, sub_state, state, dir_flag); + + for (struct AppLayerParserSubStateMapping *m = + alp_ctx.ctxs[alproto][FLOW_PROTO_DEFAULT].sub_state_mappings; + m != NULL; m = m->next) { + if (m->sub_state == sub_state) { + if (m->GetStatePolicy == NULL) { + return NULL; + } + return m->GetStatePolicy(state, dir_flag); + } + } + return NULL; +} + const char *AppLayerParserGetSubStateName(const AppProto alproto, const uint8_t sub_state) { if (!AppLayerParserIsEnabled(alproto)) diff --git a/src/app-layer-parser.h b/src/app-layer-parser.h index 5e348e1eb0..7bc01aada2 100644 --- a/src/app-layer-parser.h +++ b/src/app-layer-parser.h @@ -245,6 +245,12 @@ typedef int (*AppLayerParserGetStateIdByNameFn)(const char *name, const uint8_t * \param direction STREAM_TOSERVER/STREAM_TOCLIENT */ typedef const char *(*AppLayerParserGetStateNameByIdFn)(const int id, const uint8_t direction); +/** + * \param id progress value id to get the default firewall policy for + * \param direction STREAM_TOSERVER/STREAM_TOCLIENT + * \retval policy string such as "accept:hook", NULL to use the generic default + */ +typedef const char *(*AppLayerParserGetStatePolicyByIdFn)(const int id, const uint8_t direction); typedef int (*AppLayerParserGetFrameIdByNameFn)(const char *frame_name); typedef const char *(*AppLayerParserGetFrameNameByIdFn)(const uint8_t id); @@ -301,10 +307,12 @@ void AppLayerParserRegisterGetStateFuncs(uint8_t ipproto, AppProto alproto, AppLayerParserGetStateIdByNameFn GetStateIdByName, AppLayerParserGetStateNameByIdFn GetStateNameById); -/** \brief register state<>name funcs for a substate */ +/** \brief register state<>name funcs, the completion state and an optional + * per-state default firewall policy for a substate */ void SCAppLayerParserRegisterGetTxSubStateFuncs(AppProto alproto, const uint8_t sub_state, AppLayerParserGetStateIdByNameFn GetIdByNameFunc, - AppLayerParserGetStateNameByIdFn GetNameByIdFunc); + AppLayerParserGetStateNameByIdFn GetNameByIdFunc, const uint8_t completion, + AppLayerParserGetStatePolicyByIdFn GetPolicyFunc); void AppLayerParserRegisterTxDataFunc(uint8_t ipproto, AppProto alproto, AppLayerTxData *(*GetTxData)(void *tx)); @@ -342,6 +350,8 @@ int8_t AppLayerParserGetSubStateProgressId( const AppProto alproto, const uint8_t sub_state, const char *state, const uint8_t dir_flag); const char *AppLayerParserGetSubStateProgressName(const AppProto alproto, const uint8_t sub_state, const uint8_t state, const uint8_t dir_flag); +const char *AppLayerParserGetSubStateProgressPolicy(const AppProto alproto, const uint8_t sub_state, + const uint8_t state, const uint8_t dir_flag); uint8_t AppLayerParserGetSubStateCompletion(const AppProto alproto, const uint8_t sub_state); uint8_t AppLayerParserGetStateProgressCompletionStatus(AppProto alproto, uint8_t direction); const char *AppLayerParserGetSubStateName(const AppProto alproto, const uint8_t sub_state); diff --git a/src/detect-parse.c b/src/detect-parse.c index 2598e89648..15b7bb78ba 100644 --- a/src/detect-parse.c +++ b/src/detect-parse.c @@ -4499,9 +4499,29 @@ static int DoParseAppPolicy(const char *prefix, const AppProto app_proto, const app_pol->progress = state; app_pol->direction = (uint8_t)direction; /* init to drop:flow by default, will be overwritten by ResolveFirewallPolicy if there - * is a config for this hook. */ + * is a config for this hook. A parser may ask for another built-in for one of its states + * (see AppLayerParserGetSubStateProgressPolicy), typically a state added late, which + * must not start dropping flows as soon as it is reached. */ app_pol->policy.action = ACTION_DROP; app_pol->policy.action_scope = ACTION_SCOPE_FLOW; + if (sub_state != 0) { + const char *builtin = AppLayerParserGetSubStateProgressPolicy( + app_proto, sub_state, state, (uint8_t)direction); + if (builtin != NULL) { + /* parse into fresh locals: SigParseActionDo accumulates, so feeding it the + * seeded drop:flow would look like a second action with another scope */ + uint8_t action = 0, action_scope = 0; + if (SigParseActionDo(builtin, 0, true, &action, &action_scope) < 0 || + !FirewallScopeValidForClass(action_scope, DETECT_FIREWALL_POLICY_CLASS_APP)) { + SCLogError("%s: invalid default policy \"%s\" for state %u", app_proto_str, builtin, + state); + SCFree(app_pol); + return -1; + } + app_pol->policy.action = action; + app_pol->policy.action_scope = action_scope; + } + } int r = ResolveFirewallPolicy(&app_pol->policy, DETECT_FIREWALL_POLICY_CLASS_APP, &chain); if (r < 0) { From a35a0021f84918ab8af601edbe4960b2b9177cf6 Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Sat, 10 Oct 2026 06:35:34 +0000 Subject: [PATCH 7/9] detect/http2: give HTTP/2 trailers buffers of their own The HEADERS block that closes a stream can be a trailer block, but HTTP/2 kept it in the same header list as the opening headers, so trailer content could be matched by the header keywords. HTTP/2 also had no trailer state, so header fields that are only reachable through a header state hook, and any rule hooked at the trailer state, matched nothing: no rule could see a trailer at all. The parser marks the frames instead of storing them elsewhere. HPACK keeps one dynamic table per connection, updated by every header block in either direction, so the fields have to reach that table; the size update matcher walks the frames themselves, and continues to see trailers, while every other HTTP/2 header keyword and getter now goes through the header-section-only accessor. Three trailer accessors mirror them over the marked frames. http.trailer.raw has no wire format to reproduce for HTTP/2, so it is built the way http.header.raw is. HTTP/2 EVE output follows the buffers: a trailer field is no longer logged as a request header, like HTTP/1. A dedicated log key for trailers is a follow-up for both protocols. Ticket: #8256. --- doc/userguide/rules/http-keywords.rst | 29 ++++-- doc/userguide/upgrade.rst | 5 + rust/src/http2/detect.rs | 134 +++++++++++++++++++++++--- src/detect-http-trailer-names.c | 45 +++++++++ src/detect-http-trailer-raw.c | 43 +++++++++ src/detect-http-trailer.c | 45 +++++++++ 6 files changed, 278 insertions(+), 23 deletions(-) diff --git a/doc/userguide/rules/http-keywords.rst b/doc/userguide/rules/http-keywords.rst index 63e296b536..d68ae58fbe 100644 --- a/doc/userguide/rules/http-keywords.rst +++ b/doc/userguide/rules/http-keywords.rst @@ -1253,14 +1253,19 @@ after ``User-Agent`` but not necessarily directly after. http.trailer ------------ -The ``http.trailer`` keyword matches on the trailer fields of an HTTP/1 message. A -chunked body may end with a second block of fields, the trailers, which are parsed -once the body is complete. They are deliberately kept out of :ref:`http.header`, -:ref:`http.header.raw` and :ref:`http.header_names` as well as out of the field -keywords such as ``http.user_agent``. +The ``http.trailer`` keyword matches on the trailer fields of an HTTP message. In +HTTP/1 a chunked body may end with a second block of fields, the trailers, which are +parsed once the body is complete. In HTTP/2 they arrive as the HEADERS block that closes +the stream, with or without body data before it, which is how gRPC and telemetry uploads +report status at the end of a stream. +Trailers are deliberately kept out of :ref:`http.header`, :ref:`http.header.raw` and +:ref:`http.header_names` as well as out of the field keywords such as +``http.user_agent``. Fields are rendered as ``Name: value``, separated by \r\n, like ``http.header`` does. -In HTTP/1 repeated names are combined with a comma, as in the header section. In HTTP/2 +In HTTP/1 repeated names are combined with a comma under the same repetition limit as the +header section, so occurrences past it are not merged; :ref:`http.trailer.raw` still shows +them all. In HTTP/2 the trailer block is rendered as one line per entry, so a repeated name appears twice. Example HTTP Request:: @@ -1285,10 +1290,14 @@ Example HTTP Request:: .. note:: Since the trailer section follows the body, a rule that only matches here alerts when the last chunk is parsed, not when the headers are. To run a rule at that point regardless of what it matches, use the ``http1:request_trailer`` or - ``http1:response_trailer`` state hook. + ``http1:response_trailer`` state hook, or ``http2:stream:request_trailers`` and + ``http2:stream:response_trailers`` for HTTP/2. -.. note:: HTTP/2 does not have a trailer section; its trailing HEADERS frames are - currently part of :ref:`http.header`. +.. note:: An HTTP/2 block counts as trailers once the header section of that direction has + completed: for a request the first ``HEADERS`` that ends the section, for a response the + first one that also carries a non-1xx status. Body data is not required, so a second + ``HEADERS`` block with no ``DATA`` in between is a trailer block, while 1xx informational + responses are not. .. _http.trailer_names: @@ -1296,7 +1305,7 @@ http.trailer_names ------------------ The ``http.trailer_names`` keyword matches on the names of the trailer fields of an -HTTP/1 message. The buffer has the same shape as :ref:`http.header_names`: it starts +HTTP message. The buffer has the same shape as :ref:`http.header_names`: it starts with a \r\n, lists one name per line and ends with an extra \r\n. .. container:: example-rule diff --git a/doc/userguide/upgrade.rst b/doc/userguide/upgrade.rst index 3b1850297f..67144e2127 100644 --- a/doc/userguide/upgrade.rst +++ b/doc/userguide/upgrade.rst @@ -77,6 +77,11 @@ Major Changes example ``http2:stream:request_complete<``) now cover one more state. The new states start out with the built-in default policy ``accept:hook`` instead of ``drop:flow``, so streams carrying trailers are not blocked before a policy is chosen. + +- HTTP/2 trailer fields keep being logged in the ``http.request_headers`` and + ``http.response_headers`` arrays, as with HTTP/1: what changed for them is detection only, + through the buffers and the new stream states above. As with HTTP/1 there is no EVE key for + trailer fields yet. - ``pgsql`` is no longer enabled by default. Previously, if the ``app-layer.protocols.pgsql`` section was absent from suricata.yaml, the parser would be enabled. It's now disabled by default. Simply enabling its EVE diff --git a/rust/src/http2/detect.rs b/rust/src/http2/detect.rs index b99ac91bd2..d9b1dcdb1b 100644 --- a/rust/src/http2/detect.rs +++ b/rust/src/http2/detect.rs @@ -268,6 +268,23 @@ fn http2_detect_sizeupdate_match( return 0; } +/// Blocks of frames holding header-section fields. Kept separate from +/// http2_header_blocks so that the HPACK size-update matching still sees every frame. +fn http2_header_section_blocks(frame: &HTTP2Frame) -> Option<&[parser::HTTP2FrameHeaderBlock]> { + if frame.is_trailer { + return None; + } + http2_header_blocks(frame) +} + +/// Blocks of frames holding trailer-section fields. +fn http2_trailer_section_blocks(frame: &HTTP2Frame) -> Option<&[parser::HTTP2FrameHeaderBlock]> { + if frame.is_trailer { + return http2_header_blocks(frame); + } + None +} + fn http2_header_blocks(frame: &HTTP2Frame) -> Option<&[parser::HTTP2FrameHeaderBlock]> { match &frame.data { HTTP2FrameTypeData::HEADERS(hd) => { @@ -328,7 +345,7 @@ pub unsafe extern "C" fn SCHttp2TxGetHeaderName( match direction.into() { Direction::ToServer => { for i in 0..tx.frames_ts.len() { - if let Some(blocks) = http2_header_blocks(&tx.frames_ts[i]) { + if let Some(blocks) = http2_header_section_blocks(&tx.frames_ts[i]) { if nb < pos + blocks.len() as u32 { let value = &blocks[(nb - pos) as usize].name; *buffer = value.as_ptr(); //unsafe @@ -342,7 +359,7 @@ pub unsafe extern "C" fn SCHttp2TxGetHeaderName( } Direction::ToClient => { for i in 0..tx.frames_tc.len() { - if let Some(blocks) = http2_header_blocks(&tx.frames_tc[i]) { + if let Some(blocks) = http2_header_section_blocks(&tx.frames_tc[i]) { if nb < pos + blocks.len() as u32 { let value = &blocks[(nb - pos) as usize].name; *buffer = value.as_ptr(); //unsafe @@ -367,7 +384,7 @@ fn http2_frames_get_header_firstvalue<'a>( &tx.frames_tc }; for frame in frames { - if let Some(blocks) = http2_header_blocks(frame) { + if let Some(blocks) = http2_header_section_blocks(frame) { for block in blocks.iter() { if block.name.as_ref() == name.as_bytes() { return Ok(&block.value); @@ -391,7 +408,7 @@ pub fn http2_frames_get_header_value_vec( &tx.frames_tc }; for frame in frames { - if let Some(blocks) = http2_header_blocks(frame) { + if let Some(blocks) = http2_header_section_blocks(frame) { for block in blocks.iter() { if block.name.as_ref() == name.as_bytes() { if found == 0 { @@ -434,7 +451,7 @@ fn http2_frames_get_header_value<'a>( &tx.frames_tc }; for frame in frames { - if let Some(blocks) = http2_header_blocks(frame) { + if let Some(blocks) = http2_header_section_blocks(frame) { for block in blocks.iter() { if block.name.as_ref() == name.as_bytes() { if found == 0 { @@ -797,7 +814,7 @@ pub unsafe extern "C" fn SCHttp2TxGetHeaderNames( &tx.frames_tc }; for frame in frames { - if let Some(blocks) = http2_header_blocks(frame) { + if let Some(blocks) = http2_header_section_blocks(frame) { for block in blocks.iter() { // we do not escape linefeeds in headers names tbuf.data.extend_from_slice(&block.name); @@ -860,7 +877,7 @@ pub unsafe extern "C" fn SCHttp2TxGetHeaders( &tx.frames_tc }; for frame in frames { - if let Some(blocks) = http2_header_blocks(frame) { + if let Some(blocks) = http2_header_section_blocks(frame) { for block in blocks.iter() { if !http2_header_iscookie(direction.into(), &block.name) { // we do not escape linefeeds nor : in headers names @@ -894,7 +911,7 @@ pub unsafe extern "C" fn SCHttp2TxGetHeadersRaw( &tx.frames_tc }; for frame in frames { - if let Some(blocks) = http2_header_blocks(frame) { + if let Some(blocks) = http2_header_section_blocks(frame) { for block in blocks.iter() { // we do not escape linefeeds nor : in headers names tbuf.data.extend_from_slice(&block.name); @@ -912,6 +929,97 @@ pub unsafe extern "C" fn SCHttp2TxGetHeadersRaw( return 0; } +#[no_mangle] +pub unsafe extern "C" fn SCHttp2TxGetTrailers( + tx: &mut HTTP2Transaction, direction: u8, buffer: *mut *const u8, buffer_len: *mut u32, + tbuf: *mut c_void, +) -> u8 { + let tbuf = cast_pointer!(tbuf, DetectThreadBuf); + tbuf.data.clear(); + let frames = if direction & Direction::ToServer as u8 != 0 { + &tx.frames_ts + } else { + &tx.frames_tc + }; + for frame in frames { + if let Some(blocks) = http2_trailer_section_blocks(frame) { + for block in blocks.iter() { + tbuf.data.extend_from_slice(&block.name); + tbuf.data.extend_from_slice(b": "); + tbuf.data + .extend_from_slice(http2_header_trimspaces(&block.value)); + tbuf.data.extend_from_slice(b"\r\n"); + } + } + } + if !tbuf.data.is_empty() { + *buffer = tbuf.data.as_ptr(); //unsafe + *buffer_len = tbuf.data.len() as u32; + return 1; + } + return 0; +} + +#[no_mangle] +pub unsafe extern "C" fn SCHttp2TxGetTrailersRaw( + tx: &mut HTTP2Transaction, direction: u8, buffer: *mut *const u8, buffer_len: *mut u32, + tbuf: *mut c_void, +) -> u8 { + let tbuf = cast_pointer!(tbuf, DetectThreadBuf); + tbuf.data.clear(); + let frames = if direction & Direction::ToServer as u8 != 0 { + &tx.frames_ts + } else { + &tx.frames_tc + }; + for frame in frames { + if let Some(blocks) = http2_trailer_section_blocks(frame) { + for block in blocks.iter() { + tbuf.data.extend_from_slice(&block.name); + tbuf.data.extend_from_slice(b": "); + tbuf.data.extend_from_slice(&block.value); + tbuf.data.extend_from_slice(b"\r\n"); + } + } + } + if !tbuf.data.is_empty() { + *buffer = tbuf.data.as_ptr(); //unsafe + *buffer_len = tbuf.data.len() as u32; + return 1; + } + return 0; +} + +#[no_mangle] +pub unsafe extern "C" fn SCHttp2TxGetTrailerNames( + tx: &mut HTTP2Transaction, direction: u8, buffer: *mut *const u8, buffer_len: *mut u32, + tbuf: *mut c_void, +) -> u8 { + let tbuf = cast_pointer!(tbuf, DetectThreadBuf); + tbuf.data.clear(); + tbuf.data.extend_from_slice(b"\r\n"); + let frames = if direction & Direction::ToServer as u8 != 0 { + &tx.frames_ts + } else { + &tx.frames_tc + }; + for frame in frames { + if let Some(blocks) = http2_trailer_section_blocks(frame) { + for block in blocks.iter() { + tbuf.data.extend_from_slice(&block.name); + tbuf.data.extend_from_slice(b"\r\n"); + } + } + } + if tbuf.data.len() > 2 { + tbuf.data.extend_from_slice(b"\r\n"); + *buffer = tbuf.data.as_ptr(); //unsafe + *buffer_len = tbuf.data.len() as u32; + return 1; + } + return 0; +} + #[derive(Default)] struct Http2ThreadMultiBuf { data: Vec>, @@ -939,7 +1047,7 @@ pub unsafe extern "C" fn SCHttp2TxGetHeader( match direction.into() { Direction::ToServer => { for i in 0..tx.frames_ts.len() { - if let Some(blocks) = http2_header_blocks(&tx.frames_ts[i]) { + if let Some(blocks) = http2_header_section_blocks(&tx.frames_ts[i]) { if nb < pos + blocks.len() as u32 { if nb as usize == tbuf.data.len() { tbuf.data.push(Vec::new()); @@ -957,7 +1065,7 @@ pub unsafe extern "C" fn SCHttp2TxGetHeader( } Direction::ToClient => { for i in 0..tx.frames_tc.len() { - if let Some(blocks) = http2_header_blocks(&tx.frames_tc[i]) { + if let Some(blocks) = http2_header_section_blocks(&tx.frames_tc[i]) { if nb < pos + blocks.len() as u32 { if nb as usize == tbuf.data.len() { tbuf.data.push(Vec::new()); @@ -1003,9 +1111,9 @@ fn http2_tx_set_header(state: &mut HTTP2State, name: &[u8], input: &[u8]) { .find_or_create_tx(&head, &txdata, Direction::ToServer) .unwrap(); tx.frames_ts.push(HTTP2Frame { - is_trailer: false, header: head, data: txdata, + is_trailer: false, }); //we do not expect more data from client if let HTTP2Progress::STREAM(ref mut stream_tx) = tx.progress { @@ -1051,9 +1159,9 @@ fn http2_tx_set_settings(state: &mut HTTP2State, input: &[u8]) { .find_or_create_tx(&head, &txdata, Direction::ToServer) .unwrap(); tx.frames_ts.push(HTTP2Frame { - is_trailer: false, header: head, data: txdata, + is_trailer: false, }); } Err(_) => { @@ -1168,9 +1276,9 @@ mod tests { }; let txdata = HTTP2FrameTypeData::HEADERS(hs); tx.frames_ts.push(HTTP2Frame { - is_trailer: false, header: head, data: txdata, + is_trailer: false, }); match http2_frames_get_header_value(&tx, Direction::ToServer, "Host") { Some(Http2Header::Multiple(x)) => { diff --git a/src/detect-http-trailer-names.c b/src/detect-http-trailer-names.c index bf4fd2b23c..ed7c388854 100644 --- a/src/detect-http-trailer-names.c +++ b/src/detect-http-trailer-names.c @@ -51,6 +51,7 @@ #define BUFFER_DESC "http trailer field names" static int g_buffer_id = 0; static int g_keyword_thread_id = 0; +static int g_http2_thread_id = 0; #define BUFFER_SIZE_STEP 256 static HttpHeaderThreadDataConfig g_td_config = { BUFFER_SIZE_STEP }; @@ -58,6 +59,9 @@ static HttpHeaderThreadDataConfig g_td_config = { BUFFER_SIZE_STEP }; static InspectionBuffer *GetBuffer1ForTX(DetectEngineThreadCtx *det_ctx, const DetectEngineTransforms *transforms, Flow *_f, const uint8_t flow_flags, void *txv, const int list_id); +static InspectionBuffer *GetBuffer2ForTX(DetectEngineThreadCtx *det_ctx, + const DetectEngineTransforms *transforms, Flow *_f, const uint8_t flow_flags, void *txv, + const int list_id); static int DetectHttpTrailerNamesSetup(DetectEngineCtx *de_ctx, Signature *s, const char *str) { @@ -87,12 +91,29 @@ void DetectHttpTrailerNamesRegister(void) DetectAppLayerMpmRegister(BUFFER_NAME, SIG_FLAG_TOCLIENT, 2, PrefilterGenericMpmRegister, GetBuffer1ForTX, ALPROTO_HTTP1, HTP_RESPONSE_PROGRESS_TRAILER); + /* http2: a trailing HEADERS block completes at the trailer state */ + DetectAppLayerInspectEngineRegisterSubState(BUFFER_NAME, ALPROTO_HTTP2, SIG_FLAG_TOSERVER, + HTTP2TxTypeStream, HTTP2ProgTrailers, DetectEngineInspectBufferGeneric, + GetBuffer2ForTX); + DetectAppLayerInspectEngineRegisterSubState(BUFFER_NAME, ALPROTO_HTTP2, SIG_FLAG_TOCLIENT, + HTTP2TxTypeStream, HTTP2ProgTrailers, DetectEngineInspectBufferGeneric, + GetBuffer2ForTX); + DetectAppLayerMpmRegisterSubState(BUFFER_NAME, SIG_FLAG_TOSERVER, 2, + PrefilterGenericMpmRegister, GetBuffer2ForTX, ALPROTO_HTTP2, HTTP2TxTypeStream, + HTTP2ProgTrailers); + DetectAppLayerMpmRegisterSubState(BUFFER_NAME, SIG_FLAG_TOCLIENT, 2, + PrefilterGenericMpmRegister, GetBuffer2ForTX, ALPROTO_HTTP2, HTTP2TxTypeStream, + HTTP2ProgTrailers); + DetectBufferTypeSetDescriptionByName(BUFFER_NAME, BUFFER_DESC); g_buffer_id = DetectBufferTypeGetByName(BUFFER_NAME); g_keyword_thread_id = SCDetectRegisterThreadCtxGlobalFuncs( KEYWORD_NAME, HttpHeaderThreadDataInit, &g_td_config, HttpHeaderThreadDataFree); + + g_http2_thread_id = SCDetectRegisterThreadCtxGlobalFuncs( + "http2.trailer_names", SCDetectThreadBufDataInit, NULL, SCDetectThreadBufDataFree); } static uint8_t *GetBufferForTX( @@ -167,3 +188,27 @@ static InspectionBuffer *GetBuffer1ForTX(DetectEngineThreadCtx *det_ctx, return buffer; } + +static InspectionBuffer *GetBuffer2ForTX(DetectEngineThreadCtx *det_ctx, + const DetectEngineTransforms *transforms, Flow *_f, const uint8_t flow_flags, void *txv, + const int list_id) +{ + InspectionBuffer *buffer = SCInspectionBufferGet(det_ctx, list_id); + if (buffer->inspect == NULL) { + uint32_t b_len = 0; + const uint8_t *b = NULL; + + void *thread_buf = SCDetectThreadCtxGetGlobalKeywordThreadCtx(det_ctx, g_http2_thread_id); + if (thread_buf == NULL) + return NULL; + + if (SCHttp2TxGetTrailerNames(txv, flow_flags, &b, &b_len, thread_buf) != 1) + return NULL; + if (b == NULL || b_len == 0) + return NULL; + + SCInspectionBufferSetupAndApplyTransforms(det_ctx, list_id, buffer, b, b_len, transforms); + } + + return buffer; +} diff --git a/src/detect-http-trailer-raw.c b/src/detect-http-trailer-raw.c index d529bc4e6e..7cc5f63a34 100644 --- a/src/detect-http-trailer-raw.c +++ b/src/detect-http-trailer-raw.c @@ -49,10 +49,14 @@ #define BUFFER_NAME "http_trailer_raw" #define BUFFER_DESC "raw http trailer data" static int g_buffer_id = 0; +static int g_http2_thread_id = 0; static InspectionBuffer *GetData(DetectEngineThreadCtx *det_ctx, const DetectEngineTransforms *transforms, Flow *_f, const uint8_t flow_flags, void *txv, const int list_id); +static InspectionBuffer *GetData2(DetectEngineThreadCtx *det_ctx, + const DetectEngineTransforms *transforms, Flow *_f, const uint8_t flow_flags, void *txv, + const int list_id); static int DetectHttpTrailerRawSetup(DetectEngineCtx *de_ctx, Signature *s, const char *str) { @@ -82,9 +86,24 @@ void DetectHttpTrailerRawRegister(void) DetectAppLayerMpmRegister(BUFFER_NAME, SIG_FLAG_TOCLIENT, 2, PrefilterGenericMpmRegister, GetData, ALPROTO_HTTP1, HTP_RESPONSE_PROGRESS_TRAILER); + /* http2: a trailing HEADERS block completes at the trailer state */ + DetectAppLayerInspectEngineRegisterSubState(BUFFER_NAME, ALPROTO_HTTP2, SIG_FLAG_TOSERVER, + HTTP2TxTypeStream, HTTP2ProgTrailers, DetectEngineInspectBufferGeneric, GetData2); + DetectAppLayerInspectEngineRegisterSubState(BUFFER_NAME, ALPROTO_HTTP2, SIG_FLAG_TOCLIENT, + HTTP2TxTypeStream, HTTP2ProgTrailers, DetectEngineInspectBufferGeneric, GetData2); + DetectAppLayerMpmRegisterSubState(BUFFER_NAME, SIG_FLAG_TOSERVER, 2, + PrefilterGenericMpmRegister, GetData2, ALPROTO_HTTP2, HTTP2TxTypeStream, + HTTP2ProgTrailers); + DetectAppLayerMpmRegisterSubState(BUFFER_NAME, SIG_FLAG_TOCLIENT, 2, + PrefilterGenericMpmRegister, GetData2, ALPROTO_HTTP2, HTTP2TxTypeStream, + HTTP2ProgTrailers); + DetectBufferTypeSetDescriptionByName(BUFFER_NAME, BUFFER_DESC); g_buffer_id = DetectBufferTypeGetByName(BUFFER_NAME); + + g_http2_thread_id = SCDetectRegisterThreadCtxGlobalFuncs( + "http2.trailer_raw", SCDetectThreadBufDataInit, NULL, SCDetectThreadBufDataFree); } static InspectionBuffer *GetData(DetectEngineThreadCtx *det_ctx, @@ -116,3 +135,27 @@ static InspectionBuffer *GetData(DetectEngineThreadCtx *det_ctx, return buffer; } + +static InspectionBuffer *GetData2(DetectEngineThreadCtx *det_ctx, + const DetectEngineTransforms *transforms, Flow *_f, const uint8_t flow_flags, void *txv, + const int list_id) +{ + InspectionBuffer *buffer = SCInspectionBufferGet(det_ctx, list_id); + if (buffer->inspect == NULL) { + uint32_t b_len = 0; + const uint8_t *b = NULL; + + void *thread_buf = SCDetectThreadCtxGetGlobalKeywordThreadCtx(det_ctx, g_http2_thread_id); + if (thread_buf == NULL) + return NULL; + + if (SCHttp2TxGetTrailersRaw(txv, flow_flags, &b, &b_len, thread_buf) != 1) + return NULL; + if (b == NULL || b_len == 0) + return NULL; + + SCInspectionBufferSetupAndApplyTransforms(det_ctx, list_id, buffer, b, b_len, transforms); + } + + return buffer; +} diff --git a/src/detect-http-trailer.c b/src/detect-http-trailer.c index f691a03b08..d68eadc453 100644 --- a/src/detect-http-trailer.c +++ b/src/detect-http-trailer.c @@ -52,6 +52,7 @@ #define BUFFER_DESC "http trailer fields" static int g_buffer_id = 0; static int g_keyword_thread_id = 0; +static int g_http2_thread_id = 0; #ifdef UNITTESTS void DetectHttpTrailerRegisterTests(void); @@ -63,6 +64,9 @@ static HttpHeaderThreadDataConfig g_td_config = { BUFFER_SIZE_STEP }; static InspectionBuffer *GetBuffer1ForTX(DetectEngineThreadCtx *det_ctx, const DetectEngineTransforms *transforms, Flow *_f, const uint8_t flow_flags, void *txv, const int list_id); +static InspectionBuffer *GetBuffer2ForTX(DetectEngineThreadCtx *det_ctx, + const DetectEngineTransforms *transforms, Flow *_f, const uint8_t flow_flags, void *txv, + const int list_id); static int DetectHttpTrailerSetup(DetectEngineCtx *de_ctx, Signature *s, const char *str) { @@ -98,12 +102,29 @@ void DetectHttpTrailerRegister(void) DetectAppLayerMpmRegister(BUFFER_NAME, SIG_FLAG_TOCLIENT, 2, PrefilterGenericMpmRegister, GetBuffer1ForTX, ALPROTO_HTTP1, HTP_RESPONSE_PROGRESS_TRAILER); + /* http2: a trailing HEADERS block completes at the trailer state */ + DetectAppLayerInspectEngineRegisterSubState(BUFFER_NAME, ALPROTO_HTTP2, SIG_FLAG_TOSERVER, + HTTP2TxTypeStream, HTTP2ProgTrailers, DetectEngineInspectBufferGeneric, + GetBuffer2ForTX); + DetectAppLayerInspectEngineRegisterSubState(BUFFER_NAME, ALPROTO_HTTP2, SIG_FLAG_TOCLIENT, + HTTP2TxTypeStream, HTTP2ProgTrailers, DetectEngineInspectBufferGeneric, + GetBuffer2ForTX); + DetectAppLayerMpmRegisterSubState(BUFFER_NAME, SIG_FLAG_TOSERVER, 2, + PrefilterGenericMpmRegister, GetBuffer2ForTX, ALPROTO_HTTP2, HTTP2TxTypeStream, + HTTP2ProgTrailers); + DetectAppLayerMpmRegisterSubState(BUFFER_NAME, SIG_FLAG_TOCLIENT, 2, + PrefilterGenericMpmRegister, GetBuffer2ForTX, ALPROTO_HTTP2, HTTP2TxTypeStream, + HTTP2ProgTrailers); + DetectBufferTypeSetDescriptionByName(BUFFER_NAME, BUFFER_DESC); g_buffer_id = DetectBufferTypeGetByName(BUFFER_NAME); g_keyword_thread_id = SCDetectRegisterThreadCtxGlobalFuncs( KEYWORD_NAME, HttpHeaderThreadDataInit, &g_td_config, HttpHeaderThreadDataFree); + + g_http2_thread_id = SCDetectRegisterThreadCtxGlobalFuncs( + "http2.trailer", SCDetectThreadBufDataInit, NULL, SCDetectThreadBufDataFree); } static uint8_t *GetBufferForTX( @@ -166,6 +187,30 @@ static InspectionBuffer *GetBuffer1ForTX(DetectEngineThreadCtx *det_ctx, return buffer; } +static InspectionBuffer *GetBuffer2ForTX(DetectEngineThreadCtx *det_ctx, + const DetectEngineTransforms *transforms, Flow *_f, const uint8_t flow_flags, void *txv, + const int list_id) +{ + InspectionBuffer *buffer = SCInspectionBufferGet(det_ctx, list_id); + if (buffer->inspect == NULL) { + uint32_t b_len = 0; + const uint8_t *b = NULL; + + void *thread_buf = SCDetectThreadCtxGetGlobalKeywordThreadCtx(det_ctx, g_http2_thread_id); + if (thread_buf == NULL) + return NULL; + + if (SCHttp2TxGetTrailers(txv, flow_flags, &b, &b_len, thread_buf) != 1) + return NULL; + if (b == NULL || b_len == 0) + return NULL; + + SCInspectionBufferSetupAndApplyTransforms(det_ctx, list_id, buffer, b, b_len, transforms); + } + + return buffer; +} + #ifdef UNITTESTS #include "tests/detect-http-trailer.c" #endif From 4241dae1adf30bff06f854784f71e3564c5ac277 Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Mon, 19 Oct 2026 06:49:23 +0000 Subject: [PATCH 8/9] detect/http1: inspect HTTP/1 block buffers only when complete The raw HTTP header buffer had to be registered one state past the header state. The reason is that htp reports progress as soon as a block starts: request_progress reaches HEADERS right after the request line and TRAILER at the zero-length chunk, so a detection round at HEADERS can run while the block is still arriving. For a byte buffer that shows - negated content or a within: relative match would decide on a truncated block. HEADERS + 1 avoided it by accident of numbering, BODY being what htp reaches once the block is done. Make completion an event instead of inferring it from numbering. htp's REQUEST_HEADERS and RESPONSE_HEADERS hooks only ran in test builds, which is why production had nothing to key on; they now run always and are bound through the C API. Suricata marks per transaction that a block ended, from those hooks and from the trailer hooks that already ran, and the four HTTP/1 block buffers - http.header.raw, http.trailer, http.trailer.raw and http.trailer_names - report no data until then. Each registers at the state where its content completes, and no state offset remains in the tree. A block that ends because the stream closes needs no special case: htp's closure handling advances progress past the block state and runs the trailer hook, so the gate opens on its own and the partial content is inspected as it is today. Suricata-Verify tests/http1-raw-header- truncated covers both directions. htp progress values, and the transaction lifecycle built on them, are left alone: delaying those changes when firewall decisions become definitive and fails the ruletype-firewall state hook tests. Ticket: #8256. --- doc/userguide/upgrade.rst | 6 ++++ rust/htp/src/c_api/config.rs | 26 +++++++++++++++ rust/htp/src/config.rs | 4 --- rust/htp/src/connection_parser.rs | 2 -- src/app-layer-htp.c | 54 ++++++++++++++++++++++++++++++- src/app-layer-htp.h | 44 +++++++++++++++++++++++++ src/detect-http-raw-header.c | 14 ++++---- src/detect-http-trailer-names.c | 3 ++ src/detect-http-trailer-raw.c | 2 ++ src/detect-http-trailer.c | 3 ++ 10 files changed, 143 insertions(+), 15 deletions(-) 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)) { From 82177e0eec0512e82f9b87cdcacc7a7b676f7091 Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Tue, 20 Oct 2026 13:56:04 +0000 Subject: [PATCH 9/9] lua/http: read trailer fields from Lua Header functions in the Lua http library read the header tables and the raw header buffer. Once trailer fields moved out of those, a script could no longer see them at all, and nothing replaced the access: the only way to act on a trailer was a rule. Add the trailer twins over the tables and blocks the http.trailer buffers use: tx:request_trailer(), tx:request_trailers(), tx:request_trailers_raw() and the response versions. Name lookup is case insensitive and reports a miss as an error, like the header functions. Repeated fields need no merging here: the parser folds them while storing, so a name has one entry. HTTP/2 is out of reach of these functions: the Lua http library is driven by htp, so it has no HTTP/2 transaction to read. Ticket: #8256. --- doc/userguide/lua/libs/http.rst | 51 ++++++++++++-- doc/userguide/upgrade.rst | 13 ++-- src/util-lua-http.c | 116 ++++++++++++++++++++++++++++++++ 3 files changed, 169 insertions(+), 11 deletions(-) diff --git a/doc/userguide/lua/libs/http.rst b/doc/userguide/lua/libs/http.rst index 169020f77a..d5a5a5945e 100644 --- a/doc/userguide/lua/libs/http.rst +++ b/doc/userguide/lua/libs/http.rst @@ -41,12 +41,8 @@ HTTP is transaction based, and the current transaction must be obtained before u print(err) end -All other functions are methods on the transaction table. - -.. note:: - - The header functions below cover the header section only. HTTP trailer fields are not - reachable from Lua; match them with the ``http.trailer`` buffers in a rule instead. +All other functions are methods on the transaction table. Header functions cover the +header section; trailer fields have the functions at the end of this page. Transaction Methods ~~~~~~~~~~~~~~~~~~~ @@ -177,6 +173,49 @@ Example:: http_response_headers = tx:response_headers() print(http_response_headers) +``request_trailer()`` +^^^^^^^^^^^^^^^^^^^^^ + +Get the value of an HTTP request trailer field by name, or nil and an error if the +transaction has no field of that name. Names are matched case insensitively. + +Example:: + + local tx = http.get_tx() + local value, err = tx:request_trailer("X-Trailed") + +``response_trailer()`` +^^^^^^^^^^^^^^^^^^^^^^ + +Get the value of an HTTP response trailer field by name. + +``request_trailers()`` +^^^^^^^^^^^^^^^^^^^^^^ + +Get all HTTP request trailer fields as a table. + +Example:: + + local tx = http.get_tx() + for name, value in pairs(tx:request_trailers()) do + print(name, value) + end + +``response_trailers()`` +^^^^^^^^^^^^^^^^^^^^^^^ + +Get all HTTP response trailer fields as a table. + +``request_trailers_raw()`` +^^^^^^^^^^^^^^^^^^^^^^^^^^ + +Get the raw HTTP request trailer block, as it appeared on the wire. + +``response_trailers_raw()`` +^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +Get the raw HTTP response trailer block, as it appeared on the wire. + ``request_body()`` ^^^^^^^^^^^^^^^^^^ diff --git a/doc/userguide/upgrade.rst b/doc/userguide/upgrade.rst index 4d382ce05f..e8d6a6951b 100644 --- a/doc/userguide/upgrade.rst +++ b/doc/userguide/upgrade.rst @@ -65,11 +65,14 @@ Major Changes ``http.header_names`` or a field keyword such as ``http.user_agent`` stops matching and needs one of the trailer buffers. This also closes a hole where a trailer could extend a header value that detection and logging had already accepted. - For HTTP/1 they also drop out of the ``http.request_headers`` and - ``http.response_headers`` EVE arrays; no EVE key for trailer fields exists yet. - The Lua http library loses them the same way: ``tx:request_header()``, - ``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. + EVE output still covers them: with ``dump-all-headers`` set they appear in the + ``http.request_headers`` and ``http.response_headers`` arrays as entries of their own, + instead of being appended to the value of a header field that shares their name. So a name + used in both blocks is listed twice, once per block. No EVE key for trailer fields exists + yet. + The Lua http library has accessors of its own for them now: ``tx:request_trailer()``, + ``tx:request_trailers()``, ``tx:request_trailers_raw()`` and the response twins, next to + the header functions that keep reading the header section only. - 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 diff --git a/src/util-lua-http.c b/src/util-lua-http.c index f5599a4bb5..4b059f4b95 100644 --- a/src/util-lua-http.c +++ b/src/util-lua-http.c @@ -251,6 +251,116 @@ static int LuaHttpGetResponseHeaders(lua_State *luastate) return LuaHttpGetHeaders(luastate, 1); } +/* Trailers live in their own table now that they are kept out of the headers. Repeated + * fields are merged while parsing, so a name has a single entry. */ +static int LuaHttpGetTrailer(lua_State *luastate, int dir) +{ + struct LuaTx *tx = luaL_testudata(luastate, 1, htp_tx); + if (tx == NULL) { + lua_pushnil(luastate); + return 1; + } + + /* since arg was added at last, it must be on top of the stack */ + const char *name = LuaGetStringArgument(luastate, lua_gettop(luastate)); + if (name == NULL) { + return LuaCallbackError(luastate, "argument missing, empty or wrong type"); + } + + const htp_headers_t *table = + dir ? htp_tx_response_trailers(tx->tx) : htp_tx_request_trailers(tx->tx); + const size_t nlen = strlen(name); + const size_t count = table != NULL ? htp_headers_size(table) : 0; + for (size_t i = 0; i < count; i++) { + const htp_header_t *h = htp_headers_get_index(table, i); + if ((size_t)htp_header_name_len(h) == nlen && + strncasecmp((const char *)htp_header_name_ptr(h), name, nlen) == 0) { + return LuaPushStringBuffer(luastate, htp_header_value_ptr(h), htp_header_value_len(h)); + } + } + return LuaCallbackError(luastate, "trailer not found"); +} + +static int LuaHttpGetRequestTrailer(lua_State *luastate) +{ + return LuaHttpGetTrailer(luastate, 0 /* request */); +} + +static int LuaHttpGetResponseTrailer(lua_State *luastate) +{ + return LuaHttpGetTrailer(luastate, 1 /* response */); +} + +static int LuaHttpGetRawTrailers(lua_State *luastate, int dir) +{ + struct LuaTx *tx = luaL_testudata(luastate, 1, htp_tx); + if (tx == NULL) { + lua_pushnil(luastate); + return 1; + } + HtpTxUserData *htud = (HtpTxUserData *)htp_tx_get_user_data(tx->tx); + + uint8_t *raw = htud->request_trailers_raw; + uint32_t raw_len = htud->request_trailers_raw_len; + if (dir == 1) { + raw = htud->response_trailers_raw; + raw_len = htud->response_trailers_raw_len; + } + + if (raw == NULL || raw_len == 0) + return LuaCallbackError(luastate, "no raw trailers"); + + return LuaPushStringBuffer(luastate, raw, raw_len); +} + +static int LuaHttpGetRawRequestTrailers(lua_State *luastate) +{ + return LuaHttpGetRawTrailers(luastate, 0); +} + +static int LuaHttpGetRawResponseTrailers(lua_State *luastate) +{ + return LuaHttpGetRawTrailers(luastate, 1); +} + +static int LuaHttpGetTrailers(lua_State *luastate, int dir) +{ + struct LuaTx *tx = luaL_testudata(luastate, 1, htp_tx); + if (tx == NULL) { + lua_pushnil(luastate); + return 1; + } + + const htp_headers_t *table = + dir ? htp_tx_response_trailers(tx->tx) : htp_tx_request_trailers(tx->tx); + if (table == NULL) { + lua_pushnil(luastate); + return 1; + } + + lua_newtable(luastate); + const size_t count = htp_headers_size(table); + for (size_t i = 0; i < count; i++) { + const htp_header_t *h = htp_headers_get_index(table, i); + LuaPushStringBuffer(luastate, htp_header_name_ptr(h), htp_header_name_len(h)); + LuaPushStringBuffer(luastate, htp_header_value_ptr(h), htp_header_value_len(h)); + lua_settable(luastate, -3); + } + return 1; +} + +/** \brief return request trailers as lua table */ +static int LuaHttpGetRequestTrailers(lua_State *luastate) +{ + return LuaHttpGetTrailers(luastate, 0); +} + +/** \brief return response trailers as lua table */ +static int LuaHttpGetResponseTrailers(lua_State *luastate) +{ + return LuaHttpGetTrailers(luastate, 1); +} + static int LuaHttpGetBody(lua_State *luastate, int dir) { struct LuaTx *tx = luaL_testudata(luastate, 1, htp_tx); @@ -319,6 +429,12 @@ static const struct luaL_Reg txlib[] = { {"request_uri_normalized", LuaHttpGetRequestUriNormalized}, {"request_headers", LuaHttpGetRequestHeaders}, {"response_headers", LuaHttpGetResponseHeaders}, + {"request_trailer", LuaHttpGetRequestTrailer}, + {"response_trailer", LuaHttpGetResponseTrailer}, + {"request_trailers", LuaHttpGetRequestTrailers}, + {"response_trailers", LuaHttpGetResponseTrailers}, + {"request_trailers_raw", LuaHttpGetRawRequestTrailers}, + {"response_trailers_raw", LuaHttpGetRawResponseTrailers}, {"request_host", LuaHttpGetRequestHost}, {"request_body", LuaHttpGetRequestBody}, {"response_body", LuaHttpGetResponseBody},