From a3fa06e62add46dead954e30db64bde78ae56000 Mon Sep 17 00:00:00 2001 From: Arto Kinnunen Date: Mon, 9 Sep 2019 12:22:51 +0300 Subject: [PATCH] Review corrections 3 -Remove additional trace cause by rebase -Update remaining 16/24/32-bit values to use functions, this uses additional 36 bytes. --- library/ssl_cli.c | 22 +++++++++------------- library/ssl_srv.c | 38 +++++++++++++++++--------------------- library/ssl_ticket.c | 2 +- library/ssl_tls.c | 5 +---- 4 files changed, 28 insertions(+), 39 deletions(-) diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 331285f24..8ce158258 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -968,8 +968,7 @@ static int ssl_write_client_hello( mbedtls_ssl_context *ssl ) if( mbedtls_ssl_get_renego_status( ssl ) == MBEDTLS_SSL_INITIAL_HANDSHAKE ) { MBEDTLS_SSL_DEBUG_MSG( 3, ( "adding EMPTY_RENEGOTIATION_INFO_SCSV" ) ); - *p++ = (unsigned char)( MBEDTLS_SSL_EMPTY_RENEGOTIATION_INFO >> 8 ); - *p++ = (unsigned char)( MBEDTLS_SSL_EMPTY_RENEGOTIATION_INFO ); + p = mbedtls_platform_put_uint16_be( p, MBEDTLS_SSL_EMPTY_RENEGOTIATION_INFO ); n++; } @@ -1725,8 +1724,7 @@ static int ssl_parse_server_hello( mbedtls_ssl_context *ssl ) if( ssl->in_hslen > mbedtls_ssl_hs_hdr_len( ssl ) + 39 + n ) { - ext_len = ( ( buf[38 + n] << 8 ) - | ( buf[39 + n] ) ); + ext_len = mbedtls_platform_get_uint16_be( &buf[38 + n] ); if( ( ext_len > 0 && ext_len < 4 ) || ssl->in_hslen != mbedtls_ssl_hs_hdr_len( ssl ) + 40 + n + ext_len ) @@ -1914,10 +1912,10 @@ server_picked_valid_suite: while( ext_len ) { - unsigned int ext_id = ( ( ext[0] << 8 ) - | ( ext[1] ) ); - unsigned int ext_size = ( ( ext[2] << 8 ) - | ( ext[3] ) ); + unsigned int ext_id = (unsigned int) + mbedtls_platform_get_uint16_be( ext ); + unsigned int ext_size = (unsigned int) + mbedtls_platform_get_uint16_be( &ext[2] ); if( ext_size + 4 > ext_len ) { @@ -2287,7 +2285,7 @@ static int ssl_parse_server_psk_hint( mbedtls_ssl_context *ssl, "(psk_identity_hint length)" ) ); return( MBEDTLS_ERR_SSL_BAD_HS_SERVER_KEY_EXCHANGE ); } - len = (*p)[0] << 8 | (*p)[1]; + len = mbedtls_platform_get_uint16_be( *p ); *p += 2; if( end - (*p) < (int) len ) @@ -3248,8 +3246,7 @@ static int ssl_parse_certificate_request( mbedtls_ssl_context *ssl ) #if defined(MBEDTLS_SSL_PROTO_TLS1_2) if( mbedtls_ssl_get_minor_ver( ssl ) == MBEDTLS_SSL_MINOR_VERSION_3 ) { - size_t sig_alg_len = ( ( buf[mbedtls_ssl_hs_hdr_len( ssl ) + 1 + n] << 8 ) - | ( buf[mbedtls_ssl_hs_hdr_len( ssl ) + 2 + n] ) ); + size_t sig_alg_len = mbedtls_platform_get_uint16_be( &buf[mbedtls_ssl_hs_hdr_len( ssl ) + 1 + n] ); #if defined(MBEDTLS_DEBUG_C) unsigned char* sig_alg; size_t i; @@ -3289,8 +3286,7 @@ static int ssl_parse_certificate_request( mbedtls_ssl_context *ssl ) #endif /* MBEDTLS_SSL_PROTO_TLS1_2 */ /* certificate_authorities */ - dn_len = ( ( buf[mbedtls_ssl_hs_hdr_len( ssl ) + 1 + n] << 8 ) - | ( buf[mbedtls_ssl_hs_hdr_len( ssl ) + 2 + n] ) ); + dn_len = mbedtls_platform_get_uint16_be( &buf[mbedtls_ssl_hs_hdr_len( ssl ) + 1 + n] ); n += dn_len; if( ssl->in_hslen != mbedtls_ssl_hs_hdr_len( ssl ) + 3 + n ) diff --git a/library/ssl_srv.c b/library/ssl_srv.c index fa073e43a..902f2fdd2 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -315,7 +315,8 @@ static int ssl_parse_supported_elliptic_curves( mbedtls_ssl_context *ssl, while( list_size > 0 ) { - uint16_t const peer_tls_id = ( p[0] << 8 ) | p[1]; + uint16_t const peer_tls_id = (uint16_t) + mbedtls_platform_get_uint16_be( p ); MBEDTLS_SSL_BEGIN_FOR_EACH_SUPPORTED_EC_TLS_ID( own_tls_id ) if( own_tls_id == peer_tls_id && @@ -661,7 +662,7 @@ static int ssl_parse_alpn_ext( mbedtls_ssl_context *ssl, return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_HELLO ); } - list_len = ( buf[0] << 8 ) | buf[1]; + list_len = mbedtls_platform_get_uint16_be ( buf ); if( list_len != len - 2 ) { @@ -1447,7 +1448,7 @@ read_record_header: } MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello v3, message len.: %d", - ( ssl->in_len[0] << 8 ) | ssl->in_len[1] ) ); + (int)mbedtls_platform_get_uint16_be( ssl->in_len ) ) ); MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello v3, protocol version: [%d:%d]", buf[1], buf[2] ) ); @@ -1562,11 +1563,12 @@ read_record_header: } MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello v3, handshake len.: %d", - ( buf[1] << 16 ) | ( buf[2] << 8 ) | buf[3] ) ); + (int)mbedtls_platform_get_uint24_be( &buf[1]) ) ); /* We don't support fragmentation of ClientHello (yet?) */ if( buf[1] != 0 || - msg_len != mbedtls_ssl_hs_hdr_len( ssl ) + ( ( buf[2] << 8 ) | buf[3] ) ) + msg_len != ( mbedtls_ssl_hs_hdr_len( ssl ) + + mbedtls_platform_get_uint16_be( &buf[2]) ) ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad client hello message" ) ); return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_HELLO ); @@ -1863,8 +1865,7 @@ read_record_header: return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_HELLO ); } - ext_len = ( buf[ext_offset + 0] << 8 ) - | ( buf[ext_offset + 1] ); + ext_len = mbedtls_platform_get_uint16_be( &buf[ext_offset + 0] ); if( ( ext_len > 0 && ext_len < 4 ) || msg_len != ext_offset + 2 + ext_len ) @@ -1891,8 +1892,8 @@ read_record_header: MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR ); return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_HELLO ); } - ext_id = ( ( ext[0] << 8 ) | ( ext[1] ) ); - ext_size = ( ( ext[2] << 8 ) | ( ext[3] ) ); + ext_id = (unsigned int)mbedtls_platform_get_uint16_be( ext ); + ext_size = (unsigned int)mbedtls_platform_get_uint16_be( &ext[2] ); if( ext_size + 4 > ext_len ) { @@ -2495,8 +2496,7 @@ static void ssl_write_renegotiation_ext( mbedtls_ssl_context *ssl, MBEDTLS_SSL_DEBUG_MSG( 3, ( "server hello, secure renegotiation extension" ) ); - *p++ = (unsigned char)( ( MBEDTLS_TLS_EXT_RENEGOTIATION_INFO >> 8 ) & 0xFF ); - *p++ = (unsigned char)( ( MBEDTLS_TLS_EXT_RENEGOTIATION_INFO ) & 0xFF ); + p = mbedtls_platform_put_uint16_be( p, MBEDTLS_TLS_EXT_RENEGOTIATION_INFO ); #if defined(MBEDTLS_SSL_RENEGOTIATION) if( ssl->renego_status != MBEDTLS_SSL_INITIAL_HANDSHAKE ) @@ -2565,8 +2565,7 @@ static void ssl_write_supported_point_formats_ext( mbedtls_ssl_context *ssl, MBEDTLS_SSL_DEBUG_MSG( 3, ( "server hello, supported_point_formats extension" ) ); - *p++ = (unsigned char)( ( MBEDTLS_TLS_EXT_SUPPORTED_POINT_FORMATS >> 8 ) & 0xFF ); - *p++ = (unsigned char)( ( MBEDTLS_TLS_EXT_SUPPORTED_POINT_FORMATS ) & 0xFF ); + p = mbedtls_platform_put_uint16_be( p, MBEDTLS_TLS_EXT_SUPPORTED_POINT_FORMATS ); *p++ = 0x00; *p++ = 2; @@ -2606,8 +2605,7 @@ static void ssl_write_ecjpake_kkpp_ext( mbedtls_ssl_context *ssl, return; } - *p++ = (unsigned char)( ( MBEDTLS_TLS_EXT_ECJPAKE_KKPP >> 8 ) & 0xFF ); - *p++ = (unsigned char)( ( MBEDTLS_TLS_EXT_ECJPAKE_KKPP ) & 0xFF ); + p = mbedtls_platform_put_uint16_be( p, MBEDTLS_TLS_EXT_ECJPAKE_KKPP ); ret = mbedtls_ecjpake_write_round_one( &ssl->handshake->ecjpake_ctx, p + 2, end - p - 2, &kkpp_len, @@ -2883,8 +2881,7 @@ static int ssl_write_server_hello( mbedtls_ssl_context *ssl ) mbedtls_ssl_handshake_get_resume( ssl->handshake ) ? "a" : "no" ) ); ciphersuite = mbedtls_ssl_session_get_ciphersuite( ssl->session_negotiate ); - *p++ = (unsigned char)( ciphersuite >> 8 ); - *p++ = (unsigned char)( ciphersuite ); + p = mbedtls_platform_put_uint16_be( p, ciphersuite ); *p++ = (unsigned char)( mbedtls_ssl_session_get_compression( ssl->session_negotiate ) ); @@ -2961,8 +2958,7 @@ static int ssl_write_server_hello( mbedtls_ssl_context *ssl ) if( ext_len > 0 ) { - *p++ = (unsigned char)( ( ext_len >> 8 ) & 0xFF ); - *p++ = (unsigned char)( ( ext_len ) & 0xFF ); + p = mbedtls_platform_put_uint16_be( p, ext_len ); p += ext_len; } @@ -3802,7 +3798,7 @@ static int ssl_parse_client_dh_public( mbedtls_ssl_context *ssl, unsigned char * return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_KEY_EXCHANGE ); } - n = ( (*p)[0] << 8 ) | (*p)[1]; + n = mbedtls_platform_get_uint16_be ( *p ); *p += 2; if( *p + n > end ) @@ -4058,7 +4054,7 @@ static int ssl_parse_client_psk_identity( mbedtls_ssl_context *ssl, unsigned cha return( MBEDTLS_ERR_SSL_BAD_HS_CLIENT_KEY_EXCHANGE ); } - n = ( (*p)[0] << 8 ) | (*p)[1]; + n = mbedtls_platform_get_uint16_be( *p ); *p += 2; if( n < 1 || n > 65535 || n > (size_t) ( end - *p ) ) diff --git a/library/ssl_ticket.c b/library/ssl_ticket.c index eeaeb52da..69e14cff7 100644 --- a/library/ssl_ticket.c +++ b/library/ssl_ticket.c @@ -298,7 +298,7 @@ int mbedtls_ssl_ticket_parse( void *p_ticket, if( ( ret = ssl_ticket_update_keys( ctx ) ) != 0 ) goto cleanup; - enc_len = ( enc_len_p[0] << 8 ) | enc_len_p[1]; + enc_len = mbedtls_platform_get_uint16_be( enc_len_p ); tag = ticket + enc_len; if( len != 4 + 12 + 2 + enc_len + 16 ) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 2829d6528..7f689afbf 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -2185,9 +2185,6 @@ int mbedtls_ssl_psk_derive_premaster( mbedtls_ssl_context *ssl, mbedtls_key_exch p = mbedtls_platform_put_uint16_be( p, zlen ); p += zlen; - - MBEDTLS_SSL_DEBUG_ECDH( 3, &ssl->handshake->ecdh_ctx, - MBEDTLS_DEBUG_ECDH_Z ); } else #endif /* MBEDTLS_KEY_EXCHANGE_ECDHE_PSK_ENABLED */ @@ -5380,7 +5377,7 @@ static int ssl_parse_record_header( mbedtls_ssl_context const *ssl, #if defined(MBEDTLS_SSL_PROTO_DTLS) if( MBEDTLS_SSL_TRANSPORT_IS_DTLS( ssl->conf->transport ) ) { - rec_epoch = ( rec->ctr[0] << 8 ) | rec->ctr[1]; + rec_epoch = (uint32_t)mbedtls_platform_get_uint16_be( rec->ctr ); /* Check that the datagram is large enough to contain a record * of the advertised length. */