tequilaOS/platform_system_security
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

Confirmationui Rate Limiting App Abort Bug Fix

Browse source 
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.

Change-Id: I5f3af166391a32748a26f7709d30a5ac718499c0
This commit is contained in:
Cindy Zhou 2019-11-13 16:59:41 -08:00
parent 1281bcdb01
commit efe6ad0b83
3 changed files with 39 additions and 19 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

40
keystore/confirmation_manager.cpp
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

6
keystore/confirmation_manager.h
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;

12
keystore/confirmationui_rate_limiting.h
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