remove manual verification code and use webpki

Author: wolfv

crates/sigstore-verify/src/verify.rs

@@ -230,11 +230,11 @@ impl Verifier {
             &self.trusted_root,
         )?;
 
-        // (1): Verify that the signing certificate chains to the root of trust
-        //      and is valid at the time of signing.
+        // (1): Verify that the signing certificate chains to the root of trust,
+        //      is valid at the time of signing, and has CODE_SIGNING EKU.
         if policy.verify_certificate {
             crate::verify_impl::helpers::verify_certificate_chain(
-                &cert_der,
+                &bundle.verification_material.content,
                 validation_time,
                 &self.trusted_root,
             )?;
@@ -249,9 +249,7 @@ impl Verifier {
             )?;
         }
 
-        // (3): Verify that the signing certificate conforms to the Sigstore
-        //      X.509 profile and verify against the given `VerificationPolicy`.
-        crate::verify_impl::helpers::verify_x509_profile(&cert_der)?;
+        // (3): Verify against the given `VerificationPolicy`.
 
         // Verify against policy constraints
         if let Some(ref expected_identity) = policy.identity {

crates/sigstore-verify/src/verify_impl/helpers.rs

@@ -4,12 +4,13 @@
 //! large verification logic into manageable pieces.
 
 use crate::error::{Error, Result};
-use const_oid::db::rfc5912::{ECDSA_WITH_SHA_256, ECDSA_WITH_SHA_384, SECP_256_R_1, SECP_384_R_1};
+use const_oid::db::rfc5912::ID_KP_CODE_SIGNING;
+use rustls_pki_types::CertificateDer;
 use sigstore_crypto::CertificateInfo;
 use sigstore_trust_root::TrustedRoot;
 use sigstore_types::bundle::VerificationMaterialContent;
 use sigstore_types::{Bundle, SignatureContent};
-use x509_cert::der::Encode;
+use webpki::{anchor_from_trusted_cert, EndEntityCert, KeyUsage, ALL_VERIFICATION_ALGS};
 
 /// Extract and decode the signing certificate from verification material
 pub fn extract_certificate_der(
@@ -200,16 +201,37 @@ pub fn validate_certificate_time(validation_time: i64, cert_info: &CertificateIn
 /// Verify the certificate chain to the Fulcio root of trust
 ///
 /// This function verifies that the signing certificate chains to a trusted
-/// Fulcio root certificate at the given verification time.
+/// Fulcio root certificate at the given verification time. It also verifies
+/// that the certificate has the CODE_SIGNING extended key usage.
 pub fn verify_certificate_chain(
-    cert_der: &[u8],
-    _validation_time: i64,
+    verification_material: &VerificationMaterialContent,
+    validation_time: i64,
     trusted_root: &TrustedRoot,
 ) -> Result<()> {
-    use x509_cert::der::Decode;
-    use x509_cert::Certificate;
+    // Extract the end-entity certificate and any intermediates from the bundle
+    let (ee_cert_der, intermediate_ders) = match verification_material {
+        VerificationMaterialContent::Certificate(cert) => {
+            (cert.raw_bytes.as_bytes().to_vec(), Vec::new())
+        }
+        VerificationMaterialContent::X509CertificateChain { certificates } => {
+            if certificates.is_empty() {
+                return Err(Error::Verification("no certificates in chain".to_string()));
+            }
+            let ee = certificates[0].raw_bytes.as_bytes().to_vec();
+            let intermediates: Vec<Vec<u8>> = certificates[1..]
+                .iter()
+                .map(|c| c.raw_bytes.as_bytes().to_vec())
+                .collect();
+            (ee, intermediates)
+        }
+        VerificationMaterialContent::PublicKey { .. } => {
+            return Err(Error::Verification(
+                "public key verification not yet supported".to_string(),
+            ));
+        }
+    };
 
-    // Get Fulcio certificates from trusted root
+    // Get Fulcio certificates from trusted root to use as trust anchors
     let fulcio_certs = trusted_root
         .fulcio_certs()
         .map_err(|e| Error::Verification(format!("failed to get Fulcio certs: {}", e)))?;
@@ -220,180 +242,61 @@ pub fn verify_certificate_chain(
         ));
     }
 
-    // Parse the end-entity certificate
-    let ee_cert = Certificate::from_der(cert_der).map_err(|e| {
-        Error::Verification(format!("failed to parse end-entity certificate: {}", e))
-    })?;
-
-    // Get the issuer from the EE certificate
-    let ee_issuer = &ee_cert.tbs_certificate.issuer;
-
-    // Extract the original TBS DER bytes from the certificate
-    // CRITICAL: We must use the original DER bytes, not re-serialize, because
-    // re-serialization can produce different bytes even if semantically equivalent,
-    // which will break signature verification.
-    let tbs_der = extract_tbs_der(cert_der).map_err(|e| {
-        Error::Verification(format!("failed to extract TBS certificate bytes: {}", e))
-    })?;
-
-    // Try to find a matching Fulcio root by comparing issuers
-    let mut found_issuer = false;
-    for fulcio_cert_der in &fulcio_certs {
-        if let Ok(fulcio_cert) = Certificate::from_der(fulcio_cert_der) {
-            let fulcio_subject = &fulcio_cert.tbs_certificate.subject;
-
-            // Check if the EE certificate's issuer matches this Fulcio cert's subject
-            if ee_issuer == fulcio_subject {
-                // Verify the signature
-                let Some(signature) = ee_cert.signature.as_bytes() else {
-                    continue;
-                };
-
-                // Determine the signing scheme by combining:
-                // 1. The curve from the issuer's public key (SPKI)
-                // 2. The hash algorithm from the signature algorithm OID
-                let sig_alg_oid = ee_cert.signature_algorithm.oid;
-
-                // Get the curve from the issuer's public key
-                let issuer_spki = &fulcio_cert.tbs_certificate.subject_public_key_info;
-                let curve_oid = match extract_ec_curve_oid(issuer_spki) {
-                    Ok(oid) => oid,
-                    Err(_) => continue,
-                };
-
-                // Map (curve, hash) to SigningScheme using OID constants
-                let scheme = if curve_oid == SECP_256_R_1 && sig_alg_oid == ECDSA_WITH_SHA_256 {
-                    // P-256 with SHA-256
-                    sigstore_crypto::SigningScheme::EcdsaP256Sha256
-                } else if curve_oid == SECP_256_R_1 && sig_alg_oid == ECDSA_WITH_SHA_384 {
-                    // P-256 with SHA-384 (non-standard but valid)
-                    sigstore_crypto::SigningScheme::EcdsaP256Sha384
-                } else if curve_oid == SECP_384_R_1 && sig_alg_oid == ECDSA_WITH_SHA_384 {
-                    // P-384 with SHA-384
-                    sigstore_crypto::SigningScheme::EcdsaP384Sha384
-                } else {
-                    tracing::warn!(
-                        "Unknown curve/signature algorithm combination: curve={}, sig_alg={}",
-                        curve_oid,
-                        sig_alg_oid
-                    );
-                    continue;
-                };
-
-                let Some(issuer_pub_key) = issuer_spki.subject_public_key.as_bytes() else {
-                    continue;
-                };
-
-                if sigstore_crypto::verify_signature(issuer_pub_key, &tbs_der, signature, scheme)
-                    .is_ok()
-                {
-                    found_issuer = true;
-                    break;
-                }
-            }
-        }
-    }
-
-    if !found_issuer {
+    // Build trust anchors from Fulcio root certificates
+    let trust_anchors: Vec<_> = fulcio_certs
+        .iter()
+        .filter_map(|cert_der| {
+            let cert = CertificateDer::from(&cert_der[..]);
+            anchor_from_trusted_cert(&cert)
+                .map(|anchor| anchor.to_owned())
+                .ok()
+        })
+        .collect();
+
+    if trust_anchors.is_empty() {
         return Err(Error::Verification(
-            "certificate does not chain to any trusted Fulcio root".to_string(),
+            "failed to create trust anchors from Fulcio certificates".to_string(),
         ));
     }
 
-    // Verify certificate validity period
-    let cert_info = sigstore_crypto::parse_certificate_info(cert_der)?;
-    validate_certificate_time(_validation_time, &cert_info)?;
+    // Convert intermediate certificates to CertificateDer
+    let intermediate_certs: Vec<CertificateDer<'static>> = intermediate_ders
+        .into_iter()
+        .map(|der| CertificateDer::from(der).into_owned())
+        .collect();
 
-    Ok(())
-}
-
-/// Extract the EC curve OID from a SubjectPublicKeyInfo
-///
-/// For EC keys, the algorithm parameters contain the curve OID
-fn extract_ec_curve_oid(
-    spki: &x509_cert::spki::SubjectPublicKeyInfoOwned,
-) -> Result<const_oid::ObjectIdentifier> {
-    use const_oid::db::rfc5912::ID_EC_PUBLIC_KEY;
-    use const_oid::ObjectIdentifier;
-
-    // For EC keys, the algorithm OID should be id-ecPublicKey (1.2.840.10045.2.1)
-    if spki.algorithm.oid != ID_EC_PUBLIC_KEY {
-        return Err(Error::Verification("Not an EC public key".to_string()));
-    }
-
-    // The parameters field contains the curve OID
-    let Some(params) = &spki.algorithm.parameters else {
-        return Err(Error::Verification(
-            "EC public key missing curve parameters".to_string(),
-        ));
-    };
-
-    // The AnyRef value() gives us the raw content bytes (without tag/length).
-    // For an OID, this is the encoded OID bytes.
-    // ObjectIdentifier::from_bytes expects raw OID bytes (without tag/length header).
-    let curve_oid = ObjectIdentifier::from_bytes(params.value())
-        .map_err(|e| Error::Verification(format!("failed to parse EC curve OID: {}", e)))?;
-
-    Ok(curve_oid)
-}
+    // Parse the end-entity certificate for webpki
+    let ee_cert_der_ref = CertificateDer::from(ee_cert_der.as_slice());
+    let end_entity_cert = EndEntityCert::try_from(&ee_cert_der_ref).map_err(|e| {
+        Error::Verification(format!("failed to parse end-entity certificate: {}", e))
+    })?;
 
-/// Extract the original TBS (To Be Signed) certificate DER bytes from a certificate
-///
-/// CRITICAL: This extracts the original DER bytes without re-parsing and re-serializing,
-/// which is necessary for correct signature verification.
-fn extract_tbs_der(cert_der: &[u8]) -> Result<Vec<u8>> {
-    use x509_cert::der::{Decode, Reader, SliceReader};
-
-    // A Certificate is a SEQUENCE containing:
-    // 1. TBSCertificate (SEQUENCE)
-    // 2. signatureAlgorithm (SEQUENCE)
-    // 3. signatureValue (BIT STRING)
-    //
-    // We need to extract the raw bytes of the TBSCertificate element.
-
-    let mut reader = SliceReader::new(cert_der)
-        .map_err(|e| Error::Verification(format!("failed to create DER reader: {}", e)))?;
-
-    // Decode the outer SEQUENCE header
-    let outer_header = x509_cert::der::Header::decode(&mut reader)
-        .map_err(|e| Error::Verification(format!("failed to decode certificate header: {}", e)))?;
-
-    // The remaining bytes should be the certificate contents
-    let cert_contents = reader
-        .read_slice(outer_header.length)
-        .map_err(|e| Error::Verification(format!("failed to read certificate contents: {}", e)))?;
-
-    // Now decode the TBS header from the certificate contents
-    let mut tbs_reader = SliceReader::new(cert_contents)
-        .map_err(|e| Error::Verification(format!("failed to create TBS reader: {}", e)))?;
-
-    let tbs_header = x509_cert::der::Header::decode(&mut tbs_reader)
-        .map_err(|e| Error::Verification(format!("failed to decode TBS header: {}", e)))?;
-
-    // Calculate the total length of the TBS including its header
-    let header_len: usize = tbs_header
-        .encoded_len()
-        .map_err(|e| Error::Verification(format!("failed to encode TBS header length: {}", e)))?
-        .try_into()
-        .map_err(|_| Error::Verification("TBS header length too large".to_string()))?;
-
-    let body_len: usize = tbs_header
-        .length
-        .try_into()
-        .map_err(|_| Error::Verification("TBS body length too large".to_string()))?;
-
-    let tbs_total_len = header_len
-        .checked_add(body_len)
-        .ok_or_else(|| Error::Verification("TBS length calculation overflow".to_string()))?;
-
-    // Extract the TBS bytes (header + body)
-    if tbs_total_len > cert_contents.len() {
-        return Err(Error::Verification(
-            "TBS length exceeds certificate contents".to_string(),
-        ));
-    }
+    // Convert validation time to webpki UnixTime
+    let verification_time = webpki::types::UnixTime::since_unix_epoch(
+        std::time::Duration::from_secs(validation_time as u64),
+    );
+
+    // Verify the certificate chain with CODE_SIGNING EKU
+    // This performs:
+    // - Chain building from end-entity to trust anchor
+    // - Signature verification at each step
+    // - Time validity checking
+    // - Extended Key Usage validation (CODE_SIGNING)
+    end_entity_cert
+        .verify_for_usage(
+            ALL_VERIFICATION_ALGS,
+            &trust_anchors,
+            &intermediate_certs,
+            verification_time,
+            KeyUsage::required(ID_KP_CODE_SIGNING.as_bytes()),
+            None, // No revocation checking
+            None, // No path verification callback
+        )
+        .map_err(|e| Error::Verification(format!("certificate chain validation failed: {}", e)))?;
+
+    tracing::debug!("Certificate chain validated successfully with CODE_SIGNING EKU");
 
-    Ok(cert_contents[..tbs_total_len].to_vec())
+    Ok(())
 }
 
 /// Verify the Signed Certificate Timestamp (SCT) embedded in the certificate
@@ -473,65 +376,3 @@ fn get_issuer_spki(
         "could not find issuer certificate for SCT verification".to_string(),
     ))
 }
-
-/// Verify that the certificate conforms to the Sigstore X.509 profile
-///
-/// This checks:
-/// - KeyUsage extension contains digitalSignature
-/// - ExtendedKeyUsage extension contains codeSigning
-pub fn verify_x509_profile(cert_der: &[u8]) -> Result<()> {
-    use x509_cert::der::Decode;
-    use x509_cert::ext::pkix::{ExtendedKeyUsage, KeyUsage, KeyUsages};
-    use x509_cert::Certificate;
-
-    // OID constants for X.509 extensions
-    use const_oid::db::rfc5280::{ID_CE_EXT_KEY_USAGE, ID_CE_KEY_USAGE};
-    use const_oid::db::rfc5912::ID_KP_CODE_SIGNING;
-
-    let cert = Certificate::from_der(cert_der)
-        .map_err(|e| Error::Verification(format!("failed to parse certificate: {}", e)))?;
-
-    let extensions = cert
-        .tbs_certificate
-        .extensions
-        .as_ref()
-        .ok_or_else(|| Error::Verification("certificate has no extensions".to_string()))?;
-
-    // Check KeyUsage extension (OID 2.5.29.15)
-    let key_usage_ext = extensions
-        .iter()
-        .find(|ext| ext.extn_id == ID_CE_KEY_USAGE)
-        .ok_or_else(|| {
-            Error::Verification("certificate is missing KeyUsage extension".to_string())
-        })?;
-
-    let key_usage = KeyUsage::from_der(key_usage_ext.extn_value.as_bytes())
-        .map_err(|e| Error::Verification(format!("failed to parse KeyUsage extension: {}", e)))?;
-
-    if !key_usage.0.contains(KeyUsages::DigitalSignature) {
-        return Err(Error::Verification(
-            "KeyUsage extension does not contain digitalSignature".to_string(),
-        ));
-    }
-
-    // Check ExtendedKeyUsage extension (OID 2.5.29.37)
-    let eku_ext = extensions
-        .iter()
-        .find(|ext| ext.extn_id == ID_CE_EXT_KEY_USAGE)
-        .ok_or_else(|| {
-            Error::Verification("certificate is missing ExtendedKeyUsage extension".to_string())
-        })?;
-
-    let eku = ExtendedKeyUsage::from_der(eku_ext.extn_value.as_bytes()).map_err(|e| {
-        Error::Verification(format!("failed to parse ExtendedKeyUsage extension: {}", e))
-    })?;
-
-    // Check for code signing OID (1.3.6.1.5.5.7.3.3)
-    if !eku.0.contains(&ID_KP_CODE_SIGNING) {
-        return Err(Error::Verification(
-            "ExtendedKeyUsage extension does not contain codeSigning".to_string(),
-        ));
-    }
-
-    Ok(())
-}