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

Upstream H2 connection reset when sending invalid TE header value #30362

Closed
dvilaverde opened this issue Oct 20, 2023 · 4 comments · Fixed by #30535
Closed

Upstream H2 connection reset when sending invalid TE header value #30362

dvilaverde opened this issue Oct 20, 2023 · 4 comments · Fixed by #30535
Assignees
Labels
area/http bug no stalebot Disables stalebot from closing an issue

Comments

@dvilaverde
Copy link

dvilaverde commented Oct 20, 2023

Title: Upstream H2 connection reset when sending invalid TE header value

Description:
We have a scenario where a client using HTTP/1.1 will receive a 503 error if the upstream is using H2 and if the request contains the TE header with a value other than trailers. This is how the proxies are setup between the client and the application:

[ Client ] ---HTTP/1.1---> [ Envoy ] ---HTTP/2---> [ Envoy ] ---HTTP/1.1---> [ Application ]

When this happens the connection appears to be reset and any other client requests using the same H2 connection seems to also be reset.

The envoy proxy I'm have control over is the first in the chain and that is running v1.27.2, and I expected that envoy would not forward HTTP headers that are not valid for HTTP/2 to the upstream.

Repro steps:

I've created this docker compose to simulate the problem:

version: '2.4'
services:
  envoy-proxy1:
    image: envoyproxy/envoy:v1.27.2
    command: 
      - envoy
      - --log-level trace
      - -c /etc/envoy/envoy.yaml
    hostname: envoy1
    ports:
      - "9090:8080"
    volumes:
      - ./envoy1.yaml:/etc/envoy/envoy.yaml
  
  envoy-proxy2:
    image: envoyproxy/envoy:v1.27.2
    command: 
      - envoy
      - --log-level trace
      - -c /etc/envoy/envoy.yaml
    hostname: envoy2
    volumes:
      - ./envoy2.yaml:/etc/envoy/envoy.yaml
  
  httpbin:
    image: kennethreitz/httpbin
    hostname: httpbin
    ports:
      - "9092:80"

To reproduce I used this cURL request:

curl -v --http1.1 -H 'te: chunked;q=1.0' 'localhost:9090/anything/testing?test=1'

and get this response:

*   Trying 127.0.0.1:9090...
* Connected to localhost (127.0.0.1) port 9090 (#0)
> GET /anything/testing?test=1 HTTP/1.1
> Host: localhost:9090
> User-Agent: curl/8.1.2
> Accept: */*
> te: chunked;q=1.0
> 
< HTTP/1.1 503 Service Unavailable
< content-length: 95
< content-type: text/plain
< date: Sat, 21 Oct 2023 02:38:04 GMT
< server: test
< 
* Connection #0 to host localhost left intact
upstream connect error or disconnect/reset before headers. reset reason: connection terminatio

If I remove the "te: chunked;q=1.0" or replace the value with trailers then the request succeeds with HTTP 200 status code as expected.

Config:

These are the config files for each envoy instance in the above docker-compose file.

envoy1.yaml

admin:
  address:
    socket_address: { address: 0.0.0.0, port_value: 9901 }

static_resources:
  listeners:
  - name: listener_0
    address:
      socket_address: { address: 0.0.0.0, port_value: 8080 }
    filter_chains:
    - filters:
      - name: envoy.filters.network.http_connection_manager
        typed_config:
          "@type": type.googleapis.com/envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager
          stat_prefix: ingress_http
          codec_type: AUTO
          route_config:
            name: local_route
            virtual_hosts:
            - name: local_service
              domains: ["*"]
              routes:
              - match: { prefix: "/" }
                route: { cluster: envoy2_service }
          http_filters:
          - name: envoy.filters.http.router
            typed_config:
              "@type": type.googleapis.com/envoy.extensions.filters.http.router.v3.Router
              suppress_envoy_headers: 
          http_protocol_options:
            accept_http_10: true
            default_host_for_http_10: "*"
          server_name: test
    enable_reuse_port: false
  clusters:
  - name: envoy2_service
    connect_timeout: 1s
    type: LOGICAL_DNS
    lb_policy: ROUND_ROBIN
    upstream_connection_options:
      tcp_keepalive:
        keepalive_interval: 30
        keepalive_time: 30
    load_assignment:
      cluster_name: envoy2_service
      endpoints:
      - lb_endpoints:
        - endpoint:
            address:
              socket_address:
                address: envoy2
                port_value: 8080
    typed_extension_protocol_options:
      "envoy.extensions.upstreams.http.v3.HttpProtocolOptions":
        "@type": "type.googleapis.com/envoy.extensions.upstreams.http.v3.HttpProtocolOptions"
        common_http_protocol_options:
          idle_timeout: 30s
          max_requests_per_connection: 1000
        explicit_http_config:
          http2_protocol_options:
            max_concurrent_streams: 64

envoy2.yaml

admin:
  address:
    socket_address: { address: 0.0.0.0, port_value: 9901 }

static_resources:
  listeners:
  - name: listener_0
    address:
      socket_address: { address: 0.0.0.0, port_value: 8080 }
    filter_chains:
    - filters:
      - name: envoy.filters.network.http_connection_manager
        typed_config:
          "@type": type.googleapis.com/envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager
          stat_prefix: ingress_http
          codec_type: AUTO
          route_config:
            name: local_route
            virtual_hosts:
            - name: local_service
              domains: ["*"]
              routes:
              - match: { prefix: "/" }
                route: { cluster: http_bin }
          http_filters:
          - name: envoy.filters.http.router
            typed_config:
              "@type": type.googleapis.com/envoy.extensions.filters.http.router.v3.Router
  clusters:
  - name: http_bin
    connect_timeout: 0.25s
    type: LOGICAL_DNS
    lb_policy: ROUND_ROBIN
    load_assignment:
      cluster_name: http_bin
      endpoints:
      - lb_endpoints:
        - endpoint:
            address:
              socket_address:
                address: httpbin
                port_value: 80

Logs:

The first envoy proxy in the request path doesn't produce any error log entries but I do have this entry from the second envoy proxy though.

[2023-10-21 02:49:06.149][31][debug][http2] [source/common/http/http2/codec_impl.cc:1250] [Tags: "ConnectionId":"0"] invalid http2: Invalid HTTP header field was received: frame type: 1, stream: 3, name: [te], value: [chunked;q=1.0]

Here are the complete logs: trace_logs.txt

@dvilaverde dvilaverde added bug triage Issue requires triage labels Oct 20, 2023
@dvilaverde dvilaverde changed the title Upstream H2 connection reset when receiving invalid TE header value Upstream H2 connection reset when sending invalid TE header value Oct 20, 2023
@kyessenov kyessenov added area/http and removed triage Issue requires triage labels Oct 22, 2023
Copy link

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or "no stalebot" or other activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale stalebot believes this issue/PR has not been touched recently label Nov 22, 2023
@dvilaverde
Copy link
Author

@alyssawilk I see you created a PR for this issue. Is that something that will realistically be released in the next few months? Also can we keep this issue from being closed by the "stalebot"?

@github-actions github-actions bot removed the stale stalebot believes this issue/PR has not been touched recently label Nov 22, 2023
@alyssawilk alyssawilk added the no stalebot Disables stalebot from closing an issue label Nov 29, 2023
@alyssawilk alyssawilk self-assigned this Nov 29, 2023
@alyssawilk
Copy link
Contributor

tagged nostalebot. it's in my queue but I've also been on leave so when it failed CI and I couldn't repro locally it got punted down the priority queue. I'll try to knock it out in December so it makes the January release

RyanTheOptimist pushed a commit that referenced this issue Nov 30, 2023
Changes Envoy to clear TE header by default, as it is a hop by hop header and should not be forwarded on.

Risk Level: medium (behavioral change)
Testing: new e2e test
Docs Changes: n/a
Release Notes: inline.
[Optional Runtime guard:] yes.
Fixes #30362

Signed-off-by: Alyssa Wilk <[email protected]>
@alyssawilk
Copy link
Contributor

follow-up PR #32255

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/http bug no stalebot Disables stalebot from closing an issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants