dry tests

Signed-off-by: Zahari Dichev <zaharidichev@gmail.com>

Cargo.lock

@@ -1406,6 +1406,7 @@ dependencies = [
  "linkerd-error",
  "linkerd-identity",
  "linkerd-io",
+ "linkerd-meshtls",
  "linkerd-stack",
  "linkerd-tls",
  "linkerd-tls-test-util",
@@ -1424,6 +1425,7 @@ dependencies = [
  "linkerd-error",
  "linkerd-identity",
  "linkerd-io",
+ "linkerd-meshtls",
  "linkerd-stack",
  "linkerd-tls",
  "linkerd-tls-test-util",

linkerd/meshtls/boring/Cargo.toml

@@ -26,3 +26,5 @@ fips = ["boring/fips"]
 [dev-dependencies]
 linkerd-tls-test-util = { path = "../../tls/test-util" }
 rcgen = "0.11.3"
+linkerd-meshtls = { path = "../../meshtls" }
+

linkerd/meshtls/boring/src/client.rs

@@ -93,23 +93,23 @@ where
                     // XXX(ver) to use the boring error directly here we have to constraint the socket on Sync +
                     // std::fmt::Debug, which is a pain.
                     None => io::Error::new(io::ErrorKind::Other, "unexpected TLS handshake error"),
-                }).and_then(|io| match io.ssl().peer_certificate() {
-                    Some(ref 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),
-                            peer.cert = ?io.ssl().peer_certificate().as_deref().and_then(super::fingerprint),
-                            alpn = ?io.ssl().selected_alpn_protocol(),
-                            "Initiated TLS connection"
-                        );
-                        Ok(ClientIo(io))
-                    },
-                    None => Err(io::Error::new(
-                        io::ErrorKind::Other,
-                        "could not extract peer cert",
-                    )),
+                }).and_then(|io| {
+                    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),
+                        peer.cert = ?io.ssl().peer_certificate().as_deref().and_then(super::fingerprint),
+                        alpn = ?io.ssl().selected_alpn_protocol(),
+                        "Initiated TLS connection"
+                    );
+                    Ok(ClientIo(io))
                 })
         })
     }

linkerd/meshtls/boring/src/creds/verify.rs

@@ -6,12 +6,11 @@ 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() {
-            if let Ok(name) = n.parse::<dns::Name>() {
-                if *name == id.to_str() {
-                    return Ok(());
-                }
+            match n.parse::<dns::Name>() {
+                Ok(name) if *name == id.to_str() => return Ok(()),
+                Ok(_) => {}
+                Err(error) => tracing::warn!(%error, "DNS SAN name {} could not be parsed", n),
             }
-            tracing::warn!("DNS SAN name {} could not be parsed", n)
         }
     }
 
@@ -23,56 +22,11 @@ pub(crate) fn verify_id(cert: &X509, id: &id::Id) -> io::Result<()> {
 
 #[cfg(test)]
 mod tests {
-    use super::verify_id;
     use boring::x509::X509;
-    use linkerd_identity as id;
-    use rcgen::generate_simple_self_signed;
-    use std::str::FromStr;
 
-    fn generate_cert_with_name(name: Option<&str>) -> X509 {
-        let sans = name.map(|s| vec![s.into()]).unwrap_or_default();
-
-        let cert_data = generate_simple_self_signed(sans)
-            .expect("should generate cert")
-            .serialize_der()
-            .expect("should serialize");
-
-        X509::from_der(cert_data.as_slice()).expect("should parse")
-    }
-
-    #[test]
-    fn cert_with_dns_san_matches_dns_id() {
-        let dns_name = "foo.ns1.serviceaccount.identity.linkerd.cluster.local";
-        let cert = generate_cert_with_name(Some(dns_name));
-        let id = id::Id::from_str(dns_name).expect("should parse DNS id");
-        assert!(verify_id(&cert, &id).is_ok());
+    fn vec_to_cert(data: Vec<u8>) -> X509 {
+        X509::from_der(data.as_slice()).expect("should parse")
     }
 
-    #[test]
-    fn cert_with_dns_san_does_not_match_dns_id() {
-        let dns_name_cert = "foo.ns1.serviceaccount.identity.linkerd.cluster.local";
-        let dns_name = "bar.ns1.serviceaccount.identity.linkerd.cluster.local";
-
-        let cert = generate_cert_with_name(Some(dns_name_cert));
-        let id = id::Id::from_str(dns_name).expect("should parse DNS id");
-        assert!(verify_id(&cert, &id).is_err());
-    }
-
-    #[test]
-    fn cert_with_uri_san_does_not_match_dns_id() {
-        let uri_name_cert = "spiffe://some-trust-comain/some-system/some-component";
-        let dns_name = "bar.ns1.serviceaccount.identity.linkerd.cluster.local";
-
-        let cert = generate_cert_with_name(Some(uri_name_cert));
-        let id = 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_dns_id() {
-        let dns_name = "bar.ns1.serviceaccount.identity.linkerd.cluster.local";
-        let cert = generate_cert_with_name(None);
-        let id = id::Id::from_str(dns_name).expect("should parse DNS id");
-        assert!(verify_id(&cert, &id).is_err());
-    }
+    linkerd_meshtls::generate_verify_id_tests!(X509, vec_to_cert);
 }

linkerd/meshtls/rustls/Cargo.toml

@@ -29,4 +29,5 @@ linkerd-tls-test-util = { path = "../../tls/test-util", optional = true }
 
 [dev-dependencies]
 linkerd-tls-test-util = { path = "../../tls/test-util" }
+linkerd-meshtls = { path = "../../meshtls" }
 rcgen = "0.11.3"

linkerd/meshtls/rustls/src/creds/verify.rs

@@ -57,56 +57,11 @@ pub(crate) fn verify_id(end_entity: &Certificate, id: &id::Id) -> io::Result<()>
 
 #[cfg(test)]
 mod tests {
-    use super::verify_id;
-    use linkerd_identity as id;
-    use rcgen::generate_simple_self_signed;
-    use std::str::FromStr;
     use tokio_rustls::rustls::Certificate;
 
-    fn generate_cert_with_name(name: Option<&str>) -> Certificate {
-        let sans = name.map(|s| vec![s.into()]).unwrap_or_default();
-
-        let cert_data = generate_simple_self_signed(sans)
-            .expect("should generate cert")
-            .serialize_der()
-            .expect("should serialize");
-
-        Certificate(cert_data)
-    }
-
-    #[test]
-    fn cert_with_dns_san_matches_dns_id() {
-        let dns_name = "foo.ns1.serviceaccount.identity.linkerd.cluster.local";
-        let cert = generate_cert_with_name(Some(dns_name));
-        let id = id::Id::from_str(dns_name).expect("should parse DNS id");
-        assert!(verify_id(&cert, &id).is_ok());
+    fn vec_to_cert(data: Vec<u8>) -> Certificate {
+        Certificate(data)
     }
 
-    #[test]
-    fn cert_with_dns_san_does_not_match_dns_id() {
-        let dns_name_cert = "foo.ns1.serviceaccount.identity.linkerd.cluster.local";
-        let dns_name = "bar.ns1.serviceaccount.identity.linkerd.cluster.local";
-
-        let cert = generate_cert_with_name(Some(dns_name_cert));
-        let id = id::Id::from_str(dns_name).expect("should parse DNS id");
-        assert!(verify_id(&cert, &id).is_err());
-    }
-
-    #[test]
-    fn cert_with_uri_san_does_not_match_dns_id() {
-        let uri_name_cert = "spiffe://some-trust-comain/some-system/some-component";
-        let dns_name = "bar.ns1.serviceaccount.identity.linkerd.cluster.local";
-
-        let cert = generate_cert_with_name(Some(uri_name_cert));
-        let id = 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_dns_id() {
-        let dns_name = "bar.ns1.serviceaccount.identity.linkerd.cluster.local";
-        let cert = generate_cert_with_name(None);
-        let id = id::Id::from_str(dns_name).expect("should parse DNS id");
-        assert!(verify_id(&cert, &id).is_err());
-    }
+    linkerd_meshtls::generate_verify_id_tests!(Certificate, vec_to_cert);
 }

linkerd/meshtls/src/lib.rs

@@ -16,6 +16,8 @@ mod client;
 pub mod creds;
 mod server;
 
+pub mod verify_tests;
+
 pub use self::{
     client::{ClientIo, Connect, ConnectFuture, NewClient},
     server::{Server, ServerIo, TerminateFuture},

linkerd/meshtls/src/verify_tests.rs

@@ -0,0 +1,79 @@
+#[macro_export]
+macro_rules! generate_verify_id_tests {
+    ($cert_type:ty, $vec_to_cert: ident) => {
+        use super::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>) -> $cert_type {
+            let sans: Vec<String> = 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)
+        }
+
+        #[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::Id::from_str(dns_name).expect("should parse DNS 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::Id::from_str(dns_name).expect("should parse DNS id");
+            assert!(verify_id(&cert, &id).is_err());
+        }
+
+        #[test]
+        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]
+        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]
+        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]
+        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());
+        }
+    };
+}