Skip to content

Commit

Permalink
Separate tls::ServerName and identity::Id types
Browse files Browse the repository at this point in the history
To prepare for the introduction of SPIFFE identities into Linkerd's
identity system, this change we need to begin to separate the
representation of an endpoint's SNI value and its "identity" for the
purposes of authentication.

This change introduces a new `tls::ServerName` type to represent the SNI
use case. The `tls::ClientTls` type now has distinct `ServerName` and
`ServerId` configurations to support distinct behavior for sending an
SNI value to the server server

The `identity::Name` type is genralized as `identity::Id`. In the
future, this type will be used to encompass both DNS-like and SPIFFE
identities. This type is specifically used for peer authentication (and
not for SNI negotiation).

The `identity::LocalId` type is removed. It was only being used as a
means for the local server name configuration.

Co-authored-by: <zahari@buoyant.io>
  • Loading branch information
olix0r committed Nov 7, 2023
1 parent 833de52 commit 999044b
Show file tree
Hide file tree
Showing 44 changed files with 342 additions and 305 deletions.
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().server_name().clone().into(),
))
.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

0 comments on commit 999044b

Please sign in to comment.