From 82c0b4cca6a84423d9e3b061c6e612f2f6b06666 Mon Sep 17 00:00:00 2001 From: Wolf Vollprecht Date: Sun, 15 Feb 2026 10:00:56 +0100 Subject: [PATCH] implement security enhancements --- crates/sigstore-crypto/src/checkpoint.rs | 11 +++------- crates/sigstore-crypto/src/x509.rs | 21 +++++++++--------- crates/sigstore-verify/src/verify.rs | 22 +++++++++++++++---- .../sigstore-verify/src/verify_impl/tlog.rs | 15 +++++-------- 4 files changed, 36 insertions(+), 33 deletions(-) diff --git a/crates/sigstore-crypto/src/checkpoint.rs b/crates/sigstore-crypto/src/checkpoint.rs index 90ec7d3..ec97f5e 100644 --- a/crates/sigstore-crypto/src/checkpoint.rs +++ b/crates/sigstore-crypto/src/checkpoint.rs @@ -131,14 +131,9 @@ pub fn verify_signature_auto( match detect_key_type(public_key) { KeyType::Ed25519 => verify_ed25519(public_key, signature, message), KeyType::EcdsaP256 => verify_ecdsa_p256(public_key, signature, message), - KeyType::Unknown => { - // Fallback: try both (maintains backwards compatibility) - tracing::debug!("Unknown key type, trying Ed25519 then ECDSA P-256"); - if verify_ed25519(public_key, signature, message).is_ok() { - return Ok(()); - } - verify_ecdsa_p256(public_key, signature, message) - } + KeyType::Unknown => Err(Error::Verification( + "unsupported or unrecognized public key type".to_string(), + )), } } diff --git a/crates/sigstore-crypto/src/x509.rs b/crates/sigstore-crypto/src/x509.rs index 2076052..89e9ae5 100644 --- a/crates/sigstore-crypto/src/x509.rs +++ b/crates/sigstore-crypto/src/x509.rs @@ -102,14 +102,15 @@ fn determine_signing_scheme( } else if curve_oid == SECP_384_R_1 { return Ok(SigningScheme::EcdsaP384Sha384); } else { - // Unknown EC curve - default to P-256 for compatibility - tracing::warn!("Unknown EC curve OID: {}, defaulting to P-256", curve_oid); - return Ok(SigningScheme::EcdsaP256Sha256); + return Err(Error::InvalidCertificate(format!( + "unsupported EC curve OID: {}", + curve_oid + ))); } } else { - // EC key missing curve parameters - default to P-256 for compatibility - tracing::warn!("EC key missing curve parameters, defaulting to P-256"); - return Ok(SigningScheme::EcdsaP256Sha256); + return Err(Error::InvalidCertificate( + "EC key missing curve parameters".to_string(), + )); } } else if alg_oid == RSA_ENCRYPTION { // RSA key - default to RSA PKCS#1 SHA-256 @@ -119,12 +120,10 @@ fn determine_signing_scheme( return Ok(SigningScheme::Ed25519); } - // Unknown algorithm - default to P-256 for compatibility - tracing::warn!( - "Unknown public key algorithm OID: {}, defaulting to P-256", + Err(Error::InvalidCertificate(format!( + "unsupported public key algorithm OID: {}", alg_oid - ); - Ok(SigningScheme::EcdsaP256Sha256) + ))) } /// Extract identity from Subject Alternative Name (SAN) extension diff --git a/crates/sigstore-verify/src/verify.rs b/crates/sigstore-verify/src/verify.rs index 2f248c7..4ec68b0 100644 --- a/crates/sigstore-verify/src/verify.rs +++ b/crates/sigstore-verify/src/verify.rs @@ -272,24 +272,38 @@ impl Verifier { // Verify against policy constraints if let Some(ref expected_identity) = policy.identity { - if let Some(ref actual_identity) = result.identity { - if actual_identity != expected_identity { + match &result.identity { + Some(actual_identity) if actual_identity == expected_identity => {} + Some(actual_identity) => { return Err(Error::Verification(format!( "identity mismatch: expected {}, got {}", expected_identity, actual_identity ))); } + None => { + return Err(Error::Verification(format!( + "certificate is missing identity (SAN), but policy requires: {}", + expected_identity + ))); + } } } if let Some(ref expected_issuer) = policy.issuer { - if let Some(ref actual_issuer) = result.issuer { - if actual_issuer != expected_issuer { + match &result.issuer { + Some(actual_issuer) if actual_issuer == expected_issuer => {} + Some(actual_issuer) => { return Err(Error::Verification(format!( "issuer mismatch: expected {}, got {}", expected_issuer, actual_issuer ))); } + None => { + return Err(Error::Verification(format!( + "certificate is missing issuer (Fulcio OID extension), but policy requires: {}", + expected_issuer + ))); + } } } diff --git a/crates/sigstore-verify/src/verify_impl/tlog.rs b/crates/sigstore-verify/src/verify_impl/tlog.rs index ad04e67..9d0bdaf 100644 --- a/crates/sigstore-verify/src/verify_impl/tlog.rs +++ b/crates/sigstore-verify/src/verify_impl/tlog.rs @@ -6,7 +6,7 @@ use crate::error::{Error, Result}; use base64::Engine; use serde::Serialize; -use sigstore_crypto::{verify_signature, Checkpoint, SigningScheme}; +use sigstore_crypto::{verify_signature_auto, Checkpoint}; use sigstore_trust_root::TrustedRoot; use sigstore_types::bundle::InclusionProof; use sigstore_types::{Bundle, SignatureBytes, TransparencyLogEntry}; @@ -83,8 +83,6 @@ pub fn verify_checkpoint( inclusion_proof: &InclusionProof, trusted_root: &TrustedRoot, ) -> Result<()> { - use sigstore_crypto::verify_signature_auto; - // Parse the checkpoint (signed note) let checkpoint = Checkpoint::from_text(checkpoint_envelope) .map_err(|e| Error::Verification(format!("Failed to parse checkpoint: {}", e)))?; @@ -181,13 +179,10 @@ pub fn verify_set(entry: &TransparencyLogEntry, trusted_root: &TrustedRoot) -> R // Get signature bytes from signed timestamp let signature = SignatureBytes::new(promise.signed_entry_timestamp.as_bytes().to_vec()); - verify_signature( - &log_key, - &canonical_json, - &signature, - SigningScheme::EcdsaP256Sha256, - ) - .map_err(|e| Error::Verification(format!("SET verification failed: {}", e)))?; + // Use automatic key type detection from the SPKI structure, + // rather than hardcoding ECDSA P-256 (matches checkpoint verification behavior) + verify_signature_auto(&log_key, &signature, &canonical_json) + .map_err(|e| Error::Verification(format!("SET verification failed: {}", e)))?; Ok(()) }