SSL/TLS Implementation for ESPAsyncWebServer by dimecho · Pull Request #121 · ESP32Async/AsyncTCP

dimecho · 2026-08-25T09:44:07Z

No description provided.

mathieucarbou · 2026-08-25T10:01:39Z

Thanks! We’ll have a look as soon as we can!

me-no-dev · 2026-08-25T10:04:12Z

Since this is the AsyncTCP repo and not the WebServer one, please make the example use only AsyncTCP. WebServer example should be submitted to the WebServer repo, along with the changes necessary there.

Another thing for the example is to add a file called build_opt.h with one line inside -DASYNC_TCP_SSL_ENABLED=1 in order to turn on SSL. Same should be done for the WebServer example in the WebServer repo.

There are slight changes in mbedtls in the upcoming Arduino for ESP32 v4. They come from the updated mbedtls in ESP-IDF v6. Changes here should be compatible with both current Arduino (v3) and the upcoming one (v4).

Would be great if the example uses an actual certificate and key that can be used to directly build and test it (by CI and users)

Copilot

Pull request overview

Adds optional mbedTLS-based client/server TLS support to AsyncTCP for ESPAsyncWebServer HTTPS usage.

Changes:

Adds TLS contexts, handshakes, encrypted I/O, and certificate handling.
Exposes secure AsyncClient/AsyncServer APIs.
Adds an HTTPS example and bumps the header version.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 17 comments.

Show a summary per file

File	Description
`src/AsyncTCPVersion.h`	Updates the reported minor version.
`src/AsyncTCPTLS.h`	Declares the TLS context API.
`src/AsyncTCPTLS.cpp`	Implements mbedTLS over lwIP.
`src/AsyncTCP.h`	Exposes secure client/server interfaces.
`src/AsyncTCP.cpp`	Integrates TLS into connection and I/O flows.
`examples/AsyncWebServerSSL/AsyncWebServerSSL.ino`	Demonstrates an HTTPS web server.

Suppressed comments (2)

src/AsyncTCP.cpp:1190

Returning ERR_ABRT does not abort anything here: _connected() is invoked by handle_async_event() and its return value is discarded at AsyncTCP.cpp:324. This and the two setup-failure returns above can leave a live PCB/failing TLS context after callbacks run. Explicitly call abort() or _close() on every failure path and avoid manually firing the disconnect callback twice.

        }
        if (_discard_cb) {
          _discard_cb(_discard_cb_arg, this);
        }
        return ERR_ABRT;

src/AsyncTCP.cpp:1279

A handshake error received through this path only invokes callbacks and then returns ERR_OK; it never closes or aborts the still-live connection. Clients without a disconnect callback remain stuck and subsequent polls retry the failed TLS state. Match the poll failure path by closing the client (letting _close() issue the disconnect callback).

      async_tcp_log_e("SSL handshake failed in _recv: %d", ret);
      if (_error_cb) {
        _error_cb(_error_cb_arg, this, -60);
      }
      if (_discard_cb) {

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

mathieucarbou

Thanks a lof for having started this effort!

This is something that a few users were asking for, even if for a webserver on a MCU, support for SSL is quite a niche since any self-signed cert will cause issues on most clients / OS now. So this reduces valid use cases to ssl com with client cert, or use cases where the MCU would be exposed with a cert that is signed from a valid authority (internal company CA or valid internet authority).

So I am sure a lot of people will find it useful.

This reverts commit f5c390f.

dimecho · 2026-08-25T22:58:21Z

Thanks a lof for having started this effort!

This is something that a few users were asking for, even if for a webserver on a MCU, support for SSL is quite a niche since any self-signed cert will cause issues on most clients / OS now. So this reduces valid use cases to ssl com with client cert, or use cases where the MCU would be exposed with a cert that is signed from a valid authority (internal company CA or valid internet authority).

So I am sure a lot of people will find it useful.

There are use cases for https (oauth2, MFA/Passkeys ...etc) work arounds TLS trust can be with domain to local pointers - example https://192.168.4.1.sslip.io

willmmiles

I'm sorry to say that the changes to the core logic are grossly incorrect, suggesting a poor understanding of LwIP and the purposes of a generic TCP client library. I haven't reviewed the SSL changes in detail yet, because to be frank, this makes me think I'd be wasting my time reviewing AI slop instead of well considered code.

From an architectual standpoint, what is the rationale for integrating the SSL layer deeply in to the AsyncClient class as opposed to constructing a wrapper class? The deep integration adds a lot of complexity and penalizes all clients whether they use the feature or not.

willmmiles · 2026-09-07T14:07:51Z

+// Drain callback: ACKs and drops any late data arriving after close is initiated.
+// Without this, LwIP sends RST when data arrives on a PCB with tcp_recv=NULL.
+static err_t _tcp_drain_recv(void *arg, struct tcp_pcb *pcb, struct pbuf *p, err_t err) {
+  (void)arg; (void)err;
+  if (p) {
+    tcp_recved(pcb, p->tot_len);
+    pbuf_free(p);
+  } else {
+    // NULL pbuf = remote closed — safe to ignore, close already in progress
+  }
+  return ERR_OK;
+}
+


NACK on this. Sending RST on unacked data is the correct behaviour for a TCP library. It is not for us to enforce this -- If a client wants to ensure the incoming data is drained before closing, it's the client's responsibility to drain the buffer before closing.

willmmiles · 2026-09-07T14:11:17Z

+    // Flush pending output before close — gives tcp_close the best chance
+    // of succeeding. Without this, queued data forces tcp_close to fail,
+    // and the tcp_shutdown fallback sends FIN with unACKed data → RST.
+    tcp_output(pcb);


This is also not correct or necessary. tcp_output() is called internally by tcp_close() if necessary.

willmmiles · 2026-09-07T14:13:19Z

-      tcp_abort(pcb);
+      // tcp_close fails when unsent data remains (e.g. HTTP response not yet ACKed).
+      // Send FIN gracefully instead of RST to avoid NS_ERROR_NET_RESET.
+      tcp_shutdown(pcb, 0, 1);


No, this is also incorrect. tcp_close behaves correctly when unacked data is present. Please remove this change.

dimecho · 2026-09-08T07:55:31Z

I'm sorry to say that the changes to the core logic are grossly incorrect, suggesting a poor understanding of LwIP and the purposes of a generic TCP client library. I haven't reviewed the SSL changes in detail yet, because to be frank, this makes me think I'd be wasting my time reviewing AI slop instead of well considered code.

From an architectual standpoint, what is the rationale for integrating the SSL layer deeply in to the AsyncClient class as opposed to constructing a wrapper class? The deep integration adds a lot of complexity and penalizes all clients whether they use the feature or not.

You are right things got a bit messy with this pull request. Copilot got very strict, in the process to greenlight it.

Probably right about an "SSL wrapper class" instead of deep intergration.

Cancel this pull request, will do more harware testing. For now will keep a fork.

dimecho added 2 commits August 22, 2026 09:55

ssl implementation

da47028

debug and testing with ESPAsyncWebServer

378dbd5

mathieucarbou requested review from mathieucarbou, me-no-dev and willmmiles and a balanced review from Copilot and removed request for Copilot August 25, 2026 10:01

mathieucarbou assigned dimecho Aug 25, 2026

mathieucarbou added the Type: Examples label Aug 25, 2026

Copilot started reviewing on behalf of mathieucarbou August 25, 2026 10:02 View session

mathieucarbou added Type: Feature and removed Type: Examples labels Aug 25, 2026

Copilot AI reviewed Aug 25, 2026

View reviewed changes

mathieucarbou reviewed Aug 25, 2026

View reviewed changes

Comment thread src/AsyncTCPVersion.h Outdated

Comment thread examples/AsyncWebServerSSL/AsyncWebServerSSL.ino Outdated

Comment thread examples/AsyncWebServerSSL/AsyncWebServerSSL.ino Outdated

Comment thread src/AsyncTCP.cpp

Comment thread src/AsyncTCP.cpp

Comment thread src/AsyncTCP.h Outdated

mathieucarbou reviewed Aug 25, 2026

View reviewed changes

Comment thread src/AsyncTCP.h

dimecho added 12 commits August 25, 2026 07:45

Clear all deferred SSL parameters on close

9c1c6d7

fix TCP backpressure in the SSL path

9f9bfb2

fix plaintext downgrade

a1e6466

allow for PEM files to loaded from LittleFS (onSslFileRequest)

5adc05a

version bump for ssl feature

f5c390f

_ssl_rx_buf juggling and move mbedtls to static (saves heap)

bdc5337

do not need flushSSLTxData

2000ace

quick example, includes PEM

fd4c369

Revert "version bump for ssl feature"

4f09123

This reverts commit f5c390f.

memory leaks

69d5e05

example now vanilla ESP32

0fa4122

fix a bunch of Copilot complaints

885e255

dimecho added 2 commits August 25, 2026 15:44

ClientSSL example + tweaks

d02fac9

webserver as AP and client STA so they can connect to each other

2bee84c

dimecho added 5 commits August 25, 2026 16:43

some strange bugs creeped in

a36650d

optimizing for speed

bda3ab7

a few more fixes

5c6e79a

support mbedtls 3.x and 4.x

2a7cdaa

compiler cleanup

bd89f2e

willmmiles requested changes Sep 7, 2026

View reviewed changes

Uh oh!

Conversation

dimecho commented Aug 25, 2026

Uh oh!

mathieucarbou commented Aug 25, 2026

Uh oh!

me-no-dev commented Aug 25, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mathieucarbou left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dimecho commented Aug 25, 2026

Uh oh!

willmmiles left a comment

Choose a reason for hiding this comment

Uh oh!

willmmiles Sep 7, 2026

Choose a reason for hiding this comment

Uh oh!

willmmiles Sep 7, 2026

Choose a reason for hiding this comment

Uh oh!

willmmiles Sep 7, 2026

Choose a reason for hiding this comment

Uh oh!

dimecho commented Sep 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

dimecho commented Sep 8, 2026 •

edited

Loading