From 79594fd0d4015a7024af13b622f10549b882f20a Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Wed, 8 May 2019 09:38:41 +0100 Subject: [PATCH] Set pointer to start of plaintext at record decryption time The SSL context structure mbedtls_ssl_context contains several pointers ssl->in_hdr, ssl->in_len, ssl->in_iv, ssl->in_msg pointing to various parts of the record header in an incoming record, and they are setup in the static function ssl_update_in_pointers() based on the _expected_ transform for the next incoming record. In particular, the pointer ssl->in_msg is set to where the record plaintext should reside after record decryption, and an assertion double-checks this after each call to ssl_decrypt_buf(). This commit removes the dependency of ssl_update_in_pointers() on the expected incoming transform by setting ssl->in_msg to ssl->in_iv -- the beginning of the record content (potentially including the IV) -- and adjusting ssl->in_msg after calling ssl_decrypt_buf() on a protected record. Care has to be taken to not load ssl->in_msg before calling mbedtls_ssl_read_record(), then, which was previously the case in ssl_parse_server_hello(); the commit fixes that. --- library/ssl_cli.c | 4 ++-- library/ssl_tls.c | 37 +++++++++++++++++-------------------- 2 files changed, 19 insertions(+), 22 deletions(-) diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 45f4c4047..228e33585 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -1658,8 +1658,6 @@ static int ssl_parse_server_hello( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> parse server hello" ) ); - buf = ssl->in_msg; - if( ( ret = mbedtls_ssl_read_record( ssl, 1 ) ) != 0 ) { /* No alert on a read error. */ @@ -1667,6 +1665,8 @@ static int ssl_parse_server_hello( mbedtls_ssl_context *ssl ) return( ret ); } + buf = ssl->in_msg; + if( ssl->in_msgtype != MBEDTLS_SSL_MSG_HANDSHAKE ) { #if defined(MBEDTLS_SSL_RENEGOTIATION) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 199c41d0f..dacbde6d5 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -110,8 +110,7 @@ static int ssl_check_timer( mbedtls_ssl_context *ssl ) static void ssl_update_out_pointers( mbedtls_ssl_context *ssl, mbedtls_ssl_transform *transform ); -static void ssl_update_in_pointers( mbedtls_ssl_context *ssl, - mbedtls_ssl_transform *transform ); +static void ssl_update_in_pointers( mbedtls_ssl_context *ssl ); #define SSL_DONT_FORCE_FLUSH 0 #define SSL_FORCE_FLUSH 1 @@ -5042,12 +5041,7 @@ static int ssl_prepare_record_content( mbedtls_ssl_context *ssl ) return( ret ); } - if( ssl->in_iv + rec.data_offset != ssl->in_msg ) - { - /* Should never happen */ - return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); - } - + ssl->in_msg = rec.buf + rec.data_offset; ssl->in_msglen = rec.data_len; ssl->in_len[0] = (unsigned char)( rec.data_len >> 8 ); ssl->in_len[1] = (unsigned char)( rec.data_len ); @@ -7037,7 +7031,7 @@ int mbedtls_ssl_parse_change_cipher_spec( mbedtls_ssl_context *ssl ) #endif /* MBEDTLS_SSL_PROTO_DTLS */ memset( ssl->in_ctr, 0, 8 ); - ssl_update_in_pointers( ssl, ssl->transform_negotiate ); + ssl_update_in_pointers( ssl ); #if defined(MBEDTLS_SSL_HW_RECORD_ACCEL) if( mbedtls_ssl_hw_record_activate != NULL ) @@ -7991,9 +7985,18 @@ static void ssl_update_out_pointers( mbedtls_ssl_context *ssl, * and the caller has to make sure there's space for this. */ -static void ssl_update_in_pointers( mbedtls_ssl_context *ssl, - mbedtls_ssl_transform *transform ) +static void ssl_update_in_pointers( mbedtls_ssl_context *ssl ) { + /* This function sets the pointers to match the case + * of unprotected TLS/DTLS records, with both ssl->in_iv + * and ssl->in_msg pointing to the beginning of the record + * content. + * + * When decrypting a protected record, ssl->in_msg + * will be shifted to point to the beginning of the + * record plaintext. + */ + #if defined(MBEDTLS_SSL_PROTO_DTLS) if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM ) { @@ -8009,14 +8012,8 @@ static void ssl_update_in_pointers( mbedtls_ssl_context *ssl, ssl->in_iv = ssl->in_hdr + 5; } - /* Offset in_msg from in_iv to allow space for explicit IV, if used. */ - if( transform != NULL && - ssl->minor_ver >= MBEDTLS_SSL_MINOR_VERSION_2 ) - { - ssl->in_msg = ssl->in_iv + transform->ivlen - transform->fixed_ivlen; - } - else - ssl->in_msg = ssl->in_iv; + /* This will be adjusted at record decryption time. */ + ssl->in_msg = ssl->in_iv; } /* @@ -8049,7 +8046,7 @@ static void ssl_reset_in_out_pointers( mbedtls_ssl_context *ssl ) /* Derive other internal pointers. */ ssl_update_out_pointers( ssl, NULL /* no transform enabled */ ); - ssl_update_in_pointers ( ssl, NULL /* no transform enabled */ ); + ssl_update_in_pointers ( ssl ); } int mbedtls_ssl_setup( mbedtls_ssl_context *ssl,