FAPI: Fix error checking of state machines

* 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. Fixes: tpm2-software#2752 Signed-off-by: Juergen Repp <[email protected]>
JuergenReppSIT · Jan 19, 2024 · 0e8fb9f · 0e8fb9f
1 parent 50b7608
commit 0e8fb9f
Show file tree

Hide file tree

Showing 8 changed files with 42 additions and 9 deletions.
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_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);

diff --git a/src/tss2-fapi/api/Fapi_SetAppData.c b/src/tss2-fapi/api/Fapi_SetAppData.c
@@ -139,6 +139,10 @@ Fapi_SetAppData_Async(
     check_not_null(context);
     check_not_null(path);
 
+    if (context->state != _FAPI_STATE_INIT) {
+        return_error(TSS2_FAPI_RC_BAD_SEQUENCE, "Invalid State");
+    }
+
     /* App data is restricted to 10MB. */
     if (appDataSize > FAPI_MAX_APP_DATA_SIZE) {
         LOG_ERROR("Only 10MB are allowd for app data.");

diff --git a/src/tss2-fapi/api/Fapi_SetCertificate.c b/src/tss2-fapi/api/Fapi_SetCertificate.c
@@ -138,6 +138,10 @@ Fapi_SetCertificate_Async(
     check_not_null(context);
     check_not_null(path);
 
+    if (context->state != _FAPI_STATE_INIT) {
+        return_error(TSS2_FAPI_RC_BAD_SEQUENCE, "Invalid State");
+    }
+
     /* Helpful alias pointers */
     IFAPI_Key_SetCertificate * command = &context->cmd.Key_SetCertificate;
 

diff --git a/src/tss2-fapi/api/Fapi_SetDescription.c b/src/tss2-fapi/api/Fapi_SetDescription.c
@@ -127,6 +127,10 @@ Fapi_SetDescription_Async(
     check_not_null(context);
     check_not_null(path);
 
+    if (context->state != _FAPI_STATE_INIT) {
+        return_error(TSS2_FAPI_RC_BAD_SEQUENCE, "Invalid State");
+    }
+
     /* Check for invalid parameters */
     if (description && strlen(description) + 1 > 1024) {
         return_error(TSS2_FAPI_RC_BAD_VALUE,

diff --git a/src/tss2-fapi/api/Fapi_VerifyQuote.c b/src/tss2-fapi/api/Fapi_VerifyQuote.c
@@ -175,6 +175,10 @@ Fapi_VerifyQuote_Async(
     check_not_null(quoteInfo);
     check_not_null(signature);
 
+    if (context->state != _FAPI_STATE_INIT) {
+        return_error(TSS2_FAPI_RC_BAD_SEQUENCE, "Invalid State");
+    }
+
     /* Check for invalid parameters */
     if (qualifyingData == NULL && qualifyingDataSize != 0) {
         LOG_ERROR("qualifyingData is NULL but qualifyingDataSize is not 0");

diff --git a/src/tss2-fapi/fapi_util.c b/src/tss2-fapi/fapi_util.c
@@ -63,9 +63,9 @@ ifapi_flush_object(FAPI_CONTEXT *context, ESYS_TR handle)
         if (base_rc(r) == TSS2_BASE_RC_TRY_AGAIN)
             return TSS2_FAPI_RC_TRY_AGAIN;
 
+        context->flush_object_state = FLUSH_INIT;
         return_if_error(r, "FlushContext");
 
-        context->flush_object_state = FLUSH_INIT;
         return TSS2_RC_SUCCESS;
 
     statecasedefault(context->flush_object_state);
@@ -1256,40 +1256,40 @@ ifapi_cleanup_session(FAPI_CONTEXT *context)
                     context->session2 = ESYS_TR_NONE;
                 }
                 r = Esys_FlushContext_Async(context->esys, context->session1);
-                try_again_or_error(r, "Flush session.");
+                try_again_or_error_goto(r, "Flush session.", error);
             }
             fallthrough;
 
         statecase(context->cleanup_state, CLEANUP_SESSION1);
             if (context->session1 != ESYS_TR_NONE) {
                 r = Esys_FlushContext_Finish(context->esys);
-                try_again_or_error(r, "Flush session.");
+                try_again_or_error_goto(r, "Flush session.", error);
             }
             context->session1 = ESYS_TR_NONE;
 
             if (context->session2 != ESYS_TR_NONE) {
                 r = Esys_FlushContext_Async(context->esys, context->session2);
-                try_again_or_error(r, "Flush session.");
+                try_again_or_error_goto(r, "Flush session.", error);
             }
             fallthrough;
 
         statecase(context->cleanup_state, CLEANUP_SESSION2);
             if (context->session2 != ESYS_TR_NONE) {
                 r = Esys_FlushContext_Finish(context->esys);
-                try_again_or_error(r, "Flush session.");
+                try_again_or_error_goto(r, "Flush session.", error);
             }
             context->session2 = ESYS_TR_NONE;
 
             if (!context->srk_persistent && context->srk_handle != ESYS_TR_NONE) {
                 r = Esys_FlushContext_Async(context->esys, context->srk_handle);
-                try_again_or_error(r, "Flush SRK.");
+                try_again_or_error_goto(r, "Flush SRK.", error);
             }
             fallthrough;
 
         statecase(context->cleanup_state, CLEANUP_SRK);
             if (!context->srk_persistent && context->srk_handle != ESYS_TR_NONE) {
                 r = Esys_FlushContext_Finish(context->esys);
-                try_again_or_error(r, "Flush SRK.");
+                try_again_or_error_goto(r, "Flush SRK.", error);
 
                 context->srk_handle = ESYS_TR_NONE;
                 context->srk_persistent = false;
@@ -1299,6 +1299,9 @@ ifapi_cleanup_session(FAPI_CONTEXT *context)
 
         statecasedefault(context->state);
     }
+ error:
+    context->cleanup_state = CLEANUP_INIT;
+    return r;
 }
 
 /** Cleanup primary keys in error cases (non asynchronous).
@@ -2892,7 +2895,6 @@ ifapi_load_key(
         return_try_again(r);
         goto_if_error_reset_state(r, " Load key.", error_cleanup);
 
-        context->loadKey.prepare_state = PREPARE_LOAD_KEY_INIT;
         break;
 
     statecase(context->loadKey.prepare_state, PREPARE_LOAD_KEY_INIT_KEY);
@@ -2908,6 +2910,7 @@ ifapi_load_key(
     }
 
 error_cleanup:
+    context->loadKey.prepare_state = PREPARE_LOAD_KEY_INIT;
     return r;
 }
 
@@ -3041,7 +3044,6 @@ ifapi_key_sign(
                 strdup_check(*certificate, "", r, cleanup);
             }
         }
-        context->Key_Sign.state = SIGN_INIT;
         LOG_TRACE("success");
         r = TSS2_RC_SUCCESS;
         break;
@@ -3053,6 +3055,8 @@ ifapi_key_sign(
     if (context->Key_Sign.handle != ESYS_TR_NONE)
         Esys_FlushContext(context->esys, context->Key_Sign.handle);
     ifapi_cleanup_ifapi_object(context->Key_Sign.key_object);
+    context->Key_Sign.state = SIGN_INIT;
+
     return r;
 }
 
@@ -3960,6 +3964,7 @@ ifapi_change_auth_hierarchy(
     statecasedefault(context->hierarchy_state);
     }
 error:
+    context->hierarchy_state = HIERARCHY_CHANGE_AUTH_INIT;
     return r;
 }