From d17d2083d47d36b77cba6ffaa82834591a8d9985 Mon Sep 17 00:00:00 2001 From: houseme Date: Fri, 27 Feb 2026 21:24:49 +0800 Subject: [PATCH] feat(targets): enhance webhook TLS support with custom CA and skip-verify (#1994) Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: houseme <4829346+houseme@users.noreply.github.com> Co-authored-by: heihutu --- .config/make/tests.mak | 3 +- Cargo.lock | 3 - Cargo.toml | 1 - crates/audit/src/factory.rs | 10 +- crates/config/src/audit/webhook.rs | 8 +- crates/config/src/constants/env.rs | 2 + crates/config/src/constants/targets.rs | 2 + crates/config/src/notify/webhook.rs | 8 +- crates/e2e_test/Cargo.toml | 3 +- crates/e2e_test/src/protocols/ftps_core.rs | 9 +- crates/ecstore/src/client/transition_api.rs | 21 ++-- crates/notify/src/factory.rs | 10 +- crates/protocols/src/ftps/server.rs | 6 +- crates/targets/src/target/webhook.rs | 129 ++++++++++++++++---- crates/utils/Cargo.toml | 3 +- crates/utils/src/certs.rs | 33 +++-- rustfs/Cargo.toml | 1 - rustfs/src/server/cert.rs | 10 +- rustfs/src/server/http.rs | 3 - 19 files changed, 182 insertions(+), 83 deletions(-) diff --git a/.config/make/tests.mak b/.config/make/tests.mak index bdaabe86..5b3ce1de 100644 --- a/.config/make/tests.mak +++ b/.config/make/tests.mak @@ -1,8 +1,9 @@ ## โ€”โ€” Tests and e2e test --------------------------------------------------------------------------- +TEST_THREADS ?= 1 + .PHONY: test test: core-deps test-deps ## Run all tests -TEST_THREADS ?= 1 @echo "๐Ÿงช Running tests..." @if command -v cargo-nextest >/dev/null 2>&1; then \ cargo nextest run --all --exclude e2e_test; \ diff --git a/Cargo.lock b/Cargo.lock index fdedbe96..98e6b172 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3106,7 +3106,6 @@ dependencies = [ "rustfs-madmin", "rustfs-protos", "rustls", - "rustls-pemfile", "serde", "serde_json", "serial_test", @@ -7202,7 +7201,6 @@ dependencies = [ "rustfs-utils", "rustfs-zip", "rustls", - "rustls-pemfile", "s3s", "serde", "serde_json", @@ -7902,7 +7900,6 @@ dependencies = [ "rustfs-config", "rustix 1.1.4", "rustls", - "rustls-pemfile", "rustls-pki-types", "s3s", "serde", diff --git a/Cargo.toml b/Cargo.toml index c5fb5a30..a13dfae9 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -160,7 +160,6 @@ openidconnect = { version = "4.0", default-features = false } pbkdf2 = "0.13.0-rc.9" rsa = { version = "0.10.0-rc.15" } rustls = { version = "0.23.37", default-features = false, features = ["aws-lc-rs", "logging", "tls12", "prefer-post-quantum", "std"] } -rustls-pemfile = "2.2.0" rustls-pki-types = "1.14.0" sha1 = "0.11.0-rc.5" sha2 = "0.11.0-rc.5" diff --git a/crates/audit/src/factory.rs b/crates/audit/src/factory.rs index 18f7ebc1..6674db26 100644 --- a/crates/audit/src/factory.rs +++ b/crates/audit/src/factory.rs @@ -19,8 +19,9 @@ use rumqttc::QoS; use rustfs_config::audit::{AUDIT_MQTT_KEYS, AUDIT_WEBHOOK_KEYS, ENV_AUDIT_MQTT_KEYS, ENV_AUDIT_WEBHOOK_KEYS}; use rustfs_config::{ AUDIT_DEFAULT_DIR, DEFAULT_LIMIT, MQTT_BROKER, MQTT_KEEP_ALIVE_INTERVAL, MQTT_PASSWORD, MQTT_QOS, MQTT_QUEUE_DIR, - MQTT_QUEUE_LIMIT, MQTT_RECONNECT_INTERVAL, MQTT_TOPIC, MQTT_USERNAME, WEBHOOK_AUTH_TOKEN, WEBHOOK_CLIENT_CERT, - WEBHOOK_CLIENT_KEY, WEBHOOK_ENDPOINT, WEBHOOK_QUEUE_DIR, WEBHOOK_QUEUE_LIMIT, + MQTT_QUEUE_LIMIT, MQTT_RECONNECT_INTERVAL, MQTT_TOPIC, MQTT_USERNAME, RUSTFS_WEBHOOK_SKIP_TLS_VERIFY_DEFAULT, + WEBHOOK_AUTH_TOKEN, WEBHOOK_CLIENT_CA, WEBHOOK_CLIENT_CERT, WEBHOOK_CLIENT_KEY, WEBHOOK_ENDPOINT, WEBHOOK_QUEUE_DIR, + WEBHOOK_QUEUE_LIMIT, WEBHOOK_SKIP_TLS_VERIFY, }; use rustfs_ecstore::config::KVS; use rustfs_targets::{ @@ -75,6 +76,11 @@ impl TargetFactory for WebhookTargetFactory { .unwrap_or(DEFAULT_LIMIT), client_cert: config.lookup(WEBHOOK_CLIENT_CERT).unwrap_or_default(), client_key: config.lookup(WEBHOOK_CLIENT_KEY).unwrap_or_default(), + client_ca: config.lookup(WEBHOOK_CLIENT_CA).unwrap_or_default(), + skip_tls_verify: config + .lookup(WEBHOOK_SKIP_TLS_VERIFY) + .and_then(|v| v.parse::().ok()) + .unwrap_or(RUSTFS_WEBHOOK_SKIP_TLS_VERIFY_DEFAULT), target_type: rustfs_targets::target::TargetType::AuditLog, }; diff --git a/crates/config/src/audit/webhook.rs b/crates/config/src/audit/webhook.rs index 8ee69a18..27ba7ff3 100644 --- a/crates/config/src/audit/webhook.rs +++ b/crates/config/src/audit/webhook.rs @@ -20,9 +20,11 @@ pub const ENV_AUDIT_WEBHOOK_QUEUE_LIMIT: &str = "RUSTFS_AUDIT_WEBHOOK_QUEUE_LIMI pub const ENV_AUDIT_WEBHOOK_QUEUE_DIR: &str = "RUSTFS_AUDIT_WEBHOOK_QUEUE_DIR"; pub const ENV_AUDIT_WEBHOOK_CLIENT_CERT: &str = "RUSTFS_AUDIT_WEBHOOK_CLIENT_CERT"; pub const ENV_AUDIT_WEBHOOK_CLIENT_KEY: &str = "RUSTFS_AUDIT_WEBHOOK_CLIENT_KEY"; +pub const ENV_AUDIT_WEBHOOK_CLIENT_CA: &str = "RUSTFS_AUDIT_WEBHOOK_CLIENT_CA"; +pub const ENV_AUDIT_WEBHOOK_SKIP_TLS_VERIFY: &str = "RUSTFS_AUDIT_WEBHOOK_SKIP_TLS_VERIFY"; /// List of all environment variable keys for a webhook target. -pub const ENV_AUDIT_WEBHOOK_KEYS: &[&str; 7] = &[ +pub const ENV_AUDIT_WEBHOOK_KEYS: &[&str; 9] = &[ ENV_AUDIT_WEBHOOK_ENABLE, ENV_AUDIT_WEBHOOK_ENDPOINT, ENV_AUDIT_WEBHOOK_AUTH_TOKEN, @@ -30,6 +32,8 @@ pub const ENV_AUDIT_WEBHOOK_KEYS: &[&str; 7] = &[ ENV_AUDIT_WEBHOOK_QUEUE_DIR, ENV_AUDIT_WEBHOOK_CLIENT_CERT, ENV_AUDIT_WEBHOOK_CLIENT_KEY, + ENV_AUDIT_WEBHOOK_CLIENT_CA, + ENV_AUDIT_WEBHOOK_SKIP_TLS_VERIFY, ]; /// A list of all valid configuration keys for a webhook target. @@ -42,4 +46,6 @@ pub const AUDIT_WEBHOOK_KEYS: &[&str] = &[ crate::WEBHOOK_CLIENT_CERT, crate::WEBHOOK_CLIENT_KEY, crate::COMMENT_KEY, + crate::WEBHOOK_CLIENT_CA, + crate::WEBHOOK_SKIP_TLS_VERIFY, ]; diff --git a/crates/config/src/constants/env.rs b/crates/config/src/constants/env.rs index 84116ba5..d98fb4f0 100644 --- a/crates/config/src/constants/env.rs +++ b/crates/config/src/constants/env.rs @@ -20,6 +20,8 @@ pub const EVENT_DEFAULT_DIR: &str = "/opt/rustfs/events"; // Default directory f pub const AUDIT_DEFAULT_DIR: &str = "/opt/rustfs/audit"; // Default directory for audit store pub const DEFAULT_LIMIT: u64 = 100000; // Default store limit +pub const RUSTFS_WEBHOOK_SKIP_TLS_VERIFY_DEFAULT: bool = false; + /// Standard config keys and values. pub const ENABLE_KEY: &str = "enable"; pub const COMMENT_KEY: &str = "comment"; diff --git a/crates/config/src/constants/targets.rs b/crates/config/src/constants/targets.rs index d63506e3..d6d3bb50 100644 --- a/crates/config/src/constants/targets.rs +++ b/crates/config/src/constants/targets.rs @@ -16,6 +16,8 @@ pub const WEBHOOK_ENDPOINT: &str = "endpoint"; pub const WEBHOOK_AUTH_TOKEN: &str = "auth_token"; pub const WEBHOOK_CLIENT_CERT: &str = "client_cert"; pub const WEBHOOK_CLIENT_KEY: &str = "client_key"; +pub const WEBHOOK_CLIENT_CA: &str = "client_ca"; +pub const WEBHOOK_SKIP_TLS_VERIFY: &str = "skip_tls_verify"; pub const WEBHOOK_BATCH_SIZE: &str = "batch_size"; pub const WEBHOOK_QUEUE_LIMIT: &str = "queue_limit"; pub const WEBHOOK_QUEUE_DIR: &str = "queue_dir"; diff --git a/crates/config/src/notify/webhook.rs b/crates/config/src/notify/webhook.rs index 8481d7e1..d5dd58ed 100644 --- a/crates/config/src/notify/webhook.rs +++ b/crates/config/src/notify/webhook.rs @@ -22,6 +22,8 @@ pub const NOTIFY_WEBHOOK_KEYS: &[&str] = &[ crate::WEBHOOK_CLIENT_CERT, crate::WEBHOOK_CLIENT_KEY, crate::COMMENT_KEY, + crate::WEBHOOK_CLIENT_CA, + crate::WEBHOOK_SKIP_TLS_VERIFY, ]; // Webhook Environment Variables @@ -32,8 +34,10 @@ pub const ENV_NOTIFY_WEBHOOK_QUEUE_LIMIT: &str = "RUSTFS_NOTIFY_WEBHOOK_QUEUE_LI pub const ENV_NOTIFY_WEBHOOK_QUEUE_DIR: &str = "RUSTFS_NOTIFY_WEBHOOK_QUEUE_DIR"; pub const ENV_NOTIFY_WEBHOOK_CLIENT_CERT: &str = "RUSTFS_NOTIFY_WEBHOOK_CLIENT_CERT"; pub const ENV_NOTIFY_WEBHOOK_CLIENT_KEY: &str = "RUSTFS_NOTIFY_WEBHOOK_CLIENT_KEY"; +pub const ENV_NOTIFY_WEBHOOK_CLIENT_CA: &str = "RUSTFS_NOTIFY_WEBHOOK_CLIENT_CA"; +pub const ENV_NOTIFY_WEBHOOK_SKIP_TLS_VERIFY: &str = "RUSTFS_NOTIFY_WEBHOOK_SKIP_TLS_VERIFY"; -pub const ENV_NOTIFY_WEBHOOK_KEYS: &[&str; 7] = &[ +pub const ENV_NOTIFY_WEBHOOK_KEYS: &[&str; 9] = &[ ENV_NOTIFY_WEBHOOK_ENABLE, ENV_NOTIFY_WEBHOOK_ENDPOINT, ENV_NOTIFY_WEBHOOK_AUTH_TOKEN, @@ -41,4 +45,6 @@ pub const ENV_NOTIFY_WEBHOOK_KEYS: &[&str; 7] = &[ ENV_NOTIFY_WEBHOOK_QUEUE_DIR, ENV_NOTIFY_WEBHOOK_CLIENT_CERT, ENV_NOTIFY_WEBHOOK_CLIENT_KEY, + ENV_NOTIFY_WEBHOOK_CLIENT_CA, + ENV_NOTIFY_WEBHOOK_SKIP_TLS_VERIFY, ]; diff --git a/crates/e2e_test/Cargo.toml b/crates/e2e_test/Cargo.toml index a0fa5b78..e5646cc0 100644 --- a/crates/e2e_test/Cargo.toml +++ b/crates/e2e_test/Cargo.toml @@ -59,5 +59,4 @@ sha2 = { workspace = true } suppaftp = { workspace = true, features = ["tokio", "rustls-aws-lc-rs"] } rcgen.workspace = true anyhow.workspace = true -rustls.workspace = true -rustls-pemfile.workspace = true +rustls.workspace = true \ No newline at end of file diff --git a/crates/e2e_test/src/protocols/ftps_core.rs b/crates/e2e_test/src/protocols/ftps_core.rs index 3193d2cc..67fe350d 100644 --- a/crates/e2e_test/src/protocols/ftps_core.rs +++ b/crates/e2e_test/src/protocols/ftps_core.rs @@ -18,8 +18,7 @@ use crate::common::rustfs_binary_path; use crate::protocols::test_env::{DEFAULT_ACCESS_KEY, DEFAULT_SECRET_KEY, ProtocolTestEnvironment}; use anyhow::Result; use rcgen::generate_simple_self_signed; -use rustls::crypto::aws_lc_rs::default_provider; -use rustls::{ClientConfig, RootCertStore}; +use rustls::{ClientConfig, RootCertStore, pki_types::CertificateDer, pki_types::pem::PemObject}; use std::io::Cursor; use std::path::PathBuf; use std::sync::Arc; @@ -74,8 +73,8 @@ pub async fn test_ftps_core_operations() -> Result<()> { .await .map_err(|e| anyhow::anyhow!("{}", e))?; - // Install the aws-lc-rs crypto provider - default_provider() + // Build ServerConfig with SNI support + rustls::crypto::aws_lc_rs::default_provider() .install_default() .map_err(|e| anyhow::anyhow!("Failed to install crypto provider: {:?}", e))?; @@ -84,7 +83,7 @@ pub async fn test_ftps_core_operations() -> Result<()> { // Add the self-signed certificate to the trust store for e2e // Note: In a real environment, you'd use proper root certificates let cert_pem = default_cert.cert.pem(); - let cert_der = rustls_pemfile::certs(&mut Cursor::new(cert_pem)) + let cert_der = CertificateDer::pem_reader_iter(&mut Cursor::new(cert_pem)) .collect::, _>>() .map_err(|e| anyhow::anyhow!("Failed to parse cert: {}", e))?; diff --git a/crates/ecstore/src/client/transition_api.rs b/crates/ecstore/src/client/transition_api.rs index 606119f8..2fb980e2 100644 --- a/crates/ecstore/src/client/transition_api.rs +++ b/crates/ecstore/src/client/transition_api.rs @@ -165,7 +165,6 @@ impl TransitionClient { async fn private_new(endpoint: &str, opts: Options, tier_type: &str) -> Result { let endpoint_url = get_endpoint_url(endpoint, opts.secure)?; - let _ = rustls::crypto::aws_lc_rs::default_provider().install_default(); let scheme = endpoint_url.scheme(); let client; let tls = if let Some(store) = load_root_store_from_tls_path() { @@ -292,7 +291,7 @@ impl TransitionClient { let _ = hc_duration; } - fn dump_http(&self, req: &http::Request, resp: &http::Response) -> Result<(), std::io::Error> { + fn dump_http(&self, req: &Request, resp: &Response) -> Result<(), std::io::Error> { let mut resp_trace: Vec; //info!("{}{}", self.trace_output, "---------BEGIN-HTTP---------"); @@ -301,7 +300,7 @@ impl TransitionClient { Ok(()) } - pub async fn doit(&self, req: http::Request) -> Result, std::io::Error> { + pub async fn doit(&self, req: Request) -> Result, std::io::Error> { let req_method; let req_uri; let req_headers; @@ -353,7 +352,7 @@ impl TransitionClient { &self, method: http::Method, metadata: &mut RequestMetadata, - ) -> Result, std::io::Error> { + ) -> Result, std::io::Error> { if self.is_offline() { let mut s = self.endpoint_url.to_string(); s.push_str(" is offline."); @@ -363,7 +362,7 @@ impl TransitionClient { let retryable: bool; //let mut body_seeker: BufferReader; let mut req_retry = self.max_retries; - let mut resp: http::Response; + let mut resp: Response; //if metadata.content_body != nil { //body_seeker = BufferReader::new(metadata.content_body.read_all().await?); @@ -401,10 +400,10 @@ impl TransitionClient { err_response.message = format!("remote tier error: {}", err_response.message); if self.region == "" { - match err_response.code { + return match err_response.code { S3ErrorCode::AuthorizationHeaderMalformed | S3ErrorCode::InvalidArgument /*S3ErrorCode::InvalidRegion*/ => { //break; - return Err(std::io::Error::other(err_response)); + Err(std::io::Error::other(err_response)) } S3ErrorCode::AccessDenied => { if err_response.region == "" { @@ -421,12 +420,12 @@ impl TransitionClient { metadata.bucket_location = err_response.region.clone(); //continue; } - return Err(std::io::Error::other(err_response)); + Err(std::io::Error::other(err_response)) } _ => { - return Err(std::io::Error::other(err_response)); + Err(std::io::Error::other(err_response)) } - } + }; } if is_s3code_retryable(err_response.code.as_str()) { @@ -447,7 +446,7 @@ impl TransitionClient { &self, method: &http::Method, metadata: &mut RequestMetadata, - ) -> Result, std::io::Error> { + ) -> Result, std::io::Error> { let mut location = metadata.bucket_location.clone(); if location == "" && metadata.bucket_name != "" { location = self.get_bucket_location(&metadata.bucket_name).await?; diff --git a/crates/notify/src/factory.rs b/crates/notify/src/factory.rs index c324bf2f..0d91d8d0 100644 --- a/crates/notify/src/factory.rs +++ b/crates/notify/src/factory.rs @@ -19,8 +19,9 @@ use rumqttc::QoS; use rustfs_config::notify::{ENV_NOTIFY_MQTT_KEYS, ENV_NOTIFY_WEBHOOK_KEYS, NOTIFY_MQTT_KEYS, NOTIFY_WEBHOOK_KEYS}; use rustfs_config::{ DEFAULT_LIMIT, EVENT_DEFAULT_DIR, MQTT_BROKER, MQTT_KEEP_ALIVE_INTERVAL, MQTT_PASSWORD, MQTT_QOS, MQTT_QUEUE_DIR, - MQTT_QUEUE_LIMIT, MQTT_RECONNECT_INTERVAL, MQTT_TOPIC, MQTT_USERNAME, WEBHOOK_AUTH_TOKEN, WEBHOOK_CLIENT_CERT, - WEBHOOK_CLIENT_KEY, WEBHOOK_ENDPOINT, WEBHOOK_QUEUE_DIR, WEBHOOK_QUEUE_LIMIT, + MQTT_QUEUE_LIMIT, MQTT_RECONNECT_INTERVAL, MQTT_TOPIC, MQTT_USERNAME, RUSTFS_WEBHOOK_SKIP_TLS_VERIFY_DEFAULT, + WEBHOOK_AUTH_TOKEN, WEBHOOK_CLIENT_CA, WEBHOOK_CLIENT_CERT, WEBHOOK_CLIENT_KEY, WEBHOOK_ENDPOINT, WEBHOOK_QUEUE_DIR, + WEBHOOK_QUEUE_LIMIT, WEBHOOK_SKIP_TLS_VERIFY, }; use rustfs_ecstore::config::KVS; use rustfs_targets::{ @@ -75,6 +76,11 @@ impl TargetFactory for WebhookTargetFactory { .unwrap_or(DEFAULT_LIMIT), client_cert: config.lookup(WEBHOOK_CLIENT_CERT).unwrap_or_default(), client_key: config.lookup(WEBHOOK_CLIENT_KEY).unwrap_or_default(), + client_ca: config.lookup(WEBHOOK_CLIENT_CA).unwrap_or_default(), + skip_tls_verify: config + .lookup(WEBHOOK_SKIP_TLS_VERIFY) + .and_then(|v| v.parse::().ok()) + .unwrap_or(RUSTFS_WEBHOOK_SKIP_TLS_VERIFY_DEFAULT), target_type: rustfs_targets::target::TargetType::NotifyEvent, }; diff --git a/crates/protocols/src/ftps/server.rs b/crates/protocols/src/ftps/server.rs index 7f9e102f..292648f2 100644 --- a/crates/protocols/src/ftps/server.rs +++ b/crates/protocols/src/ftps/server.rs @@ -64,7 +64,7 @@ pub struct FtpsServer { impl FtpsServer where - S: StorageBackend + Clone + Send + Sync + 'static + std::fmt::Debug, + S: StorageBackend + Clone + Send + Sync + 'static + Debug, { /// Create a new FTPS server pub async fn new(config: FtpsConfig, storage: S) -> Result { @@ -130,9 +130,9 @@ where let server_config = rustls::ServerConfig::builder() .with_no_client_auth() - .with_cert_resolver(std::sync::Arc::new(resolver)); + .with_cert_resolver(Arc::new(resolver)); - server_builder = server_builder.ftps_manual::(std::sync::Arc::new(server_config)); + server_builder = server_builder.ftps_manual::(Arc::new(server_config)); if self.config.ftps_required { info!("FTPS is explicitly required for all connections"); diff --git a/crates/targets/src/target/webhook.rs b/crates/targets/src/target/webhook.rs index 5c505e3b..525cf5b9 100644 --- a/crates/targets/src/target/webhook.rs +++ b/crates/targets/src/target/webhook.rs @@ -35,7 +35,7 @@ use std::{ }; use tokio::net::lookup_host; use tokio::sync::mpsc; -use tracing::{debug, error, info, instrument}; +use tracing::{debug, error, info, instrument, warn}; /// Arguments for configuring a Webhook target #[derive(Debug, Clone)] @@ -54,6 +54,10 @@ pub struct WebhookArgs { pub client_cert: String, /// The client key for TLS (PEM format) pub client_key: String, + /// The path to a custom client root CA certificate file (PEM format) to trust the server. + pub client_ca: String, + /// Skip TLS certificate verification. DANGEROUS: for testing only. + pub skip_tls_verify: bool, /// the target type pub target_type: TargetType, } @@ -82,6 +86,12 @@ impl WebhookArgs { return Err(TargetError::Configuration("cert and key must be specified as a pair".to_string())); } + if self.skip_tls_verify && !self.client_ca.is_empty() { + return Err(TargetError::Configuration( + "skip_tls_verify and client_ca are mutually exclusive; remove client_ca or disable skip_tls_verify".to_string(), + )); + } + Ok(()) } } @@ -125,29 +135,9 @@ where args.validate()?; // Create a TargetID let target_id = TargetID::new(id, ChannelTargetType::Webhook.as_str().to_string()); - // Build HTTP client - let mut client_builder = Client::builder() - .timeout(Duration::from_secs(30)) - .user_agent(rustfs_utils::get_user_agent(rustfs_utils::ServiceType::Basis)); - // Supplementary certificate processing logic - if !args.client_cert.is_empty() && !args.client_key.is_empty() { - // Add client certificate - let cert = std::fs::read(&args.client_cert) - .map_err(|e| TargetError::Configuration(format!("Failed to read client cert: {e}")))?; - let key = std::fs::read(&args.client_key) - .map_err(|e| TargetError::Configuration(format!("Failed to read client key: {e}")))?; - - let identity = reqwest::Identity::from_pem(&[cert, key].concat()) - .map_err(|e| TargetError::Configuration(format!("Failed to create identity: {e}")))?; - client_builder = client_builder.identity(identity); - } - - let http_client = Arc::new( - client_builder - .build() - .map_err(|e| TargetError::Configuration(format!("Failed to build HTTP client: {e}")))?, - ); + // Build HTTP client using the helper function + let http_client = Arc::new(Self::build_http_client(&args)?); // Build storage let queue_store = if !args.queue_dir.is_empty() { @@ -196,6 +186,46 @@ where }) } + fn build_http_client(args: &WebhookArgs) -> Result { + let mut client_builder = Client::builder() + .timeout(Duration::from_secs(30)) + .user_agent(rustfs_utils::get_user_agent(rustfs_utils::ServiceType::Basis)); + + // 1. Configure server certificate verification + if args.skip_tls_verify { + // DANGEROUS: For testing only, skip all certificate verification + client_builder = client_builder.danger_accept_invalid_certs(true); + warn!( + "Webhook target '{}' is configured to skip TLS verification. This is insecure and should not be used in production.", + args.endpoint + ); + } else if !args.client_ca.is_empty() { + // Use user-provided custom CA certificate + let ca_cert_pem = std::fs::read(&args.client_ca) + .map_err(|e| TargetError::Configuration(format!("Failed to read root CA cert: {e}")))?; + let ca_cert = reqwest::Certificate::from_pem(&ca_cert_pem) + .map_err(|e| TargetError::Configuration(format!("Failed to parse root CA cert: {e}")))?; + client_builder = client_builder.add_root_certificate(ca_cert); + } + // If neither is set, use the system's default trust store + + // 2. Configure client certificate (mTLS) + if !args.client_cert.is_empty() && !args.client_key.is_empty() { + let cert = std::fs::read(&args.client_cert) + .map_err(|e| TargetError::Configuration(format!("Failed to read client cert: {e}")))?; + let key = std::fs::read(&args.client_key) + .map_err(|e| TargetError::Configuration(format!("Failed to read client key: {e}")))?; + + let identity = reqwest::Identity::from_pem(&[cert, key].concat()) + .map_err(|e| TargetError::Configuration(format!("Failed to create identity for mTLS: {e}")))?; + client_builder = client_builder.identity(identity); + } + + client_builder + .build() + .map_err(|e| TargetError::Configuration(format!("Failed to build HTTP client: {e}"))) + } + async fn init(&self) -> Result<(), TargetError> { // Use CAS operations to ensure thread-safe initialization if !self.initialized.load(Ordering::SeqCst) { @@ -423,9 +453,60 @@ where #[cfg(test)] mod tests { - use crate::target::decode_object_name; + use super::WebhookArgs; + use crate::target::{TargetType, decode_object_name}; + use url::Url; use url::form_urlencoded; + fn base_args() -> WebhookArgs { + WebhookArgs { + enable: true, + endpoint: Url::parse("https://example.com/hook").unwrap(), + auth_token: String::new(), + queue_dir: String::new(), + queue_limit: 0, + client_cert: String::new(), + client_key: String::new(), + client_ca: String::new(), + skip_tls_verify: false, + target_type: TargetType::NotifyEvent, + } + } + + #[test] + fn test_validate_skip_tls_verify_and_client_ca_mutually_exclusive() { + let args = WebhookArgs { + skip_tls_verify: true, + client_ca: "/path/to/ca.pem".to_string(), + ..base_args() + }; + let result = args.validate(); + assert!(result.is_err()); + let err_msg = result.unwrap_err().to_string(); + assert!( + err_msg.contains("skip_tls_verify") && err_msg.contains("client_ca"), + "Error message should mention both fields, got: {err_msg}" + ); + } + + #[test] + fn test_validate_skip_tls_verify_without_client_ca_is_ok() { + let args = WebhookArgs { + skip_tls_verify: true, + ..base_args() + }; + assert!(args.validate().is_ok()); + } + + #[test] + fn test_validate_client_ca_without_skip_tls_verify_is_ok() { + let args = WebhookArgs { + client_ca: "/path/to/ca.pem".to_string(), + ..base_args() + }; + assert!(args.validate().is_ok()); + } + #[test] fn test_decode_object_name_with_spaces() { // Test case from the issue: "greeting file (2).csv" diff --git a/crates/utils/Cargo.toml b/crates/utils/Cargo.toml index 9a69ada0..fe53c660 100644 --- a/crates/utils/Cargo.toml +++ b/crates/utils/Cargo.toml @@ -48,7 +48,6 @@ regex = { workspace = true, optional = true } rustix = { workspace = true, optional = true } rustfs-config = { workspace = true, features = ["constants"] } rustls = { workspace = true, optional = true } -rustls-pemfile = { workspace = true, optional = true } rustls-pki-types = { workspace = true, optional = true } s3s = { workspace = true, optional = true } serde = { workspace = true, optional = true } @@ -80,7 +79,7 @@ workspace = true [features] default = ["ip"] # features that are enabled by default ip = ["dep:local-ip-address"] # ip characteristics and their dependencies -tls = ["dep:rustls", "dep:rustls-pemfile", "dep:rustls-pki-types"] # tls characteristics and their dependencies +tls = ["dep:rustls", "dep:rustls-pki-types"] # tls characteristics and their dependencies net = ["ip", "dep:url", "dep:netif", "dep:futures", "dep:transform-stream", "dep:bytes", "dep:s3s", "dep:hyper", "dep:thiserror", "dep:tokio"] # network features with DNS resolver io = ["dep:tokio"] path = [] # path manipulation features diff --git a/crates/utils/src/certs.rs b/crates/utils/src/certs.rs index 878667cf..a9dd6945 100644 --- a/crates/utils/src/certs.rs +++ b/crates/utils/src/certs.rs @@ -15,11 +15,11 @@ use crate::get_env_bool; use rustfs_config::{RUSTFS_TLS_CERT, RUSTFS_TLS_KEY}; use rustls::RootCertStore; -use rustls::server::danger::ClientCertVerifier; -use rustls::server::{ClientHello, ResolvesServerCert, ResolvesServerCertUsingSni, WebPkiClientVerifier}; +use rustls::server::{ + ClientHello, ResolvesServerCert, ResolvesServerCertUsingSni, WebPkiClientVerifier, danger::ClientCertVerifier, +}; use rustls::sign::CertifiedKey; -use rustls_pemfile::{certs, private_key}; -use rustls_pki_types::{CertificateDer, PrivateKeyDer}; +use rustls_pki_types::{CertificateDer, PrivateKeyDer, pem::PemObject}; use std::collections::HashMap; use std::io::Error; use std::path::Path; @@ -42,7 +42,7 @@ pub fn load_certs(filename: &str) -> io::Result>> { let mut reader = io::BufReader::new(cert_file); // Load and return certificate. - let certs = certs(&mut reader) + let certs = CertificateDer::pem_reader_iter(&mut reader) .collect::, _>>() .map_err(|e| certs_error(format!("certificate file {filename} format error:{e:?}")))?; if certs.is_empty() { @@ -65,7 +65,7 @@ pub fn load_cert_bundle_der_bytes(path: &str) -> io::Result>> { let pem = fs::read(path)?; let mut reader = io::BufReader::new(&pem[..]); - let certs = certs(&mut reader) + let certs = CertificateDer::pem_reader_iter(&mut reader) .collect::, _>>() .map_err(|e| certs_error(format!("Failed to parse PEM certs from {path}: {e}")))?; @@ -139,7 +139,8 @@ pub fn load_private_key(filename: &str) -> io::Result> { let mut reader = io::BufReader::new(keyfile); // Load and return a single private key. - private_key(&mut reader)?.ok_or_else(|| certs_error(format!("no private key found in {filename}"))) + PrivateKeyDer::from_pem_reader(&mut reader) + .map_err(|e| certs_error(format!("failed to parse private key in {filename}: {e}"))) } /// error function @@ -319,13 +320,14 @@ pub fn create_multi_cert_resolver( /// * A boolean indicating whether TLS key logging is enabled based on the `RUSTFS_TLS_KEYLOG` environment variable. /// pub fn tls_key_log() -> bool { - crate::get_env_bool(rustfs_config::ENV_TLS_KEYLOG, rustfs_config::DEFAULT_TLS_KEYLOG) + get_env_bool(rustfs_config::ENV_TLS_KEYLOG, rustfs_config::DEFAULT_TLS_KEYLOG) } #[cfg(test)] mod tests { use super::*; use std::fs; + use std::io::ErrorKind; use tempfile::TempDir; #[test] @@ -333,7 +335,7 @@ mod tests { let error_msg = "Test error message"; let error = certs_error(error_msg.to_string()); - assert_eq!(error.kind(), std::io::ErrorKind::Other); + assert_eq!(error.kind(), ErrorKind::Other); assert_eq!(error.to_string(), error_msg); } @@ -343,7 +345,7 @@ mod tests { assert!(result.is_err()); let error = result.unwrap_err(); - assert_eq!(error.kind(), std::io::ErrorKind::Other); + assert_eq!(error.kind(), ErrorKind::Other); assert!(error.to_string().contains("failed to open")); } @@ -353,7 +355,7 @@ mod tests { assert!(result.is_err()); let error = result.unwrap_err(); - assert_eq!(error.kind(), std::io::ErrorKind::Other); + assert_eq!(error.kind(), ErrorKind::Other); assert!(error.to_string().contains("failed to open")); } @@ -393,7 +395,7 @@ mod tests { assert!(result.is_err()); let error = result.unwrap_err(); - assert!(error.to_string().contains("no private key found")); + assert!(error.to_string().contains("failed to parse private key in")); } #[test] @@ -406,7 +408,7 @@ mod tests { assert!(result.is_err()); let error = result.unwrap_err(); - assert!(error.to_string().contains("no private key found")); + assert!(error.to_string().contains("failed to parse private key in")); } #[test] @@ -585,11 +587,8 @@ mod tests { #[test] fn test_memory_efficiency() { - // Test that error types are reasonably sized - use std::mem; - let error = certs_error("test".to_string()); - let error_size = mem::size_of_val(&error); + let error_size = std::mem::size_of_val(&error); // Error should not be excessively large assert!(error_size < 1024, "Error size should be reasonable, got {error_size} bytes"); diff --git a/rustfs/Cargo.toml b/rustfs/Cargo.toml index 54f993c3..e3234e82 100644 --- a/rustfs/Cargo.toml +++ b/rustfs/Cargo.toml @@ -99,7 +99,6 @@ serde_urlencoded = { workspace = true } # Cryptography and Security rustls = { workspace = true } subtle = { workspace = true } -rustls-pemfile = { workspace = true } jiff = { workspace = true } time = { workspace = true, features = ["parsing", "formatting", "serde"] } diff --git a/rustfs/src/server/cert.rs b/rustfs/src/server/cert.rs index 68b9bc14..937bc928 100644 --- a/rustfs/src/server/cert.rs +++ b/rustfs/src/server/cert.rs @@ -14,7 +14,7 @@ use rustfs_common::{MtlsIdentityPem, set_global_mtls_identity, set_global_root_cert}; use rustfs_config::{RUSTFS_CA_CERT, RUSTFS_PUBLIC_CERT, RUSTFS_TLS_CERT}; -use rustls::pki_types::{CertificateDer, PrivateKeyDer}; +use rustls::pki_types::{CertificateDer, PrivateKeyDer, pem::PemObject}; use std::path::{Path, PathBuf}; use tracing::{debug, info}; @@ -47,7 +47,7 @@ impl std::error::Error for RustFSError {} fn parse_pem_certs(pem: &[u8]) -> Result>, RustFSError> { let mut out = Vec::new(); let mut reader = std::io::Cursor::new(pem); - for item in rustls_pemfile::certs(&mut reader) { + for item in CertificateDer::pem_reader_iter(&mut reader) { let c = item.map_err(|e| RustFSError::Cert(format!("parse cert pem: {e}")))?; out.push(c); } @@ -67,8 +67,7 @@ fn parse_pem_certs(pem: &[u8]) -> Result>, RustFSErr /// Returns `RustFSError` if parsing fails or no key is found. fn parse_pem_private_key(pem: &[u8]) -> Result, RustFSError> { let mut reader = std::io::Cursor::new(pem); - let key = rustls_pemfile::private_key(&mut reader).map_err(|e| RustFSError::Cert(format!("parse private key pem: {e}")))?; - key.ok_or_else(|| RustFSError::Cert("no private key found in PEM".into())) + PrivateKeyDer::from_pem_reader(&mut reader).map_err(|e| RustFSError::Cert(format!("parse private key pem: {e}"))) } /// Helper function to read a file and return its contents. @@ -103,6 +102,9 @@ pub(crate) async fn init_cert(tls_path: &str) -> Result<(), RustFSError> { info!("No TLS path configured; skipping certificate initialization"); return Ok(()); } + // Make sure to use a modern encryption suite + let _ = rustls::crypto::aws_lc_rs::default_provider().install_default(); + let tls_dir = PathBuf::from(tls_path); // Load root certificates diff --git a/rustfs/src/server/http.rs b/rustfs/src/server/http.rs index c5bfd2bf..892688b7 100644 --- a/rustfs/src/server/http.rs +++ b/rustfs/src/server/http.rs @@ -441,10 +441,7 @@ async fn setup_tls_acceptor(tls_path: &str) -> Result> { } debug!("Found TLS directory, checking for certificates"); - // Make sure to use a modern encryption suite - let _ = rustls::crypto::aws_lc_rs::default_provider().install_default(); let mtls_verifier = rustfs_utils::build_webpki_client_verifier(tls_path)?; - // 1. Attempt to load all certificates in the directory (multi-certificate support, for SNI) if let Ok(cert_key_pairs) = rustfs_utils::load_all_certs_from_directory(tls_path) && !cert_key_pairs.is_empty()