Merge changes I3002c0f6,I5163fa43,Idec1d83e,I57ea735b,Ibadfaac7
* changes:
Fix unnecessary copy initialization warnings
keystore: abort if verification token generation fails
Use TEE keymaster for ID attestation.
Fix operation pruning.
Manage DropBoxManager using sp<>
diff --git a/keystore/key_proto_handler.cpp b/keystore/key_proto_handler.cpp
index 3bf8c06..a106213 100644
--- a/keystore/key_proto_handler.cpp
+++ b/keystore/key_proto_handler.cpp
@@ -22,6 +22,7 @@
#include <keymasterV4_0/Keymaster.h>
#include <keystore/keymaster_types.h>
#include <utils/String16.h>
+#include <utils/StrongPointer.h>
#include "key_config.pb.h"
@@ -74,7 +75,7 @@
bool wasCreationSuccessful) {
KeyConfig keyConfig;
checkEnforcedCharacteristics(keyParams, &keyConfig);
- auto dropbox = std::make_unique<android::os::DropBoxManager>();
+ android::sp<android::os::DropBoxManager> dropbox(new android::os::DropBoxManager());
keyConfig.set_was_creation_successful(wasCreationSuccessful);
size_t size = keyConfig.ByteSize();
diff --git a/keystore/key_store_service.cpp b/keystore/key_store_service.cpp
index ee13006..2aaa625 100644
--- a/keystore/key_store_service.cpp
+++ b/keystore/key_store_service.cpp
@@ -1327,23 +1327,34 @@
result->outParams = outParams;
};
- ErrorCode rc =
+ KeyStoreServiceReturnCode rc =
KS_HANDLE_HIDL_ERROR(dev->begin(keyPurpose, key, opParams.hidl_data(), authToken, hidlCb));
- if (rc != ErrorCode::OK) {
- ALOGW("Got error %d from begin()", rc);
+ if (!rc.isOk()) {
+ LOG(ERROR) << "Got error " << rc << " from begin()";
+ result->resultCode = ResponseCode::SYSTEM_ERROR;
+ return Status::ok();
}
+ rc = result->resultCode;
+
// If there are too many operations abort the oldest operation that was
// started as pruneable and try again.
+ LOG(INFO) << rc << " " << mOperationMap.hasPruneableOperation();
while (rc == ErrorCode::TOO_MANY_OPERATIONS && mOperationMap.hasPruneableOperation()) {
- ALOGW("Ran out of operation handles");
+ LOG(INFO) << "Ran out of operation handles";
if (!pruneOperation()) {
break;
}
rc = KS_HANDLE_HIDL_ERROR(
dev->begin(keyPurpose, key, opParams.hidl_data(), authToken, hidlCb));
+ if (!rc.isOk()) {
+ LOG(ERROR) << "Got error " << rc << " from begin()";
+ result->resultCode = ResponseCode::SYSTEM_ERROR;
+ return Status::ok();
+ }
+ rc = result->resultCode;
}
- if (rc != ErrorCode::OK) {
+ if (!rc.isOk()) {
result->resultCode = rc;
return Status::ok();
}
@@ -1362,8 +1373,15 @@
verificationToken = token;
}));
- if (rc != ErrorCode::OK) result->resultCode = rc;
- if (result->resultCode != ErrorCode::OK) return Status::ok();
+ if (!rc.isOk()) result->resultCode = rc;
+ if (!result->resultCode.isOk()) {
+ LOG(ERROR) << "Failed to verify authorization " << rc << " from begin()";
+ rc = KS_HANDLE_HIDL_ERROR(dev->abort(result->handle));
+ if (!rc.isOk()) {
+ LOG(ERROR) << "Failed to abort operation " << rc << " from begin()";
+ }
+ return Status::ok();
+ }
}
// Note: The operation map takes possession of the contents of "characteristics".
@@ -1454,7 +1472,12 @@
// just a reminder: on success result->resultCode was set in the callback. So we only overwrite
// it if there was a communication error indicated by the ErrorCode.
- if (!rc.isOk()) result->resultCode = rc;
+ if (!rc.isOk()) {
+ result->resultCode = rc;
+ // removeOperation() will free the memory 'op' used, so the order is important
+ mAuthTokenTable.MarkCompleted(op.handle);
+ mOperationMap.removeOperation(token, /* wasOpSuccessful */ false);
+ }
return Status::ok();
}
@@ -1576,7 +1599,7 @@
}
int isDeviceIdAttestationRequested(const KeymasterArguments& params) {
- const hardware::hidl_vec<KeyParameter> paramsVec = params.getParameters();
+ const hardware::hidl_vec<KeyParameter>& paramsVec = params.getParameters();
int result = 0;
for (size_t i = 0; i < paramsVec.size(); ++i) {
switch (paramsVec[i].tag) {
@@ -1700,11 +1723,9 @@
}
// Generate temporary key.
- sp<Keymaster> dev;
- SecurityLevel securityLevel;
- std::tie(dev, securityLevel) = mKeyStore->getMostSecureDevice();
+ sp<Keymaster> dev = mKeyStore->getDevice(SecurityLevel::TRUSTED_ENVIRONMENT);
- if (securityLevel == SecurityLevel::SOFTWARE) {
+ if (!dev) {
*aidl_return = static_cast<int32_t>(ResponseCode::SYSTEM_ERROR);
return Status::ok();
}
diff --git a/keystore/operation_proto_handler.cpp b/keystore/operation_proto_handler.cpp
index 77e1b73..992232d 100644
--- a/keystore/operation_proto_handler.cpp
+++ b/keystore/operation_proto_handler.cpp
@@ -23,6 +23,7 @@
#include <keystore/keymaster_types.h>
#include <keystore/keystore_hidl_support.h>
#include <utils/String16.h>
+#include <utils/StrongPointer.h>
#include "operation_config.pb.h"
@@ -108,7 +109,7 @@
checkKeyCharacteristics(op.characteristics.softwareEnforced, &operationConfig);
checkKeyCharacteristics(op.characteristics.hardwareEnforced, &operationConfig);
checkOpCharacteristics(op.params, &operationConfig);
- auto dropbox = std::make_unique<android::os::DropBoxManager>();
+ android::sp<android::os::DropBoxManager> dropbox(new android::os::DropBoxManager);
operationConfig.set_was_op_successful(wasOpSuccessful);
size_t size = operationConfig.ByteSize();
