diff --git a/keymaster/4.0/support/attestation_record.cpp b/keymaster/4.0/support/attestation_record.cpp index 000d46e7db..27e00c173a 100644 --- a/keymaster/4.0/support/attestation_record.cpp +++ b/keymaster/4.0/support/attestation_record.cpp @@ -321,19 +321,20 @@ ErrorCode parse_root_of_trust(const uint8_t* asn1_key_desc, size_t asn1_key_desc LOG(ERROR) << AT << "Failed record parsing"; return ErrorCode::UNKNOWN_ERROR; } - if (!record->tee_enforced) { - LOG(ERROR) << AT << "Failed hardware characteristic parsing"; + + KM_ROOT_OF_TRUST* root_of_trust = nullptr; + if (record->tee_enforced && record->tee_enforced->root_of_trust) { + root_of_trust = record->tee_enforced->root_of_trust; + } else if (record->software_enforced && record->software_enforced->root_of_trust) { + root_of_trust = record->software_enforced->root_of_trust; + } else { + LOG(ERROR) << AT << " Failed root of trust parsing"; return ErrorCode::INVALID_ARGUMENT; } - if (!record->tee_enforced->root_of_trust) { - LOG(ERROR) << AT << "Failed root of trust parsing"; + if (!root_of_trust->verified_boot_key) { + LOG(ERROR) << AT << " Failed verified boot key parsing"; return ErrorCode::INVALID_ARGUMENT; } - if (!record->tee_enforced->root_of_trust->verified_boot_key) { - LOG(ERROR) << AT << "Failed verified boot key parsing"; - return ErrorCode::INVALID_ARGUMENT; - } - KM_ROOT_OF_TRUST* root_of_trust = record->tee_enforced->root_of_trust; auto& vb_key = root_of_trust->verified_boot_key; verified_boot_key->resize(vb_key->length); @@ -342,19 +343,19 @@ ErrorCode parse_root_of_trust(const uint8_t* asn1_key_desc, size_t asn1_key_desc *verified_boot_state = static_cast( ASN1_ENUMERATED_get(root_of_trust->verified_boot_state)); if (!verified_boot_state) { - LOG(ERROR) << AT << "Failed verified boot state parsing"; + LOG(ERROR) << AT << " Failed verified boot state parsing"; return ErrorCode::INVALID_ARGUMENT; } *device_locked = root_of_trust->device_locked; if (!device_locked) { - LOG(ERROR) << AT << "Failed device locked parsing"; + LOG(ERROR) << AT << " Failed device locked parsing"; return ErrorCode::INVALID_ARGUMENT; } auto& vb_hash = root_of_trust->verified_boot_hash; if (!vb_hash) { - LOG(ERROR) << AT << "Failed verified boot hash parsing"; + LOG(ERROR) << AT << " Failed verified boot hash parsing"; return ErrorCode::INVALID_ARGUMENT; } verified_boot_hash->resize(vb_hash->length); diff --git a/keymaster/4.0/vts/functional/KeymasterHidlTest.cpp b/keymaster/4.0/vts/functional/KeymasterHidlTest.cpp index 5d0e262ca5..2d2ba632cb 100644 --- a/keymaster/4.0/vts/functional/KeymasterHidlTest.cpp +++ b/keymaster/4.0/vts/functional/KeymasterHidlTest.cpp @@ -45,6 +45,7 @@ namespace test { using namespace std::literals::chrono_literals; void KeymasterHidlTest::InitializeKeymaster() { + std::string instance_name = GetParam(); keymaster_ = IKeymasterDevice::getService(GetParam()); ASSERT_NE(keymaster_, nullptr); @@ -127,7 +128,7 @@ ErrorCode KeymasterHidlTest::ImportWrappedKey(string wrapped_key, string wrappin string masking_key, const AuthorizationSet& unwrapping_params) { ErrorCode error; - ImportKey(wrapping_key_desc, KeyFormat::PKCS8, wrapping_key); + EXPECT_EQ(ErrorCode::OK, ImportKey(wrapping_key_desc, KeyFormat::PKCS8, wrapping_key)); EXPECT_TRUE(keymaster_ ->importWrappedKey(HidlBuf(wrapped_key), key_blob_, HidlBuf(masking_key), unwrapping_params.hidl_data(), 0 /* passwordSid */, @@ -196,7 +197,9 @@ void KeymasterHidlTest::CheckGetCharacteristics(const HidlBuf& key_blob, const H HidlBuf empty_buf = {}; EXPECT_EQ(ErrorCode::OK, GetCharacteristics(key_blob, client_id, app_data, key_characteristics)); - EXPECT_GT(key_characteristics->hardwareEnforced.size(), 0); + if (SecLevel() != SecurityLevel::SOFTWARE) { + EXPECT_GT(key_characteristics->hardwareEnforced.size(), 0); + } EXPECT_GT(key_characteristics->softwareEnforced.size(), 0); EXPECT_EQ(ErrorCode::INVALID_KEY_BLOB, @@ -636,23 +639,25 @@ std::vector KeymasterHidlTest::ValidKeySizes(Algorithm algorithm) { switch (algorithm) { case Algorithm::RSA: switch (SecLevel()) { + case SecurityLevel::SOFTWARE: case SecurityLevel::TRUSTED_ENVIRONMENT: return {2048, 3072, 4096}; case SecurityLevel::STRONGBOX: return {2048}; default: - CHECK(false) << "Invalid security level " << uint32_t(SecLevel()); + ADD_FAILURE() << "Invalid security level " << uint32_t(SecLevel()); break; } break; case Algorithm::EC: switch (SecLevel()) { + case SecurityLevel::SOFTWARE: case SecurityLevel::TRUSTED_ENVIRONMENT: return {224, 256, 384, 521}; case SecurityLevel::STRONGBOX: return {256}; default: - CHECK(false) << "Invalid security level " << uint32_t(SecLevel()); + ADD_FAILURE() << "Invalid security level " << uint32_t(SecLevel()); break; } break; @@ -667,25 +672,27 @@ std::vector KeymasterHidlTest::ValidKeySizes(Algorithm algorithm) { return retval; } default: - CHECK(false) << "Invalid Algorithm: " << algorithm; + ADD_FAILURE() << "Invalid Algorithm: " << algorithm; return {}; } - CHECK(false) << "Should be impossible to get here"; + ADD_FAILURE() << "Should be impossible to get here"; return {}; } + std::vector KeymasterHidlTest::InvalidKeySizes(Algorithm algorithm) { - if (SecLevel() == SecurityLevel::TRUSTED_ENVIRONMENT) return {}; - CHECK(SecLevel() == SecurityLevel::STRONGBOX); - switch (algorithm) { - case Algorithm::RSA: - return {3072, 4096}; - case Algorithm::EC: - return {224, 384, 521}; - case Algorithm::AES: - return {192}; - default: - return {}; + if (SecLevel() == SecurityLevel::STRONGBOX) { + switch (algorithm) { + case Algorithm::RSA: + return {3072, 4096}; + case Algorithm::EC: + return {224, 384, 521}; + case Algorithm::AES: + return {192}; + default: + return {}; + } } + return {}; } std::vector KeymasterHidlTest::ValidCurves() { @@ -704,6 +711,7 @@ std::vector KeymasterHidlTest::InvalidCurves() { std::vector KeymasterHidlTest::ValidDigests(bool withNone, bool withMD5) { switch (SecLevel()) { + case SecurityLevel::SOFTWARE: case SecurityLevel::TRUSTED_ENVIRONMENT: if (withNone) { if (withMD5) @@ -729,10 +737,10 @@ std::vector KeymasterHidlTest::ValidDigests(bool withNone, bool withMD5) return {Digest::SHA_2_256}; break; default: - CHECK(false) << "Invalid security level " << uint32_t(SecLevel()); + ADD_FAILURE() << "Invalid security level " << uint32_t(SecLevel()); break; } - CHECK(false) << "Should be impossible to get here"; + ADD_FAILURE() << "Should be impossible to get here"; return {}; } diff --git a/keymaster/4.0/vts/functional/KeymasterHidlTest.h b/keymaster/4.0/vts/functional/KeymasterHidlTest.h index faa7c75409..34a4473976 100644 --- a/keymaster/4.0/vts/functional/KeymasterHidlTest.h +++ b/keymaster/4.0/vts/functional/KeymasterHidlTest.h @@ -204,6 +204,11 @@ class KeymasterHidlTest : public ::testing::TestWithParam { KeyCharacteristics key_characteristics_; OperationHandle op_handle_ = kOpHandleSentinel; + static std::vector build_params() { + auto params = android::hardware::getAllHalInstanceNames(IKeymasterDevice::descriptor); + return params; + } + private: sp keymaster_; uint32_t os_version_; @@ -214,10 +219,9 @@ class KeymasterHidlTest : public ::testing::TestWithParam { hidl_string author_; }; -#define INSTANTIATE_KEYMASTER_HIDL_TEST(name) \ - INSTANTIATE_TEST_SUITE_P(PerInstance, name, \ - testing::ValuesIn(android::hardware::getAllHalInstanceNames( \ - IKeymasterDevice::descriptor)), \ +#define INSTANTIATE_KEYMASTER_HIDL_TEST(name) \ + INSTANTIATE_TEST_SUITE_P(PerInstance, name, \ + testing::ValuesIn(KeymasterHidlTest::build_params()), \ android::hardware::PrintInstanceNameToString) } // namespace test diff --git a/keymaster/4.0/vts/functional/keymaster_hidl_hal_test.cpp b/keymaster/4.0/vts/functional/keymaster_hidl_hal_test.cpp index fb450d1bb4..ace389bbd3 100644 --- a/keymaster/4.0/vts/functional/keymaster_hidl_hal_test.cpp +++ b/keymaster/4.0/vts/functional/keymaster_hidl_hal_test.cpp @@ -397,10 +397,16 @@ bool verify_attestation_record(const string& challenge, const string& app_id, // true. A provided boolean tag that can be pulled back out of the certificate indicates correct // encoding. No need to check if it's in both lists, since the AuthorizationSet compare below // will handle mismatches of tags. - EXPECT_TRUE(expected_hw_enforced.Contains(TAG_NO_AUTH_REQUIRED)); + if (security_level == SecurityLevel::SOFTWARE) { + EXPECT_TRUE(expected_sw_enforced.Contains(TAG_NO_AUTH_REQUIRED)); + } else { + EXPECT_TRUE(expected_hw_enforced.Contains(TAG_NO_AUTH_REQUIRED)); + } // Alternatively this checks the opposite - a false boolean tag (one that isn't provided in // the authorization list during key generation) isn't being attested to in the certificate. + EXPECT_FALSE(expected_sw_enforced.Contains(TAG_TRUSTED_USER_PRESENCE_REQUIRED)); + EXPECT_FALSE(att_sw_enforced.Contains(TAG_TRUSTED_USER_PRESENCE_REQUIRED)); EXPECT_FALSE(expected_hw_enforced.Contains(TAG_TRUSTED_USER_PRESENCE_REQUIRED)); EXPECT_FALSE(att_hw_enforced.Contains(TAG_TRUSTED_USER_PRESENCE_REQUIRED)); @@ -461,10 +467,10 @@ bool verify_attestation_record(const string& challenge, const string& app_id, verified_boot_key.size())); } else if (!strcmp(property_value, "red")) { EXPECT_EQ(verified_boot_state, KM_VERIFIED_BOOT_FAILED); - EXPECT_EQ(0, memcmp(verified_boot_key.data(), empty_boot_key.data(), - verified_boot_key.size())); } else { - EXPECT_TRUE(false); + EXPECT_EQ(verified_boot_state, KM_VERIFIED_BOOT_UNVERIFIED); + EXPECT_NE(0, memcmp(verified_boot_key.data(), empty_boot_key.data(), + verified_boot_key.size())); } att_sw_enforced.Sort();