fix: Restore recursive ingredient checks (#1582)

* Make logging results more consistent.
Catch bad manifest labels for V2 and > Claims
Restore recursive check since validation results work has been pushed

* formatting fix

* review changes

* Add a unit test

Author: mauricefisher64

docs/settings.md

@@ -64,7 +64,6 @@ Here's the JSON with all default values:
     "verify_timestamp_trust": true,
     "ocsp_fetch": false,
     "remote_manifest_fetch": true,
-    "check_ingredient_trust": true,
     "skip_ingredient_conflict_resolution": false,
     "strict_v1_validation": false
   },
@@ -282,7 +281,6 @@ The `verify` object specifies verification behavior.
 | `verify.verify_timestamp_trust` | Boolean | Verify time-stamp certificates | true |
 | `verify.ocsp_fetch` | Boolean | Fetch OCSP status during validation | false |
 | `verify.remote_manifest_fetch` | Boolean | Fetch remote manifests | true |
-| `verify.check_ingredient_trust` | Boolean | Verify ingredient certificates | true |
 | `verify.skip_ingredient_conflict_resolution` | Boolean | Skip ingredient conflict resolution | false |
 | `verify.strict_v1_validation` | Boolean | Use strict C2PA v1 validation | false |
 

sdk/src/claim.rs

@@ -65,7 +65,8 @@ use crate::{
         labels::{
             assertion_label_from_uri, box_name_from_uri, manifest_label_from_uri,
             manifest_label_to_parts, to_absolute_uri, to_assertion_uri, to_databox_uri,
-            to_signature_uri, ASSERTIONS, CLAIM, CREDENTIALS, DATABOX, DATABOXES, SIGNATURE,
+            to_manifest_uri, to_signature_uri, ASSERTIONS, CLAIM, CREDENTIALS, DATABOX, DATABOXES,
+            SIGNATURE,
         },
     },
     jumbf_io::get_assetio_handler,
@@ -1861,6 +1862,17 @@ impl Claim {
             .failure(validation_log, Error::ClaimMissingSignatureBox)?;
         }
 
+        // for V2 and greater claims the label must conform
+        if claim.version() > 1 && manifest_label_to_parts(claim.label()).is_none() {
+            log_item!(
+                to_manifest_uri(claim.label()),
+                "claim box label invalid",
+                "verify_claim_async"
+            )
+            .validation_status(validation_status::CLAIM_MALFORMED)
+            .failure(validation_log, Error::ClaimInvalidContent)?;
+        }
+
         let sign1 = parse_cose_sign1(&sig, &data, validation_log)?;
         let certificate_serial_num = get_signing_cert_serial_num(&sign1)?.to_string();
 
@@ -1940,6 +1952,17 @@ impl Claim {
             .failure(validation_log, Error::ClaimMissingSignatureBox)?;
         }
 
+        // for V2 and greater claims the label must conform
+        if claim.version() > 1 && manifest_label_to_parts(claim.label()).is_none() {
+            log_item!(
+                to_manifest_uri(claim.label()),
+                "claim box label invalid",
+                "verify_claim"
+            )
+            .validation_status(validation_status::CLAIM_MALFORMED)
+            .failure(validation_log, Error::ClaimInvalidContent)?;
+        }
+
         // If we are validating a claim that has been loaded from a file
         // we need the original data but if we are signing, we generate the data
         // This avoids cloning the data when we are only referencing it.
@@ -4110,6 +4133,14 @@ pub mod tests {
             1,
         );
         assert!(c4.is_err());
+
+        // bad v2
+        let c5 = Claim::new_with_user_guid(
+            "claim_generator",
+            "urn:blahblah:3fad1ead-8ed5-44d0-873b-ea5f58adea82:acme",
+            2,
+        );
+        assert!(c5.is_err());
     }
 
     #[test]

sdk/src/jumbf/labels.rs

@@ -243,6 +243,11 @@ pub(crate) fn manifest_label_to_parts(uri: &str) -> Option<ManifestParts> {
         if parts[0] == "urn" {
             is_v1 = parts[1] == "uuid";
 
+            // if > v1 it must be c2pa namespace
+            if !is_v1 && parts[1] != "c2pa" {
+                return None;
+            }
+
             guid = parts[2].to_owned();
 
             if !is_v1 {

sdk/src/settings/mod.rs

@@ -305,10 +305,7 @@ pub struct Verify {
     /// [`Ingredient`]: crate::Ingredient
     /// [`Builder`]: crate::Builder
     pub remote_manifest_fetch: bool,
-    /// Whether to verify ingredient certificates against the trust lists specific in [`Trust`].
     ///
-    /// The default value is true.
-    pub(crate) check_ingredient_trust: bool,
     /// Whether to skip ingredient conflict resolution when multiple ingredients have the same
     /// manifest identifier. This settings is only applicable for C2PA v2 validation.
     ///
@@ -332,7 +329,6 @@ impl Default for Verify {
             verify_timestamp_trust: !cfg!(test), // verify timestamp trust unless in test mode
             ocsp_fetch: false,
             remote_manifest_fetch: true,
-            check_ingredient_trust: true,
             skip_ingredient_conflict_resolution: false,
             strict_v1_validation: false,
         }
@@ -855,7 +851,6 @@ pub mod tests {
             verify_trust = true
             ocsp_fetch = false
             remote_manifest_fetch = true
-            check_ingredient_trust = true
             skip_ingredient_conflict_resolution = false
             strict_v1_validation = false
         }

sdk/src/store.rs

@@ -1669,14 +1669,11 @@ impl Store {
                                 "ingredient hash does not match found ingredient".to_string(),
                             ),
                         )?;
-                        return Err(Error::HashMismatch(
-                            "ingredient hash does not match found ingredient".to_string(),
-                        )); // hard stop regardless of StatusTracker mode
                     }
 
-                    // if manifest hash did not match and this is a V2 or greater claim then we
+                    // if manifest hash did not match because of redaction and this is a V2 or greater claim then we
                     // must try the signature validation method before proceeding
-                    if !manifests_match && ingredient_version > 1 {
+                    if !manifests_match && has_redactions && ingredient_version > 1 {
                         let claim_signature =
                             ingredient_assertion.signature().ok_or_else(|| {
                                 log_item!(
@@ -1724,10 +1721,6 @@ impl Store {
                                     "ingredient claimSignature mismatch".to_string(),
                                 ),
                             )?;
-                            return Err(Error::HashMismatch(
-                                "ingredient signature box hash does not match found ingredient"
-                                    .to_string(),
-                            )); // hard stop regardless of StatusTracker mode
                         }
                     }
 
@@ -1737,19 +1730,16 @@ impl Store {
                         Claim::verify_hash_binding(ingredient, asset_data, svi, validation_log)?;
                     }
 
-                    // if manifest hash did not match we continue on to do a full claim validation
-                    if !manifests_match {
-                        Claim::verify_claim(
-                            ingredient,
-                            asset_data,
-                            svi,
-                            check_ingredient_trust,
-                            &store.ctp,
-                            validation_log,
-                            http_resolver,
-                            settings,
-                        )?;
-                    }
+                    Claim::verify_claim(
+                        ingredient,
+                        asset_data,
+                        svi,
+                        check_ingredient_trust,
+                        &store.ctp,
+                        validation_log,
+                        http_resolver,
+                        settings,
+                    )?;
 
                     // recurse nested ingredients
                     Store::ingredient_checks(
@@ -1856,7 +1846,7 @@ impl Store {
 
                     // allow the extra ingredient trust checks
                     // these checks are to prevent the trust spoofing
-                    let check_ingredient_trust = settings.verify.check_ingredient_trust;
+                    let check_ingredient_trust = settings.verify.verify_trust;
 
                     // get the 1.1-1.2 box hash
                     let ingredient_hashes = store.get_manifest_box_hashes(ingredient);
@@ -1903,14 +1893,11 @@ impl Store {
                                 "ingredient hash does not match found ingredient".to_string(),
                             ),
                         )?;
-                        return Err(Error::HashMismatch(
-                            "ingredient hash does not match found ingredient".to_string(),
-                        )); // hard stop regardless of StatusTracker mode
                     }
 
                     // if manifest hash did not match and this is a V2 or greater claim then we
                     // must try the signature validation method before proceeding
-                    if !manifests_match && ingredient_version > 1 {
+                    if !manifests_match && has_redactions && ingredient_version > 1 {
                         let claim_signature =
                             ingredient_assertion.signature().ok_or_else(|| {
                                 log_item!(
@@ -1958,10 +1945,6 @@ impl Store {
                                     "ingredient claimSignature mismatch".to_string(),
                                 ),
                             )?;
-                            return Err(Error::HashMismatch(
-                                "ingredient signature box hash does not match found ingredient"
-                                    .to_string(),
-                            )); // hard stop regardless of StatusTracker mode
                         }
                     }
 
@@ -1971,20 +1954,17 @@ impl Store {
                         Claim::verify_hash_binding(ingredient, asset_data, svi, validation_log)?;
                     }
 
-                    // if manifest hash did not match we continue on to do a full claim validation
-                    if !manifests_match {
-                        Claim::verify_claim_async(
-                            ingredient,
-                            asset_data,
-                            svi,
-                            check_ingredient_trust,
-                            &store.ctp,
-                            validation_log,
-                            http_resolver,
-                            settings,
-                        )
-                        .await?;
-                    }
+                    Claim::verify_claim_async(
+                        ingredient,
+                        asset_data,
+                        svi,
+                        check_ingredient_trust,
+                        &store.ctp,
+                        validation_log,
+                        http_resolver,
+                        settings,
+                    )
+                    .await?;
 
                     // recurse nested ingredients
                     Box::pin(Store::ingredient_checks_async(

sdk/tests/integration.rs

@@ -349,44 +349,50 @@ mod integration_1 {
         Ok(())
     }
 
-    #[test]
-    #[cfg(feature = "file_io")]
-    fn test_certificate_status() -> Result<()> {
-        use c2pa::ValidationState;
-
-        Settings::from_toml(include_str!("../tests/fixtures/test_settings.toml"))?;
-        Settings::from_toml(
-            &toml::toml! {
-                [builder]
-                certificate_status_fetch = "all"
-                certificate_status_should_override = true
-            }
-            .to_string(),
-        )?;
-
-        // set up parent and destination paths
-        let temp_dir = tempdirectory()?;
-        let output_path = temp_dir.path().join("test_file.jpg");
-        let parent_path = fixture_path("ocsp.jpg");
-
-        // create a new Manifest
-        let mut builder = Builder::new();
-        builder.set_intent(c2pa::BuilderIntent::Update);
-
-        // sign and embed into the target file
-        let signer = Settings::signer()?;
-        builder.sign_file(signer.as_ref(), &parent_path, &output_path)?;
-
-        // std::fs::copy(&output_path, "cert_status.jpg")?;
-
-        // read our new file with embedded manifest
-        let reader = Reader::from_file(&output_path)?;
-        let reader_json = reader.json();
-        //println!("{reader}");
-        // ensure certificate status assertion was created
-        //assert!(reader_json.contains(r#"label": "c2pa.certificate-status"#));
-        assert_eq!(reader.validation_state(), ValidationState::Trusted);
-        assert!(reader_json.contains("signingCredential.ocsp.notRevoked"));
-        Ok(())
-    }
+    /*
+    This test is currently invalid.  It is using C2PA 2.2 assertions in 1.4 claims
+    This needs to be rewritten in a way that does not require network calls, or mock
+    them correctly.  Tracking issue: https://github.com/contentauth/c2pa-rs/issues/1581
+
+        #[test]
+        #[cfg(feature = "file_io")]
+        fn test_certificate_status() -> Result<()> {
+            use c2pa::ValidationState;
+
+            Settings::from_toml(include_str!("../tests/fixtures/test_settings.toml"))?;
+            Settings::from_toml(
+                &toml::toml! {
+                    [builder]
+                    certificate_status_fetch = "all"
+                    certificate_status_should_override = true
+                }
+                .to_string(),
+            )?;
+
+            // set up parent and destination paths
+            let temp_dir = tempdirectory()?;
+            let output_path = temp_dir.path().join("test_file.jpg");
+            let parent_path = fixture_path("ocsp.jpg");
+
+            // create a new Manifest
+            let mut builder = Builder::new();
+            builder.set_intent(c2pa::BuilderIntent::Update);
+
+            // sign and embed into the target file
+            let signer = Settings::signer()?;
+            builder.sign_file(signer.as_ref(), &parent_path, &output_path)?;
+
+            // std::fs::copy(&output_path, "cert_status.jpg")?;
+
+            // read our new file with embedded manifest
+            let reader = Reader::from_file(&output_path)?;
+            let reader_json = reader.json();
+            //println!("{reader}");
+            // ensure certificate status assertion was created
+            //assert!(reader_json.contains(r#"label": "c2pa.certificate-status"#));
+            assert_eq!(reader.validation_state(), ValidationState::Trusted);
+            assert!(reader_json.contains("signingCredential.ocsp.notRevoked"));
+            Ok(())
+        }
+    */
 }