Skip to content

Conversation

@bneradt
Copy link
Contributor

@bneradt bneradt commented Oct 25, 2021

A continuation of PR #7622 but against the 10-Dev branch instead of master. Includes fixes from several months of production testing.

@bneradt bneradt added the HTTP/2 label Oct 25, 2021
@bneradt bneradt added this to the 10.0.0 milestone Oct 25, 2021
@bneradt bneradt requested a review from shinrich October 25, 2021 15:30
@bneradt bneradt self-assigned this Oct 25, 2021
@bneradt
Copy link
Contributor Author

bneradt commented Oct 25, 2021

The AuTest fails because 10-Dev needs to be rebased to contain the fix for the newer jsonschema handling:
#8370

I'll work on rebasing 10-Dev.

shinrich and others added 7 commits October 25, 2021 20:33
In the HttpSM, _netvc is set when a connection is made. If the
connection fails, however, the _netvc may not be set.  This patch
handles this situation rather than tripping an assertion if _netvc is
nullptr when handling the NET_EVENT_OPEN_FAILED event.
@bneradt bneradt force-pushed the http2_to_origin_10_Dev branch from 98152a6 to 01d7063 Compare October 25, 2021 20:37
@keesspoelstra
Copy link
Contributor

We have an HPACK corruption issue when we ignore response headers on H2 to origin, this was observed on a Pulse Secure VTM.

  1. Request A: Proxy sends request headers to origin
  2. Request A: Origin responds with RST stream for whatever reason
  3. Request A: Proxy deletes stream
  4. Request A: Origin sends response headers
  5. Request A: Proxy ignores and sends RST on stream
  6. Request A: Origin sends data
  7. Request A: Proxy ignores and sends RST
  8. Request B: Proxy sends request headers to origin
  9. . Request B: Origin sends response headers
  10. Request B: Proxy processes headers (HPACK)
  11. . Request B: Proxy ignores and sends RST on stream
  12. . Request B: Origin sends data
  13. . Request B: Proxy ignores and sends RST

Because we ignored the response headers for request A in 5, we missed updating the HPACK table.
When reaching 10 and processing the response headers for request B we miss entries in our HPACK and possibly generate corrupt headers.
If we choose to RST streams or get an RST on a stream, we should still process headers to keep the HPACK table intact.
We could also choose to send a GOAWAY when we see this happening, but finishing or retrying in flight transactions will be challenging if they're not idempotent. The subsequent response headers of an inflight POST might not be salvageable.

I think we're safe on the client to proxy side, as it seems that we're processing all request headers in sequence and never drop before HPACK processing.

@github-actions
Copy link

github-actions bot commented Feb 2, 2022

This pull request has been automatically marked as stale because it has not had recent activity. Marking it stale to flag it for further consideration by the community.

@github-actions github-actions bot added the Stale label Feb 2, 2022
@github-actions github-actions bot closed this Feb 9, 2022
@zwoop zwoop removed this from the 10.0.0 milestone Jan 17, 2023
@bneradt bneradt deleted the http2_to_origin_10_Dev branch March 29, 2023 19:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants