From b6ce541c94d144943b373967de0e484ff3c13d70 Mon Sep 17 00:00:00 2001 From: Evan Date: Sun, 22 Sep 2024 12:14:39 -0700 Subject: [PATCH] feat(mTLS): adds unit tests; fixes naming; removes unnecessary config params --- Cargo.lock | 217 ++++++++++++++++++- dataplane/api-server/Cargo.toml | 6 +- dataplane/api-server/src/config.rs | 44 ++-- dataplane/api-server/src/lib.rs | 47 ++-- dataplane/api-server/tests/test_setup_tls.rs | 92 ++++++++ dataplane/loader/src/main.rs | 4 +- 6 files changed, 353 insertions(+), 57 deletions(-) create mode 100644 dataplane/api-server/tests/test_setup_tls.rs diff --git a/Cargo.lock b/Cargo.lock index 9c01403..40c8824 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -112,6 +112,8 @@ dependencies = [ "netlink-packet-route", "netlink-sys", "prost", + "rcgen", + "tempfile", "tokio", "tonic", "tonic-build", @@ -278,6 +280,12 @@ dependencies = [ "rustc-demangle", ] +[[package]] +name = "base64" +version = "0.13.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9e1b586273c5702936fe7b7d6896644d8be71e6314cfe09d3167c95f712589e8" + [[package]] name = "base64" version = "0.21.7" @@ -302,6 +310,12 @@ version = "2.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "cf4b9d6a944f767f8e5e0db018570623c85f3d925ac718db4e06d0187adb21c1" +[[package]] +name = "bumpalo" +version = "3.16.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "79296716171880943b8470b5f8d03aa55eb2e645a4874bdbb28adb49162e012c" + [[package]] name = "byteorder" version = "1.5.0" @@ -388,6 +402,15 @@ dependencies = [ "version_check", ] +[[package]] +name = "deranged" +version = "0.3.11" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b42b6fa04a440b495c8b04d0e71b707c585f83cb9cb28cf8cd0d976c315e31b4" +dependencies = [ + "powerfmt", +] + [[package]] name = "either" version = "1.11.0" @@ -655,6 +678,15 @@ version = "1.0.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "49f1f14873335454500d59611f1cf4a4b0f786f9ac11f4312a78e4cf2566695b" +[[package]] +name = "js-sys" +version = "0.3.70" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1868808506b929d7b0cfa8f75951347aa71bb21144b7791bae35d9bccfcfe37a" +dependencies = [ + "wasm-bindgen", +] + [[package]] name = "lazy_static" version = "1.4.0" @@ -798,6 +830,12 @@ dependencies = [ "log", ] +[[package]] +name = "num-conv" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "51d515d32fb182ee37cda2ccdcb92950d6a3c2893aa280e540671c2cd0f3b1d9" + [[package]] name = "num_enum" version = "0.7.2" @@ -862,6 +900,15 @@ version = "1.0.15" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "57c0d7b74b563b49d38dae00a0c37d4d6de9b432382b2892f0574ddcae73fd0a" +[[package]] +name = "pem" +version = "1.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a8835c273a76a90455d7344889b0964598e3316e2a79ede8e36f16bdcf2228b8" +dependencies = [ + "base64 0.13.1", +] + [[package]] name = "percent-encoding" version = "2.3.1" @@ -910,6 +957,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" @@ -1027,6 +1080,18 @@ dependencies = [ "getrandom", ] +[[package]] +name = "rcgen" +version = "0.9.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6413f3de1edee53342e6138e75b56d32e7bc6e332b3bd62d497b1929d4cfbcdd" +dependencies = [ + "pem", + "ring 0.16.20", + "time", + "yasna", +] + [[package]] name = "redox_syscall" version = "0.4.1" @@ -1065,6 +1130,21 @@ version = "0.8.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "adad44e29e4c806119491a7f06f03de4d1af22c3a680dd47f1e6e179439d1f56" +[[package]] +name = "ring" +version = "0.16.20" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3053cf52e236a3ed746dfc745aa9cacf1b791d846bdaf412f60a8d7d6e17c8fc" +dependencies = [ + "cc", + "libc", + "once_cell", + "spin 0.5.2", + "untrusted 0.7.1", + "web-sys", + "winapi", +] + [[package]] name = "ring" version = "0.17.8" @@ -1075,8 +1155,8 @@ dependencies = [ "cfg-if", "getrandom", "libc", - "spin", - "untrusted", + "spin 0.9.8", + "untrusted 0.9.0", "windows-sys", ] @@ -1106,7 +1186,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "bf4ef73721ac7bcd79b2b315da7779d8fc09718c6b3d2d1b2d94850eb8c18432" dependencies = [ "log", - "ring", + "ring 0.17.8", "rustls-pki-types", "rustls-webpki", "subtle", @@ -1135,9 +1215,9 @@ version = "0.102.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "faaa0a62740bedb9b2ef5afa303da42764c012f743917351dc9a237ea1663610" dependencies = [ - "ring", + "ring 0.17.8", "rustls-pki-types", - "untrusted", + "untrusted 0.9.0", ] [[package]] @@ -1206,6 +1286,12 @@ dependencies = [ "windows-sys", ] +[[package]] +name = "spin" +version = "0.5.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6e63cff320ae2c57904679ba7cb63280a3dc4613885beafb148ee7bf9aa9042d" + [[package]] name = "spin" version = "0.9.8" @@ -1273,6 +1359,25 @@ dependencies = [ "syn", ] +[[package]] +name = "time" +version = "0.3.36" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5dfd88e563464686c916c7e46e623e520ddc6d79fa6641390f2e3fa86e83e885" +dependencies = [ + "deranged", + "num-conv", + "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 = "tokio" version = "1.40.0" @@ -1486,6 +1591,12 @@ version = "1.0.12" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3354b9ac3fae1ff6755cb6db53683adb661634f67557942dea4facebec0fee4b" +[[package]] +name = "untrusted" +version = "0.7.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a156c684c91ea7d62626509bce3cb4e1d9ed5c4d978f7b4352658f96a4c26b4a" + [[package]] name = "untrusted" version = "0.9.0" @@ -1519,6 +1630,93 @@ version = "0.11.0+wasi-snapshot-preview1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9c8d87e72b64a3b4db28d11ce29237c246188f4f51057d65a7eab63b7987e423" +[[package]] +name = "wasm-bindgen" +version = "0.2.93" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a82edfc16a6c469f5f44dc7b571814045d60404b55a0ee849f9bcfa2e63dd9b5" +dependencies = [ + "cfg-if", + "once_cell", + "wasm-bindgen-macro", +] + +[[package]] +name = "wasm-bindgen-backend" +version = "0.2.93" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9de396da306523044d3302746f1208fa71d7532227f15e347e2d93e4145dd77b" +dependencies = [ + "bumpalo", + "log", + "once_cell", + "proc-macro2", + "quote", + "syn", + "wasm-bindgen-shared", +] + +[[package]] +name = "wasm-bindgen-macro" +version = "0.2.93" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "585c4c91a46b072c92e908d99cb1dcdf95c5218eeb6f3bf1efa991ee7a68cccf" +dependencies = [ + "quote", + "wasm-bindgen-macro-support", +] + +[[package]] +name = "wasm-bindgen-macro-support" +version = "0.2.93" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "afc340c74d9005395cf9dd098506f7f44e38f2b4a21c6aaacf9a105ea5e1e836" +dependencies = [ + "proc-macro2", + "quote", + "syn", + "wasm-bindgen-backend", + "wasm-bindgen-shared", +] + +[[package]] +name = "wasm-bindgen-shared" +version = "0.2.93" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c62a0a307cb4a311d3a07867860911ca130c3494e8c2719593806c08bc5d0484" + +[[package]] +name = "web-sys" +version = "0.3.70" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "26fdeaafd9bd129f65e7c031593c24d62186301e0c72c8978fa1678be7d532c0" +dependencies = [ + "js-sys", + "wasm-bindgen", +] + +[[package]] +name = "winapi" +version = "0.3.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5c839a674fcd7a98952e593242ea400abe93992746761e38641405d28b00f419" +dependencies = [ + "winapi-i686-pc-windows-gnu", + "winapi-x86_64-pc-windows-gnu", +] + +[[package]] +name = "winapi-i686-pc-windows-gnu" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ac3b87c63620426dd9b991e5ce0329eff545bccbbb34f3be09ff6fb6ab51b7b6" + +[[package]] +name = "winapi-x86_64-pc-windows-gnu" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "712e227841d057c1ee1cd2fb22fa7e5a5461ae8e48fa2ca79ec42cfc1931183f" + [[package]] name = "windows-sys" version = "0.52.0" @@ -1662,6 +1860,15 @@ dependencies = [ "tonic-build", ] +[[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.32" diff --git a/dataplane/api-server/Cargo.toml b/dataplane/api-server/Cargo.toml index c40daf5..8b8803c 100644 --- a/dataplane/api-server/Cargo.toml +++ b/dataplane/api-server/Cargo.toml @@ -8,7 +8,7 @@ version.workspace = true [dependencies] anyhow = { workspace = true } aya = { workspace = true, features = ["async_tokio"] } -clap = { workspace = true } +clap = { workspace = true, features = ["derive"] } common = { workspace = true, features = ["user"] } libc = { workspace = true } log = { workspace = true } @@ -28,3 +28,7 @@ tonic-health = { workspace = true } [build-dependencies] tonic-build = { workspace = true } + +[dev-dependencies] +tempfile = "3.3.0" +rcgen = "0.9.3" diff --git a/dataplane/api-server/src/config.rs b/dataplane/api-server/src/config.rs index 61ca44d..4fb1b84 100644 --- a/dataplane/api-server/src/config.rs +++ b/dataplane/api-server/src/config.rs @@ -3,51 +3,32 @@ Copyright 2023 The Kubernetes Authors. SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */ +use anyhow::{anyhow, Result}; use clap::Parser; use std::path::PathBuf; #[derive(Debug, Parser, Clone)] -pub struct GrpcConfig { +pub struct TLSConfig { #[clap(long, default_value = "false")] pub enable_tls: bool, #[clap(long, default_value = "false")] pub enable_mtls: bool, - pub certificate_authority_root_path: Option, pub server_certificate_path: Option, pub server_private_key_path: Option, pub client_certificate_authority_root_path: Option, - pub client_certificate_path: Option, - pub client_private_key_path: Option, } -impl GrpcConfig { - pub fn validate(&self) -> Result<(), String> { - fn validate_paths(paths: &[(&Option, &str)]) -> Result<(), String> { - for (path, name) in paths { - if path.is_none() { - return Err(format!("Missing required path: {}", name)); - } - } - Ok(()) - } - +impl TLSConfig { + pub fn validate(&self) -> Result<()> { let tls_paths = &[ - ( - &self.certificate_authority_root_path, - "certificate_authority_root_path", - ), (&self.server_certificate_path, "server_certificate_path"), (&self.server_private_key_path, "server_private_key_path"), ]; - let mtls_paths = &[ - ( - &self.client_certificate_authority_root_path, - "client_certificate_authority_root_path", - ), - (&self.client_certificate_path, "client_certificate_path"), - (&self.client_private_key_path, "client_private_key_path"), - ]; + let mtls_paths = &[( + &self.client_certificate_authority_root_path, + "client_certificate_authority_root_path", + )]; if self.enable_mtls { validate_paths(tls_paths)?; @@ -59,3 +40,12 @@ impl GrpcConfig { Ok(()) } } + +fn validate_paths(paths: &[(&Option, &str)]) -> Result<()> { + for (path, name) in paths { + if path.is_none() { + return Err(anyhow!("Missing required path: {}", name)); + } + } + Ok(()) +} diff --git a/dataplane/api-server/src/lib.rs b/dataplane/api-server/src/lib.rs index 575286a..09f8dc7 100644 --- a/dataplane/api-server/src/lib.rs +++ b/dataplane/api-server/src/lib.rs @@ -14,14 +14,14 @@ use std::{ net::{Ipv4Addr, SocketAddrV4}, }; -use anyhow::Error; +use anyhow::{Context, Result}; use aya::maps::{HashMap, MapData}; use log::info; use tonic::transport::{Certificate, Identity, Server, ServerTlsConfig}; use backends::backends_server::BackendsServer; use common::{BackendKey, BackendList, ClientKey, LoadBalancerMapping}; -use config::GrpcConfig; +use config::TLSConfig; pub async fn start( addr: Ipv4Addr, @@ -29,13 +29,15 @@ pub async fn start( backends_map: HashMap, gateway_indexes_map: HashMap, tcp_conns_map: HashMap, - tls_config: GrpcConfig, -) -> Result<(), Error> { + tls_config: TLSConfig, +) -> Result<()> { let (_, health_service) = tonic_health::server::health_reporter(); + tls_config.validate()?; + let server = server::BackendService::new(backends_map, gateway_indexes_map, tcp_conns_map); let mut server_builder = Server::builder(); - server_builder = setup_tls(server_builder, &tls_config); + server_builder = setup_tls(server_builder, &tls_config)?; server_builder .add_service(health_service) .add_service(BackendsServer::new(server)) @@ -44,19 +46,20 @@ pub async fn start( Ok(()) } -fn setup_tls(mut builder: Server, tls_config: &GrpcConfig) -> Server { +pub fn setup_tls(mut builder: Server, tls_config: &TLSConfig) -> Result { + // TLS implementation drawn from Tonic examples. + // See: https://github.com/hyperium/tonic/blob/master/examples/src/tls_client_auth/server.rs if tls_config.enable_tls || tls_config.enable_mtls { let mut tls = ServerTlsConfig::new(); - if let Some(cert_path) = &tls_config.server_certificate_path { - let cert = fs::read_to_string(cert_path).expect("Error reading server certificate"); - let key = fs::read_to_string( - tls_config - .server_private_key_path - .as_ref() - .expect("Missing private key path"), - ) - .expect("Error reading server private key"); + if let (Some(cert_path), Some(key_path)) = ( + &tls_config.server_certificate_path, + &tls_config.server_private_key_path, + ) { + let cert = fs::read_to_string(cert_path) + .with_context(|| format!("Failed to read certificate from {:?}", cert_path))?; + let key = fs::read_to_string(key_path) + .with_context(|| format!("Failed to read key from {:?}", key_path))?; let server_identity = Identity::from_pem(cert, key); tls = tls.identity(server_identity); } @@ -64,19 +67,19 @@ fn setup_tls(mut builder: Server, tls_config: &GrpcConfig) -> Server { if tls_config.enable_mtls { if let Some(client_ca_cert_path) = &tls_config.client_certificate_authority_root_path { let client_ca_cert = - fs::read_to_string(client_ca_cert_path).expect("Error reading CA certificate"); + fs::read_to_string(client_ca_cert_path).with_context(|| { + format!("Failed to read client CA from {:?}", client_ca_cert_path) + })?; let client_ca_root = Certificate::from_pem(client_ca_cert); tls = tls.client_ca_root(client_ca_root); - builder = builder - .tls_config(tls) - .expect("Error adding mTLS to server"); + builder = builder.tls_config(tls)?; info!("gRPC mTLS enabled"); - return builder; + return Ok(builder); } } - builder = builder.tls_config(tls).expect("Error adding TLS to server"); + builder = builder.tls_config(tls)?; info!("gRPC TLS enabled"); } - builder + Ok(builder) } diff --git a/dataplane/api-server/tests/test_setup_tls.rs b/dataplane/api-server/tests/test_setup_tls.rs new file mode 100644 index 0000000..be2fc4b --- /dev/null +++ b/dataplane/api-server/tests/test_setup_tls.rs @@ -0,0 +1,92 @@ +use anyhow::Result; +use api_server::config::TLSConfig; +use api_server::setup_tls; +use rcgen::{generate_simple_self_signed, Certificate, CertificateParams}; +use std::fs; +use tempfile::tempdir; +use tonic::transport::Server; + +#[tokio::test] +async fn test_tls_self_signed_cert() -> Result<()> { + // Create a temporary directory + let temp_dir = tempdir().unwrap(); + + // Generate self-signed certificate + let cert = generate_simple_self_signed(vec!["localhost".into()])?; + let cert_pem = cert.serialize_pem()?; + let key_pem = cert.serialize_private_key_pem(); + + // Paths for the server cert and private key + let cert_path = temp_dir.path().join("server.crt"); + let key_path = temp_dir.path().join("server.key"); + + // Write cert and key to temp files + fs::write(&cert_path, cert_pem.as_bytes())?; + fs::write(&key_path, key_pem.as_bytes())?; + + // Set up a TLS config with paths to the cert and key + let tls_config = TLSConfig { + enable_tls: true, + enable_mtls: false, + server_certificate_path: Some(cert_path.clone()), + server_private_key_path: Some(key_path.clone()), + client_certificate_authority_root_path: None, + }; + + // Prepare a dummy server builder + let builder = Server::builder(); + + // Run the setup_tls function and ensure no error is thrown + let result = setup_tls(builder, &tls_config); + assert!( + result.is_ok(), + "setup_tls should succeed with valid self-signed certs" + ); + Ok(()) +} + +#[tokio::test] +async fn test_mtls_self_signed_cert() -> Result<()> { + // Create a temporary directory + let temp_dir = tempdir().unwrap(); + + // Generate self-signed certificate + let cert = generate_simple_self_signed(vec!["localhost".into()])?; + let cert_pem = cert.serialize_pem()?; + let key_pem = cert.serialize_private_key_pem(); + + // Generate CA + let ca_params = CertificateParams::default(); + let ca_cert = Certificate::from_params(ca_params)?; + let ca_cert_pem = ca_cert.serialize_pem()?; + + // Cert file paths + let cert_path = temp_dir.path().join("server.crt"); + let key_path = temp_dir.path().join("server.key"); + let ca_cert_path = temp_dir.path().join("ca.crt"); + + // Write cert and key to temp files + fs::write(&cert_path, cert_pem.as_bytes())?; + fs::write(&key_path, key_pem.as_bytes())?; + fs::write(&ca_cert_path, ca_cert_pem.as_bytes())?; + + // Set up a TLS config with paths to the cert and key + let tls_config = TLSConfig { + enable_tls: true, + enable_mtls: false, + server_certificate_path: Some(cert_path.clone()), + server_private_key_path: Some(key_path.clone()), + client_certificate_authority_root_path: Some(ca_cert_path.clone()), + }; + + // Prepare a dummy server builder + let builder = Server::builder(); + + // Run the setup_tls function and ensure no error is thrown + let result = setup_tls(builder, &tls_config); + assert!( + result.is_ok(), + "setup_tls should succeed with valid self-signed certs" + ); + Ok(()) +} diff --git a/dataplane/loader/src/main.rs b/dataplane/loader/src/main.rs index 0a1eefc..06f48ba 100644 --- a/dataplane/loader/src/main.rs +++ b/dataplane/loader/src/main.rs @@ -7,7 +7,7 @@ SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) use std::net::Ipv4Addr; use anyhow::Context; -use api_server::config::GrpcConfig; +use api_server::config::TLSConfig; use api_server::start as start_api_server; use aya::maps::HashMap; use aya::programs::{tc, SchedClassifier, TcAttachType}; @@ -22,7 +22,7 @@ struct Opt { #[clap(short, long, default_value = "lo")] iface: String, #[clap(flatten)] - tls_config: GrpcConfig, + tls_config: TLSConfig, } #[tokio::main]