[tarantool-patches] [PATCH 1/1] test: fix unit/crypto test flakiness

[tarantool-patches] [PATCH 1/1] test: fix unit/crypto test flakiness

[Vladislav Shpilevoy]

One of subtests was checking if crypto_decode returns an error
when fails to decode. But due to randomness of the test sometimes
it happened, that initial vector of encrypted data somehow didn't
lead to an error. Decryption was not correct, but only in terms
of result, not in terms of decryption algorithm. -1 was not
returned, and diag was not set.

This patch checks all the cases.

Closes #4306
---
Branch: https://github.com/tarantool/tarantool/tree/gerold103/gh-4306-flaky-unit-crypto
Issue: https://github.com/tarantool/tarantool/issues/4306

[CC += Georgy] because he is an author of crypto module.
[CC += Alexander] because he is an author of the issue, and revolves around
tests.

 test/unit/crypto.c      | 16 +++++++++++-----
 test/unit/crypto.result |  4 ++--
 2 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/test/unit/crypto.c b/test/unit/crypto.c
index fad1842f1..8dc4c7332 100644
--- a/test/unit/crypto.c
+++ b/test/unit/crypto.c
@@ -86,11 +86,17 @@ test_aes128_codec(void)
 	is(rc, plain_size, "decrypt returns correct number of bytes");
 	is(memcmp(buffer2, plain, plain_size), 0,
 	   "and correctly decrypts data");
-
-	rc = crypto_codec_decrypt(c, "false iv not meaning anything",
-				  buffer1, 16, buffer2, buffer_size);
-	is(rc, -1, "decrypt can fail with wrong IV");
-	ok(! diag_is_empty(diag_get()), "diag error is set");
+	/*
+	 * Create a different IV to ensure it does not decrypt a
+	 * message with the original IV.
+	 */
+	for (int i = 0; i < CRYPTO_AES_IV_SIZE; ++i)
+		iv[i]++;
+	rc = crypto_codec_decrypt(c, iv, buffer1, 16, buffer2, buffer_size);
+	ok(rc == -1 || rc != plain_size || memcmp(buffer1, buffer2, rc) != 0,
+	   "decrypt can't correctly decode anything with a wrong IV");
+	ok(rc != -1 || ! diag_is_empty(diag_get()),
+	   "in case decrypt has totally failed, diag is set");
 
 	crypto_codec_gen_iv(c, iv2, sizeof(iv2));
 	rc = crypto_codec_encrypt(c, iv2, plain, plain_size,
diff --git a/test/unit/crypto.result b/test/unit/crypto.result
index d6ff327e6..6e01896b1 100644
--- a/test/unit/crypto.result
+++ b/test/unit/crypto.result
@@ -15,8 +15,8 @@ ok 1 - crypto checks that algo argument is correct
     ok 10 - decrypt also checks length and returns needed number of bytes
     ok 11 - decrypt returns correct number of bytes
     ok 12 - and correctly decrypts data
-    ok 13 - decrypt can fail with wrong IV
-    ok 14 - diag error is set
+    ok 13 - decrypt can't correctly decode anything with a wrong IV
+    ok 14 - in case decrypt has totally failed, diag is set
     ok 15 - encrypt with different IV and the same number of written bytes returned
     ok 16 - the encrypted data looks different
     ok 17 - decrypt works with correct but another IV
-- 
2.20.1 (Apple Git-117)

[tarantool-patches] Re: [PATCH 1/1] test: fix unit/crypto test flakiness

[Alexander Turenko]

Looks good for me.

WBR, Alexander Turenko.

On Sun, Jun 23, 2019 at 08:17:30PM +0200, Vladislav Shpilevoy wrote:
> One of subtests was checking if crypto_decode returns an error
> when fails to decode. But due to randomness of the test sometimes
> it happened, that initial vector of encrypted data somehow didn't
> lead to an error. Decryption was not correct, but only in terms
> of result, not in terms of decryption algorithm. -1 was not
> returned, and diag was not set.
> 
> This patch checks all the cases.
> 
> Closes #4306
> ---
> Branch: https://github.com/tarantool/tarantool/tree/gerold103/gh-4306-flaky-unit-crypto
> Issue: https://github.com/tarantool/tarantool/issues/4306
> 
> [CC += Georgy] because he is an author of crypto module.
> [CC += Alexander] because he is an author of the issue, and revolves around
> tests.

[tarantool-patches] Re: [PATCH 1/1] test: fix unit/crypto test flakiness

[Vladislav Shpilevoy]

Pushed to the master.

On 23/06/2019 20:17, Vladislav Shpilevoy wrote:
> One of subtests was checking if crypto_decode returns an error
> when fails to decode. But due to randomness of the test sometimes
> it happened, that initial vector of encrypted data somehow didn't
> lead to an error. Decryption was not correct, but only in terms
> of result, not in terms of decryption algorithm. -1 was not
> returned, and diag was not set.
> 
> This patch checks all the cases.
> 
> Closes #4306
> ---
> Branch: https://github.com/tarantool/tarantool/tree/gerold103/gh-4306-flaky-unit-crypto
> Issue: https://github.com/tarantool/tarantool/issues/4306
> 
> [CC += Georgy] because he is an author of crypto module.
> [CC += Alexander] because he is an author of the issue, and revolves around
> tests.
> 
>  test/unit/crypto.c      | 16 +++++++++++-----
>  test/unit/crypto.result |  4 ++--
>  2 files changed, 13 insertions(+), 7 deletions(-)
> 
> diff --git a/test/unit/crypto.c b/test/unit/crypto.c
> index fad1842f1..8dc4c7332 100644
> --- a/test/unit/crypto.c
> +++ b/test/unit/crypto.c
> @@ -86,11 +86,17 @@ test_aes128_codec(void)
>  	is(rc, plain_size, "decrypt returns correct number of bytes");
>  	is(memcmp(buffer2, plain, plain_size), 0,
>  	   "and correctly decrypts data");
> -
> -	rc = crypto_codec_decrypt(c, "false iv not meaning anything",
> -				  buffer1, 16, buffer2, buffer_size);
> -	is(rc, -1, "decrypt can fail with wrong IV");
> -	ok(! diag_is_empty(diag_get()), "diag error is set");
> +	/*
> +	 * Create a different IV to ensure it does not decrypt a
> +	 * message with the original IV.
> +	 */
> +	for (int i = 0; i < CRYPTO_AES_IV_SIZE; ++i)
> +		iv[i]++;
> +	rc = crypto_codec_decrypt(c, iv, buffer1, 16, buffer2, buffer_size);
> +	ok(rc == -1 || rc != plain_size || memcmp(buffer1, buffer2, rc) != 0,
> +	   "decrypt can't correctly decode anything with a wrong IV");
> +	ok(rc != -1 || ! diag_is_empty(diag_get()),
> +	   "in case decrypt has totally failed, diag is set");
>  
>  	crypto_codec_gen_iv(c, iv2, sizeof(iv2));
>  	rc = crypto_codec_encrypt(c, iv2, plain, plain_size,
> diff --git a/test/unit/crypto.result b/test/unit/crypto.result
> index d6ff327e6..6e01896b1 100644
> --- a/test/unit/crypto.result
> +++ b/test/unit/crypto.result
> @@ -15,8 +15,8 @@ ok 1 - crypto checks that algo argument is correct
>      ok 10 - decrypt also checks length and returns needed number of bytes
>      ok 11 - decrypt returns correct number of bytes
>      ok 12 - and correctly decrypts data
> -    ok 13 - decrypt can fail with wrong IV
> -    ok 14 - diag error is set
> +    ok 13 - decrypt can't correctly decode anything with a wrong IV
> +    ok 14 - in case decrypt has totally failed, diag is set
>      ok 15 - encrypt with different IV and the same number of written bytes returned
>      ok 16 - the encrypted data looks different
>      ok 17 - decrypt works with correct but another IV
>