pjfanning opened a new pull request, #1258: URL: https://github.com/apache/pekko-http/pull/1258
### Motivation Outgoing HTTP/2 header fields were HPACK-encoded without any check for CR, LF or NUL in the name or value. The HTTP/1.1 renderer scans each rendered header for CR/LF and drops it (`Rendering.~~(HttpHeader)`), but the HTTP/2 path had no equivalent — every field arrives at `HeaderCompression` as a `(name, value)` pair and is encoded verbatim: ```scala case (key, value: String) => encoder.encodeHeader(os, key, value, false) ``` The value can be attacker-influenced: a `RawHeader`/`CustomHeader` set from user data, or a response trailer (`Trailer(...)`, whose headers bypass `renderHeaders` entirely). RFC 9113 §8.2.1 forbids NUL, CR and LF in a field name or value. On a native HTTP/2 leg HPACK is length-prefixed, so this is not direct frame splitting, but: - it violates the spec, and - it enables **HTTP/2 → HTTP/1.1 downgrade smuggling** when an intermediary re-serialises the message, and - the CR/LF mitigation an application relies on under HTTP/1.1 **silently disappears** under HTTP/2. ### Modification `HeaderCompression` is the single point every outgoing header field passes through before HPACK encoding — regular headers, response trailers, and pseudo-headers all converge here as key/value pairs. Drop any field whose name or value contains CR, LF or NUL, logging at `debug`. `debug` rather than `warning`: the value can be attacker-influenced (reflected into a header), so a warning would be a log-flooding vector, and the HTTP/1.1 renderer drops such headers silently too. ### Result CR/LF/NUL in an HTTP/2 header name or value can no longer reach the wire — the offending field is dropped, matching the HTTP/1.1 renderer, whether it rides in the header block or a response trailer. ### Tests - `sbt "http2-tests/testOnly org.apache.pekko.http.impl.engine.http2.Http2ServerSpec"` — pass (125 tests). Two new tests assert a CRLF-bearing response header and a CRLF-bearing response trailer header are dropped, while a valid sibling trailer (`x-good`) survives. Verified both fail with the fix stashed (the injected `set-cookie` reaches the decoded headers). - `sbt http-core/mimaReportBinaryIssues` — pass (internal `impl.engine.http2` change, no public API). - Native `scalafmt` on the changed files — clean. This is the HTTP/2 companion to the HTTP/1.1 chunk trailer/extension CR/LF fix in #1256. ### References None - aligns HTTP/2 header rendering with the HTTP/1.1 CR/LF guard (RFC 9113 §8.2.1) 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
