From 7bcf2b5875d3d6b052ba94aa1f46a66e9eb33032 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Fri, 26 Jul 2019 09:02:40 +0100 Subject: [PATCH 1/2] Introduce version comparing functions This zero-cost abstraction allows to change the internal encoding of TLS/DTLS versions in the future. --- include/mbedtls/platform_util.h | 6 ++ include/mbedtls/ssl_internal.h | 41 ++++++++- library/ssl_cli.c | 31 ++++--- library/ssl_srv.c | 42 ++++++--- library/ssl_tls.c | 125 ++++++++++++++++----------- programs/ssl/ssl_client2.c | 23 +++-- programs/ssl/ssl_server2.c | 23 +++-- tests/suites/test_suite_ssl.function | 2 +- 8 files changed, 207 insertions(+), 86 deletions(-) diff --git a/include/mbedtls/platform_util.h b/include/mbedtls/platform_util.h index 09d096518..98384add7 100644 --- a/include/mbedtls/platform_util.h +++ b/include/mbedtls/platform_util.h @@ -113,6 +113,12 @@ void mbedtls_param_failed( const char *failure_condition, #endif /* MBEDTLS_CHECK_PARAMS */ +#if defined(__GNUC__) || defined(__arm__) +#define MBEDTLS_ALWAYS_INLINE __attribute__((always_inline)) +#else +#define MBEDTLS_ALWAYS_INLINE +#endif + /* Internal helper macros for deprecating API constants. */ #if !defined(MBEDTLS_DEPRECATED_REMOVED) #if defined(MBEDTLS_DEPRECATED_WARNING) diff --git a/include/mbedtls/ssl_internal.h b/include/mbedtls/ssl_internal.h index df221fe8b..a1acc8462 100644 --- a/include/mbedtls/ssl_internal.h +++ b/include/mbedtls/ssl_internal.h @@ -1237,6 +1237,44 @@ MBEDTLS_ALWAYS_INLINE static inline void mbedtls_ssl_read_version( #endif /* MBEDTLS_SSL_PROTO_TLS */ } + +MBEDTLS_ALWAYS_INLINE static inline int mbedtls_ssl_ver_leq( int v0, int v1 ) +{ + return( v0 <= v1 ); +} + +MBEDTLS_ALWAYS_INLINE static inline int mbedtls_ssl_ver_lt( int v0, int v1 ) +{ + return( v0 < v1 ); +} + +MBEDTLS_ALWAYS_INLINE static inline int mbedtls_ssl_ver_geq( int v0, int v1 ) +{ + return( v0 >= v1 ); +} + +MBEDTLS_ALWAYS_INLINE static inline int mbedtls_ssl_ver_gt( int v0, int v1 ) +{ + return( v0 > v1 ); +} + +MBEDTLS_ALWAYS_INLINE static inline size_t mbedtls_ssl_minor_ver_index( + int ver ) +{ + switch( ver ) + { + case MBEDTLS_SSL_MINOR_VERSION_0: + return( 0 ); + case MBEDTLS_SSL_MINOR_VERSION_1: + return( 1 ); + case MBEDTLS_SSL_MINOR_VERSION_2: + return( 2 ); + case MBEDTLS_SSL_MINOR_VERSION_3: + return( 3 ); + } + return( 0 ); +} + #ifdef __cplusplus } #endif @@ -1677,7 +1715,8 @@ static inline unsigned int mbedtls_ssl_conf_get_ems_enforced( #define MBEDTLS_SSL_BEGIN_FOR_EACH_CIPHERSUITE( ssl, ver, info ) \ { \ int const *__id_ptr; \ - for( __id_ptr=(ssl)->conf->ciphersuite_list[ (ver) ]; \ + for( __id_ptr=(ssl)->conf->ciphersuite_list[ \ + mbedtls_ssl_minor_ver_index( ver ) ]; \ *__id_ptr != 0; __id_ptr++ ) \ { \ const int __id = *__id_ptr; \ diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 4e99a801b..ae1c400db 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -772,8 +772,10 @@ 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 ) + if( mbedtls_ssl_ver_gt( mbedtls_ssl_suite_get_min_minor_ver( suite_info ), + max_minor_ver ) || + mbedtls_ssl_ver_lt( mbedtls_ssl_suite_get_max_minor_ver( suite_info ), + min_minor_ver ) ) { return( 1 ); } @@ -1553,10 +1555,12 @@ static int ssl_parse_hello_verify_request( mbedtls_ssl_context *ssl ) * Since the RFC is not clear on this point, accept DTLS 1.0 (TLS 1.1) * even is lower than our min version. */ - if( major_ver < MBEDTLS_SSL_MAJOR_VERSION_3 || - minor_ver < MBEDTLS_SSL_MINOR_VERSION_2 || - major_ver > mbedtls_ssl_conf_get_max_major_ver( ssl->conf ) || - minor_ver > mbedtls_ssl_conf_get_max_minor_ver( ssl->conf ) ) + if( mbedtls_ssl_ver_lt( major_ver, MBEDTLS_SSL_MAJOR_VERSION_3 ) || + mbedtls_ssl_ver_lt( minor_ver, MBEDTLS_SSL_MINOR_VERSION_2 ) || + mbedtls_ssl_ver_gt( major_ver, + mbedtls_ssl_conf_get_max_major_ver( ssl->conf ) ) || + mbedtls_ssl_ver_gt( minor_ver, + mbedtls_ssl_conf_get_max_minor_ver( ssl->conf ) ) ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad server version" ) ); @@ -1711,10 +1715,14 @@ static int ssl_parse_server_hello( mbedtls_ssl_context *ssl ) ssl->conf->transport, buf + 0 ); - 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 ) ) + if( mbedtls_ssl_ver_lt( major_ver, + mbedtls_ssl_conf_get_min_major_ver( ssl->conf ) ) || + mbedtls_ssl_ver_lt( minor_ver, + mbedtls_ssl_conf_get_min_minor_ver( ssl->conf ) ) || + mbedtls_ssl_ver_gt( major_ver, + mbedtls_ssl_conf_get_max_major_ver( ssl->conf ) ) || + mbedtls_ssl_ver_gt( 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]", @@ -2926,7 +2934,8 @@ static int ssl_in_server_key_exchange_parse( mbedtls_ssl_context *ssl, #endif /* MBEDTLS_SSL_PROTO_TLS1_2 */ #if defined(MBEDTLS_SSL_PROTO_SSL3) || defined(MBEDTLS_SSL_PROTO_TLS1) || \ defined(MBEDTLS_SSL_PROTO_TLS1_1) - if( mbedtls_ssl_get_minor_ver( ssl ) < MBEDTLS_SSL_MINOR_VERSION_3 ) + if( mbedtls_ssl_ver_lt( mbedtls_ssl_get_minor_ver( ssl ), + MBEDTLS_SSL_MINOR_VERSION_3 ) ) { pk_alg = mbedtls_ssl_get_ciphersuite_sig_pk_alg( ciphersuite_info ); diff --git a/library/ssl_srv.c b/library/ssl_srv.c index b058e7c81..11bed2fc2 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -869,7 +869,8 @@ 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( mbedtls_ssl_get_minor_ver( ssl ) < MBEDTLS_SSL_MINOR_VERSION_3 ) + if( mbedtls_ssl_ver_lt( mbedtls_ssl_get_minor_ver( ssl ), + MBEDTLS_SSL_MINOR_VERSION_3 ) ) { mbedtls_md_type_t sig_md; { @@ -936,10 +937,12 @@ 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 ) - > mbedtls_ssl_get_minor_ver( ssl ) || - mbedtls_ssl_suite_get_max_minor_ver( suite_info ) - < mbedtls_ssl_get_minor_ver( ssl ) ) + if( mbedtls_ssl_ver_gt( + mbedtls_ssl_suite_get_min_minor_ver( suite_info ), + mbedtls_ssl_get_minor_ver( ssl ) ) || + mbedtls_ssl_ver_lt( + 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 ); @@ -1111,7 +1114,8 @@ static int ssl_parse_client_hello_v2( mbedtls_ssl_context *ssl ) ? buf[4] : mbedtls_ssl_conf_get_max_minor_ver( ssl->conf ); #endif - if( mbedtls_ssl_get_minor_ver( ssl ) < mbedtls_ssl_conf_get_min_minor_ver( ssl->conf ) ) + if( mbedtls_ssl_ver_lt( 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]", @@ -1237,8 +1241,9 @@ static int ssl_parse_client_hello_v2( mbedtls_ssl_context *ssl ) { MBEDTLS_SSL_DEBUG_MSG( 3, ( "received FALLBACK_SCSV" ) ); - if( mbedtls_ssl_get_minor_ver( ssl ) < - mbedtls_ssl_conf_get_max_minor_ver( ssl->conf ) ) + if( mbedtls_ssl_ver_lt( + mbedtls_ssl_get_minor_ver( ssl ), + mbedtls_ssl_conf_get_max_minor_ver( ssl->conf ) ) ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "inapropriate fallback" ) ); @@ -1652,8 +1657,10 @@ read_record_header: #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 ) ) + if( mbedtls_ssl_ver_lt( major_ver, + mbedtls_ssl_conf_get_min_major_ver( ssl->conf ) ) || + mbedtls_ssl_ver_lt( 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]", @@ -1665,13 +1672,19 @@ read_record_header: return( MBEDTLS_ERR_SSL_BAD_HS_PROTOCOL_VERSION ); } - if( major_ver > mbedtls_ssl_conf_get_max_major_ver( ssl->conf ) ) + if( mbedtls_ssl_ver_gt( + 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 ) ) + else if( mbedtls_ssl_ver_gt( + 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; @@ -2061,8 +2074,9 @@ read_record_header: { MBEDTLS_SSL_DEBUG_MSG( 2, ( "received FALLBACK_SCSV" ) ); - if( mbedtls_ssl_get_minor_ver( ssl ) < - mbedtls_ssl_conf_get_max_minor_ver( ssl->conf ) ) + if( mbedtls_ssl_ver_lt( + mbedtls_ssl_get_minor_ver( ssl ), + 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 d4d51b5ef..d600668fe 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -861,7 +861,7 @@ MBEDTLS_ALWAYS_INLINE static inline int ssl_prf( int minor_ver, else #endif #if defined(MBEDTLS_SSL_PROTO_TLS1) || defined(MBEDTLS_SSL_PROTO_TLS1_1) - if( minor_ver < MBEDTLS_SSL_MINOR_VERSION_3 ) + if( mbedtls_ssl_ver_lt( minor_ver, MBEDTLS_SSL_MINOR_VERSION_3 ) ) return( tls1_prf( secret, slen, label, random, rlen, dstbuf, dlen ) ); else #endif @@ -1160,7 +1160,7 @@ MBEDTLS_ALWAYS_INLINE static inline int ssl_calc_finished( else #endif #if defined(MBEDTLS_SSL_PROTO_TLS1) || defined(MBEDTLS_SSL_PROTO_TLS1_1) - if( minor_ver < MBEDTLS_SSL_MINOR_VERSION_3 ) + if( mbedtls_ssl_ver_lt( minor_ver, MBEDTLS_SSL_MINOR_VERSION_3 ) ) ssl_calc_finished_tls( ssl, buf, from ); else #endif @@ -1484,7 +1484,7 @@ int ssl_populate_transform( mbedtls_ssl_transform *transform, #endif /* MBEDTLS_SSL_PROTO_SSL3 */ #if defined(MBEDTLS_SSL_PROTO_TLS1) || defined(MBEDTLS_SSL_PROTO_TLS1_1) || \ defined(MBEDTLS_SSL_PROTO_TLS1_2) - if( minor_ver >= MBEDTLS_SSL_MINOR_VERSION_1 ) + if( mbedtls_ssl_ver_geq( minor_ver, MBEDTLS_SSL_MINOR_VERSION_1 ) ) { /* For HMAC-based ciphersuites, initialize the HMAC transforms. For AEAD-based ciphersuites, there is nothing to do here. */ @@ -1759,7 +1759,7 @@ int mbedtls_ssl_calc_verify( int minor_ver, else #endif #if defined(MBEDTLS_SSL_PROTO_TLS1) || defined(MBEDTLS_SSL_PROTO_TLS1_1) - if( minor_ver < MBEDTLS_SSL_MINOR_VERSION_3 ) + if( mbedtls_ssl_ver_lt( minor_ver, MBEDTLS_SSL_MINOR_VERSION_3 ) ) ssl_calc_verify_tls( ssl, dst, hlen ); else #endif @@ -2513,8 +2513,9 @@ 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( mbedtls_ssl_transform_get_minor_ver( transform ) >= - MBEDTLS_SSL_MINOR_VERSION_1 ) + if( mbedtls_ssl_ver_geq( + mbedtls_ssl_transform_get_minor_ver( transform ), + MBEDTLS_SSL_MINOR_VERSION_1 ) ) { unsigned char mac[MBEDTLS_SSL_MAC_ADD]; @@ -2693,8 +2694,9 @@ 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( mbedtls_ssl_transform_get_minor_ver( transform ) >= - MBEDTLS_SSL_MINOR_VERSION_2 ) + if( mbedtls_ssl_ver_geq( + mbedtls_ssl_transform_get_minor_ver( transform ), + MBEDTLS_SSL_MINOR_VERSION_2 ) ) { if( f_rng == NULL ) { @@ -2743,8 +2745,9 @@ int mbedtls_ssl_encrypt_buf( mbedtls_ssl_context *ssl, } #if defined(MBEDTLS_SSL_PROTO_SSL3) || defined(MBEDTLS_SSL_PROTO_TLS1) - if( mbedtls_ssl_transform_get_minor_ver( transform ) < - MBEDTLS_SSL_MINOR_VERSION_2 ) + if( mbedtls_ssl_ver_lt( + mbedtls_ssl_transform_get_minor_ver( transform ), + MBEDTLS_SSL_MINOR_VERSION_2 ) ) { /* * Save IV in SSL3 and TLS1 @@ -3001,8 +3004,9 @@ int mbedtls_ssl_decrypt_buf( mbedtls_ssl_context const *ssl, * Check immediate ciphertext sanity */ #if defined(MBEDTLS_SSL_PROTO_TLS1_1) || defined(MBEDTLS_SSL_PROTO_TLS1_2) - if( mbedtls_ssl_transform_get_minor_ver( transform ) >= - MBEDTLS_SSL_MINOR_VERSION_2 ) + if( mbedtls_ssl_ver_geq( + mbedtls_ssl_transform_get_minor_ver( transform ), + MBEDTLS_SSL_MINOR_VERSION_2 ) ) { /* The ciphertext is prefixed with the CBC IV. */ minlen += transform->ivlen; @@ -3107,8 +3111,9 @@ int mbedtls_ssl_decrypt_buf( mbedtls_ssl_context const *ssl, /* * Initialize for prepended IV for block cipher in TLS v1.1 and up */ - if( mbedtls_ssl_transform_get_minor_ver( transform ) >= - MBEDTLS_SSL_MINOR_VERSION_2 ) + if( mbedtls_ssl_ver_geq( + mbedtls_ssl_transform_get_minor_ver( transform ), + MBEDTLS_SSL_MINOR_VERSION_2 ) ) { /* Safe because data_len >= minlen + ivlen = 2 * ivlen. */ memcpy( transform->iv_dec, data, transform->ivlen ); @@ -3137,8 +3142,9 @@ int mbedtls_ssl_decrypt_buf( mbedtls_ssl_context const *ssl, } #if defined(MBEDTLS_SSL_PROTO_SSL3) || defined(MBEDTLS_SSL_PROTO_TLS1) - if( mbedtls_ssl_transform_get_minor_ver( transform ) < - MBEDTLS_SSL_MINOR_VERSION_2 ) + if( mbedtls_ssl_ver_lt( + mbedtls_ssl_transform_get_minor_ver( transform ), + MBEDTLS_SSL_MINOR_VERSION_2 ) ) { /* * Save IV in SSL3 and TLS1, where CBC decryption of consecutive @@ -3201,8 +3207,9 @@ int mbedtls_ssl_decrypt_buf( mbedtls_ssl_context const *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( mbedtls_ssl_transform_get_minor_ver( transform ) > - MBEDTLS_SSL_MINOR_VERSION_0 ) + if( mbedtls_ssl_ver_gt( + 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 @@ -3300,8 +3307,9 @@ int mbedtls_ssl_decrypt_buf( mbedtls_ssl_context const *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( mbedtls_ssl_transform_get_minor_ver( transform ) > - MBEDTLS_SSL_MINOR_VERSION_0 ) + if( mbedtls_ssl_ver_gt( + mbedtls_ssl_transform_get_minor_ver( transform ), + MBEDTLS_SSL_MINOR_VERSION_0 ) ) { /* * Process MAC and always update for padlen afterwards to make @@ -5320,7 +5328,8 @@ static int ssl_parse_record_header( mbedtls_ssl_context const *ssl, return( MBEDTLS_ERR_SSL_INVALID_RECORD ); } - if( minor_ver > mbedtls_ssl_conf_get_max_minor_ver( ssl->conf ) ) + if( mbedtls_ssl_ver_gt( 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 ); @@ -8136,7 +8145,9 @@ static void ssl_update_out_pointers( mbedtls_ssl_context *ssl, /* Adjust out_msg to make space for explicit IV, if used. */ if( transform != NULL && - mbedtls_ssl_get_minor_ver( ssl ) >= MBEDTLS_SSL_MINOR_VERSION_2 ) + mbedtls_ssl_ver_geq( + mbedtls_ssl_get_minor_ver( ssl ), + MBEDTLS_SSL_MINOR_VERSION_2 ) ) { ssl->out_msg = ssl->out_iv + transform->ivlen - transform->fixed_ivlen; } @@ -8625,10 +8636,10 @@ int mbedtls_ssl_set_session( mbedtls_ssl_context *ssl, const mbedtls_ssl_session void mbedtls_ssl_conf_ciphersuites( mbedtls_ssl_config *conf, const int *ciphersuites ) { - conf->ciphersuite_list[MBEDTLS_SSL_MINOR_VERSION_0] = ciphersuites; - conf->ciphersuite_list[MBEDTLS_SSL_MINOR_VERSION_1] = ciphersuites; - conf->ciphersuite_list[MBEDTLS_SSL_MINOR_VERSION_2] = ciphersuites; - conf->ciphersuite_list[MBEDTLS_SSL_MINOR_VERSION_3] = ciphersuites; + conf->ciphersuite_list[0] = ciphersuites; + conf->ciphersuite_list[1] = ciphersuites; + conf->ciphersuite_list[2] = ciphersuites; + conf->ciphersuite_list[3] = ciphersuites; } void mbedtls_ssl_conf_ciphersuites_for_version( mbedtls_ssl_config *conf, @@ -8638,10 +8649,14 @@ void mbedtls_ssl_conf_ciphersuites_for_version( mbedtls_ssl_config *conf, if( major != MBEDTLS_SSL_MAJOR_VERSION_3 ) return; - if( minor < MBEDTLS_SSL_MINOR_VERSION_0 || minor > MBEDTLS_SSL_MINOR_VERSION_3 ) + if( mbedtls_ssl_ver_lt( minor, MBEDTLS_SSL_MINOR_VERSION_0 ) || + mbedtls_ssl_ver_gt( minor, MBEDTLS_SSL_MINOR_VERSION_3 ) ) + { return; + } - conf->ciphersuite_list[minor] = ciphersuites; + conf->ciphersuite_list[mbedtls_ssl_minor_ver_index( minor )] = + ciphersuites; } #endif /* MBEDTLS_SSL_CONF_SINGLE_CIPHERSUITE */ @@ -9395,8 +9410,12 @@ 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( mbedtls_ssl_get_minor_ver( ssl ) >= MBEDTLS_SSL_MINOR_VERSION_2 ) + if( mbedtls_ssl_ver_geq( + 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 */ break; @@ -10602,7 +10621,9 @@ 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( mbedtls_ssl_get_minor_ver( ssl ) >= MBEDTLS_SSL_MINOR_VERSION_1 ) + if( mbedtls_ssl_ver_geq( + mbedtls_ssl_get_minor_ver( ssl ), + MBEDTLS_SSL_MINOR_VERSION_1 ) ) { ret = mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_WARNING, @@ -10810,7 +10831,9 @@ static int ssl_write_split( mbedtls_ssl_context *ssl, if( ssl->conf->cbc_record_splitting == MBEDTLS_SSL_CBC_RECORD_SPLITTING_DISABLED || len <= 1 || - mbedtls_ssl_get_minor_ver( ssl ) > MBEDTLS_SSL_MINOR_VERSION_1 || + mbedtls_ssl_ver_gt( + 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 ) { @@ -11406,14 +11429,18 @@ static int ssl_context_load( mbedtls_ssl_context *ssl, * least check it matches the requirements for serializing. */ if( MBEDTLS_SSL_TRANSPORT_IS_TLS( ssl->conf->transport ) || - mbedtls_ssl_conf_get_max_major_ver( ssl->conf ) < - MBEDTLS_SSL_MAJOR_VERSION_3 || - mbedtls_ssl_conf_get_min_major_ver( ssl->conf ) > - MBEDTLS_SSL_MAJOR_VERSION_3 || - mbedtls_ssl_conf_get_max_minor_ver( ssl->conf ) < - MBEDTLS_SSL_MINOR_VERSION_3 || - mbedtls_ssl_conf_get_min_minor_ver( ssl->conf ) > - MBEDTLS_SSL_MINOR_VERSION_3 || + mbedtls_ssl_ver_lt( + mbedtls_ssl_conf_get_max_major_ver( ssl->conf ), + MBEDTLS_SSL_MAJOR_VERSION_3 ) || + mbedtls_ssl_ver_gt( + mbedtls_ssl_conf_get_min_major_ver( ssl->conf ), + MBEDTLS_SSL_MAJOR_VERSION_3 ) || + mbedtls_ssl_ver_lt( + mbedtls_ssl_conf_get_max_minor_ver( ssl->conf ), + MBEDTLS_SSL_MINOR_VERSION_3 ) || + mbedtls_ssl_ver_gt( + mbedtls_ssl_conf_get_min_minor_ver( ssl->conf ), + MBEDTLS_SSL_MINOR_VERSION_3 ) || mbedtls_ssl_conf_is_renegotiation_enabled( ssl->conf ) ) { return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA ); @@ -11938,11 +11965,11 @@ int mbedtls_ssl_config_defaults( mbedtls_ssl_config *conf, #endif /* !MBEDTLS_SSL_CONF_MAX_MINOR_VER */ #if !defined(MBEDTLS_SSL_CONF_SINGLE_CIPHERSUITE) - conf->ciphersuite_list[MBEDTLS_SSL_MINOR_VERSION_0] = - conf->ciphersuite_list[MBEDTLS_SSL_MINOR_VERSION_1] = - conf->ciphersuite_list[MBEDTLS_SSL_MINOR_VERSION_2] = - conf->ciphersuite_list[MBEDTLS_SSL_MINOR_VERSION_3] = - ssl_preset_suiteb_ciphersuites; + conf->ciphersuite_list[0] = + conf->ciphersuite_list[1] = + conf->ciphersuite_list[2] = + conf->ciphersuite_list[3] = + ssl_preset_suiteb_ciphersuites; #endif /* MBEDTLS_SSL_CONF_SINGLE_CIPHERSUITE */ #if defined(MBEDTLS_X509_CRT_PARSE_C) @@ -11990,11 +12017,11 @@ int mbedtls_ssl_config_defaults( mbedtls_ssl_config *conf, #endif /* !MBEDTLS_SSL_CONF_MAX_MINOR_VER */ #if !defined(MBEDTLS_SSL_CONF_SINGLE_CIPHERSUITE) - conf->ciphersuite_list[MBEDTLS_SSL_MINOR_VERSION_0] = - conf->ciphersuite_list[MBEDTLS_SSL_MINOR_VERSION_1] = - conf->ciphersuite_list[MBEDTLS_SSL_MINOR_VERSION_2] = - conf->ciphersuite_list[MBEDTLS_SSL_MINOR_VERSION_3] = - mbedtls_ssl_list_ciphersuites(); + conf->ciphersuite_list[0] = + conf->ciphersuite_list[1] = + conf->ciphersuite_list[2] = + conf->ciphersuite_list[3] = + mbedtls_ssl_list_ciphersuites(); #endif /* MBEDTLS_SSL_CONF_SINGLE_CIPHERSUITE */ #if defined(MBEDTLS_X509_CRT_PARSE_C) diff --git a/programs/ssl/ssl_client2.c b/programs/ssl/ssl_client2.c index 788793a49..5c13f8a26 100644 --- a/programs/ssl/ssl_client2.c +++ b/programs/ssl/ssl_client2.c @@ -69,6 +69,8 @@ int main( void ) #include "mbedtls/debug.h" #include "mbedtls/timing.h" +#include "mbedtls/ssl_internal.h" + #include #include #include @@ -1506,14 +1508,18 @@ int main( int argc, char *argv[] ) mbedtls_ssl_ciphersuite_from_id( opt.force_ciphersuite[0] ); if( opt.max_version != -1 && - mbedtls_ssl_suite_get_min_minor_ver( ciphersuite_info ) > opt.max_version ) + mbedtls_ssl_ver_gt( + mbedtls_ssl_suite_get_min_minor_ver( ciphersuite_info ), + opt.max_version ) ) { mbedtls_printf( "forced ciphersuite not allowed with this protocol version\n" ); ret = 2; goto usage; } if( opt.min_version != -1 && - mbedtls_ssl_suite_get_max_minor_ver( ciphersuite_info ) < opt.min_version ) + mbedtls_ssl_ver_lt( + mbedtls_ssl_suite_get_max_minor_ver( ciphersuite_info ), + opt.min_version ) ) { mbedtls_printf( "forced ciphersuite not allowed with this protocol version\n" ); ret = 2; @@ -1523,17 +1529,24 @@ int main( int argc, char *argv[] ) /* If the server selects a version that's not supported by * this suite, then there will be no common ciphersuite... */ if( opt.max_version == -1 || - opt.max_version > mbedtls_ssl_suite_get_max_minor_ver( ciphersuite_info ) ) + mbedtls_ssl_ver_gt( + opt.max_version, + mbedtls_ssl_suite_get_max_minor_ver( ciphersuite_info ) ) ) { opt.max_version = mbedtls_ssl_suite_get_max_minor_ver( ciphersuite_info ); } - if( opt.min_version < mbedtls_ssl_suite_get_min_minor_ver( ciphersuite_info ) ) + if( mbedtls_ssl_ver_lt( + opt.min_version, + mbedtls_ssl_suite_get_min_minor_ver( ciphersuite_info ) ) ) { opt.min_version = mbedtls_ssl_suite_get_min_minor_ver( ciphersuite_info ); /* DTLS starts with TLS 1.1 */ if( opt.transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM && - opt.min_version < MBEDTLS_SSL_MINOR_VERSION_2 ) + mbedtls_ssl_ver_lt( opt.min_version, + MBEDTLS_SSL_MINOR_VERSION_2 ) ) + { opt.min_version = MBEDTLS_SSL_MINOR_VERSION_2; + } } /* Enable RC4 if needed and not explicitly disabled */ diff --git a/programs/ssl/ssl_server2.c b/programs/ssl/ssl_server2.c index 890725e75..a7b5b4d85 100644 --- a/programs/ssl/ssl_server2.c +++ b/programs/ssl/ssl_server2.c @@ -68,6 +68,8 @@ int main( void ) #include "mbedtls/debug.h" #include "mbedtls/timing.h" +#include "mbedtls/ssl_internal.h" + #include #include #include @@ -2232,14 +2234,18 @@ int main( int argc, char *argv[] ) mbedtls_ssl_ciphersuite_from_id( opt.force_ciphersuite[0] ); if( opt.max_version != -1 && - mbedtls_ssl_suite_get_min_minor_ver( ciphersuite_info ) > opt.max_version ) + mbedtls_ssl_ver_gt( + mbedtls_ssl_suite_get_min_minor_ver( ciphersuite_info ), + opt.max_version ) ) { mbedtls_printf( "forced ciphersuite not allowed with this protocol version\n" ); ret = 2; goto usage; } if( opt.min_version != -1 && - mbedtls_ssl_suite_get_max_minor_ver( ciphersuite_info ) < opt.min_version ) + mbedtls_ssl_ver_lt( + mbedtls_ssl_suite_get_max_minor_ver( ciphersuite_info ), + opt.min_version ) ) { mbedtls_printf( "forced ciphersuite not allowed with this protocol version\n" ); ret = 2; @@ -2249,17 +2255,24 @@ int main( int argc, char *argv[] ) /* If we select a version that's not supported by * this suite, then there will be no common ciphersuite... */ if( opt.max_version == -1 || - opt.max_version > mbedtls_ssl_suite_get_max_minor_ver( ciphersuite_info ) ) + mbedtls_ssl_ver_gt( + opt.max_version, + mbedtls_ssl_suite_get_max_minor_ver( ciphersuite_info ) ) ) { opt.max_version = mbedtls_ssl_suite_get_max_minor_ver( ciphersuite_info ); } - if( opt.min_version < mbedtls_ssl_suite_get_min_minor_ver( ciphersuite_info ) ) + if( mbedtls_ssl_ver_lt( + opt.min_version, + mbedtls_ssl_suite_get_min_minor_ver( ciphersuite_info ) ) ) { opt.min_version = mbedtls_ssl_suite_get_min_minor_ver( ciphersuite_info ); /* DTLS starts with TLS 1.1 */ if( opt.transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM && - opt.min_version < MBEDTLS_SSL_MINOR_VERSION_2 ) + mbedtls_ssl_ver_lt( opt.min_version, + MBEDTLS_SSL_MINOR_VERSION_2 ) ) + { opt.min_version = MBEDTLS_SSL_MINOR_VERSION_2; + } } /* Enable RC4 if needed and not explicitly disabled */ diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index b177779e7..268d56cea 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -121,7 +121,7 @@ static int build_transforms( mbedtls_ssl_transform *t_in, CHK( mbedtls_md_setup( &t_in->md_ctx_enc, md_info, 1 ) == 0 ); CHK( mbedtls_md_setup( &t_in->md_ctx_dec, md_info, 1 ) == 0 ); - if( ver > MBEDTLS_SSL_MINOR_VERSION_0 ) + if( mbedtls_ssl_ver_gt( ver, MBEDTLS_SSL_MINOR_VERSION_0 ) ) { CHK( mbedtls_md_hmac_starts( &t_in->md_ctx_enc, md0, maclen ) == 0 ); From d5cfe6fbd00c76d3513836aafb62c02543ff9b2e Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Fri, 26 Jul 2019 11:59:45 +0100 Subject: [PATCH 2/2] Use native DTLS version encoding if only DTLS is enabled This commit changes the internal identifiers MBEDTLS_SSL_MINOR_VERSION_XXX in DTLS-only builds to match the version encoding used by the DTLS standard, encoding DTLS 1.0 as 255 and DTLS 1.2 as DTLS 1.0. Accordingly, the version comparison functions introduced in the previous commit must be re-implemented, as older version have _larger_ identifiers now. Further, since we identify DTLS 1.0 as MBEDTLS_SSL_MINOR_VERSION_2 and DTLS 1.2 as MBEDTLS_SSL_MINOR_VERSION_3, what remains is to define MBEDTLS_SSL_MINOR_VERSION_{0|1}. While these don't have any meaning meaning in DTLS, they still need to be set and obey the ordering in the sense that the version comparison functions '<=' should attest that MBEDTLS_SSL_MINOR_VERSION_i '<=' MBEDTLS_SSL_MINOR_VERSION_j for i <= j. Since '<=' is actually >= and the wire format value for DTLS 1.0 == MBEDTLS_SSL_MINOR_VERSION_2 is the 255, this forces us to use values beyond 255, and hence to extend the storage type for minor versions from uint8_t to uint16_t. --- include/mbedtls/ssl.h | 22 ++++++++++------ include/mbedtls/ssl_internal.h | 46 ++++++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 7 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 7efb411f3..e3548422f 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -139,11 +139,19 @@ /* * Various constants */ +#if !defined(MBEDTLS_SSL_PROTO_NO_TLS) #define MBEDTLS_SSL_MAJOR_VERSION_3 3 #define MBEDTLS_SSL_MINOR_VERSION_0 0 /*!< SSL v3.0 */ #define MBEDTLS_SSL_MINOR_VERSION_1 1 /*!< TLS v1.0 */ #define MBEDTLS_SSL_MINOR_VERSION_2 2 /*!< TLS v1.1 */ #define MBEDTLS_SSL_MINOR_VERSION_3 3 /*!< TLS v1.2 */ +#else /* MBEDTLS_SSL_PROTO_NO_TLS */ +#define MBEDTLS_SSL_MAJOR_VERSION_3 254 +#define MBEDTLS_SSL_MINOR_VERSION_0 257 /*!< unused */ +#define MBEDTLS_SSL_MINOR_VERSION_1 256 /*!< unused */ +#define MBEDTLS_SSL_MINOR_VERSION_2 255 /*!< DTLS v1.0 */ +#define MBEDTLS_SSL_MINOR_VERSION_3 253 /*!< DTLS v1.2 */ +#endif /* MBEDTLS_SSL_PROTO_NO_TLS */ #define MBEDTLS_SSL_TRANSPORT_STREAM 0 /*!< TLS */ #define MBEDTLS_SSL_TRANSPORT_DATAGRAM 1 /*!< DTLS */ @@ -1151,18 +1159,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_MAX_MAJOR_VER) + unsigned char max_major_ver; /*!< max. major version used */ +#endif /* !MBEDTLS_SSL_CONF_MAX_MAJOR_VER */ #if !defined(MBEDTLS_SSL_CONF_MIN_MINOR_VER) - unsigned char min_minor_ver; /*!< min. minor version used */ + uint16_t min_minor_ver; /*!< min. minor version used */ #endif /* !MBEDTLS_SSL_CONF_MIN_MINOR_VER */ +#if !defined(MBEDTLS_SSL_CONF_MAX_MINOR_VER) + uint16_t max_minor_ver; /*!< max. minor version used */ +#endif /* !MBEDTLS_SSL_CONF_MAX_MINOR_VER */ /* * Flags (bitfields) diff --git a/include/mbedtls/ssl_internal.h b/include/mbedtls/ssl_internal.h index a1acc8462..a16811542 100644 --- a/include/mbedtls/ssl_internal.h +++ b/include/mbedtls/ssl_internal.h @@ -1176,6 +1176,8 @@ 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 */ +#if defined(MBEDTLS_SSL_PROTO_TLS) + /* * Convert version numbers to/from wire format * and, for DTLS, to/from TLS equivalent. @@ -1258,6 +1260,50 @@ MBEDTLS_ALWAYS_INLINE static inline int mbedtls_ssl_ver_gt( int v0, int v1 ) return( v0 > v1 ); } +#else /* MBEDTLS_SSL_PROTO_TLS */ + +/* If only DTLS is enabled, we can match the internal encoding + * with the standard's encoding of versions. */ +static inline void mbedtls_ssl_write_version( int major, int minor, + int transport, + unsigned char ver[2] ) +{ + ((void) transport); + ver[0] = (unsigned char) major; + ver[1] = (unsigned char) minor; +} + +static inline void mbedtls_ssl_read_version( int *major, int *minor, + int transport, + const unsigned char ver[2] ) +{ + ((void) transport); + *major = ver[0]; + *minor = ver[1]; +} + +MBEDTLS_ALWAYS_INLINE static inline int mbedtls_ssl_ver_leq( int v0, int v1 ) +{ + return( v0 >= v1 ); +} + +MBEDTLS_ALWAYS_INLINE static inline int mbedtls_ssl_ver_lt( int v0, int v1 ) +{ + return( v0 > v1 ); +} + +MBEDTLS_ALWAYS_INLINE static inline int mbedtls_ssl_ver_geq( int v0, int v1 ) +{ + return( v0 <= v1 ); +} + +MBEDTLS_ALWAYS_INLINE static inline int mbedtls_ssl_ver_gt( int v0, int v1 ) +{ + return( v0 < v1 ); +} + +#endif /* MBEDTLS_SSL_PROTO_TLS */ + MBEDTLS_ALWAYS_INLINE static inline size_t mbedtls_ssl_minor_ver_index( int ver ) {