Make review changes

az-cvm-vtpm/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "az-cvm-vtpm"
-version = "0.4.1"
+version = "0.4.2"
 edition = "2021"
 repository = "https://github.com/kinvolk/azure-cvm-tooling/"
 license = "MIT"
@@ -12,7 +12,7 @@ description = "Package with shared code for Azure Confidential VMs"
 members = [
 	"az-snp-vtpm",
 	"az-tdx-vtpm",
-    "az-snp-vtpm/example",
+	"az-snp-vtpm/example",
 ]
 
 [lib]

az-cvm-vtpm/src/vtpm/mod.rs

@@ -3,9 +3,9 @@
 
 use rsa::{BigUint, RsaPublicKey};
 use serde::{Deserialize, Serialize};
-use sha2::{Digest, Sha256};
 use thiserror::Error;
 use tss_esapi::abstraction::nv;
+use tss_esapi::abstraction::pcr;
 use tss_esapi::abstraction::public::DecodedKey;
 use tss_esapi::handles::TpmHandle;
 use tss_esapi::interface_types::algorithm::HashingAlgorithm;
@@ -112,6 +112,10 @@ pub enum QuoteError {
     NotAQuote,
     #[error("Wrong signature, that should not occur")]
     WrongSignature,
+    #[error("PCR bank not found")]
+    PcrBankNotFound,
+    #[error("PCR reading error")]
+    PcrRead,
 }
 
 #[derive(Serialize, Deserialize, Debug, Clone)]
@@ -128,24 +132,6 @@ impl Quote {
         let nonce = attest.extra_data().to_vec();
         Ok(nonce)
     }
-
-    pub fn verify_pcrs(&self) -> Result<bool, QuoteError> {
-        let attest = Attest::unmarshall(&self.message)?;
-        let AttestInfo::Quote { info } = attest.attested() else {
-            return Err(QuoteError::NotAQuote);
-        };
-
-        let pcr_digest = info.pcr_digest();
-
-        // Read hashes of all the PCRs.
-        let mut hasher = Sha256::new();
-        for pcr in self.pcrs.iter() {
-            hasher.update(pcr);
-        }
-        let digest = hasher.finalize();
-
-        Ok(digest[..] == pcr_digest[..])
-    }
 }
 
 /// Get a signed vTPM Quote
@@ -172,8 +158,12 @@ pub fn get_quote(data: &[u8]) -> Result<Quote, QuoteError> {
     let auth_session = AuthSession::Password;
     context.set_sessions((Some(auth_session), None, None));
 
-    let (attest, signature) =
-        context.quote(key_handle.into(), quote_data, scheme, selection_list)?;
+    let (attest, signature) = context.quote(
+        key_handle.into(),
+        quote_data,
+        scheme,
+        selection_list.clone(),
+    )?;
 
     let AttestInfo::Quote { .. } = attest.attested() else {
         return Err(QuoteError::NotAQuote);
@@ -185,9 +175,17 @@ pub fn get_quote(data: &[u8]) -> Result<Quote, QuoteError> {
     let signature = rsa_sig.signature().to_vec();
     let message = attest.marshall()?;
 
-    // Drop the context because access to the tpm device is again needed in the `read_all_pcrs` function.
-    drop(context);
-    let pcrs = read_all_pcrs()?;
+    context.clear_sessions();
+    let pcr_data = pcr::read_all(&mut context, selection_list)?;
+
+    let pcr_bank = pcr_data
+        .pcr_bank(hash_algo)
+        .ok_or(QuoteError::PcrBankNotFound)?;
+
+    let pcrs = pcr_bank
+        .into_iter()
+        .map(|(_, x)| x.value().to_vec())
+        .collect();
 
     Ok(Quote {
         signature,
@@ -196,29 +194,6 @@ pub fn get_quote(data: &[u8]) -> Result<Quote, QuoteError> {
     })
 }
 
-/// Extract the 256 bank of PCRs from the TPM.
-fn read_all_pcrs() -> Result<Vec<Vec<u8>>, QuoteError> {
-    let mut pcrs: Vec<Vec<u8>> = Vec::new();
-
-    let conf: TctiNameConf = TctiNameConf::Device(DeviceConfig::default());
-    let mut context = Context::new(conf)?;
-    let hash_algo = HashingAlgorithm::Sha256;
-
-    // Reading one PCR slot at a time instead of feeding all the slots to the
-    // PcrSelectionListBuilder because `pcr_read` can only return limited results.
-    for slot in VTPM_QUOTE_PCR_SLOTS.iter() {
-        let selection_list = PcrSelectionListBuilder::new()
-            .with_selection(hash_algo, &[*slot])
-            .build()?;
-
-        let (_, _, digest_list) = context.pcr_read(selection_list)?;
-        let v = &digest_list.value()[0];
-        pcrs.push(v.value().to_vec());
-    }
-
-    Ok(pcrs)
-}
-
 #[cfg(feature = "verifier")]
 #[derive(Error, Debug)]
 pub enum VerifyError {

az-cvm-vtpm/src/vtpm/verify.rs

@@ -5,7 +5,10 @@ use super::{Quote, QuoteError};
 use openssl::hash::MessageDigest;
 use openssl::pkey::{PKey, Public};
 use openssl::sign::Verifier;
+use sha2::{Digest, Sha256};
 use thiserror::Error;
+use tss_esapi::structures::{Attest, AttestInfo};
+use tss_esapi::traits::UnMarshall;
 
 #[derive(Error, Debug)]
 pub enum VerifyError {
@@ -19,6 +22,8 @@ pub enum VerifyError {
     NonceMismatch,
     #[error("quote error")]
     Quote(#[from] QuoteError),
+    #[error("pcr mismatch")]
+    PcrMismatch,
 }
 
 impl Quote {
@@ -36,6 +41,9 @@ impl Quote {
         if nonce != quote_nonce {
             return Err(VerifyError::NonceMismatch);
         }
+
+        self.verify_pcrs()?;
+
         Ok(())
     }
 
@@ -53,6 +61,30 @@ impl Quote {
         }
         Ok(())
     }
+
+    /// Verify a Quote's PCR values
+    ///
+    pub fn verify_pcrs(&self) -> Result<(), VerifyError> {
+        let attest = Attest::unmarshall(&self.message)?;
+        let AttestInfo::Quote { info } = attest.attested() else {
+            return Err(VerifyError::Quote(QuoteError::NotAQuote));
+        };
+
+        let pcr_digest = info.pcr_digest();
+
+        // Read hashes of all the PCRs.
+        let mut hasher = Sha256::new();
+        for pcr in self.pcrs.iter() {
+            hasher.update(pcr);
+        }
+        let digest = hasher.finalize();
+
+        if digest[..] != pcr_digest[..] {
+            return Err(VerifyError::PcrMismatch);
+        }
+
+        Ok(())
+    }
 }
 
 #[cfg(test)]
@@ -72,7 +104,24 @@ mod tests {
         // `tpm2_quote -c 0x81000003 -l sha256:5,8 -q cafe -m quote_msg -s quote_sig`
         let message = include_bytes!("../../test/quote_msg").to_vec();
         let signature = include_bytes!("../../test/quote_sig").to_vec();
-        let quote = Quote { signature, message };
+
+        // // The PCR values of sha256 bank can be retrived using the following command:
+        // // tpm2_pcrread sha256:0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23 | grep -v sha256 | cut -d'x' -f2 | xxd -r -p > pcrs_256.bin
+        // let pcrs: Vec<Vec<u8>> =
+        //     fs::read(env!("CARGO_MANIFEST_DIR").to_owned() + "/test/pcrs_256.bin")
+        //         .unwrap()
+        //         .chunks(32)
+        //         .map(|chunk| chunk.to_vec())
+        //         .collect();
+
+        // Dummy PCR value.
+        let pcrs = Vec::new();
+
+        let quote = Quote {
+            signature,
+            message,
+            pcrs,
+        };
 
         // proper nonce in message
         let nonce = vec![1, 2, 3];
@@ -98,4 +147,27 @@ mod tests {
             "Expected nonce verification error"
         );
     }
+
+    #[test]
+    fn test_pcr_values() {
+        /// Generate the pcrs.bin using the following code:
+        ///
+        /// use az_snp_vtpm::vtpm;
+        /// use bincode;
+        /// use std::error::Error;
+        /// use std::fs;
+        ///
+        /// fn main() -> Result<(), Box<dyn Error>> {
+        ///     let nonce = "challenge".as_bytes().to_vec();
+        ///     let quote = vtpm::get_quote(&nonce)?;
+        ///     let quote_encoded: Vec<u8> = bincode::serialize(&quote).unwrap();
+        ///     fs::write("/tmp/pcrs.bin", quote_encoded).expect("Unable to write file");
+        ///     Ok(())
+        /// }
+        ///
+        let quote_bytes: Vec<u8> = include_bytes!("../../test/pcrs.bin").to_vec();
+        let quote: Quote = bincode::deserialize(&quote_bytes[..]).unwrap();
+        let result = quote.verify_pcrs();
+        assert!(result.is_ok(), "PCR verification should not fail");
+    }
 }