Skip to content

fix(mqtt): close the socket when the TLS handshake fails - #3300

Open
csutcliff wants to merge 1 commit into
emsesp:devfrom
csutcliff:pr/mqtt-tls-close-socket
Open

csutcliff wants to merge 1 commit into
emsesp:devfrom
csutcliff:pr/mqtt-tls-close-socket

Conversation

@csutcliff

@csutcliff csutcliff commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

Problem

ESP_SSLClient::stop() returns early when the session never became secure. After a failed TLS handshake (a certificate check before NTP has synced, a host name mismatch, a CA that doesn't match) the underlying TCP socket therefore stays open, and espMqttClient sits in disconnectingTcp2 until the server closes the connection. Only then can it retry. A server that is still waiting for the client's half of the handshake may never close it, and then MQTT stays down.

Fix

ClientSecureSync::stop() also stops the plain WiFiClient underneath, so a failed handshake releases the socket straight away.

Testing

A TLS server EMS-ESP can't validate: openssl s_server with a P-521 certificate, while EMS-ESP has the Let's Encrypt ISRG Root X2 CA configured. The handshake gets as far as the server's certificate, then fails validation. Packets captured on the server:

Result
Before No FIN or RST from EMS-ESP. The socket was still established 5 min 21 s later, with no retry and MQTT disconnected the whole time
After EMS-ESP closes the socket about 50 ms after connecting and retries every 2 s

Related MQTT TLS fixes, independent of each other: #3301 (task stack), #3302 (TLS on any port), #3303 (receive buffer).

Checklist

  • Against the dev branch
  • Formatted per .clang-format
  • New logic is commented
  • One fix only, only relevant files touched
  • CHANGELOG_LATEST.md updated
  • I accept the CLA in CONTRIBUTING.md

@proddy

proddy commented Oct 19, 2026

Copy link
Copy Markdown
Contributor

this looks like a valid fix, but should be fixed upstream in the owner's project at https://github.com/bertmelis/espMqttClient . Please do a PR there.

@proddy proddy closed this Oct 19, 2026
@csutcliff

Copy link
Copy Markdown
Contributor Author

Thanks for looking. I don't think espMqttClient can take this one: its ClientSecureSync uses WiFiClientSecure, and the ESP_SSLClient version only exists in EMS-ESP's copy (0fe45a2, "use ESP_SSLClient for mqtt"). The root cause is in ESP_SSLClient itself: SSLClient::stop() returns straight away when the session never became secure, so after a failed handshake the underlying WiFiClient is never stopped. I'll raise that at mobizt/ESP_SSLClient. Until a release with the fix is pinned in platformio.ini, EMS-ESP still needs this one-line workaround, so could this be reopened as a stopgap?

@csutcliff

Copy link
Copy Markdown
Contributor Author

Raised at mobizt/ESP_SSLClient#35. With that change alone, without the workaround here, the socket is closed straight away after a failed handshake as well.

@csutcliff

Copy link
Copy Markdown
Contributor Author

mobizt/ESP_SSLClient#35 is merged (aa7fb64) but isn't in a release yet, the latest is still 3.1.3. Three options:

  • merge the workaround here now, and once 3.1.4 is out I'll replace it with the version bump in platformio.ini
  • replace it now with a commit pin, https://github.com/mobizt/ESP_SSLClient.git#aa7fb646bc21d7184aff7db5fcffd721f6c757ad
  • leave this open until 3.1.4 is released and bump the version then, though if that isn't before 3.9.0 the bug ships

The workaround is harmless alongside the fixed library (stopping an already stopped WiFiClient does nothing), so nothing breaks whichever lands first.

ESP_SSLClient::stop() returns early if the session never became secure,
so after a failed handshake (e.g. certificate check before NTP sync or a
host name mismatch) the TCP socket stayed open and the client waited in
disconnectingTcp2 until the broker closed it. Stop the socket as well.
@csutcliff
csutcliff force-pushed the pr/mqtt-tls-close-socket branch from 4982c34 to 82db41a Compare October 20, 2026 11:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants