Broker, client, and MQTT v5 packet validation and reliability fixes

examples/aws/awsiot.c

@@ -51,6 +51,7 @@
 #include "awsiot.h"
 #include "examples/mqttexample.h"
 #include "examples/mqttnet.h"
+#include "examples/mqtt_log.h"
 #include <wolfmqtt/version.h>
 
 /* Locals */
@@ -291,11 +292,18 @@ static int mqtt_aws_tls_verify_cb(int preverify, WOLFSSL_X509_STORE_CTX* store)
         PRINTF("  Rejecting cert: verification must succeed under"
                " WOLFSSL_NO_ASN_STRICT");
         return 0;
+#elif defined(WOLFMQTT_ALLOW_INSECURE_TLS)
+        /* Development/testing override only: strict ASN parsing drops
+         * Starfield Services Root CA G2 (serialNumber=0), so the chain can
+         * legitimately fail here. Accept anyway to keep the demo running.
+         * MUST NOT be defined in production - it disables authentication. */
+        PRINTF("  Allowing cert anyways (WOLFMQTT_ALLOW_INSECURE_TLS)");
 #else
-        /* Strict ASN parsing drops Starfield Services Root CA G2
-         * (serialNumber=0), so chain verification can legitimately
-         * fail here. Keep the demo running. */
-        PRINTF("  Allowing cert anyways");
+        /* Reject on any verification error by default. To run the AWS IoT
+         * demo against the live endpoint, build with WOLFSSL_NO_ASN_STRICT
+         * (loads the full trust bundle) or supply a trusted chain. */
+        PRINTF("  Rejecting cert: verification failed");
+        return 0;
 #endif
     }
 
@@ -352,6 +360,7 @@ static int mqtt_message_cb(MqttClient *client, MqttMessage *msg,
 {
     MQTTCtx* mqttCtx = (MQTTCtx*)client->ctx;
     byte buf[PRINT_BUFFER_SIZE+1];
+    char safebuf[PRINT_BUFFER_SIZE+1];
     word32 len;
 
     (void)mqttCtx;
@@ -367,7 +376,7 @@ static int mqtt_message_cb(MqttClient *client, MqttMessage *msg,
 
         /* Print incoming message */
         PRINTF("MQTT Message: Topic %s, Qos %d, Len %u",
-            buf, msg->qos, msg->total_len);
+            mqtt_log_sanitize(safebuf, (word32)sizeof(safebuf), (char*)buf), msg->qos, msg->total_len);
     }
 
     /* Print message payload */
@@ -378,7 +387,7 @@ static int mqtt_message_cb(MqttClient *client, MqttMessage *msg,
     XMEMCPY(buf, msg->buffer, len);
     buf[len] = '\0'; /* Make sure its null terminated */
     PRINTF("Payload (%d - %d) printing %d bytes:" LINE_END "%s",
-        msg->buffer_pos, msg->buffer_pos + msg->buffer_len, len, buf);
+        msg->buffer_pos, msg->buffer_pos + msg->buffer_len, len, mqtt_log_sanitize(safebuf, (word32)sizeof(safebuf), (char*)buf));
 
     if (msg_done) {
         PRINTF("MQTT Message: Done");
@@ -395,11 +404,30 @@ static int mqtt_message_cb(MqttClient *client, MqttMessage *msg,
 /* The property callback is called after decoding a packet that contains at
    least one property. The property list is deallocated after returning from
    the callback. */
+/* Copy a length-delimited, broker-controlled property string into dst and
+ * sanitize it for safe logging. */
+static const char* awsiot_log_prop(char* dst, word32 dstLen,
+    const char* src, word32 srcLen)
+{
+    char tmp[PRINT_BUFFER_SIZE + 1];
+    word32 n = srcLen;
+    if (n > PRINT_BUFFER_SIZE) {
+        n = PRINT_BUFFER_SIZE;
+    }
+    if (src != NULL && n > 0) {
+        XMEMCPY(tmp, src, n);
+    }
+    tmp[n] = '\0';
+    return mqtt_log_sanitize(dst, dstLen, tmp);
+}
+
 static int mqtt_property_cb(MqttClient *client, MqttProp *head, void *ctx)
 {
     MqttProp *prop = head;
     int rc = 0;
     MQTTCtx* mqttCtx;
+    char safebuf[PRINT_BUFFER_SIZE + 1];
+    char safebuf2[PRINT_BUFFER_SIZE + 1];
 
     if ((client == NULL) || (client->ctx == NULL)) {
         return MQTT_CODE_ERROR_BAD_ARG;
@@ -447,14 +475,17 @@ static int mqtt_property_cb(MqttClient *client, MqttProp *head, void *ctx)
                 break;
 
             case MQTT_PROP_REASON_STR:
-                PRINTF("Reason String: %.*s",
-                        prop->data_str.len, prop->data_str.str);
+                PRINTF("Reason String: %s",
+                        awsiot_log_prop(safebuf, (word32)sizeof(safebuf),
+                            prop->data_str.str, prop->data_str.len));
                 break;
 
             case MQTT_PROP_USER_PROP:
-                PRINTF("User property: key=\"%.*s\", value=\"%.*s\"",
-                        prop->data_str.len, prop->data_str.str,
-                        prop->data_str2.len, prop->data_str2.str);
+                PRINTF("User property: key=\"%s\", value=\"%s\"",
+                        awsiot_log_prop(safebuf, (word32)sizeof(safebuf),
+                            prop->data_str.str, prop->data_str.len),
+                        awsiot_log_prop(safebuf2, (word32)sizeof(safebuf2),
+                            prop->data_str2.str, prop->data_str2.len));
                 break;
 
             case MQTT_PROP_ASSIGNED_CLIENT_ID:

examples/azure/azureiothub.c

@@ -65,6 +65,7 @@
 #include "azureiothub.h"
 #include "examples/mqttexample.h"
 #include "examples/mqttnet.h"
+#include "examples/mqtt_log.h"
 
 /* Locals */
 static int mStopRead = 0;
@@ -134,6 +135,7 @@ static int mqtt_message_cb(MqttClient *client, MqttMessage *msg,
 {
     MQTTCtx* mqttCtx = (MQTTCtx*)client->ctx;
     byte buf[PRINT_BUFFER_SIZE+1];
+    char safebuf[PRINT_BUFFER_SIZE+1];
     word32 len;
 
     (void)mqttCtx;
@@ -149,7 +151,7 @@ static int mqtt_message_cb(MqttClient *client, MqttMessage *msg,
 
         /* Print incoming message */
         PRINTF("MQTT Message: Topic %s, Qos %d, Len %u",
-            buf, msg->qos, msg->total_len);
+            mqtt_log_sanitize(safebuf, (word32)sizeof(safebuf), (char*)buf), msg->qos, msg->total_len);
     }
 
     /* Print message payload */
@@ -160,7 +162,7 @@ static int mqtt_message_cb(MqttClient *client, MqttMessage *msg,
     XMEMCPY(buf, msg->buffer, len);
     buf[len] = '\0'; /* Make sure its null terminated */
     PRINTF("Payload (%d - %d) printing %d bytes:" LINE_END "%s",
-        msg->buffer_pos, msg->buffer_pos + msg->buffer_len, len, buf);
+        msg->buffer_pos, msg->buffer_pos + msg->buffer_len, len, mqtt_log_sanitize(safebuf, (word32)sizeof(safebuf), (char*)buf));
 
     if (msg_done) {
         PRINTF("MQTT Message: Done");

examples/firmware/fwclient.c

@@ -118,13 +118,29 @@ static int fw_message_process(MQTTCtx *mqttCtx, byte* buffer, word32 len)
 #ifdef ENABLE_FIRMWARE_SIG
     ecc_key eccKey;
 #endif
-    word32 check_len = sizeof(FirmwareHeader) + header->sigLen +
-        header->pubKeyLen + header->fwLen;
+    word32 remaining;
 
-    /* Verify entire message was received */
-    if (len != check_len) {
-        PRINTF("Message header vs. actual size mismatch! %d != %d",
-            len, check_len);
+    /* Validate field sizes sequentially; a summed length check can overflow. */
+    if (len < sizeof(FirmwareHeader)) {
+        PRINTF("Message smaller than firmware header! %u", (unsigned int)len);
+        return EXIT_FAILURE;
+    }
+    remaining = len - sizeof(FirmwareHeader);
+    if (header->sigLen > remaining) {
+        PRINTF("Firmware sigLen exceeds message! %u",
+            (unsigned int)header->sigLen);
+        return EXIT_FAILURE;
+    }
+    remaining -= header->sigLen;
+    if (header->pubKeyLen > remaining) {
+        PRINTF("Firmware pubKeyLen exceeds message! %u",
+            (unsigned int)header->pubKeyLen);
+        return EXIT_FAILURE;
+    }
+    remaining -= header->pubKeyLen;
+    if (header->fwLen != remaining) {
+        PRINTF("Message header vs. actual size mismatch! %u != %u",
+            (unsigned int)header->fwLen, (unsigned int)remaining);
         return EXIT_FAILURE;
     }
 
@@ -171,11 +187,14 @@ static int mqtt_message_cb(MqttClient *client, MqttMessage *msg,
     byte msg_new, byte msg_done)
 {
     MQTTCtx* mqttCtx = (MQTTCtx*)client->ctx;
+    size_t topic_len = XSTRLEN(mqttCtx->topic_name);
 
-    /* Verify this message is for the firmware topic */
+    /* Verify this message is for the firmware topic. Compare lengths without
+     * narrowing to word16 so an overlong configured topic cannot alias a
+     * 16-bit received length and over-read the length-delimited topic_name. */
     if (msg_new &&
-        XSTRNCMP(msg->topic_name, mqttCtx->topic_name,
-            msg->topic_name_len) == 0 &&
+        (size_t)msg->topic_name_len == topic_len &&
+        XMEMCMP(msg->topic_name, mqttCtx->topic_name, topic_len) == 0 &&
         !mFwBuf)
     {
         /* Allocate buffer for entire message */

examples/mqtt_log.h

@@ -40,7 +40,7 @@ extern "C" {
     #define MQTT_LOG_MAYBE_UNUSED
 #endif
 
-/* Sanitize an untrusted string for safe logging (CWE-117 defense).
+/* Sanitize an untrusted string for safe logging.
  *
  * MQTT/MQTT-SN strings such as a REGISTER topic name are controlled by the
  * remote peer. For MQTT-SN this peer is reachable over UDP, so an attacker who

examples/mqttexample.c

@@ -632,13 +632,24 @@ static int mqtt_tls_verify_cb(int preverify, WOLFSSL_X509_STORE_CTX* store)
                     wolfSSL_ERR_error_string(store->error, buffer) : "none");
     PRINTF("  Subject's domain name is %s", store->domain);
 
+#ifdef WOLFMQTT_ALLOW_INSECURE_TLS
+    /* Development/testing override only: accept any certificate. MUST NOT be
+     * defined in production builds - it disables server authentication. */
     if (store->error != 0) {
-        /* Allowing to continue */
-        /* Should check certificate and return 0 if not okay */
-        PRINTF("  Allowing cert anyways");
+        PRINTF("  Allowing cert anyways (WOLFMQTT_ALLOW_INSECURE_TLS)");
     }
-
     return 1;
+#else
+    /* With no CA configured there is no trust anchor to validate against
+     * (getting-started/demo mode), so accept and warn. When a CA is provided
+     * the chain-validation result is enforced so a bad certificate
+     * (self-signed, expired, wrong host, untrusted CA) fails the handshake. */
+    if (mqttCtx != NULL && mqttCtx->ca_file == NULL) {
+        PRINTF("  Warning: no CA configured, skipping server authentication");
+        return 1;
+    }
+    return preverify;
+#endif
 }
 
 /* Use this callback to setup TLS certificates and verify callbacks */

examples/mqttnet.c

@@ -856,14 +856,13 @@ mqttcurl_connect(SocketContext* sock, const char* host, word16 port,
             return MQTT_CODE_ERROR_CURL;
         }
 
-        /* Only do server host verification when not running against
-         * localhost broker. */
-        if (XSTRCMP(host, "localhost") == 0) {
-            res = curl_easy_setopt(sock->curl, CURLOPT_SSL_VERIFYHOST, 0L);
-        }
-        else {
-            res = curl_easy_setopt(sock->curl, CURLOPT_SSL_VERIFYHOST, 2L);
-        }
+        /* Enforce server hostname verification. The cert's CN/SAN must match
+         * the connect host (broker_test certs carry a localhost SAN). */
+#ifdef WOLFMQTT_ALLOW_INSECURE_TLS
+        res = curl_easy_setopt(sock->curl, CURLOPT_SSL_VERIFYHOST, 0L);
+#else
+        res = curl_easy_setopt(sock->curl, CURLOPT_SSL_VERIFYHOST, 2L);
+#endif
 
         if (res != CURLE_OK) {
             PRINTF("error: curl_easy_setopt(SSL_VERIFYHOST) returned: %d",

examples/mqttsimple/mqttsimple.c

@@ -28,6 +28,7 @@
 
 #include "wolfmqtt/mqtt_client.h"
 #include "examples/mqttport.h"
+#include "examples/mqtt_log.h"
 #include "mqttsimple.h"
 
 /* Requires BSD Style Socket */
@@ -84,6 +85,7 @@ static int mqtt_message_cb(MqttClient *client, MqttMessage *msg,
     byte msg_new, byte msg_done)
 {
     byte buf[PRINT_BUFFER_SIZE+1];
+    char safebuf[PRINT_BUFFER_SIZE+1];
     word32 len;
 
     (void)client;
@@ -99,7 +101,7 @@ static int mqtt_message_cb(MqttClient *client, MqttMessage *msg,
 
         /* Print incoming message */
         PRINTF("MQTT Message: Topic %s, Qos %d, Len %u",
-            buf, msg->qos, msg->total_len);
+            mqtt_log_sanitize(safebuf, (word32)sizeof(safebuf), (char*)buf), msg->qos, msg->total_len);
     }
 
     /* Print message payload */
@@ -110,7 +112,7 @@ static int mqtt_message_cb(MqttClient *client, MqttMessage *msg,
     XMEMCPY(buf, msg->buffer, len);
     buf[len] = '\0'; /* Make sure its null terminated */
     PRINTF("Payload (%d - %d) printing %d bytes:" LINE_END "%s",
-        msg->buffer_pos, msg->buffer_pos + msg->buffer_len, len, buf);
+        msg->buffer_pos, msg->buffer_pos + msg->buffer_len, len, mqtt_log_sanitize(safebuf, (word32)sizeof(safebuf), (char*)buf));
 
     if (msg_done) {
         PRINTF("MQTT Message: Done");
@@ -295,13 +297,18 @@ static int mqtt_tls_verify_cb(int preverify, WOLFSSL_X509_STORE_CTX* store)
             wolfSSL_ERR_error_string(store->error, buffer) : "none");
     PRINTF("  Subject's domain name is %s", store->domain);
 
+#ifdef WOLFMQTT_ALLOW_INSECURE_TLS
+    /* Development/testing override only: accept any certificate. MUST NOT be
+     * defined in production builds - it disables server authentication. */
     if (store->error != 0) {
-        /* Allowing to continue */
-        /* Should check certificate and return 0 if not okay */
-        PRINTF("  Allowing cert anyways");
+        PRINTF("  Allowing cert anyways (WOLFMQTT_ALLOW_INSECURE_TLS)");
     }
-
     return 1;
+#else
+    /* Propagate wolfSSL's chain-validation result so a bad certificate
+     * (self-signed, expired, wrong host, untrusted CA) fails the handshake. */
+    return preverify;
+#endif
 }
 
 /* Use this callback to setup TLS certificates and verify callbacks */

examples/multithread/multithread.c

@@ -28,6 +28,7 @@
 
 #include "multithread.h"
 #include "examples/mqttnet.h"
+#include "examples/mqtt_log.h"
 #include "examples/mqttexample.h"
 
 #include <stdint.h>
@@ -165,6 +166,7 @@ static int mqtt_message_cb(MqttClient *client, MqttMessage *msg,
     byte msg_new, byte msg_done)
 {
     byte buf[PRINT_BUFFER_SIZE+1];
+    char safebuf[PRINT_BUFFER_SIZE+1];
     word32 len;
     MQTTCtx* mqttCtx = (MQTTCtx*)client->ctx;
     (void)mqttCtx;
@@ -181,7 +183,7 @@ static int mqtt_message_cb(MqttClient *client, MqttMessage *msg,
 
             /* Print incoming message */
             PRINTF("MQTT Message: Topic %s, Qos %d, Id %d, Len %u, %u, %u",
-                buf, msg->qos, msg->packet_id, msg->total_len, msg->buffer_len,
+                mqtt_log_sanitize(safebuf, (word32)sizeof(safebuf), (char*)buf), msg->qos, msg->packet_id, msg->total_len, msg->buffer_len,
                 msg->buffer_pos);
         }
 
@@ -193,7 +195,7 @@ static int mqtt_message_cb(MqttClient *client, MqttMessage *msg,
         XMEMCPY(buf, msg->buffer, len);
         buf[len] = '\0'; /* Make sure its null terminated */
         PRINTF("Payload (%d - %d) printing %d bytes:" LINE_END "%s",
-            msg->buffer_pos, msg->buffer_pos + msg->buffer_len, len, buf);
+            msg->buffer_pos, msg->buffer_pos + msg->buffer_len, len, mqtt_log_sanitize(safebuf, (word32)sizeof(safebuf), (char*)buf));
 
         if (msg_done) {
             /* for test mode: count the number of messages received */

examples/websocket/net_libwebsockets.c

@@ -68,8 +68,13 @@ static int callback_mqtt(struct lws *wsi, enum lws_callback_reasons reason,
     else if (reason == LWS_CALLBACK_CLIENT_CONNECTION_ERROR) {
         net->status = -1;
     }
-    else if (reason == LWS_CALLBACK_CLOSED) {
+    else if (reason == LWS_CALLBACK_CLOSED ||
+             reason == LWS_CALLBACK_CLIENT_CLOSED) {
         net->status = 0;
+        /* libwebsockets frees the wsi after this callback returns; clear the
+         * dangling pointer so NetWebsocket_Disconnect's `if (net->wsi)` guard
+         * skips lws_close_reason() on freed memory. */
+        net->wsi = NULL;
     }
     else if (reason == LWS_CALLBACK_CLIENT_RECEIVE) {
         if (in && len > 0) {
@@ -185,8 +190,13 @@ int NetWebsocket_Connect(void *ctx, const char* host, word16 port,
     /* Set SSL options for the connection if TLS is enabled */
     if (mqttCtx && mqttCtx->use_tls) {
         conn_info.ssl_connection = LCCSCF_USE_SSL;
-        conn_info.ssl_connection |= LCCSCF_ALLOW_SELFSIGNED;
-        conn_info.ssl_connection |= LCCSCF_SKIP_SERVER_CERT_HOSTNAME_CHECK;
+        /* Only relax verification when no CA was supplied (dev/self-signed).
+         * When the operator provides a CA via -A, perform full chain and
+         * hostname verification rather than silently disabling it. */
+        if (mqttCtx->ca_file == NULL) {
+            conn_info.ssl_connection |= LCCSCF_ALLOW_SELFSIGNED;
+            conn_info.ssl_connection |= LCCSCF_SKIP_SERVER_CERT_HOSTNAME_CHECK;
+        }
     }
 #endif /* ENABLE_MQTT_TLS */