From c3de1caf4327dc367a95f7416cba19827428bd1b Mon Sep 17 00:00:00 2001 From: David Drysdale Date: Tue, 9 May 2023 08:10:36 +0100 Subject: [PATCH] Skip ATTEST_KEY using variant on waivered devices Bug: 281452355 Bug: 289451966 Test: VtsAidlKeyMintTargetTest Change-Id: Id448edae88569518deb2db4ab7bf50d16f33709a --- .../vts/functional/KeyBlobUpgradeTest.cpp | 25 +++++++++++- .../vts/functional/KeyMintAidlTestBase.cpp | 40 ++++++++++++++----- .../aidl/vts/functional/KeyMintAidlTestBase.h | 1 + 3 files changed, 56 insertions(+), 10 deletions(-) diff --git a/security/keymint/aidl/vts/functional/KeyBlobUpgradeTest.cpp b/security/keymint/aidl/vts/functional/KeyBlobUpgradeTest.cpp index 4830422f4d..7ccd246375 100644 --- a/security/keymint/aidl/vts/functional/KeyBlobUpgradeTest.cpp +++ b/security/keymint/aidl/vts/functional/KeyBlobUpgradeTest.cpp @@ -74,6 +74,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", @@ -87,6 +90,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; } @@ -210,6 +218,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; @@ -358,6 +371,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; @@ -441,6 +459,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) { @@ -540,7 +563,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 ee490a37b7..e0e3d988fe 100644 --- a/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.cpp +++ b/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.cpp @@ -1623,17 +1623,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 5d32268cbb..8934a57e7b 100644 --- a/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.h +++ b/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.h @@ -370,6 +370,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: