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

Separate tls::ServerName and identity::Id types #2506

Merged
merged 7 commits into from
Nov 8, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -1364,6 +1364,7 @@ version = "0.1.0"
dependencies = [
"futures",
"linkerd-conditional",
"linkerd-dns-name",
"linkerd-error",
"linkerd-identity",
"linkerd-io",
Expand Down Expand Up @@ -1403,6 +1404,7 @@ name = "linkerd-meshtls-rustls"
version = "0.1.0"
dependencies = [
"futures",
"linkerd-dns-name",
"linkerd-error",
"linkerd-identity",
"linkerd-io",
Expand Down Expand Up @@ -1577,6 +1579,7 @@ version = "0.1.0"
dependencies = [
"futures",
"http-body",
"linkerd-dns-name",
"linkerd-error",
"linkerd-identity",
"linkerd-metrics",
Expand Down
2 changes: 1 addition & 1 deletion linkerd/app/admin/src/stack.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ struct NonHttpClient(Remote<ClientAddr>);

#[derive(Debug, Error)]
#[error("Unexpected TLS connection to {} from {}", self.0, self.1)]
struct UnexpectedSni(tls::ServerId, Remote<ClientAddr>);
struct UnexpectedSni(tls::ServerName, Remote<ClientAddr>);

#[derive(Clone, Debug)]
struct Tcp {
Expand Down
7 changes: 3 additions & 4 deletions linkerd/app/gateway/src/http.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
use super::Gateway;
use inbound::{GatewayAddr, GatewayDomainInvalid};
use linkerd_app_core::{
identity,
metrics::ServerLabel,
profiles,
proxy::{
Expand Down Expand Up @@ -104,9 +103,9 @@ impl Gateway {
// Discard `T` and its associated client-specific metadata.
.push_map_target(Target::discard_parent)
// Add headers to prevent loops.
.push(NewHttpGateway::layer(identity::LocalId(
self.inbound.identity().name().clone(),
)))
.push(NewHttpGateway::layer(
self.inbound.identity().local_id().clone(),
))
.push_on_service(svc::LoadShed::layer())
.lift_new()
// After protocol-downgrade, we need to build an inner stack for
Expand Down
13 changes: 7 additions & 6 deletions linkerd/app/gateway/src/http/gateway.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use futures::{future, TryFutureExt};
use linkerd_app_core::{
identity as id,
proxy::http,
svc::{self, layer},
tls, Error,
Expand All @@ -15,7 +16,7 @@ use std::{
#[derive(Clone, Debug)]
pub(crate) struct NewHttpGateway<N> {
inner: N,
local_id: tls::LocalId,
local_id: id::Id,
}

/// A `Service` middleware that fails requests that would loop. It reads and
Expand All @@ -25,19 +26,19 @@ pub(crate) struct HttpGateway<S> {
inner: S,
host: String,
client_id: tls::ClientId,
local_id: tls::LocalId,
local_id: id::Id,
}

type ResponseFuture<T> = Pin<Box<dyn Future<Output = Result<T, Error>> + Send + 'static>>;

// === impl NewHttpGateway ===

impl<N> NewHttpGateway<N> {
pub fn new(inner: N, local_id: tls::LocalId) -> Self {
pub fn new(inner: N, local_id: id::Id) -> Self {
Self { inner, local_id }
}

pub fn layer(local_id: tls::LocalId) -> impl layer::Layer<N, Service = Self> + Clone {
pub fn layer(local_id: id::Id) -> impl layer::Layer<N, Service = Self> + Clone {
layer::mk(move |inner| Self::new(inner, local_id.clone()))
}
}
Expand Down Expand Up @@ -77,7 +78,7 @@ where
#[inline]
fn poll_ready(&mut self, cx: &mut Context<'_>) -> Poll<Result<(), Self::Error>> {
// If the client ID is the same as the gateway's, then we're in a loop.
if *self.client_id == *self.local_id {
if *self.client_id == self.local_id {
return Poll::Ready(Err(GatewayLoop.into()));
}

Expand All @@ -95,7 +96,7 @@ where
{
if let Some(by) = fwd_by(forwarded) {
tracing::info!(%forwarded);
if by == self.local_id.as_str() {
if by == self.local_id.to_str() {
return Box::pin(future::err(GatewayLoop.into()));
}
}
Expand Down
2 changes: 1 addition & 1 deletion linkerd/app/gateway/src/http/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ impl Test {

let new = NewHttpGateway::new(
move |_: _| outbound.clone(),
tls::LocalId("gateway.id.test".parse().unwrap()),
"gateway.id.test".parse().unwrap(),
);

#[derive(Clone, Debug)]
Expand Down
4 changes: 2 additions & 2 deletions linkerd/app/inbound/src/detect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -396,8 +396,8 @@ impl svc::Param<http::normalize_uri::DefaultAuthority> for Http {
}
}

impl svc::Param<Option<identity::Name>> for Http {
fn param(&self) -> Option<identity::Name> {
impl svc::Param<Option<identity::Id>> for Http {
fn param(&self) -> Option<identity::Id> {
self.tls
.status
.value()
Expand Down
4 changes: 2 additions & 2 deletions linkerd/app/inbound/src/http.rs
Original file line number Diff line number Diff line change
Expand Up @@ -252,8 +252,8 @@ pub mod fuzz {
}
}

impl svc::Param<Option<identity::Name>> for Target {
fn param(&self) -> Option<identity::Name> {
impl svc::Param<Option<identity::Id>> for Target {
fn param(&self) -> Option<identity::Id> {
None
}
}
Expand Down
2 changes: 1 addition & 1 deletion linkerd/app/inbound/src/http/set_identity_header.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ where
.and_then(|tls| match tls {
tls::ServerTls::Established { client_id, .. } => {
client_id.as_ref().and_then(|id| {
match http::HeaderValue::from_str(id.as_str()) {
match http::HeaderValue::from_str(&id.to_str()) {
Ok(v) => Some(v),
Err(error) => {
tracing::warn!(%error, "identity not a valid header value");
Expand Down
4 changes: 2 additions & 2 deletions linkerd/app/inbound/src/http/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -770,8 +770,8 @@ impl svc::Param<http::normalize_uri::DefaultAuthority> for Target {
}
}

impl svc::Param<Option<identity::Name>> for Target {
fn param(&self) -> Option<identity::Name> {
impl svc::Param<Option<identity::Id>> for Target {
fn param(&self) -> Option<identity::Id> {
None
}
}
3 changes: 2 additions & 1 deletion linkerd/app/inbound/src/policy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,8 @@ fn is_authorized(
client_id: Some(tls::server::ClientId(ref id)),
..
}) => {
identities.contains(id.as_str()) || suffixes.iter().any(|s| s.contains(id.as_str()))
identities.contains(&*id.to_str())
|| suffixes.iter().any(|s| s.contains(&id.to_str()))
}
_ => false,
},
Expand Down
16 changes: 7 additions & 9 deletions linkerd/app/outbound/src/http/concrete.rs
Original file line number Diff line number Diff line change
Expand Up @@ -375,15 +375,13 @@ impl<T> svc::Param<tls::ConditionalClientTls> for Endpoint<T> {
.identity()
.cloned()
.map(move |server_id| {
tls::ConditionalClientTls::Some(tls::ClientTls {
server_id,
alpn: if use_transport_header {
use linkerd_app_core::transport_header::PROTOCOL;
Some(tls::client::AlpnProtocols(vec![PROTOCOL.into()]))
} else {
None
},
})
let alpn = if use_transport_header {
use linkerd_app_core::transport_header::PROTOCOL;
Some(tls::client::AlpnProtocols(vec![PROTOCOL.into()]))
} else {
None
};
tls::ConditionalClientTls::Some(tls::ClientTls::new(server_id, alpn))
})
.unwrap_or(tls::ConditionalClientTls::None(
tls::NoClientTls::NotProvidedByServiceDiscovery,
Expand Down
7 changes: 4 additions & 3 deletions linkerd/app/outbound/src/http/require_id_header.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use futures::{future, TryFutureExt};
use linkerd_app_core::{identity, svc, tls, Conditional, Error};
use linkerd_app_core::{dns, identity, svc, tls, Conditional, Error};
use std::task::{Context, Poll};
use thiserror::Error;
use tracing::{debug, trace};
Expand Down Expand Up @@ -57,9 +57,10 @@ type ResponseFuture<F, T, E> =

impl<S> RequireIdentity<S> {
#[inline]
fn extract_id<B>(req: &mut http::Request<B>) -> Option<identity::Name> {
fn extract_id<B>(req: &mut http::Request<B>) -> Option<identity::Id> {
let v = req.headers_mut().remove(HEADER_NAME)?;
v.to_str().ok()?.parse().ok()
let n = v.to_str().ok()?.parse::<dns::Name>().ok()?;
Some(n.into())
}
}

Expand Down
16 changes: 7 additions & 9 deletions linkerd/app/outbound/src/opaq/concrete.rs
Original file line number Diff line number Diff line change
Expand Up @@ -284,15 +284,13 @@ impl<T> svc::Param<tls::ConditionalClientTls> for Endpoint<T> {
.identity()
.cloned()
.map(move |server_id| {
tls::ConditionalClientTls::Some(tls::ClientTls {
server_id,
alpn: if use_transport_header {
use linkerd_app_core::transport_header::PROTOCOL;
Some(tls::client::AlpnProtocols(vec![PROTOCOL.into()]))
} else {
None
},
})
let alpn = if use_transport_header {
use linkerd_app_core::transport_header::PROTOCOL;
Some(tls::client::AlpnProtocols(vec![PROTOCOL.into()]))
} else {
None
};
tls::ConditionalClientTls::Some(tls::ClientTls::new(server_id, alpn))
})
.unwrap_or(tls::ConditionalClientTls::None(
tls::NoClientTls::NotProvidedByServiceDiscovery,
Expand Down
35 changes: 10 additions & 25 deletions linkerd/app/outbound/src/tcp/tagged_transport.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,6 @@ mod test {
use super::*;
use futures::future;
use linkerd_app_core::{
identity,
io::{self, AsyncWriteExt},
tls,
transport::{ClientAddr, Local},
Expand All @@ -163,12 +162,10 @@ mod test {
self.server_id
.clone()
.map(|server_id| {
tls::ConditionalClientTls::Some(tls::ClientTls {
server_id,
alpn: Some(tls::client::AlpnProtocols(vec![
transport_header::PROTOCOL.into()
])),
})
let alpn = Some(tls::client::AlpnProtocols(vec![
transport_header::PROTOCOL.into()
]));
tls::ConditionalClientTls::Some(tls::ClientTls::new(server_id, alpn))
})
.unwrap_or(tls::ConditionalClientTls::None(
tls::NoClientTls::NotProvidedByServiceDiscovery,
Expand Down Expand Up @@ -261,9 +258,7 @@ mod test {

let e = Endpoint {
port_override: Some(4143),
server_id: Some(tls::ServerId(
identity::Name::from_str("server.id").unwrap(),
)),
server_id: Some(tls::ServerId("server.id".parse().unwrap())),
authority: None,
proto: None,
};
Expand All @@ -285,9 +280,7 @@ mod test {

let e = Endpoint {
port_override: Some(4143),
server_id: Some(tls::ServerId(
identity::Name::from_str("server.id").unwrap(),
)),
server_id: Some(tls::ServerId("server.id".parse().unwrap())),
authority: Some(http::uri::Authority::from_str("foo.bar.example.com:5555").unwrap()),
proto: None,
};
Expand All @@ -309,9 +302,7 @@ mod test {

let e = Endpoint {
port_override: Some(4143),
server_id: Some(tls::ServerId(
identity::Name::from_str("server.id").unwrap(),
)),
server_id: Some(tls::ServerId("server.id".parse().unwrap())),
authority: None,
proto: None,
};
Expand All @@ -333,9 +324,7 @@ mod test {

let e = Endpoint {
port_override: Some(4143),
server_id: Some(tls::ServerId(
identity::Name::from_str("server.id").unwrap(),
)),
server_id: Some(tls::ServerId("server.id".parse().unwrap())),
authority: None,
proto: Some(SessionProtocol::Http1),
};
Expand All @@ -357,9 +346,7 @@ mod test {

let e = Endpoint {
port_override: Some(4143),
server_id: Some(tls::ServerId(
identity::Name::from_str("server.id").unwrap(),
)),
server_id: Some(tls::ServerId("server.id".parse().unwrap())),
authority: Some(http::uri::Authority::from_str("foo.bar.example.com:5555").unwrap()),
proto: Some(SessionProtocol::Http1),
};
Expand All @@ -381,9 +368,7 @@ mod test {

let e = Endpoint {
port_override: Some(4143),
server_id: Some(tls::ServerId(
identity::Name::from_str("server.id").unwrap(),
)),
server_id: Some(tls::ServerId("server.id".parse().unwrap())),
authority: None,
proto: Some(SessionProtocol::Http1),
};
Expand Down
19 changes: 13 additions & 6 deletions linkerd/app/src/env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -982,8 +982,15 @@ fn parse_port_range_set(s: &str) -> Result<RangeInclusiveSet<u16>, ParseError> {
Ok(set)
}

pub(super) fn parse_identity(s: &str) -> Result<identity::Name, ParseError> {
identity::Name::from_str(s).map_err(|identity::InvalidName| {
pub(super) fn parse_dns_name(s: &str) -> Result<dns::Name, ParseError> {
s.parse().map_err(|_| {
error!("Not a valid identity name: {}", s);
ParseError::NameError
})
}

pub(super) fn parse_identity(s: &str) -> Result<identity::Id, ParseError> {
s.parse().map_err(|_| {
error!("Not a valid identity name: {}", s);
ParseError::NameError
})
Expand Down Expand Up @@ -1130,7 +1137,7 @@ pub fn parse_control_addr<S: Strings>(
base: &str,
) -> Result<Option<ControlAddr>, EnvError> {
let a = parse(strings, &format!("{}_ADDR", base), parse_addr)?;
let n = parse(strings, &format!("{}_NAME", base), parse_identity)?;
let n = parse(strings, &format!("{}_NAME", base), parse_dns_name)?;
match (a, n) {
(None, None) => Ok(None),
(Some(ref addr), _) if addr.is_loopback() => Ok(Some(ControlAddr {
Expand All @@ -1139,7 +1146,7 @@ pub fn parse_control_addr<S: Strings>(
})),
(Some(addr), Some(name)) => Ok(Some(ControlAddr {
addr,
identity: Conditional::Some(tls::ServerId(name).into()),
identity: Conditional::Some(tls::ClientTls::new(tls::ServerId(name.into()), None)),
})),
_ => {
error!("{}_ADDR and {}_NAME must be specified together", base, base);
Expand All @@ -1165,7 +1172,7 @@ pub fn parse_identity_config<S: Strings>(
ParseError::InvalidTokenSource
})
});
let li = parse(strings, ENV_IDENTITY_IDENTITY_LOCAL_NAME, parse_identity);
let li = parse(strings, ENV_IDENTITY_IDENTITY_LOCAL_NAME, parse_dns_name);
let min_refresh = parse(strings, ENV_IDENTITY_MIN_REFRESH, parse_duration);
let max_refresh = parse(strings, ENV_IDENTITY_MAX_REFRESH, parse_duration);

Expand Down Expand Up @@ -1235,7 +1242,7 @@ pub fn parse_identity_config<S: Strings>(
max_refresh: max_refresh.unwrap_or(DEFAULT_IDENTITY_MAX_REFRESH),
};
let docs = identity::Documents {
id: identity::LocalId(local_name),
server_name: local_name,
trust_anchors_pem,
key_pkcs8: key?,
csr_der: csr?,
Expand Down
Loading