diff --git a/Cargo.lock b/Cargo.lock index fc5c708a4d..b4b26c4d1f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -141,6 +141,12 @@ version = "0.13.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9e1b586273c5702936fe7b7d6896644d8be71e6314cfe09d3167c95f712589e8" +[[package]] +name = "base64" +version = "0.21.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "35636a1494ede3b646cc98f74f8e62c773a38a659ebc777a2cf26b9b74171df9" + [[package]] name = "bindgen" version = "0.66.1" @@ -285,6 +291,15 @@ dependencies = [ "gzip-header", ] +[[package]] +name = "deranged" +version = "0.3.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0f32d04922c60427da6f9fef14d042d9edddef64cb9d4ce0d64d0685fbeb1fd3" +dependencies = [ + "powerfmt", +] + [[package]] name = "derive_arbitrary" version = "1.3.0" @@ -1391,6 +1406,8 @@ dependencies = [ "linkerd-error", "linkerd-identity", "linkerd-io", + "linkerd-meshtls", + "linkerd-meshtls-test-util", "linkerd-stack", "linkerd-tls", "linkerd-tls-test-util", @@ -1408,6 +1425,8 @@ dependencies = [ "linkerd-error", "linkerd-identity", "linkerd-io", + "linkerd-meshtls", + "linkerd-meshtls-test-util", "linkerd-stack", "linkerd-tls", "linkerd-tls-test-util", @@ -1420,6 +1439,14 @@ dependencies = [ "tracing", ] +[[package]] +name = "linkerd-meshtls-test-util" +version = "0.1.0" +dependencies = [ + "linkerd-identity", + "rcgen", +] + [[package]] name = "linkerd-metrics" version = "0.1.0" @@ -1850,7 +1877,7 @@ dependencies = [ name = "linkerd-trace-context" version = "0.1.0" dependencies = [ - "base64", + "base64 0.13.1", "bytes", "futures", "hex", @@ -2151,6 +2178,16 @@ version = "0.1.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "19b17cddbe7ec3f8bc800887bab5e717348c95ea2ca0b1bf0837fb964dc67099" +[[package]] +name = "pem" +version = "3.0.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3163d2912b7c3b52d651a055f2c7eec9ba5cd22d26ef75b8dd3a59980b185923" +dependencies = [ + "base64 0.21.5", + "serde", +] + [[package]] name = "percent-encoding" version = "2.2.0" @@ -2199,6 +2236,12 @@ version = "0.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8b870d8c151b6f2fb93e84a13146138f05d02ed11c7e7c54f8826aaaf7c9f184" +[[package]] +name = "powerfmt" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "439ee305def115ba05938db6eb1644ff94165c5ab5e9420d1c1bcedbba909391" + [[package]] name = "ppv-lite86" version = "0.2.17" @@ -2351,6 +2394,18 @@ version = "1.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8b9283c6b06096b47afc7109834fdedab891175bb5241ee5d4f7d2546549f263" +[[package]] +name = "rcgen" +version = "0.11.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "52c4f3084aa3bc7dfbba4eff4fab2a54db4324965d8872ab933565e6fbd83bc6" +dependencies = [ + "pem", + "ring", + "time", + "yasna", +] + [[package]] name = "redox_syscall" version = "0.2.15" @@ -2472,7 +2527,7 @@ version = "1.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0864aeff53f8c05aa08d86e5ef839d3dfcf07aeba2db32f12db0ef716e87bd55" dependencies = [ - "base64", + "base64 0.13.1", ] [[package]] @@ -2521,9 +2576,9 @@ checksum = "bebd363326d05ec3e2f532ab7660680f3b02130d780c299bca73469d521bc0ed" [[package]] name = "serde" -version = "1.0.159" +version = "1.0.185" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3c04e8343c3daeec41f58990b9d77068df31209f2af111e059e9fe9646693065" +checksum = "be9b6f69f1dfd54c3b568ffa45c310d6973a5e5148fd40cf515acaf38cf5bc31" [[package]] name = "serde_json" @@ -2672,6 +2727,24 @@ dependencies = [ "once_cell", ] +[[package]] +name = "time" +version = "0.3.30" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c4a34ab300f2dee6e562c10a046fc05e358b29f9bf92277f30c3c8d82275f6f5" +dependencies = [ + "deranged", + "powerfmt", + "serde", + "time-core", +] + +[[package]] +name = "time-core" +version = "0.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ef927ca75afb808a4d64dd374f00a2adf8d0fcff8e7b184af886c3c87ec4a3f3" + [[package]] name = "tinyvec" version = "1.6.0" @@ -2797,7 +2870,7 @@ dependencies = [ "async-stream", "async-trait", "axum", - "base64", + "base64 0.13.1", "bytes", "futures-core", "futures-util", @@ -3349,6 +3422,15 @@ dependencies = [ "winapi", ] +[[package]] +name = "yasna" +version = "0.5.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e17bb3549cc1321ae1296b9cdc2698e2b6cb1992adfa19a8c72e5b7a738f44cd" +dependencies = [ + "time", +] + [[package]] name = "zerocopy" version = "0.7.3" diff --git a/Cargo.toml b/Cargo.toml index b96f0e90ed..407cca0e76 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -36,6 +36,7 @@ members = [ "linkerd/meshtls", "linkerd/meshtls/boring", "linkerd/meshtls/rustls", + "linkerd/meshtls/test-util", "linkerd/metrics", "linkerd/opencensus", "linkerd/proxy/api-resolve", diff --git a/deny.toml b/deny.toml index 82a7a4b867..441e22fbf2 100644 --- a/deny.toml +++ b/deny.toml @@ -73,6 +73,9 @@ skip = [ # `tonic` v0.6 depends on `bitflags` v1.x, while `boring-sys` depends on # `bitflags` v2.x. Allow both versions to coexist peacefully for now. { name = "bitflags", version = "1" }, + # `linkerd-trace-context`, `rustls-pemfile` and `tonic` depend on `base64` + # v0.13.1 while `rcgen` depends on v0.21.5 + { name = "base64" }, ] skip-tree = [ # right now we have a mix of versions of this crate in the ecosystem diff --git a/linkerd/meshtls/boring/Cargo.toml b/linkerd/meshtls/boring/Cargo.toml index 3de97a0557..ef08409db7 100644 --- a/linkerd/meshtls/boring/Cargo.toml +++ b/linkerd/meshtls/boring/Cargo.toml @@ -25,3 +25,6 @@ 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 9415f9af2d..8d972ae2cf 100644 --- a/linkerd/meshtls/boring/src/client.rs +++ b/linkerd/meshtls/boring/src/client.rs @@ -1,9 +1,11 @@ +use crate::creds::verify; use crate::creds::CredsRx; +use linkerd_identity as id; use linkerd_io as io; use linkerd_stack::{NewService, Service}; use linkerd_tls::{client::AlpnProtocols, ClientTls, NegotiatedProtocolRef, ServerName}; use std::{future::Future, pin::Pin, sync::Arc, task::Context}; -use tracing::debug; +use tracing::{debug, trace}; #[derive(Clone)] pub struct NewClient(CredsRx); @@ -12,6 +14,7 @@ pub struct NewClient(CredsRx); pub struct Connect { rx: CredsRx, alpn: Option]>>, + id: id::Id, server: ServerName, } @@ -50,6 +53,7 @@ impl Connect { rx, alpn: client_tls.alpn.map(|AlpnProtocols(ps)| ps.into()), server: client_tls.server_name, + id: client_tls.server_id.into(), } } } @@ -68,25 +72,43 @@ where fn call(&mut self, io: I) -> Self::Future { let server_name = self.server.clone(); + let server_id = self.id.clone(); let connector = self .rx .borrow() .connector(self.alpn.as_deref().unwrap_or(&[])); Box::pin(async move { - let conn = connector.map_err(|e| io::Error::new(io::ErrorKind::Other, e))?; - let config = conn + let config = connector + .map_err(|e| io::Error::new(io::ErrorKind::Other, e))? .configure() .map_err(|e| io::Error::new(io::ErrorKind::Other, e))?; - let io = tokio_boring::connect(config, server_name.as_str(), io) + + // Establish a TLS connection to the server using the provided + // `server_name` as an SNI value to the server. + // + // Hostname verification is DISABLED, as we do not require that the + // peer's certificate actually matches the `server_name`. Instead, + // the `server_id` is used to perform the appropriate form of + // verification after the session is established. + let io = tokio_boring::connect(config.verify_hostname(false), server_name.as_str(), io) .await .map_err(|e| match e.as_io_error() { // TODO(ver) boring should let us take ownership of the error directly. Some(ioe) => io::Error::new(ioe.kind(), ioe.to_string()), - // XXX(ver) to use the boring error directly here we have to constraint the socket on Sync + - // std::fmt::Debug, which is a pain. + // XXX(ver) to use the boring error directly here we have to + // constrain the socket on Sync + std::fmt::Debug, which is + // a pain. None => io::Error::new(io::ErrorKind::Other, "unexpected TLS handshake error"), })?; + // Servers must present a peer certificate. We extract the x509 cert + // and verify it manually against the `server_id`. + 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))?; + debug!( tls = io.ssl().version_str(), client.cert = ?io.ssl().certificate().and_then(super::fingerprint), @@ -94,6 +116,7 @@ where alpn = ?io.ssl().selected_alpn_protocol(), "Initiated TLS connection" ); + trace!(peer.id = %server_id, peer.name = %server_name); Ok(ClientIo(io)) }) } diff --git a/linkerd/meshtls/boring/src/creds.rs b/linkerd/meshtls/boring/src/creds.rs index 49798f7362..9b38e60e22 100644 --- a/linkerd/meshtls/boring/src/creds.rs +++ b/linkerd/meshtls/boring/src/creds.rs @@ -1,5 +1,6 @@ mod receiver; mod store; +pub(crate) mod verify; pub use self::{receiver::Receiver, store::Store}; use boring::{ @@ -27,8 +28,8 @@ pub fn watch( }; let (tx, rx) = watch::channel(Creds::from(creds.clone())); - let rx = Receiver::new(local_id, server_name.clone(), rx); - let store = Store::new(creds, csr, server_name, tx); + let rx = Receiver::new(local_id.clone(), server_name, rx); + let store = Store::new(creds, csr, local_id, tx); Ok((store, rx)) } diff --git a/linkerd/meshtls/boring/src/creds/store.rs b/linkerd/meshtls/boring/src/creds/store.rs index a9223ea662..96b436f9cb 100644 --- a/linkerd/meshtls/boring/src/creds/store.rs +++ b/linkerd/meshtls/boring/src/creds/store.rs @@ -1,6 +1,5 @@ -use super::{BaseCreds, Certs, Creds, CredsTx}; +use super::{verify, BaseCreds, Certs, Creds, CredsTx}; use boring::x509::{X509StoreContext, X509}; -use linkerd_dns_name as dns; use linkerd_error::Result; use linkerd_identity as id; use std::sync::Arc; @@ -8,35 +7,21 @@ use std::sync::Arc; pub struct Store { creds: Arc, csr: Vec, - name: dns::Name, + id: id::Id, tx: CredsTx, } // === impl Store === impl Store { - pub(super) fn new(creds: Arc, csr: &[u8], name: dns::Name, tx: CredsTx) -> Self { + pub(super) fn new(creds: Arc, csr: &[u8], id: id::Id, tx: CredsTx) -> Self { Self { creds, csr: csr.into(), - name, + id, tx, } } - - fn cert_matches_name(&self, cert: &X509) -> bool { - for san in cert.subject_alt_names().into_iter().flatten() { - if let Some(n) = san.dnsname() { - if let Ok(name) = n.parse::() { - if name == self.name { - return true; - } - } - } - } - - false - } } impl id::Credentials for Store { @@ -53,9 +38,8 @@ impl id::Credentials for Store { _expiry: std::time::SystemTime, ) -> Result<()> { let leaf = X509::from_der(&leaf)?; - if !self.cert_matches_name(&leaf) { - return Err("certificate does not have a DNS name SAN for the local identity".into()); - } + + verify::verify_id(&leaf, &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 new file mode 100644 index 0000000000..b53f9088ec --- /dev/null +++ b/linkerd/meshtls/boring/src/creds/verify.rs @@ -0,0 +1,62 @@ +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/rustls/Cargo.toml b/linkerd/meshtls/rustls/Cargo.toml index 1378f158f1..8086751744 100644 --- a/linkerd/meshtls/rustls/Cargo.toml +++ b/linkerd/meshtls/rustls/Cargo.toml @@ -29,3 +29,6 @@ linkerd-tls-test-util = { path = "../../tls/test-util", optional = true } [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 a6318e1e36..7024bb40c0 100644 --- a/linkerd/meshtls/rustls/src/client.rs +++ b/linkerd/meshtls/rustls/src/client.rs @@ -1,4 +1,6 @@ +use super::creds::verify; use futures::prelude::*; +use linkerd_identity as id; use linkerd_io as io; use linkerd_stack::{NewService, Service}; use linkerd_tls::{client::AlpnProtocols, ClientTls, NegotiatedProtocolRef}; @@ -15,14 +17,12 @@ pub struct NewClient { /// A `Service` that initiates client-side TLS connections. #[derive(Clone)] pub struct Connect { + server_id: id::Id, server_name: rustls::ServerName, config: Arc, } -pub type ConnectFuture = futures::future::MapOk< - tokio_rustls::Connect, - fn(tokio_rustls::client::TlsStream) -> ClientIo, ->; +pub type ConnectFuture = Pin>> + Send>>; #[derive(Debug)] pub struct ClientIo(tokio_rustls::client::TlsStream); @@ -72,15 +72,23 @@ impl Connect { .expect("identity must be a valid DNS name"); Self { + server_id: client_tls.server_id.into(), server_name, config, } } } +fn extract_cert(c: &rustls::ClientConnection) -> io::Result<&rustls::Certificate> { + match c.peer_certificates().and_then(|certs| certs.first()) { + Some(leaf_cert) => io::Result::Ok(leaf_cert), + None => Err(io::Error::new(io::ErrorKind::Other, "missing tls end cert")), + } +} + impl Service for Connect where - I: io::AsyncRead + io::AsyncWrite + Send + Unpin, + I: io::AsyncRead + io::AsyncWrite + Send + Unpin + 'static, { type Response = ClientIo; type Error = io::Error; @@ -91,9 +99,24 @@ where } fn call(&mut self, io: I) -> Self::Future { - tokio_rustls::TlsConnector::from(self.config.clone()) - .connect(self.server_name.clone(), io) - .map_ok(ClientIo) + let server_id = self.server_id.clone(); + Box::pin( + // Connect to the server, sending the `server_name` SNI in the + // client handshake. The provided config should use the + // `AnySanVerifier` to ignore the server certificate's DNS SANs. + // Instead, we extract the server's leaf certificate after the + // handshake and verify that it matches the provided `server_id``. + tokio_rustls::TlsConnector::from(self.config.clone()) + // XXX(eliza): it's a bummer that the server name has to be cloned here... + .connect(self.server_name.clone(), io) + .map(move |s| { + let s = s?; + let (_, conn) = s.get_ref(); + let end_cert = extract_cert(conn)?; + verify::verify_id(end_cert, &server_id)?; + Ok(ClientIo(s)) + }), + ) } } diff --git a/linkerd/meshtls/rustls/src/creds.rs b/linkerd/meshtls/rustls/src/creds.rs index cc7d6631ca..33dc0756e8 100644 --- a/linkerd/meshtls/rustls/src/creds.rs +++ b/linkerd/meshtls/rustls/src/creds.rs @@ -1,5 +1,6 @@ mod receiver; mod store; +pub(crate) mod verify; pub use self::{receiver::Receiver, store::Store}; use linkerd_dns_name as dns; @@ -55,10 +56,7 @@ pub fn watch( // controlling the set of trusted signature algorithms), but they provide good enough // defaults for now. // TODO: lock down the verification further. - let server_cert_verifier = Arc::new(rustls::client::WebPkiVerifier::new( - roots.clone(), - None, // no certificate transparency policy - )); + let server_cert_verifier = Arc::new(verify::AnySanVerifier::new(roots.clone())); let (client_tx, client_rx) = { // Since we don't have a certificate yet, build a client configuration @@ -82,12 +80,13 @@ pub fn watch( watch::channel(store::server_config(roots.clone(), empty_resolver)) }; - let rx = Receiver::new(local_id, server_name.clone(), client_rx, server_rx); + let rx = Receiver::new(local_id.clone(), server_name.clone(), client_rx, server_rx); let store = Store::new( roots, server_cert_verifier, key, csr, + local_id, server_name, client_tx, server_tx, diff --git a/linkerd/meshtls/rustls/src/creds/store.rs b/linkerd/meshtls/rustls/src/creds/store.rs index 533e1d770a..d3ef9c75d4 100644 --- a/linkerd/meshtls/rustls/src/creds/store.rs +++ b/linkerd/meshtls/rustls/src/creds/store.rs @@ -1,4 +1,5 @@ use super::params::*; +use super::verify; use linkerd_dns_name as dns; use linkerd_error::Result; use linkerd_identity as id; @@ -13,6 +14,7 @@ pub struct Store { server_cert_verifier: Arc, key: Arc, csr: Arc<[u8]>, + server_id: id::Id, server_name: dns::Name, client_tx: watch::Sender>, server_tx: watch::Sender>, @@ -71,11 +73,13 @@ pub(super) fn server_config( // === impl Store === impl Store { + #[allow(clippy::too_many_arguments)] pub(super) fn new( roots: rustls::RootCertStore, server_cert_verifier: Arc, key: EcdsaKeyPair, csr: &[u8], + server_id: id::Id, server_name: dns::Name, client_tx: watch::Sender>, server_tx: watch::Sender>, @@ -85,6 +89,7 @@ impl Store { key: Arc::new(key), server_cert_verifier, csr: csr.into(), + server_id, server_name, client_tx, server_tx, @@ -121,8 +126,9 @@ impl Store { NO_OCSP, now, )?; - debug!("Certified"); - Ok(()) + + // 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) } } @@ -236,27 +242,6 @@ impl rustls::server::ResolvesServerCert for CertResolver { &self, hello: rustls::server::ClientHello<'_>, ) -> Option> { - let server_name = match hello.server_name() { - Some(name) => { - let name = webpki::DnsNameRef::try_from_ascii_str(name) - .expect("server name must be a valid server name"); - webpki::SubjectNameRef::DnsName(name) - } - None => { - debug!("no SNI -> no certificate"); - return None; - } - }; - - // Verify that our certificate is valid for the given SNI name. - let c = self.0.cert.first()?; - if let Err(error) = webpki::EndEntityCert::try_from(c.as_ref()) - .and_then(|c| c.verify_is_valid_for_subject_name(server_name)) - { - debug!(%error, "Local certificate is not valid for SNI"); - return None; - }; - self.resolve_(hello.signature_schemes()) } } diff --git a/linkerd/meshtls/rustls/src/creds/verify.rs b/linkerd/meshtls/rustls/src/creds/verify.rs new file mode 100644 index 0000000000..5a19bbf558 --- /dev/null +++ b/linkerd/meshtls/rustls/src/creds/verify.rs @@ -0,0 +1,106 @@ +use linkerd_identity as id; +use std::convert::TryFrom; +use std::time::SystemTime; +use std::{io, sync::Arc}; +use tokio_rustls::rustls::{ + self, + client::{self, ServerCertVerified, ServerCertVerifier}, + server::ParsedCertificate, + Certificate, RootCertStore, ServerName, +}; +use tracing::trace; + +pub(crate) struct AnySanVerifier(Arc); + +impl AnySanVerifier { + pub(crate) fn new(roots: impl Into>) -> Self { + Self(roots.into()) + } +} + +// This is derived from `rustls::client::WebPkiVerifier`. +// +// Copyright (c) 2016, Joseph Birr-Pixton +// https://github.com/rustls/rustls/blob/ccb79947a4811412ee7dcddcd0f51ea56bccf101/rustls/src/webpki/server_verifier.rs#L239 +// +// The only difference is that we omit the step that performs +// DNS SAN validation. The reason for that stems from the fact that +// SAN validation in rustls is limited to DNS SANs only while we +// want to support alternative SAN types (e.g. URI). +impl ServerCertVerifier for AnySanVerifier { + /// Will verify the certificate is valid in the following ways: + /// - Signed by a trusted `RootCertStore` CA + /// - Not Expired + fn verify_server_cert( + &self, + end_entity: &Certificate, + intermediates: &[Certificate], + _: &ServerName, + _: &mut dyn Iterator, + ocsp_response: &[u8], + now: SystemTime, + ) -> Result { + let cert = ParsedCertificate::try_from(end_entity)?; + + client::verify_server_cert_signed_by_trust_anchor(&cert, &self.0, intermediates, now)?; + + if !ocsp_response.is_empty() { + trace!("Unvalidated OCSP response: {ocsp_response:?}"); + } + + 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/test-util/Cargo.toml b/linkerd/meshtls/test-util/Cargo.toml new file mode 100644 index 0000000000..0ef56f08d2 --- /dev/null +++ b/linkerd/meshtls/test-util/Cargo.toml @@ -0,0 +1,11 @@ +[package] +name = "linkerd-meshtls-test-util" +version = "0.1.0" +authors = ["Linkerd Developers "] +license = "Apache-2.0" +edition = "2018" +publish = false + +[dependencies] +linkerd-identity = { path = "../../identity" } +rcgen = "0.11.3" diff --git a/linkerd/meshtls/test-util/src/lib.rs b/linkerd/meshtls/test-util/src/lib.rs new file mode 100644 index 0000000000..94df109c8b --- /dev/null +++ b/linkerd/meshtls/test-util/src/lib.rs @@ -0,0 +1,86 @@ +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()); +}