Further cleanups to broker code.

Author: dgarske

src/mqtt_broker.c

@@ -6,7 +6,7 @@
  *
  * wolfMQTT is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
+ * the Free Software Foundation; either version 3 of the License, or
  * (at your option) any later version.
  *
  * wolfMQTT is distributed in the hope that it will be useful,
@@ -24,6 +24,7 @@
     #include <config.h>
 #endif
 
+#include "wolfmqtt/mqtt_types.h"
 #include "wolfmqtt/mqtt_broker.h"
 #include "wolfmqtt/mqtt_client.h"
 #include "wolfmqtt/mqtt_packet.h"
@@ -46,22 +47,14 @@
     #include <sys/socket.h>
     #include <time.h>
     #include <unistd.h>
-#ifdef ENABLE_MQTT_TLS
-    #include <wolfssl/ssl.h>
-#endif
 #endif /* !WOLFMQTT_BROKER_CUSTOM_NET */
 
 /* -------------------------------------------------------------------------- */
 /* Default time abstraction                                                    */
 /* -------------------------------------------------------------------------- */
 #ifndef WOLFMQTT_BROKER_GET_TIME_S
-    #ifndef WOLFMQTT_BROKER_CUSTOM_NET
-        #define WOLFMQTT_BROKER_GET_TIME_S() \
-            ((WOLFMQTT_BROKER_TIME_T)time(NULL))
-    #else
-        #error "WOLFMQTT_BROKER_CUSTOM_NET requires " \
-               "WOLFMQTT_BROKER_GET_TIME_S to be defined"
-    #endif
+    #define WOLFMQTT_BROKER_GET_TIME_S() \
+        ((WOLFMQTT_BROKER_TIME_T)time(NULL))
 #endif
 
 /* -------------------------------------------------------------------------- */
@@ -70,11 +63,8 @@
 #ifndef BROKER_SLEEP_MS
     #ifdef USE_WINDOWS_API
         #define BROKER_SLEEP_MS(ms) Sleep(ms)
-    #elif !defined(WOLFMQTT_BROKER_CUSTOM_NET)
-        #define BROKER_SLEEP_MS(ms) usleep((unsigned)(ms) * 1000)
     #else
-        #error "WOLFMQTT_BROKER_CUSTOM_NET requires " \
-               "BROKER_SLEEP_MS to be defined"
+        #define BROKER_SLEEP_MS(ms) usleep((unsigned)(ms) * 1000)
     #endif
 #endif
 
@@ -351,84 +341,96 @@ int MqttBrokerNet_Init(MqttBrokerNet* net)
 #ifdef ENABLE_MQTT_TLS
 static int BrokerTls_Init(MqttBroker* broker)
 {
-    WOLFSSL_CTX* ctx;
+    WOLFSSL_CTX* ctx = NULL;
     int rc;
 
     rc = wolfSSL_Init();
     if (rc != WOLFSSL_SUCCESS) {
         PRINTF("broker: wolfSSL_Init failed %d", rc);
-        return MQTT_CODE_ERROR_BAD_ARG;
+        rc = MQTT_CODE_ERROR_BAD_ARG;
     }
 
     /* Select TLS method based on version preference */
-    if (broker->tls_version == 12) {
-        ctx = wolfSSL_CTX_new(wolfTLSv1_2_server_method());
-    }
-    else if (broker->tls_version == 13) {
-        ctx = wolfSSL_CTX_new(wolfTLSv1_3_server_method());
-    }
-    else {
-        /* Default: auto-negotiate (supports TLS 1.2 and 1.3) */
-        ctx = wolfSSL_CTX_new(wolfSSLv23_server_method());
-    }
-    if (ctx == NULL) {
-        PRINTF("broker: wolfSSL_CTX_new failed");
-        wolfSSL_Cleanup();
-        return MQTT_CODE_ERROR_MEMORY;
+    if (rc == WOLFSSL_SUCCESS) {
+        if (broker->tls_version == 12) {
+            ctx = wolfSSL_CTX_new(wolfTLSv1_2_server_method());
+        }
+        else if (broker->tls_version == 13) {
+            ctx = wolfSSL_CTX_new(wolfTLSv1_3_server_method());
+        }
+        else {
+            ctx = wolfSSL_CTX_new(wolfSSLv23_server_method());
+        }
+        if (ctx == NULL) {
+            PRINTF("broker: wolfSSL_CTX_new failed");
+            rc = MQTT_CODE_ERROR_MEMORY;
+        }
     }
 
     /* Load server certificate */
-    if (broker->tls_cert == NULL) {
-        PRINTF("broker: TLS cert not set (-c)");
-        wolfSSL_CTX_free(ctx);
-        wolfSSL_Cleanup();
-        return MQTT_CODE_ERROR_BAD_ARG;
+    if (rc == WOLFSSL_SUCCESS) {
+        if (broker->tls_cert == NULL) {
+            PRINTF("broker: TLS cert not set (-c)");
+            rc = MQTT_CODE_ERROR_BAD_ARG;
+        }
     }
-    rc = wolfSSL_CTX_use_certificate_file(ctx, broker->tls_cert,
-        WOLFSSL_FILETYPE_PEM);
-    if (rc != WOLFSSL_SUCCESS) {
-        PRINTF("broker: load cert failed %d (%s)", rc, broker->tls_cert);
-        wolfSSL_CTX_free(ctx);
-        wolfSSL_Cleanup();
-        return MQTT_CODE_ERROR_BAD_ARG;
+    if (rc == WOLFSSL_SUCCESS) {
+        rc = wolfSSL_CTX_use_certificate_file(ctx, broker->tls_cert,
+            WOLFSSL_FILETYPE_PEM);
+        if (rc != WOLFSSL_SUCCESS) {
+            PRINTF("broker: load cert failed %d (%s)", rc, broker->tls_cert);
+            rc = MQTT_CODE_ERROR_BAD_ARG;
+        }
     }
 
     /* Load server private key */
-    if (broker->tls_key == NULL) {
-        PRINTF("broker: TLS key not set (-K)");
-        wolfSSL_CTX_free(ctx);
-        wolfSSL_Cleanup();
-        return MQTT_CODE_ERROR_BAD_ARG;
+    if (rc == WOLFSSL_SUCCESS) {
+        if (broker->tls_key == NULL) {
+            PRINTF("broker: TLS key not set (-K)");
+            rc = MQTT_CODE_ERROR_BAD_ARG;
+        }
     }
-    rc = wolfSSL_CTX_use_PrivateKey_file(ctx, broker->tls_key,
-        WOLFSSL_FILETYPE_PEM);
-    if (rc != WOLFSSL_SUCCESS) {
-        PRINTF("broker: load key failed %d (%s)", rc, broker->tls_key);
-        wolfSSL_CTX_free(ctx);
-        wolfSSL_Cleanup();
-        return MQTT_CODE_ERROR_BAD_ARG;
+    if (rc == WOLFSSL_SUCCESS) {
+        rc = wolfSSL_CTX_use_PrivateKey_file(ctx, broker->tls_key,
+            WOLFSSL_FILETYPE_PEM);
+        if (rc != WOLFSSL_SUCCESS) {
+            PRINTF("broker: load key failed %d (%s)", rc, broker->tls_key);
+            rc = MQTT_CODE_ERROR_BAD_ARG;
+        }
     }
 
-    /* Set wolfSSL IO callbacks (reuse existing WOLFMQTT_API functions) */
-    wolfSSL_CTX_SetIORecv(ctx, MqttSocket_TlsSocketReceive);
-    wolfSSL_CTX_SetIOSend(ctx, MqttSocket_TlsSocketSend);
+    /* Set wolfSSL IO callbacks */
+    if (rc == WOLFSSL_SUCCESS) {
+        wolfSSL_CTX_SetIORecv(ctx, MqttSocket_TlsSocketReceive);
+        wolfSSL_CTX_SetIOSend(ctx, MqttSocket_TlsSocketSend);
+    }
 
     /* Mutual TLS: load CA and require client certificate */
-    if (broker->tls_ca != NULL) {
+    if (rc == WOLFSSL_SUCCESS && broker->tls_ca != NULL) {
         rc = wolfSSL_CTX_load_verify_locations(ctx, broker->tls_ca, NULL);
         if (rc != WOLFSSL_SUCCESS) {
             PRINTF("broker: load CA failed %d (%s)", rc, broker->tls_ca);
-            wolfSSL_CTX_free(ctx);
-            wolfSSL_Cleanup();
-            return MQTT_CODE_ERROR_BAD_ARG;
+            rc = MQTT_CODE_ERROR_BAD_ARG;
+        }
+        else {
+            wolfSSL_CTX_set_verify(ctx,
+                WOLFSSL_VERIFY_PEER | WOLFSSL_VERIFY_FAIL_IF_NO_PEER_CERT,
+                NULL);
+            PRINTF("broker: mutual TLS enabled (CA=%s)", broker->tls_ca);
         }
-        wolfSSL_CTX_set_verify(ctx,
-            WOLFSSL_VERIFY_PEER | WOLFSSL_VERIFY_FAIL_IF_NO_PEER_CERT, NULL);
-        PRINTF("broker: mutual TLS enabled (CA=%s)", broker->tls_ca);
     }
 
-    broker->tls_ctx = ctx;
-    return MQTT_CODE_SUCCESS;
+    if (rc == WOLFSSL_SUCCESS) {
+        broker->tls_ctx = ctx;
+        rc = MQTT_CODE_SUCCESS;
+    }
+    else {
+        if (ctx != NULL) {
+            wolfSSL_CTX_free(ctx);
+        }
+        wolfSSL_Cleanup();
+    }
+    return rc;
 }
 
 static void BrokerTls_Free(MqttBroker* broker)
@@ -1179,6 +1181,7 @@ static int BrokerPendingWill_Add(MqttBroker* broker, BrokerClient* bc)
 {
     BrokerPendingWill* pw = NULL;
     WOLFMQTT_BROKER_TIME_T now = WOLFMQTT_BROKER_GET_TIME_S();
+    int rc = MQTT_CODE_SUCCESS;
 
 #ifdef WOLFMQTT_STATIC_MEMORY
     {
@@ -1190,69 +1193,96 @@ static int BrokerPendingWill_Add(MqttBroker* broker, BrokerClient* bc)
             }
         }
         if (pw == NULL) {
-            return MQTT_CODE_ERROR_MEMORY;
+            rc = MQTT_CODE_ERROR_MEMORY;
         }
-        XMEMSET(pw, 0, sizeof(*pw));
-        pw->in_use = 1;
-        XSTRNCPY(pw->client_id, bc->client_id, BROKER_MAX_CLIENT_ID_LEN - 1);
-        pw->client_id[BROKER_MAX_CLIENT_ID_LEN - 1] = '\0';
-        XSTRNCPY(pw->topic, bc->will_topic, BROKER_MAX_TOPIC_LEN - 1);
-        pw->topic[BROKER_MAX_TOPIC_LEN - 1] = '\0';
-        if (bc->will_payload_len > 0) {
-            word16 len = bc->will_payload_len;
-            if (len > BROKER_MAX_WILL_PAYLOAD_LEN) {
-                len = BROKER_MAX_WILL_PAYLOAD_LEN;
+        if (rc == MQTT_CODE_SUCCESS) {
+            XMEMSET(pw, 0, sizeof(*pw));
+            pw->in_use = 1;
+            {
+                int len = (int)XSTRLEN(bc->client_id);
+                if (len >= BROKER_MAX_CLIENT_ID_LEN) {
+                    len = BROKER_MAX_CLIENT_ID_LEN - 1;
+                }
+                XMEMCPY(pw->client_id, bc->client_id, len);
+                pw->client_id[len] = '\0';
+            }
+            {
+                int len = (int)XSTRLEN(bc->will_topic);
+                if (len >= BROKER_MAX_TOPIC_LEN) {
+                    len = BROKER_MAX_TOPIC_LEN - 1;
+                }
+                XMEMCPY(pw->topic, bc->will_topic, len);
+                pw->topic[len] = '\0';
+            }
+            if (bc->will_payload_len > 0) {
+                word16 len = bc->will_payload_len;
+                if (len > BROKER_MAX_WILL_PAYLOAD_LEN) {
+                    len = BROKER_MAX_WILL_PAYLOAD_LEN;
+                }
+                XMEMCPY(pw->payload, bc->will_payload, len);
+                pw->payload_len = len;
             }
-            XMEMCPY(pw->payload, bc->will_payload, len);
-            pw->payload_len = len;
         }
     }
 #else
     pw = (BrokerPendingWill*)WOLFMQTT_MALLOC(sizeof(BrokerPendingWill));
     if (pw == NULL) {
-        return MQTT_CODE_ERROR_MEMORY;
+        rc = MQTT_CODE_ERROR_MEMORY;
     }
-    XMEMSET(pw, 0, sizeof(*pw));
-    {
+    if (rc == MQTT_CODE_SUCCESS) {
         int id_len = (int)XSTRLEN(bc->client_id);
+        int t_len = (int)XSTRLEN(bc->will_topic);
+        XMEMSET(pw, 0, sizeof(*pw));
         pw->client_id = (char*)WOLFMQTT_MALLOC((size_t)id_len + 1);
-        if (pw->client_id == NULL) {
-            WOLFMQTT_FREE(pw);
-            return MQTT_CODE_ERROR_MEMORY;
+        if (pw->client_id != NULL) {
+            XMEMCPY(pw->client_id, bc->client_id, (size_t)id_len + 1);
         }
-        XMEMCPY(pw->client_id, bc->client_id, (size_t)id_len + 1);
-    }
-    {
-        int t_len = (int)XSTRLEN(bc->will_topic);
-        pw->topic = (char*)WOLFMQTT_MALLOC((size_t)t_len + 1);
-        if (pw->topic == NULL) {
-            WOLFMQTT_FREE(pw->client_id);
-            WOLFMQTT_FREE(pw);
-            return MQTT_CODE_ERROR_MEMORY;
+        else {
+            rc = MQTT_CODE_ERROR_MEMORY;
+        }
+        if (rc == MQTT_CODE_SUCCESS) {
+            pw->topic = (char*)WOLFMQTT_MALLOC((size_t)t_len + 1);
+            if (pw->topic != NULL) {
+                XMEMCPY(pw->topic, bc->will_topic, (size_t)t_len + 1);
+            }
+            else {
+                rc = MQTT_CODE_ERROR_MEMORY;
+            }
+        }
+        if (rc == MQTT_CODE_SUCCESS && bc->will_payload_len > 0) {
+            pw->payload = (byte*)WOLFMQTT_MALLOC(bc->will_payload_len);
+            if (pw->payload != NULL) {
+                XMEMCPY(pw->payload, bc->will_payload, bc->will_payload_len);
+                pw->payload_len = bc->will_payload_len;
+            }
+            else {
+                rc = MQTT_CODE_ERROR_MEMORY;
+            }
         }
-        XMEMCPY(pw->topic, bc->will_topic, (size_t)t_len + 1);
     }
-    if (bc->will_payload_len > 0) {
-        pw->payload = (byte*)WOLFMQTT_MALLOC(bc->will_payload_len);
-        if (pw->payload == NULL) {
+    if (rc == MQTT_CODE_SUCCESS) {
+        pw->next = broker->pending_wills;
+        broker->pending_wills = pw;
+    }
+    else if (pw != NULL) {
+        if (pw->topic) {
             WOLFMQTT_FREE(pw->topic);
+        }
+        if (pw->client_id) {
             WOLFMQTT_FREE(pw->client_id);
-            WOLFMQTT_FREE(pw);
-            return MQTT_CODE_ERROR_MEMORY;
         }
-        XMEMCPY(pw->payload, bc->will_payload, bc->will_payload_len);
-        pw->payload_len = bc->will_payload_len;
+        WOLFMQTT_FREE(pw);
     }
-    pw->next = broker->pending_wills;
-    broker->pending_wills = pw;
 #endif
-    pw->qos = bc->will_qos;
-    pw->retain = bc->will_retain;
-    pw->publish_time = now + (WOLFMQTT_BROKER_TIME_T)bc->will_delay_sec;
 
-    PRINTF("broker: will deferred sock=%d client_id=%s delay=%u",
-        (int)bc->sock, bc->client_id, (unsigned)bc->will_delay_sec);
-    return MQTT_CODE_SUCCESS;
+    if (rc == MQTT_CODE_SUCCESS) {
+        pw->qos = bc->will_qos;
+        pw->retain = bc->will_retain;
+        pw->publish_time = now + (WOLFMQTT_BROKER_TIME_T)bc->will_delay_sec;
+        PRINTF("broker: will deferred sock=%d client_id=%s delay=%u",
+            (int)bc->sock, bc->client_id, (unsigned)bc->will_delay_sec);
+    }
+    return rc;
 }
 
 /* Cancel a pending will for the given client_id (client reconnected) */

wolfmqtt/mqtt_broker.h

@@ -6,7 +6,7 @@
  *
  * wolfMQTT is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
+ * the Free Software Foundation; either version 3 of the License, or
  * (at your option) any later version.
  *
  * wolfMQTT is distributed in the hope that it will be useful,