diff --git a/ChangeLog.d/tls13-psa_crypto_init.txt b/ChangeLog.d/tls13-psa_crypto_init.txt new file mode 100644 index 0000000000..311db65585 --- /dev/null +++ b/ChangeLog.d/tls13-psa_crypto_init.txt @@ -0,0 +1,9 @@ +Bugfix + * Fix TLS connections failing when the handshake selects TLS 1.3 + in an application that does not call psa_crypto_init(). + Fixes #9072. + +Changes + * A TLS handshake may now call psa_crypto_init() if TLS 1.3 is enabled. + This can happen even if TLS 1.3 is offered but eventually not selected + in the protocol version negotiation. diff --git a/include/mbedtls/mbedtls_config.h b/include/mbedtls/mbedtls_config.h index 7ed1a433ba..4c01cd5c5b 100644 --- a/include/mbedtls/mbedtls_config.h +++ b/include/mbedtls/mbedtls_config.h @@ -1798,8 +1798,9 @@ * Requires: MBEDTLS_PSA_CRYPTO_C * * \note TLS 1.3 uses PSA crypto for cryptographic operations that are - * directly performed by TLS 1.3 code. As a consequence, you must - * call psa_crypto_init() before the first TLS 1.3 handshake. + * directly performed by TLS 1.3 code. As a consequence, when TLS 1.3 + * is enabled, a TLS handshake may call psa_crypto_init(), even + * if it ends up negotiating a different TLS version. * * \note Cryptographic operations performed indirectly via another module * (X.509, PK) or by code shared with TLS 1.2 (record protection, diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 4b59e78532..466c734d37 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -4923,10 +4923,13 @@ int mbedtls_ssl_get_session(const mbedtls_ssl_context *ssl, * currently being processed might or might not contain further * DTLS records. * - * \note If the context is configured to allow TLS 1.3, or if - * #MBEDTLS_USE_PSA_CRYPTO is enabled, the PSA crypto + * \note If #MBEDTLS_USE_PSA_CRYPTO is enabled, the PSA crypto * subsystem must have been initialized by calling * psa_crypto_init() before calling this function. + * Otherwise, the handshake may call psa_crypto_init() + * if a negotiation involving TLS 1.3 takes place (this may + * be the case even if TLS 1.3 is offered but eventually + * not selected). */ int mbedtls_ssl_handshake(mbedtls_ssl_context *ssl); diff --git a/library/ssl_misc.h b/library/ssl_misc.h index 8402fe86ae..082bc9bd93 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -1891,6 +1891,26 @@ static inline int mbedtls_ssl_conf_is_hybrid_tls12_tls13(const mbedtls_ssl_confi #endif /* MBEDTLS_SSL_PROTO_TLS1_2 && MBEDTLS_SSL_PROTO_TLS1_3 */ #if defined(MBEDTLS_SSL_PROTO_TLS1_3) + +/** \brief Initialize the PSA crypto subsystem if necessary. + * + * Call this function before doing any cryptography in a TLS 1.3 handshake. + * + * This is necessary in Mbed TLS 3.x for backward compatibility. + * Up to Mbed TLS 3.5, in the default configuration, you could perform + * a TLS connection with default parameters without having called + * psa_crypto_init(), since the TLS layer only supported TLS 1.2 and + * did not use PSA crypto. (TLS 1.2 only uses PSA crypto if + * MBEDTLS_USE_PSA_CRYPTO is enabled, which is not the case in the default + * configuration.) Starting with Mbed TLS 3.6.0, TLS 1.3 is enabled + * by default, and the TLS 1.3 layer uses PSA crypto. This means that + * applications that are not otherwise using PSA crypto and that worked + * with Mbed TLS 3.5 started failing in TLS 3.6.0 if they connected to + * a peer that supports TLS 1.3. See + * https://github.com/Mbed-TLS/mbedtls/issues/9072 + */ +int mbedtls_ssl_tls13_crypto_init(mbedtls_ssl_context *ssl); + extern const uint8_t mbedtls_ssl_tls13_hello_retry_request_magic[ MBEDTLS_SERVER_HELLO_RANDOM_LEN]; MBEDTLS_CHECK_RETURN_CRITICAL diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index 162e3a3146..b63b5e63c5 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -1141,6 +1141,11 @@ int mbedtls_ssl_tls13_write_client_hello_exts(mbedtls_ssl_context *ssl, *out_len = 0; + ret = mbedtls_ssl_tls13_crypto_init(ssl); + if (ret != 0) { + return ret; + } + /* Write supported_versions extension * * Supported Versions Extension is mandatory with TLS 1.3. diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index 8ac6579e05..4251027c3e 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -27,7 +27,6 @@ #include "psa/crypto.h" #include "psa_util_internal.h" -#if defined(MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_SOME_EPHEMERAL_ENABLED) /* Define a local translating function to save code size by not using too many * arguments in each translating place. */ static int local_err_translation(psa_status_t status) @@ -37,7 +36,16 @@ static int local_err_translation(psa_status_t status) psa_generic_status_to_mbedtls); } #define PSA_TO_MBEDTLS_ERR(status) local_err_translation(status) -#endif + +int mbedtls_ssl_tls13_crypto_init(mbedtls_ssl_context *ssl) +{ + psa_status_t status = psa_crypto_init(); + if (status != PSA_SUCCESS) { + (void) ssl; // unused when debugging is disabled + MBEDTLS_SSL_DEBUG_RET(1, "psa_crypto_init", status); + } + return PSA_TO_MBEDTLS_ERR(status); +} const uint8_t mbedtls_ssl_tls13_hello_retry_request_magic[ MBEDTLS_SERVER_HELLO_RANDOM_LEN] = diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index 9c949bd0b1..693edc7b0b 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -1412,6 +1412,12 @@ static int ssl_tls13_parse_client_hello(mbedtls_ssl_context *ssl, ssl->session_negotiate->tls_version = MBEDTLS_SSL_VERSION_TLS1_3; ssl->session_negotiate->endpoint = ssl->conf->endpoint; + /* Before doing any crypto, make sure we can. */ + ret = mbedtls_ssl_tls13_crypto_init(ssl); + if (ret != 0) { + return ret; + } + /* * We are negotiating the version 1.3 of the protocol. Do what we have * postponed: copy of the client random bytes, copy of the legacy session diff --git a/programs/ssl/ssl_client2.c b/programs/ssl/ssl_client2.c index d494de3a60..cd839c1610 100644 --- a/programs/ssl/ssl_client2.c +++ b/programs/ssl/ssl_client2.c @@ -818,8 +818,6 @@ int main(int argc, char *argv[]) psa_key_attributes_t key_attributes; #endif psa_status_t status; -#elif defined(MBEDTLS_SSL_PROTO_TLS1_3) - psa_status_t status; #endif rng_context_t rng; @@ -894,7 +892,15 @@ int main(int argc, char *argv[]) memset((void *) alpn_list, 0, sizeof(alpn_list)); #endif -#if defined(MBEDTLS_USE_PSA_CRYPTO) || defined(MBEDTLS_SSL_PROTO_TLS1_3) + /* For builds with TLS 1.3 enabled but not MBEDTLS_USE_PSA_CRYPTO, + * we deliberately do not call psa_crypto_init() here, to test that + * the library is backward-compatible with versions prior to 3.6.0 + * where calling psa_crypto_init() was not required to open a TLS + * connection in the default configuration. See + * https://github.com/Mbed-TLS/mbedtls/issues/9072 and + * mbedtls_ssl_tls13_crypto_init(). + */ +#if defined(MBEDTLS_USE_PSA_CRYPTO) status = psa_crypto_init(); if (status != PSA_SUCCESS) { mbedtls_fprintf(stderr, "Failed to initialize PSA Crypto implementation: %d\n", @@ -3192,6 +3198,9 @@ exit: /* For builds with MBEDTLS_TEST_USE_PSA_CRYPTO_RNG psa crypto * resources are freed by rng_free(). */ + /* For builds with MBEDTLS_SSL_PROTO_TLS1_3, PSA may have been + * initialized under the hood by the TLS layer. See + * mbedtls_ssl_tls13_crypto_init(). */ #if (defined(MBEDTLS_USE_PSA_CRYPTO) || defined(MBEDTLS_SSL_PROTO_TLS1_3)) && \ !defined(MBEDTLS_TEST_USE_PSA_CRYPTO_RNG) mbedtls_psa_crypto_free(); diff --git a/programs/ssl/ssl_server2.c b/programs/ssl/ssl_server2.c index 480b262446..79a742e152 100644 --- a/programs/ssl/ssl_server2.c +++ b/programs/ssl/ssl_server2.c @@ -1594,7 +1594,7 @@ int main(int argc, char *argv[]) int i; char *p, *q; const int *list; -#if defined(MBEDTLS_USE_PSA_CRYPTO) || defined(MBEDTLS_SSL_PROTO_TLS1_3) +#if defined(MBEDTLS_USE_PSA_CRYPTO) psa_status_t status; #endif unsigned char eap_tls_keymaterial[16]; @@ -1660,7 +1660,15 @@ int main(int argc, char *argv[]) mbedtls_ssl_cookie_init(&cookie_ctx); #endif -#if defined(MBEDTLS_USE_PSA_CRYPTO) || defined(MBEDTLS_SSL_PROTO_TLS1_3) + /* For builds with TLS 1.3 enabled but not MBEDTLS_USE_PSA_CRYPTO, + * we deliberately do not call psa_crypto_init() here, to test that + * the library is backward-compatible with versions prior to 3.6.0 + * where calling psa_crypto_init() was not required to open a TLS + * connection in the default configuration. See + * https://github.com/Mbed-TLS/mbedtls/issues/9072 and + * mbedtls_ssl_tls13_crypto_init(). + */ +#if defined(MBEDTLS_USE_PSA_CRYPTO) status = psa_crypto_init(); if (status != PSA_SUCCESS) { mbedtls_fprintf(stderr, "Failed to initialize PSA Crypto implementation: %d\n", @@ -4309,6 +4317,9 @@ exit: /* For builds with MBEDTLS_TEST_USE_PSA_CRYPTO_RNG psa crypto * resources are freed by rng_free(). */ + /* For builds with MBEDTLS_SSL_PROTO_TLS1_3, PSA may have been + * initialized under the hood by the TLS layer. See + * mbedtls_ssl_tls13_crypto_init(). */ #if (defined(MBEDTLS_USE_PSA_CRYPTO) || defined(MBEDTLS_SSL_PROTO_TLS1_3)) \ && !defined(MBEDTLS_TEST_USE_PSA_CRYPTO_RNG) mbedtls_psa_crypto_free(); diff --git a/tests/include/test/psa_crypto_helpers.h b/tests/include/test/psa_crypto_helpers.h index 71ba0fc021..1039712aef 100644 --- a/tests/include/test/psa_crypto_helpers.h +++ b/tests/include/test/psa_crypto_helpers.h @@ -334,9 +334,18 @@ uint64_t mbedtls_test_parse_binary_string(data_t *bin_string); * This is like #PSA_DONE except it does nothing under the same conditions as * #USE_PSA_INIT. */ -#if defined(MBEDTLS_USE_PSA_CRYPTO) || defined(MBEDTLS_SSL_PROTO_TLS1_3) +#if defined(MBEDTLS_USE_PSA_CRYPTO) #define USE_PSA_INIT() PSA_INIT() #define USE_PSA_DONE() PSA_DONE() +#elif defined(MBEDTLS_SSL_PROTO_TLS1_3) +/* TLS 1.3 must work without having called psa_crypto_init(), for backward + * compatibility with Mbed TLS <= 3.5 when connecting with a peer that + * supports both TLS 1.2 and TLS 1.3. See mbedtls_ssl_tls13_crypto_init() + * and https://github.com/Mbed-TLS/mbedtls/issues/9072 . */ +#define USE_PSA_INIT() ((void) 0) +/* TLS 1.3 may have initialized the PSA subsystem. Shut it down cleanly, + * otherwise Asan and Valgrind would notice a resource leak. */ +#define USE_PSA_DONE() PSA_DONE() #else /* MBEDTLS_USE_PSA_CRYPTO || MBEDTLS_SSL_PROTO_TLS1_3 */ /* Define empty macros so that we can use them in the preamble and teardown * of every test function that uses PSA conditionally based on @@ -408,13 +417,12 @@ uint64_t mbedtls_test_parse_binary_string(data_t *bin_string); * This is like #PSA_DONE except it does nothing under the same conditions as * #MD_OR_USE_PSA_INIT. */ -#if defined(MBEDTLS_MD_SOME_PSA) || \ - defined(MBEDTLS_USE_PSA_CRYPTO) || defined(MBEDTLS_SSL_PROTO_TLS1_3) +#if defined(MBEDTLS_MD_SOME_PSA) #define MD_OR_USE_PSA_INIT() PSA_INIT() #define MD_OR_USE_PSA_DONE() PSA_DONE() #else -#define MD_OR_USE_PSA_INIT() ((void) 0) -#define MD_OR_USE_PSA_DONE() ((void) 0) +#define MD_OR_USE_PSA_INIT() USE_PSA_INIT() +#define MD_OR_USE_PSA_DONE() USE_PSA_DONE() #endif /** \def AES_PSA_INIT