From e07bc2015540b3b7e724971fa36bf3fffd556ddb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 26 Feb 2020 09:53:42 +0100 Subject: [PATCH 01/17] Fix compile errors with MBEDTLS_SSL_HW_RECORD_ACCEL MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- library/ssl_msg.c | 23 +++++++++++++++++------ library/ssl_tls.c | 4 ++-- 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/library/ssl_msg.c b/library/ssl_msg.c index 9c2d61509..7aa4ba93c 100644 --- a/library/ssl_msg.c +++ b/library/ssl_msg.c @@ -2032,7 +2032,7 @@ void mbedtls_ssl_flight_free( mbedtls_ssl_flight_item *flight ) /* * Swap transform_out and out_ctr with the alternative ones */ -static void ssl_swap_epochs( mbedtls_ssl_context *ssl ) +static int ssl_swap_epochs( mbedtls_ssl_context *ssl ) { mbedtls_ssl_transform *tmp_transform; unsigned char tmp_out_ctr[8]; @@ -2040,7 +2040,7 @@ static void ssl_swap_epochs( mbedtls_ssl_context *ssl ) if( ssl->transform_out == ssl->handshake->alt_transform_out ) { MBEDTLS_SSL_DEBUG_MSG( 3, ( "skip swap epochs" ) ); - return; + return( 0 ); } MBEDTLS_SSL_DEBUG_MSG( 3, ( "swap epochs" ) ); @@ -2061,13 +2061,16 @@ static void ssl_swap_epochs( mbedtls_ssl_context *ssl ) #if defined(MBEDTLS_SSL_HW_RECORD_ACCEL) if( mbedtls_ssl_hw_record_activate != NULL ) { - if( ( ret = mbedtls_ssl_hw_record_activate( ssl, MBEDTLS_SSL_CHANNEL_OUTBOUND ) ) != 0 ) + int ret = mbedtls_ssl_hw_record_activate( ssl, MBEDTLS_SSL_CHANNEL_OUTBOUND ); + if( ret != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_hw_record_activate", ret ); return( MBEDTLS_ERR_SSL_HW_ACCEL_FAILED ); } } #endif + + return( 0 ); } /* @@ -2104,7 +2107,9 @@ int mbedtls_ssl_flight_transmit( mbedtls_ssl_context *ssl ) ssl->handshake->cur_msg = ssl->handshake->flight; ssl->handshake->cur_msg_p = ssl->handshake->flight->p + 12; - ssl_swap_epochs( ssl ); + ret = ssl_swap_epochs( ssl ); + if( ret != 0 ) + return( ret ); ssl->handshake->retransmit_state = MBEDTLS_SSL_RETRANS_SENDING; } @@ -2127,7 +2132,9 @@ int mbedtls_ssl_flight_transmit( mbedtls_ssl_context *ssl ) if( is_finished && ssl->handshake->cur_msg_p == ( cur->p + 12 ) ) { MBEDTLS_SSL_DEBUG_MSG( 2, ( "swap epochs to send finished message" ) ); - ssl_swap_epochs( ssl ); + ret = ssl_swap_epochs( ssl ); + if( ret != 0 ) + return( ret ); } ret = ssl_get_remaining_payload_in_datagram( ssl ); @@ -2164,7 +2171,11 @@ int mbedtls_ssl_flight_transmit( mbedtls_ssl_context *ssl ) if( ( max_frag_len < 12 ) || ( max_frag_len == 12 && hs_len != 0 ) ) { if( is_finished ) - ssl_swap_epochs( ssl ); + { + ret = ssl_swap_epochs( ssl ); + if( ret != 0 ) + return( ret ); + } if( ( ret = mbedtls_ssl_flush_output( ssl ) ) != 0 ) return( ret ); diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 63bc5c850..d81d1e1e6 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -804,7 +804,7 @@ typedef int ssl_tls_prf_t(const unsigned char *, size_t, const char *, * - [in] minor_ver: SSL/TLS minor version * - [in] endpoint: client or server * - [in] ssl: optionally used for: - * - MBEDTLS_SSL_HW_RECORD_ACCEL: whole context + * - MBEDTLS_SSL_HW_RECORD_ACCEL: whole context (non-const) * - MBEDTLS_SSL_EXPORT_KEYS: ssl->conf->{f,p}_export_keys * - MBEDTLS_DEBUG_C: ssl->conf->{f,p}_dbg */ @@ -826,7 +826,7 @@ static int ssl_populate_transform( mbedtls_ssl_transform *transform, const unsigned char randbytes[64], int minor_ver, unsigned endpoint, - const mbedtls_ssl_context *ssl ) + mbedtls_ssl_context *ssl ) { int ret = 0; #if defined(MBEDTLS_USE_PSA_CRYPTO) From dd8807f52d0279bab6a7bd4b3d44b8154474613a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 26 Feb 2020 09:56:27 +0100 Subject: [PATCH 02/17] Add build with MBEDTLS_SSL_HW_RECORD_ACCEL to all.sh MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- tests/scripts/all.sh | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 2ade64dac..105f1bcde 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -1380,6 +1380,12 @@ component_build_armcc () { armc6_build_test "--target=aarch64-arm-none-eabi -march=armv8.2-a" } +component_build_ssl_hw_record_accel() { + msg "build: default config with MBEDTLS_SSL_HW_RECORD_ACCEL enabled" + scripts/config.pl set MBEDTLS_SSL_HW_RECORD_ACCEL + make CFLAGS='-Werror -O1' +} + component_test_allow_sha1 () { msg "build: allow SHA1 in certificates by default" scripts/config.py set MBEDTLS_TLS_DEFAULT_ALLOW_SHA1_IN_CERTIFICATES From 830540b5980500a4968fd81adb13d6425d218417 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 26 Feb 2020 09:58:26 +0100 Subject: [PATCH 03/17] Add ChangeLog entry for SSL_HW_RECORD_ACCEL fix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- ChangeLog | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ChangeLog b/ChangeLog index 3f2f59371..fb850d8e3 100644 --- a/ChangeLog +++ b/ChangeLog @@ -6,6 +6,10 @@ New deprecations * Deprecate MBEDTLS_SSL_HW_RECORD_ACCEL that enables function hooks in the SSL module for hardware acceleration of individual records. +Bugfix + * Fix compilation failure when both MBEDTLS_SSL_PROTO_DTLS and + MBEDTLS_SSL_HW_RECORD_ACCEL are enabled. + = mbed TLS 2.21.0 branch released 2020-02-20 New deprecations From 033c42a90bf66c3fe1755fe606f751ed6b18d559 Mon Sep 17 00:00:00 2001 From: Andrzej Kurek Date: Tue, 3 Mar 2020 05:57:59 -0500 Subject: [PATCH 04/17] Fix I/O buffer sizes when MBEDTLS_SSL_DTLS_CONNECTION_ID is enabled Fix a typo that made the buffer sizes smaller with CID enabled. Fixes #3077. Signed-off-by: Andrzej Kurek --- include/mbedtls/ssl_internal.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/mbedtls/ssl_internal.h b/include/mbedtls/ssl_internal.h index 6332d148f..ba4a60815 100644 --- a/include/mbedtls/ssl_internal.h +++ b/include/mbedtls/ssl_internal.h @@ -238,7 +238,7 @@ implicit sequence number. */ #define MBEDTLS_SSL_HEADER_LEN 13 -#if defined(MBEDTLS_SSL_DTLS_CONNECTION_ID) +#if !defined(MBEDTLS_SSL_DTLS_CONNECTION_ID) #define MBEDTLS_SSL_IN_BUFFER_LEN \ ( ( MBEDTLS_SSL_HEADER_LEN ) + ( MBEDTLS_SSL_IN_PAYLOAD_LEN ) ) #else @@ -247,7 +247,7 @@ + ( MBEDTLS_SSL_CID_IN_LEN_MAX ) ) #endif -#if defined(MBEDTLS_SSL_DTLS_CONNECTION_ID) +#if !defined(MBEDTLS_SSL_DTLS_CONNECTION_ID) #define MBEDTLS_SSL_OUT_BUFFER_LEN \ ( ( MBEDTLS_SSL_HEADER_LEN ) + ( MBEDTLS_SSL_OUT_PAYLOAD_LEN ) ) #else From b33cc7688ee7857a49b53c45f09450246b7e4e7e Mon Sep 17 00:00:00 2001 From: Darryl Green Date: Thu, 28 Nov 2019 14:29:44 +0000 Subject: [PATCH 05/17] Add I/O buffer length fields to mbedtls_ssl_context Signed-off-by: Andrzej Kurek Signed-off-by: Darryl Green --- include/mbedtls/config.h | 7 +++++ include/mbedtls/ssl.h | 6 ++++ library/ssl_msg.c | 55 ++++++++++++++++++++++++++---------- library/ssl_tls.c | 43 ++++++++++++++++++++++------ library/version_features.c | 3 ++ programs/test/query_config.c | 8 ++++++ 6 files changed, 99 insertions(+), 23 deletions(-) diff --git a/include/mbedtls/config.h b/include/mbedtls/config.h index 70a97405a..ac03884a7 100644 --- a/include/mbedtls/config.h +++ b/include/mbedtls/config.h @@ -1779,6 +1779,13 @@ */ #define MBEDTLS_SSL_TRUNCATED_HMAC +/** + * \def MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH + * + * Enable modifying the maximum I/O buffer size. + */ +//#define MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH + /** * \def MBEDTLS_SSL_TRUNCATED_HMAC_COMPAT * diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 1666f8c86..b3c5288c0 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -1215,6 +1215,9 @@ struct mbedtls_ssl_context int in_msgtype; /*!< record header: message type */ size_t in_msglen; /*!< record header: message length */ size_t in_left; /*!< amount of data read so far */ +#if defined(MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH) + size_t in_buf_len; /*!< length of input buffer */ +#endif #if defined(MBEDTLS_SSL_PROTO_DTLS) uint16_t in_epoch; /*!< DTLS epoch for incoming records */ size_t next_record_offset; /*!< offset of the next record in datagram @@ -1254,6 +1257,9 @@ struct mbedtls_ssl_context int out_msgtype; /*!< record header: message type */ size_t out_msglen; /*!< record header: message length */ size_t out_left; /*!< amount of data not yet written */ +#if defined(MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH) + size_t out_buf_len; /*!< length of output buffer */ +#endif unsigned char cur_out_ctr[8]; /*!< Outgoing record sequence number. */ diff --git a/library/ssl_msg.c b/library/ssl_msg.c index 9c2d61509..df0d0fcad 100644 --- a/library/ssl_msg.c +++ b/library/ssl_msg.c @@ -179,11 +179,16 @@ static int ssl_next_record_is_in_datagram( mbedtls_ssl_context *ssl ); static size_t ssl_get_maximum_datagram_size( mbedtls_ssl_context const *ssl ) { size_t mtu = mbedtls_ssl_get_current_mtu( ssl ); +#if defined(MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH) + size_t out_buf_len = ssl->out_buf_len; +#else + size_t out_buf_len = MBEDTLS_SSL_OUT_BUFFER_LEN; +#endif - if( mtu != 0 && mtu < MBEDTLS_SSL_OUT_BUFFER_LEN ) + if( mtu != 0 && mtu < out_buf_len ) return( mtu ); - return( MBEDTLS_SSL_OUT_BUFFER_LEN ); + return( out_buf_len ); } static int ssl_get_remaining_space_in_datagram( mbedtls_ssl_context const *ssl ) @@ -1574,6 +1579,11 @@ static int ssl_compress_buf( mbedtls_ssl_context *ssl ) ptrdiff_t bytes_written = ssl->out_msg - ssl->out_buf; size_t len_pre = ssl->out_msglen; unsigned char *msg_pre = ssl->compress_buf; +#if defined(MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH) + size_t out_buf_len = ssl->out_buf_len; +#else + size_t out_buf_len = MBEDTLS_SSL_OUT_BUFFER_LEN; +#endif MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> compress buf" ) ); @@ -1591,7 +1601,7 @@ static int ssl_compress_buf( mbedtls_ssl_context *ssl ) ssl->transform_out->ctx_deflate.next_in = msg_pre; ssl->transform_out->ctx_deflate.avail_in = len_pre; ssl->transform_out->ctx_deflate.next_out = msg_post; - ssl->transform_out->ctx_deflate.avail_out = MBEDTLS_SSL_OUT_BUFFER_LEN - bytes_written; + ssl->transform_out->ctx_deflate.avail_out = out_buf_len - bytes_written; ret = deflate( &ssl->transform_out->ctx_deflate, Z_SYNC_FLUSH ); if( ret != Z_OK ) @@ -1600,7 +1610,7 @@ static int ssl_compress_buf( mbedtls_ssl_context *ssl ) return( MBEDTLS_ERR_SSL_COMPRESSION_FAILED ); } - ssl->out_msglen = MBEDTLS_SSL_OUT_BUFFER_LEN - + ssl->out_msglen = out_buf_len - ssl->transform_out->ctx_deflate.avail_out - bytes_written; MBEDTLS_SSL_DEBUG_MSG( 3, ( "after compression: msglen = %d, ", @@ -1621,6 +1631,11 @@ static int ssl_decompress_buf( mbedtls_ssl_context *ssl ) ptrdiff_t header_bytes = ssl->in_msg - ssl->in_buf; size_t len_pre = ssl->in_msglen; unsigned char *msg_pre = ssl->compress_buf; +#if defined(MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH) + size_t in_buf_len = ssl->in_buf_len; +#else + size_t in_buf_len = MBEDTLS_SSL_IN_BUFFER_LEN; +#endif MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> decompress buf" ) ); @@ -1638,8 +1653,7 @@ static int ssl_decompress_buf( mbedtls_ssl_context *ssl ) ssl->transform_in->ctx_inflate.next_in = msg_pre; ssl->transform_in->ctx_inflate.avail_in = len_pre; ssl->transform_in->ctx_inflate.next_out = msg_post; - ssl->transform_in->ctx_inflate.avail_out = MBEDTLS_SSL_IN_BUFFER_LEN - - header_bytes; + ssl->transform_in->ctx_inflate.avail_out = in_buf_len - header_bytes; ret = inflate( &ssl->transform_in->ctx_inflate, Z_SYNC_FLUSH ); if( ret != Z_OK ) @@ -1648,7 +1662,7 @@ static int ssl_decompress_buf( mbedtls_ssl_context *ssl ) return( MBEDTLS_ERR_SSL_COMPRESSION_FAILED ); } - ssl->in_msglen = MBEDTLS_SSL_IN_BUFFER_LEN - + ssl->in_msglen = in_buf_len - ssl->transform_in->ctx_inflate.avail_out - header_bytes; MBEDTLS_SSL_DEBUG_MSG( 3, ( "after decompression: msglen = %d, ", @@ -1682,6 +1696,11 @@ int mbedtls_ssl_fetch_input( mbedtls_ssl_context *ssl, size_t nb_want ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; size_t len; +#if defined(MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH) + size_t in_buf_len = ssl->in_buf_len; +#else + size_t in_buf_len = MBEDTLS_SSL_IN_BUFFER_LEN; +#endif MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> fetch input" ) ); @@ -1692,7 +1711,7 @@ int mbedtls_ssl_fetch_input( mbedtls_ssl_context *ssl, size_t nb_want ) return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA ); } - if( nb_want > MBEDTLS_SSL_IN_BUFFER_LEN - (size_t)( ssl->in_hdr - ssl->in_buf ) ) + if( nb_want > in_buf_len - (size_t)( ssl->in_hdr - ssl->in_buf ) ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "requesting more data than fits" ) ); return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA ); @@ -1778,7 +1797,7 @@ int mbedtls_ssl_fetch_input( mbedtls_ssl_context *ssl, size_t nb_want ) } else { - len = MBEDTLS_SSL_IN_BUFFER_LEN - ( ssl->in_hdr - ssl->in_buf ); + len = in_buf_len - ( ssl->in_hdr - ssl->in_buf ); if( ssl->state != MBEDTLS_SSL_HANDSHAKE_OVER ) timeout = ssl->handshake->retransmit_timeout; @@ -2523,7 +2542,11 @@ int mbedtls_ssl_write_record( mbedtls_ssl_context *ssl, uint8_t force_flush ) { unsigned i; size_t protected_record_size; - +#if defined(MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH) + size_t out_buf_len = ssl->out_buf_len; +#else + size_t out_buf_len = MBEDTLS_SSL_OUT_BUFFER_LEN; +#endif /* Skip writing the record content type to after the encryption, * as it may change when using the CID extension. */ @@ -2539,8 +2562,7 @@ int mbedtls_ssl_write_record( mbedtls_ssl_context *ssl, uint8_t force_flush ) mbedtls_record rec; rec.buf = ssl->out_iv; - rec.buf_len = MBEDTLS_SSL_OUT_BUFFER_LEN - - ( ssl->out_iv - ssl->out_buf ); + rec.buf_len = out_buf_len - ( ssl->out_iv - ssl->out_buf ); rec.data_len = ssl->out_msglen; rec.data_offset = ssl->out_msg - rec.buf; @@ -4216,7 +4238,11 @@ static int ssl_load_buffered_record( mbedtls_ssl_context *ssl ) unsigned char * rec; size_t rec_len; unsigned rec_epoch; - +#if defined(MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH) + size_t in_buf_len = ssl->in_buf_len; +#else + size_t in_buf_len = MBEDTLS_SSL_IN_BUFFER_LEN; +#endif if( ssl->conf->transport != MBEDTLS_SSL_TRANSPORT_DATAGRAM ) return( 0 ); @@ -4246,8 +4272,7 @@ static int ssl_load_buffered_record( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_DEBUG_MSG( 2, ( "Found buffered record from current epoch - load" ) ); /* Double-check that the record is not too large */ - if( rec_len > MBEDTLS_SSL_IN_BUFFER_LEN - - (size_t)( ssl->in_hdr - ssl->in_buf ) ) + if( rec_len > in_buf_len - (size_t)( ssl->in_hdr - ssl->in_buf ) ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "should never happen" ) ); return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 63bc5c850..60ffa612d 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -3729,6 +3729,8 @@ int mbedtls_ssl_setup( mbedtls_ssl_context *ssl, const mbedtls_ssl_config *conf ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; + size_t in_buf_len = MBEDTLS_SSL_IN_BUFFER_LEN; + size_t out_buf_len = MBEDTLS_SSL_OUT_BUFFER_LEN; ssl->conf = conf; @@ -3739,18 +3741,24 @@ int mbedtls_ssl_setup( mbedtls_ssl_context *ssl, /* Set to NULL in case of an error condition */ ssl->out_buf = NULL; - ssl->in_buf = mbedtls_calloc( 1, MBEDTLS_SSL_IN_BUFFER_LEN ); +#if defined(MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH) + ssl->in_buf_len = in_buf_len; +#endif + ssl->in_buf = mbedtls_calloc( 1, in_buf_len ); if( ssl->in_buf == NULL ) { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "alloc(%d bytes) failed", MBEDTLS_SSL_IN_BUFFER_LEN) ); + MBEDTLS_SSL_DEBUG_MSG( 1, ( "alloc(%d bytes) failed", in_buf_len ) ); ret = MBEDTLS_ERR_SSL_ALLOC_FAILED; goto error; } - ssl->out_buf = mbedtls_calloc( 1, MBEDTLS_SSL_OUT_BUFFER_LEN ); +#if defined(MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH) + ssl->out_buf_len = out_buf_len; +#endif + ssl->out_buf = mbedtls_calloc( 1, out_buf_len ); if( ssl->out_buf == NULL ) { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "alloc(%d bytes) failed", MBEDTLS_SSL_OUT_BUFFER_LEN) ); + MBEDTLS_SSL_DEBUG_MSG( 1, ( "alloc(%d bytes) failed", out_buf_len ) ); ret = MBEDTLS_ERR_SSL_ALLOC_FAILED; goto error; } @@ -3768,6 +3776,10 @@ error: ssl->conf = NULL; +#if defined(MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH) + ssl->in_buf_len = 0; + ssl->out_buf_len = 0; +#endif ssl->in_buf = NULL; ssl->out_buf = NULL; @@ -3796,6 +3808,13 @@ error: int mbedtls_ssl_session_reset_int( mbedtls_ssl_context *ssl, int partial ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; +#if defined(MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH) + size_t in_buf_len = ssl->in_buf_len; + size_t out_buf_len = ssl->out_buf_len; +#else + size_t in_buf_len = MBEDTLS_SSL_IN_BUFFER_LEN; + size_t out_buf_len = MBEDTLS_SSL_OUT_BUFFER_LEN; +#endif #if !defined(MBEDTLS_SSL_DTLS_CLIENT_PORT_REUSE) || \ !defined(MBEDTLS_SSL_SRV_C) @@ -3851,14 +3870,14 @@ int mbedtls_ssl_session_reset_int( mbedtls_ssl_context *ssl, int partial ) ssl->session_in = NULL; ssl->session_out = NULL; - memset( ssl->out_buf, 0, MBEDTLS_SSL_OUT_BUFFER_LEN ); + memset( ssl->out_buf, 0, out_buf_len ); #if defined(MBEDTLS_SSL_DTLS_CLIENT_PORT_REUSE) && defined(MBEDTLS_SSL_SRV_C) if( partial == 0 ) #endif /* MBEDTLS_SSL_DTLS_CLIENT_PORT_REUSE && MBEDTLS_SSL_SRV_C */ { ssl->in_left = 0; - memset( ssl->in_buf, 0, MBEDTLS_SSL_IN_BUFFER_LEN ); + memset( ssl->in_buf, 0, in_buf_len ); } #if defined(MBEDTLS_SSL_HW_RECORD_ACCEL) @@ -6463,6 +6482,14 @@ int mbedtls_ssl_context_load( mbedtls_ssl_context *context, */ void mbedtls_ssl_free( mbedtls_ssl_context *ssl ) { +#if defined(MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH) + size_t in_buf_len = ssl->in_buf_len; + size_t out_buf_len = ssl->out_buf_len; +#else + size_t in_buf_len = MBEDTLS_SSL_IN_BUFFER_LEN; + size_t out_buf_len = MBEDTLS_SSL_OUT_BUFFER_LEN; +#endif + if( ssl == NULL ) return; @@ -6470,13 +6497,13 @@ void mbedtls_ssl_free( mbedtls_ssl_context *ssl ) if( ssl->out_buf != NULL ) { - mbedtls_platform_zeroize( ssl->out_buf, MBEDTLS_SSL_OUT_BUFFER_LEN ); + mbedtls_platform_zeroize( ssl->out_buf, out_buf_len ); mbedtls_free( ssl->out_buf ); } if( ssl->in_buf != NULL ) { - mbedtls_platform_zeroize( ssl->in_buf, MBEDTLS_SSL_IN_BUFFER_LEN ); + mbedtls_platform_zeroize( ssl->in_buf, in_buf_len ); mbedtls_free( ssl->in_buf ); } diff --git a/library/version_features.c b/library/version_features.c index cc47dacc9..7b2914f5f 100644 --- a/library/version_features.c +++ b/library/version_features.c @@ -537,6 +537,9 @@ static const char * const features[] = { #if defined(MBEDTLS_SSL_TRUNCATED_HMAC) "MBEDTLS_SSL_TRUNCATED_HMAC", #endif /* MBEDTLS_SSL_TRUNCATED_HMAC */ +#if defined(MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH) + "MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH", +#endif /* MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH */ #if defined(MBEDTLS_SSL_TRUNCATED_HMAC_COMPAT) "MBEDTLS_SSL_TRUNCATED_HMAC_COMPAT", #endif /* MBEDTLS_SSL_TRUNCATED_HMAC_COMPAT */ diff --git a/programs/test/query_config.c b/programs/test/query_config.c index 71693c787..df532fde0 100644 --- a/programs/test/query_config.c +++ b/programs/test/query_config.c @@ -1474,6 +1474,14 @@ int query_config( const char *config ) } #endif /* MBEDTLS_SSL_TRUNCATED_HMAC */ +#if defined(MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH) + if( strcmp( "MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH", config ) == 0 ) + { + MACRO_EXPANSION_TO_STR( MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH ); + return( 0 ); + } +#endif /* MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH */ + #if defined(MBEDTLS_SSL_TRUNCATED_HMAC_COMPAT) if( strcmp( "MBEDTLS_SSL_TRUNCATED_HMAC_COMPAT", config ) == 0 ) { From aad82f9bbb4dad08e5f678a817e2ad47c2331a90 Mon Sep 17 00:00:00 2001 From: Darryl Green Date: Mon, 2 Dec 2019 10:53:11 +0000 Subject: [PATCH 06/17] Add variable buffer length tests to all.sh Exercise the feature alone, with record splitting and DTLS connection ID. Signed-off-by: Andrzej Kurek Signed-off-by: Darryl Green --- tests/scripts/all.sh | 52 ++++++++++++++++++++++++++++ tests/suites/test_suite_ssl.function | 4 ++- 2 files changed, 55 insertions(+), 1 deletion(-) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 2ade64dac..a6fbb6744 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -1142,6 +1142,58 @@ component_test_no_max_fragment_length_small_ssl_out_content_len () { if_build_succeeded tests/ssl-opt.sh -f "Max fragment length\|Large buffer" } +component_test_variable_ssl_in_out_buffer_len () { + msg "build: MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH enabled (ASan build)" + scripts/config.py set MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH + CC=gcc cmake -D CMAKE_BUILD_TYPE:String=Asan . + make + + msg "test: MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH enabled" + make test + + msg "test: ssl-opt.sh, MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH enabled" + if_build_succeeded tests/ssl-opt.sh + + msg "test: compat.sh, MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH enabled" + if_build_succeeded tests/compat.sh +} + +component_test_variable_ssl_in_out_buffer_len_CID () { + msg "build: MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH and MBEDTLS_SSL_DTLS_CONNECTION_ID enabled (ASan build)" + scripts/config.py set MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH + scripts/config.py set MBEDTLS_SSL_DTLS_CONNECTION_ID + + CC=gcc cmake -D CMAKE_BUILD_TYPE:String=Asan . + make + + msg "test: MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH and MBEDTLS_SSL_DTLS_CONNECTION_ID" + make test + + msg "test: ssl-opt.sh, MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH and MBEDTLS_SSL_DTLS_CONNECTION_ID enabled" + if_build_succeeded tests/ssl-opt.sh + + msg "test: compat.sh, MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH and MBEDTLS_SSL_DTLS_CONNECTION_ID enabled" + if_build_succeeded tests/compat.sh +} + +component_test_variable_ssl_in_out_buffer_len_record_splitting () { + msg "build: MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH and MBEDTLS_SSL_CBC_RECORD_SPLITTING enabled (ASan build)" + scripts/config.py set MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH + scripts/config.py set MBEDTLS_SSL_CBC_RECORD_SPLITTING + + CC=gcc cmake -D CMAKE_BUILD_TYPE:String=Asan . + make + + msg "test: MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH and MBEDTLS_SSL_CBC_RECORD_SPLITTING" + make test + + msg "test: ssl-opt.sh, MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH and MBEDTLS_SSL_CBC_RECORD_SPLITTING enabled" + if_build_succeeded tests/ssl-opt.sh + + msg "test: compat.sh, MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH and MBEDTLS_SSL_CBC_RECORD_SPLITTING enabled" + if_build_succeeded tests/compat.sh +} + component_test_when_no_ciphersuites_have_mac () { msg "build: when no ciphersuites have MAC" scripts/config.py unset MBEDTLS_CIPHER_NULL_CIPHER diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index 44e222741..5485d9e68 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -3746,7 +3746,7 @@ void handshake_serialization( ) } /* END_CASE */ -/* BEGIN_CASE depends_on:MBEDTLS_X509_CRT_PARSE_C:!MBEDTLS_USE_PSA_CRYPTO:MBEDTLS_PKCS1_V15:MBEDTLS_RSA_C:MBEDTLS_ECP_DP_SECP384R1_ENABLED:MBEDTLS_DEBUG_C:MBEDTLS_SSL_MAX_FRAGMENT_LENGTH */ +/* BEGIN_CASE depends_on:MBEDTLS_X509_CRT_PARSE_C:!MBEDTLS_USE_PSA_CRYPTO:MBEDTLS_PKCS1_V15:MBEDTLS_RSA_C:MBEDTLS_ECP_DP_SECP384R1_ENABLED:MBEDTLS_DEBUG_C:MBEDTLS_SSL_MAX_FRAGMENT_LENGTH:MBEDTLS_CIPHER_MODE_CBC */ void handshake_fragmentation( int mfl, int expected_srv_hs_fragmentation, int expected_cli_hs_fragmentation) { handshake_test_options options; @@ -3759,6 +3759,8 @@ void handshake_fragmentation( int mfl, int expected_srv_hs_fragmentation, int ex init_handshake_options( &options ); options.dtls = 1; options.mfl = mfl; + /* Set cipher to one using CBC so that record splitting can be tested */ + options.cipher = "TLS-DHE-RSA-WITH-AES-256-CBC-SHA256"; options.srv_auth_mode = MBEDTLS_SSL_VERIFY_REQUIRED; options.srv_log_obj = &srv_pattern; options.cli_log_obj = &cli_pattern; From 0afa2a1b659a73abb14a1e053f76f010ba5a8849 Mon Sep 17 00:00:00 2001 From: Andrzej Kurek Date: Tue, 3 Mar 2020 10:39:58 -0500 Subject: [PATCH 07/17] Add I/O buffer resizing in handshake init and free Add a conditional buffer resizing feature. Introduce tests exercising it in various setups (serialization, renegotiation, mfl manipulations). Signed-off-by: Andrzej Kurek --- include/mbedtls/ssl_internal.h | 26 ++++++ library/ssl_tls.c | 116 ++++++++++++++++++++++++++ tests/suites/test_suite_ssl.data | 48 +++++++++++ tests/suites/test_suite_ssl.function | 119 +++++++++++++++++++++++++++ 4 files changed, 309 insertions(+) diff --git a/include/mbedtls/ssl_internal.h b/include/mbedtls/ssl_internal.h index ba4a60815..472098823 100644 --- a/include/mbedtls/ssl_internal.h +++ b/include/mbedtls/ssl_internal.h @@ -256,6 +256,32 @@ + ( MBEDTLS_SSL_CID_OUT_LEN_MAX ) ) #endif +#if defined(MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH) +static inline uint32_t mbedtls_ssl_get_output_buflen( const mbedtls_ssl_context *ctx ) +{ +#if defined (MBEDTLS_SSL_DTLS_CONNECTION_ID) + return (uint32_t) mbedtls_ssl_get_max_frag_len( ctx ) + + MBEDTLS_SSL_HEADER_LEN + MBEDTLS_SSL_PAYLOAD_OVERHEAD + + MBEDTLS_SSL_CID_OUT_LEN_MAX; +#else + return (uint32_t) mbedtls_ssl_get_max_frag_len( ctx ) + + MBEDTLS_SSL_HEADER_LEN + MBEDTLS_SSL_PAYLOAD_OVERHEAD; +#endif +} + +static inline uint32_t mbedtls_ssl_get_input_buflen( const mbedtls_ssl_context *ctx ) +{ +#if defined (MBEDTLS_SSL_DTLS_CONNECTION_ID) + return (uint32_t) mbedtls_ssl_get_max_frag_len( ctx ) + + MBEDTLS_SSL_HEADER_LEN + MBEDTLS_SSL_PAYLOAD_OVERHEAD + + MBEDTLS_SSL_CID_IN_LEN_MAX; +#else + return (uint32_t) mbedtls_ssl_get_max_frag_len( ctx ) + + MBEDTLS_SSL_HEADER_LEN + MBEDTLS_SSL_PAYLOAD_OVERHEAD; +#endif +} +#endif + #ifdef MBEDTLS_ZLIB_SUPPORT /* Compression buffer holds both IN and OUT buffers, so should be size of the larger */ #define MBEDTLS_SSL_COMPRESS_BUFFER_LEN ( \ diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 60ffa612d..db2cdb63a 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -245,6 +245,29 @@ int mbedtls_ssl_session_copy( mbedtls_ssl_session *dst, return( 0 ); } +#if defined(MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH) +static int resize_buffer( unsigned char **buffer, size_t len_new, size_t *len_old ) +{ + unsigned char* resized_buffer = mbedtls_calloc( 1, len_new ); + if( resized_buffer == NULL ) + return -1; + + /* We want to copy len_new bytes when downsizing the buffer, and + * len_old bytes when upsizing, so we choose the smaller of two sizes, + * to fit one buffer into another. Size checks, ensuring that no data is + * lost, are done outside of this function. */ + memcpy( resized_buffer, *buffer, + ( len_new < *len_old ) ? len_new : *len_old ); + mbedtls_platform_zeroize( *buffer, *len_old ); + mbedtls_free( *buffer ); + + *buffer = resized_buffer; + *len_old = len_new; + + return 0; +} +#endif /* MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH */ + /* * Key material generation */ @@ -3643,6 +3666,43 @@ static int ssl_handshake_init( mbedtls_ssl_context *ssl ) { ssl->handshake = mbedtls_calloc( 1, sizeof(mbedtls_ssl_handshake_params) ); } +#if defined(MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH) + /* If the buffers are too small - reallocate */ + { + int modified = 0; + 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_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. */ + mbedtls_ssl_reset_in_out_pointers( ssl ); + } + } +#endif /* All pointers should exist and can be directly freed without issue */ if( ssl->handshake == NULL || @@ -5818,6 +5878,60 @@ void mbedtls_ssl_handshake_free( mbedtls_ssl_context *ssl ) mbedtls_platform_zeroize( handshake, sizeof( mbedtls_ssl_handshake_params ) ); + +#if defined(MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH) + /* If the buffers are too big - reallocate. Because of the way Mbed TLS + * 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; + size_t written_out = 0; + if( ssl->in_buf != NULL && + ssl->in_buf_len > buf_len && + ssl->in_left < buf_len ) + { + written_in = ssl->in_msg - ssl->in_buf; + 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 && + ssl->out_buf_len > mbedtls_ssl_get_output_buflen( ssl ) && + ssl->out_left < buf_len ) + { + written_out = ssl->out_msg - ssl->out_buf; + 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. */ + mbedtls_ssl_reset_in_out_pointers( ssl ); + /* Fields below might not be properly updated with record + * splitting, so they are manually updated here. */ + ssl->out_msg = ssl->out_buf + written_out; + ssl->in_msg = ssl->in_buf + written_in; + } + } +#endif } void mbedtls_ssl_session_free( mbedtls_ssl_session *session ) @@ -6499,12 +6613,14 @@ void mbedtls_ssl_free( mbedtls_ssl_context *ssl ) { mbedtls_platform_zeroize( ssl->out_buf, out_buf_len ); mbedtls_free( ssl->out_buf ); + ssl->out_buf = NULL; } if( ssl->in_buf != NULL ) { mbedtls_platform_zeroize( ssl->in_buf, in_buf_len ); mbedtls_free( ssl->in_buf ); + ssl->in_buf = NULL; } #if defined(MBEDTLS_ZLIB_SUPPORT) diff --git a/tests/suites/test_suite_ssl.data b/tests/suites/test_suite_ssl.data index 39a9d54ff..1a241055f 100644 --- a/tests/suites/test_suite_ssl.data +++ b/tests/suites/test_suite_ssl.data @@ -368,6 +368,54 @@ renegotiation:MBEDTLS_SSL_LEGACY_ALLOW_RENEGOTIATION DTLS renegotiation: legacy break handshake renegotiation:MBEDTLS_SSL_LEGACY_BREAK_HANDSHAKE +DTLS serialization with MFL=512 +resize_buffers_serialize_mfl:MBEDTLS_SSL_MAX_FRAG_LEN_512 + +DTLS serialization with MFL=1024 +resize_buffers_serialize_mfl:MBEDTLS_SSL_MAX_FRAG_LEN_1024 + +DTLS serialization with MFL=2048 +resize_buffers_serialize_mfl:MBEDTLS_SSL_MAX_FRAG_LEN_2048 + +DTLS serialization with MFL=4096 +resize_buffers_serialize_mfl:MBEDTLS_SSL_MAX_FRAG_LEN_4096 + +DTLS no legacy renegotiation with MFL=512 +resize_buffers_renegotiate_mfl:MBEDTLS_SSL_MAX_FRAG_LEN_512:MBEDTLS_SSL_LEGACY_NO_RENEGOTIATION + +DTLS no legacy renegotiation with MFL=1024 +resize_buffers_renegotiate_mfl:MBEDTLS_SSL_MAX_FRAG_LEN_1024:MBEDTLS_SSL_LEGACY_NO_RENEGOTIATION + +DTLS no legacy renegotiation with MFL=2048 +resize_buffers_renegotiate_mfl:MBEDTLS_SSL_MAX_FRAG_LEN_2048:MBEDTLS_SSL_LEGACY_NO_RENEGOTIATION + +DTLS no legacy renegotiation with MFL=4096 +resize_buffers_renegotiate_mfl:MBEDTLS_SSL_MAX_FRAG_LEN_4096:MBEDTLS_SSL_LEGACY_NO_RENEGOTIATION + +DTLS legacy allow renegotiation with MFL=512 +resize_buffers_renegotiate_mfl:MBEDTLS_SSL_MAX_FRAG_LEN_512:MBEDTLS_SSL_LEGACY_ALLOW_RENEGOTIATION + +DTLS legacy allow renegotiation with MFL=1024 +resize_buffers_renegotiate_mfl:MBEDTLS_SSL_MAX_FRAG_LEN_1024:MBEDTLS_SSL_LEGACY_ALLOW_RENEGOTIATION + +DTLS legacy allow renegotiation with MFL=2048 +resize_buffers_renegotiate_mfl:MBEDTLS_SSL_MAX_FRAG_LEN_2048:MBEDTLS_SSL_LEGACY_ALLOW_RENEGOTIATION + +DTLS legacy allow renegotiation with MFL=4096 +resize_buffers_renegotiate_mfl:MBEDTLS_SSL_MAX_FRAG_LEN_4096:MBEDTLS_SSL_LEGACY_ALLOW_RENEGOTIATION + +DTLS legacy break handshake renegotiation with MFL=512 +resize_buffers_renegotiate_mfl:MBEDTLS_SSL_MAX_FRAG_LEN_512:MBEDTLS_SSL_LEGACY_BREAK_HANDSHAKE + +DTLS legacy break handshake renegotiation with MFL=1024 +resize_buffers_renegotiate_mfl:MBEDTLS_SSL_MAX_FRAG_LEN_1024:MBEDTLS_SSL_LEGACY_BREAK_HANDSHAKE + +DTLS legacy break handshake renegotiation with MFL=2048 +resize_buffers_renegotiate_mfl:MBEDTLS_SSL_MAX_FRAG_LEN_2048:MBEDTLS_SSL_LEGACY_BREAK_HANDSHAKE + +DTLS legacy break handshake renegotiation with MFL=4096 +resize_buffers_renegotiate_mfl:MBEDTLS_SSL_MAX_FRAG_LEN_4096:MBEDTLS_SSL_LEGACY_BREAK_HANDSHAKE + SSL DTLS replay: initial state, seqnum 0 ssl_dtls_replay:"":"000000000000":0 diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index 5485d9e68..9b61a502d 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -55,6 +55,7 @@ typedef struct handshake_test_options void *cli_log_obj; void (*srv_log_fun)(void *, int, const char *, int, const char *); void (*cli_log_fun)(void *, int, const char *, int, const char *); + int resize_buffers; } handshake_test_options; void init_handshake_options( handshake_test_options *opts ) @@ -77,6 +78,7 @@ void init_handshake_options( handshake_test_options *opts ) opts->srv_log_obj = NULL; opts->srv_log_fun = NULL; opts->cli_log_fun = NULL; + opts->resize_buffers = 1; } /* * Buffer structure for custom I/O callbacks. @@ -1767,6 +1769,17 @@ void perform_handshake( handshake_test_options* options ) &(server.socket), BUFFSIZE ) == 0 ); +#if defined(MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH) + if( options->resize_buffers != 0 ) + { + /* Ensure that the buffer sizes are appropriate before resizes */ + TEST_ASSERT( client.ssl.out_buf_len == MBEDTLS_SSL_OUT_BUFFER_LEN ); + TEST_ASSERT( client.ssl.in_buf_len == MBEDTLS_SSL_IN_BUFFER_LEN ); + TEST_ASSERT( server.ssl.out_buf_len == MBEDTLS_SSL_OUT_BUFFER_LEN ); + TEST_ASSERT( server.ssl.in_buf_len == MBEDTLS_SSL_IN_BUFFER_LEN ); + } +#endif + TEST_ASSERT( mbedtls_move_handshake_to_state( &(client.ssl), &(server.ssl), MBEDTLS_SSL_HANDSHAKE_OVER ) @@ -1774,6 +1787,31 @@ void perform_handshake( handshake_test_options* options ) TEST_ASSERT( client.ssl.state == MBEDTLS_SSL_HANDSHAKE_OVER ); TEST_ASSERT( server.ssl.state == MBEDTLS_SSL_HANDSHAKE_OVER ); +#if defined(MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH) + if( options->resize_buffers != 0 ) + { + /* Note - the case below will have to updated, since due to a 1n-1 + * split against BEAST the fragment count is different + * than expected when preparing the fragment counting code. */ + if( options->version != MBEDTLS_SSL_MINOR_VERSION_0 && + options->version != MBEDTLS_SSL_MINOR_VERSION_1 ) + { + /* A server, when using DTLS, might delay a buffer resize to happen + * after it receives a message, so we force it. */ + TEST_ASSERT( exchange_data( &(client.ssl), &(server.ssl) ) == 0 ); + + TEST_ASSERT( client.ssl.out_buf_len == + mbedtls_ssl_get_output_buflen( &client.ssl ) ); + TEST_ASSERT( client.ssl.in_buf_len == + mbedtls_ssl_get_input_buflen( &client.ssl ) ); + TEST_ASSERT( server.ssl.out_buf_len == + mbedtls_ssl_get_output_buflen( &server.ssl ) ); + TEST_ASSERT( server.ssl.in_buf_len == + mbedtls_ssl_get_input_buflen( &server.ssl ) ); + } + } +#endif + if( options->cli_msg_len != 0 || options->srv_msg_len != 0 ) { /* Start data exchanging test */ @@ -1813,10 +1851,28 @@ void perform_handshake( handshake_test_options* options ) mbedtls_ssl_set_timer_cb( &server.ssl, &timer_server, mbedtls_timing_set_delay, mbedtls_timing_get_delay ); +#endif +#if defined(MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH) + if( options->resize_buffers != 0 ) + { + /* Ensure that the buffer sizes are appropriate before resizes */ + TEST_ASSERT( server.ssl.out_buf_len == MBEDTLS_SSL_OUT_BUFFER_LEN ); + TEST_ASSERT( server.ssl.in_buf_len == MBEDTLS_SSL_IN_BUFFER_LEN ); + } #endif TEST_ASSERT( mbedtls_ssl_context_load( &( server.ssl ), context_buf, context_buf_len ) == 0 ); +#if defined(MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH) + /* Validate buffer sizes after context deserialization */ + if( options->resize_buffers != 0 ) + { + TEST_ASSERT( server.ssl.out_buf_len == + mbedtls_ssl_get_output_buflen( &server.ssl ) ); + TEST_ASSERT( server.ssl.in_buf_len == + mbedtls_ssl_get_input_buflen( &server.ssl ) ); + } +#endif /* Retest writing/reading */ if( options->cli_msg_len != 0 || options->srv_msg_len != 0 ) { @@ -1830,6 +1886,7 @@ void perform_handshake( handshake_test_options* options ) } } #endif /* MBEDTLS_SSL_CONTEXT_SERIALIZATION */ + #if defined(MBEDTLS_SSL_RENEGOTIATION) if( options->renegotiate ) { @@ -1859,6 +1916,14 @@ void perform_handshake( handshake_test_options* options ) * function will return waiting error on the socket. All rest of * renegotiation should happen during data exchanging */ ret = mbedtls_ssl_renegotiate( &(client.ssl) ); +#if defined(MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH) + if( options->resize_buffers != 0 ) + { + /* Ensure that the buffer sizes are appropriate before resizes */ + TEST_ASSERT( client.ssl.out_buf_len == MBEDTLS_SSL_OUT_BUFFER_LEN ); + TEST_ASSERT( client.ssl.in_buf_len == MBEDTLS_SSL_IN_BUFFER_LEN ); + } +#endif TEST_ASSERT( ret == 0 || ret == MBEDTLS_ERR_SSL_WANT_READ || ret == MBEDTLS_ERR_SSL_WANT_WRITE ); @@ -1872,6 +1937,20 @@ void perform_handshake( handshake_test_options* options ) MBEDTLS_SSL_RENEGOTIATION_DONE ); TEST_ASSERT( client.ssl.renego_status == MBEDTLS_SSL_RENEGOTIATION_DONE ); +#if defined(MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH) + /* Validate buffer sizes after renegotiation */ + if( options->resize_buffers != 0 ) + { + TEST_ASSERT( client.ssl.out_buf_len == + mbedtls_ssl_get_output_buflen( &client.ssl ) ); + TEST_ASSERT( client.ssl.in_buf_len == + mbedtls_ssl_get_input_buflen( &client.ssl ) ); + TEST_ASSERT( server.ssl.out_buf_len == + mbedtls_ssl_get_output_buflen( &server.ssl ) ); + TEST_ASSERT( server.ssl.in_buf_len == + mbedtls_ssl_get_input_buflen( &server.ssl ) ); + } +#endif /* MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH */ } #endif /* MBEDTLS_SSL_RENEGOTIATION */ @@ -3797,3 +3876,43 @@ void renegotiation( int legacy_renegotiation ) goto exit; } /* END_CASE */ + +/* BEGIN_CASE depends_on:MBEDTLS_X509_CRT_PARSE_C:!MBEDTLS_USE_PSA_CRYPTO:MBEDTLS_PKCS1_V15:MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH:MBEDTLS_SSL_PROTO_TLS1_2:MBEDTLS_RSA_C:MBEDTLS_ECP_DP_SECP384R1_ENABLED */ +void resize_buffers( int mfl, int renegotiation, int legacy_renegotiation, + int serialize, int dtls ) +{ + handshake_test_options options; + init_handshake_options( &options ); + + options.mfl = mfl; + options.renegotiate = renegotiation; + options.legacy_renegotiation = legacy_renegotiation; + options.serialize = serialize; + options.dtls = dtls; + options.resize_buffers = 1; + + perform_handshake( &options ); + /* The goto below is used to avoid an "unused label" warning.*/ + goto exit; +} +/* END_CASE */ + +/* BEGIN_CASE depends_on:MBEDTLS_X509_CRT_PARSE_C:!MBEDTLS_USE_PSA_CRYPTO:MBEDTLS_PKCS1_V15:MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH:MBEDTLS_SSL_CONTEXT_SERIALIZATION:MBEDTLS_SSL_PROTO_TLS1_2:MBEDTLS_RSA_C:MBEDTLS_ECP_DP_SECP384R1_ENABLED:MBEDTLS_SSL_PROTO_DTLS */ +void resize_buffers_serialize_mfl( int mfl ) +{ + test_resize_buffers( mfl, 0, MBEDTLS_SSL_LEGACY_NO_RENEGOTIATION, 1, 1 ); + + /* The goto below is used to avoid an "unused label" warning.*/ + goto exit; +} +/* END_CASE */ + +/* BEGIN_CASE depends_on:MBEDTLS_X509_CRT_PARSE_C:!MBEDTLS_USE_PSA_CRYPTO:MBEDTLS_PKCS1_V15:MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH:MBEDTLS_SSL_RENEGOTIATION:MBEDTLS_SSL_PROTO_TLS1_2:MBEDTLS_RSA_C:MBEDTLS_ECP_DP_SECP384R1_ENABLED */ +void resize_buffers_renegotiate_mfl( int mfl, int legacy_renegotiation ) +{ + test_resize_buffers( mfl, 1, legacy_renegotiation, 0, 1 ); + + /* The goto below is used to avoid an "unused label" warning.*/ + goto exit; +} +/* END_CASE */ From 0937ed29b9358542c0e273fa659dca7bb76269a9 Mon Sep 17 00:00:00 2001 From: Piotr Nowicki Date: Tue, 26 Nov 2019 16:32:40 +0100 Subject: [PATCH 08/17] Add an acceptance test for memory usage after handshake Signed-off-by: Piotr Nowicki Signed-off-by: Andrzej Kurek --- programs/ssl/ssl_server2.c | 12 ++++- tests/scripts/all.sh | 17 +++++++ tests/ssl-opt.sh | 99 +++++++++++++++++++++++++++++++++++++- 3 files changed, 126 insertions(+), 2 deletions(-) diff --git a/programs/ssl/ssl_server2.c b/programs/ssl/ssl_server2.c index d23a700f8..13031d5d5 100644 --- a/programs/ssl/ssl_server2.c +++ b/programs/ssl/ssl_server2.c @@ -1838,7 +1838,10 @@ int main( int argc, char *argv[] ) #if defined(MBEDTLS_MEMORY_BUFFER_ALLOC_C) mbedtls_memory_buffer_alloc_init( alloc_buf, sizeof(alloc_buf) ); -#endif +#if defined(MBEDTLS_MEMORY_DEBUG) + size_t current_heap_memory, peak_heap_memory, heap_blocks; +#endif /* MBEDTLS_MEMORY_DEBUG */ +#endif /* MBEDTLS_MEMORY_BUFFER_ALLOC_C */ /* * Make sure memory references are valid in case we exit early. @@ -3742,6 +3745,13 @@ handshake: } #endif /* MBEDTLS_SSL_DTLS_CONNECTION_ID */ +#if defined(MBEDTLS_MEMORY_DEBUG) + mbedtls_memory_buffer_alloc_cur_get( ¤t_heap_memory, &heap_blocks ); + mbedtls_memory_buffer_alloc_max_get( &peak_heap_memory, &heap_blocks ); + mbedtls_printf( "Heap memory usage after handshake: %lu bytes. Peak memory usage was %lu\n", + (unsigned long) current_heap_memory, (unsigned long) peak_heap_memory ); +#endif /* MBEDTLS_MEMORY_DEBUG */ + if( opt.exchanges == 0 ) goto close_notify; diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index a6fbb6744..9246eaeaf 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -1194,6 +1194,23 @@ component_test_variable_ssl_in_out_buffer_len_record_splitting () { if_build_succeeded tests/compat.sh } +component_test_ssl_alloc_buffer_and_mfl () { + msg "build: default config with memory buffer allocator and MFL extension" + scripts/config.py set MBEDTLS_MEMORY_BUFFER_ALLOC_C + scripts/config.py set MBEDTLS_PLATFORM_MEMORY + scripts/config.py set MBEDTLS_MEMORY_DEBUG + scripts/config.py set MBEDTLS_SSL_MAX_FRAGMENT_LENGTH + scripts/config.py set MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH + CC=gcc cmake . + make + + msg "test: MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH, MBEDTLS_MEMORY_BUFFER_ALLOC_C, MBEDTLS_MEMORY_DEBUG and MBEDTLS_SSL_MAX_FRAGMENT_LENGTH" + make test + + msg "test: MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH, MBEDTLS_MEMORY_BUFFER_ALLOC_C, MBEDTLS_MEMORY_DEBUG and MBEDTLS_SSL_MAX_FRAGMENT_LENGTH" + if_build_succeeded tests/ssl-opt.sh -f "Handshake memory usage" +} + component_test_when_no_ciphersuites_have_mac () { msg "build: when no ciphersuites have MAC" scripts/config.py unset MBEDTLS_CIPHER_NULL_CIPHER diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index e65b8eca9..cc6bc13ac 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -223,7 +223,7 @@ requires_config_value_at_most() { } requires_ciphersuite_enabled() { - if [ -z "$($P_CLI --help | grep $1)" ]; then + if [ -z "$($P_CLI --help 2>/dev/null | grep $1)" ]; then SKIP_NEXT="YES" fi } @@ -525,6 +525,45 @@ check_server_hello_time() { fi } +# Get handshake memory usage from server or client output and put it into the variable specified by the first argument +handshake_memory_get() { + OUTPUT_VARIABLE="$1" + OUTPUT_FILE="$2" + + # Get memory usage from a pattern like "Heap memory usage after handshake: 23112 bytes. Peak memory usage was 33112" + MEM_USAGE=$(sed -n 's/.*Heap memory usage after handshake: //p' < "$OUTPUT_FILE" | grep -o "[0-9]*" | head -1) + + # Check if memory usage was read + if [ -z "$MEM_USAGE" ]; then + echo "Error: Can not read the value of handshake memory usage" + return 1 + else + eval "$OUTPUT_VARIABLE=$MEM_USAGE" + return 0 + fi +} + +# Get handshake memory usage from server or client output and check if this value +# is not higher than the maximum given by the first argument +handshake_memory_check() { + MAX_MEMORY="$1" + OUTPUT_FILE="$2" + + # Get memory usage + if ! handshake_memory_get "MEMORY_USAGE" "$OUTPUT_FILE"; then + return 1 + fi + + # Check if memory usage is below max value + if [ "$MEMORY_USAGE" -gt "$MAX_MEMORY" ]; then + echo "\nFailed: Handshake memory usage was $MEMORY_USAGE bytes," \ + "but should be below $MAX_MEMORY bytes" + return 1 + else + return 0 + fi +} + # wait for client to terminate and set CLI_EXIT # must be called right after starting the client wait_client_done() { @@ -865,6 +904,58 @@ run_test_psa_force_curve() { -C "error" } +# Test that the server's memory usage after a handshake is reduced when a client specifies +# a maximum fragment length. +# first argument ($1) is MFL for SSL client +# second argument ($2) is memory usage for SSL client with default MFL (16k) +run_test_memory_after_hanshake_with_mfl() +{ + # The test passes if the difference is around 2*(16k-MFL) + local MEMORY_USAGE_LIMIT="$(( $2 - ( 2 * ( 16384 - $1 )) ))" + + # Leave some margin for robustness + MEMORY_USAGE_LIMIT="$(( ( MEMORY_USAGE_LIMIT * 110 ) / 100 ))" + + run_test "Handshake memory usage (MFL $1)" \ + "$P_SRV debug_level=3 auth_mode=required force_version=tls1_2" \ + "$P_CLI debug_level=3 force_version=tls1_2 \ + crt_file=data_files/server5.crt key_file=data_files/server5.key \ + force_ciphersuite=TLS-ECDHE-ECDSA-WITH-AES-128-CCM max_frag_len=$1" \ + 0 \ + -F "handshake_memory_check $MEMORY_USAGE_LIMIT" +} + + +# Test that the server's memory usage after a handshake is reduced when a client specifies +# different values of Maximum Fragment Length: default (16k), 4k, 2k, 1k and 512 bytes +run_tests_memory_after_hanshake() +{ + # all tests in this sequence requires the same configuration (see requires_config_enabled()) + SKIP_THIS_TESTS="$SKIP_NEXT" + + # first test with default MFU is to get reference memory usage + MEMORY_USAGE_MFL_16K=0 + run_test "Handshake memory usage initial (MFL 16384 - default)" \ + "$P_SRV debug_level=3 auth_mode=required force_version=tls1_2" \ + "$P_CLI debug_level=3 force_version=tls1_2 \ + crt_file=data_files/server5.crt key_file=data_files/server5.key \ + force_ciphersuite=TLS-ECDHE-ECDSA-WITH-AES-128-CCM" \ + 0 \ + -F "handshake_memory_get MEMORY_USAGE_MFL_16K" + + SKIP_NEXT="$SKIP_THIS_TESTS" + run_test_memory_after_hanshake_with_mfl 4096 "$MEMORY_USAGE_MFL_16K" + + SKIP_NEXT="$SKIP_THIS_TESTS" + run_test_memory_after_hanshake_with_mfl 2048 "$MEMORY_USAGE_MFL_16K" + + SKIP_NEXT="$SKIP_THIS_TESTS" + run_test_memory_after_hanshake_with_mfl 1024 "$MEMORY_USAGE_MFL_16K" + + SKIP_NEXT="$SKIP_THIS_TESTS" + run_test_memory_after_hanshake_with_mfl 512 "$MEMORY_USAGE_MFL_16K" +} + cleanup() { rm -f $CLI_OUT $SRV_OUT $PXY_OUT $SESSION test -n "${SRV_PID:-}" && kill $SRV_PID >/dev/null 2>&1 @@ -8820,6 +8911,12 @@ run_test "export keys functionality" \ -c "exported keylen is " \ -c "exported ivlen is " +# Test heap memory usage after handshake +requires_config_enabled MBEDTLS_MEMORY_DEBUG +requires_config_enabled MBEDTLS_MEMORY_BUFFER_ALLOC_C +requires_config_enabled MBEDTLS_SSL_MAX_FRAGMENT_LENGTH +run_tests_memory_after_hanshake + # Final report echo "------------------------------------------------------------------------" From 45916ba9165e8d722f194fa049dcca5897ee7bf8 Mon Sep 17 00:00:00 2001 From: Andrzej Kurek Date: Thu, 5 Mar 2020 14:46:22 -0500 Subject: [PATCH 09/17] test_suite_ssl: fix coverity issues with uninitialized members Initialize variables to NULL before doing any operations that might fail. This fixes a case where the allocation fails on the first context, which previously made the code free the second, uninitialized context. Signed-off-by: Andrzej Kurek --- tests/suites/test_suite_ssl.function | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index 44e222741..55ab2d6f3 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -552,6 +552,13 @@ typedef struct mbedtls_test_message_socket_context mbedtls_mock_socket* socket; } mbedtls_test_message_socket_context; +void mbedtls_message_socket_init( mbedtls_test_message_socket_context *ctx ) +{ + ctx->queue_input = NULL; + ctx->queue_output = NULL; + ctx->socket = NULL; +} + /* * Setup a given mesasge socket context including initialization of * input/output queues to a chosen capacity of messages. Also set the @@ -1652,6 +1659,8 @@ void perform_handshake( handshake_test_options* options ) mbedtls_test_message_queue server_queue, client_queue; mbedtls_test_message_socket_context server_context, client_context; + mbedtls_message_socket_init( &server_context ); + mbedtls_message_socket_init( &client_context ); /* Client side */ if( options->dtls != 0 ) @@ -2503,6 +2512,8 @@ void ssl_message_mock_uninitialized( ) mbedtls_mock_socket client, server; mbedtls_test_message_queue server_queue, client_queue; mbedtls_test_message_socket_context server_context, client_context; + mbedtls_message_socket_init( &server_context ); + mbedtls_message_socket_init( &client_context ); /* Send with a NULL context */ TEST_ASSERT( mbedtls_mock_tcp_send_msg( NULL, message, MSGLEN ) @@ -2548,6 +2559,8 @@ void ssl_message_mock_basic( ) unsigned i; mbedtls_test_message_queue server_queue, client_queue; mbedtls_test_message_socket_context server_context, client_context; + mbedtls_message_socket_init( &server_context ); + mbedtls_message_socket_init( &client_context ); TEST_ASSERT( mbedtls_message_socket_setup( &server_queue, &client_queue, 1, &server, @@ -2601,6 +2614,8 @@ void ssl_message_mock_queue_overflow_underflow( ) unsigned i; mbedtls_test_message_queue server_queue, client_queue; mbedtls_test_message_socket_context server_context, client_context; + mbedtls_message_socket_init( &server_context ); + mbedtls_message_socket_init( &client_context ); TEST_ASSERT( mbedtls_message_socket_setup( &server_queue, &client_queue, 2, &server, @@ -2657,6 +2672,8 @@ void ssl_message_mock_socket_overflow( ) unsigned i; mbedtls_test_message_queue server_queue, client_queue; mbedtls_test_message_socket_context server_context, client_context; + mbedtls_message_socket_init( &server_context ); + mbedtls_message_socket_init( &client_context ); TEST_ASSERT( mbedtls_message_socket_setup( &server_queue, &client_queue, 2, &server, @@ -2704,6 +2721,8 @@ void ssl_message_mock_truncated( ) unsigned i; mbedtls_test_message_queue server_queue, client_queue; mbedtls_test_message_socket_context server_context, client_context; + mbedtls_message_socket_init( &server_context ); + mbedtls_message_socket_init( &client_context ); TEST_ASSERT( mbedtls_message_socket_setup( &server_queue, &client_queue, 2, &server, @@ -2761,6 +2780,8 @@ void ssl_message_mock_socket_read_error( ) unsigned i; mbedtls_test_message_queue server_queue, client_queue; mbedtls_test_message_socket_context server_context, client_context; + mbedtls_message_socket_init( &server_context ); + mbedtls_message_socket_init( &client_context ); TEST_ASSERT( mbedtls_message_socket_setup( &server_queue, &client_queue, 1, &server, @@ -2813,6 +2834,8 @@ void ssl_message_mock_interleaved_one_way( ) unsigned i; mbedtls_test_message_queue server_queue, client_queue; mbedtls_test_message_socket_context server_context, client_context; + mbedtls_message_socket_init( &server_context ); + mbedtls_message_socket_init( &client_context ); TEST_ASSERT( mbedtls_message_socket_setup( &server_queue, &client_queue, 3, &server, @@ -2871,6 +2894,8 @@ void ssl_message_mock_interleaved_two_ways( ) unsigned i; mbedtls_test_message_queue server_queue, client_queue; mbedtls_test_message_socket_context server_context, client_context; + mbedtls_message_socket_init( &server_context ); + mbedtls_message_socket_init( &client_context ); TEST_ASSERT( mbedtls_message_socket_setup( &server_queue, &client_queue, 3, &server, From 69101224691c5570acba4aafd467253d0ebbf638 Mon Sep 17 00:00:00 2001 From: Simon Butcher Date: Thu, 5 Mar 2020 15:18:53 +0000 Subject: [PATCH 10/17] Add Python, Perl and Pylint to output_env.sh Add the versions of Python, Perl, and Pylint to the version dump provided by the output_env.sh script. Signed-off-by: Simon Butcher --- scripts/output_env.sh | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/scripts/output_env.sh b/scripts/output_env.sh index 22bef92b9..069e572d2 100755 --- a/scripts/output_env.sh +++ b/scripts/output_env.sh @@ -23,7 +23,7 @@ print_version() shift ARGS="$1" shift - VARIANT=$1 + VARIANT="$1" shift if [ -n "$VARIANT" ]; then @@ -45,6 +45,10 @@ print_version() VERSION_STR=`echo "$VERSION_STR" | $FILTER` done + if [ -z "$VERSION_STR" ]; then + VERSION_STR="Version could not be determined." + fi + echo " * ${BIN##*/}$VARIANT: ${BIN} : ${VERSION_STR} " } @@ -61,6 +65,7 @@ fi echo print_version "uname" "-a" "" + echo echo echo "** Tool Versions:" @@ -94,6 +99,15 @@ echo print_version "gdb" "--version" "" "head -n 1" echo +print_version "perl" "--version" "" "head -n 2" "grep ." +echo + +print_version "python" "--version" "" "head -n 1" +echo + +print_version "pylint3" "--version" "" "head -n 2" "tail -n 1" +echo + : ${OPENSSL:=openssl} print_version "$OPENSSL" "version" "default" echo From 9693ea2490b7b0640d8e0f09989f45ef471650a6 Mon Sep 17 00:00:00 2001 From: Simon Butcher Date: Fri, 6 Mar 2020 14:50:49 +0000 Subject: [PATCH 11/17] Fix output_env.sh for varying pylint3 output `pylint3 --version` will output to stderr the status of the config file it's using. This can be "No config file found" or "Using config file" or nothing. This means the pylint version may or may not be on the first line. Therefore this commit changes the filters on the pylint3 version output to first strip out the config line, and then to select only the pylint line. Signed-off-by: Simon Butcher --- scripts/output_env.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/output_env.sh b/scripts/output_env.sh index 069e572d2..0e0679479 100755 --- a/scripts/output_env.sh +++ b/scripts/output_env.sh @@ -105,7 +105,7 @@ echo print_version "python" "--version" "" "head -n 1" echo -print_version "pylint3" "--version" "" "head -n 2" "tail -n 1" +print_version "pylint3" "--version" "" "sed /^.*config/d" "grep pylint" echo : ${OPENSSL:=openssl} From 89bdc580ca4eac633e68a886fc1500b023eb8bdb Mon Sep 17 00:00:00 2001 From: Andrzej Kurek Date: Mon, 9 Mar 2020 06:29:43 -0400 Subject: [PATCH 12/17] test_suite_ssl: check for errors during queue setup Signed-off-by: Andrzej Kurek --- tests/suites/test_suite_ssl.function | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index 44e222741..e5247dfe2 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -2380,7 +2380,7 @@ void ssl_message_queue_sanity( ) TEST_ASSERT( mbedtls_test_message_queue_pop_info( NULL, 1 ) == MBEDTLS_TEST_ERROR_ARG_NULL ); - mbedtls_test_message_queue_setup( &queue, 3 ); + TEST_ASSERT( mbedtls_test_message_queue_setup( &queue, 3 ) == 0 ); TEST_ASSERT( queue.capacity == 3 ); TEST_ASSERT( queue.num == 0 ); @@ -2394,7 +2394,7 @@ void ssl_message_queue_basic( ) { mbedtls_test_message_queue queue; - mbedtls_test_message_queue_setup( &queue, 3 ); + TEST_ASSERT( mbedtls_test_message_queue_setup( &queue, 3 ) == 0 ); /* Sanity test - 3 pushes and 3 pops with sufficient space */ TEST_ASSERT( mbedtls_test_message_queue_push_info( &queue, 1 ) == 1 ); @@ -2421,7 +2421,7 @@ void ssl_message_queue_overflow_underflow( ) { mbedtls_test_message_queue queue; - mbedtls_test_message_queue_setup( &queue, 3 ); + TEST_ASSERT( mbedtls_test_message_queue_setup( &queue, 3 ) == 0 ); /* 4 pushes (last one with an error), 4 pops (last one with an error) */ TEST_ASSERT( mbedtls_test_message_queue_push_info( &queue, 1 ) == 1 ); @@ -2447,7 +2447,7 @@ void ssl_message_queue_interleaved( ) { mbedtls_test_message_queue queue; - mbedtls_test_message_queue_setup( &queue, 3 ); + TEST_ASSERT( mbedtls_test_message_queue_setup( &queue, 3 ) == 0 ); /* Interleaved test - [2 pushes, 1 pop] twice, and then two pops * (to wrap around the buffer) */ @@ -2483,7 +2483,7 @@ void ssl_message_queue_insufficient_buffer( ) size_t message_len = 10; size_t buffer_len = 5; - mbedtls_test_message_queue_setup( &queue, 1 ); + TEST_ASSERT( mbedtls_test_message_queue_setup( &queue, 1 ) == 0 ); /* Popping without a sufficient buffer */ TEST_ASSERT( mbedtls_test_message_queue_push_info( &queue, message_len ) From 438bf3b6672186a9dc425c672c9166f7406e75e7 Mon Sep 17 00:00:00 2001 From: Piotr Nowicki Date: Tue, 10 Mar 2020 12:59:10 +0100 Subject: [PATCH 13/17] App data with 1/n-1 splitting in test suite Counting of the fragments has been shifted from the writing section to the reading. This is more reliable because one reading is made for one fragment and during one write the library can internally divide data into two fragments Signed-off-by: Piotr Nowicki --- tests/suites/test_suite_ssl.data | 2 -- tests/suites/test_suite_ssl.function | 34 ++++++++++++---------------- 2 files changed, 15 insertions(+), 21 deletions(-) diff --git a/tests/suites/test_suite_ssl.data b/tests/suites/test_suite_ssl.data index 1a241055f..a8a852e26 100644 --- a/tests/suites/test_suite_ssl.data +++ b/tests/suites/test_suite_ssl.data @@ -199,12 +199,10 @@ move_handshake_to_state:MBEDTLS_SSL_IS_CLIENT:MBEDTLS_SSL_SERVER_HELLO_VERIFY_RE Negative test moving servers ssl to state: NEW_SESSION_TICKET move_handshake_to_state:MBEDTLS_SSL_IS_SERVER:MBEDTLS_SSL_SERVER_NEW_SESSION_TICKET:0 -# Note - the case below will have to updated, since the test sends no data due to a 1n-1 split against BEAST, that was not expected when preparing the fragment counting code. Handshake, SSL3 depends_on:MBEDTLS_SSL_PROTO_SSL3:MBEDTLS_RSA_C:MBEDTLS_ECP_DP_SECP384R1_ENABLED handshake_version:MBEDTLS_SSL_MINOR_VERSION_0:0 -# Note - the case below will have to updated, since the test sends no data due to a 1n-1 split against BEAST, that was not expected when preparing the fragment counting code. Handshake, tls1 depends_on:MBEDTLS_SSL_PROTO_TLS1:MBEDTLS_CIPHER_MODE_CBC handshake_version:MBEDTLS_SSL_MINOR_VERSION_1:0 diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index 5df4b4753..e0970e71a 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -13,7 +13,8 @@ typedef struct log_pattern size_t counter; } log_pattern; -/* This function can be passed to mbedtls to receive output logs from it. In +/* + * This function can be passed to mbedtls to receive output logs from it. In * this case, it will count the instances of a log_pattern in the received * logged messages. */ @@ -1009,17 +1010,15 @@ int mbedtls_move_handshake_to_state( mbedtls_ssl_context *ssl, #endif /* MBEDTLS_X509_CRT_PARSE_C */ /* - * Write application data. Increase write counter and fragments counter if - * necessary. + * Write application data. Increase write counter if necessary. */ int mbedtls_ssl_write_fragment( mbedtls_ssl_context *ssl, unsigned char *buf, int buf_len, int *written, - int *fragments, const int expected_fragments ) + const int expected_fragments ) { int ret = mbedtls_ssl_write( ssl, buf + *written, buf_len - *written ); if( ret > 0 ) { - (*fragments)++; *written += ret; } @@ -1055,15 +1054,16 @@ exit: } /* - * Read application data and increase read counter if necessary. + * Read application data and increase read counter and fragments counter if necessary. */ int mbedtls_ssl_read_fragment( mbedtls_ssl_context *ssl, unsigned char *buf, int buf_len, int *read, - const int expected_fragments ) + int *fragments, const int expected_fragments ) { int ret = mbedtls_ssl_read( ssl, buf + *read, buf_len - *read ); if( ret > 0 ) { + ( *fragments )++; *read += ret; } @@ -1552,7 +1552,6 @@ int mbedtls_exchange_data( mbedtls_ssl_context *ssl_1, { ret = mbedtls_ssl_write_fragment( ssl_1, msg_buf_1, msg_len_1, &written_1, - &fragments_1, expected_fragments_1 ); if( expected_fragments_1 == 0 ) { @@ -1572,7 +1571,6 @@ int mbedtls_exchange_data( mbedtls_ssl_context *ssl_1, { ret = mbedtls_ssl_write_fragment( ssl_2, msg_buf_2, msg_len_2, &written_2, - &fragments_2, expected_fragments_2 ); if( expected_fragments_2 == 0 ) { @@ -1592,7 +1590,8 @@ int mbedtls_exchange_data( mbedtls_ssl_context *ssl_1, { ret = mbedtls_ssl_read_fragment( ssl_1, in_buf_1, msg_len_2, &read_1, - expected_fragments_1 ); + &fragments_2, + expected_fragments_2 ); TEST_ASSERT( ret == 0 ); } @@ -1601,7 +1600,8 @@ int mbedtls_exchange_data( mbedtls_ssl_context *ssl_1, { ret = mbedtls_ssl_read_fragment( ssl_2, in_buf_2, msg_len_1, &read_2, - expected_fragments_2 ); + &fragments_1, + expected_fragments_1 ); TEST_ASSERT( ret == 0 ); } } @@ -1799,9 +1799,6 @@ void perform_handshake( handshake_test_options* options ) #if defined(MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH) if( options->resize_buffers != 0 ) { - /* Note - the case below will have to updated, since due to a 1n-1 - * split against BEAST the fragment count is different - * than expected when preparing the fragment counting code. */ if( options->version != MBEDTLS_SSL_MINOR_VERSION_0 && options->version != MBEDTLS_SSL_MINOR_VERSION_1 ) { @@ -3747,14 +3744,13 @@ void handshake_version( int version, int dtls ) options.version = version; options.dtls = dtls; - /* Note - the case below will have to updated, since the test sends no data - * due to a 1n-1 split against BEAST, that was not expected when preparing - * the fragment counting code. */ + /* By default, SSLv3.0 and TLSv1.0 use 1/n-1 splitting when sending data, so + * the number of fragments will be twice as big. */ if( version == MBEDTLS_SSL_MINOR_VERSION_0 || version == MBEDTLS_SSL_MINOR_VERSION_1 ) { - options.cli_msg_len = 0; - options.srv_msg_len = 0; + options.expected_cli_fragments = 2; + options.expected_srv_fragments = 2; } perform_handshake( &options ); From 7ae6ed4435cee8a6ec5bb48e9f93ee53c2da2fec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Fri, 13 Mar 2020 11:28:19 +0100 Subject: [PATCH 14/17] Keep SSL context const when hw accel is disabled MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- library/ssl_tls.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index d81d1e1e6..973a36ac3 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -826,6 +826,9 @@ static int ssl_populate_transform( mbedtls_ssl_transform *transform, const unsigned char randbytes[64], int minor_ver, unsigned endpoint, +#if !defined(MBEDTLS_SSL_HW_RECORD_ACCEL) + const +#endif mbedtls_ssl_context *ssl ) { int ret = 0; From e30d03e4f43b3c98be0adf88ea4f85e1c07f11d8 Mon Sep 17 00:00:00 2001 From: Simon Butcher Date: Mon, 16 Mar 2020 11:30:46 +0000 Subject: [PATCH 15/17] Change the use of pylint to optionally use pylint3 Pylint when installed as a distro package can be installed as pylint3, whilst as a PEP egg, it can be installed as pylint. This commit changes the scripts to first use pylint if installed, and optionally look for pylint3 if not installed. This is to allow a preference for the PEP version over the distro version, assuming the PEP one is more likely to be the correct one. Signed-off-by: Simon Butcher --- scripts/output_env.sh | 11 ++++++++++- tests/scripts/all.sh | 8 +++++++- tests/scripts/check-python-files.sh | 13 ++++++++++++- 3 files changed, 29 insertions(+), 3 deletions(-) diff --git a/scripts/output_env.sh b/scripts/output_env.sh index 0e0679479..04edc3812 100755 --- a/scripts/output_env.sh +++ b/scripts/output_env.sh @@ -105,7 +105,16 @@ echo print_version "python" "--version" "" "head -n 1" echo -print_version "pylint3" "--version" "" "sed /^.*config/d" "grep pylint" +# Find the installed version of Pylint. Installed as a distro package this can +# be pylint3 and as a PEP egg, pylint. In test scripts We prefer pylint over +# pylint3 +if type pylint >/dev/null 2>/dev/null; then + print_version "pylint" "--version" "" "sed /^.*config/d" "grep pylint" +elif type pylint3 >/dev/null 2>/dev/null; then + print_version "pylint3" "--version" "" "sed /^.*config/d" "grep pylint" +else + echo " * pylint or pylint3: Not found." +fi echo : ${OPENSSL:=openssl} diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 25bf4255d..7abc395ea 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -1595,7 +1595,13 @@ component_test_zeroize () { } support_check_python_files () { - type pylint3 >/dev/null 2>/dev/null + # Find the installed version of Pylint. Installed as a distro package this can + # be pylint3 and as a PEP egg, pylint. + if type pylint >/dev/null 2>/dev/null || type pylint3 >/dev/null 2>/dev/null; then + true; + else + false; + fi } component_check_python_files () { msg "Lint: Python scripts" diff --git a/tests/scripts/check-python-files.sh b/tests/scripts/check-python-files.sh index 929041822..6b864d264 100755 --- a/tests/scripts/check-python-files.sh +++ b/tests/scripts/check-python-files.sh @@ -9,4 +9,15 @@ # Run 'pylint' on Python files for programming errors and helps enforcing # PEP8 coding standards. -pylint3 -j 2 scripts/*.py tests/scripts/*.py +# Find the installed version of Pylint. Installed as a distro package this can +# be pylint3 and as a PEP egg, pylint. We prefer pylint over pylint3 +if type pylint >/dev/null 2>/dev/null; then + PYLINT=pylint +elif type pylint3 >/dev/null 2>/dev/null; then + PYLINT=pylint3 +else + echo 'Pylint was not found.' + exit 1 +fi + +$PYLINT -j 2 scripts/*.py tests/scripts/*.py From 03da0846df6b4e459ed93e15070fbda54d01da85 Mon Sep 17 00:00:00 2001 From: Andrzej Kurek Date: Tue, 17 Mar 2020 11:11:36 -0400 Subject: [PATCH 16/17] ssl - improve documentation on mbedtls_ssl_read and PEER_CLOSE_NOTIFY Add a description of MBEDTLS_ERR_SSL_PEER_CLOSE_NOTIFY to the documentation, as suggested by hanno-arm. Signed-off-by: Andrzej Kurek --- include/mbedtls/ssl.h | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index b3c5288c0..136d7e6c6 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -3739,7 +3739,14 @@ int mbedtls_ssl_renegotiate( mbedtls_ssl_context *ssl ); * * \return The (positive) number of bytes read if successful. * \return \c 0 if the read end of the underlying transport was closed - * - in this case you must stop using the context (see below). + * without sending a CloseNotify beforehand, which might happen + * because of various reasons (internal error of an underlying + * stack, non-conformant peer not sending a CloseNotify and + * such) - in this case you must stop using the context + * (see below). + * \return #MBEDTLS_ERR_SSL_PEER_CLOSE_NOTIFY if the underlying + * transport is still functional, but the peer has + * acknowledged to not send anything anymore. * \return #MBEDTLS_ERR_SSL_WANT_READ or #MBEDTLS_ERR_SSL_WANT_WRITE * if the handshake is incomplete and waiting for data to * be available for reading from or writing to the underlying From 7ed01e8c683d0cfe68ffcc2874417bbd53dcef37 Mon Sep 17 00:00:00 2001 From: Andrzej Kurek Date: Wed, 18 Mar 2020 11:51:59 -0400 Subject: [PATCH 17/17] ssl_srv.c: initialize flags on each iteration of the loop Although the 'flags' variable is not checked or used after a call to mbedtls_ssl_check_cert_usage, it might be in the future. With this fix, after each iteration, the flags will apply only to the most recent certificate, not to any of the previous ones checked. This fix also stops any reads and writes via a '|=' from/to an uninitialized variable happening. This commit fixes #2444. Signed-off-by: Andrzej Kurek --- library/ssl_srv.c | 1 + 1 file changed, 1 insertion(+) diff --git a/library/ssl_srv.c b/library/ssl_srv.c index b0b09cd97..2ceb3f684 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -840,6 +840,7 @@ static int ssl_pick_cert( mbedtls_ssl_context *ssl, for( cur = list; cur != NULL; cur = cur->next ) { + flags = 0; MBEDTLS_SSL_DEBUG_CRT( 3, "candidate certificate chain, certificate", cur->cert );