Extract TLS id verification out of TLS backends

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"

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",

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

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" }
+

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<Arc<[Vec<u8>]>>,
+    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,32 +72,51 @@ 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),
                 peer.cert = ?io.ssl().peer_certificate().as_deref().and_then(super::fingerprint),
                 alpn = ?io.ssl().selected_alpn_protocol(),
                 "Initiated TLS connection"
             );
+            trace!(peer.id = %server_id, peer.name = %server_name);
             Ok(ClientIo(io))
         })
     }

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))
 }

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

@@ -1,42 +1,27 @@
-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;
 
 pub struct Store {
     creds: Arc<BaseCreds>,
     csr: Vec<u8>,
-    name: dns::Name,
+    id: id::Id,
     tx: CredsTx,
 }
 
 // === impl Store ===
 
 impl Store {
-    pub(super) fn new(creds: Arc<BaseCreds>, csr: &[u8], name: dns::Name, tx: CredsTx) -> Self {
+    pub(super) fn new(creds: Arc<BaseCreds>, 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::<dns::Name>() {
-                    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()