diff --git a/src/lib/src/signature/keyless/fulcio.rs b/src/lib/src/signature/keyless/fulcio.rs index 48eacdb..1636fb2 100644 --- a/src/lib/src/signature/keyless/fulcio.rs +++ b/src/lib/src/signature/keyless/fulcio.rs @@ -89,10 +89,11 @@ impl FulcioClient { /// /// # Certificate Pinning /// - /// Certificate pinning is ENFORCED by default using embedded pins for Sigstore - /// production infrastructure. Custom pins can be set via `WSC_FULCIO_PINS`. - /// Set `WSC_REQUIRE_CERT_PINNING=1` to fail if pinning cannot be configured. - pub fn new() -> Self { + /// Certificate pinning is unconditionally ENFORCED using embedded pins for + /// Sigstore production infrastructure. Custom pins can be set via + /// `WSC_FULCIO_PINS`. If the pinned agent cannot be constructed, this + /// returns an error rather than silently downgrading to unpinned TLS. + pub fn new() -> Result { Self::with_url("https://fulcio.sigstore.dev".to_string()) } @@ -103,36 +104,28 @@ impl FulcioClient { /// /// # Certificate Pinning /// - /// Certificate pinning is now ENFORCED when configured via `WSC_FULCIO_PINS`. - /// Set `WSC_REQUIRE_CERT_PINNING=1` to fail if pinning cannot be configured. - pub fn with_url(base_url: String) -> Self { + /// Certificate pinning is unconditionally ENFORCED. Returns an error if the + /// pinned agent cannot be constructed ā€” there is no unpinned fallback. + pub fn with_url(base_url: String) -> Result { #[cfg(not(target_os = "wasi"))] { use super::transport::create_agent_with_optional_pinning; use super::cert_pinning::PinningConfig; - // Create pinning configuration for Fulcio - let pinning = Some(PinningConfig::fulcio()); + // Certificate pinning is mandatory for Sigstore endpoints: a + // failure to build the pinned agent is a hard error, never a + // silent downgrade to unpinned TLS (audit finding C-4). + let agent = create_agent_with_optional_pinning(Some(PinningConfig::fulcio()))?; - // Create agent with certificate pinning (or fall back to standard TLS) - let agent = match create_agent_with_optional_pinning(pinning) { - Ok(agent) => agent, - Err(e) => { - // Log error but don't panic - fall back to standard agent - log::error!("Failed to create pinned agent for Fulcio: {}. Using standard TLS.", e); - super::transport::create_standard_agent() - } - }; - - Self { + Ok(Self { base_url, client: agent, - } + }) } #[cfg(target_os = "wasi")] { - Self { base_url } + Ok(Self { base_url }) } } @@ -458,12 +451,6 @@ impl FulcioClient { } } -impl Default for FulcioClient { - fn default() -> Self { - Self::new() - } -} - // Add pem crate for parsing PEM certificates mod pem { use crate::error::WSError; @@ -518,13 +505,13 @@ mod tests { #[test] fn test_fulcio_client_creation() { - let client = FulcioClient::new(); + let client = FulcioClient::new().unwrap(); assert_eq!(client.base_url, "https://fulcio.sigstore.dev"); } #[test] fn test_fulcio_client_with_custom_url() { - let client = FulcioClient::with_url("https://custom.fulcio.dev".to_string()); + let client = FulcioClient::with_url("https://custom.fulcio.dev".to_string()).unwrap(); assert_eq!(client.base_url, "https://custom.fulcio.dev"); } diff --git a/src/lib/src/signature/keyless/rekor.rs b/src/lib/src/signature/keyless/rekor.rs index a83c6b9..7321ecc 100644 --- a/src/lib/src/signature/keyless/rekor.rs +++ b/src/lib/src/signature/keyless/rekor.rs @@ -8,8 +8,8 @@ /// ```no_run /// use wsc::keyless::{RekorClient, FulcioCertificate}; /// -/// // Create a Rekor client -/// let client = RekorClient::new(); +/// // Create a Rekor client (certificate pinning is enforced) +/// let client = RekorClient::new().expect("pinned Rekor client"); /// /// // Create a mock certificate (in real use, get from Fulcio) /// let certificate = FulcioCertificate { @@ -160,10 +160,11 @@ impl RekorClient { /// /// # Certificate Pinning /// - /// Certificate pinning is ENFORCED by default using embedded pins for Sigstore - /// production infrastructure. Custom pins can be set via `WSC_REKOR_PINS`. - /// Set `WSC_REQUIRE_CERT_PINNING=1` to fail if pinning cannot be configured. - pub fn new() -> Self { + /// Certificate pinning is unconditionally ENFORCED using embedded pins for + /// Sigstore production infrastructure. Custom pins can be set via + /// `WSC_REKOR_PINS`. If the pinned agent cannot be constructed, this + /// returns an error rather than silently downgrading to unpinned TLS. + pub fn new() -> Result { Self::with_url("https://rekor.sigstore.dev".to_string()) } @@ -171,36 +172,28 @@ impl RekorClient { /// /// # Certificate Pinning /// - /// Certificate pinning is ENFORCED when configured via `WSC_REKOR_PINS`. - /// Set `WSC_REQUIRE_CERT_PINNING=1` to fail if pinning cannot be configured. - pub fn with_url(base_url: String) -> Self { + /// Certificate pinning is unconditionally ENFORCED. Returns an error if the + /// pinned agent cannot be constructed ā€” there is no unpinned fallback. + pub fn with_url(base_url: String) -> Result { #[cfg(not(target_os = "wasi"))] { use super::transport::create_agent_with_optional_pinning; use super::cert_pinning::PinningConfig; - // Create pinning configuration for Rekor - let pinning = Some(PinningConfig::rekor()); + // Certificate pinning is mandatory for Sigstore endpoints: a + // failure to build the pinned agent is a hard error, never a + // silent downgrade to unpinned TLS (audit finding C-4). + let agent = create_agent_with_optional_pinning(Some(PinningConfig::rekor()))?; - // Create agent with certificate pinning (or fall back to standard TLS) - let agent = match create_agent_with_optional_pinning(pinning) { - Ok(agent) => agent, - Err(e) => { - // Log error but don't panic - fall back to standard agent - log::error!("Failed to create pinned agent for Rekor: {}. Using standard TLS.", e); - super::transport::create_standard_agent() - } - }; - - Self { + Ok(Self { base_url, client: agent, - } + }) } #[cfg(target_os = "wasi")] { - Self { base_url } + Ok(Self { base_url }) } } @@ -446,12 +439,6 @@ impl RekorClient { } } -impl Default for RekorClient { - fn default() -> Self { - Self::new() - } -} - /// Build a `RekorEntry` from a parsed Rekor upload response (audit H-5). /// /// Pure helper so the empty-SET / empty-inclusion-proof rejection rules can be @@ -540,14 +527,14 @@ mod tests { #[test] fn test_rekor_client_new() { - let client = RekorClient::new(); + let client = RekorClient::new().unwrap(); assert_eq!(client.base_url, "https://rekor.sigstore.dev"); } #[test] fn test_rekor_client_with_url() { let custom_url = "https://custom.rekor.server".to_string(); - let client = RekorClient::with_url(custom_url.clone()); + let client = RekorClient::with_url(custom_url.clone()).unwrap(); assert_eq!(client.base_url, custom_url); } @@ -582,7 +569,7 @@ mod tests { #[test] fn test_upload_entry_invalid_hash_length() { - let client = RekorClient::new(); + let client = RekorClient::new().unwrap(); // Create stub certificate let cert = FulcioCertificate { @@ -609,7 +596,7 @@ mod tests { #[test] fn test_verify_inclusion_rejects_invalid() { - let client = RekorClient::new(); + let client = RekorClient::new().unwrap(); let entry = RekorEntry { uuid: "test-uuid".to_string(), log_index: 1, @@ -687,7 +674,7 @@ mod tests { #[test] fn test_mock_rekor_entry_flow() { // This test demonstrates the expected flow with mock data - let client = RekorClient::new(); + let client = RekorClient::new().unwrap(); // Create mock certificate let cert = FulcioCertificate { diff --git a/src/lib/src/signature/keyless/signer.rs b/src/lib/src/signature/keyless/signer.rs index aaa35a1..d9dd7b3 100644 --- a/src/lib/src/signature/keyless/signer.rs +++ b/src/lib/src/signature/keyless/signer.rs @@ -205,20 +205,20 @@ impl KeylessSigner { // Create Fulcio client with appropriate URL let fulcio = if let Some(url) = &config.fulcio_url { - FulcioClient::with_url(url.clone()) + FulcioClient::with_url(url.clone())? } else if use_staging { - FulcioClient::with_url("https://fulcio.staging.sigstore.dev".to_string()) + FulcioClient::with_url("https://fulcio.staging.sigstore.dev".to_string())? } else { - FulcioClient::new() + FulcioClient::new()? }; // Create Rekor client with appropriate URL let rekor = if let Some(url) = &config.rekor_url { - RekorClient::with_url(url.clone()) + RekorClient::with_url(url.clone())? } else if use_staging { - RekorClient::with_url("https://rekor.staging.sigstore.dev".to_string()) + RekorClient::with_url("https://rekor.staging.sigstore.dev".to_string())? } else { - RekorClient::new() + RekorClient::new()? }; Ok(Self { diff --git a/src/lib/src/signature/keyless/transport.rs b/src/lib/src/signature/keyless/transport.rs index 7981a9e..a827a30 100644 --- a/src/lib/src/signature/keyless/transport.rs +++ b/src/lib/src/signature/keyless/transport.rs @@ -257,10 +257,12 @@ pub fn create_standard_agent() -> ureq::Agent { /// Create an HTTP agent with optional certificate pinning. /// -/// This is the recommended function for creating HTTP clients. It automatically: -/// - Enables certificate pinning if a valid `PinningConfig` is provided -/// - Falls back to standard WebPKI if pinning config is empty or fails -/// - Respects `WSC_REQUIRE_CERT_PINNING` environment variable +/// This is the recommended function for creating HTTP clients: +/// - When a non-empty `PinningConfig` is supplied, pinning is **mandatory** ā€” +/// the pinned agent is built or a hard error is returned. There is no +/// silent downgrade to unpinned TLS. (This closes audit finding C-4.) +/// - When no pins are supplied, the caller has explicitly opted out and a +/// standard WebPKI agent is returned. /// /// # Arguments /// * `pinning` - Optional certificate pinning configuration @@ -269,38 +271,34 @@ pub fn create_standard_agent() -> ureq::Agent { /// A configured ureq::Agent /// /// # Errors -/// Returns error only if `WSC_REQUIRE_CERT_PINNING=1` and pinning cannot be configured +/// Returns `WSError::CertificatePinningError` if a non-empty pin set is +/// supplied but the pinned agent cannot be constructed. +/// +/// # Deprecated environment variable +/// `WSC_REQUIRE_CERT_PINNING` is retained for backwards compatibility but is +/// redundant: pinning is now unconditionally enforced whenever a non-empty +/// pin set is supplied. The Sigstore production path (`PinningConfig::fulcio`, +/// `PinningConfig::rekor`) always supplies one. The variable now only affects +/// the explicit no-pins opt-out branch below. #[cfg(not(target_os = "wasi"))] pub fn create_agent_with_optional_pinning( pinning: Option, ) -> Result { - let require_pinning = std::env::var("WSC_REQUIRE_CERT_PINNING") - .unwrap_or_default() - == "1"; - match pinning { - Some(config) if config.is_enabled() => { - match create_pinned_agent(config) { - Ok(agent) => Ok(agent), - Err(e) => { - if require_pinning { - Err(e) - } else { - log::warn!("Failed to enable certificate pinning: {}. Falling back to standard TLS.", e); - Ok(create_standard_agent()) - } - } - } - } + // Pins are configured: pinning is MANDATORY. A configured pin set + // must never silently downgrade to unpinned TLS ā€” that silent + // posture downgrade was audit finding C-4. Any failure to build the + // pinned agent is propagated as a hard error. + Some(config) if config.is_enabled() => create_pinned_agent(config), + // No pins configured: the caller explicitly opted out of pinning. _ => { - if require_pinning { - Err(WSError::CertificatePinningError( - "Certificate pinning required (WSC_REQUIRE_CERT_PINNING=1) but no pins configured".to_string() - )) - } else { - log::debug!("Certificate pinning disabled, using standard TLS"); - Ok(create_standard_agent()) + if std::env::var("WSC_REQUIRE_CERT_PINNING").unwrap_or_default() == "1" { + return Err(WSError::CertificatePinningError( + "Certificate pinning required (WSC_REQUIRE_CERT_PINNING=1) but no pins configured".to_string(), + )); } + log::debug!("Certificate pinning disabled, using standard TLS"); + Ok(create_standard_agent()) } } } diff --git a/src/lib/tests/keyless_integration.rs b/src/lib/tests/keyless_integration.rs index f6776ac..031ab43 100644 --- a/src/lib/tests/keyless_integration.rs +++ b/src/lib/tests/keyless_integration.rs @@ -93,7 +93,7 @@ fn test_github_actions_keyless_signing() { println!("\nšŸ” Testing Rekor verification with REAL production data..."); use wsc::keyless::RekorClient; - let rekor_client = RekorClient::new(); + let rekor_client = RekorClient::new().expect("pinned Rekor client"); let verification_result = rekor_client.verify_inclusion(&signature.rekor_entry); match &verification_result {