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 000000000..1f9e0aa35 --- /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 dfb732993..dffe3adca 100644 --- a/library/cipher.c +++ b/library/cipher.c @@ -500,7 +500,7 @@ int mbedtls_cipher_update_ad( mbedtls_cipher_context_t *ctx, } #endif - return( 0 ); + return( MBEDTLS_ERR_CIPHER_FEATURE_UNAVAILABLE ); } #endif /* MBEDTLS_GCM_C || MBEDTLS_CHACHAPOLY_C */ @@ -1129,7 +1129,7 @@ int mbedtls_cipher_write_tag( mbedtls_cipher_context_t *ctx, } #endif - return( 0 ); + return( MBEDTLS_ERR_CIPHER_FEATURE_UNAVAILABLE ); } int mbedtls_cipher_check_tag( mbedtls_cipher_context_t *ctx, @@ -1156,11 +1156,8 @@ int mbedtls_cipher_check_tag( mbedtls_cipher_context_t *ctx, } #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 b7c3b5144..ff936dfba 100644 --- a/tests/suites/test_suite_cipher.function +++ b/tests/suites/test_suite_cipher.function @@ -450,8 +450,12 @@ void enc_dec_buf( int cipher_id, char * cipher_string, int key_len, 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 @@ void enc_dec_buf( int cipher_id, char * cipher_string, int key_len, 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 @@ void enc_dec_buf( int cipher_id, char * cipher_string, int key_len, 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 @@ void enc_fail( int cipher_id, int pad_mode, int key_len, int length_val, 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 @@ void dec_empty_buf( int cipher, 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 @@ void enc_dec_buf_multipart( int cipher_id, int key_len, int first_length_val, 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 @@ void decrypt_test_vec( int cipher_id, int pad_mode, data_t * key, 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 @@ void decrypt_test_vec( int cipher_id, int pad_mode, data_t * key, &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 */