Skip to content

Commit

Permalink
Fixes issue where nested assertion-uri-hash errors were being reporte…
Browse files Browse the repository at this point in the history
…d at the active manifest level. (#593)

* Improve reporting of ingredient errors by reporting full paths.
* Fix Dedup of nested assertion errors.
* Add Unit test for hashed-uri errors.
* Add make_images script to generate test case.
* Update and add Reader unit tests.
Co-authored-by: Maurice Fisher <[email protected]>
  • Loading branch information
gpeacock authored Sep 24, 2024
1 parent 91aac1e commit 37ec391
Show file tree
Hide file tree
Showing 5 changed files with 84 additions and 26 deletions.
2 changes: 2 additions & 0 deletions make_test_images/tests.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@
{ "op": "uri", "parent": "CA", "output": "E-uri-CA" },
{ "op": "clm", "parent": "CAICAI", "output": "E-clm-CAICAI" },
{ "op": "make", "ingredients": ["E-sig-CA"], "output": "CIE-sig-CA" },
{ "op": "make", "ingredients": ["E-uri-CA"], "output": "CAE-uri-CA" },
{ "op": "make", "ingredients": ["CAE-uri-CA"], "output": "CACAE-uri-CA" },
{ "op": "uri", "parent": "CIE-sig-CA", "output": "E-uri-CIE-sig-CA" },
{ "op": "make", "parent": "A.jpg", "ingredients": ["C", "A.jpg", "I.jpg", "CA", "CI", "CAI", "CICA"], "output": "CAIAIIICAICIICAIICICA" }
]
Expand Down
32 changes: 16 additions & 16 deletions sdk/src/claim.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ use crate::{
CAICBORAssertionBox, CAIJSONAssertionBox, CAIUUIDAssertionBox, JumbfEmbeddedFileBox,
},
labels::{
box_name_from_uri, manifest_label_from_uri, to_databox_uri, ASSERTIONS, CREDENTIALS,
DATABOX, DATABOXES, SIGNATURE,
box_name_from_uri, manifest_label_from_uri, to_absolute_uri, to_databox_uri,
ASSERTIONS, CREDENTIALS, DATABOX, DATABOXES, SIGNATURE,
},
},
jumbf_io::get_assetio_handler,
Expand Down Expand Up @@ -207,10 +207,6 @@ pub struct Claim {
#[serde(skip_deserializing, skip_serializing)]
ingredients_store: HashMap<String, Vec<Claim>>,

// internal scratch objects
#[serde(skip_deserializing, skip_serializing)]
box_prefix: String, // where in JUMBF hierarchy should this claim exist

#[serde(skip_deserializing, skip_serializing)]
signature_val: Vec<u8>, // the signature of the loaded/saved claim

Expand Down Expand Up @@ -325,7 +321,6 @@ impl Claim {

Claim {
remote_manifest: RemoteManifest::NoRemote,
box_prefix: "self#jumbf".to_string(),
root: jumbf::labels::MANIFEST_STORE.to_string(),
signature_val: Vec::new(),
ingredients_store: HashMap::new(),
Expand Down Expand Up @@ -360,7 +355,6 @@ impl Claim {
pub fn new_with_user_guid<S: Into<String>>(claim_generator: S, user_guid: S) -> Self {
Claim {
remote_manifest: RemoteManifest::NoRemote,
box_prefix: "self#jumbf".to_string(),
root: jumbf::labels::MANIFEST_STORE.to_string(),
signature_val: Vec::new(),
ingredients_store: HashMap::new(),
Expand Down Expand Up @@ -426,7 +420,8 @@ impl Claim {

// Add link to the signature box for this claim.
fn add_signature_box_link(&mut self) {
self.signature = format!("{}={}", self.box_prefix, jumbf::labels::SIGNATURE);
// full path to signature box
self.signature = self.signature_uri();
}

/// set signature of the claim
Expand Down Expand Up @@ -1230,30 +1225,35 @@ impl Claim {
// verify assertion structure comparing hashes from assertion list to contents of assertion store
for assertion in claim.assertions() {
let (label, instance) = Claim::assertion_label_from_link(&assertion.url());
let assertion_absolute_uri = if assertion.is_relative_url() {
to_absolute_uri(claim.label(), &assertion.url())
} else {
assertion.url()
};
match claim.get_claim_assertion(&label, instance) {
// get the assertion if label and hash match
Some(ca) => {
if !vec_compare(ca.hash(), &assertion.hash()) {
let log_item = log_item!(
assertion.url(),
assertion_absolute_uri.clone(),
format!("hash does not match assertion data: {}", assertion.url()),
"verify_internal"
)
.error(Error::HashMismatch(format!(
"Assertion hash failure: {}",
assertion.url()
assertion_absolute_uri.clone(),
)))
.validation_status(validation_status::ASSERTION_HASHEDURI_MISMATCH);
validation_log.log(
log_item,
Some(Error::HashMismatch(format!(
"Assertion hash failure: {}",
assertion.url()
assertion_absolute_uri.clone(),
))),
)?;
} else {
let log_item = log_item!(
assertion.url(),
assertion_absolute_uri.clone(),
format!("hashed uri matched: {}", assertion.url()),
"verify_internal"
)
Expand All @@ -1263,18 +1263,18 @@ impl Claim {
}
None => {
let log_item = log_item!(
assertion.url(),
assertion_absolute_uri.clone(),
format!("cannot find matching assertion: {}", assertion.url()),
"verify_internal"
)
.error(Error::AssertionMissing {
url: assertion.url(),
url: assertion_absolute_uri.clone(),
})
.validation_status(validation_status::ASSERTION_MISSING);
validation_log.log(
log_item,
Some(Error::AssertionMissing {
url: assertion.url(),
url: assertion_absolute_uri.clone(),
}),
)?;
}
Expand Down
32 changes: 32 additions & 0 deletions sdk/src/reader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -270,3 +270,35 @@ impl std::fmt::Debug for Reader {
f.write_str(&report.to_string())
}
}

#[test]
#[cfg(feature = "file_io")]
fn test_reader_from_file_no_manifest() -> Result<()> {
let result = Reader::from_file("tests/fixtures/IMG_0003.jpg");
assert!(matches!(result, Err(Error::JumbfNotFound)));
Ok(())
}

#[test]
#[cfg(feature = "file_io")]
#[allow(clippy::unwrap_used)]
fn test_reader_from_file_validation_err() -> Result<()> {
let reader = Reader::from_file("tests/fixtures/XCA.jpg")?;
assert!(reader.validation_status().is_some());
assert_eq!(
reader.validation_status().unwrap()[0].code(),
crate::validation_status::ASSERTION_DATAHASH_MISMATCH
);
Ok(())
}

#[test]
#[cfg(feature = "file_io")]
/// Test that the reader can validate a file with nested assertion errors
fn test_reader_from_file_nested_errors() -> Result<()> {
let reader = Reader::from_file("tests/fixtures/CACAE-uri-CA.jpg")?;
println!("{reader}");
assert_eq!(reader.validation_status(), None);
assert_eq!(reader.manifest_store.manifests().len(), 3);
Ok(())
}
44 changes: 34 additions & 10 deletions sdk/src/validation_status.rs
Original file line number Diff line number Diff line change
Expand Up @@ -176,33 +176,57 @@ pub fn status_for_store(

// This closure returns true if the URI references the store's active manifest.
let is_active_manifest = |uri: Option<&str>| {
uri.filter(|uri| jumbf::labels::manifest_label_from_uri(uri) == active_manifest)
.is_some()
uri.map_or(false, |uri| {
jumbf::labels::manifest_label_from_uri(uri) == active_manifest
})
};

// Convert any relative manifest urls found in ingredient validation statuses to absolute.
let make_absolute =
|active_manifest: Option<crate::hashed_uri::HashedUri>,
validation_status: Option<Vec<ValidationStatus>>| {
validation_status.map(|mut statuses| {
if let Some(label) = active_manifest
.map(|m| m.url())
.and_then(|uri| jumbf::labels::manifest_label_from_uri(&uri))
{
for status in &mut statuses {
if let Some(url) = &status.url {
if url.starts_with("self#jumbf") {
// Some are just labels (i.e. "Cose_Sign1")
status.url =
Some(dbg!(jumbf::labels::to_absolute_uri(&label, url)));
}
}
}
}
statuses
})
};

// We only need to do the more detailed filtering if there are any status
// reports that reference ingredients.
if statuses
.iter()
.any(|s| !is_active_manifest(s.url.as_deref()))
{
// Collect all the ValidationStatus records from all the ingredients in the store.
let ingredient_statuses: Vec<ValidationStatus> = claim
.ingredient_assertions()
let ingredient_statuses: Vec<ValidationStatus> = store
.claims()
.iter()
.flat_map(|c| c.ingredient_assertions())
.filter_map(|a| Ingredient::from_assertion(a).ok())
.filter_map(|i| i.validation_status)
.flat_map(|x| x.into_iter())
.filter_map(|i| make_absolute(i.c2pa_manifest, i.validation_status))
.flatten()
.collect();

// Filter to only contain the active statuses and nested statuses not found in active.
// Filter statuses to only contain those from the active manifest and those not found in any ingredient.
return statuses
.iter()
.into_iter()
.filter(|s| {
is_active_manifest(s.url.as_deref())
|| !ingredient_statuses.iter().any(|i| s == &i)
|| !ingredient_statuses.iter().any(|i| i == s)
})
.map(|s| s.to_owned())
.collect();
}
}
Expand Down
Binary file added sdk/tests/fixtures/CACAE-uri-CA.jpg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.

0 comments on commit 37ec391

Please sign in to comment.