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

outbound: Do not support TCP-forwarding in ingress-mode #995

Merged
merged 4 commits into from
Apr 30, 2021

Conversation

olix0r
Copy link
Member

@olix0r olix0r commented Apr 29, 2021

The ingress-mode proxy has some simplistic support for TCP forwarding;
but this is somewhat of a misfeature. Specifically, we don't do any
mTLS/discovery for these connections, so detection failures can manifest
as unexpected insecure connections.

For now, we should remove TCP-forwarding support from ingress-mode
proxies. This will help support further simplifications to the
ingress-mode proxy.

The ingress-mode proxy has some simplistic support for TCP forwarding;
but this is somewhat of a misfeature. Specifically, we don't do any
mTLS/discovery for these connections, so detection failures can manifest
as unexpected insecure connections.

For now, we should remove TCP-forwarding support from ingress-mode
proxies. This will help support further simplifications to the
ingress-mode proxy.
@olix0r olix0r requested a review from a team April 29, 2021 20:29
Copy link
Contributor

@hawkw hawkw left a comment

Choose a reason for hiding this comment

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

looks good to me overall, couple very minor nits. is there a corresponding docs change we ought to make to remove indications that ingress mode can route TCP traffic?

/// Routes HTTP requests according to the l5d-dst-override header.
///
/// Forwards TCP connections without discovery/routing (or mTLS).
///
/// This is only intended for Ingress configurations, where we assume all
/// outbound traffic is either HTTP or TLS'd by the ingress proxy.
pub fn to_ingress<T, I, N, NSvc, H, HSvc, P>(
&self,
pub fn into_ingress<T, I, HSvc, P>(
Copy link
Contributor

Choose a reason for hiding this comment

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

nit/TIOLI: maybe this should be called into_http_ingress, or something? just to make it clearer that it is now http only.

Comment on lines 44 to 45
///
/// Forwards TCP connections without discovery/routing (or mTLS).
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be removed :)

Suggested change
///
/// Forwards TCP connections without discovery/routing (or mTLS).

@olix0r olix0r merged commit bae44e1 into main Apr 30, 2021
@olix0r olix0r deleted the ver/ingress-http-only branch April 30, 2021 21:08
olix0r added a commit to linkerd/linkerd2 that referenced this pull request May 12, 2021
This release simplifies internals so that endpoint-forwarding logic is
completely distinct from handling of load balanced services.

The ingress-mode outbound proxy has been simplified to *require* the
`l5d-dst-override` header and to fail non-HTTP communication. This
ensures that the ingress-mode proxy does not unexpectedly revert to
insecure communication.

Finally, a regression was recently introduced that caused all proxy logs
to be output with ANSI control characters. Logs are now output in
plaintext by default

---

* discover: replace `linkerd-channel` with `tokio-util` `PollSender` (linkerd/linkerd2-proxy#969)
* replace `linkerd-channel` with `tokio-stream` (linkerd/linkerd2-proxy#970)
* concurrency-limit: use `tokio-util`'s `PollSemaphore` (linkerd/linkerd2-proxy#968)
* http: Do not fail fuzz tests when all IO is not read (linkerd/linkerd2-proxy#973)
* transport: Fix orig-dst compilation on non-Linux targets  (linkerd/linkerd2-proxy#974)
* Update trust-dns to fix possible panic (linkerd/linkerd2-proxy#975)
*  addr: fix `to_http_authority` panic with IPv6 (linkerd/linkerd2-proxy#976)
* outbound: skip logical stacks when no profile is discovered (linkerd/linkerd2-proxy#963)
* split: change traffic splits to require a profile (linkerd/linkerd2-proxy#964)
* inbound: only build profile route stacks when a profile is resolved (linkerd/linkerd2-proxy#966)
* profiles: make receiver param in `route_request` non-optional (linkerd/linkerd2-proxy#967)
* outbound: move target types into stack modules (linkerd/linkerd2-proxy#971)
* outbound: only build logical stacks for profiles with logical addrs (linkerd/linkerd2-proxy#972)
* app: inbound: add fuzzer (linkerd/linkerd2-proxy#977)
* admin: Fail connections when HTTP detection fails (linkerd/linkerd2-proxy#979)
* reduce error boilerplate with `thiserror` (linkerd/linkerd2-proxy#980)
* app: inbound: fuzzer: generalise fuzzers and solve coverage build (linkerd/linkerd2-proxy#978)
* admin: Assume meshed connections are HTTP/2 (linkerd/linkerd2-proxy#982)
* chore: Fix deprecations on nightly (linkerd/linkerd2-proxy#983)
* fuzz: Add logging to fuzz targets (linkerd/linkerd2-proxy#985)
* fuzz: don't panic when the proxy closes the conn (linkerd/linkerd2-proxy#986)
* Commit lock files for fuzzers (linkerd/linkerd2-proxy#984)
* fuzz: allow client requests to fail  (linkerd/linkerd2-proxy#989)
* tower: update dependency to 0.4.7 (linkerd/linkerd2-proxy#990)
* outbound: Make the Endpoint::logical_addr field optional (linkerd/linkerd2-proxy#991)
* trace: explicitly disable ANSI terminal colors (linkerd/linkerd2-proxy#994)
* ingress: Require the l5d-dst-override header (linkerd/linkerd2-proxy#992)
* outbound: Do not support TCP-forwarding in ingress-mode (linkerd/linkerd2-proxy#995)
* Decouple tcp forward stack from Endpoint target (linkerd/linkerd2-proxy#996)
* Pickup linkerd-await wrapper in docker build (linkerd/linkerd2-proxy#999)
* docs: Add fuzzing report (linkerd/linkerd2-proxy#1000)
* tests: Use io::Error in mocked connector (linkerd/linkerd2-proxy#1001)
* outbound: Decouple endpoint & logical stacks (linkerd/linkerd2-proxy#1002)
olix0r added a commit to linkerd/linkerd2 that referenced this pull request May 18, 2021
This release simplifies internals so that endpoint-forwarding logic is
completely distinct from handling of load balanced services.

The ingress-mode outbound proxy has been simplified to *require* the
`l5d-dst-override` header and to fail non-HTTP communication. This
ensures that the ingress-mode proxy does not unexpectedly revert to
insecure communication.

Finally, a regression was recently introduced that caused all proxy logs
to be output with ANSI control characters. Logs are now output in
plaintext by default

---

* discover: replace `linkerd-channel` with `tokio-util` `PollSender` (linkerd/linkerd2-proxy#969)
* replace `linkerd-channel` with `tokio-stream` (linkerd/linkerd2-proxy#970)
* concurrency-limit: use `tokio-util`'s `PollSemaphore` (linkerd/linkerd2-proxy#968)
* http: Do not fail fuzz tests when all IO is not read (linkerd/linkerd2-proxy#973)
* transport: Fix orig-dst compilation on non-Linux targets  (linkerd/linkerd2-proxy#974)
* Update trust-dns to fix possible panic (linkerd/linkerd2-proxy#975)
*  addr: fix `to_http_authority` panic with IPv6 (linkerd/linkerd2-proxy#976)
* outbound: skip logical stacks when no profile is discovered (linkerd/linkerd2-proxy#963)
* split: change traffic splits to require a profile (linkerd/linkerd2-proxy#964)
* inbound: only build profile route stacks when a profile is resolved (linkerd/linkerd2-proxy#966)
* profiles: make receiver param in `route_request` non-optional (linkerd/linkerd2-proxy#967)
* outbound: move target types into stack modules (linkerd/linkerd2-proxy#971)
* outbound: only build logical stacks for profiles with logical addrs (linkerd/linkerd2-proxy#972)
* app: inbound: add fuzzer (linkerd/linkerd2-proxy#977)
* admin: Fail connections when HTTP detection fails (linkerd/linkerd2-proxy#979)
* reduce error boilerplate with `thiserror` (linkerd/linkerd2-proxy#980)
* app: inbound: fuzzer: generalise fuzzers and solve coverage build (linkerd/linkerd2-proxy#978)
* admin: Assume meshed connections are HTTP/2 (linkerd/linkerd2-proxy#982)
* chore: Fix deprecations on nightly (linkerd/linkerd2-proxy#983)
* fuzz: Add logging to fuzz targets (linkerd/linkerd2-proxy#985)
* fuzz: don't panic when the proxy closes the conn (linkerd/linkerd2-proxy#986)
* Commit lock files for fuzzers (linkerd/linkerd2-proxy#984)
* fuzz: allow client requests to fail  (linkerd/linkerd2-proxy#989)
* tower: update dependency to 0.4.7 (linkerd/linkerd2-proxy#990)
* outbound: Make the Endpoint::logical_addr field optional (linkerd/linkerd2-proxy#991)
* trace: explicitly disable ANSI terminal colors (linkerd/linkerd2-proxy#994)
* ingress: Require the l5d-dst-override header (linkerd/linkerd2-proxy#992)
* outbound: Do not support TCP-forwarding in ingress-mode (linkerd/linkerd2-proxy#995)
* Decouple tcp forward stack from Endpoint target (linkerd/linkerd2-proxy#996)
* Pickup linkerd-await wrapper in docker build (linkerd/linkerd2-proxy#999)
* docs: Add fuzzing report (linkerd/linkerd2-proxy#1000)
* tests: Use io::Error in mocked connector (linkerd/linkerd2-proxy#1001)
* outbound: Decouple endpoint & logical stacks (linkerd/linkerd2-proxy#1002)
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.

2 participants