From 45b478f32e91fe29f4396524de5bde91bdc05a11 Mon Sep 17 00:00:00 2001 From: Rajesh Nyamagoud Date: Wed, 26 Jul 2023 01:20:02 +0000 Subject: [PATCH] Whenever `generateKey` fails updated AttestKeyTests to abort instead of continuing the execution of the test. If generateKey fails and execution continues then it leads to issues while verifying the attest records and causing the crash. Test: atest VtsAidlKeyMintTargetTest Bug: 292300030 Change-Id: I66bd650423e9e5bbbfe8411a1455c4ea5846f1ff --- .../aidl/vts/functional/AttestKeyTest.cpp | 55 +++++++++++-------- 1 file changed, 33 insertions(+), 22 deletions(-) diff --git a/security/keymint/aidl/vts/functional/AttestKeyTest.cpp b/security/keymint/aidl/vts/functional/AttestKeyTest.cpp index 0499079032..732c896af3 100644 --- a/security/keymint/aidl/vts/functional/AttestKeyTest.cpp +++ b/security/keymint/aidl/vts/functional/AttestKeyTest.cpp @@ -133,7 +133,7 @@ TEST_P(AttestKeyTest, AllRsaSizes) { vector attested_key_blob; vector attested_key_characteristics; vector attested_key_cert_chain; - EXPECT_EQ(ErrorCode::OK, + ASSERT_EQ(ErrorCode::OK, GenerateKey(AuthorizationSetBuilder() .RsaSigningKey(2048, 65537) .Authorization(TAG_NO_AUTH_REQUIRED) @@ -144,9 +144,11 @@ TEST_P(AttestKeyTest, AllRsaSizes) { &attested_key_cert_chain)); KeyBlobDeleter attested_deleter(keymint_, attested_key_blob); + ASSERT_GT(attested_key_cert_chain.size(), 0); + AuthorizationSet hw_enforced = HwEnforcedAuthorizations(attested_key_characteristics); AuthorizationSet sw_enforced = SwEnforcedAuthorizations(attested_key_characteristics); - EXPECT_TRUE(verify_attestation_record(AidlVersion(), "foo", "bar", sw_enforced, hw_enforced, + ASSERT_TRUE(verify_attestation_record(AidlVersion(), "foo", "bar", sw_enforced, hw_enforced, SecLevel(), attested_key_cert_chain[0].encodedCertificate)); @@ -163,7 +165,7 @@ TEST_P(AttestKeyTest, AllRsaSizes) { */ attested_key_characteristics.resize(0); attested_key_cert_chain.resize(0); - EXPECT_EQ(ErrorCode::OK, + ASSERT_EQ(ErrorCode::OK, GenerateKey(AuthorizationSetBuilder() .RsaEncryptionKey(2048, 65537) .Digest(Digest::NONE) @@ -176,9 +178,11 @@ TEST_P(AttestKeyTest, AllRsaSizes) { &attested_key_cert_chain)); KeyBlobDeleter attested_deleter2(keymint_, attested_key_blob); + ASSERT_GT(attested_key_cert_chain.size(), 0); + hw_enforced = HwEnforcedAuthorizations(attested_key_characteristics); sw_enforced = SwEnforcedAuthorizations(attested_key_characteristics); - EXPECT_TRUE(verify_attestation_record(AidlVersion(), "foo2", "bar2", sw_enforced, + ASSERT_TRUE(verify_attestation_record(AidlVersion(), "foo2", "bar2", sw_enforced, hw_enforced, SecLevel(), attested_key_cert_chain[0].encodedCertificate)); @@ -196,7 +200,7 @@ TEST_P(AttestKeyTest, AllRsaSizes) { attested_key_characteristics.resize(0); attested_key_cert_chain.resize(0); uint64_t timestamp = 1619621648000; - EXPECT_EQ(ErrorCode::OK, + ASSERT_EQ(ErrorCode::OK, GenerateKey(AuthorizationSetBuilder() .EcdsaSigningKey(EcCurve::P_256) .Authorization(TAG_NO_AUTH_REQUIRED) @@ -208,6 +212,8 @@ TEST_P(AttestKeyTest, AllRsaSizes) { &attested_key_cert_chain)); KeyBlobDeleter attested_deleter3(keymint_, attested_key_blob); + ASSERT_GT(attested_key_cert_chain.size(), 0); + // The returned key characteristics will include CREATION_DATETIME (checked below) // in SecurityLevel::KEYSTORE; this will be stripped out in the CheckCharacteristics() // call below, to match what getKeyCharacteristics() returns (which doesn't include @@ -223,7 +229,7 @@ TEST_P(AttestKeyTest, AllRsaSizes) { EXPECT_TRUE(sw_enforced.Contains(TAG_CREATION_DATETIME, timestamp)) << "expected CREATION_TIMESTAMP in sw_enforced:" << sw_enforced << " not in hw_enforced:" << hw_enforced; - EXPECT_TRUE(verify_attestation_record(AidlVersion(), "foo", "bar", sw_enforced, hw_enforced, + ASSERT_TRUE(verify_attestation_record(AidlVersion(), "foo", "bar", sw_enforced, hw_enforced, SecLevel(), attested_key_cert_chain[0].encodedCertificate)); @@ -313,7 +319,7 @@ TEST_P(AttestKeyTest, RsaAttestedAttestKeys) { AuthorizationSet hw_enforced = HwEnforcedAuthorizations(attest_key_characteristics); AuthorizationSet sw_enforced = SwEnforcedAuthorizations(attest_key_characteristics); - EXPECT_TRUE(verify_attestation_record(AidlVersion(), challenge, app_id, // + ASSERT_TRUE(verify_attestation_record(AidlVersion(), challenge, app_id, // sw_enforced, hw_enforced, SecLevel(), attest_key_cert_chain[0].encodedCertificate)); @@ -331,7 +337,7 @@ TEST_P(AttestKeyTest, RsaAttestedAttestKeys) { uint64_t serial_int2 = 255; vector serial_blob2(build_serial_blob(serial_int2)); - EXPECT_EQ(ErrorCode::OK, + ASSERT_EQ(ErrorCode::OK, GenerateKey(AuthorizationSetBuilder() .RsaSigningKey(2048, 65537) .Authorization(TAG_NO_AUTH_REQUIRED) @@ -344,9 +350,11 @@ TEST_P(AttestKeyTest, RsaAttestedAttestKeys) { &attested_key_cert_chain)); KeyBlobDeleter attested_deleter(keymint_, attested_key_blob); + ASSERT_GT(attested_key_cert_chain.size(), 0); + AuthorizationSet hw_enforced2 = HwEnforcedAuthorizations(attested_key_characteristics); AuthorizationSet sw_enforced2 = SwEnforcedAuthorizations(attested_key_characteristics); - EXPECT_TRUE(verify_attestation_record(AidlVersion(), "foo", "bar", sw_enforced2, hw_enforced2, + ASSERT_TRUE(verify_attestation_record(AidlVersion(), "foo", "bar", sw_enforced2, hw_enforced2, SecLevel(), attested_key_cert_chain[0].encodedCertificate)); @@ -414,7 +422,7 @@ TEST_P(AttestKeyTest, RsaAttestKeyChaining) { AuthorizationSet hw_enforced = HwEnforcedAuthorizations(attested_key_characteristics); AuthorizationSet sw_enforced = SwEnforcedAuthorizations(attested_key_characteristics); ASSERT_GT(cert_chain_list[i].size(), 0); - EXPECT_TRUE(verify_attestation_record(AidlVersion(), "foo", "bar", sw_enforced, hw_enforced, + ASSERT_TRUE(verify_attestation_record(AidlVersion(), "foo", "bar", sw_enforced, hw_enforced, SecLevel(), cert_chain_list[i][0].encodedCertificate)); @@ -489,7 +497,7 @@ TEST_P(AttestKeyTest, EcAttestKeyChaining) { AuthorizationSet hw_enforced = HwEnforcedAuthorizations(attested_key_characteristics); AuthorizationSet sw_enforced = SwEnforcedAuthorizations(attested_key_characteristics); ASSERT_GT(cert_chain_list[i].size(), 0); - EXPECT_TRUE(verify_attestation_record(AidlVersion(), "foo", "bar", sw_enforced, hw_enforced, + ASSERT_TRUE(verify_attestation_record(AidlVersion(), "foo", "bar", sw_enforced, hw_enforced, SecLevel(), cert_chain_list[i][0].encodedCertificate)); @@ -605,7 +613,7 @@ TEST_P(AttestKeyTest, AlternateAttestKeyChaining) { AuthorizationSet hw_enforced = HwEnforcedAuthorizations(attested_key_characteristics); AuthorizationSet sw_enforced = SwEnforcedAuthorizations(attested_key_characteristics); ASSERT_GT(cert_chain_list[i].size(), 0); - EXPECT_TRUE(verify_attestation_record(AidlVersion(), "foo", "bar", sw_enforced, hw_enforced, + ASSERT_TRUE(verify_attestation_record(AidlVersion(), "foo", "bar", sw_enforced, hw_enforced, SecLevel(), cert_chain_list[i][0].encodedCertificate)); @@ -655,7 +663,7 @@ TEST_P(AttestKeyTest, MissingChallenge) { vector attested_key_blob; vector attested_key_characteristics; vector attested_key_cert_chain; - EXPECT_EQ(ErrorCode::ATTESTATION_CHALLENGE_MISSING, + ASSERT_EQ(ErrorCode::ATTESTATION_CHALLENGE_MISSING, GenerateKey(AuthorizationSetBuilder() .RsaSigningKey(2048, 65537) .Authorization(TAG_NO_AUTH_REQUIRED) @@ -664,7 +672,7 @@ TEST_P(AttestKeyTest, MissingChallenge) { attest_key, &attested_key_blob, &attested_key_characteristics, &attested_key_cert_chain)); - EXPECT_EQ(ErrorCode::ATTESTATION_CHALLENGE_MISSING, + ASSERT_EQ(ErrorCode::ATTESTATION_CHALLENGE_MISSING, GenerateKey(AuthorizationSetBuilder() .EcdsaSigningKey(EcCurve::P_256) .Authorization(TAG_NO_AUTH_REQUIRED) @@ -702,7 +710,7 @@ TEST_P(AttestKeyTest, AllEcCurves) { vector attested_key_blob; vector attested_key_characteristics; vector attested_key_cert_chain; - EXPECT_EQ(ErrorCode::OK, + ASSERT_EQ(ErrorCode::OK, GenerateKey(AuthorizationSetBuilder() .RsaSigningKey(2048, 65537) .Authorization(TAG_NO_AUTH_REQUIRED) @@ -717,7 +725,7 @@ TEST_P(AttestKeyTest, AllEcCurves) { AuthorizationSet hw_enforced = HwEnforcedAuthorizations(attested_key_characteristics); AuthorizationSet sw_enforced = SwEnforcedAuthorizations(attested_key_characteristics); - EXPECT_TRUE(verify_attestation_record(AidlVersion(), "foo", "bar", sw_enforced, hw_enforced, + ASSERT_TRUE(verify_attestation_record(AidlVersion(), "foo", "bar", sw_enforced, hw_enforced, SecLevel(), attested_key_cert_chain[0].encodedCertificate)); @@ -733,7 +741,7 @@ TEST_P(AttestKeyTest, AllEcCurves) { /* * Use attestation key to sign EC key */ - EXPECT_EQ(ErrorCode::OK, + ASSERT_EQ(ErrorCode::OK, GenerateKey(AuthorizationSetBuilder() .EcdsaSigningKey(EcCurve::P_256) .Authorization(TAG_NO_AUTH_REQUIRED) @@ -748,7 +756,7 @@ TEST_P(AttestKeyTest, AllEcCurves) { hw_enforced = HwEnforcedAuthorizations(attested_key_characteristics); sw_enforced = SwEnforcedAuthorizations(attested_key_characteristics); - EXPECT_TRUE(verify_attestation_record(AidlVersion(), "foo", "bar", sw_enforced, hw_enforced, + ASSERT_TRUE(verify_attestation_record(AidlVersion(), "foo", "bar", sw_enforced, hw_enforced, SecLevel(), attested_key_cert_chain[0].encodedCertificate)); @@ -786,7 +794,7 @@ TEST_P(AttestKeyTest, AttestWithNonAttestKey) { vector attested_key_blob; vector attested_key_characteristics; vector attested_key_cert_chain; - EXPECT_EQ(ErrorCode::INCOMPATIBLE_PURPOSE, + ASSERT_EQ(ErrorCode::INCOMPATIBLE_PURPOSE, GenerateKey(AuthorizationSetBuilder() .EcdsaSigningKey(EcCurve::P_256) .Authorization(TAG_NO_AUTH_REQUIRED) @@ -881,6 +889,7 @@ TEST_P(AttestKeyTest, EcdsaAttestationID) { } ASSERT_EQ(result, ErrorCode::OK); + ASSERT_GT(attested_key_cert_chain.size(), 0); KeyBlobDeleter attested_deleter(keymint_, attested_key_blob); AuthorizationSet hw_enforced = HwEnforcedAuthorizations(attested_key_characteristics); @@ -891,7 +900,7 @@ TEST_P(AttestKeyTest, EcdsaAttestationID) { // attestation extension should contain them, so make sure the extra tag is added. hw_enforced.push_back(tag); - EXPECT_TRUE(verify_attestation_record(AidlVersion(), "challenge", "foo", sw_enforced, + ASSERT_TRUE(verify_attestation_record(AidlVersion(), "challenge", "foo", sw_enforced, hw_enforced, SecLevel(), attested_key_cert_chain[0].encodedCertificate)); } @@ -1011,6 +1020,7 @@ TEST_P(AttestKeyTest, SecondIMEIAttestationIDSuccess) { } ASSERT_EQ(result, ErrorCode::OK); + ASSERT_GT(attested_key_cert_chain.size(), 0); KeyBlobDeleter attested_deleter(keymint_, attested_key_blob); AuthorizationSet hw_enforced = HwEnforcedAuthorizations(attested_key_characteristics); @@ -1023,7 +1033,7 @@ TEST_P(AttestKeyTest, SecondIMEIAttestationIDSuccess) { KeyParameter imei_tag = Authorization(TAG_ATTESTATION_ID_SECOND_IMEI, imei_blob); hw_enforced.push_back(imei_tag); - EXPECT_TRUE(verify_attestation_record(AidlVersion(), "challenge", "foo", sw_enforced, + ASSERT_TRUE(verify_attestation_record(AidlVersion(), "challenge", "foo", sw_enforced, hw_enforced, SecLevel(), attested_key_cert_chain[0].encodedCertificate)); } @@ -1088,6 +1098,7 @@ TEST_P(AttestKeyTest, MultipleIMEIAttestationIDSuccess) { } ASSERT_EQ(result, ErrorCode::OK); + ASSERT_GT(attested_key_cert_chain.size(), 0); KeyBlobDeleter attested_deleter(keymint_, attested_key_blob); AuthorizationSet hw_enforced = HwEnforcedAuthorizations(attested_key_characteristics); @@ -1103,7 +1114,7 @@ TEST_P(AttestKeyTest, MultipleIMEIAttestationIDSuccess) { KeyParameter sec_imei_tag = Authorization(TAG_ATTESTATION_ID_SECOND_IMEI, sec_imei_blob); hw_enforced.push_back(sec_imei_tag); - EXPECT_TRUE(verify_attestation_record(AidlVersion(), "challenge", "foo", sw_enforced, + ASSERT_TRUE(verify_attestation_record(AidlVersion(), "challenge", "foo", sw_enforced, hw_enforced, SecLevel(), attested_key_cert_chain[0].encodedCertificate)); }