Skip to content

http: Strip illegal headers from CONNECT responses#1699

Merged
olix0r merged 1 commit intomainfrom
ver/h1-strip-connect
May 23, 2022
Merged

http: Strip illegal headers from CONNECT responses#1699
olix0r merged 1 commit intomainfrom
ver/h1-strip-connect

Conversation

@olix0r
Copy link
Member

@olix0r olix0r commented May 23, 2022

When the proxy transports CONNECT requests, the client silently
ignores content-length and transport-encoding response headers, as
it should. When the proxy passees these headers to the hyper server, it
errors (as is dictated by the RFC).

This change updates the HTTP client to remove these headers from
responses so that the proxy no longer propagates these illegal headers.

Fixes linkerd/linkerd2#8539

Signed-off-by: Oliver Gould [email protected]

When the proxy transports `CONNECT` requests, the client silently
ignores `content-length` and `transport-encoding` response headers, as
it should. When the proxy passees these headers to the hyper server, it
errors (as is dictated by the RFC).

This change updates the HTTP client to remove these headers from
responses so that the proxy no longer propagates these illegal headers.

Fixes linkerd/linkerd2#8539

Signed-off-by: Oliver Gould <[email protected]>
@olix0r olix0r requested a review from a team as a code owner May 23, 2022 21:13
@olix0r
Copy link
Member Author

olix0r commented May 23, 2022

Reviewed/approved in #1698

@olix0r olix0r merged commit 5431c44 into main May 23, 2022
@olix0r olix0r deleted the ver/h1-strip-connect branch May 23, 2022 21:14
olix0r added a commit to linkerd/linkerd2 that referenced this pull request May 24, 2022
This release fixes a problem with HTTP/1.1 `CONNECT` requests. When a
server responds to a `CONNECT` request with `content-length` or
`transfer-encoding` headers (in violation of RFC 7231), the proxy must
actively strip these headers to avoid making the Hyper server fail the
response.

This release also fixes an issue with the way proxies discover control
plane components via DNS. When `SRV` records cannot be resolved, the
proxy would no not necessarily fall back to resolving `A` records. This
has been fixed.

Finally, the inbound proxy can now discover policies dynamically. Ports
that are configured in the `LINKERD2_PROXY_INBOUND_PORTS` are discovered
as the proxy starts up; but now the proxy will discover policies for
ports that are not in this list. The pod's default policy is used
initially, but once a policy is received from the control plane it is
used.

---

* build(deps): bump syn from 1.0.93 to 1.0.94 (linkerd/linkerd2-proxy#1664)
* build(deps): bump tj-actions/changed-files from 19 to 20 (linkerd/linkerd2-proxy#1665)
* build(deps): bump rustls from 0.20.4 to 0.20.5 (linkerd/linkerd2-proxy#1666)
* build(deps): bump ryu from 1.0.9 to 1.0.10 (linkerd/linkerd2-proxy#1667)
* build(deps): bump tokio-util from 0.7.1 to 0.7.2 (linkerd/linkerd2-proxy#1668)
* build(deps): bump itoa from 1.0.1 to 1.0.2 (linkerd/linkerd2-proxy#1669)
* build(deps): bump tonic from 0.7.1 to 0.7.2 (linkerd/linkerd2-proxy#1652)
* dns: Fall back to A record when SRV resolution fails (linkerd/linkerd2-proxy#1670)
* Consolidate error chain inspection with `cause_ref` (linkerd/linkerd2-proxy#1671)
* ci: change how warnings are denied on CI (linkerd/linkerd2-proxy#1662)
* build(deps): bump proc-macro2 from 1.0.38 to 1.0.39 (linkerd/linkerd2-proxy#1673)
* build(deps): bump libc from 0.2.125 to 0.2.126 (linkerd/linkerd2-proxy#1674)
* build(deps): bump syn from 1.0.94 to 1.0.95 (linkerd/linkerd2-proxy#1675)
* ci: Use the cargo-action-fmt setup action (linkerd/linkerd2-proxy#1672)
* opencensus: Include empty generated protobuf (linkerd/linkerd2-proxy#1676)
* build(deps): bump rustls from 0.20.5 to 0.20.6 (linkerd/linkerd2-proxy#1679)
* Revert "build(deps): bump socket2 from 0.4.4 to 0.4.5 (linkerd/linkerd2-proxy#1654)" (#1681)
* build(deps): bump EmbarkStudios/cargo-deny-action from 1.2.17 to 1.3.0 (linkerd/linkerd2-proxy#1678)
* build(deps): bump clang-sys from 1.3.1 to 1.3.2 (linkerd/linkerd2-proxy#1680)
* cache: generalize `Cache` into a key-value cache (linkerd/linkerd2-proxy#1683)
* build(deps): bump once_cell from 1.10.0 to 1.11.0 (linkerd/linkerd2-proxy#1687)
* build(deps): bump EmbarkStudios/cargo-deny-action from 1.3.0 to 1.3.1 (linkerd/linkerd2-proxy#1686)
* trace: add `/logs.json` endpoint to admin server (linkerd/linkerd2-proxy#1642)
* Dynamically discover policies for undocumented ports (linkerd/linkerd2-proxy#1677)
* Don't allow a policy to be used if `check_port_allowed` fails (linkerd/linkerd2-proxy#1689)
* ci: Simplify release workflow (linkerd/linkerd2-proxy#1690)
* build(deps): bump petgraph from 0.6.0 to 0.6.1 (linkerd/linkerd2-proxy#1696)
* build(deps): bump actions/upload-artifact from 3.0.0 to 3.1.0 (linkerd/linkerd2-proxy#1692)
* build(deps): bump tj-actions/changed-files from 20 to 20.1 (linkerd/linkerd2-proxy#1693)
* build(deps): bump http-body from 0.4.4 to 0.4.5 (linkerd/linkerd2-proxy#1694)
* build(deps): bump regex from 1.5.5 to 1.5.6 (linkerd/linkerd2-proxy#1695)
* build(deps): bump regex-syntax from 0.6.25 to 0.6.26 (linkerd/linkerd2-proxy#1697)
* http: Strip illegal headers from CONNECT responses (linkerd/linkerd2-proxy#1699)
* dev: Replace `Makefile` with `justfile` (linkerd/linkerd2-proxy#1691)

Signed-off-by: Oliver Gould <[email protected]>
olix0r added a commit to linkerd/linkerd2 that referenced this pull request May 24, 2022
This release fixes a problem with HTTP/1.1 `CONNECT` requests. When a
server responds to a `CONNECT` request with `content-length` or
`transfer-encoding` headers (in violation of RFC 7231), the proxy must
actively strip these headers to avoid making the Hyper server fail the
response.

This release also fixes an issue with the way proxies discover control
plane components via DNS. When `SRV` records cannot be resolved, the
proxy would no not necessarily fall back to resolving `A` records. This
has been fixed.

Finally, the inbound proxy can now discover policies dynamically. Ports
that are configured in the `LINKERD2_PROXY_INBOUND_PORTS` are discovered
as the proxy starts up; but now the proxy will discover policies for
ports that are not in this list. The pod's default policy is used
initially, but once a policy is received from the control plane it is
used.

---

* build(deps): bump syn from 1.0.93 to 1.0.94 (linkerd/linkerd2-proxy#1664)
* build(deps): bump tj-actions/changed-files from 19 to 20 (linkerd/linkerd2-proxy#1665)
* build(deps): bump rustls from 0.20.4 to 0.20.5 (linkerd/linkerd2-proxy#1666)
* build(deps): bump ryu from 1.0.9 to 1.0.10 (linkerd/linkerd2-proxy#1667)
* build(deps): bump tokio-util from 0.7.1 to 0.7.2 (linkerd/linkerd2-proxy#1668)
* build(deps): bump itoa from 1.0.1 to 1.0.2 (linkerd/linkerd2-proxy#1669)
* build(deps): bump tonic from 0.7.1 to 0.7.2 (linkerd/linkerd2-proxy#1652)
* dns: Fall back to A record when SRV resolution fails (linkerd/linkerd2-proxy#1670)
* Consolidate error chain inspection with `cause_ref` (linkerd/linkerd2-proxy#1671)
* ci: change how warnings are denied on CI (linkerd/linkerd2-proxy#1662)
* build(deps): bump proc-macro2 from 1.0.38 to 1.0.39 (linkerd/linkerd2-proxy#1673)
* build(deps): bump libc from 0.2.125 to 0.2.126 (linkerd/linkerd2-proxy#1674)
* build(deps): bump syn from 1.0.94 to 1.0.95 (linkerd/linkerd2-proxy#1675)
* ci: Use the cargo-action-fmt setup action (linkerd/linkerd2-proxy#1672)
* opencensus: Include empty generated protobuf (linkerd/linkerd2-proxy#1676)
* build(deps): bump rustls from 0.20.5 to 0.20.6 (linkerd/linkerd2-proxy#1679)
* Revert "build(deps): bump socket2 from 0.4.4 to 0.4.5 (linkerd/linkerd2-proxy#1654)" (#1681)
* build(deps): bump EmbarkStudios/cargo-deny-action from 1.2.17 to 1.3.0 (linkerd/linkerd2-proxy#1678)
* build(deps): bump clang-sys from 1.3.1 to 1.3.2 (linkerd/linkerd2-proxy#1680)
* cache: generalize `Cache` into a key-value cache (linkerd/linkerd2-proxy#1683)
* build(deps): bump once_cell from 1.10.0 to 1.11.0 (linkerd/linkerd2-proxy#1687)
* build(deps): bump EmbarkStudios/cargo-deny-action from 1.3.0 to 1.3.1 (linkerd/linkerd2-proxy#1686)
* trace: add `/logs.json` endpoint to admin server (linkerd/linkerd2-proxy#1642)
* Dynamically discover policies for undocumented ports (linkerd/linkerd2-proxy#1677)
* Don't allow a policy to be used if `check_port_allowed` fails (linkerd/linkerd2-proxy#1689)
* ci: Simplify release workflow (linkerd/linkerd2-proxy#1690)
* build(deps): bump petgraph from 0.6.0 to 0.6.1 (linkerd/linkerd2-proxy#1696)
* build(deps): bump actions/upload-artifact from 3.0.0 to 3.1.0 (linkerd/linkerd2-proxy#1692)
* build(deps): bump tj-actions/changed-files from 20 to 20.1 (linkerd/linkerd2-proxy#1693)
* build(deps): bump http-body from 0.4.4 to 0.4.5 (linkerd/linkerd2-proxy#1694)
* build(deps): bump regex from 1.5.5 to 1.5.6 (linkerd/linkerd2-proxy#1695)
* build(deps): bump regex-syntax from 0.6.25 to 0.6.26 (linkerd/linkerd2-proxy#1697)
* http: Strip illegal headers from CONNECT responses (linkerd/linkerd2-proxy#1699)
* dev: Replace `Makefile` with `justfile` (linkerd/linkerd2-proxy#1691)

Signed-off-by: Oliver Gould <[email protected]>
hawkw pushed a commit that referenced this pull request Jun 30, 2022
When the proxy transports `CONNECT` requests, the client silently
ignores `content-length` and `transport-encoding` response headers, as
it should. When the proxy passees these headers to the hyper server, it
errors (as is dictated by the RFC).

This change updates the HTTP client to remove these headers from
responses so that the proxy no longer propagates these illegal headers.

Fixes linkerd/linkerd2#8539

Signed-off-by: Oliver Gould <[email protected]>
hawkw pushed a commit that referenced this pull request Jun 30, 2022
When the proxy transports `CONNECT` requests, the client silently
ignores `content-length` and `transport-encoding` response headers, as
it should. When the proxy passees these headers to the hyper server, it
errors (as is dictated by the RFC).

This change updates the HTTP client to remove these headers from
responses so that the proxy no longer propagates these illegal headers.

Fixes linkerd/linkerd2#8539

Signed-off-by: Oliver Gould <[email protected]>
hawkw added a commit to linkerd/linkerd2 that referenced this pull request Jul 1, 2022
large number of service discovery updates, for incorrect handling of
`CONNECT` requests with `Content-Length` headers, and for a failure to
correctly resolve DNS SRV records for the control plane.

---

1817b3f3 update Tower to 0.4.13 to fix load balancer panic (linkerd/linkerd2-proxy#1758)
bc512765 dns: Fall back to A record when SRV resolution fails (linkerd/linkerd2-proxy#1670)
ac1039b0 http: Strip illegal headers from CONNECT responses (linkerd/linkerd2-proxy#1699)
ebcf7cbd Dedupe discovery updates (linkerd/linkerd2-proxy#1759)
hawkw added a commit to linkerd/linkerd2 that referenced this pull request Jul 1, 2022
This release cherry-picks commits from the `main` branch into the
`release/v2.161` branch, including fixes for a panic when processing a
large number of service discovery updates, for incorrect handling of
`CONNECT` requests with `Content-Length` headers, and for a failure to
correctly resolve DNS SRV records for the control plane.

---

1817b3f3 update Tower to 0.4.13 to fix load balancer panic (linkerd/linkerd2-proxy#1758)
bc512765 dns: Fall back to A record when SRV resolution fails (linkerd/linkerd2-proxy#1670)
ac1039b0 http: Strip illegal headers from CONNECT responses (linkerd/linkerd2-proxy#1699)
ebcf7cbd Dedupe discovery updates (linkerd/linkerd2-proxy#1759)
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.

Linkerd drops CONNECT requests with content-length header set

1 participant