Confirmationui Rate Limiting App Abort Bug Fix

Increment the rate limiting counter when the application sends an abort
message.

Bug: 138655142
Test: Ran keystore_unit_tests and manually checked behavior of
keystore application with confimrationui.
Merged-In: I5f3af166391a32748a26f7709d30a5ac718499c0
Change-Id: I5f3af166391a32748a26f7709d30a5ac718499c0

keystore/confirmation_manager.cpp

@@ -116,12 +116,30 @@ Status ConfirmationManager::cancelConfirmationPrompt(const sp<IBinder>& listener
     }
     mMutex.unlock();
 
-    finalizeTransaction(ConfirmationResponseCode::Aborted, {}, true);
+    cancelPrompt();
 
     *aidl_return = static_cast<int32_t>(ConfirmationResponseCode::OK);
     return Status::ok();
 }
 
+void ConfirmationManager::cancelPrompt() {
+    mMutex.lock();
+    mRateLimiting.cancelPrompt();
+    if (mCurrentListener != nullptr) {
+        mCurrentListener->unlinkToDeath(mDeathRecipient);
+        mCurrentListener = nullptr;
+    }
+    sp<IConfirmationUI> confirmationUI = mCurrentConfirmationUI;
+    if (mCurrentConfirmationUI != nullptr) {
+        mCurrentConfirmationUI->unlinkToDeath(this);
+        mCurrentConfirmationUI = nullptr;
+    }
+    mMutex.unlock();
+    if (confirmationUI != nullptr) {
+        confirmationUI->abort();
+    }
+}
+
 // Called by keystore main thread.
 Status ConfirmationManager::isConfirmationPromptSupported(bool* aidl_return) {
     sp<IConfirmationUI> confirmationUI = IConfirmationUI::tryGetService();
@@ -136,13 +154,7 @@ Status ConfirmationManager::isConfirmationPromptSupported(bool* aidl_return) {
 }
 
 void ConfirmationManager::finalizeTransaction(ConfirmationResponseCode responseCode,
-                                              hidl_vec<uint8_t> dataThatWasConfirmed,
+                                              hidl_vec<uint8_t> dataThatWasConfirmed) {
-                                              bool callAbortOnHal) {
-    // Note that confirmationUI->abort() may make the remote HAL process do an IPC call back
-    // into our process resulting in confirmationResultCallback() to be called... this in turn
-    // calls finalizeTransaction(). So we have to be careful a) not holding any locks;
-    // and b) ensure state has been cleared; before doing this...
-
     mMutex.lock();
     mRateLimiting.processResult(responseCode);
     sp<IBinder> listener = mCurrentListener;
@@ -150,18 +162,12 @@ void ConfirmationManager::finalizeTransaction(ConfirmationResponseCode responseC
         mCurrentListener->unlinkToDeath(mDeathRecipient);
         mCurrentListener = nullptr;
     }
-    sp<IConfirmationUI> confirmationUI = mCurrentConfirmationUI;
     if (mCurrentConfirmationUI != nullptr) {
         mCurrentConfirmationUI->unlinkToDeath(this);
         mCurrentConfirmationUI = nullptr;
     }
     mMutex.unlock();
 
-    // Tell the HAL to shut down the confirmation dialog, if requested.
-    if (confirmationUI != nullptr && callAbortOnHal) {
-        confirmationUI->abort();
-    }
-
     // Deliver result to the application that started the operation.
     if (listener != nullptr) {
         sp<BpConfirmationPromptCallback> obj = new BpConfirmationPromptCallback(listener);
@@ -178,7 +184,7 @@ void ConfirmationManager::finalizeTransaction(ConfirmationResponseCode responseC
 Return<void> ConfirmationManager::result(ConfirmationResponseCode responseCode,
                                          const hidl_vec<uint8_t>& dataThatWasConfirmed,
                                          const hidl_vec<uint8_t>& confirmationToken) {
-    finalizeTransaction(responseCode, dataThatWasConfirmed, false);
+    finalizeTransaction(responseCode, dataThatWasConfirmed);
     lock_guard<mutex> lock(mMutex);
     mLatestConfirmationToken = confirmationToken;
     return Return<void>();
@@ -201,7 +207,7 @@ void ConfirmationManager::binderDied(const wp<IBinder>& who) {
         mCurrentListener = nullptr;
         mMutex.unlock();
         ALOGW("The process which requested the confirmation dialog died.\n");
-        finalizeTransaction(ConfirmationResponseCode::SystemError, {}, true);
+        cancelPrompt();
     } else {
         mMutex.unlock();
     }
@@ -210,7 +216,7 @@ void ConfirmationManager::binderDied(const wp<IBinder>& who) {
 void ConfirmationManager::serviceDied(uint64_t /* cookie */,
                                       const wp<android::hidl::base::V1_0::IBase>& /* who */) {
     ALOGW("The ConfirmationUI HAL died.\n");
-    finalizeTransaction(ConfirmationResponseCode::SystemError, {}, false);
+    finalizeTransaction(ConfirmationResponseCode::SystemError, {});
 }
 
 }  // namespace keystore

keystore/confirmation_manager.h

@@ -84,8 +84,12 @@ class ConfirmationManager : public android::hardware::hidl_death_recipient,
   private:
     friend class ConfirmationResultCallback;
 
+    // Set rate limiting to not decrement on next abort and aborts
+    // confirmationui.
+    void cancelPrompt();
+
     void finalizeTransaction(ConfirmationResponseCode responseCode,
-                             hidl_vec<uint8_t> dataThatWasConfirmed, bool callAbortOnHal);
+                             hidl_vec<uint8_t> dataThatWasConfirmed);
 
     // This mutex protects all data below it.
     std::mutex mMutex;

keystore/confirmationui_rate_limiting.h

@@ -29,8 +29,8 @@ namespace keystore {
 
 using ConfirmationResponseCode = android::hardware::confirmationui::V1_0::ResponseCode;
 
-using std::chrono::time_point;
 using std::chrono::duration;
+using std::chrono::time_point;
 
 template <typename Clock = std::chrono::steady_clock> class RateLimiting {
   private:
@@ -96,7 +96,17 @@ template <typename Clock = std::chrono::steady_clock> class RateLimiting {
         return false;
     }
 
+    // The app is penalized for cancelling a request. Request are rolled back only if
+    // the prompt was cancelled by the system: e.g. a system error or asynchronous event.
+    // When the user cancels the prompt, it is subject to rate limiting.
+    static constexpr const uint_t kInvalidRequester = -1;
+
+    void cancelPrompt() { latest_requester_ = kInvalidRequester; }
+
     void processResult(ConfirmationResponseCode rc) {
+        if (latest_requester_ == kInvalidRequester) {
+            return;
+        }
         switch (rc) {
         case ConfirmationResponseCode::OK:
             // reset the counter slot