Fix flaky corrupted padding tests
Transfer the fix in http://aosp/1745035 from the KeyMint VTS test back
into the keymaster VTS test.
Bug: 189261752
Test: VtsHalKeymasterV4_0TargetTest
Change-Id: I5f0a69255cfe980dd6e71fa29ff06a84cb668f6d
diff --git a/keymaster/4.0/vts/functional/keymaster_hidl_hal_test.cpp b/keymaster/4.0/vts/functional/keymaster_hidl_hal_test.cpp
index 476eed8..773715e 100644
--- a/keymaster/4.0/vts/functional/keymaster_hidl_hal_test.cpp
+++ b/keymaster/4.0/vts/functional/keymaster_hidl_hal_test.cpp
@@ -81,6 +81,12 @@
namespace test {
namespace {
+// The maximum number of times we'll attempt to verify that corruption
+// of an encrypted blob results in an error. Retries are necessary as there
+// is a small (roughly 1/256) chance that corrupting ciphertext still results
+// in valid PKCS7 padding.
+constexpr size_t kMaxPaddingCorruptionRetries = 8;
+
template <TagType tag_type, Tag tag, typename ValueT>
bool contains(hidl_vec<KeyParameter>& set, TypedTag<tag_type, tag> ttag, ValueT expected_value) {
size_t count = std::count_if(set.begin(), set.end(), [&](const KeyParameter& param) {
@@ -2849,11 +2855,22 @@
string ciphertext = EncryptMessage(message, params);
EXPECT_EQ(16U, ciphertext.size());
EXPECT_NE(ciphertext, message);
- ++ciphertext[ciphertext.size() / 2];
- EXPECT_EQ(ErrorCode::OK, Begin(KeyPurpose::DECRYPT, params));
- string plaintext;
- EXPECT_EQ(ErrorCode::INVALID_INPUT_LENGTH, Finish(message, &plaintext));
+ for (size_t i = 0; i < kMaxPaddingCorruptionRetries; ++i) {
+ ++ciphertext[ciphertext.size() / 2];
+
+ EXPECT_EQ(ErrorCode::OK, Begin(KeyPurpose::DECRYPT, params));
+ string plaintext;
+ ErrorCode error = Finish(message, &plaintext);
+ if (error == ErrorCode::INVALID_INPUT_LENGTH) {
+ // This is the expected error, we can exit the test now.
+ return;
+ } else {
+ // Very small chance we got valid decryption, so try again.
+ ASSERT_EQ(error, ErrorCode::OK);
+ }
+ }
+ FAIL() << "Corrupt ciphertext should have failed to decrypt by now.";
}
HidlBuf CopyIv(const AuthorizationSet& set) {
@@ -3876,17 +3893,30 @@
string ciphertext = EncryptMessage(message, BlockMode::ECB, PaddingMode::PKCS7);
EXPECT_EQ(8U, ciphertext.size());
EXPECT_NE(ciphertext, message);
- ++ciphertext[ciphertext.size() / 2];
AuthorizationSetBuilder begin_params;
begin_params.push_back(TAG_BLOCK_MODE, BlockMode::ECB);
begin_params.push_back(TAG_PADDING, PaddingMode::PKCS7);
- EXPECT_EQ(ErrorCode::OK, Begin(KeyPurpose::DECRYPT, begin_params));
- string plaintext;
- size_t input_consumed;
- EXPECT_EQ(ErrorCode::OK, Update(ciphertext, &plaintext, &input_consumed));
- EXPECT_EQ(ciphertext.size(), input_consumed);
- EXPECT_EQ(ErrorCode::INVALID_ARGUMENT, Finish(&plaintext));
+
+ for (size_t i = 0; i < kMaxPaddingCorruptionRetries; ++i) {
+ ++ciphertext[ciphertext.size() / 2];
+
+ EXPECT_EQ(ErrorCode::OK, Begin(KeyPurpose::DECRYPT, begin_params));
+ string plaintext;
+
+ size_t input_consumed;
+ EXPECT_EQ(ErrorCode::OK, Update(ciphertext, &plaintext, &input_consumed));
+ EXPECT_EQ(ciphertext.size(), input_consumed);
+ ErrorCode error = Finish(&plaintext);
+ if (error == ErrorCode::INVALID_ARGUMENT) {
+ // This is the expected error, we can exit the test now.
+ return;
+ } else {
+ // Very small chance we got valid decryption, so try again.
+ ASSERT_EQ(error, ErrorCode::OK);
+ }
+ }
+ FAIL() << "Corrupt ciphertext should have failed to decrypt by now.";
}
struct TripleDesTestVector {
@@ -4187,18 +4217,28 @@
string ciphertext = EncryptMessage(message, BlockMode::CBC, PaddingMode::PKCS7, &iv);
EXPECT_EQ(8U, ciphertext.size());
EXPECT_NE(ciphertext, message);
- ++ciphertext[ciphertext.size() / 2];
auto begin_params = AuthorizationSetBuilder()
.BlockMode(BlockMode::CBC)
.Padding(PaddingMode::PKCS7)
.Authorization(TAG_NONCE, iv);
- EXPECT_EQ(ErrorCode::OK, Begin(KeyPurpose::DECRYPT, begin_params));
- string plaintext;
- size_t input_consumed;
- EXPECT_EQ(ErrorCode::OK, Update(ciphertext, &plaintext, &input_consumed));
- EXPECT_EQ(ciphertext.size(), input_consumed);
- EXPECT_EQ(ErrorCode::INVALID_ARGUMENT, Finish(&plaintext));
+ for (size_t i = 0; i < kMaxPaddingCorruptionRetries; ++i) {
+ ++ciphertext[ciphertext.size() / 2];
+ EXPECT_EQ(ErrorCode::OK, Begin(KeyPurpose::DECRYPT, begin_params));
+ string plaintext;
+ size_t input_consumed;
+ EXPECT_EQ(ErrorCode::OK, Update(ciphertext, &plaintext, &input_consumed));
+ EXPECT_EQ(ciphertext.size(), input_consumed);
+ ErrorCode error = Finish(&plaintext);
+ if (error == ErrorCode::INVALID_ARGUMENT) {
+ // This is the expected error, we can exit the test now.
+ return;
+ } else {
+ // Very small chance we got valid decryption, so try again.
+ ASSERT_EQ(error, ErrorCode::OK);
+ }
+ }
+ FAIL() << "Corrupt ciphertext should have failed to decrypt by now.";
}
/*
diff --git a/security/keymint/aidl/vts/functional/KeyMintTest.cpp b/security/keymint/aidl/vts/functional/KeyMintTest.cpp
index 92aa2ac..53a5490 100644
--- a/security/keymint/aidl/vts/functional/KeyMintTest.cpp
+++ b/security/keymint/aidl/vts/functional/KeyMintTest.cpp
@@ -70,7 +70,7 @@
namespace {
// The maximum number of times we'll attempt to verify that corruption
-// of an ecrypted blob results in an error. Retries are necessary as there
+// of an encrypted blob results in an error. Retries are necessary as there
// is a small (roughly 1/256) chance that corrupting ciphertext still results
// in valid PKCS7 padding.
constexpr size_t kMaxPaddingCorruptionRetries = 8;