keystore: abort if verification token generation fails

Fix a corner case of operation leakage: if verification
token generation fails, then abort the corresponding
operation on citadel.

Bug: 116055338
Test: pending
Change-Id: I5163fa43fcff505deef5555318148178118ff41e
(cherry picked from commit f8feed620bd607427ded702cce91bb0eb749bc6a)

1 files changed, 14 insertions, 2 deletions

diff --git a/keystore/key_store_service.cpp b/keystore/key_store_service.cpp
index 9d035c8..d2b17ab 100644
--- a/keystore/key_store_service.cpp
+++ b/keystore/key_store_service.cpp
@@ -1360,7 +1360,14 @@ Status KeyStoreService::begin(const sp<IBinder>& appToken, const String16& name,
                                                             }));
 
         if (rc != ErrorCode::OK) result->resultCode = rc;
-        if (result->resultCode != ErrorCode::OK) return Status::ok();
+        if (result->resultCode != ErrorCode::OK) {
+            ALOGW("Failed to verify authorization %d from begin()", rc);
+            rc = KS_HANDLE_HIDL_ERROR(dev->abort(result->handle));
+            if (rc != ErrorCode::OK) {
+                ALOGW("Failed to abort operation %d from begin()", rc);
+            }
+            return Status::ok();
+        }
     }
 
     // Note: The operation map takes possession of the contents of "characteristics".
@@ -1451,7 +1458,12 @@ Status KeyStoreService::update(const sp<IBinder>& token, const KeymasterArgument
 
     // just a reminder: on success result->resultCode was set in the callback. So we only overwrite
     // it if there was a communication error indicated by the ErrorCode.
-    if (!rc.isOk()) result->resultCode = rc;
+    if (!rc.isOk()) {
+        result->resultCode = rc;
+        // removeOperation() will free the memory 'op' used, so the order is important
+        mAuthTokenTable.MarkCompleted(op.handle);
+        mOperationMap.removeOperation(token, /* wasOpSuccessful */ false);
+    }
 
     return Status::ok();
 }