diff --git a/src/helpers/bridges/MQTTBridge.cpp b/src/helpers/bridges/MQTTBridge.cpp index a45dd6124..056af62a6 100644 --- a/src/helpers/bridges/MQTTBridge.cpp +++ b/src/helpers/bridges/MQTTBridge.cpp @@ -1,4 +1,5 @@ #include "MQTTBridge.h" +#include "MQTTErrorLabels.h" #include "../WifiPowerSavePolicy.h" #include "../MQTTConnectionPolicy.h" #include "../MQTTMessageBuilder.h" @@ -22,6 +23,7 @@ #ifdef ESP_PLATFORM #include +#include #include #include #include @@ -463,41 +465,52 @@ const char* MQTTBridge::getSlotPresetName(int slot_index) const { return MQTT_PRESET_CUSTOM; } +// The label tables live in MQTTErrorLabels.h so host tests can exercise them. +// These asserts are the contract between that pure table and the SDK actually +// being compiled against: a framework bump that renumbers a reason or an +// esp-tls error fails here instead of silently mislabelling it in the field. +#ifdef ESP_PLATFORM +static_assert(MQTTErrorLabels::kWifiNoApFound == WIFI_REASON_NO_AP_FOUND, "wifi reason drift"); +static_assert(MQTTErrorLabels::kWifiAuthFail == WIFI_REASON_AUTH_FAIL, "wifi reason drift"); +static_assert(MQTTErrorLabels::kWifiAssocFail == WIFI_REASON_ASSOC_FAIL, "wifi reason drift"); +static_assert(MQTTErrorLabels::kWifiHandshakeTimeout == WIFI_REASON_HANDSHAKE_TIMEOUT, "wifi reason drift"); +static_assert(MQTTErrorLabels::kWifiConnectionFail == WIFI_REASON_CONNECTION_FAIL, "wifi reason drift"); +static_assert(MQTTErrorLabels::kWifiBeaconTimeout == WIFI_REASON_BEACON_TIMEOUT, "wifi reason drift"); +static_assert(MQTTErrorLabels::kWifiApTsfReset == WIFI_REASON_AP_TSF_RESET, "wifi reason drift"); +static_assert(MQTTErrorLabels::kWifiRoaming == WIFI_REASON_ROAMING, "wifi reason drift"); +static_assert(MQTTErrorLabels::kWifiSaQueryTimeout == WIFI_REASON_SA_QUERY_TIMEOUT, "wifi reason drift"); +static_assert(MQTTErrorLabels::kWifiAuthExpire == WIFI_REASON_AUTH_EXPIRE, "wifi reason drift"); +static_assert(MQTTErrorLabels::kWifiAssocExpire == WIFI_REASON_ASSOC_EXPIRE, "wifi reason drift"); +static_assert(MQTTErrorLabels::kWifiAssocLeave == WIFI_REASON_ASSOC_LEAVE, "wifi reason drift"); +static_assert(MQTTErrorLabels::kWifiBssTransitionDisassoc == WIFI_REASON_BSS_TRANSITION_DISASSOC, "wifi reason drift"); +static_assert(MQTTErrorLabels::kWifi4WayHandshakeTimeout == WIFI_REASON_4WAY_HANDSHAKE_TIMEOUT, "wifi reason drift"); +static_assert(MQTTErrorLabels::kWifiGroupCipherInvalid == WIFI_REASON_GROUP_CIPHER_INVALID, "wifi reason drift"); +static_assert(MQTTErrorLabels::kWifiCipherSuiteRejected == WIFI_REASON_CIPHER_SUITE_REJECTED, "wifi reason drift"); +static_assert(MQTTErrorLabels::kWifiMissingAcks == WIFI_REASON_MISSING_ACKS, "wifi reason drift"); +static_assert(MQTTErrorLabels::kWifiTimeout == WIFI_REASON_TIMEOUT, "wifi reason drift"); +static_assert(MQTTErrorLabels::kWifiInvalidPmkid == WIFI_REASON_INVALID_PMKID, "wifi reason drift"); + +static_assert(MQTTErrorLabels::kTlsCannotResolveHostname == ESP_ERR_ESP_TLS_CANNOT_RESOLVE_HOSTNAME, "esp-tls error drift"); +static_assert(MQTTErrorLabels::kTlsCannotCreateSocket == ESP_ERR_ESP_TLS_CANNOT_CREATE_SOCKET, "esp-tls error drift"); +static_assert(MQTTErrorLabels::kTlsUnsupportedProtoFamily == ESP_ERR_ESP_TLS_UNSUPPORTED_PROTOCOL_FAMILY, "esp-tls error drift"); +static_assert(MQTTErrorLabels::kTlsFailedConnectToHost == ESP_ERR_ESP_TLS_FAILED_CONNECT_TO_HOST, "esp-tls error drift"); +static_assert(MQTTErrorLabels::kTlsSocketSetoptFailed == ESP_ERR_ESP_TLS_SOCKET_SETOPT_FAILED, "esp-tls error drift"); +static_assert(MQTTErrorLabels::kTlsConnectionTimeout == ESP_ERR_ESP_TLS_CONNECTION_TIMEOUT, "esp-tls error drift"); +static_assert(MQTTErrorLabels::kTlsTcpClosedFin == ESP_ERR_ESP_TLS_TCP_CLOSED_FIN, "esp-tls error drift"); +static_assert(MQTTErrorLabels::kTlsMbedtlsCertPartlyOk == ESP_ERR_MBEDTLS_CERT_PARTLY_OK, "esp-tls error drift"); +static_assert(MQTTErrorLabels::kTlsMbedtlsSetHostname == ESP_ERR_MBEDTLS_SSL_SET_HOSTNAME_FAILED, "esp-tls error drift"); +static_assert(MQTTErrorLabels::kTlsMbedtlsX509ParseFailed == ESP_ERR_MBEDTLS_X509_CRT_PARSE_FAILED, "esp-tls error drift"); +static_assert(MQTTErrorLabels::kTlsMbedtlsSslSetupFailed == ESP_ERR_MBEDTLS_SSL_SETUP_FAILED, "esp-tls error drift"); +static_assert(MQTTErrorLabels::kTlsMbedtlsSslWriteFailed == ESP_ERR_MBEDTLS_SSL_WRITE_FAILED, "esp-tls error drift"); +static_assert(MQTTErrorLabels::kTlsMbedtlsHandshakeFailed == ESP_ERR_MBEDTLS_SSL_HANDSHAKE_FAILED, "esp-tls error drift"); +#endif + const char* MQTTBridge::wifiReasonStr(uint8_t reason) { - switch (reason) { - case 2: return "auth expired"; - case 4: return "assoc timeout"; - case 8: return "AP disconnected"; - case 15: return "4-way handshake timeout"; - case 18: return "group cipher mismatch"; - case 40: return "cipher suite rejected"; - case 49: return "invalid PMKID"; - case 61: return "AP BSS management"; - case 88: return "AP BSS management"; - case 168: return "AP band-steering kick"; - case 34: return "AP state mismatch (class 3 frame)"; - case 39: return "SSID not found"; - case 63: return "SA query timeout (PMF)"; - case 200: return "signal lost"; - case 201: return "security mismatch"; - case 202: return "auth mode rejected"; - case 204: return "handshake timeout"; - default: return nullptr; - } + return MQTTErrorLabels::wifiReason(reason); } const char* MQTTBridge::tlsErrorStr(int32_t err) { - switch (err) { - case 0x8001: return "DNS failed"; - case 0x8002: return "socket error"; - case 0x8004: return "connect refused"; - case 0x8006: return "TLS timeout"; - case 0x8008: return "connection timeout"; - case 0x800B: return "cert verify failed"; - case 0x8010: return "mbedTLS error"; - case 0x801A: return "TLS handshake failed"; - default: return nullptr; - } + return MQTTErrorLabels::tlsError(err); } void MQTTBridge::formatSlotDiagReply(char* buf, size_t bufsize, int slot_index) { @@ -564,6 +577,16 @@ void MQTTBridge::formatSlotDiagReply(char* buf, size_t bufsize, int slot_index) if (slot.connected && slot.last_error_time == 0) { replyAppendf(buf, bufsize, &pos, ", no errors"); } else if (slot.last_error_time > 0) { + // Broker refusal: the CONNECT reached a broker that answered "no". Reported + // first because the transport fields below are empty or irrelevant then. + if (slot.last_connack_code != 0) { + const char* why = MQTTErrorLabels::connackReason(slot.last_connack_code); + if (why) { + replyAppendf(buf, bufsize, &pos, ", refused: %s (%u)", why, (unsigned)slot.last_connack_code); + } else { + replyAppendf(buf, bufsize, &pos, ", refused: code %u", (unsigned)slot.last_connack_code); + } + } // TLS error with human-friendly description if (slot.last_tls_err != 0) { const char* desc = tlsErrorStr(slot.last_tls_err); @@ -1660,6 +1683,7 @@ bool MQTTBridge::ensureSlotClient(int index) { _slots[index].last_tls_err = 0; _slots[index].last_tls_stack_err = 0; _slots[index].last_sock_errno = 0; + _slots[index].last_connack_code = 0; _slots[index].last_error_time = 0; _slots[index].current_outage_started_ms = 0; // clear current-outage timer for AlertReporter updateCachedConnectionStatus(); // bool store — safe from this (esp-mqtt) task @@ -1689,6 +1713,11 @@ bool MQTTBridge::ensureSlotClient(int index) { _slots[index].last_tls_stack_err = error.esp_tls_stack_err; _slots[index].last_sock_errno = error.esp_transport_sock_errno; _slots[index].last_error_time = millis(); + // Cleared on any other error type so the diag describes the latest failure + // rather than pairing a fresh transport error with an old refusal. + _slots[index].last_connack_code = + (error.error_type == MQTT_ERROR_TYPE_CONNECTION_REFUSED) + ? (uint8_t)error.connect_return_code : 0; if (error.error_type == MQTT_ERROR_TYPE_CONNECTION_REFUSED) { _slot_force_jwt_mint[index] = true; // Broker rejected the MQTT CONNECT itself — not a transport failure. diff --git a/src/helpers/bridges/MQTTBridge.h b/src/helpers/bridges/MQTTBridge.h index b4ee7b992..9dd3f29c4 100644 --- a/src/helpers/bridges/MQTTBridge.h +++ b/src/helpers/bridges/MQTTBridge.h @@ -135,6 +135,10 @@ private: int32_t last_tls_err; // esp_tls_last_esp_err (0 = no error) int32_t last_tls_stack_err; // mbedTLS stack error int last_sock_errno; // socket errno + // CONNACK return code from the last broker refusal (0 = none). A refusal is + // not a transport failure, and without this the diag shows a slot with wrong + // credentials as an unexplained disconnect. + uint8_t last_connack_code; unsigned long last_error_time; // millis() of last error uint32_t disconnect_count; // Number of disconnect callbacks since boot unsigned long first_disconnect_time; // millis() of first disconnect after boot diff --git a/src/helpers/bridges/MQTTErrorLabels.h b/src/helpers/bridges/MQTTErrorLabels.h new file mode 100644 index 000000000..cee1b6880 --- /dev/null +++ b/src/helpers/bridges/MQTTErrorLabels.h @@ -0,0 +1,125 @@ +#pragma once + +#include + +// Operator-facing labels for the SDK error codes the observer reports through +// `get wifi.status`, `get mqttN.diag` and the web config panel. +// +// Pure lookup: no Arduino or ESP headers, so the table is host-testable. The +// numeric values below are the installed SDK's; MQTTBridge.cpp static_asserts +// every one of them against the real enum/#define, so a framework bump either +// keeps these labels honest or fails the build. Codes with no entry return +// nullptr and the caller prints the raw number — an unknown code must never be +// given a neighbouring code's name. +namespace MQTTErrorLabels { + +// esp_wifi_types.h wifi_err_reason_t. Values below 200 are 802.11 reason codes +// forwarded from the AP; 200+ are Espressif's own local failures. +enum WifiReason : uint16_t { + kWifiAuthExpire = 2, + kWifiAssocExpire = 4, + kWifiAssocLeave = 8, + kWifi4WayHandshakeTimeout = 15, + kWifiGroupCipherInvalid = 18, + kWifiCipherSuiteRejected = 24, + kWifiMissingAcks = 34, + kWifiTimeout = 39, + kWifiInvalidPmkid = 49, + kWifiBssTransitionDisassoc = 12, + kWifiBeaconTimeout = 200, + kWifiNoApFound = 201, + kWifiAuthFail = 202, + kWifiAssocFail = 203, + kWifiHandshakeTimeout = 204, + kWifiConnectionFail = 205, + kWifiApTsfReset = 206, + kWifiRoaming = 207, + kWifiSaQueryTimeout = 209, +}; + +// esp_tls_errors.h, ESP_ERR_ESP_TLS_BASE (0x8000) + offset. +enum TlsError : int32_t { + kTlsCannotResolveHostname = 0x8001, + kTlsCannotCreateSocket = 0x8002, + kTlsUnsupportedProtoFamily= 0x8003, + kTlsFailedConnectToHost = 0x8004, + kTlsSocketSetoptFailed = 0x8005, + kTlsConnectionTimeout = 0x8006, + kTlsTcpClosedFin = 0x8008, + kTlsMbedtlsCertPartlyOk = 0x8010, + kTlsMbedtlsSetHostname = 0x8012, + kTlsMbedtlsX509ParseFailed= 0x8015, + kTlsMbedtlsSslSetupFailed = 0x8017, + kTlsMbedtlsSslWriteFailed = 0x8018, + kTlsMbedtlsHandshakeFailed= 0x801A, +}; + +// MQTT 3.1.1 CONNACK return codes (esp_mqtt_error_codes.connect_return_code). +enum ConnackCode : uint8_t { + kConnackAccepted = 0, + kConnackBadProtocol = 1, + kConnackIdRejected = 2, + kConnackServerUnavail = 3, + kConnackBadCredentials = 4, + kConnackNotAuthorized = 5, +}; + +// Short enough to sit inside a LoRa-bounded diagnostic reply next to the raw code. +static inline const char* wifiReason(uint8_t reason) { + switch (reason) { + case kWifiAuthExpire: return "auth expired"; + case kWifiAssocExpire: return "assoc expired"; + case kWifiAssocLeave: return "AP disconnected"; + case kWifiBssTransitionDisassoc: return "AP steered us away"; + case kWifi4WayHandshakeTimeout: return "4-way handshake timeout"; + case kWifiGroupCipherInvalid: return "group cipher mismatch"; + case kWifiCipherSuiteRejected: return "cipher suite rejected"; + case kWifiMissingAcks: return "AP saw no acks"; + case kWifiTimeout: return "802.11 timeout"; + case kWifiInvalidPmkid: return "invalid PMKID"; + case kWifiBeaconTimeout: return "beacon lost"; + case kWifiNoApFound: return "SSID not found"; + case kWifiAuthFail: return "auth failed (check password)"; + case kWifiAssocFail: return "association failed"; + case kWifiHandshakeTimeout: return "handshake timeout"; + case kWifiConnectionFail: return "connect failed"; + case kWifiApTsfReset: return "AP restarted"; + case kWifiRoaming: return "roaming"; + case kWifiSaQueryTimeout: return "SA query timeout (PMF)"; + default: return nullptr; + } +} + +static inline const char* tlsError(int32_t err) { + switch (err) { + case kTlsCannotResolveHostname: return "DNS failed"; + case kTlsCannotCreateSocket: return "socket error"; + case kTlsUnsupportedProtoFamily: return "unsupported protocol family"; + case kTlsFailedConnectToHost: return "connect failed"; + case kTlsSocketSetoptFailed: return "socket setopt failed"; + case kTlsConnectionTimeout: return "connection timeout"; + case kTlsTcpClosedFin: return "server closed connection"; + case kTlsMbedtlsCertPartlyOk: return "cert chain partly parsed"; + case kTlsMbedtlsSetHostname: return "SNI hostname rejected"; + case kTlsMbedtlsX509ParseFailed: return "cert parse failed"; + case kTlsMbedtlsSslSetupFailed: return "TLS setup failed"; + case kTlsMbedtlsSslWriteFailed: return "TLS write failed"; + case kTlsMbedtlsHandshakeFailed: return "TLS handshake failed"; + default: return nullptr; + } +} + +// The broker answered and refused: this is what an operator needs instead of a +// transport error, which is why the bridge keeps the CONNACK code separately. +static inline const char* connackReason(uint8_t code) { + switch (code) { + case kConnackBadProtocol: return "protocol rejected"; + case kConnackIdRejected: return "client id rejected"; + case kConnackServerUnavail: return "server unavailable"; + case kConnackBadCredentials: return "bad user/password"; + case kConnackNotAuthorized: return "not authorized"; + default: return nullptr; + } +} + +} // namespace MQTTErrorLabels diff --git a/test/test_mqtt_error_labels/test_mqtt_error_labels.cpp b/test/test_mqtt_error_labels/test_mqtt_error_labels.cpp new file mode 100644 index 000000000..604789bcd --- /dev/null +++ b/test/test_mqtt_error_labels/test_mqtt_error_labels.cpp @@ -0,0 +1,78 @@ +#include "helpers/bridges/MQTTErrorLabels.h" + +#include + +#include + +using namespace MQTTErrorLabels; + +namespace { + +bool labelIs(const char* actual, const char* expected) { + return actual != nullptr && strcmp(actual, expected) == 0; +} + +} // namespace + +// F14: these four were the mislabels found in review. 201 is "no AP found" +// (usually a wrong/hidden SSID), not a security mismatch; 202 is an auth +// failure (usually a wrong password), not a rejected auth mode. +TEST(MQTTErrorLabels, WifiReasonsMatchTheSdkMeaning) { + EXPECT_TRUE(labelIs(wifiReason(201), "SSID not found")); + EXPECT_TRUE(labelIs(wifiReason(202), "auth failed (check password)")); + EXPECT_TRUE(labelIs(wifiReason(39), "802.11 timeout")); // was "SSID not found" + EXPECT_TRUE(labelIs(wifiReason(34), "AP saw no acks")); // was "AP state mismatch" +} + +TEST(MQTTErrorLabels, TlsErrorsMatchTheSdkMeaning) { + EXPECT_TRUE(labelIs(tlsError(0x8008), "server closed connection")); // was "connection timeout" + EXPECT_TRUE(labelIs(tlsError(0x8010), "cert chain partly parsed")); // was "mbedTLS error" + EXPECT_TRUE(labelIs(tlsError(0x8006), "connection timeout")); + EXPECT_TRUE(labelIs(tlsError(0x8001), "DNS failed")); + EXPECT_TRUE(labelIs(tlsError(0x801A), "TLS handshake failed")); +} + +// An unknown code must fall through to the caller's raw-number formatting +// rather than borrow a neighbouring code's label. 0x800B is the one the review +// caught: no such esp-tls error exists, but it was labelled "cert verify failed". +TEST(MQTTErrorLabels, UnknownCodesHaveNoLabel) { + EXPECT_EQ(nullptr, tlsError(0x800B)); + EXPECT_EQ(nullptr, tlsError(0)); + EXPECT_EQ(nullptr, tlsError(-1)); + EXPECT_EQ(nullptr, tlsError(0x8099)); + + EXPECT_EQ(nullptr, wifiReason(0)); + EXPECT_EQ(nullptr, wifiReason(61)); // not defined by the installed SDK + EXPECT_EQ(nullptr, wifiReason(88)); + EXPECT_EQ(nullptr, wifiReason(168)); + EXPECT_EQ(nullptr, wifiReason(255)); +} + +TEST(MQTTErrorLabels, ConnackReasonsCoverTheRefusalCodes) { + EXPECT_TRUE(labelIs(connackReason(1), "protocol rejected")); + EXPECT_TRUE(labelIs(connackReason(2), "client id rejected")); + EXPECT_TRUE(labelIs(connackReason(3), "server unavailable")); + EXPECT_TRUE(labelIs(connackReason(4), "bad user/password")); + EXPECT_TRUE(labelIs(connackReason(5), "not authorized")); + EXPECT_EQ(nullptr, connackReason(0)); // accepted: not an error to report + EXPECT_EQ(nullptr, connackReason(6)); +} + +// Labels share a bounded LoRa reply with the rest of the diagnostic line. +TEST(MQTTErrorLabels, LabelsStayShortEnoughForABoundedReply) { + for (int i = 0; i <= 255; i++) { + const char* w = wifiReason((uint8_t)i); + if (w) EXPECT_LE(strlen(w), 30u) << "wifi reason " << i; + const char* c = connackReason((uint8_t)i); + if (c) EXPECT_LE(strlen(c), 30u) << "connack code " << i; + } + for (int32_t e = 0x8000; e <= 0x8040; e++) { + const char* t = tlsError(e); + if (t) EXPECT_LE(strlen(t), 30u) << "tls error " << e; + } +} + +int main(int argc, char** argv) { + ::testing::InitGoogleTest(&argc, argv); + return RUN_ALL_TESTS(); +}