From e965bd397e2299280184228109fc57847b68b104 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Wed, 12 Jun 2019 14:04:34 +0100 Subject: [PATCH 01/17] Allow hardcoding of min/max minor/major SSL version at compile-time This commit introduces the numeric compile-time constants - MBEDTLS_SSL_CONF_MIN_MINOR_VER - MBEDTLS_SSL_CONF_MAX_MINOR_VER - MBEDTLS_SSL_CONF_MIN_MAJOR_VER - MBEDTLS_SSL_CONF_MAX_MAJOR_VER which, when defined, overwrite the runtime configurable fields mbedtls_ssl_config::min_major_ver etc. in the SSL configuration. As for the preceding case of the ExtendedMasterSecret configuration, it also introduces and puts to use getter functions for these variables which evaluate to either a field access or the macro value, maintaining readability of the code. The runtime configuration API mbedtls_ssl_conf_{min|max}_version() is kept for now but has no effect if MBEDTLS_SSL_CONF_XXX are set. This is likely to be changed in a later commit but deliberately omitted for now, in order to be able to study code-size benefits earlier in the process. --- configs/baremetal.h | 4 +++ include/mbedtls/config.h | 6 ++++ include/mbedtls/ssl.h | 8 +++++ include/mbedtls/ssl_internal.h | 44 +++++++++++++++++++++++++++ library/ssl_cli.c | 54 +++++++++++++++++++--------------- library/ssl_srv.c | 33 ++++++++++++--------- library/ssl_tls.c | 51 +++++++++++++++++++++++++++++--- 7 files changed, 159 insertions(+), 41 deletions(-) diff --git a/configs/baremetal.h b/configs/baremetal.h index 2e92e76ac..09391ddb0 100644 --- a/configs/baremetal.h +++ b/configs/baremetal.h @@ -100,6 +100,10 @@ #define MBEDTLS_SSL_CONF_SEND mbedtls_net_send #define MBEDTLS_SSL_CONF_RECV_TIMEOUT mbedtls_net_recv_timeout #define MBEDTLS_SSL_CONF_RNG mbedtls_hmac_drbg_random +#define MBEDTLS_SSL_CONF_MIN_MINOR_VER MBEDTLS_SSL_MINOR_VERSION_3 +#define MBEDTLS_SSL_CONF_MAX_MINOR_VER MBEDTLS_SSL_MINOR_VERSION_3 +#define MBEDTLS_SSL_CONF_MIN_MAJOR_VER MBEDTLS_SSL_MAJOR_VERSION_3 +#define MBEDTLS_SSL_CONF_MAX_MAJOR_VER MBEDTLS_SSL_MAJOR_VERSION_3 #define MBEDTLS_SSL_CONF_EXTENDED_MASTER_SECRET \ MBEDTLS_SSL_EXTENDED_MS_ENABLED #define MBEDTLS_SSL_CONF_ENFORCE_EXTENDED_MASTER_SECRET \ diff --git a/include/mbedtls/config.h b/include/mbedtls/config.h index 8290c516d..e18c11bee 100644 --- a/include/mbedtls/config.h +++ b/include/mbedtls/config.h @@ -3636,6 +3636,12 @@ */ //#define MBEDTLS_SSL_CONF_RNG mbedtls_ctr_drbg_random +/* TLS version */ +//#define MBEDTLS_SSL_CONF_MIN_MINOR_VER MBEDTLS_SSL_MINOR_VERSION_3 +//#define MBEDTLS_SSL_CONF_MAX_MINOR_VER MBEDTLS_SSL_MINOR_VERSION_3 +//#define MBEDTLS_SSL_CONF_MIN_MAJOR_VER MBEDTLS_SSL_MAJOR_VERSION_3 +//#define MBEDTLS_SSL_CONF_MAX_MAJOR_VER MBEDTLS_SSL_MAJOR_VERSION_3 + /* ExtendedMasterSecret extension * The following two options must be set/unset simultaneously. */ //#define MBEDTLS_SSL_CONF_EXTENDED_MASTER_SECRET MBEDTLS_SSL_EXTENDED_MS_ENABLED diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 169e054c0..ee8bd818b 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -1132,10 +1132,18 @@ struct mbedtls_ssl_config unsigned int dhm_min_bitlen; /*!< min. bit length of the DHM prime */ #endif +#if !defined(MBEDTLS_SSL_CONF_MAX_MAJOR_VER) unsigned char max_major_ver; /*!< max. major version used */ +#endif /* !MBEDTLS_SSL_CONF_MAX_MAJOR_VER */ +#if !defined(MBEDTLS_SSL_CONF_MAX_MINOR_VER) unsigned char max_minor_ver; /*!< max. minor version used */ +#endif /* !MBEDTLS_SSL_CONF_MAX_MINOR_VER */ +#if !defined(MBEDTLS_SSL_CONF_MIN_MAJOR_VER) unsigned char min_major_ver; /*!< min. major version used */ +#endif /* !MBEDTLS_SSL_CONF_MIN_MAJOR_VER */ +#if !defined(MBEDTLS_SSL_CONF_MIN_MINOR_VER) unsigned char min_minor_ver; /*!< min. minor version used */ +#endif /* !MBEDTLS_SSL_CONF_MIN_MINOR_VER */ /* * Flags (bitfields) diff --git a/include/mbedtls/ssl_internal.h b/include/mbedtls/ssl_internal.h index 8a515772d..d2299ea38 100644 --- a/include/mbedtls/ssl_internal.h +++ b/include/mbedtls/ssl_internal.h @@ -1423,6 +1423,50 @@ static inline mbedtls_frng_t* mbedtls_ssl_conf_get_frng( } #endif /* MBEDTLS_SSL_CONF_RNG */ +static inline int mbedtls_ssl_conf_get_max_major_ver( + mbedtls_ssl_config const *conf ) +{ +#if !defined(MBEDTLS_SSL_CONF_MAX_MAJOR_VER) + return( conf->max_major_ver ); +#else + ((void) conf); + return( MBEDTLS_SSL_CONF_MAX_MAJOR_VER ); +#endif /* MBEDTLS_SSL_CONF_MAX_MAJOR_VER */ +} + +static inline int mbedtls_ssl_conf_get_min_major_ver( + mbedtls_ssl_config const *conf ) +{ +#if !defined(MBEDTLS_SSL_CONF_MIN_MAJOR_VER) + return( conf->min_major_ver ); +#else /* !MBEDTLS_SSL_CONF_MIN_MAJOR_VER */ + ((void) conf); + return( MBEDTLS_SSL_CONF_MIN_MAJOR_VER ); +#endif /* MBEDTLS_SSL_CONF_MIN_MAJOR_VER */ +} + +static inline int mbedtls_ssl_conf_get_max_minor_ver( + mbedtls_ssl_config const *conf ) +{ +#if !defined(MBEDTLS_SSL_CONF_MAX_MINOR_VER) + return( conf->max_minor_ver ); +#else /* !MBEDTLS_SSL_CONF_MAX_MINOR_VER */ + ((void) conf); + return( MBEDTLS_SSL_CONF_MAX_MINOR_VER ); +#endif /* MBEDTLS_SSL_CONF_MAX_MINOR_VER */ +} + +static inline int mbedtls_ssl_conf_get_min_minor_ver( + mbedtls_ssl_config const *conf ) +{ +#if !defined(MBEDTLS_SSL_CONF_MIN_MINOR_VER) + return( conf->min_minor_ver ); +#else /* !MBEDTLS_SSL_CONF_MIN_MINOR_VER */ + ((void) conf); + return( MBEDTLS_SSL_CONF_MIN_MINOR_VER ); +#endif /* MBEDTLS_SSL_CONF_MIN_MINOR_VER */ +} + #if defined(MBEDTLS_SSL_EXTENDED_MASTER_SECRET) static inline unsigned int mbedtls_ssl_conf_get_ems( mbedtls_ssl_config const *conf ) diff --git a/library/ssl_cli.c b/library/ssl_cli.c index d45f3d3fe..7291fd753 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -180,8 +180,11 @@ static void ssl_write_signature_algorithms_ext( mbedtls_ssl_context *ssl, *olen = 0; - if( ssl->conf->max_minor_ver != MBEDTLS_SSL_MINOR_VERSION_3 ) + if( mbedtls_ssl_conf_get_max_minor_ver( ssl->conf ) != + MBEDTLS_SSL_MINOR_VERSION_3 ) + { return; + } MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, adding signature_algorithms extension" ) ); @@ -558,7 +561,8 @@ static void ssl_write_encrypt_then_mac_ext( mbedtls_ssl_context *ssl, *olen = 0; if( ssl->conf->encrypt_then_mac == MBEDTLS_SSL_ETM_DISABLED || - ssl->conf->max_minor_ver == MBEDTLS_SSL_MINOR_VERSION_0 ) + mbedtls_ssl_conf_get_max_minor_ver( ssl->conf ) == + MBEDTLS_SSL_MINOR_VERSION_0 ) { return; } @@ -593,7 +597,8 @@ static void ssl_write_extended_ms_ext( mbedtls_ssl_context *ssl, if( mbedtls_ssl_conf_get_ems( ssl->conf ) == MBEDTLS_SSL_EXTENDED_MS_DISABLED || - ssl->conf->max_minor_ver == MBEDTLS_SSL_MINOR_VERSION_0 ) + mbedtls_ssl_conf_get_max_minor_ver( ssl->conf ) == + MBEDTLS_SSL_MINOR_VERSION_0 ) { return; } @@ -788,7 +793,6 @@ static int ssl_validate_ciphersuite( mbedtls_ssl_ciphersuite_handle_t suite_info if( suite_info == MBEDTLS_SSL_CIPHERSUITE_INVALID_HANDLE ) return( 1 ); - if( mbedtls_ssl_suite_get_min_minor_ver( suite_info ) > max_minor_ver || mbedtls_ssl_suite_get_max_minor_ver( suite_info ) < min_minor_ver ) { @@ -846,11 +850,11 @@ static int ssl_write_client_hello( mbedtls_ssl_context *ssl ) if( mbedtls_ssl_get_renego_status( ssl ) == MBEDTLS_SSL_INITIAL_HANDSHAKE ) { - ssl->major_ver = ssl->conf->min_major_ver; - ssl->minor_ver = ssl->conf->min_minor_ver; + ssl->major_ver = mbedtls_ssl_conf_get_min_major_ver( ssl->conf ); + ssl->minor_ver = mbedtls_ssl_conf_get_min_minor_ver( ssl->conf ); } - if( ssl->conf->max_major_ver == 0 ) + if( mbedtls_ssl_conf_get_max_major_ver( ssl->conf ) == 0 ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "configured max major version is invalid, " "consider using mbedtls_ssl_config_defaults()" ) ); @@ -867,8 +871,9 @@ static int ssl_write_client_hello( mbedtls_ssl_context *ssl ) buf = ssl->out_msg; p = buf + 4; - mbedtls_ssl_write_version( ssl->conf->max_major_ver, ssl->conf->max_minor_ver, - ssl->conf->transport, p ); + mbedtls_ssl_write_version( mbedtls_ssl_conf_get_max_major_ver( ssl->conf ), + mbedtls_ssl_conf_get_max_minor_ver( ssl->conf ), + ssl->conf->transport, p ); p += 2; MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, max version: [%d:%d]", @@ -981,8 +986,8 @@ static int ssl_write_client_hello( mbedtls_ssl_context *ssl ) ciphersuite_info ) { if( ssl_validate_ciphersuite( ciphersuite_info, ssl, - ssl->conf->min_minor_ver, - ssl->conf->max_minor_ver ) != 0 ) + mbedtls_ssl_conf_get_min_minor_ver( ssl->conf ), + mbedtls_ssl_conf_get_max_minor_ver( ssl->conf ) ) != 0 ) { continue; } @@ -1563,8 +1568,8 @@ static int ssl_parse_hello_verify_request( mbedtls_ssl_context *ssl ) */ if( major_ver < MBEDTLS_SSL_MAJOR_VERSION_3 || minor_ver < MBEDTLS_SSL_MINOR_VERSION_2 || - major_ver > ssl->conf->max_major_ver || - minor_ver > ssl->conf->max_minor_ver ) + major_ver > mbedtls_ssl_conf_get_max_major_ver( ssl->conf ) || + minor_ver > mbedtls_ssl_conf_get_max_minor_ver( ssl->conf ) ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad server version" ) ); @@ -1715,16 +1720,18 @@ static int ssl_parse_server_hello( mbedtls_ssl_context *ssl ) mbedtls_ssl_read_version( &ssl->major_ver, &ssl->minor_ver, ssl->conf->transport, buf + 0 ); - if( ssl->major_ver < ssl->conf->min_major_ver || - ssl->minor_ver < ssl->conf->min_minor_ver || - ssl->major_ver > ssl->conf->max_major_ver || - ssl->minor_ver > ssl->conf->max_minor_ver ) + if( ssl->major_ver < mbedtls_ssl_conf_get_min_major_ver( ssl->conf ) || + ssl->minor_ver < mbedtls_ssl_conf_get_min_minor_ver( ssl->conf ) || + ssl->major_ver > mbedtls_ssl_conf_get_max_major_ver( ssl->conf ) || + ssl->minor_ver > mbedtls_ssl_conf_get_max_minor_ver( ssl->conf ) ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "server version out of bounds - " " min: [%d:%d], server: [%d:%d], max: [%d:%d]", - ssl->conf->min_major_ver, ssl->conf->min_minor_ver, + mbedtls_ssl_conf_get_min_major_ver( ssl->conf ), + mbedtls_ssl_conf_get_min_minor_ver( ssl->conf ), ssl->major_ver, ssl->minor_ver, - ssl->conf->max_major_ver, ssl->conf->max_minor_ver ) ); + mbedtls_ssl_conf_get_max_major_ver( ssl->conf ), + mbedtls_ssl_conf_get_max_minor_ver( ssl->conf ) ) ); mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, MBEDTLS_SSL_ALERT_MSG_PROTOCOL_VERSION ); @@ -1886,8 +1893,8 @@ static int ssl_parse_server_hello( mbedtls_ssl_context *ssl ) ciphersuite_info ) { if( ssl_validate_ciphersuite( ciphersuite_info, ssl, - ssl->conf->min_minor_ver, - ssl->conf->max_minor_ver ) != 0 ) + mbedtls_ssl_conf_get_min_minor_ver( ssl->conf ), + mbedtls_ssl_conf_get_max_minor_ver( ssl->conf ) ) != 0 ) { continue; } @@ -2360,8 +2367,9 @@ static int ssl_write_encrypted_pms( mbedtls_ssl_context *ssl, * opaque random[46]; * } PreMasterSecret; */ - mbedtls_ssl_write_version( ssl->conf->max_major_ver, ssl->conf->max_minor_ver, - ssl->conf->transport, p ); + mbedtls_ssl_write_version( mbedtls_ssl_conf_get_max_major_ver( ssl->conf ), + mbedtls_ssl_conf_get_max_minor_ver( ssl->conf ), + ssl->conf->transport, p ); if( ( ret = mbedtls_ssl_conf_get_frng( ssl->conf ) ( ssl->conf->p_rng, p + 2, 46 ) ) != 0 ) diff --git a/library/ssl_srv.c b/library/ssl_srv.c index e743eff87..553ded29d 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -1090,15 +1090,17 @@ static int ssl_parse_client_hello_v2( mbedtls_ssl_context *ssl ) } ssl->major_ver = MBEDTLS_SSL_MAJOR_VERSION_3; - ssl->minor_ver = ( buf[4] <= ssl->conf->max_minor_ver ) - ? buf[4] : ssl->conf->max_minor_ver; + ssl->minor_ver = + ( buf[4] <= mbedtls_ssl_conf_get_max_minor_ver( ssl->conf ) ) + ? buf[4] : mbedtls_ssl_conf_get_max_minor_ver( ssl->conf ); - if( ssl->minor_ver < ssl->conf->min_minor_ver ) + if( ssl->minor_ver < mbedtls_ssl_conf_get_min_minor_ver( ssl->conf ) ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "client only supports ssl smaller than minimum" " [%d:%d] < [%d:%d]", ssl->major_ver, ssl->minor_ver, - ssl->conf->min_major_ver, ssl->conf->min_minor_ver ) ); + mbedtls_ssl_conf_get_min_major_ver( ssl->conf ), + mbedtls_ssl_conf_get_min_minor_ver( ssl->conf ) ) ); mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, MBEDTLS_SSL_ALERT_MSG_PROTOCOL_VERSION ); @@ -1213,7 +1215,8 @@ static int ssl_parse_client_hello_v2( mbedtls_ssl_context *ssl ) { MBEDTLS_SSL_DEBUG_MSG( 3, ( "received FALLBACK_SCSV" ) ); - if( ssl->minor_ver < ssl->conf->max_minor_ver ) + if( ssl->minor_ver < + mbedtls_ssl_conf_get_max_minor_ver( ssl->conf ) ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "inapropriate fallback" ) ); @@ -1624,25 +1627,26 @@ read_record_header: ssl->handshake->max_major_ver = ssl->major_ver; ssl->handshake->max_minor_ver = ssl->minor_ver; - if( ssl->major_ver < ssl->conf->min_major_ver || - ssl->minor_ver < ssl->conf->min_minor_ver ) + if( ssl->major_ver < mbedtls_ssl_conf_get_min_major_ver( ssl->conf ) || + ssl->minor_ver < mbedtls_ssl_conf_get_min_minor_ver( ssl->conf ) ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "client only supports ssl smaller than minimum" " [%d:%d] < [%d:%d]", ssl->major_ver, ssl->minor_ver, - ssl->conf->min_major_ver, ssl->conf->min_minor_ver ) ); + mbedtls_ssl_conf_get_min_major_ver( ssl->conf ), + mbedtls_ssl_conf_get_min_minor_ver( ssl->conf ) ) ); mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, MBEDTLS_SSL_ALERT_MSG_PROTOCOL_VERSION ); return( MBEDTLS_ERR_SSL_BAD_HS_PROTOCOL_VERSION ); } - if( ssl->major_ver > ssl->conf->max_major_ver ) + if( ssl->major_ver > mbedtls_ssl_conf_get_max_major_ver( ssl->conf ) ) { - ssl->major_ver = ssl->conf->max_major_ver; - ssl->minor_ver = ssl->conf->max_minor_ver; + ssl->major_ver = mbedtls_ssl_conf_get_max_major_ver( ssl->conf ); + ssl->minor_ver = mbedtls_ssl_conf_get_max_minor_ver( ssl->conf ); } - else if( ssl->minor_ver > ssl->conf->max_minor_ver ) - ssl->minor_ver = ssl->conf->max_minor_ver; + else if( ssl->minor_ver > mbedtls_ssl_conf_get_max_minor_ver( ssl->conf ) ) + ssl->minor_ver = mbedtls_ssl_conf_get_max_minor_ver( ssl->conf ); /* * Save client random (inc. Unix time) @@ -2019,7 +2023,8 @@ read_record_header: { MBEDTLS_SSL_DEBUG_MSG( 2, ( "received FALLBACK_SCSV" ) ); - if( ssl->minor_ver < ssl->conf->max_minor_ver ) + if( ssl->minor_ver < + mbedtls_ssl_conf_get_max_minor_ver( ssl->conf ) ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "inapropriate fallback" ) ); diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 2a2d3219e..0c4ba9afb 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -4705,7 +4705,7 @@ static int ssl_parse_record_header( mbedtls_ssl_context *ssl ) return( MBEDTLS_ERR_SSL_INVALID_RECORD ); } - if( minor_ver > ssl->conf->max_minor_ver ) + if( minor_ver > mbedtls_ssl_conf_get_max_minor_ver( ssl->conf ) ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "minor version mismatch" ) ); return( MBEDTLS_ERR_SSL_INVALID_RECORD ); @@ -8717,14 +8717,42 @@ const char *mbedtls_ssl_get_alpn_protocol( const mbedtls_ssl_context *ssl ) void mbedtls_ssl_conf_max_version( mbedtls_ssl_config *conf, int major, int minor ) { +#if defined(MBEDTLS_SSL_CONF_MAX_MAJOR_VER) && \ + defined(MBEDTLS_SSL_CONF_MAX_MINOR_VER) + ((void) conf); +#endif + +#if !defined(MBEDTLS_SSL_CONF_MAX_MAJOR_VER) conf->max_major_ver = major; +#else + ((void) major); +#endif /* MBEDTLS_SSL_CONF_MAX_MAJOR_VER */ + +#if !defined(MBEDTLS_SSL_CONF_MAX_MINOR_VER) conf->max_minor_ver = minor; +#else + ((void) minor); +#endif /* !MBEDTLS_SSL_CONF_MAX_MINOR_VER */ } void mbedtls_ssl_conf_min_version( mbedtls_ssl_config *conf, int major, int minor ) { +#if defined(MBEDTLS_SSL_CONF_MIN_MAJOR_VER) && \ + defined(MBEDTLS_SSL_CONF_MIN_MINOR_VER) + ((void) conf); +#endif + +#if !defined(MBEDTLS_SSL_CONF_MIN_MAJOR_VER) conf->min_major_ver = major; +#else + ((void) major); +#endif /* MBEDTLS_SSL_CONF_MIN_MAJOR_VER */ + +#if !defined(MBEDTLS_SSL_CONF_MIN_MINOR_VER) conf->min_minor_ver = minor; +#else + ((void) minor); +#endif /* !MBEDTLS_SSL_CONF_MIN_MINOR_VER */ } #if defined(MBEDTLS_SSL_FALLBACK_SCSV) && defined(MBEDTLS_SSL_CLI_C) @@ -10961,10 +10989,18 @@ int mbedtls_ssl_config_defaults( mbedtls_ssl_config *conf, * NSA Suite B */ case MBEDTLS_SSL_PRESET_SUITEB: +#if !defined(MBEDTLS_SSL_CONF_MIN_MAJOR_VER) conf->min_major_ver = MBEDTLS_SSL_MAJOR_VERSION_3; +#endif /* !MBEDTLS_SSL_CONF_MIN_MAJOR_VER */ +#if !defined(MBEDTLS_SSL_CONF_MIN_MINOR_VER) conf->min_minor_ver = MBEDTLS_SSL_MINOR_VERSION_3; /* TLS 1.2 */ +#endif /* !MBEDTLS_SSL_CONF_MIN_MINOR_VER */ +#if !defined(MBEDTLS_SSL_CONF_MAX_MAJOR_VER) conf->max_major_ver = MBEDTLS_SSL_MAX_MAJOR_VERSION; +#endif /* !MBEDTLS_SSL_CONF_MAX_MAJOR_VER */ +#if !defined(MBEDTLS_SSL_CONF_MAX_MINOR_VER) conf->max_minor_ver = MBEDTLS_SSL_MAX_MINOR_VERSION; +#endif /* !MBEDTLS_SSL_CONF_MAX_MINOR_VER */ #if !defined(MBEDTLS_SSL_CONF_SINGLE_CIPHERSUITE) conf->ciphersuite_list[MBEDTLS_SSL_MINOR_VERSION_0] = @@ -10991,21 +11027,28 @@ int mbedtls_ssl_config_defaults( mbedtls_ssl_config *conf, * Default */ default: +#if !defined(MBEDTLS_SSL_CONF_MIN_MAJOR_VER) conf->min_major_ver = ( MBEDTLS_SSL_MIN_MAJOR_VERSION > MBEDTLS_SSL_MIN_VALID_MAJOR_VERSION ) ? MBEDTLS_SSL_MIN_MAJOR_VERSION : MBEDTLS_SSL_MIN_VALID_MAJOR_VERSION; +#endif /* !MBEDTLS_SSL_CONF_MIN_MAJOR_VER */ +#if !defined(MBEDTLS_SSL_CONF_MIN_MINOR_VER) conf->min_minor_ver = ( MBEDTLS_SSL_MIN_MINOR_VERSION > MBEDTLS_SSL_MIN_VALID_MINOR_VERSION ) ? MBEDTLS_SSL_MIN_MINOR_VERSION : MBEDTLS_SSL_MIN_VALID_MINOR_VERSION; - conf->max_major_ver = MBEDTLS_SSL_MAX_MAJOR_VERSION; - conf->max_minor_ver = MBEDTLS_SSL_MAX_MINOR_VERSION; - #if defined(MBEDTLS_SSL_PROTO_DTLS) if( MBEDTLS_SSL_TRANSPORT_IS_DTLS( transport ) ) conf->min_minor_ver = MBEDTLS_SSL_MINOR_VERSION_2; #endif +#endif /* !MBEDTLS_SSL_CONF_MIN_MINOR_VER */ +#if !defined(MBEDTLS_SSL_CONF_MAX_MAJOR_VER) + conf->max_major_ver = MBEDTLS_SSL_MAX_MAJOR_VERSION; +#endif /* !MBEDTLS_SSL_CONF_MAX_MAJOR_VER */ +#if !defined(MBEDTLS_SSL_CONF_MAX_MINOR_VER) + conf->max_minor_ver = MBEDTLS_SSL_MAX_MINOR_VERSION; +#endif /* !MBEDTLS_SSL_CONF_MAX_MINOR_VER */ #if !defined(MBEDTLS_SSL_CONF_SINGLE_CIPHERSUITE) conf->ciphersuite_list[MBEDTLS_SSL_MINOR_VERSION_0] = From 3fa1ee567caae5f65ac373ba88c7f9cf7e0aed90 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Wed, 22 May 2019 14:44:37 +0100 Subject: [PATCH 02/17] Set SSL minor version only after validation --- library/ssl_cli.c | 44 ++++++++++++++++++++++++++------------------ library/ssl_srv.c | 47 +++++++++++++++++++++++++++-------------------- 2 files changed, 53 insertions(+), 38 deletions(-) diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 7291fd753..87dc25d0d 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -1716,27 +1716,35 @@ static int ssl_parse_server_hello( mbedtls_ssl_context *ssl ) */ buf += mbedtls_ssl_hs_hdr_len( ssl ); - MBEDTLS_SSL_DEBUG_BUF( 3, "server hello, version", buf + 0, 2 ); - mbedtls_ssl_read_version( &ssl->major_ver, &ssl->minor_ver, - ssl->conf->transport, buf + 0 ); - - if( ssl->major_ver < mbedtls_ssl_conf_get_min_major_ver( ssl->conf ) || - ssl->minor_ver < mbedtls_ssl_conf_get_min_minor_ver( ssl->conf ) || - ssl->major_ver > mbedtls_ssl_conf_get_max_major_ver( ssl->conf ) || - ssl->minor_ver > mbedtls_ssl_conf_get_max_minor_ver( ssl->conf ) ) { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "server version out of bounds - " - " min: [%d:%d], server: [%d:%d], max: [%d:%d]", - mbedtls_ssl_conf_get_min_major_ver( ssl->conf ), - mbedtls_ssl_conf_get_min_minor_ver( ssl->conf ), - ssl->major_ver, ssl->minor_ver, - mbedtls_ssl_conf_get_max_major_ver( ssl->conf ), - mbedtls_ssl_conf_get_max_minor_ver( ssl->conf ) ) ); + int major_ver, minor_ver; - mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, - MBEDTLS_SSL_ALERT_MSG_PROTOCOL_VERSION ); + MBEDTLS_SSL_DEBUG_BUF( 3, "server hello, version", buf + 0, 2 ); + mbedtls_ssl_read_version( &major_ver, &minor_ver, + ssl->conf->transport, + buf + 0 ); - return( MBEDTLS_ERR_SSL_BAD_HS_PROTOCOL_VERSION ); + if( major_ver < mbedtls_ssl_conf_get_min_major_ver( ssl->conf ) || + minor_ver < mbedtls_ssl_conf_get_min_minor_ver( ssl->conf ) || + major_ver > mbedtls_ssl_conf_get_max_major_ver( ssl->conf ) || + minor_ver > mbedtls_ssl_conf_get_max_minor_ver( ssl->conf ) ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "server version out of bounds - " + " min: [%d:%d], server: [%d:%d], max: [%d:%d]", + mbedtls_ssl_conf_get_min_major_ver( ssl->conf ), + mbedtls_ssl_conf_get_min_minor_ver( ssl->conf ), + major_ver, minor_ver, + mbedtls_ssl_conf_get_max_major_ver( ssl->conf ), + mbedtls_ssl_conf_get_max_minor_ver( ssl->conf ) ) ); + + mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, + MBEDTLS_SSL_ALERT_MSG_PROTOCOL_VERSION ); + + return( MBEDTLS_ERR_SSL_BAD_HS_PROTOCOL_VERSION ); + } + + ssl->minor_ver = minor_ver; + ssl->major_ver = major_ver; } MBEDTLS_SSL_DEBUG_MSG( 3, ( "server hello, current time: %lu", diff --git a/library/ssl_srv.c b/library/ssl_srv.c index 553ded29d..38c841d89 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -1621,32 +1621,39 @@ read_record_header: */ MBEDTLS_SSL_DEBUG_BUF( 3, "client hello, version", buf, 2 ); - mbedtls_ssl_read_version( &ssl->major_ver, &ssl->minor_ver, - ssl->conf->transport, buf ); - - ssl->handshake->max_major_ver = ssl->major_ver; - ssl->handshake->max_minor_ver = ssl->minor_ver; - - if( ssl->major_ver < mbedtls_ssl_conf_get_min_major_ver( ssl->conf ) || - ssl->minor_ver < mbedtls_ssl_conf_get_min_minor_ver( ssl->conf ) ) { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "client only supports ssl smaller than minimum" + int minor_ver, major_ver; + mbedtls_ssl_read_version( &major_ver, &minor_ver, + ssl->conf->transport, + buf ); + + ssl->handshake->max_major_ver = major_ver; + ssl->handshake->max_minor_ver = minor_ver; + + if( major_ver < mbedtls_ssl_conf_get_min_major_ver( ssl->conf ) || + minor_ver < mbedtls_ssl_conf_get_min_minor_ver( ssl->conf ) ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "client only supports ssl smaller than minimum" " [%d:%d] < [%d:%d]", - ssl->major_ver, ssl->minor_ver, + major_ver, minor_ver, mbedtls_ssl_conf_get_min_major_ver( ssl->conf ), mbedtls_ssl_conf_get_min_minor_ver( ssl->conf ) ) ); - mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, - MBEDTLS_SSL_ALERT_MSG_PROTOCOL_VERSION ); - return( MBEDTLS_ERR_SSL_BAD_HS_PROTOCOL_VERSION ); - } + mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, + MBEDTLS_SSL_ALERT_MSG_PROTOCOL_VERSION ); + return( MBEDTLS_ERR_SSL_BAD_HS_PROTOCOL_VERSION ); + } - if( ssl->major_ver > mbedtls_ssl_conf_get_max_major_ver( ssl->conf ) ) - { - ssl->major_ver = mbedtls_ssl_conf_get_max_major_ver( ssl->conf ); - ssl->minor_ver = mbedtls_ssl_conf_get_max_minor_ver( ssl->conf ); + if( major_ver > mbedtls_ssl_conf_get_max_major_ver( ssl->conf ) ) + { + major_ver = mbedtls_ssl_conf_get_max_major_ver( ssl->conf ); + minor_ver = mbedtls_ssl_conf_get_max_minor_ver( ssl->conf ); + } + else if( minor_ver > mbedtls_ssl_conf_get_max_minor_ver( ssl->conf ) ) + minor_ver = mbedtls_ssl_conf_get_max_minor_ver( ssl->conf ); + + ssl->major_ver = major_ver; + ssl->minor_ver = minor_ver; } - else if( ssl->minor_ver > mbedtls_ssl_conf_get_max_minor_ver( ssl->conf ) ) - ssl->minor_ver = mbedtls_ssl_conf_get_max_minor_ver( ssl->conf ); /* * Save client random (inc. Unix time) From 2881d8013873736e39cbeafa97892d1dcf14a57c Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Wed, 22 May 2019 14:44:53 +0100 Subject: [PATCH 03/17] Introduce getter function for max/min SSL version This is a first step towards hardcoding ssl->{major|minor}_ver in configurations which accept only a single version. --- include/mbedtls/ssl_internal.h | 10 +++++++ library/ssl_cli.c | 31 +++++++++---------- library/ssl_srv.c | 54 +++++++++++++++++++--------------- library/ssl_tls.c | 53 +++++++++++++++++---------------- 4 files changed, 84 insertions(+), 64 deletions(-) diff --git a/include/mbedtls/ssl_internal.h b/include/mbedtls/ssl_internal.h index d2299ea38..00b941dc6 100644 --- a/include/mbedtls/ssl_internal.h +++ b/include/mbedtls/ssl_internal.h @@ -960,6 +960,16 @@ int mbedtls_ssl_check_sig_hash( const mbedtls_ssl_context *ssl, mbedtls_md_type_t md ); #endif +static inline int mbedtls_ssl_get_minor_ver( mbedtls_ssl_context const *ssl ) +{ + return( ssl->minor_ver ); +} + +static inline int mbedtls_ssl_get_major_ver( mbedtls_ssl_context const *ssl ) +{ + return( ssl->major_ver ); +} + #if defined(MBEDTLS_X509_CRT_PARSE_C) static inline mbedtls_pk_context *mbedtls_ssl_own_key( mbedtls_ssl_context *ssl ) { diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 87dc25d0d..d69bd1ced 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -1333,7 +1333,7 @@ static int ssl_parse_encrypt_then_mac_ext( mbedtls_ssl_context *ssl, size_t len ) { if( ssl->conf->encrypt_then_mac == MBEDTLS_SSL_ETM_DISABLED || - ssl->minor_ver == MBEDTLS_SSL_MINOR_VERSION_0 || + mbedtls_ssl_get_minor_ver( ssl ) == MBEDTLS_SSL_MINOR_VERSION_0 || len != 0 ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "non-matching encrypt-then-MAC extension" ) ); @@ -1357,7 +1357,7 @@ static int ssl_parse_extended_ms_ext( mbedtls_ssl_context *ssl, { if( mbedtls_ssl_conf_get_ems( ssl->conf ) == MBEDTLS_SSL_EXTENDED_MS_DISABLED || - ssl->minor_ver == MBEDTLS_SSL_MINOR_VERSION_0 || + mbedtls_ssl_get_minor_ver( ssl ) == MBEDTLS_SSL_MINOR_VERSION_0 || len != 0 ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "non-matching extended master secret extension" ) ); @@ -1901,8 +1901,8 @@ static int ssl_parse_server_hello( mbedtls_ssl_context *ssl ) ciphersuite_info ) { if( ssl_validate_ciphersuite( ciphersuite_info, ssl, - mbedtls_ssl_conf_get_min_minor_ver( ssl->conf ), - mbedtls_ssl_conf_get_max_minor_ver( ssl->conf ) ) != 0 ) + mbedtls_ssl_conf_get_min_minor_ver( ssl->conf ), + mbedtls_ssl_conf_get_max_minor_ver( ssl->conf ) ) != 0 ) { continue; } @@ -1929,7 +1929,7 @@ server_picked_valid_suite: #if defined(MBEDTLS_SSL__ECP_RESTARTABLE) if( mbedtls_ssl_suite_get_key_exchange( server_suite_info ) == MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA && - ssl->minor_ver == MBEDTLS_SSL_MINOR_VERSION_3 ) + mbedtls_ssl_get_minor_ver( ssl ) == MBEDTLS_SSL_MINOR_VERSION_3 ) { ssl->handshake->ecrs_enabled = 1; } @@ -1943,14 +1943,15 @@ server_picked_valid_suite: { MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad server hello message" ) ); mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, - MBEDTLS_SSL_ALERT_MSG_ILLEGAL_PARAMETER ); + MBEDTLS_SSL_ALERT_MSG_ILLEGAL_PARAMETER ); return( MBEDTLS_ERR_SSL_BAD_HS_SERVER_HELLO ); } ssl->session_negotiate->compression = comp; ext = buf + 40 + n; - MBEDTLS_SSL_DEBUG_MSG( 2, ( "server hello, total extension length: %d", ext_len ) ); + MBEDTLS_SSL_DEBUG_MSG( 2, ( "server hello, total extension length: %d", + ext_len ) ); while( ext_len ) { @@ -1963,7 +1964,7 @@ server_picked_valid_suite: { MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad server hello message" ) ); mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, - MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR ); + MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR ); return( MBEDTLS_ERR_SSL_BAD_HS_SERVER_HELLO ); } @@ -2358,7 +2359,7 @@ static int ssl_write_encrypted_pms( mbedtls_ssl_context *ssl, size_t pms_offset ) { int ret; - size_t len_bytes = ssl->minor_ver == MBEDTLS_SSL_MINOR_VERSION_0 ? 0 : 2; + size_t len_bytes = mbedtls_ssl_get_minor_ver( ssl ) == MBEDTLS_SSL_MINOR_VERSION_0 ? 0 : 2; unsigned char *p = ssl->handshake->premaster + pms_offset; mbedtls_pk_context *peer_pk = NULL; @@ -2475,7 +2476,7 @@ static int ssl_parse_signature_algorithm( mbedtls_ssl_context *ssl, *pk_alg = MBEDTLS_PK_NONE; /* Only in TLS 1.2 */ - if( ssl->minor_ver != MBEDTLS_SSL_MINOR_VERSION_3 ) + if( mbedtls_ssl_get_minor_ver( ssl ) != MBEDTLS_SSL_MINOR_VERSION_3 ) { return( 0 ); } @@ -2802,7 +2803,7 @@ start_processing: * Handle the digitally-signed structure */ #if defined(MBEDTLS_SSL_PROTO_TLS1_2) - if( ssl->minor_ver == MBEDTLS_SSL_MINOR_VERSION_3 ) + if( mbedtls_ssl_get_minor_ver( ssl ) == MBEDTLS_SSL_MINOR_VERSION_3 ) { if( ssl_parse_signature_algorithm( ssl, &p, end, &md_alg, &pk_alg ) != 0 ) @@ -2825,7 +2826,7 @@ start_processing: #endif /* MBEDTLS_SSL_PROTO_TLS1_2 */ #if defined(MBEDTLS_SSL_PROTO_SSL3) || defined(MBEDTLS_SSL_PROTO_TLS1) || \ defined(MBEDTLS_SSL_PROTO_TLS1_1) - if( ssl->minor_ver < MBEDTLS_SSL_MINOR_VERSION_3 ) + if( mbedtls_ssl_get_minor_ver( ssl ) < MBEDTLS_SSL_MINOR_VERSION_3 ) { pk_alg = mbedtls_ssl_get_ciphersuite_sig_pk_alg( ciphersuite_info ); @@ -3097,7 +3098,7 @@ static int ssl_parse_certificate_request( mbedtls_ssl_context *ssl ) /* supported_signature_algorithms */ #if defined(MBEDTLS_SSL_PROTO_TLS1_2) - if( ssl->minor_ver == MBEDTLS_SSL_MINOR_VERSION_3 ) + 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] ) ); @@ -3590,7 +3591,7 @@ sign: #if defined(MBEDTLS_SSL_PROTO_SSL3) || defined(MBEDTLS_SSL_PROTO_TLS1) || \ defined(MBEDTLS_SSL_PROTO_TLS1_1) - if( ssl->minor_ver != MBEDTLS_SSL_MINOR_VERSION_3 ) + if( mbedtls_ssl_get_minor_ver( ssl ) != MBEDTLS_SSL_MINOR_VERSION_3 ) { /* * digitally-signed struct { @@ -3620,7 +3621,7 @@ sign: #endif /* MBEDTLS_SSL_PROTO_SSL3 || MBEDTLS_SSL_PROTO_TLS1 || \ MBEDTLS_SSL_PROTO_TLS1_1 */ #if defined(MBEDTLS_SSL_PROTO_TLS1_2) - if( ssl->minor_ver == MBEDTLS_SSL_MINOR_VERSION_3 ) + if( mbedtls_ssl_get_minor_ver( ssl ) == MBEDTLS_SSL_MINOR_VERSION_3 ) { /* * digitally-signed struct { diff --git a/library/ssl_srv.c b/library/ssl_srv.c index 38c841d89..84cb04b4a 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -536,7 +536,7 @@ static int ssl_parse_encrypt_then_mac_ext( mbedtls_ssl_context *ssl, ((void) buf); if( ssl->conf->encrypt_then_mac == MBEDTLS_SSL_ETM_ENABLED && - ssl->minor_ver != MBEDTLS_SSL_MINOR_VERSION_0 ) + mbedtls_ssl_get_minor_ver( ssl ) != MBEDTLS_SSL_MINOR_VERSION_0 ) { ssl->session_negotiate->encrypt_then_mac = MBEDTLS_SSL_ETM_ENABLED; } @@ -864,7 +864,7 @@ static int ssl_pick_cert( mbedtls_ssl_context *ssl, * present them a SHA-higher cert rather than failing if it's the only * one we got that satisfies the other conditions. */ - if( ssl->minor_ver < MBEDTLS_SSL_MINOR_VERSION_3 ) + if( mbedtls_ssl_get_minor_ver( ssl ) < MBEDTLS_SSL_MINOR_VERSION_3 ) { mbedtls_md_type_t sig_md; { @@ -930,8 +930,10 @@ static int ssl_ciphersuite_is_match( mbedtls_ssl_context *ssl, MBEDTLS_SSL_DEBUG_MSG( 3, ( "trying ciphersuite: %s", mbedtls_ssl_suite_get_name( suite_info ) ) ); - if( mbedtls_ssl_suite_get_min_minor_ver( suite_info ) > ssl->minor_ver || - mbedtls_ssl_suite_get_max_minor_ver( suite_info ) < ssl->minor_ver ) + if( mbedtls_ssl_suite_get_min_minor_ver( suite_info ) + > mbedtls_ssl_get_minor_ver( ssl ) || + mbedtls_ssl_suite_get_max_minor_ver( suite_info ) + < mbedtls_ssl_get_minor_ver( ssl ) ) { MBEDTLS_SSL_DEBUG_MSG( 3, ( "ciphersuite mismatch: version" ) ); return( 0 ); @@ -993,7 +995,7 @@ static int ssl_ciphersuite_is_match( mbedtls_ssl_context *ssl, defined(MBEDTLS_KEY_EXCHANGE__WITH_CERT__ENABLED) /* If the ciphersuite requires signing, check whether * a suitable hash algorithm is present. */ - if( ssl->minor_ver == MBEDTLS_SSL_MINOR_VERSION_3 ) + if( mbedtls_ssl_get_minor_ver( ssl ) == MBEDTLS_SSL_MINOR_VERSION_3 ) { sig_type = mbedtls_ssl_get_ciphersuite_sig_alg( suite_info ); if( sig_type != MBEDTLS_PK_NONE && @@ -1094,11 +1096,12 @@ static int ssl_parse_client_hello_v2( mbedtls_ssl_context *ssl ) ( buf[4] <= mbedtls_ssl_conf_get_max_minor_ver( ssl->conf ) ) ? buf[4] : mbedtls_ssl_conf_get_max_minor_ver( ssl->conf ); - if( ssl->minor_ver < mbedtls_ssl_conf_get_min_minor_ver( ssl->conf ) ) + if( mbedtls_ssl_get_minor_ver( ssl ) < mbedtls_ssl_conf_get_min_minor_ver( ssl->conf ) ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "client only supports ssl smaller than minimum" " [%d:%d] < [%d:%d]", - ssl->major_ver, ssl->minor_ver, + mbedtls_ssl_get_major_ver( ssl ), + mbedtls_ssl_get_minor_ver( ssl ), mbedtls_ssl_conf_get_min_major_ver( ssl->conf ), mbedtls_ssl_conf_get_min_minor_ver( ssl->conf ) ) ); @@ -1215,7 +1218,7 @@ static int ssl_parse_client_hello_v2( mbedtls_ssl_context *ssl ) { MBEDTLS_SSL_DEBUG_MSG( 3, ( "received FALLBACK_SCSV" ) ); - if( ssl->minor_ver < + if( mbedtls_ssl_get_minor_ver( ssl ) < mbedtls_ssl_conf_get_max_minor_ver( ssl->conf ) ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "inapropriate fallback" ) ); @@ -1804,7 +1807,8 @@ read_record_header: /* Do not parse the extensions if the protocol is SSLv3 */ #if defined(MBEDTLS_SSL_PROTO_SSL3) - if( ( ssl->major_ver != 3 ) || ( ssl->minor_ver != 0 ) ) + if( ( mbedtls_ssl_get_major_ver( ssl ) != 3 ) || + ( mbedtls_ssl_get_minor_ver( ssl ) != 0 ) ) { #endif /* @@ -2030,7 +2034,7 @@ read_record_header: { MBEDTLS_SSL_DEBUG_MSG( 2, ( "received FALLBACK_SCSV" ) ); - if( ssl->minor_ver < + if( mbedtls_ssl_get_minor_ver( ssl ) < mbedtls_ssl_conf_get_max_minor_ver( ssl->conf ) ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "inapropriate fallback" ) ); @@ -2243,7 +2247,7 @@ have_ciphersuite: #if defined(MBEDTLS_DEBUG_C) && \ defined(MBEDTLS_SSL_PROTO_TLS1_2) && \ defined(MBEDTLS_KEY_EXCHANGE__WITH_CERT__ENABLED) - if( ssl->minor_ver == MBEDTLS_SSL_MINOR_VERSION_3 ) + if( mbedtls_ssl_get_minor_ver( ssl ) == MBEDTLS_SSL_MINOR_VERSION_3 ) { mbedtls_pk_type_t sig_alg = mbedtls_ssl_get_ciphersuite_sig_alg( mbedtls_ssl_handshake_get_ciphersuite( ssl->handshake ) ); @@ -2351,7 +2355,7 @@ static void ssl_write_encrypt_then_mac_ext( mbedtls_ssl_context *ssl, const mbedtls_cipher_info_t *cipher = NULL; if( ssl->session_negotiate->encrypt_then_mac == MBEDTLS_SSL_ETM_DISABLED || - ssl->minor_ver == MBEDTLS_SSL_MINOR_VERSION_0 ) + mbedtls_ssl_get_minor_ver( ssl ) == MBEDTLS_SSL_MINOR_VERSION_0 ) { *olen = 0; return; @@ -2401,7 +2405,7 @@ static void ssl_write_extended_ms_ext( mbedtls_ssl_context *ssl, if( mbedtls_ssl_hs_get_extended_ms( ssl->handshake ) == MBEDTLS_SSL_EXTENDED_MS_DISABLED || - ssl->minor_ver == MBEDTLS_SSL_MINOR_VERSION_0 ) + mbedtls_ssl_get_minor_ver( ssl ) == MBEDTLS_SSL_MINOR_VERSION_0 ) { *olen = 0; return; @@ -2645,8 +2649,9 @@ static int ssl_write_hello_verify_request( mbedtls_ssl_context *ssl ) /* The RFC is not clear on this point, but sending the actual negotiated * version looks like the most interoperable thing to do. */ - mbedtls_ssl_write_version( ssl->major_ver, ssl->minor_ver, - ssl->conf->transport, p ); + mbedtls_ssl_write_version( mbedtls_ssl_get_major_ver( ssl ), + mbedtls_ssl_get_minor_ver( ssl ), + ssl->conf->transport, p ); MBEDTLS_SSL_DEBUG_BUF( 3, "server version", p, 2 ); p += 2; @@ -2738,8 +2743,9 @@ static int ssl_write_server_hello( mbedtls_ssl_context *ssl ) buf = ssl->out_msg; p = buf + 4; - mbedtls_ssl_write_version( ssl->major_ver, ssl->minor_ver, - ssl->conf->transport, p ); + mbedtls_ssl_write_version( mbedtls_ssl_get_major_ver( ssl ), + mbedtls_ssl_get_minor_ver( ssl ), + ssl->conf->transport, p ); p += 2; MBEDTLS_SSL_DEBUG_MSG( 3, ( "server hello, chosen version: [%d:%d]", @@ -2867,7 +2873,7 @@ static int ssl_write_server_hello( mbedtls_ssl_context *ssl ) /* Do not write the extensions if the protocol is SSLv3 */ #if defined(MBEDTLS_SSL_PROTO_SSL3) - if( ( ssl->major_ver != 3 ) || ( ssl->minor_ver != 0 ) ) + if( ( mbedtls_ssl_get_major_ver( ssl ) != 3 ) || ( mbedtls_ssl_get_minor_ver( ssl ) != 0 ) ) { #endif @@ -3049,7 +3055,7 @@ static int ssl_write_certificate_request( mbedtls_ssl_context *ssl ) * enum { (255) } HashAlgorithm; * enum { (255) } SignatureAlgorithm; */ - if( ssl->minor_ver == MBEDTLS_SSL_MINOR_VERSION_3 ) + if( mbedtls_ssl_get_minor_ver( ssl ) == MBEDTLS_SSL_MINOR_VERSION_3 ) { const int *cur; @@ -3407,7 +3413,7 @@ static int ssl_prepare_server_key_exchange( mbedtls_ssl_context *ssl, #if defined(MBEDTLS_SSL_PROTO_TLS1_2) mbedtls_pk_type_t sig_alg = mbedtls_ssl_get_ciphersuite_sig_pk_alg( ciphersuite_info ); - if( ssl->minor_ver == MBEDTLS_SSL_MINOR_VERSION_3 ) + if( mbedtls_ssl_get_minor_ver( ssl ) == MBEDTLS_SSL_MINOR_VERSION_3 ) { /* A: For TLS 1.2, obey signature-hash-algorithm extension * (RFC 5246, Sec. 7.4.1.4.1). */ @@ -3483,7 +3489,7 @@ static int ssl_prepare_server_key_exchange( mbedtls_ssl_context *ssl, * 2.3: Compute and add the signature */ #if defined(MBEDTLS_SSL_PROTO_TLS1_2) - if( ssl->minor_ver == MBEDTLS_SSL_MINOR_VERSION_3 ) + if( mbedtls_ssl_get_minor_ver( ssl ) == MBEDTLS_SSL_MINOR_VERSION_3 ) { /* * For TLS 1.2, we need to specify signature and hash algorithm @@ -3787,7 +3793,7 @@ static int ssl_decrypt_encrypted_pms( mbedtls_ssl_context *ssl, #if defined(MBEDTLS_SSL_PROTO_TLS1) || defined(MBEDTLS_SSL_PROTO_TLS1_1) || \ defined(MBEDTLS_SSL_PROTO_TLS1_2) #if defined(MBEDTLS_SSL_PROTO_SSL3) - if( ssl->minor_ver != MBEDTLS_SSL_MINOR_VERSION_0 ) + if( mbedtls_ssl_get_minor_ver( ssl ) != MBEDTLS_SSL_MINOR_VERSION_0 ) #endif /* MBEDTLS_SSL_PROTO_SSL3 */ { if( len < 2 ) @@ -4407,7 +4413,7 @@ static int ssl_parse_certificate_verify( mbedtls_ssl_context *ssl ) */ #if defined(MBEDTLS_SSL_PROTO_SSL3) || defined(MBEDTLS_SSL_PROTO_TLS1) || \ defined(MBEDTLS_SSL_PROTO_TLS1_1) - if( ssl->minor_ver != MBEDTLS_SSL_MINOR_VERSION_3 ) + if( mbedtls_ssl_get_minor_ver( ssl ) != MBEDTLS_SSL_MINOR_VERSION_3 ) { md_alg = MBEDTLS_MD_NONE; hashlen = 36; @@ -4424,7 +4430,7 @@ static int ssl_parse_certificate_verify( mbedtls_ssl_context *ssl ) #endif /* MBEDTLS_SSL_PROTO_SSL3 || MBEDTLS_SSL_PROTO_TLS1 || MBEDTLS_SSL_PROTO_TLS1_1 */ #if defined(MBEDTLS_SSL_PROTO_TLS1_2) - if( ssl->minor_ver == MBEDTLS_SSL_MINOR_VERSION_3 ) + if( mbedtls_ssl_get_minor_ver( ssl ) == MBEDTLS_SSL_MINOR_VERSION_3 ) { if( i + 2 > ssl->in_hslen ) { diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 0c4ba9afb..5e1d7bdb5 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -1347,7 +1347,7 @@ int mbedtls_ssl_derive_keys( mbedtls_ssl_context *ssl ) /* Set PRF, calc_verify and calc_finished function pointers */ ret = ssl_set_handshake_prfs( ssl->handshake, - ssl->minor_ver, + mbedtls_ssl_get_minor_ver( ssl ), mbedtls_ssl_suite_get_mac( ciphersuite_info ) ); if( ret != 0 ) { @@ -1393,7 +1393,7 @@ int mbedtls_ssl_derive_keys( mbedtls_ssl_context *ssl ) #endif ssl->handshake->tls_prf, ssl->handshake->randbytes, - ssl->minor_ver, + mbedtls_ssl_get_minor_ver( ssl ), mbedtls_ssl_conf_get_endpoint( ssl->conf ), ssl ); if( ret != 0 ) @@ -3736,7 +3736,7 @@ int mbedtls_ssl_write_handshake_msg( mbedtls_ssl_context *ssl ) { /* In SSLv3, the client might send a NoCertificate alert. */ #if defined(MBEDTLS_SSL_PROTO_SSL3) && defined(MBEDTLS_SSL_CLI_C) - if( ! ( ssl->minor_ver == MBEDTLS_SSL_MINOR_VERSION_0 && + if( ! ( mbedtls_ssl_get_minor_ver( ssl ) == MBEDTLS_SSL_MINOR_VERSION_0 && ssl->out_msgtype == MBEDTLS_SSL_MSG_ALERT && mbedtls_ssl_conf_get_endpoint( ssl->conf ) == MBEDTLS_SSL_IS_CLIENT ) ) @@ -3926,8 +3926,9 @@ int mbedtls_ssl_write_record( mbedtls_ssl_context *ssl, uint8_t force_flush ) /* Skip writing the record content type to after the encryption, * as it may change when using the CID extension. */ - mbedtls_ssl_write_version( ssl->major_ver, ssl->minor_ver, - ssl->conf->transport, ssl->out_hdr + 1 ); + mbedtls_ssl_write_version( mbedtls_ssl_get_major_ver( ssl ), + mbedtls_ssl_get_minor_ver( ssl ), + ssl->conf->transport, ssl->out_hdr + 1 ); memcpy( ssl->out_ctr, ssl->cur_out_ctr, 8 ); ssl->out_len[0] = (unsigned char)( len >> 8 ); @@ -3944,7 +3945,8 @@ int mbedtls_ssl_write_record( mbedtls_ssl_context *ssl, uint8_t force_flush ) rec.data_offset = ssl->out_msg - rec.buf; memcpy( &rec.ctr[0], ssl->out_ctr, 8 ); - mbedtls_ssl_write_version( ssl->major_ver, ssl->minor_ver, + mbedtls_ssl_write_version( mbedtls_ssl_get_major_ver( ssl ), + mbedtls_ssl_get_minor_ver( ssl ), ssl->conf->transport, rec.ver ); rec.type = ssl->out_msgtype; @@ -4699,7 +4701,7 @@ static int ssl_parse_record_header( mbedtls_ssl_context *ssl ) } /* Check version */ - if( major_ver != ssl->major_ver ) + if( major_ver != mbedtls_ssl_get_major_ver( ssl ) ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "major version mismatch" ) ); return( MBEDTLS_ERR_SSL_INVALID_RECORD ); @@ -4830,7 +4832,7 @@ static int ssl_parse_record_header( mbedtls_ssl_context *ssl ) } #if defined(MBEDTLS_SSL_PROTO_SSL3) - if( ssl->minor_ver == MBEDTLS_SSL_MINOR_VERSION_0 && + if( mbedtls_ssl_get_minor_ver( ssl ) == MBEDTLS_SSL_MINOR_VERSION_0 && ssl->in_msglen > ssl->transform_in->minlen + MBEDTLS_SSL_IN_CONTENT_LEN ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad message length" ) ); @@ -4842,7 +4844,7 @@ static int ssl_parse_record_header( mbedtls_ssl_context *ssl ) /* * TLS encrypted messages can have up to 256 bytes of padding */ - if( ssl->minor_ver >= MBEDTLS_SSL_MINOR_VERSION_1 && + if( mbedtls_ssl_get_minor_ver( ssl ) >= MBEDTLS_SSL_MINOR_VERSION_1 && ssl->in_msglen > ssl->transform_in->minlen + MBEDTLS_SSL_IN_CONTENT_LEN + 256 ) { @@ -4896,7 +4898,8 @@ static int ssl_prepare_record_content( mbedtls_ssl_context *ssl ) #endif /* MBEDTLS_SSL_DTLS_CONNECTION_ID */ memcpy( &rec.ctr[0], ssl->in_ctr, 8 ); - mbedtls_ssl_write_version( ssl->major_ver, ssl->minor_ver, + mbedtls_ssl_write_version( mbedtls_ssl_get_major_ver( ssl ), + mbedtls_ssl_get_minor_ver( ssl ), ssl->conf->transport, rec.ver ); rec.type = ssl->in_msgtype; if( ( ret = mbedtls_ssl_decrypt_buf( ssl, ssl->transform_in, @@ -4962,7 +4965,7 @@ static int ssl_prepare_record_content( mbedtls_ssl_context *ssl ) else if( ssl->in_msglen == 0 ) { #if defined(MBEDTLS_SSL_PROTO_TLS1_2) - if( ssl->minor_ver == MBEDTLS_SSL_MINOR_VERSION_3 + if( mbedtls_ssl_get_minor_ver( ssl ) == MBEDTLS_SSL_MINOR_VERSION_3 && ssl->in_msgtype != MBEDTLS_SSL_MSG_APPLICATION_DATA ) { /* TLS v1.2 explicitly disallows zero-length messages which are not application data */ @@ -5955,7 +5958,7 @@ int mbedtls_ssl_handle_message_type( mbedtls_ssl_context *ssl ) #endif #if defined(MBEDTLS_SSL_PROTO_SSL3) && defined(MBEDTLS_SSL_SRV_C) - if( ssl->minor_ver == MBEDTLS_SSL_MINOR_VERSION_0 && + if( mbedtls_ssl_get_minor_ver( ssl ) == MBEDTLS_SSL_MINOR_VERSION_0 && mbedtls_ssl_conf_get_endpoint( ssl->conf ) == MBEDTLS_SSL_IS_SERVER && ssl->in_msg[0] == MBEDTLS_SSL_ALERT_LEVEL_WARNING && @@ -6143,7 +6146,7 @@ int mbedtls_ssl_write_certificate( mbedtls_ssl_context *ssl ) * (otherwise an empty Certificate message will be sent). */ if( mbedtls_ssl_own_cert( ssl ) == NULL && - ssl->minor_ver == MBEDTLS_SSL_MINOR_VERSION_0 ) + mbedtls_ssl_get_minor_ver( ssl ) == MBEDTLS_SSL_MINOR_VERSION_0 ) { ssl->out_msglen = 2; ssl->out_msgtype = MBEDTLS_SSL_MSG_ALERT; @@ -6435,7 +6438,7 @@ static int ssl_srv_check_client_no_crt_notification( mbedtls_ssl_context *ssl ) /* * Check if the client sent an empty certificate */ - if( ssl->minor_ver == MBEDTLS_SSL_MINOR_VERSION_0 ) + if( mbedtls_ssl_get_minor_ver( ssl ) == MBEDTLS_SSL_MINOR_VERSION_0 ) { if( ssl->in_msglen == 2 && ssl->in_msgtype == MBEDTLS_SSL_MSG_ALERT && @@ -6987,7 +6990,7 @@ void mbedtls_ssl_optimize_checksum( mbedtls_ssl_context *ssl, #if defined(MBEDTLS_SSL_PROTO_SSL3) || defined(MBEDTLS_SSL_PROTO_TLS1) || \ defined(MBEDTLS_SSL_PROTO_TLS1_1) - if( ssl->minor_ver < MBEDTLS_SSL_MINOR_VERSION_3 ) + if( mbedtls_ssl_get_minor_ver( ssl ) < MBEDTLS_SSL_MINOR_VERSION_3 ) ssl->handshake->update_checksum = ssl_update_checksum_md5sha1; else #endif @@ -7420,7 +7423,7 @@ int mbedtls_ssl_write_finished( mbedtls_ssl_context *ssl ) * ciphersuite does this (and this is unlikely to change as activity has * moved to TLS 1.3 now) so we can keep the hardcoded 12 here. */ - hash_len = ( ssl->minor_ver == MBEDTLS_SSL_MINOR_VERSION_0 ) ? 36 : 12; + hash_len = ( mbedtls_ssl_get_minor_ver( ssl ) == MBEDTLS_SSL_MINOR_VERSION_0 ) ? 36 : 12; #if defined(MBEDTLS_SSL_RENEGOTIATION) ssl->verify_data_len = hash_len; @@ -7567,7 +7570,7 @@ int mbedtls_ssl_parse_finished( mbedtls_ssl_context *ssl ) /* There is currently no ciphersuite using another length with TLS 1.2 */ #if defined(MBEDTLS_SSL_PROTO_SSL3) - if( ssl->minor_ver == MBEDTLS_SSL_MINOR_VERSION_0 ) + if( mbedtls_ssl_get_minor_ver( ssl ) == MBEDTLS_SSL_MINOR_VERSION_0 ) hash_len = 36; else #endif @@ -7835,7 +7838,7 @@ static void ssl_update_out_pointers( mbedtls_ssl_context *ssl, /* Adjust out_msg to make space for explicit IV, if used. */ if( transform != NULL && - ssl->minor_ver >= MBEDTLS_SSL_MINOR_VERSION_2 ) + mbedtls_ssl_get_minor_ver( ssl ) >= MBEDTLS_SSL_MINOR_VERSION_2 ) { ssl->out_msg = ssl->out_iv + transform->ivlen - transform->fixed_ivlen; } @@ -9012,7 +9015,7 @@ const char *mbedtls_ssl_get_version( const mbedtls_ssl_context *ssl ) #if defined(MBEDTLS_SSL_PROTO_DTLS) if( MBEDTLS_SSL_TRANSPORT_IS_DTLS( ssl->conf->transport ) ) { - switch( ssl->minor_ver ) + switch( mbedtls_ssl_get_minor_ver( ssl ) ) { case MBEDTLS_SSL_MINOR_VERSION_2: return( "DTLSv1.0" ); @@ -9028,7 +9031,7 @@ const char *mbedtls_ssl_get_version( const mbedtls_ssl_context *ssl ) #endif /* MBEDTLS_SSL_PROTO_DTLS */ #if defined(MBEDTLS_SSL_PROTO_TLS) { - switch( ssl->minor_ver ) + switch( mbedtls_ssl_get_minor_ver( ssl ) ) { case MBEDTLS_SSL_MINOR_VERSION_0: return( "SSLv3.0" ); @@ -9090,7 +9093,7 @@ int mbedtls_ssl_get_record_expansion( const mbedtls_ssl_context *ssl ) /* For TLS 1.1 or higher, an explicit IV is added * after the record header. */ #if defined(MBEDTLS_SSL_PROTO_TLS1_1) || defined(MBEDTLS_SSL_PROTO_TLS1_2) - if( ssl->minor_ver >= MBEDTLS_SSL_MINOR_VERSION_2 ) + if( mbedtls_ssl_get_minor_ver( ssl ) >= MBEDTLS_SSL_MINOR_VERSION_2 ) transform_expansion += block_size; #endif /* MBEDTLS_SSL_PROTO_TLS1_1 || MBEDTLS_SSL_PROTO_TLS1_2 */ @@ -10214,7 +10217,7 @@ int mbedtls_ssl_read( mbedtls_ssl_context *ssl, unsigned char *buf, size_t len ) MBEDTLS_SSL_DEBUG_MSG( 3, ( "refusing renegotiation, sending alert" ) ); #if defined(MBEDTLS_SSL_PROTO_SSL3) - if( ssl->minor_ver == MBEDTLS_SSL_MINOR_VERSION_0 ) + if( mbedtls_ssl_get_minor_ver( ssl ) == MBEDTLS_SSL_MINOR_VERSION_0 ) { /* SSLv3 does not have a "no_renegotiation" warning, so we send a fatal alert and abort the connection. */ @@ -10226,7 +10229,7 @@ int mbedtls_ssl_read( mbedtls_ssl_context *ssl, unsigned char *buf, size_t len ) #endif /* MBEDTLS_SSL_PROTO_SSL3 */ #if defined(MBEDTLS_SSL_PROTO_TLS1) || defined(MBEDTLS_SSL_PROTO_TLS1_1) || \ defined(MBEDTLS_SSL_PROTO_TLS1_2) - if( ssl->minor_ver >= MBEDTLS_SSL_MINOR_VERSION_1 ) + if( mbedtls_ssl_get_minor_ver( ssl ) >= MBEDTLS_SSL_MINOR_VERSION_1 ) { if( ( ret = mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_WARNING, @@ -10435,7 +10438,7 @@ static int ssl_write_split( mbedtls_ssl_context *ssl, if( ssl->conf->cbc_record_splitting == MBEDTLS_SSL_CBC_RECORD_SPLITTING_DISABLED || len <= 1 || - ssl->minor_ver > MBEDTLS_SSL_MINOR_VERSION_1 || + mbedtls_ssl_get_minor_ver( ssl ) > MBEDTLS_SSL_MINOR_VERSION_1 || mbedtls_cipher_get_cipher_mode( &ssl->transform_out->cipher_ctx_enc ) != MBEDTLS_MODE_CBC ) { @@ -11473,7 +11476,7 @@ void mbedtls_ssl_read_version( int *major, int *minor, int transport, int mbedtls_ssl_set_calc_verify_md( mbedtls_ssl_context *ssl, int md ) { #if defined(MBEDTLS_SSL_PROTO_TLS1_2) - if( ssl->minor_ver != MBEDTLS_SSL_MINOR_VERSION_3 ) + if( mbedtls_ssl_get_minor_ver( ssl ) != MBEDTLS_SSL_MINOR_VERSION_3 ) return MBEDTLS_ERR_SSL_INVALID_VERIFY_HASH; switch( md ) From 381eaa59763d58ba4ab82acee4007b3771bd2d01 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Wed, 12 Jun 2019 14:43:01 +0100 Subject: [PATCH 04/17] Remove min/maj version from SSL context if only one version enabled If the minor/major version is enforced at compile-time, the `major_ver` and `minor_ver` fields in `mbedtls_ssl_context` are redundant and can be removed. --- include/mbedtls/ssl.h | 16 ++++++++++++++++ include/mbedtls/ssl_internal.h | 10 ++++++++++ library/ssl_cli.c | 9 +++++++++ library/ssl_srv.c | 4 ++++ 4 files changed, 39 insertions(+) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index ee8bd818b..5e9954462 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -63,6 +63,18 @@ #include "platform_time.h" #endif +#if defined(MBEDTLS_SSL_CONF_MAX_MAJOR_VER) && \ + defined(MBEDTLS_SSL_CONF_MIN_MAJOR_VER) && \ + ( MBEDTLS_SSL_CONF_MAX_MAJOR_VER == MBEDTLS_SSL_CONF_MIN_MAJOR_VER ) +#define MBEDTLS_SSL_CONF_FIXED_MAJOR_VER MBEDTLS_SSL_CONF_MIN_MAJOR_VER +#endif + +#if defined(MBEDTLS_SSL_CONF_MAX_MINOR_VER) && \ + defined(MBEDTLS_SSL_CONF_MIN_MINOR_VER) && \ + ( MBEDTLS_SSL_CONF_MAX_MINOR_VER == MBEDTLS_SSL_CONF_MIN_MINOR_VER ) +#define MBEDTLS_SSL_CONF_FIXED_MINOR_VER MBEDTLS_SSL_CONF_MIN_MINOR_VER +#endif + /* * SSL Error codes */ @@ -1229,8 +1241,12 @@ struct mbedtls_ssl_context renego_max_records is < 0 */ #endif /* MBEDTLS_SSL_RENEGOTIATION */ +#if !defined(MBEDTLS_SSL_CONF_FIXED_MAJOR_VER) int major_ver; /*!< equal to MBEDTLS_SSL_MAJOR_VERSION_3 */ +#endif /* !MBEDTLS_SSL_CONF_FIXED_MAJOR_VER */ +#if !defined(MBEDTLS_SSL_CONF_FIXED_MINOR_VER) int minor_ver; /*!< either 0 (SSL3) or 1 (TLS1.0) */ +#endif /* !MBEDTLS_SSL_CONF_FIXED_MINOR_VER */ #if defined(MBEDTLS_SSL_DTLS_BADMAC_LIMIT) unsigned badmac_seen; /*!< records with a bad MAC received */ diff --git a/include/mbedtls/ssl_internal.h b/include/mbedtls/ssl_internal.h index 00b941dc6..43443bf59 100644 --- a/include/mbedtls/ssl_internal.h +++ b/include/mbedtls/ssl_internal.h @@ -962,12 +962,22 @@ int mbedtls_ssl_check_sig_hash( const mbedtls_ssl_context *ssl, static inline int mbedtls_ssl_get_minor_ver( mbedtls_ssl_context const *ssl ) { +#if !defined(MBEDTLS_SSL_CONF_FIXED_MINOR_VER) return( ssl->minor_ver ); +#else /* !MBEDTLS_SSL_CONF_FIXED_MINOR_VER */ + ((void) ssl); + return( MBEDTLS_SSL_CONF_FIXED_MINOR_VER ); +#endif /* MBEDTLS_SSL_CONF_FIXED_MINOR_VER */ } static inline int mbedtls_ssl_get_major_ver( mbedtls_ssl_context const *ssl ) { +#if !defined(MBEDTLS_SSL_CONF_FIXED_MAJOR_VER) return( ssl->major_ver ); +#else /* !MBEDTLS_SSL_CONF_FIXED_MAJOR_VER */ + ((void) ssl); + return( MBEDTLS_SSL_CONF_FIXED_MAJOR_VER ); +#endif /* MBEDTLS_SSL_CONF_FIXED_MAJOR_VER */ } #if defined(MBEDTLS_X509_CRT_PARSE_C) diff --git a/library/ssl_cli.c b/library/ssl_cli.c index d69bd1ced..c7a18f58b 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -850,8 +850,12 @@ static int ssl_write_client_hello( mbedtls_ssl_context *ssl ) if( mbedtls_ssl_get_renego_status( ssl ) == MBEDTLS_SSL_INITIAL_HANDSHAKE ) { +#if !defined(MBEDTLS_SSL_CONF_FIXED_MAJOR_VER) ssl->major_ver = mbedtls_ssl_conf_get_min_major_ver( ssl->conf ); +#endif /* !MBEDTLS_SSL_CONF_FIXED_MAJOR_VER */ +#if !defined(MBEDTLS_SSL_CONF_FIXED_MINOR_VER) ssl->minor_ver = mbedtls_ssl_conf_get_min_minor_ver( ssl->conf ); +#endif /* !MBEDTLS_SSL_CONF_FIXED_MINOR_VER */ } if( mbedtls_ssl_conf_get_max_major_ver( ssl->conf ) == 0 ) @@ -1743,8 +1747,13 @@ static int ssl_parse_server_hello( mbedtls_ssl_context *ssl ) return( MBEDTLS_ERR_SSL_BAD_HS_PROTOCOL_VERSION ); } +#if !defined(MBEDTLS_SSL_CONF_FIXED_MINOR_VER) ssl->minor_ver = minor_ver; +#endif /* !MBEDTLS_SSL_CONF_FIXED_MINOR_VER */ + +#if !defined(MBEDTLS_SSL_CONF_FIXED_MAJOR_VER) ssl->major_ver = major_ver; +#endif /* !MBEDTLS_SSL_CONF_FIXED_MAJOR_VER */ } MBEDTLS_SSL_DEBUG_MSG( 3, ( "server hello, current time: %lu", diff --git a/library/ssl_srv.c b/library/ssl_srv.c index 84cb04b4a..87fe4c973 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -1654,8 +1654,12 @@ read_record_header: else if( minor_ver > mbedtls_ssl_conf_get_max_minor_ver( ssl->conf ) ) minor_ver = mbedtls_ssl_conf_get_max_minor_ver( ssl->conf ); +#if !defined(MBEDTLS_SSL_CONF_FIXED_MAJOR_VER) ssl->major_ver = major_ver; +#endif /* MBEDTLS_SSL_CONF_FIXED_MAJOR_VER */ +#if !defined(MBEDTLS_SSL_CONF_FIXED_MINOR_VER) ssl->minor_ver = minor_ver; +#endif /* MBEDTLS_SSL_CONF_FIXED_MINOR_VER */ } /* From 7b628e5b887b6013a6113fcbf0044983ca50b741 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Wed, 12 Jun 2019 14:45:11 +0100 Subject: [PATCH 05/17] Make mbedtls_ssl_read/write_version static inline Reasons: - If the transport type is fixed at compile-time, mbedtls_ssl_read_version() and mbedtls_ssl_write_version() are called with a compile-time determined `transport` parameter, so the transport-type branch in their body can be eliminated at compile-time. - mbedtls_ssl_read_version() is called with addresses of local variables, which so far need to be put on the stack to be addressable. Inlining the call allows to read directly into the registers holding these local variables. This saves 60 bytes w.r.t. the measurement performed by > ./scripts/baremetal.sh --rom --gcc --- include/mbedtls/ssl_internal.h | 68 +++++++++++++++++++++++++++++++--- library/ssl_tls.c | 61 ------------------------------ 2 files changed, 63 insertions(+), 66 deletions(-) diff --git a/include/mbedtls/ssl_internal.h b/include/mbedtls/ssl_internal.h index 43443bf59..5ba2f30b6 100644 --- a/include/mbedtls/ssl_internal.h +++ b/include/mbedtls/ssl_internal.h @@ -1020,11 +1020,6 @@ int mbedtls_ssl_check_cert_usage( const mbedtls_x509_crt *cert, uint32_t *flags ); #endif /* MBEDTLS_X509_CRT_PARSE_C */ -void mbedtls_ssl_write_version( int major, int minor, int transport, - unsigned char ver[2] ); -void mbedtls_ssl_read_version( int *major, int *minor, int transport, - const unsigned char ver[2] ); - static inline size_t mbedtls_ssl_in_hdr_len( const mbedtls_ssl_context *ssl ) { return( (size_t) ( ssl->in_iv - ssl->in_hdr ) ); @@ -1104,6 +1099,69 @@ int mbedtls_ssl_get_key_exchange_md_tls1_2( mbedtls_ssl_context *ssl, #endif /* MBEDTLS_SSL_PROTO_TLS1 || MBEDTLS_SSL_PROTO_TLS1_1 || \ MBEDTLS_SSL_PROTO_TLS1_2 */ +/* + * Convert version numbers to/from wire format + * and, for DTLS, to/from TLS equivalent. + * + * For TLS this is the identity. + * For DTLS, use 1's complement (v -> 255 - v, and then map as follows: + * 1.0 <-> 3.2 (DTLS 1.0 is based on TLS 1.1) + * 1.x <-> 3.x+1 for x != 0 (DTLS 1.2 based on TLS 1.2) + */ +static inline void mbedtls_ssl_write_version( int major, int minor, + int transport, + unsigned char ver[2] ) +{ +#if !defined(MBEDTLS_SSL_TRANSPORT__BOTH) + ((void) transport); +#endif + +#if defined(MBEDTLS_SSL_PROTO_DTLS) + if( MBEDTLS_SSL_TRANSPORT_IS_DTLS( transport ) ) + { + if( minor == MBEDTLS_SSL_MINOR_VERSION_2 ) + --minor; /* DTLS 1.0 stored as TLS 1.1 internally */ + + ver[0] = (unsigned char)( 255 - ( major - 2 ) ); + ver[1] = (unsigned char)( 255 - ( minor - 1 ) ); + } + MBEDTLS_SSL_TRANSPORT_ELSE +#endif +#if defined(MBEDTLS_SSL_PROTO_TLS) + { + ver[0] = (unsigned char) major; + ver[1] = (unsigned char) minor; + } +#endif +} + +static inline void mbedtls_ssl_read_version( int *major, int *minor, + int transport, + const unsigned char ver[2] ) +{ +#if !defined(MBEDTLS_SSL_TRANSPORT__BOTH) + ((void) transport); +#endif + +#if defined(MBEDTLS_SSL_PROTO_DTLS) + if( MBEDTLS_SSL_TRANSPORT_IS_DTLS( transport ) ) + { + *major = 255 - ver[0] + 2; + *minor = 255 - ver[1] + 1; + + if( *minor == MBEDTLS_SSL_MINOR_VERSION_1 ) + ++*minor; /* DTLS 1.0 stored as TLS 1.1 internally */ + } + MBEDTLS_SSL_TRANSPORT_ELSE +#endif /* MBEDTLS_SSL_PROTO_DTLS */ +#if defined(MBEDTLS_SSL_PROTO_TLS) + { + *major = ver[0]; + *minor = ver[1]; + } +#endif /* MBEDTLS_SSL_PROTO_TLS */ +} + #ifdef __cplusplus } #endif diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 5e1d7bdb5..814bb27a1 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -11412,67 +11412,6 @@ int mbedtls_ssl_check_cert_usage( const mbedtls_x509_crt *cert, } #endif /* MBEDTLS_X509_CRT_PARSE_C */ -/* - * Convert version numbers to/from wire format - * and, for DTLS, to/from TLS equivalent. - * - * For TLS this is the identity. - * For DTLS, use 1's complement (v -> 255 - v, and then map as follows: - * 1.0 <-> 3.2 (DTLS 1.0 is based on TLS 1.1) - * 1.x <-> 3.x+1 for x != 0 (DTLS 1.2 based on TLS 1.2) - */ -void mbedtls_ssl_write_version( int major, int minor, int transport, - unsigned char ver[2] ) -{ -#if !defined(MBEDTLS_SSL_TRANSPORT__BOTH) - ((void) transport); -#endif - -#if defined(MBEDTLS_SSL_PROTO_DTLS) - if( MBEDTLS_SSL_TRANSPORT_IS_DTLS( transport ) ) - { - if( minor == MBEDTLS_SSL_MINOR_VERSION_2 ) - --minor; /* DTLS 1.0 stored as TLS 1.1 internally */ - - ver[0] = (unsigned char)( 255 - ( major - 2 ) ); - ver[1] = (unsigned char)( 255 - ( minor - 1 ) ); - } - MBEDTLS_SSL_TRANSPORT_ELSE -#endif -#if defined(MBEDTLS_SSL_PROTO_TLS) - { - ver[0] = (unsigned char) major; - ver[1] = (unsigned char) minor; - } -#endif -} - -void mbedtls_ssl_read_version( int *major, int *minor, int transport, - const unsigned char ver[2] ) -{ -#if !defined(MBEDTLS_SSL_TRANSPORT__BOTH) - ((void) transport); -#endif - -#if defined(MBEDTLS_SSL_PROTO_DTLS) - if( MBEDTLS_SSL_TRANSPORT_IS_DTLS( transport ) ) - { - *major = 255 - ver[0] + 2; - *minor = 255 - ver[1] + 1; - - if( *minor == MBEDTLS_SSL_MINOR_VERSION_1 ) - ++*minor; /* DTLS 1.0 stored as TLS 1.1 internally */ - } - MBEDTLS_SSL_TRANSPORT_ELSE -#endif /* MBEDTLS_SSL_PROTO_DTLS */ -#if defined(MBEDTLS_SSL_PROTO_TLS) - { - *major = ver[0]; - *minor = ver[1]; - } -#endif /* MBEDTLS_SSL_PROTO_TLS */ -} - int mbedtls_ssl_set_calc_verify_md( mbedtls_ssl_context *ssl, int md ) { #if defined(MBEDTLS_SSL_PROTO_TLS1_2) From 18729aeaac3bf2b2626416147cfdc4d448ca86a0 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Wed, 12 Jun 2019 14:47:21 +0100 Subject: [PATCH 06/17] Guard RSA-only max_major/minor_ver fields from SSL handshake params The fields - mbedtls_ssl_handshake_params::max_major_ver, - mbedtls_ssl_handshake_params::max_minor_ver are used only for server-side RSA-based key exchanges can be removed otherwise. --- include/mbedtls/ssl_internal.h | 6 ++++++ library/ssl_srv.c | 8 ++++++++ 2 files changed, 14 insertions(+) diff --git a/include/mbedtls/ssl_internal.h b/include/mbedtls/ssl_internal.h index 5ba2f30b6..457fc28ee 100644 --- a/include/mbedtls/ssl_internal.h +++ b/include/mbedtls/ssl_internal.h @@ -514,8 +514,14 @@ struct mbedtls_ssl_handshake_params #if !defined(MBEDTLS_SSL_NO_SESSION_RESUMPTION) int resume; /*!< session resume indicator*/ #endif /* !MBEDTLS_SSL_NO_SESSION_RESUMPTION */ + +#if defined(MBEDTLS_SSL_SRV_C) && \ + ( defined(MBEDTLS_KEY_EXCHANGE_RSA_ENABLED) || \ + defined(MBEDTLS_KEY_EXCHANGE_RSA_PSK_ENABLED ) ) int max_major_ver; /*!< max. major version client*/ int max_minor_ver; /*!< max. minor version client*/ +#endif /* MBEDTLS_SSL_SRV_C && ( MBEDTLS_KEY_EXCHANGE_RSA_PSK_ENABLED || + MBEDTLS_KEY_EXCHANGE_RSA_PSK_ENABLED ) */ int cli_exts; /*!< client extension presence*/ #if defined(MBEDTLS_SSL_SESSION_TICKETS) diff --git a/library/ssl_srv.c b/library/ssl_srv.c index 87fe4c973..660b30f2e 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -1110,8 +1110,12 @@ static int ssl_parse_client_hello_v2( mbedtls_ssl_context *ssl ) return( MBEDTLS_ERR_SSL_BAD_HS_PROTOCOL_VERSION ); } +#if defined(MBEDTLS_KEY_EXCHANGE_RSA_ENABLED) || \ + defined(MBEDTLS_KEY_EXCHANGE_RSA_PSK_ENABLED) ssl->handshake->max_major_ver = buf[3]; ssl->handshake->max_minor_ver = buf[4]; +#endif /* MBEDTLS_KEY_EXCHANGE_RSA_PSK_ENABLED || + MBEDTLS_KEY_EXCHANGE_RSA_PSK_ENABLED */ if( ( ret = mbedtls_ssl_fetch_input( ssl, 2 + n ) ) != 0 ) { @@ -1630,8 +1634,12 @@ read_record_header: ssl->conf->transport, buf ); +#if defined(MBEDTLS_KEY_EXCHANGE_RSA_ENABLED) || \ + defined(MBEDTLS_KEY_EXCHANGE_RSA_PSK_ENABLED) ssl->handshake->max_major_ver = major_ver; ssl->handshake->max_minor_ver = minor_ver; +#endif /* MBEDTLS_KEY_EXCHANGE_RSA_PSK_ENABLED || + MBEDTLS_KEY_EXCHANGE_RSA_PSK_ENABLED */ if( major_ver < mbedtls_ssl_conf_get_min_major_ver( ssl->conf ) || minor_ver < mbedtls_ssl_conf_get_min_minor_ver( ssl->conf ) ) From f1c2a331895114f70dfd9b5fcf4501043ad22eaf Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Wed, 12 Jun 2019 15:27:23 +0100 Subject: [PATCH 07/17] Note in SSL doc'n that version bounds can be set at compile-time --- include/mbedtls/ssl.h | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 5e9954462..3a8a5d7a1 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -3019,6 +3019,11 @@ const char *mbedtls_ssl_get_alpn_protocol( const mbedtls_ssl_context *ssl ); * \note With DTLS, use MBEDTLS_SSL_MINOR_VERSION_2 for DTLS 1.0 and * MBEDTLS_SSL_MINOR_VERSION_3 for DTLS 1.2 * + * \note On constrained systems, the minimum major/minor version can + * also be configured at compile-time by setting + * MBEDTLS_SSL_CONF_MIN_MAJOR_VER and + * MBEDTLS_SSL_CONF_MIN_MINOR_VER. + * * \param conf SSL configuration * \param major Major version number (only MBEDTLS_SSL_MAJOR_VERSION_3 supported) * \param minor Minor version number (MBEDTLS_SSL_MINOR_VERSION_0, @@ -3039,6 +3044,11 @@ void mbedtls_ssl_conf_max_version( mbedtls_ssl_config *conf, int major, int mino * \note With DTLS, use MBEDTLS_SSL_MINOR_VERSION_2 for DTLS 1.0 and * MBEDTLS_SSL_MINOR_VERSION_3 for DTLS 1.2 * + * \note On constrained systems, the minimum major/minor version can + * also be configured at compile-time by setting + * MBEDTLS_SSL_CONF_MAX_MAJOR_VER and + * MBEDTLS_SSL_CONF_MAX_MINOR_VER. + * * \param conf SSL configuration * \param major Major version number (only MBEDTLS_SSL_MAJOR_VERSION_3 supported) * \param minor Minor version number (MBEDTLS_SSL_MINOR_VERSION_0, From 0f902b71a82f376eca8270c89b2c8f7dc73890a9 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Fri, 14 Jun 2019 09:47:45 +0100 Subject: [PATCH 08/17] Add new compile-time options to programs/ssl/query_config.c --- programs/ssl/query_config.c | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/programs/ssl/query_config.c b/programs/ssl/query_config.c index 574d4d7a5..6a1cb1bb7 100644 --- a/programs/ssl/query_config.c +++ b/programs/ssl/query_config.c @@ -2754,6 +2754,38 @@ int query_config( const char *config ) } #endif /* MBEDTLS_SSL_CONF_RNG */ +#if defined(MBEDTLS_SSL_CONF_MIN_MINOR_VER) + if( strcmp( "MBEDTLS_SSL_CONF_MIN_MINOR_VER", config ) == 0 ) + { + MACRO_EXPANSION_TO_STR( MBEDTLS_SSL_CONF_MIN_MINOR_VER ); + return( 0 ); + } +#endif /* MBEDTLS_SSL_CONF_MIN_MINOR_VER */ + +#if defined(MBEDTLS_SSL_CONF_MAX_MINOR_VER) + if( strcmp( "MBEDTLS_SSL_CONF_MAX_MINOR_VER", config ) == 0 ) + { + MACRO_EXPANSION_TO_STR( MBEDTLS_SSL_CONF_MAX_MINOR_VER ); + return( 0 ); + } +#endif /* MBEDTLS_SSL_CONF_MAX_MINOR_VER */ + +#if defined(MBEDTLS_SSL_CONF_MIN_MAJOR_VER) + if( strcmp( "MBEDTLS_SSL_CONF_MIN_MAJOR_VER", config ) == 0 ) + { + MACRO_EXPANSION_TO_STR( MBEDTLS_SSL_CONF_MIN_MAJOR_VER ); + return( 0 ); + } +#endif /* MBEDTLS_SSL_CONF_MIN_MAJOR_VER */ + +#if defined(MBEDTLS_SSL_CONF_MAX_MAJOR_VER) + if( strcmp( "MBEDTLS_SSL_CONF_MAX_MAJOR_VER", config ) == 0 ) + { + MACRO_EXPANSION_TO_STR( MBEDTLS_SSL_CONF_MAX_MAJOR_VER ); + return( 0 ); + } +#endif /* MBEDTLS_SSL_CONF_MAX_MAJOR_VER */ + #if defined(MBEDTLS_SSL_CONF_EXTENDED_MASTER_SECRET) if( strcmp( "MBEDTLS_SSL_CONF_EXTENDED_MASTER_SECRET", config ) == 0 ) { From 0a92b8156dc0b6f46e5a5d1fd9d52fb6fb848fce Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 24 Jun 2019 15:46:40 +0100 Subject: [PATCH 09/17] Remove mbedtls_ssl_transform::minor_ver if the version is hardcoded --- include/mbedtls/ssl_internal.h | 13 ++++++++++++ library/ssl_tls.c | 38 ++++++++++++++++++++++++---------- 2 files changed, 40 insertions(+), 11 deletions(-) diff --git a/include/mbedtls/ssl_internal.h b/include/mbedtls/ssl_internal.h index 457fc28ee..0c812bc56 100644 --- a/include/mbedtls/ssl_internal.h +++ b/include/mbedtls/ssl_internal.h @@ -709,7 +709,10 @@ struct mbedtls_ssl_transform mbedtls_cipher_context_t cipher_ctx_enc; /*!< encryption context */ mbedtls_cipher_context_t cipher_ctx_dec; /*!< decryption context */ + +#if !defined(MBEDTLS_SSL_CONF_FIXED_MINOR_VER) int minor_ver; +#endif #if defined(MBEDTLS_SSL_DTLS_CONNECTION_ID) uint8_t in_cid_len; @@ -727,6 +730,16 @@ struct mbedtls_ssl_transform #endif }; +static inline int mbedtls_ssl_transform_get_minor_ver( mbedtls_ssl_transform const *transform ) +{ +#if !defined(MBEDTLS_SSL_CONF_FIXED_MINOR_VER) + return( transform->minor_ver ); +#else + ((void) transform); + return( MBEDTLS_SSL_CONF_FIXED_MINOR_VER ); +#endif +} + /* * Internal representation of record frames * diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 814bb27a1..e0f5b2b4b 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -817,7 +817,12 @@ static int ssl_populate_transform( mbedtls_ssl_transform *transform, defined(MBEDTLS_SSL_SOME_MODES_USE_MAC) transform->encrypt_then_mac = encrypt_then_mac; #endif + +#if !defined(MBEDTLS_SSL_CONF_FIXED_MINOR_VER) transform->minor_ver = minor_ver; +#else + ((void) minor_ver); +#endif /* !MBEDTLS_SSL_CONF_FIXED_MINOR_VER */ /* * Get various info structures @@ -1994,7 +1999,8 @@ int mbedtls_ssl_encrypt_buf( mbedtls_ssl_context *ssl, } #if defined(MBEDTLS_SSL_PROTO_SSL3) - if( transform->minor_ver == MBEDTLS_SSL_MINOR_VERSION_0 ) + if( mbedtls_ssl_transform_get_minor_ver( transform ) == + MBEDTLS_SSL_MINOR_VERSION_0 ) { unsigned char mac[SSL_MAC_MAX_BYTES]; ssl_mac( &transform->md_ctx_enc, transform->mac_enc, @@ -2005,7 +2011,8 @@ int mbedtls_ssl_encrypt_buf( mbedtls_ssl_context *ssl, #endif #if defined(MBEDTLS_SSL_PROTO_TLS1) || defined(MBEDTLS_SSL_PROTO_TLS1_1) || \ defined(MBEDTLS_SSL_PROTO_TLS1_2) - if( transform->minor_ver >= MBEDTLS_SSL_MINOR_VERSION_1 ) + if( mbedtls_ssl_transform_get_minor_ver( transform ) >= + MBEDTLS_SSL_MINOR_VERSION_1 ) { unsigned char mac[MBEDTLS_SSL_MAC_ADD]; @@ -2184,7 +2191,8 @@ int mbedtls_ssl_encrypt_buf( mbedtls_ssl_context *ssl, * Prepend per-record IV for block cipher in TLS v1.1 and up as per * Method 1 (6.2.3.2. in RFC4346 and RFC5246) */ - if( transform->minor_ver >= MBEDTLS_SSL_MINOR_VERSION_2 ) + if( mbedtls_ssl_transform_get_minor_ver( transform ) >= + MBEDTLS_SSL_MINOR_VERSION_2 ) { if( f_rng == NULL ) { @@ -2233,7 +2241,8 @@ int mbedtls_ssl_encrypt_buf( mbedtls_ssl_context *ssl, } #if defined(MBEDTLS_SSL_PROTO_SSL3) || defined(MBEDTLS_SSL_PROTO_TLS1) - if( transform->minor_ver < MBEDTLS_SSL_MINOR_VERSION_2 ) + if( mbedtls_ssl_transform_get_minor_ver( transform ) < + MBEDTLS_SSL_MINOR_VERSION_2 ) { /* * Save IV in SSL3 and TLS1 @@ -2482,7 +2491,8 @@ int mbedtls_ssl_decrypt_buf( mbedtls_ssl_context *ssl, * Check immediate ciphertext sanity */ #if defined(MBEDTLS_SSL_PROTO_TLS1_1) || defined(MBEDTLS_SSL_PROTO_TLS1_2) - if( transform->minor_ver >= MBEDTLS_SSL_MINOR_VERSION_2 ) + if( mbedtls_ssl_transform_get_minor_ver( transform ) >= + MBEDTLS_SSL_MINOR_VERSION_2 ) { /* The ciphertext is prefixed with the CBC IV. */ minlen += transform->ivlen; @@ -2573,7 +2583,8 @@ int mbedtls_ssl_decrypt_buf( mbedtls_ssl_context *ssl, /* * Initialize for prepended IV for block cipher in TLS v1.1 and up */ - if( transform->minor_ver >= MBEDTLS_SSL_MINOR_VERSION_2 ) + if( mbedtls_ssl_transform_get_minor_ver( transform ) >= + MBEDTLS_SSL_MINOR_VERSION_2 ) { /* This is safe because data_len >= minlen + maclen + 1 initially, * and at this point we have at most subtracted maclen (note that @@ -2601,7 +2612,8 @@ int mbedtls_ssl_decrypt_buf( mbedtls_ssl_context *ssl, } #if defined(MBEDTLS_SSL_PROTO_SSL3) || defined(MBEDTLS_SSL_PROTO_TLS1) - if( transform->minor_ver < MBEDTLS_SSL_MINOR_VERSION_2 ) + if( mbedtls_ssl_transform_get_minor_ver( transform ) < + MBEDTLS_SSL_MINOR_VERSION_2 ) { /* * Save IV in SSL3 and TLS1 @@ -2643,7 +2655,8 @@ int mbedtls_ssl_decrypt_buf( mbedtls_ssl_context *ssl, * we have data_len >= padlen here. */ #if defined(MBEDTLS_SSL_PROTO_SSL3) - if( transform->minor_ver == MBEDTLS_SSL_MINOR_VERSION_0 ) + if( mbedtls_ssl_transform_get_minor_ver( transform ) == + MBEDTLS_SSL_MINOR_VERSION_0 ) { if( padlen > transform->ivlen ) { @@ -2659,7 +2672,8 @@ int mbedtls_ssl_decrypt_buf( mbedtls_ssl_context *ssl, #endif /* MBEDTLS_SSL_PROTO_SSL3 */ #if defined(MBEDTLS_SSL_PROTO_TLS1) || defined(MBEDTLS_SSL_PROTO_TLS1_1) || \ defined(MBEDTLS_SSL_PROTO_TLS1_2) - if( transform->minor_ver > MBEDTLS_SSL_MINOR_VERSION_0 ) + if( mbedtls_ssl_transform_get_minor_ver( transform ) > + MBEDTLS_SSL_MINOR_VERSION_0 ) { /* The padding check involves a series of up to 256 * consecutive memory reads at the end of the record @@ -2745,7 +2759,8 @@ int mbedtls_ssl_decrypt_buf( mbedtls_ssl_context *ssl, ssl_extract_add_data_from_record( add_data, &add_data_len, rec ); #if defined(MBEDTLS_SSL_PROTO_SSL3) - if( transform->minor_ver == MBEDTLS_SSL_MINOR_VERSION_0 ) + if( mbedtls_ssl_transform_get_minor_ver( transform ) == + MBEDTLS_SSL_MINOR_VERSION_0 ) { ssl_mac( &transform->md_ctx_dec, transform->mac_dec, @@ -2757,7 +2772,8 @@ int mbedtls_ssl_decrypt_buf( mbedtls_ssl_context *ssl, #endif /* MBEDTLS_SSL_PROTO_SSL3 */ #if defined(MBEDTLS_SSL_PROTO_TLS1) || defined(MBEDTLS_SSL_PROTO_TLS1_1) || \ defined(MBEDTLS_SSL_PROTO_TLS1_2) - if( transform->minor_ver > MBEDTLS_SSL_MINOR_VERSION_0 ) + if( mbedtls_ssl_transform_get_minor_ver( transform ) > + MBEDTLS_SSL_MINOR_VERSION_0 ) { /* * Process MAC and always update for padlen afterwards to make From 94c40d17f709122eeaabeb5d1f2bfc155267970a Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Fri, 5 Jul 2019 11:15:37 +0100 Subject: [PATCH 10/17] [Fixup] Fix typos in documentation of min/max version macros --- include/mbedtls/ssl.h | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 3a8a5d7a1..d47102c68 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -3019,10 +3019,10 @@ const char *mbedtls_ssl_get_alpn_protocol( const mbedtls_ssl_context *ssl ); * \note With DTLS, use MBEDTLS_SSL_MINOR_VERSION_2 for DTLS 1.0 and * MBEDTLS_SSL_MINOR_VERSION_3 for DTLS 1.2 * - * \note On constrained systems, the minimum major/minor version can + * \note On constrained systems, the maximum major/minor version can * also be configured at compile-time by setting - * MBEDTLS_SSL_CONF_MIN_MAJOR_VER and - * MBEDTLS_SSL_CONF_MIN_MINOR_VER. + * MBEDTLS_SSL_CONF_MAX_MAJOR_VER and + * MBEDTLS_SSL_CONF_MAX_MINOR_VER. * * \param conf SSL configuration * \param major Major version number (only MBEDTLS_SSL_MAJOR_VERSION_3 supported) @@ -3046,8 +3046,8 @@ void mbedtls_ssl_conf_max_version( mbedtls_ssl_config *conf, int major, int mino * * \note On constrained systems, the minimum major/minor version can * also be configured at compile-time by setting - * MBEDTLS_SSL_CONF_MAX_MAJOR_VER and - * MBEDTLS_SSL_CONF_MAX_MINOR_VER. + * MBEDTLS_SSL_CONF_MIN_MAJOR_VER and + * MBEDTLS_SSL_CONF_MIN_MINOR_VER. * * \param conf SSL configuration * \param major Major version number (only MBEDTLS_SSL_MAJOR_VERSION_3 supported) From cb8774b6e88b1f4a9cbb091bc08dfb0eb0b14630 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Fri, 5 Jul 2019 11:20:49 +0100 Subject: [PATCH 11/17] Enforce that all SSL version bounds must be hardcoded simultaneously --- include/mbedtls/check_config.h | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/include/mbedtls/check_config.h b/include/mbedtls/check_config.h index 735a1e419..82e4dad59 100644 --- a/include/mbedtls/check_config.h +++ b/include/mbedtls/check_config.h @@ -614,6 +614,18 @@ #error "MBEDTLS_SSL_DTLS_ANTI_REPLAY defined, but not all prerequisites" #endif +#if defined(MBEDTLS_SSL_CONF_MIN_MINOR_VER) || \ + defined(MBEDTLS_SSL_CONF_MAX_MINOR_VER) || \ + defined(MBEDTLS_SSL_CONF_MIN_MAJOR_VER) || \ + defined(MBEDTLS_SSL_CONF_MAX_MAJOR_VER) +#if !( defined(MBEDTLS_SSL_CONF_MIN_MINOR_VER) && \ + defined(MBEDTLS_SSL_CONF_MAX_MINOR_VER) && \ + defined(MBEDTLS_SSL_CONF_MIN_MAJOR_VER) && \ + defined(MBEDTLS_SSL_CONF_MAX_MAJOR_VER) ) +#error "MBEDTLS_SSL_CONF_MIN_MINOR_VER, MBEDTLS_SSL_CONF_MAX_MINOR_VER, MBEDTLS_SSL_CONF_MIN_MAJOR_VER, MBEDTLS_SSL_CONF_MAX_MAJOR_VER must be defined simultaneously" +#endif +#endif + #if defined(MBEDTLS_SSL_DTLS_CONNECTION_ID) && \ ( !defined(MBEDTLS_SSL_TLS_C) || !defined(MBEDTLS_SSL_PROTO_DTLS) ) #error "MBEDTLS_SSL_DTLS_CONNECTION_ID defined, but not all prerequisites" From 33b9b25a480620ad069e0fd7cda7f5b3cd74170f Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Fri, 5 Jul 2019 11:23:25 +0100 Subject: [PATCH 12/17] Remove SSL version configuration API if versions are hardcoded --- include/mbedtls/ssl.h | 8 ++++++++ library/ssl_tls.c | 42 ++++++++++++------------------------------ 2 files changed, 20 insertions(+), 30 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index d47102c68..e00f7b9e8 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -3009,6 +3009,8 @@ int mbedtls_ssl_conf_alpn_protocols( mbedtls_ssl_config *conf, const char **prot const char *mbedtls_ssl_get_alpn_protocol( const mbedtls_ssl_context *ssl ); #endif /* MBEDTLS_SSL_ALPN */ +#if !defined(MBEDTLS_SSL_CONF_MAX_MINOR_VER) || \ + !defined(MBEDTLS_SSL_CONF_MAX_MAJOR_VER) /** * \brief Set the maximum supported version sent from the client side * and/or accepted at the server side @@ -3031,7 +3033,11 @@ const char *mbedtls_ssl_get_alpn_protocol( const mbedtls_ssl_context *ssl ); * MBEDTLS_SSL_MINOR_VERSION_3 supported) */ void mbedtls_ssl_conf_max_version( mbedtls_ssl_config *conf, int major, int minor ); +#endif /* MBEDTLS_SSL_CONF_MAX_MINOR_VER || + MBEDTLS_SSL_CONF_MAX_MAJOR_VER */ +#if !defined(MBEDTLS_SSL_CONF_MIN_MINOR_VER) || \ + !defined(MBEDTLS_SSL_CONF_MIN_MAJOR_VER) /** * \brief Set the minimum accepted SSL/TLS protocol version * (Default: TLS 1.0) @@ -3056,6 +3062,8 @@ void mbedtls_ssl_conf_max_version( mbedtls_ssl_config *conf, int major, int mino * MBEDTLS_SSL_MINOR_VERSION_3 supported) */ void mbedtls_ssl_conf_min_version( mbedtls_ssl_config *conf, int major, int minor ); +#endif /* MBEDTLS_SSL_CONF_MIN_MINOR_VER || + MBEDTLS_SSL_CONF_MIN_MAJOR_VER */ #if defined(MBEDTLS_SSL_FALLBACK_SCSV) && defined(MBEDTLS_SSL_CLI_C) /** diff --git a/library/ssl_tls.c b/library/ssl_tls.c index e0f5b2b4b..8fb548c46 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -8734,45 +8734,27 @@ const char *mbedtls_ssl_get_alpn_protocol( const mbedtls_ssl_context *ssl ) } #endif /* MBEDTLS_SSL_ALPN */ -void mbedtls_ssl_conf_max_version( mbedtls_ssl_config *conf, int major, int minor ) +#if !defined(MBEDTLS_SSL_CONF_MAX_MINOR_VER) || \ + !defined(MBEDTLS_SSL_CONF_MAX_MAJOR_VER) +void mbedtls_ssl_conf_max_version( mbedtls_ssl_config *conf, + int major, int minor ) { -#if defined(MBEDTLS_SSL_CONF_MAX_MAJOR_VER) && \ - defined(MBEDTLS_SSL_CONF_MAX_MINOR_VER) - ((void) conf); -#endif - -#if !defined(MBEDTLS_SSL_CONF_MAX_MAJOR_VER) conf->max_major_ver = major; -#else - ((void) major); -#endif /* MBEDTLS_SSL_CONF_MAX_MAJOR_VER */ - -#if !defined(MBEDTLS_SSL_CONF_MAX_MINOR_VER) conf->max_minor_ver = minor; -#else - ((void) minor); -#endif /* !MBEDTLS_SSL_CONF_MAX_MINOR_VER */ } +#endif /* MBEDTLS_SSL_CONF_MAX_MINOR_VER || + MBEDTLS_SSL_CONF_MAX_MAJOR_VER */ -void mbedtls_ssl_conf_min_version( mbedtls_ssl_config *conf, int major, int minor ) +#if !defined(MBEDTLS_SSL_CONF_MIN_MINOR_VER) || \ + !defined(MBEDTLS_SSL_CONF_MIN_MAJOR_VER) +void mbedtls_ssl_conf_min_version( mbedtls_ssl_config *conf, + int major, int minor ) { -#if defined(MBEDTLS_SSL_CONF_MIN_MAJOR_VER) && \ - defined(MBEDTLS_SSL_CONF_MIN_MINOR_VER) - ((void) conf); -#endif - -#if !defined(MBEDTLS_SSL_CONF_MIN_MAJOR_VER) conf->min_major_ver = major; -#else - ((void) major); -#endif /* MBEDTLS_SSL_CONF_MIN_MAJOR_VER */ - -#if !defined(MBEDTLS_SSL_CONF_MIN_MINOR_VER) conf->min_minor_ver = minor; -#else - ((void) minor); -#endif /* !MBEDTLS_SSL_CONF_MIN_MINOR_VER */ } +#endif /* MBEDTLS_SSL_CONF_MIN_MINOR_VER || + MBEDTLS_SSL_CONF_MIN_MAJOR_VER */ #if defined(MBEDTLS_SSL_FALLBACK_SCSV) && defined(MBEDTLS_SSL_CLI_C) void mbedtls_ssl_conf_fallback( mbedtls_ssl_config *conf, char fallback ) From 72e5ffc9d64763fa001c38e5eee624f33996bf15 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Fri, 5 Jul 2019 11:35:08 +0100 Subject: [PATCH 13/17] Remove ver cfg in ssl_client2/ssl_server2 if ver hardcoded --- programs/ssl/ssl_client2.c | 67 ++++++++++++++++++++++------------ programs/ssl/ssl_server2.c | 73 ++++++++++++++++++++++++++------------ 2 files changed, 96 insertions(+), 44 deletions(-) diff --git a/programs/ssl/ssl_client2.c b/programs/ssl/ssl_client2.c index 205f27aae..268be15c8 100644 --- a/programs/ssl/ssl_client2.c +++ b/programs/ssl/ssl_client2.c @@ -315,6 +315,20 @@ int main( void ) #define USAGE_READ_TIMEOUT "" #endif +#if !defined(MBEDTLS_SSL_CONF_MIN_MINOR_VER) || \ + !defined(MBEDTLS_SSL_CONF_MIN_MAJOR_VER) || \ + !defined(MBEDTLS_SSL_CONF_MAX_MINOR_VER) || \ + !defined(MBEDTLS_SSL_CONF_MAX_MAJOR_VER) +#define USAGE_MAX_VERSION " max_version=%%s default: (library default: tls1_2)\n" +#define USAGE_MIN_VERSION " min_version=%%s default: (library default: tls1)\n" +#define USAGE_FORCE_VERSION " force_version=%%s default: \"\" (none)\n" \ + " options: ssl3, tls1, tls1_1, tls1_2, dtls1, dtls1_2\n" +#else +#define USAGE_MAX_VERSION "" +#define USAGE_MIN_VERSION "" +#define USAGE_FORCE_VERSION "" +#endif + #define USAGE \ "\n usage: ssl_client2 param=<>...\n" \ "\n acceptable parameters:\n" \ @@ -368,10 +382,9 @@ int main( void ) "\n" \ " arc4=%%d default: (library default: 0)\n" \ " allow_sha1=%%d default: 0\n" \ - " min_version=%%s default: (library default: tls1)\n" \ - " max_version=%%s default: (library default: tls1_2)\n" \ - " force_version=%%s default: \"\" (none)\n" \ - " options: ssl3, tls1, tls1_1, tls1_2, dtls1, dtls1_2\n" \ + USAGE_MIN_VERSION \ + USAGE_MAX_VERSION \ + USAGE_FORCE_VERSION \ "\n" \ " force_ciphersuite= default: all enabled\n"\ " query_config= return 0 if the specified\n" \ @@ -1113,6 +1126,10 @@ int main( int argc, char *argv[] ) default: goto usage; } } +#if !defined(MBEDTLS_SSL_CONF_MIN_MINOR_VER) || \ + !defined(MBEDTLS_SSL_CONF_MIN_MAJOR_VER) || \ + !defined(MBEDTLS_SSL_CONF_MAX_MINOR_VER) || \ + !defined(MBEDTLS_SSL_CONF_MAX_MAJOR_VER) else if( strcmp( p, "min_version" ) == 0 ) { if( strcmp( q, "ssl3" ) == 0 ) @@ -1143,24 +1160,6 @@ int main( int argc, char *argv[] ) else goto usage; } - else if( strcmp( p, "arc4" ) == 0 ) - { - switch( atoi( q ) ) - { - case 0: opt.arc4 = MBEDTLS_SSL_ARC4_DISABLED; break; - case 1: opt.arc4 = MBEDTLS_SSL_ARC4_ENABLED; break; - default: goto usage; - } - } - else if( strcmp( p, "allow_sha1" ) == 0 ) - { - switch( atoi( q ) ) - { - case 0: opt.allow_sha1 = 0; break; - case 1: opt.allow_sha1 = 1; break; - default: goto usage; - } - } else if( strcmp( p, "force_version" ) == 0 ) { if( strcmp( q, "ssl3" ) == 0 ) @@ -1198,6 +1197,25 @@ int main( int argc, char *argv[] ) else goto usage; } +#endif + else if( strcmp( p, "arc4" ) == 0 ) + { + switch( atoi( q ) ) + { + case 0: opt.arc4 = MBEDTLS_SSL_ARC4_DISABLED; break; + case 1: opt.arc4 = MBEDTLS_SSL_ARC4_ENABLED; break; + default: goto usage; + } + } + else if( strcmp( p, "allow_sha1" ) == 0 ) + { + switch( atoi( q ) ) + { + case 0: opt.allow_sha1 = 0; break; + case 1: opt.allow_sha1 = 1; break; + default: goto usage; + } + } #if !defined(MBEDTLS_SSL_CONF_AUTHMODE) else if( strcmp( p, "auth_mode" ) == 0 ) { @@ -1847,6 +1865,10 @@ int main( int argc, char *argv[] ) } #endif +#if !defined(MBEDTLS_SSL_CONF_MIN_MINOR_VER) || \ + !defined(MBEDTLS_SSL_CONF_MIN_MAJOR_VER) || \ + !defined(MBEDTLS_SSL_CONF_MAX_MINOR_VER) || \ + !defined(MBEDTLS_SSL_CONF_MAX_MAJOR_VER) if( opt.min_version != DFL_MIN_VERSION ) mbedtls_ssl_conf_min_version( &conf, MBEDTLS_SSL_MAJOR_VERSION_3, opt.min_version ); @@ -1854,6 +1876,7 @@ int main( int argc, char *argv[] ) if( opt.max_version != DFL_MAX_VERSION ) mbedtls_ssl_conf_max_version( &conf, MBEDTLS_SSL_MAJOR_VERSION_3, opt.max_version ); +#endif #if defined(MBEDTLS_SSL_FALLBACK_SCSV) if( opt.fallback != DFL_FALLBACK ) diff --git a/programs/ssl/ssl_server2.c b/programs/ssl/ssl_server2.c index 160997ad7..ab9b9500d 100644 --- a/programs/ssl/ssl_server2.c +++ b/programs/ssl/ssl_server2.c @@ -431,6 +431,20 @@ int main( void ) #define USAGE_CERT_REQ_CA_LIST "" #endif +#if !defined(MBEDTLS_SSL_CONF_MIN_MINOR_VER) || \ + !defined(MBEDTLS_SSL_CONF_MIN_MAJOR_VER) || \ + !defined(MBEDTLS_SSL_CONF_MAX_MINOR_VER) || \ + !defined(MBEDTLS_SSL_CONF_MAX_MAJOR_VER) +#define USAGE_MAX_VERSION " max_version=%%s default: (library default: tls1_2)\n" +#define USAGE_MIN_VERSION " min_version=%%s default: (library default: tls1)\n" +#define USAGE_FORCE_VERSION " force_version=%%s default: \"\" (none)\n" \ + " options: ssl3, tls1, tls1_1, tls1_2, dtls1, dtls1_2\n" +#else +#define USAGE_MAX_VERSION "" +#define USAGE_MIN_VERSION "" +#define USAGE_FORCE_VERSION "" +#endif + #define USAGE \ "\n usage: ssl_server2 param=<>...\n" \ "\n acceptable parameters:\n" \ @@ -477,10 +491,9 @@ int main( void ) "\n" \ " arc4=%%d default: (library default: 0)\n" \ " allow_sha1=%%d default: 0\n" \ - " min_version=%%s default: (library default: tls1)\n" \ - " max_version=%%s default: (library default: tls1_2)\n" \ - " force_version=%%s default: \"\" (none)\n" \ - " options: ssl3, tls1, tls1_1, tls1_2, dtls1, dtls1_2\n" \ + USAGE_MIN_VERSION \ + USAGE_MAX_VERSION \ + USAGE_FORCE_VERSION \ "\n" \ " version_suites=a,b,c,d per-version ciphersuites\n" \ " in order from ssl3 to tls1_2\n" \ @@ -1749,6 +1762,10 @@ int main( int argc, char *argv[] ) if( opt.exchanges < 0 ) goto usage; } +#if !defined(MBEDTLS_SSL_CONF_MIN_MINOR_VER) || \ + !defined(MBEDTLS_SSL_CONF_MIN_MAJOR_VER) || \ + !defined(MBEDTLS_SSL_CONF_MAX_MINOR_VER) || \ + !defined(MBEDTLS_SSL_CONF_MAX_MAJOR_VER) else if( strcmp( p, "min_version" ) == 0 ) { if( strcmp( q, "ssl3" ) == 0 ) @@ -1779,24 +1796,6 @@ int main( int argc, char *argv[] ) else goto usage; } - else if( strcmp( p, "arc4" ) == 0 ) - { - switch( atoi( q ) ) - { - case 0: opt.arc4 = MBEDTLS_SSL_ARC4_DISABLED; break; - case 1: opt.arc4 = MBEDTLS_SSL_ARC4_ENABLED; break; - default: goto usage; - } - } - else if( strcmp( p, "allow_sha1" ) == 0 ) - { - switch( atoi( q ) ) - { - case 0: opt.allow_sha1 = 0; break; - case 1: opt.allow_sha1 = 1; break; - default: goto usage; - } - } else if( strcmp( p, "force_version" ) == 0 ) { if( strcmp( q, "ssl3" ) == 0 ) @@ -1834,6 +1833,31 @@ int main( int argc, char *argv[] ) else goto usage; } +#endif + else if( strcmp( p, "arc4" ) == 0 ) + { + switch( atoi( q ) ) + { + case 0: opt.arc4 = MBEDTLS_SSL_ARC4_DISABLED; break; + case 1: opt.arc4 = MBEDTLS_SSL_ARC4_ENABLED; break; + default: goto usage; + } + } + else if( strcmp( p, "allow_sha1" ) == 0 ) + { + switch( atoi( q ) ) + { + case 0: opt.allow_sha1 = 0; break; + case 1: opt.allow_sha1 = 1; break; + default: goto usage; + } + } +#if !defined(MBEDTLS_SSL_CONF_MIN_MINOR_VER) || \ + !defined(MBEDTLS_SSL_CONF_MIN_MAJOR_VER) || \ + !defined(MBEDTLS_SSL_CONF_MAX_MINOR_VER) || \ + !defined(MBEDTLS_SSL_CONF_MAX_MAJOR_VER) + +#endif #if !defined(MBEDTLS_SSL_CONF_AUTHMODE) else if( strcmp( p, "auth_mode" ) == 0 ) { @@ -2863,11 +2887,16 @@ int main( int argc, char *argv[] ) } #endif +#if !defined(MBEDTLS_SSL_CONF_MIN_MINOR_VER) || \ + !defined(MBEDTLS_SSL_CONF_MIN_MAJOR_VER) || \ + !defined(MBEDTLS_SSL_CONF_MAX_MINOR_VER) || \ + !defined(MBEDTLS_SSL_CONF_MAX_MAJOR_VER) if( opt.min_version != DFL_MIN_VERSION ) mbedtls_ssl_conf_min_version( &conf, MBEDTLS_SSL_MAJOR_VERSION_3, opt.min_version ); if( opt.max_version != DFL_MIN_VERSION ) mbedtls_ssl_conf_max_version( &conf, MBEDTLS_SSL_MAJOR_VERSION_3, opt.max_version ); +#endif if( ( ret = mbedtls_ssl_setup( &ssl, &conf ) ) != 0 ) { From d82a03084fa83e1e27531103bfb7ea13c97ce47d Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Fri, 5 Jul 2019 11:40:52 +0100 Subject: [PATCH 14/17] ssl-opt.sh: Detect mismatching cmd line and hardcoded version config --- tests/ssl-opt.sh | 83 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 83 insertions(+) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 553ece426..acbf4143e 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -563,6 +563,84 @@ check_cmdline_legacy_renego_compat() { fi } +check_cmdline_min_minor_version_compat() { + __VAL="$( get_config_value_or_default "MBEDTLS_SSL_CONF_MIN_MINOR_VER" )" + if [ ! -z "$__VAL" ]; then + extract_cmdline_argument "min_version" + if [ "$__ARG" = "ssl3" ] && [ "$__VAL" != "0" ]; then + SKIP_NEXT="YES"; + elif [ "$__ARG" = "tls1" ] && [ "$__VAL" != "1" ]; then + SKIP_NEXT="YES" + elif [ "$__ARG" = "tls1_1" ] && [ "$__VAL" != "2" ]; then + SKIP_NEXT="YES" + elif [ "$__ARG" = "tls1_2" ] && [ "$__VAL" != "3" ]; then + SKIP_NEXT="YES" + fi + fi +} + +check_cmdline_max_minor_version_compat() { + __VAL="$( get_config_value_or_default "MBEDTLS_SSL_CONF_MAX_MINOR_VER" )" + if [ ! -z "$__VAL" ]; then + extract_cmdline_argument "max_version" + if [ "$__ARG" = "ssl3" ] && [ "$__VAL" != "0" ]; then + SKIP_NEXT="YES"; + elif [ "$__ARG" = "tls1" ] && [ "$__VAL" != "1" ]; then + SKIP_NEXT="YES" + elif [ "$__ARG" = "tls1_1" ] && [ "$__VAL" != "2" ]; then + SKIP_NEXT="YES" + elif [ "$__ARG" = "tls1_2" ] && [ "$__VAL" != "3" ]; then + SKIP_NEXT="YES" + fi + fi +} + +check_cmdline_force_version_compat() { + __VAL_MAX="$( get_config_value_or_default "MBEDTLS_SSL_CONF_MAX_MINOR_VER" )" + __VAL_MIN="$( get_config_value_or_default "MBEDTLS_SSL_CONF_MIN_MINOR_VER" )" + if [ ! -z "$__VAL_MIN" ]; then + + # SSL cli/srv cmd line + + extract_cmdline_argument "force_version" + if [ "$__ARG" = "ssl3" ] && \ + ( [ "$__VAL_MIN" != "0" ] || [ "$__VAL_MAX" != "0" ] ); then + SKIP_NEXT="YES"; + elif [ "$__ARG" = "tls1" ] && \ + ( [ "$__VAL_MIN" != "1" ] || [ "$__VAL_MAX" != "1" ] ); then + SKIP_NEXT="YES" + elif ( [ "$__ARG" = "tls1_1" ] || [ "$__ARG" = "dtls1" ] ) && \ + ( [ "$__VAL_MIN" != "2" ] || [ "$__VAL_MAX" != "2" ] ); then + SKIP_NEXT="YES" + elif ( [ "$__ARG" = "tls1_2" ] || [ "$__ARG" = "dtls1_2" ] ) && \ + ( [ "$__VAL_MIN" != "3" ] || [ "$__VAL_MAX" != "3" ] ); then + echo "FORCE SKIP" + SKIP_NEXT="YES" + fi + + # OpenSSL cmd line + + if echo "$CMD" | grep -e "-tls1\($\|[^_]\)" > /dev/null; then + if [ "$__VAL_MIN" != "1" ] || [ "$__VAL_MAX" != "1" ]; then + SKIP_NEXT="YES" + fi + fi + + if echo "$CMD" | grep -e "-\(dtls1\($\|[^_]\)\|tls1_1\)" > /dev/null; then + if [ "$__VAL_MIN" != "2" ] || [ "$__VAL_MAX" != "2" ]; then + SKIP_NEXT="YES" + fi + fi + + if echo "$CMD" | grep -e "-\(dtls1_2\($\|[^_]\)\|tls1_2\)" > /dev/null; then + if [ "$__VAL_MIN" != "3" ] || [ "$__VAL_MAX" != "3" ]; then + SKIP_NEXT="YES" + fi + fi + + fi +} + # Go through all options that can be hardcoded at compile-time and # detect whether the command line configures them in a conflicting # way. If so, skip the test. Otherwise, remove the corresponding @@ -592,6 +670,11 @@ check_cmdline_compat() { # Legacy renegotiation check_cmdline_legacy_renego_compat + + # Version configuration + check_cmdline_min_minor_version_compat + check_cmdline_max_minor_version_compat + check_cmdline_force_version_compat } # Usage: run_test name [-p proxy_cmd] srv_cmd cli_cmd cli_exit [option [...]] From 930fbf60d62bd77fce8d12c760eed93f54b6ae31 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Fri, 5 Jul 2019 13:31:30 +0100 Subject: [PATCH 15/17] Add TEST_ASSUME macro to allow skipping tests at runtime This commit adds a macro TEST_ASSUME to the test infrastructure which allows to skip tests based on unmet conditions determined at runtime. --- tests/suites/helpers.function | 36 ++++++++++++++++++++++++++++++--- tests/suites/host_test.function | 12 ++++++++--- tests/suites/main_test.function | 2 +- 3 files changed, 43 insertions(+), 7 deletions(-) diff --git a/tests/suites/helpers.function b/tests/suites/helpers.function index a33cc0345..ccd4d4255 100644 --- a/tests/suites/helpers.function +++ b/tests/suites/helpers.function @@ -108,6 +108,21 @@ typedef enum } \ } while( 0 ) +/** + * \brief This macro tests the expression passed to it and skips the + * running test if it doesn't evaluate to 'true'. + * + * \param TEST The test expression to be tested. + */ +#define TEST_ASSUME( TEST ) \ + do { \ + if( ! (TEST) ) \ + { \ + test_skip( #TEST, __LINE__, __FILE__ ); \ + goto exit; \ + } \ + } while( 0 ) + #if defined(MBEDTLS_CHECK_PARAMS) && !defined(MBEDTLS_PARAM_FAILED_ALT) /** * \brief This macro tests the statement passed to it as a test step or @@ -241,10 +256,17 @@ typedef enum /*----------------------------------------------------------------------------*/ /* Global variables */ +typedef enum +{ + TEST_RESULT_SUCCESS = 0, + TEST_RESULT_FAILED, + TEST_RESULT_SKIPPED +} test_result_t; + static struct { paramfail_test_state_t paramfail_test_state; - int failed; + test_result_t result; const char *test; const char *filename; int line_no; @@ -280,7 +302,15 @@ jmp_buf jmp_tmp; void test_fail( const char *test, int line_no, const char* filename ) { - test_info.failed = 1; + test_info.result = TEST_RESULT_FAILED; + test_info.test = test; + test_info.line_no = line_no; + test_info.filename = filename; +} + +void test_skip( const char *test, int line_no, const char* filename ) +{ + test_info.result = TEST_RESULT_SKIPPED; test_info.test = test; test_info.line_no = line_no; test_info.filename = filename; @@ -319,7 +349,7 @@ void mbedtls_param_failed( const char *failure_condition, /* Record the location of the failure, but not as a failure yet, in case * it was part of the test */ test_fail( failure_condition, line, file ); - test_info.failed = 0; + test_info.result = TEST_RESULT_SUCCESS; longjmp( param_fail_jmp, 1 ); } diff --git a/tests/suites/host_test.function b/tests/suites/host_test.function index fe6a2bc07..0f98d23aa 100644 --- a/tests/suites/host_test.function +++ b/tests/suites/host_test.function @@ -498,7 +498,8 @@ int execute_tests( int argc , const char ** argv ) if( ( ret = get_line( file, buf, sizeof(buf) ) ) != 0 ) break; - mbedtls_fprintf( stdout, "%s%.66s", test_info.failed ? "\n" : "", buf ); + mbedtls_fprintf( stdout, "%s%.66s", + test_info.result == TEST_RESULT_FAILED ? "\n" : "", buf ); mbedtls_fprintf( stdout, " " ); for( i = strlen( buf ) + 1; i < 67; i++ ) mbedtls_fprintf( stdout, "." ); @@ -545,7 +546,7 @@ int execute_tests( int argc , const char ** argv ) // If there are no unmet dependencies execute the test if( unmet_dep_count == 0 ) { - test_info.failed = 0; + test_info.result = TEST_RESULT_SUCCESS; test_info.paramfail_test_state = PARAMFAIL_TESTSTATE_IDLE; #if defined(__unix__) || (defined(__APPLE__) && defined(__MACH__)) @@ -610,10 +611,15 @@ int execute_tests( int argc , const char ** argv ) } else if( ret == DISPATCH_TEST_SUCCESS ) { - if( test_info.failed == 0 ) + if( test_info.result == TEST_RESULT_SUCCESS ) { mbedtls_fprintf( stdout, "PASS\n" ); } + else if( test_info.result == TEST_RESULT_SKIPPED ) + { + mbedtls_fprintf( stdout, "----\n" ); + total_skipped++; + } else { total_errors++; diff --git a/tests/suites/main_test.function b/tests/suites/main_test.function index ca4783dcf..1f4180de3 100644 --- a/tests/suites/main_test.function +++ b/tests/suites/main_test.function @@ -156,7 +156,7 @@ void execute_function_ptr(TestWrapper_t fp, void **params) else { /* Unexpected parameter validation error */ - test_info.failed = 1; + test_info.result = TEST_RESULT_FAILED; } memset( param_fail_jmp, 0, sizeof(jmp_buf) ); From ac8c9847846867cf9837510770ec602a43472c43 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Fri, 5 Jul 2019 13:53:06 +0100 Subject: [PATCH 16/17] SSL tests: Skip tests using version not matching hardcoded version --- tests/suites/test_suite_ssl.function | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index 9707935ee..d37dd7407 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -168,8 +168,10 @@ static int build_transforms( mbedtls_ssl_transform *t_in, ((void) etm); #endif +#if !defined(MBEDTLS_SSL_CONF_FIXED_MINOR_VER) t_out->minor_ver = ver; t_in->minor_ver = ver; +#endif t_out->ivlen = ivlen; t_in->ivlen = ivlen; @@ -432,6 +434,11 @@ void ssl_crypt_record( int cipher_type, int hash_id, mbedtls_ssl_init( &ssl ); mbedtls_ssl_transform_init( &t0 ); mbedtls_ssl_transform_init( &t1 ); + +#if defined(MBEDTLS_SSL_CONF_FIXED_MINOR_VER) + TEST_ASSUME( ver == MBEDTLS_SSL_CONF_FIXED_MINOR_VER ); +#endif + TEST_ASSERT( build_transforms( &t0, &t1, cipher_type, hash_id, etm, tag_mode, ver, (size_t) cid0_len, @@ -573,6 +580,10 @@ void ssl_crypt_record_small( int cipher_type, int hash_id, (size_t) cid0_len, (size_t) cid1_len ) == 0 ); +#if defined(MBEDTLS_SSL_CONF_FIXED_MINOR_VER) + TEST_ASSUME( ver == MBEDTLS_SSL_CONF_FIXED_MINOR_VER ); +#endif + TEST_ASSERT( ( buf = mbedtls_calloc( 1, buflen ) ) != NULL ); for( mode=1; mode <= 3; mode++ ) From 90f7b753546217c1ca80a31c8e9933845a9163e8 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 8 Jul 2019 11:29:13 +0100 Subject: [PATCH 17/17] Fix unused variable warning in SSL test suite --- tests/suites/test_suite_ssl.function | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index d37dd7407..7d7845e77 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -171,6 +171,8 @@ static int build_transforms( mbedtls_ssl_transform *t_in, #if !defined(MBEDTLS_SSL_CONF_FIXED_MINOR_VER) t_out->minor_ver = ver; t_in->minor_ver = ver; +#else + ((void) ver); #endif t_out->ivlen = ivlen; t_in->ivlen = ivlen;