Add fenrir suggested test cases

AGENTS.md

@@ -147,6 +147,33 @@ Uses `.clang-format` with LLVM base style:
 - Tab indentation (4-space tabs)
 - K&R inspired style
 
+## Test Integrity
+Never modify, delete, skip, or weaken tests to make them pass.
+Never fabricate, adjust, or derive expected values from the code under test just to force a pass; fixed expected values are acceptable when they come from an independent oracle, such as committed test vectors or other externally verified results.
+A passing test suite achieved by changing the tests (not the implementation) is not a passing result.
+Fix the code. If the code cannot be fixed within scope, escalate.
+
+Do not use the code under test as its only oracle where an independent oracle is required, especially for crypto, KDFs, canonical encodings, and other security-sensitive transformations. In those cases, tests should use known external test vectors, cross-validation against an independent implementation, or bit-exact comparison against a trusted reference path. For example, a test that only encrypts with function A and decrypts with function A is insufficient to validate the correctness of the cryptographic primitive.
+
+Roundtrip/property tests are still acceptable where they match the behavior being validated, such as encode/decode or serialize/parse flows already used elsewhere in this repository, but they should not be the sole oracle when stronger independent validation is needed.
+
+## No Fabrication
+Never report status, results, or completion that does not reflect work actually performed.
+If you are uncertain whether a step succeeded, say so explicitly. Do not paper over uncertainty with confident-sounding output.
+
+## Exit Code Discipline
+Every shell command's exit code must be checked.
+Never proceed after a silent failure.
+A command that failed and was ignored is not a completed step.
+
+### Test vector discipline
+Never derive test vectors from the code under test. Acceptable oracles:
+- OpenSSL (`openssl kdf`, `openssl enc`, `openssl pkcs8 ... | sha256sum`)
+- pyca/cryptography (`pkcs12.load_key_and_certificates`, `private_bytes(DER, PKCS8, NoEncryption)`)
+- Bouncy Castle test vectors
+Compute once, hardcode as `hex!(...)` literals or committed binary fixtures. Tests must be fully offline.
+For PKCS12KDF: use `hexpass:` (BMP/UTF-16BE + null terminator), NOT `pass:` (raw ASCII).
+
 ## Dependencies
 
 - **wolfSSL** - Required for TLS support

scripts/broker.test

@@ -297,6 +297,86 @@ else
 fi
 fi # has_will
 
+# --- Test 6b: Keep-alive timeout window precision ---
+echo ""
+echo "--- Test 6b: Keep-alive timeout window ---"
+if [ "$has_will" = "no" ]; then
+    echo "SKIP: Keep-alive timeout window (not built with will support)"
+else
+# Validate that the broker's 1.5x keep_alive multiplier (mqtt_broker.c:
+# keep_alive_sec * 3 / 2) is honored. Test 6 above only checks that the LWT
+# is eventually delivered, so silent mutations (e.g. *1, *2, *3) would still
+# pass. This test measures wall-clock time from client freeze to LWT
+# delivery and asserts a tight bound that catches significant multiplier
+# changes.
+#
+# k=4 is used to widen the discrimination window. The broker checks timing
+# at integer-second resolution, so the keepalive fire happens at
+# last_rx_int + threshold + 1 wall seconds, where threshold = floor(k*mult).
+#
+# Expected fire window (relative to client freeze):
+#   mult  threshold  fire window (s)  +will_delay (v5)
+#   1.0       4           4-5              9-10
+#   1.5       6           6-7             11-12   <- spec compliant
+#   2.0       8           8-9             13-14
+#   3.0      12          12-13            17-18
+#   4.0      16          16-17            21-22
+v5_active=no
+if ./$sub_bin -h 2>&1 | grep -q "Max packet size"; then
+    v5_active=yes
+fi
+if [ "$v5_active" = "yes" ]; then
+    LO_BOUND=10
+    HI_BOUND=13
+else
+    LO_BOUND=5
+    HI_BOUND=8
+fi
+rm -f "${TMP_DIR}/t6b_watcher.ready" "${TMP_DIR}/t6b_client.ready"
+timeout 30 ./$sub_bin -T -h 127.0.0.1 -p $port -n "wolfMQTT/example/lwttopic" \
+    -i "t6b_watcher" -R "${TMP_DIR}/t6b_watcher.ready" \
+    >"${TMP_DIR}/t6b_sub.log" 2>&1 &
+T6B_WATCHER_PID=$!
+TEST_PIDS+=($T6B_WATCHER_PID)
+if ! wait_for_file "${TMP_DIR}/t6b_watcher.ready" 5; then
+    echo "FAIL: Keep-alive timeout window (watcher did not become ready)"
+    FAIL=1
+fi
+./$sub_bin -T -h 127.0.0.1 -p $port -n "test/ka2_trigger" -i "ka2_client" -l -k 4 \
+    -R "${TMP_DIR}/t6b_client.ready" >"${TMP_DIR}/t6b_client.log" 2>&1 &
+T6B_CLIENT_PID=$!
+TEST_PIDS+=($T6B_CLIENT_PID)
+if ! wait_for_file "${TMP_DIR}/t6b_client.ready" 5; then
+    echo "FAIL: Keep-alive timeout window (client did not become ready)"
+    FAIL=1
+fi
+T6B_T_START=$(date +%s)
+kill -STOP $T6B_CLIENT_PID 2>/dev/null
+T6B_ELAPSED=-1
+T6B_DEADLINE=$((T6B_T_START + HI_BOUND + 5))
+while [ $(date +%s) -lt $T6B_DEADLINE ]; do
+    if grep -q "ka2_client" "${TMP_DIR}/t6b_sub.log" 2>/dev/null; then
+        T6B_ELAPSED=$(($(date +%s) - T6B_T_START))
+        break
+    fi
+    sleep 0.1
+done
+kill -9 $T6B_CLIENT_PID 2>/dev/null
+wait $T6B_CLIENT_PID 2>/dev/null || true
+kill $T6B_WATCHER_PID 2>/dev/null
+wait $T6B_WATCHER_PID 2>/dev/null || true
+TEST_PIDS=()
+if [ $T6B_ELAPSED -lt 0 ]; then
+    echo "FAIL: Keep-alive timeout window (no LWT within $((HI_BOUND + 5))s)"
+    FAIL=1
+elif [ $T6B_ELAPSED -lt $LO_BOUND ] || [ $T6B_ELAPSED -gt $HI_BOUND ]; then
+    echo "FAIL: Keep-alive timeout window (elapsed=${T6B_ELAPSED}s, expected ${LO_BOUND}-${HI_BOUND}s)"
+    FAIL=1
+else
+    echo "PASS: Keep-alive timeout window (elapsed=${T6B_ELAPSED}s, expected ${LO_BOUND}-${HI_BOUND}s)"
+fi
+fi # has_will
+
 fi # skip_plain (tests 1-6)
 
 # --- Test 7: Username/password auth ---

src/mqtt_packet.c

@@ -801,8 +801,18 @@ int MqttEncode_Connect(byte *tx_buf, int tx_buf_len, MqttConnect *mc_connect)
     }
 #endif
 
-    remain_len += (int)XSTRLEN(mc_connect->client_id) + MQTT_DATA_LEN_SIZE;
+    /* MQTT UTF-8 strings are limited to 65535 bytes [MQTT-1.5.3]. Check here
+     * (before writing the fixed header) so a later MqttEncode_String failure
+     * cannot corrupt tx_payload via `tx_payload += -1`. */
+    {
+        size_t str_len = XSTRLEN(mc_connect->client_id);
+        if (str_len > (size_t)0xFFFF) {
+            return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_BAD_ARG);
+        }
+        remain_len += (int)str_len + MQTT_DATA_LEN_SIZE;
+    }
     if (mc_connect->enable_lwt) {
+        size_t str_len;
         /* Verify all required fields are present */
         if (mc_connect->lwt_msg == NULL ||
             mc_connect->lwt_msg->topic_name == NULL ||
@@ -811,8 +821,12 @@ int MqttEncode_Connect(byte *tx_buf, int tx_buf_len, MqttConnect *mc_connect)
         {
             return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_BAD_ARG);
         }
+        str_len = XSTRLEN(mc_connect->lwt_msg->topic_name);
+        if (str_len > (size_t)0xFFFF) {
+            return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_BAD_ARG);
+        }
 
-        remain_len += (int)XSTRLEN(mc_connect->lwt_msg->topic_name);
+        remain_len += (int)str_len;
         remain_len += MQTT_DATA_LEN_SIZE;
         /* LWT payload uses word16 length prefix, validate it fits */
         if (mc_connect->lwt_msg->total_len > (word32)0xFFFF) {
@@ -841,10 +855,18 @@ int MqttEncode_Connect(byte *tx_buf, int tx_buf_len, MqttConnect *mc_connect)
 #endif
     }
     if (mc_connect->username) {
-        remain_len += (int)XSTRLEN(mc_connect->username) + MQTT_DATA_LEN_SIZE;
+        size_t str_len = XSTRLEN(mc_connect->username);
+        if (str_len > (size_t)0xFFFF) {
+            return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_BAD_ARG);
+        }
+        remain_len += (int)str_len + MQTT_DATA_LEN_SIZE;
     }
     if (mc_connect->password) {
-        remain_len += (int)XSTRLEN(mc_connect->password) + MQTT_DATA_LEN_SIZE;
+        size_t str_len = XSTRLEN(mc_connect->password);
+        if (str_len > (size_t)0xFFFF) {
+            return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_BAD_ARG);
+        }
+        remain_len += (int)str_len + MQTT_DATA_LEN_SIZE;
     }
 
     /* Encode fixed header */
@@ -1302,9 +1324,18 @@ int MqttEncode_Publish(byte *tx_buf, int tx_buf_len, MqttPublish *publish,
     if (tx_buf == NULL || publish == NULL) {
         return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_BAD_ARG);
     }
+    /* MQTT UTF-8 strings are limited to 65535 bytes [MQTT-1.5.3]. Check here
+     * before writing the fixed header so a later MqttEncode_String failure
+     * cannot corrupt tx_payload via `tx_payload += -1`. */
+    {
+        size_t str_len = XSTRLEN(publish->topic_name);
+        if (str_len > (size_t)0xFFFF) {
+            return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_BAD_ARG);
+        }
 
-    /* Determine packet length */
-    variable_len = (int)XSTRLEN(publish->topic_name) + MQTT_DATA_LEN_SIZE;
+        /* Determine packet length */
+        variable_len = (int)str_len + MQTT_DATA_LEN_SIZE;
+    }
     if (publish->qos > MQTT_QOS_0) {
         if (publish->packet_id == 0) {
             return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_PACKET_ID);
@@ -1721,7 +1752,12 @@ int MqttEncode_Subscribe(byte *tx_buf, int tx_buf_len,
     for (i = 0; i < subscribe->topic_count; i++) {
         topic = &subscribe->topics[i];
         if ((topic != NULL) && (topic->topic_filter != NULL)) {
-            remain_len += (int)XSTRLEN(topic->topic_filter) + MQTT_DATA_LEN_SIZE;
+            /* MQTT UTF-8 strings are limited to 65535 bytes [MQTT-1.5.3] */
+            size_t str_len = XSTRLEN(topic->topic_filter);
+            if (str_len > (size_t)0xFFFF) {
+                return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_BAD_ARG);
+            }
+            remain_len += (int)str_len + MQTT_DATA_LEN_SIZE;
             remain_len++; /* For QoS */
         }
         else {
@@ -2016,8 +2052,12 @@ int MqttEncode_Unsubscribe(byte *tx_buf, int tx_buf_len,
     for (i = 0; i < unsubscribe->topic_count; i++) {
         topic = &unsubscribe->topics[i];
         if ((topic != NULL) && (topic->topic_filter != NULL)) {
-            remain_len += (int)XSTRLEN(topic->topic_filter) +
-                                MQTT_DATA_LEN_SIZE;
+            /* MQTT UTF-8 strings are limited to 65535 bytes [MQTT-1.5.3] */
+            size_t str_len = XSTRLEN(topic->topic_filter);
+            if (str_len > (size_t)0xFFFF) {
+                return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_BAD_ARG);
+            }
+            remain_len += (int)str_len + MQTT_DATA_LEN_SIZE;
         }
         else {
             /* Topic count is invalid */
@@ -2675,7 +2715,8 @@ int MqttDecode_Auth(byte *rx_buf, int rx_buf_len, MqttAuth *auth)
         /* Decode variable header */
         auth->reason_code = *rx_payload++;
         if ((auth->reason_code == MQTT_REASON_SUCCESS) ||
-            (auth->reason_code == MQTT_REASON_CONT_AUTH))
+            (auth->reason_code == MQTT_REASON_CONT_AUTH) ||
+            (auth->reason_code == MQTT_REASON_REAUTH))
         {
             /* Decode Length of Properties */
             if (rx_buf_len < (rx_payload - rx_buf)) {

tests/fuzz/gen_corpus.py

@@ -180,6 +180,11 @@ def main():
                connect + gen_subscribe("test/#"))
     write_seed("seq_connect_subscribe_wildcard.bin",
                connect + gen_subscribe("+/data/#"))
+    # SUBSCRIBE options byte with QoS bits = 0b11 (reserved). Exercises the
+    # BrokerHandle_Subscribe cap that clamps surfaced QoS=3 down to QoS 2
+    # before BrokerSubs_Add / SUBACK.
+    write_seed("seq_connect_subscribe_qos3.bin",
+               connect + gen_subscribe("test/#", qos=3))
     write_seed("seq_connect_unsubscribe.bin",
                connect + gen_unsubscribe("test/#"))
     write_seed("seq_connect_pingreq.bin",

tests/test_mqtt_client.c

@@ -287,6 +287,103 @@ TEST(connect_with_mock_network)
     ASSERT_EQ(MQTT_CODE_ERROR_NETWORK, rc);
 }
 
+/* Regression test for tx_buf credential zeroing after CONNECT is sent.
+ * Guards CLIENT_FORCE_ZERO(client->tx_buf, xfer) in MqttClient_Connect: the
+ * original issue being prevented is plaintext credentials lingering in the
+ * client's tx_buf after the CONNECT packet is written. Without this test, a
+ * regression that deletes that line (or passes length 0) would pass silently. */
+#define TEST_CONNECT_USERNAME "user"
+#define TEST_CONNECT_PASSWORD "secret"
+
+static int connect_mock_xfer;
+static byte connect_mock_sent[TEST_TX_BUF_SIZE];
+
+static int mock_net_write_accept(void *context, const byte* buf, int buf_len,
+    int timeout_ms)
+{
+    (void)context; (void)timeout_ms;
+    if (buf != NULL && buf_len > 0 &&
+        buf_len <= (int)sizeof(connect_mock_sent)) {
+        XMEMCPY(connect_mock_sent, buf, (size_t)buf_len);
+        connect_mock_xfer = buf_len;
+    }
+    /* Pretend the full packet was sent so MqttClient_Connect reaches the
+     * CLIENT_FORCE_ZERO step. */
+    return buf_len;
+}
+
+static int buf_contains(const byte* buf, int buf_len,
+    const char* needle, int needle_len)
+{
+    int i;
+    if (buf == NULL || needle_len <= 0 || buf_len < needle_len) {
+        return 0;
+    }
+    for (i = 0; i + needle_len <= buf_len; i++) {
+        if (XMEMCMP(&buf[i], needle, (size_t)needle_len) == 0) {
+            return 1;
+        }
+    }
+    return 0;
+}
+
+TEST(connect_clears_tx_buf_credentials)
+{
+    int rc;
+    int i;
+    MqttConnect connect;
+    const int user_len = (int)sizeof(TEST_CONNECT_USERNAME) - 1;
+    const int pass_len = (int)sizeof(TEST_CONNECT_PASSWORD) - 1;
+
+    rc = test_init_client();
+    ASSERT_EQ(MQTT_CODE_SUCCESS, rc);
+
+    /* Swap in a write mock that accepts the packet and records what was
+     * sent. Read still returns MQTT_CODE_ERROR_NETWORK so MqttClient_Connect
+     * returns after the CLIENT_FORCE_ZERO step. */
+    connect_mock_xfer = 0;
+    XMEMSET(connect_mock_sent, 0, sizeof(connect_mock_sent));
+    test_net.write = mock_net_write_accept;
+
+    XMEMSET(&connect, 0, sizeof(connect));
+    connect.keep_alive_sec = 60;
+    connect.clean_session = 1;
+    connect.client_id = "test_client";
+    connect.username = TEST_CONNECT_USERNAME;
+    connect.password = TEST_CONNECT_PASSWORD;
+
+    rc = MqttClient_Connect(&test_client, &connect);
+    /* The read mock cannot deliver a CONNECT_ACK, so a successful return
+     * would be wrong regardless of the zeroing step. */
+    ASSERT_NE(MQTT_CODE_SUCCESS, rc);
+
+    /* Confirm the write path actually ran with credentials present. Without
+     * this, the zeroing assertion below could pass trivially. */
+    ASSERT_TRUE(connect_mock_xfer > 0);
+    ASSERT_TRUE(buf_contains(connect_mock_sent, connect_mock_xfer,
+                             TEST_CONNECT_USERNAME, user_len));
+    ASSERT_TRUE(buf_contains(connect_mock_sent, connect_mock_xfer,
+                             TEST_CONNECT_PASSWORD, pass_len));
+
+    /* Core regression check: credentials must not remain in tx_buf after
+     * MqttClient_Connect returns. Scans the full buffer because the zeroed
+     * region covers [0..xfer) and the remainder was zero-initialized at
+     * setup. */
+    ASSERT_FALSE(buf_contains(test_client.tx_buf, TEST_TX_BUF_SIZE,
+                              TEST_CONNECT_USERNAME, user_len));
+    ASSERT_FALSE(buf_contains(test_client.tx_buf, TEST_TX_BUF_SIZE,
+                              TEST_CONNECT_PASSWORD, pass_len));
+
+    /* Stronger boundary check: every byte the mock observed being written
+     * must now be zero. This catches both deletion of the CLIENT_FORCE_ZERO
+     * call and an `xfer` -> `0` mutation that turns the wipe into a no-op. */
+    for (i = 0; i < connect_mock_xfer; i++) {
+        if (test_client.tx_buf[i] != 0) {
+            FAIL("tx_buf byte within xfer range is non-zero after CONNECT");
+        }
+    }
+}
+
 /* ============================================================================
  * MqttClient_Disconnect Tests
  * ============================================================================ */
@@ -527,6 +624,7 @@ void run_mqtt_client_tests(void)
     RUN_TEST(connect_null_connect);
     RUN_TEST(connect_both_null);
     RUN_TEST(connect_with_mock_network);
+    RUN_TEST(connect_clears_tx_buf_credentials);
 
     /* MqttClient_Disconnect tests */
     RUN_TEST(disconnect_null_client);