From 5a7be1041936e9dbdea16eff5051fafbcfde021b Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 23 Jun 2021 21:51:32 +0200 Subject: [PATCH] Add output_length parameter to mbedtls_gcm_finish Without this parameter, it would be hard for callers to know how many bytes of output the function wrote into the output buffer. It would be possible, since the cumulated output must have the same length as the cumulated input, but it would be cumbersome for the caller to keep track. Signed-off-by: Gilles Peskine --- include/mbedtls/gcm.h | 5 +++++ library/cipher.c | 13 +++++++++++-- library/gcm.c | 8 +++++--- tests/suites/test_suite_gcm.function | 14 ++++++++++---- 4 files changed, 31 insertions(+), 9 deletions(-) diff --git a/include/mbedtls/gcm.h b/include/mbedtls/gcm.h index f3c30350d..06b06b48c 100644 --- a/include/mbedtls/gcm.h +++ b/include/mbedtls/gcm.h @@ -339,6 +339,10 @@ int mbedtls_gcm_update( mbedtls_gcm_context *ctx, * then mbedtls_gcm_finish() never produces any output, * so \p output_size can be \c 0. * - \p output_size never needs to be more than \c 15. + * \param output_length On success, \p *output_length contains the actual + * length of the output written in \p output. + * On failure, the content of \p *output_length is + * unspecified. * * \return \c 0 on success. * \return #MBEDTLS_ERR_GCM_BAD_INPUT on failure: @@ -347,6 +351,7 @@ int mbedtls_gcm_update( mbedtls_gcm_context *ctx, */ int mbedtls_gcm_finish( mbedtls_gcm_context *ctx, unsigned char *output, size_t output_size, + size_t *output_length, unsigned char *tag, size_t tag_len ); /** diff --git a/library/cipher.c b/library/cipher.c index 4f56b5288..546cace55 100644 --- a/library/cipher.c +++ b/library/cipher.c @@ -1109,9 +1109,14 @@ int mbedtls_cipher_write_tag( mbedtls_cipher_context_t *ctx, #if defined(MBEDTLS_GCM_C) if( MBEDTLS_MODE_GCM == ctx->cipher_info->mode ) + { + size_t output_length; + /* The code here doesn't yet support alternative implementations + * that can delay up to a block of output. */ return( mbedtls_gcm_finish( (mbedtls_gcm_context *) ctx->cipher_ctx, - NULL, 0, + NULL, 0, &output_length, tag, tag_len ) ); + } #endif #if defined(MBEDTLS_CHACHAPOLY_C) @@ -1158,12 +1163,16 @@ int mbedtls_cipher_check_tag( mbedtls_cipher_context_t *ctx, #if defined(MBEDTLS_GCM_C) if( MBEDTLS_MODE_GCM == ctx->cipher_info->mode ) { + size_t output_length; + /* The code here doesn't yet support alternative implementations + * that can delay up to a block of output. */ + if( tag_len > sizeof( check_tag ) ) return( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); if( 0 != ( ret = mbedtls_gcm_finish( (mbedtls_gcm_context *) ctx->cipher_ctx, - NULL, 0, + NULL, 0, &output_length, check_tag, tag_len ) ) ) { return( ret ); diff --git a/library/gcm.c b/library/gcm.c index 8fa4ee779..835b1b285 100644 --- a/library/gcm.c +++ b/library/gcm.c @@ -532,6 +532,7 @@ int mbedtls_gcm_update( mbedtls_gcm_context *ctx, int mbedtls_gcm_finish( mbedtls_gcm_context *ctx, unsigned char *output, size_t output_size, + size_t *output_length, unsigned char *tag, size_t tag_len ) { unsigned char work_buf[16]; @@ -546,6 +547,7 @@ int mbedtls_gcm_finish( mbedtls_gcm_context *ctx, * for the sake of alternative implementations. */ (void) output; (void) output_size; + *output_length = 0; orig_len = ctx->len * 8; orig_add_len = ctx->add_len * 8; @@ -616,7 +618,7 @@ int mbedtls_gcm_crypt_and_tag( mbedtls_gcm_context *ctx, output, length, &olen ) ) != 0 ) return( ret ); - if( ( ret = mbedtls_gcm_finish( ctx, NULL, 0, tag, tag_len ) ) != 0 ) + if( ( ret = mbedtls_gcm_finish( ctx, NULL, 0, &olen, tag, tag_len ) ) != 0 ) return( ret ); return( 0 ); @@ -1068,7 +1070,7 @@ int mbedtls_gcm_self_test( int verbose ) goto exit; } - ret = mbedtls_gcm_finish( &ctx, NULL, 0, tag_buf, 16 ); + ret = mbedtls_gcm_finish( &ctx, NULL, 0, &olen, tag_buf, 16 ); if( ret != 0 ) goto exit; @@ -1140,7 +1142,7 @@ int mbedtls_gcm_self_test( int verbose ) goto exit; } - ret = mbedtls_gcm_finish( &ctx, NULL, 0, tag_buf, 16 ); + ret = mbedtls_gcm_finish( &ctx, NULL, 0, &olen, tag_buf, 16 ); if( ret != 0 ) goto exit; diff --git a/tests/suites/test_suite_gcm.function b/tests/suites/test_suite_gcm.function index 49859dda9..c530e6b42 100644 --- a/tests/suites/test_suite_gcm.function +++ b/tests/suites/test_suite_gcm.function @@ -50,7 +50,8 @@ static int check_multipart( mbedtls_gcm_context *ctx, output = NULL; ASSERT_ALLOC( output, tag->len ); - TEST_EQUAL( 0, mbedtls_gcm_finish( ctx, NULL, 0, output, tag->len ) ); + TEST_EQUAL( 0, mbedtls_gcm_finish( ctx, NULL, 0, &olen, output, tag->len ) ); + TEST_EQUAL( 0, olen ); ASSERT_COMPARE( output, tag->len, tag->x, tag->len ); mbedtls_free( output ); output = NULL; @@ -96,7 +97,8 @@ static void check_cipher_with_empty_ad( mbedtls_gcm_context *ctx, output = NULL; ASSERT_ALLOC( output, tag->len ); - TEST_EQUAL( 0, mbedtls_gcm_finish( ctx, NULL, 0, output, tag->len ) ); + TEST_EQUAL( 0, mbedtls_gcm_finish( ctx, NULL, 0, &olen, output, tag->len ) ); + TEST_EQUAL( 0, olen ); ASSERT_COMPARE( output, tag->len, tag->x, tag->len ); exit: @@ -125,7 +127,9 @@ static void check_empty_cipher_with_ad( mbedtls_gcm_context *ctx, } ASSERT_ALLOC( output_tag, tag->len ); - TEST_EQUAL( 0, mbedtls_gcm_finish( ctx, NULL, 0, output_tag, tag->len ) ); + TEST_EQUAL( 0, mbedtls_gcm_finish( ctx, NULL, 0, &olen, + output_tag, tag->len ) ); + TEST_EQUAL( 0, olen ); ASSERT_COMPARE( output_tag, tag->len, tag->x, tag->len ); exit: @@ -138,11 +142,13 @@ static void check_no_cipher_no_ad( mbedtls_gcm_context *ctx, const data_t *tag ) { uint8_t *output = NULL; + size_t olen = 0; TEST_EQUAL( 0, mbedtls_gcm_starts( ctx, mode, iv->x, iv->len ) ); ASSERT_ALLOC( output, tag->len ); - TEST_EQUAL( 0, mbedtls_gcm_finish( ctx, NULL, 0, output, tag->len ) ); + TEST_EQUAL( 0, mbedtls_gcm_finish( ctx, NULL, 0, &olen, output, tag->len ) ); + TEST_EQUAL( 0, olen ); ASSERT_COMPARE( output, tag->len, tag->x, tag->len ); exit: