Merge "Fix flaky corrupted padding tests" am: a33f46bc2a

Original change: https://android-review.googlesource.com/c/platform/hardware/interfaces/+/1903312

Change-Id: I5e5b1e62c017e2e1db8ce099e1cabda6501ce44c

1 files changed, 58 insertions, 18 deletions

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 93fb19d23d..2449268943 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 @@ bool operator==(const KeyCharacteristics& a, const KeyCharacteristics& b) {
 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) {
@@ -2853,11 +2859,22 @@ TEST_P(EncryptionOperationsTest, AesEcbPkcs7PaddingCorrupted) {
     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) {
@@ -3880,17 +3897,30 @@ TEST_P(EncryptionOperationsTest, TripleDesEcbPkcs7PaddingCorrupted) {
     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 {
@@ -4191,18 +4221,28 @@ TEST_P(EncryptionOperationsTest, TripleDesCbcPkcs7PaddingCorrupted) {
     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.";
 }
 
 /*