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
diff --git a/keystore/confirmation_manager.cpp b/keystore/confirmation_manager.cpp
index 3396359..c6ae1a0 100644
--- a/keystore/confirmation_manager.cpp
+++ b/keystore/confirmation_manager.cpp
@@ -116,12 +116,30 @@
}
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 @@
}
void ConfirmationManager::finalizeTransaction(ConfirmationResponseCode responseCode,
- 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...
-
+ hidl_vec<uint8_t> dataThatWasConfirmed) {
mMutex.lock();
mRateLimiting.processResult(responseCode);
sp<IBinder> listener = mCurrentListener;
@@ -150,18 +162,12 @@
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 @@
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 @@
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::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
diff --git a/keystore/confirmation_manager.h b/keystore/confirmation_manager.h
index 46b623c..7f0a11d 100644
--- a/keystore/confirmation_manager.h
+++ b/keystore/confirmation_manager.h
@@ -84,8 +84,12 @@
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;
diff --git a/keystore/confirmationui_rate_limiting.h b/keystore/confirmationui_rate_limiting.h
index 12c20fa..658bf41 100644
--- a/keystore/confirmationui_rate_limiting.h
+++ b/keystore/confirmationui_rate_limiting.h
@@ -29,8 +29,8 @@
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 @@
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