From fd4c197f45ded5b77e175f1c11776e3d4d057386 Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Sat, 10 Oct 2026 04:42:29 +0000 Subject: [PATCH] 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) {