SSL: rework restart state handling

As done by previous commits for ECC and ECDSA:
- use explicit state assignments rather than increment
- always place the state update right before the operation label

This will make it easier to add restart support for other operations later if
desired.

SSL-specific changes:
- remove useless states: when the last restartable operation on a message is
  complete, ssl->state is incremented already, so we don't need any additional
state update: ecrs_state is only meant to complement ssl->state
- rename remaining states consistently as <message>_<operation>
- move some labels closer to the actual operation when possible (no assignment
  to variables used after the label between its previous and current position)
This commit is contained in:
Manuel Pégourié-Gonnard 2017-08-24 12:08:33 +02:00
parent 6348181da9
commit 0b23f167ba
3 changed files with 30 additions and 53 deletions

View File

@ -229,18 +229,14 @@ struct mbedtls_ssl_handshake_params
#if defined(MBEDTLS_SSL__ECP_RESTARTABLE) #if defined(MBEDTLS_SSL__ECP_RESTARTABLE)
int ecrs_enabled; /*!< Handshake supports EC restart? */ int ecrs_enabled; /*!< Handshake supports EC restart? */
mbedtls_x509_crt_restart_ctx ecrs_ctx; /*!< restart context */ mbedtls_x509_crt_restart_ctx ecrs_ctx; /*!< restart context */
enum { enum { /* this complements ssl->state with info on intra-state operations */
ssl_ecrs_init = 0, /*!< just getting started */ ssl_ecrs_none = 0, /*!< nothing going on (yet) */
ssl_ecrs_crt_parsed, /*!< server certificate was parsed */ ssl_ecrs_crt_verify, /*!< Certificate: crt_verify() */
ssl_ecrs_crt_verified, /*!< server certificate was verified*/ ssl_ecrs_ske_start_processing, /*!< ServerKeyExchange: step 1 */
ssl_ecrs_ske_read, /*!< ServerKeyExchange was read */ ssl_ecrs_ske_ecdh_calc_secret, /*!< ServerKeyExchange: ECDH step 2 */
ssl_ecrs_ske_verified, /*!< ServerKeyExchange was verified */ ssl_ecrs_crt_vrfy_sign, /*!< CertificateVerify: pk_sign() */
ssl_ecrs_ecdh_public_done, /*!< wrote ECDHE public share */ } ecrs_state; /*!< current (or last) operation */
ssl_ecrs_ecdh_completed, /*!< completed ECDHE key exchange */ size_t ecrs_n; /*!< place for saving a length */
ssl_ecrs_keys_derived, /*!< ssl_derive_keys() done */
ssl_ecrs_pk_sign_done, /*!< done writing CertificateVerify */
} ecrs_state; /*!< state for restartable ECC */
size_t ecrs_n; /*!< place for seving a length */
#endif #endif
#if defined(MBEDTLS_SSL_PROTO_DTLS) #if defined(MBEDTLS_SSL_PROTO_DTLS)
unsigned int out_msg_seq; /*!< Outgoing handshake sequence number */ unsigned int out_msg_seq; /*!< Outgoing handshake sequence number */

View File

@ -2305,9 +2305,9 @@ static int ssl_parse_server_key_exchange( mbedtls_ssl_context *ssl )
#if defined(MBEDTLS_SSL__ECP_RESTARTABLE) #if defined(MBEDTLS_SSL__ECP_RESTARTABLE)
if( ssl->handshake->ecrs_enabled && if( ssl->handshake->ecrs_enabled &&
ssl->handshake->ecrs_state == ssl_ecrs_ske_read ) ssl->handshake->ecrs_state == ssl_ecrs_ske_start_processing )
{ {
goto ske_process; goto start_processing;
} }
#endif #endif
@ -2317,12 +2317,6 @@ static int ssl_parse_server_key_exchange( mbedtls_ssl_context *ssl )
return( ret ); return( ret );
} }
#if defined(MBEDTLS_SSL__ECP_RESTARTABLE)
if( ssl->handshake->ecrs_enabled )
ssl->handshake->ecrs_state++;
ske_process:
#endif
if( ssl->in_msgtype != MBEDTLS_SSL_MSG_HANDSHAKE ) if( ssl->in_msgtype != MBEDTLS_SSL_MSG_HANDSHAKE )
{ {
MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad server key exchange message" ) ); MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad server key exchange message" ) );
@ -2354,6 +2348,12 @@ ske_process:
return( MBEDTLS_ERR_SSL_UNEXPECTED_MESSAGE ); return( MBEDTLS_ERR_SSL_UNEXPECTED_MESSAGE );
} }
#if defined(MBEDTLS_SSL__ECP_RESTARTABLE)
if( ssl->handshake->ecrs_enabled )
ssl->handshake->ecrs_state = ssl_ecrs_ske_start_processing;
start_processing:
#endif
p = ssl->in_msg + mbedtls_ssl_hs_hdr_len( ssl ); p = ssl->in_msg + mbedtls_ssl_hs_hdr_len( ssl );
end = ssl->in_msg + ssl->in_hslen; end = ssl->in_msg + ssl->in_hslen;
MBEDTLS_SSL_DEBUG_BUF( 3, "server key exchange", p, end - p ); MBEDTLS_SSL_DEBUG_BUF( 3, "server key exchange", p, end - p );
@ -2630,11 +2630,6 @@ ske_process:
MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_pk_verify", ret ); MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_pk_verify", ret );
return( ret ); return( ret );
} }
#if defined(MBEDTLS_SSL__ECP_RESTARTABLE)
if( ssl->handshake->ecrs_enabled )
ssl->handshake->ecrs_state++;
#endif
} }
#endif /* MBEDTLS_KEY_EXCHANGE__WITH_SERVER_SIGNATURE__ENABLED */ #endif /* MBEDTLS_KEY_EXCHANGE__WITH_SERVER_SIGNATURE__ENABLED */
@ -2901,7 +2896,7 @@ static int ssl_write_client_key_exchange( mbedtls_ssl_context *ssl )
#if defined(MBEDTLS_SSL__ECP_RESTARTABLE) #if defined(MBEDTLS_SSL__ECP_RESTARTABLE)
if( ssl->handshake->ecrs_enabled ) if( ssl->handshake->ecrs_enabled )
{ {
if( ssl->handshake->ecrs_state == ssl_ecrs_ecdh_public_done ) if( ssl->handshake->ecrs_state == ssl_ecrs_ske_ecdh_calc_secret )
goto ecdh_calc_secret; goto ecdh_calc_secret;
mbedtls_ecdh_enable_restart( &ssl->handshake->ecdh_ctx ); mbedtls_ecdh_enable_restart( &ssl->handshake->ecdh_ctx );
@ -2924,7 +2919,7 @@ static int ssl_write_client_key_exchange( mbedtls_ssl_context *ssl )
if( ssl->handshake->ecrs_enabled ) if( ssl->handshake->ecrs_enabled )
{ {
ssl->handshake->ecrs_n = n; ssl->handshake->ecrs_n = n;
ssl->handshake->ecrs_state++; ssl->handshake->ecrs_state = ssl_ecrs_ske_ecdh_calc_secret;
} }
ecdh_calc_secret: ecdh_calc_secret:
@ -2942,11 +2937,6 @@ ecdh_calc_secret:
} }
MBEDTLS_SSL_DEBUG_MPI( 3, "ECDH: z", &ssl->handshake->ecdh_ctx.z ); MBEDTLS_SSL_DEBUG_MPI( 3, "ECDH: z", &ssl->handshake->ecdh_ctx.z );
#if defined(MBEDTLS_SSL__ECP_RESTARTABLE)
if( ssl->handshake->ecrs_enabled )
ssl->handshake->ecrs_state++;
#endif
} }
else else
#endif /* MBEDTLS_KEY_EXCHANGE_ECDHE_RSA_ENABLED || #endif /* MBEDTLS_KEY_EXCHANGE_ECDHE_RSA_ENABLED ||
@ -3167,9 +3157,9 @@ static int ssl_write_certificate_verify( mbedtls_ssl_context *ssl )
#if defined(MBEDTLS_SSL__ECP_RESTARTABLE) #if defined(MBEDTLS_SSL__ECP_RESTARTABLE)
if( ssl->handshake->ecrs_enabled && if( ssl->handshake->ecrs_enabled &&
ssl->handshake->ecrs_state == ssl_ecrs_keys_derived ) ssl->handshake->ecrs_state == ssl_ecrs_crt_vrfy_sign )
{ {
goto keys_derived; goto sign;
} }
#endif #endif
@ -3179,12 +3169,6 @@ static int ssl_write_certificate_verify( mbedtls_ssl_context *ssl )
return( ret ); return( ret );
} }
#if defined(MBEDTLS_SSL__ECP_RESTARTABLE)
if( ssl->handshake->ecrs_enabled )
ssl->handshake->ecrs_state++;
keys_derived:
#endif
if( ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_PSK || if( ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_PSK ||
ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_RSA_PSK || ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_RSA_PSK ||
ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_ECDHE_PSK || ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_ECDHE_PSK ||
@ -3210,8 +3194,15 @@ keys_derived:
} }
/* /*
* Make an RSA signature of the handshake digests * Make a signature of the handshake digests
*/ */
#if defined(MBEDTLS_SSL__ECP_RESTARTABLE)
if( ssl->handshake->ecrs_enabled )
ssl->handshake->ecrs_state = ssl_ecrs_crt_vrfy_sign;
sign:
#endif
ssl->handshake->calc_verify( ssl, hash ); ssl->handshake->calc_verify( ssl, hash );
#if defined(MBEDTLS_SSL_PROTO_SSL3) || defined(MBEDTLS_SSL_PROTO_TLS1) || \ #if defined(MBEDTLS_SSL_PROTO_SSL3) || defined(MBEDTLS_SSL_PROTO_TLS1) || \
@ -3302,11 +3293,6 @@ keys_derived:
return( ret ); return( ret );
} }
#if defined(MBEDTLS_SSL__ECP_RESTARTABLE)
if( ssl->handshake->ecrs_enabled )
ssl->handshake->ecrs_state++;
#endif
ssl->out_msg[4 + offset] = (unsigned char)( n >> 8 ); ssl->out_msg[4 + offset] = (unsigned char)( n >> 8 );
ssl->out_msg[5 + offset] = (unsigned char)( n ); ssl->out_msg[5 + offset] = (unsigned char)( n );

View File

@ -4554,7 +4554,7 @@ int mbedtls_ssl_parse_certificate( mbedtls_ssl_context *ssl )
#if defined(MBEDTLS_SSL__ECP_RESTARTABLE) #if defined(MBEDTLS_SSL__ECP_RESTARTABLE)
if( ssl->handshake->ecrs_enabled && if( ssl->handshake->ecrs_enabled &&
ssl->handshake->ecrs_state == ssl_ecrs_crt_parsed ) ssl->handshake->ecrs_state == ssl_ecrs_crt_verify )
{ {
goto crt_verify; goto crt_verify;
} }
@ -4584,7 +4584,7 @@ int mbedtls_ssl_parse_certificate( mbedtls_ssl_context *ssl )
#if defined(MBEDTLS_SSL__ECP_RESTARTABLE) #if defined(MBEDTLS_SSL__ECP_RESTARTABLE)
if( ssl->handshake->ecrs_enabled) if( ssl->handshake->ecrs_enabled)
ssl->handshake->ecrs_state++; ssl->handshake->ecrs_state = ssl_ecrs_crt_verify;
crt_verify: crt_verify:
if( ssl->handshake->ecrs_enabled) if( ssl->handshake->ecrs_enabled)
@ -4726,11 +4726,6 @@ crt_verify:
#endif /* MBEDTLS_DEBUG_C */ #endif /* MBEDTLS_DEBUG_C */
} }
#if defined(MBEDTLS_SSL__ECP_RESTARTABLE)
if( ssl->handshake->ecrs_enabled)
ssl->handshake->ecrs_state++;
#endif
ssl->state++; ssl->state++;
MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= parse certificate" ) ); MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= parse certificate" ) );