esys: close TR handle for input persistent handle in EvictControl_Finish

The TSS ESAPI spec (1.00 rev 14) says:

> If the ESYS_TR object on input was a persistent object, [the ESYS_TR
> output] parameter will not return a ESYS_TR object but instead will
> return ESYS_TR_NULL. Also the metadata in the ESYS_CONTEXT regarding
> the ESYS_TR object will be removed and the ESYS_TR object will be
> marked as invalid.

However, currently although the output parameter is set to
ESYS_TR_NONE, the input ESYS_TR object isn't closed. Other than the
obvious leak, this can cause an error when Esys_TR_FromTPMPublic is
called twice, with the object having been recreated with different
public data and persisted with Esys_EvictControl in between, since
Esys_TR_FromTPMPublic now verifies that the public data hasn't changed
for the ESYS_TR object (which it now has).

Address this by closing the input ESYS_TR handle in the case that it
referred to a persistent object.

Signed-off-by: Rob Shearman <rob@graphiant.com>

src/tss2-esys/api/Esys_EvictControl.c

@@ -361,6 +361,9 @@ Esys_EvictControl_Finish(
     /* The object was already persistent */
     if (iesys_get_handle_type(objectHandleNode->rsrc.handle) == TPM2_HT_PERSISTENT) {
         *newObjectHandle = ESYS_TR_NONE;
+        r = Esys_TR_Close(esysContext, &objectHandle);
+        if (r != TSS2_RC_SUCCESS)
+            return r;
     } else {
         /* A new resource is created and updated with date from the not persistent object */
         RSRC_NODE_T *newObjectHandleNode = NULL;

test/integration/esys-tr-fromTpmPublic-key.int.c

@@ -105,51 +105,72 @@ test_esys_tr_fromTpmPublic_key(ESYS_CONTEXT * ectx)
         .count = 0,
     };
 
-    r = Esys_CreatePrimary(ectx, ESYS_TR_RH_OWNER,
-                           ESYS_TR_PASSWORD, ESYS_TR_NONE, ESYS_TR_NONE,
-                           &inSensitivePrimary, &inPublic, &outsideInfo,
-                           &creationPCR,
-                           &primaryHandle, NULL, NULL, NULL, NULL);
-    goto_if_error(r, "Create primary", error);
-
-    r = Esys_ReadPublic(ectx, primaryHandle,
-                        ESYS_TR_NONE, ESYS_TR_NONE, ESYS_TR_NONE,
-                        NULL, &name1, NULL);
-    goto_if_error(r, "Read Public", error);
-
-    r = Esys_EvictControl(ectx, ESYS_TR_RH_OWNER, primaryHandle,
-                          ESYS_TR_PASSWORD, ESYS_TR_NONE, ESYS_TR_NONE,
-                          TPM2_PERSISTENT_FIRST, &keyHandle);
-    goto_if_error(r, "EvictControl make persistent", error_name1);
-
-    r = Esys_FlushContext(ectx, primaryHandle);
-    goto_if_error(r, "Flushing primary", error_name1);
-
-    r = Esys_TR_Close(ectx, &keyHandle);
-    goto_if_error(r, "TR close on nv object", error_name1);
-
-    r = Esys_TR_FromTPMPublic(ectx, TPM2_PERSISTENT_FIRST,
-                              ESYS_TR_NONE, ESYS_TR_NONE, ESYS_TR_NONE,
-                              &keyHandle);
-    goto_if_error(r, "TR from TPM public", error_name1);
-
-    r = Esys_TR_GetName(ectx, keyHandle, &name2);
-    goto_if_error(r, "TR get name", error_name1);
-
-    r = Esys_EvictControl(ectx, ESYS_TR_RH_OWNER, keyHandle,
-                          ESYS_TR_PASSWORD, ESYS_TR_NONE, ESYS_TR_NONE,
-                          TPM2_PERSISTENT_FIRST, &keyHandle);
-    goto_if_error(r, "EvictControl delete", error_name2);
-
-    if (name1->size != name2->size ||
-        memcmp(&name1->name[0], &name2->name[0], name1->size) != 0)
-    {
-        LOG_ERROR("Names mismatch between NV_GetPublic and TR_GetName");
-        goto error_name2;
-    }
+    /*
+     * Do this twice to test that Esys_TR_FromTPMPublic doesn't error
+     * due to the handle not being closed by Esys_EvictControl
+     */
+    for (int i = 0; i < 2; i++) {
+        /*
+         * Change public data to verify that handle is closed on
+         * Esys_EvictControl delete
+         */
+        if (i == 1) {
+            inPublic.publicArea.unique.rsa.size = 2048 / 8;
+            inPublic.publicArea.unique.rsa.buffer[0] = 1;
+        }
 
-    free(name1);
-    free(name2);
+        r = Esys_CreatePrimary(ectx, ESYS_TR_RH_OWNER,
+                               ESYS_TR_PASSWORD, ESYS_TR_NONE, ESYS_TR_NONE,
+                               &inSensitivePrimary, &inPublic, &outsideInfo,
+                               &creationPCR,
+                               &primaryHandle, NULL, NULL, NULL, NULL);
+        goto_if_error(r, "Create primary", error);
+
+        r = Esys_ReadPublic(ectx, primaryHandle,
+                            ESYS_TR_NONE, ESYS_TR_NONE, ESYS_TR_NONE,
+                            NULL, &name1, NULL);
+        goto_if_error(r, "Read Public", error);
+
+        r = Esys_EvictControl(ectx, ESYS_TR_RH_OWNER, primaryHandle,
+                              ESYS_TR_PASSWORD, ESYS_TR_NONE, ESYS_TR_NONE,
+                              TPM2_PERSISTENT_FIRST, &keyHandle);
+        goto_if_error(r, "EvictControl make persistent", error_name1);
+
+        LOG_ERROR("Key handle (1) 0x%x", keyHandle);
+
+        r = Esys_FlushContext(ectx, primaryHandle);
+        goto_if_error(r, "Flushing primary", error_name1);
+
+        r = Esys_TR_Close(ectx, &keyHandle);
+        goto_if_error(r, "TR close on nv object", error_name1);
+
+        r = Esys_TR_FromTPMPublic(ectx, TPM2_PERSISTENT_FIRST,
+                                  ESYS_TR_NONE, ESYS_TR_NONE, ESYS_TR_NONE,
+                                  &keyHandle);
+        goto_if_error(r, "TR from TPM public", error_name1);
+
+        LOG_ERROR("Key handle (2) 0x%x", keyHandle);
+
+        r = Esys_TR_GetName(ectx, keyHandle, &name2);
+        goto_if_error(r, "TR get name", error_name1);
+
+        r = Esys_EvictControl(ectx, ESYS_TR_RH_OWNER, keyHandle,
+                              ESYS_TR_PASSWORD, ESYS_TR_NONE, ESYS_TR_NONE,
+                              TPM2_PERSISTENT_FIRST, &keyHandle);
+        goto_if_error(r, "EvictControl delete", error_name2);
+
+        LOG_ERROR("Key handle (after delete) 0x%x", keyHandle);
+
+        if (name1->size != name2->size ||
+            memcmp(&name1->name[0], &name2->name[0], name1->size) != 0)
+        {
+            LOG_ERROR("Names mismatch between NV_GetPublic and TR_GetName");
+            goto error_name2;
+        }
+
+        free(name1);
+        free(name2);
+    }
 
     return EXIT_SUCCESS;