-
Notifications
You must be signed in to change notification settings - Fork 175
AAEL eventlog replay support for az-snp-vtpm #1565
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
8afbdd8
d9720fd
c2247c4
f883f18
39f7960
44c6a68
b2be44a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,7 +12,7 @@ use crate::snp::{ | |
| LOADER_SPL_OID, SNP_SPL_OID, TEE_SPL_OID, UCODE_SPL_OID, | ||
| }; | ||
| use crate::{InitDataHash, ReportData}; | ||
| use anyhow::{bail, Context, Result}; | ||
| use anyhow::{anyhow, bail, Context, Result}; | ||
| use async_trait::async_trait; | ||
| use az_snp_vtpm::certs::{AmdChain, Vcek}; | ||
| use az_snp_vtpm::hcl::HclReport; | ||
|
|
@@ -21,6 +21,7 @@ use az_snp_vtpm::vtpm::QuoteError; | |
| use base64::engine::general_purpose::STANDARD; | ||
| use base64::Engine; | ||
| pub(crate) use compat::TpmQuote; | ||
| use eventlog::{ccel::tcg_enum::TcgAlgorithm, CcEventLog, ReferenceMeasurement}; | ||
| use openssl::hash::MessageDigest; | ||
| use openssl::pkey::PKey; | ||
| use openssl::sign::Verifier as OsslVerifier; | ||
|
|
@@ -30,7 +31,7 @@ use serde::Deserialize; | |
| use serde_json::{json, Value}; | ||
| use sev::parser::ByteParser; | ||
| use thiserror::Error; | ||
| use tracing::{debug, instrument}; | ||
| use tracing::{debug, instrument, warn}; | ||
| use tss_esapi::structures::{Attest, AttestInfo}; | ||
| use tss_esapi::traits::UnMarshall; | ||
| use x509_parser::prelude::*; | ||
|
|
@@ -40,6 +41,32 @@ const INITDATA_PCR: usize = 8; | |
| const SNP_REPORT_SIGNATURE_OFFSET: usize = 0x2a0; // 672 bytes | ||
| const SHA256_LEN: usize = 32; | ||
|
|
||
| /// vTPM DRTM measurement registers (PCR17-22, per the TCG PC Client | ||
| /// Platform Firmware Profile). These reset to all-0xFF, not all-zero, | ||
| /// before any event extends them -- unlike general-purpose registers | ||
|
Comment on lines
+45
to
+46
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm curious if you see all-zeros for the measured boot range <16? My understanding is DRTM is not really applicable in this Azure setup.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not quite. DRTM is not applicable in Azure, but SRTM is.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. From confidential-containers/guest-components#1605 I see that writes to PCR17-22 fail. What prevents us to use one of the PCRs that are not used at the moment? Alternatively, we could include full
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree, if we are able to use the standard tcg eventlog, that would be preferable over a custom coco log (if that’s feasible)
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe we are getting sidetracked here. Your comments seem to concern the guest-components side of things. This PR has no say in what PCRs are used or what are the contents of the logs. It simply validates the eventlog (be it CCEL or AAEL) and uses the PCRs defined in the log events themselves. But to answer your questions:
Still, I think this is a conversation to be had in the guest-components PR: confidential-containers/guest-components#1605
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @aramirez-capacity I believe these two PRs are related so not completely side-tracking here. What's relevant for this PR is the question whether how to handle the replay of 17-22 for Azure vTPMs since they don't seem to work and has impact to
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry, I don't follow. I thought we were discussing the writing of the log, hence why I mentioned the other PR. If you are asking how would this PR handle an eventlog that uses PCR17-22, it would replay the log as usual and it would check if the final result matches the content of the specified PCR (which in the case of Azure it won't, as those PCRs are always -1). These replays would use -1 as the initial value as per the TCG spec. |
||
| /// (e.g. PCR23) AAEL events may target instead. See Table 7 of | ||
| /// <https://trustedcomputinggroup.org/wp-content/uploads/PC-Client-Specific-Platform-TPM-Profile-for-TPM-2p0-v1p05p_r14_pub.pdf>. | ||
| /// Azure's vTPM additionally rejects locality-0 extends on this range, so real | ||
| /// AAEL events land elsewhere in practice -- see | ||
| /// <https://github.com/confidential-containers/guest-components/pull/1605#issuecomment-5119810216> | ||
| /// -- the eventlog records which register each event actually targeted, so | ||
| /// the replay seeds per-register from this class rather than assuming a | ||
| /// single fixed PCR/seed pair. | ||
| const DRTM_PCR_MIN: u32 = 17; | ||
| const DRTM_PCR_MAX: u32 = 22; | ||
| const DRTM_SEED: [u8; 32] = [0xFF; 32]; | ||
|
|
||
| /// Seed to replay a register's AAEL events from: the DRTM reset value for | ||
| /// DRTM PCRs, or empty (zero-fill, the `eventlog` crate's default) for any | ||
| /// other register. | ||
| fn initial_seed_for_pcr(index: u32) -> Vec<u8> { | ||
| if (DRTM_PCR_MIN..=DRTM_PCR_MAX).contains(&index) { | ||
| DRTM_SEED.to_vec() | ||
| } else { | ||
| Vec::new() | ||
| } | ||
| } | ||
|
|
||
| pub struct AzSnpVtpm; | ||
|
|
||
| #[derive(Error, Debug)] | ||
|
|
@@ -85,6 +112,49 @@ fn extract_nonce(message: &[u8]) -> Result<Vec<u8>> { | |
| Ok(attest.extra_data().to_vec()) | ||
| } | ||
|
|
||
| /// Decode and verify the AAEL runtime eventlog against `tpm_quote` (if | ||
| /// present) -- one register per distinct PCR index the log's events | ||
| /// actually target, each seeded per `initial_seed_for_pcr`. Returns the | ||
| /// parsed log on success, or `None` if the evidence carried no eventlog. | ||
| fn verify_eventlog(cc_eventlog: Option<&str>, tpm_quote: &TpmQuote) -> Result<Option<CcEventLog>> { | ||
| let Some(el) = cc_eventlog.filter(|el| !el.is_empty()) else { | ||
| warn!("No AAEL eventlog included inside the az-snp-vtpm evidence, skipping replay."); | ||
| return Ok(None); | ||
| }; | ||
|
|
||
| let ccel_data = STANDARD | ||
| .decode(el) | ||
| .context("Failed to base64-decode cc_eventlog")?; | ||
| let ccel = CcEventLog::try_from(ccel_data) | ||
| .map_err(|e| anyhow!("Failed to parse AAEL eventlog: {:?}", e))?; | ||
|
|
||
| let mut targeted_pcrs: Vec<u32> = ccel.log.iter().map(|entry| entry.index).collect(); | ||
| targeted_pcrs.sort_unstable(); | ||
| targeted_pcrs.dedup(); | ||
|
|
||
| let compare_obj = targeted_pcrs | ||
| .iter() | ||
| .map(|&index| { | ||
| let reference = tpm_quote | ||
| .pcrs | ||
| .get(index as usize) | ||
| .with_context(|| format!("TPM quote does not contain PCR{index}"))? | ||
| .clone(); | ||
| Ok(ReferenceMeasurement { | ||
| index, | ||
| algorithm: TcgAlgorithm::Sha256, | ||
| reference, | ||
| initial_value: initial_seed_for_pcr(index), | ||
| }) | ||
| }) | ||
| .collect::<Result<Vec<_>>>()?; | ||
|
|
||
| ccel.replay_and_match(compare_obj)?; | ||
| debug!("AAEL eventlog replay succeeded for registers {targeted_pcrs:?}"); | ||
|
|
||
| Ok(Some(ccel)) | ||
| } | ||
|
|
||
| #[derive(Deserialize, Debug)] | ||
| struct VarDataUserData { | ||
| #[serde(rename = "user-data")] | ||
|
|
@@ -192,6 +262,8 @@ impl Verifier for AzSnpVtpm { | |
|
|
||
| let mut claim = parse_tee_evidence_az(&snp_report); | ||
| extend_claim(&mut claim, tpm_quote)?; | ||
| let ccel = verify_eventlog(evidence.cc_eventlog(), tpm_quote)?; | ||
| crate::extend_eventlog_claim(&mut claim, ccel)?; | ||
|
|
||
| Ok(vec![(claim, "cpu".to_string())]) | ||
| } | ||
|
|
@@ -370,8 +442,10 @@ pub(crate) fn parse_tee_evidence_az(report: &AttestationReport) -> TeeEvidencePa | |
| #[cfg(test)] | ||
| mod tests { | ||
| use super::*; | ||
| use crate::extend_eventlog_claim; | ||
| use rstest::rstest; | ||
| use serde_json::json; | ||
| use sha2::{Digest, Sha256}; | ||
|
|
||
| const REPORT: &[u8; 2600] = include_bytes!("../../test_data/az-snp-vtpm/hcl-report.bin"); | ||
| const TPM_QUOTE_V1_JSON: &str = include_str!("../../test_data/az-snp-vtpm/tpm-quote-v1.json"); | ||
|
|
@@ -687,4 +761,127 @@ mod tests { | |
| let report_data = map.get("report_data").unwrap().as_str().unwrap(); | ||
| assert_eq!(report_data, hex::encode(REPORT_DATA)); | ||
| } | ||
|
|
||
| // Fixtures under test_data/az-snp-vtpm/aael-*.bin are raw AAEL eventlogs | ||
| // (a TCG_PCR_EVENT `EV_NO_ACTION` TCG_EfiSpecIDEvent header declaring | ||
| // SHA-256/SHA-384/SM3, followed by one or more TCG_PCR_EVENT2 entries | ||
| // using `EV_IPL` as a generic, parser-agnostic event type -- same as | ||
| // guest-components' own eventlog unit tests). Each entry's event data is | ||
| // one of the payloads below; its digest is recomputed here rather than | ||
| // baked into the fixture, since that's what the replay logic itself | ||
| // hashes against. | ||
| const PULL_IMAGE_EVENT: &[u8] = b"pull-image-event"; | ||
| const DRTM_EVENT: &[u8] = b"drtm-event"; | ||
| const APP_SUPPORT_EVENT: &[u8] = b"app-support-event"; | ||
|
|
||
| const AAEL_PCR17_PULL_IMAGE: &[u8] = | ||
| include_bytes!("../../test_data/az-snp-vtpm/aael-pcr17-pull-image.bin"); | ||
| const AAEL_PCR23_PULL_IMAGE: &[u8] = | ||
| include_bytes!("../../test_data/az-snp-vtpm/aael-pcr23-pull-image.bin"); | ||
| const AAEL_MULTI_PCR17_PCR23: &[u8] = | ||
| include_bytes!("../../test_data/az-snp-vtpm/aael-multi-pcr17-pcr23.bin"); | ||
|
|
||
| #[test] | ||
| fn test_extend_eventlog_claim_none_is_noop() { | ||
| let mut claim = json!({}); | ||
| let tpm_quote = load_tpm_quote(); | ||
| let ccel = verify_eventlog(None, &tpm_quote).unwrap(); | ||
| assert!(ccel.is_none()); | ||
| extend_eventlog_claim(&mut claim, ccel).unwrap(); | ||
| assert!(claim.as_object().unwrap().is_empty()); | ||
| } | ||
|
|
||
| /// An example DRTM PCR (see `DRTM_PCR_MIN..=DRTM_PCR_MAX`) used across | ||
| /// these tests to build events against. | ||
| const DRTM_TEST_PCR: u32 = 17; | ||
|
|
||
| /// PCR 23 ("application support") is a general-purpose, locality-0 | ||
| /// extendable register that resets to all-zero — unlike PCR17's DRTM | ||
| /// reset value. Azure's vTPM rejects locality-0 extends on PCR17, so | ||
| /// real deployments target PCR23 instead; the replay must seed from | ||
| /// zero for it, not the DRTM 0xFF constant. | ||
| const APP_SUPPORT_PCR: u32 = 23; | ||
|
|
||
| #[rstest] | ||
| #[case::drtm_pcr_seeds_from_drtm_reset_value(DRTM_TEST_PCR, DRTM_SEED, true)] | ||
| #[case::app_support_pcr_seeds_from_zero(APP_SUPPORT_PCR, [0u8; 32], true)] | ||
| #[case::drtm_pcr_rejects_zero_seed(DRTM_TEST_PCR, [0u8; 32], false)] | ||
| fn test_verify_eventlog_seed_matrix( | ||
| #[case] target_pcr: u32, | ||
| #[case] seed: [u8; 32], | ||
| #[case] expect_ok: bool, | ||
| ) { | ||
| let ccel_bytes: &[u8] = if target_pcr == DRTM_TEST_PCR { | ||
| AAEL_PCR17_PULL_IMAGE | ||
| } else { | ||
| AAEL_PCR23_PULL_IMAGE | ||
| }; | ||
| let event_digest: [u8; 32] = Sha256::digest(PULL_IMAGE_EVENT).into(); | ||
|
|
||
| let mut hasher = Sha256::new(); | ||
| hasher.update(seed); | ||
| hasher.update(event_digest); | ||
| let expected_pcr: Vec<u8> = hasher.finalize().to_vec(); | ||
|
|
||
| let mut tpm_quote = load_tpm_quote(); | ||
| // Guard against a stale hardcoded-PCR17 implementation coincidentally | ||
| // passing when the register under test isn't 17: corrupt PCR17's | ||
| // fixture value (already sitting at the DRTM baseline) whenever it | ||
| // isn't the register being exercised. | ||
| if target_pcr != DRTM_TEST_PCR { | ||
| tpm_quote.pcrs[DRTM_TEST_PCR as usize] = vec![0u8; 32]; | ||
| } | ||
| tpm_quote.pcrs[target_pcr as usize] = expected_pcr; | ||
|
|
||
| let cc_eventlog = STANDARD.encode(ccel_bytes); | ||
| let result = verify_eventlog(Some(cc_eventlog.as_str()), &tpm_quote); | ||
| assert_eq!(result.is_ok(), expect_ok); | ||
|
|
||
| if let Ok(ccel) = result { | ||
| let mut claim = json!({}); | ||
| extend_eventlog_claim(&mut claim, ccel).unwrap(); | ||
| let events = claim | ||
| .as_object() | ||
| .unwrap() | ||
| .get("uefi_event_logs") | ||
| .expect("claim should contain uefi_event_logs") | ||
| .as_array() | ||
| .unwrap(); | ||
| assert_eq!(events.len(), 1); | ||
| } | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_extend_eventlog_claim_replays_multiple_registers_with_correct_seeds() { | ||
| let drtm_digest: [u8; 32] = Sha256::digest(DRTM_EVENT).into(); | ||
| let app_support_digest: [u8; 32] = Sha256::digest(APP_SUPPORT_EVENT).into(); | ||
|
|
||
| let mut drtm_hasher = Sha256::new(); | ||
| drtm_hasher.update(DRTM_SEED); | ||
| drtm_hasher.update(drtm_digest); | ||
| let expected_drtm_pcr: Vec<u8> = drtm_hasher.finalize().to_vec(); | ||
|
|
||
| let mut app_support_hasher = Sha256::new(); | ||
| app_support_hasher.update([0u8; 32]); | ||
| app_support_hasher.update(app_support_digest); | ||
| let expected_app_support_pcr: Vec<u8> = app_support_hasher.finalize().to_vec(); | ||
|
|
||
| let mut tpm_quote = load_tpm_quote(); | ||
| tpm_quote.pcrs[DRTM_TEST_PCR as usize] = expected_drtm_pcr; | ||
| tpm_quote.pcrs[APP_SUPPORT_PCR as usize] = expected_app_support_pcr; | ||
|
|
||
| let cc_eventlog = STANDARD.encode(AAEL_MULTI_PCR17_PCR23); | ||
| let mut claim = json!({}); | ||
| let ccel = verify_eventlog(Some(cc_eventlog.as_str()), &tpm_quote).unwrap(); | ||
| extend_eventlog_claim(&mut claim, ccel).unwrap(); | ||
|
|
||
| let events = claim | ||
| .as_object() | ||
| .unwrap() | ||
| .get("uefi_event_logs") | ||
| .expect("claim should contain uefi_event_logs") | ||
| .as_array() | ||
| .unwrap(); | ||
| assert_eq!(events.len(), 2); | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be an Option? Also, please provide some documentation for this nonzero reset claim.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@aramirez-capacity This non-zero case would cause a problem. If allow non-zero initial value, an malwared attester can fake this field with a non-zero value. Also, trim the eventlog items with only later "good parts", and squash previous "bad parts" into a
initial_valuehere.Now we assume all fields to be zero-initialized, thus no such attack could happen - as all eventlog entries should be included so reflect the PCR final value.
If we want this field, we should let the verifier explicitly know the expected value and check it. One way is to add this into the parsed claims and let the policy cover this. Also, I suggest to change default policy to assert these fields to be all zeros. This prevents users from inadvertently allowing malicious initial PCR values due to a lack of configuration.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The non-zero initial value is part of the spec: https://trustedcomputinggroup.org/wp-content/uploads/PC-Client-Specific-Platform-TPM-Profile-for-TPM-2p0-v1p05p_r14_pub.pdf (see Table 7).
It's not an arbitrary value; it's either 0 or -1, depending on the PCR.
I added a mention in the comment.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh ok. I misunderstood this. Thanks for the explanation. Another question is: Is DRTM the always behavior for Azure thus 17~22 be reset to -1?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Confirmed there's no DRTM launch on Azure's SEV-SNP vTPM (
az-snp-vtpm).Here's a live quote showing PCR23 holds a real measurement while PCR17-22 still contain their initial values (
0xFF):There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cc @mkulke please help to check if this is expected from white box view.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should be -1/0xff by the TCG standard, so that's expected I think: