From f42238c99ffe0df2e51cec84a96ed859a878b2b0 Mon Sep 17 00:00:00 2001 From: David Drysdale Date: Thu, 15 Jun 2023 09:41:05 +0100 Subject: [PATCH] Allow extra error code in device ID attestation Generalize the existing helper function to allow more variants. Remove a couple of pointless invocations of the existing helper. Bug: 286733800 Test: VtsAidlKeyMintTargetTest Change-Id: Ic01c53cbe79f55c2d403a66acbfd04029395c287 --- .../aidl/vts/functional/AttestKeyTest.cpp | 9 +------ .../DeviceUniqueAttestationTest.cpp | 4 ++-- .../vts/functional/KeyMintAidlTestBase.cpp | 24 ++++++++++++++++--- .../aidl/vts/functional/KeyMintAidlTestBase.h | 2 +- 4 files changed, 25 insertions(+), 14 deletions(-) diff --git a/security/keymint/aidl/vts/functional/AttestKeyTest.cpp b/security/keymint/aidl/vts/functional/AttestKeyTest.cpp index c25c9ac710..0fc359a6da 100644 --- a/security/keymint/aidl/vts/functional/AttestKeyTest.cpp +++ b/security/keymint/aidl/vts/functional/AttestKeyTest.cpp @@ -950,10 +950,7 @@ TEST_P(AttestKeyTest, EcdsaAttestationMismatchID) { vector attested_key_cert_chain; auto result = GenerateKey(builder, attest_key, &attested_key_blob, &attested_key_characteristics, &attested_key_cert_chain); - - ASSERT_TRUE(result == ErrorCode::CANNOT_ATTEST_IDS || result == ErrorCode::INVALID_TAG) - << "result = " << result; - device_id_attestation_vsr_check(result); + device_id_attestation_check_acceptable_error(invalid_tag.tag, result); } } @@ -1016,8 +1013,6 @@ TEST_P(AttestKeyTest, SecondIMEIAttestationIDSuccess) { ASSERT_EQ(result, ErrorCode::OK); KeyBlobDeleter attested_deleter(keymint_, attested_key_blob); - device_id_attestation_vsr_check(result); - AuthorizationSet hw_enforced = HwEnforcedAuthorizations(attested_key_characteristics); AuthorizationSet sw_enforced = SwEnforcedAuthorizations(attested_key_characteristics); @@ -1095,8 +1090,6 @@ TEST_P(AttestKeyTest, MultipleIMEIAttestationIDSuccess) { ASSERT_EQ(result, ErrorCode::OK); KeyBlobDeleter attested_deleter(keymint_, attested_key_blob); - device_id_attestation_vsr_check(result); - AuthorizationSet hw_enforced = HwEnforcedAuthorizations(attested_key_characteristics); AuthorizationSet sw_enforced = SwEnforcedAuthorizations(attested_key_characteristics); diff --git a/security/keymint/aidl/vts/functional/DeviceUniqueAttestationTest.cpp b/security/keymint/aidl/vts/functional/DeviceUniqueAttestationTest.cpp index 55bb5b4fab..8e9adedf5c 100644 --- a/security/keymint/aidl/vts/functional/DeviceUniqueAttestationTest.cpp +++ b/security/keymint/aidl/vts/functional/DeviceUniqueAttestationTest.cpp @@ -374,8 +374,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); + + device_id_attestation_check_acceptable_error(invalid_tag.tag, result); } } diff --git a/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.cpp b/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.cpp index b2fd08eb60..b79700ffcd 100644 --- a/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.cpp +++ b/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.cpp @@ -2162,14 +2162,32 @@ 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() > __ANDROID_API_T__) { - ASSERT_FALSE(result == ErrorCode::INVALID_TAG) +// Check the error code from an attempt to perform device ID attestation with an invalid value. +void device_id_attestation_check_acceptable_error(Tag tag, const ErrorCode& result) { + // Standard/default error code for ID mismatch. + if (result == ErrorCode::CANNOT_ATTEST_IDS) { + return; + } + + // Depending on the situation, other error codes may be acceptable. First, allow older + // implementations to use INVALID_TAG. + if (result == ErrorCode::INVALID_TAG) { + ASSERT_FALSE(get_vsr_api_level() > __ANDROID_API_T__) << "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."; } + + // If the device is not a phone, it will not have IMEI/MEID values available. Allow + // ATTESTATION_IDS_NOT_PROVISIONED in this case. + if (result == ErrorCode::ATTESTATION_IDS_NOT_PROVISIONED) { + ASSERT_TRUE((tag == TAG_ATTESTATION_ID_IMEI || tag == TAG_ATTESTATION_ID_MEID || + tag == TAG_ATTESTATION_ID_SECOND_IMEI)) + << "incorrect error code on attestation ID mismatch"; + } + ADD_FAILURE() << "Error code " << result + << " returned on attestation ID mismatch, should be CANNOT_ATTEST_IDS"; } // Check whether the given named feature is available. diff --git a/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.h b/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.h index aa3069a0a6..0d0790f797 100644 --- a/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.h +++ b/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.h @@ -432,7 +432,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); +void device_id_attestation_check_acceptable_error(Tag tag, const ErrorCode& result); bool check_feature(const std::string& name); AuthorizationSet HwEnforcedAuthorizations(const vector& key_characteristics);