From 36dd93e7453c2c185c5b95fb46cac152142476e4 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 13 Apr 2021 13:02:03 +0200 Subject: [PATCH 01/15] Test the multipart GCM interface The existing GCM test suite only exercises the one-shot API. Also test the multipart interface: systematically run it on the same test data, with the input (plaintext or ciphertext) split in two parts. Given the current limitations of the GCM API, the associated data is always passed in a single shot to mbedtls_gcm_starts(), and the first part of the input is a nonzero multiple of 16. Signed-off-by: Gilles Peskine --- tests/suites/test_suite_gcm.function | 83 +++++++++++++++++++++++++--- 1 file changed, 76 insertions(+), 7 deletions(-) diff --git a/tests/suites/test_suite_gcm.function b/tests/suites/test_suite_gcm.function index 9b7b0ee14..c2f353496 100644 --- a/tests/suites/test_suite_gcm.function +++ b/tests/suites/test_suite_gcm.function @@ -1,5 +1,57 @@ /* BEGIN_HEADER */ #include "mbedtls/gcm.h" + +/* Use the multipart interface to process the encrypted data in two parts + * and check that the output matches the expected output. + * The context must have been set up with the key. */ +static int check_multipart( mbedtls_gcm_context *ctx, + int mode, + const data_t *iv, + const data_t *add, + const data_t *input, + const data_t *expected_output, + const data_t *tag, + size_t n1 ) +{ + int ok = 0; + uint8_t *output = NULL; + size_t n2 = input->len - n1; + + /* Sanity checks on the test data */ + TEST_ASSERT( n1 <= input->len ); + TEST_EQUAL( input->len, expected_output->len ); + + TEST_EQUAL( 0, mbedtls_gcm_starts( ctx, mode, + iv->x, iv->len, + add->x, add->len ) ); + + /* Allocate a tight buffer for each update call. This way, if the function + * tries to write beyond the advertised required buffer size, this will + * count as an overflow for memory sanitizers and static checkers. */ + ASSERT_ALLOC( output, n1 ); + TEST_EQUAL( 0, mbedtls_gcm_update( ctx, n1, input->x, output ) ); + ASSERT_COMPARE( output, n1, expected_output->x, n1 ); + mbedtls_free( output ); + output = NULL; + + ASSERT_ALLOC( output, n2 ); + TEST_EQUAL( 0, mbedtls_gcm_update( ctx, n2, input->x + n1, output ) ); + ASSERT_COMPARE( output, n2, expected_output->x + n1, n2 ); + mbedtls_free( output ); + output = NULL; + + ASSERT_ALLOC( output, tag->len ); + TEST_EQUAL( 0, mbedtls_gcm_finish( ctx, output, tag->len ) ); + ASSERT_COMPARE( output, tag->len, tag->x, tag->len ); + mbedtls_free( output ); + output = NULL; + + ok = 1; +exit: + mbedtls_free( output ); + return( ok ); +} + /* END_HEADER */ /* BEGIN_DEPENDENCIES @@ -43,6 +95,7 @@ void gcm_encrypt_and_tag( int cipher_id, data_t * key_str, unsigned char tag_output[16]; mbedtls_gcm_context ctx; size_t tag_len = tag_len_bits / 8; + size_t n1; mbedtls_gcm_init( &ctx ); @@ -55,10 +108,18 @@ void gcm_encrypt_and_tag( int cipher_id, data_t * key_str, { TEST_ASSERT( mbedtls_gcm_crypt_and_tag( &ctx, MBEDTLS_GCM_ENCRYPT, src_str->len, iv_str->x, iv_str->len, add_str->x, add_str->len, src_str->x, output, tag_len, tag_output ) == 0 ); - TEST_ASSERT( mbedtls_test_hexcmp( output, dst->x, - src_str->len, dst->len ) == 0 ); - TEST_ASSERT( mbedtls_test_hexcmp( tag_output, tag->x, - tag_len, tag->len ) == 0 ); + ASSERT_COMPARE( output, src_str->len, dst->x, dst->len ); + ASSERT_COMPARE( tag_output, tag_len, tag->x, tag->len ); + + for( n1 = 16; n1 < src_str->len; n1 += 16 ) + { + mbedtls_test_set_step( n1 ); + if( !check_multipart( &ctx, MBEDTLS_GCM_ENCRYPT, + iv_str, add_str, src_str, + dst, tag, + n1 ) ) + goto exit; + } } exit: @@ -77,6 +138,7 @@ void gcm_decrypt_and_verify( int cipher_id, data_t * key_str, mbedtls_gcm_context ctx; int ret; size_t tag_len = tag_len_bits / 8; + size_t n1; mbedtls_gcm_init( &ctx ); @@ -95,10 +157,17 @@ void gcm_decrypt_and_verify( int cipher_id, data_t * key_str, else { TEST_ASSERT( ret == 0 ); + ASSERT_COMPARE( output, src_str->len, pt_result->x, pt_result->len ); - TEST_ASSERT( mbedtls_test_hexcmp( output, pt_result->x, - src_str->len, - pt_result->len ) == 0 ); + for( n1 = 16; n1 < src_str->len; n1 += 16 ) + { + mbedtls_test_set_step( n1 ); + if( !check_multipart( &ctx, MBEDTLS_GCM_DECRYPT, + iv_str, add_str, src_str, + pt_result, tag_str, + n1 ) ) + goto exit; + } } } From 58fc272af9359a171e6a9549ab6385feedc9a2dd Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 13 Apr 2021 15:58:27 +0200 Subject: [PATCH 02/15] Remove alignment requirement for mbedtls_gcm_update: implementation mbedtls_gcm_update now accepts inputs of arbitrary size. There is no longer a requirement that all calls except the last one pass a multiple of 16 bytes. This commit updates the library code and adjusts the GCM tests to exercise arbitrarily aligned input sizes. Signed-off-by: Gilles Peskine --- library/gcm.c | 113 ++++++++++++++++++++------- tests/suites/test_suite_gcm.function | 4 +- 2 files changed, 86 insertions(+), 31 deletions(-) diff --git a/library/gcm.c b/library/gcm.c index 300521ecd..b3105d80d 100644 --- a/library/gcm.c +++ b/library/gcm.c @@ -355,21 +355,64 @@ int mbedtls_gcm_starts( mbedtls_gcm_context *ctx, return( 0 ); } +/* Increment the counter. */ +static void gcm_incr( unsigned char y[16] ) +{ + size_t i; + for( i = 16; i > 12; i-- ) + if( ++y[i - 1] != 0 ) + break; +} + +/* Calculate and apply the encryption mask. Process use_len bytes of data, + * starting at position offset in the mask block. */ +static int gcm_mask( mbedtls_gcm_context *ctx, + unsigned char ectr[16], + size_t offset, size_t use_len, + const unsigned char *input, + unsigned char *output ) +{ + size_t i; + size_t olen = 0; + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; + + if( ( ret = mbedtls_cipher_update( &ctx->cipher_ctx, ctx->y, 16, ectr, + &olen ) ) != 0 ) + { + mbedtls_platform_zeroize( ectr, 16 ); + return( ret ); + } + + for( i = 0; i < use_len; i++ ) + { + if( ctx->mode == MBEDTLS_GCM_DECRYPT ) + ctx->buf[offset + i] ^= input[i]; + output[i] = ectr[offset + i] ^ input[i]; + if( ctx->mode == MBEDTLS_GCM_ENCRYPT ) + ctx->buf[offset + i] ^= output[i]; + } + return( 0 ); +} + int mbedtls_gcm_update( mbedtls_gcm_context *ctx, size_t length, const unsigned char *input, unsigned char *output ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; - unsigned char ectr[16]; - size_t i; - const unsigned char *p; + const unsigned char *p = input; unsigned char *out_p = output; - size_t use_len, olen = 0; + size_t offset; + unsigned char ectr[16]; + + /* Exit early if length==0 so that we don't do any pointer arithmetic on + * a potentially null pointer. */ + if( length == 0 ) + return( 0 ); GCM_VALIDATE_RET( ctx != NULL ); - GCM_VALIDATE_RET( length == 0 || input != NULL ); - GCM_VALIDATE_RET( length == 0 || output != NULL ); + GCM_VALIDATE_RET( input != NULL ); + GCM_VALIDATE_RET( output != NULL ); if( output > input && (size_t) ( output - input ) < length ) return( MBEDTLS_ERR_GCM_BAD_INPUT ); @@ -382,39 +425,48 @@ int mbedtls_gcm_update( mbedtls_gcm_context *ctx, return( MBEDTLS_ERR_GCM_BAD_INPUT ); } - ctx->len += length; - - p = input; - while( length > 0 ) + offset = ctx->len % 16; + if( offset != 0 ) { - use_len = ( length < 16 ) ? length : 16; + size_t use_len = 16 - offset; + if( use_len > length ) + use_len = length; - for( i = 16; i > 12; i-- ) - if( ++ctx->y[i - 1] != 0 ) - break; - - if( ( ret = mbedtls_cipher_update( &ctx->cipher_ctx, ctx->y, 16, ectr, - &olen ) ) != 0 ) - { + if( ( ret = gcm_mask( ctx, ectr, offset, use_len, p, out_p ) ) != 0 ) return( ret ); - } - for( i = 0; i < use_len; i++ ) - { - if( ctx->mode == MBEDTLS_GCM_DECRYPT ) - ctx->buf[i] ^= p[i]; - out_p[i] = ectr[i] ^ p[i]; - if( ctx->mode == MBEDTLS_GCM_ENCRYPT ) - ctx->buf[i] ^= out_p[i]; - } - - gcm_mult( ctx, ctx->buf, ctx->buf ); + if( offset + use_len == 16 ) + gcm_mult( ctx, ctx->buf, ctx->buf ); + ctx->len += use_len; length -= use_len; p += use_len; out_p += use_len; } + ctx->len += length; + + while( length >= 16 ) + { + gcm_incr( ctx->y ); + if( ( ret = gcm_mask( ctx, ectr, 0, 16, p, out_p ) ) != 0 ) + return( ret ); + + gcm_mult( ctx, ctx->buf, ctx->buf ); + + length -= 16; + p += 16; + out_p += 16; + } + + if( length > 0 ) + { + gcm_incr( ctx->y ); + if( ( ret = gcm_mask( ctx, ectr, 0, length, p, out_p ) ) != 0 ) + return( ret ); + } + + mbedtls_platform_zeroize( ectr, sizeof( ectr ) ); return( 0 ); } @@ -436,6 +488,9 @@ int mbedtls_gcm_finish( mbedtls_gcm_context *ctx, if( tag_len > 16 || tag_len < 4 ) return( MBEDTLS_ERR_GCM_BAD_INPUT ); + if( ctx->len % 16 != 0 ) + gcm_mult( ctx, ctx->buf, ctx->buf ); + memcpy( tag, ctx->base_ectr, tag_len ); if( orig_len || orig_add_len ) diff --git a/tests/suites/test_suite_gcm.function b/tests/suites/test_suite_gcm.function index c2f353496..d4dce9398 100644 --- a/tests/suites/test_suite_gcm.function +++ b/tests/suites/test_suite_gcm.function @@ -111,7 +111,7 @@ void gcm_encrypt_and_tag( int cipher_id, data_t * key_str, ASSERT_COMPARE( output, src_str->len, dst->x, dst->len ); ASSERT_COMPARE( tag_output, tag_len, tag->x, tag->len ); - for( n1 = 16; n1 < src_str->len; n1 += 16 ) + for( n1 = 0; n1 <= src_str->len; n1 += 1 ) { mbedtls_test_set_step( n1 ); if( !check_multipart( &ctx, MBEDTLS_GCM_ENCRYPT, @@ -159,7 +159,7 @@ void gcm_decrypt_and_verify( int cipher_id, data_t * key_str, TEST_ASSERT( ret == 0 ); ASSERT_COMPARE( output, src_str->len, pt_result->x, pt_result->len ); - for( n1 = 16; n1 < src_str->len; n1 += 16 ) + for( n1 = 0; n1 <= src_str->len; n1 += 1 ) { mbedtls_test_set_step( n1 ); if( !check_multipart( &ctx, MBEDTLS_GCM_DECRYPT, From 441907ec30a837e20bdd4cd7d6f4ec9e0b3928f0 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 13 Apr 2021 16:09:32 +0200 Subject: [PATCH 03/15] Remove alignment requirement for mbedtls_gcm_update: documentation Signed-off-by: Gilles Peskine --- ChangeLog.d/gcm-update.txt | 4 ++++ include/mbedtls/cipher.h | 5 ----- include/mbedtls/gcm.h | 7 +------ 3 files changed, 5 insertions(+), 11 deletions(-) create mode 100644 ChangeLog.d/gcm-update.txt diff --git a/ChangeLog.d/gcm-update.txt b/ChangeLog.d/gcm-update.txt new file mode 100644 index 000000000..4511fec32 --- /dev/null +++ b/ChangeLog.d/gcm-update.txt @@ -0,0 +1,4 @@ +Features + * The multi-part GCM interface (mbedtls_gcm_update() or + mbedtls_cipher_update()) no longer requires the size of partial inputs to + be a multiple of 16. diff --git a/include/mbedtls/cipher.h b/include/mbedtls/cipher.h index bfc911fc1..eaa49b1ff 100644 --- a/include/mbedtls/cipher.h +++ b/include/mbedtls/cipher.h @@ -727,11 +727,6 @@ int mbedtls_cipher_update_ad( mbedtls_cipher_context_t *ctx, * Exception: For MBEDTLS_MODE_ECB, expects a single block * in size. For example, 16 Bytes for AES. * - * \note If the underlying cipher is used in GCM mode, all calls - * to this function, except for the last one before - * mbedtls_cipher_finish(), must have \p ilen as a - * multiple of the block size of the cipher. - * * \param ctx The generic cipher context. This must be initialized and * bound to a key. * \param input The buffer holding the input data. This must be a diff --git a/include/mbedtls/gcm.h b/include/mbedtls/gcm.h index 6b673616f..8a26ebb96 100644 --- a/include/mbedtls/gcm.h +++ b/include/mbedtls/gcm.h @@ -249,17 +249,12 @@ int mbedtls_gcm_starts( mbedtls_gcm_context *ctx, * \brief This function feeds an input buffer into an ongoing GCM * encryption or decryption operation. * - * ` The function expects input to be a multiple of 16 - * Bytes. Only the last call before calling - * mbedtls_gcm_finish() can be less than 16 Bytes. - * * \note For decryption, the output buffer cannot be the same as * input buffer. If the buffers overlap, the output buffer * must trail at least 8 Bytes behind the input buffer. * * \param ctx The GCM context. This must be initialized. - * \param length The length of the input data. This must be a multiple of - * 16 except in the last call before mbedtls_gcm_finish(). + * \param length The length of the input data. * \param input The buffer holding the input data. If \p length is greater * than zero, this must be a readable buffer of at least that * size in Bytes. From 9461e45a17b0172b96f33567a09d051f77da3a7b Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 15 Apr 2021 16:48:32 +0200 Subject: [PATCH 04/15] Add output parameter to mbedtls_gcm_finish Alternative implementations of GCM may delay the output of partial blocks from mbedtls_gcm_update(). Add an output parameter to mbedtls_gcm_finish() to allow such implementations to pass the final partial block back to the caller. With the software implementation, this final output is always empty. Signed-off-by: Gilles Peskine --- ChangeLog.d/gcm-update.txt | 10 ++++++++++ include/mbedtls/gcm.h | 14 ++++++++++++-- library/cipher.c | 2 ++ library/gcm.c | 15 ++++++++++----- tests/suites/test_suite_gcm.function | 6 +++--- 5 files changed, 37 insertions(+), 10 deletions(-) diff --git a/ChangeLog.d/gcm-update.txt b/ChangeLog.d/gcm-update.txt index 4511fec32..d38455102 100644 --- a/ChangeLog.d/gcm-update.txt +++ b/ChangeLog.d/gcm-update.txt @@ -1,3 +1,13 @@ +API changes + * The interface of the GCM module has changed to remove restrictions on + how the input to multipart operations is broken down. mbedtls_gcm_finish() + now takes an extra output parameter for the last partial output block. + The software implementation always produces the full output at each + call to mbedtls_gcm_update(), but alternative implementations activated + by MBEDTLS_GCM_ALT may delay partial blocks to the next call to + mbedtls_gcm_update() or mbedtls_gcm_finish(). + These changes are backward compatible for users of the cipher API. + Features * The multi-part GCM interface (mbedtls_gcm_update() or mbedtls_cipher_update()) no longer requires the size of partial inputs to diff --git a/include/mbedtls/gcm.h b/include/mbedtls/gcm.h index 8a26ebb96..951ee00c4 100644 --- a/include/mbedtls/gcm.h +++ b/include/mbedtls/gcm.h @@ -282,13 +282,23 @@ int mbedtls_gcm_update( mbedtls_gcm_context *ctx, * buffer of at least \p tag_len Bytes. * \param tag_len The length of the tag to generate. This must be at least * four. + * \param output The buffer for the final output. + * This must be a writable buffer of at least \p output_len + * bytes. + * With the built-in implementation, there is no final + * output and this can be \p NULL. + * Alternative implementations may return a partial block + * of output. + * \param output_len The size of the \p output buffer in bytes. + * With the built-in implementation, this can be \c 0. + * Alternative implementations may require a 15-byte buffer. * * \return \c 0 on success. * \return #MBEDTLS_ERR_GCM_BAD_INPUT on failure. */ int mbedtls_gcm_finish( mbedtls_gcm_context *ctx, - unsigned char *tag, - size_t tag_len ); + unsigned char *output, size_t output_len, + unsigned char *tag, size_t tag_len ); /** * \brief This function clears a GCM context and the underlying diff --git a/library/cipher.c b/library/cipher.c index c88d6666d..63eaba88f 100644 --- a/library/cipher.c +++ b/library/cipher.c @@ -1101,6 +1101,7 @@ int mbedtls_cipher_write_tag( mbedtls_cipher_context_t *ctx, #if defined(MBEDTLS_GCM_C) if( MBEDTLS_MODE_GCM == ctx->cipher_info->mode ) return( mbedtls_gcm_finish( (mbedtls_gcm_context *) ctx->cipher_ctx, + NULL, 0, tag, tag_len ) ); #endif @@ -1153,6 +1154,7 @@ int mbedtls_cipher_check_tag( mbedtls_cipher_context_t *ctx, if( 0 != ( ret = mbedtls_gcm_finish( (mbedtls_gcm_context *) ctx->cipher_ctx, + NULL, 0, check_tag, tag_len ) ) ) { return( ret ); diff --git a/library/gcm.c b/library/gcm.c index b3105d80d..de766bc76 100644 --- a/library/gcm.c +++ b/library/gcm.c @@ -471,8 +471,8 @@ int mbedtls_gcm_update( mbedtls_gcm_context *ctx, } int mbedtls_gcm_finish( mbedtls_gcm_context *ctx, - unsigned char *tag, - size_t tag_len ) + unsigned char *output, size_t output_len, + unsigned char *tag, size_t tag_len ) { unsigned char work_buf[16]; size_t i; @@ -482,6 +482,11 @@ int mbedtls_gcm_finish( mbedtls_gcm_context *ctx, GCM_VALIDATE_RET( ctx != NULL ); GCM_VALIDATE_RET( tag != NULL ); + /* We never pass any output in finish(). The output parameter exists only + * for the sake of alternative implementations. */ + (void) output; + (void) output_len; + orig_len = ctx->len * 8; orig_add_len = ctx->add_len * 8; @@ -541,7 +546,7 @@ int mbedtls_gcm_crypt_and_tag( mbedtls_gcm_context *ctx, if( ( ret = mbedtls_gcm_update( ctx, length, input, output ) ) != 0 ) return( ret ); - if( ( ret = mbedtls_gcm_finish( ctx, tag, tag_len ) ) != 0 ) + if( ( ret = mbedtls_gcm_finish( ctx, NULL, 0, tag, tag_len ) ) != 0 ) return( ret ); return( 0 ); @@ -979,7 +984,7 @@ int mbedtls_gcm_self_test( int verbose ) goto exit; } - ret = mbedtls_gcm_finish( &ctx, tag_buf, 16 ); + ret = mbedtls_gcm_finish( &ctx, NULL, 0, tag_buf, 16 ); if( ret != 0 ) goto exit; @@ -1039,7 +1044,7 @@ int mbedtls_gcm_self_test( int verbose ) goto exit; } - ret = mbedtls_gcm_finish( &ctx, tag_buf, 16 ); + ret = mbedtls_gcm_finish( &ctx, NULL, 0, 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 d4dce9398..965d15482 100644 --- a/tests/suites/test_suite_gcm.function +++ b/tests/suites/test_suite_gcm.function @@ -41,7 +41,7 @@ static int check_multipart( mbedtls_gcm_context *ctx, output = NULL; ASSERT_ALLOC( output, tag->len ); - TEST_EQUAL( 0, mbedtls_gcm_finish( ctx, output, tag->len ) ); + TEST_EQUAL( 0, mbedtls_gcm_finish( ctx, NULL, 0, output, tag->len ) ); ASSERT_COMPARE( output, tag->len, tag->x, tag->len ); mbedtls_free( output ); output = NULL; @@ -326,10 +326,10 @@ void gcm_invalid_param( ) /* mbedtls_gcm_finish() */ TEST_INVALID_PARAM_RET( MBEDTLS_ERR_GCM_BAD_INPUT, - mbedtls_gcm_finish( NULL, valid_buffer, valid_len ) ); + mbedtls_gcm_finish( NULL, NULL, 0, valid_buffer, valid_len ) ); TEST_INVALID_PARAM_RET( MBEDTLS_ERR_GCM_BAD_INPUT, - mbedtls_gcm_finish( &ctx, NULL, valid_len ) ); + mbedtls_gcm_finish( &ctx, NULL, 0, NULL, valid_len ) ); exit: mbedtls_gcm_free( &ctx ); From a56c4486364c48ec4f0b0708487152252e6de735 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 15 Apr 2021 17:22:35 +0200 Subject: [PATCH 05/15] Add output length parameters to mbedtls_gcm_update Alternative implementations of GCM may delay the output of partial blocks from mbedtls_gcm_update(). Add an output length parameter to mbedtls_gcm_update() to allow such implementations to delay the output of partial blocks. With the software implementation, there is no such delay. Signed-off-by: Gilles Peskine --- ChangeLog.d/gcm-update.txt | 1 + include/mbedtls/gcm.h | 42 +++++++++---- library/cipher.c | 6 +- library/gcm.c | 88 ++++++++++++++++++---------- tests/suites/test_suite_gcm.function | 30 ++++++---- 5 files changed, 112 insertions(+), 55 deletions(-) diff --git a/ChangeLog.d/gcm-update.txt b/ChangeLog.d/gcm-update.txt index d38455102..10d53efd7 100644 --- a/ChangeLog.d/gcm-update.txt +++ b/ChangeLog.d/gcm-update.txt @@ -2,6 +2,7 @@ API changes * The interface of the GCM module has changed to remove restrictions on how the input to multipart operations is broken down. mbedtls_gcm_finish() now takes an extra output parameter for the last partial output block. + mbedtls_gcm_update() now takes extra parameters for the output length. The software implementation always produces the full output at each call to mbedtls_gcm_update(), but alternative implementations activated by MBEDTLS_GCM_ALT may delay partial blocks to the next call to diff --git a/include/mbedtls/gcm.h b/include/mbedtls/gcm.h index 951ee00c4..0bd6e1e0f 100644 --- a/include/mbedtls/gcm.h +++ b/include/mbedtls/gcm.h @@ -253,22 +253,42 @@ int mbedtls_gcm_starts( mbedtls_gcm_context *ctx, * input buffer. If the buffers overlap, the output buffer * must trail at least 8 Bytes behind the input buffer. * - * \param ctx The GCM context. This must be initialized. - * \param length The length of the input data. - * \param input The buffer holding the input data. If \p length is greater - * than zero, this must be a readable buffer of at least that - * size in Bytes. - * \param output The buffer for holding the output data. If \p length is - * greater than zero, this must be a writable buffer of at - * least that size in Bytes. + * \param ctx The GCM context. This must be initialized. + * \param input The buffer holding the input data. If \p input_length + * is greater than zero, this must be a readable buffer + * of at least \p input_length bytes. + * \param input_length The length of the input data in bytes. + * \param output The buffer for the output data. If \p output_length + * is greater than zero, this must be a writable buffer of + * of at least \p output_size bytes. + * This function may withhold the end of the output if + * it is a partial block for the underlying block cipher. + * That is, if the cumulated input passed to + * mbedtls_gcm_update() so far (including the current call) + * is 16 *n* + *p* with *p* < 16, this function may + * withhold the last *p* bytes, which will be output by + * a subsequent call to mbedtls_gcm_update() or + * mbedtls_gcm_finish(). + * \param output_size The size of the output buffer in bytes. + * This must be at least \p input_length plus the length + * of the input withheld by the previous call to + * mbedtls_gcm_update(). Therefore: + * - With arbitrary inputs, \p output_size may need to + * be as large as `input_length + 15`. + * - If all input lengths are a multiple of 16, then + * \p output_size = \p input_length is sufficient. + * \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. */ int mbedtls_gcm_update( mbedtls_gcm_context *ctx, - size_t length, - const unsigned char *input, - unsigned char *output ); + const unsigned char *input, size_t input_length, + unsigned char *output, size_t output_size, + size_t *output_length ); /** * \brief This function finishes the GCM operation and generates diff --git a/library/cipher.c b/library/cipher.c index 63eaba88f..7e6d0e02c 100644 --- a/library/cipher.c +++ b/library/cipher.c @@ -545,9 +545,9 @@ int mbedtls_cipher_update( mbedtls_cipher_context_t *ctx, const unsigned char *i #if defined(MBEDTLS_GCM_C) if( ctx->cipher_info->mode == MBEDTLS_MODE_GCM ) { - *olen = ilen; - return( mbedtls_gcm_update( (mbedtls_gcm_context *) ctx->cipher_ctx, ilen, input, - output ) ); + return( mbedtls_gcm_update( (mbedtls_gcm_context *) ctx->cipher_ctx, + input, ilen, + output, ilen, olen ) ); } #endif diff --git a/library/gcm.c b/library/gcm.c index de766bc76..13e729643 100644 --- a/library/gcm.c +++ b/library/gcm.c @@ -395,9 +395,9 @@ static int gcm_mask( mbedtls_gcm_context *ctx, } int mbedtls_gcm_update( mbedtls_gcm_context *ctx, - size_t length, - const unsigned char *input, - unsigned char *output ) + const unsigned char *input, size_t input_length, + unsigned char *output, size_t output_size, + size_t *output_length ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; const unsigned char *p = input; @@ -405,22 +405,27 @@ int mbedtls_gcm_update( mbedtls_gcm_context *ctx, size_t offset; unsigned char ectr[16]; - /* Exit early if length==0 so that we don't do any pointer arithmetic on - * a potentially null pointer. */ - if( length == 0 ) + if( output_size < input_length ) + return( MBEDTLS_ERR_GCM_BAD_INPUT ); + GCM_VALIDATE_RET( output_length != NULL ); + *output_length = input_length; + + /* Exit early if input_length==0 so that we don't do any pointer arithmetic + * on a potentially null pointer. */ + if( input_length == 0 ) return( 0 ); GCM_VALIDATE_RET( ctx != NULL ); GCM_VALIDATE_RET( input != NULL ); GCM_VALIDATE_RET( output != NULL ); - if( output > input && (size_t) ( output - input ) < length ) + if( output > input && (size_t) ( output - input ) < input_length ) return( MBEDTLS_ERR_GCM_BAD_INPUT ); /* Total length is restricted to 2^39 - 256 bits, ie 2^36 - 2^5 bytes * Also check for possible overflow */ - if( ctx->len + length < ctx->len || - (uint64_t) ctx->len + length > 0xFFFFFFFE0ull ) + if( ctx->len + input_length < ctx->len || + (uint64_t) ctx->len + input_length > 0xFFFFFFFE0ull ) { return( MBEDTLS_ERR_GCM_BAD_INPUT ); } @@ -429,8 +434,8 @@ int mbedtls_gcm_update( mbedtls_gcm_context *ctx, if( offset != 0 ) { size_t use_len = 16 - offset; - if( use_len > length ) - use_len = length; + if( use_len > input_length ) + use_len = input_length; if( ( ret = gcm_mask( ctx, ectr, offset, use_len, p, out_p ) ) != 0 ) return( ret ); @@ -439,14 +444,14 @@ int mbedtls_gcm_update( mbedtls_gcm_context *ctx, gcm_mult( ctx, ctx->buf, ctx->buf ); ctx->len += use_len; - length -= use_len; + input_length -= use_len; p += use_len; out_p += use_len; } - ctx->len += length; + ctx->len += input_length; - while( length >= 16 ) + while( input_length >= 16 ) { gcm_incr( ctx->y ); if( ( ret = gcm_mask( ctx, ectr, 0, 16, p, out_p ) ) != 0 ) @@ -454,15 +459,15 @@ int mbedtls_gcm_update( mbedtls_gcm_context *ctx, gcm_mult( ctx, ctx->buf, ctx->buf ); - length -= 16; + input_length -= 16; p += 16; out_p += 16; } - if( length > 0 ) + if( input_length > 0 ) { gcm_incr( ctx->y ); - if( ( ret = gcm_mask( ctx, ectr, 0, length, p, out_p ) ) != 0 ) + if( ( ret = gcm_mask( ctx, ectr, 0, input_length, p, out_p ) ) != 0 ) return( ret ); } @@ -532,6 +537,7 @@ int mbedtls_gcm_crypt_and_tag( mbedtls_gcm_context *ctx, unsigned char *tag ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; + size_t olen; GCM_VALIDATE_RET( ctx != NULL ); GCM_VALIDATE_RET( iv != NULL ); @@ -543,7 +549,8 @@ int mbedtls_gcm_crypt_and_tag( mbedtls_gcm_context *ctx, if( ( ret = mbedtls_gcm_starts( ctx, mode, iv, iv_len, add, add_len ) ) != 0 ) return( ret ); - if( ( ret = mbedtls_gcm_update( ctx, length, input, output ) ) != 0 ) + if( ( ret = mbedtls_gcm_update( ctx, input, length, + output, length, &olen ) ) != 0 ) return( ret ); if( ( ret = mbedtls_gcm_finish( ctx, NULL, 0, tag, tag_len ) ) != 0 ) @@ -840,6 +847,7 @@ int mbedtls_gcm_self_test( int verbose ) unsigned char tag_buf[16]; int i, j, ret; mbedtls_cipher_id_t cipher = MBEDTLS_CIPHER_ID_AES; + size_t olen; for( j = 0; j < 3; j++ ) { @@ -963,25 +971,34 @@ int mbedtls_gcm_self_test( int verbose ) if( pt_len_test_data[i] > 32 ) { size_t rest_len = pt_len_test_data[i] - 32; - ret = mbedtls_gcm_update( &ctx, 32, + ret = mbedtls_gcm_update( &ctx, pt_test_data[pt_index_test_data[i]], - buf ); + 32, + buf, sizeof( buf ), &olen ); if( ret != 0 ) goto exit; + if( olen != 32 ) + goto exit; - ret = mbedtls_gcm_update( &ctx, rest_len, - pt_test_data[pt_index_test_data[i]] + 32, - buf + 32 ); + ret = mbedtls_gcm_update( &ctx, + pt_test_data[pt_index_test_data[i]] + 32, + rest_len, + buf + 32, sizeof( buf ) - 32, &olen ); if( ret != 0 ) goto exit; + if( olen != rest_len ) + goto exit; } else { - ret = mbedtls_gcm_update( &ctx, pt_len_test_data[i], + ret = mbedtls_gcm_update( &ctx, pt_test_data[pt_index_test_data[i]], - buf ); + pt_len_test_data[i], + buf, sizeof( buf ), &olen ); if( ret != 0 ) goto exit; + if( olen != pt_len_test_data[i] ) + goto exit; } ret = mbedtls_gcm_finish( &ctx, NULL, 0, tag_buf, 16 ); @@ -1024,24 +1041,33 @@ int mbedtls_gcm_self_test( int verbose ) if( pt_len_test_data[i] > 32 ) { size_t rest_len = pt_len_test_data[i] - 32; - ret = mbedtls_gcm_update( &ctx, 32, ct_test_data[j * 6 + i], - buf ); + ret = mbedtls_gcm_update( &ctx, + ct_test_data[j * 6 + i], 32, + buf, sizeof( buf ), &olen ); if( ret != 0 ) goto exit; + if( olen != 32 ) + goto exit; - ret = mbedtls_gcm_update( &ctx, rest_len, + ret = mbedtls_gcm_update( &ctx, ct_test_data[j * 6 + i] + 32, - buf + 32 ); + rest_len, + buf + 32, sizeof( buf ) - 32, &olen ); if( ret != 0 ) goto exit; + if( olen != rest_len ) + goto exit; } else { - ret = mbedtls_gcm_update( &ctx, pt_len_test_data[i], + ret = mbedtls_gcm_update( &ctx, ct_test_data[j * 6 + i], - buf ); + pt_len_test_data[i], + buf, sizeof( buf ), &olen ); if( ret != 0 ) goto exit; + if( olen != pt_len_test_data[i] ) + goto exit; } ret = mbedtls_gcm_finish( &ctx, NULL, 0, tag_buf, 16 ); diff --git a/tests/suites/test_suite_gcm.function b/tests/suites/test_suite_gcm.function index 965d15482..da6aea899 100644 --- a/tests/suites/test_suite_gcm.function +++ b/tests/suites/test_suite_gcm.function @@ -16,6 +16,7 @@ static int check_multipart( mbedtls_gcm_context *ctx, int ok = 0; uint8_t *output = NULL; size_t n2 = input->len - n1; + size_t olen; /* Sanity checks on the test data */ TEST_ASSERT( n1 <= input->len ); @@ -29,14 +30,18 @@ static int check_multipart( mbedtls_gcm_context *ctx, * tries to write beyond the advertised required buffer size, this will * count as an overflow for memory sanitizers and static checkers. */ ASSERT_ALLOC( output, n1 ); - TEST_EQUAL( 0, mbedtls_gcm_update( ctx, n1, input->x, output ) ); - ASSERT_COMPARE( output, n1, expected_output->x, n1 ); + olen = 0xdeadbeef; + TEST_EQUAL( 0, mbedtls_gcm_update( ctx, input->x, n1, output, n1, &olen ) ); + TEST_EQUAL( n1, olen ); + ASSERT_COMPARE( output, olen, expected_output->x, n1 ); mbedtls_free( output ); output = NULL; ASSERT_ALLOC( output, n2 ); - TEST_EQUAL( 0, mbedtls_gcm_update( ctx, n2, input->x + n1, output ) ); - ASSERT_COMPARE( output, n2, expected_output->x + n1, n2 ); + olen = 0xdeadbeef; + TEST_EQUAL( 0, mbedtls_gcm_update( ctx, input->x + n1, n2, output, n2, &olen ) ); + TEST_EQUAL( n2, olen ); + ASSERT_COMPARE( output, olen, expected_output->x + n1, n2 ); mbedtls_free( output ); output = NULL; @@ -185,6 +190,7 @@ void gcm_invalid_param( ) int valid_mode = MBEDTLS_GCM_ENCRYPT; int valid_len = sizeof(valid_buffer); int valid_bitlen = 128, invalid_bitlen = 1; + size_t olen; mbedtls_gcm_init( &ctx ); @@ -312,16 +318,20 @@ void gcm_invalid_param( ) /* mbedtls_gcm_update() */ TEST_INVALID_PARAM_RET( MBEDTLS_ERR_GCM_BAD_INPUT, - mbedtls_gcm_update( NULL, valid_len, - valid_buffer, valid_buffer ) ); + mbedtls_gcm_update( NULL, valid_buffer, valid_len, + valid_buffer, valid_len, &olen ) ); TEST_INVALID_PARAM_RET( MBEDTLS_ERR_GCM_BAD_INPUT, - mbedtls_gcm_update( &ctx, valid_len, - NULL, valid_buffer ) ); + mbedtls_gcm_update( &ctx, NULL, valid_len, + valid_buffer, valid_len, &olen ) ); TEST_INVALID_PARAM_RET( MBEDTLS_ERR_GCM_BAD_INPUT, - mbedtls_gcm_update( &ctx, valid_len, - valid_buffer, NULL ) ); + mbedtls_gcm_update( &ctx, valid_buffer, valid_len, + NULL, valid_len, &olen ) ); + TEST_INVALID_PARAM_RET( + MBEDTLS_ERR_GCM_BAD_INPUT, + mbedtls_gcm_update( &ctx, valid_buffer, valid_len, + valid_buffer, valid_len, NULL ) ); /* mbedtls_gcm_finish() */ TEST_INVALID_PARAM_RET( From 295fc13ef33c9ce610be812f83ed00b366be1c39 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 15 Apr 2021 18:32:23 +0200 Subject: [PATCH 06/15] Split mbedtls_gcm_update_ad out of mbedtls_gcm_starts The GCM interface now has separate functions to start the operation and to pass the associated data. This is in preparation for allowing the associated data to be passed in chunks with repeatated calls to mbedtls_gcm_update_ad(). Signed-off-by: Gilles Peskine --- ChangeLog.d/gcm-update.txt | 4 +- include/mbedtls/gcm.h | 29 +++++++++++--- library/cipher.c | 13 +++++- library/gcm.c | 60 +++++++++++++++++++--------- tests/suites/test_suite_gcm.function | 12 +++--- 5 files changed, 83 insertions(+), 35 deletions(-) diff --git a/ChangeLog.d/gcm-update.txt b/ChangeLog.d/gcm-update.txt index 10d53efd7..0fffd094d 100644 --- a/ChangeLog.d/gcm-update.txt +++ b/ChangeLog.d/gcm-update.txt @@ -6,7 +6,9 @@ API changes The software implementation always produces the full output at each call to mbedtls_gcm_update(), but alternative implementations activated by MBEDTLS_GCM_ALT may delay partial blocks to the next call to - mbedtls_gcm_update() or mbedtls_gcm_finish(). + mbedtls_gcm_update() or mbedtls_gcm_finish(). Furthermore, applications + no longer pass the associated data to mbedtls_gcm_starts(), but to the + new function mbedtls_gcm_update_ad(). These changes are backward compatible for users of the cipher API. Features diff --git a/include/mbedtls/gcm.h b/include/mbedtls/gcm.h index 0bd6e1e0f..7218081e3 100644 --- a/include/mbedtls/gcm.h +++ b/include/mbedtls/gcm.h @@ -231,6 +231,26 @@ int mbedtls_gcm_auth_decrypt( mbedtls_gcm_context *ctx, * \param iv The initialization vector. This must be a readable buffer of * at least \p iv_len Bytes. * \param iv_len The length of the IV. + * + * \return \c 0 on success. + */ +int mbedtls_gcm_starts( mbedtls_gcm_context *ctx, + int mode, + const unsigned char *iv, + size_t iv_len ); + +/** + * \brief This function starts a GCM encryption or decryption + * operation. + * + * \note This function may only be called once per operation: + * you must pass the whole associated data in a single + * call. This limitation will be lifted in a future version + * of Mbed TLS. + * + * \param ctx The GCM context. This must have been started with + * mbedtls_gcm_starts() and must not have yet received + * any input with mbedtls_gcm_update(). * \param add The buffer holding the additional data, or \c NULL * if \p add_len is \c 0. * \param add_len The length of the additional data. If \c 0, @@ -238,12 +258,9 @@ int mbedtls_gcm_auth_decrypt( mbedtls_gcm_context *ctx, * * \return \c 0 on success. */ -int mbedtls_gcm_starts( mbedtls_gcm_context *ctx, - int mode, - const unsigned char *iv, - size_t iv_len, - const unsigned char *add, - size_t add_len ); +int mbedtls_gcm_update_ad( mbedtls_gcm_context *ctx, + const unsigned char *add, + size_t add_len ); /** * \brief This function feeds an input buffer into an ongoing GCM diff --git a/library/cipher.c b/library/cipher.c index 7e6d0e02c..e09130ac5 100644 --- a/library/cipher.c +++ b/library/cipher.c @@ -415,6 +415,15 @@ int mbedtls_cipher_set_iv( mbedtls_cipher_context_t *ctx, } #endif +#if defined(MBEDTLS_GCM_C) + if( MBEDTLS_MODE_GCM == ctx->cipher_info->mode ) + { + return( mbedtls_gcm_starts( (mbedtls_gcm_context *) ctx->cipher_ctx, + ctx->operation, + iv, iv_len ) ); + } +#endif + if ( actual_iv_size != 0 ) { memcpy( ctx->iv, iv, actual_iv_size ); @@ -466,8 +475,8 @@ int mbedtls_cipher_update_ad( mbedtls_cipher_context_t *ctx, #if defined(MBEDTLS_GCM_C) if( MBEDTLS_MODE_GCM == ctx->cipher_info->mode ) { - return( mbedtls_gcm_starts( (mbedtls_gcm_context *) ctx->cipher_ctx, ctx->operation, - ctx->iv, ctx->iv_size, ad, ad_len ) ); + return( mbedtls_gcm_update_ad( (mbedtls_gcm_context *) ctx->cipher_ctx, + ad, ad_len ) ); } #endif diff --git a/library/gcm.c b/library/gcm.c index 13e729643..ee10093c0 100644 --- a/library/gcm.c +++ b/library/gcm.c @@ -269,11 +269,8 @@ static void gcm_mult( mbedtls_gcm_context *ctx, const unsigned char x[16], } int mbedtls_gcm_starts( mbedtls_gcm_context *ctx, - int mode, - const unsigned char *iv, - size_t iv_len, - const unsigned char *add, - size_t add_len ) + int mode, + const unsigned char *iv, size_t iv_len ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; unsigned char work_buf[16]; @@ -283,16 +280,11 @@ int mbedtls_gcm_starts( mbedtls_gcm_context *ctx, GCM_VALIDATE_RET( ctx != NULL ); GCM_VALIDATE_RET( iv != NULL ); - GCM_VALIDATE_RET( add_len == 0 || add != NULL ); - /* IV and AD are limited to 2^64 bits, so 2^61 bytes */ + /* IV is are limited to 2^64 bits, so 2^61 bytes */ /* IV is not allowed to be zero length */ - if( iv_len == 0 || - ( (uint64_t) iv_len ) >> 61 != 0 || - ( (uint64_t) add_len ) >> 61 != 0 ) - { + if( iv_len == 0 || (uint64_t) iv_len >> 61 != 0 ) return( MBEDTLS_ERR_GCM_BAD_INPUT ); - } memset( ctx->y, 0x00, sizeof(ctx->y) ); memset( ctx->buf, 0x00, sizeof(ctx->buf) ); @@ -337,6 +329,26 @@ int mbedtls_gcm_starts( mbedtls_gcm_context *ctx, return( ret ); } + return( 0 ); +} + + +int mbedtls_gcm_update_ad( mbedtls_gcm_context *ctx, + const unsigned char *add, size_t add_len ) +{ + const unsigned char *p; + size_t use_len, i; + + GCM_VALIDATE_RET( add_len == 0 || add != NULL ); + + /* IV is are limited to 2^64 bits, so 2^61 bytes */ + if( (uint64_t) add_len >> 61 != 0 ) + return( MBEDTLS_ERR_GCM_BAD_INPUT ); + + /* Calling update_ad multiple times is not yet supported */ + if( ctx->add_len != 0 ) + return( MBEDTLS_ERR_GCM_BAD_INPUT ); + ctx->add_len = add_len; p = add; while( add_len > 0 ) @@ -546,7 +558,10 @@ int mbedtls_gcm_crypt_and_tag( mbedtls_gcm_context *ctx, GCM_VALIDATE_RET( length == 0 || output != NULL ); GCM_VALIDATE_RET( tag != NULL ); - if( ( ret = mbedtls_gcm_starts( ctx, mode, iv, iv_len, add, add_len ) ) != 0 ) + if( ( ret = mbedtls_gcm_starts( ctx, mode, iv, iv_len ) ) != 0 ) + return( ret ); + + if( ( ret = mbedtls_gcm_update_ad( ctx, add, add_len ) ) != 0 ) return( ret ); if( ( ret = mbedtls_gcm_update( ctx, input, length, @@ -961,10 +976,14 @@ int mbedtls_gcm_self_test( int verbose ) goto exit; ret = mbedtls_gcm_starts( &ctx, MBEDTLS_GCM_ENCRYPT, - iv_test_data[iv_index_test_data[i]], - iv_len_test_data[i], - additional_test_data[add_index_test_data[i]], - add_len_test_data[i] ); + iv_test_data[iv_index_test_data[i]], + iv_len_test_data[i] ); + if( ret != 0 ) + goto exit; + + ret = mbedtls_gcm_update_ad( &ctx, + additional_test_data[add_index_test_data[i]], + add_len_test_data[i] ); if( ret != 0 ) goto exit; @@ -1031,8 +1050,11 @@ int mbedtls_gcm_self_test( int verbose ) goto exit; ret = mbedtls_gcm_starts( &ctx, MBEDTLS_GCM_DECRYPT, - iv_test_data[iv_index_test_data[i]], - iv_len_test_data[i], + iv_test_data[iv_index_test_data[i]], + iv_len_test_data[i] ); + if( ret != 0 ) + goto exit; + ret = mbedtls_gcm_update_ad( &ctx, additional_test_data[add_index_test_data[i]], add_len_test_data[i] ); if( ret != 0 ) diff --git a/tests/suites/test_suite_gcm.function b/tests/suites/test_suite_gcm.function index da6aea899..9733eb235 100644 --- a/tests/suites/test_suite_gcm.function +++ b/tests/suites/test_suite_gcm.function @@ -23,8 +23,8 @@ static int check_multipart( mbedtls_gcm_context *ctx, TEST_EQUAL( input->len, expected_output->len ); TEST_EQUAL( 0, mbedtls_gcm_starts( ctx, mode, - iv->x, iv->len, - add->x, add->len ) ); + iv->x, iv->len ) ); + TEST_EQUAL( 0, mbedtls_gcm_update_ad( ctx, add->x, add->len ) ); /* Allocate a tight buffer for each update call. This way, if the function * tries to write beyond the advertised required buffer size, this will @@ -300,19 +300,17 @@ void gcm_invalid_param( ) TEST_INVALID_PARAM_RET( MBEDTLS_ERR_GCM_BAD_INPUT, mbedtls_gcm_starts( NULL, valid_mode, - valid_buffer, valid_len, valid_buffer, valid_len ) ); TEST_INVALID_PARAM_RET( MBEDTLS_ERR_GCM_BAD_INPUT, mbedtls_gcm_starts( &ctx, valid_mode, - NULL, valid_len, - valid_buffer, valid_len ) ); + NULL, valid_len ) ); + /* mbedtls_gcm_update_ad() */ TEST_INVALID_PARAM_RET( MBEDTLS_ERR_GCM_BAD_INPUT, - mbedtls_gcm_starts( &ctx, valid_mode, - valid_buffer, valid_len, + mbedtls_gcm_update_ad( &ctx, NULL, valid_len ) ); /* mbedtls_gcm_update() */ From 52118189da539dcd1a2937eff272a10c97a7a7e1 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 18 May 2021 20:38:33 +0200 Subject: [PATCH 07/15] Fix copypasta in the description of mbedtls_gcm_update_ad Signed-off-by: Gilles Peskine --- include/mbedtls/gcm.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/include/mbedtls/gcm.h b/include/mbedtls/gcm.h index 7218081e3..78f2a438f 100644 --- a/include/mbedtls/gcm.h +++ b/include/mbedtls/gcm.h @@ -240,8 +240,9 @@ int mbedtls_gcm_starts( mbedtls_gcm_context *ctx, size_t iv_len ); /** - * \brief This function starts a GCM encryption or decryption - * operation. + * \brief This function feeds an input buffer as associated data + * (authenticated but not encrypted data) in a GCM + * encryption or decryption operation. * * \note This function may only be called once per operation: * you must pass the whole associated data in a single From 518fdb00e85866d4661e3609ef179d2ae9087753 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 18 May 2021 20:43:31 +0200 Subject: [PATCH 08/15] Fix size/length confusion in documentation Signed-off-by: Gilles Peskine --- include/mbedtls/gcm.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/mbedtls/gcm.h b/include/mbedtls/gcm.h index 78f2a438f..995578ff8 100644 --- a/include/mbedtls/gcm.h +++ b/include/mbedtls/gcm.h @@ -276,7 +276,7 @@ int mbedtls_gcm_update_ad( mbedtls_gcm_context *ctx, * is greater than zero, this must be a readable buffer * of at least \p input_length bytes. * \param input_length The length of the input data in bytes. - * \param output The buffer for the output data. If \p output_length + * \param output The buffer for the output data. If \p output_size * is greater than zero, this must be a writable buffer of * of at least \p output_size bytes. * This function may withhold the end of the output if From 8e8cdd150ac813739769bfb4a58aeaae51938e0c Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 18 May 2021 21:02:13 +0200 Subject: [PATCH 09/15] Add some information about the multipart calling sequence Signed-off-by: Gilles Peskine --- include/mbedtls/gcm.h | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/include/mbedtls/gcm.h b/include/mbedtls/gcm.h index 995578ff8..ae41c6047 100644 --- a/include/mbedtls/gcm.h +++ b/include/mbedtls/gcm.h @@ -244,6 +244,11 @@ int mbedtls_gcm_starts( mbedtls_gcm_context *ctx, * (authenticated but not encrypted data) in a GCM * encryption or decryption operation. * + * Call this function after mbedtls_gcm_starts() to pass + * the associated data. If the associated data is empty, + * you do not need to call this function. You may not + * call this function after calling mbedtls_cipher_update(). + * * \note This function may only be called once per operation: * you must pass the whole associated data in a single * call. This limitation will be lifted in a future version @@ -267,6 +272,12 @@ int mbedtls_gcm_update_ad( mbedtls_gcm_context *ctx, * \brief This function feeds an input buffer into an ongoing GCM * encryption or decryption operation. * + * You may call this function zero, one or more times + * to pass successive parts of the input: the plaintext to + * encrypt, or the ciphertext (not including the tag) to + * decrypt. After the last part of the input, call + * mbedtls_gcm_finish(). + * * \note For decryption, the output buffer cannot be the same as * input buffer. If the buffers overlap, the output buffer * must trail at least 8 Bytes behind the input buffer. From d9380b5270afeef00aa5167225ad2771b2bf8c18 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 18 May 2021 21:02:52 +0200 Subject: [PATCH 10/15] Document reasons for MBEDTLS_ERR_GCM_BAD_INPUT in update and finish Signed-off-by: Gilles Peskine --- include/mbedtls/gcm.h | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/include/mbedtls/gcm.h b/include/mbedtls/gcm.h index ae41c6047..2e998e71d 100644 --- a/include/mbedtls/gcm.h +++ b/include/mbedtls/gcm.h @@ -312,7 +312,10 @@ int mbedtls_gcm_update_ad( mbedtls_gcm_context *ctx, * unspecified. * * \return \c 0 on success. - * \return #MBEDTLS_ERR_GCM_BAD_INPUT on failure. + * \return #MBEDTLS_ERR_GCM_BAD_INPUT on failure: + * total input length too long, + * unsupported input/output buffer overlap detected, + * or \p output_size too small. */ int mbedtls_gcm_update( mbedtls_gcm_context *ctx, const unsigned char *input, size_t input_length, @@ -343,7 +346,9 @@ int mbedtls_gcm_update( mbedtls_gcm_context *ctx, * Alternative implementations may require a 15-byte buffer. * * \return \c 0 on success. - * \return #MBEDTLS_ERR_GCM_BAD_INPUT on failure. + * \return #MBEDTLS_ERR_GCM_BAD_INPUT on failure: + * invalid value of \p tag_len, + * or \p output_len too small. */ int mbedtls_gcm_finish( mbedtls_gcm_context *ctx, unsigned char *output, size_t output_len, From b7bb0687f75e515e6d9ad1af5f004baf553a5d4c Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 18 May 2021 22:31:53 +0200 Subject: [PATCH 11/15] Rework and reword the guarantees on output_size To simplify the documentation, and because there are only two realistic behaviors in practice, only allow two behaviors for multipart output: immediate output, or buffering of the last partial block. State some simple rules that applications can follow if they don't care about the details. Explicitly state how much output is needed for finish(). Only require the buffer size to be the size of the actual output, not the size of the potential output in the worst case. Rename the parameter from output_len to output_size since it's a buffer size and not necessarily the length of the data. No longer guarantee that the built-in implementation produces immediate output. Signed-off-by: Gilles Peskine --- include/mbedtls/gcm.h | 53 +++++++++++++++++++++++-------------------- library/gcm.c | 4 ++-- 2 files changed, 30 insertions(+), 27 deletions(-) diff --git a/include/mbedtls/gcm.h b/include/mbedtls/gcm.h index 2e998e71d..e717aa1bf 100644 --- a/include/mbedtls/gcm.h +++ b/include/mbedtls/gcm.h @@ -278,6 +278,22 @@ int mbedtls_gcm_update_ad( mbedtls_gcm_context *ctx, * decrypt. After the last part of the input, call * mbedtls_gcm_finish(). * + * This function may produce output in one of the following + * ways: + * - Immediate output: the output length is always equal + * to the input length. + * - Buffered output: the output consists of a whole number + * of 16-byte blocks. If the total input length so far + * (not including associated data) is 16 \* *B* + *A* + * with *A* < 16 then the total output length is 16 \* *B*. + * + * In particular: + * - It is always correct to call this function with + * \c output_size >= \c input_size + 15. + * - If \c input_size is a multiple of 16 for all the calls + * to this function during an operation, then it is + * correct to use \c output_size = \c input_size. + * * \note For decryption, the output buffer cannot be the same as * input buffer. If the buffers overlap, the output buffer * must trail at least 8 Bytes behind the input buffer. @@ -290,22 +306,8 @@ int mbedtls_gcm_update_ad( mbedtls_gcm_context *ctx, * \param output The buffer for the output data. If \p output_size * is greater than zero, this must be a writable buffer of * of at least \p output_size bytes. - * This function may withhold the end of the output if - * it is a partial block for the underlying block cipher. - * That is, if the cumulated input passed to - * mbedtls_gcm_update() so far (including the current call) - * is 16 *n* + *p* with *p* < 16, this function may - * withhold the last *p* bytes, which will be output by - * a subsequent call to mbedtls_gcm_update() or - * mbedtls_gcm_finish(). * \param output_size The size of the output buffer in bytes. - * This must be at least \p input_length plus the length - * of the input withheld by the previous call to - * mbedtls_gcm_update(). Therefore: - * - With arbitrary inputs, \p output_size may need to - * be as large as `input_length + 15`. - * - If all input lengths are a multiple of 16, then - * \p output_size = \p input_length is sufficient. + * See the function description regarding the output size. * \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 @@ -335,15 +337,16 @@ int mbedtls_gcm_update( mbedtls_gcm_context *ctx, * \param tag_len The length of the tag to generate. This must be at least * four. * \param output The buffer for the final output. - * This must be a writable buffer of at least \p output_len - * bytes. - * With the built-in implementation, there is no final - * output and this can be \p NULL. - * Alternative implementations may return a partial block - * of output. - * \param output_len The size of the \p output buffer in bytes. - * With the built-in implementation, this can be \c 0. - * Alternative implementations may require a 15-byte buffer. + * If \p output_size is nonzero, this must be a writable + * buffer of at least \p output_size bytes. + * \param output_size The size of the \p output buffer in bytes. + * This must be large enough for the output that + * mbedtls_gcm_update() has not produced. In particular: + * - If mbedtls_gcm_update() produces immediate output, + * or if the total input size is a multiple of \c 16, + * 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. * * \return \c 0 on success. * \return #MBEDTLS_ERR_GCM_BAD_INPUT on failure: @@ -351,7 +354,7 @@ int mbedtls_gcm_update( mbedtls_gcm_context *ctx, * or \p output_len too small. */ int mbedtls_gcm_finish( mbedtls_gcm_context *ctx, - unsigned char *output, size_t output_len, + unsigned char *output, size_t output_size, unsigned char *tag, size_t tag_len ); /** diff --git a/library/gcm.c b/library/gcm.c index ee10093c0..2bd907115 100644 --- a/library/gcm.c +++ b/library/gcm.c @@ -488,7 +488,7 @@ int mbedtls_gcm_update( mbedtls_gcm_context *ctx, } int mbedtls_gcm_finish( mbedtls_gcm_context *ctx, - unsigned char *output, size_t output_len, + unsigned char *output, size_t output_size, unsigned char *tag, size_t tag_len ) { unsigned char work_buf[16]; @@ -502,7 +502,7 @@ int mbedtls_gcm_finish( mbedtls_gcm_context *ctx, /* We never pass any output in finish(). The output parameter exists only * for the sake of alternative implementations. */ (void) output; - (void) output_len; + (void) output_size; orig_len = ctx->len * 8; orig_add_len = ctx->add_len * 8; From fe561fe717e83a928d169cc7be3159149f23aa1b Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 19 May 2021 12:21:53 +0200 Subject: [PATCH 12/15] Doxygen: use \p for parameter names Signed-off-by: Gilles Peskine --- include/mbedtls/gcm.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/include/mbedtls/gcm.h b/include/mbedtls/gcm.h index e717aa1bf..c7d82fc2c 100644 --- a/include/mbedtls/gcm.h +++ b/include/mbedtls/gcm.h @@ -289,10 +289,10 @@ int mbedtls_gcm_update_ad( mbedtls_gcm_context *ctx, * * In particular: * - It is always correct to call this function with - * \c output_size >= \c input_size + 15. - * - If \c input_size is a multiple of 16 for all the calls + * \p output_size >= \p input_size + 15. + * - If \p input_size is a multiple of 16 for all the calls * to this function during an operation, then it is - * correct to use \c output_size = \c input_size. + * correct to use \p output_size = \p input_size. * * \note For decryption, the output buffer cannot be the same as * input buffer. If the buffers overlap, the output buffer From af5b26aeaa4ec64ced9add267caaee3d4aa9e5a2 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 19 May 2021 12:24:33 +0200 Subject: [PATCH 13/15] Fix parameter names in documentation Signed-off-by: Gilles Peskine --- include/mbedtls/gcm.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/include/mbedtls/gcm.h b/include/mbedtls/gcm.h index c7d82fc2c..e893a9979 100644 --- a/include/mbedtls/gcm.h +++ b/include/mbedtls/gcm.h @@ -289,10 +289,10 @@ int mbedtls_gcm_update_ad( mbedtls_gcm_context *ctx, * * In particular: * - It is always correct to call this function with - * \p output_size >= \p input_size + 15. - * - If \p input_size is a multiple of 16 for all the calls + * \p output_size >= \p input_length + 15. + * - If \p input_length is a multiple of 16 for all the calls * to this function during an operation, then it is - * correct to use \p output_size = \p input_size. + * correct to use \p output_size = \p input_length. * * \note For decryption, the output buffer cannot be the same as * input buffer. If the buffers overlap, the output buffer @@ -351,7 +351,7 @@ int mbedtls_gcm_update( mbedtls_gcm_context *ctx, * \return \c 0 on success. * \return #MBEDTLS_ERR_GCM_BAD_INPUT on failure: * invalid value of \p tag_len, - * or \p output_len too small. + * or \p output_size too small. */ int mbedtls_gcm_finish( mbedtls_gcm_context *ctx, unsigned char *output, size_t output_size, From 7f312c811b6a3f3889c0c24cf8edf46e27318f36 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 20 May 2021 11:51:46 +0200 Subject: [PATCH 14/15] Add migration guides for GCM Signed-off-by: Gilles Peskine --- docs/3.0-migration-guide.d/gcm-alt.md | 10 ++++++++++ docs/3.0-migration-guide.d/gcm-multipart.md | 15 +++++++++++++++ 2 files changed, 25 insertions(+) create mode 100644 docs/3.0-migration-guide.d/gcm-alt.md create mode 100644 docs/3.0-migration-guide.d/gcm-multipart.md diff --git a/docs/3.0-migration-guide.d/gcm-alt.md b/docs/3.0-migration-guide.d/gcm-alt.md new file mode 100644 index 000000000..388e2bedd --- /dev/null +++ b/docs/3.0-migration-guide.d/gcm-alt.md @@ -0,0 +1,10 @@ +GCM interface changes: impact for alternative implementations +------------------------------------------------------------- + +The GCM multipart interface has changed as described in [“GCM multipart interface: application changes”](#gcm-multipart-interface:-application-changes). The consequences for an alternative implementation of GCM (`MBEDTLS_GCM_ALT`) are as follows: + +* `mbedtls_gcm_starts()` now only sets the mode and the nonce (IV). The new function `mbedtls_gcm_update_ad()` receives the associated data. It may be called multiple times. +* `mbedtls_gcm_update()` now allows arbitrary-length inputs, takes an extra parameter to indicate the actual output length. Alternative implementations may choose between two modes: + * Always return the partial output immediately, even if it does not consist of a whole number of blocks. + * Buffer the data for the last partial block, to be returned in the next call to `mbedtls_gcm_update()` or `mbedtls_gcm_finish()`. +* `mbedtls_gcm_finish()` now takes an extra output buffer for the last partial block if needed. diff --git a/docs/3.0-migration-guide.d/gcm-multipart.md b/docs/3.0-migration-guide.d/gcm-multipart.md new file mode 100644 index 000000000..f37f7e8e0 --- /dev/null +++ b/docs/3.0-migration-guide.d/gcm-multipart.md @@ -0,0 +1,15 @@ +GCM multipart interface: application changes +-------------------------------------------- + +The GCM module now supports arbitrary chunked input in the multipart interface. + +For applications using GCM for multipart operations, this means the following changes: + +* `mbedtls_gcm_starts()` now only sets the mode and the nonce (IV). Call the new function `mbedtls_gcm_update_ad()` to pass the associated data. +* The current implementation has a limitation that `mbedtls_gcm_update_ad()` may only be called once. This limitation will be lifted shortly; watch https://github.com/ARMmbed/mbedtls/issues/4351 for updates. +* `mbedtls_gcm_update()` now takes an extra parameter to indicate the actual output length. In Mbed TLS 2.x, applications had to pass inputs consisting of whole 16-byte blocks except for the last block (this limitation has been lifted). In this case: + * As long as the input remains block-aligned, the output length is exactly the input length, as before. + * If the length of the last input is not a multiple of 16, alternative implementations may return the last partial block in the call to `mbedtls_gcm_finish()` instead of returning it in the last call to `mbedtls_gcm_update()`. +* `mbedtls_gcm_finish()` now takes an extra output buffer for the last partial block. This is needed for alternative implementations that can only process a whole block at a time. + +Applications using one-shot GCM or using GCM via the `mbedtls_cipher_xxx` or `psa_aead_xxx` interfaces do not require any changes. From 15c7b40ab71e49e50a8bb82667b3794c884fcd9c Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 20 May 2021 12:11:19 +0200 Subject: [PATCH 15/15] Reorder the text to say who is affected first Signed-off-by: Gilles Peskine --- docs/3.0-migration-guide.d/gcm-multipart.md | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/docs/3.0-migration-guide.d/gcm-multipart.md b/docs/3.0-migration-guide.d/gcm-multipart.md index f37f7e8e0..98e9fad2e 100644 --- a/docs/3.0-migration-guide.d/gcm-multipart.md +++ b/docs/3.0-migration-guide.d/gcm-multipart.md @@ -2,8 +2,8 @@ GCM multipart interface: application changes -------------------------------------------- The GCM module now supports arbitrary chunked input in the multipart interface. - -For applications using GCM for multipart operations, this means the following changes: +This changes the interface for applications using the GCM module directly for multipart operations. +Applications using one-shot GCM or using GCM via the `mbedtls_cipher_xxx` or `psa_aead_xxx` interfaces do not require any changes. * `mbedtls_gcm_starts()` now only sets the mode and the nonce (IV). Call the new function `mbedtls_gcm_update_ad()` to pass the associated data. * The current implementation has a limitation that `mbedtls_gcm_update_ad()` may only be called once. This limitation will be lifted shortly; watch https://github.com/ARMmbed/mbedtls/issues/4351 for updates. @@ -11,5 +11,3 @@ For applications using GCM for multipart operations, this means the following ch * As long as the input remains block-aligned, the output length is exactly the input length, as before. * If the length of the last input is not a multiple of 16, alternative implementations may return the last partial block in the call to `mbedtls_gcm_finish()` instead of returning it in the last call to `mbedtls_gcm_update()`. * `mbedtls_gcm_finish()` now takes an extra output buffer for the last partial block. This is needed for alternative implementations that can only process a whole block at a time. - -Applications using one-shot GCM or using GCM via the `mbedtls_cipher_xxx` or `psa_aead_xxx` interfaces do not require any changes.