Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit b809329d authored by David Drysdale's avatar David Drysdale
Browse files

Fix AES corrupt padding test 

The AesEcbPkcs7PaddingCorrupted test has been incorrect since it was
originally introduced -- it was feeding the original message as input to
the decryption operation, rather than the corrupted ciphertext.  As a
result, the expected error code was also wrong -- INVALID_INPUT_LENGTH
is appropriate for a too-short cipher text (length 1 in this case),
whereas a corrupt-but-correct-length cipher text should give
INVALID_ARGUMENT.

Fix the test, and add a separate test to cover what was inadvertently
being tested before. Add a sentence to the HAL spec to describe what
expected and tested by CTS/VTS.

Bug: 194126736
Test: VtsAidlKeyMintTargetTest, VtsHalKeymasterV4_0TargetTest
Change-Id: Iaa5e42768814197f373797831093cf344d342b77
parent dc1a419b

keymaster/4.0/vts/functional/keymaster_hidl_hal_test.cpp

+2 −2
Original line number Diff line number Diff line
@@ -2866,8 +2866,8 @@ TEST_P(EncryptionOperationsTest, AesEcbPkcs7PaddingCorrupted) { 


        EXPECT_EQ(ErrorCode::OK, Begin(KeyPurpose::DECRYPT, params));

        string plaintext;

        ErrorCode error = Finish(message, &plaintext);

        if (error == ErrorCode::INVALID_INPUT_LENGTH) {

        ErrorCode error = Finish(ciphertext, &plaintext);

        if (error == ErrorCode::INVALID_ARGUMENT) {

            // This is the expected error, we can exit the test now.

            return;

        } else {

security/keymint/aidl/android/hardware/security/keymint/IKeyMintOperation.aidl

+2 −1
Original line number Diff line number Diff line
@@ -242,7 +242,8 @@ interface IKeyMintOperation { 
     *   not a multiple of the AES block size, finish() must return

     *   ErrorCode::INVALID_INPUT_LENGTH.  If padding is PaddingMode::PKCS7, pad the data per the

     *   PKCS#7 specification, including adding an additional padding block if the data is a

     *   multiple of the block length.

     *   multiple of the block length.  If padding is PaddingMode::PKCS7 and decryption does not

     *   result in valid padding, return ErrorCode::INVALID_ARGUMENT.

     *

     * o BlockMode::GCM.  During encryption, after processing all plaintext, compute the tag

     *   (Tag::MAC_LENGTH bytes) and append it to the returned ciphertext.  During decryption,

security/keymint/aidl/vts/functional/KeyMintTest.cpp

+30 −3
Original line number Diff line number Diff line
@@ -5481,18 +5481,45 @@ TEST_P(EncryptionOperationsTest, AesEcbPkcs7PaddingCorrupted) { 


        EXPECT_EQ(ErrorCode::OK, Begin(KeyPurpose::DECRYPT, params));

        string plaintext;

        ErrorCode error = Finish(message, &plaintext);

        if (error == ErrorCode::INVALID_INPUT_LENGTH) {

        ErrorCode error = Finish(ciphertext, &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);

            ASSERT_EQ(error, ErrorCode::OK)

                    << "Expected INVALID_ARGUMENT or (rarely) OK, got " << error;

        }

    }

    FAIL() << "Corrupt ciphertext should have failed to decrypt by now.";

}



/*

 * EncryptionOperationsTest.AesEcbPkcs7CiphertextTooShort

 *

 * Verifies that AES decryption fails in the correct way when the padding is corrupted.

 */

TEST_P(EncryptionOperationsTest, AesEcbPkcs7CiphertextTooShort) {

    ASSERT_EQ(ErrorCode::OK, GenerateKey(AuthorizationSetBuilder()

                                                 .Authorization(TAG_NO_AUTH_REQUIRED)

                                                 .AesEncryptionKey(128)

                                                 .Authorization(TAG_BLOCK_MODE, BlockMode::ECB)

                                                 .Padding(PaddingMode::PKCS7)));



    auto params = AuthorizationSetBuilder().BlockMode(BlockMode::ECB).Padding(PaddingMode::PKCS7);



    string message = "a";

    string ciphertext = EncryptMessage(message, params);

    EXPECT_EQ(16U, ciphertext.size());

    EXPECT_NE(ciphertext, message);



    // Shorten the ciphertext.

    ciphertext.resize(ciphertext.size() - 1);

    EXPECT_EQ(ErrorCode::OK, Begin(KeyPurpose::DECRYPT, params));

    string plaintext;

    EXPECT_EQ(ErrorCode::INVALID_INPUT_LENGTH, Finish(ciphertext, &plaintext));

}



vector<uint8_t> CopyIv(const AuthorizationSet& set) {

    auto iv = set.GetTagValue(TAG_NONCE);

    EXPECT_TRUE(iv);