FAPI: Fix error handling in state machines 3.2.x

* In some sub state machines, which had no async function, the state was not reset to the initial state in error cases. * In some FAPI commands in the async part it was not checked whether the command is state is equal _FAPI_STATE_INIT. * In several FAPI commands the current state of the FAPI context was not reset to _FAPI_STATE_INIT in error cases. Thus it was possible that a sequence error did occur after a normal error case. Signed-off-by: Juergen Repp <[email protected]>
tpm2-software · Jan 24, 2024 · 77d254e · 77d254e
1 parent d0e8252
commit 77d254e
Show file tree

Hide file tree

Showing 30 changed files with 93 additions and 54 deletions.
diff --git a/src/tss2-fapi/api/Fapi_AuthorizePolicy.c b/src/tss2-fapi/api/Fapi_AuthorizePolicy.c
@@ -366,7 +366,6 @@ Fapi_AuthorizePolicy_Finish(
             r = ifapi_cleanup_session(context);
             try_again_or_error_goto(r, "Cleanup", cleanup);
 
-            context->state = _FAPI_STATE_INIT;
             break;
 
        statecasedefault(context->state);
@@ -383,6 +382,7 @@ Fapi_AuthorizePolicy_Finish(
     ifapi_cleanup_ifapi_object(&context->loadKey.auth_object);
     SAFE_FREE(command->policyPath);
     SAFE_FREE(command->signingKeyPath);
+    context->state = _FAPI_STATE_INIT;
     LOG_TRACE("finished");
     return r;
 }
diff --git a/src/tss2-fapi/api/Fapi_ChangeAuth.c b/src/tss2-fapi/api/Fapi_ChangeAuth.c
@@ -445,7 +445,6 @@ Fapi_ChangeAuth_Finish(
             r = ifapi_cleanup_session(context);
             try_again_or_error_goto(r, "Cleanup", error_cleanup);
 
-            context->state = _FAPI_STATE_INIT;
             LOG_TRACE("success");
             break;
 
@@ -609,5 +608,6 @@ Fapi_ChangeAuth_Finish(
         SAFE_FREE(command->pathlist);
     }
     LOG_TRACE("finished");
+    context->state = _FAPI_STATE_INIT;
     return r;
 }
diff --git a/src/tss2-fapi/api/Fapi_CreateKey.c b/src/tss2-fapi/api/Fapi_CreateKey.c
@@ -307,6 +307,7 @@ Fapi_CreateKey_Finish(
     ifapi_cleanup_ifapi_object(&context->createPrimary.pkey_object);
     ifapi_cleanup_ifapi_object(context->loadKey.key_object);
     ifapi_cleanup_ifapi_object(&context->loadKey.auth_object);
+    context->state = _FAPI_STATE_INIT;
     LOG_TRACE("finished");
     return r;
 }
diff --git a/src/tss2-fapi/api/Fapi_Decrypt.c b/src/tss2-fapi/api/Fapi_Decrypt.c
@@ -409,6 +409,6 @@ Fapi_Decrypt_Finish(
     ifapi_cleanup_ifapi_object(&context->createPrimary.pkey_object);
     ifapi_cleanup_ifapi_object(&context->loadKey.auth_object);
     ifapi_cleanup_ifapi_object(context->loadKey.key_object);
-
+    context->state = _FAPI_STATE_INIT;
     return r;
 }
diff --git a/src/tss2-fapi/api/Fapi_Delete.c b/src/tss2-fapi/api/Fapi_Delete.c
@@ -550,7 +550,7 @@ Fapi_Delete_Finish(
 
             /* Load the object metadata from the keystore. */
             r = ifapi_keystore_load_async(&context->keystore, &context->io, path);
-            return_if_error2(r, "Could not open: %s", path);
+            goto_if_error2(r, "Could not open: %s", error_cleanup, path);
 
             fallthrough;
 
@@ -559,7 +559,7 @@ Fapi_Delete_Finish(
                TPM operations; e.g. persistent key or NV index. */
             r = ifapi_keystore_load_finish(&context->keystore, &context->io, object);
             return_try_again(r);
-            return_if_error_reset_state(r, "read_finish failed");
+            goto_if_error(r, "read_finish failed", error_cleanup);
 
             /* Initialize the ESYS object for the persistent key or NV Index. */
             r = ifapi_initialize_object(context->esys, object);
@@ -579,7 +579,7 @@ Fapi_Delete_Finish(
                 /* Check whether hierarchy file has been read. */
                 if (authObject->objectType == IFAPI_OBJ_NONE) {
                     r = ifapi_keystore_load_async(&context->keystore, &context->io, "/HS");
-                    return_if_error2(r, "Could not open hierarchy /HS");
+                    goto_if_error(r, "Could not open hierarchy /HS", error_cleanup);
 
                     command->auth_index = ESYS_TR_RH_OWNER;
                 } else {
@@ -624,15 +624,15 @@ Fapi_Delete_Finish(
         statecase(context->state, ENTITY_DELETE_KEY);
             if (object->misc.key.persistent_handle) {
                 r = ifapi_keystore_load_async(&context->keystore, &context->io, "/HS");
-                return_if_error2(r, "Could not open hierarchy /HS");
+                goto_if_error(r, "Could not open hierarchy /HS", error_cleanup);
             }
             fallthrough;
 
         statecase(context->state, ENTITY_DELETE_KEY_WAIT_FOR_HIERARCHY);
             if (object->misc.key.persistent_handle) {
                 r = ifapi_keystore_load_finish(&context->keystore, &context->io, authObject);
                 return_try_again(r);
-                return_if_error(r, "read_finish failed");
+                goto_if_error(r, "read_finish failed", error_cleanup);
 
                 r = ifapi_initialize_object(context->esys, authObject);
                 goto_if_error_reset_state(r, "Initialize hierarchy object", error_cleanup);
@@ -786,5 +786,6 @@ Fapi_Delete_Finish(
     SAFE_FREE(command->pathlist);
     ifapi_session_clean(context);
     ifapi_cleanup_ifapi_object(&context->createPrimary.pkey_object);
+    context->state = _FAPI_STATE_INIT;
     return r;
 }
diff --git a/src/tss2-fapi/api/Fapi_ExportPolicy.c b/src/tss2-fapi/api/Fapi_ExportPolicy.c
@@ -287,14 +287,14 @@ Fapi_ExportPolicy_Finish(
             /* Load the key meta data from the keystore. */
             r = ifapi_keystore_load_async(&context->keystore, &context->io,
                                           command->path);
-            return_if_error2(r, "Could not open: %s", command->path);
+            goto_if_error2(r, "Could not open: %s", error_cleanup, command->path);
             fallthrough;
 
         statecase(context->state, POLICY_EXPORT_READ_OBJECT_FINISH);
             r = ifapi_keystore_load_finish(&context->keystore, &context->io,
                                            &command->object);
             return_try_again(r);
-            return_if_error_reset_state(r, "read_finish failed");
+            goto_if_error(r, "read_finish failed", error_cleanup);
 
             goto_if_null2(command->object.policy,
                           "Object has no policy",

diff --git a/src/tss2-fapi/api/Fapi_GetAppData.c b/src/tss2-fapi/api/Fapi_GetAppData.c
@@ -215,7 +215,6 @@ Fapi_GetAppData_Finish(
             if (appDataSize)
                 *appDataSize = objAppData->size;
 
-            context->state = _FAPI_STATE_INIT;
             r = TSS2_RC_SUCCESS;
             break;
 
@@ -228,6 +227,7 @@ Fapi_GetAppData_Finish(
     ifapi_cleanup_ifapi_object(&context->loadKey.auth_object);
     ifapi_cleanup_ifapi_object(context->loadKey.key_object);
     ifapi_cleanup_ifapi_object(&context->createPrimary.pkey_object);
+    context->state = _FAPI_STATE_INIT;
     LOG_TRACE("finished");
     return r;
 }
diff --git a/src/tss2-fapi/api/Fapi_GetCertificate.c b/src/tss2-fapi/api/Fapi_GetCertificate.c
@@ -135,6 +135,10 @@ Fapi_GetCertificate_Async(
     check_not_null(context);
     check_not_null(path);
 
+    if (context->state != _FAPI_STATE_INIT) {
+        return_error(TSS2_FAPI_RC_BAD_SEQUENCE, "Invalid State");
+    }
+
     r = ifapi_non_tpm_mode_init(context);
     return_if_error(r, "Initialize GetCertificate");
 

diff --git a/src/tss2-fapi/api/Fapi_GetDescription.c b/src/tss2-fapi/api/Fapi_GetDescription.c
@@ -123,6 +123,10 @@ Fapi_GetDescription_Async(
     check_not_null(context);
     check_not_null(path);
 
+    if (context->state != _FAPI_STATE_INIT) {
+        return_error(TSS2_FAPI_RC_BAD_SEQUENCE, "Invalid State");
+    }
+
     /* Load the object metadata from keystore. */
     r = ifapi_keystore_load_async(&context->keystore, &context->io, path);
     return_if_error2(r, "Could not open: %s", path);

diff --git a/src/tss2-fapi/api/Fapi_GetEsysBlob.c b/src/tss2-fapi/api/Fapi_GetEsysBlob.c
@@ -401,5 +401,6 @@ Fapi_GetEsysBlob_Finish(
     ifapi_cleanup_ifapi_object(&context->loadKey.auth_object);
     ifapi_cleanup_ifapi_object(context->loadKey.key_object);
     ifapi_cleanup_ifapi_object(&context->createPrimary.pkey_object);
+    context->state = _FAPI_STATE_INIT;
     return r;
 }
diff --git a/src/tss2-fapi/api/Fapi_GetRandom.c b/src/tss2-fapi/api/Fapi_GetRandom.c
@@ -263,6 +263,7 @@ Fapi_GetRandom_Finish(
     ifapi_cleanup_ifapi_object(&context->createPrimary.pkey_object);
     ifapi_session_clean(context);
     SAFE_FREE(context->get_random.data);
+    context->state = _FAPI_STATE_INIT;
     LOG_TRACE("finished");
     return r;
 }
diff --git a/src/tss2-fapi/api/Fapi_GetTpmBlobs.c b/src/tss2-fapi/api/Fapi_GetTpmBlobs.c
@@ -137,6 +137,10 @@ Fapi_GetTpmBlobs_Async(
     check_not_null(context);
     check_not_null(path);
 
+    if (context->state != _FAPI_STATE_INIT) {
+        return_error(TSS2_FAPI_RC_BAD_SEQUENCE, "Invalid State");
+    }
+
     /* Load the object from the key store. */
     r = ifapi_keystore_load_async(&context->keystore, &context->io, path);
     return_if_error2(r, "Could not open: %s", path);
@@ -270,5 +274,6 @@ Fapi_GetTpmBlobs_Finish(
     ifapi_cleanup_ifapi_object(context->loadKey.key_object);
     ifapi_cleanup_ifapi_object(&context->createPrimary.pkey_object);
     LOG_TRACE("finished");
+    context->state = _FAPI_STATE_INIT;
     return r;
 }
diff --git a/src/tss2-fapi/api/Fapi_Import.c b/src/tss2-fapi/api/Fapi_Import.c
@@ -668,5 +668,6 @@ Fapi_Import_Finish(
     ifapi_cleanup_ifapi_object(&context->loadKey.auth_object);
     ifapi_cleanup_ifapi_object(context->loadKey.key_object);
     ifapi_cleanup_ifapi_object(&context->createPrimary.pkey_object);
+    context->state = _FAPI_STATE_INIT;
     return r;
 }
diff --git a/src/tss2-fapi/api/Fapi_NvExtend.c b/src/tss2-fapi/api/Fapi_NvExtend.c
@@ -370,7 +370,7 @@ Fapi_NvExtend_Finish(
         /* Compute Digest of the current event */
         hashAlg = object->misc.nv.public.nvPublic.nameAlg;
         r = ifapi_crypto_hash_start(&cryptoContext, hashAlg);
-        return_if_error(r, "crypto hash start");
+        goto_if_error(r, "crypto hash start", error_cleanup);
 
         HASH_UPDATE_BUFFER(cryptoContext,
                            &auxData->buffer[0], auxData->size,
@@ -380,7 +380,7 @@ Fapi_NvExtend_Finish(
                                      (uint8_t *)
                                      &event->digests.digests[0].digest,
                                      &hashSize);
-        return_if_error(r, "crypto hash finish");
+        goto_if_error(r, "crypto hash finish", error_cleanup);
 
         event->digests.digests[0].hashAlg = hashAlg;
         event->digests.count = 1;
@@ -448,15 +448,14 @@ Fapi_NvExtend_Finish(
         /* Finish writing the NV object to the key store */
         r = ifapi_keystore_store_finish(&context->io);
         return_try_again(r);
-        return_if_error_reset_state(r, "write_finish failed");
+        goto_if_error(r, "write_finish failed", error_cleanup);
         fallthrough;
 
     statecase(context->state, NV_EXTEND_CLEANUP)
         /* Cleanup the session. */
         r = ifapi_cleanup_session(context);
         try_again_or_error_goto(r, "Cleanup", error_cleanup);
 
-        context->state = _FAPI_STATE_INIT;
         r = TSS2_RC_SUCCESS;
 
         break;
@@ -483,5 +482,6 @@ Fapi_NvExtend_Finish(
     SAFE_FREE(object->misc.nv.event_log);
     ifapi_session_clean(context);
     LOG_TRACE("finished");
+    context->state = _FAPI_STATE_INIT;
     return r;
 }
diff --git a/src/tss2-fapi/api/Fapi_NvIncrement.c b/src/tss2-fapi/api/Fapi_NvIncrement.c
@@ -336,7 +336,6 @@ Fapi_NvIncrement_Finish(
         r = ifapi_cleanup_session(context);
         try_again_or_error_goto(r, "Cleanup", error_cleanup);
 
-        context->state = _FAPI_STATE_INIT;
         break;
 
     statecasedefault(context->state);
@@ -351,6 +350,7 @@ Fapi_NvIncrement_Finish(
     SAFE_FREE(command->nvPath);
     SAFE_FREE(jso);
     ifapi_session_clean(context);
+    context->state = _FAPI_STATE_INIT;
     LOG_TRACE("finished");
     return r;
 }
diff --git a/src/tss2-fapi/api/Fapi_NvRead.c b/src/tss2-fapi/api/Fapi_NvRead.c
@@ -330,7 +330,6 @@ Fapi_NvRead_Finish(
         *data = command->rdata;
         if (size)
             *size = command->size;
-        context->state = _FAPI_STATE_INIT;
         break;
 
     statecasedefault(context->state);
@@ -345,6 +344,7 @@ Fapi_NvRead_Finish(
     SAFE_FREE(command->nvPath);
     //SAFE_FREE(context->nv_cmd.tes);
     ifapi_session_clean(context);
+    context->state = _FAPI_STATE_INIT;
     LOG_TRACE("finished");
     return r;
 }
diff --git a/src/tss2-fapi/api/Fapi_NvSetBits.c b/src/tss2-fapi/api/Fapi_NvSetBits.c
@@ -336,7 +336,7 @@ Fapi_NvSetBits_Finish(
         /* Finish writing the NV object to the key store */
         r = ifapi_keystore_store_finish(&context->io);
         return_try_again(r);
-        return_if_error_reset_state(r, "write_finish failed");
+        goto_if_error(r, "write_finish failed", error_cleanup);
 
         fallthrough;
 
@@ -345,9 +345,7 @@ Fapi_NvSetBits_Finish(
         r = ifapi_cleanup_session(context);
         try_again_or_error_goto(r, "Cleanup", error_cleanup);
 
-        context->state = _FAPI_STATE_INIT;
         LOG_DEBUG("success");
-
         break;
 
     statecasedefault(context->state);
@@ -362,6 +360,7 @@ Fapi_NvSetBits_Finish(
     ifapi_cleanup_ifapi_object(&context->loadKey.auth_object);
     ifapi_cleanup_ifapi_object(context->loadKey.key_object);
     ifapi_cleanup_ifapi_object(&context->createPrimary.pkey_object);
+    context->state = _FAPI_STATE_INIT;
     LOG_TRACE("finished");
     return r;
 }
diff --git a/src/tss2-fapi/api/Fapi_NvWrite.c b/src/tss2-fapi/api/Fapi_NvWrite.c
@@ -290,7 +290,6 @@ Fapi_NvWrite_Finish(
         r = ifapi_cleanup_session(context);
         try_again_or_error_goto(r, "Cleanup", error_cleanup);
 
-        context->state = _FAPI_STATE_INIT;
         break;
 
     statecasedefault(context->state);
@@ -307,7 +306,7 @@ Fapi_NvWrite_Finish(
     SAFE_FREE(command->data);
     SAFE_FREE(jso);
     ifapi_session_clean(context);
-
+    context->state = _FAPI_STATE_INIT;
     LOG_TRACE("finished");
     return r;
 }
diff --git a/src/tss2-fapi/api/Fapi_PcrExtend.c b/src/tss2-fapi/api/Fapi_PcrExtend.c
@@ -258,7 +258,7 @@ Fapi_PcrExtend_Finish(
             /* Construct the filename for the eventlog file */
             r = ifapi_asprintf(&command->event_log_file, "%s/%s%i",
                                context->eventlog.log_dir, IFAPI_PCR_LOG_FILE, command->pcrIndex);
-            return_if_error(r, "Out of memory.");
+            return_if_error_reset_state(r, "Out of memory.");
 
             /* Check wheter the event log has to be read. */
             if (ifapi_io_path_exists(command->event_log_file)) {
@@ -296,7 +296,7 @@ Fapi_PcrExtend_Finish(
             r = Esys_PCR_Event_Async(context->esys, command->pcrIndex,
                                      context->session1, ESYS_TR_NONE, ESYS_TR_NONE,
                                      &command->event);
-            return_if_error(r, "Esys_PCR_Event_Async");
+            goto_if_error(r, "Esys_PCR_Event_Async", error_cleanup);
             command->event_digests = NULL;
 
             fallthrough;
@@ -333,7 +333,6 @@ Fapi_PcrExtend_Finish(
             r = ifapi_cleanup_session(context);
             try_again_or_error_goto(r, "Cleanup", error_cleanup);
 
-            context->state =  _FAPI_STATE_INIT;
             break;
 
         statecasedefault(context->state);
@@ -350,6 +349,7 @@ Fapi_PcrExtend_Finish(
     ifapi_cleanup_ifapi_object(&context->createPrimary.pkey_object);
     ifapi_cleanup_event(pcrEvent);
     ifapi_session_clean(context);
+    context->state =  _FAPI_STATE_INIT;
     LOG_TRACE("finished");
     return r;
 }
diff --git a/src/tss2-fapi/api/Fapi_PcrRead.c b/src/tss2-fapi/api/Fapi_PcrRead.c
@@ -266,7 +266,6 @@ Fapi_PcrRead_Finish(
                 *pcrValue = command->pcrValue;
             if (pcrValueSize)
                 *pcrValueSize = command->pcrValueSize;
-            context->state = _FAPI_STATE_INIT;
             break;
 
         statecasedefault(context->state);
@@ -279,6 +278,7 @@ Fapi_PcrRead_Finish(
         SAFE_FREE(command->pcrValue);
     }
     SAFE_FREE(command->pcrValues);
+    context->state = _FAPI_STATE_INIT;
     LOG_TRACE("finished");
     return r;
 }
diff --git a/src/tss2-fapi/api/Fapi_Provision.c b/src/tss2-fapi/api/Fapi_Provision.c
@@ -906,9 +906,9 @@ Fapi_Provision_Finish(FAPI_CONTEXT *context)
                 if (command->auth_state & TPMA_PERMANENT_LOCKOUTAUTHSET) {
                     hierarchy_lockout->misc.hierarchy.with_auth = TPM2_YES;
                     r = ifapi_get_description(hierarchy_lockout, &description);
-                    return_if_error(r, "Get description");
+                    goto_if_error(r, "Get description", error_cleanup);
                     r = ifapi_set_auth(context, hierarchy_lockout, description);
-                    return_if_error(r, "Set auth value");
+                    goto_if_error(r, "Set auth value", error_cleanup);
                 } else {
                     hierarchy_lockout->misc.hierarchy.with_auth = TPM2_NO;
                 }
@@ -1269,7 +1269,7 @@ Fapi_Provision_Finish(FAPI_CONTEXT *context)
             /* Finish writing the endorsement hierarchy to the key store */
             r = ifapi_keystore_store_finish(&context->io);
             return_try_again(r);
-            return_if_error_reset_state(r, "write_finish failed");
+            goto_if_error_reset_state(r, "write_finish failed", error_cleanup);
 
             /* Write all endorsement hierarchies. */
             command->hierarchy = hierarchy_he;
@@ -1541,6 +1541,7 @@ Fapi_Provision_Finish(FAPI_CONTEXT *context)
         }
         SAFE_FREE(command->pathlist);
     }
+    context->state = _FAPI_STATE_INIT;
     LOG_TRACE("finished");
     return r;
 }