From f763693d8d1540b37b245f05585e4be712de6e0f Mon Sep 17 00:00:00 2001 From: Tom Binder Date: Wed, 3 Jul 2024 08:55:33 +0000 Subject: [PATCH] Clarify skipping via absence of rekor_public_key. Make failure messages clearer by catching absence early on. Extended tests to cover all combinations. Change-Id: Iede72087888b6a1919185582ec5224c5ecb1cee2 --- .../src/endorsement.rs | 40 ++++-- .../tests/endorsement_tests.rs | 136 +++++++++++++++++- .../generated/oak.attestation.v1.rs | 8 +- proto/attestation/reference_value.proto | 8 +- 4 files changed, 171 insertions(+), 21 deletions(-) diff --git a/oak_attestation_verification/src/endorsement.rs b/oak_attestation_verification/src/endorsement.rs index 694a20298b5..91d71d5d81b 100644 --- a/oak_attestation_verification/src/endorsement.rs +++ b/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(()) diff --git a/oak_attestation_verification/tests/endorsement_tests.rs b/oak_attestation_verification/tests/endorsement_tests.rs index 1f9701aa0fe..825d0b8d1d2 100644 --- a/oak_attestation_verification/tests/endorsement_tests.rs +++ b/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, signature: Vec, @@ -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,7 +99,25 @@ 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); @@ -101,7 +125,7 @@ fn test_verify_endorser_public_key() { } #[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); +} diff --git a/oak_proto_rust/generated/oak.attestation.v1.rs b/oak_proto_rust/generated/oak.attestation.v1.rs index db611ff9761..3760ce420e9 100644 --- a/oak_proto_rust/generated/oak.attestation.v1.rs +++ b/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, - /// 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, } diff --git a/proto/attestation/reference_value.proto b/proto/attestation/reference_value.proto index dd8c3f17178..33c55dd1822 100644 --- a/proto/attestation/reference_value.proto +++ b/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; }