diff --git a/security/keymint/aidl/vts/functional/KeyBlobUpgradeTest.cpp b/security/keymint/aidl/vts/functional/KeyBlobUpgradeTest.cpp index 68924422d1..fb014cfe0b 100644 --- a/security/keymint/aidl/vts/functional/KeyBlobUpgradeTest.cpp +++ b/security/keymint/aidl/vts/functional/KeyBlobUpgradeTest.cpp @@ -71,6 +71,9 @@ namespace aidl::android::hardware::security::keymint::test { namespace { +// Names for individual key types to create and use. Note that some the names +// induce specific behaviour, as indicated by the functions below. + std::vector keyblob_names_tee = { "aes-key", "aes-key-rr", "des-key", "hmac-key", "rsa-key", "p256-key", "ed25519-key", "x25519-key", @@ -84,6 +87,11 @@ std::vector keyblob_names_sb = {"aes-key", "aes-key-rr", "hmac-key", "rsa-key", "p256-key", "rsa-attest-key", "p256-attest-key"}; +// Helper functions to detect particular key types based on the name. +bool requires_attest_key(const std::string& name) { + return name.find("-attest-key") != std::string::npos; +} + bool requires_rr(const std::string& name) { return name.find("-rr") != std::string::npos; } @@ -207,6 +215,11 @@ class KeyBlobUpgradeTest : public KeyMintAidlTestBase { } for (std::string name : keyblob_names()) { + if (requires_attest_key(name) && shouldSkipAttestKeyTest()) { + std::cerr << "Skipping variant '" << name + << "' which requires ATTEST_KEY support that has been waivered\n"; + continue; + } for (bool with_hidden : {false, true}) { std::string app_id; std::string app_data; @@ -355,6 +368,11 @@ TEST_P(KeyBlobUpgradeTest, CreateKeyBlobsBefore) { }}; for (std::string name : keyblob_names()) { + if (requires_attest_key(name) && shouldSkipAttestKeyTest()) { + std::cerr << "Skipping variant '" << name + << "' which requires ATTEST_KEY support that has been waivered\n"; + continue; + } auto entry = keys_info.find(name); ASSERT_NE(entry, keys_info.end()) << "no builder for " << name; auto builder = entry->second; @@ -432,6 +450,11 @@ TEST_P(KeyBlobUpgradeTest, UseKeyBlobsBeforeOrAfter) { } for (std::string name : keyblob_names()) { + if (requires_attest_key(name) && shouldSkipAttestKeyTest()) { + std::cerr << "Skipping variant '" << name + << "' which requires ATTEST_KEY support that has been waivered\n"; + continue; + } for (bool with_hidden : {false, true}) { auto builder = AuthorizationSetBuilder(); if (with_hidden) { @@ -531,7 +554,7 @@ TEST_P(KeyBlobUpgradeTest, UseKeyBlobsBeforeOrAfter) { // Both ways round should agree. EXPECT_EQ(keymint_data, local_data); - } else if (name.find("-attest-key") != std::string::npos) { + } else if (requires_attest_key(name)) { // Covers rsa-attest-key, p256-attest-key, ed25519-attest-key. // Use attestation key to sign RSA signing key diff --git a/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.cpp b/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.cpp index 284af941b1..79c6b12f03 100644 --- a/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.cpp +++ b/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.cpp @@ -1617,17 +1617,39 @@ bool KeyMintAidlTestBase::is_chipset_allowed_km4_strongbox(void) const { return false; } -// Skip the test if all the following conditions hold: -// 1. ATTEST_KEY feature is disabled -// 2. STRONGBOX is enabled -// 3. The device is running one of the chipsets that have received a waiver -// allowing it to be launched with Android S (or later) with Keymaster 4.0 +// Indicate whether a test that involves use of the ATTEST_KEY feature should be +// skipped. +// +// In general, every KeyMint implementation should support ATTEST_KEY; +// however, there is a waiver for some specific devices that ship with a +// combination of Keymaster/StrongBox and KeyMint/TEE. On these devices, the +// ATTEST_KEY feature is disabled in the KeyMint/TEE implementation so that +// the device has consistent ATTEST_KEY behavior (ie. UNIMPLEMENTED) across both +// HAL implementations. +// +// This means that a test involving ATTEST_KEY test should be skipped if all of +// the following conditions hold: +// 1. The device is running one of the chipsets that have received a waiver +// allowing it to be launched with Android S or T with Keymaster 4.0 // in StrongBox -void KeyMintAidlTestBase::skipAttestKeyTest(void) const { +// 2. The device has a STRONGBOX implementation present. +// 3. ATTEST_KEY feature is advertised as disabled. +// +// Note that in this scenario, ATTEST_KEY tests should be skipped for both +// the StrongBox implementation (which is Keymaster, therefore not tested here) +// and for the TEE implementation (which is adjusted to return UNIMPLEMENTED +// specifically for this waiver). +bool KeyMintAidlTestBase::shouldSkipAttestKeyTest(void) const { // Check the chipset first as that doesn't require a round-trip to Package Manager. - if (is_chipset_allowed_km4_strongbox() && is_strongbox_enabled() && - is_attest_key_feature_disabled()) { - GTEST_SKIP() << "Test is not applicable"; + return (is_chipset_allowed_km4_strongbox() && is_strongbox_enabled() && + is_attest_key_feature_disabled()); +} + +// Skip a test that involves use of the ATTEST_KEY feature in specific configurations +// where ATTEST_KEY is not supported (for either StrongBox or TEE). +void KeyMintAidlTestBase::skipAttestKeyTest(void) const { + if (shouldSkipAttestKeyTest()) { + GTEST_SKIP() << "Test using ATTEST_KEY is not applicable on waivered device"; } } diff --git a/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.h b/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.h index 6318514d6b..d8c9f112f2 100644 --- a/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.h +++ b/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.h @@ -359,6 +359,7 @@ class KeyMintAidlTestBase : public ::testing::TestWithParam { bool is_attest_key_feature_disabled(void) const; bool is_strongbox_enabled(void) const; bool is_chipset_allowed_km4_strongbox(void) const; + bool shouldSkipAttestKeyTest(void) const; void skipAttestKeyTest(void) const; protected: