From 704f493730b12db10a156c9e3b5d2a20d9fbaece Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Thu, 8 Jun 2017 13:08:45 +0100 Subject: [PATCH] Simplify retaining of messages for future processing There are situations in which it is not clear what message to expect next. For example, the message following the ServerHello might be either a Certificate, a ServerKeyExchange or a CertificateRequest. We deal with this situation in the following way: Initially, the message processing function for one of the allowed message types is called, which fetches and decodes a new message. If that message is not the expected one, the function returns successfully (instead of throwing an error as usual for unexpected messages), and the handshake continues to the processing function for the next possible message. To not have this function fetch a new message, a flag in the SSL context structure is used to indicate that the last message was retained for further processing, and if that's set, the following processing function will not fetch a new record. This commit simplifies the usage of this message-retaining parameter by doing the check within the record-fetching routine instead of the specific message-processing routines. The code gets cleaner this way and allows retaining messages to be used in other situations as well without much effort. This will be used in the next commits. --- include/mbedtls/ssl.h | 4 ++- library/ssl_cli.c | 63 +++++++++++++++++++------------------------ library/ssl_tls.c | 11 +++++++- 3 files changed, 41 insertions(+), 37 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 5775f8a49..906866ded 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -714,7 +714,9 @@ struct mbedtls_ssl_context size_t in_hslen; /*!< current handshake message length, including the handshake header */ int nb_zero; /*!< # of 0-length encrypted messages */ - int record_read; /*!< record is already present */ + + int keep_current_message; /*!< drop or reuse current message + on next call to record layer? */ /* * Record layer (outgoing data) diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 67cbccc90..8d0374f3b 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -2102,11 +2102,14 @@ static int ssl_parse_server_key_exchange( mbedtls_ssl_context *ssl ) if( ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_PSK || ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_RSA_PSK ) { - ssl->record_read = 1; + /* Current message is probably either + * CertificateRequest or ServerHelloDone */ + ssl->keep_current_message = 1; goto exit; } - MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad server key exchange message" ) ); + MBEDTLS_SSL_DEBUG_MSG( 1, ( "server key exchange message must " + "not be skipped" ) ); return( MBEDTLS_ERR_SSL_UNEXPECTED_MESSAGE ); } @@ -2389,36 +2392,30 @@ static int ssl_parse_certificate_request( mbedtls_ssl_context *ssl ) return( 0 ); } - if( ssl->record_read == 0 ) + if( ( ret = mbedtls_ssl_read_record( ssl ) ) != 0 ) { - if( ( ret = mbedtls_ssl_read_record( ssl ) ) != 0 ) - { - MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_read_record", ret ); - return( ret ); - } - - if( ssl->in_msgtype != MBEDTLS_SSL_MSG_HANDSHAKE ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad certificate request message" ) ); - return( MBEDTLS_ERR_SSL_UNEXPECTED_MESSAGE ); - } - - ssl->record_read = 1; + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_read_record", ret ); + return( ret ); } - ssl->client_auth = 0; - ssl->state++; + if( ssl->in_msgtype != MBEDTLS_SSL_MSG_HANDSHAKE ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad certificate request message" ) ); + return( MBEDTLS_ERR_SSL_UNEXPECTED_MESSAGE ); + } - if( ssl->in_msg[0] == MBEDTLS_SSL_HS_CERTIFICATE_REQUEST ) - ssl->client_auth++; + ssl->state++; + ssl->client_auth = ( ssl->in_msg[0] == MBEDTLS_SSL_HS_CERTIFICATE_REQUEST ); MBEDTLS_SSL_DEBUG_MSG( 3, ( "got %s certificate request", ssl->client_auth ? "a" : "no" ) ); if( ssl->client_auth == 0 ) + { + /* Current message is probably the ServerHelloDone */ + ssl->keep_current_message = 1; goto exit; - - ssl->record_read = 0; + } // TODO: handshake_failure alert for an anonymous server to request // client authentication @@ -2517,21 +2514,17 @@ static int ssl_parse_server_hello_done( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> parse server hello done" ) ); - if( ssl->record_read == 0 ) + if( ( ret = mbedtls_ssl_read_record( ssl ) ) != 0 ) { - if( ( ret = mbedtls_ssl_read_record( ssl ) ) != 0 ) - { - MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_read_record", ret ); - return( ret ); - } - - if( ssl->in_msgtype != MBEDTLS_SSL_MSG_HANDSHAKE ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad server hello done message" ) ); - return( MBEDTLS_ERR_SSL_UNEXPECTED_MESSAGE ); - } + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_read_record", ret ); + return( ret ); + } + + if( ssl->in_msgtype != MBEDTLS_SSL_MSG_HANDSHAKE ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad server hello done message" ) ); + return( MBEDTLS_ERR_SSL_UNEXPECTED_MESSAGE ); } - ssl->record_read = 0; if( ssl->in_hslen != mbedtls_ssl_hs_hdr_len( ssl ) || ssl->in_msg[0] != MBEDTLS_SSL_HS_SERVER_HELLO_DONE ) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 22c3f9936..b5c166ecc 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -3716,6 +3716,15 @@ int mbedtls_ssl_read_record( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> read record" ) ); + if( ssl->keep_current_message == 1 ) + { + MBEDTLS_SSL_DEBUG_MSG( 2, ( "reuse previously read message" ) ); + MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= read record" ) ); + ssl->keep_current_message = 0; + + return( 0 ); + } + if( ssl->in_hslen != 0 && ssl->in_hslen < ssl->in_msglen ) { /* @@ -5452,7 +5461,7 @@ static int ssl_session_reset_int( mbedtls_ssl_context *ssl, int partial ) ssl->in_hslen = 0; ssl->nb_zero = 0; - ssl->record_read = 0; + ssl->keep_current_message = 0; ssl->out_msg = ssl->out_buf + 13; ssl->out_msgtype = 0;