fix: drop HTTP/2 header fields containing CR, LF or NUL (#1258) - #1271
Open
pjfanning wants to merge 1 commit into
Open
fix: drop HTTP/2 header fields containing CR, LF or NUL (#1258)#1271pjfanning wants to merge 1 commit into
pjfanning wants to merge 1 commit into
Conversation
Motivation: Outgoing HTTP/2 header fields were HPACK-encoded without checking 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, but the HTTP/2 path had no equivalent, so an attacker-influenced value (a RawHeader, CustomHeader or a response trailer built from user data) was encoded verbatim. RFC 9113 8.2.1 forbids these characters 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 enables HTTP/2 -> HTTP/1.1 downgrade smuggling when an intermediary re-serialises the message, and it means the mitigation an application relies on under HTTP/1.1 silently disappears under HTTP/2. Modification: In `HeaderCompression`, the single point every outgoing header field (regular headers, trailers and pseudo-headers all arrive here as key/value pairs) passes through before HPACK encoding, drop any field whose name or value contains CR, LF or NUL, logging at debug. Debug rather than warning because the value can be attacker-influenced, so a warning would be a log-flooding vector, and the HTTP/1.1 renderer drops 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 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). References: None - aligns HTTP/2 header rendering with the HTTP/1.1 CR/LF guard (RFC 9113 8.2.1)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
cherry pick 46d2ffc #1258
Motivation:
Outgoing HTTP/2 header fields were HPACK-encoded without checking 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, but the HTTP/2 path had no equivalent, so an attacker-influenced value (a RawHeader, CustomHeader or a response trailer built from user data) was encoded verbatim. RFC 9113 8.2.1 forbids these characters 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 enables HTTP/2 -> HTTP/1.1 downgrade smuggling when an intermediary re-serialises the message, and it means the mitigation an application relies on under HTTP/1.1 silently disappears under HTTP/2.
Modification:
In
HeaderCompression, the single point every outgoing header field (regular headers, trailers and pseudo-headers all arrive here as key/value pairs) passes through before HPACK encoding, drop any field whose name or value contains CR, LF or NUL, logging at debug. Debug rather than warning because the value can be attacker-influenced, so a warning would be a log-flooding vector, and the HTTP/1.1 renderer drops 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:
References:
None - aligns HTTP/2 header rendering with the HTTP/1.1 CR/LF guard (RFC 9113 8.2.1)