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
Merged-In: Ifceece542d6f3536ad87d053145c7aa8dd6d6603
Change-Id: Ifceece542d6f3536ad87d053145c7aa8dd6d6603
diff --git a/keystore/keymaster_worker.cpp b/keystore/keymaster_worker.cpp
index d2175b8..922ef0a 100644
--- a/keystore/keymaster_worker.cpp
+++ b/keystore/keymaster_worker.cpp
@@ -70,6 +70,10 @@
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 @@
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 @@
};
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 @@
}
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 @@
// 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 @@
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 @@
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 @@
// 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 @@
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 @@
};
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 @@
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 @@
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 @@
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 @@
}
};
- 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 @@
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 @@
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 @@
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 @@
}
};
- 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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
};
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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]() {