From 9ad041cc8ce0398e374b3299f7a74bfaa1c1c6b0 Mon Sep 17 00:00:00 2001 From: Ashish Sharma Date: Thu, 21 May 2026 14:06:20 +0800 Subject: [PATCH] fix(mbedtls): fixes TLS1.3 server failing with dynamic buffer --- components/mbedtls/CMakeLists.txt | 1 + .../port/dynamic/esp_mbedtls_dynamic_impl.c | 17 +++--- components/mbedtls/port/dynamic/esp_ssl_srv.c | 55 ++++++++++++++++++- 3 files changed, 65 insertions(+), 8 deletions(-) diff --git a/components/mbedtls/CMakeLists.txt b/components/mbedtls/CMakeLists.txt index 32aa4d87afb..38c0b1c82ff 100644 --- a/components/mbedtls/CMakeLists.txt +++ b/components/mbedtls/CMakeLists.txt @@ -503,6 +503,7 @@ if(CONFIG_MBEDTLS_DYNAMIC_BUFFER) mbedtls_ssl_handshake_client_step mbedtls_ssl_tls13_handshake_client_step mbedtls_ssl_handshake_server_step + mbedtls_ssl_tls13_handshake_server_step mbedtls_ssl_read mbedtls_ssl_write mbedtls_ssl_free diff --git a/components/mbedtls/port/dynamic/esp_mbedtls_dynamic_impl.c b/components/mbedtls/port/dynamic/esp_mbedtls_dynamic_impl.c index 3973b9a068a..dec0ca78626 100644 --- a/components/mbedtls/port/dynamic/esp_mbedtls_dynamic_impl.c +++ b/components/mbedtls/port/dynamic/esp_mbedtls_dynamic_impl.c @@ -408,14 +408,17 @@ int esp_mbedtls_add_rx_buffer(mbedtls_ssl_context *ssl) buffer_len = tx_buffer_len(ssl, in_msglen); #if defined(MBEDTLS_SSL_PROTO_TLS1_3) - /* For TLS 1.3 ENCRYPTED_EXTENSIONS state, allocate max size buffer. - * This is needed because ChangeCipherSpec (1 byte) arrives first, - * followed immediately by EncryptedExtensions (potentially large). - * Since mbedtls processes both in the same read loop without returning - * to the wrapper, we need to allocate sufficient space upfront. */ - if (ssl->MBEDTLS_PRIVATE(state) == MBEDTLS_SSL_ENCRYPTED_EXTENSIONS) { + /* In TLS 1.3 middlebox-compat mode (RFC 8446 D.4) a 1-byte dummy CCS + * record may precede a handshake record. mbedtls silently skips the CCS + * and, in the same read loop, reads the (larger) record that follows into + * this same RX buffer. The header peek above only saw the 1-byte CCS, so + * whenever the peeked record is such a CCS, size for the max record + * instead to avoid overflowing the buffer when the next record is read. */ + if (ssl->MBEDTLS_PRIVATE(tls_version) == MBEDTLS_SSL_VERSION_TLS1_3 && + ssl->MBEDTLS_PRIVATE(in_msgtype) == MBEDTLS_SSL_MSG_CHANGE_CIPHER_SPEC) { buffer_len = tx_buffer_len(ssl, MBEDTLS_SSL_IN_CONTENT_LEN); - ESP_LOGV(TAG, "TLS 1.3 ENCRYPTED_EXTENSIONS: allocating max buffer %d bytes", buffer_len); + ESP_LOGV(TAG, "TLS 1.3 CCS peeked: allocating max RX buffer %d bytes", + buffer_len); } #endif diff --git a/components/mbedtls/port/dynamic/esp_ssl_srv.c b/components/mbedtls/port/dynamic/esp_ssl_srv.c index 9c50894fa72..54d06450bbc 100644 --- a/components/mbedtls/port/dynamic/esp_ssl_srv.c +++ b/components/mbedtls/port/dynamic/esp_ssl_srv.c @@ -8,8 +8,12 @@ int __real_mbedtls_ssl_handshake_server_step(mbedtls_ssl_context *ssl); +int __real_mbedtls_ssl_tls13_handshake_server_step(mbedtls_ssl_context *ssl); + int __wrap_mbedtls_ssl_handshake_server_step(mbedtls_ssl_context *ssl); +int __wrap_mbedtls_ssl_tls13_handshake_server_step(mbedtls_ssl_context *ssl); + static const char *TAG = "SSL Server"; #ifdef CONFIG_MBEDTLS_DYNAMIC_FREE_CONFIG_DATA @@ -33,7 +37,8 @@ static int manage_resource(mbedtls_ssl_context *ssl, bool add, int prev_state) { int state = add ? ssl->MBEDTLS_PRIVATE(state) : prev_state; - if (mbedtls_ssl_is_handshake_over(ssl) || ssl->MBEDTLS_PRIVATE(handshake) == NULL) { + if (ssl->MBEDTLS_PRIVATE(state) == MBEDTLS_SSL_HANDSHAKE_OVER || + ssl->MBEDTLS_PRIVATE(handshake) == NULL) { return 0; } @@ -158,6 +163,18 @@ static int manage_resource(mbedtls_ssl_context *ssl, bool add, int prev_state) } break; case MBEDTLS_SSL_CERTIFICATE_VERIFY: +#if defined(MBEDTLS_SSL_PROTO_TLS1_3) + /* In TLS 1.3 server flow, CERTIFICATE_VERIFY is the server's own + * CertificateVerify (outgoing). In TLS 1.2 it is the client's + * CertificateVerify (incoming). The constant is shared but the + * direction is reversed, so the required buffer differs. */ + if (ssl->MBEDTLS_PRIVATE(tls_version) == MBEDTLS_SSL_VERSION_TLS1_3) { + if (add) { + CHECK_OK(esp_mbedtls_add_tx_buffer(ssl, MBEDTLS_SSL_OUT_BUFFER_LEN)); + } + break; + } +#endif /* MBEDTLS_SSL_PROTO_TLS1_3 */ if (add) { CHECK_OK(esp_mbedtls_add_rx_buffer(ssl)); } else { @@ -198,6 +215,30 @@ static int manage_resource(mbedtls_ssl_context *ssl, bool add, int prev_state) break; case MBEDTLS_SSL_HANDSHAKE_WRAPUP: break; +#if defined(MBEDTLS_SSL_PROTO_TLS1_3) + /* TLS 1.3-only server outgoing states: ensure a TX buffer exists. + * The shared free-on-empty logic at the top handles teardown. */ + case MBEDTLS_SSL_HELLO_RETRY_REQUEST: + case MBEDTLS_SSL_ENCRYPTED_EXTENSIONS: + case MBEDTLS_SSL_SERVER_CCS_AFTER_HELLO_RETRY_REQUEST: + case MBEDTLS_SSL_SERVER_CCS_AFTER_SERVER_HELLO: + case MBEDTLS_SSL_TLS1_3_NEW_SESSION_TICKET: + if (add) { + CHECK_OK(esp_mbedtls_add_tx_buffer(ssl, MBEDTLS_SSL_OUT_BUFFER_LEN)); + } + break; + /* TLS 1.3-only server incoming states: allocate RX before, free after. */ + case MBEDTLS_SSL_CLIENT_CERTIFICATE_VERIFY: + case MBEDTLS_SSL_END_OF_EARLY_DATA: + if (add) { + CHECK_OK(esp_mbedtls_add_rx_buffer(ssl)); + } else { + CHECK_OK(esp_mbedtls_free_rx_buffer(ssl)); + } + break; + case MBEDTLS_SSL_TLS1_3_NEW_SESSION_TICKET_FLUSH: + break; +#endif /* MBEDTLS_SSL_PROTO_TLS1_3 */ default: break; } @@ -216,3 +257,15 @@ int __wrap_mbedtls_ssl_handshake_server_step(mbedtls_ssl_context *ssl) return 0; } + +int __wrap_mbedtls_ssl_tls13_handshake_server_step(mbedtls_ssl_context *ssl) +{ + int prev_state = ssl->MBEDTLS_PRIVATE(state); + CHECK_OK(manage_resource(ssl, true, prev_state)); + + CHECK_OK(__real_mbedtls_ssl_tls13_handshake_server_step(ssl)); + + CHECK_OK(manage_resource(ssl, false, prev_state)); + + return 0; +}