Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Harden against HTTP Desync attacks #3444

Open
Sytten opened this issue Nov 24, 2023 · 2 comments
Open

Harden against HTTP Desync attacks #3444

Sytten opened this issue Nov 24, 2023 · 2 comments
Labels
B-rfc Blocked: More comments would be useful in determine next steps. C-feature Category: feature. This is adding a new feature. S-waiting-on-author Status: waiting on the author to provide more info, or make changes.

Comments

@Sytten
Copy link

Sytten commented Nov 24, 2023

Problem
HTTP Desync attacks are pretty bad and there are a lot of bad/permissive http implementations out there that act as proxies.
I didn't see any discussion / tests of hyper surrounding that so I wanted to start the discussion around the topic given that hyper is now 1.0 (and used in production in a lot of places).

Ideas

  • A suite of tests to demonstrate that an hyper server reacts as best it can (given it doesn't control the proxy)
  • Creating a flag for hardened/more strict version of hyper (server mostly)

For the second point, I would take a lot of work that has been done by AWS already in https://github.com/aws/http-desync-guardian and try to incorporate it in hyper where it makes sense. This includes going more strict than the spec to avoid confusions.

Things like ignoring a CL when a TE is present is risky (even if the spec says that it must be ignored) since a proxy might very well not ignore the CL. So even if hyper is "correct", that won't prevent a leak. Under an hypothetical strict flag, this would result in the request being rejected.

hyper/src/proto/h1/role.rs

Lines 251 to 253 in 210bfaa

if is_te {
continue;
}

Additional context

@Sytten Sytten added the C-feature Category: feature. This is adding a new feature. label Nov 24, 2023
@seanmonstar
Copy link
Member

Thanks for the suggestion! I agree with the premise, for sure! I have mentioned in other issues the idea of having a "strict" option. (I kinda wonder if it would eventually need to be made multiple options, as people want some strict things but not others.) Another point I think is to consider whether these things need to live in hyper itself, or if it can be part of recommended middleware.

More tests is always welcome, too!

@Sytten
Copy link
Author

Sytten commented Nov 25, 2023

Most checks can probably live in middlewares but desync attack operate on the write buffer of the connection to leak data of other users when the proxy reuses the same connection. So I think this would need to live in hyper. Regarding the multiple flags I do agree, maybe similar to the desync attack prevention modes provided by AWS for the ALB?

@seanmonstar seanmonstar added B-rfc Blocked: More comments would be useful in determine next steps. S-waiting-on-author Status: waiting on the author to provide more info, or make changes. labels Feb 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B-rfc Blocked: More comments would be useful in determine next steps. C-feature Category: feature. This is adding a new feature. S-waiting-on-author Status: waiting on the author to provide more info, or make changes.
Projects
None yet
Development

No branches or pull requests

2 participants