From ba8b1eb5d90bc866f12d78e5df36dbab3135affe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Mon, 17 Jun 2019 15:21:07 +0200 Subject: [PATCH 1/9] Use negated option for controlling TLS support. A positive option looks better, but comes with the following compatibility issue: people using a custom config.h that is not based on the default config.h and need TLS support would need to manually change their config in order to still get TLS. Work around that by making the public option negative. Internally the positive option is used, though. In the future (when preparing the next major version), we might want to switch back to a positive option as this would be more consistent with other options we have. --- configs/config-ccm-psk-tls1_2.h | 1 - configs/config-mini-tls1_1.h | 1 - configs/config-suite-b.h | 1 - configs/config-thread.h | 1 + include/mbedtls/check_config.h | 2 +- include/mbedtls/config.h | 27 ++++++++++++--------------- include/mbedtls/ssl.h | 4 ++-- include/mbedtls/ssl_internal.h | 6 ++++++ library/version_features.c | 6 +++--- programs/ssl/query_config.c | 8 ++++---- programs/ssl/ssl_client1.c | 4 ++-- programs/ssl/ssl_fork_server.c | 6 +++--- programs/ssl/ssl_mail_client.c | 6 +++--- programs/ssl/ssl_pthread_server.c | 6 +++--- programs/ssl/ssl_server.c | 6 +++--- scripts/config.pl | 2 ++ 16 files changed, 45 insertions(+), 42 deletions(-) diff --git a/configs/config-ccm-psk-tls1_2.h b/configs/config-ccm-psk-tls1_2.h index bd2c1a3b8..c9b58dd53 100644 --- a/configs/config-ccm-psk-tls1_2.h +++ b/configs/config-ccm-psk-tls1_2.h @@ -41,7 +41,6 @@ /* mbed TLS feature support */ #define MBEDTLS_KEY_EXCHANGE_PSK_ENABLED #define MBEDTLS_SSL_PROTO_TLS1_2 -#define MBEDTLS_SSL_PROTO_TLS /* mbed TLS modules */ #define MBEDTLS_AES_C diff --git a/configs/config-mini-tls1_1.h b/configs/config-mini-tls1_1.h index 349ea8e57..013bc0300 100644 --- a/configs/config-mini-tls1_1.h +++ b/configs/config-mini-tls1_1.h @@ -40,7 +40,6 @@ #define MBEDTLS_PKCS1_V15 #define MBEDTLS_KEY_EXCHANGE_RSA_ENABLED #define MBEDTLS_SSL_PROTO_TLS1_1 -#define MBEDTLS_SSL_PROTO_TLS /* mbed TLS modules */ #define MBEDTLS_AES_C diff --git a/configs/config-suite-b.h b/configs/config-suite-b.h index e6fad1c0e..18e2c4036 100644 --- a/configs/config-suite-b.h +++ b/configs/config-suite-b.h @@ -47,7 +47,6 @@ #define MBEDTLS_ECP_DP_SECP384R1_ENABLED #define MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED #define MBEDTLS_SSL_PROTO_TLS1_2 -#define MBEDTLS_SSL_PROTO_TLS /* mbed TLS modules */ #define MBEDTLS_AES_C diff --git a/configs/config-thread.h b/configs/config-thread.h index 3166aa970..4fa0b8d19 100644 --- a/configs/config-thread.h +++ b/configs/config-thread.h @@ -49,6 +49,7 @@ #define MBEDTLS_SSL_MAX_FRAGMENT_LENGTH #define MBEDTLS_SSL_PROTO_TLS1_2 #define MBEDTLS_SSL_PROTO_DTLS +#define MBEDTLS_SSL_PROTO_NO_TLS #define MBEDTLS_SSL_DTLS_ANTI_REPLAY #define MBEDTLS_SSL_DTLS_HELLO_VERIFY #define MBEDTLS_SSL_EXPORT_KEYS diff --git a/include/mbedtls/check_config.h b/include/mbedtls/check_config.h index b3677b528..34f1a3bee 100644 --- a/include/mbedtls/check_config.h +++ b/include/mbedtls/check_config.h @@ -566,7 +566,7 @@ #endif #if defined(MBEDTLS_SSL_TLS_C) && \ - ( !defined(MBEDTLS_SSL_PROTO_TLS) && !defined(MBEDTLS_SSL_PROTO_DTLS) ) + ( defined(MBEDTLS_SSL_PROTO_NO_TLS) && !defined(MBEDTLS_SSL_PROTO_DTLS) ) #error "MBEDTLS_SSL_TLS_C defined, but neither TLS or DTLS is active" #endif diff --git a/include/mbedtls/config.h b/include/mbedtls/config.h index e0b5ba41c..1653f8950 100644 --- a/include/mbedtls/config.h +++ b/include/mbedtls/config.h @@ -1508,7 +1508,7 @@ * Enable this and MBEDTLS_SSL_PROTO_TLS1_2 to enable DTLS 1.2, * and/or this and MBEDTLS_SSL_PROTO_TLS1_1 to enable DTLS 1.0. * - * \see MBEDTLS_SSL_PROTO_TLS + * \see MBEDTLS_SSL_PROTO_NO_TLS * * Requires: MBEDTLS_SSL_PROTO_TLS1_1 * or MBEDTLS_SSL_PROTO_TLS1_2 @@ -1518,25 +1518,22 @@ #define MBEDTLS_SSL_PROTO_DTLS /** - * \def MBEDTLS_SSL_PROTO_TLS + * \def MBEDTLS_SSL_PROTO_NO_TLS * - * Enable support for SSL/TLS (all available versions). + * Disable support for SSL/TLS (all available versions) - this doesn't affect + * support for DTLS which is controlled by #MBEDTLS_SSL_PROTO_DTLS. * - * Enable this and MBEDTLS_SSL_PROTO_TLS1_2 to enable TLS 1.2; - * enable this and MBEDTLS_SSL_PROTO_TLS1_1 to enable TLS 1.1; - * enable this and MBEDTLS_SSL_PROTO_TLS1 to enable TLS 1.0; - * and/or this and MBEDTLS_SSL_PROTO_SSL3 to enable SSL 3.0 (deprecated). + * Disable this and enable MBEDTLS_SSL_PROTO_TLS1_2 to enable TLS 1.2; + * disable this and enable MBEDTLS_SSL_PROTO_TLS1_1 to enable TLS 1.1; + * disable this and enable MBEDTLS_SSL_PROTO_TLS1 to enable TLS 1.0; + * disable this and enable MBEDTLS_SSL_PROTO_SSL3 to enable SSL 3.0. * - * \see MBEDTLS_SSL_PROTO_DTLS + * Requirements: if this macro is disabled, at least one of the above + * TLS versions needs to be enabled. * - * Requires: MBEDTLS_SSL_PROTO_TLS1_2 - * or MBEDTLS_SSL_PROTO_TLS1_1 - * or MBEDTLS_SSL_PROTO_TLS1 - * or MBEDTLS_SSL_PROTO_SSL3 (deprecated) - * - * Comment this macro to disable support for TLS + * Uncomment this macro to disable support for TLS. */ -#define MBEDTLS_SSL_PROTO_TLS +//#define MBEDTLS_SSL_PROTO_NO_TLS /** * \def MBEDTLS_SSL_ALPN diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 653f857cc..1a4eaf663 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -1337,8 +1337,8 @@ void mbedtls_ssl_conf_endpoint( mbedtls_ssl_config *conf, int endpoint ); /** * \brief Set the transport type (TLS or DTLS). - * Default: TLS if #MBEDTLS_SSL_PROTO_TLS is defined, else - * DTLS. + * Default: TLS unless #MBEDTLS_SSL_PROTO_NO_TLS is defined, + * else DTLS. * * \note For DTLS, you must either provide a recv callback that * doesn't block, or one that handles timeouts, see diff --git a/include/mbedtls/ssl_internal.h b/include/mbedtls/ssl_internal.h index 1c8709f3f..e6c829d3a 100644 --- a/include/mbedtls/ssl_internal.h +++ b/include/mbedtls/ssl_internal.h @@ -58,6 +58,12 @@ #define inline __inline #endif +/* The public option is negative for backwards compatibility, + * but internally a poisitive option is more convenient. */ +#if !defined(MBEDTLS_SSL_PROTO_NO_TLS) +#define MBEDTLS_SSL_PROTO_TLS +#endif + /* Determine minimum supported version */ #define MBEDTLS_SSL_MIN_MAJOR_VERSION MBEDTLS_SSL_MAJOR_VERSION_3 diff --git a/library/version_features.c b/library/version_features.c index fc0b1f8f0..b1458a4ed 100644 --- a/library/version_features.c +++ b/library/version_features.c @@ -486,9 +486,9 @@ static const char *features[] = { #if defined(MBEDTLS_SSL_PROTO_DTLS) "MBEDTLS_SSL_PROTO_DTLS", #endif /* MBEDTLS_SSL_PROTO_DTLS */ -#if defined(MBEDTLS_SSL_PROTO_TLS) - "MBEDTLS_SSL_PROTO_TLS", -#endif /* MBEDTLS_SSL_PROTO_TLS */ +#if defined(MBEDTLS_SSL_PROTO_NO_TLS) + "MBEDTLS_SSL_PROTO_NO_TLS", +#endif /* MBEDTLS_SSL_PROTO_NO_TLS */ #if defined(MBEDTLS_SSL_ALPN) "MBEDTLS_SSL_ALPN", #endif /* MBEDTLS_SSL_ALPN */ diff --git a/programs/ssl/query_config.c b/programs/ssl/query_config.c index be35a76ce..d04f5123f 100644 --- a/programs/ssl/query_config.c +++ b/programs/ssl/query_config.c @@ -1338,13 +1338,13 @@ int query_config( const char *config ) } #endif /* MBEDTLS_SSL_PROTO_DTLS */ -#if defined(MBEDTLS_SSL_PROTO_TLS) - if( strcmp( "MBEDTLS_SSL_PROTO_TLS", config ) == 0 ) +#if defined(MBEDTLS_SSL_PROTO_NO_TLS) + if( strcmp( "MBEDTLS_SSL_PROTO_NO_TLS", config ) == 0 ) { - MACRO_EXPANSION_TO_STR( MBEDTLS_SSL_PROTO_TLS ); + MACRO_EXPANSION_TO_STR( MBEDTLS_SSL_PROTO_NO_TLS ); return( 0 ); } -#endif /* MBEDTLS_SSL_PROTO_TLS */ +#endif /* MBEDTLS_SSL_PROTO_NO_TLS */ #if defined(MBEDTLS_SSL_ALPN) if( strcmp( "MBEDTLS_SSL_ALPN", config ) == 0 ) diff --git a/programs/ssl/ssl_client1.c b/programs/ssl/ssl_client1.c index fc601ecd6..5d6758302 100644 --- a/programs/ssl/ssl_client1.c +++ b/programs/ssl/ssl_client1.c @@ -44,14 +44,14 @@ !defined(MBEDTLS_NET_C) || !defined(MBEDTLS_RSA_C) || \ !defined(MBEDTLS_CERTS_C) || !defined(MBEDTLS_PEM_PARSE_C) || \ !defined(MBEDTLS_CTR_DRBG_C) || !defined(MBEDTLS_X509_CRT_PARSE_C) || \ - !defined(MBEDTLS_SSL_PROTO_TLS) + defined(MBEDTLS_SSL_PROTO_NO_TLS) int main( void ) { mbedtls_printf("MBEDTLS_BIGNUM_C and/or MBEDTLS_ENTROPY_C and/or " "MBEDTLS_SSL_TLS_C and/or MBEDTLS_SSL_CLI_C and/or " "MBEDTLS_NET_C and/or MBEDTLS_RSA_C and/or " "MBEDTLS_CTR_DRBG_C and/or MBEDTLS_X509_CRT_PARSE_C and/or" - "MBEDTLS_SSL_PROTO_TLS not defined.\n"); + "not defined, and/or MBEDTLS_SSL_PROTO_NO_TLS defined.\n"); return( 0 ); } #else diff --git a/programs/ssl/ssl_fork_server.c b/programs/ssl/ssl_fork_server.c index 62b4c4098..bbe61fd2a 100644 --- a/programs/ssl/ssl_fork_server.c +++ b/programs/ssl/ssl_fork_server.c @@ -44,7 +44,7 @@ !defined(MBEDTLS_RSA_C) || !defined(MBEDTLS_CTR_DRBG_C) || \ !defined(MBEDTLS_X509_CRT_PARSE_C) || !defined(MBEDTLS_TIMING_C) || \ !defined(MBEDTLS_FS_IO) || !defined(MBEDTLS_PEM_PARSE_C) || \ - !defined(MBEDTLS_SSL_PROTO_TLS) + defined(MBEDTLS_SSL_PROTO_NO_TLS) int main( int argc, char *argv[] ) { ((void) argc); @@ -54,8 +54,8 @@ int main( int argc, char *argv[] ) "and/or MBEDTLS_SSL_TLS_C and/or MBEDTLS_SSL_SRV_C and/or " "MBEDTLS_NET_C and/or MBEDTLS_RSA_C and/or " "MBEDTLS_CTR_DRBG_C and/or MBEDTLS_X509_CRT_PARSE_C and/or " - "MBEDTLS_TIMING_C and/or MBEDTLS_PEM_PARSE_C and/or " - "MBEDTLS_SSL_PROTO_TLS not defined.\n"); + "MBEDTLS_TIMING_C and/or MBEDTLS_PEM_PARSE_C not defined, and/or " + "MBEDTLS_SSL_PROTO_NO_TLS defined.\n"); return( 0 ); } #elif defined(_WIN32) diff --git a/programs/ssl/ssl_mail_client.c b/programs/ssl/ssl_mail_client.c index 55c90c645..bc3fc8bcd 100644 --- a/programs/ssl/ssl_mail_client.c +++ b/programs/ssl/ssl_mail_client.c @@ -48,14 +48,14 @@ !defined(MBEDTLS_SSL_TLS_C) || !defined(MBEDTLS_SSL_CLI_C) || \ !defined(MBEDTLS_NET_C) || !defined(MBEDTLS_RSA_C) || \ !defined(MBEDTLS_CTR_DRBG_C) || !defined(MBEDTLS_X509_CRT_PARSE_C) || \ - !defined(MBEDTLS_FS_IO) || !defined(MBEDTLS_SSL_PROTO_TLS) + !defined(MBEDTLS_FS_IO) || defined(MBEDTLS_SSL_PROTO_NO_TLS) int main( void ) { mbedtls_printf("MBEDTLS_BIGNUM_C and/or MBEDTLS_ENTROPY_C and/or " "MBEDTLS_SSL_TLS_C and/or MBEDTLS_SSL_CLI_C and/or " "MBEDTLS_NET_C and/or MBEDTLS_RSA_C and/or " - "MBEDTLS_CTR_DRBG_C and/or MBEDTLS_X509_CRT_PARSE_C and/or " - "MBEDTLS_SSL_PROTO_TLS not defined.\n"); + "MBEDTLS_CTR_DRBG_C and/or MBEDTLS_X509_CRT_PARSE_C " + "not defined, and/or MBEDTLS_SSL_PROTO_NO_TLS defined.\n"); return( 0 ); } #else diff --git a/programs/ssl/ssl_pthread_server.c b/programs/ssl/ssl_pthread_server.c index b00f47617..17f4584f0 100644 --- a/programs/ssl/ssl_pthread_server.c +++ b/programs/ssl/ssl_pthread_server.c @@ -45,7 +45,7 @@ !defined(MBEDTLS_RSA_C) || !defined(MBEDTLS_CTR_DRBG_C) || \ !defined(MBEDTLS_X509_CRT_PARSE_C) || !defined(MBEDTLS_FS_IO) || \ !defined(MBEDTLS_THREADING_C) || !defined(MBEDTLS_THREADING_PTHREAD) || \ - !defined(MBEDTLS_PEM_PARSE_C) || !defined(MBEDTLS_SSL_PROTO_TLS) + !defined(MBEDTLS_PEM_PARSE_C) || defined(MBEDTLS_SSL_PROTO_NO_TLS) int main( void ) { mbedtls_printf("MBEDTLS_BIGNUM_C and/or MBEDTLS_CERTS_C and/or MBEDTLS_ENTROPY_C " @@ -53,8 +53,8 @@ int main( void ) "MBEDTLS_NET_C and/or MBEDTLS_RSA_C and/or " "MBEDTLS_CTR_DRBG_C and/or MBEDTLS_X509_CRT_PARSE_C and/or " "MBEDTLS_THREADING_C and/or MBEDTLS_THREADING_PTHREAD " - "and/or MBEDTLS_PEM_PARSE_C and/or " - "MBEDTLS_SSL_PROTO_TLS not defined.\n"); + "and/or MBEDTLS_PEM_PARSE_C not defined, and/or " + "MBEDTLS_SSL_PROTO_NO_TLS defined.\n"); return( 0 ); } #else diff --git a/programs/ssl/ssl_server.c b/programs/ssl/ssl_server.c index 05d58fa74..97918562a 100644 --- a/programs/ssl/ssl_server.c +++ b/programs/ssl/ssl_server.c @@ -44,15 +44,15 @@ !defined(MBEDTLS_SSL_SRV_C) || !defined(MBEDTLS_NET_C) || \ !defined(MBEDTLS_RSA_C) || !defined(MBEDTLS_CTR_DRBG_C) || \ !defined(MBEDTLS_X509_CRT_PARSE_C) || !defined(MBEDTLS_FS_IO) || \ - !defined(MBEDTLS_PEM_PARSE_C) || !defined(MBEDTLS_SSL_PROTO_TLS) + !defined(MBEDTLS_PEM_PARSE_C) || defined(MBEDTLS_SSL_PROTO_NO_TLS) int main( void ) { mbedtls_printf("MBEDTLS_BIGNUM_C and/or MBEDTLS_CERTS_C and/or MBEDTLS_ENTROPY_C " "and/or MBEDTLS_SSL_TLS_C and/or MBEDTLS_SSL_SRV_C and/or " "MBEDTLS_NET_C and/or MBEDTLS_RSA_C and/or " "MBEDTLS_CTR_DRBG_C and/or MBEDTLS_X509_CRT_PARSE_C " - "and/or MBEDTLS_PEM_PARSE_C and/or " - "MBEDTLS_SSL_PROTO_TLS not defined.\n"); + "and/or MBEDTLS_PEM_PARSE_C not defined, and/or " + "MBEDTLS_SSL_PROTO_NO_TLS defined.\n"); return( 0 ); } #else diff --git a/scripts/config.pl b/scripts/config.pl index 86af55394..ed9aa02a6 100755 --- a/scripts/config.pl +++ b/scripts/config.pl @@ -31,6 +31,7 @@ # MBEDTLS_REMOVE_ARC4_CIPHERSUITES # MBEDTLS_REMOVE_3DES_CIPHERSUITES # MBEDTLS_SSL_HW_RECORD_ACCEL +# MBEDTLS_SSL_PROTO_NO_DTLS # MBEDTLS_RSA_NO_CRT # MBEDTLS_X509_ALLOW_EXTENSIONS_NON_V3 # MBEDTLS_X509_ALLOW_UNSUPPORTED_CRITICAL_EXTENSION @@ -92,6 +93,7 @@ MBEDTLS_RSA_NO_CRT MBEDTLS_REMOVE_ARC4_CIPHERSUITES MBEDTLS_REMOVE_3DES_CIPHERSUITES MBEDTLS_SSL_HW_RECORD_ACCEL +MBEDTLS_SSL_PROTO_NO_TLS MBEDTLS_X509_ALLOW_EXTENSIONS_NON_V3 MBEDTLS_X509_ALLOW_UNSUPPORTED_CRITICAL_EXTENSION MBEDTLS_ZLIB_SUPPORT From 41efa2109ed270af332c7fbb2802e3b231a54cc2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 18 Jun 2019 10:28:52 +0200 Subject: [PATCH 2/9] Improve documentation of PROTO_NO_TLS --- include/mbedtls/config.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/include/mbedtls/config.h b/include/mbedtls/config.h index 1653f8950..b701fbf99 100644 --- a/include/mbedtls/config.h +++ b/include/mbedtls/config.h @@ -1521,7 +1521,8 @@ * \def MBEDTLS_SSL_PROTO_NO_TLS * * Disable support for SSL/TLS (all available versions) - this doesn't affect - * support for DTLS which is controlled by #MBEDTLS_SSL_PROTO_DTLS. + * support for DTLS which is controlled by #MBEDTLS_SSL_PROTO_DTLS. This is + * useful to reduce code size in configurations where only DTLS is used. * * Disable this and enable MBEDTLS_SSL_PROTO_TLS1_2 to enable TLS 1.2; * disable this and enable MBEDTLS_SSL_PROTO_TLS1_1 to enable TLS 1.1; From c84511fb1f5db99f9753afc388ca4fd759377eb5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 18 Jun 2019 10:32:25 +0200 Subject: [PATCH 3/9] Add check for undocumented positive option --- include/mbedtls/check_config.h | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/include/mbedtls/check_config.h b/include/mbedtls/check_config.h index 34f1a3bee..00fb9b367 100644 --- a/include/mbedtls/check_config.h +++ b/include/mbedtls/check_config.h @@ -565,6 +565,12 @@ #error "MBEDTLS_SSL_TLS_C defined, but no protocol version is active" #endif +/* PROTO_TLS is not a documented option so far, but still check for conflicts + * involving it, in preparation for making it the documented option later */ +#if defined(MBEDTLS_SSL_PROTO_TLS) && defined(MBEDTLS_SSL_PROTO_NO_TLS) +#error "MBEDTLS_SSL_PROTO_TLS and MBEDTLS_SSL_PROTO_NO_TLS both defined" +#endif + #if defined(MBEDTLS_SSL_TLS_C) && \ ( defined(MBEDTLS_SSL_PROTO_NO_TLS) && !defined(MBEDTLS_SSL_PROTO_DTLS) ) #error "MBEDTLS_SSL_TLS_C defined, but neither TLS or DTLS is active" From 11d3282f5ac0bed4fd1ed8d0219048a248d4e6a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 18 Jun 2019 10:34:15 +0200 Subject: [PATCH 4/9] Add a ChangeLog entry. --- ChangeLog | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ChangeLog b/ChangeLog index e769dc27a..141536aa3 100644 --- a/ChangeLog +++ b/ChangeLog @@ -24,6 +24,8 @@ Features mbedtls_ssl_session_load() to allow serializing a session, for example to store it in non-volatile storage, and later using it for TLS session resumption. + * Add new configuration option MBEDTLS_SSL_PROTO_NO_TLS that enables code + size savings in configurations where only DTLS is used. Bugfix * Server's RSA certificate in certs.c was SHA-1 signed. In the default From 19e8132e1e76074d506f7d184c2e49cef48e2498 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 18 Jun 2019 10:54:25 +0200 Subject: [PATCH 5/9] Add NO_TLS to configs/baremetal.h Was missed for some reason in commit ba8b1eb5d90b --- configs/baremetal.h | 1 + 1 file changed, 1 insertion(+) diff --git a/configs/baremetal.h b/configs/baremetal.h index 2fbc35971..12321289b 100644 --- a/configs/baremetal.h +++ b/configs/baremetal.h @@ -77,6 +77,7 @@ #define MBEDTLS_SSL_SESSION_TICKETS #define MBEDTLS_SSL_COOKIE_C #define MBEDTLS_SSL_PROTO_DTLS +#define MBEDTLS_SSL_PROTO_NO_TLS #define MBEDTLS_SSL_DTLS_ANTI_REPLAY #define MBEDTLS_SSL_DTLS_HELLO_VERIFY #define MBEDTLS_SSL_DTLS_BADMAC_LIMIT From 889bbc70b640a4ba4a663fb9c49c9d5af8038511 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 18 Jun 2019 10:56:09 +0200 Subject: [PATCH 6/9] Fix unreachable code warnings with armc5 Some TLS-only code paths were not protected by an #ifdef and while some compiler are happy to just silently remove them, armc5 complains: Warning: #111-D: statement is unreachable Let's make armc5 happy. --- include/mbedtls/ssl_internal.h | 5 +++- library/ssl_tls.c | 47 +++++++++++++++++++++++----------- 2 files changed, 36 insertions(+), 16 deletions(-) diff --git a/include/mbedtls/ssl_internal.h b/include/mbedtls/ssl_internal.h index e6c829d3a..8089441c4 100644 --- a/include/mbedtls/ssl_internal.h +++ b/include/mbedtls/ssl_internal.h @@ -969,8 +969,11 @@ static inline size_t mbedtls_ssl_hs_hdr_len( const mbedtls_ssl_context *ssl ) #if defined(MBEDTLS_SSL_PROTO_DTLS) if( MBEDTLS_SSL_TRANSPORT_IS_DTLS( ssl->conf->transport ) ) return( 12 ); + MBEDTLS_SSL_TRANSPORT_ELSE +#endif +#if defined(MBEDTLS_SSL_PROTO_TLS) + return( 4 ); #endif - return( 4 ); } #if defined(MBEDTLS_SSL_PROTO_DTLS) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index e3ce5908f..912fdec80 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -68,8 +68,11 @@ static inline size_t ssl_ep_len( const mbedtls_ssl_context *ssl ) #if defined(MBEDTLS_SSL_PROTO_DTLS) if( MBEDTLS_SSL_TRANSPORT_IS_DTLS( ssl->conf->transport ) ) return( 2 ); + MBEDTLS_SSL_TRANSPORT_ELSE #endif +#if defined(MBEDTLS_SSL_PROTO_TLS) return( 0 ); +#endif } /* @@ -8598,25 +8601,29 @@ const char *mbedtls_ssl_get_version( const mbedtls_ssl_context *ssl ) return( "unknown (DTLS)" ); } } -#endif - - switch( ssl->minor_ver ) + MBEDTLS_SSL_TRANSPORT_ELSE +#endif /* MBEDTLS_SSL_PROTO_DTLS */ +#if defined(MBEDTLS_SSL_PROTO_TLS) { - case MBEDTLS_SSL_MINOR_VERSION_0: - return( "SSLv3.0" ); + switch( ssl->minor_ver ) + { + case MBEDTLS_SSL_MINOR_VERSION_0: + return( "SSLv3.0" ); - case MBEDTLS_SSL_MINOR_VERSION_1: - return( "TLSv1.0" ); + case MBEDTLS_SSL_MINOR_VERSION_1: + return( "TLSv1.0" ); - case MBEDTLS_SSL_MINOR_VERSION_2: - return( "TLSv1.1" ); + case MBEDTLS_SSL_MINOR_VERSION_2: + return( "TLSv1.1" ); - case MBEDTLS_SSL_MINOR_VERSION_3: - return( "TLSv1.2" ); + case MBEDTLS_SSL_MINOR_VERSION_3: + return( "TLSv1.2" ); - default: - return( "unknown" ); + default: + return( "unknown" ); + } } +#endif /* MBEDTLS_SSL_PROTO_TLS */ } int mbedtls_ssl_get_record_expansion( const mbedtls_ssl_context *ssl ) @@ -9610,8 +9617,13 @@ int mbedtls_ssl_read( mbedtls_ssl_context *ssl, unsigned char *buf, size_t len ) { continue; } + MBEDTLS_SSL_TRANSPORT_ELSE +#endif +#if defined(MBEDTLS_SSL_PROTO_TLS) + { + return( MBEDTLS_ERR_SSL_UNEXPECTED_MESSAGE ); + } #endif - return( MBEDTLS_ERR_SSL_UNEXPECTED_MESSAGE ); } #endif /* MBEDTLS_SSL_CLI_C */ @@ -9627,8 +9639,13 @@ int mbedtls_ssl_read( mbedtls_ssl_context *ssl, unsigned char *buf, size_t len ) { continue; } + MBEDTLS_SSL_TRANSPORT_ELSE +#endif +#if defined(MBEDTLS_SSL_PROTO_TLS) + { + return( MBEDTLS_ERR_SSL_UNEXPECTED_MESSAGE ); + } #endif - return( MBEDTLS_ERR_SSL_UNEXPECTED_MESSAGE ); } #endif /* MBEDTLS_SSL_SRV_C */ From 070f107a6157f4721472519c48b92ea68d418c86 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 18 Jun 2019 11:17:17 +0200 Subject: [PATCH 7/9] Add --check option to scripts/baremetal.sh Only effective together with --rom, makes two changes: - abort in case of build warnings - skip writing statistics The goal is to make sure we build cleanly in the configuration used for measuring code size, with all the compilers we use, both because we care about that configuration and those compilers, and because any warnings would cast a shadow on the code size measurements. Currently the build fails with armc5 due to a pre-existing warning in PK, this will be fixed in the next commit. The next commit will also add an all.sh component to make sure we have no regression in the future. (Which is the motivation for --check skipping statistics: an all.sh component should probably not leave files around.) While at it, fix two things: 1. The call to gcc --version was redundant with the echo line below 2. WARNING_CFLAGS shouldn't be overriden with armclang, as it would remove the -Wall -Wextra and any directory-specific warning (such as -Wdeclaration-after-statement in library). It's meant to be overriden only with compilers that don't accept the default value (namely armc5 here). --- scripts/baremetal.sh | 38 ++++++++++++++++++++++++++++++-------- 1 file changed, 30 insertions(+), 8 deletions(-) diff --git a/scripts/baremetal.sh b/scripts/baremetal.sh index 86fac5687..88413252f 100755 --- a/scripts/baremetal.sh +++ b/scripts/baremetal.sh @@ -75,15 +75,20 @@ baremetal_build_gcc() echo "Create 32-bit library-only baremetal build (GCC, config: $BAREMETAL_CONFIG)" gcc_ver=$($GCC_CC --version | head -n 1 | sed -n 's/^.*\([0-9]\.[0-9]\.[0-9]\).*$/\1/p') - CFLAGS_BAREMETAL="-Os -mthumb -mcpu=cortex-m0plus" + CFLAGS_BAREMETAL="-Os -mthumb -mcpu=cortex-m0plus --std=c99" + if [ $check -ne 0 ]; then + CFLAGS_BAREMETAL="$CFLAGS_BAREMETAL -Werror" + fi CFLAGS="$CFLAGS_BAREMETAL $CFLAGS_CONFIG" - $GCC_CC --version - echo "GCC version: $gcc_ver" echo "Flags: $CFLAGS_BAREMETAL" make CC=$GCC_CC AR=$GCC_AR CFLAGS="$CFLAGS" lib -j > /dev/null + if [ $check -ne 0 ]; then + return + fi + ROM_OUT_FILE="rom_files__${date}__${NAME}__gcc_${gcc_ver}" ROM_OUT_SYMS="rom_syms__${date}__${NAME}__gcc_${gcc_ver}" echo "Generate file statistics..." @@ -108,10 +113,18 @@ baremetal_build_armc5() CFLAGS="$CFLAGS_BAREMETAL $CFLAGS_CONFIG" WARNING_CFLAGS="--strict --c99" + if [ $check -ne 0 ]; then + WARNING_CFLAGS="$WARNING_CFLAGS --diag_error=warning" + fi + echo "ARMC5 version: $armc5_ver" echo "Flags: $WARNING_CFLAGS $CFLAGS_BAREMETAL" make WARNING_CFLAGS="$WARNING_CFLAGS" CC=$ARMC5_CC AR=$ARMC5_AR CFLAGS="$CFLAGS" lib -j > /dev/null + if [ $check -ne 0 ]; then + return + fi + ROM_OUT_FILE="rom_files__${date}__${NAME}__armc5_${armc5_ver}" ROM_OUT_SYMS="rom_syms__${date}__${NAME}__armc5_${armc5_ver}" echo "Generate file statistics..." @@ -132,13 +145,19 @@ baremetal_build_armc6() echo "Create 32-bit library-only baremetal build (ARMC6, Config: $BAREMETAL_CONFIG)" armc6_ver=$($ARMC6_CC --version | sed -n 's/.*ARM Compiler \([^ ]*\)$/\1/p') - CFLAGS_BAREMETAL="-Os --target=arm-arm-none-eabi -mthumb -mcpu=cortex-m0plus" + CFLAGS_BAREMETAL="-Os --target=arm-arm-none-eabi -mthumb -mcpu=cortex-m0plus -xc --std=c99" + if [ $check -ne 0 ]; then + CFLAGS_BAREMETAL="$CFLAGS_BAREMETAL -Werror" + fi CFLAGS="$CFLAGS_BAREMETAL $CFLAGS_CONFIG" - WARNING_CFLAGS="-xc -std=c99" echo "ARMC6 version: $armc6_ver" - echo "Flags: $WARNING_CFLAGS $CFLAGS_BAREMETAL" - make WARNING_CFLAGS="$WARNING_CFLAGS" CC=$ARMC6_CC AR=$ARMC6_AR CFLAGS="$CFLAGS" lib -j > /dev/null + echo "Flags: $CFLAGS_BAREMETAL" + make CC=$ARMC6_CC AR=$ARMC6_AR CFLAGS="$CFLAGS" lib -j > /dev/null + + if [ $check -ne 0 ]; then + return + fi ROM_OUT_FILE="rom_files__${date}__${NAME}__armc6_${armc6_ver}" ROM_OUT_SYMS="rom_syms__${date}__${NAME}__armc6_${armc6_ver}" @@ -284,7 +303,7 @@ baremetal_ram_stack() { } show_usage() { - echo "Usage: $0 [--rom [--gcc] [--armc5] [--armc6]|--ram [--stack] [--heap]]" + echo "Usage: $0 [--rom [--check] [--gcc] [--armc5] [--armc6]|--ram [--stack] [--heap]]" } test_build=0 @@ -297,6 +316,8 @@ build_armc6=0 measure_heap=0 measure_stack=0 +check=0 + while [ $# -gt 0 ]; do case "$1" in --gcc) build_gcc=1;; @@ -306,6 +327,7 @@ while [ $# -gt 0 ]; do --rom) raw_build=1;; --heap) measure_heap=1;; --stack) measure_stack=1;; + --check) check=1;; -*) echo >&2 "Unknown option: $1" show_usage From e83b2c2a50bb0beadb86832f564a2d1c357f0413 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 18 Jun 2019 11:31:59 +0200 Subject: [PATCH 8/9] Fix unused variable warnings in pkparse.c In a reduced configuration without PEM, PKCS5 or PKCS12, armc5 found that ret was set but not used. Fixing that lead to a new warning about the variable not being used at all. Now the variable is only declared when it's needed. --- library/pkparse.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/library/pkparse.c b/library/pkparse.c index ae210bca6..4ec63e4bb 100644 --- a/library/pkparse.c +++ b/library/pkparse.c @@ -1164,7 +1164,11 @@ int mbedtls_pk_parse_key( mbedtls_pk_context *pk, const unsigned char *key, size_t keylen, const unsigned char *pwd, size_t pwdlen ) { +#if defined(MBEDTLS_PKCS12_C) || \ + defined(MBEDTLS_PKCS5_C) || \ + defined(MBEDTLS_PEM_PARSE_C) int ret; +#endif const mbedtls_pk_info_t *pk_info; #if defined(MBEDTLS_PEM_PARSE_C) size_t len; @@ -1327,7 +1331,7 @@ int mbedtls_pk_parse_key( mbedtls_pk_context *pk, } #endif /* MBEDTLS_PKCS12_C || MBEDTLS_PKCS5_C */ - if( ( ret = pk_parse_key_pkcs8_unencrypted_der( pk, key, keylen ) ) == 0 ) + if( pk_parse_key_pkcs8_unencrypted_der( pk, key, keylen ) == 0 ) return( 0 ); mbedtls_pk_free( pk ); From 31ae7facb30a46ba5635e8e5aeddde188973e6ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 18 Jun 2019 12:03:51 +0200 Subject: [PATCH 9/9] Add test for build warnings with baremetal.h --- tests/scripts/all.sh | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 9af040fba..99c873692 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -1155,6 +1155,12 @@ component_build_armcc () { armc6_build_test "--target=aarch64-arm-none-eabi -march=armv8.2-a" } +# need _armcc in the name for pre_check_tools() +component_build_baremetal_script_gcc_armcc () { + msg "build: scripts/baremetal.sh gcc/armc5/armc6" + scripts/baremetal.sh --rom --gcc --armc5 --armc6 --check +} + component_build_armcc_tinycrypt_baremetal () { msg "build: ARM Compiler 5, make with tinycrypt and baremetal" scripts/config.pl baremetal