-
Notifications
You must be signed in to change notification settings - Fork 270
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
Split cryptographic dependencies into a dedicated crate #1307
Changes from all commits
bc679c1
97f4f21
9baee40
075ce6d
e6b36c0
77345e5
3ae896e
7926c08
ae94c6a
54d40d7
960ad0d
3ef9c5c
003bf42
f53671a
117dced
a333817
058cda3
aea2d5c
f5e8c64
05c2be8
4c1f719
c28a124
5b91cb7
dd198fe
94dd975
9efd9c8
491e0ba
efa8420
e18578b
b5cf662
8d7d70b
ffb8c09
4335c43
bb3d11d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,14 +1,14 @@ | ||
use crate::{policy, Inbound}; | ||
use linkerd_app_core::{ | ||
io, | ||
proxy::identity::LocalCrtKey, | ||
identity::LocalCrtKey, | ||
io, rustls, | ||
svc::{self, ExtractParam, InsertParam, Param}, | ||
tls, | ||
transport::{self, metrics::SensorIo, ClientAddr, OrigDstAddr, Remote, ServerAddr}, | ||
transport_header::{self, NewTransportHeaderServer, SessionProtocol, TransportHeader}, | ||
Conditional, Error, NameAddr, Result, | ||
}; | ||
use std::{convert::TryFrom, fmt::Debug}; | ||
use std::{convert::TryFrom, fmt::Debug, task}; | ||
use thiserror::Error; | ||
use tracing::{debug_span, info_span}; | ||
|
||
|
@@ -52,8 +52,9 @@ pub struct ClientInfo { | |
pub local_addr: OrigDstAddr, | ||
} | ||
|
||
type FwdIo<I> = SensorIo<io::PrefixedIo<tls::server::Io<I>>>; | ||
pub type GatewayIo<I> = io::EitherIo<FwdIo<I>, SensorIo<tls::server::Io<I>>>; | ||
type TlsIo<I> = tls::server::Io<rustls::ServerIo<tls::server::DetectIo<I>>, I>; | ||
type FwdIo<I> = SensorIo<io::PrefixedIo<TlsIo<I>>>; | ||
pub type GatewayIo<I> = io::EitherIo<FwdIo<I>, SensorIo<TlsIo<I>>>; | ||
|
||
#[derive(Clone)] | ||
struct TlsParams { | ||
|
@@ -102,7 +103,6 @@ impl<N> Inbound<N> { | |
rt.metrics.proxy.transport.clone(), | ||
)) | ||
.instrument(|_: &_| debug_span!("opaque")) | ||
.check_new_service::<Local, _>() | ||
// When the transport header is present, it may be used for either local TCP | ||
// forwarding, or we may be processing an HTTP gateway connection. HTTP gateway | ||
// connections that have a transport header must provide a target name as a part of | ||
|
@@ -129,8 +129,13 @@ impl<N> Inbound<N> { | |
negotiated_protocol: client.alpn, | ||
}, | ||
); | ||
let permit = allow.check_authorized(client.client_addr, &tls)?; | ||
Ok(svc::Either::A(Local { addr: Remote(ServerAddr(addr)), permit, client_id: client.client_id, })) | ||
let permit = | ||
allow.check_authorized(client.client_addr, &tls)?; | ||
Ok(svc::Either::A(Local { | ||
addr: Remote(ServerAddr(addr)), | ||
permit, | ||
client_id: client.client_id, | ||
})) | ||
Comment on lines
+132
to
+138
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it took me a min to realize that this (and some of the other code in this file) was not actually changed, and rustfmt just changed its mind about which line to wrap at or something. :/ There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure why but rustfmt was just ignoring this whole stack before and something changed and now it wants to format it all There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. wacky! |
||
} | ||
TransportHeader { | ||
port, | ||
|
@@ -167,30 +172,27 @@ impl<N> Inbound<N> { | |
.instrument( | ||
|g: &GatewayTransportHeader| info_span!("gateway", dst = %g.target), | ||
) | ||
.check_new_service::<GatewayTransportHeader, io::PrefixedIo<tls::server::Io<I>>>() | ||
.into_inner(), | ||
) | ||
// Use ALPN to determine whether a transport header should be read. | ||
.push(NewTransportHeaderServer::layer(detect_timeout)) | ||
.push_request_filter( | ||
|client: ClientInfo| -> Result<_> { | ||
if client.header_negotiated() { | ||
Ok(client) | ||
} else { | ||
Err(RefusedNoTarget.into()) | ||
} | ||
}, | ||
) | ||
.check_new_service::<ClientInfo, tls::server::Io<I>>() | ||
.push_request_filter(|client: ClientInfo| -> Result<_> { | ||
if client.header_negotiated() { | ||
Ok(client) | ||
} else { | ||
Err(RefusedNoTarget.into()) | ||
} | ||
}) | ||
// Build a ClientInfo target for each accepted connection. Refuse the | ||
// connection if it doesn't include an mTLS identity. | ||
.push_request_filter(ClientInfo::try_from) | ||
.push(svc::ArcNewService::layer()) | ||
.push(tls::NewDetectTls::<WithTransportHeaderAlpn, _, _>::layer(TlsParams { | ||
timeout: tls::server::Timeout(detect_timeout), | ||
identity: WithTransportHeaderAlpn(rt.identity.clone()), | ||
})) | ||
.check_new_service::<T, I>() | ||
.push(tls::NewDetectTls::<WithTransportHeaderAlpn, _, _>::layer( | ||
TlsParams { | ||
timeout: tls::server::Timeout(detect_timeout), | ||
identity: WithTransportHeaderAlpn(rt.identity.clone()), | ||
}, | ||
)) | ||
.push_on_service(svc::BoxService::layer()) | ||
.push(svc::ArcNewService::layer()) | ||
}) | ||
|
@@ -293,8 +295,20 @@ impl Param<tls::ConditionalServerTls> for GatewayTransportHeader { | |
|
||
// === impl WithTransportHeaderAlpn === | ||
|
||
impl svc::Param<tls::server::Config> for WithTransportHeaderAlpn { | ||
fn param(&self) -> tls::server::Config { | ||
impl<I> svc::Service<I> for WithTransportHeaderAlpn | ||
where | ||
I: io::AsyncRead + io::AsyncWrite + Send + Unpin, | ||
{ | ||
type Response = (tls::ServerTls, rustls::ServerIo<I>); | ||
type Error = io::Error; | ||
type Future = rustls::TerminateFuture<I>; | ||
|
||
#[inline] | ||
fn poll_ready(&mut self, _: &mut task::Context<'_>) -> task::Poll<Result<(), io::Error>> { | ||
task::Poll::Ready(Ok(())) | ||
} | ||
|
||
fn call(&mut self, io: I) -> Self::Future { | ||
// Copy the underlying TLS config and set an ALPN value. | ||
// | ||
// TODO: Avoid cloning the server config for every connection. It would | ||
|
@@ -304,7 +318,7 @@ impl svc::Param<tls::server::Config> for WithTransportHeaderAlpn { | |
config | ||
.alpn_protocols | ||
.push(transport_header::PROTOCOL.into()); | ||
config.into() | ||
rustls::terminate(config.into(), io) | ||
} | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
take it or leave it, but i'm kind of on the fence about reexporting this
as rustls
? it seems like it's kind of unclear in downstream code whether imports from this module are library APIs or our code, which could be confusing to future readers?not a b locker either way though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I expect this to change in a followup