Clearly indicate vendor errors from keymaster in logcat

This patch addes verbose logging whenever an error code in the vendor
error code range is returned by keymaster.

Bug: 123562864
Test: atest android.keystore.cts
Change-Id: Ifceece542d6f3536ad87d053145c7aa8dd6d6603

5 files changed, 79 insertions, 68 deletions

diff --git a/keystore/KeyStore.cpp b/keystore/KeyStore.cpp
index 6e8a4b2a..d4219bd6 100644
--- a/keystore/KeyStore.cpp
+++ b/keystore/KeyStore.cpp
@@ -309,8 +309,8 @@ ResponseCode KeyStore::del(const LockedKeyBlobEntry& blobfile) {
     auto dev = getDevice(keyBlob);
 
     if (keyBlob.getType() == ::TYPE_KEYMASTER_10) {
-        dev->deleteKey(blob2hidlVec(keyBlob), [alias, uid](Return<ErrorCode> rc) {
-            auto ret = KS_HANDLE_HIDL_ERROR(rc);
+        dev->deleteKey(blob2hidlVec(keyBlob), [dev, alias, uid](Return<ErrorCode> rc) {
+            auto ret = KS_HANDLE_HIDL_ERROR(dev, rc);
             // A device doesn't have to implement delete_key.
             bool success = ret == ErrorCode::OK || ret == ErrorCode::UNIMPLEMENTED;
             if (__android_log_security()) {
diff --git a/keystore/include/keystore/keystore_hidl_support.h b/keystore/include/keystore/keystore_hidl_support.h
index 781b153a..d1d7f165 100644
--- a/keystore/include/keystore/keystore_hidl_support.h
+++ b/keystore/include/keystore/keystore_hidl_support.h
@@ -52,17 +52,20 @@ template <typename... Args> inline static std::string argsToString(Args&&... arg
     return s.str();
 }
 
-template <typename... Msgs>
-inline static ErrorCode ksHandleHidlError(const Return<ErrorCode>& error, Msgs&&... msgs) {
+template <typename KMDevice, typename... Msgs>
+inline static ErrorCode ksHandleHidlError(KMDevice dev, const Return<ErrorCode>& error,
+                                          Msgs&&... msgs) {
     if (!error.isOk()) {
-        ALOGE("HIDL call failed with %s @ %s", error.description().c_str(),
-              argsToString(msgs...).c_str());
+        LOG(ERROR) << "HIDL call failed with " << error.description().c_str() << " @ "
+                   << argsToString(msgs...);
         return ErrorCode::UNKNOWN_ERROR;
     }
-    return ErrorCode(error);
+    auto ec = ErrorCode(error);
+    dev->logIfKeymasterVendorError(ec);
+    return ec;
 }
-template <typename... Msgs>
-inline static ErrorCode ksHandleHidlError(const Return<void>& error, Msgs&&... msgs) {
+template <typename KMDevice, typename... Msgs>
+inline static ErrorCode ksHandleHidlError(KMDevice, const Return<void>& error, Msgs&&... msgs) {
     if (!error.isOk()) {
         ALOGE("HIDL call failed with %s @ %s", error.description().c_str(),
               argsToString(msgs...).c_str());
@@ -71,8 +74,8 @@ inline static ErrorCode ksHandleHidlError(const Return<void>& error, Msgs&&... m
     return ErrorCode::OK;
 }
 
-#define KS_HANDLE_HIDL_ERROR(rc)                                                                   \
-    ::keystore::ksHandleHidlError(rc, __FILE__, ":", __LINE__, ":", __PRETTY_FUNCTION__)
+#define KS_HANDLE_HIDL_ERROR(dev, rc)                                                              \
+    ::keystore::ksHandleHidlError(dev, rc, __FILE__, ":", __LINE__, ":", __PRETTY_FUNCTION__)
 
 template <typename T, typename OutIter>
 inline static OutIter copy_bytes_to_iterator(const T& value, OutIter dest) {
diff --git a/keystore/key_store_service.cpp b/keystore/key_store_service.cpp
index bbf93ad5..0bd3e03a 100644
--- a/keystore/key_store_service.cpp
+++ b/keystore/key_store_service.cpp
@@ -617,8 +617,8 @@ Status KeyStoreService::addRngEntropy(
         return AIDL_RETURN(ErrorCode::HARDWARE_TYPE_UNAVAILABLE);
     }
 
-    device->addRngEntropy(entropy, [cb](Return<ErrorCode> rc) {
-        cb->onFinished(KeyStoreServiceReturnCode(KS_HANDLE_HIDL_ERROR(rc)));
+    device->addRngEntropy(entropy, [device, cb](Return<ErrorCode> rc) {
+        cb->onFinished(KeyStoreServiceReturnCode(KS_HANDLE_HIDL_ERROR(device, rc)));
     });
 
     return AIDL_RETURN(ResponseCode::NO_ERROR);
@@ -1012,11 +1012,13 @@ Status KeyStoreService::attestKey(
     auto hidlKey = blob2hidlVec(keyBlob);
     dev->attestKey(
         std::move(hidlKey), mutableParams.hidl_data(),
-        [cb](Return<void> rc, std::tuple<ErrorCode, hidl_vec<hidl_vec<uint8_t>>>&& hidlResult) {
+        [dev, cb](Return<void> rc,
+                  std::tuple<ErrorCode, hidl_vec<hidl_vec<uint8_t>>>&& hidlResult) {
             auto& [ret, certChain] = hidlResult;
             if (!rc.isOk()) {
                 cb->onFinished(KeyStoreServiceReturnCode(ResponseCode::SYSTEM_ERROR), {});
             } else if (ret != ErrorCode::OK) {
+                dev->logIfKeymasterVendorError(ret);
                 cb->onFinished(KeyStoreServiceReturnCode(ret), {});
             } else {
                 cb->onFinished(KeyStoreServiceReturnCode(ret),
@@ -1094,6 +1096,7 @@ Status KeyStoreService::attestDeviceIds(
                 return;
             }
             if (ret != ErrorCode::OK) {
+                dev->logIfKeymasterVendorError(ret);
                 cb->onFinished(KeyStoreServiceReturnCode(ret), {});
                 return;
             }
@@ -1104,9 +1107,9 @@ Status KeyStoreService::attestDeviceIds(
                               std::tuple<ErrorCode, hidl_vec<hidl_vec<uint8_t>>>&& hidlResult) {
                     auto& [ret, certChain] = hidlResult;
                     // schedule temp key for deletion
-                    dev->deleteKey(std::move(hidlKeyBlob), [](Return<ErrorCode> rc) {
+                    dev->deleteKey(std::move(hidlKeyBlob), [dev](Return<ErrorCode> rc) {
                         // log error but don't return an error
-                        KS_HANDLE_HIDL_ERROR(rc);
+                        KS_HANDLE_HIDL_ERROR(dev, rc);
                     });
                     if (!rc.isOk()) {
                         cb->onFinished(KeyStoreServiceReturnCode(ResponseCode::SYSTEM_ERROR), {});
@@ -1117,6 +1120,7 @@ Status KeyStoreService::attestDeviceIds(
                             KeyStoreServiceReturnCode(ret),
                             ::android::security::keymaster::KeymasterCertificateChain(certChain));
                     } else {
+                        dev->logIfKeymasterVendorError(ret);
                         cb->onFinished(KeyStoreServiceReturnCode(ret), {});
                     }
                 });
diff --git a/keystore/keymaster_worker.cpp b/keystore/keymaster_worker.cpp
index d2175b8f..922ef0a4 100644
--- a/keystore/keymaster_worker.cpp
+++ b/keystore/keymaster_worker.cpp
@@ -70,6 +70,10 @@ KeymasterWorker::KeymasterWorker(sp<Keymaster> keymasterDevice, KeyStore* keySto
     if (keymasterDevice_) keymasterDevice_->halVersion();
 }
 
+void KeymasterWorker::logIfKeymasterVendorError(ErrorCode ec) const {
+    keymasterDevice_->logIfKeymasterVendorError(ec);
+}
+
 std::tuple<KeyStoreServiceReturnCode, Blob>
 KeymasterWorker::upgradeKeyBlob(const LockedKeyBlobEntry& lockedEntry,
                                 const AuthorizationSet& params) {
@@ -98,6 +102,7 @@ KeymasterWorker::upgradeKeyBlob(const LockedKeyBlobEntry& lockedEntry,
     auto& dev = keymasterDevice_;
 
     auto hidlCb = [&](ErrorCode ret, const ::std::vector<uint8_t>& upgradedKeyBlob) {
+        dev->logIfKeymasterVendorError(ret);
         error = ret;
         if (!error.isOk()) {
             if (error == ErrorCode::INVALID_KEY_BLOB) {
@@ -128,7 +133,7 @@ KeymasterWorker::upgradeKeyBlob(const LockedKeyBlobEntry& lockedEntry,
     };
 
     KeyStoreServiceReturnCode error2;
-    error2 = KS_HANDLE_HIDL_ERROR(dev->upgradeKey(hidlKey, params.hidl_data(), hidlCb));
+    error2 = KS_HANDLE_HIDL_ERROR(dev, dev->upgradeKey(hidlKey, params.hidl_data(), hidlCb));
     if (!error2.isOk()) {
         return error = error2, result;
     }
@@ -171,6 +176,7 @@ KeymasterWorker::createKeyCharacteristicsCache(const LockedKeyBlobEntry& lockedE
     }
 
     auto hidlCb = [&](ErrorCode ret, const KeyCharacteristics& keyCharacteristics) {
+        dev->logIfKeymasterVendorError(ret);
         error = ret;
         if (!error.isOk()) {
             if (error == ErrorCode::INVALID_KEY_BLOB) {
@@ -207,7 +213,7 @@ KeymasterWorker::createKeyCharacteristicsCache(const LockedKeyBlobEntry& lockedE
         // this updates the key characteristics cache file to the new format or creates one in
         // in the first place
         rc = KS_HANDLE_HIDL_ERROR(
-            dev->getKeyCharacteristics(hidlKeyBlob, clientId, appData, hidlCb));
+            dev, dev->getKeyCharacteristics(hidlKeyBlob, clientId, appData, hidlCb));
         if (!rc.isOk()) {
             return result;
         }
@@ -228,7 +234,7 @@ KeymasterWorker::createKeyCharacteristicsCache(const LockedKeyBlobEntry& lockedE
             auto upgradedHidlKeyBlob = blob2hidlVec(keyBlob);
 
             rc = KS_HANDLE_HIDL_ERROR(
-                dev->getKeyCharacteristics(upgradedHidlKeyBlob, clientId, appData, hidlCb));
+                dev, dev->getKeyCharacteristics(upgradedHidlKeyBlob, clientId, appData, hidlCb));
             if (!rc.isOk()) {
                 return result;
             }
@@ -299,7 +305,7 @@ KeyStoreServiceReturnCode KeymasterWorker::abort(const sp<IBinder>& token) {
     auto op = operationMap_.removeOperation(token, false /* wasOpSuccessful */);
     if (op) {
         keyStore_->getAuthTokenTable().MarkCompleted(op->handle);
-        return KS_HANDLE_HIDL_ERROR(keymasterDevice_->abort(op->handle));
+        return KS_HANDLE_HIDL_ERROR(keymasterDevice_, keymasterDevice_->abort(op->handle));
     } else {
         return ErrorCode::INVALID_OPERATION_HANDLE;
     }
@@ -376,7 +382,7 @@ void KeymasterWorker::begin(LockedKeyBlobEntry lockedEntry, sp<IBinder> appToken
 
         // Add entropy to the device first.
         if (entropy.size()) {
-            rc = KS_HANDLE_HIDL_ERROR(dev->addRngEntropy(entropy));
+            rc = KS_HANDLE_HIDL_ERROR(dev, dev->addRngEntropy(entropy));
             if (!rc.isOk()) {
                 return worker_cb(operationFailed(rc));
             }
@@ -414,6 +420,7 @@ void KeymasterWorker::begin(LockedKeyBlobEntry lockedEntry, sp<IBinder> appToken
 
         auto hidlCb = [&](ErrorCode ret, const hidl_vec<KeyParameter>& outParams,
                           uint64_t operationHandle) {
+            dev->logIfKeymasterVendorError(ret);
             result.resultCode = ret;
             if (!result.resultCode.isOk()) {
                 if (result.resultCode == ErrorCode::INVALID_KEY_BLOB) {
@@ -426,8 +433,8 @@ void KeymasterWorker::begin(LockedKeyBlobEntry lockedEntry, sp<IBinder> appToken
         };
 
         do {
-            rc = KS_HANDLE_HIDL_ERROR(dev->begin(purpose, blob2hidlVec(keyBlob),
-                                                 opParams.hidl_data(), authToken, hidlCb));
+            rc = KS_HANDLE_HIDL_ERROR(dev, dev->begin(purpose, blob2hidlVec(keyBlob),
+                                                      opParams.hidl_data(), authToken, hidlCb));
             if (!rc.isOk()) {
                 LOG(ERROR) << "Got error " << rc << " from begin()";
                 return worker_cb(operationFailed(ResponseCode::SYSTEM_ERROR));
@@ -439,8 +446,8 @@ void KeymasterWorker::begin(LockedKeyBlobEntry lockedEntry, sp<IBinder> appToken
                     return worker_cb(operationFailed(rc));
                 }
 
-                rc = KS_HANDLE_HIDL_ERROR(dev->begin(purpose, blob2hidlVec(keyBlob),
-                                                     opParams.hidl_data(), authToken, hidlCb));
+                rc = KS_HANDLE_HIDL_ERROR(dev, dev->begin(purpose, blob2hidlVec(keyBlob),
+                                                          opParams.hidl_data(), authToken, hidlCb));
                 if (!rc.isOk()) {
                     LOG(ERROR) << "Got error " << rc << " from begin()";
                     return worker_cb(operationFailed(ResponseCode::SYSTEM_ERROR));
@@ -557,7 +564,7 @@ void KeymasterWorker::update(sp<IBinder> token, AuthorizationSet params, hidl_ve
         Finalize abort_operation_in_case_of_error([&] {
             operationMap_.removeOperation(token, false);
             keyStore_->getAuthTokenTable().MarkCompleted(op->handle);
-            KS_HANDLE_HIDL_ERROR(keymasterDevice_->abort(op->handle));
+            KS_HANDLE_HIDL_ERROR(keymasterDevice_, keymasterDevice_->abort(op->handle));
         });
 
         rc = getOperationAuthTokenIfNeeded(op);
@@ -577,6 +584,7 @@ void KeymasterWorker::update(sp<IBinder> token, AuthorizationSet params, hidl_ve
         auto hidlCb = [&](ErrorCode ret, uint32_t inputConsumed,
                           const hidl_vec<KeyParameter>& outParams,
                           const ::std::vector<uint8_t>& output) {
+            op->device->logIfKeymasterVendorError(ret);
             result.resultCode = ret;
             if (result.resultCode.isOk()) {
                 result.inputConsumed = inputConsumed;
@@ -585,7 +593,8 @@ void KeymasterWorker::update(sp<IBinder> token, AuthorizationSet params, hidl_ve
             }
         };
 
-        rc = KS_HANDLE_HIDL_ERROR(op->device->update(op->handle, params.hidl_data(), data,
+        rc = KS_HANDLE_HIDL_ERROR(op->device,
+                                  op->device->update(op->handle, params.hidl_data(), data,
                                                      op->authToken, op->verificationToken, hidlCb));
 
         // just a reminder: on success result->resultCode was set in the callback. So we only
@@ -634,7 +643,8 @@ void KeymasterWorker::finish(sp<IBinder> token, AuthorizationSet params, hidl_ve
         Finalize abort_operation_in_case_of_error([&] {
             operationMap_.removeOperation(token, finished && rc.isOk());
             keyStore_->getAuthTokenTable().MarkCompleted(op->handle);
-            if (!finished) KS_HANDLE_HIDL_ERROR(keymasterDevice_->abort(op->handle));
+            if (!finished)
+                KS_HANDLE_HIDL_ERROR(keymasterDevice_, keymasterDevice_->abort(op->handle));
         });
 
         if (!checkAllowedOperationParams(params.begin(), params.end())) {
@@ -665,7 +675,7 @@ void KeymasterWorker::finish(sp<IBinder> token, AuthorizationSet params, hidl_ve
         if (!rc.isOk()) return worker_cb(operationFailed(rc));
 
         if (entropy.size()) {
-            rc = KS_HANDLE_HIDL_ERROR(op->device->addRngEntropy(entropy));
+            rc = KS_HANDLE_HIDL_ERROR(op->device, op->device->addRngEntropy(entropy));
             if (!rc.isOk()) {
                 return worker_cb(operationFailed(rc));
             }
@@ -674,6 +684,7 @@ void KeymasterWorker::finish(sp<IBinder> token, AuthorizationSet params, hidl_ve
         OperationResult result;
         auto hidlCb = [&](ErrorCode ret, const hidl_vec<KeyParameter>& outParams,
                           const ::std::vector<uint8_t>& output) {
+            op->device->logIfKeymasterVendorError(ret);
             result.resultCode = ret;
             if (result.resultCode.isOk()) {
                 result.outParams = outParams;
@@ -681,9 +692,9 @@ void KeymasterWorker::finish(sp<IBinder> token, AuthorizationSet params, hidl_ve
             }
         };
 
-        rc = KS_HANDLE_HIDL_ERROR(op->device->finish(op->handle, params.hidl_data(), input,
-                                                     signature, op->authToken,
-                                                     op->verificationToken, hidlCb));
+        rc = KS_HANDLE_HIDL_ERROR(op->device, op->device->finish(op->handle, params.hidl_data(),
+                                                                 input, signature, op->authToken,
+                                                                 op->verificationToken, hidlCb));
 
         if (rc.isOk()) {
             // inform the finalizer that the finish call went through
@@ -709,11 +720,14 @@ void KeymasterWorker::verifyAuthorization(uint64_t challenge, hidl_vec<KeyParame
                         CAPTURE_MOVE(worker_cb)]() {
         KeyStoreServiceReturnCode error;
         VerificationToken verificationToken;
-        KeyStoreServiceReturnCode rc = KS_HANDLE_HIDL_ERROR(keymasterDevice_->verifyAuthorization(
-            challenge, params, token, [&](ErrorCode error_, const VerificationToken& vToken) {
-                error = error_;
-                verificationToken = vToken;
-            }));
+        KeyStoreServiceReturnCode rc = KS_HANDLE_HIDL_ERROR(
+            keymasterDevice_,
+            keymasterDevice_->verifyAuthorization(
+                challenge, params, token, [&](ErrorCode ret, const VerificationToken& vToken) {
+                    keymasterDevice_->logIfKeymasterVendorError(ret);
+                    error = ret;
+                    verificationToken = vToken;
+                }));
         worker_cb(rc.isOk() ? error : rc, std::move(token), std::move(verificationToken));
     });
 }
@@ -739,7 +753,7 @@ void KeymasterWorker::generateKey(LockedKeyBlobEntry lockedEntry, hidl_vec<KeyPa
     Worker::addRequest([this, CAPTURE_MOVE(lockedEntry), CAPTURE_MOVE(keyParams),
                         CAPTURE_MOVE(entropy), CAPTURE_MOVE(worker_cb), flags]() mutable {
         KeyStoreServiceReturnCode rc =
-            KS_HANDLE_HIDL_ERROR(keymasterDevice_->addRngEntropy(entropy));
+            KS_HANDLE_HIDL_ERROR(keymasterDevice_, keymasterDevice_->addRngEntropy(entropy));
         if (!rc.isOk()) {
             return worker_cb(rc, {});
         }
@@ -757,6 +771,7 @@ void KeymasterWorker::generateKey(LockedKeyBlobEntry lockedEntry, hidl_vec<KeyPa
         KeyStoreServiceReturnCode error;
         auto hidl_cb = [&](ErrorCode ret, const hidl_vec<uint8_t>& hidlKeyBlob,
                            const KeyCharacteristics& keyCharacteristics) {
+            keymasterDevice_->logIfKeymasterVendorError(ret);
             error = ret;
             if (!error.isOk()) {
                 return;
@@ -788,7 +803,8 @@ void KeymasterWorker::generateKey(LockedKeyBlobEntry lockedEntry, hidl_vec<KeyPa
             error = keyStore_->put(lockedEntry, std::move(keyBlob), std::move(keyCharBlob));
         };
 
-        rc = KS_HANDLE_HIDL_ERROR(keymasterDevice_->generateKey(keyParams, hidl_cb));
+        rc = KS_HANDLE_HIDL_ERROR(keymasterDevice_,
+                                  keymasterDevice_->generateKey(keyParams, hidl_cb));
         if (!rc.isOk()) {
             return worker_cb(rc, {});
         }
@@ -859,6 +875,7 @@ void KeymasterWorker::importKey(LockedKeyBlobEntry lockedEntry, hidl_vec<KeyPara
         KeyStoreServiceReturnCode error;
         auto hidl_cb = [&](ErrorCode ret, const hidl_vec<uint8_t>& hidlKeyBlob,
                            const KeyCharacteristics& keyCharacteristics) {
+            keymasterDevice_->logIfKeymasterVendorError(ret);
             error = ret;
             if (!error.isOk()) {
                 LOG(INFO) << "importKey failed";
@@ -892,7 +909,7 @@ void KeymasterWorker::importKey(LockedKeyBlobEntry lockedEntry, hidl_vec<KeyPara
         };
 
         KeyStoreServiceReturnCode rc = KS_HANDLE_HIDL_ERROR(
-            keymasterDevice_->importKey(keyParams, keyFormat, keyData, hidl_cb));
+            keymasterDevice_, keymasterDevice_->importKey(keyParams, keyFormat, keyData, hidl_cb));
         if (!rc.isOk()) {
             return worker_cb(rc, {});
         }
@@ -949,6 +966,7 @@ void KeymasterWorker::importWrappedKey(LockedKeyBlobEntry wrappingLockedEntry,
 
         auto hidlCb = [&](ErrorCode ret, const hidl_vec<uint8_t>& hidlKeyBlob,
                           const KeyCharacteristics& keyCharacteristics) {
+            keymasterDevice_->logIfKeymasterVendorError(ret);
             error = ret;
             if (!error.isOk()) {
                 return;
@@ -972,9 +990,10 @@ void KeymasterWorker::importWrappedKey(LockedKeyBlobEntry wrappingLockedEntry,
             error = keyStore_->put(wrapppedLockedEntry, std::move(keyBlob), std::move(keyCharBlob));
         };
 
-        KeyStoreServiceReturnCode rc = KS_HANDLE_HIDL_ERROR(keymasterDevice_->importWrappedKey(
-            wrappedKeyData, hidlWrappingKey, maskingKey, unwrappingParams, passwordSid,
-            biometricSid, hidlCb));
+        KeyStoreServiceReturnCode rc = KS_HANDLE_HIDL_ERROR(
+            keymasterDevice_, keymasterDevice_->importWrappedKey(
+                                  wrappedKeyData, hidlWrappingKey, maskingKey, unwrappingParams,
+                                  passwordSid, biometricSid, hidlCb));
 
         // possible hidl error
         if (!rc.isOk()) {
@@ -989,9 +1008,10 @@ void KeymasterWorker::importWrappedKey(LockedKeyBlobEntry wrappingLockedEntry,
 
             auto upgradedHidlKeyBlob = blob2hidlVec(wrappingBlob);
 
-            rc = KS_HANDLE_HIDL_ERROR(keymasterDevice_->importWrappedKey(
-                wrappedKeyData, upgradedHidlKeyBlob, maskingKey, unwrappingParams, passwordSid,
-                biometricSid, hidlCb));
+            rc = KS_HANDLE_HIDL_ERROR(keymasterDevice_,
+                                      keymasterDevice_->importWrappedKey(
+                                          wrappedKeyData, upgradedHidlKeyBlob, maskingKey,
+                                          unwrappingParams, passwordSid, biometricSid, hidlCb));
             if (!rc.isOk()) {
                 error = rc;
             }
@@ -1011,6 +1031,7 @@ void KeymasterWorker::exportKey(LockedKeyBlobEntry lockedEntry, KeyFormat export
         ExportResult result;
         auto hidlCb = [&](ErrorCode ret,
                           const ::android::hardware::hidl_vec<uint8_t>& keyMaterial) {
+            keymasterDevice_->logIfKeymasterVendorError(ret);
             result.resultCode = ret;
             if (!result.resultCode.isOk()) {
                 if (result.resultCode == ErrorCode::INVALID_KEY_BLOB) {
@@ -1021,6 +1042,7 @@ void KeymasterWorker::exportKey(LockedKeyBlobEntry lockedEntry, KeyFormat export
             result.exportData = keyMaterial;
         };
         KeyStoreServiceReturnCode rc = KS_HANDLE_HIDL_ERROR(
+            keymasterDevice_,
             keymasterDevice_->exportKey(exportFormat, key, clientId, appData, hidlCb));
 
         // Overwrite result->resultCode only on HIDL error. Otherwise we want the result set in the
@@ -1044,7 +1066,8 @@ void KeymasterWorker::exportKey(LockedKeyBlobEntry lockedEntry, KeyFormat export
 
             auto upgradedHidlKeyBlob = blob2hidlVec(keyBlob);
 
-            rc = KS_HANDLE_HIDL_ERROR(keymasterDevice_->exportKey(exportFormat, upgradedHidlKeyBlob,
+            rc = KS_HANDLE_HIDL_ERROR(keymasterDevice_,
+                                      keymasterDevice_->exportKey(exportFormat, upgradedHidlKeyBlob,
                                                                   clientId, appData, hidlCb));
             if (!rc.isOk()) {
                 result.resultCode = rc;
@@ -1058,21 +1081,10 @@ void KeymasterWorker::attestKey(hidl_vec<uint8_t> keyToAttest, hidl_vec<KeyParam
     addRequest(&Keymaster::attestKey, std::move(worker_cb), std::move(keyToAttest),
                std::move(attestParams));
 }
-void KeymasterWorker::upgradeKey(hidl_vec<uint8_t> keyBlobToUpgrade,
-                                 hidl_vec<KeyParameter> upgradeParams, upgradeKey_cb _hidl_cb) {
-    addRequest(&Keymaster::upgradeKey, std::move(_hidl_cb), std::move(keyBlobToUpgrade),
-               std::move(upgradeParams));
-}
 
 void KeymasterWorker::deleteKey(hidl_vec<uint8_t> keyBlob, deleteKey_cb _hidl_cb) {
     addRequest(&Keymaster::deleteKey, std::move(_hidl_cb), std::move(keyBlob));
 }
-void KeymasterWorker::deleteAllKeys(deleteAllKeys_cb _hidl_cb) {
-    addRequest(&Keymaster::deleteAllKeys, std::move(_hidl_cb));
-}
-void KeymasterWorker::destroyAttestationIds(destroyAttestationIds_cb _hidl_cb) {
-    addRequest(&Keymaster::destroyAttestationIds, move(_hidl_cb));
-}
 
 void KeymasterWorker::binderDied(android::wp<IBinder> who) {
     Worker::addRequest([this, who]() {
diff --git a/keystore/keymaster_worker.h b/keystore/keymaster_worker.h
index c02d389c..e1a1c023 100644
--- a/keystore/keymaster_worker.h
+++ b/keystore/keymaster_worker.h
@@ -207,6 +207,8 @@ class KeymasterWorker : protected Worker {
   public:
     KeymasterWorker(sp<Keymaster> keymasterDevice, KeyStore* keyStore);
 
+    void logIfKeymasterVendorError(ErrorCode ec) const;
+
     using worker_begin_cb = std::function<void(::android::security::keymaster::OperationResult)>;
     void begin(LockedKeyBlobEntry, sp<IBinder> appToken, Blob keyBlob, Blob charBlob,
                bool pruneable, KeyPurpose purpose, AuthorizationSet opParams,
@@ -279,19 +281,9 @@ class KeymasterWorker : protected Worker {
     void attestKey(hidl_vec<uint8_t> keyToAttest, hidl_vec<KeyParameter> attestParams,
                    attestKey_cb _hidl_cb);
 
-    using upgradeKey_cb = MakeKeymasterWorkerCB_t<Return<void>, Keymaster::upgradeKey_cb>;
-    void upgradeKey(hidl_vec<uint8_t> keyBlobToUpgrade, hidl_vec<KeyParameter> upgradeParams,
-                    upgradeKey_cb _hidl_cb);
-
     using deleteKey_cb = MakeKeymasterWorkerCB_t<Return<ErrorCode>>;
     void deleteKey(hidl_vec<uint8_t> keyBlob, deleteKey_cb _hidl_cb);
 
-    using deleteAllKeys_cb = MakeKeymasterWorkerCB_t<Return<ErrorCode>>;
-    void deleteAllKeys(deleteAllKeys_cb _hidl_cb);
-
-    using destroyAttestationIds_cb = MakeKeymasterWorkerCB_t<Return<ErrorCode>>;
-    void destroyAttestationIds(destroyAttestationIds_cb _hidl_cb);
-
     using begin_cb = MakeKeymasterWorkerCB_t<Return<void>, Keymaster::begin_cb>;
     void begin(KeyPurpose purpose, hidl_vec<uint8_t> key, hidl_vec<KeyParameter> inParams,
                HardwareAuthToken authToken, begin_cb _hidl_cb);