Merge "Confirmationui Rate Limiting App Abort Bug Fix"

This commit is contained in:
Treehugger Robot 2020-01-24 17:37:58 +00:00 committed by Gerrit Code Review
commit 2d5b18b9b7
3 changed files with 39 additions and 19 deletions

View file

@ -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,
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 @@ 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

View file

@ -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;

View file

@ -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