fix: correct validation of OCSP signature

sdk/Cargo.toml

@@ -176,7 +176,7 @@ uuid = { version = "1.18.0", features = ["serde", "v4"] }
 web-time = "1.1"
 x509-parser = "0.18.0"
 zeroize = { version = "1.8", features = ["zeroize_derive"] }
-zip = { version = "7.4.0", default-features = false }
+zip = { version = "8.1.0", default-features = false }
 
 # Use the asm feature of sha2 on aarch64 macOS for better performance. This nearly
 # halves hashing time for large assets (> 50gb mp4, for example).

sdk/src/crypto/cose/certificate_trust_policy.rs

@@ -351,6 +351,11 @@ impl CertificateTrustPolicy {
         }
     }
 
+    /// Remove the current EKUs
+    pub fn clear_ekus(&mut self) {
+        self.additional_ekus.clear();
+    }
+
     /// Set whether we only want to use system trust_achors and ignores user_anchors,
     /// returns last trust_anchors_value
     pub fn set_trust_anchors_only(&mut self, trust_anchors_only: bool) -> bool {

sdk/src/crypto/cose/ocsp.rs

@@ -28,9 +28,14 @@ use crate::{
     log_item,
     settings::Settings,
     status_tracker::StatusTracker,
-    validation_status::{self, SIGNING_CREDENTIAL_NOT_REVOKED, SIGNING_CREDENTIAL_REVOKED},
+    validation_status::{
+        self, SIGNING_CREDENTIAL_NOT_REVOKED, SIGNING_CREDENTIAL_OCSP_INACCESSIBLE,
+        SIGNING_CREDENTIAL_REVOKED,
+    },
 };
 
+const OCSP_OID_STR: &str = "1.3.6.1.5.5.7.3.9";
+
 /// Given a COSE signature, extract the OCSP data and validate the status of
 /// that report.
 #[async_generic(async_signature(
@@ -90,14 +95,15 @@ pub fn check_ocsp_status(
 
     match get_ocsp_der(sign1) {
         Some(ocsp_response_der) => {
-            if _sync {
+            let mut ocsp_log = StatusTracker::default();
+            let result = if _sync {
                 check_stapled_ocsp_response(
                     sign1,
                     &ocsp_response_der,
                     data,
                     ctp,
                     tst_info,
-                    validation_log,
+                    &mut ocsp_log,
                     context.settings(),
                 )
             } else {
@@ -107,11 +113,49 @@ pub fn check_ocsp_status(
                     data,
                     ctp,
                     tst_info,
-                    validation_log,
+                    &mut ocsp_log,
                     context.settings(),
                 )
                 .await
+            };
+
+            // we only care about OCSP value log info the result is OK
+            if let Ok(ocsp_response) = result {
+                if ocsp_log.has_status(validation_status::SIGNING_CREDENTIAL_REVOKED) {
+                    log_item!(
+                        "",
+                        format!(
+                            "signing cert revoked: {}",
+                            ocsp_response.certificate_serial_num
+                        ),
+                        "check_ocsp_status"
+                    )
+                    .validation_status(SIGNING_CREDENTIAL_REVOKED)
+                    .informational(validation_log);
+
+                    return Err(CoseError::CertificateTrustError(
+                        CertificateTrustError::CertificateNotTrusted,
+                    ));
+                }
+
+                // If certificate is confirmed not revoked, return success
+                if ocsp_log.has_status(validation_status::SIGNING_CREDENTIAL_NOT_REVOKED) {
+                    log_item!(
+                        "",
+                        format!(
+                            "signing cert not revoked: {}",
+                            ocsp_response.certificate_serial_num
+                        ),
+                        "check_ocsp_status"
+                    )
+                    .validation_status(SIGNING_CREDENTIAL_NOT_REVOKED)
+                    .informational(validation_log);
+
+                    return Ok(ocsp_response);
+                }
             }
+            // errors mean we don't interpret the value
+            Ok(OcspResponse::default())
         }
 
         None => match fetch_policy {
@@ -244,6 +288,7 @@ fn process_ocsp_responses(
             }
         }
     }
+
     Ok(OcspResponse::default())
 }
 
@@ -315,20 +360,36 @@ fn check_stapled_ocsp_response(
     };
 
     // If we get a valid response, validate the certs.
-    if ocsp_data.revoked_at.is_none() {
-        if let Some(ocsp_certs) = &ocsp_data.ocsp_certs {
-            // if the OCSP signing cert cannot be validated do not use this response
-            if check_end_entity_certificate_profile(
+    if let Some(ocsp_certs) = &ocsp_data.ocsp_certs {
+        // make sure this is an OCSP signing EKU
+        let mut new_ctp = ctp.clone();
+        new_ctp.clear_ekus();
+        new_ctp.add_valid_ekus(OCSP_OID_STR.as_bytes()); // ocsp signing EKU
+        if check_end_entity_certificate_profile(
+            &ocsp_certs[0],
+            &new_ctp,
+            validation_log,
+            tst_info.as_ref(),
+        )
+        .is_err()
+        {
+            return Ok(OcspResponse::default());
+        }
+
+        // validate the trust
+        if new_ctp
+            .check_certificate_trust(
+                ocsp_certs,
                 &ocsp_certs[0],
-                ctp,
-                &mut current_validation_log,
-                tst_info.as_ref(),
+                signing_time.map(|t| t.timestamp()),
             )
             .is_err()
-            {
-                return Ok(OcspResponse::default());
-            }
+        {
+            return Ok(OcspResponse::default());
         }
+    } else {
+        // we cannot validate the OCSP response was signed by a valid authorized responder so treat as unknown
+        return Ok(OcspResponse::default());
     }
     // only append usable OCSP responses to validation_log
     validation_log.append(&current_validation_log);
@@ -361,6 +422,14 @@ pub(crate) fn fetch_and_check_ocsp_response(
     };
 
     let Some(ocsp_response_der) = ocsp_der else {
+        log_item!(
+            "",
+            "signing cert not fetched".to_string(),
+            "fetch_and_check_ocsp_response"
+        )
+        .validation_status(SIGNING_CREDENTIAL_OCSP_INACCESSIBLE)
+        .informational(validation_log);
+
         return Ok(OcspResponse::default());
     };
 
@@ -387,10 +456,22 @@ pub(crate) fn fetch_and_check_ocsp_response(
         };
 
     // If we get a valid response validate the certs.
-    if ocsp_data.revoked_at.is_none() {
-        if let Some(ocsp_certs) = &ocsp_data.ocsp_certs {
-            check_end_entity_certificate_profile(&ocsp_certs[0], ctp, validation_log, None)?;
+    if let Some(ocsp_certs) = &ocsp_data.ocsp_certs {
+        // make sure this is an OCSP signing EKU
+        let mut new_ctp = ctp.clone();
+        new_ctp.clear_ekus();
+        new_ctp.add_valid_ekus(OCSP_OID_STR.as_bytes()); // ocsp signing EKU
+
+        if check_end_entity_certificate_profile(&ocsp_certs[0], &new_ctp, validation_log, None)
+            .is_err()
+        {
+            return Ok(OcspResponse::default());
         }
+
+        // no need to check trust here, that is checked during validation
+    } else {
+        // OCSP response must be signed by and the cert chain provided
+        return Ok(OcspResponse::default());
     }
 
     Ok(ocsp_data)

sdk/src/crypto/ocsp/mod.rs

@@ -13,13 +13,17 @@
 
 //! Tools for working with OCSP responses.
 
+use std::str::FromStr;
+
 use chrono::{DateTime, NaiveDateTime, Utc};
 use rasn_ocsp::{BasicOcspResponse, CertStatus, OcspResponseStatus};
 use rasn_pkix::CrlReason;
 use thiserror::Error;
 
 use crate::{
-    crypto::internal::time, log_item, status_tracker::StatusTracker,
+    crypto::{internal::time, raw_signature::RawSignatureValidationError},
+    log_item,
+    status_tracker::StatusTracker,
     validation_results::validation_codes,
 };
 
@@ -102,9 +106,48 @@ impl OcspResponse {
                 cert_der_vec.push(cert_der);
             }
 
-            if output.ocsp_certs.is_none() {
+            // make sure the certificate was correctly signed
+
+            // alg used for signature
+            let Ok(sig_alg) =
+                bcder::Oid::from_str(&basic_response.signature_algorithm.algorithm.to_string())
+            else {
+                return Ok(output);
+            };
+
+            let Some(hash_alg) =
+                hash_alg_for_sig_alg(&basic_response.signature_algorithm.algorithm)
+            else {
+                return Ok(output);
+            };
+
+            // grab signature value.
+            let sig_val = bcder::OctetString::new(bytes::Bytes::copy_from_slice(
+                basic_response.signature.as_raw_slice(),
+            ));
+
+            // grab the to be signed data
+            let Ok(tbs) = rasn::der::encode(&basic_response.tbs_response_data) else {
+                return Ok(output);
+            };
+
+            // grab the signing key
+            let Ok(signing_key_der) =
+                rasn::der::encode(&ocsp_certs[0].tbs_certificate.subject_public_key_info)
+            else {
+                return Ok(output);
+            };
+
+            // if not valid we will not add the cert to list to be checked for trust later
+            if validate_ocsp_sig(&sig_alg, &hash_alg, &sig_val, &tbs, &signing_key_der).is_ok() {
                 output.ocsp_certs = Some(cert_der_vec);
+            } else {
+                // signature failed so don't use
+                return Ok(OcspResponse::default());
             }
+        } else {
+            // we cannot validate the OCSP response signature, so treat as unknown
+            return Ok(OcspResponse::default());
         }
 
         for single_response in &response_data.responses {
@@ -293,6 +336,38 @@ impl OcspResponse {
     }
 }
 
+fn validate_ocsp_sig(
+    sig_alg: &bcder::Oid,
+    hash_alg: &bcder::Oid,
+    sig_val: &bcder::OctetString,
+    tbs: &[u8],
+    signing_key_der: &[u8],
+) -> Result<(), RawSignatureValidationError> {
+    if let Some(validator) =
+        crate::crypto::raw_signature::validator_for_sig_and_hash_algs(sig_alg, hash_alg)
+    {
+        validator
+            .validate(&sig_val.to_bytes(), tbs, signing_key_der)
+            .map_err(|e| RawSignatureValidationError::CryptoLibraryError(e.to_string()))
+    } else {
+        Err(RawSignatureValidationError::UnsupportedAlgorithm)
+    }
+}
+
+/// Return the hash algorithm oid for the given signature algorithm.
+fn hash_alg_for_sig_alg(sig_alg: &rasn::types::ObjectIdentifier) -> Option<bcder::Oid> {
+    match sig_alg.to_string().as_ref() {
+        "1.2.840.10045.4.3.2" => Some(bcder::Oid::from_str("2.16.840.1.101.3.4.2.1").ok()?),
+        "1.2.840.10045.4.3.3" => Some(bcder::Oid::from_str("2.16.840.1.101.3.4.2.2").ok()?),
+        "1.2.840.10045.4.3.4" => Some(bcder::Oid::from_str("2.16.840.1.101.3.4.2.3").ok()?),
+        "1.2.840.113549.1.1.11" => Some(bcder::Oid::from_str("2.16.840.1.101.3.4.2.1").ok()?),
+        "1.2.840.113549.1.1.12" => Some(bcder::Oid::from_str("2.16.840.1.101.3.4.2.2").ok()?),
+        "1.2.840.113549.1.1.13" => Some(bcder::Oid::from_str("2.16.840.1.101.3.4.2.3").ok()?),
+        "1.3.101.112" => Some(bcder::Oid::from_str("2.16.840.1.101.3.4.2.3").ok()?),
+        _ => None,
+    }
+}
+
 /// Describes errors that can be identified when parsing an OCSP response.
 #[derive(Debug, Eq, Error, PartialEq)]
 #[allow(unused)] // InvalidSystemTime may not exist on all platforms.

sdk/src/crypto/raw_signature/oids.rs

@@ -41,6 +41,7 @@ pub(crate) const SECP384R1_OID: Oid<'static> = oid!(1.3.132 .0 .34);
 pub(crate) const PRIME256V1_OID: Oid<'static> = oid!(1.2.840 .10045 .3 .1 .7);
 
 pub(crate) const ED25519_OID: Oid<'static> = oid!(1.3.101 .112);
+pub(crate) const ED25519_PUBLICKEY_OID: Oid<'static> = oid!(1.3.101 .110);
 
 // utility function to make using Oid between crates easier
 pub(crate) fn ans1_oid_bcder_oid(asn1_oid: &asn1_rs::Oid) -> Option<bcder::Oid> {

sdk/src/crypto/raw_signature/openssl/validators/mod.rs

@@ -47,6 +47,31 @@ pub(crate) fn validator_for_sig_and_hash_algs<T: AsRef<[u8]>, U: AsRef<[u8]>>(
     sig_alg: &Oid<T>,
     hash_alg: &Oid<U>,
 ) -> Option<Box<dyn RawSignatureValidator>> {
+    // try signature algs first
+    if sig_alg.as_ref() == ECDSA_WITH_SHA256_OID.as_bytes() {
+        return Some(Box::new(EcdsaValidator::Es256));
+    }
+    if sig_alg.as_ref() == ECDSA_WITH_SHA384_OID.as_bytes() {
+        return Some(Box::new(EcdsaValidator::Es384));
+    }
+    if sig_alg.as_ref() == ECDSA_WITH_SHA512_OID.as_bytes() {
+        return Some(Box::new(EcdsaValidator::Es512));
+    }
+    if sig_alg.as_ref() == SHA256_WITH_RSAENCRYPTION_OID.as_bytes() {
+        return Some(Box::new(RsaLegacyValidator::Rsa256));
+    }
+    if sig_alg.as_ref() == SHA384_WITH_RSAENCRYPTION_OID.as_bytes() {
+        return Some(Box::new(RsaLegacyValidator::Rsa384));
+    }
+    if sig_alg.as_ref() == SHA512_WITH_RSAENCRYPTION_OID.as_bytes() {
+        return Some(Box::new(RsaLegacyValidator::Rsa512));
+    }
+    if sig_alg.as_ref() == ED25519_OID.as_bytes() {
+        return Some(Box::new(Ed25519Validator {}));
+    }
+
+    // Test for public key algs next
+
     if sig_alg.as_ref() == RSA_OID.as_bytes() {
         if hash_alg.as_ref() == SHA1_OID.as_bytes() {
             return Some(Box::new(RsaLegacyValidator::Sha1));

sdk/src/crypto/raw_signature/rust_native/validators/mod.rs

@@ -95,6 +95,31 @@ pub(crate) fn validator_for_sig_and_hash_algs<T: AsRef<[u8]>, U: AsRef<[u8]>>(
     sig_alg: &Oid<T>,
     hash_alg: &Oid<U>,
 ) -> Option<Box<dyn RawSignatureValidator>> {
+    // try signature algs first
+    if sig_alg.as_ref() == ECDSA_WITH_SHA256_OID.as_bytes() {
+        return Some(Box::new(EcdsaValidator::Es256));
+    }
+    if sig_alg.as_ref() == ECDSA_WITH_SHA384_OID.as_bytes() {
+        return Some(Box::new(EcdsaValidator::Es384));
+    }
+    if sig_alg.as_ref() == ECDSA_WITH_SHA512_OID.as_bytes() {
+        return Some(Box::new(EcdsaValidator::Es512));
+    }
+    if sig_alg.as_ref() == SHA256_WITH_RSAENCRYPTION_OID.as_bytes() {
+        return Some(Box::new(RsaLegacyValidator::Rsa256));
+    }
+    if sig_alg.as_ref() == SHA384_WITH_RSAENCRYPTION_OID.as_bytes() {
+        return Some(Box::new(RsaLegacyValidator::Rsa384));
+    }
+    if sig_alg.as_ref() == SHA512_WITH_RSAENCRYPTION_OID.as_bytes() {
+        return Some(Box::new(RsaLegacyValidator::Rsa512));
+    }
+    if sig_alg.as_ref() == ED25519_OID.as_bytes() {
+        return Some(Box::new(Ed25519Validator {}));
+    }
+
+    // Test for public key algs next
+
     // Handle legacy RSA.
     if sig_alg.as_ref() == RSA_OID.as_bytes() {
         if hash_alg.as_ref() == SHA256_OID.as_bytes() {