Clarify skipping via absence of rekor_public_key. Make failure messag…

…es clearer by catching absence early on.

Extended tests to cover all combinations.

Change-Id: Iede72087888b6a1919185582ec5224c5ecb1cee2

oak_attestation_verification/src/endorsement.rs

@@ -29,6 +29,13 @@ use crate::{
 };
 
 /// Verifies the binary endorsement against log entry and public keys.
+///
+/// `endorser_public_key` and `signature` must be present.
+///
+/// If `rekor_public_key` is present, then presence of the log entry is
+/// required and the log entry will be verified. If `rekor_public_key`
+/// is empty (not set), then verification is skipped no matter if the log
+/// entry is present or not.
 pub fn verify_binary_endorsement(
     now_utc_millis: i64,
     endorsement: &[u8],
@@ -37,23 +44,32 @@ pub fn verify_binary_endorsement(
     endorser_public_key: &[u8],
     rekor_public_key: &[u8],
 ) -> anyhow::Result<()> {
-    let statement = parse_endorsement_statement(endorsement)
-        .map_err(|e| anyhow::anyhow!(e))
-        .context("parsing endorsement statement")?;
+    if signature.is_empty() {
+        anyhow::bail!("signature of endorsement is required");
+    }
+    if endorser_public_key.is_empty() {
+        anyhow::bail!("endorser's public key is required");
+    }
+
+    // The signature verification is also part of log entry verification,
+    // so in some cases this check will be dispensable. We verify the
+    // signature nonetheless before parsing the endorsement.
+    verify_signature_raw(signature, endorsement, endorser_public_key)
+        .context("verifying signature")?;
+
+    let statement =
+        parse_endorsement_statement(endorsement).context("parsing endorsement statement")?;
+    verify_endorsement_statement(now_utc_millis, &statement)
+        .context("verifying endorsement statement")?;
 
-    if !log_entry.is_empty() {
-        verify_endorsement_statement(now_utc_millis, &statement)
-            .context("verifying endorsement statement")?;
+    if !rekor_public_key.is_empty() {
+        if log_entry.is_empty() {
+            anyhow::bail!("log entry unavailable but verification was requested");
+        }
         verify_rekor_log_entry(log_entry, rekor_public_key, endorsement)
             .context("verifying rekor log entry")?;
         verify_endorser_public_key(log_entry, endorser_public_key)
             .context("verifying endorser public key")?;
-    } else {
-        if !rekor_public_key.is_empty() {
-            anyhow::bail!("log entry unavailable but verification was requested");
-        }
-        verify_signature_raw(signature, endorsement, endorser_public_key)
-            .context("verifying raw signature")?;
     }
 
     Ok(())

oak_attestation_verification/tests/endorsement_tests.rs

@@ -39,6 +39,12 @@ const REKOR_PUBLIC_KEY_PATH: &str = "testdata/rekor_public_key.pem";
 // Pretend the tests run at this time: 1 March 2024, 12:00 UTC
 const NOW_UTC_MILLIS: i64 = 1709294400000;
 
+// Endorsement statement was invalid on: 28 March 2023, 10:40 UTC
+const TOO_EARLY_UTC_MILLIS: i64 = 1680000000000;
+
+// Endorsement statement was invalid on: 26 March 2025, 14:40 UTC
+const TOO_LATE_UTC_MILLIS: i64 = 1743000000000;
+
 struct TestData {
     endorsement: Vec<u8>,
     signature: Vec<u8>,
@@ -65,14 +71,14 @@ fn load_testdata() -> TestData {
 }
 
 #[test]
-fn test_verify_rekor_signature() {
+fn test_verify_rekor_signature_success() {
     let testdata = load_testdata();
     let result = verify_rekor_signature(&testdata.log_entry, &testdata.rekor_public_key);
     assert!(result.is_ok());
 }
 
 #[test]
-fn test_verify_rekor_log_entry() {
+fn test_verify_rekor_log_entry_success() {
     let testdata = load_testdata();
 
     let result = verify_rekor_log_entry(
@@ -84,7 +90,7 @@ fn test_verify_rekor_log_entry() {
 }
 
 #[test]
-fn test_verify_endorsement_statement() {
+fn test_verify_endorsement_statement_success() {
     let testdata = load_testdata();
     let statement = parse_endorsement_statement(&testdata.endorsement)
         .expect("could not parse endorsement statement");
@@ -93,15 +99,33 @@ fn test_verify_endorsement_statement() {
 }
 
 #[test]
-fn test_verify_endorser_public_key() {
+fn test_verify_endorsement_statement_fails_too_early() {
+    let testdata = load_testdata();
+    let statement = parse_endorsement_statement(&testdata.endorsement)
+        .expect("could not parse endorsement statement");
+    let result = verify_endorsement_statement(TOO_EARLY_UTC_MILLIS, &statement);
+    assert!(result.is_err(), "{:?}", result);
+}
+
+#[test]
+fn test_verify_endorsement_statement_fails_too_late() {
+    let testdata = load_testdata();
+    let statement = parse_endorsement_statement(&testdata.endorsement)
+        .expect("could not parse endorsement statement");
+    let result = verify_endorsement_statement(TOO_LATE_UTC_MILLIS, &statement);
+    assert!(result.is_err(), "{:?}", result);
+}
+
+#[test]
+fn test_verify_endorser_public_key_success() {
     let testdata = load_testdata();
 
     let result = verify_endorser_public_key(&testdata.log_entry, &testdata.endorser_public_key);
     assert!(result.is_ok(), "{:?}", result);
 }
 
 #[test]
-fn test_verify_binary_endorsement() {
+fn test_verify_binary_endorsement_success() {
     let testdata = load_testdata();
 
     let result = verify_binary_endorsement(
@@ -115,6 +139,83 @@ fn test_verify_binary_endorsement() {
     assert!(result.is_ok(), "{:?}", result);
 }
 
+#[test]
+fn test_verify_binary_endorsement_fails_with_empty_signature() {
+    let testdata = load_testdata();
+
+    let result = verify_binary_endorsement(
+        NOW_UTC_MILLIS,
+        &testdata.endorsement,
+        &Vec::new(),
+        &testdata.log_entry,
+        &testdata.endorser_public_key,
+        &testdata.rekor_public_key,
+    );
+    assert!(result.is_err(), "{:?}", result);
+
+    let result = verify_binary_endorsement(
+        NOW_UTC_MILLIS,
+        &testdata.endorsement,
+        &Vec::new(),
+        &Vec::new(),
+        &testdata.endorser_public_key,
+        &Vec::new(),
+    );
+    assert!(result.is_err(), "{:?}", result);
+}
+
+#[test]
+fn test_verify_binary_endorsement_fails_with_invalid_signature() {
+    let mut testdata = load_testdata();
+
+    testdata.signature[0] ^= 1;
+
+    let result = verify_binary_endorsement(
+        NOW_UTC_MILLIS,
+        &testdata.endorsement,
+        &testdata.signature,
+        &testdata.log_entry,
+        &testdata.endorser_public_key,
+        &testdata.rekor_public_key,
+    );
+    assert!(result.is_err(), "{:?}", result);
+
+    let result = verify_binary_endorsement(
+        NOW_UTC_MILLIS,
+        &testdata.endorsement,
+        &testdata.signature,
+        &Vec::new(),
+        &testdata.endorser_public_key,
+        &Vec::new(),
+    );
+    assert!(result.is_err(), "{:?}", result);
+}
+
+#[test]
+fn test_verify_binary_endorsement_fails_with_empty_endorser_public_key() {
+    let testdata = load_testdata();
+
+    let result = verify_binary_endorsement(
+        NOW_UTC_MILLIS,
+        &testdata.endorsement,
+        &testdata.signature,
+        &testdata.log_entry,
+        &Vec::new(),
+        &testdata.rekor_public_key,
+    );
+    assert!(result.is_err(), "{:?}", result);
+
+    let result = verify_binary_endorsement(
+        NOW_UTC_MILLIS,
+        &testdata.endorsement,
+        &testdata.signature,
+        &Vec::new(),
+        &Vec::new(),
+        &Vec::new(),
+    );
+    assert!(result.is_err(), "{:?}", result);
+}
+
 #[test]
 fn test_verify_binary_endorsement_fails_with_invalid_endorser_public_key() {
     let testdata = load_testdata();
@@ -161,3 +262,28 @@ fn test_verify_binary_endorsement_fails_with_rekor_key_but_no_log_entry() {
     );
     assert!(result.is_err(), "{:?}", result);
 }
+
+#[test]
+fn test_verify_binary_endorsement_succeeds_with_no_rekor_key() {
+    let testdata = load_testdata();
+
+    let result = verify_binary_endorsement(
+        NOW_UTC_MILLIS,
+        &testdata.endorsement,
+        &testdata.signature,
+        &testdata.log_entry,
+        &testdata.endorser_public_key,
+        &Vec::new(),
+    );
+    assert!(result.is_ok(), "{:?}", result);
+
+    let result = verify_binary_endorsement(
+        NOW_UTC_MILLIS,
+        &testdata.endorsement,
+        &testdata.signature,
+        &Vec::new(),
+        &testdata.endorser_public_key,
+        &Vec::new(),
+    );
+    assert!(result.is_ok(), "{:?}", result);
+}

oak_proto_rust/generated/oak.attestation.v1.rs

@@ -602,10 +602,14 @@ pub struct SkipVerification {}
 #[allow(clippy::derive_partial_eq_without_eq)]
 #[derive(Clone, PartialEq, ::prost_derive::Message)]
 pub struct EndorsementReferenceValue {
-    /// The endorser's public signing key for signature verification.
+    /// The endorser's public verifying key for signature verification. The
+    /// attestation verification requires that all endorsements need to be
+    /// signed, therefore this cannot be empty.
     #[prost(bytes = "vec", tag = "1")]
     pub endorser_public_key: ::prost::alloc::vec::Vec<u8>,
-    /// Rekor's public signing key for signature verification.
+    /// Rekor's public verifying key for log entry verification. Needs to be
+    /// set when a log entry is present that should be verified. If it is not set,
+    /// then log entry verification is skipped.
     #[prost(bytes = "vec", tag = "2")]
     pub rekor_public_key: ::prost::alloc::vec::Vec<u8>,
 }

proto/attestation/reference_value.proto

@@ -30,10 +30,14 @@ message SkipVerification {}
 
 // Verifies the transparency log entry, including signatures and the digest.
 message EndorsementReferenceValue {
-  // The endorser's public signing key for signature verification.
+  // The endorser's public verifying key for signature verification. The
+  // attestation verification requires that all endorsements need to be
+  // signed, therefore this cannot be empty.
   bytes endorser_public_key = 1;
 
-  // Rekor's public signing key for signature verification.
+  // Rekor's public verifying key for log entry verification. Needs to be
+  // set when a log entry is present that should be verified. If it is not set,
+  // then log entry verification is skipped.
   bytes rekor_public_key = 2;
 }