meshtls: Consolidate TLS ID verification (#2534)

The rustls and boring meshtls crates each implement SAN verification independently. In preparation to support URI SAN verification, this change introduces a new meshtls-verifier crate that uses x509-parser to implement DNS SAN verification. To support this, we have disabled the TLS backends' default certificate verification mechanisms. Signed-off-by: Zahari Dichev <[email protected]>
linkerd · Nov 28, 2023 · 374e75f · 374e75f
1 parent 9c56e91
commit 374e75f
Show file tree

Hide file tree

Showing 16 changed files with 328 additions and 266 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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<id::DerX509>,
         _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()