Merge branch 'fix/esp_tls_check_tls_conn_before_read_write' into 'master'

Fix/esp tls check tls conn before read write

Closes IDF-15501

See merge request espressif/esp-idf!47068

components/esp-tls/esp_tls.c

@@ -145,6 +145,15 @@ ssize_t esp_tls_conn_read(esp_tls_t *tls, void  *data, size_t datalen)
     if (!tls) {
         return -1;
     }
+    if (!tls->read) {
+        return -1;
+    }
+#if CONFIG_MBEDTLS_DYNAMIC_BUFFER
+    if (tls->is_tls && tls->conn_state != ESP_TLS_DONE) {
+        ESP_LOGE(TAG, "TLS handshake has not completed, read operation not permitted");
+        return -1;
+    }
+#endif
     return tls->read(tls, (char *)data, datalen);
 }
 
@@ -153,6 +162,15 @@ ssize_t esp_tls_conn_write(esp_tls_t *tls, const void  *data, size_t datalen)
     if (!tls || !data) {
         return -1;
     }
+    if (!tls->write) {
+        return -1;
+    }
+#if CONFIG_MBEDTLS_DYNAMIC_BUFFER
+    if (tls->is_tls && tls->conn_state != ESP_TLS_DONE) {
+        ESP_LOGE(TAG, "TLS handshake has not completed, write operation not permitted");
+        return -1;
+    }
+#endif
     return tls->write(tls, (char *)data, datalen);
 }
 
@@ -574,12 +592,12 @@ int esp_tls_conn_new_sync(const char *hostname, int hostlen, int port, const esp
         } else if (ret == -1) {
             ESP_LOGE(TAG, "Failed to open new connection");
             return -1;
-        } else if (ret == 0 && cfg->timeout_ms >= 0) {
+        } else if (ret == 0 && cfg->timeout_ms > 0) {
             uint64_t elapsed_time_us = esp_tls_get_platform_time() - start_time_us;
             if ((elapsed_time_us / 1000) >= cfg->timeout_ms) {
                 ESP_LOGW(TAG, "Failed to open new connection in specified timeout");
                 ESP_INT_EVENT_TRACKER_CAPTURE(tls->error_handle, ESP_TLS_ERR_TYPE_ESP, ESP_ERR_ESP_TLS_CONNECTION_TIMEOUT);
-                return 0;
+                return -1;
             }
         }
     }

components/esp-tls/esp_tls.h

@@ -420,10 +420,17 @@ esp_tls_t *esp_tls_init(void);
  *                       structure should be zero-initialized
  * @param[in]  tls       Pointer to esp-tls as esp-tls handle.
  *
+ * @note       The cfg->timeout_ms parameter controls the connection timeout:
+ *             - timeout_ms > 0:  The connection attempt will be aborted if it does not
+ *                                complete within the specified duration.
+ *             - timeout_ms <= 0: No application-level timeout is applied. The connection
+ *                                relies on the underlying socket timeout (ESP_TLS_DEFAULT_CONN_TIMEOUT).
+ *             On timeout, the function returns -1 and records
+ *             ESP_ERR_ESP_TLS_CONNECTION_TIMEOUT in the error handle.
+ *
  * @return
- *             - -1      If connection establishment fails.
+ *             - -1      If connection establishment fails (including timeout).
  *             -  1      If connection establishment is successful.
- *             -  0      If connection state is in progress.
  */
 int esp_tls_conn_new_sync(const char *hostname, int hostlen, int port, const esp_tls_cfg_t *cfg, esp_tls_t *tls);
 

components/esp-tls/esp_tls_mbedtls.c

@@ -1196,6 +1196,7 @@ int esp_mbedtls_server_session_create(esp_tls_cfg_server_t *cfg, int sockfd, esp
             return ESP_ERR_ESP_TLS_SERVER_HANDSHAKE_TIMEOUT;
         }
     }
+    tls->conn_state = ESP_TLS_DONE;
     return ret;
 }
 

components/esp-tls/test_apps/.build-test-rules.yml

@@ -0,0 +1,6 @@
+# Documentation: .gitlab/ci/README.md#manifest-file-to-control-the-buildtest-apps
+
+components/esp-tls/test_apps:
+  disable:
+    - if: IDF_TARGET not in ["esp32c3"]
+      reason: Testing on one target is enough

components/esp-tls/test_apps/README.md

@@ -1,2 +1,2 @@
-| Supported Targets | ESP32 | ESP32-C2 | ESP32-C3 | ESP32-C5 | ESP32-C6 | ESP32-C61 | ESP32-H2 | ESP32-H21 | ESP32-H4 | ESP32-P4 | ESP32-S2 | ESP32-S3 | ESP32-S31 |
+| Supported Targets | ESP32-C3 |
-| ----------------- | ----- | -------- | -------- | -------- | -------- | --------- | -------- | --------- | -------- | -------- | -------- | -------- | --------- |
+| ----------------- | -------- |

components/esp-tls/test_apps/main/CMakeLists.txt

@@ -1,3 +1,7 @@
 idf_component_register(SRC_DIRS "."
                         PRIV_REQUIRES test_utils esp-tls unity
                         WHOLE_ARCHIVE)
+
+# Expose esp-tls private headers for internal component tests
+idf_component_get_property(esp_tls_dir esp-tls COMPONENT_DIR)
+target_include_directories(${COMPONENT_LIB} PRIVATE "${esp_tls_dir}/private_include")

components/esp-tls/test_apps/main/test_esp_tls.c

@@ -7,6 +7,7 @@
 #include "memory_checks.h"
 #include "esp_tls.h"
 #include "esp_tls_custom_stack.h"
+#include "esp_tls_private.h"
 #include "unity.h"
 #include "esp_err.h"
 #include "esp_log.h"
@@ -60,6 +61,46 @@ const char *test_key_pem =    "-----BEGIN PRIVATE KEY-----\n"\
                               "Aogx44Fozd1t2hYcozPuZD4s\n"\
                               "-----END PRIVATE KEY-----\n";
 
+static ssize_t dummy_read(esp_tls_t *tls, char *data, size_t datalen) { return (ssize_t)datalen; }
+
+TEST_CASE("esp_tls_conn_write/read reject NULL tls", "[esp-tls]")
+{
+    char buf[16] = {0};
+    TEST_ASSERT_EQUAL(-1, esp_tls_conn_write(NULL, buf, sizeof(buf)));
+    TEST_ASSERT_EQUAL(-1, esp_tls_conn_read(NULL, buf, sizeof(buf)));
+}
+
+TEST_CASE("esp_tls_conn_write reject NULL data buffer", "[esp-tls]")
+{
+    esp_tls_t *tls = esp_tls_init();
+    TEST_ASSERT_NOT_NULL(tls);
+    TEST_ASSERT_EQUAL(-1, esp_tls_conn_write(tls, NULL, 16));
+    esp_tls_conn_destroy(tls);
+}
+
+TEST_CASE("esp_tls_conn_read accepts zero datalen", "[esp-tls]")
+{
+    esp_tls_t *tls = esp_tls_init();
+    TEST_ASSERT_NOT_NULL(tls);
+    tls->is_tls = true;
+    tls->read = dummy_read;
+    TEST_ASSERT_EQUAL(ESP_OK, esp_tls_set_conn_state(tls, ESP_TLS_DONE));
+    /* datalen=0 on read is used in some projects, to get the data in the SSL buffers */
+    TEST_ASSERT_EQUAL(0, esp_tls_conn_read(tls, NULL, 0));
+    esp_tls_conn_destroy(tls);
+}
+
+TEST_CASE("esp_tls_conn_write/read reject unconnected tls", "[esp-tls]")
+{
+    esp_tls_t *tls = esp_tls_init();
+    TEST_ASSERT_NOT_NULL(tls);
+    /* read/write function pointers are NULL right after init */
+    char buf[16] = {0};
+    TEST_ASSERT_EQUAL(-1, esp_tls_conn_write(tls, buf, sizeof(buf)));
+    TEST_ASSERT_EQUAL(-1, esp_tls_conn_read(tls, buf, sizeof(buf)));
+    esp_tls_conn_destroy(tls);
+}
+
 #if CONFIG_ESP_TLS_USING_MBEDTLS
 TEST_CASE("esp-tls init deinit", "[esp-tls]")
 {

components/esp-tls/test_apps/pytest_esp_tls.py

@@ -0,0 +1,12 @@
+# SPDX-FileCopyrightText: 2022-2026 Espressif Systems (Shanghai) CO LTD
+# SPDX-License-Identifier: CC0-1.0
+import pytest
+from pytest_embedded import Dut
+from pytest_embedded_idf.utils import idf_parametrize
+
+
+@pytest.mark.generic
+@idf_parametrize('config', ['default'], indirect=['config'])
+@idf_parametrize('target', ['esp32c3'], indirect=['target'])
+def test_esp_tls(dut: Dut) -> None:
+    dut.run_all_single_board_cases()