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

http: disallow unknown transfer encodings #9316

Merged
merged 3 commits into from
Dec 13, 2019

Conversation

alyssawilk
Copy link
Contributor

This tests that Envoy does not proxy requests with content length and transfer encoding, protecting downstream servers from smuggling attacks.

This also blocks Envoy from proxying unknown transfer encodings. http_parser doesn't for example treat identity,chunked as chunked which is technically buggy. It's not an exploitable bug, because no http requests can be smuggles in a chunk length header but it's still poor form.

Minor improvements to matcher utils, now printing the headers that don't match because why wouldn't you?

Risk Level: medium (blocking legitimate but sketchy transfer encodings)
Testing: new unit tests, integration tests
Docs Changes: no
Release Notes: yes
Fixes https://github.com/envoyproxy/envoy-setec/issues/52
Fixes https://github.com/envoyproxy/envoy-setec/issues/50

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
Copy link
Contributor

@snowp snowp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mattklein123 mattklein123 merged commit 81f5a53 into envoyproxy:master Dec 13, 2019
prakhag1 pushed a commit to prakhag1/envoy that referenced this pull request Jan 3, 2020
Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
Signed-off-by: Prakhar <prakhar_au@yahoo.com>
@alyssawilk alyssawilk deleted the transfer branch May 18, 2020 14:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants