Fixes from review

Author: embhorn

src/mqtt_broker.c

@@ -35,7 +35,19 @@
 
 #ifdef WOLFMQTT_BROKER
 
-#define BROKER_FORCE_ZERO(mem, len) Mqtt_ForceZero(mem, (word32)(len))
+/* Secure memory zeroing - uses volatile pointer to prevent the compiler
+ * from optimizing away the stores (dead-store elimination). */
+static void MqttBroker_ForceZero(void* mem, word32 len)
+{
+    volatile byte* p = (volatile byte*)mem;
+    word32 i;
+    for (i = 0; i < len; i++) {
+        p[i] = 0;
+    }
+}
+
+#define BROKER_FORCE_ZERO(mem, len) \
+    MqttBroker_ForceZero(mem, (word32)(len))
 
 /* -------------------------------------------------------------------------- */
 /* Platform includes                                                           */
@@ -2693,6 +2705,13 @@ static int BrokerHandle_Connect(BrokerClient* bc, int rx_len,
         return rc;
     }
 
+#ifdef WOLFMQTT_V5
+    /* Initialize early so every `goto send_connack` below produces a CONNACK
+     * matching the client's protocol level (v5 CONNACK has a Properties
+     * length and uses v5 reason codes). */
+    ack.protocol_level = mc.protocol_level;
+#endif
+
     /* Store client ID */
 #ifdef WOLFMQTT_STATIC_MEMORY
     bc->client_id[0] = '\0';
@@ -2909,7 +2928,6 @@ static int BrokerHandle_Connect(BrokerClient* bc, int rx_len,
     ack.flags = 0;
     ack.return_code = MQTT_CONNECT_ACK_CODE_ACCEPTED;
 #ifdef WOLFMQTT_V5
-    ack.protocol_level = mc.protocol_level;
     ack.props = NULL;
 #endif
 

src/mqtt_client.c

@@ -26,7 +26,19 @@
 
 #include "wolfmqtt/mqtt_client.h"
 
-#define CLIENT_FORCE_ZERO(mem, len) Mqtt_ForceZero(mem, (word32)(len))
+/* Secure memory zeroing - uses volatile pointer to prevent the compiler
+ * from optimizing away the stores (dead-store elimination). */
+static void MqttClient_ForceZero(void* mem, word32 len)
+{
+    volatile byte* p = (volatile byte*)mem;
+    word32 i;
+    for (i = 0; i < len; i++) {
+        p[i] = 0;
+    }
+}
+
+#define CLIENT_FORCE_ZERO(mem, len) \
+    MqttClient_ForceZero(mem, (word32)(len))
 
 /* DOCUMENTED BUILD OPTIONS:
  *
@@ -2918,6 +2930,7 @@ int MqttClient_NetDisconnect(MqttClient *client)
 {
 #ifdef WOLFMQTT_MULTITHREAD
     MqttPendResp *tmpResp;
+    MqttPendResp *nextResp;
     int rc;
 #endif
 
@@ -2932,7 +2945,6 @@ int MqttClient_NetDisconnect(MqttClient *client)
     #ifdef WOLFMQTT_DEBUG_CLIENT
         PRINTF("Net Disconnect: Removing pending responses");
     #endif
-        MqttPendResp *nextResp;
         for (tmpResp = client->firstPendResp;
              tmpResp != NULL;
              tmpResp = nextResp) {

src/mqtt_packet.c

@@ -2741,19 +2741,24 @@ int MqttDecode_Auth(byte *rx_buf, int rx_buf_len, MqttAuth *auth)
                     if (tmp < 0)
                         return tmp;
                     rx_payload += tmp;
-            }
-            else if (auth->reason_code != MQTT_REASON_SUCCESS) {
-                /* The Reason Code and Property Length can be omitted if the
-                   Reason Code is 0x00 (Success) and there are no Properties.
-                   In this case the AUTH has a Remaining Length of 0. */
-                return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_MALFORMED_DATA);
-            }
-            if (auth->props != NULL) {
-                /* Must have Authentication Method */
+                }
+                else if (auth->reason_code != MQTT_REASON_SUCCESS) {
+                    /* The Reason Code and Property Length can be omitted if
+                       the Reason Code is 0x00 (Success) and there are no
+                       Properties. In this case the AUTH has a Remaining
+                       Length of 0. */
+                    return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_MALFORMED_DATA);
+                }
+                if (auth->props != NULL) {
+                    /* Must have Authentication Method */
 
-                /* Must have Authentication Data */
+                    /* Must have Authentication Data */
 
-                /* May have zero or more User Property pairs */
+                    /* May have zero or more User Property pairs */
+                }
+                else {
+                    return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_MALFORMED_DATA);
+                }
             }
             else {
                 return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_MALFORMED_DATA);
@@ -2762,10 +2767,6 @@ int MqttDecode_Auth(byte *rx_buf, int rx_buf_len, MqttAuth *auth)
         else {
             return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_MALFORMED_DATA);
         }
-        }
-        else {
-            return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_MALFORMED_DATA);
-        }
     }
     else {
         /* Per MQTT 5.0 section 3.15.2: Remaining Length of 0 implies

wolfmqtt/mqtt_types.h

@@ -358,17 +358,6 @@ enum MqttPacketResponseCodes {
     #define WOLFMQTT_NORETURN
 #endif
 
-/* Secure memory zeroing - uses volatile pointer to prevent compiler
- * from optimizing away the stores (dead-store elimination). */
-static INLINE void Mqtt_ForceZero(void* mem, word32 len)
-{
-    volatile byte* p = (volatile byte*)mem;
-    word32 i;
-    for (i = 0; i < len; i++) {
-        p[i] = 0;
-    }
-}
-
 /* Logging / Tracing */
 #ifdef WOLFMQTT_NO_STDIO
     #undef WOLFMQTT_DEBUG_CLIENT