FAPI: Cleanup policy error handling.

Various errors occurred during policy error handling. The flush for esys policy objects was sometimes executed several times. The policy error handling is now unified. Unneeded duplicate code is removed. Unneeded code is removed. The state was not initialized correctly in error cases in the policy utility state machines. Signed-off-by: Juergen Repp <[email protected]>
JuergenReppSIT · Feb 18, 2024 · f87820d · f87820d
1 parent 0b19f25
commit f87820d
Show file tree

Hide file tree

Showing 9 changed files with 48 additions and 55 deletions.
diff --git a/src/tss2-fapi/api/Fapi_Encrypt.c b/src/tss2-fapi/api/Fapi_Encrypt.c
@@ -435,7 +435,8 @@ Fapi_Encrypt_Finish(
 
 error_cleanup:
     /* Cleanup any intermediate results and state stored in the context. */
-    if (command->key_handle != ESYS_TR_NONE)
+    if (command->key_handle != ESYS_TR_NONE &&
+        command->key_object && !command->key_object->misc.key.persistent_handle)
         Esys_FlushContext(context->esys,  command->key_handle);
     if (r)
         SAFE_FREE(command->cipherText);

diff --git a/src/tss2-fapi/api/Fapi_ExportKey.c b/src/tss2-fapi/api/Fapi_ExportKey.c
@@ -430,6 +430,8 @@ Fapi_ExportKey_Finish(
             return_try_again(r);
             goto_if_error(r, "Flush key", cleanup);
 
+            command->key_object->public.handle = ESYS_TR_NONE;
+
             fallthrough;
 
         statecase(context->state, EXPORT_KEY_WAIT_FOR_FLUSH2);
@@ -438,6 +440,8 @@ Fapi_ExportKey_Finish(
             return_try_again(r);
             goto_if_error(r, "Flush key", cleanup);
 
+            command->handle_ext_key = ESYS_TR_NONE;
+
             fallthrough;
 
         statecase(context->state, EXPORT_KEY_CLEANUP)

diff --git a/src/tss2-fapi/api/Fapi_GetEsysBlob.c b/src/tss2-fapi/api/Fapi_GetEsysBlob.c
@@ -337,10 +337,6 @@ Fapi_GetEsysBlob_Finish(
             SAFE_FREE(key_context);
             goto_if_error(r, "Marshaling context", error_cleanup);
 
-            /* Cleanup policy session if an error did occur. */
-            ifapi_flush_policy_session(context, context->policy.session, r);
-            goto_if_error(r, "Cleanup policy session", error_cleanup);
-
             /* Flush current object used for blob computation. */
             if (!key_object->misc.key.persistent_handle) {
                 r = Esys_FlushContext_Async(context->esys, key_object->public.handle);

diff --git a/src/tss2-fapi/api/Fapi_Import.c b/src/tss2-fapi/api/Fapi_Import.c
@@ -652,6 +652,8 @@ Fapi_Import_Finish(
             if (!command->parent_object->misc.key.persistent_handle) {
                 r = ifapi_flush_object(context, command->parent_object->public.handle);
                 return_try_again(r);
+
+                command->parent_object->public.handle = ESYS_TR_NONE;
                 ifapi_cleanup_ifapi_object(command->parent_object);
                 goto_if_error(r, "Flush key", error_cleanup);
             } else {

diff --git a/src/tss2-fapi/fapi_int.h b/src/tss2-fapi/fapi_int.h
@@ -240,6 +240,7 @@ typedef struct {
     TPM2B_AUTH auth;            /**< The Password */
     IFAPI_NV nv_obj;            /**< The NV Object */
     ESYS_TR auth_index;         /**< The ESAPI handle of the authorization object */
+    ESYS_TR auth_session;       /**< The autorization session for a nv object */
     uint64_t bitmap;            /**< The bitmask for the SetBits command */
     IFAPI_NV_TEMPLATE public_templ; /**< The template for nv creation, adjusted
                                          appropriate by the passed flags */

diff --git a/src/tss2-fapi/fapi_util.c b/src/tss2-fapi/fapi_util.c
@@ -1156,7 +1156,6 @@ ifapi_session_init(FAPI_CONTEXT *context)
 
     context->session1 = ESYS_TR_NONE;
     context->session2 = ESYS_TR_NONE;
-    context->policy.session = ESYS_TR_NONE;
     context->srk_handle = ESYS_TR_NONE;
     return TSS2_RC_SUCCESS;
 }
@@ -1185,7 +1184,6 @@ ifapi_non_tpm_mode_init(FAPI_CONTEXT *context)
 
     context->session1 = ESYS_TR_NONE;
     context->session2 = ESYS_TR_NONE;
-    context->policy.session = ESYS_TR_NONE;
     context->srk_handle = ESYS_TR_NONE;
     return TSS2_RC_SUCCESS;
 }
@@ -1200,9 +1198,6 @@ ifapi_non_tpm_mode_init(FAPI_CONTEXT *context)
 void
 ifapi_session_clean(FAPI_CONTEXT *context)
 {
-    if (context->policy_session && context->policy_session != ESYS_TR_NONE) {
-        Esys_FlushContext(context->esys, context->policy_session);
-    }
     if (context->session1 != ESYS_TR_NONE && context->session1 != ESYS_TR_PASSWORD) {
         if (context->session1 == context->session2) {
             context->session2 = ESYS_TR_NONE;
@@ -1246,7 +1241,6 @@ ifapi_cleanup_session(FAPI_CONTEXT *context)
     TSS2_RC r;
 
     /* Policy sessions were closed after successful execution. */
-    context->policy_session = ESYS_TR_NONE;
 
     switch (context->cleanup_state) {
         statecase(context->cleanup_state, CLEANUP_INIT);
@@ -2096,27 +2090,6 @@ get_name_alg(FAPI_CONTEXT *context, IFAPI_OBJECT *object)
     }
 }
 
-/** Check whether policy session has to be flushed.
- *
- * Policy sessions with cleared continue session flag are not flushed in error
- * cases. Therefore the return code will be checked and if a policy session was
- * used the session will be flushed if the command was not executed successfully.
- *
- * @param[in,out] context for storing all state information.
- * @param[in] session the session to be checked whether flush is needed.
- * @param[in] r The return code of the command using the session.
- */
-void
-ifapi_flush_policy_session(FAPI_CONTEXT *context, ESYS_TR session, TSS2_RC r)
-{
-    if (session != context->session1) {
-        /* A policy session was used instead auf the default session. */
-        if (r != TSS2_RC_SUCCESS) {
-            Esys_FlushContext(context->esys, session);
-        }
-    }
-}
-
 /** State machine to authorize a key, a NV object of a hierarchy.
  *
  * @param[in,out] context for storing all state information.
@@ -2229,6 +2202,7 @@ ifapi_authorize_object(FAPI_CONTEXT *context, IFAPI_OBJECT *object, ESYS_TR *ses
 error:
     /* No policy call was executed session can be flushed */
     Esys_FlushContext(context->esys, *session);
+    *session = ESYS_TR_NONE;
     return r;
 }
 
@@ -2370,6 +2344,8 @@ ifapi_nv_write(
         r = ifapi_authorize_object(context, auth_object, &auth_session);
         FAPI_SYNC(r, "Authorize NV object.", error_cleanup);
 
+        context->nv_cmd.auth_session = auth_session;
+
         /* Prepare the writing to NV ram. */
         r = Esys_NV_Write_Async(context->esys,
                                 context->nv_cmd.auth_index,
@@ -2409,11 +2385,8 @@ ifapi_nv_write(
                 r = Esys_NV_Write_Async(context->esys,
                                         context->nv_cmd.auth_index,
                                         nv_index,
-                                        (!context->policy.session
-                                         || context->policy.session == ESYS_TR_NONE) ? context->session1 :
-                                        context->policy.session,
-                                        (context->policy.session && context->policy.session != ESYS_TR_NONE) ?
-                                        context->session2 : ESYS_TR_NONE,
+                                        context->nv_cmd.auth_session,
+                                        ENC_SESSION_IF_POLICY(context->nv_cmd.auth_session),
                                         ESYS_TR_NONE,
                                         aux_data,
                                         context->nv_cmd.data_idx);
@@ -2975,9 +2948,7 @@ ifapi_key_sign(
         context->Key_Sign.handle = sig_key_object->public.handle;
 
         r = ifapi_authorize_object(context, sig_key_object, &session);
-        FAPI_SYNC(r, "Authorize signature key.", cleanup);
-
-        context->policy.session = session;
+        return_try_again(r);
 
         r = ifapi_get_sig_scheme(context, sig_key_object, padding, digest, &sig_scheme);
         goto_if_error(r, "Get signature scheme", cleanup);
@@ -3000,7 +2971,6 @@ ifapi_key_sign(
                              &context->Key_Sign.signature);
         return_try_again(r);
         context->session2 = ESYS_TR_NONE;
-        ifapi_flush_policy_session(context, context->policy.session, r);
         goto_if_error(r, "Error: Sign", cleanup);
 
         /* Prepare the flushing of the signing key. */
@@ -3717,6 +3687,8 @@ ifapi_key_create(
             r = ifapi_flush_object(context, context->loadKey.handle);
             return_try_again(r);
             goto_if_error(r, "Flush key", error_cleanup);
+
+            context->loadKey.handle = ESYS_TR_NONE;
         }
 
         fallthrough;
@@ -4892,6 +4864,8 @@ ifapi_create_primary(
         return_try_again(r);
         goto_if_error(r, "Flush key", error_cleanup);
 
+        context->cmd.Key_Create.handle = ESYS_TR_NONE;
+
         fallthrough;
 
     statecase(context->cmd.Key_Create.state, KEY_CREATE_PRIMARY_WRITE_PREPARE);

diff --git a/src/tss2-fapi/fapi_util.h b/src/tss2-fapi/fapi_util.h
@@ -125,12 +125,6 @@ ifapi_nv_read(
     uint8_t     **data,
     size_t       *size);
 
-void
-ifapi_flush_policy_session(
-    FAPI_CONTEXT *context,
-    ESYS_TR session,
-    TSS2_RC r);
-
 TSS2_RC
 ifapi_nv_write(
     FAPI_CONTEXT *context,

diff --git a/src/tss2-fapi/ifapi_policy_execute.c b/src/tss2-fapi/ifapi_policy_execute.c
@@ -574,8 +574,10 @@ execute_policy_signed(
     SAFE_FREE(current_policy->buffer);
     SAFE_FREE(current_policy->pem_key);
     /* In error cases object might not have been flushed. */
-    if (current_policy->object_handle != ESYS_TR_NONE)
+    if (current_policy->object_handle != ESYS_TR_NONE)  {
         Esys_FlushContext(esys_ctx, current_policy->object_handle);
+        current_policy->object_handle = ESYS_TR_NONE;
+    }
     return r;
 }
 
@@ -745,9 +747,10 @@ execute_policy_authorize(
     }
 cleanup:
     /* In error cases object might not have been flushed. */
-    if (current_policy->object_handle != ESYS_TR_NONE)
+    if (current_policy->object_handle != ESYS_TR_NONE) {
         Esys_FlushContext(esys_ctx, current_policy->object_handle);
-
+        current_policy->object_handle = ESYS_TR_NONE;
+    }
     return r;
 }
 
@@ -955,6 +958,7 @@ execute_policy_secret(
     statecase(current_policy->state, POLICY_FLUSH_KEY);
         r = Esys_FlushContext_Finish(esys_ctx);
         try_again_or_error(r, "Flush key finish.");
+        current_policy->auth_handle = ESYS_TR_NONE;
         current_policy->state = POLICY_EXECUTE_INIT;
         break;
 
@@ -964,8 +968,9 @@ execute_policy_secret(
     return r;
 
  cleanup:
-    if (current_policy->flush_handle) {
+    if (current_policy->flush_handle && current_policy->auth_handle != ESYS_TR_NONE) {
          Esys_FlushContext(esys_ctx, current_policy->auth_handle);
+         current_policy->auth_handle = ESYS_TR_NONE;
     }
     SAFE_FREE(current_policy->nonceTPM);
     return r;
@@ -1907,7 +1912,6 @@ ifapi_policyeval_execute(
         if (r != TSS2_RC_SUCCESS) {
             if (do_flush) {
                 Esys_FlushContext(esys_ctx, current_policy->session);
-                current_policy->session = ESYS_TR_NONE;
             }
             ifapi_free_node_list(current_policy->policy_elements);
 

diff --git a/src/tss2-fapi/ifapi_policyutil_execute.c b/src/tss2-fapi/ifapi_policyutil_execute.c
@@ -122,8 +122,13 @@ create_session(
 
     case WAIT_FOR_CREATE_SESSION:
         r = Esys_StartAuthSession_Finish(context->esys, session);
-        if (r != TSS2_RC_SUCCESS)
+        if (r == TSS2_FAPI_RC_TRY_AGAIN) {
             return r;
+        }
+        if (r != TSS2_RC_SUCCESS) {
+            context->policy.create_session_state = CREATE_SESSION_INIT;
+            return r;
+        }
 
         context->policy.create_session_state = CREATE_SESSION_INIT;
         break;
@@ -284,8 +289,6 @@ ifapi_policyutil_execute(FAPI_CONTEXT *context, ESYS_TR *session)
                 goto_if_error(r, "Create policy session", error);
 
                 pol_util_ctx->pol_exec_ctx->session = pol_util_ctx->policy_session;
-                /* Save policy session for cleanup in error case. */
-                context->policy_session = pol_util_ctx->policy_session;
             } else {
                 pol_util_ctx->pol_exec_ctx->session = *session;
             }
@@ -299,13 +302,26 @@ ifapi_policyutil_execute(FAPI_CONTEXT *context, ESYS_TR *session)
                 context->policy.util_current_policy = pol_util_ctx->prev;
                 return TSS2_FAPI_RC_TRY_AGAIN;
             }
+
+            if (r) {
+                /* Cleanup stack */
+                IFAPI_POLICYUTIL_STACK  *utl_ctx = pol_util_ctx->prev;
+                while (utl_ctx) {
+                    if (utl_ctx->pol_exec_ctx->session ==  pol_util_ctx->pol_exec_ctx->session) {
+                        utl_ctx->pol_exec_ctx->session = ESYS_TR_NONE;
+                    }
+                    utl_ctx = utl_ctx->prev;
+                }
+                pol_util_ctx->pol_exec_ctx->session = ESYS_TR_NONE;
+            }
             goto_if_error(r, "Execute policy.", error);
 
             break;
 
         statecasedefault(pol_util_ctx->state);
     }
     *session = pol_util_ctx->policy_session;
+    pol_util_ctx->state = POLICY_UTIL_INIT;
 
     pol_util_ctx = pol_util_ctx->prev;
 
@@ -318,6 +334,7 @@ ifapi_policyutil_execute(FAPI_CONTEXT *context, ESYS_TR *session)
     return r;
 
 error:
+    pol_util_ctx->state = POLICY_UTIL_INIT;
     pol_util_ctx = pol_util_ctx->prev;
     if (context->policy.util_current_policy)
         clear_current_policy(context);