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

Whitespace trimmed when header value split across tcp packets #12197

Closed
jeremybaumont opened this issue Jul 21, 2020 · 3 comments
Closed

Whitespace trimmed when header value split across tcp packets #12197

jeremybaumont opened this issue Jul 21, 2020 · 3 comments
Labels
question Questions that are neither investigations, bugs, nor enhancements

Comments

@jeremybaumont
Copy link

jeremybaumont commented Jul 21, 2020

Whitespace trimmed when header value split across tcp packets

Description

Whitespace trimming issue when a header value is split across 2 tcp packets where a whitespace character (or many whitespaces) is the last char of the first packet or the first char of the second packet.

We noticed reports from our users complaining of intermittent 4xx due to corrupted Authorization header value where the space between the token type (Bearer) and the encoded token string disappeared.

We tracked down the issue to envoy with tcpdump captures of incoming and outgoing traffic. And we could reproduce the issue locally.

Repro steps

  1. Clone repository at https://github.com/jeremybaumont/envoy-httpbin , it uses envoy as front proxy and httpbin as upstream service.We wrote a script to simulate the split between 2 tcp packets at https://gist.github.com/jeremybaumont/f5d7ddc63f6a3a75431a4a6a5016efbe The script call the headers endpoint of the httpbin service that displays the headers of the http request.

  2. Start the docker-compose and run the above script:

docker-compose build --build-arg ENVOY_VERSION=v1.14.4
docker-compose up -d
curl -sSL https://gist.githubusercontent.com/jeremybaumont/f5d7ddc63f6a3a75431a4a6a5016efbe/raw| python -
docker-compose down

The output of the script is similar to:

HTTP/1.1 200 OK
server: envoy
date: Mon, 20 Jul 2020 23:44:43 GMT
content-type: application/json
content-length: 235
access-control-allow-origin: *
access-control-allow-credentials: true
x-envoy-upstream-service-time: 1

{
  "headers": {
    "Accept": "application/json",
    "Content-Length": "0",
    "Host": "localhost:8080",
    "User-Agent": "foo",
    "X-Envoy-Expected-Rq-Timeout-Ms": "15000",
    "X-Header-With-Space": "SpaceComesHere"
  }
}

BUG!

We observed that >= v1.12.2, v1.13.x, v1.14.x versions were affected, but prior v1.12.1 and v1.15.0 are not affected .

for version in v1.14.1 v1.14.2 v1.14.3 v1.14.4 v1.15.0; do docker-compose build --build-arg ENVOY_VERSION=$version; docker-compose up -d --no-build; curl -sSL   https://gist.githubusercontent.com/jeremybaumont/f5d7ddc63f6a3a75431a4a6a5016efbe/raw | python - ; curl -s http://localhost:8001/server_info | grep '"version"'; docker-compose down; done

We think the bug is introduced by the following line but not sure: https://github.com/envoyproxy/envoy/blob/master/source/common/http/http1/codec_impl.cc#L613

We don’t know if v1.15.0 fixed the bug accidentally, we did not see much github issues reporting the same symptoms (except #10270) and could not identify something related in the changelog of v1.15.0 https://www.envoyproxy.io/docs/envoy/latest/version_history/v1.15.0.

We would like to make sure regression tests are implemented if not yet present, and hope by reporting the symptoms, it will avoid extra troubleshooting to other users that may be affected.

@mattklein123
Copy link
Member

cc @antoniovicente who I believe fixed this on purpose recently so there should be a test.

@mattklein123 mattklein123 added the question Questions that are neither investigations, bugs, nor enhancements label Jul 21, 2020
@antoniovicente
Copy link
Contributor

antoniovicente commented Jul 21, 2020

Yes, this issue looks like a dup of #10270

The fix is #10886 also known as
b962a8e

The fix is part of the v1.15.0 release.

@jeremybaumont
Copy link
Author

Nice, thanks for the quick answer folks, we really appreciate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Questions that are neither investigations, bugs, nor enhancements
Projects
None yet
Development

No branches or pull requests

3 participants