Skip ATTEST_KEY using variant on waivered devices
Bug: 281452355
Bug: 289451966
Test: VtsAidlKeyMintTargetTest
(cherry picked from https://android-review.googlesource.com/q/commit:c3de1caf4327dc367a95f7416cba19827428bd1b)
Merged-In: Id448edae88569518deb2db4ab7bf50d16f33709a
Change-Id: Id448edae88569518deb2db4ab7bf50d16f33709a
diff --git a/security/keymint/aidl/vts/functional/KeyBlobUpgradeTest.cpp b/security/keymint/aidl/vts/functional/KeyBlobUpgradeTest.cpp
index 6892442..fb014cf 100644
--- a/security/keymint/aidl/vts/functional/KeyBlobUpgradeTest.cpp
+++ b/security/keymint/aidl/vts/functional/KeyBlobUpgradeTest.cpp
@@ -71,6 +71,9 @@
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<std::string> keyblob_names_tee = {
"aes-key", "aes-key-rr", "des-key", "hmac-key",
"rsa-key", "p256-key", "ed25519-key", "x25519-key",
@@ -84,6 +87,11 @@
"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 @@
}
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 @@
}};
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 @@
}
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 @@
// 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 a8ea407..ec7256e 100644
--- a/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.cpp
+++ b/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.cpp
@@ -1617,17 +1617,39 @@
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 30ac452..1763e94 100644
--- a/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.h
+++ b/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.h
@@ -359,6 +359,7 @@
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: