Apply safe-by-default H2Options on HTTP/2 handshake#903
Open
pigri wants to merge 1 commit into
Open
Conversation
h2's defaults allow a 16 MiB decoded header list and an effectively unbounded number of concurrent inbound streams. An HTTP/2 listener that does not set `H2Options` can therefore be exhausted by an HPACK dynamic-table bomb combined with a flow-control window stall, a memory-exhaustion denial of service. Route the `None` case in `handshake` through a new `safe_h2_options()` that bounds the decoded header-list size (64 KiB), concurrent inbound streams (100), and pending accept reset streams (32). Callers that pass their own `H2Options` keep full control and override every value. Signed-off-by: David Papp <pigri@users.noreply.github.com>
Collaborator
|
Thanks for your submission, we are working on this issue internally. |
Author
|
I understand I will close this pull request once your fix is implemented, if that's acceptable to you. |
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.
Problem
pingora_core::protocols::http::v2::server::handshakefalls back toh2's permissive defaults when the application does not provideH2Options:h2's defaultMAX_HEADER_LIST_SIZEis 16 MiB and the inbound concurrent-stream count is effectively unbounded (config.remote_max_initiated.unwrap_or(usize::MAX)). A remote HTTP/2 client can exploit this with an HPACK dynamic-table bomb plus a flow-control window stall:a: "", then emit many one-byte0xbeindexed references — each wire byte decodes to one header thath2accounts asname + value + 32 = 33bytes;SETTINGS_INITIAL_WINDOW_SIZE=0and dripWINDOW_UPDATEframes to keep the inflated request state resident.The result is a memory-exhaustion DoS against any Pingora HTTP/2 listener that does not explicitly set
app.h2_options.Public write-up and PoC exploit: https://github.com/califio/publications/tree/main/MADBugs/http2-bomb/pingora
Fix
Route the
Nonecase through a newsafe_h2_options()that bounds the decoded header-list size, concurrent inbound streams, and pending accept reset streams. Applications that pass their ownH2Optionsare unaffected and override every value.max_header_list_sizeis the lever that defeats the bomb:h2refuses a decoded header list above the cap before request state is allocated.Evidence
Minimal Pingora h2c server, no
H2Optionsset, attacked with the publishedhpack_bomb.py(64 streams × 32,000 headers + window stall), peak process RSS:unwrap_or_default)headers=31996)safe_h2_options)h2refuses the inflated header listSetting
H2Optionsexplicitly at the application layer already mitigates this; this change just makes the default safe so an unconfigured listener is not trivially exhausted.Notes
write_timeoutdefault is intentionally left unchanged: with the header-list and stream caps in place the parked footprint is bounded regardless of how long a client stalls the window, so a finite write timeout is defense-in-depth rather than required here.cargo build -p pingora-corepasses with the change.Signed-off-by.