Skip to content

Commit

Permalink
feat: Simplify StatusTracker interface (#937)
Browse files Browse the repository at this point in the history
Having `StatusTracker` as a non-object-safe trait made it impossible to use in further trait APIs.

See early draft of #936 for an example of this problem.

This PR changes `StatusTracker` to use an enum to describe the only two implementations of `StatusTracker` that have ever existed.

Remove the `take_errors` API in favor of a new `has_any_error` function.
  • Loading branch information
scouten-adobe authored Feb 26, 2025
1 parent 1cc1066 commit 16b2a31
Show file tree
Hide file tree
Showing 30 changed files with 410 additions and 464 deletions.
4 changes: 2 additions & 2 deletions cawg_identity/src/x509/x509_signature_verifier.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use c2pa_crypto::{
cose::{parse_cose_sign1, CertificateInfo, CoseError, Verifier},
raw_signature::RawSignatureValidationError,
};
use c2pa_status_tracker::DetailedStatusTracker;
use c2pa_status_tracker::StatusTracker;
use coset::CoseSign1;
use serde::Serialize;

Expand Down Expand Up @@ -58,7 +58,7 @@ impl SignatureVerifier for X509SignatureVerifier {
let verifier = Verifier::IgnoreProfileAndTrustPolicy;

// TO DO: Figure out how to provide a validation log.
let mut validation_log = DetailedStatusTracker::default();
let mut validation_log = StatusTracker::default();

let cose_sign1 = parse_cose_sign1(signature, &signer_payload_cbor, &mut validation_log)?;

Expand Down
2 changes: 1 addition & 1 deletion internal/crypto/src/cose/certificate_profile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ use crate::{asn1::rfc3161::TstInfo, cose::CertificateTrustPolicy};
pub fn check_certificate_profile(
certificate_der: &[u8],
ctp: &CertificateTrustPolicy,
validation_log: &mut impl StatusTracker,
validation_log: &mut StatusTracker,
_tst_info_opt: Option<&TstInfo>,
) -> Result<(), CertificateProfileError> {
let (_rem, signcert) = X509Certificate::from_der(certificate_der).map_err(|_err| {
Expand Down
6 changes: 3 additions & 3 deletions internal/crypto/src/cose/ocsp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ pub fn check_ocsp_status(
data: &[u8],
fetch_policy: OcspFetchPolicy,
ctp: &CertificateTrustPolicy,
validation_log: &mut impl StatusTracker,
validation_log: &mut StatusTracker,
) -> Result<OcspResponse, CoseError> {
match get_ocsp_der(sign1) {
Some(ocsp_response_der) => {
Expand Down Expand Up @@ -76,7 +76,7 @@ fn check_stapled_ocsp_response(
ocsp_response_der: &[u8],
data: &[u8],
ctp: &CertificateTrustPolicy,
validation_log: &mut impl StatusTracker,
validation_log: &mut StatusTracker,
) -> Result<OcspResponse, CoseError> {
let time_stamp_info = if _sync {
validate_cose_tst_info(sign1, data)
Expand Down Expand Up @@ -112,7 +112,7 @@ fn fetch_and_check_ocsp_response(
sign1: &CoseSign1,
data: &[u8],
ctp: &CertificateTrustPolicy,
validation_log: &mut impl StatusTracker,
validation_log: &mut StatusTracker,
) -> Result<OcspResponse, CoseError> {
#[cfg(target_arch = "wasm32")]
{
Expand Down
2 changes: 1 addition & 1 deletion internal/crypto/src/cose/sign1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ use crate::{
pub fn parse_cose_sign1(
cose_bytes: &[u8],
data: &[u8],
validation_log: &mut impl StatusTracker,
validation_log: &mut StatusTracker,
) -> Result<CoseSign1, CoseError> {
let mut sign1 = <coset::CoseSign1 as TaggedCborSerializable>::from_tagged_slice(cose_bytes)
.map_err(|coset_error| {
Expand Down
6 changes: 3 additions & 3 deletions internal/crypto/src/cose/verifier.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ impl Verifier<'_> {
cose_sign1: &[u8],
data: &[u8],
additional_data: &[u8],
validation_log: &mut impl StatusTracker,
validation_log: &mut StatusTracker,
) -> Result<CertificateInfo, CoseError> {
let mut sign1 = parse_cose_sign1(cose_sign1, data, validation_log)?;

Expand Down Expand Up @@ -184,7 +184,7 @@ impl Verifier<'_> {
&self,
sign1: &CoseSign1,
tst_info_res: &Result<TstInfo, CoseError>,
validation_log: &mut impl StatusTracker,
validation_log: &mut StatusTracker,
) -> Result<(), CoseError> {
let ctp = match self {
Self::VerifyTrustPolicy(ctp) => *ctp,
Expand Down Expand Up @@ -254,7 +254,7 @@ impl Verifier<'_> {
&self,
sign1: &CoseSign1,
tst_info_res: &Result<TstInfo, CoseError>,
validation_log: &mut impl StatusTracker,
validation_log: &mut StatusTracker,
) -> Result<(), CoseError> {
// IMPORTANT: This function assumes that verify_profile has already been called.

Expand Down
6 changes: 3 additions & 3 deletions internal/crypto/src/ocsp/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

//! Tools for working with OCSP responses.
use c2pa_status_tracker::{log_item, validation_codes, DetailedStatusTracker, StatusTracker};
use c2pa_status_tracker::{log_item, validation_codes, StatusTracker};
use chrono::{DateTime, NaiveDateTime, Utc};
use rasn_ocsp::{BasicOcspResponse, CertStatus, OcspResponseStatus};
use rasn_pkix::CrlReason;
Expand Down Expand Up @@ -53,7 +53,7 @@ impl OcspResponse {
pub(crate) fn from_der_checked(
der: &[u8],
signing_time: Option<DateTime<Utc>>,
validation_log: &mut impl StatusTracker,
validation_log: &mut StatusTracker,
) -> Result<Self, OcspError> {
let mut output = OcspResponse {
ocsp_der: der.to_vec(),
Expand All @@ -79,7 +79,7 @@ impl OcspResponse {
return Ok(output);
};

let mut internal_validation_log = DetailedStatusTracker::default();
let mut internal_validation_log = StatusTracker::default();
let response_data = &basic_response.tbs_response_data;

// get OCSP cert chain if available
Expand Down
8 changes: 3 additions & 5 deletions internal/crypto/src/tests/cose/certificate_profile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,7 @@
// specific language governing permissions and limitations under
// each license.

use c2pa_status_tracker::{
validation_codes::SIGNING_CREDENTIAL_EXPIRED, DetailedStatusTracker, StatusTracker,
};
use c2pa_status_tracker::{validation_codes::SIGNING_CREDENTIAL_EXPIRED, StatusTracker};
#[cfg(all(target_arch = "wasm32", not(target_os = "wasi")))]
use wasm_bindgen_test::wasm_bindgen_test;
use x509_parser::pem::Pem;
Expand All @@ -27,7 +25,7 @@ use crate::cose::{check_certificate_profile, CertificateTrustPolicy};
)]
fn expired_cert() {
let ctp = CertificateTrustPolicy::default();
let mut validation_log = DetailedStatusTracker::default();
let mut validation_log = StatusTracker::default();

let cert_der = x509_der_from_pem(include_bytes!(
"../fixtures/cose/rsa-pss256_key-expired.pub"
Expand All @@ -51,7 +49,7 @@ fn expired_cert() {
fn cert_algorithms() {
let ctp = CertificateTrustPolicy::default();

let mut validation_log = DetailedStatusTracker::default();
let mut validation_log = StatusTracker::default();

let es256_cert = x509_der_from_pem(include_bytes!("../fixtures/raw_signature/es256.pub"));
let es384_cert = x509_der_from_pem(include_bytes!("../fixtures/raw_signature/es384.pub"));
Expand Down
10 changes: 4 additions & 6 deletions internal/crypto/src/tests/ocsp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
// specific language governing permissions and limitations under
// each license.

use c2pa_status_tracker::DetailedStatusTracker;
use c2pa_status_tracker::StatusTracker;
use chrono::{TimeZone, Utc};
#[cfg(all(target_arch = "wasm32", not(target_os = "wasi")))]
use wasm_bindgen_test::wasm_bindgen_test;
Expand All @@ -26,7 +26,7 @@ use crate::ocsp::OcspResponse;
fn good() {
let rsp_data = include_bytes!("fixtures/ocsp/good.data");

let mut validation_log = DetailedStatusTracker::default();
let mut validation_log = StatusTracker::default();

let test_time = Utc.with_ymd_and_hms(2023, 2, 1, 8, 0, 0).unwrap();

Expand All @@ -45,15 +45,13 @@ fn good() {
fn revoked() {
let rsp_data = include_bytes!("fixtures/ocsp/revoked.data");

let mut validation_log = DetailedStatusTracker::default();
let mut validation_log = StatusTracker::default();

let test_time = Utc.with_ymd_and_hms(2024, 2, 1, 8, 0, 0).unwrap();

let ocsp_data =
OcspResponse::from_der_checked(rsp_data, Some(test_time), &mut validation_log).unwrap();

let errors = validation_log.take_errors();

assert!(ocsp_data.revoked_at.is_some());
assert!(!errors.is_empty());
assert!(validation_log.has_any_error());
}
4 changes: 1 addition & 3 deletions internal/status-tracker/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,7 @@ mod log;
pub use log::{LogItem, LogKind};

mod status_tracker;
pub use status_tracker::{
detailed::DetailedStatusTracker, one_shot::OneShotStatusTracker, StatusTracker,
};
pub use status_tracker::{ErrorBehavior, StatusTracker};

pub mod validation_codes;

Expand Down
8 changes: 4 additions & 4 deletions internal/status-tracker/src/log.rs
Original file line number Diff line number Diff line change
Expand Up @@ -177,14 +177,14 @@ impl LogItem {

/// Set the log item kind to [`LogKind::Success`] and add it to the
/// [`StatusTracker`].
pub fn success(mut self, tracker: &mut impl StatusTracker) {
pub fn success(mut self, tracker: &mut StatusTracker) {
self.kind = LogKind::Success;
tracker.add_non_error(self);
}

/// Set the log item kind to [`LogKind::Informational`] and add it to the
/// [`StatusTracker`].
pub fn informational(mut self, tracker: &mut impl StatusTracker) {
pub fn informational(mut self, tracker: &mut StatusTracker) {
self.kind = LogKind::Informational;
tracker.add_non_error(self);
}
Expand All @@ -197,7 +197,7 @@ impl LogItem {
///
/// If the implementation is configured to aggregate all log messages, this
/// function will return `Ok(())`.
pub fn failure<E: Debug>(mut self, tracker: &mut impl StatusTracker, err: E) -> Result<(), E> {
pub fn failure<E: Debug>(mut self, tracker: &mut StatusTracker, err: E) -> Result<(), E> {
self.kind = LogKind::Failure;
self.err_val = Some(format!("{err:?}").into());
tracker.add_error(self, err)
Expand All @@ -208,7 +208,7 @@ impl LogItem {
///
/// Does not return a [`Result`] and thus ignores the [`StatusTracker`]
/// error-handling configuration.
pub fn failure_no_throw<E: Debug>(mut self, tracker: &mut impl StatusTracker, err: E) {
pub fn failure_no_throw<E: Debug>(mut self, tracker: &mut StatusTracker, err: E) {
self.kind = LogKind::Failure;
self.err_val = Some(format!("{err:?}").into());

Expand Down
156 changes: 156 additions & 0 deletions internal/status-tracker/src/status_tracker.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,156 @@
// Copyright 2022 Adobe. All rights reserved.
// This file is licensed to you under the Apache License,
// Version 2.0 (http://www.apache.org/licenses/LICENSE-2.0)
// or the MIT license (http://opensource.org/licenses/MIT),
// at your option.

// Unless required by applicable law or agreed to in writing,
// this software is distributed on an "AS IS" BASIS, WITHOUT
// WARRANTIES OR REPRESENTATIONS OF ANY KIND, either express or
// implied. See the LICENSE-MIT and LICENSE-APACHE files for the
// specific language governing permissions and limitations under
// each license.

use std::{fmt::Debug, iter::Iterator};

use crate::LogItem;

/// A `StatusTracker` is used in the validation logic of c2pa-rs and
/// related crates to control error-handling behavior and optionally
/// aggregate log messages as they are generated.
#[derive(Debug, Default)]
pub struct StatusTracker {
error_behavior: ErrorBehavior,
logged_items: Vec<LogItem>,
ingredient_uris: Vec<String>,
}

impl StatusTracker {
/// Returns a [`StatusTracker`] with the specified [`ErrorBehavior`].
pub fn with_error_behavior(error_behavior: ErrorBehavior) -> Self {
Self {
error_behavior,
logged_items: vec![],
ingredient_uris: vec![],
}
}

/// Returns the current list of validation log items.
pub fn logged_items(&self) -> &[LogItem] {
&self.logged_items
}

/// Returns a list of validation log items that can be mutated if needed.
pub fn logged_items_mut(&mut self) -> &mut [LogItem] {
&mut self.logged_items
}

/// Appends the contents of another [`StatusTracker`] to this list of
/// validation log items.
pub fn append(&mut self, other: &StatusTracker) {
for log_item in other.logged_items() {
self.add_non_error(log_item.clone());
}
}

/// Adds a non-error [`LogItem`] to this status tracker.
///
/// Primarily intended for use by [`LogItem::success()`]
/// or [`LogItem::informational()`].
pub fn add_non_error(&mut self, mut log_item: LogItem) {
if let Some(ingredient_uri) = self.ingredient_uris.last() {
log_item.ingredient_uri = Some(ingredient_uri.to_string().into());
}
self.logged_items.push(log_item);
}

/// Adds an error-case [`LogItem`] to this status tracker.
///
/// Will return `Err(err)` if configured to stop immediately on errors or
/// `Ok(())` if configured to continue on errors. _(See [`ErrorBehavior`].)_
///
/// Primarily intended for use by [`LogItem::failure()`].
pub fn add_error<E>(&mut self, mut log_item: LogItem, err: E) -> Result<(), E> {
if let Some(ingredient_uri) = self.ingredient_uris.last() {
log_item.ingredient_uri = Some(ingredient_uri.to_string().into());
}

self.logged_items.push(log_item);

match self.error_behavior {
ErrorBehavior::StopOnFirstError => Err(err),
ErrorBehavior::ContinueWhenPossible => Ok(()),
}
}

/// Returns the [`LogItem`]s that have error conditions (`err_val` is
/// populated).
///
/// Removes matching items from the list of log items.
pub fn filter_errors(&self) -> impl Iterator<Item = &LogItem> {
self.logged_items()
.iter()
.filter(|item| item.err_val.is_some())
}

/// Returns `true` if the validation log contains a specific C2PA status
/// code.
pub fn has_status(&self, val: &str) -> bool {
self.logged_items().iter().any(|vi| {
if let Some(vs) = &vi.validation_status {
vs == val
} else {
false
}
})
}

/// Returns `true` if the validation log contains a specific error.
pub fn has_error<E: Debug>(&self, err: E) -> bool {
let err_type = format!("{:?}", &err);
self.logged_items().iter().any(|vi| {
if let Some(e) = &vi.err_val {
e == &err_type
} else {
false
}
})
}

/// Returns `true` if the validation log contains any error.
pub fn has_any_error(&self) -> bool {
self.filter_errors().next().is_some()
}

/// Keeps track of the current ingredient URI, if any.
///
/// The current URI may be added to any log items that are created.
pub fn push_ingredient_uri<S: Into<String>>(&mut self, uri: S) {
self.ingredient_uris.push(uri.into());
}

/// Removes the current ingredient URI, if any.
pub fn pop_ingredient_uri(&mut self) -> Option<String> {
self.ingredient_uris.pop()
}
}

/// `ErrorBehavior` configures the behavior of [`StatusTracker`] when its
/// [`add_error`] function is called.
///
/// [`add_error`]: StatusTracker::add_error
#[derive(Debug, Eq, PartialEq)]
pub enum ErrorBehavior {
/// If an error is encountered, stop validation immediately.
StopOnFirstError,

/// If an error is encountered, log it and continue validation as much as
/// possible.
ContinueWhenPossible,
}

impl Default for ErrorBehavior {
fn default() -> Self {
Self::ContinueWhenPossible
}
}
Loading

0 comments on commit 16b2a31

Please sign in to comment.