From d27d1a5a82b4b0cf63cef05698bc8918883e7868 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 15 Aug 2017 11:49:08 +0200 Subject: [PATCH] Clean up existing SSL restartable ECC code - more consistent naming with ecrs prefix for everything - always check it enabled before touching the rest - rm duplicated code in parse_server_hello() --- include/mbedtls/ssl_internal.h | 4 +-- library/ssl_cli.c | 63 +++++++++++++++++++--------------- library/ssl_tls.c | 4 +-- 3 files changed, 40 insertions(+), 31 deletions(-) diff --git a/include/mbedtls/ssl_internal.h b/include/mbedtls/ssl_internal.h index 51dd4e1df..bc38b8b03 100644 --- a/include/mbedtls/ssl_internal.h +++ b/include/mbedtls/ssl_internal.h @@ -227,8 +227,8 @@ struct mbedtls_ssl_handshake_params #endif /* MBEDTLS_SSL_SERVER_NAME_INDICATION */ #endif /* MBEDTLS_X509_CRT_PARSE_C */ #if defined(MBEDTLS_SSL__ECP_RESTARTABLE) - int ec_restart_enabled; /*!< Handshake supports EC restart? */ - mbedtls_ecdsa_restart_ctx rs_ctx; /*!< ECDSA restart context */ + int ecrs_enabled; /*!< Handshake supports EC restart? */ + mbedtls_ecdsa_restart_ctx ecrs_ctx; /*!< ECDSA restart context */ enum { ssl_ecrs_init = 0, /*!< just getting started */ ssl_ecrs_ske_read, /*!< ServerKeyExchange was read */ diff --git a/library/ssl_cli.c b/library/ssl_cli.c index cbd46475c..db57713a6 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -1701,7 +1701,7 @@ static int ssl_parse_server_hello( mbedtls_ssl_context *ssl ) if( suite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA && ssl->minor_ver == MBEDTLS_SSL_MINOR_VERSION_3 ) { - ssl->handshake->ec_restart_enabled = 1; + ssl->handshake->ecrs_enabled = 1; } #endif @@ -1723,14 +1723,6 @@ static int ssl_parse_server_hello( mbedtls_ssl_context *ssl ) } } -#if defined(MBEDTLS_SSL__ECP_RESTARTABLE) - if( suite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA && - ssl->minor_ver == MBEDTLS_SSL_MINOR_VERSION_3 ) - { - ssl->handshake->ec_restart_enabled = 1; - } -#endif - if( comp != MBEDTLS_SSL_COMPRESS_NULL #if defined(MBEDTLS_ZLIB_SUPPORT) && comp != MBEDTLS_SSL_COMPRESS_DEFLATE @@ -2312,8 +2304,11 @@ static int ssl_parse_server_key_exchange( mbedtls_ssl_context *ssl ) MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA_ENABLED */ #if defined(MBEDTLS_SSL__ECP_RESTARTABLE) - if( ssl->handshake->ecrs_state == ssl_ecrs_ske_read ) - goto ske_process; + if( ssl->handshake->ecrs_enabled && + ssl->handshake->ecrs_state == ssl_ecrs_ske_read ) + { + goto ske_process; + } #endif if( ( ret = mbedtls_ssl_read_record( ssl ) ) != 0 ) @@ -2323,6 +2318,7 @@ static int ssl_parse_server_key_exchange( mbedtls_ssl_context *ssl ) } #if defined(MBEDTLS_SSL__ECP_RESTARTABLE) + if( ssl->handshake->ecrs_enabled ) ssl->handshake->ecrs_state++; ske_process: @@ -2618,8 +2614,8 @@ ske_process: } #if defined(MBEDTLS_SSL__ECP_RESTARTABLE) - if( ssl->handshake->ec_restart_enabled ) - rs_ctx = &ssl->handshake->rs_ctx; + if( ssl->handshake->ecrs_enabled ) + rs_ctx = &ssl->handshake->ecrs_ctx; #endif if( ( ret = mbedtls_pk_verify_restartable( @@ -2636,7 +2632,8 @@ ske_process: } #if defined(MBEDTLS_SSL__ECP_RESTARTABLE) - ssl->handshake->ecrs_state++; + if( ssl->handshake->ecrs_enabled ) + ssl->handshake->ecrs_state++; #endif } #endif /* MBEDTLS_KEY_EXCHANGE__WITH_SERVER_SIGNATURE__ENABLED */ @@ -2902,11 +2899,13 @@ static int ssl_write_client_key_exchange( mbedtls_ssl_context *ssl ) i = 4; #if defined(MBEDTLS_SSL__ECP_RESTARTABLE) - if( ssl->handshake->ec_restart_enabled) - mbedtls_ecdh_enable_restart( &ssl->handshake->ecdh_ctx ); + if( ssl->handshake->ecrs_enabled ) + { + if( ssl->handshake->ecrs_state == ssl_ecrs_ecdh_public_done ) + goto ecdh_calc_secret; - if( ssl->handshake->ecrs_state == ssl_ecrs_ecdh_public_done ) - goto ecdh_calc_secret; + mbedtls_ecdh_enable_restart( &ssl->handshake->ecdh_ctx ); + } #endif ret = mbedtls_ecdh_make_public( &ssl->handshake->ecdh_ctx, @@ -2922,11 +2921,15 @@ static int ssl_write_client_key_exchange( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_DEBUG_ECP( 3, "ECDH: Q", &ssl->handshake->ecdh_ctx.Q ); #if defined(MBEDTLS_SSL__ECP_RESTARTABLE) - ssl->handshake->ecrs_n = n; - ssl->handshake->ecrs_state++; + if( ssl->handshake->ecrs_enabled ) + { + ssl->handshake->ecrs_n = n; + ssl->handshake->ecrs_state++; + } ecdh_calc_secret: - n = ssl->handshake->ecrs_n; + if( ssl->handshake->ecrs_enabled ) + n = ssl->handshake->ecrs_n; #endif if( ( ret = mbedtls_ecdh_calc_secret( &ssl->handshake->ecdh_ctx, &ssl->handshake->pmslen, @@ -2941,7 +2944,8 @@ ecdh_calc_secret: MBEDTLS_SSL_DEBUG_MPI( 3, "ECDH: z", &ssl->handshake->ecdh_ctx.z ); #if defined(MBEDTLS_SSL__ECP_RESTARTABLE) - ssl->handshake->ecrs_state++; + if( ssl->handshake->ecrs_enabled ) + ssl->handshake->ecrs_state++; #endif } else @@ -3162,8 +3166,11 @@ static int ssl_write_certificate_verify( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> write certificate verify" ) ); #if defined(MBEDTLS_SSL__ECP_RESTARTABLE) - if( ssl->handshake->ecrs_state == ssl_ecrs_keys_derived ) + if( ssl->handshake->ecrs_enabled && + ssl->handshake->ecrs_state == ssl_ecrs_keys_derived ) + { goto keys_derived; + } #endif if( ( ret = mbedtls_ssl_derive_keys( ssl ) ) != 0 ) @@ -3173,7 +3180,8 @@ static int ssl_write_certificate_verify( mbedtls_ssl_context *ssl ) } #if defined(MBEDTLS_SSL__ECP_RESTARTABLE) - ssl->handshake->ecrs_state++; + if( ssl->handshake->ecrs_enabled ) + ssl->handshake->ecrs_state++; keys_derived: #endif @@ -3281,8 +3289,8 @@ keys_derived: } #if defined(MBEDTLS_SSL__ECP_RESTARTABLE) - if( ssl->handshake->ec_restart_enabled ) - rs_ctx = &ssl->handshake->rs_ctx; + if( ssl->handshake->ecrs_enabled ) + rs_ctx = &ssl->handshake->ecrs_ctx; #endif if( ( ret = mbedtls_pk_sign_restartable( mbedtls_ssl_own_key( ssl ), @@ -3295,7 +3303,8 @@ keys_derived: } #if defined(MBEDTLS_SSL__ECP_RESTARTABLE) - ssl->handshake->ecrs_state++; + if( ssl->handshake->ecrs_enabled ) + ssl->handshake->ecrs_state++; #endif ssl->out_msg[4 + offset] = (unsigned char)( n >> 8 ); diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 50222c375..90331efa1 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -5473,7 +5473,7 @@ static void ssl_handshake_params_init( mbedtls_ssl_handshake_params *handshake ) #endif #if defined(MBEDTLS_SSL__ECP_RESTARTABLE) - mbedtls_ecdsa_restart_init( &handshake->rs_ctx ); + mbedtls_ecdsa_restart_init( &handshake->ecrs_ctx ); #endif #if defined(MBEDTLS_SSL_SERVER_NAME_INDICATION) @@ -7309,7 +7309,7 @@ void mbedtls_ssl_handshake_free( mbedtls_ssl_handshake_params *handshake ) #endif /* MBEDTLS_X509_CRT_PARSE_C && MBEDTLS_SSL_SERVER_NAME_INDICATION */ #if defined(MBEDTLS_SSL__ECP_RESTARTABLE) - mbedtls_ecdsa_restart_free( &handshake->rs_ctx ); + mbedtls_ecdsa_restart_free( &handshake->ecrs_ctx ); #endif #if defined(MBEDTLS_SSL_PROTO_DTLS)