From 2eaf2c79692abcfef2980c64ed5139efd5581736 Mon Sep 17 00:00:00 2001 From: Jaeden Amero Date: Wed, 5 Jun 2019 13:32:08 +0100 Subject: [PATCH] ssl: Don't access non-existent encrypt_then_mac field When MBEDTLS_SSL_ENCRYPT_THEN_MAC is enabled, but not MBEDTLS_SSL_SOME_MODES_USE_MAC, mbedtls_ssl_derive_keys() and build_transforms() will attempt to use a non-existent `encrypt_then_mac` field in the ssl_transform. Compile [ 93.7%]: ssl_tls.c [Error] ssl_tls.c@865,14: 'mbedtls_ssl_transform {aka struct mbedtls_ssl_transform}' ha s no member named 'encrypt_then_mac' [ERROR] ./mbed-os/features/mbedtls/src/ssl_tls.c: In function 'mbedtls_ssl_derive_keys' : ./mbed-os/features/mbedtls/src/ssl_tls.c:865:14: error: 'mbedtls_ssl_transform {aka str uct mbedtls_ssl_transform}' has no member named 'encrypt_then_mac' transform->encrypt_then_mac = session->encrypt_then_mac; ^~ Change mbedtls_ssl_derive_keys() and build_transforms() to only access `encrypt_then_mac` if `encrypt_then_mac` is actually present. Fix any unused variable warnings along the way, by additionally wrapping function parameters with MBEDTLS_SSL_SOME_MODES_USE_MAC. Add a regression test to detect when we have regressions with configurations that do not include any MAC ciphersuites. Fixes 92231325a765 ("Reduce size of `ssl_transform` if no MAC ciphersuite is enabled") --- library/ssl_tls.c | 7 ++++++- tests/scripts/all.sh | 14 ++++++++++++++ tests/suites/test_suite_ssl.function | 3 ++- 3 files changed, 22 insertions(+), 2 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index b61453fe5..dc5202464 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -730,12 +730,14 @@ typedef int ssl_tls_prf_t(const unsigned char *, size_t, const char *, static int ssl_populate_transform( mbedtls_ssl_transform *transform, int ciphersuite, const unsigned char master[48], +#if defined(MBEDTLS_SSL_SOME_MODES_USE_MAC) #if defined(MBEDTLS_SSL_ENCRYPT_THEN_MAC) int encrypt_then_mac, #endif #if defined(MBEDTLS_SSL_TRUNCATED_HMAC) int trunc_hmac, #endif +#endif /* MBEDTLS_SSL_SOME_MODES_USE_MAC */ #if defined(MBEDTLS_ZLIB_SUPPORT) int compression, #endif @@ -766,7 +768,8 @@ static int ssl_populate_transform( mbedtls_ssl_transform *transform, #endif /* Copy info about negotiated version and extensions */ -#if defined(MBEDTLS_SSL_ENCRYPT_THEN_MAC) +#if defined(MBEDTLS_SSL_ENCRYPT_THEN_MAC) && \ + defined(MBEDTLS_SSL_SOME_MODES_USE_MAC) transform->encrypt_then_mac = encrypt_then_mac; #endif transform->minor_ver = minor_ver; @@ -1327,12 +1330,14 @@ int mbedtls_ssl_derive_keys( mbedtls_ssl_context *ssl ) ret = ssl_populate_transform( ssl->transform_negotiate, ssl->session_negotiate->ciphersuite, ssl->session_negotiate->master, +#if defined(MBEDTLS_SSL_SOME_MODES_USE_MAC) #if defined(MBEDTLS_SSL_ENCRYPT_THEN_MAC) ssl->session_negotiate->encrypt_then_mac, #endif #if defined(MBEDTLS_SSL_TRUNCATED_HMAC) ssl->session_negotiate->trunc_hmac, #endif +#endif /* MBEDTLS_SSL_SOME_MODES_USE_MAC */ #if defined(MBEDTLS_ZLIB_SUPPORT) ssl->session_negotiate->compression, #endif diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 9af040fba..3d94bc85d 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -883,6 +883,20 @@ component_test_no_max_fragment_length_small_ssl_out_content_len () { if_build_succeeded tests/ssl-opt.sh -f "Max fragment length\|Large buffer" } +component_test_when_no_ciphersuites_have_mac () { + msg "build: when no ciphersuites have MAC" + scripts/config.pl unset MBEDTLS_CIPHER_NULL_CIPHER + scripts/config.pl unset MBEDTLS_ARC4_C + scripts/config.pl unset MBEDTLS_CIPHER_MODE_CBC + make + + msg "test: !MBEDTLS_SSL_SOME_MODES_USE_MAC" + make test + + msg "test ssl-opt.sh: !MBEDTLS_SSL_SOME_MODES_USE_MAC" + if_build_succeeded tests/ssl-opt.sh +} + component_test_null_entropy () { msg "build: default config with MBEDTLS_TEST_NULL_ENTROPY (ASan build)" scripts/config.pl set MBEDTLS_TEST_NULL_ENTROPY diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index 65f585274..c6dd56452 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -159,7 +159,8 @@ static int build_transforms( mbedtls_ssl_transform *t_in, * Setup transforms */ -#if defined(MBEDTLS_SSL_ENCRYPT_THEN_MAC) +#if defined(MBEDTLS_SSL_ENCRYPT_THEN_MAC) && \ + defined(MBEDTLS_SSL_SOME_MODES_USE_MAC) t_out->encrypt_then_mac = etm; t_in->encrypt_then_mac = etm; #else