From f6881530afc8cfec256de80bfb952df39fcacded Mon Sep 17 00:00:00 2001 From: Samuel Laferriere <9342524+samlaf@users.noreply.github.com> Date: Fri, 28 Aug 2026 17:49:42 -0400 Subject: [PATCH] fix(attestation)!: label PCR values by the registers a quote attests A vTPM quote names the registers it covers in the pcrSelect bitmap inside the signed TPMS_ATTEST. The parser skipped that bitmap, so verification labelled the PCR values by their position in the accompanying list - the first value is PCR0, the second PCR1, and so on. That is only correct for a quote selecting a contiguous run from zero. The sending party chooses the selection, so it can choose one that shifts a favourable value into a pinned register's position. Take a machine whose PCR13 and PCR15 hold something the policy forbids. Azure leaves PCR16 and PCR23 at zero, so the machine quotes 0..=12, 16, 17, 23 instead: sixteen of its own unaltered values, a pcrDigest its vTPM computes over exactly those, and a valid AK signature. Counted off by position, PCR16's and PCR23's zeros land where the policy checks 13 and 15, and the machine is accepted. Nothing is forged and every cryptographic check passes. Read off the bitmap, 13 and 15 are not in the quote at all, and check_measurement refuses a pinned register it cannot find. Registers now come from the quote. tpms_attest decodes pcrSelect into ascending register numbers, and accepts exactly one PCR bank, which must be SHA-256 since the values travel as fixed-width [u8; 32]. verify returns the values already paired with their registers, so the pairing cannot be obtained without the checks that give it meaning - the digest comparison is also what fixes the order the values are read in. indexed_pcrs_unverified serves get_measurements, which verifies nothing and now says so in its name. A value list that does not fill its selection is refused rather than paired against a prefix, and a pcrSelect bitmap longer than four octets is refused rather than expanded - the spec bounds it by the TPM's own PCR count. Evidence produced by this repo was never affected: az-cvm-vtpm's get_quote always selects PCRs 0-23, where position and selection agree. Regression tests cover the crafted subset end to end, the fixture's pairing, a non-contiguous selection, a short value list, an oversized bitmap, a second bank and a non-SHA-256 bank. The differential test against tss-esapi now compares the selection alongside extraData and pcrDigest, so the field the labelling depends on is checked against an independent implementation. BREAKING CHANGE: MultiMeasurements::from_pcrs is replaced by from_indexed_pcrs; AttestError loses PcrSelectionCount and gains PcrSelectionBanks, PcrSelectionAlgorithm and PcrSelectionSize; TpmQuoteError gains PcrCountMismatch; TpmQuote::verify returns the PCR pairing instead of (). Closes https://github.com/flashbots/attested-tls/issues/89 --- crates/attestation/src/azure/attester/mod.rs | 15 ++ crates/attestation/src/azure/tpm_quote.rs | 224 ++++++++++++++++++- crates/attestation/src/azure/tpms_attest.rs | 214 ++++++++++++++---- crates/attestation/src/azure/verify.rs | 10 +- crates/attestation/src/measurements.rs | 30 ++- 5 files changed, 437 insertions(+), 56 deletions(-) diff --git a/crates/attestation/src/azure/attester/mod.rs b/crates/attestation/src/azure/attester/mod.rs index 4e16a96..7f47efd 100644 --- a/crates/attestation/src/azure/attester/mod.rs +++ b/crates/attestation/src/azure/attester/mod.rs @@ -400,6 +400,21 @@ mod tests { let parsed = TpmsAttest::parse(&message).unwrap(); assert_eq!(parsed.extra_data(), attest.extra_data().as_slice()); assert_eq!(parsed.pcr_digest(), info.pcr_digest().as_slice()); + + // The selection decides which register each PCR value is attributed + // to, so it is cross-checked as well. `PcrSlot` is a bit flag, so a + // register's number is the position of its one bit. Sorted, because + // agreement on the set is the claim being tested; our own ascending + // order is pinned by the tests in `tpms_attest`. + let mut tss_registers: Vec = info + .pcr_selection() + .get_selections() + .iter() + .flat_map(|selection| selection.selected()) + .map(|slot| (slot as u32).trailing_zeros()) + .collect(); + tss_registers.sort_unstable(); + assert_eq!(parsed.selected_pcrs(), tss_registers.as_slice()); } #[test] diff --git a/crates/attestation/src/azure/tpm_quote.rs b/crates/attestation/src/azure/tpm_quote.rs index 4c872d3..8cefaab 100644 --- a/crates/attestation/src/azure/tpm_quote.rs +++ b/crates/attestation/src/azure/tpm_quote.rs @@ -42,6 +42,8 @@ pub enum TpmQuoteError { NonceMismatch, #[error("PCR digest does not match PCR values")] PcrMismatch, + #[error("quote attests {selected} PCR(s) but carries {carried} value(s)")] + PcrCountMismatch { selected: usize, carried: usize }, } /// A vTPM quote: AK signature, marshalled `TPMS_ATTEST` message, and the @@ -57,13 +59,15 @@ pub(super) struct TpmQuote { } impl TpmQuote { - /// Retrieve sha256 PCR values from the quote - pub(super) fn pcrs_sha256(&self) -> impl Iterator { - self.pcrs.iter() - } - - /// Verify the quote's signature, nonce, and PCR digest - pub(super) fn verify(&self, pub_key: &PKey, nonce: &[u8]) -> Result<(), TpmQuoteError> { + /// Verify the quote's signature, nonce, and PCR digest, and return its + /// sha256 PCR values paired with the registers they measure. + /// + /// The registers are the ones the quote's `pcrSelect` bitmap names. + pub(super) fn verify( + &self, + pub_key: &PKey, + nonce: &[u8], + ) -> Result, TpmQuoteError> { self.verify_signature(pub_key)?; let attest = TpmsAttest::parse(&self.message)?; @@ -71,7 +75,38 @@ impl TpmQuote { return Err(TpmQuoteError::NonceMismatch); } - self.verify_pcrs(&attest) + self.verify_pcrs(&attest)?; + self.pair_with_registers(&attest) + } + + /// The same pairing [`TpmQuote::verify`] returns, without performing + /// any of the verification: no signature, no nonce, no digest over the + /// values. + pub(super) fn indexed_pcrs_unverified(&self) -> Result, TpmQuoteError> { + let attest = TpmsAttest::parse(&self.message)?; + self.pair_with_registers(&attest) + } + + /// Pair each value with the register the quote attests it for. + /// + /// A quote's value list has to fill its attested selection exactly: + /// anything else cannot be paired with it, and pairing a prefix would + /// hand a policy values from registers it did not ask about. + fn pair_with_registers( + &self, + attest: &TpmsAttest, + ) -> Result, TpmQuoteError> { + self.check_pcr_count(attest)?; + Ok(attest.selected_pcrs().iter().copied().zip(self.pcrs.iter().copied()).collect()) + } + + /// A quote's value list has to fill its attested selection exactly. + fn check_pcr_count(&self, attest: &TpmsAttest) -> Result<(), TpmQuoteError> { + let selected = attest.selected_pcrs().len(); + if selected != self.pcrs.len() { + return Err(TpmQuoteError::PcrCountMismatch { selected, carried: self.pcrs.len() }); + } + Ok(()) } /// Verify the quote's signature (SHA-256 RSA) over the message @@ -87,6 +122,10 @@ impl TpmQuote { /// Verify that the attested PCR digest matches the digest of the /// bundled PCR values fn verify_pcrs(&self, attest: &TpmsAttest) -> Result<(), TpmQuoteError> { + // The digest below is taken over the whole value list, so a list + // of the wrong length fails there too; checking the count first + // names the actual complaint. + self.check_pcr_count(attest)?; let mut hasher = Sha256::new(); for pcr in &self.pcrs { hasher.update(pcr); @@ -101,7 +140,19 @@ impl TpmQuote { #[cfg(test)] mod tests { + use std::collections::HashMap; + use super::*; + use crate::{ + AttestationType, + azure::tpms_attest::test_support::{SHA256_BANK, build_attest, pcr_bitmap}, + measurements::{ + ExpectedMeasurements, + MeasurementPolicy, + MeasurementRecord, + MultiMeasurements, + }, + }; const FIXTURE: &[u8] = include_bytes!("../../test-assets/azure-tdx-with-ak-intermediates-1780922561.yaml"); @@ -121,6 +172,163 @@ mod tests { assert_eq!(serde_json::to_value("e).unwrap(), quote_value); } + /// The registers come from the quote, not from list position. Azure + /// selects PCRs 0-23 in one SHA-256 bank, so the fixture's values pair + /// with 0..=23 — and `pcr4` in a measurement policy means register 4 + /// because the quote says so, not because it is the fifth value. + #[test] + fn values_pair_with_the_registers_the_quote_selects() { + let quote: TpmQuote = serde_json::from_value(fixture_quote_value()).unwrap(); + let attest = TpmsAttest::parse("e.message).unwrap(); + + let expected: Vec = (0..24).collect(); + assert_eq!(attest.selected_pcrs(), expected.as_slice()); + + let indexed = quote.indexed_pcrs_unverified().unwrap(); + assert_eq!(indexed.len(), 24); + for (position, (register, value)) in indexed.iter().enumerate() { + assert_eq!(*register, position as u32); + assert_eq!(value, "e.pcrs[position]); + } + } + + /// A measurement policy pinning the given registers to the given + /// values, and nothing else. + fn azure_policy(expected: &[(u32, [u8; 32])]) -> MeasurementPolicy { + MeasurementPolicy { + accepted_measurements: vec![MeasurementRecord { + measurement_id: "test image".to_string(), + attestation_type: AttestationType::AzureTdx, + measurements: ExpectedMeasurements::Azure( + expected.iter().map(|(register, value)| (*register, vec![*value])).collect(), + ), + }], + } + } + + /// A quote over `selection`, carrying those registers' values from + /// `machine`, with the pcrDigest a vTPM signs over them. + fn quote_over(selection: &[u32], machine: &[[u8; 32]]) -> TpmQuote { + let pcrs: Vec<[u8; 32]> = + selection.iter().map(|register| machine[*register as usize]).collect(); + let mut hasher = Sha256::new(); + for pcr in &pcrs { + hasher.update(pcr); + } + let bitmap = pcr_bitmap(selection); + let message = build_attest(b"nonce", &[(SHA256_BANK, &bitmap)], &hasher.finish()); + TpmQuote { signature: Vec::new(), message, pcrs } + } + + /// Labelling a quote's values by their position in the list, as if the + /// first were PCR0 and the next PCR1, and so on. + fn labelled_by_position(quote: &TpmQuote) -> MultiMeasurements { + MultiMeasurements::Azure( + quote + .pcrs + .iter() + .copied() + .enumerate() + .map(|(position, value)| (position as u32, value)) + .collect(), + ) + } + + /// A machine the policy refuses, whose honest evidence position-based + /// labelling accepts. + /// + /// The machine loaded something the policy forbids, which extended PCRs + /// 13 and 15 away from zero. The policy pins PCRs 0-7 to the image and + /// requires 13 and 15 to still be zero. Azure leaves PCRs 16 and 23 at + /// zero — the fixture shows both — so the machine quotes the selection + /// `0..=12, 16, 17, 23`: sixteen of its own unaltered PCR values, over + /// a digest its vTPM will sign. It forges nothing. + /// + /// Counted off by position, the fourteenth value is PCR16's zero and + /// the sixteenth is PCR23's zero, and the policy's `pcr13` and `pcr15` + /// are satisfied by registers it never asked about. Read off the + /// quote's own `pcrSelect`, 13 and 15 are not attested at all. + #[test] + fn a_subset_selection_cannot_pass_one_register_off_as_another() { + let fixture: TpmQuote = serde_json::from_value(fixture_quote_value()).unwrap(); + let mut machine = fixture.pcrs; + machine[13] = [0x13; 32]; + machine[15] = [0x15; 32]; + + let mut pinned: Vec<(u32, [u8; 32])> = + (0..8).map(|register| (register, machine[register as usize])).collect(); + pinned.push((13, [0; 32])); + pinned.push((15, [0; 32])); + let policy = azure_policy(&pinned); + + // Quoting every register describes the machine as it is, and the + // policy refuses it — under either labelling, since a full + // selection makes the two agree. + let full = quote_over(&(0..24).collect::>(), &machine); + let full_measurements = + MultiMeasurements::from_indexed_pcrs(full.indexed_pcrs_unverified().unwrap()); + assert_eq!(full_measurements, labelled_by_position(&full)); + assert!( + policy.check_measurement(&full_measurements, None).is_err(), + "the machine is not one the policy accepts: {full_measurements:?}" + ); + + // The same machine, quoting a subset. + let mut selection: Vec = (0..=12).collect(); + selection.extend([16, 17, 23]); + let crafted = quote_over(&selection, &machine); + + // Nothing about this quote is malformed: the digest covers exactly + // the values it carries, which are the machine's own. + let attest = TpmsAttest::parse(&crafted.message).unwrap(); + crafted.verify_pcrs(&attest).unwrap(); + + // Labelled by position, it satisfies a policy that has to refuse + // it. This is the bypass. + let by_position = labelled_by_position(&crafted); + policy + .check_measurement(&by_position, None) + .expect("position labelling is what the bypass relies on"); + + // Labelled by the registers the quote attests, PCR13 and PCR15 are + // absent and the policy has nothing to accept. + let by_selection = + MultiMeasurements::from_indexed_pcrs(crafted.indexed_pcrs_unverified().unwrap()); + assert!( + policy.check_measurement(&by_selection, None).is_err(), + "policy accepted a machine whose PCR13 and PCR15 it never saw: {by_selection:?}" + ); + assert_eq!( + by_selection, + MultiMeasurements::Azure( + selection + .iter() + .map(|register| (*register, machine[*register as usize])) + .collect::>() + ) + ); + } + + /// A quote whose value list does not fill its attested selection cannot + /// be paired with it, so it fails rather than pairing a prefix. + #[test] + fn a_short_value_list_is_refused() { + let mut quote: TpmQuote = serde_json::from_value(fixture_quote_value()).unwrap(); + quote.pcrs.pop(); + + let err = quote.indexed_pcrs_unverified().unwrap_err(); + assert!( + matches!(err, TpmQuoteError::PcrCountMismatch { selected: 24, carried: 23 }), + "{err:?}" + ); + + let attest = TpmsAttest::parse("e.message).unwrap(); + assert!(matches!( + quote.verify_pcrs(&attest).unwrap_err(), + TpmQuoteError::PcrCountMismatch { .. } + )); + } + #[test] fn verify_pcrs_accepts_fixture_and_rejects_tampered_pcr() { let mut quote: TpmQuote = serde_json::from_value(fixture_quote_value()).unwrap(); diff --git a/crates/attestation/src/azure/tpms_attest.rs b/crates/attestation/src/azure/tpms_attest.rs index 2a3a037..719fd96 100644 --- a/crates/attestation/src/azure/tpms_attest.rs +++ b/crates/attestation/src/azure/tpms_attest.rs @@ -22,11 +22,17 @@ const TPM_GENERATED_VALUE: u32 = 0xff54_4347; /// (TPM 2.0 spec Part 2, section 6.9). const TPM_ST_ATTEST_QUOTE: u16 = 0x8018; -/// Maximum number of TPMS_PCR_SELECTION entries accepted in a -/// TPML_PCR_SELECTION. The spec bounds the list by the number of hash -/// algorithms the TPM implements (TPM 2.0 spec Part 2, section 10.9.7); -/// 16 is far above any real TPM and merely bounds work on hostile input. -const MAX_PCR_SELECTIONS: u32 = 16; +/// TPM_ALG_SHA256, the only PCR bank a quote may select here: the PCR +/// values travel as `[u8; 32]`, so no other digest size can be carried +/// (TPM 2.0 spec Part 2, section 6.3). +const TPM_ALG_SHA256: u16 = 0x000b; + +/// Maximum octets accepted in a `pcrSelect` bitmap. The spec bounds it by +/// PCR_SELECT_MAX, the octets needed for the TPM's own PCR count (TPM 2.0 +/// spec Part 2, section 10.6.2); four covers 32 registers, more than any +/// TPM allocates. Bounding it keeps one attacker-supplied byte from +/// deciding how much work the expansion below does. +const MAX_SIZE_OF_SELECT: usize = 4; #[derive(Error, Debug)] pub enum AttestError { @@ -36,8 +42,12 @@ pub enum AttestError { Magic, #[error("TPMS_ATTEST is not a quote")] NotAQuote, - #[error("TPML_PCR_SELECTION count is implausibly large")] - PcrSelectionCount, + #[error("quote selects {0} PCR banks; exactly one SHA-256 bank is required")] + PcrSelectionBanks(u32), + #[error("quote selects PCR bank with hash algorithm {0:#06x}, not SHA-256")] + PcrSelectionAlgorithm(u16), + #[error("pcrSelect bitmap is {0} octets; at most 4 are allowed")] + PcrSelectionSize(usize), #[error("trailing bytes after TPMS_ATTEST")] TrailingData, } @@ -47,6 +57,7 @@ pub enum AttestError { pub(crate) struct TpmsAttest { extra_data: Vec, pcr_digest: Vec, + selected_pcrs: Vec, } impl TpmsAttest { @@ -71,21 +82,44 @@ impl TpmsAttest { // attested.quote: TPMS_QUOTE_INFO, starting with the pcrSelect // list (TPML_PCR_SELECTION) let selection_count = reader.read_u32()?; - if selection_count > MAX_PCR_SELECTIONS { - return Err(AttestError::PcrSelectionCount); + // Exactly one bank, and it has to be SHA-256. The values a quote + // carries alongside this structure are fixed-width `[u8; 32]`, so a + // second bank would have no room and a different algorithm would not + // fit at all. Refusing beats guessing: which value belongs to which + // register is what a measurement policy is compared against. + if selection_count != 1 { + return Err(AttestError::PcrSelectionBanks(selection_count)); } - for _ in 0..selection_count { - // TPMS_PCR_SELECTION: hash algorithm, sizeofSelect, pcrSelect - reader.skip(2)?; - let size_of_select = reader.read_u8()? as usize; - reader.skip(size_of_select)?; + // TPMS_PCR_SELECTION: hash algorithm, sizeofSelect, pcrSelect + let hash_algorithm = reader.read_u16()?; + if hash_algorithm != TPM_ALG_SHA256 { + return Err(AttestError::PcrSelectionAlgorithm(hash_algorithm)); } + let size_of_select = reader.read_u8()? as usize; + if size_of_select > MAX_SIZE_OF_SELECT { + return Err(AttestError::PcrSelectionSize(size_of_select)); + } + let selection_bitmap = reader.take(size_of_select)?; + // pcrSelect is a bitmap, LSB first within each octet: octet i bit j + // selects PCR i * 8 + j (TPM 2.0 spec Part 2, section 10.6.2). + // + // Ascending order is what lets a caller pair this list with the + // values a quote ships, and the quote's own signature is what + // enforces it: the TPM computes pcrDigest over the selected values + // concatenated in selection order, which is ascending by register. + // A sender that reorders the values it ships therefore fails the + // digest comparison in `TpmQuote::verify_pcrs`. The pairing means + // nothing until that comparison has run. + let selected_pcrs = (0..size_of_select * 8) + .filter(|pcr| selection_bitmap[pcr / 8] & (1 << (pcr % 8)) != 0) + .map(|pcr| pcr as u32) + .collect(); // attested.quote.pcrDigest: TPM2B_DIGEST let pcr_digest = reader.read_tpm2b()?.to_vec(); if reader.offset != bytes.len() { return Err(AttestError::TrailingData); } - Ok(Self { extra_data, pcr_digest }) + Ok(Self { extra_data, pcr_digest, selected_pcrs }) } /// The `extraData` field: caller-provided qualifying data (the nonce). @@ -97,6 +131,12 @@ impl TpmsAttest { pub(crate) fn pcr_digest(&self) -> &[u8] { &self.pcr_digest } + + /// The PCR registers this quote attests, ascending — the registers the + /// quote's values belong to, in the order they arrive. + pub(crate) fn selected_pcrs(&self) -> &[u32] { + &self.selected_pcrs + } } struct Reader<'a> { @@ -136,12 +176,56 @@ impl<'a> Reader<'a> { } } +/// Marshalling helpers shared by the tests of this module and of +/// `tpm_quote`, which needs quotes over selections other than the usual +/// PCRs 0-23. #[cfg(test)] -mod tests { - use super::*; +pub(crate) mod test_support { + use super::{TPM_ALG_SHA256, TPM_GENERATED_VALUE, TPM_ST_ATTEST_QUOTE}; + + /// The SHA-256 bank identifier, for building selections over + /// registers other than the usual 0-23. + pub(crate) const SHA256_BANK: u16 = TPM_ALG_SHA256; + + /// A single SHA-256 TPMS_PCR_SELECTION of PCRs 0-23, what an Azure + /// vTPM quote carries. + pub(crate) const ALL_24_PCRS: &[(u16, &[u8])] = &[(TPM_ALG_SHA256, &[0xff, 0xff, 0xff])]; + + /// A `pcrSelect` bitmap selecting the given registers: octet i bit j + /// selects PCR i * 8 + j. + pub(crate) fn pcr_bitmap(registers: &[u32]) -> Vec { + let octets = registers.iter().map(|pcr| pcr / 8 + 1).max().unwrap_or(0) as usize; + let mut bitmap = vec![0u8; octets]; + for pcr in registers { + bitmap[(pcr / 8) as usize] |= 1 << (pcr % 8); + } + bitmap + } + + /// Marshal a quote-type TPMS_ATTEST over the given PCR selections. + pub(crate) fn build_attest( + extra_data: &[u8], + selections: &[(u16, &[u8])], + pcr_digest: &[u8], + ) -> Vec { + build_attest_tagged( + TPM_GENERATED_VALUE, + TPM_ST_ATTEST_QUOTE, + extra_data, + selections, + pcr_digest, + ) + } - /// Build a minimal marshalled quote-type TPMS_ATTEST. - fn build_attest(magic: u32, attest_type: u16, extra_data: &[u8], pcr_digest: &[u8]) -> Vec { + /// As [`build_attest`], with the leading magic and structure tag under + /// the caller's control. + pub(crate) fn build_attest_tagged( + magic: u32, + attest_type: u16, + extra_data: &[u8], + selections: &[(u16, &[u8])], + pcr_digest: &[u8], + ) -> Vec { let mut bytes = Vec::new(); bytes.extend_from_slice(&magic.to_be_bytes()); bytes.extend_from_slice(&attest_type.to_be_bytes()); @@ -154,58 +238,90 @@ mod tests { // clockInfo + firmwareVersion bytes.extend_from_slice(&[0; 8 + 4 + 4 + 1]); bytes.extend_from_slice(&[0; 8]); - // TPML_PCR_SELECTION: one SHA-256 selection of PCRs 0-23 - bytes.extend_from_slice(&1u32.to_be_bytes()); - bytes.extend_from_slice(&0x000bu16.to_be_bytes()); - bytes.push(3); - bytes.extend_from_slice(&[0xff, 0xff, 0xff]); + // attested.quote.pcrSelect: TPML_PCR_SELECTION + bytes.extend_from_slice(&(selections.len() as u32).to_be_bytes()); + for (hash_algorithm, bitmap) in selections { + // TPMS_PCR_SELECTION: hash algorithm, sizeofSelect, pcrSelect + bytes.extend_from_slice(&hash_algorithm.to_be_bytes()); + bytes.push(bitmap.len() as u8); + bytes.extend_from_slice(bitmap); + } // pcrDigest: TPM2B_DIGEST bytes.extend_from_slice(&(pcr_digest.len() as u16).to_be_bytes()); bytes.extend_from_slice(pcr_digest); bytes } +} + +#[cfg(test)] +mod tests { + use super::{test_support::*, *}; #[test] fn parses_quote_fields() { let extra_data = b"challenge"; let pcr_digest = [0x42; 32]; - let bytes = build_attest(TPM_GENERATED_VALUE, TPM_ST_ATTEST_QUOTE, extra_data, &pcr_digest); + let bytes = build_attest(extra_data, ALL_24_PCRS, &pcr_digest); let attest = TpmsAttest::parse(&bytes).unwrap(); assert_eq!(attest.extra_data(), extra_data); assert_eq!(attest.pcr_digest(), pcr_digest); + assert_eq!(attest.selected_pcrs(), (0..24).collect::>().as_slice()); + } + + /// `pcrSelect` is a bitmap, LSB first within each octet, and the + /// registers it names come out ascending: that order is what pairs a + /// quote's values with their registers. + #[test] + fn reads_the_selected_registers_off_the_bitmap() { + let registers = [0, 1, 4, 7, 8, 15, 16, 23]; + let bytes = build_attest(b"x", &[(TPM_ALG_SHA256, &pcr_bitmap(®isters))], &[0; 32]); + let attest = TpmsAttest::parse(&bytes).unwrap(); + assert_eq!(attest.selected_pcrs(), registers); } #[test] fn rejects_bad_magic() { - let bytes = build_attest(0xdeadbeef, TPM_ST_ATTEST_QUOTE, b"x", &[0; 32]); + let bytes = + build_attest_tagged(0xdeadbeef, TPM_ST_ATTEST_QUOTE, b"x", ALL_24_PCRS, &[0; 32]); assert!(matches!(TpmsAttest::parse(&bytes), Err(AttestError::Magic))); } #[test] fn rejects_non_quote_attestation() { // TPM_ST_ATTEST_CERTIFY - let bytes = build_attest(TPM_GENERATED_VALUE, 0x8017, b"x", &[0; 32]); + let bytes = build_attest_tagged(TPM_GENERATED_VALUE, 0x8017, b"x", ALL_24_PCRS, &[0; 32]); assert!(matches!(TpmsAttest::parse(&bytes), Err(AttestError::NotAQuote))); } + /// The values a quote carries are fixed-width SHA-256 digests in one + /// list, so a second bank has nowhere to put its values. #[test] - fn rejects_trailing_bytes() { - let mut bytes = build_attest(TPM_GENERATED_VALUE, TPM_ST_ATTEST_QUOTE, b"x", &[0; 32]); - bytes.push(0); - assert!(matches!(TpmsAttest::parse(&bytes), Err(AttestError::TrailingData))); + fn rejects_more_than_one_pcr_bank() { + let sha1 = (0x0004u16, &[0xff, 0xff, 0xff][..]); + let bytes = build_attest(b"x", &[ALL_24_PCRS[0], sha1], &[0; 32]); + assert!(matches!(TpmsAttest::parse(&bytes), Err(AttestError::PcrSelectionBanks(2)))); } #[test] - fn rejects_truncation_at_every_length() { - let bytes = build_attest(TPM_GENERATED_VALUE, TPM_ST_ATTEST_QUOTE, b"x", &[0; 32]); - for len in 0..bytes.len() { - assert!( - matches!(TpmsAttest::parse(&bytes[..len]), Err(AttestError::Truncated)), - "unexpected result at length {len}" - ); - } + fn rejects_a_bank_that_is_not_sha256() { + let sha384 = (0x000cu16, &[0xff, 0xff, 0xff][..]); + let bytes = build_attest(b"x", &[sha384], &[0; 32]); + assert!(matches!( + TpmsAttest::parse(&bytes), + Err(AttestError::PcrSelectionAlgorithm(0x000c)) + )); } + /// One attacker-supplied byte must not decide how many registers the + /// expansion walks. + #[test] + fn rejects_an_oversized_pcr_select_bitmap() { + let bytes = build_attest(b"x", &[(TPM_ALG_SHA256, &[0xff; 5][..])], &[0; 32]); + assert!(matches!(TpmsAttest::parse(&bytes), Err(AttestError::PcrSelectionSize(5)))); + } + + /// A hostile selection count is refused before anything is read on the + /// strength of it. #[test] fn rejects_implausible_pcr_selection_count() { let mut bytes = Vec::new(); @@ -215,6 +331,24 @@ mod tests { bytes.extend_from_slice(&0u16.to_be_bytes()); // extraData bytes.extend_from_slice(&[0; 8 + 4 + 4 + 1 + 8]); // clockInfo + firmwareVersion bytes.extend_from_slice(&u32::MAX.to_be_bytes()); // pcrSelect count - assert!(matches!(TpmsAttest::parse(&bytes), Err(AttestError::PcrSelectionCount))); + assert!(matches!(TpmsAttest::parse(&bytes), Err(AttestError::PcrSelectionBanks(u32::MAX)))); + } + + #[test] + fn rejects_trailing_bytes() { + let mut bytes = build_attest(b"x", ALL_24_PCRS, &[0; 32]); + bytes.push(0); + assert!(matches!(TpmsAttest::parse(&bytes), Err(AttestError::TrailingData))); + } + + #[test] + fn rejects_truncation_at_every_length() { + let bytes = build_attest(b"x", ALL_24_PCRS, &[0; 32]); + for len in 0..bytes.len() { + assert!( + matches!(TpmsAttest::parse(&bytes[..len]), Err(AttestError::Truncated)), + "unexpected result at length {len}" + ); + } } } diff --git a/crates/attestation/src/azure/verify.rs b/crates/attestation/src/azure/verify.rs index 49daf37..52dca43 100644 --- a/crates/attestation/src/azure/verify.rs +++ b/crates/attestation/src/azure/verify.rs @@ -223,9 +223,7 @@ fn finish_azure_attestation_verification( let vtpm_quote = tpm_attestation.quote; let hcl_ak_pub_der = hcl_ak_pub.key.try_to_der().map_err(|_| MaaError::JwkConversion)?; let pub_key = PKey::public_key_from_der(&hcl_ak_pub_der)?; - vtpm_quote.verify(&pub_key, &expected_input_data[..32])?; - - let pcrs = vtpm_quote.pcrs_sha256(); + let pcrs = vtpm_quote.verify(&pub_key, &expected_input_data[..32])?; // Parse AK certificate let (_type_label, ak_certificate_der) = @@ -257,7 +255,7 @@ fn finish_azure_attestation_verification( now, )?; - Ok(MultiMeasurements::from_pcrs(pcrs)) + Ok(MultiMeasurements::from_indexed_pcrs(pcrs)) } /// Extract the measurements from the attestation, but do not verify @@ -268,8 +266,8 @@ pub fn get_measurements(input: &[u8]) -> Result { let attestation_document: AttestationDocument = serde_json::from_slice(input)?; let vtpm_quote = attestation_document.tpm_attestation.quote; - let pcrs = vtpm_quote.pcrs_sha256(); - Ok(MultiMeasurements::from_pcrs(pcrs)) + let pcrs = vtpm_quote.indexed_pcrs_unverified()?; + Ok(MultiMeasurements::from_indexed_pcrs(pcrs)) } /// JSON Web Key used in [HclRuntimeClaims] diff --git a/crates/attestation/src/measurements.rs b/crates/attestation/src/measurements.rs index dc36551..e23d72b 100644 --- a/crates/attestation/src/measurements.rs +++ b/crates/attestation/src/measurements.rs @@ -164,6 +164,15 @@ impl fmt::Debug for DcapMeasurements { #[derive(Clone, PartialEq)] pub enum MultiMeasurements { Dcap(DcapMeasurements), + /// Azure vTPM PCR values, keyed by the register each one measures. + /// + /// The keys are the registers the quote attested, named by its + /// `pcrSelect` bitmap. That is any subset the sending party chose, so a + /// register absent from the map was not attested. Azure's own attester + /// selects all 24 of a vTPM's registers, but nothing here requires it. + /// + /// Values are SHA-256 digests, the only PCR bank a quote may select + /// here. Azure(HashMap), NoAttestation, } @@ -217,6 +226,16 @@ impl fmt::Debug for AzureHexDebug<'_> { pub enum ExpectedMeasurements { Image(DcapImageHashes), Dcap(HashMap>), + /// Accepted Azure vTPM PCR values, keyed by PCR register. + /// + /// These are the registers the policy constrains, not the registers a + /// quote carries. Keys come from the policy's field names, spelled + /// either `"4"` or `"pcr4"`, which [`parse_azure_pcr_index`] rejects + /// above 23. Each register maps to every value that satisfies it, so + /// one policy can accept several images. + /// + /// A register absent from the map is unconstrained. A register present + /// here but absent from the evidence rejects that evidence. Azure(HashMap>), NoAttestation, } @@ -298,8 +317,15 @@ impl MultiMeasurements { ))) } - pub fn from_pcrs<'a>(pcrs: impl Iterator) -> Self { - Self::Azure(pcrs.copied().enumerate().map(|(index, value)| (index as u32, value)).collect()) + /// Azure measurements from PCR values already paired with the registers + /// they measure. + /// + /// The pairing belongs to the quote — its `pcrSelect` bitmap says which + /// registers it attests — so it arrives here rather than being inferred + /// from list position. `pcrN` in a measurement policy names register N, + /// and nothing else may decide that. + pub fn from_indexed_pcrs(pcrs: impl IntoIterator) -> Self { + Self::Azure(pcrs.into_iter().collect()) } }