Use a single TPM context and avoid race conditions during tests

keylime-agent/src/agent_handler.rs

@@ -21,7 +21,7 @@
 // It should return a AgentInfo object as JSON
 async fn info(
     req: HttpRequest,
-    data: web::Data<QuoteData>,
+    data: web::Data<QuoteData<'_>>,
 ) -> impl Responder {
     debug!("Returning agent information");
 
@@ -87,7 +87,7 @@
 
     #[actix_rt::test]
     async fn test_agent_info() {
-        let mut quotedata = QuoteData::fixture().unwrap(); //#[allow_ci]
+        let (mut quotedata, mutex) = QuoteData::fixture().await.unwrap(); //#[allow_ci]
         quotedata.hash_alg = keylime::algorithms::HashAlgorithm::Sha256;
         quotedata.enc_alg = keylime::algorithms::EncryptionAlgorithm::Rsa;
         quotedata.sign_alg = keylime::algorithms::SignAlgorithm::RsaSsa;
@@ -112,6 +112,9 @@
         assert_eq!(result.results.tpm_hash_alg.as_str(), "sha256");
         assert_eq!(result.results.tpm_enc_alg.as_str(), "rsa");
         assert_eq!(result.results.tpm_sign_alg.as_str(), "rsassa");
+
+        // Explicitly drop QuoteData to cleanup keys
+        drop(data);
     }
 
     #[actix_rt::test]

keylime-agent/src/common.rs

@@ -274,9 +274,10 @@ mod tests {
         Context,
     };
 
+    #[tokio::test]
     #[cfg(feature = "testing")]
-    #[test]
-    fn test_agent_data() -> Result<()> {
+    async fn test_agent_data() -> Result<()> {
+        let _mutex = tpm::testing::lock_tests().await;
         let mut config = KeylimeConfig::default();
 
         let mut ctx = tpm::Context::new()?;
@@ -319,13 +320,21 @@ mod tests {
             tpm_signing_alg,
             ek_hash.as_bytes(),
         );
+
         assert!(valid);
+
+        // Cleanup created keys
+        let ak_handle = ctx.load_ak(ek_result.key_handle, &ak)?;
+        ctx.flush_context(ak_handle.into());
+        ctx.flush_context(ek_result.key_handle.into());
+
         Ok(())
     }
 
+    #[tokio::test]
     #[cfg(feature = "testing")]
-    #[test]
-    fn test_hash() -> Result<()> {
+    async fn test_hash() -> Result<()> {
+        let _mutex = tpm::testing::lock_tests().await;
         let mut config = KeylimeConfig::default();
 
         let mut ctx = tpm::Context::new()?;
@@ -342,6 +351,10 @@ mod tests {
         let result = hash_ek_pubkey(ek_result.public);
 
         assert!(result.is_ok());
+
+        // Cleanup created keys
+        ctx.flush_context(ek_result.key_handle.into());
+
         Ok(())
     }
 }

keylime-agent/src/keys_handler.rs

@@ -133,7 +133,7 @@
 async fn u_key(
     body: web::Json<KeylimeUKey>,
     req: HttpRequest,
-    quote_data: web::Data<QuoteData>,
+    quote_data: web::Data<QuoteData<'_>>,
 ) -> impl Responder {
     debug!("Received ukey");
 
@@ -247,7 +247,7 @@
 async fn v_key(
     body: web::Json<KeylimeVKey>,
     req: HttpRequest,
-    quote_data: web::Data<QuoteData>,
+    quote_data: web::Data<QuoteData<'_>>,
 ) -> impl Responder {
     debug!("Received vkey");
 
@@ -317,7 +317,7 @@
 
 async fn pubkey(
     req: HttpRequest,
-    data: web::Data<QuoteData>,
+    data: web::Data<QuoteData<'_>>,
 ) -> impl Responder {
     match crypto::pkey_pub_to_pem(&data.pub_key) {
         Ok(pubkey) => {
@@ -367,7 +367,7 @@
 async fn verify(
     param: web::Query<KeylimeChallenge>,
     req: HttpRequest,
-    data: web::Data<QuoteData>,
+    data: web::Data<QuoteData<'_>>,
 ) -> impl Responder {
     if param.challenge.is_empty() {
         warn!(
@@ -875,7 +875,7 @@
     #[cfg(feature = "testing")]
     async fn test_u_or_v_key(key_len: usize, payload: Option<&[u8]>) {
         let test_config = KeylimeConfig::default();
-        let mut fixture = QuoteData::fixture().unwrap(); //#[allow_ci]
+        let (mut fixture, mutex) = QuoteData::fixture().await.unwrap(); //#[allow_ci]
 
         // Create temporary working directory and secure mount
         let temp_workdir = tempfile::tempdir().unwrap(); //#[allow_ci]
@@ -1073,6 +1073,9 @@
         keys_tx.send((KeyMessage::Shutdown, None)).await.unwrap(); //#[allow_ci]
         payload_tx.send(PayloadMessage::Shutdown).await.unwrap(); //#[allow_ci]
         arbiter.join();
+
+        // Explicitly drop QuoteData to cleanup keys
+        drop(quotedata);
     }
 
     #[cfg(feature = "testing")]
@@ -1090,7 +1093,8 @@
     #[cfg(feature = "testing")]
     #[actix_rt::test]
     async fn test_pubkey() {
-        let quotedata = web::Data::new(QuoteData::fixture().unwrap()); //#[allow_ci]
+        let (fixture, mutex) = QuoteData::fixture().await.unwrap(); //#[allow_ci]
+        let quotedata = web::Data::new(fixture);
         let mut app =
             test::init_service(App::new().app_data(quotedata.clone()).route(
                 &format!("/{API_VERSION}/keys/pubkey"),
@@ -1110,5 +1114,8 @@
         assert!(pkey_pub_from_pem(&result.results.pubkey)
             .unwrap() //#[allow_ci]
             .public_eq(&quotedata.pub_key));
+
+        // Explicitly drop QuoteData to cleanup keys
+        drop(quotedata);
     }
 }

keylime-agent/src/main.rs

@@ -98,8 +98,8 @@
 // This data is passed in to the actix httpserver threads that
 // handle quotes.
 #[derive(Debug)]
-pub struct QuoteData {
-    tpmcontext: Mutex<tpm::Context>,
+pub struct QuoteData<'a> {
+    tpmcontext: Mutex<tpm::Context<'a>>,
     priv_key: PKey<Private>,
     pub_key: PKey<Public>,
     ak_handle: KeyHandle,
@@ -273,14 +273,6 @@
 
     let mut ctx = tpm::Context::new()?;
 
-    //  Retrieve the TPM Vendor, this allows us to warn if someone is using a
-    // Software TPM ("SW")
-    if tss_esapi::utils::get_tpm_vendor(ctx.as_mut())?.contains("SW") {
-        warn!("INSECURE: Keylime is currently using a software TPM emulator rather than a real hardware TPM.");
-        warn!("INSECURE: The security of Keylime is NOT linked to a hardware root of trust.");
-        warn!("INSECURE: Only use Keylime in this mode for testing or debugging purposes.");
-    }
-
     cfg_if::cfg_if! {
         if #[cfg(feature = "legacy-python-actions")] {
             warn!("The support for legacy python revocation actions is deprecated and will be removed on next major release");
@@ -313,7 +305,7 @@
         } else {
             Auth::try_from(tpm_ownerpassword.as_bytes())?
         };
-        ctx.as_mut().tr_set_auth(Hierarchy::Endorsement.into(), auth)
+        ctx.tr_set_auth(Hierarchy::Endorsement.into(), auth)
             .map_err(|e| {
                 Error::Configuration(config::KeylimeConfigError::Generic(format!(
                     "Failed to set TPM context password for Endorsement Hierarchy: {e}"
@@ -406,7 +398,7 @@
             /// If handle is not set in config, recreate IDevID according to template
             info!("Recreating IDevID.");
             let regen_idev = ctx.create_idevid(asym_alg, name_alg)?;
-            ctx.as_mut().flush_context(regen_idev.handle.into())?;
+            ctx.flush_context(regen_idev.handle.into())?;
             // Flush after creating to make room for AK and EK and IAK
             regen_idev
         } else {
@@ -780,7 +772,7 @@
         )?;
         // Flush EK if we created it
         if config.agent.ek_handle.is_empty() {
-            ctx.as_mut().flush_context(ek_result.key_handle.into())?;
+            ctx.flush_context(ek_result.key_handle.into())?;
         }
         let mackey = general_purpose::STANDARD.encode(key.value());
         let auth_tag =
@@ -1052,6 +1044,11 @@
     use crate::{config::KeylimeConfig, crypto::CryptoError};
     use thiserror::Error;
 
+    use std::sync::{Arc, Mutex, OnceLock};
+    use tokio::sync::{Mutex as AsyncMutex, MutexGuard as AsyncMutexGuard};
+
+    use keylime::tpm::testing::lock_tests;
+
     #[derive(Error, Debug)]
     pub(crate) enum MainTestError {
         /// Algorithm error
@@ -1083,8 +1080,22 @@
         TSSError(#[from] tss_esapi::Error),
     }
 
-    impl QuoteData {
-        pub(crate) fn fixture() -> std::result::Result<Self, MainTestError> {
+    impl<'a> Drop for QuoteData<'a> {
+        /// Flush the created AK when dropping
+        fn drop(&mut self) {
+            self.tpmcontext
+                .lock()
+                .unwrap() //#[allow_ci]
+                .flush_context(self.ak_handle.into());
+        }
+    }
+
+    impl<'a> QuoteData<'a> {
+        pub(crate) async fn fixture() -> std::result::Result<
+            (Self, AsyncMutexGuard<'static, ()>),
+            MainTestError,
+        > {
+            let mutex = lock_tests().await;
             let test_config = KeylimeConfig::default();
             let mut ctx = tpm::Context::new()?;
 
@@ -1093,9 +1104,6 @@
                     test_config.agent.tpm_encryption_alg.as_str(),
                 )?;
 
-            // Gather EK and AK key values and certs
-            let ek_result = ctx.create_ek(tpm_encryption_alg, None)?;
-
             let tpm_hash_alg = keylime::algorithms::HashAlgorithm::try_from(
                 test_config.agent.tpm_hash_alg.as_str(),
             )?;
@@ -1105,14 +1113,19 @@
                     test_config.agent.tpm_signing_alg.as_str(),
                 )?;
 
-            let ak_result = ctx.create_ak(
-                ek_result.key_handle,
-                tpm_hash_alg,
-                tpm_signing_alg,
-            )?;
-            let ak_handle = ctx.load_ak(ek_result.key_handle, &ak_result)?;
-            let ak_tpm2b_pub =
-                PublicBuffer::try_from(ak_result.public)?.marshall()?;
+            // Gather EK and AK key values and certs
+            let ek_result = ctx.create_ek(tpm_encryption_alg, None).unwrap(); //#[allow_ci]
+            let ak_result = ctx
+                .create_ak(
+                    ek_result.key_handle,
+                    tpm_hash_alg,
+                    tpm_signing_alg,
+                )
+                .unwrap(); //#[allow_ci]
+            let ak_handle =
+                ctx.load_ak(ek_result.key_handle, &ak_result).unwrap(); //#[allow_ci]
+
+            ctx.flush_context(ek_result.key_handle.into()).unwrap(); //#[allow_ci]
 
             let rsa_key_path = Path::new(env!("CARGO_MANIFEST_DIR"))
                 .join("test-data")
@@ -1167,28 +1180,31 @@
                     Err(err) => None,
                 };
 
-            Ok(QuoteData {
-                tpmcontext: Mutex::new(ctx),
-                priv_key: nk_priv,
-                pub_key: nk_pub,
-                ak_handle,
-                keys_tx,
-                payload_tx,
-                revocation_tx,
-                hash_alg: keylime::algorithms::HashAlgorithm::Sha256,
-                enc_alg: keylime::algorithms::EncryptionAlgorithm::Rsa,
-                sign_alg: keylime::algorithms::SignAlgorithm::RsaSsa,
-                agent_uuid: test_config.agent.uuid,
-                allow_payload_revocation_actions: test_config
-                    .agent
-                    .allow_payload_revocation_actions,
-                secure_size: test_config.agent.secure_size,
-                work_dir,
-                ima_ml_file,
-                measuredboot_ml_file,
-                ima_ml: Mutex::new(MeasurementList::new()),
-                secure_mount,
-            })
+            Ok((
+                QuoteData {
+                    tpmcontext: Mutex::new(ctx),
+                    priv_key: nk_priv,
+                    pub_key: nk_pub,
+                    ak_handle,
+                    keys_tx,
+                    payload_tx,
+                    revocation_tx,
+                    hash_alg: keylime::algorithms::HashAlgorithm::Sha256,
+                    enc_alg: keylime::algorithms::EncryptionAlgorithm::Rsa,
+                    sign_alg: keylime::algorithms::SignAlgorithm::RsaSsa,
+                    agent_uuid: test_config.agent.uuid,
+                    allow_payload_revocation_actions: test_config
+                        .agent
+                        .allow_payload_revocation_actions,
+                    secure_size: test_config.agent.secure_size,
+                    work_dir,
+                    ima_ml_file,
+                    measuredboot_ml_file,
+                    ima_ml: Mutex::new(MeasurementList::new()),
+                    secure_mount,
+                },
+                mutex,
+            ))
         }
     }
 }

keylime-agent/src/notifications_handler.rs

@@ -15,7 +15,7 @@ use std::path::{Path, PathBuf};
 async fn revocation(
     body: web::Json<Revocation>,
     req: HttpRequest,
-    data: web::Data<QuoteData>,
+    data: web::Data<QuoteData<'_>>,
 ) -> impl Responder {
     info!("Received revocation");
 
@@ -138,7 +138,7 @@ mod tests {
             PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("tests/actions"),
         );
 
-        let mut fixture = QuoteData::fixture().unwrap(); //#[allow_ci]
+        let (mut fixture, mutex) = QuoteData::fixture().await.unwrap(); //#[allow_ci]
 
         // Replace the channels on the fixture with some local ones
         let (mut revocation_tx, mut revocation_rx) =
@@ -188,5 +188,8 @@ mod tests {
 
         let resp = test::call_service(&app, req).await;
         assert!(resp.status().is_success());
+
+        // Explicitly drop QuoteData to cleanup keys
+        drop(quotedata);
     }
 }