From aabbb582eb90754390a399b15cc56aebbf97c1c0 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 11 Jun 2019 13:43:27 +0100 Subject: [PATCH 01/11] Exemplify harcoding SSL config at compile-time in example of ExtMS This commit is the first in a series demonstrating how code-size can be reduced by hardcoding parts of the SSL configuration at compile-time, focusing on the example of the configuration of the ExtendedMasterSecret extension. The flexibility of an SSL configuration defined a runtime vs. compile-time is necessary for the use of Mbed TLS as a dynamically linked library, but is undesirable in constrained environments because it introduces the following overhead: - Definition of SSL configuration API (code-size overhead) (and on the application-side: The API needs to be called) - Additional fields in the SSL configuration (RAM overhead, and potentially code-size overhead if structures grow beyond immediate-offset bounds). - Dereferencing is needed to obtain configuration settings. - Code contains branches and potentially additional structure fields to distinguish between different configurations. Considering the example of the ExtendedMasterSecret extension, this instantiates as follows: - mbedtls_ssl_conf_extended_master_secret() and mbedtls_ssl_conf_extended_master_secret_enforced() are introduced to configure the ExtendedMasterSecret extension. - mbedtls_ssl_config contains bitflags `extended_ms` and `enforce_extended_master_secret` reflecting the runtime configuration of the ExtendedMasterSecret extension. - Whenever we need to access these fields, we need a chain of dereferences `ssl->conf->extended_ms`. - Determining whether Client/Server should write the ExtendedMasterSecret extension needs a branch depending on `extended_ms`, and the state of the ExtendedMasterSecret negotiation needs to be stored in a new handshake-local variable mbedtls_ssl_handshake_params::extended_ms. Finally (that's the point of ExtendedMasterSecret) key derivation depends on this handshake-local state of ExtendedMasterSecret. All this is unnecessary if it is known at compile-time that the ExtendedMasterSecret extension is used and enforced: - No API calls are necessary because the configuration is fixed at compile-time. - No SSL config fields are necessary because there are corresponding compile-time constants instead. - Accordingly, no dereferences for field accesses are necessary, and these accesses can instead be replaced by the corresponding compile-time constants. - Branches can be eliminated at compile-time because the compiler knows the configuration. Also, specifically for the ExtendedMasterSecret extension, the field `extended_ms` in the handshake structure is unnecessary, because we can fail immediately during the Hello- stage of the handshake if the ExtendedMasterSecret extension is not negotiated; accordingly, the non-ExtendedMS code-path can be eliminated from the key derivation logic. A way needs to be found to allow fixing parts of the SSL configuration at compile-time which removes this overhead in case it is used, while at the same time maintaining readability and backwards compatibility. This commit proposes the following approach: From the user perspective, for aspect of the SSL configuration mbedtls_ssl_config that should be configurable at compile-time, introduce a compile-time option MBEDTLS_SSL_CONF_FIELD_NAME. If this option is not defined, the field is kept and configurable at runtime as usual. If the option is defined, the field is logically forced to the value of the option at compile time. Internally, read-access to fields in the SSL configuration which are configurable at compile-time gets replaced by new `static inline` getter functions which evaluate to the corresponding field access or to the constant MBEDTLS_SSL_CONF_FIELD_NAME, depending on whether the latter is defined or not. Write-access to fields which are configurable at compile-time needs to be removed: Specifically, the corresponding API itself either needs to be removed or replaced by a stub function without effect. This commit takes the latter approach, which has the benefit of not requiring any change on the example applications, but introducing the risk of mismatching API calls and compile-time configuration, in case a user doesn't correctly keep track of which parts of the configuration have been fixed at compile-time, and which haven't. Write-access for the purpose of setting defaults is simply omitted. --- configs/baremetal.h | 6 ++++++ include/mbedtls/config.h | 19 +++++++++++++++++++ include/mbedtls/ssl.h | 32 ++++++++++++++++++++++++++++++++ library/ssl_cli.c | 15 +++++++++------ library/ssl_srv.c | 12 +++++++----- library/ssl_tls.c | 16 +++++++++++++++- programs/ssl/query_config.c | 16 ++++++++++++++++ 7 files changed, 104 insertions(+), 12 deletions(-) diff --git a/configs/baremetal.h b/configs/baremetal.h index f82e5f2b9..330b513fc 100644 --- a/configs/baremetal.h +++ b/configs/baremetal.h @@ -79,6 +79,12 @@ #define MBEDTLS_SSL_DTLS_BADMAC_LIMIT #define MBEDTLS_SSL_DTLS_CONNECTION_ID +/* Compile-time fixed parts of the SSL configuration */ +#define MBEDTLS_SSL_CONF_EXTENDED_MASTER_SECRET \ + MBEDTLS_SSL_EXTENDED_MS_ENABLED +#define MBEDTLS_SSL_CONF_ENFORCE_EXTENDED_MASTER_SECRET \ + MBEDTLS_SSL_EXTENDED_MS_ENFORCE_ENABLED + /* X.509 CRT parsing */ #define MBEDTLS_X509_USE_C #define MBEDTLS_X509_CRT_PARSE_C diff --git a/include/mbedtls/config.h b/include/mbedtls/config.h index e5d593312..8dcb81c5f 100644 --- a/include/mbedtls/config.h +++ b/include/mbedtls/config.h @@ -3438,6 +3438,25 @@ /* \} name SECTION: Customisation configuration options */ +/** + * \name SECTION: Compile-time SSL configuration + * + * This section allows to fix parts of the SSL configuration + * at compile-time. If a field is fixed at compile-time, the + * corresponding SSL configuration API `mbedtls_ssl_conf_xxx()` + * remains present, but takes no effect anymore. + * + * This can be used on constrained systems to reduce code-size. + * \{ + */ + +/* ExtendedMasterSecret extension + * The following two options must be set/unset simultaneously. */ +//#define MBEDTLS_SSL_CONF_EXTENDED_MASTER_SECRET MBEDTLS_SSL_EXTENDED_MS_ENABLED +//#define MBEDTLS_SSL_CONF_ENFORCE_EXTENDED_MASTER_SECRET MBEDTLS_SSL_EXTENDED_MS_ENFORCE_DISABLED + +/* \} SECTION: Compile-time SSL configuration */ + /* Target and application specific configurations * * Allow user to override any previous default. diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index eeb03e145..562fdac92 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -1060,10 +1060,14 @@ struct mbedtls_ssl_config unsigned int encrypt_then_mac : 1 ; /*!< negotiate encrypt-then-mac? */ #endif #if defined(MBEDTLS_SSL_EXTENDED_MASTER_SECRET) +#if !defined(MBEDTLS_SSL_CONF_EXTENDED_MASTER_SECRET) unsigned int extended_ms : 1; /*!< negotiate extended master secret? */ +#endif /* !MBEDTLS_SSL_EXTENDED_MASTER_SECRET */ +#if !defined(MBEDTLS_SSL_CONF_ENFORCE_EXTENDED_MASTER_SECRET) unsigned int enforce_extended_master_secret : 1; /*!< enforce the usage * of extended master * secret */ +#endif /* !MBEDTLS_SSL_CONF_ENFORCE_EXTENDED_MASTER_SECRET */ #endif #if defined(MBEDTLS_SSL_DTLS_ANTI_REPLAY) unsigned int anti_replay : 1; /*!< detect and prevent replay? */ @@ -1094,6 +1098,34 @@ struct mbedtls_ssl_config #endif /* MBEDTLS_SSL_DTLS_CONNECTION_ID */ }; +/* + * Getter functions for fields in mbedtls_ssl_config which may + * be fixed at compile time via one of MBEDTLS_SSL_SSL_CONF_XXX. + */ + +#if defined(MBEDTLS_SSL_EXTENDED_MASTER_SECRET) +static inline unsigned int mbedtls_ssl_conf_get_ems( + mbedtls_ssl_config const *conf ) +{ +#if !defined(MBEDTLS_SSL_CONF_EXTENDED_MASTER_SECRET) + return( conf->extended_ms ); +#else + ((void) conf); + return( MBEDTLS_SSL_CONF_EXTENDED_MASTER_SECRET ); +#endif /* MBEDTLS_SSL_CONF_EXTENDED_MASTER_SECRET */ +} + +static inline unsigned int +mbedtls_ssl_conf_get_ems_enforced( mbedtls_ssl_config const *conf ) +{ +#if !defined(MBEDTLS_SSL_CONF_ENFORCE_EXTENDED_MASTER_SECRET) + return( conf->enforce_extended_master_secret ); +#else + ((void) conf); + return( MBEDTLS_SSL_CONF_ENFORCE_EXTENDED_MASTER_SECRET ); +#endif /* MBEDTLS_SSL_CONF_ENFORCE_EXTENDED_MASTER_SECRET */ +} +#endif /* MBEDTLS_SSL_EXTENDED_MASTER_SECRET */ struct mbedtls_ssl_context { diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 174e8b150..238eeb14d 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -590,7 +590,8 @@ static void ssl_write_extended_ms_ext( mbedtls_ssl_context *ssl, *olen = 0; - if( ssl->conf->extended_ms == MBEDTLS_SSL_EXTENDED_MS_DISABLED || + if( mbedtls_ssl_conf_get_ems( ssl->conf ) == + MBEDTLS_SSL_EXTENDED_MS_DISABLED || ssl->conf->max_minor_ver == MBEDTLS_SSL_MINOR_VERSION_0 ) { return; @@ -1328,7 +1329,8 @@ static int ssl_parse_extended_ms_ext( mbedtls_ssl_context *ssl, const unsigned char *buf, size_t len ) { - if( ssl->conf->extended_ms == MBEDTLS_SSL_EXTENDED_MS_DISABLED || + if( mbedtls_ssl_conf_get_ems( ssl->conf ) == + MBEDTLS_SSL_EXTENDED_MS_DISABLED || ssl->minor_ver == MBEDTLS_SSL_MINOR_VERSION_0 || len != 0 ) { @@ -2089,10 +2091,11 @@ static int ssl_parse_server_hello( mbedtls_ssl_context *ssl ) * Check if extended master secret is being enforced */ #if defined(MBEDTLS_SSL_EXTENDED_MASTER_SECRET) - if( ssl->conf->extended_ms == MBEDTLS_SSL_EXTENDED_MS_ENABLED && - ssl->conf->enforce_extended_master_secret == - MBEDTLS_SSL_EXTENDED_MS_ENFORCE_ENABLED && - ssl->handshake->extended_ms == MBEDTLS_SSL_EXTENDED_MS_DISABLED ) + if( mbedtls_ssl_conf_get_ems( ssl->conf ) == + MBEDTLS_SSL_EXTENDED_MS_ENABLED && + mbedtls_ssl_conf_get_ems_enforced( ssl->conf ) == + MBEDTLS_SSL_EXTENDED_MS_ENFORCE_ENABLED && + ssl->handshake->extended_ms == MBEDTLS_SSL_EXTENDED_MS_DISABLED) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "Peer not offering extended master " "secret, while it is enforced") ); diff --git a/library/ssl_srv.c b/library/ssl_srv.c index a8821f319..657fbaece 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -567,7 +567,8 @@ static int ssl_parse_extended_ms_ext( mbedtls_ssl_context *ssl, ((void) buf); - if( ssl->conf->extended_ms == MBEDTLS_SSL_EXTENDED_MS_ENABLED && + if( mbedtls_ssl_conf_get_ems( ssl->conf ) == + MBEDTLS_SSL_EXTENDED_MS_ENABLED && ssl->minor_ver != MBEDTLS_SSL_MINOR_VERSION_0 ) { ssl->handshake->extended_ms = MBEDTLS_SSL_EXTENDED_MS_ENABLED; @@ -2039,10 +2040,11 @@ read_record_header: * Check if extended master secret is being enforced */ #if defined(MBEDTLS_SSL_EXTENDED_MASTER_SECRET) - if( ssl->conf->extended_ms == MBEDTLS_SSL_EXTENDED_MS_ENABLED && - ssl->conf->enforce_extended_master_secret == - MBEDTLS_SSL_EXTENDED_MS_ENFORCE_ENABLED && - ssl->handshake->extended_ms == MBEDTLS_SSL_EXTENDED_MS_DISABLED ) + if( mbedtls_ssl_conf_get_ems( ssl->conf ) == + MBEDTLS_SSL_EXTENDED_MS_ENABLED && + mbedtls_ssl_conf_get_ems_enforced( ssl->conf ) == + MBEDTLS_SSL_EXTENDED_MS_ENFORCE_ENABLED && + ssl->handshake->extended_ms == MBEDTLS_SSL_EXTENDED_MS_DISABLED) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "Peer not offering extended master " "secret, while it is enforced") ); diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 3257732e8..5c8a08eb7 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -8611,15 +8611,25 @@ void mbedtls_ssl_conf_encrypt_then_mac( mbedtls_ssl_config *conf, char etm ) #if defined(MBEDTLS_SSL_EXTENDED_MASTER_SECRET) void mbedtls_ssl_conf_extended_master_secret( mbedtls_ssl_config *conf, char ems ) { +#if !defined(MBEDTLS_SSL_CONF_EXTENDED_MASTER_SECRET) conf->extended_ms = ems; +#else + ((void) conf); + ((void) ems); +#endif /* !MBEDTLS_SSL_CONF_EXTENDED_MASTER_SECRET */ } void mbedtls_ssl_conf_extended_master_secret_enforce( mbedtls_ssl_config *conf, char ems_enf ) { +#if !defined(MBEDTLS_SSL_CONF_ENFORCE_EXTENDED_MASTER_SECRET) conf->enforce_extended_master_secret = ems_enf; +#else + ((void) conf); + ((void) ems_enf); +#endif /* !MBEDTLS_SSL_CONF_ENFORCE_EXTENDED_MASTER_SECRET */ } -#endif +#endif /* !MBEDTLS_SSL_EXTENDED_MASTER_SECRET */ #if defined(MBEDTLS_ARC4_C) void mbedtls_ssl_conf_arc4_support( mbedtls_ssl_config *conf, char arc4 ) @@ -10716,9 +10726,13 @@ int mbedtls_ssl_config_defaults( mbedtls_ssl_config *conf, #endif #if defined(MBEDTLS_SSL_EXTENDED_MASTER_SECRET) +#if !defined(MBEDTLS_SSL_CONF_EXTENDED_MASTER_SECRET) conf->extended_ms = MBEDTLS_SSL_EXTENDED_MS_ENABLED; +#endif /* !MBEDTLS_SSL_CONF_EXTENDED_MASTER_SECRET */ +#if !defined(MBEDTLS_SSL_CONF_ENFORCE_EXTENDED_MASTER_SECRET) conf->enforce_extended_master_secret = MBEDTLS_SSL_EXTENDED_MS_ENFORCE_DISABLED; +#endif /* !MBEDTLS_SSL_CONF_ENFORCE_EXTENDED_MASTER_SECRET */ #endif #if defined(MBEDTLS_SSL_CBC_RECORD_SPLITTING) diff --git a/programs/ssl/query_config.c b/programs/ssl/query_config.c index ab3c772ab..dd5370959 100644 --- a/programs/ssl/query_config.c +++ b/programs/ssl/query_config.c @@ -2098,6 +2098,22 @@ int query_config( const char *config ) } #endif /* MBEDTLS_XTEA_C */ +#if defined(MBEDTLS_SSL_CONF_EXTENDED_MASTER_SECRET) + if( strcmp( "MBEDTLS_SSL_CONF_EXTENDED_MASTER_SECRET", config ) == 0 ) + { + MACRO_EXPANSION_TO_STR( MBEDTLS_SSL_CONF_EXTENDED_MASTER_SECRET ); + return( 0 ); + } +#endif /* MBEDTLS_SSL_CONF_EXTENDED_MASTER_SECRET */ + +#if defined(MBEDTLS_SSL_CONF_ENFORCE_EXTENDED_MASTER_SECRET) + if( strcmp( "MBEDTLS_SSL_CONF_ENFORCE_EXTENDED_MASTER_SECRET", config ) == 0 ) + { + MACRO_EXPANSION_TO_STR( MBEDTLS_SSL_CONF_ENFORCE_EXTENDED_MASTER_SECRET ); + return( 0 ); + } +#endif /* MBEDTLS_SSL_CONF_ENFORCE_EXTENDED_MASTER_SECRET */ + #if defined(MBEDTLS_MPI_WINDOW_SIZE) if( strcmp( "MBEDTLS_MPI_WINDOW_SIZE", config ) == 0 ) { From 03b64fa6c12140793aaad7a25df906ad70182bda Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 11 Jun 2019 14:39:38 +0100 Subject: [PATCH 02/11] Rearrange ExtendedMasterSecret parsing logic `mbedtls_ssl_handshake_params::extended_ms` holds the state of the ExtendedMasterSecret extension in the current handshake. Initially set to 'disabled' for both client and server, - the client sets it to 'enabled' as soon as it finds the ExtendedMS extension in the `ServerHello` and it has advertised that extension in its ClientHello, - the server sets it to 'enabled' as soon as it finds the ExtendedMS extension in the `ClientHello` and is willing to advertise is in its `ServerHello`. This commit slightly restructures this logic in prepraration for the removal of `mbedtls_ssl_handshake_params::extended_ms` in case both the use and the enforcement of the ExtendedMasterSecret extension have been fixed at compile-time. Namely, in this case there is no need for the `extended_ms` field in the handshake structure, as the ExtendedMS must be in use if the handshake progresses beyond the Hello stage. Paving the way for the removal of mbedtls_ssl_handshake_params::extended_ms this commit introduces a temporary variable tracking the presence of the ExtendedMS extension in the ClientHello/ServerHello messages, leaving the derivation of `extended_ms` (and potential failure) to the end of the parsing routine. --- library/ssl_cli.c | 24 +++++++++++++++--------- library/ssl_srv.c | 30 ++++++++++++++++-------------- 2 files changed, 31 insertions(+), 23 deletions(-) diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 238eeb14d..257a517f4 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -1341,9 +1341,6 @@ static int ssl_parse_extended_ms_ext( mbedtls_ssl_context *ssl, } ((void) buf); - - ssl->handshake->extended_ms = MBEDTLS_SSL_EXTENDED_MS_ENABLED; - return( 0 ); } #endif /* MBEDTLS_SSL_EXTENDED_MASTER_SECRET */ @@ -1603,6 +1600,9 @@ static int ssl_parse_server_hello( mbedtls_ssl_context *ssl ) #endif #if defined(MBEDTLS_SSL_RENEGOTIATION) int renegotiation_info_seen = 0; +#endif +#if defined(MBEDTLS_SSL_EXTENDED_MASTER_SECRET) + int extended_ms_seen = 0; #endif int handshake_failure = 0; const mbedtls_ssl_ciphersuite_t *suite_info; @@ -1984,6 +1984,7 @@ static int ssl_parse_server_hello( mbedtls_ssl_context *ssl ) { return( ret ); } + extended_ms_seen = 1; break; #endif /* MBEDTLS_SSL_EXTENDED_MASTER_SECRET */ @@ -2092,14 +2093,19 @@ static int ssl_parse_server_hello( mbedtls_ssl_context *ssl ) */ #if defined(MBEDTLS_SSL_EXTENDED_MASTER_SECRET) if( mbedtls_ssl_conf_get_ems( ssl->conf ) == - MBEDTLS_SSL_EXTENDED_MS_ENABLED && - mbedtls_ssl_conf_get_ems_enforced( ssl->conf ) == - MBEDTLS_SSL_EXTENDED_MS_ENFORCE_ENABLED && - ssl->handshake->extended_ms == MBEDTLS_SSL_EXTENDED_MS_DISABLED) + MBEDTLS_SSL_EXTENDED_MS_ENABLED ) { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "Peer not offering extended master " + if( extended_ms_seen ) + { + ssl->handshake->extended_ms = MBEDTLS_SSL_EXTENDED_MS_ENABLED; + } + else if( mbedtls_ssl_conf_get_ems_enforced( ssl->conf ) == + MBEDTLS_SSL_EXTENDED_MS_ENFORCE_ENABLED ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "Peer not offering extended master " "secret, while it is enforced") ); - handshake_failure = 1; + handshake_failure = 1; + } } #endif /* MBEDTLS_SSL_EXTENDED_MASTER_SECRET */ diff --git a/library/ssl_srv.c b/library/ssl_srv.c index 657fbaece..b01291841 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -567,13 +567,6 @@ static int ssl_parse_extended_ms_ext( mbedtls_ssl_context *ssl, ((void) buf); - if( mbedtls_ssl_conf_get_ems( ssl->conf ) == - MBEDTLS_SSL_EXTENDED_MS_ENABLED && - ssl->minor_ver != MBEDTLS_SSL_MINOR_VERSION_0 ) - { - ssl->handshake->extended_ms = MBEDTLS_SSL_EXTENDED_MS_ENABLED; - } - return( 0 ); } #endif /* MBEDTLS_SSL_EXTENDED_MASTER_SECRET */ @@ -1266,6 +1259,9 @@ static int ssl_parse_client_hello( mbedtls_ssl_context *ssl ) unsigned char *buf, *p, *ext; #if defined(MBEDTLS_SSL_RENEGOTIATION) int renegotiation_info_seen = 0; +#endif +#if defined(MBEDTLS_SSL_EXTENDED_MASTER_SECRET) + int extended_ms_seen = 0; #endif int handshake_failure = 0; const int *ciphersuites; @@ -1894,6 +1890,7 @@ read_record_header: ret = ssl_parse_extended_ms_ext( ssl, ext + 4, ext_size ); if( ret != 0 ) return( ret ); + extended_ms_seen = 1; break; #endif /* MBEDTLS_SSL_EXTENDED_MASTER_SECRET */ @@ -2041,14 +2038,19 @@ read_record_header: */ #if defined(MBEDTLS_SSL_EXTENDED_MASTER_SECRET) if( mbedtls_ssl_conf_get_ems( ssl->conf ) == - MBEDTLS_SSL_EXTENDED_MS_ENABLED && - mbedtls_ssl_conf_get_ems_enforced( ssl->conf ) == - MBEDTLS_SSL_EXTENDED_MS_ENFORCE_ENABLED && - ssl->handshake->extended_ms == MBEDTLS_SSL_EXTENDED_MS_DISABLED) + MBEDTLS_SSL_EXTENDED_MS_ENABLED ) { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "Peer not offering extended master " - "secret, while it is enforced") ); - handshake_failure = 1; + if( extended_ms_seen ) + { + ssl->handshake->extended_ms = MBEDTLS_SSL_EXTENDED_MS_ENABLED; + } + else if( mbedtls_ssl_conf_get_ems_enforced( ssl->conf ) == + MBEDTLS_SSL_EXTENDED_MS_ENFORCE_ENABLED ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "Peer not offering extended master " + "secret, while it is enforced") ); + handshake_failure = 1; + } } #endif /* MBEDTLS_SSL_EXTENDED_MASTER_SECRET */ From 3010d55a3b7f656dcbbc6f55178a75a92cfedab0 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 11 Jun 2019 14:46:16 +0100 Subject: [PATCH 03/11] Introduce helper macro indicating if use of ExtendedMS is enforced --- include/mbedtls/ssl_internal.h | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/include/mbedtls/ssl_internal.h b/include/mbedtls/ssl_internal.h index 3eb37b8c3..e188c9706 100644 --- a/include/mbedtls/ssl_internal.h +++ b/include/mbedtls/ssl_internal.h @@ -321,6 +321,18 @@ #define MBEDTLS_SSL_TRANSPORT_ELSE /* empty: no other branch */ #endif /* TLS and/or DTLS */ +/* Check if the use of the ExtendedMasterSecret extension + * is enforced at compile-time. If so, we don't need to + * track its status in the handshake parameters. */ +#if defined(MBEDTLS_SSL_CONF_EXTENDED_MASTER_SECRET) && \ + defined(MBEDTLS_SSL_CONF_ENFORCE_EXTENDED_MASTER_SECRET) && \ + MBEDTLS_SSL_CONF_EXTENDED_MASTER_SECRET == \ + MBEDTLS_SSL_EXTENDED_MS_ENABLED && \ + MBEDTLS_SSL_CONF_ENFORCE_EXTENDED_MASTER_SECRET == \ + MBEDTLS_SSL_EXTENDED_MS_ENFORCE_ENABLED +#define MBEDTLS_SSL_EXTENDED_MS_ENFORCED +#endif + #ifdef __cplusplus extern "C" { #endif From a49ec56f5199ddd895a8ddf7a646049a9aa1b7c8 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 11 Jun 2019 14:47:55 +0100 Subject: [PATCH 04/11] Introduce getter function for `extended_ms` field in HS struct --- include/mbedtls/ssl_internal.h | 18 ++++++++++++++++++ library/ssl_srv.c | 3 ++- library/ssl_tls.c | 3 ++- 3 files changed, 22 insertions(+), 2 deletions(-) diff --git a/include/mbedtls/ssl_internal.h b/include/mbedtls/ssl_internal.h index e188c9706..35b3a9001 100644 --- a/include/mbedtls/ssl_internal.h +++ b/include/mbedtls/ssl_internal.h @@ -535,6 +535,24 @@ struct mbedtls_ssl_handshake_params #endif /* MBEDTLS_SSL_ASYNC_PRIVATE */ }; +/* + * Getter functions for fields in mbedtls_ssl_handshake_params which + * may be statically implied by the configuration and hence be omitted + * from the structure. + */ +#if defined(MBEDTLS_SSL_EXTENDED_MASTER_SECRET) +static inline int mbedtls_ssl_hs_get_extended_ms( + mbedtls_ssl_handshake_params const *params ) +{ +#if !defined(MBEDTLS_SSL_EXTENDED_MS_ENFORCED) + return( params->extended_ms ); +#else + ((void) params); + return( MBEDTLS_SSL_EXTENDED_MS_ENABLED ); +#endif /* MBEDTLS_SSL_EXTENDED_MS_ENFORCED */ +} +#endif /* MBEDTLS_SSL_EXTENDED_MASTER_SECRET */ + typedef struct mbedtls_ssl_hs_buffer mbedtls_ssl_hs_buffer; /* diff --git a/library/ssl_srv.c b/library/ssl_srv.c index b01291841..023e0a86d 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -2270,7 +2270,8 @@ static void ssl_write_extended_ms_ext( mbedtls_ssl_context *ssl, { unsigned char *p = buf; - if( ssl->handshake->extended_ms == MBEDTLS_SSL_EXTENDED_MS_DISABLED || + if( mbedtls_ssl_hs_get_extended_ms( ssl->handshake ) + == MBEDTLS_SSL_EXTENDED_MS_DISABLED || ssl->minor_ver == MBEDTLS_SSL_MINOR_VERSION_0 ) { *olen = 0; diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 5c8a08eb7..0864fc247 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -1273,7 +1273,8 @@ static int ssl_compute_master( mbedtls_ssl_handshake_params *handshake, handshake->pmslen ); #if defined(MBEDTLS_SSL_EXTENDED_MASTER_SECRET) - if( handshake->extended_ms == MBEDTLS_SSL_EXTENDED_MS_ENABLED ) + if( mbedtls_ssl_hs_get_extended_ms( handshake ) + == MBEDTLS_SSL_EXTENDED_MS_ENABLED ) { unsigned char session_hash[48]; size_t hash_len; From 1ab322bb514b5a4dbc1a33293e5bbde503665065 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 11 Jun 2019 14:50:54 +0100 Subject: [PATCH 05/11] Remove extended_ms field from HS param if ExtendedMS enforced --- include/mbedtls/ssl_internal.h | 3 ++- library/ssl_cli.c | 2 ++ library/ssl_srv.c | 2 ++ 3 files changed, 6 insertions(+), 1 deletion(-) diff --git a/include/mbedtls/ssl_internal.h b/include/mbedtls/ssl_internal.h index 35b3a9001..c9253bf9d 100644 --- a/include/mbedtls/ssl_internal.h +++ b/include/mbedtls/ssl_internal.h @@ -517,7 +517,8 @@ struct mbedtls_ssl_handshake_params #if defined(MBEDTLS_SSL_SESSION_TICKETS) int new_session_ticket; /*!< use NewSessionTicket? */ #endif /* MBEDTLS_SSL_SESSION_TICKETS */ -#if defined(MBEDTLS_SSL_EXTENDED_MASTER_SECRET) +#if defined(MBEDTLS_SSL_EXTENDED_MASTER_SECRET) && \ + !defined(MBEDTLS_SSL_EXTENDED_MS_ENFORCED) int extended_ms; /*!< use Extended Master Secret? */ #endif diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 257a517f4..17611d6fc 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -2097,7 +2097,9 @@ static int ssl_parse_server_hello( mbedtls_ssl_context *ssl ) { if( extended_ms_seen ) { +#if !defined(MBEDTLS_SSL_EXTENDED_MS_ENFORCED) ssl->handshake->extended_ms = MBEDTLS_SSL_EXTENDED_MS_ENABLED; +#endif /* !MBEDTLS_SSL_EXTENDED_MS_ENFORCED */ } else if( mbedtls_ssl_conf_get_ems_enforced( ssl->conf ) == MBEDTLS_SSL_EXTENDED_MS_ENFORCE_ENABLED ) diff --git a/library/ssl_srv.c b/library/ssl_srv.c index 023e0a86d..ecde1b0b5 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -2042,7 +2042,9 @@ read_record_header: { if( extended_ms_seen ) { +#if !defined(MBEDTLS_SSL_EXTENDED_MS_ENFORCED) ssl->handshake->extended_ms = MBEDTLS_SSL_EXTENDED_MS_ENABLED; +#endif /* !MBEDTLS_SSL_EXTENDED_MS_ENFORCED */ } else if( mbedtls_ssl_conf_get_ems_enforced( ssl->conf ) == MBEDTLS_SSL_EXTENDED_MS_ENFORCE_ENABLED ) From 4c4a2e1a0bf537c336324599a2276682dc964fb1 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Wed, 12 Jun 2019 12:45:12 +0100 Subject: [PATCH 06/11] Don't break func'def after linkage type, fixing check-names.sh --- include/mbedtls/ssl.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 562fdac92..16241029c 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -1115,8 +1115,8 @@ static inline unsigned int mbedtls_ssl_conf_get_ems( #endif /* MBEDTLS_SSL_CONF_EXTENDED_MASTER_SECRET */ } -static inline unsigned int -mbedtls_ssl_conf_get_ems_enforced( mbedtls_ssl_config const *conf ) +static inline unsigned int mbedtls_ssl_conf_get_ems_enforced( + mbedtls_ssl_config const *conf ) { #if !defined(MBEDTLS_SSL_CONF_ENFORCE_EXTENDED_MASTER_SECRET) return( conf->enforce_extended_master_secret ); From 57e72c750c092900f3fc035e7c9f77af62c684e5 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Wed, 12 Jun 2019 12:46:31 +0100 Subject: [PATCH 07/11] Move getter functions for SSL configuration to ssl_internal.h --- include/mbedtls/ssl.h | 29 ----------------------------- include/mbedtls/ssl_internal.h | 30 ++++++++++++++++++++++++++++++ 2 files changed, 30 insertions(+), 29 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 16241029c..d3ba9d136 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -1098,35 +1098,6 @@ struct mbedtls_ssl_config #endif /* MBEDTLS_SSL_DTLS_CONNECTION_ID */ }; -/* - * Getter functions for fields in mbedtls_ssl_config which may - * be fixed at compile time via one of MBEDTLS_SSL_SSL_CONF_XXX. - */ - -#if defined(MBEDTLS_SSL_EXTENDED_MASTER_SECRET) -static inline unsigned int mbedtls_ssl_conf_get_ems( - mbedtls_ssl_config const *conf ) -{ -#if !defined(MBEDTLS_SSL_CONF_EXTENDED_MASTER_SECRET) - return( conf->extended_ms ); -#else - ((void) conf); - return( MBEDTLS_SSL_CONF_EXTENDED_MASTER_SECRET ); -#endif /* MBEDTLS_SSL_CONF_EXTENDED_MASTER_SECRET */ -} - -static inline unsigned int mbedtls_ssl_conf_get_ems_enforced( - mbedtls_ssl_config const *conf ) -{ -#if !defined(MBEDTLS_SSL_CONF_ENFORCE_EXTENDED_MASTER_SECRET) - return( conf->enforce_extended_master_secret ); -#else - ((void) conf); - return( MBEDTLS_SSL_CONF_ENFORCE_EXTENDED_MASTER_SECRET ); -#endif /* MBEDTLS_SSL_CONF_ENFORCE_EXTENDED_MASTER_SECRET */ -} -#endif /* MBEDTLS_SSL_EXTENDED_MASTER_SECRET */ - struct mbedtls_ssl_context { const mbedtls_ssl_config *conf; /*!< configuration information */ diff --git a/include/mbedtls/ssl_internal.h b/include/mbedtls/ssl_internal.h index c9253bf9d..7009c4f8b 100644 --- a/include/mbedtls/ssl_internal.h +++ b/include/mbedtls/ssl_internal.h @@ -1079,4 +1079,34 @@ int mbedtls_ssl_decrypt_buf( mbedtls_ssl_context *ssl, mbedtls_ssl_transform *transform, mbedtls_record *rec ); + +/* + * Getter functions for fields in mbedtls_ssl_config which may + * be fixed at compile time via one of MBEDTLS_SSL_SSL_CONF_XXX. + */ + +#if defined(MBEDTLS_SSL_EXTENDED_MASTER_SECRET) +static inline unsigned int mbedtls_ssl_conf_get_ems( + mbedtls_ssl_config const *conf ) +{ +#if !defined(MBEDTLS_SSL_CONF_EXTENDED_MASTER_SECRET) + return( conf->extended_ms ); +#else + ((void) conf); + return( MBEDTLS_SSL_CONF_EXTENDED_MASTER_SECRET ); +#endif /* MBEDTLS_SSL_CONF_EXTENDED_MASTER_SECRET */ +} + +static inline unsigned int mbedtls_ssl_conf_get_ems_enforced( + mbedtls_ssl_config const *conf ) +{ +#if !defined(MBEDTLS_SSL_CONF_ENFORCE_EXTENDED_MASTER_SECRET) + return( conf->enforce_extended_master_secret ); +#else + ((void) conf); + return( MBEDTLS_SSL_CONF_ENFORCE_EXTENDED_MASTER_SECRET ); +#endif /* MBEDTLS_SSL_CONF_ENFORCE_EXTENDED_MASTER_SECRET */ +} +#endif /* MBEDTLS_SSL_EXTENDED_MASTER_SECRET */ + #endif /* ssl_internal.h */ From f765ce617f8d46cb2b78810fede24286cca0eaf2 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Fri, 21 Jun 2019 13:17:14 +0100 Subject: [PATCH 08/11] Remove ExtendedMS configuration API if hardcoded at compile-time If the ExtendedMasterSecret extension is configured at compile-time by setting MBEDTLS_SSL_CONF_EXTENDED_MASTER_SECRET and/or MBEDTLS_SSL_CONF_ENFORCE_EXTENDED_MASTER_SECRET, the runtime configuration APIs mbedtls_ssl_conf_extended_master_secret() and mbedtls_ssl_conf_extended_master_secret_enforce() must either be removed or modified to take no effect (or at most check that the runtime value matches the hardcoded one, but that would undermine the code-size benefits the hardcoding is supposed to bring in the first place). Previously, the API was kept but modified to have no effect. While convenient for us because we don't have to adapt example applications, this comes at the danger of users calling the runtime configuration API, forgetting that the respective fields are potentially already hardcoded at compile-time - and hence silently using a configuration they don't intend to use. This commit changes the approach to removing the configuration API in case the respective field is hardcoded at compile-time, and exemplifies it in the only case implemented so far, namely the configuration of the ExtendedMasterSecret extension. It adapts ssl_client2 and ssl_server2 by omitting the call to the corresponding API if MBEDTLS_SSL_CONF_XXX are defined and removing the command line parameters for the runtime configuration of the ExtendedMasterSecret extension. --- include/mbedtls/check_config.h | 7 +++++++ include/mbedtls/config.h | 2 +- library/ssl_tls.c | 15 ++++----------- programs/ssl/ssl_client2.c | 8 ++++++-- programs/ssl/ssl_server2.c | 8 ++++++-- 5 files changed, 24 insertions(+), 16 deletions(-) diff --git a/include/mbedtls/check_config.h b/include/mbedtls/check_config.h index 67cb77856..88f47011b 100644 --- a/include/mbedtls/check_config.h +++ b/include/mbedtls/check_config.h @@ -650,6 +650,13 @@ #error "MBEDTLS_SSL_EXTENDED_MASTER_SECRET defined, but not all prerequsites" #endif +#if ( defined(MBEDTLS_SSL_CONF_EXTENDED_MASTER_SECRET) && \ + !defined(MBEDTLS_SSL_CONF_ENFORCE_EXTENDED_MASTER_SECRET) ) || \ + ( !defined(MBEDTLS_SSL_CONF_EXTENDED_MASTER_SECRET) && \ + defined(MBEDTLS_SSL_CONF_ENFORCE_EXTENDED_MASTER_SECRET) ) +#define "MBEDTLS_SSL_CONF_EXTENDED_MASTER_SECRET and MBEDTLS_SSL_CONF_ENFORCE_EXTENDED_MASTER_SECRET must be defined together." +#endif + #if defined(MBEDTLS_SSL_TICKET_C) && !defined(MBEDTLS_CIPHER_C) #error "MBEDTLS_SSL_TICKET_C defined, but not all prerequisites" #endif diff --git a/include/mbedtls/config.h b/include/mbedtls/config.h index 8dcb81c5f..2116521dc 100644 --- a/include/mbedtls/config.h +++ b/include/mbedtls/config.h @@ -3444,7 +3444,7 @@ * This section allows to fix parts of the SSL configuration * at compile-time. If a field is fixed at compile-time, the * corresponding SSL configuration API `mbedtls_ssl_conf_xxx()` - * remains present, but takes no effect anymore. + * is removed. * * This can be used on constrained systems to reduce code-size. * \{ diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 0864fc247..fff20ff1b 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -8610,26 +8610,19 @@ void mbedtls_ssl_conf_encrypt_then_mac( mbedtls_ssl_config *conf, char etm ) #endif #if defined(MBEDTLS_SSL_EXTENDED_MASTER_SECRET) +#if !defined(MBEDTLS_SSL_CONF_EXTENDED_MASTER_SECRET) void mbedtls_ssl_conf_extended_master_secret( mbedtls_ssl_config *conf, char ems ) { -#if !defined(MBEDTLS_SSL_CONF_EXTENDED_MASTER_SECRET) conf->extended_ms = ems; -#else - ((void) conf); - ((void) ems); -#endif /* !MBEDTLS_SSL_CONF_EXTENDED_MASTER_SECRET */ } - +#endif /* !MBEDTLS_SSL_CONF_EXTENDED_MASTER_SECRET */ +#if !defined(MBEDTLS_SSL_CONF_ENFORCE_EXTENDED_MASTER_SECRET) void mbedtls_ssl_conf_extended_master_secret_enforce( mbedtls_ssl_config *conf, char ems_enf ) { -#if !defined(MBEDTLS_SSL_CONF_ENFORCE_EXTENDED_MASTER_SECRET) conf->enforce_extended_master_secret = ems_enf; -#else - ((void) conf); - ((void) ems_enf); -#endif /* !MBEDTLS_SSL_CONF_ENFORCE_EXTENDED_MASTER_SECRET */ } +#endif /* !MBEDTLS_SSL_CONF_ENFORCE_EXTENDED_MASTER_SECRET */ #endif /* !MBEDTLS_SSL_EXTENDED_MASTER_SECRET */ #if defined(MBEDTLS_ARC4_C) diff --git a/programs/ssl/ssl_client2.c b/programs/ssl/ssl_client2.c index d859101c1..982857659 100644 --- a/programs/ssl/ssl_client2.c +++ b/programs/ssl/ssl_client2.c @@ -245,7 +245,9 @@ int main( void ) #define USAGE_FALLBACK "" #endif -#if defined(MBEDTLS_SSL_EXTENDED_MASTER_SECRET) +#if defined(MBEDTLS_SSL_EXTENDED_MASTER_SECRET) && \ + !defined(MBEDTLS_SSL_CONF_EXTENDED_MASTER_SECRET) && \ + !defined(MBEDTLS_SSL_CONF_ENFORCE_EXTENDED_MASTER_SECRET) #define USAGE_EMS \ " extended_ms=0/1 default: (library default: on)\n" \ " enforce_extended_master_secret=0/1 default: (library default: off)\n" @@ -1706,7 +1708,9 @@ int main( int argc, char *argv[] ) mbedtls_ssl_conf_truncated_hmac( &conf, opt.trunc_hmac ); #endif -#if defined(MBEDTLS_SSL_EXTENDED_MASTER_SECRET) +#if defined(MBEDTLS_SSL_EXTENDED_MASTER_SECRET) && \ + !defined(MBEDTLS_SSL_CONF_EXTENDED_MASTER_SECRET) && \ + !defined(MBEDTLS_SSL_CONF_ENFORCE_EXTENDED_MASTER_SECRET) if( opt.extended_ms != DFL_EXTENDED_MS ) mbedtls_ssl_conf_extended_master_secret( &conf, opt.extended_ms ); if( opt.enforce_extended_master_secret != DFL_EXTENDED_MS_ENFORCE ) diff --git a/programs/ssl/ssl_server2.c b/programs/ssl/ssl_server2.c index 8a12de23d..5d751b6a7 100644 --- a/programs/ssl/ssl_server2.c +++ b/programs/ssl/ssl_server2.c @@ -344,7 +344,9 @@ int main( void ) #define USAGE_DTLS "" #endif -#if defined(MBEDTLS_SSL_EXTENDED_MASTER_SECRET) +#if defined(MBEDTLS_SSL_EXTENDED_MASTER_SECRET) && \ + !defined(MBEDTLS_SSL_CONF_EXTENDED_MASTER_SECRET) && \ + !defined(MBEDTLS_SSL_CONF_ENFORCE_EXTENDED_MASTER_SECRET) #define USAGE_EMS \ " extended_ms=0/1 default: (library default: on)\n" \ " enforce_extended_master_secret=0/1 default: (library default: off)\n" @@ -2491,7 +2493,9 @@ int main( int argc, char *argv[] ) mbedtls_ssl_conf_truncated_hmac( &conf, opt.trunc_hmac ); #endif -#if defined(MBEDTLS_SSL_EXTENDED_MASTER_SECRET) +#if defined(MBEDTLS_SSL_EXTENDED_MASTER_SECRET) && \ + !defined(MBEDTLS_SSL_CONF_EXTENDED_MASTER_SECRET) && \ + !defined(MBEDTLS_SSL_CONF_ENFORCE_EXTENDED_MASTER_SECRET) if( opt.extended_ms != DFL_EXTENDED_MS ) mbedtls_ssl_conf_extended_master_secret( &conf, opt.extended_ms ); if( opt.enforce_extended_master_secret != DFL_EXTENDED_MS_ENFORCE ) From ab1ce766824d6393a38041c8d1cf6480a077d1c8 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Wed, 12 Jun 2019 13:35:03 +0100 Subject: [PATCH 09/11] Mention possibility of hardcoding SSL config in ssl.h --- include/mbedtls/ssl.h | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index d3ba9d136..b51708970 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -2845,6 +2845,7 @@ void mbedtls_ssl_conf_encrypt_then_mac( mbedtls_ssl_config *conf, char etm ); #endif /* MBEDTLS_SSL_ENCRYPT_THEN_MAC */ #if defined(MBEDTLS_SSL_EXTENDED_MASTER_SECRET) +#if !defined(MBEDTLS_SSL_CONF_EXTENDED_MASTER_SECRET) /** * \brief Enable or disable Extended Master Secret negotiation. * (Default: MBEDTLS_SSL_EXTENDED_MS_ENABLED) @@ -2853,11 +2854,20 @@ void mbedtls_ssl_conf_encrypt_then_mac( mbedtls_ssl_config *conf, char etm ); * protocol, and should not cause any interoperability issue * (used only if the peer supports it too). * + * \note On constrained systems, this option can also be + * fixed at compile-time by defining the constant + * MBEDTLS_SSL_CONF_ENFORCE_EXTENDED_MASTER_SECRET + * as MBEDTLS_SSL_EXTENDED_MS_ENABLED or + * MBEDTLS_SSL_EXTENDED_MS_DISABLED. + * * \param conf SSL configuration - * \param ems MBEDTLS_SSL_EXTENDED_MS_ENABLED or MBEDTLS_SSL_EXTENDED_MS_DISABLED + * \param ems MBEDTLS_SSL_EXTENDED_MS_ENABLED or + * MBEDTLS_SSL_EXTENDED_MS_DISABLED */ void mbedtls_ssl_conf_extended_master_secret( mbedtls_ssl_config *conf, char ems ); +#endif /* !MBEDTLS_SSL_CONF_EXTENDED_MASTER_SECRET */ +#if !defined(MBEDTLS_SSL_CONF_ENFORCE_EXTENDED_MASTER_SECRET) /** * \brief Enable or disable Extended Master Secret enforcing. * (Default: MBEDTLS_SSL_EXTENDED_MS_ENFORCE_DISABLED) @@ -2874,9 +2884,17 @@ void mbedtls_ssl_conf_extended_master_secret( mbedtls_ssl_config *conf, char ems * \param conf Currently used SSL configuration struct. * \param ems_enf MBEDTLS_SSL_EXTENDED_MS_ENFORCE_ENABLED or * MBEDTLS_SSL_EXTENDED_MS_ENFORCE_DISABLED + + * \note On constrained systems, this option can also be + * fixed at compile-time by defining the constant + * MBEDTLS_SSL_CONF_ENFORCE_EXTENDED_MASTER_SECRET + * as MBEDTLS_SSL_EXTENDED_MS_ENFORCE_ENABLED or + * MBEDTLS_SSL_EXTENDED_MS_ENFORCE_DISABLED. + * */ void mbedtls_ssl_conf_extended_master_secret_enforce( mbedtls_ssl_config *conf, char ems_enf ); +#endif /* !MBEDTLS_SSL_CONF_ENFORCE_EXTENDED_MASTER_SECRET */ #endif /* MBEDTLS_SSL_EXTENDED_MASTER_SECRET */ #if defined(MBEDTLS_ARC4_C) From aa9fc6dd33a77224a6ee1a39baa60b4ed2fee1c6 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Thu, 13 Jun 2019 11:02:44 +0100 Subject: [PATCH 10/11] Update query_config.c --- programs/ssl/query_config.c | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/programs/ssl/query_config.c b/programs/ssl/query_config.c index dd5370959..d45a6634f 100644 --- a/programs/ssl/query_config.c +++ b/programs/ssl/query_config.c @@ -2098,22 +2098,6 @@ int query_config( const char *config ) } #endif /* MBEDTLS_XTEA_C */ -#if defined(MBEDTLS_SSL_CONF_EXTENDED_MASTER_SECRET) - if( strcmp( "MBEDTLS_SSL_CONF_EXTENDED_MASTER_SECRET", config ) == 0 ) - { - MACRO_EXPANSION_TO_STR( MBEDTLS_SSL_CONF_EXTENDED_MASTER_SECRET ); - return( 0 ); - } -#endif /* MBEDTLS_SSL_CONF_EXTENDED_MASTER_SECRET */ - -#if defined(MBEDTLS_SSL_CONF_ENFORCE_EXTENDED_MASTER_SECRET) - if( strcmp( "MBEDTLS_SSL_CONF_ENFORCE_EXTENDED_MASTER_SECRET", config ) == 0 ) - { - MACRO_EXPANSION_TO_STR( MBEDTLS_SSL_CONF_ENFORCE_EXTENDED_MASTER_SECRET ); - return( 0 ); - } -#endif /* MBEDTLS_SSL_CONF_ENFORCE_EXTENDED_MASTER_SECRET */ - #if defined(MBEDTLS_MPI_WINDOW_SIZE) if( strcmp( "MBEDTLS_MPI_WINDOW_SIZE", config ) == 0 ) { @@ -2594,6 +2578,22 @@ int query_config( const char *config ) } #endif /* MBEDTLS_PLATFORM_GMTIME_R_ALT */ +#if defined(MBEDTLS_SSL_CONF_EXTENDED_MASTER_SECRET) + if( strcmp( "MBEDTLS_SSL_CONF_EXTENDED_MASTER_SECRET", config ) == 0 ) + { + MACRO_EXPANSION_TO_STR( MBEDTLS_SSL_CONF_EXTENDED_MASTER_SECRET ); + return( 0 ); + } +#endif /* MBEDTLS_SSL_CONF_EXTENDED_MASTER_SECRET */ + +#if defined(MBEDTLS_SSL_CONF_ENFORCE_EXTENDED_MASTER_SECRET) + if( strcmp( "MBEDTLS_SSL_CONF_ENFORCE_EXTENDED_MASTER_SECRET", config ) == 0 ) + { + MACRO_EXPANSION_TO_STR( MBEDTLS_SSL_CONF_ENFORCE_EXTENDED_MASTER_SECRET ); + return( 0 ); + } +#endif /* MBEDTLS_SSL_CONF_ENFORCE_EXTENDED_MASTER_SECRET */ + /* If the symbol is not found, return an error */ return( 1 ); } From af5ab918d92313c938a5628aa6e24fd3a288353c Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Fri, 21 Jun 2019 12:59:46 +0100 Subject: [PATCH 11/11] Detect mismatching compile-time and cmd line config in ssl-opt.sh --- tests/ssl-opt.sh | 97 ++++++++++++++++++++++++++++++++++-------------- 1 file changed, 69 insertions(+), 28 deletions(-) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 3dd69a5f2..7bcba2438 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -503,6 +503,49 @@ detect_dtls() { fi } +# Strip off a particular parameter from the command line +# and return its value. +# Parameter 1: Command line parameter to strip off +# ENV I/O: CMD command line to search and modify +extract_cmdline_argument() { + __ARG=$(echo "$CMD" | sed -n "s/^.* $1=\([^ ]*\).*$/\1/p") + CMD=$(echo "$CMD" | sed "s/$1=\([^ ]*\)//") +} + +# Check compatibility of the ssl_client2/ssl_server2 command-line +# with a particular compile-time configurable option. +# Parameter 1: Command-line argument (e.g. extended_ms) +# Parameter 2: Corresponding compile-time configuration +# (e.g. MBEDTLS_SSL_CONF_EXTENDED_MASTER_SECRET) +# ENV I/O: CMD command line to search and modify +# SKIP_NEXT set to "YES" on a mismatch +check_cmdline_param_compat() { + __VAL="$( get_config_value_or_default "$2" )" + if [ ! -z "$__VAL" ]; then + extract_cmdline_argument "$1" + if [ ! -z "$__ARG" ] && [ "$__ARG" != "$__VAL" ]; then + SKIP_NEXT="YES" + 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 +# entry. +# Parameter 1: Command line to inspect +# Output: Modified command line +# ENV I/O: SKIP_TEST set to 1 on mismatch. +check_cmdline_compat() { + CMD="$1" + + # ExtendedMasterSecret configuration + check_cmdline_param_compat "extended_ms" \ + "MBEDTLS_SSL_CONF_EXTENDED_MASTER_SECRET" + check_cmdline_param_compat "enforce_extended_master_secret" \ + "MBEDTLS_SSL_CONF_ENFORCE_EXTENDED_MASTER_SECRET" +} + # Usage: run_test name [-p proxy_cmd] srv_cmd cli_cmd cli_exit [option [...]] # Options: -s pattern pattern that must be present in server output # -c pattern pattern that must be present in client output @@ -531,14 +574,6 @@ run_test() { SKIP_NEXT="YES" fi - # should we skip? - if [ "X$SKIP_NEXT" = "XYES" ]; then - SKIP_NEXT="NO" - echo "SKIP" - SKIPS=$(( $SKIPS + 1 )) - return - fi - # does this test use a proxy? if [ "X$1" = "X-p" ]; then PXY_CMD="$2" @@ -553,6 +588,12 @@ run_test() { CLI_EXPECT="$3" shift 3 + check_cmdline_compat "$SRV_CMD" + SRV_CMD="$CMD" + + check_cmdline_compat "$CLI_CMD" + CLI_CMD="$CMD" + # Check if test uses files TEST_USES_FILES=$(echo "$SRV_CMD $CLI_CMD" | grep "\.\(key\|crt\|pem\)" ) if [ ! -z "$TEST_USES_FILES" ]; then @@ -1836,8 +1877,8 @@ run_test "Encrypt then MAC: client enabled, server SSLv3" \ # Tests for Extended Master Secret extension run_test "Extended Master Secret: default (not enforcing)" \ - "$P_SRV debug_level=3" \ - "$P_CLI debug_level=3" \ + "$P_SRV debug_level=3 extended_ms=1 enforce_extended_master_secret=0 " \ + "$P_CLI debug_level=3 extended_ms=1 enforce_extended_master_secret=0" \ 0 \ -c "client hello, adding extended_master_secret extension" \ -s "found extended master secret extension" \ @@ -1847,8 +1888,8 @@ run_test "Extended Master Secret: default (not enforcing)" \ -s "session hash for extended master secret" run_test "Extended Master Secret: both enabled, both enforcing" \ - "$P_SRV debug_level=3 enforce_extended_master_secret=1" \ - "$P_CLI debug_level=3 enforce_extended_master_secret=1" \ + "$P_SRV debug_level=3 extended_ms=1 enforce_extended_master_secret=1" \ + "$P_CLI debug_level=3 extended_ms=1 enforce_extended_master_secret=1" \ 0 \ -c "client hello, adding extended_master_secret extension" \ -s "found extended master secret extension" \ @@ -1858,8 +1899,8 @@ run_test "Extended Master Secret: both enabled, both enforcing" \ -s "session hash for extended master secret" run_test "Extended Master Secret: both enabled, client enforcing" \ - "$P_SRV debug_level=3 enforce_extended_master_secret=0" \ - "$P_CLI debug_level=3 enforce_extended_master_secret=1" \ + "$P_SRV debug_level=3 extended_ms=1 enforce_extended_master_secret=0" \ + "$P_CLI debug_level=3 extended_ms=1 enforce_extended_master_secret=1" \ 0 \ -c "client hello, adding extended_master_secret extension" \ -s "found extended master secret extension" \ @@ -1869,8 +1910,8 @@ run_test "Extended Master Secret: both enabled, client enforcing" \ -s "session hash for extended master secret" run_test "Extended Master Secret: both enabled, server enforcing" \ - "$P_SRV debug_level=3 enforce_extended_master_secret=1" \ - "$P_CLI debug_level=3 enforce_extended_master_secret=0" \ + "$P_SRV debug_level=3 extended_ms=1 enforce_extended_master_secret=1" \ + "$P_CLI debug_level=3 extended_ms=1 enforce_extended_master_secret=0" \ 0 \ -c "client hello, adding extended_master_secret extension" \ -s "found extended master secret extension" \ @@ -1880,7 +1921,7 @@ run_test "Extended Master Secret: both enabled, server enforcing" \ -s "session hash for extended master secret" run_test "Extended Master Secret: client enabled, server disabled, client enforcing" \ - "$P_SRV debug_level=3 extended_ms=0" \ + "$P_SRV debug_level=3 extended_ms=0 enforce_extended_master_secret=0" \ "$P_CLI debug_level=3 extended_ms=1 enforce_extended_master_secret=1" \ 1 \ -c "client hello, adding extended_master_secret extension" \ @@ -1891,7 +1932,7 @@ run_test "Extended Master Secret: client enabled, server disabled, client enf run_test "Extended Master Secret enforced: client disabled, server enabled, server enforcing" \ "$P_SRV debug_level=3 extended_ms=1 enforce_extended_master_secret=1" \ - "$P_CLI debug_level=3 extended_ms=0" \ + "$P_CLI debug_level=3 extended_ms=0 enforce_extended_master_secret=0" \ 1 \ -C "client hello, adding extended_master_secret extension" \ -S "found extended master secret extension" \ @@ -1900,8 +1941,8 @@ run_test "Extended Master Secret enforced: client disabled, server enabled, s -s "Peer not offering extended master secret, while it is enforced" run_test "Extended Master Secret: client enabled, server disabled, not enforcing" \ - "$P_SRV debug_level=3 extended_ms=0" \ - "$P_CLI debug_level=3 extended_ms=1" \ + "$P_SRV debug_level=3 extended_ms=0 enforce_extended_master_secret=0" \ + "$P_CLI debug_level=3 extended_ms=1 enforce_extended_master_secret=0" \ 0 \ -c "client hello, adding extended_master_secret extension" \ -s "found extended master secret extension" \ @@ -1911,8 +1952,8 @@ run_test "Extended Master Secret: client enabled, server disabled, not enforc -S "session hash for extended master secret" run_test "Extended Master Secret: client disabled, server enabled, not enforcing" \ - "$P_SRV debug_level=3 extended_ms=1" \ - "$P_CLI debug_level=3 extended_ms=0" \ + "$P_SRV debug_level=3 extended_ms=1 enforce_extended_master_secret=0" \ + "$P_CLI debug_level=3 extended_ms=0 enforce_extended_master_secret=0" \ 0 \ -C "client hello, adding extended_master_secret extension" \ -S "found extended master secret extension" \ @@ -1922,8 +1963,8 @@ run_test "Extended Master Secret: client disabled, server enabled, not enforc -S "session hash for extended master secret" run_test "Extended Master Secret: client disabled, server disabled" \ - "$P_SRV debug_level=3 extended_ms=0" \ - "$P_CLI debug_level=3 extended_ms=0" \ + "$P_SRV debug_level=3 extended_ms=0 enforce_extended_master_secret=0" \ + "$P_CLI debug_level=3 extended_ms=0 enforce_extended_master_secret=0" \ 0 \ -C "client hello, adding extended_master_secret extension" \ -S "found extended master secret extension" \ @@ -1934,8 +1975,8 @@ run_test "Extended Master Secret: client disabled, server disabled" \ requires_config_enabled MBEDTLS_SSL_PROTO_SSL3 run_test "Extended Master Secret: client SSLv3, server enabled" \ - "$P_SRV debug_level=3 min_version=ssl3" \ - "$P_CLI debug_level=3 force_version=ssl3" \ + "$P_SRV debug_level=3 min_version=ssl3 extended_ms=1 enforce_extended_master_secret=0" \ + "$P_CLI debug_level=3 force_version=ssl3 extended_ms=1 enforce_extended_master_secret=0" \ 0 \ -C "client hello, adding extended_master_secret extension" \ -S "found extended master secret extension" \ @@ -1946,8 +1987,8 @@ run_test "Extended Master Secret: client SSLv3, server enabled" \ requires_config_enabled MBEDTLS_SSL_PROTO_SSL3 run_test "Extended Master Secret: client enabled, server SSLv3" \ - "$P_SRV debug_level=3 force_version=ssl3" \ - "$P_CLI debug_level=3 min_version=ssl3" \ + "$P_SRV debug_level=3 force_version=ssl3 extended_ms=1 enforce_extended_master_secret=0" \ + "$P_CLI debug_level=3 min_version=ssl3 extended_ms=1 enforce_extended_master_secret=0" \ 0 \ -c "client hello, adding extended_master_secret extension" \ -S "found extended master secret extension" \