From a97ec69e4b545cdde3e5afea15bb2974d79cb0fe Mon Sep 17 00:00:00 2001 From: Max Bires Date: Mon, 21 Nov 2022 23:37:54 -0800 Subject: [PATCH] Fix overly relaxed Device ID Test. Previous versions of VTS had to allow a Device ID attestation failure to return INVALID_TAG even though this is inconsistent with the KeyMint spec. This was due to previous KM implementations returning this before the test was added to validate the precise error code being returned from Device ID attestation. For VSR-14 and newer devices, the test will now enforce that only CANNOT_ATTEST_IDS is returned from a failed device ID attestation call. Test: atest VtsAidlKeyMintTargetTest Change-Id: I6acff3fd32f3f251f946e3603283535f36d99a5d --- security/keymint/aidl/vts/functional/AttestKeyTest.cpp | 1 + .../vts/functional/DeviceUniqueAttestationTest.cpp | 2 +- .../aidl/vts/functional/KeyMintAidlTestBase.cpp | 10 ++++++++++ .../keymint/aidl/vts/functional/KeyMintAidlTestBase.h | 1 + 4 files changed, 13 insertions(+), 1 deletion(-) diff --git a/security/keymint/aidl/vts/functional/AttestKeyTest.cpp b/security/keymint/aidl/vts/functional/AttestKeyTest.cpp index f4c009574f..ea4ba1811c 100644 --- a/security/keymint/aidl/vts/functional/AttestKeyTest.cpp +++ b/security/keymint/aidl/vts/functional/AttestKeyTest.cpp @@ -892,6 +892,7 @@ TEST_P(AttestKeyTest, EcdsaAttestationMismatchID) { ASSERT_TRUE(result == ErrorCode::CANNOT_ATTEST_IDS || result == ErrorCode::INVALID_TAG) << "result = " << result; + device_id_attestation_vsr_check(result); } CheckedDeleteKey(&attest_key.keyBlob); } diff --git a/security/keymint/aidl/vts/functional/DeviceUniqueAttestationTest.cpp b/security/keymint/aidl/vts/functional/DeviceUniqueAttestationTest.cpp index cd140c8dac..26dc3f510f 100644 --- a/security/keymint/aidl/vts/functional/DeviceUniqueAttestationTest.cpp +++ b/security/keymint/aidl/vts/functional/DeviceUniqueAttestationTest.cpp @@ -348,8 +348,8 @@ TEST_P(DeviceUniqueAttestationTest, EcdsaDeviceUniqueAttestationMismatchID) { // Add the tag that doesn't match the local device's real ID. builder.push_back(invalid_tag); auto result = GenerateKey(builder, &key_blob, &key_characteristics); - ASSERT_TRUE(result == ErrorCode::CANNOT_ATTEST_IDS || result == ErrorCode::INVALID_TAG); + device_id_attestation_vsr_check(result); } } diff --git a/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.cpp b/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.cpp index 80abd920d6..43ad30aa5e 100644 --- a/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.cpp +++ b/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.cpp @@ -2031,6 +2031,16 @@ void p256_pub_key(const vector& coseKeyData, EVP_PKEY_Ptr* signingKey) *signingKey = std::move(pubKey); } +void device_id_attestation_vsr_check(const ErrorCode& result) { + if (get_vsr_api_level() >= 34) { + ASSERT_FALSE(result == ErrorCode::INVALID_TAG) + << "It is a specification violation for INVALID_TAG to be returned due to ID " + << "mismatch in a Device ID Attestation call. INVALID_TAG is only intended to " + << "be used for a case where updateAad() is called after update(). As of " + << "VSR-14, this is now enforced as an error."; + } +} + } // namespace test } // namespace aidl::android::hardware::security::keymint diff --git a/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.h b/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.h index 67e8b216de..5b09ca5c3e 100644 --- a/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.h +++ b/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.h @@ -395,6 +395,7 @@ vector make_name_from_str(const string& name); void check_maced_pubkey(const MacedPublicKey& macedPubKey, bool testMode, vector* payload_value); void p256_pub_key(const vector& coseKeyData, EVP_PKEY_Ptr* signingKey); +void device_id_attestation_vsr_check(const ErrorCode& result); AuthorizationSet HwEnforcedAuthorizations(const vector& key_characteristics); AuthorizationSet SwEnforcedAuthorizations(const vector& key_characteristics);