Merge "Skip ATTEST_KEY using variant on waivered devices" into stage-aosp-udc-ts-dev

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<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 @@ std::vector<std::string> 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

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";
     }
 }
 

security/keymint/aidl/vts/functional/KeyMintAidlTestBase.h

@@ -359,6 +359,7 @@ class KeyMintAidlTestBase : public ::testing::TestWithParam<string> {
     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: