From 24f75791306dc1cbd8da4e06c3de8b106f206e4c Mon Sep 17 00:00:00 2001 From: Prashant Patil Date: Thu, 7 Sep 2023 15:25:14 +0000 Subject: [PATCH] Enable EcdsaAttestationIdTags VTS for GSI Earlier, attestation properties didn't match on GSI images, hence EcdsaAttestationIdTags VTS test case was skipped on GSI images. Recently attestation properties reading priority changed as ro.product.*_for_attestation -> ro.product.vendor.* -> ro.product.* that means on GSI images ro.product.vendor.* properties could be used and hence attestation should work. Incase ro.product.vendor.* properties are not same as provisioned values to KM. They should be set as ro.product.*_for_attestation on base build. Bug: 298586194 Test: atest VtsAidlKeyMintTargetTest:PerInstance/NewKeyGenerationTest#EcdsaAttestationIdTags/0_android_hardware_security_keymint_IKeyMintDevice_default Change-Id: Ie945bd8f7060e0e768daf9681d121ea5f170a6e1 --- .../aidl/vts/functional/AttestKeyTest.cpp | 24 ----------- .../aidl/vts/functional/KeyMintAidlTestBase.h | 23 +++++++++++ .../aidl/vts/functional/KeyMintTest.cpp | 41 +++---------------- 3 files changed, 28 insertions(+), 60 deletions(-) diff --git a/security/keymint/aidl/vts/functional/AttestKeyTest.cpp b/security/keymint/aidl/vts/functional/AttestKeyTest.cpp index 4c4671982b..cc97c13735 100644 --- a/security/keymint/aidl/vts/functional/AttestKeyTest.cpp +++ b/security/keymint/aidl/vts/functional/AttestKeyTest.cpp @@ -88,30 +88,6 @@ string get_imei(int slot) { return imei; } - -// Use `ro.product._for_attestation` property for attestation if it is present else -// fallback to use `ro.product.vendor.` if it is present else fallback to -// `ro.product.`. Similar logic can be seen in Java method `getVendorDeviceIdProperty` -// in frameworks/base/core/java/android/os/Build.java. -template -void add_attestation_id(AuthorizationSetBuilder* attestation_id_tags, - TypedTag tag_type, const char* prop) { - ::android::String8 prop_name = - ::android::String8::format("ro.product.%s_for_attestation", prop); - std::string prop_value = ::android::base::GetProperty(prop_name.c_str(), /* default= */ ""); - if (!prop_value.empty()) { - add_tag_from_prop(attestation_id_tags, tag_type, prop_name.c_str()); - } else { - prop_name = ::android::String8::format("ro.product.vendor.%s", prop); - prop_value = ::android::base::GetProperty(prop_name.c_str(), /* default= */ ""); - if (!prop_value.empty()) { - add_tag_from_prop(attestation_id_tags, tag_type, prop_name.c_str()); - } else { - prop_name = ::android::String8::format("ro.product.%s", prop); - add_tag_from_prop(attestation_id_tags, tag_type, prop_name.c_str()); - } - } -} } // namespace class AttestKeyTest : public KeyMintAidlTestBase { diff --git a/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.h b/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.h index 8934a57e7b..9778b3d0f9 100644 --- a/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.h +++ b/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.h @@ -454,6 +454,29 @@ ErrorCode GetReturnErrorCode(const Status& result); ::android::PrintInstanceNameToString); \ GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(name); +// Use `ro.product._for_attestation` property for attestation if it is present else +// fallback to use `ro.product.vendor.` if it is present else fallback to +// `ro.product.`. Similar logic can be seen in Java method `getVendorDeviceIdProperty` +// in frameworks/base/core/java/android/os/Build.java. +template +void add_attestation_id(AuthorizationSetBuilder* attestation_id_tags, + TypedTag tag_type, const char* prop) { + ::android::String8 prop_name = + ::android::String8::format("ro.product.%s_for_attestation", prop); + std::string prop_value = ::android::base::GetProperty(prop_name.c_str(), /* default= */ ""); + if (!prop_value.empty()) { + add_tag_from_prop(attestation_id_tags, tag_type, prop_name.c_str()); + } else { + prop_name = ::android::String8::format("ro.product.vendor.%s", prop); + prop_value = ::android::base::GetProperty(prop_name.c_str(), /* default= */ ""); + if (!prop_value.empty()) { + add_tag_from_prop(attestation_id_tags, tag_type, prop_name.c_str()); + } else { + prop_name = ::android::String8::format("ro.product.%s", prop); + add_tag_from_prop(attestation_id_tags, tag_type, prop_name.c_str()); + } + } +} } // namespace test } // namespace aidl::android::hardware::security::keymint diff --git a/security/keymint/aidl/vts/functional/KeyMintTest.cpp b/security/keymint/aidl/vts/functional/KeyMintTest.cpp index 022dd3fe7d..c0aa1a613b 100644 --- a/security/keymint/aidl/vts/functional/KeyMintTest.cpp +++ b/security/keymint/aidl/vts/functional/KeyMintTest.cpp @@ -2082,11 +2082,6 @@ TEST_P(NewKeyGenerationTest, EcdsaAttestationTags) { * attestation extension. */ TEST_P(NewKeyGenerationTest, EcdsaAttestationIdTags) { - if (is_gsi_image()) { - // GSI sets up a standard set of device identifiers that may not match - // the device identifiers held by the device. - GTEST_SKIP() << "Test not applicable under GSI"; - } auto challenge = "hello"; auto app_id = "foo"; auto subject = "cert subj 2"; @@ -2106,38 +2101,12 @@ TEST_P(NewKeyGenerationTest, EcdsaAttestationIdTags) { // Various ATTESTATION_ID_* tags that map to fields in the attestation extension ASN.1 schema. auto extra_tags = AuthorizationSetBuilder(); - // Use ro.product.brand_for_attestation property for attestation if it is present else fallback - // to ro.product.brand - std::string prop_value = - ::android::base::GetProperty("ro.product.brand_for_attestation", /* default= */ ""); - if (!prop_value.empty()) { - add_tag_from_prop(&extra_tags, TAG_ATTESTATION_ID_BRAND, - "ro.product.brand_for_attestation"); - } else { - add_tag_from_prop(&extra_tags, TAG_ATTESTATION_ID_BRAND, "ro.product.brand"); - } - add_tag_from_prop(&extra_tags, TAG_ATTESTATION_ID_DEVICE, "ro.product.device"); - // Use ro.product.name_for_attestation property for attestation if it is present else fallback - // to ro.product.name - prop_value = ::android::base::GetProperty("ro.product.name_for_attestation", /* default= */ ""); - if (!prop_value.empty()) { - add_tag_from_prop(&extra_tags, TAG_ATTESTATION_ID_PRODUCT, - "ro.product.name_for_attestation"); - } else { - add_tag_from_prop(&extra_tags, TAG_ATTESTATION_ID_PRODUCT, "ro.product.name"); - } + add_attestation_id(&extra_tags, TAG_ATTESTATION_ID_BRAND, "brand"); + add_attestation_id(&extra_tags, TAG_ATTESTATION_ID_DEVICE, "device"); + add_attestation_id(&extra_tags, TAG_ATTESTATION_ID_PRODUCT, "name"); + add_attestation_id(&extra_tags, TAG_ATTESTATION_ID_MANUFACTURER, "manufacturer"); + add_attestation_id(&extra_tags, TAG_ATTESTATION_ID_MODEL, "model"); add_tag_from_prop(&extra_tags, TAG_ATTESTATION_ID_SERIAL, "ro.serialno"); - add_tag_from_prop(&extra_tags, TAG_ATTESTATION_ID_MANUFACTURER, "ro.product.manufacturer"); - // Use ro.product.model_for_attestation property for attestation if it is present else fallback - // to ro.product.model - prop_value = - ::android::base::GetProperty("ro.product.model_for_attestation", /* default= */ ""); - if (!prop_value.empty()) { - add_tag_from_prop(&extra_tags, TAG_ATTESTATION_ID_MODEL, - "ro.product.model_for_attestation"); - } else { - add_tag_from_prop(&extra_tags, TAG_ATTESTATION_ID_MODEL, "ro.product.model"); - } for (const KeyParameter& tag : extra_tags) { SCOPED_TRACE(testing::Message() << "tag-" << tag);