Fix for handles not being unique.

- Handles returned from tpm2-tss APIs does not need to be unique.
  This an attempt to fix problem #383 by removing the check in
  HandleManager that made a check for uniqueness. This has been replaced
  with handle consistency check.

- Enables the integration tests that was disabled due to this issue.

Co-authored-by: Ionut Mihalcea <[email protected]>
Co-authored-by: Jesper Brynolf <[email protected]>

Signed-off-by: Jesper Brynolf <[email protected]>

Author: Superhepper

tss-esapi/src/abstraction/nv.rs

@@ -42,6 +42,8 @@ pub fn read_full(
 }
 
 /// Returns the NvPublic and Name associated with an NV index TPM handle
+///
+/// NOTE: This call _may_ close existing ESYS handles to the NV Index.
 fn get_nv_index_info(
     context: &mut Context,
     nv_index_tpm_handle: NvIndexTpmHandle,
@@ -63,6 +65,8 @@ fn get_nv_index_info(
 }
 
 /// Lists all the currently defined NV Indexes' names and public components
+///
+/// NOTE: This call _may_ close existing ESYS handles to the NV Index.
 pub fn list(context: &mut Context) -> Result<Vec<(NvPublic, Name)>> {
     context.execute_without_session(|ctx| {
         ctx.get_capability(
@@ -166,6 +170,8 @@ pub fn max_nv_buffer_size(ctx: &mut Context) -> Result<usize> {
 /// Provides methods and trait implementations to interact with a non-volatile storage index that has been opened.
 ///
 /// Use [`NvOpenOptions::open`] to obtain an [`NvReaderWriter`] object.
+///
+/// NOTE: When the `NvReaderWriter` is dropped, any existing ESYS handles to NV Indexes _may_ be closed.
 #[derive(Debug)]
 pub struct NvReaderWriter<'a> {
     context: &'a mut Context,

tss-esapi/src/context/handle_manager.rs

@@ -40,10 +40,14 @@ impl HandleManager {
             return Err(Error::local_error(WrapperErrorKind::InvalidParam));
         }
 
-        if self.open_handles.contains_key(&handle) {
-            error!("Handle({}) is already open", ESYS_TR::from(handle));
-            return Err(Error::local_error(WrapperErrorKind::InvalidHandleState));
+        // The TSS might return the same handle, see #383
+        if let Some(stored_handle_drop_action) = self.open_handles.get(&handle) {
+            if handle_drop_action != *stored_handle_drop_action {
+                error!("Handle drop action inconsistency");
+                return Err(Error::local_error(WrapperErrorKind::InconsistentParams));
+            }
         }
+
         let _ = self.open_handles.insert(handle, handle_drop_action);
         Ok(())
     }

tss-esapi/tests/integration_tests/abstraction_tests/ek_tests.rs

@@ -8,7 +8,6 @@ use tss_esapi::{
 
 use crate::common::create_ctx_without_session;
 
-#[ignore = "issues with tpm2-tss"]
 #[test]
 fn test_retrieve_ek_pubcert() {
     let mut context = create_ctx_without_session();

tss-esapi/tests/integration_tests/abstraction_tests/nv_tests.rs

@@ -60,7 +60,6 @@ fn write_nv_index(context: &mut Context, nv_index: NvIndexTpmHandle) -> NvIndexH
     owner_nv_index_handle
 }
 
-#[ignore = "issues with tpm2-tss"]
 #[test]
 fn list() {
     let mut context = create_ctx_with_session();
@@ -73,31 +72,52 @@ fn list() {
         .map(|(public, _)| public.nv_index())
         .any(|x| x == nv_index));
 
-    let owner_nv_index_handle = write_nv_index(&mut context, nv_index);
+    let initial_owner_nv_index_handle = write_nv_index(&mut context, nv_index);
 
     assert!(nv::list(&mut context)
         .unwrap()
         .iter()
         .map(|(public, _)| public.nv_index())
         .any(|x| x == nv_index));
 
+    // Need to get the ESYS handle again, as it was closed by nv::list above.
+    // 1. If this fails with a TssError something is seriously wrong but it
+    // does not hurt to to try to clean the NV space up using the initial handle.
+    //
+    // 2. If this fails with a WrapperError then there is only an inconsistency
+    // in how the handle should be closed and cleaning up the NV space should
+    // using the initial handle should still be possible.
+    let owner_nv_index_handle = context
+        .tr_from_tpm_public(nv_index.into())
+        .map_or_else(|_| initial_owner_nv_index_handle, NvIndexHandle::from);
+
     context
         .nv_undefine_space(Provision::Owner, owner_nv_index_handle)
         .expect("Call to nv_undefine_space failed");
 }
 
-#[ignore = "issues with tpm2-tss"]
 #[test]
 fn read_full() {
     let mut context = create_ctx_with_session();
 
     let nv_index = NvIndexTpmHandle::new(0x01500015).unwrap();
 
-    let owner_nv_index_handle = write_nv_index(&mut context, nv_index);
+    let initial_owner_nv_index_handle = write_nv_index(&mut context, nv_index);
 
     // Now read it back
     let read_result = nv::read_full(&mut context, NvAuth::Owner, nv_index);
 
+    // Need to get the ESYS handle again, as it was closed by nv::list above.
+    // 1. If this fails with a TssError something is seriously wrong but it
+    // does not hurt to to try to clean the NV space up using the initial handle.
+    //
+    // 2. If this fails with a WrapperError then there is only an inconsistency
+    // in how the handle should be closed and cleaning up the NV space should
+    // using the initial handle should still be possible.
+    let owner_nv_index_handle = context
+        .tr_from_tpm_public(nv_index.into())
+        .map_or_else(|_| initial_owner_nv_index_handle, NvIndexHandle::from);
+
     context
         .nv_undefine_space(Provision::Owner, owner_nv_index_handle)
         .expect("Call to nv_undefine_space failed");