Merge pull request #1075 from jvanz/mtls

feat: enable mTLS.

Author: flavio

Cargo.lock

@@ -42,6 +42,7 @@ rustls = { version = "0.23", default-features = false, features = [
   "tls12",
 ] }
 rustls-pki-types = { version = "1", features = ["alloc"] }
+rustls-pemfile = "2.2.0"
 rayon = "1.10"
 regex = "1.10"
 serde_json = "1.0"

Cargo.toml

@@ -25,6 +25,9 @@ This document contains the help content for the `policy-server` command-line pro
 * `--always-accept-admission-reviews-on-namespace <NAMESPACE>` — Always accept AdmissionReviews that target the given namespace
 * `--cert-file <CERT_FILE>` — Path to an X.509 certificate file for HTTPS
 
+  Default value: ``
+* `--client-ca-file <CLIENT_CA_FILE>` — Path to an CA certificate file that issued the client certificate. Required to enable mTLS
+
   Default value: ``
 * `--daemon` — If set, runs policy-server in detached mode as a daemon
 * `--daemon-pid-file <DAEMON-PID-FILE>` — Path to the PID file, used only when running in daemon mode

cli-docs.md

@@ -88,6 +88,13 @@ pub(crate) fn build_cli() -> Command {
             .env("KUBEWARDEN_KEY_FILE")
             .help("Path to an X.509 private key file for HTTPS"),
 
+        Arg::new("client-ca-file")
+            .long("client-ca-file")
+            .value_name("CLIENT_CA_FILE")
+            .default_value("")
+            .env("KUBEWARDEN_CLIENT_CA_FILE")
+            .help("Path to an CA certificate file that issued the client certificate. Required to enable mTLS"),
+
         Arg::new("policies")
             .long("policies")
             .value_name("POLICIES_FILE")

src/cli.rs

@@ -53,6 +53,7 @@ pub struct Config {
 pub struct TlsConfig {
     pub cert_file: String,
     pub key_file: String,
+    pub client_ca_cert_file: Option<String>,
 }
 
 impl Config {
@@ -127,15 +128,8 @@ impl Config {
             .expect("clap should have assigned a default value")
             .to_owned();
 
-        let (cert_file, key_file) = tls_files(matches)?;
-        let tls_config = if cert_file.is_empty() {
-            None
-        } else {
-            Some(TlsConfig {
-                cert_file,
-                key_file,
-            })
-        };
+        let tls_config = Some(build_tls_config(matches)?);
+
         let enable_pprof = matches
             .get_one::<bool>("enable-pprof")
             .expect("clap should have assigned a default value")
@@ -182,14 +176,18 @@ fn api_bind_address(matches: &clap::ArgMatches) -> Result<SocketAddr> {
     .map_err(|e| anyhow!("error parsing arguments: {}", e))
 }
 
-fn tls_files(matches: &clap::ArgMatches) -> Result<(String, String)> {
+fn build_tls_config(matches: &clap::ArgMatches) -> Result<TlsConfig> {
     let cert_file = matches.get_one::<String>("cert-file").unwrap().to_owned();
     let key_file = matches.get_one::<String>("key-file").unwrap().to_owned();
+    let client_ca_cert_file = matches.get_one::<String>("client-ca-file").cloned();
     if cert_file.is_empty() != key_file.is_empty() {
-        Err(anyhow!("error parsing arguments: either both --cert-file and --key-file must be provided, or neither"))
-    } else {
-        Ok((cert_file, key_file))
+        return Err(anyhow!("error parsing arguments: either both --cert-file and --key-file must be provided, or neither"));
     }
+    Ok(TlsConfig {
+        cert_file,
+        key_file,
+        client_ca_cert_file,
+    })
 }
 
 fn policies(matches: &clap::ArgMatches) -> Result<HashMap<String, PolicyOrPolicyGroup>> {

src/config.rs

@@ -271,6 +271,84 @@ impl PolicyServer {
     }
 }
 
+// Load the ServerConfig to be used by the Policy Server configuring the server
+// certificate and mTLS when necessary
+//
+// RustlsConfig does not offer a function to load the client CA certificate together with the
+// service certificates. Therefore, we need to load everything and build the ServerConfig
+async fn build_tls_server_config(tls_config: &TlsConfig) -> Result<rustls::ServerConfig> {
+    use std::{fs::File, io::BufReader};
+
+    use rustls::{server::WebPkiClientVerifier, RootCertStore, ServerConfig};
+    use rustls_pemfile::Item;
+    use rustls_pki_types::{CertificateDer, PrivateKeyDer};
+
+    let cert_file = &mut BufReader::new(File::open(tls_config.cert_file.clone())?);
+    let key_file = &mut BufReader::new(File::open(tls_config.key_file.clone())?);
+    let cert: Vec<CertificateDer> = rustls_pemfile::certs(cert_file)
+        .filter_map(|it| {
+            if let Err(ref e) = it {
+                warn!("Cannot parse certificate: {e}");
+                return None;
+            }
+            it.ok()
+        })
+        .collect();
+    if cert.len() > 1 {
+        return Err(anyhow!("Multiple certificates provided in cert file"));
+    }
+    let mut key_vec: Vec<Vec<u8>> = rustls_pemfile::read_all(key_file)
+        .filter_map(|i| match i.ok()? {
+            Item::Sec1Key(key) => Some(key.secret_sec1_der().to_vec()),
+            Item::Pkcs1Key(key) => Some(key.secret_pkcs1_der().to_vec()),
+            Item::Pkcs8Key(key) => Some(key.secret_pkcs8_der().to_vec()),
+            _ => {
+                info!("Ignoring non-key item in key file");
+                None
+            }
+        })
+        .collect();
+    if key_vec.is_empty() {
+        return Err(anyhow!("No key provided in key file"));
+    }
+    if key_vec.len() > 1 {
+        return Err(anyhow!("Multiple keys provided in key file"));
+    }
+    let key = PrivateKeyDer::try_from(key_vec.pop().unwrap())
+        .map_err(|e| anyhow!("Cannot parse server key: {e}"))?;
+
+    let config = if let Some(client_ca_cert_file_path) = tls_config.client_ca_cert_file.clone() {
+        // we have the client CA. Therefore, we should enable mTLS.
+        let client_ca_cert_file = &mut BufReader::new(File::open(client_ca_cert_file_path)?);
+
+        let mut ca_certs = RootCertStore::empty();
+        let client_ca_certs: Vec<_> = rustls_pemfile::certs(client_ca_cert_file)
+            .filter_map(|it| {
+                if let Err(ref e) = it {
+                    warn!("Cannot parse client CA certificate: {e}");
+                }
+                it.ok()
+            })
+            .collect();
+        let (cert_added, cert_ignored) = ca_certs.add_parsable_certificates(client_ca_certs);
+        info!(
+            client_ca_certs_added = cert_added,
+            client_ca_certs_ignored = cert_ignored,
+            "Loaded client CA certificates"
+        );
+        let client_verifier = WebPkiClientVerifier::builder(Arc::new(ca_certs)).build()?;
+
+        ServerConfig::builder()
+            .with_client_cert_verifier(client_verifier)
+            .with_single_cert(cert, key)?
+    } else {
+        ServerConfig::builder()
+            .with_no_client_auth()
+            .with_single_cert(cert, key)?
+    };
+    Ok(config)
+}
+
 /// There's no watching of the certificate files on non-linux platforms
 /// since we rely on inotify to watch for changes
 #[cfg(not(target_os = "linux"))]
@@ -293,24 +371,36 @@ async fn create_tls_config_and_watch_certificate_changes(
 ) -> Result<RustlsConfig> {
     use ::tracing::error;
 
-    let cert_file = tls_config.cert_file.clone();
-    let key_file = tls_config.key_file.clone();
+    let cert_file_path = tls_config.cert_file.clone();
+    let key_file_path = tls_config.key_file.clone();
+    let client_ca_cert_path = tls_config.client_ca_cert_file.clone();
+
+    let config = build_tls_server_config(&tls_config).await?;
 
-    let rust_config =
-        RustlsConfig::from_pem_file(tls_config.cert_file, tls_config.key_file).await?;
+    let rust_config = RustlsConfig::from_config(Arc::new(config));
     let reloadable_rust_config = rust_config.clone();
 
     let inotify =
         inotify::Inotify::init().map_err(|e| anyhow!("Cannot initialize inotify: {e}"))?;
     let cert_watch = inotify
         .watches()
-        .add(cert_file.clone(), inotify::WatchMask::CLOSE_WRITE)
+        .add(cert_file_path.clone(), inotify::WatchMask::CLOSE_WRITE)
         .map_err(|e| anyhow!("Cannot watch certificate file: {e}"))?;
     let key_watch = inotify
         .watches()
-        .add(key_file.clone(), inotify::WatchMask::CLOSE_WRITE)
+        .add(key_file_path.clone(), inotify::WatchMask::CLOSE_WRITE)
         .map_err(|e| anyhow!("Cannot watch key file: {e}"))?;
 
+    let mut client_cert_watch = None;
+    if let Some(ref client_cert_file) = client_ca_cert_path {
+        client_cert_watch = Some(
+            inotify
+                .watches()
+                .add(client_cert_file.clone(), inotify::WatchMask::CLOSE_WRITE)
+                .map_err(|e| anyhow!("Cannot watch client certificate file: {e}"))?,
+        );
+    }
+
     let buffer = [0; 1024];
     let stream = inotify
         .into_event_stream(buffer)
@@ -320,6 +410,7 @@ async fn create_tls_config_and_watch_certificate_changes(
         tokio::pin!(stream);
         let mut cert_changed = false;
         let mut key_changed = false;
+        let mut client_cert_changed = false;
 
         while let Some(event) = stream.next().await {
             let event = match event {
@@ -338,18 +429,29 @@ async fn create_tls_config_and_watch_certificate_changes(
                 info!("TLS key file has been modified");
                 key_changed = true;
             }
+            if let Some(ref client_cert_watch) = client_cert_watch {
+                if event.wd == *client_cert_watch {
+                    info!("TLS client certificate file has been modified");
+                    client_cert_changed = true;
+                }
+            }
 
-            if key_changed && cert_changed {
-                info!("reloading TLS certificate");
+            // if both the certificate and the key have been changed or there is no change in the
+            // server cert and key, but the client cert changed, reload the certificate
+            if (key_changed && cert_changed)
+                || (client_cert_changed && (key_changed == cert_changed))
+            {
+                info!("reloading TLS certificates");
 
                 cert_changed = false;
                 key_changed = false;
-                if let Err(e) = reloadable_rust_config
-                    .reload_from_pem_file(cert_file.clone(), key_file.clone())
-                    .await
-                {
+                client_cert_changed = false;
+                let server_config = build_tls_server_config(&tls_config).await;
+                if let Err(e) = server_config {
                     error!("Failed to reload TLS certificate: {}", e);
+                    continue;
                 }
+                reloadable_rust_config.reload_from_config(Arc::new(server_config.unwrap()))
             }
         }
     });

src/lib.rs

@@ -654,9 +654,18 @@ mod certificate_reload_helpers {
         }
     }
 
-    pub async fn policy_server_is_ready(address: &str) -> anyhow::Result<StatusCode> {
+    pub async fn policy_server_is_ready(
+        address: &str,
+        client_tls_pem_bundle: Option<String>,
+    ) -> anyhow::Result<StatusCode> {
         // wait for the server to start
-        let client = reqwest::Client::builder()
+        let mut client_builder = reqwest::Client::builder();
+
+        if let Some(tls_data) = client_tls_pem_bundle {
+            let identity = reqwest::Identity::from_pem(tls_data.as_bytes())?;
+            client_builder = client_builder.identity(identity)
+        };
+        let client = client_builder
             .danger_accept_invalid_certs(true)
             .build()
             .unwrap();
@@ -677,17 +686,21 @@ async fn test_detect_certificate_rotation() {
     let certs_dir = tempfile::tempdir().unwrap();
     let cert_file = certs_dir.path().join("policy-server.pem");
     let key_file = certs_dir.path().join("policy-server-key.pem");
+    let client_ca = certs_dir.path().join("client_cert.pem");
 
     let hostname1 = "cert1.example.com";
     let tls_data1 = create_cert(hostname1);
+    let tls_data_client = create_cert(hostname1);
 
     std::fs::write(&cert_file, tls_data1.cert).unwrap();
     std::fs::write(&key_file, tls_data1.key).unwrap();
+    std::fs::write(&client_ca, tls_data_client.cert.clone()).unwrap();
 
     let mut config = default_test_config();
     config.tls_config = Some(policy_server::config::TlsConfig {
-        cert_file: cert_file.to_str().unwrap().to_string(),
-        key_file: key_file.to_str().unwrap().to_string(),
+        cert_file: cert_file.to_str().unwrap().to_owned(),
+        key_file: key_file.to_str().unwrap().to_owned(),
+        client_ca_cert_file: Some(client_ca.to_str().unwrap().to_owned()),
     });
     config.policies = HashMap::new();
 
@@ -706,11 +719,18 @@ async fn test_detect_certificate_rotation() {
         .with_max_delay(Duration::from_secs(30))
         .with_max_times(5);
 
-    let status_code =
-        (|| async { policy_server_is_ready(format!("{domain_ip}:{domain_port}").as_str()).await })
-            .retry(exponential_backoff)
-            .await
-            .unwrap();
+    let client_cert = tls_data_client.cert.clone();
+    let client_key = tls_data_client.key.clone();
+    let status_code = (|| async {
+        policy_server_is_ready(
+            format!("{domain_ip}:{domain_port}").as_str(),
+            Some(format!("{client_cert}\n{client_key}")),
+        )
+        .await
+    })
+    .retry(exponential_backoff)
+    .await
+    .unwrap();
     assert_eq!(status_code, reqwest::StatusCode::OK);
 
     check_tls_san_name(&domain_ip, &domain_port, hostname1)
@@ -721,6 +741,7 @@ async fn test_detect_certificate_rotation() {
 
     let hostname2 = "cert2.example.com";
     let tls_data2 = create_cert(hostname2);
+    let client_ca2 = create_cert(hostname2);
 
     // write only the cert file
     std::fs::write(&cert_file, tls_data2.cert).unwrap();
@@ -742,6 +763,29 @@ async fn test_detect_certificate_rotation() {
     check_tls_san_name(&domain_ip, &domain_port, hostname2)
         .await
         .expect("certificate hasn't been reloaded");
+
+    // Let test if the server is reloading client certificate
+    std::fs::write(&client_ca, client_ca2.cert.clone()).unwrap();
+
+    // give inotify some time to ensure it detected the cert change
+    tokio::time::sleep(std::time::Duration::from_secs(4)).await;
+
+    assert!(policy_server_is_ready(
+        format!("{domain_ip}:{domain_port}").as_str(),
+        Some(format!("{client_cert}\n{client_key}")),
+    )
+    .await
+    .is_err());
+
+    let client_cert = client_ca2.cert.clone();
+    let client_key = client_ca2.key.clone();
+    let status_code = policy_server_is_ready(
+        format!("{domain_ip}:{domain_port}").as_str(),
+        Some(format!("{client_cert}\n{client_key}")),
+    )
+    .await
+    .unwrap();
+    assert_eq!(status_code, reqwest::StatusCode::OK);
 }
 
 #[tokio::test]