From 2f8c4a00bcf595ea4265f3549e8b70a6ae35a49c Mon Sep 17 00:00:00 2001 From: Zahari Dichev Date: Thu, 16 Nov 2023 14:38:59 +0000 Subject: [PATCH] add support for SPIFFE TLS Ids Signed-off-by: Zahari Dichev --- Cargo.lock | 160 +++++++++- Cargo.toml | 2 +- linkerd/app/outbound/src/http/concrete.rs | 4 +- .../outbound/src/http/require_id_header.rs | 5 +- linkerd/app/outbound/src/opaq/concrete.rs | 4 +- .../app/outbound/src/tcp/tagged_transport.rs | 42 ++- linkerd/app/src/env.rs | 6 +- linkerd/identity/Cargo.toml | 2 + linkerd/identity/src/lib.rs | 44 ++- linkerd/meshtls/boring/Cargo.toml | 3 +- linkerd/meshtls/boring/src/client.rs | 6 +- linkerd/meshtls/boring/src/creds.rs | 1 - linkerd/meshtls/boring/src/creds/store.rs | 9 +- linkerd/meshtls/boring/src/creds/verify.rs | 62 ---- linkerd/meshtls/boring/src/server.rs | 34 +-- linkerd/meshtls/rustls/Cargo.toml | 2 +- linkerd/meshtls/rustls/src/client.rs | 4 +- linkerd/meshtls/rustls/src/creds/store.rs | 4 +- linkerd/meshtls/rustls/src/creds/verify.rs | 56 +--- linkerd/meshtls/rustls/src/server.rs | 29 +- linkerd/meshtls/test-util/src/lib.rs | 86 ------ linkerd/meshtls/tests/util.rs | 25 +- .../meshtls/{test-util => util}/Cargo.toml | 12 +- linkerd/meshtls/util/src/lib.rs | 287 ++++++++++++++++++ linkerd/proxy/api-resolve/src/metadata.rs | 7 +- linkerd/proxy/api-resolve/src/pb.rs | 13 +- linkerd/tls/src/client.rs | 13 +- linkerd/tls/test-util/src/lib.rs | 5 + 28 files changed, 606 insertions(+), 321 deletions(-) delete mode 100644 linkerd/meshtls/boring/src/creds/verify.rs delete mode 100644 linkerd/meshtls/test-util/src/lib.rs rename linkerd/meshtls/{test-util => util}/Cargo.toml (58%) create mode 100644 linkerd/meshtls/util/src/lib.rs diff --git a/Cargo.lock b/Cargo.lock index b4b26c4d1f..824193769c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -51,6 +51,45 @@ dependencies = [ "derive_arbitrary", ] +[[package]] +name = "asn1-rs" +version = "0.5.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7f6fd5ddaf0351dff5b8da21b2fb4ff8e08ddd02857f0bf69c47639106c0fff0" +dependencies = [ + "asn1-rs-derive", + "asn1-rs-impl", + "displaydoc", + "nom", + "num-traits", + "rusticata-macros", + "thiserror", + "time", +] + +[[package]] +name = "asn1-rs-derive" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "726535892e8eae7e70657b4c8ea93d26b8553afb1ce617caee529ef96d7dee6c" +dependencies = [ + "proc-macro2", + "quote", + "syn 1.0.109", + "synstructure", +] + +[[package]] +name = "asn1-rs-impl" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2777730b2039ac0f95f093556e61b6d26cebed5393ca6f152717777cec3a42ed" +dependencies = [ + "proc-macro2", + "quote", + "syn 1.0.109", +] + [[package]] name = "async-stream" version = "0.3.4" @@ -291,6 +330,20 @@ dependencies = [ "gzip-header", ] +[[package]] +name = "der-parser" +version = "8.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dbd676fbbab537128ef0278adb5576cf363cff6aa22a7b24effe97347cfab61e" +dependencies = [ + "asn1-rs", + "displaydoc", + "nom", + "num-bigint", + "num-traits", + "rusticata-macros", +] + [[package]] name = "deranged" version = "0.3.9" @@ -311,6 +364,17 @@ dependencies = [ "syn 1.0.109", ] +[[package]] +name = "displaydoc" +version = "0.2.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "487585f4d0c6655fe74905e2504d8ad6908e4db67f744eb140876906c2f3175d" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.12", +] + [[package]] name = "drain" version = "0.1.1" @@ -1341,8 +1405,10 @@ dependencies = [ name = "linkerd-identity" version = "0.1.0" dependencies = [ + "http", "linkerd-dns-name", "linkerd-error", + "thiserror", ] [[package]] @@ -1407,7 +1473,7 @@ dependencies = [ "linkerd-identity", "linkerd-io", "linkerd-meshtls", - "linkerd-meshtls-test-util", + "linkerd-meshtls-util", "linkerd-stack", "linkerd-tls", "linkerd-tls-test-util", @@ -1426,7 +1492,7 @@ dependencies = [ "linkerd-identity", "linkerd-io", "linkerd-meshtls", - "linkerd-meshtls-test-util", + "linkerd-meshtls-util", "linkerd-stack", "linkerd-tls", "linkerd-tls-test-util", @@ -1440,11 +1506,14 @@ dependencies = [ ] [[package]] -name = "linkerd-meshtls-test-util" +name = "linkerd-meshtls-util" version = "0.1.0" dependencies = [ + "linkerd-error", "linkerd-identity", "rcgen", + "tracing", + "x509-parser", ] [[package]] @@ -2107,6 +2176,27 @@ dependencies = [ "winapi", ] +[[package]] +name = "num-bigint" +version = "0.4.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f93ab6289c7b344a8a9f60f88d80aa20032336fe78da341afc91c8a2341fc75f" +dependencies = [ + "autocfg", + "num-integer", + "num-traits", +] + +[[package]] +name = "num-integer" +version = "0.1.45" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "225d3389fb3509a24c93f5c29eb6bde2586b98d9f016636dff58d7c6f7569cd9" +dependencies = [ + "autocfg", + "num-traits", +] + [[package]] name = "num-traits" version = "0.2.15" @@ -2126,6 +2216,15 @@ dependencies = [ "libc", ] +[[package]] +name = "oid-registry" +version = "0.6.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9bedf36ffb6ba96c2eb7144ef6270557b52e54b20c0a8e1eb2ff99a6c6959bff" +dependencies = [ + "asn1-rs", +] + [[package]] name = "once_cell" version = "1.17.1" @@ -2481,6 +2580,15 @@ version = "1.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "08d43f7aa6b08d49f382cde6a7982047c3426db949b1424bc4b7ec9ae12c6ce2" +[[package]] +name = "rusticata-macros" +version = "4.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "faf0c4a6ece9950b9abdb62b1cfcf2a68b3b67a10ba445b3bb85be2a293d0632" +dependencies = [ + "nom", +] + [[package]] name = "rustix" version = "0.36.16" @@ -2674,6 +2782,18 @@ version = "0.1.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2047c6ded9c721764247e62cd3b03c09ffc529b2ba5b10ec482ae507a4a70160" +[[package]] +name = "synstructure" +version = "0.12.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f36bdaa60a83aca3921b5259d5400cbf5e90fc51931376a9bd4a0eb79aa7210f" +dependencies = [ + "proc-macro2", + "quote", + "syn 1.0.109", + "unicode-xid", +] + [[package]] name = "tempfile" version = "3.5.0" @@ -2734,9 +2854,11 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c4a34ab300f2dee6e562c10a046fc05e358b29f9bf92277f30c3c8d82275f6f5" dependencies = [ "deranged", + "itoa", "powerfmt", "serde", "time-core", + "time-macros", ] [[package]] @@ -2745,6 +2867,15 @@ version = "0.1.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ef927ca75afb808a4d64dd374f00a2adf8d0fcff8e7b184af886c3c87ec4a3f3" +[[package]] +name = "time-macros" +version = "0.2.15" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4ad70d68dba9e1f8aceda7aa6711965dfec1cac869f311a51bd08b3a2ccbce20" +dependencies = [ + "time-core", +] + [[package]] name = "tinyvec" version = "1.6.0" @@ -3118,6 +3249,12 @@ dependencies = [ "tinyvec", ] +[[package]] +name = "unicode-xid" +version = "0.2.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f962df74c8c05a667b5ee8bcf162993134c104e96440b663c8daa176dc772d8c" + [[package]] name = "untrusted" version = "0.7.1" @@ -3422,6 +3559,23 @@ dependencies = [ "winapi", ] +[[package]] +name = "x509-parser" +version = "0.15.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7069fba5b66b9193bd2c5d3d4ff12b839118f6bcbef5328efafafb5395cf63da" +dependencies = [ + "asn1-rs", + "data-encoding", + "der-parser", + "lazy_static", + "nom", + "oid-registry", + "rusticata-macros", + "thiserror", + "time", +] + [[package]] name = "yasna" version = "0.5.2" diff --git a/Cargo.toml b/Cargo.toml index 37bcb093b3..94a2bb8fcd 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -36,7 +36,7 @@ members = [ "linkerd/meshtls", "linkerd/meshtls/boring", "linkerd/meshtls/rustls", - "linkerd/meshtls/test-util", + "linkerd/meshtls/util", "linkerd/metrics", "linkerd/opencensus", "linkerd/proxy/api-resolve", diff --git a/linkerd/app/outbound/src/http/concrete.rs b/linkerd/app/outbound/src/http/concrete.rs index c2fc6c75a6..237f9175bf 100644 --- a/linkerd/app/outbound/src/http/concrete.rs +++ b/linkerd/app/outbound/src/http/concrete.rs @@ -374,14 +374,14 @@ impl svc::Param for Endpoint { self.metadata .identity() .cloned() - .map(move |server_id| { + .map(move |(server_id, server_name)| { 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)) + tls::ConditionalClientTls::Some(tls::ClientTls::new(server_id, server_name, alpn)) }) .unwrap_or(tls::ConditionalClientTls::None( tls::NoClientTls::NotProvidedByServiceDiscovery, diff --git a/linkerd/app/outbound/src/http/require_id_header.rs b/linkerd/app/outbound/src/http/require_id_header.rs index 8ac1cdc5c3..451169ffad 100644 --- a/linkerd/app/outbound/src/http/require_id_header.rs +++ b/linkerd/app/outbound/src/http/require_id_header.rs @@ -1,5 +1,5 @@ use futures::{future, TryFutureExt}; -use linkerd_app_core::{dns, identity, svc, tls, Conditional, Error}; +use linkerd_app_core::{identity, svc, tls, Conditional, Error}; use std::task::{Context, Poll}; use thiserror::Error; use tracing::{debug, trace}; @@ -59,8 +59,7 @@ impl RequireIdentity { #[inline] fn extract_id(req: &mut http::Request) -> Option { let v = req.headers_mut().remove(HEADER_NAME)?; - let n = v.to_str().ok()?.parse::().ok()?; - Some(n.into()) + v.to_str().ok()?.parse::().ok() } } diff --git a/linkerd/app/outbound/src/opaq/concrete.rs b/linkerd/app/outbound/src/opaq/concrete.rs index 566390afea..d88ddf6559 100644 --- a/linkerd/app/outbound/src/opaq/concrete.rs +++ b/linkerd/app/outbound/src/opaq/concrete.rs @@ -283,14 +283,14 @@ impl svc::Param for Endpoint { self.metadata .identity() .cloned() - .map(move |server_id| { + .map(move |(server_id, server_name)| { 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)) + tls::ConditionalClientTls::Some(tls::ClientTls::new(server_id, server_name, alpn)) }) .unwrap_or(tls::ConditionalClientTls::None( tls::NoClientTls::NotProvidedByServiceDiscovery, diff --git a/linkerd/app/outbound/src/tcp/tagged_transport.rs b/linkerd/app/outbound/src/tcp/tagged_transport.rs index d10d0a7ba2..9712799d15 100644 --- a/linkerd/app/outbound/src/tcp/tagged_transport.rs +++ b/linkerd/app/outbound/src/tcp/tagged_transport.rs @@ -153,19 +153,23 @@ mod test { struct Endpoint { port_override: Option, authority: Option, - server_id: Option, + identity: Option<(tls::ServerId, tls::ServerName)>, proto: Option, } impl svc::Param for Endpoint { fn param(&self) -> tls::ConditionalClientTls { - self.server_id + self.identity .clone() - .map(|server_id| { + .map(|(server_id, server_name)| { let alpn = Some(tls::client::AlpnProtocols(vec![ transport_header::PROTOCOL.into() ])); - tls::ConditionalClientTls::Some(tls::ClientTls::new(server_id, alpn)) + tls::ConditionalClientTls::Some(tls::ClientTls::new( + server_id, + server_name, + alpn, + )) }) .unwrap_or(tls::ConditionalClientTls::None( tls::NoClientTls::NotProvidedByServiceDiscovery, @@ -256,9 +260,12 @@ mod test { })), }; + let server_id = tls::ServerId("server.id".parse().unwrap()); + let server_name = tls::ServerName("server.name".parse().unwrap()); + let e = Endpoint { port_override: Some(4143), - server_id: Some(tls::ServerId("server.id".parse().unwrap())), + identity: Some((server_id, server_name)), authority: None, proto: None, }; @@ -278,9 +285,12 @@ mod test { })), }; + let server_id = tls::ServerId("server.id".parse().unwrap()); + let server_name = tls::ServerName("server.name".parse().unwrap()); + let e = Endpoint { port_override: Some(4143), - server_id: Some(tls::ServerId("server.id".parse().unwrap())), + identity: Some((server_id, server_name)), authority: Some(http::uri::Authority::from_str("foo.bar.example.com:5555").unwrap()), proto: None, }; @@ -300,9 +310,12 @@ mod test { })), }; + let server_id = tls::ServerId("server.id".parse().unwrap()); + let server_name = tls::ServerName("server.name".parse().unwrap()); + let e = Endpoint { port_override: Some(4143), - server_id: Some(tls::ServerId("server.id".parse().unwrap())), + identity: Some((server_id, server_name)), authority: None, proto: None, }; @@ -322,9 +335,12 @@ mod test { })), }; + let server_id = tls::ServerId("server.id".parse().unwrap()); + let server_name = tls::ServerName("server.name".parse().unwrap()); + let e = Endpoint { port_override: Some(4143), - server_id: Some(tls::ServerId("server.id".parse().unwrap())), + identity: Some((server_id, server_name)), authority: None, proto: Some(SessionProtocol::Http1), }; @@ -344,9 +360,12 @@ mod test { })), }; + let server_id = tls::ServerId("server.id".parse().unwrap()); + let server_name = tls::ServerName("server.name".parse().unwrap()); + let e = Endpoint { port_override: Some(4143), - server_id: Some(tls::ServerId("server.id".parse().unwrap())), + identity: Some((server_id, server_name)), authority: Some(http::uri::Authority::from_str("foo.bar.example.com:5555").unwrap()), proto: Some(SessionProtocol::Http1), }; @@ -366,9 +385,12 @@ mod test { })), }; + let server_id = tls::ServerId("server.id".parse().unwrap()); + let server_name = tls::ServerName("server.name".parse().unwrap()); + let e = Endpoint { port_override: Some(4143), - server_id: Some(tls::ServerId("server.id".parse().unwrap())), + identity: Some((server_id, server_name)), authority: None, proto: Some(SessionProtocol::Http1), }; diff --git a/linkerd/app/src/env.rs b/linkerd/app/src/env.rs index 969d3c656b..45d8c5063d 100644 --- a/linkerd/app/src/env.rs +++ b/linkerd/app/src/env.rs @@ -1146,7 +1146,11 @@ pub fn parse_control_addr( })), (Some(addr), Some(name)) => Ok(Some(ControlAddr { addr, - identity: Conditional::Some(tls::ClientTls::new(tls::ServerId(name.into()), None)), + identity: Conditional::Some(tls::ClientTls::new( + tls::ServerId(name.clone().into()), + tls::ServerName(name), + None, + )), })), _ => { error!("{}_ADDR and {}_NAME must be specified together", base, base); diff --git a/linkerd/identity/Cargo.toml b/linkerd/identity/Cargo.toml index b2358627f1..525ff5a444 100644 --- a/linkerd/identity/Cargo.toml +++ b/linkerd/identity/Cargo.toml @@ -9,3 +9,5 @@ publish = false [dependencies] linkerd-dns-name = { path = "../dns/name" } linkerd-error = { path = "../error" } +http = "0.2" +thiserror = "1" diff --git a/linkerd/identity/src/lib.rs b/linkerd/identity/src/lib.rs index 99e2d40ee7..c2b3d66e73 100644 --- a/linkerd/identity/src/lib.rs +++ b/linkerd/identity/src/lib.rs @@ -4,17 +4,28 @@ mod credentials; pub use self::credentials::{Credentials, DerX509}; +use linkerd_error::Result; +use thiserror::Error; + +const SPIFFE_URI_SCHEME: &str = "spiffe://"; /// An endpoint identity descriptor used for authentication. /// -/// Practically speaking, this is a DNS-like identity string that matches an -/// x509 DNS SAN. This will in the future be updated to support SPIFFE IDs as -/// well. +/// Practically speaking, this could be: +/// - a DNS-like identity string that matches an x509 DNS SAN +/// - a SPIFFE ID in URI form, that matches an x509 /// /// This isn't restricted to TLS or x509 uses. An authenticated Id could be /// provided by, e.g., a JWT. #[derive(Clone, Debug, Eq, PartialEq, Hash)] -pub struct Id(pub linkerd_dns_name::Name); +pub enum Id { + Dns(linkerd_dns_name::Name), + Uri(http::Uri), +} + +#[derive(Copy, Clone, Debug, Eq, PartialEq, Error)] +#[error("invalid TLS id")] +pub struct InvalidId; // === impl Id === @@ -22,29 +33,42 @@ impl std::str::FromStr for Id { type Err = linkerd_error::Error; fn from_str(s: &str) -> Result { - // TODO support spiffe:// URIs. + if s.starts_with(SPIFFE_URI_SCHEME) { + // TODO: we need to ensure the SPIFFE id is valid + // according to requirements otlined in: + // https://github.com/spiffe/spiffe/blob/main/standards/SPIFFE-ID.md#2-spiffe-identity + return http::Uri::try_from(s).map(Self::Uri).map_err(Into::into); + } + + // if id does not start with a SPIFFE prefix, assume it is in DNS form let n = linkerd_dns_name::Name::from_str(s)?; if n.ends_with('.') { - return Err(linkerd_dns_name::InvalidName.into()); + return Err(InvalidId.into()); } - Ok(Self(n)) + Ok(Self::Dns(n)) } } impl Id { pub fn to_str(&self) -> std::borrow::Cow<'_, str> { - self.0.as_str().into() + match self { + Self::Dns(dns) => dns.as_str().into(), + Self::Uri(uri) => uri.to_string().into(), + } } } impl From for Id { fn from(n: linkerd_dns_name::Name) -> Self { - Self(n) + Self::Dns(n) } } impl std::fmt::Display for Id { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - self.0.without_trailing_dot().fmt(f) + match self { + Self::Dns(dns) => dns.without_trailing_dot().fmt(f), + Self::Uri(uri) => uri.fmt(f), + } } } diff --git a/linkerd/meshtls/boring/Cargo.toml b/linkerd/meshtls/boring/Cargo.toml index ef08409db7..41474bb866 100644 --- a/linkerd/meshtls/boring/Cargo.toml +++ b/linkerd/meshtls/boring/Cargo.toml @@ -16,6 +16,8 @@ linkerd-identity = { path = "../../identity" } linkerd-io = { path = "../../io" } linkerd-stack = { path = "../../stack" } linkerd-tls = { path = "../../tls" } +linkerd-meshtls-util = { path = "../util" } + tokio = { version = "1", features = ["macros", "sync"] } tokio-boring = "3" tracing = "0.1" @@ -26,5 +28,4 @@ fips = ["boring/fips"] [dev-dependencies] linkerd-tls-test-util = { path = "../../tls/test-util" } linkerd-meshtls = { path = "../../meshtls" } -linkerd-meshtls-test-util = { path = "../test-util" } diff --git a/linkerd/meshtls/boring/src/client.rs b/linkerd/meshtls/boring/src/client.rs index 8d972ae2cf..2daaf8da20 100644 --- a/linkerd/meshtls/boring/src/client.rs +++ b/linkerd/meshtls/boring/src/client.rs @@ -1,7 +1,7 @@ -use crate::creds::verify; use crate::creds::CredsRx; use linkerd_identity as id; use linkerd_io as io; +use linkerd_meshtls_util as util; use linkerd_stack::{NewService, Service}; use linkerd_tls::{client::AlpnProtocols, ClientTls, NegotiatedProtocolRef, ServerName}; use std::{future::Future, pin::Pin, sync::Arc, task::Context}; @@ -106,8 +106,8 @@ where let cert = io.ssl().peer_certificate().ok_or_else(|| { io::Error::new(io::ErrorKind::Other, "could not extract peer cert") })?; - verify::verify_id(&cert, &server_id) - .map_err(|e| io::Error::new(io::ErrorKind::Other, e))?; + let cert_der = id::DerX509(cert.to_der()?); + util::verify_id(&cert_der, &server_id)?; debug!( tls = io.ssl().version_str(), diff --git a/linkerd/meshtls/boring/src/creds.rs b/linkerd/meshtls/boring/src/creds.rs index 9b38e60e22..87d37e4460 100644 --- a/linkerd/meshtls/boring/src/creds.rs +++ b/linkerd/meshtls/boring/src/creds.rs @@ -1,6 +1,5 @@ mod receiver; mod store; -pub(crate) mod verify; pub use self::{receiver::Receiver, store::Store}; use boring::{ diff --git a/linkerd/meshtls/boring/src/creds/store.rs b/linkerd/meshtls/boring/src/creds/store.rs index 96b436f9cb..11817e8b7e 100644 --- a/linkerd/meshtls/boring/src/creds/store.rs +++ b/linkerd/meshtls/boring/src/creds/store.rs @@ -1,7 +1,8 @@ -use super::{verify, BaseCreds, Certs, Creds, CredsTx}; +use super::{BaseCreds, Certs, Creds, CredsTx}; use boring::x509::{X509StoreContext, X509}; use linkerd_error::Result; use linkerd_identity as id; +use linkerd_meshtls_util as util; use std::sync::Arc; pub struct Store { @@ -33,13 +34,13 @@ impl id::Credentials for Store { /// Publishes TLS client and server configurations using fn set_certificate( &mut self, - id::DerX509(leaf): id::DerX509, + id::DerX509(leaf_der): id::DerX509, intermediates: Vec, _expiry: std::time::SystemTime, ) -> Result<()> { - let leaf = X509::from_der(&leaf)?; + let leaf = X509::from_der(&leaf_der)?; - verify::verify_id(&leaf, &self.id)?; + util::verify_id(&leaf_der, &self.id)?; let intermediates = intermediates .into_iter() diff --git a/linkerd/meshtls/boring/src/creds/verify.rs b/linkerd/meshtls/boring/src/creds/verify.rs deleted file mode 100644 index b53f9088ec..0000000000 --- a/linkerd/meshtls/boring/src/creds/verify.rs +++ /dev/null @@ -1,62 +0,0 @@ -use boring::x509::X509; -use linkerd_dns_name as dns; -use linkerd_identity as id; -use std::io; - -pub(crate) fn verify_id(cert: &X509, id: &id::Id) -> io::Result<()> { - for san in cert.subject_alt_names().into_iter().flatten() { - if let Some(n) = san.dnsname() { - match n.parse::() { - Ok(name) if *name == id.to_str() => return Ok(()), - Ok(_) => {} - Err(error) => tracing::warn!(%error, "DNS SAN name {} could not be parsed", n), - } - } - } - - Err(io::Error::new( - io::ErrorKind::Other, - "certificate does not match TLS identity", - )) -} - -#[cfg(test)] -mod tests { - use super::verify_id; - use boring::x509::X509; - use linkerd_meshtls_test_util as test_util; - - fn vec_to_cert(data: Vec) -> X509 { - X509::from_der(data.as_slice()).expect("should parse") - } - - #[test] - fn cert_with_dns_san_matches_dns_id() { - test_util::cert_with_dns_san_matches_dns_id(verify_id, vec_to_cert); - } - - #[test] - fn cert_with_dns_san_does_not_match_dns_id() { - test_util::cert_with_dns_san_does_not_match_dns_id(verify_id, vec_to_cert); - } - - #[test] - fn cert_with_uri_san_does_not_match_dns_id() { - test_util::cert_with_uri_san_does_not_match_dns_id(verify_id, vec_to_cert); - } - - #[test] - fn cert_with_no_san_does_not_verify_for_dns_id() { - test_util::cert_with_no_san_does_not_verify_for_dns_id(verify_id, vec_to_cert); - } - - #[test] - fn cert_with_dns_multiple_sans_one_matches_dns_id() { - test_util::cert_with_dns_multiple_sans_one_matches_dns_id(verify_id, vec_to_cert); - } - - #[test] - fn cert_with_dns_multiple_sans_none_matches_dns_id() { - test_util::cert_with_dns_multiple_sans_none_matches_dns_id(verify_id, vec_to_cert); - } -} diff --git a/linkerd/meshtls/boring/src/server.rs b/linkerd/meshtls/boring/src/server.rs index 0203106667..08a358979c 100644 --- a/linkerd/meshtls/boring/src/server.rs +++ b/linkerd/meshtls/boring/src/server.rs @@ -1,6 +1,7 @@ use crate::creds::CredsRx; use linkerd_dns_name as dns; use linkerd_io as io; +use linkerd_meshtls_util as util; use linkerd_stack::{Param, Service}; use linkerd_tls::{ClientId, NegotiatedProtocol, ServerName, ServerTls}; use std::{future::Future, pin::Pin, sync::Arc, task::Context}; @@ -110,25 +111,22 @@ impl ServerIo { } fn client_identity(&self) -> Option { - let cert = self.0.ssl().peer_certificate().or_else(|| { - debug!("Connection missing peer certificate"); - None - })?; - let sans = cert.subject_alt_names().or_else(|| { - debug!("Peer certificate missing SANs"); - None - })?; - sans.into_iter() - .filter_map(|san| { - let dns = san.dnsname()?; - let name = dns.parse::().ok()?; - Some(ClientId(name.into())) - }) - .next() - .or_else(|| { - debug!("Peer certificate missing DNS SANs"); + match self.0.ssl().peer_certificate() { + Some(cert) => { + let der = cert + .to_der() + .map_err( + |error| tracing::warn!(%error, "Failed to encode client end cert to der"), + ) + .ok()?; + + util::client_identity(&der).map(ClientId) + } + None => { + debug!("Connection missing peer certificate"); None - }) + } + } } } diff --git a/linkerd/meshtls/rustls/Cargo.toml b/linkerd/meshtls/rustls/Cargo.toml index 8086751744..756f9db646 100644 --- a/linkerd/meshtls/rustls/Cargo.toml +++ b/linkerd/meshtls/rustls/Cargo.toml @@ -26,9 +26,9 @@ linkerd-identity = { path = "../../identity" } linkerd-stack = { path = "../../stack" } linkerd-tls = { path = "../../tls" } linkerd-tls-test-util = { path = "../../tls/test-util", optional = true } +linkerd-meshtls-util = { path = "../util" } [dev-dependencies] linkerd-tls-test-util = { path = "../../tls/test-util" } linkerd-meshtls = { path = "../../meshtls" } -linkerd-meshtls-test-util = { path = "../test-util" } diff --git a/linkerd/meshtls/rustls/src/client.rs b/linkerd/meshtls/rustls/src/client.rs index 7024bb40c0..3b88de6a10 100644 --- a/linkerd/meshtls/rustls/src/client.rs +++ b/linkerd/meshtls/rustls/src/client.rs @@ -1,7 +1,7 @@ -use super::creds::verify; use futures::prelude::*; use linkerd_identity as id; use linkerd_io as io; +use linkerd_meshtls_util as util; use linkerd_stack::{NewService, Service}; use linkerd_tls::{client::AlpnProtocols, ClientTls, NegotiatedProtocolRef}; use std::{convert::TryFrom, pin::Pin, sync::Arc, task::Context}; @@ -113,7 +113,7 @@ where let s = s?; let (_, conn) = s.get_ref(); let end_cert = extract_cert(conn)?; - verify::verify_id(end_cert, &server_id)?; + util::verify_id(&end_cert.0, &server_id)?; Ok(ClientIo(s)) }), ) diff --git a/linkerd/meshtls/rustls/src/creds/store.rs b/linkerd/meshtls/rustls/src/creds/store.rs index d3ef9c75d4..ff1ee4fc77 100644 --- a/linkerd/meshtls/rustls/src/creds/store.rs +++ b/linkerd/meshtls/rustls/src/creds/store.rs @@ -1,8 +1,8 @@ use super::params::*; -use super::verify; use linkerd_dns_name as dns; use linkerd_error::Result; use linkerd_identity as id; +use linkerd_meshtls_util as util; use ring::{rand, signature::EcdsaKeyPair}; use std::{convert::TryFrom, sync::Arc}; use tokio::sync::watch; @@ -128,7 +128,7 @@ impl Store { )?; // verify the id as the cert verifier does not do that (on purpose) - verify::verify_id(end_entity, &self.server_id).map_err(Into::into) + util::verify_id(&end_entity.0, &self.server_id).map_err(Into::into) } } diff --git a/linkerd/meshtls/rustls/src/creds/verify.rs b/linkerd/meshtls/rustls/src/creds/verify.rs index 5a19bbf558..be7058bf57 100644 --- a/linkerd/meshtls/rustls/src/creds/verify.rs +++ b/linkerd/meshtls/rustls/src/creds/verify.rs @@ -1,7 +1,6 @@ -use linkerd_identity as id; use std::convert::TryFrom; +use std::sync::Arc; use std::time::SystemTime; -use std::{io, sync::Arc}; use tokio_rustls::rustls::{ self, client::{self, ServerCertVerified, ServerCertVerifier}, @@ -51,56 +50,3 @@ impl ServerCertVerifier for AnySanVerifier { Ok(ServerCertVerified::assertion()) } } - -pub(crate) fn verify_id(end_entity: &Certificate, id: &id::Id) -> io::Result<()> { - // XXX(zahari) Currently this logic is the same as in rustls::client::WebPkiVerifier. - // This will eventually change to support SPIFFE IDs - let cert = ParsedCertificate::try_from(end_entity) - .map_err(|e| io::Error::new(io::ErrorKind::Other, e))?; - let server_name = - rustls::ServerName::try_from(id.to_str().as_ref()).expect("id must be a valid DNS name"); - - rustls::client::verify_server_name(&cert, &server_name) - .map_err(|e| io::Error::new(io::ErrorKind::Other, e)) -} - -#[cfg(test)] -mod tests { - use super::verify_id; - use linkerd_meshtls_test_util as test_util; - use tokio_rustls::rustls::Certificate; - - fn vec_to_cert(data: Vec) -> Certificate { - Certificate(data) - } - - #[test] - fn cert_with_dns_san_matches_dns_id() { - test_util::cert_with_dns_san_matches_dns_id(verify_id, vec_to_cert); - } - - #[test] - fn cert_with_dns_san_does_not_match_dns_id() { - test_util::cert_with_dns_san_does_not_match_dns_id(verify_id, vec_to_cert); - } - - #[test] - fn cert_with_uri_san_does_not_match_dns_id() { - test_util::cert_with_uri_san_does_not_match_dns_id(verify_id, vec_to_cert); - } - - #[test] - fn cert_with_no_san_does_not_verify_for_dns_id() { - test_util::cert_with_no_san_does_not_verify_for_dns_id(verify_id, vec_to_cert); - } - - #[test] - fn cert_with_dns_multiple_sans_one_matches_dns_id() { - test_util::cert_with_dns_multiple_sans_one_matches_dns_id(verify_id, vec_to_cert); - } - - #[test] - fn cert_with_dns_multiple_sans_none_matches_dns_id() { - test_util::cert_with_dns_multiple_sans_none_matches_dns_id(verify_id, vec_to_cert); - } -} diff --git a/linkerd/meshtls/rustls/src/server.rs b/linkerd/meshtls/rustls/src/server.rs index 60f7b456da..5d8be7d677 100644 --- a/linkerd/meshtls/rustls/src/server.rs +++ b/linkerd/meshtls/rustls/src/server.rs @@ -1,9 +1,10 @@ use futures::prelude::*; use linkerd_dns_name as dns; use linkerd_io as io; +use linkerd_meshtls_util as util; use linkerd_stack::{Param, Service}; use linkerd_tls::{ClientId, NegotiatedProtocol, NegotiatedProtocolRef, ServerName, ServerTls}; -use std::{convert::TryFrom, pin::Pin, sync::Arc, task::Context}; +use std::{pin::Pin, sync::Arc, task::Context}; use thiserror::Error; use tokio::sync::watch; use tokio_rustls::rustls::{Certificate, ServerConfig}; @@ -129,30 +130,8 @@ fn client_identity(tls: &tokio_rustls::server::TlsStream) -> Option() - .map_err(|error| tracing::warn!(%error, "Client certificate contained an invalid DNS name")) - .ok()?; - Some(ClientId(n.into())) + + util::client_identity(c).map(ClientId) } // === impl ServerIo === diff --git a/linkerd/meshtls/test-util/src/lib.rs b/linkerd/meshtls/test-util/src/lib.rs deleted file mode 100644 index 94df109c8b..0000000000 --- a/linkerd/meshtls/test-util/src/lib.rs +++ /dev/null @@ -1,86 +0,0 @@ -use linkerd_identity as id; -use rcgen::generate_simple_self_signed; -use std::io::Result; -use std::str::FromStr; - -fn generate_cert_with_names(names: Vec<&str>, vec_to_cert: impl FnOnce(Vec) -> C) -> C { - let sans: Vec = names.into_iter().map(|s| s.into()).collect(); - - let cert_data = generate_simple_self_signed(sans) - .expect("should generate cert") - .serialize_der() - .expect("should serialize"); - - vec_to_cert(cert_data) -} - -pub fn cert_with_dns_san_matches_dns_id( - verify_id: impl FnOnce(&C, &id::Id) -> Result<()>, - vec_to_cert: impl FnOnce(Vec) -> C, -) { - let dns_name = "foo.ns1.serviceaccount.identity.linkerd.cluster.local"; - let cert = generate_cert_with_names(vec![dns_name], vec_to_cert); - let id = id::Id::from_str(dns_name).expect("should parse DNS id"); - assert!(verify_id(&cert, &id).is_ok()); -} - -pub fn cert_with_dns_san_does_not_match_dns_id( - verify_id: impl FnOnce(&C, &id::Id) -> Result<()>, - vec_to_cert: impl FnOnce(Vec) -> C, -) { - let dns_name_cert = vec!["foo.ns1.serviceaccount.identity.linkerd.cluster.local"]; - let dns_name = "bar.ns1.serviceaccount.identity.linkerd.cluster.local"; - - let cert = generate_cert_with_names(dns_name_cert, vec_to_cert); - let id = id::Id::from_str(dns_name).expect("should parse DNS id"); - assert!(verify_id(&cert, &id).is_err()); -} - -pub fn cert_with_uri_san_does_not_match_dns_id( - verify_id: impl FnOnce(&C, &id::Id) -> Result<()>, - vec_to_cert: impl FnOnce(Vec) -> C, -) { - let uri_name_cert = vec!["spiffe://some-trust-comain/some-system/some-component"]; - let dns_name = "bar.ns1.serviceaccount.identity.linkerd.cluster.local"; - - let cert = generate_cert_with_names(uri_name_cert, vec_to_cert); - let id = id::Id::from_str(dns_name).expect("should parse DNS id"); - assert!(verify_id(&cert, &id).is_err()); -} - -pub fn cert_with_no_san_does_not_verify_for_dns_id( - verify_id: impl FnOnce(&C, &id::Id) -> Result<()>, - vec_to_cert: impl FnOnce(Vec) -> C, -) { - let dns_name = "bar.ns1.serviceaccount.identity.linkerd.cluster.local"; - let cert = generate_cert_with_names(vec![], vec_to_cert); - let id = id::Id::from_str(dns_name).expect("should parse DNS id"); - assert!(verify_id(&cert, &id).is_err()); -} - -pub fn cert_with_dns_multiple_sans_one_matches_dns_id( - verify_id: impl FnOnce(&C, &id::Id) -> Result<()>, - vec_to_cert: impl FnOnce(Vec) -> C, -) { - let foo_dns_id = "foo.ns1.serviceaccount.identity.linkerd.cluster.local"; - let bar_dns_id = "bar.ns1.serviceaccount.identity.linkerd.cluster.local"; - let spiffe_id = "spiffe://some-trust-comain/some-system/some-component"; - - let cert = generate_cert_with_names(vec![foo_dns_id, bar_dns_id, spiffe_id], vec_to_cert); - let id = id::Id::from_str(foo_dns_id).expect("should parse DNS id"); - assert!(verify_id(&cert, &id).is_ok()); -} - -pub fn cert_with_dns_multiple_sans_none_matches_dns_id( - verify_id: impl FnOnce(&C, &id::Id) -> Result<()>, - vec_to_cert: impl FnOnce(Vec) -> C, -) { - let foo_dns_id = "foo.ns1.serviceaccount.identity.linkerd.cluster.local"; - let bar_dns_id = "bar.ns1.serviceaccount.identity.linkerd.cluster.local"; - let nar_dns_id = "nar.ns1.serviceaccount.identity.linkerd.cluster.local"; - let spiffe_id = "spiffe://some-trust-comain/some-system/some-component"; - - let cert = generate_cert_with_names(vec![foo_dns_id, bar_dns_id, spiffe_id], vec_to_cert); - let id = id::Id::from_str(nar_dns_id).expect("should parse DNS id"); - assert!(verify_id(&cert, &id).is_err()); -} diff --git a/linkerd/meshtls/tests/util.rs b/linkerd/meshtls/tests/util.rs index bac3d1326a..2cf1e38a8c 100644 --- a/linkerd/meshtls/tests/util.rs +++ b/linkerd/meshtls/tests/util.rs @@ -3,7 +3,6 @@ use futures::prelude::*; use linkerd_conditional::Conditional; -use linkerd_dns_name as dns; use linkerd_error::Infallible; use linkerd_identity::{Credentials, DerX509}; use linkerd_io::{self as io, AsyncRead, AsyncReadExt, AsyncWrite, AsyncWriteExt}; @@ -50,10 +49,11 @@ pub async fn plaintext(mode: meshtls::Mode) { pub async fn proxy_to_proxy_tls_works(mode: meshtls::Mode) { let (_foo, _, server_tls) = load(mode, &test_util::FOO_NS1); let (_bar, client_tls, _) = load(mode, &test_util::BAR_NS1); - let server_id = tls::ServerId(test_util::FOO_NS1.name.parse().unwrap()); + let server_id = tls::ServerId(test_util::FOO_NS1.id.parse().unwrap()); + let server_name = tls::ServerName(test_util::FOO_NS1.name.parse().unwrap()); let (client_result, server_result) = run_test( client_tls.clone(), - Conditional::Some(server_id.clone()), + Conditional::Some((server_id.clone(), server_name.clone())), |conn| write_then_read(conn, PING), server_tls, |(_, conn)| read_then_write(conn, PING.len(), PONG), @@ -61,7 +61,11 @@ pub async fn proxy_to_proxy_tls_works(mode: meshtls::Mode) { .await; assert_eq!( client_result.tls, - Some(Conditional::Some(tls::ClientTls::new(server_id, None))) + Some(Conditional::Some(tls::ClientTls::new( + server_id, + server_name, + None + ))) ); assert_eq!(&client_result.result.expect("pong")[..], PONG); assert_eq!( @@ -80,11 +84,12 @@ pub async fn proxy_to_proxy_tls_pass_through_when_identity_does_not_match(mode: // Misuse the client's identity instead of the server's identity. Any // identity other than `server_tls.server_identity` would work. let (_bar, client_tls, _) = load(mode, &test_util::BAR_NS1); - let server_name = test_util::BAR_NS1.name.parse::().unwrap(); + let server_id = test_util::BAR_NS1.id.parse::().unwrap(); + let server_name = test_util::BAR_NS1.name.parse::().unwrap(); let (client_result, server_result) = run_test( client_tls, - Conditional::Some(tls::ServerId(server_name.clone().into())), + Conditional::Some((server_id.clone(), server_name.clone())), |conn| write_then_read(conn, PING), server_tls, |(_, conn)| read_then_write(conn, START_OF_TLS.len(), PONG), @@ -98,7 +103,7 @@ pub async fn proxy_to_proxy_tls_pass_through_when_identity_does_not_match(mode: assert_eq!( server_result.tls, Some(Conditional::Some(tls::ServerTls::Passthru { - sni: tls::ServerName(server_name) + sni: server_name })) ); assert_eq!(&server_result.result.unwrap()[..], START_OF_TLS); @@ -152,7 +157,7 @@ type ClientIo = /// side. async fn run_test( client_tls: meshtls::NewClient, - client_server_id: Conditional, + client_server_id: Conditional<(tls::ServerId, tls::ServerName), tls::NoClientTls>, client: C, server_tls: meshtls::Server, server: S, @@ -232,14 +237,14 @@ where let tls = Some( client_server_id .clone() - .map(|s| tls::ClientTls::new(s, None)), + .map(|(id, name)| tls::ClientTls::new(id, name, None)), ); let client = async move { let conn = tls::Client::layer(client_tls) .layer(ConnectTcp::new(Keepalive(None))) .oneshot(Target( server_addr.into(), - client_server_id.map(|s| tls::ClientTls::new(s, None)), + client_server_id.map(|(id, name)| tls::ClientTls::new(id, name, None)), )) .await; match conn { diff --git a/linkerd/meshtls/test-util/Cargo.toml b/linkerd/meshtls/util/Cargo.toml similarity index 58% rename from linkerd/meshtls/test-util/Cargo.toml rename to linkerd/meshtls/util/Cargo.toml index 0ef56f08d2..caa66b84ac 100644 --- a/linkerd/meshtls/test-util/Cargo.toml +++ b/linkerd/meshtls/util/Cargo.toml @@ -1,11 +1,19 @@ [package] -name = "linkerd-meshtls-test-util" +name = "linkerd-meshtls-util" version = "0.1.0" authors = ["Linkerd Developers "] license = "Apache-2.0" -edition = "2018" +edition = "2021" publish = false [dependencies] +tracing = "0.1" +x509-parser = "0.15.1" + +linkerd-error = { path = "../../error" } linkerd-identity = { path = "../../identity" } + + +[dev-dependencies] rcgen = "0.11.3" + diff --git a/linkerd/meshtls/util/src/lib.rs b/linkerd/meshtls/util/src/lib.rs new file mode 100644 index 0000000000..16401dcf6f --- /dev/null +++ b/linkerd/meshtls/util/src/lib.rs @@ -0,0 +1,287 @@ +use linkerd_error::Result; +use linkerd_identity::Id; +use std::io; +use std::str::FromStr; + +fn extract_ids_from_cert(cert: &[u8]) -> Result> { + use x509_parser::prelude::*; + let (_, c) = X509Certificate::from_der(cert)?; + let names = c + .subject_alternative_name()? + .map(|x| &x.value.general_names); + + if let Some(names) = names { + return Ok(names + .iter() + .filter_map(|n| { + let name = match n { + GeneralName::DNSName(dns) => Some(dns), + GeneralName::URI(uri) => Some(uri), + _ => None, + }?; + + if *name == "*" { + // Wildcards can perhaps be handled in a future path... + return None; + } + + match Id::from_str(name) { + Ok(id) => Some(id), + Err(error) => { + tracing::warn!(%error, "SAN name {} could not be parsed", n); + None + } + } + }) + .collect()); + } + Ok(Vec::default()) +} + +pub fn client_identity(cert: &[u8]) -> Option { + let ids = extract_ids_from_cert(cert) + .map_err(|error| tracing::warn!(%error, "Failed to extract tls id from client end cert")) + .ok()?; + + ids.first().cloned() +} + +pub fn verify_id(cert: &[u8], expected_id: &Id) -> io::Result<()> { + // TODO: we need to ensure that we follow SVID verification + // requirementes for x509 certs outlined in: + // https://github.com/spiffe/spiffe/blob/main/standards/X509-SVID.md#the-x509-spiffe-verifiable-identity-document + let ids = extract_ids_from_cert(cert) + .map_err(|error| tracing::warn!(%error, "Failed to extract tls id from client end cert")) + .ok() + .unwrap_or_default(); + + if ids.contains(expected_id) { + return Ok(()); + } + + Err(io::Error::new( + io::ErrorKind::Other, + "certificate does not match TLS identity", + )) +} + +#[cfg(test)] +mod tests { + use crate::{client_identity, verify_id, Id}; + use rcgen::generate_simple_self_signed; + use std::str::FromStr; + + fn generate_cert_with_names(names: Vec<&str>) -> Vec { + let sans: Vec = names.into_iter().map(|s| s.into()).collect(); + + generate_simple_self_signed(sans) + .expect("should generate cert") + .serialize_der() + .expect("should serialize") + } + + #[test] + fn cert_with_dns_san_matches_dns_id() { + let dns_name = "foo.ns1.serviceaccount.identity.linkerd.cluster.local"; + let cert = generate_cert_with_names(vec![dns_name]); + let id = Id::from_str(dns_name).expect("should parse DNS id"); + assert!(verify_id(&cert, &id).is_ok()); + } + + #[test] + fn cert_with_spiffe_san_matches_spiffe_id() { + let spiffe_uri = "spiffe://identity.linkerd.cluster.local/ns/ns1/sa/foo"; + let cert = generate_cert_with_names(vec![spiffe_uri]); + let id = Id::from_str(spiffe_uri).expect("should parse SPIFFE id"); + assert!(verify_id(&cert, &id).is_ok()); + } + + #[test] + fn cert_with_dns_san_does_not_match_dns_id() { + let dns_name_cert = vec!["foo.ns1.serviceaccount.identity.linkerd.cluster.local"]; + let dns_name = "bar.ns1.serviceaccount.identity.linkerd.cluster.local"; + + let cert = generate_cert_with_names(dns_name_cert); + let id = Id::from_str(dns_name).expect("should parse DNS id"); + assert!(verify_id(&cert, &id).is_err()); + } + + #[test] + fn cert_with_spiffe_san_does_not_match_spiffe_id() { + let spiffe_uri_cert = vec!["spiffe://identity.linkerd.cluster.local/ns/ns1/sa/foo"]; + let spiffe_uri = "spiffe://identity.linkerd.cluster.local/ns/ns1/sa/bar"; + + let cert = generate_cert_with_names(spiffe_uri_cert); + let id = Id::from_str(spiffe_uri).expect("should parse SPIFFE id"); + assert!(verify_id(&cert, &id).is_err()); + } + + #[test] + fn cert_with_spiffe_san_does_not_match_dns_id() { + let uri_cert = vec!["spiffe://some-trust-comain/some-system/some-component"]; + let dns_name = "bar.ns1.serviceaccount.identity.linkerd.cluster.local"; + + let cert = generate_cert_with_names(uri_cert); + let id = Id::from_str(dns_name).expect("should parse DNS id"); + assert!(verify_id(&cert, &id).is_err()); + } + + #[test] + fn cert_with_dns_san_does_not_match_spiffe_id() { + let dns_name_cert = vec!["bar.ns1.serviceaccount.identity.linkerd.cluster.local"]; + let spiffe_uri = "spiffe://some-trust-comain/some-system/some-component"; + + let cert = generate_cert_with_names(dns_name_cert); + let id = Id::from_str(spiffe_uri).expect("should parse SPIFFE id"); + assert!(verify_id(&cert, &id).is_err()); + } + + #[test] + fn cert_with_no_san_does_not_verify_for_dns_id() { + let dns_name = "bar.ns1.serviceaccount.identity.linkerd.cluster.local"; + let cert = generate_cert_with_names(vec![]); + let id = Id::from_str(dns_name).expect("should parse DNS id"); + assert!(verify_id(&cert, &id).is_err()); + } + + #[test] + fn cert_with_no_san_does_not_verify_for_spiffe_id() { + let spiffe_uri = "spiffe://some-trust-comain/some-system/some-component"; + let cert = generate_cert_with_names(vec![]); + let id = Id::from_str(spiffe_uri).expect("should parse SPIFFE id"); + assert!(verify_id(&cert, &id).is_err()); + } + + #[test] + fn cert_with_multiple_sans_one_matches_dns_id() { + let foo_dns_id = "foo.ns1.serviceaccount.identity.linkerd.cluster.local"; + let bar_dns_id = "bar.ns1.serviceaccount.identity.linkerd.cluster.local"; + let spiffe_id = "spiffe://some-trust-comain/some-system/some-component"; + + let cert = generate_cert_with_names(vec![foo_dns_id, bar_dns_id, spiffe_id]); + let id = Id::from_str(foo_dns_id).expect("should parse DNS id"); + assert!(verify_id(&cert, &id).is_ok()); + } + + #[test] + fn cert_with_dns_multiple_sans_one_matches_spiffe_id() { + let foo_dns_id = "foo.ns1.serviceaccount.identity.linkerd.cluster.local"; + let bar_dns_id = "bar.ns1.serviceaccount.identity.linkerd.cluster.local"; + let spiffe_id = "spiffe://some-trust-comain/some-system/some-component"; + + let cert = generate_cert_with_names(vec![foo_dns_id, bar_dns_id, spiffe_id]); + let id = Id::from_str(spiffe_id).expect("should parse SPIFFE id"); + assert!(verify_id(&cert, &id).is_ok()); + } + + #[test] + fn cert_with_dns_multiple_sans_none_matches_dns_id() { + let foo_dns_id = "foo.ns1.serviceaccount.identity.linkerd.cluster.local"; + let bar_dns_id = "bar.ns1.serviceaccount.identity.linkerd.cluster.local"; + let nar_dns_id = "nar.ns1.serviceaccount.identity.linkerd.cluster.local"; + let spiffe_id = "spiffe://some-trust-comain/some-system/some-component"; + + let cert = generate_cert_with_names(vec![foo_dns_id, bar_dns_id, spiffe_id]); + let id = Id::from_str(nar_dns_id).expect("should parse DNS id"); + assert!(verify_id(&cert, &id).is_err()); + } + + #[test] + fn cert_with_multiple_sans_none_matches_spiffe_id() { + let foo_dns_id = "foo.ns1.serviceaccount.identity.linkerd.cluster.local"; + let bar_dns_id = "bar.ns1.serviceaccount.identity.linkerd.cluster.local"; + let nar_dns_id = "nar.ns1.serviceaccount.identity.linkerd.cluster.local"; + let spiffe_id = "spiffe://some-trust-comain/some-system/some-component"; + + let cert = generate_cert_with_names(vec![foo_dns_id, bar_dns_id, nar_dns_id]); + let id = Id::from_str(spiffe_id).expect("should parse SPIFFE id"); + assert!(verify_id(&cert, &id).is_err()); + } + + #[test] + fn can_extract_spiffe_client_identity_one_san() { + let spiffe_id = "spiffe://some-trust-comain/some-system/some-component"; + + let cert = generate_cert_with_names(vec![spiffe_id]); + let id = Id::from_str(spiffe_id).expect("should parse SPIFFE id"); + let client_id = client_identity(&cert); + assert_eq!(client_id, Some(id)); + } + + #[test] + fn can_extract_spiffe_client_identity_many_sans() { + let spiffe_id = "spiffe://some-trust-comain/some-system/some-component"; + let bar_dns_id = "bar.ns1.serviceaccount.identity.linkerd.cluster.local"; + let nar_dns_id = "nar.ns1.serviceaccount.identity.linkerd.cluster.local"; + + let cert = generate_cert_with_names(vec![spiffe_id, bar_dns_id, nar_dns_id, spiffe_id]); + let id = Id::from_str(spiffe_id).expect("should parse SPIFFE id"); + let client_id = client_identity(&cert); + assert_eq!(client_id, Some(id)); + } + + #[test] + fn can_extract_dns_client_identity_one_san() { + let dns_id = "foo.ns1.serviceaccount.identity.linkerd.cluster.local"; + + let cert = generate_cert_with_names(vec![dns_id]); + let id = Id::from_str(dns_id).expect("should parse DNS id"); + let client_id = client_identity(&cert); + assert_eq!(client_id, Some(id)); + } + + #[test] + fn can_extract_dns_client_identity_many_sans() { + let dns_id = "foo.ns1.serviceaccount.identity.linkerd.cluster.local"; + let bar_dns_id = "bar.ns1.serviceaccount.identity.linkerd.cluster.local"; + let nar_dns_id = "nar.ns1.serviceaccount.identity.linkerd.cluster.local"; + let spiffe_id = "spiffe://some-trust-comain/some-system/some-component"; + + let cert = generate_cert_with_names(vec![dns_id, bar_dns_id, nar_dns_id, spiffe_id]); + let id = Id::from_str(dns_id).expect("should parse DNS id"); + let client_id = client_identity(&cert); + assert_eq!(client_id, Some(id)); + } + + #[test] + fn extracts_dns_client_id_when_others_not_uri_or_dns() { + let dns_id = "foo.ns1.serviceaccount.identity.linkerd.cluster.local"; + let email_san_1 = "foo@foo.com"; + let email_san_2 = "bar@bar.com"; + + let cert = generate_cert_with_names(vec![dns_id, email_san_1, email_san_2]); + let id = Id::from_str(dns_id).expect("should parse DNS id"); + let client_id = client_identity(&cert); + assert_eq!(client_id, Some(id)); + } + + #[test] + fn extracts_spiffe_client_id_when_others_not_uri_or_dns() { + let spiffe_id = "spiffe://some-trust-comain/some-system/some-component"; + let email_san_1 = "foo@foo.com"; + let email_san_2 = "bar@bar.com"; + + let cert = generate_cert_with_names(vec![spiffe_id, email_san_1, email_san_2]); + let id = Id::from_str(spiffe_id).expect("should parse SPIFFE id"); + let client_id = client_identity(&cert); + assert_eq!(client_id, Some(id)); + } + + #[test] + fn skips_uri_san_with_non_spiffe_scheme() { + let spiffe_id = "https://some-trust-comain/some-system/some-component"; + + let cert = generate_cert_with_names(vec![spiffe_id]); + let client_id = client_identity(&cert); + assert_eq!(client_id, None); + } + + #[test] + fn skips_dns_san_with_trailing_dor() { + let dns_id = "foo.ns1.serviceaccount.identity.linkerd.cluster.local."; + + let cert = generate_cert_with_names(vec![dns_id]); + let client_id = client_identity(&cert); + assert_eq!(client_id, None); + } +} diff --git a/linkerd/proxy/api-resolve/src/metadata.rs b/linkerd/proxy/api-resolve/src/metadata.rs index dd1623a959..936f42d65d 100644 --- a/linkerd/proxy/api-resolve/src/metadata.rs +++ b/linkerd/proxy/api-resolve/src/metadata.rs @@ -1,5 +1,6 @@ use http::uri::Authority; use linkerd_tls::client::ServerId; +use linkerd_tls::ServerName; use std::collections::BTreeMap; /// Endpoint labels are lexigraphically ordered by key. @@ -18,7 +19,7 @@ pub struct Metadata { tagged_transport_port: Option, /// How to verify TLS for the endpoint. - identity: Option, + identity: Option<(ServerId, ServerName)>, /// Used to override the the authority if needed authority_override: Option, @@ -55,7 +56,7 @@ impl Metadata { labels: impl IntoIterator, protocol_hint: ProtocolHint, tagged_transport_port: Option, - identity: Option, + identity: Option<(ServerId, ServerName)>, authority_override: Option, ) -> Self { Self { @@ -76,7 +77,7 @@ impl Metadata { self.protocol_hint } - pub fn identity(&self) -> Option<&ServerId> { + pub fn identity(&self) -> Option<&(ServerId, ServerName)> { self.identity.as_ref() } diff --git a/linkerd/proxy/api-resolve/src/pb.rs b/linkerd/proxy/api-resolve/src/pb.rs index 889dc6860c..d54350ad7e 100644 --- a/linkerd/proxy/api-resolve/src/pb.rs +++ b/linkerd/proxy/api-resolve/src/pb.rs @@ -8,6 +8,7 @@ use crate::{ }; use http::uri::Authority; use linkerd_tls::client::ServerId; +use linkerd_tls::ServerName; use std::{collections::HashMap, net::SocketAddr, str::FromStr}; /// Construct a new labeled `SocketAddr `from a protobuf `WeightedAddr`. @@ -39,7 +40,7 @@ pub fn to_addr_meta( } } - let tls_id = pb.tls_identity.and_then(to_id); + let tls_id = pb.tls_identity.and_then(to_identity); let meta = Metadata::new( labels, proto_hint, @@ -50,13 +51,15 @@ pub fn to_addr_meta( Some((addr, meta)) } -fn to_id(pb: TlsIdentity) -> Option { +fn to_identity(pb: TlsIdentity) -> Option<(ServerId, ServerName)> { + // TODO: differenciate between id and name, once this is + // available in the API use crate::api::destination::tls_identity::Strategy; let Strategy::DnsLikeIdentity(i) = pb.strategy?; - match ServerId::from_str(&i.name) { - Ok(i) => Some(i), - Err(_) => { + match (ServerId::from_str(&i.name), ServerName::from_str(&i.name)) { + (Ok(i), Ok(name)) => Some((i, name)), + (_, _) => { tracing::warn!("Ignoring invalid identity: {}", i.name); None } diff --git a/linkerd/tls/src/client.rs b/linkerd/tls/src/client.rs index ca18c84e37..79f9cac809 100644 --- a/linkerd/tls/src/client.rs +++ b/linkerd/tls/src/client.rs @@ -1,7 +1,6 @@ use crate::{NegotiatedProtocol, ServerName}; use futures::prelude::*; use linkerd_conditional::Conditional; -use linkerd_dns_name as dns; use linkerd_identity as id; use linkerd_io as io; use linkerd_stack::{layer, MakeConnection, NewService, Oneshot, Param, Service, ServiceExt}; @@ -78,11 +77,9 @@ pub struct ConnectMeta { // === impl ClientTls === impl ClientTls { - // XXX(ver) We'll have to change this when ServerIds are not necessarily DNS names. - pub fn new(server_id: ServerId, alpn: Option) -> Self { - let ServerId(linkerd_identity::Id(name)) = server_id.clone(); + pub fn new(server_id: ServerId, server_name: ServerName, alpn: Option) -> Self { Self { - server_name: ServerName(name), + server_name, server_id, alpn, } @@ -211,11 +208,9 @@ impl Deref for ServerId { } impl FromStr for ServerId { - type Err = dns::InvalidName; + type Err = linkerd_error::Error; fn from_str(s: &str) -> Result { - // TODO Handle SPIFFE IDs. - let n = dns::Name::from_str(s)?; - Ok(Self(n.into())) + id::Id::from_str(s).map(Self) } } diff --git a/linkerd/tls/test-util/src/lib.rs b/linkerd/tls/test-util/src/lib.rs index 7ed17257e5..20a6ee70a6 100644 --- a/linkerd/tls/test-util/src/lib.rs +++ b/linkerd/tls/test-util/src/lib.rs @@ -3,6 +3,7 @@ pub struct Entity { pub name: &'static str, + pub id: &'static str, pub trust_anchors: &'static [u8], pub crt: &'static [u8], pub key: &'static [u8], @@ -10,6 +11,7 @@ pub struct Entity { pub static DEFAULT_DEFAULT: Entity = Entity { name: "default.default.serviceaccount.identity.linkerd.cluster.local", + id: "default.default.serviceaccount.identity.linkerd.cluster.local", trust_anchors: include_bytes!("testdata/ca1.pem"), crt: include_bytes!("testdata/default-default-ca1/crt.der"), key: include_bytes!("testdata/default-default-ca1/key.p8"), @@ -17,6 +19,7 @@ pub static DEFAULT_DEFAULT: Entity = Entity { pub static FOO_NS1: Entity = Entity { name: "foo.ns1.serviceaccount.identity.linkerd.cluster.local", + id: "foo.ns1.serviceaccount.identity.linkerd.cluster.local", trust_anchors: include_bytes!("testdata/ca1.pem"), crt: include_bytes!("testdata/foo-ns1-ca1/crt.der"), key: include_bytes!("testdata/foo-ns1-ca1/key.p8"), @@ -24,6 +27,7 @@ pub static FOO_NS1: Entity = Entity { pub static FOO_NS1_CA2: Entity = Entity { name: "foo.ns1.serviceaccount.identity.linkerd.cluster.local", + id: "foo.ns1.serviceaccount.identity.linkerd.cluster.local", trust_anchors: include_bytes!("testdata/ca2.pem"), crt: include_bytes!("testdata/foo-ns1-ca2/crt.der"), key: include_bytes!("testdata/foo-ns1-ca2/key.p8"), @@ -31,6 +35,7 @@ pub static FOO_NS1_CA2: Entity = Entity { pub static BAR_NS1: Entity = Entity { name: "bar.ns1.serviceaccount.identity.linkerd.cluster.local", + id: "bar.ns1.serviceaccount.identity.linkerd.cluster.local", trust_anchors: include_bytes!("testdata/ca1.pem"), crt: include_bytes!("testdata/bar-ns1-ca1/crt.der"), key: include_bytes!("testdata/bar-ns1-ca1/key.p8"),