From 79db2f14da0babb481719b8b852bc51799e1dc31 Mon Sep 17 00:00:00 2001 From: Andrzej Kurek Date: Tue, 20 Oct 2020 17:10:38 +0200 Subject: [PATCH 1/3] Refactor the buffer resize feature to reduce codesize Extract a common part of the code to a function. Signed-off-by: Andrzej Kurek --- library/ssl_tls.c | 178 +++++++++++++++++----------------------------- 1 file changed, 64 insertions(+), 114 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 8a014ced3..2edec1698 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -666,6 +666,66 @@ static int resize_buffer( unsigned char **buffer, size_t len_new, size_t *len_ol return 0; } + +static void handle_buffer_resizing( mbedtls_ssl_context *ssl, int downsizing, + uint32_t in_buf_new_len, + uint32_t out_buf_new_len ) +{ + int modified = 0; + size_t written_in = 0, len_offset_in = 0; + size_t written_out = 0, iv_offset_out = 0, len_offset_out = 0; + if( ssl->in_buf != NULL ) + { + written_in = ssl->in_msg - ssl->in_buf; + len_offset_in = ssl->in_len - ssl->in_buf; + if( ( downsizing && ssl->in_buf_len > in_buf_new_len && ssl->in_left < in_buf_new_len ) || + ( !downsizing && ssl->in_buf_len < in_buf_new_len ) ) + { + if( resize_buffer( &ssl->in_buf, in_buf_new_len, &ssl->in_buf_len ) != 0 ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "input buffer resizing failed - out of memory" ) ); + } + else + { + MBEDTLS_SSL_DEBUG_MSG( 2, ( "Reallocating in_buf to %d", in_buf_new_len ) ); + modified = 1; + } + } + } + + if( ssl->out_buf != NULL ) + { + written_out = ssl->out_msg - ssl->out_buf; + iv_offset_out = ssl->out_iv - ssl->out_buf; + len_offset_out = ssl->out_len - ssl->out_buf; + if( ( downsizing && ssl->out_buf_len > out_buf_new_len && ssl->out_left < out_buf_new_len ) || + ( !downsizing && ssl->out_buf_len < out_buf_new_len ) ) + { + if( resize_buffer( &ssl->out_buf, out_buf_new_len, &ssl->out_buf_len ) != 0 ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "output buffer resizing failed - out of memory" ) ); + } + else + { + MBEDTLS_SSL_DEBUG_MSG( 2, ( "Reallocating out_buf to %d", out_buf_new_len ) ); + modified = 1; + } + } + } + if( modified ) + { + /* Update pointers here to avoid doing it twice. */ + ssl_reset_in_out_pointers( ssl ); + /* Fields below might not be properly updated with record + * splitting or with CID, so they are manually updated here. */ + ssl->out_msg = ssl->out_buf + written_out; + ssl->out_len = ssl->out_buf + len_offset_out; + ssl->out_iv = ssl->out_buf + iv_offset_out; + + ssl->in_msg = ssl->in_buf + written_in; + ssl->in_len = ssl->in_buf + len_offset_in; + } +} #endif /* MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH */ #if defined(MBEDTLS_SSL_HW_RECORD_ACCEL) @@ -8814,62 +8874,8 @@ static int ssl_handshake_init( mbedtls_ssl_context *ssl ) #if defined(MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH) /* If the buffers are too small - reallocate */ - { - int modified = 0; - size_t written_in = 0, len_offset_in = 0; - size_t written_out = 0, iv_offset_out = 0, len_offset_out = 0; - if( ssl->in_buf != NULL ) - { - written_in = ssl->in_msg - ssl->in_buf; - len_offset_in = ssl->in_len - ssl->in_buf; - if( ssl->in_buf_len < MBEDTLS_SSL_IN_BUFFER_LEN ) - { - if( resize_buffer( &ssl->in_buf, MBEDTLS_SSL_IN_BUFFER_LEN, - &ssl->in_buf_len ) != 0 ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "input buffer resizing failed - out of memory" ) ); - } - else - { - MBEDTLS_SSL_DEBUG_MSG( 2, ( "Reallocating in_buf to %d", MBEDTLS_SSL_IN_BUFFER_LEN ) ); - modified = 1; - } - } - } - - if( ssl->out_buf != NULL ) - { - written_out = ssl->out_msg - ssl->out_buf; - iv_offset_out = ssl->out_iv - ssl->out_buf; - len_offset_out = ssl->out_len - ssl->out_buf; - if( ssl->out_buf_len < MBEDTLS_SSL_OUT_BUFFER_LEN ) - { - if( resize_buffer( &ssl->out_buf, MBEDTLS_SSL_OUT_BUFFER_LEN, - &ssl->out_buf_len ) != 0 ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "output buffer resizing failed - out of memory" ) ); - } - else - { - MBEDTLS_SSL_DEBUG_MSG( 2, ( "Reallocating out_buf to %d", MBEDTLS_SSL_OUT_BUFFER_LEN ) ); - modified = 1; - } - } - } - if( modified ) - { - /* Update pointers here to avoid doing it twice. */ - ssl_reset_in_out_pointers( ssl ); - /* Fields below might not be properly updated with record - * splitting or with CID, so they are manually updated here. */ - ssl->out_msg = ssl->out_buf + written_out; - ssl->out_len = ssl->out_buf + len_offset_out; - ssl->out_iv = ssl->out_buf + iv_offset_out; - - ssl->in_msg = ssl->in_buf + written_in; - ssl->in_len = ssl->in_buf + len_offset_in; - } - } + handle_buffer_resizing( ssl, 0, MBEDTLS_SSL_IN_BUFFER_LEN, + MBEDTLS_SSL_OUT_BUFFER_LEN ); #endif /* All pointers should exist and can be directly freed without issue */ @@ -12060,64 +12066,8 @@ void mbedtls_ssl_handshake_free( mbedtls_ssl_context *ssl ) * processes datagrams and the fact that a datagram is allowed to have * several records in it, it is possible that the I/O buffers are not * empty at this stage */ - { - int modified = 0; - uint32_t buf_len = mbedtls_ssl_get_input_buflen( ssl ); - size_t written_in = 0, len_offset_in = 0; - size_t written_out = 0, iv_offset_out = 0, len_offset_out = 0; - if( ssl->in_buf != NULL ) - { - written_in = ssl->in_msg - ssl->in_buf; - len_offset_in = ssl->in_len - ssl->in_buf; - if( ssl->in_buf_len > buf_len && ssl->in_left < buf_len ) - { - if( resize_buffer( &ssl->in_buf, buf_len, &ssl->in_buf_len ) != 0 ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "input buffer resizing failed - out of memory" ) ); - } - else - { - MBEDTLS_SSL_DEBUG_MSG( 2, ( "Reallocating in_buf to %d", buf_len ) ); - modified = 1; - } - } - } - - - buf_len = mbedtls_ssl_get_output_buflen( ssl ); - if( ssl->out_buf != NULL ) - { - written_out = ssl->out_msg - ssl->out_buf; - iv_offset_out = ssl->out_iv - ssl->out_buf; - len_offset_out = ssl->out_len - ssl->out_buf; - if( ssl->out_buf_len > mbedtls_ssl_get_output_buflen( ssl ) && - ssl->out_left < buf_len ) - { - if( resize_buffer( &ssl->out_buf, buf_len, &ssl->out_buf_len ) != 0 ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "output buffer resizing failed - out of memory" ) ); - } - else - { - MBEDTLS_SSL_DEBUG_MSG( 2, ( "Reallocating out_buf to %d", buf_len ) ); - modified = 1; - } - } - } - if( modified ) - { - /* Update pointers here to avoid doing it twice. */ - ssl_reset_in_out_pointers( ssl ); - /* Fields below might not be properly updated with record - * splitting or with CID, so they are manually updated here. */ - ssl->out_msg = ssl->out_buf + written_out; - ssl->out_len = ssl->out_buf + len_offset_out; - ssl->out_iv = ssl->out_buf + iv_offset_out; - - ssl->in_msg = ssl->in_buf + written_in; - ssl->in_len = ssl->in_buf + len_offset_in; - } - } + handle_buffer_resizing( ssl, 1, mbedtls_ssl_get_input_buflen( ssl ), + mbedtls_ssl_get_output_buflen( ssl ) ); #endif } From cd9a6ff3c1a31873ba8f7c733c7c46981eef2780 Mon Sep 17 00:00:00 2001 From: Andrzej Kurek Date: Thu, 22 Oct 2020 11:12:07 +0200 Subject: [PATCH 2/3] Introduce additional flags for buffer upsizing and downsizing Signed-off-by: Andrzej Kurek --- library/ssl_tls.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 2edec1698..eaa0f6e56 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -667,6 +667,8 @@ static int resize_buffer( unsigned char **buffer, size_t len_new, size_t *len_ol return 0; } +#define BUFFER_UPSIZING 0 +#define BUFFER_DOWNSIZING 1 static void handle_buffer_resizing( mbedtls_ssl_context *ssl, int downsizing, uint32_t in_buf_new_len, uint32_t out_buf_new_len ) @@ -8874,8 +8876,8 @@ static int ssl_handshake_init( mbedtls_ssl_context *ssl ) #if defined(MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH) /* If the buffers are too small - reallocate */ - handle_buffer_resizing( ssl, 0, MBEDTLS_SSL_IN_BUFFER_LEN, - MBEDTLS_SSL_OUT_BUFFER_LEN ); + handle_buffer_resizing( ssl, BUFFER_UPSIZING, MBEDTLS_SSL_IN_BUFFER_LEN, + MBEDTLS_SSL_OUT_BUFFER_LEN ); #endif /* All pointers should exist and can be directly freed without issue */ @@ -12066,8 +12068,9 @@ void mbedtls_ssl_handshake_free( mbedtls_ssl_context *ssl ) * processes datagrams and the fact that a datagram is allowed to have * several records in it, it is possible that the I/O buffers are not * empty at this stage */ - handle_buffer_resizing( ssl, 1, mbedtls_ssl_get_input_buflen( ssl ), - mbedtls_ssl_get_output_buflen( ssl ) ); + handle_buffer_resizing( ssl, BUFFER_DOWNSIZING, + mbedtls_ssl_get_input_buflen( ssl ), + mbedtls_ssl_get_output_buflen( ssl ) ); #endif } From 2e49d079d606893314c7a2269f0289f6e72c0ac9 Mon Sep 17 00:00:00 2001 From: Andrzej Kurek Date: Thu, 22 Oct 2020 11:16:25 +0200 Subject: [PATCH 3/3] Describe the behaviour of buffer resizing on an out-of-memory error Signed-off-by: Andrzej Kurek --- include/mbedtls/config.h | 6 +++++- library/ssl_tls.c | 4 ++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/include/mbedtls/config.h b/include/mbedtls/config.h index 5534ec4f8..cf7f4033a 100644 --- a/include/mbedtls/config.h +++ b/include/mbedtls/config.h @@ -1912,7 +1912,11 @@ /** * \def MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH * - * Enable modifying the maximum I/O buffer size. + * Enable modifying the maximum I/O buffer size in runtime. + * + * If the library runs out of memory during the resizing of an I/O buffer, + * there is no error returned. The operation continues as usual on an + * unchanged buffer without any negative impact on the flow. */ //#define MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH diff --git a/library/ssl_tls.c b/library/ssl_tls.c index eaa0f6e56..eb212d2ca 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -685,6 +685,8 @@ static void handle_buffer_resizing( mbedtls_ssl_context *ssl, int downsizing, { if( resize_buffer( &ssl->in_buf, in_buf_new_len, &ssl->in_buf_len ) != 0 ) { + /* No need to return an error here; The buffer will remain as + * is with no negative impact on the flow. */ MBEDTLS_SSL_DEBUG_MSG( 1, ( "input buffer resizing failed - out of memory" ) ); } else @@ -705,6 +707,8 @@ static void handle_buffer_resizing( mbedtls_ssl_context *ssl, int downsizing, { if( resize_buffer( &ssl->out_buf, out_buf_new_len, &ssl->out_buf_len ) != 0 ) { + /* No need to return an error here; The buffer will remain as + * is with no negative impact on the flow. */ MBEDTLS_SSL_DEBUG_MSG( 1, ( "output buffer resizing failed - out of memory" ) ); } else