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

@@ -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

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

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,
         });

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<HTTP2Frame>,
     pub frames_ts: Vec<HTTP2Frame>,
+    /// 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<bool>,
+    open_block_trailer_tc: Option<bool>,
 
     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<Vec<u8>> {
         //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<parser::HTTP2FrameHeaderBlock> {
+        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"
+        );
+    }
+}

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" {

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))

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);

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) {