Merge pull request #6381 from tom-cosgrove-arm/pr2164
mbedtls: fix possible false success in mbedtls_cipher_check_tag()
diff --git a/ChangeLog.d/fix-possible-false-success-in-mbedtls_cipher_check_tag.txt b/ChangeLog.d/fix-possible-false-success-in-mbedtls_cipher_check_tag.txt
new file mode 100644
index 0000000..1f9e0aa
--- /dev/null
+++ b/ChangeLog.d/fix-possible-false-success-in-mbedtls_cipher_check_tag.txt
@@ -0,0 +1,5 @@
+Changes
+ * Calling AEAD tag-specific functions for non-AEAD algorithms (which
+ should not be done - they are documented for use only by AES-GCM and
+ ChaCha20+Poly1305) now returns MBEDTLS_ERR_CIPHER_FEATURE_UNAVAILABLE
+ instead of success (0).
diff --git a/library/cipher.c b/library/cipher.c
index dfb7329..dffe3ad 100644
--- a/library/cipher.c
+++ b/library/cipher.c
@@ -500,7 +500,7 @@
}
#endif
- return( 0 );
+ return( MBEDTLS_ERR_CIPHER_FEATURE_UNAVAILABLE );
}
#endif /* MBEDTLS_GCM_C || MBEDTLS_CHACHAPOLY_C */
@@ -1129,7 +1129,7 @@
}
#endif
- return( 0 );
+ return( MBEDTLS_ERR_CIPHER_FEATURE_UNAVAILABLE );
}
int mbedtls_cipher_check_tag( mbedtls_cipher_context_t *ctx,
@@ -1156,11 +1156,8 @@
}
#endif /* MBEDTLS_USE_PSA_CRYPTO */
- /* Status to return on a non-authenticated algorithm. It would make sense
- * to return MBEDTLS_ERR_CIPHER_INVALID_CONTEXT or perhaps
- * MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, but at the time I write this our
- * unit tests assume 0. */
- ret = 0;
+ /* Status to return on a non-authenticated algorithm. */
+ ret = MBEDTLS_ERR_CIPHER_FEATURE_UNAVAILABLE;
#if defined(MBEDTLS_GCM_C)
if( MBEDTLS_MODE_GCM == ctx->cipher_info->mode )
diff --git a/tests/suites/test_suite_cipher.function b/tests/suites/test_suite_cipher.function
index b7c3b51..ff936df 100644
--- a/tests/suites/test_suite_cipher.function
+++ b/tests/suites/test_suite_cipher.function
@@ -450,8 +450,12 @@
TEST_ASSERT( 0 == mbedtls_cipher_reset( &ctx_enc ) );
#if defined(MBEDTLS_GCM_C) || defined(MBEDTLS_CHACHAPOLY_C)
- TEST_ASSERT( 0 == mbedtls_cipher_update_ad( &ctx_dec, ad, sizeof( ad ) - i ) );
- TEST_ASSERT( 0 == mbedtls_cipher_update_ad( &ctx_enc, ad, sizeof( ad ) - i ) );
+ int expected = ( cipher_info->mode == MBEDTLS_MODE_GCM ||
+ cipher_info->type == MBEDTLS_CIPHER_CHACHA20_POLY1305 ) ?
+ 0 : MBEDTLS_ERR_CIPHER_FEATURE_UNAVAILABLE;
+
+ TEST_EQUAL( expected, mbedtls_cipher_update_ad( &ctx_dec, ad, sizeof(ad) - i ) );
+ TEST_EQUAL( expected, mbedtls_cipher_update_ad( &ctx_enc, ad, sizeof(ad) - i ) );
#endif
block_size = mbedtls_cipher_get_block_size( &ctx_enc );
@@ -470,7 +474,7 @@
total_len += outlen;
#if defined(MBEDTLS_GCM_C) || defined(MBEDTLS_CHACHAPOLY_C)
- TEST_ASSERT( 0 == mbedtls_cipher_write_tag( &ctx_enc, tag, sizeof( tag ) ) );
+ TEST_EQUAL( expected, mbedtls_cipher_write_tag( &ctx_enc, tag, sizeof(tag) ) );
#endif
TEST_ASSERT( total_len == length ||
@@ -491,7 +495,7 @@
total_len += outlen;
#if defined(MBEDTLS_GCM_C) || defined(MBEDTLS_CHACHAPOLY_C)
- TEST_ASSERT( 0 == mbedtls_cipher_check_tag( &ctx_dec, tag, sizeof( tag ) ) );
+ TEST_EQUAL( expected, mbedtls_cipher_check_tag( &ctx_dec, tag, sizeof(tag) ) );
#endif
/* check result */
@@ -547,7 +551,11 @@
TEST_ASSERT( 0 == mbedtls_cipher_set_iv( &ctx, iv, 16 ) );
TEST_ASSERT( 0 == mbedtls_cipher_reset( &ctx ) );
#if defined(MBEDTLS_GCM_C) || defined(MBEDTLS_CHACHAPOLY_C)
- TEST_ASSERT( 0 == mbedtls_cipher_update_ad( &ctx, NULL, 0 ) );
+ int expected = ( cipher_info->mode == MBEDTLS_MODE_GCM ||
+ cipher_info->type == MBEDTLS_CIPHER_CHACHA20_POLY1305 ) ?
+ 0 : MBEDTLS_ERR_CIPHER_FEATURE_UNAVAILABLE;
+
+ TEST_EQUAL( expected, mbedtls_cipher_update_ad( &ctx, NULL, 0 ) );
#endif
/* encode length number of bytes from inbuf */
@@ -609,7 +617,11 @@
TEST_ASSERT( 0 == mbedtls_cipher_reset( &ctx_dec ) );
#if defined(MBEDTLS_GCM_C) || defined(MBEDTLS_CHACHAPOLY_C)
- TEST_ASSERT( 0 == mbedtls_cipher_update_ad( &ctx_dec, NULL, 0 ) );
+ int expected = ( cipher_info->mode == MBEDTLS_MODE_GCM ||
+ cipher_info->type == MBEDTLS_CIPHER_CHACHA20_POLY1305 ) ?
+ 0 : MBEDTLS_ERR_CIPHER_FEATURE_UNAVAILABLE;
+
+ TEST_EQUAL( expected, mbedtls_cipher_update_ad( &ctx_dec, NULL, 0 ) );
#endif
/* decode 0-byte string */
@@ -710,8 +722,12 @@
TEST_ASSERT( 0 == mbedtls_cipher_reset( &ctx_enc ) );
#if defined(MBEDTLS_GCM_C) || defined(MBEDTLS_CHACHAPOLY_C)
- TEST_ASSERT( 0 == mbedtls_cipher_update_ad( &ctx_dec, NULL, 0 ) );
- TEST_ASSERT( 0 == mbedtls_cipher_update_ad( &ctx_enc, NULL, 0 ) );
+ int expected = ( cipher_info->mode == MBEDTLS_MODE_GCM ||
+ cipher_info->type == MBEDTLS_CIPHER_CHACHA20_POLY1305 ) ?
+ 0 : MBEDTLS_ERR_CIPHER_FEATURE_UNAVAILABLE;
+
+ TEST_EQUAL( expected, mbedtls_cipher_update_ad( &ctx_dec, NULL, 0 ) );
+ TEST_EQUAL( expected, mbedtls_cipher_update_ad( &ctx_enc, NULL, 0 ) );
#endif
block_size = mbedtls_cipher_get_block_size( &ctx_enc );
@@ -795,7 +811,11 @@
TEST_ASSERT( 0 == mbedtls_cipher_set_iv( &ctx, iv->x, iv->len ) );
TEST_ASSERT( 0 == mbedtls_cipher_reset( &ctx ) );
#if defined(MBEDTLS_GCM_C) || defined(MBEDTLS_CHACHAPOLY_C)
- TEST_ASSERT( 0 == mbedtls_cipher_update_ad( &ctx, ad->x, ad->len ) );
+ int expected = ( ctx.cipher_info->mode == MBEDTLS_MODE_GCM ||
+ ctx.cipher_info->type == MBEDTLS_CIPHER_CHACHA20_POLY1305 ) ?
+ 0 : MBEDTLS_ERR_CIPHER_FEATURE_UNAVAILABLE;
+
+ TEST_EQUAL( expected, mbedtls_cipher_update_ad( &ctx, ad->x, ad->len ) );
#endif
/* decode buffer and check tag->x */
@@ -806,7 +826,11 @@
&outlen ) );
total_len += outlen;
#if defined(MBEDTLS_GCM_C) || defined(MBEDTLS_CHACHAPOLY_C)
- TEST_ASSERT( tag_result == mbedtls_cipher_check_tag( &ctx, tag->x, tag->len ) );
+ int tag_expected = ( ctx.cipher_info->mode == MBEDTLS_MODE_GCM ||
+ ctx.cipher_info->type == MBEDTLS_CIPHER_CHACHA20_POLY1305 ) ?
+ tag_result : MBEDTLS_ERR_CIPHER_FEATURE_UNAVAILABLE;
+
+ TEST_EQUAL( tag_expected, mbedtls_cipher_check_tag( &ctx, tag->x, tag->len ) );
#endif
/* check plaintext only if everything went fine */