Amend tests on GSI that rely on RKP-only props
GSI replaces the values for remote_prov_prop properties (since they’re
system_internal_prop properties), so on GSI the properties are not
reliable indicators of whether StrongBox/TEE are RKP-only or not.
Also included is the removal of the helper skipAttestKeyTestIfNeeded()
so the skipping can happen in the tests directly.
Bug: 348159232
Test: VtsAidlKeyMintTargetTest
Change-Id: I2075e1f76ddd0f87620a212e1aa389803139a117
diff --git a/security/keymint/aidl/vts/functional/AttestKeyTest.cpp b/security/keymint/aidl/vts/functional/AttestKeyTest.cpp
index 5106561..464883e 100644
--- a/security/keymint/aidl/vts/functional/AttestKeyTest.cpp
+++ b/security/keymint/aidl/vts/functional/AttestKeyTest.cpp
@@ -39,7 +39,9 @@
class AttestKeyTest : public KeyMintAidlTestBase {
public:
void SetUp() override {
- skipAttestKeyTestIfNeeded();
+ if (shouldSkipAttestKeyTest()) {
+ GTEST_SKIP() << "Test using ATTEST_KEY is not applicable on waivered device";
+ }
KeyMintAidlTestBase::SetUp();
}
};
@@ -251,7 +253,11 @@
.SetDefaultValidity(),
{} /* attestation signing key */, &attest_key.keyBlob,
&attest_key_characteristics, &attest_key_cert_chain);
- if (isRkpOnly() && result == ErrorCode::ATTESTATION_KEYS_NOT_PROVISIONED) {
+ std::optional<bool> rkpOnly = isRkpOnly();
+ if (!rkpOnly.has_value()) {
+ GTEST_SKIP() << "Test not applicable because RKP-only status cannot be determined";
+ }
+ if (rkpOnly.value() && result == ErrorCode::ATTESTATION_KEYS_NOT_PROVISIONED) {
GTEST_SKIP() << "RKP-only devices do not have a factory key";
}
ASSERT_EQ(ErrorCode::OK, result);
@@ -355,7 +361,8 @@
.Authorization(TAG_CERTIFICATE_SUBJECT, subject_der)
.SetDefaultValidity();
// In RKP-only systems, the first key cannot be attested due to lack of batch key
- if (!isRkpOnly() || i > 0) {
+ bool confirmedNotRkpOnly = !isRkpOnly().value_or(true);
+ if (confirmedNotRkpOnly || i > 0) {
auth_set_builder.AttestationChallenge("foo");
}
auto result = GenerateAttestKey(auth_set_builder, attest_key_opt, &key_blob_list[i],
@@ -363,7 +370,7 @@
ASSERT_EQ(ErrorCode::OK, result);
deleters.push_back(KeyBlobDeleter(keymint_, key_blob_list[i]));
- if (!isRkpOnly() || i > 0) {
+ if (confirmedNotRkpOnly || i > 0) {
AuthorizationSet hw_enforced = HwEnforcedAuthorizations(attested_key_characteristics);
AuthorizationSet sw_enforced = SwEnforcedAuthorizations(attested_key_characteristics);
ASSERT_GT(cert_chain_list[i].size(), 0);
@@ -386,7 +393,7 @@
}
EXPECT_TRUE(ChainSignaturesAreValid(cert_chain_list[i]));
- EXPECT_GT(cert_chain_list[i].size(), i + (isRkpOnly() ? 0 : 1));
+ EXPECT_GT(cert_chain_list[i].size(), i + (confirmedNotRkpOnly ? 1 : 0));
verify_subject_and_serial(cert_chain_list[i][0], serial_int, subject, false);
}
}
@@ -432,7 +439,8 @@
.Authorization(TAG_NO_AUTH_REQUIRED)
.SetDefaultValidity();
// In RKP-only systems, the first key cannot be attested due to lack of batch key
- if (!isRkpOnly() || i > 0) {
+ bool confirmedNotRkpOnly = !isRkpOnly().value_or(true);
+ if (confirmedNotRkpOnly || i > 0) {
auth_set_builder.AttestationChallenge("foo");
}
auto result = GenerateAttestKey(auth_set_builder, attest_key_opt, &key_blob_list[i],
@@ -440,7 +448,7 @@
ASSERT_EQ(ErrorCode::OK, result);
deleters.push_back(KeyBlobDeleter(keymint_, key_blob_list[i]));
- if (!isRkpOnly() || i > 0) {
+ if (confirmedNotRkpOnly || i > 0) {
AuthorizationSet hw_enforced = HwEnforcedAuthorizations(attested_key_characteristics);
AuthorizationSet sw_enforced = SwEnforcedAuthorizations(attested_key_characteristics);
ASSERT_GT(cert_chain_list[i].size(), 0);
@@ -459,7 +467,7 @@
}
EXPECT_TRUE(ChainSignaturesAreValid(cert_chain_list[i]));
- EXPECT_GT(cert_chain_list[i].size(), i + (isRkpOnly() ? 0 : 1));
+ EXPECT_GT(cert_chain_list[i].size(), i + (confirmedNotRkpOnly ? 1 : 0));
verify_subject_and_serial(cert_chain_list[i][0], serial_int, subject, false);
}
}
@@ -530,7 +538,8 @@
.Authorization(TAG_NO_AUTH_REQUIRED)
.SetDefaultValidity();
// In RKP-only systems, the first key cannot be attested due to lack of batch key
- if (!isRkpOnly() || i > 0) {
+ bool confirmedNotRkpOnly = !isRkpOnly().value_or(true);
+ if (confirmedNotRkpOnly || i > 0) {
auth_set_builder.AttestationChallenge("foo");
}
if ((i & 0x1) == 1) {
@@ -543,7 +552,7 @@
ASSERT_EQ(ErrorCode::OK, result);
deleters.push_back(KeyBlobDeleter(keymint_, key_blob_list[i]));
- if (!isRkpOnly() || i > 0) {
+ if (confirmedNotRkpOnly || i > 0) {
AuthorizationSet hw_enforced = HwEnforcedAuthorizations(attested_key_characteristics);
AuthorizationSet sw_enforced = SwEnforcedAuthorizations(attested_key_characteristics);
ASSERT_GT(cert_chain_list[i].size(), 0);
@@ -566,7 +575,7 @@
}
EXPECT_TRUE(ChainSignaturesAreValid(cert_chain_list[i]));
- EXPECT_GT(cert_chain_list[i].size(), i + (isRkpOnly() ? 0 : 1));
+ EXPECT_GT(cert_chain_list[i].size(), i + (confirmedNotRkpOnly ? 1 : 0));
verify_subject_and_serial(cert_chain_list[i][0], serial_int, subject, false);
}
}
diff --git a/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.cpp b/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.cpp
index cef8120..2ba75a3 100644
--- a/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.cpp
+++ b/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.cpp
@@ -250,7 +250,13 @@
return AidlVersion() >= 3 && property_get_int32("ro.vendor.api_level", 0) > __ANDROID_API_T__;
}
-bool KeyMintAidlTestBase::isRkpOnly() {
+std::optional<bool> KeyMintAidlTestBase::isRkpOnly() {
+ // GSI replaces the values for remote_prov_prop properties (since they’re system_internal_prop
+ // properties), so on GSI the properties are not reliable indicators of whether StrongBox/TEE is
+ // RKP-only or not.
+ if (is_gsi_image()) {
+ return std::nullopt;
+ }
if (SecLevel() == SecurityLevel::STRONGBOX) {
return property_get_bool("remote_provisioning.strongbox.rkp_only", false);
}
@@ -318,8 +324,11 @@
vector<Certificate> attest_cert_chain;
// If an attestation is requested, but the system is RKP-only, we need to supply an explicit
// attestation key. Else the result is a key without an attestation.
- if (isRkpOnly() && key_desc.Contains(TAG_ATTESTATION_CHALLENGE)) {
- skipAttestKeyTestIfNeeded();
+ // If the RKP-only value is undeterminable (i.e., when running on GSI), generate and use the
+ // attest key anyways. In the case that using an attest key is not supported
+ // (shouldSkipAttestKeyTest), assume the device has factory keys (so not RKP-only).
+ if (isRkpOnly().value_or(true) && key_desc.Contains(TAG_ATTESTATION_CHALLENGE) &&
+ !shouldSkipAttestKeyTest()) {
AuthorizationSet attest_key_desc =
AuthorizationSetBuilder().EcdsaKey(EcCurve::P_256).AttestKey().SetDefaultValidity();
attest_key.emplace();
@@ -1677,14 +1686,6 @@
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::skipAttestKeyTestIfNeeded() const {
- if (shouldSkipAttestKeyTest()) {
- GTEST_SKIP() << "Test using ATTEST_KEY is not applicable on waivered device";
- }
-}
-
void verify_serial(X509* cert, const uint64_t expected_serial) {
BIGNUM_Ptr ser(BN_new());
EXPECT_TRUE(ASN1_INTEGER_to_BN(X509_get_serialNumber(cert), ser.get()));
diff --git a/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.h b/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.h
index 1bf2d9d..0368bba 100644
--- a/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.h
+++ b/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.h
@@ -104,7 +104,7 @@
uint32_t boot_patch_level();
bool isDeviceIdAttestationRequired();
bool isSecondImeiIdAttestationRequired();
- bool isRkpOnly();
+ std::optional<bool> isRkpOnly();
bool Curve25519Supported();
@@ -356,7 +356,6 @@
bool is_strongbox_enabled(void) const;
bool is_chipset_allowed_km4_strongbox(void) const;
bool shouldSkipAttestKeyTest(void) const;
- void skipAttestKeyTestIfNeeded() const;
void assert_mgf_digests_present_or_not_in_key_characteristics(
const vector<KeyCharacteristics>& key_characteristics,