Use a single TPM context and avoid race conditions during tests

This implements a singleton to use a single instance of the
tss_espi::Context.

This also avoids race conditions by using a thread-safe mutex to access
the context.

The tests were modified to cleanup generated key handles between test
cases, preventing leftover handles to fill the TPM memory.

Introduce a mutex to allow only a single test that create keys in the
TPM to run at once.  It is required that tests that create keys in the
TPM to run the tpm::testing::lock_tests() to obtain the mutex and be
able to continue executing without the risk of other tests filling the
TPM memory.

The QuoteData::fixture implementation (used only during tests) was
modified to flush the created key contexts when dropped. It also was
modified to lock the global mutex by calling the
tpm::testing::lock_tests() and provide the respective guard, preventing
other tests that use the fixture (which necessarily create TPM keys) to
run in parallel.

Note that it is necessary to drop the QuoteData explicitly to guarantee
that the keys are flushed before releasing the global mutex.

Finally, the following changes were made to the tests/run.sh:

* Do not use tpm2-abrmd anymore, and use only the swtpm socket instead
* Stop the started processes at exit

With all these changes, the tests/run.sh can now be executed locally
without any special setting.

Signed-off-by: Anderson Toshiyuki Sasaki <[email protected]>

Author: ansasaki

Cargo.lock

@@ -21,7 +21,7 @@ pub(crate) struct AgentInfo {
 // 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 @@ mod tests {
 
     #[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 @@ mod tests {
         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/agent_handler.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/common.rs

@@ -133,7 +133,7 @@ fn try_combine_keys(
 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 u_key(
 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 v_key(
 
 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 get_symm_key(
 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 @@ mod tests {
     #[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 @@ mod tests {
         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 @@ mod tests {
     #[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 @@ mod tests {
         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/keys_handler.rs

@@ -98,8 +98,8 @@ static NOTFOUND: &[u8] = b"Not Found";
 // 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 @@ async fn main() -> Result<()> {
 
     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 @@ async fn main() -> Result<()> {
         } 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 @@ async fn main() -> Result<()> {
             /// 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 @@ async fn main() -> Result<()> {
         )?;
         // 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 =
@@ -1062,6 +1054,11 @@ mod testing {
     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
@@ -1093,8 +1090,22 @@ mod testing {
         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()?;
 
@@ -1103,9 +1114,6 @@ mod testing {
                     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(),
             )?;
@@ -1115,14 +1123,19 @@ mod testing {
                     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")
@@ -1177,28 +1190,31 @@ mod testing {
                     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/main.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);
     }
 }