From ba0d5d01bde427b7d7a22cec84cd9304c00b4e14 Mon Sep 17 00:00:00 2001 From: Shawn Willden Date: Mon, 25 Apr 2016 08:52:58 -0600 Subject: Fix SoftKeymaster handling of EC curve specification. Keymaster2 should accept EC curve specification either by key size (as done in KM1) or with the new KM_TAG_EC_CURVE, filling in the other value if not specified, and validating that they match if both are provided. SoftKeymaster doesn't correctly implement this KM2 requirement. Bug: 28365747 Change-Id: I27d98b71730b69bb2f0c2543af6c027b1a5670f1 --- android_keymaster_test.cpp | 20 ++++---- android_keymaster_utils.cpp | 50 ++++++++++++++++++++ authorization_set.cpp | 7 +++ ec_key_factory.cpp | 71 ++++++++++++++++++++++++++--- ec_keymaster0_key.cpp | 8 ++-- include/keymaster/android_keymaster_utils.h | 3 ++ include/keymaster/authorization_set.h | 9 ++++ include/keymaster/ec_key_factory.h | 7 ++- 8 files changed, 154 insertions(+), 21 deletions(-) diff --git a/android_keymaster_test.cpp b/android_keymaster_test.cpp index 1ea0c34..325a4c8 100644 --- a/android_keymaster_test.cpp +++ b/android_keymaster_test.cpp @@ -334,17 +334,17 @@ TEST_P(NewKeyGeneration, EcdsaDefaultSize) { } TEST_P(NewKeyGeneration, EcdsaInvalidSize) { - if (GetParam()->algorithm_in_km0_hardware(KM_ALGORITHM_EC)) - ASSERT_EQ( - KM_ERROR_UNKNOWN_ERROR, - GenerateKey(AuthorizationSetBuilder().EcdsaSigningKey(190).Digest(KM_DIGEST_NONE))); - else - ASSERT_EQ( - KM_ERROR_UNSUPPORTED_KEY_SIZE, - GenerateKey(AuthorizationSetBuilder().EcdsaSigningKey(190).Digest(KM_DIGEST_NONE))); + ASSERT_EQ(KM_ERROR_UNSUPPORTED_KEY_SIZE, + GenerateKey(AuthorizationSetBuilder().EcdsaSigningKey(190).Digest(KM_DIGEST_NONE))); + EXPECT_EQ(0, GetParam()->keymaster0_calls()); +} - if (GetParam()->algorithm_in_km0_hardware(KM_ALGORITHM_EC)) - EXPECT_EQ(1, GetParam()->keymaster0_calls()); +TEST_P(NewKeyGeneration, EcdsaMismatchKeySize) { + ASSERT_EQ(KM_ERROR_INVALID_ARGUMENT, + GenerateKey(AuthorizationSetBuilder() + .EcdsaSigningKey(224) + .Authorization(TAG_EC_CURVE, KM_EC_CURVE_P_256) + .Digest(KM_DIGEST_NONE))); } TEST_P(NewKeyGeneration, EcdsaAllValidSizes) { diff --git a/android_keymaster_utils.cpp b/android_keymaster_utils.cpp index 053e72a..5e92745 100644 --- a/android_keymaster_utils.cpp +++ b/android_keymaster_utils.cpp @@ -42,4 +42,54 @@ int memcmp_s(const void* p1, const void* p2, size_t length) { return result == 0 ? 0 : 1; } +keymaster_error_t EcKeySizeToCurve(uint32_t key_size_bits, keymaster_ec_curve_t* curve) { + switch (key_size_bits) { + default: + return KM_ERROR_UNSUPPORTED_KEY_SIZE; + + case 224: + *curve = KM_EC_CURVE_P_224; + break; + + case 256: + *curve = KM_EC_CURVE_P_256; + break; + + case 384: + *curve = KM_EC_CURVE_P_384; + break; + + case 521: + *curve = KM_EC_CURVE_P_521; + break; + } + + return KM_ERROR_OK; +} + +keymaster_error_t EcCurveToKeySize(keymaster_ec_curve_t curve, uint32_t* key_size_bits) { + switch (curve) { + default: + return KM_ERROR_UNSUPPORTED_EC_CURVE; + + case KM_EC_CURVE_P_224: + *key_size_bits = 224; + break; + + case KM_EC_CURVE_P_256: + *key_size_bits = 256; + break; + + case KM_EC_CURVE_P_384: + *key_size_bits = 384; + break; + + case KM_EC_CURVE_P_521: + *key_size_bits = 521; + break; + } + + return KM_ERROR_OK; +} + } // namespace keymaster diff --git a/authorization_set.cpp b/authorization_set.cpp index b241b78..9f6810d 100644 --- a/authorization_set.cpp +++ b/authorization_set.cpp @@ -630,4 +630,11 @@ bool AuthorizationSet::ContainsEnumValue(keymaster_tag_t tag, uint32_t value) co return false; } +bool AuthorizationSet::ContainsIntValue(keymaster_tag_t tag, uint32_t value) const { + for (auto& entry : *this) + if (entry.tag == tag && entry.integer == value) + return true; + return false; +} + } // namespace keymaster diff --git a/ec_key_factory.cpp b/ec_key_factory.cpp index 14c8327..36ec433 100644 --- a/ec_key_factory.cpp +++ b/ec_key_factory.cpp @@ -40,6 +40,37 @@ OperationFactory* EcKeyFactory::GetOperationFactory(keymaster_purpose_t purpose) } } +/* static */ +keymaster_error_t EcKeyFactory::GetCurveAndSize(const AuthorizationSet& key_description, + keymaster_ec_curve_t* curve, + uint32_t* key_size_bits) { + if (!key_description.GetTagValue(TAG_EC_CURVE, curve)) { + // Curve not specified. Fall back to deducing curve from key size. + if (!key_description.GetTagValue(TAG_KEY_SIZE, key_size_bits)) { + LOG_E("%s", "No curve or key size specified for EC key generation"); + return KM_ERROR_UNSUPPORTED_KEY_SIZE; + } + keymaster_error_t error = EcKeySizeToCurve(*key_size_bits, curve); + if (error != KM_ERROR_OK) { + return KM_ERROR_UNSUPPORTED_KEY_SIZE; + } + } else { + keymaster_error_t error = EcCurveToKeySize(*curve, key_size_bits); + if (error != KM_ERROR_OK) { + return error; + } + uint32_t tag_key_size_bits; + if (key_description.GetTagValue(TAG_KEY_SIZE, &tag_key_size_bits) && + *key_size_bits != tag_key_size_bits) { + LOG_E("Curve key size %d and specified key size %d don't match", key_size_bits, + tag_key_size_bits); + return KM_ERROR_INVALID_ARGUMENT; + } + } + + return KM_ERROR_OK; +} + keymaster_error_t EcKeyFactory::GenerateKey(const AuthorizationSet& key_description, KeymasterKeyBlob* key_blob, AuthorizationSet* hw_enforced, @@ -49,10 +80,15 @@ keymaster_error_t EcKeyFactory::GenerateKey(const AuthorizationSet& key_descript AuthorizationSet authorizations(key_description); + keymaster_ec_curve_t ec_curve; uint32_t key_size; - if (!authorizations.GetTagValue(TAG_KEY_SIZE, &key_size)) { - LOG_E("%s", "No key size specified for EC key generation"); - return KM_ERROR_UNSUPPORTED_KEY_SIZE; + keymaster_error_t error = GetCurveAndSize(authorizations, &ec_curve, &key_size); + if (error != KM_ERROR_OK) { + return error; + } else if (!authorizations.Contains(TAG_KEY_SIZE, key_size)) { + authorizations.push_back(TAG_KEY_SIZE, key_size); + } else if (!authorizations.Contains(TAG_EC_CURVE, ec_curve)) { + authorizations.push_back(TAG_EC_CURVE, ec_curve); } UniquePtr ec_key(EC_KEY_new()); @@ -60,9 +96,9 @@ keymaster_error_t EcKeyFactory::GenerateKey(const AuthorizationSet& key_descript if (ec_key.get() == NULL || pkey.get() == NULL) return KM_ERROR_MEMORY_ALLOCATION_FAILED; - UniquePtr group(choose_group(key_size)); + UniquePtr group(ChooseGroup(ec_curve)); if (group.get() == NULL) { - LOG_E("Unable to get EC group for key of size %d", key_size); + LOG_E("Unable to get EC group for curve %d", ec_curve); return KM_ERROR_UNSUPPORTED_KEY_SIZE; } @@ -80,7 +116,7 @@ keymaster_error_t EcKeyFactory::GenerateKey(const AuthorizationSet& key_descript return TranslateLastOpenSslError(); KeymasterKeyBlob key_material; - keymaster_error_t error = EvpKeyToKeyMaterial(pkey.get(), &key_material); + error = EvpKeyToKeyMaterial(pkey.get(), &key_material); if (error != KM_ERROR_OK) return error; @@ -149,7 +185,7 @@ keymaster_error_t EcKeyFactory::UpdateImportKeyDescription(const AuthorizationSe } /* static */ -EC_GROUP* EcKeyFactory::choose_group(size_t key_size_bits) { +EC_GROUP* EcKeyFactory::ChooseGroup(size_t key_size_bits) { switch (key_size_bits) { case 224: return EC_GROUP_new_by_curve_name(NID_secp224r1); @@ -169,6 +205,27 @@ EC_GROUP* EcKeyFactory::choose_group(size_t key_size_bits) { } } +/* static */ +EC_GROUP* EcKeyFactory::ChooseGroup(keymaster_ec_curve_t ec_curve) { + switch (ec_curve) { + case KM_EC_CURVE_P_224: + return EC_GROUP_new_by_curve_name(NID_secp224r1); + break; + case KM_EC_CURVE_P_256: + return EC_GROUP_new_by_curve_name(NID_X9_62_prime256v1); + break; + case KM_EC_CURVE_P_384: + return EC_GROUP_new_by_curve_name(NID_secp384r1); + break; + case KM_EC_CURVE_P_521: + return EC_GROUP_new_by_curve_name(NID_secp521r1); + break; + default: + return nullptr; + break; + } +} + keymaster_error_t EcKeyFactory::CreateEmptyKey(const AuthorizationSet& hw_enforced, const AuthorizationSet& sw_enforced, UniquePtr* key) const { diff --git a/ec_keymaster0_key.cpp b/ec_keymaster0_key.cpp index 705ee73..e11c887 100644 --- a/ec_keymaster0_key.cpp +++ b/ec_keymaster0_key.cpp @@ -43,10 +43,11 @@ keymaster_error_t EcdsaKeymaster0KeyFactory::GenerateKey(const AuthorizationSet& if (!engine_ || !engine_->supports_ec()) return super::GenerateKey(key_description, key_blob, hw_enforced, sw_enforced); + keymaster_ec_curve_t ec_curve; uint32_t key_size; - if (!key_description.GetTagValue(TAG_KEY_SIZE, &key_size)) { - LOG_E("%s", "No key size specified for EC key generation"); - return KM_ERROR_UNSUPPORTED_KEY_SIZE; + keymaster_error_t error = GetCurveAndSize(key_description, &ec_curve, &key_size); + if (error != KM_ERROR_OK) { + return error; } KeymasterKeyBlob key_material; @@ -57,6 +58,7 @@ keymaster_error_t EcdsaKeymaster0KeyFactory::GenerateKey(const AuthorizationSet& // context_->CreateKeyBlob doesn't put them in sw_enforced. hw_enforced->push_back(TAG_ALGORITHM, KM_ALGORITHM_EC); hw_enforced->push_back(TAG_KEY_SIZE, key_size); + hw_enforced->push_back(TAG_EC_CURVE, ec_curve); hw_enforced->push_back(TAG_ORIGIN, KM_ORIGIN_UNKNOWN); return context_->CreateKeyBlob(key_description, KM_ORIGIN_UNKNOWN, key_material, key_blob, diff --git a/include/keymaster/android_keymaster_utils.h b/include/keymaster/android_keymaster_utils.h index c190e04..17688a6 100644 --- a/include/keymaster/android_keymaster_utils.h +++ b/include/keymaster/android_keymaster_utils.h @@ -325,6 +325,9 @@ struct CertificateChainDelete { } }; +keymaster_error_t EcKeySizeToCurve(uint32_t key_size_bits, keymaster_ec_curve_t* curve); +keymaster_error_t EcCurveToKeySize(keymaster_ec_curve_t curve, uint32_t* key_size_bits); + } // namespace keymaster #endif // SYSTEM_KEYMASTER_ANDROID_KEYMASTER_UTILS_H_ diff --git a/include/keymaster/authorization_set.h b/include/keymaster/authorization_set.h index 5be5d83..ec68de0 100644 --- a/include/keymaster/authorization_set.h +++ b/include/keymaster/authorization_set.h @@ -230,6 +230,14 @@ class AuthorizationSet : public Serializable, public keymaster_key_param_set_t { return ContainsEnumValue(tag, val); } + /** + * Returns true if the set contains the specified tag and value. + */ + template + bool Contains(TypedTag tag, uint32_t val) const { + return ContainsIntValue(tag, val); + } + /** * If the specified integer-typed \p tag exists, places its value in \p val and returns true. * If \p tag is not present, leaves \p val unmodified and returns false. @@ -442,6 +450,7 @@ class AuthorizationSet : public Serializable, public keymaster_key_param_set_t { bool GetTagValueBool(keymaster_tag_t tag) const; bool ContainsEnumValue(keymaster_tag_t tag, uint32_t val) const; + bool ContainsIntValue(keymaster_tag_t tag, uint32_t val) const; // Define elems_ and elems_size_ as aliases to params and length, respectively. This is to // avoid using the variables without the trailing underscore in the implementation. diff --git a/include/keymaster/ec_key_factory.h b/include/keymaster/ec_key_factory.h index 2715e79..91c491c 100644 --- a/include/keymaster/ec_key_factory.h +++ b/include/keymaster/ec_key_factory.h @@ -52,7 +52,12 @@ class EcKeyFactory : public AsymmetricKeyFactory { OperationFactory* GetOperationFactory(keymaster_purpose_t purpose) const override; - static EC_GROUP* choose_group(size_t key_size_bits); + protected: + static EC_GROUP* ChooseGroup(size_t key_size_bits); + static EC_GROUP* ChooseGroup(keymaster_ec_curve_t ec_curve); + + static keymaster_error_t GetCurveAndSize(const AuthorizationSet& key_description, + keymaster_ec_curve_t* curve, uint32_t* key_size_bits); }; } // namespace keymaster -- cgit v1.2.3 From 0fcef5c3927c524fa2e737ea78a5b2b284534811 Mon Sep 17 00:00:00 2001 From: Shawn Willden Date: Mon, 25 Apr 2016 07:48:46 -0600 Subject: Add EC curve tag to keymaster1 SoftKeymaster1 attestations. Key attestations for EC keys must include the EC curve tag, which didn't exist in keymaster1. When SoftKeymaster produces attestations for keymaster1 keys, it must deduce the curve (based on key size; the mapping is unambiguous) and add the curve tag to the attestation. Bug: 28366732 Change-Id: I8705aac6cf39b82754ee2c9f17d60484d3263ece --- attestation_record.cpp | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/attestation_record.cpp b/attestation_record.cpp index 4edac3e..befb8e2 100644 --- a/attestation_record.cpp +++ b/attestation_record.cpp @@ -378,6 +378,27 @@ static keymaster_error_t build_auth_list(const AuthorizationSet& auth_list, KM_A } } + keymaster_ec_curve_t ec_curve; + uint32_t key_size; + if (auth_list.Contains(TAG_ALGORITHM, KM_ALGORITHM_EC) && // + !auth_list.Contains(TAG_EC_CURVE) && // + auth_list.GetTagValue(TAG_KEY_SIZE, &key_size)) { + // This must be a keymaster1 key. It's an EC key with no curve. Insert the curve. + + keymaster_error_t error = EcKeySizeToCurve(key_size, &ec_curve); + if (error != KM_ERROR_OK) + return error; + + UniquePtr value(ASN1_INTEGER_new()); + if (!value.get()) + return KM_ERROR_MEMORY_ALLOCATION_FAILED; + + if (!ASN1_INTEGER_set(value.get(), ec_curve)) + return TranslateLastOpenSslError(); + + insert_integer(value.release(), &record->ec_curve, nullptr); + } + return KM_ERROR_OK; } @@ -455,7 +476,7 @@ keymaster_error_t build_attestation_record(const AuthorizationSet& attestation_p return KM_ERROR_INVALID_KEY_BLOB; } - keymaster_blob_t application_id = {}; + keymaster_blob_t application_id = {nullptr, 0}; sw_enforced.GetTagValue(TAG_APPLICATION_ID, &application_id); Buffer unique_id; -- cgit v1.2.3