From 2114dca2aa84f25793ee2094d0d5fec9452d775a Mon Sep 17 00:00:00 2001 From: Prashant Patil Date: Thu, 21 Sep 2023 14:57:10 +0000 Subject: [PATCH] RSA OAEP MGF1 digest VTS relaxed for keymint < V3 Since there were no VTS test to strictly check RSA_OAEP_MGF_DIGEST, there are released devices with Keymint which do not include this tag in key characteristics, hence these test fails on such Keymint and UDC Android framework. Hence version check is added before asserting MGF digest checks. Bug: 297306437 Test: atest VtsAidlKeyMintTargetTest Change-Id: I43054f8dbbd46de53deef5e6771c736e770280e0 --- .../vts/functional/KeyMintAidlTestBase.cpp | 57 +++++++++------ .../aidl/vts/functional/KeyMintAidlTestBase.h | 16 +++-- .../aidl/vts/functional/KeyMintTest.cpp | 71 +++++++++++-------- 3 files changed, 87 insertions(+), 57 deletions(-) diff --git a/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.cpp b/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.cpp index 3a8a1e8eb5..822770d155 100644 --- a/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.cpp +++ b/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.cpp @@ -79,7 +79,8 @@ size_t count_tag_invalid_entries(const std::vector& authorizations typedef KeyMintAidlTestBase::KeyData KeyData; // Predicate for testing basic characteristics validity in generation or import. bool KeyCharacteristicsBasicallyValid(SecurityLevel secLevel, - const vector& key_characteristics) { + const vector& key_characteristics, + int32_t aidl_version) { if (key_characteristics.empty()) return false; std::unordered_set levels_seen; @@ -89,7 +90,12 @@ bool KeyCharacteristicsBasicallyValid(SecurityLevel secLevel, return false; } - EXPECT_EQ(count_tag_invalid_entries(entry.authorizations), 0); + // There was no test to assert that INVALID tag should not present in authorization list + // before Keymint V3, so there are some Keymint implementations where asserting for INVALID + // tag fails(b/297306437), hence skipping for Keymint < 3. + if (aidl_version >= 3) { + EXPECT_EQ(count_tag_invalid_entries(entry.authorizations), 0); + } // Just ignore the SecurityLevel::KEYSTORE as the KM won't do any enforcement on this. if (entry.securityLevel == SecurityLevel::KEYSTORE) continue; @@ -264,7 +270,7 @@ void KeyMintAidlTestBase::InitializeKeyMint(std::shared_ptr keyM vendor_patch_level_ = getVendorPatchlevel(); } -int32_t KeyMintAidlTestBase::AidlVersion() { +int32_t KeyMintAidlTestBase::AidlVersion() const { int32_t version = 0; auto status = keymint_->getInterfaceVersion(&version); if (!status.isOk()) { @@ -294,8 +300,8 @@ ErrorCode KeyMintAidlTestBase::GenerateKey(const AuthorizationSet& key_desc, KeyCreationResult creationResult; Status result = keymint_->generateKey(key_desc.vector_data(), attest_key, &creationResult); if (result.isOk()) { - EXPECT_PRED2(KeyCharacteristicsBasicallyValid, SecLevel(), - creationResult.keyCharacteristics); + EXPECT_PRED3(KeyCharacteristicsBasicallyValid, SecLevel(), + creationResult.keyCharacteristics, AidlVersion()); EXPECT_GT(creationResult.keyBlob.size(), 0); *key_blob = std::move(creationResult.keyBlob); *key_characteristics = std::move(creationResult.keyCharacteristics); @@ -367,8 +373,8 @@ ErrorCode KeyMintAidlTestBase::ImportKey(const AuthorizationSet& key_desc, KeyFo {} /* attestationSigningKeyBlob */, &creationResult); if (result.isOk()) { - EXPECT_PRED2(KeyCharacteristicsBasicallyValid, SecLevel(), - creationResult.keyCharacteristics); + EXPECT_PRED3(KeyCharacteristicsBasicallyValid, SecLevel(), + creationResult.keyCharacteristics, AidlVersion()); EXPECT_GT(creationResult.keyBlob.size(), 0); *key_blob = std::move(creationResult.keyBlob); @@ -411,8 +417,8 @@ ErrorCode KeyMintAidlTestBase::ImportWrappedKey(string wrapped_key, string wrapp unwrapping_params.vector_data(), password_sid, biometric_sid, &creationResult); if (result.isOk()) { - EXPECT_PRED2(KeyCharacteristicsBasicallyValid, SecLevel(), - creationResult.keyCharacteristics); + EXPECT_PRED3(KeyCharacteristicsBasicallyValid, SecLevel(), + creationResult.keyCharacteristics, AidlVersion()); EXPECT_GT(creationResult.keyBlob.size(), 0); key_blob_ = std::move(creationResult.keyBlob); @@ -2068,27 +2074,36 @@ vector make_name_from_str(const string& name) { return retval; } -void assert_mgf_digests_present_in_key_characteristics( +void KeyMintAidlTestBase::assert_mgf_digests_present_or_not_in_key_characteristics( + std::vector& expected_mgf_digests, + bool is_mgf_digest_expected) const { + assert_mgf_digests_present_or_not_in_key_characteristics( + key_characteristics_, expected_mgf_digests, is_mgf_digest_expected); +} + +void KeyMintAidlTestBase::assert_mgf_digests_present_or_not_in_key_characteristics( const vector& key_characteristics, - std::vector& expected_mgf_digests) { + std::vector& expected_mgf_digests, + bool is_mgf_digest_expected) const { + // There was no test to assert that MGF1 digest was present in generated/imported key + // characteristics before Keymint V3, so there are some Keymint implementations where + // asserting for MGF1 digest fails(b/297306437), hence skipping for Keymint < 3. + if (AidlVersion() < 3) { + return; + } AuthorizationSet auths; for (auto& entry : key_characteristics) { auths.push_back(AuthorizationSet(entry.authorizations)); } for (auto digest : expected_mgf_digests) { - ASSERT_TRUE(auths.Contains(TAG_RSA_OAEP_MGF_DIGEST, digest)); + if (is_mgf_digest_expected) { + ASSERT_TRUE(auths.Contains(TAG_RSA_OAEP_MGF_DIGEST, digest)); + } else { + ASSERT_FALSE(auths.Contains(TAG_RSA_OAEP_MGF_DIGEST, digest)); + } } } -bool is_mgf_digest_present(const vector& key_characteristics, - android::hardware::security::keymint::Digest expected_mgf_digest) { - AuthorizationSet auths; - for (auto& entry : key_characteristics) { - auths.push_back(AuthorizationSet(entry.authorizations)); - } - return auths.Contains(TAG_RSA_OAEP_MGF_DIGEST, expected_mgf_digest); -} - namespace { void check_cose_key(const vector& data, bool testMode) { diff --git a/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.h b/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.h index 9778b3d0f9..4fb711c7bb 100644 --- a/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.h +++ b/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.h @@ -95,7 +95,7 @@ class KeyMintAidlTestBase : public ::testing::TestWithParam { void InitializeKeyMint(std::shared_ptr keyMint); IKeyMintDevice& keyMint() { return *keymint_; } - int32_t AidlVersion(); + int32_t AidlVersion() const; uint32_t os_version() { return os_version_; } uint32_t os_patch_level() { return os_patch_level_; } uint32_t vendor_patch_level() { return vendor_patch_level_; } @@ -373,6 +373,15 @@ class KeyMintAidlTestBase : public ::testing::TestWithParam { bool shouldSkipAttestKeyTest(void) const; void skipAttestKeyTest(void) const; + void assert_mgf_digests_present_or_not_in_key_characteristics( + const vector& key_characteristics, + std::vector& expected_mgf_digests, + bool is_mgf_digest_expected) const; + + void assert_mgf_digests_present_or_not_in_key_characteristics( + std::vector& expected_mgf_digests, + bool is_mgf_digest_expected) const; + protected: std::shared_ptr keymint_; uint32_t os_version_; @@ -430,11 +439,6 @@ string bin2hex(const vector& data); X509_Ptr parse_cert_blob(const vector& blob); ASN1_OCTET_STRING* get_attestation_record(X509* certificate); vector make_name_from_str(const string& name); -void assert_mgf_digests_present_in_key_characteristics( - const vector& key_characteristics, - std::vector& expected_mgf_digests); -bool is_mgf_digest_present(const vector& key_characteristics, - android::hardware::security::keymint::Digest expected_mgf_digest); void check_maced_pubkey(const MacedPublicKey& macedPubKey, bool testMode, vector* payload_value); void p256_pub_key(const vector& coseKeyData, EVP_PKEY_Ptr* signingKey); diff --git a/security/keymint/aidl/vts/functional/KeyMintTest.cpp b/security/keymint/aidl/vts/functional/KeyMintTest.cpp index de563c4be9..a8f17dd1b2 100644 --- a/security/keymint/aidl/vts/functional/KeyMintTest.cpp +++ b/security/keymint/aidl/vts/functional/KeyMintTest.cpp @@ -4743,6 +4743,12 @@ TEST_P(ImportKeyTest, GetKeyCharacteristics) { * should have the correct characteristics. */ TEST_P(ImportKeyTest, RsaOaepMGFDigestSuccess) { + // There was no test to assert that MGF1 digest was present in generated/imported key + // characteristics before Keymint V3, so there are some Keymint implementations where + // this test case fails(b/297306437), hence this test is skipped for Keymint < 3. + if (AidlVersion() < 3) { + GTEST_SKIP() << "Test not applicable to Keymint < V3"; + } auto mgf_digests = ValidDigests(false /* withNone */, true /* withMD5 */); size_t key_size = 2048; @@ -4763,7 +4769,7 @@ TEST_P(ImportKeyTest, RsaOaepMGFDigestSuccess) { CheckOrigin(); // Make sure explicitly specified mgf-digests exist in key characteristics. - assert_mgf_digests_present_in_key_characteristics(key_characteristics_, mgf_digests); + assert_mgf_digests_present_or_not_in_key_characteristics(mgf_digests, true); string message = "Hello"; @@ -4827,8 +4833,9 @@ TEST_P(ImportKeyTest, RsaOaepMGFDigestDefaultSuccess) { CheckCryptoParam(TAG_PADDING, PaddingMode::RSA_OAEP); CheckOrigin(); + vector defaultDigest = {Digest::SHA1}; // Make sure default mgf-digest (SHA1) is not included in Key characteristics. - ASSERT_FALSE(is_mgf_digest_present(key_characteristics_, Digest::SHA1)); + assert_mgf_digests_present_or_not_in_key_characteristics(defaultDigest, false); } INSTANTIATE_KEYMINT_AIDL_TEST(ImportKeyTest); @@ -5256,7 +5263,7 @@ TEST_P(EncryptionOperationsTest, RsaNoPaddingShortMessage) { */ TEST_P(EncryptionOperationsTest, RsaOaepSuccess) { auto digests = ValidDigests(false /* withNone */, true /* withMD5 */); - auto mgf_digest = Digest::SHA1; + auto mgf_digest = vector{Digest::SHA1}; size_t key_size = 2048; // Need largish key for SHA-512 test. ASSERT_EQ(ErrorCode::OK, GenerateKey(AuthorizationSetBuilder() @@ -5264,11 +5271,11 @@ TEST_P(EncryptionOperationsTest, RsaOaepSuccess) { .RsaEncryptionKey(key_size, 65537) .Padding(PaddingMode::RSA_OAEP) .Digest(digests) - .Authorization(TAG_RSA_OAEP_MGF_DIGEST, mgf_digest) + .OaepMGFDigest(mgf_digest) .SetDefaultValidity())); // Make sure explicitly specified mgf-digest exist in key characteristics. - ASSERT_TRUE(is_mgf_digest_present(key_characteristics_, mgf_digest)); + assert_mgf_digests_present_or_not_in_key_characteristics(mgf_digest, true); string message = "Hello"; @@ -5393,21 +5400,21 @@ TEST_P(EncryptionOperationsTest, RsaOaepWithMGFDigestSuccess) { .Padding(PaddingMode::RSA_OAEP) .Digest(Digest::SHA_2_256) .SetDefaultValidity())); + if (AidlVersion() >= 3) { + std::vector mgf1DigestsInAuths; + mgf1DigestsInAuths.reserve(digests.size()); + const auto& hw_auths = SecLevelAuthorizations(key_characteristics_); + std::for_each(hw_auths.begin(), hw_auths.end(), [&](auto& param) { + if (param.tag == Tag::RSA_OAEP_MGF_DIGEST) { + KeyParameterValue value = param.value; + mgf1DigestsInAuths.push_back(param.value.template get()); + } + }); - std::vector mgf1DigestsInAuths; - mgf1DigestsInAuths.reserve(digests.size()); - const auto& hw_auths = SecLevelAuthorizations(key_characteristics_); - std::for_each(hw_auths.begin(), hw_auths.end(), [&](auto& param) { - if (param.tag == Tag::RSA_OAEP_MGF_DIGEST) { - KeyParameterValue value = param.value; - mgf1DigestsInAuths.push_back(param.value.template get()); - } - }); - - std::sort(digests.begin(), digests.end()); - std::sort(mgf1DigestsInAuths.begin(), mgf1DigestsInAuths.end()); - EXPECT_EQ(digests, mgf1DigestsInAuths); - + std::sort(digests.begin(), digests.end()); + std::sort(mgf1DigestsInAuths.begin(), mgf1DigestsInAuths.end()); + EXPECT_EQ(digests, mgf1DigestsInAuths); + } string message = "Hello"; for (auto digest : digests) { @@ -5462,8 +5469,9 @@ TEST_P(EncryptionOperationsTest, RsaOaepMGFDigestDefaultSuccess) { .Digest(Digest::SHA_2_256) .SetDefaultValidity())); + vector defaultDigest = vector{Digest::SHA1}; // Make sure default mgf-digest (SHA1) is not included in Key characteristics. - ASSERT_FALSE(is_mgf_digest_present(key_characteristics_, Digest::SHA1)); + assert_mgf_digests_present_or_not_in_key_characteristics(defaultDigest, false); // Do local RSA encryption using the default MGF digest of SHA-1. string message = "Hello"; @@ -5499,19 +5507,20 @@ TEST_P(EncryptionOperationsTest, RsaOaepMGFDigestDefaultSuccess) { */ TEST_P(EncryptionOperationsTest, RsaOaepMGFDigestDefaultFail) { size_t key_size = 2048; - auto mgf_digest = Digest::SHA_2_256; + auto mgf_digest = vector{Digest::SHA_2_256}; ASSERT_EQ(ErrorCode::OK, GenerateKey(AuthorizationSetBuilder() .Authorization(TAG_NO_AUTH_REQUIRED) - .Authorization(TAG_RSA_OAEP_MGF_DIGEST, mgf_digest) + .OaepMGFDigest(mgf_digest) .RsaEncryptionKey(key_size, 65537) .Padding(PaddingMode::RSA_OAEP) .Digest(Digest::SHA_2_256) .SetDefaultValidity())); // Make sure explicitly specified mgf-digest exist in key characteristics. - ASSERT_TRUE(is_mgf_digest_present(key_characteristics_, mgf_digest)); + assert_mgf_digests_present_or_not_in_key_characteristics(mgf_digest, true); + vector defaultDigest = vector{Digest::SHA1}; // Make sure default mgf-digest is not included in key characteristics. - ASSERT_FALSE(is_mgf_digest_present(key_characteristics_, Digest::SHA1)); + assert_mgf_digests_present_or_not_in_key_characteristics(defaultDigest, false); // Do local RSA encryption using the default MGF digest of SHA-1. string message = "Hello"; @@ -5535,16 +5544,17 @@ TEST_P(EncryptionOperationsTest, RsaOaepMGFDigestDefaultFail) { * with incompatible MGF digest. */ TEST_P(EncryptionOperationsTest, RsaOaepWithMGFIncompatibleDigest) { - auto mgf_digest = Digest::SHA_2_256; + auto mgf_digest = vector{Digest::SHA_2_256}; ASSERT_EQ(ErrorCode::OK, GenerateKey(AuthorizationSetBuilder() - .Authorization(TAG_RSA_OAEP_MGF_DIGEST, mgf_digest) + .OaepMGFDigest(mgf_digest) .Authorization(TAG_NO_AUTH_REQUIRED) .RsaEncryptionKey(2048, 65537) .Padding(PaddingMode::RSA_OAEP) .Digest(Digest::SHA_2_256) .SetDefaultValidity())); + // Make sure explicitly specified mgf-digest exist in key characteristics. - ASSERT_TRUE(is_mgf_digest_present(key_characteristics_, mgf_digest)); + assert_mgf_digests_present_or_not_in_key_characteristics(mgf_digest, true); string message = "Hello World!"; @@ -5562,16 +5572,17 @@ TEST_P(EncryptionOperationsTest, RsaOaepWithMGFIncompatibleDigest) { * with unsupported MGF digest. */ TEST_P(EncryptionOperationsTest, RsaOaepWithMGFUnsupportedDigest) { - auto mgf_digest = Digest::SHA_2_256; + auto mgf_digest = vector{Digest::SHA_2_256}; ASSERT_EQ(ErrorCode::OK, GenerateKey(AuthorizationSetBuilder() - .Authorization(TAG_RSA_OAEP_MGF_DIGEST, mgf_digest) + .OaepMGFDigest(mgf_digest) .Authorization(TAG_NO_AUTH_REQUIRED) .RsaEncryptionKey(2048, 65537) .Padding(PaddingMode::RSA_OAEP) .Digest(Digest::SHA_2_256) .SetDefaultValidity())); + // Make sure explicitly specified mgf-digest exist in key characteristics. - ASSERT_TRUE(is_mgf_digest_present(key_characteristics_, mgf_digest)); + assert_mgf_digests_present_or_not_in_key_characteristics(mgf_digest, true); string message = "Hello World!";