Introduce a gateway between inbound and outbound by olix0r · Pull Request #540 · linkerd/linkerd2-proxy

olix0r · 2020-05-28T15:34:21Z

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_GATEWAY_SUFFIXES environment variable.

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

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_GATEWAY_SUFFIXES` environment variable. All requests that do not meet these criteria are failed with a `403 Forbidden` status.

olix0r · 2020-05-28T15:38:47Z

I've tested this locally on a docker harness to ensure that we get 403s without identity... I'll do some more testing that this actually works as intended in a proper k8s setup.

I've also not yet implemented loop-detection or identity propagation. That will follow in a separate PR.

Note that the gateway isn't particularly stacked/decomposed... I think that's okay for now. Once the gateway takes its shape it may be worth splitting out into components, but for now it's expedient and isolated (none of the gateway types escape the module due to impl trait). We may find that this general approach is more ergonomic: Focus on splitting functionality into modules that may use stacks internally but ultimately expose some opaque type.

hawkw

overall, this looks good to me; i had a few nits, but nothing major.

re:

I've tested this locally on a docker harness to ensure that we get 403s without identity... I'll do some more testing that this actually works as intended in a proper k8s setup.

it seems like we really ought to have integration tests for this, at least in the simple case without identity. is there a reason this isn't practical to write integration tests in linkerd2-app-integration?

linkerd/app/gateway/src/config.rs

hawkw · 2020-05-28T17:23:43Z

linkerd/app/gateway/src/gateway.rs

+    Forbidden,
+    Outbound {
+        // Source-metadata is available via request extensions set by the
+        // inbound, but we


looks like this comment was not finished?

linkerd/app/gateway/src/gateway.rs

linkerd/app/gateway/src/make.rs

linkerd/app/gateway/src/gateway.rs

linkerd/app/outbound/src/lib.rs

linkerd/app/src/env.rs

linkerd/app/src/lib.rs

Ensures tracing is initiated.

kleimkuhler

Looks good!

olix0r · 2020-05-29T22:32:44Z

it seems like we really ought to have integration tests for this, at least in the simple case without identity. is there a reason this isn't practical to write integration tests in linkerd2-app-integration

No, but the failure case isn't particularly interesting to test with integrations (because we can only exercise the identity-based failure). To do real testing we need to mock identity or stub the transports entirely.

olix0r · 2020-05-29T22:45:55Z

I've finally got this all tested manually. I'm going to proceed merging this, assuming tests pass; and then follow up with loop detection.

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.

The proxy can now operate as gateway, routing requests from its inbound proxy to the outbound proxy, without passing the requests to a local application. This supports Linkerd's multicluster feature by adding a `Forwarded` header to propagate the original client identity and assist in loop detection. --- * Add loop detection to inbound & TCP forwarding (linkerd/linkerd2-proxy#527) * Test loop detection (linkerd/linkerd2-proxy#532) * fallback: Unwrap errors recursively (linkerd/linkerd2-proxy#534) * app: Split inbound/outbound constructors into components (linkerd/linkerd2-proxy#533) * Introduce a gateway between inbound and outbound (linkerd/linkerd2-proxy#540) * gateway: Add a Forwarded header (linkerd/linkerd2-proxy#544) * gateway: Return errors instead of responses (linkerd/linkerd2-proxy#547) * Fail requests that loop through the gateway (linkerd/linkerd2-proxy#545)

This change modifies the linkerd-gateway component to use the inbound proxy, rather than nginx, for gateway. This allows us to detect loops and propagate identity through the gateway. This change also cleans up port naming to `mc-gateway` and `mc-probe` to resolve conflicts with Kubernetes validation. --- * proxy: v2.99.0 The proxy can now operate as gateway, routing requests from its inbound proxy to the outbound proxy, without passing the requests to a local application. This supports Linkerd's multicluster feature by adding a `Forwarded` header to propagate the original client identity and assist in loop detection. --- * Add loop detection to inbound & TCP forwarding (linkerd/linkerd2-proxy#527) * Test loop detection (linkerd/linkerd2-proxy#532) * fallback: Unwrap errors recursively (linkerd/linkerd2-proxy#534) * app: Split inbound/outbound constructors into components (linkerd/linkerd2-proxy#533) * Introduce a gateway between inbound and outbound (linkerd/linkerd2-proxy#540) * gateway: Add a Forwarded header (linkerd/linkerd2-proxy#544) * gateway: Return errors instead of responses (linkerd/linkerd2-proxy#547) * Fail requests that loop through the gateway (linkerd/linkerd2-proxy#545) * inject: Support config.linkerd.io/enable-gateway This change introduces a new annotation, config.linkerd.io/enable-gateway, that, when set, enables the proxy to act as a gateway, routing all traffic targetting the inbound listener through the outbound proxy. This also removes the nginx default listener and gateway port of 4180, instead using 4143 (the inbound port). * proxy: v2.100.0 This change modifies the inbound gateway caching so that requests may be routed to multiple leaves of a traffic split. --- * inbound: Do not cache gateway services (linkerd/linkerd2-proxy#549)

* 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.

olix0r requested a review from a team May 28, 2020 15:34

hawkw approved these changes May 28, 2020

View reviewed changes

olix0r added 4 commits May 28, 2020 18:39

Cache gatewayed services and track inbound telemetry

ebc6134

Ensures tracing is initiated.

+recursion

1332de5

Cleanup logging

43707c4

more logging improvements

9421ac0

kleimkuhler approved these changes May 29, 2020

View reviewed changes

+debug log

429c29f

olix0r added 5 commits May 29, 2020 22:33

+typo

c7cdd63

fixup revert layer move

b2b9331

typo

4e44c8e

GatewayMake => MakeGateway

16f4218

Complete comment

fa2f036

olix0r merged commit 9a9fa33 into master May 29, 2020

olix0r deleted the ver/gateway branch May 29, 2020 23:42

olix0r mentioned this pull request Jun 1, 2020

multicluster: Use the proxy as an HTTP gateway linkerd/linkerd2#4528

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce a gateway between inbound and outbound#540

Introduce a gateway between inbound and outbound#540
olix0r merged 11 commits intomasterfrom
ver/gateway

olix0r commented May 28, 2020

Uh oh!

olix0r commented May 28, 2020

Uh oh!

hawkw left a comment

Uh oh!

Uh oh!

hawkw May 28, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kleimkuhler left a comment

Uh oh!

olix0r commented May 29, 2020

Uh oh!

olix0r commented May 29, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

olix0r commented May 28, 2020

Uh oh!

olix0r commented May 28, 2020

Uh oh!

hawkw left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

hawkw May 28, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kleimkuhler left a comment

Choose a reason for hiding this comment

Uh oh!

olix0r commented May 29, 2020

Uh oh!

olix0r commented May 29, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants