From ed9a255fc6e66715d7f14cc44f1ccbd767c0f3c5 Mon Sep 17 00:00:00 2001 From: Janis Danisevskis Date: Tue, 21 Jan 2020 14:33:30 -0800 Subject: Add permission check on onKeyguardVisibilityChanged Without this permission check any app can toggle the locked state of keymaster once it has been unlocked for the first time. Bug: 144285084 Test: Manually tested with debugger that the requred code paths are run. Merged-In: Idb8a200dc2963e1085e9fddd0c565c5172465e65 Change-Id: Idb8a200dc2963e1085e9fddd0c565c5172465e65 (cherry picked from commit 21f452c3722ad7fa39c7d84c4723bcbb723ab164) --- keystore/key_store_service.cpp | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/keystore/key_store_service.cpp b/keystore/key_store_service.cpp index 5e7efab0..e0ee9374 100644 --- a/keystore/key_store_service.cpp +++ b/keystore/key_store_service.cpp @@ -1354,12 +1354,23 @@ bool KeyStoreService::checkAllowedOperationParams(const hidl_vec& } Status KeyStoreService::onKeyguardVisibilityChanged(bool isShowing, int32_t userId, - int32_t* aidl_return) { + int32_t* _aidl_return) { KEYSTORE_SERVICE_LOCK; + if (isShowing) { + if (!checkBinderPermission(P_LOCK, UID_SELF)) { + LOG(WARNING) << "onKeyguardVisibilityChanged called with isShowing == true but " + "without LOCK permission"; + return AIDL_RETURN(ResponseCode::PERMISSION_DENIED); + } + } else { + if (!checkBinderPermission(P_UNLOCK, UID_SELF)) { + LOG(WARNING) << "onKeyguardVisibilityChanged called with isShowing == false but " + "without UNLOCK permission"; + return AIDL_RETURN(ResponseCode::PERMISSION_DENIED); + } + } mKeyStore->getEnforcementPolicy().set_device_locked(isShowing, userId); - *aidl_return = static_cast(ResponseCode::NO_ERROR); - - return Status::ok(); + return AIDL_RETURN(ResponseCode::NO_ERROR); } } // namespace keystore -- cgit v1.2.3 From 3cac4c660ad0392c34f0c688bfc188a10d4f28d3 Mon Sep 17 00:00:00 2001 From: Janis Danisevskis Date: Tue, 21 Jan 2020 14:33:30 -0800 Subject: Add permission check on onKeyguardVisibilityChanged Without this permission check any app can toggle the locked state of keymaster once it has been unlocked for the first time. Bug: 144285084 Test: Manually tested with debugger that the requred code paths are run. Change-Id: Idb8a200dc2963e1085e9fddd0c565c5172465e65 Merged-In: Idb8a200dc2963e1085e9fddd0c565c5172465e65 (cherry picked from commit 21f452c3722ad7fa39c7d84c4723bcbb723ab164) --- keystore/key_store_service.cpp | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/keystore/key_store_service.cpp b/keystore/key_store_service.cpp index 400c8142..999da59c 100644 --- a/keystore/key_store_service.cpp +++ b/keystore/key_store_service.cpp @@ -2353,15 +2353,24 @@ KeyStoreServiceReturnCode KeyStoreService::upgradeKeyBlob(const String16& name, } Status KeyStoreService::onKeyguardVisibilityChanged(bool isShowing, int32_t userId, - int32_t* aidl_return) { + int32_t* _aidl_return) { KEYSTORE_SERVICE_LOCK; - enforcement_policy.set_device_locked(isShowing, userId); - if (!isShowing) { + if (isShowing) { + if (!checkBinderPermission(P_LOCK, UID_SELF)) { + LOG(WARNING) << "onKeyguardVisibilityChanged called with isShowing == true but " + "without LOCK permission"; + return AIDL_RETURN(ResponseCode::PERMISSION_DENIED); + } + } else { + if (!checkBinderPermission(P_UNLOCK, UID_SELF)) { + LOG(WARNING) << "onKeyguardVisibilityChanged called with isShowing == false but " + "without UNLOCK permission"; + return AIDL_RETURN(ResponseCode::PERMISSION_DENIED); + } mActiveUserId = userId; } - *aidl_return = static_cast(ResponseCode::NO_ERROR); - - return Status::ok(); + enforcement_policy.set_device_locked(isShowing, userId); + return AIDL_RETURN(ResponseCode::NO_ERROR); } } // namespace keystore -- cgit v1.2.3