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

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

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;

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

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
+

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

rust/htp/src/test/main.rs

@@ -771,6 +771,96 @@ fn ResponseTrailerData() {
     assert_eq!(4, *tx.user_data::<i32>().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());

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,

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 {