diff --git a/Cargo.lock b/Cargo.lock index 338c724708..b7503bf74f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -60,6 +60,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" @@ -361,6 +400,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" @@ -391,6 +444,17 @@ dependencies = [ "crypto-common", ] +[[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.39", +] + [[package]] name = "drain" version = "0.1.1" @@ -1517,7 +1581,7 @@ dependencies = [ "linkerd-identity", "linkerd-io", "linkerd-meshtls", - "linkerd-meshtls-test-util", + "linkerd-meshtls-verifier", "linkerd-stack", "linkerd-tls", "linkerd-tls-test-util", @@ -1536,7 +1600,7 @@ dependencies = [ "linkerd-identity", "linkerd-io", "linkerd-meshtls", - "linkerd-meshtls-test-util", + "linkerd-meshtls-verifier", "linkerd-stack", "linkerd-tls", "linkerd-tls-test-util", @@ -1550,11 +1614,14 @@ dependencies = [ ] [[package]] -name = "linkerd-meshtls-test-util" +name = "linkerd-meshtls-verifier" version = "0.1.0" dependencies = [ + "linkerd-error", "linkerd-identity", "rcgen", + "tracing", + "x509-parser", ] [[package]] @@ -2248,6 +2315,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" @@ -2276,6 +2364,15 @@ dependencies = [ "memchr", ] +[[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" @@ -2743,6 +2840,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" @@ -2976,6 +3082,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" @@ -3036,9 +3154,11 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c4a34ab300f2dee6e562c10a046fc05e358b29f9bf92277f30c3c8d82275f6f5" dependencies = [ "deranged", + "itoa", "powerfmt", "serde", "time-core", + "time-macros", ] [[package]] @@ -3047,6 +3167,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" @@ -3426,6 +3555,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" @@ -3736,6 +3871,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..118ce889c2 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/verifier", "linkerd/metrics", "linkerd/opencensus", "linkerd/proxy/api-resolve", diff --git a/linkerd/meshtls/boring/Cargo.toml b/linkerd/meshtls/boring/Cargo.toml index ef08409db7..ae928c0e90 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-verifier = { path = "../verifier" } + 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..d4f1d16cf0 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_verifier as verifier; 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()?); + verifier::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..885177e592 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_verifier as verifier; 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)?; + verifier::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..214b6de09c 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_verifier as verifier; 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()?; + + verifier::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..5a92f74d7a 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-verifier = { path = "../verifier" } [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..962135d42d 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_verifier as verifier; 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)?; + verifier::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..b1c4ffa6f5 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_verifier as verifier; 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) + verifier::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..6bde857856 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_verifier as verifier; 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())) + + verifier::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/test-util/Cargo.toml b/linkerd/meshtls/verifier/Cargo.toml similarity index 58% rename from linkerd/meshtls/test-util/Cargo.toml rename to linkerd/meshtls/verifier/Cargo.toml index 0ef56f08d2..241a2da58c 100644 --- a/linkerd/meshtls/test-util/Cargo.toml +++ b/linkerd/meshtls/verifier/Cargo.toml @@ -1,11 +1,18 @@ [package] -name = "linkerd-meshtls-test-util" +name = "linkerd-meshtls-verifier" 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/verifier/src/lib.rs b/linkerd/meshtls/verifier/src/lib.rs new file mode 100644 index 0000000000..7abe3872d2 --- /dev/null +++ b/linkerd/meshtls/verifier/src/lib.rs @@ -0,0 +1,127 @@ +use linkerd_error::Result; +use linkerd_identity::Id; +use std::io; + +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 dns_san = match n { + GeneralName::DNSName(dns) => Some(dns), + _ => None, + }?; + + dns_san.parse::().ok() + }) + .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 make sure that we handle SPIFFE IDs as well + 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::verify_id; + use linkerd_identity as 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] + pub 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::Id::from_str(dns_name).expect("should parse DNS id"); + assert!(verify_id(&cert, &id).is_ok()); + } + + #[test] + pub 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::Id::from_str(dns_name).expect("should parse DNS id"); + assert!(verify_id(&cert, &id).is_err()); + } + + #[test] + pub fn cert_with_uri_san_does_not_match_dns_id() { + 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); + let id = id::Id::from_str(dns_name).expect("should parse DNS id"); + assert!(verify_id(&cert, &id).is_err()); + } + + #[test] + pub 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::Id::from_str(dns_name).expect("should parse DNS id"); + assert!(verify_id(&cert, &id).is_err()); + } + + #[test] + pub fn cert_with_dns_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::Id::from_str(foo_dns_id).expect("should parse DNS id"); + assert!(verify_id(&cert, &id).is_ok()); + } + + #[test] + pub 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::Id::from_str(nar_dns_id).expect("should parse DNS id"); + assert!(verify_id(&cert, &id).is_err()); + } +}