fix(mqtt): 8 KB MQTT task stack when TLS verifies a CA certificate

On boards with PSRAM the MQTT client runs in its own task with a 5120
byte stack. Verifying an EC certificate chain against a CA, such as
Let's Encrypt ISRG Root X2, needs about 6 KB, so the task overflowed
and the board boot looped on every connect.

The espMqttClient constructors now take the stack size, and
startClient() passes EMSESP_MQTT_STACKSIZE_CA (8192) when TLS is on
with a CA, keeping the 5120 default for insecure TLS and plain MQTT.
The client is recreated when that changes. The stack usage log
reports against the size the task was created with.

CHANGELOG_LATEST.md

@@ -71,6 +71,7 @@ This release is based on the latest Espressif/Arduino core version 3. It brings
 - a second boiler monitor `0xE4` carrying `0xFFFF` cleared a valid `pc0Flow`. That empty value is now ignored [#3224](https://github.com/emsesp/EMS-ESP32/issues/3224)
 - writing a negative custom entity value was always 1 too high because float-to-int rounding always added +0.5. Negative values now round the same way as positive ones [#3297](https://github.com/emsesp/EMS-ESP32/issues/3297)
 - after a cold boot, SoftAP fallback no longer abandons STA forever. If a home WiFi SSID is configured, EMS-ESP keeps retrying STA while the captive portal is up so a late-booting router (e.g. after a power outage) can still be joined; SoftAP stops once STA gets an IP
+- MQTT with TLS and a CA certificate crashed on every connect (a boot loop) on boards with PSRAM when the broker's certificate chain is EC, such as Let's Encrypt ISRG Root X2: verifying it needs about 6 KB of stack and the MQTT task only had 5120 bytes. The MQTT task now gets an 8 KB stack when TLS verifies a CA certificate, and keeps 5120 bytes for insecure TLS and plain MQTT
 
 ## Changed
 

lib/espMqttClient/src/MqttClient.cpp

@@ -13,7 +13,7 @@ using espMqttClientInternals::PacketType;
 using espMqttClientTypes::DisconnectReason;
 using espMqttClientTypes::Error;
 
-MqttClient::MqttClient(espMqttClientTypes::UseInternalTask useInternalTask, uint8_t priority, uint8_t core)
+MqttClient::MqttClient(espMqttClientTypes::UseInternalTask useInternalTask, uint8_t priority, uint8_t core, uint32_t stackSize)
 : _useInternalTask(useInternalTask)
 , _transport(nullptr)
 , _onConnectCallback(nullptr)
@@ -55,6 +55,7 @@ MqttClient::MqttClient(espMqttClientTypes::UseInternalTask useInternalTask, uint
 , _disconnectReason(DisconnectReason::TCP_DISCONNECTED)
 #if defined(ARDUINO_ARCH_ESP32) && ARDUHAL_LOG_LEVEL >= ARDUHAL_LOG_LEVEL_INFO
 , _highWaterMark(4294967295)
+, _stackSize(stackSize)
 #endif
   {
   EMC_GENERATE_CLIENTID(_generatedClientId);
@@ -64,12 +65,12 @@ MqttClient::MqttClient(espMqttClientTypes::UseInternalTask useInternalTask, uint
   if (_useInternalTask == espMqttClientTypes::UseInternalTask::YES) {
 #ifdef CONFIG_FREERTOS_UNICORE
     // no core to pin to on single-core targets (S2, C3, C6)
-    xTaskCreate((TaskFunction_t)_loop, "mqttclient", EMC_TASK_STACK_SIZE, this, priority, &_taskHandle);
+    xTaskCreate((TaskFunction_t)_loop, "mqttclient", stackSize, this, priority, &_taskHandle);
 #else
     if (core > 1) {
-      xTaskCreate((TaskFunction_t)_loop, "mqttclient", EMC_TASK_STACK_SIZE, this, priority, &_taskHandle);
+      xTaskCreate((TaskFunction_t)_loop, "mqttclient", stackSize, this, priority, &_taskHandle);
     } else {
-      xTaskCreatePinnedToCore((TaskFunction_t)_loop, "mqttclient", EMC_TASK_STACK_SIZE, this, priority, &_taskHandle, core);
+      xTaskCreatePinnedToCore((TaskFunction_t)_loop, "mqttclient", stackSize, this, priority, &_taskHandle, core);
     }
 #endif
   }
@@ -77,6 +78,7 @@ MqttClient::MqttClient(espMqttClientTypes::UseInternalTask useInternalTask, uint
   (void) useInternalTask;
   (void) priority;
   (void) core;
+  (void) stackSize;
 #endif
   _clientId = _generatedClientId;
   _core = core;
@@ -320,7 +322,7 @@ void MqttClient::loop() {
   size_t waterMark = uxTaskGetStackHighWaterMark(NULL);
   if (waterMark < _highWaterMark) {
     _highWaterMark = waterMark;
-    emc_log_i("Stack usage: %zu/%i", EMC_TASK_STACK_SIZE - _highWaterMark, EMC_TASK_STACK_SIZE);
+    emc_log_i("Stack usage: %zu/%zu", _stackSize - _highWaterMark, _stackSize);
   }
   #endif
 }

lib/espMqttClient/src/MqttClient.h

@@ -81,7 +81,7 @@ class MqttClient {
   }
     
  protected:
-  explicit MqttClient(espMqttClientTypes::UseInternalTask useInternalTask, uint8_t priority = 1, uint8_t core = 1);
+  explicit MqttClient(espMqttClientTypes::UseInternalTask useInternalTask, uint8_t priority = 1, uint8_t core = 1, uint32_t stackSize = EMC_TASK_STACK_SIZE);
   espMqttClientTypes::UseInternalTask _useInternalTask;
   espMqttClientInternals::Transport* _transport;
 
@@ -207,6 +207,7 @@ class MqttClient {
   #if defined(ARDUINO_ARCH_ESP32)
   #if ARDUHAL_LOG_LEVEL >= ARDUHAL_LOG_LEVEL_INFO
   size_t _highWaterMark;
+  size_t _stackSize;
   #endif
   #endif
 };

lib/espMqttClient/src/MqttClientSetup.h

@@ -208,8 +208,8 @@ class MqttClientSetup : public MqttClient {
   */
 
  protected:
-  explicit MqttClientSetup(espMqttClientTypes::UseInternalTask useInternalTask, uint8_t priority = 1, uint8_t core = 1)
+  explicit MqttClientSetup(espMqttClientTypes::UseInternalTask useInternalTask, uint8_t priority = 1, uint8_t core = 1, uint32_t stackSize = EMC_TASK_STACK_SIZE)
-  : MqttClient(useInternalTask, priority, core) {
+  : MqttClient(useInternalTask, priority, core, stackSize) {
     #if EMC_MULTIPLE_CALLBACKS
     _onConnectCallback = [this](bool sessionPresent) {
       for (auto callback : _onConnectCallbacks) if (callback.first) callback.first(sessionPresent);

lib/espMqttClient/src/espMqttClient.cpp

@@ -15,8 +15,8 @@ espMqttClient::espMqttClient(espMqttClientTypes::UseInternalTask useInternalTask
     _transport = &_client;
 }
 
-espMqttClient::espMqttClient(uint8_t priority, uint8_t core)
+espMqttClient::espMqttClient(uint8_t priority, uint8_t core, uint32_t stackSize)
-    : MqttClientSetup(espMqttClientTypes::UseInternalTask::YES, priority, core)
+    : MqttClientSetup(espMqttClientTypes::UseInternalTask::YES, priority, core, stackSize)
     , _client() {
     _transport = &_client;
 }
@@ -27,8 +27,8 @@ espMqttClientSecure::espMqttClientSecure(espMqttClientTypes::UseInternalTask use
     _transport = &_client;
 }
 
-espMqttClientSecure::espMqttClientSecure(uint8_t priority, uint8_t core)
+espMqttClientSecure::espMqttClientSecure(uint8_t priority, uint8_t core, uint32_t stackSize)
-    : MqttClientSetup(espMqttClientTypes::UseInternalTask::YES, priority, core)
+    : MqttClientSetup(espMqttClientTypes::UseInternalTask::YES, priority, core, stackSize)
     , _client() {
     _transport = &_client;
 }

lib/espMqttClient/src/espMqttClient.h

@@ -48,7 +48,7 @@ class espMqttClientSecure : public MqttClientSetup<espMqttClientSecure> {
 class espMqttClient : public MqttClientSetup<espMqttClient> {
   public:
     explicit espMqttClient(espMqttClientTypes::UseInternalTask useInternalTask);
-    explicit espMqttClient(uint8_t priority = 1, uint8_t core = 1);
+    explicit espMqttClient(uint8_t priority = 1, uint8_t core = 1, uint32_t stackSize = EMC_TASK_STACK_SIZE);
 
   protected:
     espMqttClientInternals::ClientSync _client;
@@ -57,7 +57,7 @@ class espMqttClient : public MqttClientSetup<espMqttClient> {
 class espMqttClientSecure : public MqttClientSetup<espMqttClientSecure> {
   public:
     explicit espMqttClientSecure(espMqttClientTypes::UseInternalTask useInternalTask);
-    explicit espMqttClientSecure(uint8_t priority = 1, uint8_t core = 1);
+    explicit espMqttClientSecure(uint8_t priority = 1, uint8_t core = 1, uint32_t stackSize = EMC_TASK_STACK_SIZE);
     espMqttClientSecure & setInsecure();
     espMqttClientSecure & setCACert(const char * rootCA);
     espMqttClientSecure & setCertificate(const char * clientCa);

src/ESP32React/MqttSettingsService.cpp

@@ -33,9 +33,12 @@ void MqttSettingsService::begin() {
 
 void MqttSettingsService::startClient() {
     static bool     isSecure  = false;
+    static uint32_t stackSize = EMC_TASK_STACK_SIZE;
+    // only verifying a CA chain needs the larger stack, insecure TLS and plain MQTT fit in the default
+    const uint32_t wantStack = (_state.enableTLS && _state.rootCA != "insecure") ? EMSESP_MQTT_STACKSIZE_CA : EMC_TASK_STACK_SIZE;
     if (_mqttClient) {
         // do we need to change the client?
-        if ((isSecure && _state.enableTLS) || (!isSecure && !_state.enableTLS)) {
+        if (isSecure == _state.enableTLS && stackSize == wantStack) {
             return;
         }
 
@@ -43,12 +46,14 @@ void MqttSettingsService::startClient() {
         _mqttClient = nullptr;
     }
 
+    stackSize = wantStack;
+
     if (_state.enableTLS) {
         isSecure = true;
         if (emsesp::EMSESP::system_.PSram() == 0) {
             _mqttClient = new espMqttClientSecure(espMqttClientTypes::UseInternalTask::NO);
         } else {
-            _mqttClient = new espMqttClientSecure(EMSESP_MQTT_PRIORITY, EMSESP_MQTT_RUNNING_CORE);
+            _mqttClient = new espMqttClientSecure(EMSESP_MQTT_PRIORITY, EMSESP_MQTT_RUNNING_CORE, wantStack);
         }
 
         if (!_mqttClient) {

src/ESP32React/MqttSettingsService.h

@@ -60,6 +60,11 @@
 #define EMC_TASK_STACK_SIZE EMSESP_MQTT_STACKSIZE
 #endif
 
+// MQTT task stack when TLS verifies the broker against a CA. BearSSL needs ~6 KB to verify an EC (P-384, P-521) chain
+#ifndef EMSESP_MQTT_STACKSIZE_CA
+#define EMSESP_MQTT_STACKSIZE_CA 8192
+#endif
+
 #ifndef EMSESP_MQTT_PRIORITY
 #define EMSESP_MQTT_PRIORITY 1
 #endif