From a771160799d325b2b0aef10270959a2d4707a5bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Mon, 18 Mar 2019 10:18:37 +0100 Subject: [PATCH 1/8] Introduce new config.h flag for TLS For now the option has no effect. Adapted existing example config files. The fact that I needed to do this highlights that this is a slightly incompatible change: existing users need to update their existing custom configs (if standalone as opposed to based on the default config) in order to still get the same behaviour. The alternative would be to have a negative config option (eg NO_TLS or DTLS_ONLY) but this doesn't fit as nicely with the existing options, so hopefully the minor incompatibility is acceptable. I don't think it's worth adding a new component to all.sh: - builds with both DTLS and TLS are done in the default (and full) config - TLS-only builds are done with eg config-suite-b.h in test-ref-configs - a DTLS-only build is done with config-thread.h in test-ref-configs - builds with none of them (and SSL_TLS_C enabled) are forbidden --- 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 | 7 ++++++- include/mbedtls/config.h | 27 +++++++++++++++++++++++---- library/version_features.c | 3 +++ programs/ssl/query_config.c | 8 ++++++++ 8 files changed, 44 insertions(+), 5 deletions(-) diff --git a/configs/config-ccm-psk-tls1_2.h b/configs/config-ccm-psk-tls1_2.h index c9b58dd538..bd2c1a3b84 100644 --- a/configs/config-ccm-psk-tls1_2.h +++ b/configs/config-ccm-psk-tls1_2.h @@ -41,6 +41,7 @@ /* 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 013bc0300d..349ea8e577 100644 --- a/configs/config-mini-tls1_1.h +++ b/configs/config-mini-tls1_1.h @@ -40,6 +40,7 @@ #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 18e2c40369..e6fad1c0ef 100644 --- a/configs/config-suite-b.h +++ b/configs/config-suite-b.h @@ -47,6 +47,7 @@ #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 25db16bf0f..3166aa9701 100644 --- a/configs/config-thread.h +++ b/configs/config-thread.h @@ -29,6 +29,7 @@ * Distinguishing features: * - no RSA or classic DH, fully based on ECC * - no X.509 + * - no TLS, only DTLS * - support for experimental EC J-PAKE key exchange * * See README.txt for usage instructions. diff --git a/include/mbedtls/check_config.h b/include/mbedtls/check_config.h index 48555f68a1..fccf104390 100644 --- a/include/mbedtls/check_config.h +++ b/include/mbedtls/check_config.h @@ -562,7 +562,12 @@ #if defined(MBEDTLS_SSL_TLS_C) && (!defined(MBEDTLS_SSL_PROTO_SSL3) && \ !defined(MBEDTLS_SSL_PROTO_TLS1) && !defined(MBEDTLS_SSL_PROTO_TLS1_1) && \ !defined(MBEDTLS_SSL_PROTO_TLS1_2)) -#error "MBEDTLS_SSL_TLS_C defined, but no protocols are active" +#error "MBEDTLS_SSL_TLS_C defined, but no protocol version is active" +#endif + +#if defined(MBEDTLS_SSL_TLS_C) && \ + (!defined(MBEDTLS_SSL_PROTO_TLS) && !defined(MBEDTLS_SSL_PROTO_DTLS)) +#error "MBEDTLS_SSL_TLS_C defined, but neither TLS or DTLS is active" #endif #if defined(MBEDTLS_SSL_TLS_C) && (defined(MBEDTLS_SSL_PROTO_SSL3) && \ diff --git a/include/mbedtls/config.h b/include/mbedtls/config.h index f5b2de90cf..69f68dda4f 100644 --- a/include/mbedtls/config.h +++ b/include/mbedtls/config.h @@ -1453,7 +1453,7 @@ /** * \def MBEDTLS_SSL_PROTO_SSL3 * - * Enable support for SSL 3.0. + * Enable support for SSL 3.0 (if TLS is enabled). * * Requires: MBEDTLS_MD5_C * MBEDTLS_SHA1_C @@ -1465,7 +1465,7 @@ /** * \def MBEDTLS_SSL_PROTO_TLS1 * - * Enable support for TLS 1.0. + * Enable support for TLS 1.0 (if TLS is enabled). * * Requires: MBEDTLS_MD5_C * MBEDTLS_SHA1_C @@ -1477,7 +1477,8 @@ /** * \def MBEDTLS_SSL_PROTO_TLS1_1 * - * Enable support for TLS 1.1 (and DTLS 1.0 if DTLS is enabled). + * Enable support for TLS 1.1 (if TLS is enabled) and DTLS 1.0 (if DTLS is + * enabled). * * Requires: MBEDTLS_MD5_C * MBEDTLS_SHA1_C @@ -1489,7 +1490,8 @@ /** * \def MBEDTLS_SSL_PROTO_TLS1_2 * - * Enable support for TLS 1.2 (and DTLS 1.2 if DTLS is enabled). + * Enable support for TLS 1.2 (if TLS is enabled) and DTLS 1.2 (if DTLS is + * enabled). * * Requires: MBEDTLS_SHA1_C or MBEDTLS_SHA256_C or MBEDTLS_SHA512_C * (Depends on ciphersuites) @@ -1513,6 +1515,23 @@ */ #define MBEDTLS_SSL_PROTO_DTLS +/** + * \def MBEDTLS_SSL_PROTO_TLS + * + * Enable support for TLS (all available versions). + * + * Enable this and MBEDTLS_SSL_PROTO_TLS1 to enable TLS 1.0, + * Enable this and MBEDTLS_SSL_PROTO_TLS1_1 to enable TLS 1.1, + * and/or this and MBEDTLS_SSL_PROTO_TLS1_2 to enable TLS 1.2. + * + * Requires: MBEDTLS_SSL_PROTO_TLS1_1 + * or MBEDTLS_SSL_PROTO_TLS1_1 + * or MBEDTLS_SSL_PROTO_TLS1_2 + * + * Comment this macro to disable support for TLS + */ +#define MBEDTLS_SSL_PROTO_TLS + /** * \def MBEDTLS_SSL_ALPN * diff --git a/library/version_features.c b/library/version_features.c index 7494b42870..fc0b1f8f03 100644 --- a/library/version_features.c +++ b/library/version_features.c @@ -486,6 +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_ALPN) "MBEDTLS_SSL_ALPN", #endif /* MBEDTLS_SSL_ALPN */ diff --git a/programs/ssl/query_config.c b/programs/ssl/query_config.c index e1d1332f2f..be35a76ce5 100644 --- a/programs/ssl/query_config.c +++ b/programs/ssl/query_config.c @@ -1338,6 +1338,14 @@ 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 ) + { + MACRO_EXPANSION_TO_STR( MBEDTLS_SSL_PROTO_TLS ); + return( 0 ); + } +#endif /* MBEDTLS_SSL_PROTO_TLS */ + #if defined(MBEDTLS_SSL_ALPN) if( strcmp( "MBEDTLS_SSL_ALPN", config ) == 0 ) { From e744eab3b1c882a5d5c3766be66c5bb81849b57e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Mon, 18 Mar 2019 10:51:18 +0100 Subject: [PATCH 2/8] Adapt defaults and programs documentation --- include/mbedtls/ssl.h | 2 +- library/ssl_tls.c | 4 ++++ programs/ssl/ssl_client1.c | 7 ++++--- programs/ssl/ssl_client2.c | 2 +- programs/ssl/ssl_fork_server.c | 6 ++++-- programs/ssl/ssl_mail_client.c | 6 +++--- programs/ssl/ssl_pthread_server.c | 5 +++-- programs/ssl/ssl_server.c | 5 +++-- programs/ssl/ssl_server2.c | 2 +- 9 files changed, 24 insertions(+), 15 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 7f073afba8..8e3161c3dc 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -1337,7 +1337,7 @@ void mbedtls_ssl_conf_endpoint( mbedtls_ssl_config *conf, int endpoint ); /** * \brief Set the transport type (TLS or DTLS). - * Default: TLS + * Default: TLS if both are enabled, or DTLS. * * \note For DTLS, you must either provide a recv callback that * doesn't block, or one that handles timeouts, see diff --git a/library/ssl_tls.c b/library/ssl_tls.c index b61453fe53..9afcc96e80 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -10216,6 +10216,10 @@ void mbedtls_ssl_free( mbedtls_ssl_context *ssl ) void mbedtls_ssl_config_init( mbedtls_ssl_config *conf ) { memset( conf, 0, sizeof( mbedtls_ssl_config ) ); + +#if !defined(MBEDTLS_SSL_PROTO_TLS) + conf->transport = MBEDTLS_SSL_TRANSPORT_DATAGRAM; +#endif } #if defined(MBEDTLS_KEY_EXCHANGE__WITH_CERT__ENABLED) diff --git a/programs/ssl/ssl_client1.c b/programs/ssl/ssl_client1.c index 646909f114..fc601ecd62 100644 --- a/programs/ssl/ssl_client1.c +++ b/programs/ssl/ssl_client1.c @@ -43,14 +43,15 @@ !defined(MBEDTLS_SSL_TLS_C) || !defined(MBEDTLS_SSL_CLI_C) || \ !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_CTR_DRBG_C) || !defined(MBEDTLS_X509_CRT_PARSE_C) || \ + !defined(MBEDTLS_SSL_PROTO_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 " - "not defined.\n"); + "MBEDTLS_CTR_DRBG_C and/or MBEDTLS_X509_CRT_PARSE_C and/or" + "MBEDTLS_SSL_PROTO_TLS not defined.\n"); return( 0 ); } #else diff --git a/programs/ssl/ssl_client2.c b/programs/ssl/ssl_client2.c index 38c94be605..1665784281 100644 --- a/programs/ssl/ssl_client2.c +++ b/programs/ssl/ssl_client2.c @@ -223,7 +223,7 @@ int main( void ) #if defined(MBEDTLS_SSL_PROTO_DTLS) #define USAGE_DTLS \ - " dtls=%%d default: 0 (TLS)\n" \ + " dtls=%%d default: 0 (TLS) (if both enabled)\n" \ " hs_timeout=%%d-%%d default: (library default: 1000-60000)\n" \ " range of DTLS handshake timeouts in millisecs\n" \ " mtu=%%d default: (library default: unlimited)\n" \ diff --git a/programs/ssl/ssl_fork_server.c b/programs/ssl/ssl_fork_server.c index b6f1cc4fdd..62b4c40987 100644 --- a/programs/ssl/ssl_fork_server.c +++ b/programs/ssl/ssl_fork_server.c @@ -43,7 +43,8 @@ !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_TIMING_C) || \ - !defined(MBEDTLS_FS_IO) || !defined(MBEDTLS_PEM_PARSE_C) + !defined(MBEDTLS_FS_IO) || !defined(MBEDTLS_PEM_PARSE_C) || \ + !defined(MBEDTLS_SSL_PROTO_TLS) int main( int argc, char *argv[] ) { ((void) argc); @@ -53,7 +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 not defined.\n"); + "MBEDTLS_TIMING_C and/or MBEDTLS_PEM_PARSE_C and/or " + "MBEDTLS_SSL_PROTO_TLS not defined.\n"); return( 0 ); } #elif defined(_WIN32) diff --git a/programs/ssl/ssl_mail_client.c b/programs/ssl/ssl_mail_client.c index c73297c2ab..55c90c645b 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_FS_IO) || !defined(MBEDTLS_SSL_PROTO_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 " - "not defined.\n"); + "MBEDTLS_CTR_DRBG_C and/or MBEDTLS_X509_CRT_PARSE_C and/or " + "MBEDTLS_SSL_PROTO_TLS not defined.\n"); return( 0 ); } #else diff --git a/programs/ssl/ssl_pthread_server.c b/programs/ssl/ssl_pthread_server.c index b5026959a6..b00f476175 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_PEM_PARSE_C) || !defined(MBEDTLS_SSL_PROTO_TLS) int main( void ) { mbedtls_printf("MBEDTLS_BIGNUM_C and/or MBEDTLS_CERTS_C and/or MBEDTLS_ENTROPY_C " @@ -53,7 +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 not defined.\n"); + "and/or MBEDTLS_PEM_PARSE_C and/or " + "MBEDTLS_SSL_PROTO_TLS not defined.\n"); return( 0 ); } #else diff --git a/programs/ssl/ssl_server.c b/programs/ssl/ssl_server.c index 1852b2badf..05d58fa74e 100644 --- a/programs/ssl/ssl_server.c +++ b/programs/ssl/ssl_server.c @@ -44,14 +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_PEM_PARSE_C) || !defined(MBEDTLS_SSL_PROTO_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 not defined.\n"); + "and/or MBEDTLS_PEM_PARSE_C and/or " + "MBEDTLS_SSL_PROTO_TLS not defined.\n"); return( 0 ); } #else diff --git a/programs/ssl/ssl_server2.c b/programs/ssl/ssl_server2.c index ec18dd91ce..6d6293adc6 100644 --- a/programs/ssl/ssl_server2.c +++ b/programs/ssl/ssl_server2.c @@ -329,7 +329,7 @@ int main( void ) #if defined(MBEDTLS_SSL_PROTO_DTLS) #define USAGE_DTLS \ - " dtls=%%d default: 0 (TLS)\n" \ + " dtls=%%d default: 0 (TLS) (if both enabled)\n" \ " hs_timeout=%%d-%%d default: (library default: 1000-60000)\n" \ " range of DTLS handshake timeouts in millisecs\n" \ " mtu=%%d default: (library default: unlimited)\n" \ From 25838b795f79f53d31e3f41bdec191a300cfe3d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 19 Mar 2019 10:23:56 +0100 Subject: [PATCH 3/8] Introduce tools for transport-specific code And use those tools in a few places. For now the purpose is just to validate those tools before using them in all occurrences of transport-specific code. The effect of these changes was measured with the following script: ``` set -eu build() { printf "\n$1\n" CC=arm-none-eabi-gcc CFLAGS='-Werror -Os -march=armv6-m -mthumb' \ AR=arm-none-eabi-ar LD=arm-none-eabi-ld make clean lib >/dev/null arm-none-eabi-size -t library/libmbedtls.a } git checkout -- include/mbedtls/config.h scripts/config.pl unset MBEDTLS_NET_C scripts/config.pl unset MBEDTLS_TIMING_C scripts/config.pl unset MBEDTLS_FS_IO scripts/config.pl unset MBEDTLS_ENTROPY_NV_SEED scripts/config.pl set MBEDTLS_NO_PLATFORM_ENTROPY build "both" scripts/config.pl unset MBEDTLS_SSL_PROTO_TLS build "DTLS-only" scripts/config.pl set MBEDTLS_SSL_PROTO_TLS scripts/config.pl unset MBEDTLS_SSL_PROTO_DTLS scripts/config.pl unset MBEDTLS_SSL_DTLS_HELLO_VERIFY scripts/config.pl unset MBEDTLS_SSL_DTLS_ANTI_REPLAY scripts/config.pl unset MBEDTLS_SSL_DTLS_BADMAC_LIMIT scripts/config.pl unset MBEDTLS_SSL_DTLS_CLIENT_PORT_REUSE build "TLS-only" git checkout -- include/mbedtls/config.h ``` The output of the script is as follows: ``` both text data bss dec hex filename 1820 0 4 1824 720 debug.o (ex library/libmbedtls.a) 0 0 0 0 0 net_sockets.o (ex library/libmbedtls.a) 548 0 0 548 224 ssl_cache.o (ex library/libmbedtls.a) 11155 0 596 11751 2de7 ssl_ciphersuites.o (ex library/libmbedtls.a) 17160 0 0 17160 4308 ssl_cli.o (ex library/libmbedtls.a) 460 0 0 460 1cc ssl_cookie.o (ex library/libmbedtls.a) 17637 0 0 17637 44e5 ssl_srv.o (ex library/libmbedtls.a) 800 0 0 800 320 ssl_ticket.o (ex library/libmbedtls.a) 39322 60 0 39382 99d6 ssl_tls.o (ex library/libmbedtls.a) 88902 60 600 89562 15dda (TOTALS) DTLS-only text data bss dec hex filename 1820 0 4 1824 720 debug.o (ex library/libmbedtls.a) 0 0 0 0 0 net_sockets.o (ex library/libmbedtls.a) 548 0 0 548 224 ssl_cache.o (ex library/libmbedtls.a) 11155 0 596 11751 2de7 ssl_ciphersuites.o (ex library/libmbedtls.a) 17072 0 0 17072 42b0 ssl_cli.o (ex library/libmbedtls.a) 460 0 0 460 1cc ssl_cookie.o (ex library/libmbedtls.a) 17565 0 0 17565 449d ssl_srv.o (ex library/libmbedtls.a) 800 0 0 800 320 ssl_ticket.o (ex library/libmbedtls.a) 38953 60 0 39013 9865 ssl_tls.o (ex library/libmbedtls.a) 88373 60 600 89033 15bc9 (TOTALS) TLS-only text data bss dec hex filename 1820 0 4 1824 720 debug.o (ex library/libmbedtls.a) 0 0 0 0 0 net_sockets.o (ex library/libmbedtls.a) 548 0 0 548 224 ssl_cache.o (ex library/libmbedtls.a) 11155 0 596 11751 2de7 ssl_ciphersuites.o (ex library/libmbedtls.a) 14916 0 0 14916 3a44 ssl_cli.o (ex library/libmbedtls.a) 460 0 0 460 1cc ssl_cookie.o (ex library/libmbedtls.a) 15852 0 0 15852 3dec ssl_srv.o (ex library/libmbedtls.a) 800 0 0 800 320 ssl_ticket.o (ex library/libmbedtls.a) 27623 60 0 27683 6c23 ssl_tls.o (ex library/libmbedtls.a) 73174 60 600 73834 1206a (TOTALS) ``` It can be seen that a DTLS-only build is now starting to be a bit smaller than a dual-mode build, which is the purpose of the new build option. --- include/mbedtls/ssl_internal.h | 61 +++++++++++++++++++++++++++++++--- library/ssl_tls.c | 8 +++-- 2 files changed, 62 insertions(+), 7 deletions(-) diff --git a/include/mbedtls/ssl_internal.h b/include/mbedtls/ssl_internal.h index 45ea26ae28..173b6d58b0 100644 --- a/include/mbedtls/ssl_internal.h +++ b/include/mbedtls/ssl_internal.h @@ -264,6 +264,57 @@ #define MBEDTLS_TLS_EXT_SUPPORTED_POINT_FORMATS_PRESENT (1 << 0) #define MBEDTLS_TLS_EXT_ECJPAKE_KKPP_OK (1 << 1) +/* + * Helpers for code specific to TLS or DTLS. + * + * Goals for these helpers: + * - generate minimal code, eg don't test if mode is DTLS in a DTLS-only build + * - make the flow clear to the compiler, ie that in dual-mode builds, + * when there are two branchs, exactly one of them is taken + * - preserve readability + * + * There are three macros: + * - MBEDTLS_SSL_TRANSPORT_IS_TLS( transport ) + * - MBEDTLS_SSL_TRANSPORT_IS_DTLS( transport ) + * - MBEDTLS_SSL_TRANSPORT_ELSE + * + * The first two are macros rather than static inline functions because some + * compilers (eg arm-none-eabi-gcc 5.4.1 20160919) don't propagate constants + * well enough for us with static inline functions. + * + * Usage 1 (can replace DTLS with TLS): + * #if defined(MBEDTLS_SSL_PROTO_DTLS) + * if( MBEDTLS_SSL_TRANSPORT_IS_DTLS( transport ) ) + * // DTLS-specific code + * #endif + * + * Usage 2 (can swap DTLS and TLS); + * #if defined(MBEDTLS_SSL_PROTO_DTLS) + * if( MBEDTLS_SSL_TRANSPORT_IS_DTLS( transport ) ) + * // DTLS-specific code + * MBEDTLS_SSL_TRANSPORT_ELSE + * #endif + * #if defined(MBEDTLS_SSL_PROTO_TLS) + * // TLS-specific code + * #endif + */ +#if defined(MBEDTLS_SSL_PROTO_DTLS) && defined(MBEDTLS_SSL_PROTO_TLS) /* both */ +#define MBEDTLS_SSL_TRANSPORT__BOTH /* shorcut for future tests */ +#define MBEDTLS_SSL_TRANSPORT_IS_TLS( transport ) \ + ( (transport) == MBEDTLS_SSL_TRANSPORT_STREAM ) +#define MBEDTLS_SSL_TRANSPORT_IS_DTLS( transport ) \ + ( (transport) == MBEDTLS_SSL_TRANSPORT_DATAGRAM ) +#define MBEDTLS_SSL_TRANSPORT_ELSE else +#elif defined(MBEDTLS_SSL_PROTO_DTLS) /* DTLS only */ +#define MBEDTLS_SSL_TRANSPORT_IS_TLS( transport ) 0 +#define MBEDTLS_SSL_TRANSPORT_IS_DTLS( transport ) 1 +#define MBEDTLS_SSL_TRANSPORT_ELSE /* empty: no other branch */ +#else /* TLS only */ +#define MBEDTLS_SSL_TRANSPORT_IS_TLS( transport ) 1 +#define MBEDTLS_SSL_TRANSPORT_IS_DTLS( transport ) 0 +#define MBEDTLS_SSL_TRANSPORT_ELSE /* empty: no other branch */ +#endif /* TLS and/or DTLS */ + #ifdef __cplusplus extern "C" { #endif @@ -905,12 +956,14 @@ static inline size_t mbedtls_ssl_out_hdr_len( const mbedtls_ssl_context *ssl ) static inline size_t mbedtls_ssl_hs_hdr_len( const mbedtls_ssl_context *ssl ) { -#if defined(MBEDTLS_SSL_PROTO_DTLS) - if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM ) - return( 12 ); -#else +#if !defined(MBEDTLS_SSL_PROTO__BOTH) ((void) ssl); #endif + +#if defined(MBEDTLS_SSL_PROTO_DTLS) + if( MBEDTLS_SSL_TRANSPORT_IS_DTLS( ssl->conf->transport ) ) + return( 12 ); +#endif return( 4 ); } diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 9afcc96e80..4cfef3f574 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -3023,7 +3023,7 @@ int mbedtls_ssl_fetch_input( mbedtls_ssl_context *ssl, size_t nb_want ) } #if defined(MBEDTLS_SSL_PROTO_DTLS) - if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM ) + if( MBEDTLS_SSL_TRANSPORT_IS_DTLS( ssl->conf->transport ) ) { uint32_t timeout; @@ -3164,8 +3164,9 @@ int mbedtls_ssl_fetch_input( mbedtls_ssl_context *ssl, size_t nb_want ) ssl->in_left = ret; } - else -#endif + MBEDTLS_SSL_TRANSPORT_ELSE +#endif /* MBEDTLS_SSL_PROTO_DTLS */ +#if defined(MBEDTLS_SSL_PROTO_TLS) { MBEDTLS_SSL_DEBUG_MSG( 2, ( "in_left: %d, nb_want: %d", ssl->in_left, nb_want ) ); @@ -3212,6 +3213,7 @@ int mbedtls_ssl_fetch_input( mbedtls_ssl_context *ssl, size_t nb_want ) ssl->in_left += ret; } } +#endif /* MBEDTLS_SSL_PROTO_TLS */ MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= fetch input" ) ); From ff4bd9f405fb32a96e4dc2e21913f18583e132b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 6 Jun 2019 10:34:48 +0200 Subject: [PATCH 4/8] Use new tools for all cases with TLS-specific code This commit handles occurrences of case 2 and 3 in the following list: 1. Some DTLS-specific code with no TLS-specific code (most frequent) 2. Some specific code for each protocol 3. Some TLS-specific code with no DTLS-specific code (least frequent) Case 3 previously had a weird structure in that the TLS-specific code was always present, but the if structure was conditional on DTLS being enabled. This is changed by this commit to a more logical structure where both the code and the test are conditional on TLS being enabled. Case 2 doesn't require any change in the code structure in general. However, there is one occurrence where the if/else structure is simplified to assigning the result of a boolean operation, and one occurrence where I also noticed a useless use of `ssl_ep_len()` in a TLS-specific branch, that I turned to the constant 0 as it makes more sense. Case 1 will be handled in the next commit, as it can easily be handled in an automated way - only cases 2 and 3 (sometimes) required manual intervention. The list of occurrences for cases 2 and 3 was established manually by looking for occurrences of '= MBEDTLS_SSL_TRANSPORT_' in the code and manually checking if there was a TLS-specific branch. New sizes (see previous commit for the measuring script): ``` both text data bss dec hex filename 1820 0 4 1824 720 debug.o (ex library/libmbedtls.a) 0 0 0 0 0 net_sockets.o (ex library/libmbedtls.a) 548 0 0 548 224 ssl_cache.o (ex library/libmbedtls.a) 11155 0 596 11751 2de7 ssl_ciphersuites.o (ex library/libmbedtls.a) 17156 0 0 17156 4304 ssl_cli.o (ex library/libmbedtls.a) 460 0 0 460 1cc ssl_cookie.o (ex library/libmbedtls.a) 17649 0 0 17649 44f1 ssl_srv.o (ex library/libmbedtls.a) 800 0 0 800 320 ssl_ticket.o (ex library/libmbedtls.a) 39286 60 0 39346 99b2 ssl_tls.o (ex library/libmbedtls.a) 88874 60 600 89534 15dbe (TOTALS) DTLS-only text data bss dec hex filename 1820 0 4 1824 720 debug.o (ex library/libmbedtls.a) 0 0 0 0 0 net_sockets.o (ex library/libmbedtls.a) 548 0 0 548 224 ssl_cache.o (ex library/libmbedtls.a) 11155 0 596 11751 2de7 ssl_ciphersuites.o (ex library/libmbedtls.a) 17068 0 0 17068 42ac ssl_cli.o (ex library/libmbedtls.a) 460 0 0 460 1cc ssl_cookie.o (ex library/libmbedtls.a) 17553 0 0 17553 4491 ssl_srv.o (ex library/libmbedtls.a) 800 0 0 800 320 ssl_ticket.o (ex library/libmbedtls.a) 38499 60 0 38559 969f ssl_tls.o (ex library/libmbedtls.a) 87903 60 600 88563 159f3 (TOTALS) TLS-only text data bss dec hex filename 1820 0 4 1824 720 debug.o (ex library/libmbedtls.a) 0 0 0 0 0 net_sockets.o (ex library/libmbedtls.a) 548 0 0 548 224 ssl_cache.o (ex library/libmbedtls.a) 11155 0 596 11751 2de7 ssl_ciphersuites.o (ex library/libmbedtls.a) 14912 0 0 14912 3a40 ssl_cli.o (ex library/libmbedtls.a) 460 0 0 460 1cc ssl_cookie.o (ex library/libmbedtls.a) 15868 0 0 15868 3dfc ssl_srv.o (ex library/libmbedtls.a) 800 0 0 800 320 ssl_ticket.o (ex library/libmbedtls.a) 27619 60 0 27679 6c1f ssl_tls.o (ex library/libmbedtls.a) 73182 60 600 73842 12072 (TOTALS) ``` --- library/ssl_cli.c | 7 +-- library/ssl_srv.c | 33 +++++++---- library/ssl_tls.c | 147 +++++++++++++++++++++++++++++----------------- 3 files changed, 116 insertions(+), 71 deletions(-) diff --git a/library/ssl_cli.c b/library/ssl_cli.c index be80de71d0..0c587e9d4a 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -1762,12 +1762,7 @@ static int ssl_parse_server_hello( mbedtls_ssl_context *ssl ) #if defined(MBEDTLS_ZLIB_SUPPORT) /* See comments in ssl_write_client_hello() */ -#if defined(MBEDTLS_SSL_PROTO_DTLS) - if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM ) - accept_comp = 0; - else -#endif - accept_comp = 1; + accept_comp = MBEDTLS_SSL_TRANSPORT_IS_TLS( ssl->conf->transport ); if( comp != MBEDTLS_SSL_COMPRESS_NULL && ( comp != MBEDTLS_SSL_COMPRESS_DEFLATE || accept_comp == 0 ) ) diff --git a/library/ssl_srv.c b/library/ssl_srv.c index c152bc3a8a..3704f6ad98 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -1304,12 +1304,13 @@ read_record_header: buf = ssl->in_hdr; -#if defined(MBEDTLS_SSL_SRV_SUPPORT_SSLV2_CLIENT_HELLO) -#if defined(MBEDTLS_SSL_PROTO_DTLS) - if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_STREAM ) -#endif - if( ( buf[0] & 0x80 ) != 0 ) - return( ssl_parse_client_hello_v2( ssl ) ); +#if defined(MBEDTLS_SSL_SRV_SUPPORT_SSLV2_CLIENT_HELLO) && \ + defined(MBEDTLS_SSL_PROTO_TLS) + if( MBEDTLS_SSL_TRANSPORT_IS_TLS( ssl->conf->transport ) && + ( buf[0] & 0x80 ) != 0 ) + { + return( ssl_parse_client_hello_v2( ssl ) ); + } #endif MBEDTLS_SSL_DEBUG_BUF( 4, "record header", buf, mbedtls_ssl_in_hdr_len( ssl ) ); @@ -1407,13 +1408,19 @@ read_record_header: return( ret ); } - /* Done reading this record, get ready for the next one */ + /* Done reading this record, get ready for the next one */ #if defined(MBEDTLS_SSL_PROTO_DTLS) - if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM ) + if( MBEDTLS_SSL_TRANSPORT_IS_DTLS( ssl->conf->transport ) ) + { ssl->next_record_offset = msg_len + mbedtls_ssl_in_hdr_len( ssl ); - else + } + MBEDTLS_SSL_TRANSPORT_ELSE #endif +#if defined(MBEDTLS_SSL_PROTO_TLS) + { ssl->in_left = 0; + } +#endif } buf = ssl->in_msg; @@ -1595,7 +1602,7 @@ read_record_header: * Check the cookie length and content */ #if defined(MBEDTLS_SSL_PROTO_DTLS) - if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM ) + if( MBEDTLS_SSL_TRANSPORT_IS_DTLS( ssl->conf->transport ) ) { cookie_offset = 35 + sess_len; cookie_len = buf[cookie_offset]; @@ -1650,9 +1657,13 @@ read_record_header: */ ciph_offset = cookie_offset + 1 + cookie_len; } - else + MBEDTLS_SSL_TRANSPORT_ELSE #endif /* MBEDTLS_SSL_PROTO_DTLS */ +#if defined(MBEDTLS_SSL_PROTO_TLS) + { ciph_offset = 35 + sess_len; + } +#endif ciph_len = ( buf[ciph_offset + 0] << 8 ) | ( buf[ciph_offset + 1] ); diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 4cfef3f574..7d56f66761 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -61,12 +61,14 @@ static uint32_t ssl_get_hs_total_len( mbedtls_ssl_context const *ssl ); /* Length of the "epoch" field in the record header */ static inline size_t ssl_ep_len( const mbedtls_ssl_context *ssl ) { -#if defined(MBEDTLS_SSL_PROTO_DTLS) - if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM ) - return( 2 ); -#else +#if !defined(MBEDTLS_SSL_TRANSPORT__BOTH) ((void) ssl); #endif + +#if defined(MBEDTLS_SSL_PROTO_DTLS) + if( MBEDTLS_SSL_TRANSPORT_IS_DTLS( ssl->conf->transport ) ) + return( 2 ); +#endif return( 0 ); } @@ -3269,15 +3271,17 @@ int mbedtls_ssl_flush_output( mbedtls_ssl_context *ssl ) } #if defined(MBEDTLS_SSL_PROTO_DTLS) - if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM ) + if( MBEDTLS_SSL_TRANSPORT_IS_DTLS( ssl->conf->transport ) ) { ssl->out_hdr = ssl->out_buf; } - else + MBEDTLS_SSL_TRANSPORT_ELSE #endif +#if defined(MBEDTLS_SSL_PROTO_TLS) { ssl->out_hdr = ssl->out_buf + 8; } +#endif ssl_update_out_pointers( ssl, ssl->transform_out ); MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= flush output" ) ); @@ -4137,7 +4141,7 @@ int mbedtls_ssl_prepare_handshake_record( mbedtls_ssl_context *ssl ) ssl->in_msglen, ssl->in_msg[0], ssl->in_hslen ) ); #if defined(MBEDTLS_SSL_PROTO_DTLS) - if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM ) + if( MBEDTLS_SSL_TRANSPORT_IS_DTLS( ssl->conf->transport ) ) { int ret; unsigned int recv_msg_seq = ( ssl->in_msg[4] << 8 ) | ssl->in_msg[5]; @@ -4201,14 +4205,18 @@ int mbedtls_ssl_prepare_handshake_record( mbedtls_ssl_context *ssl ) return( MBEDTLS_ERR_SSL_EARLY_MESSAGE ); } } - else + MBEDTLS_SSL_TRANSPORT_ELSE #endif /* MBEDTLS_SSL_PROTO_DTLS */ - /* With TLS we don't handle fragmentation (for now) */ - if( ssl->in_msglen < ssl->in_hslen ) +#if defined(MBEDTLS_SSL_PROTO_TLS) { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "TLS handshake fragmentation not supported" ) ); - return( MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE ); + /* With TLS we don't handle fragmentation (for now) */ + if( ssl->in_msglen < ssl->in_hslen ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "TLS handshake fragmentation not supported" ) ); + return( MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE ); + } } +#endif return( 0 ); } @@ -4604,13 +4612,15 @@ static int ssl_parse_record_header( mbedtls_ssl_context *ssl ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "unknown record type" ) ); -#if defined(MBEDTLS_SSL_PROTO_DTLS) +#if defined(MBEDTLS_SSL_PROTO_TLS) /* Silently ignore invalid DTLS records as recommended by RFC 6347 - * Section 4.1.2.7 */ - if( ssl->conf->transport != MBEDTLS_SSL_TRANSPORT_DATAGRAM ) -#endif /* MBEDTLS_SSL_PROTO_DTLS */ + * Section 4.1.2.7, that is, send alert only with TLS */ + if( MBEDTLS_SSL_TRANSPORT_IS_TLS( ssl->conf->transport ) ) + { mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, MBEDTLS_SSL_ALERT_MSG_UNEXPECTED_MESSAGE ); + } +#endif /* MBEDTLS_SSL_PROTO_TLS */ return( MBEDTLS_ERR_SSL_INVALID_RECORD ); } @@ -4906,26 +4916,23 @@ static int ssl_prepare_record_content( mbedtls_ssl_context *ssl ) else ssl->nb_zero = 0; -#if defined(MBEDTLS_SSL_PROTO_DTLS) - if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM ) - { - ; /* in_ctr read from peer, not maintained internally */ - } - else -#endif + /* Only needed for TLS, as with DTLS in_ctr is read from the header */ +#if defined(MBEDTLS_SSL_PROTO_TLS) + if( MBEDTLS_SSL_TRANSPORT_IS_TLS( ssl->conf->transport ) ) { unsigned i; - for( i = 8; i > ssl_ep_len( ssl ); i-- ) + for( i = 8; i > 0; i-- ) if( ++ssl->in_ctr[i - 1] != 0 ) break; /* The loop goes to its end iff the counter is wrapping */ - if( i == ssl_ep_len( ssl ) ) + if( i == 0 ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "incoming message counter would wrap" ) ); return( MBEDTLS_ERR_SSL_COUNTER_WRAPPING ); } } +#endif /* MBEDTLS_SSL_PROTO_TLS */ } @@ -5700,7 +5707,7 @@ static int ssl_get_next_record( mbedtls_ssl_context *ssl ) /* Done reading this record, get ready for the next one */ #if defined(MBEDTLS_SSL_PROTO_DTLS) - if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM ) + if( MBEDTLS_SSL_TRANSPORT_IS_DTLS( ssl->conf->transport ) ) { ssl->next_record_offset = ssl->in_msglen + mbedtls_ssl_in_hdr_len( ssl ); if( ssl->next_record_offset < ssl->in_left ) @@ -5708,14 +5715,18 @@ static int ssl_get_next_record( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_DEBUG_MSG( 3, ( "more than one record within datagram" ) ); } } - else + MBEDTLS_SSL_TRANSPORT_ELSE #endif +#if defined(MBEDTLS_SSL_PROTO_TLS) + { ssl->in_left = 0; + } +#endif if( ( ret = ssl_prepare_record_content( ssl ) ) != 0 ) { #if defined(MBEDTLS_SSL_PROTO_DTLS) - if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM ) + if( MBEDTLS_SSL_TRANSPORT_IS_DTLS( ssl->conf->transport ) ) { /* Silently discard invalid records */ if( ret == MBEDTLS_ERR_SSL_INVALID_MAC ) @@ -5758,8 +5769,9 @@ static int ssl_get_next_record( mbedtls_ssl_context *ssl ) return( ret ); } - else -#endif + MBEDTLS_SSL_TRANSPORT_ELSE +#endif /* MBEDTLS_SSL_PROTO_DTLS */ +#if defined(MBEDTLS_SSL_PROTO_TLS) { /* Error out (and send alert) on invalid records */ #if defined(MBEDTLS_SSL_ALL_ALERT_MESSAGES) @@ -5772,6 +5784,7 @@ static int ssl_get_next_record( mbedtls_ssl_context *ssl ) #endif return( ret ); } +#endif /* MBEDTLS_SSL_PROTO_TLS */ } return( 0 ); @@ -6614,7 +6627,7 @@ int mbedtls_ssl_parse_change_cipher_spec( mbedtls_ssl_context *ssl ) ssl->session_in = ssl->session_negotiate; #if defined(MBEDTLS_SSL_PROTO_DTLS) - if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM ) + if( MBEDTLS_SSL_TRANSPORT_IS_DTLS( ssl->conf->transport ) ) { #if defined(MBEDTLS_SSL_DTLS_ANTI_REPLAY) ssl_dtls_replay_reset( ssl ); @@ -6629,9 +6642,13 @@ int mbedtls_ssl_parse_change_cipher_spec( mbedtls_ssl_context *ssl ) return( MBEDTLS_ERR_SSL_COUNTER_WRAPPING ); } } - else + MBEDTLS_SSL_TRANSPORT_ELSE #endif /* MBEDTLS_SSL_PROTO_DTLS */ - memset( ssl->in_ctr, 0, 8 ); +#if defined(MBEDTLS_SSL_PROTO_TLS) + { + memset( ssl->in_ctr, 0, 8 ); + } +#endif ssl_update_in_pointers( ssl ); @@ -7130,7 +7147,7 @@ int mbedtls_ssl_write_finished( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_DEBUG_MSG( 3, ( "switching to new transform spec for outbound data" ) ); #if defined(MBEDTLS_SSL_PROTO_DTLS) - if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM ) + if( MBEDTLS_SSL_TRANSPORT_IS_DTLS( ssl->conf->transport ) ) { unsigned char i; @@ -7153,9 +7170,13 @@ int mbedtls_ssl_write_finished( mbedtls_ssl_context *ssl ) return( MBEDTLS_ERR_SSL_COUNTER_WRAPPING ); } } - else + MBEDTLS_SSL_TRANSPORT_ELSE #endif /* MBEDTLS_SSL_PROTO_DTLS */ - memset( ssl->cur_out_ctr, 0, 8 ); +#if defined(MBEDTLS_SSL_PROTO_TLS) + { + memset( ssl->cur_out_ctr, 0, 8 ); + } +#endif ssl->transform_out = ssl->transform_negotiate; ssl->session_out = ssl->session_negotiate; @@ -7461,7 +7482,7 @@ static void ssl_update_out_pointers( mbedtls_ssl_context *ssl, mbedtls_ssl_transform *transform ) { #if defined(MBEDTLS_SSL_PROTO_DTLS) - if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM ) + if( MBEDTLS_SSL_TRANSPORT_IS_DTLS( ssl->conf->transport ) ) { ssl->out_ctr = ssl->out_hdr + 3; #if defined(MBEDTLS_SSL_DTLS_CONNECTION_ID) @@ -7474,8 +7495,9 @@ static void ssl_update_out_pointers( mbedtls_ssl_context *ssl, #endif /* MBEDTLS_SSL_DTLS_CONNECTION_ID */ ssl->out_iv = ssl->out_len + 2; } - else -#endif + MBEDTLS_SSL_TRANSPORT_ELSE +#endif /* MBEDTLS_SSL_PROTO_DTLS */ +#if defined(MBEDTLS_SSL_PROTO_TLS) { ssl->out_ctr = ssl->out_hdr - 8; ssl->out_len = ssl->out_hdr + 3; @@ -7484,6 +7506,7 @@ static void ssl_update_out_pointers( mbedtls_ssl_context *ssl, #endif ssl->out_iv = ssl->out_hdr + 5; } +#endif /* MBEDTLS_SSL_PROTO_TLS */ /* Adjust out_msg to make space for explicit IV, if used. */ if( transform != NULL && @@ -7516,7 +7539,7 @@ static void ssl_update_in_pointers( mbedtls_ssl_context *ssl ) */ #if defined(MBEDTLS_SSL_PROTO_DTLS) - if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM ) + if( MBEDTLS_SSL_TRANSPORT_IS_DTLS( ssl->conf->transport ) ) { /* This sets the header pointers to match records * without CID. When we receive a record containing @@ -7531,8 +7554,9 @@ static void ssl_update_in_pointers( mbedtls_ssl_context *ssl ) #endif /* MBEDTLS_SSL_DTLS_CONNECTION_ID */ ssl->in_iv = ssl->in_len + 2; } - else -#endif + MBEDTLS_SSL_TRANSPORT_ELSE +#endif /* MBEDTLS_SSL_PROTO_DTLS */ +#if defined(MBEDTLS_SSL_PROTO_TLS) { ssl->in_ctr = ssl->in_hdr - 8; ssl->in_len = ssl->in_hdr + 3; @@ -7541,6 +7565,7 @@ static void ssl_update_in_pointers( mbedtls_ssl_context *ssl ) #endif ssl->in_iv = ssl->in_hdr + 5; } +#endif /* MBEDTLS_SSL_PROTO_TLS */ /* This will be adjusted at record decryption time. */ ssl->in_msg = ssl->in_iv; @@ -7562,17 +7587,19 @@ static void ssl_reset_in_out_pointers( mbedtls_ssl_context *ssl ) { /* Set the incoming and outgoing record pointers. */ #if defined(MBEDTLS_SSL_PROTO_DTLS) - if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM ) + if( MBEDTLS_SSL_TRANSPORT_IS_DTLS( ssl->conf->transport ) ) { ssl->out_hdr = ssl->out_buf; ssl->in_hdr = ssl->in_buf; } - else + MBEDTLS_SSL_TRANSPORT_ELSE #endif /* MBEDTLS_SSL_PROTO_DTLS */ +#if defined(MBEDTLS_SSL_PROTO_TLS) { ssl->out_hdr = ssl->out_buf + 8; ssl->in_hdr = ssl->in_buf + 8; } +#endif /* MBEDTLS_SSL_PROTO_TLS */ /* Derive other internal pointers. */ ssl_update_out_pointers( ssl, NULL /* no transform enabled */ ); @@ -9795,16 +9822,20 @@ static int ssl_write_real( mbedtls_ssl_context *ssl, if( len > max_len ) { #if defined(MBEDTLS_SSL_PROTO_DTLS) - if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM ) + if( MBEDTLS_SSL_TRANSPORT_IS_DTLS( ssl->conf->transport ) ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "fragment larger than the (negotiated) " "maximum fragment length: %d > %d", len, max_len ) ); return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA ); } - else + MBEDTLS_SSL_TRANSPORT_ELSE #endif +#if defined(MBEDTLS_SSL_PROTO_TLS) + { len = max_len; + } +#endif } if( ssl->out_left != 0 ) @@ -10770,8 +10801,12 @@ int mbedtls_ssl_check_cert_usage( const mbedtls_x509_crt *cert, void mbedtls_ssl_write_version( int major, int minor, int transport, unsigned char ver[2] ) { +#if !defined(MBEDTLS_SSL_TRANSPORT__BOTH) + ((void) transport); +#endif + #if defined(MBEDTLS_SSL_PROTO_DTLS) - if( transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM ) + if( MBEDTLS_SSL_TRANSPORT_IS_DTLS( transport ) ) { if( minor == MBEDTLS_SSL_MINOR_VERSION_2 ) --minor; /* DTLS 1.0 stored as TLS 1.1 internally */ @@ -10779,21 +10814,25 @@ void mbedtls_ssl_write_version( int major, int minor, int transport, ver[0] = (unsigned char)( 255 - ( major - 2 ) ); ver[1] = (unsigned char)( 255 - ( minor - 1 ) ); } - else -#else - ((void) transport); + MBEDTLS_SSL_TRANSPORT_ELSE #endif +#if defined(MBEDTLS_SSL_PROTO_TLS) { ver[0] = (unsigned char) major; ver[1] = (unsigned char) minor; } +#endif } void mbedtls_ssl_read_version( int *major, int *minor, int transport, const unsigned char ver[2] ) { +#if !defined(MBEDTLS_SSL_TRANSPORT__BOTH) + ((void) transport); +#endif + #if defined(MBEDTLS_SSL_PROTO_DTLS) - if( transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM ) + if( MBEDTLS_SSL_TRANSPORT_IS_DTLS( transport ) ) { *major = 255 - ver[0] + 2; *minor = 255 - ver[1] + 1; @@ -10801,14 +10840,14 @@ void mbedtls_ssl_read_version( int *major, int *minor, int transport, if( *minor == MBEDTLS_SSL_MINOR_VERSION_1 ) ++*minor; /* DTLS 1.0 stored as TLS 1.1 internally */ } - else -#else - ((void) transport); + MBEDTLS_SSL_TRANSPORT_ELSE #endif +#if defined(MBEDTLS_SSL_PROTO_TLS) { *major = ver[0]; *minor = ver[1]; } +#endif } int mbedtls_ssl_set_calc_verify_md( mbedtls_ssl_context *ssl, int md ) From 64c1681fbcbed64fc1af007b1d309e1ad552eeff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 6 Jun 2019 12:43:51 +0200 Subject: [PATCH 5/8] Use new macros for all TLS/DTLS tests sed -i -e 's/\([^ ]*transport\) == MBEDTLS_SSL_TRANSPORT_DATAGRAM/MBEDTLS_SSL_TRANSPORT_IS_DTLS( \1 )/' -e 's/\([^ ]*transport\) \(!= MBEDTLS_SSL_TRANSPORT_DATAGRAM\|== MBEDTLS_SSL_TRANSPORT_STREAM\)/MBEDTLS_SSL_TRANSPORT_IS_TLS( \1 )/' library/ssl_*.c New sizes (see 2nd-previous commit for measuring script): ``` both text data bss dec hex filename 1820 0 4 1824 720 debug.o (ex library/libmbedtls.a) 0 0 0 0 0 net_sockets.o (ex library/libmbedtls.a) 548 0 0 548 224 ssl_cache.o (ex library/libmbedtls.a) 11155 0 596 11751 2de7 ssl_ciphersuites.o (ex library/libmbedtls.a) 17156 0 0 17156 4304 ssl_cli.o (ex library/libmbedtls.a) 460 0 0 460 1cc ssl_cookie.o (ex library/libmbedtls.a) 17649 0 0 17649 44f1 ssl_srv.o (ex library/libmbedtls.a) 800 0 0 800 320 ssl_ticket.o (ex library/libmbedtls.a) 39286 60 0 39346 99b2 ssl_tls.o (ex library/libmbedtls.a) 88874 60 600 89534 15dbe (TOTALS) DTLS-only text data bss dec hex filename 1820 0 4 1824 720 debug.o (ex library/libmbedtls.a) 0 0 0 0 0 net_sockets.o (ex library/libmbedtls.a) 548 0 0 548 224 ssl_cache.o (ex library/libmbedtls.a) 11155 0 596 11751 2de7 ssl_ciphersuites.o (ex library/libmbedtls.a) 16948 0 0 16948 4234 ssl_cli.o (ex library/libmbedtls.a) 460 0 0 460 1cc ssl_cookie.o (ex library/libmbedtls.a) 17437 0 0 17437 441d ssl_srv.o (ex library/libmbedtls.a) 800 0 0 800 320 ssl_ticket.o (ex library/libmbedtls.a) 38147 60 0 38207 953f ssl_tls.o (ex library/libmbedtls.a) 87315 60 600 87975 157a7 (TOTALS) TLS-only text data bss dec hex filename 1820 0 4 1824 720 debug.o (ex library/libmbedtls.a) 0 0 0 0 0 net_sockets.o (ex library/libmbedtls.a) 548 0 0 548 224 ssl_cache.o (ex library/libmbedtls.a) 11155 0 596 11751 2de7 ssl_ciphersuites.o (ex library/libmbedtls.a) 14912 0 0 14912 3a40 ssl_cli.o (ex library/libmbedtls.a) 460 0 0 460 1cc ssl_cookie.o (ex library/libmbedtls.a) 15868 0 0 15868 3dfc ssl_srv.o (ex library/libmbedtls.a) 800 0 0 800 320 ssl_ticket.o (ex library/libmbedtls.a) 27619 60 0 27679 6c1f ssl_tls.o (ex library/libmbedtls.a) 73182 60 600 73842 12072 (TOTALS) ``` --- library/ssl_cli.c | 22 +++++++++--------- library/ssl_srv.c | 22 +++++++++--------- library/ssl_tls.c | 58 +++++++++++++++++++++++------------------------ 3 files changed, 51 insertions(+), 51 deletions(-) diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 0c587e9d4a..214ac5e943 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -452,7 +452,7 @@ static void ssl_write_cid_ext( mbedtls_ssl_context *ssl, */ *olen = 0; - if( ssl->conf->transport != MBEDTLS_SSL_TRANSPORT_DATAGRAM || + if( MBEDTLS_SSL_TRANSPORT_IS_TLS( ssl->conf->transport ) || ssl->negotiate_cid == MBEDTLS_SSL_CID_DISABLED ) { return; @@ -734,7 +734,7 @@ static int ssl_generate_random( mbedtls_ssl_context *ssl ) * When responding to a verify request, MUST reuse random (RFC 6347 4.2.1) */ #if defined(MBEDTLS_SSL_PROTO_DTLS) - if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM && + if( MBEDTLS_SSL_TRANSPORT_IS_DTLS( ssl->conf->transport ) && ssl->handshake->verify_cookie != NULL ) { return( 0 ); @@ -785,7 +785,7 @@ static int ssl_validate_ciphersuite( const mbedtls_ssl_ciphersuite_t * suite_inf return( 1 ); #if defined(MBEDTLS_SSL_PROTO_DTLS) - if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM && + if( MBEDTLS_SSL_TRANSPORT_IS_DTLS( ssl->conf->transport ) && ( suite_info->flags & MBEDTLS_CIPHERSUITE_NODTLS ) ) return( 1 ); #endif @@ -926,7 +926,7 @@ static int ssl_write_client_hello( mbedtls_ssl_context *ssl ) * DTLS cookie */ #if defined(MBEDTLS_SSL_PROTO_DTLS) - if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM ) + if( MBEDTLS_SSL_TRANSPORT_IS_DTLS( ssl->conf->transport ) ) { if( ssl->handshake->verify_cookie == NULL ) { @@ -1021,7 +1021,7 @@ static int ssl_write_client_hello( mbedtls_ssl_context *ssl ) * an actual need for it. */ #if defined(MBEDTLS_SSL_PROTO_DTLS) - if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM ) + if( MBEDTLS_SSL_TRANSPORT_IS_DTLS( ssl->conf->transport ) ) offer_compress = 0; #endif @@ -1137,7 +1137,7 @@ static int ssl_write_client_hello( mbedtls_ssl_context *ssl ) ssl->state++; #if defined(MBEDTLS_SSL_PROTO_DTLS) - if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM ) + if( MBEDTLS_SSL_TRANSPORT_IS_DTLS( ssl->conf->transport ) ) mbedtls_ssl_send_flight_completed( ssl ); #endif @@ -1148,7 +1148,7 @@ static int ssl_write_client_hello( mbedtls_ssl_context *ssl ) } #if defined(MBEDTLS_SSL_PROTO_DTLS) - if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM && + if( MBEDTLS_SSL_TRANSPORT_IS_DTLS( ssl->conf->transport ) && ( ret = mbedtls_ssl_flight_transmit( ssl ) ) != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_flight_transmit", ret ); @@ -1252,7 +1252,7 @@ static int ssl_parse_cid_ext( mbedtls_ssl_context *ssl, size_t peer_cid_len; if( /* CID extension only makes sense in DTLS */ - ssl->conf->transport != MBEDTLS_SSL_TRANSPORT_DATAGRAM || + MBEDTLS_SSL_TRANSPORT_IS_TLS( ssl->conf->transport ) || /* The server must only send the CID extension if we have offered it. */ ssl->negotiate_cid == MBEDTLS_SSL_CID_DISABLED ) { @@ -1645,7 +1645,7 @@ static int ssl_parse_server_hello( mbedtls_ssl_context *ssl ) } #if defined(MBEDTLS_SSL_PROTO_DTLS) - if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM ) + if( MBEDTLS_SSL_TRANSPORT_IS_DTLS( ssl->conf->transport ) ) { if( buf[0] == MBEDTLS_SSL_HS_HELLO_VERIFY_REQUEST ) { @@ -2996,7 +2996,7 @@ static int ssl_parse_server_hello_done( mbedtls_ssl_context *ssl ) ssl->state++; #if defined(MBEDTLS_SSL_PROTO_DTLS) - if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM ) + if( MBEDTLS_SSL_TRANSPORT_IS_DTLS( ssl->conf->transport ) ) mbedtls_ssl_recv_flight_completed( ssl ); #endif @@ -3628,7 +3628,7 @@ int mbedtls_ssl_handshake_client_step( mbedtls_ssl_context *ssl ) return( ret ); #if defined(MBEDTLS_SSL_PROTO_DTLS) - if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM && + if( MBEDTLS_SSL_TRANSPORT_IS_DTLS( ssl->conf->transport ) && ssl->handshake->retransmit_state == MBEDTLS_SSL_RETRANS_SENDING ) { if( ( ret = mbedtls_ssl_flight_transmit( ssl ) ) != 0 ) diff --git a/library/ssl_srv.c b/library/ssl_srv.c index 3704f6ad98..fda0324eac 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -441,7 +441,7 @@ static int ssl_parse_cid_ext( mbedtls_ssl_context *ssl, size_t peer_cid_len; /* CID extension only makes sense in DTLS */ - if( ssl->conf->transport != MBEDTLS_SSL_TRANSPORT_DATAGRAM ) + if( MBEDTLS_SSL_TRANSPORT_IS_TLS( ssl->conf->transport ) ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad client hello message" ) ); mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, @@ -899,7 +899,7 @@ static int ssl_ciphersuite_match( mbedtls_ssl_context *ssl, int suite_id, } #if defined(MBEDTLS_SSL_PROTO_DTLS) - if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM && + if( MBEDTLS_SSL_TRANSPORT_IS_DTLS( ssl->conf->transport ) && ( suite_info->flags & MBEDTLS_CIPHERSUITE_NODTLS ) ) return( 0 ); #endif @@ -1354,7 +1354,7 @@ read_record_header: /* For DTLS if this is the initial handshake, remember the client sequence * number to use it in our next message (RFC 6347 4.2.1) */ #if defined(MBEDTLS_SSL_PROTO_DTLS) - if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM + if( MBEDTLS_SSL_TRANSPORT_IS_DTLS( ssl->conf->transport ) #if defined(MBEDTLS_SSL_RENEGOTIATION) && ssl->renego_status == MBEDTLS_SSL_INITIAL_HANDSHAKE #endif @@ -1463,7 +1463,7 @@ read_record_header: } #if defined(MBEDTLS_SSL_PROTO_DTLS) - if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM ) + if( MBEDTLS_SSL_TRANSPORT_IS_DTLS( ssl->conf->transport ) ) { /* * Copy the client's handshake message_seq on initial handshakes, @@ -1715,7 +1715,7 @@ read_record_header: /* See comments in ssl_write_client_hello() */ #if defined(MBEDTLS_SSL_PROTO_DTLS) - if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM ) + if( MBEDTLS_SSL_TRANSPORT_IS_DTLS( ssl->conf->transport ) ) ssl->session_negotiate->compression = MBEDTLS_SSL_COMPRESS_NULL; #endif @@ -2097,7 +2097,7 @@ have_ciphersuite: ssl->state++; #if defined(MBEDTLS_SSL_PROTO_DTLS) - if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM ) + if( MBEDTLS_SSL_TRANSPORT_IS_DTLS( ssl->conf->transport ) ) mbedtls_ssl_recv_flight_completed( ssl ); #endif @@ -2532,7 +2532,7 @@ static int ssl_write_hello_verify_request( mbedtls_ssl_context *ssl ) } #if defined(MBEDTLS_SSL_PROTO_DTLS) - if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM && + if( MBEDTLS_SSL_TRANSPORT_IS_DTLS( ssl->conf->transport ) && ( ret = mbedtls_ssl_flight_transmit( ssl ) ) != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_flight_transmit", ret ); @@ -2558,7 +2558,7 @@ static int ssl_write_server_hello( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> write server hello" ) ); #if defined(MBEDTLS_SSL_DTLS_HELLO_VERIFY) - if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM && + if( MBEDTLS_SSL_TRANSPORT_IS_DTLS( ssl->conf->transport ) && ssl->handshake->verify_cookie_len != 0 ) { MBEDTLS_SSL_DEBUG_MSG( 2, ( "client hello was not authenticated" ) ); @@ -3516,7 +3516,7 @@ static int ssl_write_server_hello_done( mbedtls_ssl_context *ssl ) ssl->state++; #if defined(MBEDTLS_SSL_PROTO_DTLS) - if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM ) + if( MBEDTLS_SSL_TRANSPORT_IS_DTLS( ssl->conf->transport ) ) mbedtls_ssl_send_flight_completed( ssl ); #endif @@ -3527,7 +3527,7 @@ static int ssl_write_server_hello_done( mbedtls_ssl_context *ssl ) } #if defined(MBEDTLS_SSL_PROTO_DTLS) - if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM && + if( MBEDTLS_SSL_TRANSPORT_IS_DTLS( ssl->conf->transport ) && ( ret = mbedtls_ssl_flight_transmit( ssl ) ) != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_flight_transmit", ret ); @@ -4412,7 +4412,7 @@ int mbedtls_ssl_handshake_server_step( mbedtls_ssl_context *ssl ) return( ret ); #if defined(MBEDTLS_SSL_PROTO_DTLS) - if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM && + if( MBEDTLS_SSL_TRANSPORT_IS_DTLS( ssl->conf->transport ) && ssl->handshake->retransmit_state == MBEDTLS_SSL_RETRANS_SENDING ) { if( ( ret = mbedtls_ssl_flight_transmit( ssl ) ) != 0 ) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 7d56f66761..bdf93abfb6 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -137,7 +137,7 @@ int mbedtls_ssl_set_cid( mbedtls_ssl_context *ssl, unsigned char const *own_cid, size_t own_cid_len ) { - if( ssl->conf->transport != MBEDTLS_SSL_TRANSPORT_DATAGRAM ) + if( MBEDTLS_SSL_TRANSPORT_IS_TLS( ssl->conf->transport ) ) return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA ); ssl->negotiate_cid = enable; @@ -172,7 +172,7 @@ int mbedtls_ssl_get_peer_cid( mbedtls_ssl_context *ssl, { *enabled = MBEDTLS_SSL_CID_DISABLED; - if( ssl->conf->transport != MBEDTLS_SSL_TRANSPORT_DATAGRAM || + if( MBEDTLS_SSL_TRANSPORT_IS_TLS( ssl->conf->transport ) || ssl->state != MBEDTLS_SSL_HANDSHAKE_OVER ) { return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA ); @@ -3692,7 +3692,7 @@ int mbedtls_ssl_write_handshake_msg( mbedtls_ssl_context *ssl ) } #if defined(MBEDTLS_SSL_PROTO_DTLS) - if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM && + if( MBEDTLS_SSL_TRANSPORT_IS_DTLS( ssl->conf->transport ) && ssl->handshake != NULL && ssl->handshake->retransmit_state == MBEDTLS_SSL_RETRANS_SENDING ) { @@ -3735,7 +3735,7 @@ int mbedtls_ssl_write_handshake_msg( mbedtls_ssl_context *ssl ) * uint24 fragment_length; */ #if defined(MBEDTLS_SSL_PROTO_DTLS) - if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM ) + if( MBEDTLS_SSL_TRANSPORT_IS_DTLS( ssl->conf->transport ) ) { /* Make room for the additional DTLS fields */ if( MBEDTLS_SSL_OUT_CONTENT_LEN - ssl->out_msglen < 8 ) @@ -3777,7 +3777,7 @@ int mbedtls_ssl_write_handshake_msg( mbedtls_ssl_context *ssl ) /* Either send now, or just save to be sent (and resent) later */ #if defined(MBEDTLS_SSL_PROTO_DTLS) - if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM && + if( MBEDTLS_SSL_TRANSPORT_IS_DTLS( ssl->conf->transport ) && ! ( ssl->out_msgtype == MBEDTLS_SSL_MSG_HANDSHAKE && hs_type == MBEDTLS_SSL_HS_HELLO_REQUEST ) ) { @@ -3915,7 +3915,7 @@ int mbedtls_ssl_write_record( mbedtls_ssl_context *ssl, uint8_t force_flush ) #if defined(MBEDTLS_SSL_PROTO_DTLS) /* In case of DTLS, double-check that we don't exceed * the remaining space in the datagram. */ - if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM ) + if( MBEDTLS_SSL_TRANSPORT_IS_DTLS( ssl->conf->transport ) ) { ret = ssl_get_remaining_space_in_datagram( ssl ); if( ret < 0 ) @@ -3957,7 +3957,7 @@ int mbedtls_ssl_write_record( mbedtls_ssl_context *ssl, uint8_t force_flush ) } #if defined(MBEDTLS_SSL_PROTO_DTLS) - if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM && + if( MBEDTLS_SSL_TRANSPORT_IS_DTLS( ssl->conf->transport ) && flush == SSL_DONT_FORCE_FLUSH ) { size_t remaining; @@ -4232,7 +4232,7 @@ void mbedtls_ssl_update_handshake_status( mbedtls_ssl_context *ssl ) /* Handshake message is complete, increment counter */ #if defined(MBEDTLS_SSL_PROTO_DTLS) - if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM && + if( MBEDTLS_SSL_TRANSPORT_IS_DTLS( ssl->conf->transport ) && ssl->handshake != NULL ) { unsigned offset; @@ -4584,7 +4584,7 @@ static int ssl_parse_record_header( mbedtls_ssl_context *ssl ) /* Check record type */ #if defined(MBEDTLS_SSL_DTLS_CONNECTION_ID) - if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM && + if( MBEDTLS_SSL_TRANSPORT_IS_DTLS( ssl->conf->transport ) && ssl->in_msgtype == MBEDTLS_SSL_MSG_CID && ssl->conf->cid_len != 0 ) { @@ -4681,7 +4681,7 @@ static int ssl_parse_record_header( mbedtls_ssl_context *ssl ) * record leads to the entire datagram being dropped. */ #if defined(MBEDTLS_SSL_PROTO_DTLS) - if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM ) + if( MBEDTLS_SSL_TRANSPORT_IS_DTLS( ssl->conf->transport ) ) { unsigned int rec_epoch = ( ssl->in_ctr[0] << 8 ) | ssl->in_ctr[1]; @@ -4949,7 +4949,7 @@ static int ssl_prepare_record_content( mbedtls_ssl_context *ssl ) #endif /* MBEDTLS_ZLIB_SUPPORT */ #if defined(MBEDTLS_SSL_DTLS_ANTI_REPLAY) - if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM ) + if( MBEDTLS_SSL_TRANSPORT_IS_DTLS( ssl->conf->transport ) ) { mbedtls_ssl_dtls_replay_update( ssl ); } @@ -4995,7 +4995,7 @@ int mbedtls_ssl_read_record( mbedtls_ssl_context *ssl, /* We only check for buffered messages if the * current datagram is fully consumed. */ - if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM && + if( MBEDTLS_SSL_TRANSPORT_IS_DTLS( ssl->conf->transport ) && ssl_next_record_is_in_datagram( ssl ) == 0 ) { if( ssl_load_buffered_message( ssl ) == 0 ) @@ -5518,7 +5518,7 @@ static int ssl_load_buffered_record( mbedtls_ssl_context *ssl ) size_t rec_len; unsigned rec_epoch; - if( ssl->conf->transport != MBEDTLS_SSL_TRANSPORT_DATAGRAM ) + if( MBEDTLS_SSL_TRANSPORT_IS_TLS( ssl->conf->transport ) ) return( 0 ); if( hs == NULL ) @@ -5656,7 +5656,7 @@ static int ssl_get_next_record( mbedtls_ssl_context *ssl ) if( ( ret = ssl_parse_record_header( ssl ) ) != 0 ) { #if defined(MBEDTLS_SSL_PROTO_DTLS) - if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM && + if( MBEDTLS_SSL_TRANSPORT_IS_DTLS( ssl->conf->transport ) && ret != MBEDTLS_ERR_SSL_CLIENT_RECONNECT ) { if( ret == MBEDTLS_ERR_SSL_EARLY_MESSAGE ) @@ -5822,7 +5822,7 @@ int mbedtls_ssl_handle_message_type( mbedtls_ssl_context *ssl ) } #if defined(MBEDTLS_SSL_PROTO_DTLS) - if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM && + if( MBEDTLS_SSL_TRANSPORT_IS_DTLS( ssl->conf->transport ) && ssl->state != MBEDTLS_SSL_CLIENT_CHANGE_CIPHER_SPEC && ssl->state != MBEDTLS_SSL_SERVER_CHANGE_CIPHER_SPEC ) { @@ -5897,7 +5897,7 @@ int mbedtls_ssl_handle_message_type( mbedtls_ssl_context *ssl ) } #if defined(MBEDTLS_SSL_PROTO_DTLS) - if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM ) + if( MBEDTLS_SSL_TRANSPORT_IS_DTLS( ssl->conf->transport ) ) { /* Drop unexpected ApplicationData records, * except at the beginning of renegotiations */ @@ -7076,7 +7076,7 @@ void mbedtls_ssl_handshake_wrapup( mbedtls_ssl_context *ssl ) } #if defined(MBEDTLS_SSL_PROTO_DTLS) - if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM && + if( MBEDTLS_SSL_TRANSPORT_IS_DTLS( ssl->conf->transport ) && ssl->handshake->flight != NULL ) { /* Cancel handshake timer */ @@ -7193,7 +7193,7 @@ int mbedtls_ssl_write_finished( mbedtls_ssl_context *ssl ) #endif #if defined(MBEDTLS_SSL_PROTO_DTLS) - if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM ) + if( MBEDTLS_SSL_TRANSPORT_IS_DTLS( ssl->conf->transport ) ) mbedtls_ssl_send_flight_completed( ssl ); #endif @@ -7204,7 +7204,7 @@ int mbedtls_ssl_write_finished( mbedtls_ssl_context *ssl ) } #if defined(MBEDTLS_SSL_PROTO_DTLS) - if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM && + if( MBEDTLS_SSL_TRANSPORT_IS_DTLS( ssl->conf->transport ) && ( ret = mbedtls_ssl_flight_transmit( ssl ) ) != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_flight_transmit", ret ); @@ -7293,7 +7293,7 @@ int mbedtls_ssl_parse_finished( mbedtls_ssl_context *ssl ) ssl->state++; #if defined(MBEDTLS_SSL_PROTO_DTLS) - if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM ) + if( MBEDTLS_SSL_TRANSPORT_IS_DTLS( ssl->conf->transport ) ) mbedtls_ssl_recv_flight_completed( ssl ); #endif @@ -7425,7 +7425,7 @@ static int ssl_handshake_init( mbedtls_ssl_context *ssl ) ssl_handshake_params_init( ssl->handshake ); #if defined(MBEDTLS_SSL_PROTO_DTLS) - if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM ) + if( MBEDTLS_SSL_TRANSPORT_IS_DTLS( ssl->conf->transport ) ) { ssl->handshake->alt_transform_out = ssl->transform_out; @@ -8525,7 +8525,7 @@ int mbedtls_ssl_check_pending( const mbedtls_ssl_context *ssl ) */ #if defined(MBEDTLS_SSL_PROTO_DTLS) - if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM && + if( MBEDTLS_SSL_TRANSPORT_IS_DTLS( ssl->conf->transport ) && ssl->in_left > ssl->next_record_offset ) { MBEDTLS_SSL_DEBUG_MSG( 3, ( "ssl_check_pending: more records within current datagram" ) ); @@ -8584,7 +8584,7 @@ const char *mbedtls_ssl_get_ciphersuite( const mbedtls_ssl_context *ssl ) const char *mbedtls_ssl_get_version( const mbedtls_ssl_context *ssl ) { #if defined(MBEDTLS_SSL_PROTO_DTLS) - if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM ) + if( MBEDTLS_SSL_TRANSPORT_IS_DTLS( ssl->conf->transport ) ) { switch( ssl->minor_ver ) { @@ -9379,7 +9379,7 @@ static int ssl_start_renegotiation( mbedtls_ssl_context *ssl ) /* RFC 6347 4.2.2: "[...] the HelloRequest will have message_seq = 0 and * the ServerHello will have message_seq = 1" */ #if defined(MBEDTLS_SSL_PROTO_DTLS) - if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM && + if( MBEDTLS_SSL_TRANSPORT_IS_DTLS( ssl->conf->transport ) && ssl->renego_status == MBEDTLS_SSL_RENEGOTIATION_PENDING ) { if( ssl->conf->endpoint == MBEDTLS_SSL_IS_SERVER ) @@ -9505,7 +9505,7 @@ int mbedtls_ssl_read( mbedtls_ssl_context *ssl, unsigned char *buf, size_t len ) MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> read" ) ); #if defined(MBEDTLS_SSL_PROTO_DTLS) - if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM ) + if( MBEDTLS_SSL_TRANSPORT_IS_DTLS( ssl->conf->transport ) ) { if( ( ret = mbedtls_ssl_flush_output( ssl ) ) != 0 ) return( ret ); @@ -9606,7 +9606,7 @@ int mbedtls_ssl_read( mbedtls_ssl_context *ssl, unsigned char *buf, size_t len ) /* With DTLS, drop the packet (probably from last handshake) */ #if defined(MBEDTLS_SSL_PROTO_DTLS) - if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM ) + if( MBEDTLS_SSL_TRANSPORT_IS_DTLS( ssl->conf->transport ) ) { continue; } @@ -9623,7 +9623,7 @@ int mbedtls_ssl_read( mbedtls_ssl_context *ssl, unsigned char *buf, size_t len ) /* With DTLS, drop the packet (probably from last handshake) */ #if defined(MBEDTLS_SSL_PROTO_DTLS) - if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM ) + if( MBEDTLS_SSL_TRANSPORT_IS_DTLS( ssl->conf->transport ) ) { continue; } @@ -9645,7 +9645,7 @@ int mbedtls_ssl_read( mbedtls_ssl_context *ssl, unsigned char *buf, size_t len ) /* DTLS clients need to know renego is server-initiated */ #if defined(MBEDTLS_SSL_PROTO_DTLS) - if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM && + if( MBEDTLS_SSL_TRANSPORT_IS_DTLS( ssl->conf->transport ) && ssl->conf->endpoint == MBEDTLS_SSL_IS_CLIENT ) { ssl->renego_status = MBEDTLS_SSL_RENEGOTIATION_PENDING; @@ -10428,7 +10428,7 @@ int mbedtls_ssl_config_defaults( mbedtls_ssl_config *conf, conf->max_minor_ver = MBEDTLS_SSL_MAX_MINOR_VERSION; #if defined(MBEDTLS_SSL_PROTO_DTLS) - if( transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM ) + if( MBEDTLS_SSL_TRANSPORT_IS_DTLS( transport ) ) conf->min_minor_ver = MBEDTLS_SSL_MINOR_VERSION_2; #endif From c6d9e3a28fb495688eeb4dbf090bab6611e76bbc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Fri, 7 Jun 2019 11:07:53 +0200 Subject: [PATCH 6/8] Clarify documentation of MBEDTLS_SSL_PROTO_TLS --- include/mbedtls/config.h | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/include/mbedtls/config.h b/include/mbedtls/config.h index 69f68dda4f..96cc98602c 100644 --- a/include/mbedtls/config.h +++ b/include/mbedtls/config.h @@ -1505,8 +1505,10 @@ * * Enable support for DTLS (all available versions). * - * Enable this and MBEDTLS_SSL_PROTO_TLS1_1 to enable DTLS 1.0, - * and/or this and MBEDTLS_SSL_PROTO_TLS1_2 to enable DTLS 1.2. + * 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 * * Requires: MBEDTLS_SSL_PROTO_TLS1_1 * or MBEDTLS_SSL_PROTO_TLS1_2 @@ -1518,15 +1520,19 @@ /** * \def MBEDTLS_SSL_PROTO_TLS * - * Enable support for TLS (all available versions). + * Enable support for SSL/TLS (all available versions). * - * Enable this and MBEDTLS_SSL_PROTO_TLS1 to enable TLS 1.0, - * Enable this and MBEDTLS_SSL_PROTO_TLS1_1 to enable TLS 1.1, - * and/or this and MBEDTLS_SSL_PROTO_TLS1_2 to enable TLS 1.2. + * 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). * - * Requires: MBEDTLS_SSL_PROTO_TLS1_1 + * \see MBEDTLS_SSL_PROTO_DTLS + * + * Requires: MBEDTLS_SSL_PROTO_TLS1_2 * or MBEDTLS_SSL_PROTO_TLS1_1 - * or MBEDTLS_SSL_PROTO_TLS1_2 + * or MBEDTLS_SSL_PROTO_TLS1 + * or MBEDTLS_SSL_PROTO_SSL3 (deprecated) * * Comment this macro to disable support for TLS */ From 8794a4290d6da9704d840e0e3795d7ce67d82db2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 11 Jun 2019 10:04:57 +0200 Subject: [PATCH 7/8] Clarify a few more comments and documentation --- include/mbedtls/ssl.h | 3 ++- include/mbedtls/ssl_internal.h | 6 +++--- library/ssl_tls.c | 4 ++-- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 8e3161c3dc..653f857cc7 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -1337,7 +1337,8 @@ void mbedtls_ssl_conf_endpoint( mbedtls_ssl_config *conf, int endpoint ); /** * \brief Set the transport type (TLS or DTLS). - * Default: TLS if both are enabled, or DTLS. + * Default: TLS if #MBEDTLS_SSL_PROTO_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 173b6d58b0..511715f4b6 100644 --- a/include/mbedtls/ssl_internal.h +++ b/include/mbedtls/ssl_internal.h @@ -269,8 +269,8 @@ * * Goals for these helpers: * - generate minimal code, eg don't test if mode is DTLS in a DTLS-only build - * - make the flow clear to the compiler, ie that in dual-mode builds, - * when there are two branchs, exactly one of them is taken + * - make the flow clear to the compiler, so that in TLS and DTLS combined + * builds, when there are two branches, it knows exactly one of them is taken * - preserve readability * * There are three macros: @@ -299,7 +299,7 @@ * #endif */ #if defined(MBEDTLS_SSL_PROTO_DTLS) && defined(MBEDTLS_SSL_PROTO_TLS) /* both */ -#define MBEDTLS_SSL_TRANSPORT__BOTH /* shorcut for future tests */ +#define MBEDTLS_SSL_TRANSPORT__BOTH /* shortcut for future tests */ #define MBEDTLS_SSL_TRANSPORT_IS_TLS( transport ) \ ( (transport) == MBEDTLS_SSL_TRANSPORT_STREAM ) #define MBEDTLS_SSL_TRANSPORT_IS_DTLS( transport ) \ diff --git a/library/ssl_tls.c b/library/ssl_tls.c index bdf93abfb6..dbfcde2adc 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -4925,7 +4925,7 @@ static int ssl_prepare_record_content( mbedtls_ssl_context *ssl ) if( ++ssl->in_ctr[i - 1] != 0 ) break; - /* The loop goes to its end iff the counter is wrapping */ + /* The loop goes to its end only if the counter is wrapping around */ if( i == 0 ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "incoming message counter would wrap" ) ); @@ -10841,7 +10841,7 @@ void mbedtls_ssl_read_version( int *major, int *minor, int transport, ++*minor; /* DTLS 1.0 stored as TLS 1.1 internally */ } MBEDTLS_SSL_TRANSPORT_ELSE -#endif +#endif /* MBEDTLS_SSL_PROTO_DTLS */ #if defined(MBEDTLS_SSL_PROTO_TLS) { *major = ver[0]; From ec1c22294781cb1e43f87990f7399ff91e3c2e04 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 12 Jun 2019 10:18:26 +0200 Subject: [PATCH 8/8] Fix a few style issues --- include/mbedtls/check_config.h | 2 +- include/mbedtls/config.h | 2 +- include/mbedtls/ssl_internal.h | 2 +- library/ssl_srv.c | 2 +- library/ssl_tls.c | 4 ++-- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/include/mbedtls/check_config.h b/include/mbedtls/check_config.h index fccf104390..b3677b5287 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_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 96cc98602c..e0b5ba41cc 100644 --- a/include/mbedtls/config.h +++ b/include/mbedtls/config.h @@ -1525,7 +1525,7 @@ * 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). + * and/or this and MBEDTLS_SSL_PROTO_SSL3 to enable SSL 3.0 (deprecated). * * \see MBEDTLS_SSL_PROTO_DTLS * diff --git a/include/mbedtls/ssl_internal.h b/include/mbedtls/ssl_internal.h index 511715f4b6..1c8709f3f4 100644 --- a/include/mbedtls/ssl_internal.h +++ b/include/mbedtls/ssl_internal.h @@ -270,7 +270,7 @@ * Goals for these helpers: * - generate minimal code, eg don't test if mode is DTLS in a DTLS-only build * - make the flow clear to the compiler, so that in TLS and DTLS combined - * builds, when there are two branches, it knows exactly one of them is taken + * builds, when there are two branches, it knows exactly one of them is taken * - preserve readability * * There are three macros: diff --git a/library/ssl_srv.c b/library/ssl_srv.c index fda0324eac..a8fd49b6c7 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -1663,7 +1663,7 @@ read_record_header: { ciph_offset = 35 + sess_len; } -#endif +#endif /* MBEDTLS_SSL_PROTO_TLS */ ciph_len = ( buf[ciph_offset + 0] << 8 ) | ( buf[ciph_offset + 1] ); diff --git a/library/ssl_tls.c b/library/ssl_tls.c index dbfcde2adc..e3ce5908fd 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -4216,7 +4216,7 @@ int mbedtls_ssl_prepare_handshake_record( mbedtls_ssl_context *ssl ) return( MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE ); } } -#endif +#endif /* MBEDTLS_SSL_PROTO_TLS */ return( 0 ); } @@ -10847,7 +10847,7 @@ void mbedtls_ssl_read_version( int *major, int *minor, int transport, *major = ver[0]; *minor = ver[1]; } -#endif +#endif /* MBEDTLS_SSL_PROTO_TLS */ } int mbedtls_ssl_set_calc_verify_md( mbedtls_ssl_context *ssl, int md )