From 05b2aab8b0ee727526aabc001ae242109430c867 Mon Sep 17 00:00:00 2001 From: Jun Kurihara Date: Wed, 12 Jul 2023 21:40:08 +0900 Subject: [PATCH] refactor: remove explict cert file path from backend mods and define abstracted trait for the cert source preparing librarization --- src/backend/mod.rs | 27 +++------------------------ src/cert_file_reader.rs | 15 ++++++++++----- src/certs.rs | 5 +++++ src/config/parse.rs | 14 +++++++++----- src/config/toml.rs | 31 ++++++++++++++++--------------- src/error.rs | 5 ++--- src/handler/handler_main.rs | 7 ++++++- src/proxy/crypto_service.rs | 11 ++++------- 8 files changed, 55 insertions(+), 60 deletions(-) diff --git a/src/backend/mod.rs b/src/backend/mod.rs index 9bc28e5..524f30b 100644 --- a/src/backend/mod.rs +++ b/src/backend/mod.rs @@ -19,7 +19,7 @@ use crate::{ }; use derive_builder::Builder; use rustc_hash::FxHashMap as HashMap; -use std::{borrow::Cow, path::PathBuf}; +use std::borrow::Cow; /// Struct serving information to route incoming connections, like server name to be handled and tls certs/keys settings. #[derive(Builder)] @@ -36,16 +36,11 @@ where /// struct of reverse proxy serving incoming request pub reverse_proxy: ReverseProxy, - /// tls settings - #[builder(setter(custom), default)] - pub tls_cert_path: Option, - #[builder(setter(custom), default)] - pub tls_cert_key_path: Option, + /// tls settings: https redirection with 30x #[builder(default)] pub https_redirection: Option, - #[builder(setter(custom), default)] - pub client_ca_cert_path: Option, + /// TLS settings: source meta for server cert, key, client ca cert #[builder(default)] pub crypto_source: Option, } @@ -57,22 +52,6 @@ where self.server_name = Some(server_name.into().to_ascii_lowercase()); self } - pub fn tls_cert_path(&mut self, v: &Option) -> &mut Self { - self.tls_cert_path = Some(opt_string_to_opt_pathbuf(v)); - self - } - pub fn tls_cert_key_path(&mut self, v: &Option) -> &mut Self { - self.tls_cert_key_path = Some(opt_string_to_opt_pathbuf(v)); - self - } - pub fn client_ca_cert_path(&mut self, v: &Option) -> &mut Self { - self.client_ca_cert_path = Some(opt_string_to_opt_pathbuf(v)); - self - } -} - -fn opt_string_to_opt_pathbuf(input: &Option) -> Option { - input.to_owned().as_ref().map(PathBuf::from) } /// HashMap and some meta information for multiple Backend structs. diff --git a/src/cert_file_reader.rs b/src/cert_file_reader.rs index dc25b09..e25dcf7 100644 --- a/src/cert_file_reader.rs +++ b/src/cert_file_reader.rs @@ -36,8 +36,8 @@ impl CryptoFileSourceBuilder { self.tls_cert_key_path = Some(PathBuf::from(v)); self } - pub fn client_ca_cert_path(&mut self, v: &str) -> &mut Self { - self.client_ca_cert_path = Some(Some(PathBuf::from(v))); + pub fn client_ca_cert_path(&mut self, v: &Option) -> &mut Self { + self.client_ca_cert_path = Some(v.to_owned().as_ref().map(PathBuf::from)); self } } @@ -45,6 +45,7 @@ impl CryptoFileSourceBuilder { #[async_trait] impl CryptoSource for CryptoFileSource { type Error = io::Error; + /// read crypto materials from source async fn read(&self) -> Result { read_certs_and_keys( &self.tls_cert_path, @@ -52,10 +53,14 @@ impl CryptoSource for CryptoFileSource { self.client_ca_cert_path.as_ref(), ) } + /// Returns true when mutual tls is enabled + fn is_mutual_tls(&self) -> bool { + self.client_ca_cert_path.is_some() + } } /// Read certificates and private keys from file -pub(crate) fn read_certs_and_keys( +fn read_certs_and_keys( cert_path: &PathBuf, cert_key_path: &PathBuf, client_ca_cert_path: Option<&PathBuf>, @@ -162,11 +167,11 @@ mod tests { async fn read_server_crt_key_files_with_client_ca_crt() { let tls_cert_path = "example-certs/server.crt"; let tls_cert_key_path = "example-certs/server.key"; - let client_ca_cert_path = "example-certs/client.ca.crt"; + let client_ca_cert_path = Some("example-certs/client.ca.crt".to_string()); let crypto_file_source = CryptoFileSourceBuilder::default() .tls_cert_key_path(tls_cert_key_path) .tls_cert_path(tls_cert_path) - .client_ca_cert_path(client_ca_cert_path) + .client_ca_cert_path(&client_ca_cert_path) .build(); assert!(crypto_file_source.is_ok()); diff --git a/src/certs.rs b/src/certs.rs index da51e14..2ed0198 100644 --- a/src/certs.rs +++ b/src/certs.rs @@ -13,5 +13,10 @@ pub struct CertsAndKeys { // Trait to read certs and keys anywhere from KVS, file, sqlite, etc. pub trait CryptoSource { type Error; + + /// read crypto materials from source async fn read(&self) -> Result; + + /// Returns true when mutual tls is enabled + fn is_mutual_tls(&self) -> bool; } diff --git a/src/config/parse.rs b/src/config/parse.rs index 83aa546..1dc2545 100644 --- a/src/config/parse.rs +++ b/src/config/parse.rs @@ -1,12 +1,16 @@ use super::toml::ConfigToml; -use crate::{backend::Backends, certs::CryptoSource, error::*, globals::*, log::*, utils::BytesName}; +use crate::{ + backend::Backends, + cert_file_reader::CryptoFileSource, + error::{anyhow, ensure}, + globals::*, + log::*, + utils::BytesName, +}; use clap::Arg; use tokio::runtime::Handle; -pub fn build_globals(runtime_handle: Handle) -> std::result::Result, anyhow::Error> -where - T: CryptoSource + Clone, -{ +pub fn build_globals(runtime_handle: Handle) -> std::result::Result, anyhow::Error> { let _ = include_str!("../../Cargo.toml"); let options = clap::command!().arg( Arg::new("config_file") diff --git a/src/config/toml.rs b/src/config/toml.rs index a68b82b..f33ea4d 100644 --- a/src/config/toml.rs +++ b/src/config/toml.rs @@ -1,8 +1,8 @@ use crate::{ backend::{Backend, BackendBuilder, ReverseProxy, Upstream, UpstreamGroup, UpstreamGroupBuilder, UpstreamOption}, - certs::CryptoSource, + cert_file_reader::{CryptoFileSource, CryptoFileSourceBuilder}, constants::*, - error::*, + error::{anyhow, ensure}, globals::ProxyConfig, utils::PathNameBytesExp, }; @@ -164,20 +164,17 @@ impl TryInto for &ConfigToml { } impl ConfigToml { - pub fn new(config_file: &str) -> std::result::Result { - let config_str = fs::read_to_string(config_file).map_err(RpxyError::Io)?; + pub fn new(config_file: &str) -> std::result::Result { + let config_str = fs::read_to_string(config_file)?; - toml::from_str(&config_str).map_err(RpxyError::TomlDe) + toml::from_str(&config_str).map_err(|e| anyhow!(e)) } } -impl TryInto> for &Application -where - T: CryptoSource + Clone, -{ +impl TryInto> for &Application { type Error = anyhow::Error; - fn try_into(self) -> std::result::Result, Self::Error> { + fn try_into(self) -> std::result::Result, Self::Error> { let server_name_string = self.server_name.as_ref().ok_or(anyhow!("Missing server_name"))?; // backend builder @@ -203,11 +200,15 @@ where tls.https_redirection }; - backend_builder - .tls_cert_path(&tls.tls_cert_path) - .tls_cert_key_path(&tls.tls_cert_key_path) - .https_redirection(https_redirection) + let crypto_source = CryptoFileSourceBuilder::default() + .tls_cert_path(tls.tls_cert_path.as_ref().unwrap()) + .tls_cert_key_path(tls.tls_cert_key_path.as_ref().unwrap()) .client_ca_cert_path(&tls.client_ca_cert_path) + .build()?; + + backend_builder + .https_redirection(https_redirection) + .crypto_source(Some(crypto_source)) .build()? }; Ok(backend) @@ -255,7 +256,7 @@ impl TryInto for &Application { } impl TryInto for &UpstreamParams { - type Error = RpxyError; + type Error = anyhow::Error; fn try_into(self) -> std::result::Result { let scheme = match self.tls { diff --git a/src/error.rs b/src/error.rs index 18b4307..187c993 100644 --- a/src/error.rs +++ b/src/error.rs @@ -29,9 +29,8 @@ pub enum RpxyError { #[error("I/O Error")] Io(#[from] io::Error), - #[error("Toml Deserialization Error")] - TomlDe(#[from] toml::de::Error), - + // #[error("Toml Deserialization Error")] + // TomlDe(#[from] toml::de::Error), #[cfg(feature = "http3")] #[error("Quic Connection Error")] QuicConn(#[from] quinn::ConnectionError), diff --git a/src/handler/handler_main.rs b/src/handler/handler_main.rs index 2016f2c..c23fd24 100644 --- a/src/handler/handler_main.rs +++ b/src/handler/handler_main.rs @@ -209,7 +209,12 @@ where #[cfg(feature = "http3")] { // TODO: Workaround for avoid h3 for client authentication - if self.globals.proxy_config.http3 && chosen_backend.client_ca_cert_path.is_none() { + if self.globals.proxy_config.http3 + && chosen_backend + .crypto_source + .as_ref() + .is_some_and(|v| !v.is_mutual_tls()) + { if let Some(port) = self.globals.proxy_config.https_port { add_header_entry_overwrite_if_exist( headers, diff --git a/src/proxy/crypto_service.rs b/src/proxy/crypto_service.rs index bb582fa..8d7f00d 100644 --- a/src/proxy/crypto_service.rs +++ b/src/proxy/crypto_service.rs @@ -1,5 +1,4 @@ use crate::{ - cert_file_reader::read_certs_and_keys, // TODO: Trait defining read_certs_and_keys and add struct implementing the trait to backend when build backend certs::{CertsAndKeys, CryptoSource}, globals::Globals, log::*, @@ -55,13 +54,11 @@ where let mut certs_and_keys_map = ServerCryptoBase::default(); for (server_name_bytes_exp, backend) in self.globals.backends.apps.iter() { - if backend.tls_cert_key_path.is_some() && backend.tls_cert_path.is_some() { - let tls_cert_key_path = backend.tls_cert_key_path.as_ref().unwrap(); - let tls_cert_path = backend.tls_cert_path.as_ref().unwrap(); - let tls_client_ca_cert_path = backend.client_ca_cert_path.as_ref(); - let certs_and_keys = read_certs_and_keys(tls_cert_path, tls_cert_key_path, tls_client_ca_cert_path) + if let Some(crypto_source) = &backend.crypto_source { + let certs_and_keys = crypto_source + .read() + .await .map_err(|_e| ReloaderError::::Reload("Failed to reload cert, key or ca cert"))?; - certs_and_keys_map .inner .insert(server_name_bytes_exp.to_owned(), certs_and_keys);