From 03b64fa6c12140793aaad7a25df906ad70182bda Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 11 Jun 2019 14:39:38 +0100 Subject: [PATCH] Rearrange ExtendedMasterSecret parsing logic `mbedtls_ssl_handshake_params::extended_ms` holds the state of the ExtendedMasterSecret extension in the current handshake. Initially set to 'disabled' for both client and server, - the client sets it to 'enabled' as soon as it finds the ExtendedMS extension in the `ServerHello` and it has advertised that extension in its ClientHello, - the server sets it to 'enabled' as soon as it finds the ExtendedMS extension in the `ClientHello` and is willing to advertise is in its `ServerHello`. This commit slightly restructures this logic in prepraration for the removal of `mbedtls_ssl_handshake_params::extended_ms` in case both the use and the enforcement of the ExtendedMasterSecret extension have been fixed at compile-time. Namely, in this case there is no need for the `extended_ms` field in the handshake structure, as the ExtendedMS must be in use if the handshake progresses beyond the Hello stage. Paving the way for the removal of mbedtls_ssl_handshake_params::extended_ms this commit introduces a temporary variable tracking the presence of the ExtendedMS extension in the ClientHello/ServerHello messages, leaving the derivation of `extended_ms` (and potential failure) to the end of the parsing routine. --- library/ssl_cli.c | 24 +++++++++++++++--------- library/ssl_srv.c | 30 ++++++++++++++++-------------- 2 files changed, 31 insertions(+), 23 deletions(-) diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 238eeb14d..257a517f4 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -1341,9 +1341,6 @@ static int ssl_parse_extended_ms_ext( mbedtls_ssl_context *ssl, } ((void) buf); - - ssl->handshake->extended_ms = MBEDTLS_SSL_EXTENDED_MS_ENABLED; - return( 0 ); } #endif /* MBEDTLS_SSL_EXTENDED_MASTER_SECRET */ @@ -1603,6 +1600,9 @@ static int ssl_parse_server_hello( mbedtls_ssl_context *ssl ) #endif #if defined(MBEDTLS_SSL_RENEGOTIATION) int renegotiation_info_seen = 0; +#endif +#if defined(MBEDTLS_SSL_EXTENDED_MASTER_SECRET) + int extended_ms_seen = 0; #endif int handshake_failure = 0; const mbedtls_ssl_ciphersuite_t *suite_info; @@ -1984,6 +1984,7 @@ static int ssl_parse_server_hello( mbedtls_ssl_context *ssl ) { return( ret ); } + extended_ms_seen = 1; break; #endif /* MBEDTLS_SSL_EXTENDED_MASTER_SECRET */ @@ -2092,14 +2093,19 @@ static int ssl_parse_server_hello( mbedtls_ssl_context *ssl ) */ #if defined(MBEDTLS_SSL_EXTENDED_MASTER_SECRET) if( mbedtls_ssl_conf_get_ems( ssl->conf ) == - MBEDTLS_SSL_EXTENDED_MS_ENABLED && - mbedtls_ssl_conf_get_ems_enforced( ssl->conf ) == - MBEDTLS_SSL_EXTENDED_MS_ENFORCE_ENABLED && - ssl->handshake->extended_ms == MBEDTLS_SSL_EXTENDED_MS_DISABLED) + MBEDTLS_SSL_EXTENDED_MS_ENABLED ) { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "Peer not offering extended master " + if( extended_ms_seen ) + { + ssl->handshake->extended_ms = MBEDTLS_SSL_EXTENDED_MS_ENABLED; + } + else if( mbedtls_ssl_conf_get_ems_enforced( ssl->conf ) == + MBEDTLS_SSL_EXTENDED_MS_ENFORCE_ENABLED ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "Peer not offering extended master " "secret, while it is enforced") ); - handshake_failure = 1; + handshake_failure = 1; + } } #endif /* MBEDTLS_SSL_EXTENDED_MASTER_SECRET */ diff --git a/library/ssl_srv.c b/library/ssl_srv.c index 657fbaece..b01291841 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -567,13 +567,6 @@ static int ssl_parse_extended_ms_ext( mbedtls_ssl_context *ssl, ((void) buf); - if( mbedtls_ssl_conf_get_ems( ssl->conf ) == - MBEDTLS_SSL_EXTENDED_MS_ENABLED && - ssl->minor_ver != MBEDTLS_SSL_MINOR_VERSION_0 ) - { - ssl->handshake->extended_ms = MBEDTLS_SSL_EXTENDED_MS_ENABLED; - } - return( 0 ); } #endif /* MBEDTLS_SSL_EXTENDED_MASTER_SECRET */ @@ -1266,6 +1259,9 @@ static int ssl_parse_client_hello( mbedtls_ssl_context *ssl ) unsigned char *buf, *p, *ext; #if defined(MBEDTLS_SSL_RENEGOTIATION) int renegotiation_info_seen = 0; +#endif +#if defined(MBEDTLS_SSL_EXTENDED_MASTER_SECRET) + int extended_ms_seen = 0; #endif int handshake_failure = 0; const int *ciphersuites; @@ -1894,6 +1890,7 @@ read_record_header: ret = ssl_parse_extended_ms_ext( ssl, ext + 4, ext_size ); if( ret != 0 ) return( ret ); + extended_ms_seen = 1; break; #endif /* MBEDTLS_SSL_EXTENDED_MASTER_SECRET */ @@ -2041,14 +2038,19 @@ read_record_header: */ #if defined(MBEDTLS_SSL_EXTENDED_MASTER_SECRET) if( mbedtls_ssl_conf_get_ems( ssl->conf ) == - MBEDTLS_SSL_EXTENDED_MS_ENABLED && - mbedtls_ssl_conf_get_ems_enforced( ssl->conf ) == - MBEDTLS_SSL_EXTENDED_MS_ENFORCE_ENABLED && - ssl->handshake->extended_ms == MBEDTLS_SSL_EXTENDED_MS_DISABLED) + MBEDTLS_SSL_EXTENDED_MS_ENABLED ) { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "Peer not offering extended master " - "secret, while it is enforced") ); - handshake_failure = 1; + if( extended_ms_seen ) + { + ssl->handshake->extended_ms = MBEDTLS_SSL_EXTENDED_MS_ENABLED; + } + else if( mbedtls_ssl_conf_get_ems_enforced( ssl->conf ) == + MBEDTLS_SSL_EXTENDED_MS_ENFORCE_ENABLED ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "Peer not offering extended master " + "secret, while it is enforced") ); + handshake_failure = 1; + } } #endif /* MBEDTLS_SSL_EXTENDED_MASTER_SECRET */