From 2ecd6597f3be11d76c5c3b11b6e3c67aedae3823 Mon Sep 17 00:00:00 2001 From: Janis Danisevskis Date: Mon, 8 Oct 2018 07:30:31 -0700 Subject: [PATCH] Various fixes for async keystore. * Added missing Tag::HARDWARE_TYPE and Tag::TRUSTED_CONFIRMATION_REQUIRED * Made AuthorizationSet::hidl_data() safer to use. hidl_data() initializes a hidl_vec with the internal data of std::vector using setToExternal and returns it by value. This means the returned temporay does not own the buffer which has the life cycle of the AuthorizationSet. This is fine if passed as parameter to a function where it is bound to a cont reference. But if the temporary gets assigned to something with longer life cycle move semantics kicks in and the buffer is now tracked by something with a longer life cycle. This patch marks the returned temporary const, so that it can no longer be moved. It can still be bound to a const reference, but when assigned to a variable it must get copied. * Add Filter function to AuthorizationSet. Bug: 111443219 Test: KeyStore CTS tests Change-Id: I4744b7c87d01fbd905c3afb8ebeefba93605994b --- keymaster/4.0/support/authorization_set.cpp | 23 ++++++++++++++++--- .../support/include/keymasterV4_0/Keymaster.h | 3 +++ .../include/keymasterV4_0/authorization_set.h | 7 +++++- .../include/keymasterV4_0/keymaster_tags.h | 15 +++++++----- 4 files changed, 38 insertions(+), 10 deletions(-) diff --git a/keymaster/4.0/support/authorization_set.cpp b/keymaster/4.0/support/authorization_set.cpp index afbcdac6d7..10d272a9bf 100644 --- a/keymaster/4.0/support/authorization_set.cpp +++ b/keymaster/4.0/support/authorization_set.cpp @@ -18,6 +18,8 @@ #include +#include + namespace android { namespace hardware { namespace keymaster { @@ -97,10 +99,10 @@ void AuthorizationSet::Deduplicate() { if (prev->tag == Tag::INVALID) continue; if (!keyParamEqual(*prev, *curr)) { - result.emplace_back(std::move(*prev)); + result.push_back(std::move(*prev)); } } - result.emplace_back(std::move(*prev)); + result.push_back(std::move(*prev)); std::swap(data_, result); } @@ -127,6 +129,16 @@ void AuthorizationSet::Subtract(const AuthorizationSet& other) { } } +void AuthorizationSet::Filter(std::function doKeep) { + std::vector result; + for (auto& param : data_) { + if (doKeep(param)) { + result.push_back(std::move(param)); + } + } + std::swap(data_, result); +} + KeyParameter& AuthorizationSet::operator[](int at) { return data_[at]; } @@ -248,7 +260,12 @@ struct choose_serializer> { template <> struct choose_serializer<> { - static OutStreams& serialize(OutStreams& out, const KeyParameter&) { return out; } + static OutStreams& serialize(OutStreams& out, const KeyParameter& param) { + LOG(FATAL) << "Trying to serialize unknown tag " << unsigned(param.tag) + << ". Did you forget to add it to all_tags_t?"; + abort(); + return out; + } }; template diff --git a/keymaster/4.0/support/include/keymasterV4_0/Keymaster.h b/keymaster/4.0/support/include/keymasterV4_0/Keymaster.h index 83b1d69990..458053a4ea 100644 --- a/keymaster/4.0/support/include/keymasterV4_0/Keymaster.h +++ b/keymaster/4.0/support/include/keymasterV4_0/Keymaster.h @@ -20,6 +20,9 @@ #include +#include +#include + namespace android { namespace hardware { namespace keymaster { diff --git a/keymaster/4.0/support/include/keymasterV4_0/authorization_set.h b/keymaster/4.0/support/include/keymasterV4_0/authorization_set.h index 193e4ea25e..ac96c863ae 100644 --- a/keymaster/4.0/support/include/keymasterV4_0/authorization_set.h +++ b/keymaster/4.0/support/include/keymasterV4_0/authorization_set.h @@ -141,6 +141,11 @@ class AuthorizationSet { */ std::vector::const_iterator end() const { return data_.end(); } + /** + * Modifies this Authorization set such that it only keeps the entries for which doKeep + * returns true. + */ + void Filter(std::function doKeep); /** * Returns the nth element of the set. * Like for std::vector::operator[] there is no range check performed. Use of out of range @@ -209,7 +214,7 @@ class AuthorizationSet { } } - hidl_vec hidl_data() const { + const hidl_vec hidl_data() const { hidl_vec result; result.setToExternal(const_cast(data()), size()); return result; diff --git a/keymaster/4.0/support/include/keymasterV4_0/keymaster_tags.h b/keymaster/4.0/support/include/keymasterV4_0/keymaster_tags.h index 9e7d252226..61c444c161 100644 --- a/keymaster/4.0/support/include/keymasterV4_0/keymaster_tags.h +++ b/keymaster/4.0/support/include/keymasterV4_0/keymaster_tags.h @@ -122,6 +122,7 @@ DECLARE_TYPED_TAG(CONFIRMATION_TOKEN); DECLARE_TYPED_TAG(CREATION_DATETIME); DECLARE_TYPED_TAG(DIGEST); DECLARE_TYPED_TAG(EC_CURVE); +DECLARE_TYPED_TAG(HARDWARE_TYPE); DECLARE_TYPED_TAG(INCLUDE_UNIQUE_ID); DECLARE_TYPED_TAG(INVALID); DECLARE_TYPED_TAG(KEY_SIZE); @@ -162,12 +163,13 @@ using all_tags_t = TAG_USER_SECURE_ID_t, TAG_NO_AUTH_REQUIRED_t, TAG_AUTH_TIMEOUT_t, TAG_ALLOW_WHILE_ON_BODY_t, TAG_UNLOCKED_DEVICE_REQUIRED_t, TAG_APPLICATION_ID_t, TAG_APPLICATION_DATA_t, TAG_CREATION_DATETIME_t, TAG_ROLLBACK_RESISTANCE_t, - TAG_ROOT_OF_TRUST_t, TAG_ASSOCIATED_DATA_t, TAG_NONCE_t, TAG_BOOTLOADER_ONLY_t, - TAG_OS_VERSION_t, TAG_OS_PATCHLEVEL_t, TAG_UNIQUE_ID_t, TAG_ATTESTATION_CHALLENGE_t, - TAG_ATTESTATION_APPLICATION_ID_t, TAG_RESET_SINCE_ID_ROTATION_t, TAG_PURPOSE_t, - TAG_ALGORITHM_t, TAG_BLOCK_MODE_t, TAG_DIGEST_t, TAG_PADDING_t, - TAG_BLOB_USAGE_REQUIREMENTS_t, TAG_ORIGIN_t, TAG_USER_AUTH_TYPE_t, TAG_EC_CURVE_t, - TAG_BOOT_PATCHLEVEL_t, TAG_VENDOR_PATCHLEVEL_t, TAG_TRUSTED_USER_PRESENCE_REQUIRED_t>; + TAG_HARDWARE_TYPE_t, TAG_ROOT_OF_TRUST_t, TAG_ASSOCIATED_DATA_t, TAG_NONCE_t, + TAG_BOOTLOADER_ONLY_t, TAG_OS_VERSION_t, TAG_OS_PATCHLEVEL_t, TAG_UNIQUE_ID_t, + TAG_ATTESTATION_CHALLENGE_t, TAG_ATTESTATION_APPLICATION_ID_t, + TAG_RESET_SINCE_ID_ROTATION_t, TAG_PURPOSE_t, TAG_ALGORITHM_t, TAG_BLOCK_MODE_t, + TAG_DIGEST_t, TAG_PADDING_t, TAG_BLOB_USAGE_REQUIREMENTS_t, TAG_ORIGIN_t, + TAG_USER_AUTH_TYPE_t, TAG_EC_CURVE_t, TAG_BOOT_PATCHLEVEL_t, TAG_VENDOR_PATCHLEVEL_t, + TAG_TRUSTED_CONFIRMATION_REQUIRED_t, TAG_TRUSTED_USER_PRESENCE_REQUIRED_t>; template struct TypedTag2ValueType; @@ -220,6 +222,7 @@ MAKE_TAG_ENUM_VALUE_ACCESSOR(TAG_ORIGIN, f.origin) MAKE_TAG_ENUM_VALUE_ACCESSOR(TAG_PADDING, f.paddingMode) MAKE_TAG_ENUM_VALUE_ACCESSOR(TAG_PURPOSE, f.purpose) MAKE_TAG_ENUM_VALUE_ACCESSOR(TAG_USER_AUTH_TYPE, f.hardwareAuthenticatorType) +MAKE_TAG_ENUM_VALUE_ACCESSOR(TAG_HARDWARE_TYPE, f.hardwareType) template inline KeyParameter makeKeyParameter(TypedTag ttag, ValueT&& value) {