Reject connection-specific headers sent via HPACK/QPACK indexed names - #565
Closed
DeagleGross wants to merge 1 commit into
Closed
Reject connection-specific headers sent via HPACK/QPACK indexed names#565DeagleGross wants to merge 1 commit into
DeagleGross wants to merge 1 commit into
Conversation
Connection-specific header fields (e.g. Transfer-Encoding) were only validated on the literal name+value path. Using HPACK's indexed-name representation (transfer-encoding is static table index 57) routed the header through OnStaticIndexedHeader/OnDynamicIndexedHeader, skipping the IsConnectionSpecificHeaderField check and allowing Transfer-Encoding: chunked through on HTTP/2. HTTP/3 shares the same structural gap via QPACK dynamic-table indexed names. Move the connection-specific header check so it runs for every header representation in both Http2Connection and Http3Stream. No legitimate headers are affected. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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.
Summary
Fixes a gap in dotnet#66669. Connection-specific header fields (
connection,transfer-encoding,keep-alive,proxy-connection,upgrade,te) were only rejected when sent as an HPACK/QPACK literal name + value. When the header name was sent using an indexed-name representation, the check was skipped.Concretely,
transfer-encodingis entry 57 in the HPACK static table. A request encoding it as Literal Header Field — Indexed Name (index 57) with a literal valuechunkedwas routed throughOnStaticIndexedHeader(index, value)→HeaderType.StaticAndValue, which never calledValidateHeaderContent, soIsConnectionSpecificHeaderFieldwas never evaluated. This allowedTransfer-Encoding: chunkedthrough on HTTP/2.HTTP/3 has the same structural gap: QPACK's static table contains no connection-specific header, but an entry inserted into the QPACK dynamic table via the encoder stream (which isn't validated on that path) could then be referenced by a dynamic indexed name (
HeaderType.Dynamic), bypassing the same check.Fix
Move the
IsConnectionSpecificHeaderFieldcheck out ofValidateHeaderContentand to the top ofOnHeaderCorein bothHttp2ConnectionandHttp3Stream, so it runs for every header representation — fully indexed, indexed-name + literal value, and literal name + value — as well as trailers. The decodedname/valuespans are always populated for these callbacks, so no additional decoding is required.Only connection-specific header fields are affected; no other/legitimate headers change behavior. Headers that were already validated on the literal path behave exactly as before.
Tests
HEADERS_Received_TransferEncodingWithHPackIndexedName_ConnectionError, which sends a hand-crafted HPACK block referencingtransfer-encodingby static index 57 with a literalchunkedvalue and asserts aPROTOCOL_ERRORconnection error withHttpErrorConnectionSpecificHeaderField.Refs dotnet#66669.