Skip to content

outbound: Only honor the l5d-proxy-connection header when meshed#1283

Merged
olix0r merged 3 commits intomainfrom
ver/close-meshed
Sep 22, 2021
Merged

outbound: Only honor the l5d-proxy-connection header when meshed#1283
olix0r merged 3 commits intomainfrom
ver/close-meshed

Conversation

@olix0r
Copy link
Member

@olix0r olix0r commented Sep 22, 2021

The proxy uses the l5d-proxy-connection header to determine when
it should close proxied connections. This header must only be set by a
peer inbound proxy.

This change modifies the outbound proxy so that this connection is only
honored when the peer is actually meshed. If this header is set by an
unmeshed peer, the header is ignored.

This change also modifies the outbound proxy to always clear this
header so that it can't be sent to the application.

@olix0r olix0r requested a review from a team September 22, 2021 19:35
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.

lgtm, couple nits that don't actually matter

/// Wraps an HTTP `Service` so that a `P`-typed `Param` is cloned into each
/// request's extensions.
#[pin_project::pin_project]
#[derive(Debug)]
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: this Debug impl will require V: Debug and B: Debug, because they're parameters of the type...but they aren't needed to implement Debug. we probably should replace this with a manual impl (if we care)

Comment on lines +15 to +25
#[derive(Debug)]
pub struct NewInsert<P, N> {
inner: N,
_marker: PhantomData<fn() -> P>,
}

/// Wraps an HTTP `Service` so that a `P`-typed `Param` is cloned into each
/// response's extensions.
#[derive(Debug)]
pub struct NewResponseInsert<P, N> {
inner: N,
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: these should probably have manual Debug implementations so that they don't require P: Debug...but, this probably doesn't actually matter

Comment on lines +92 to +101
Some(tls::ConditionalClientTls::Some(_)) => {
if let Some(error) = rsp
.headers()
.get(L5D_PROXY_ERROR)
.and_then(|v| v.to_str().ok())
{
tracing::info!(%error, "Closing application connection for remote proxy");
} else {
tracing::info!("Closing application connection for remote proxy");
}
Copy link
Contributor

Choose a reason for hiding this comment

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

...whew okay i should really merge the impl Value for Option<T: Value> PR in tracing...

@olix0r olix0r merged commit 09ee8f5 into main Sep 22, 2021
@olix0r olix0r deleted the ver/close-meshed branch September 22, 2021 20:44
olix0r added a commit to linkerd/linkerd2 that referenced this pull request Sep 23, 2021
This release improves the proxy's error handling, introducing a new
`l5d-proxy-connection` header to signal from an inbound proxy when its
peers outbound connections should be torn down.

Furthermore, error handling has been improved so that the
`l5d-proxy-error` header is only sent to trusted peers--the inbound
proxy only emits this header when its client is meshed; and the outbound
proxy can be configured to disable these headers via configuration.

---

* build(deps): bump hyper from 0.14.12 to 0.14.13 (linkerd/linkerd2-proxy#1273)
* build(deps): bump tracing-subscriber from 0.2.22 to 0.2.23 (linkerd/linkerd2-proxy#1274)
* tracing: use `Span::or_current` when spawning tasks (linkerd/linkerd2-proxy#1272)
* dns: Log TTL with resolution (linkerd/linkerd2-proxy#1275)
* error-respond: Support stack target configuration (linkerd/linkerd2-proxy#1276)
* build(deps): bump tracing-subscriber from 0.2.23 to 0.2.24 (linkerd/linkerd2-proxy#1277)
* build(deps): bump tracing from 0.1.27 to 0.1.28 (linkerd/linkerd2-proxy#1278)
* build(deps): bump tokio from 1.11.0 to 1.12.0 (linkerd/linkerd2-proxy#1279)
* build(deps): bump http from 0.2.4 to 0.2.5 (linkerd/linkerd2-proxy#1280)
* Support a `l5d-proxy-connection: close` header (linkerd/linkerd2-proxy#1281)
* Avoid emitting informational headers to untrusted clients (linkerd/linkerd2-proxy#1282)
* outbound: Only honor the `l5d-proxy-connection` header when meshed (linkerd/linkerd2-proxy#1283)
* outbound: Disable informational headers by config (linkerd/linkerd2-proxy#1284)
* outbound: Strip peer-sent `l5d-proxy-error` headers (linkerd/linkerd2-proxy#1285)
alpeb pushed a commit to linkerd/linkerd2 that referenced this pull request Sep 23, 2021
This release improves the proxy's error handling, introducing a new
`l5d-proxy-connection` header to signal from an inbound proxy when its
peers outbound connections should be torn down.

Furthermore, error handling has been improved so that the
`l5d-proxy-error` header is only sent to trusted peers--the inbound
proxy only emits this header when its client is meshed; and the outbound
proxy can be configured to disable these headers via configuration.

---

* build(deps): bump hyper from 0.14.12 to 0.14.13 (linkerd/linkerd2-proxy#1273)
* build(deps): bump tracing-subscriber from 0.2.22 to 0.2.23 (linkerd/linkerd2-proxy#1274)
* tracing: use `Span::or_current` when spawning tasks (linkerd/linkerd2-proxy#1272)
* dns: Log TTL with resolution (linkerd/linkerd2-proxy#1275)
* error-respond: Support stack target configuration (linkerd/linkerd2-proxy#1276)
* build(deps): bump tracing-subscriber from 0.2.23 to 0.2.24 (linkerd/linkerd2-proxy#1277)
* build(deps): bump tracing from 0.1.27 to 0.1.28 (linkerd/linkerd2-proxy#1278)
* build(deps): bump tokio from 1.11.0 to 1.12.0 (linkerd/linkerd2-proxy#1279)
* build(deps): bump http from 0.2.4 to 0.2.5 (linkerd/linkerd2-proxy#1280)
* Support a `l5d-proxy-connection: close` header (linkerd/linkerd2-proxy#1281)
* Avoid emitting informational headers to untrusted clients (linkerd/linkerd2-proxy#1282)
* outbound: Only honor the `l5d-proxy-connection` header when meshed (linkerd/linkerd2-proxy#1283)
* outbound: Disable informational headers by config (linkerd/linkerd2-proxy#1284)
* outbound: Strip peer-sent `l5d-proxy-error` headers (linkerd/linkerd2-proxy#1285)
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