esp-idf icon indicating copy to clipboard operation
esp-idf copied to clipboard

`esp_tls_conn_new_async` does not work if real time is set (IDFGH-12594)

Open ngc7293 opened this issue 1 year ago • 3 comments

Answers checklist.

  • [X] I have read the documentation ESP-IDF Programming Guide and the issue is not addressed there.
  • [X] I have updated my IDF branch (master or release) to the latest version and checked that the issue is present there.
  • [X] I have searched the issue tracker for a similar issue and not found a similar issue.

IDF version.

master

Espressif SoC revision.

ESP32-S2

Operating System used.

Linux

How did you build your project?

Command line with CMake

If you are using Windows, please specify command line type.

None

Development Kit.

ESP32-S2-DevKitM-1

Power Supply used.

USB

What is the expected behavior?

esp_tls_conn_new_sync waits up to timeout_ms milliseconds before returning (set in the passed esp_tls_cfg_t)

What is the actual behavior?

esp_tls_conn_new_sync returns immediately and logs a warning "Failed to open new connection in specified timeout"

Steps to reproduce.

  1. Set the proper time (I used esp_sntp)
  2. Call esp_tls_conn_new_sync with timeout_ms = INT32_MAX

Debug Logs.

No response

More Information.

The issue here is that the function's body uses uint32_t to store the epoch timestamp in milliseconds. When the "system" time has not been set this works fine, but if you have properly set the time the current epoch timestamp exceeds the holding capacity of uint32_t.

Additionally, there could be a case where the time is set during the connection process which would also break the timeout check.

The reliance on gettimeofday should probably be replaced with an internal counter, like FreeRTOS's tick count or esp_timer's esp_timer_get_time

See esp_tls.c:542 See gettimeofday

ngc7293 avatar Apr 11 '24 16:04 ngc7293

Hi @ngc7293 Thanks for finding the issue. The use of uint32_t type for 64bit time seems wrong. Will make the relevant changes and update here. Yes and regarding gettimeofday, I see it is better to use esp_timer_get_time at this place which will give a value that shall not be changed.

Thanks, Aditya

AdityaHPatwardhan avatar Apr 23 '24 04:04 AdityaHPatwardhan

@AdityaHPatwardhan Do you have ETA for the fix? (I think this needs urgent fix for release branches)

AxelLin avatar May 03 '24 02:05 AxelLin

@AxelLin It is been merged in the internal code-base. Should be available on GitHub in the release branches in a few days.

AdityaHPatwardhan avatar May 03 '24 03:05 AdityaHPatwardhan

@AdityaHPatwardhan @mahavirj While checking v5.3-beta2 chances, I found below fix is not in v5.2 branch. Could you check if this fix is required for v5.2 branch? Initialize mbedtls RNG context prior to setting client config in ESP-TLS, this fixes ECC key parsing issue for specific cases ( a02b096)

AxelLin avatar Jun 08 '24 01:06 AxelLin

Hi @AxelLin Yes, we have backported the fix. Should be visible in a few days on v5.2.

AdityaHPatwardhan avatar Jun 14 '24 03:06 AdityaHPatwardhan

@AdityaHPatwardhan v5.1 branch still needs backport fix.

AxelLin avatar Jul 02 '24 01:07 AxelLin

Hi @AxelLin, have backported this fix to v5.1, and v5.0 as well. Should be visible in a few days.

AdityaHPatwardhan avatar Jul 02 '24 01:07 AdityaHPatwardhan