Skip to content

outbound: Prevent loops#525

Merged
olix0r merged 1 commit intomasterfrom
ver/prevent-loop
May 22, 2020
Merged

outbound: Prevent loops#525
olix0r merged 1 commit intomasterfrom
ver/prevent-loop

Conversation

@olix0r
Copy link
Member

@olix0r olix0r commented May 22, 2020

This change modifies the outbound proxy to fail to build services
targetting localhost:4140 (where 4140 is the outbound port). This
prevents looping and will result in 502s.

This change modifies the outbound proxy to fail to build services
targetting localhost:4140 (where 4140 is the outbound port). This
prevents looping and will result in 502s.
@olix0r olix0r requested a review from a team May 22, 2020 00:35
@olix0r olix0r self-assigned this May 22, 2020
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.

Seems reasonable to me! Just so we're clear, this will be logged as a warning or something, someplace?

@olix0r
Copy link
Member Author

olix0r commented May 22, 2020

@hawkw

[   110.714608758s]  WARN outbound:accept{peer.addr=127.0.0.1:59504}:source{target.addr=127.0.0.1:4140}: linkerd2_app_core::errors: Failed to proxy request: outbound requests must not target localhost:4140

Copy link
Contributor

@kleimkuhler kleimkuhler left a comment

Choose a reason for hiding this comment

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

👍

@olix0r olix0r merged commit e77fe18 into master May 22, 2020
@olix0r olix0r deleted the ver/prevent-loop branch May 22, 2020 01:44
olix0r added a commit to linkerd/linkerd2 that referenced this pull request May 22, 2020
In some ingress setups, the proxy could be tricked into looping requests
through the outbound proxy. We now detect these loops and fail these
requests with a 502, saving your precious CPU.

---

* outbound: Prevent loops (linkerd/linkerd2-proxy#525)
olix0r added a commit that referenced this pull request May 30, 2020
This change modifies the outbound proxy to fail to build services
targetting localhost:4140 (where 4140 is the outbound port). This
prevents looping and will result in 502s.
olix0r added a commit that referenced this pull request Jun 11, 2020
* outbound: Prevent loops (#525)

This change modifies the outbound proxy to fail to build services
targetting localhost:4140 (where 4140 is the outbound port). This
prevents looping and will result in 502s.

* Add loop detection to inbound & TCP forwarding (#527)

e77fe18 introduced loop detection to the outbound HTTP proxy. This
change extends this behavior to the inbound HTTP proxy and the TCP
proxy for both inbound and outbound. This helps ensure malicious
requests can't consume proxy resources.

* Test loop detection (#532)

This change adds (flakey) tests for loop detection. The tests are flakey
because they require static ports to work properly. (We cannot configure
the original dst port to be the same as the interface port if the
interface port is not known).

* fallback: Unwrap errors recursively (#534)

This change modifies the fallback layer to inspect error sources
recursively to determine if the given error type is satisfied.

A stack-helper is also added for this case.

* app: Split inbound/outbound constructors into components (#533)

This change does not change any functionality. It only restructures the
inbound and outbound proxy modules so that the clients and servers can
be instantiated separately. This will support gatewaying requests between
the inbound and outbound proxy.

* Introduce a gateway between inbound and outbound (#540)

When the proxy receives inbound requests without an original dst address
(or with a original dst address matching the inbound listener), the
proxy currently fails these requests.

This change modifies the proxy to attempt to accept these requests and
forward them back through the outbound router.

The gateway requires that all requests are received over an mTLS-secured
connection. It also refines the destination through DNS to determine the
canonical-form name as well as an outbound original dst IP. All
gatewayed destinations must have a suffix as set by the
`LINKERD2_PROXY_INBOUND_GATEWAY_SUFFIXES` environment variable.

All requests that do not meet these criteria are failed with a `403
Forbidden` status.

* gateway: Add a Forwarded header

When the gateway forwards requests, it now adds a `Forwarded` header
including the source identity, the local identity, and the destination
authority.

* Fail requests that loop through the gateway

This change uses the gateway's `Forwarded` header to detect if the
request has already transited through this gateway. This is
determination is made by comparing ID strings, so this will prevent
gateway daisy-chaining when clusters do not use distinct identity
domains.

* gateway: Return errors instead of responses (#547)

This ensures that error metrics are recorded and that logging is emitted
uniformly. This also ensures that gRPC requests don't get HTTP error
responses.

* Fail requests that loop through the gateway (#545)

This change uses the gateway's `Forwarded` header to detect if the
request has already transited through this gateway. This is
determination is made by comparing ID strings, so this will prevent
gateway daisy-chaining when clusters do not use distinct identity
domains.

* inbound: Do not cache gateway services (#549)

When the inbound caches gateway services, it eagerly obtains an
outbound service to cache. If the outbound service employs a traffic
split, this inbound service is pinned to a specific leaf, and requests
will never be routed to the other leaf.

This change moves the gateway fallback to be outside all of the inbound
caches, so that outbound splits work as intended.
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