diff --git a/AGENTS.md b/AGENTS.md index 629f7068c..a63d98027 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -147,6 +147,50 @@ 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. + +## MQTT Specification Discipline +Wire format and protocol behavior are governed by the published MQTT specifications. Treat the spec as the source of truth, not the code. + +Relevant specifications: +- MQTT v3.1.1 — OASIS Standard (`mqtt-v3.1.1-os`) +- MQTT v5.0 — OASIS Standard (`mqtt-v5.0`) +- MQTT-SN v1.2 — OASIS + +When implementing or testing a normative requirement, cite it in a comment so reviewers can verify against the spec: +- MQTT v3.1.1 / v5.0: bracketed conformance identifiers, e.g. `[MQTT-3.8.1-1]`, `[MQTT-2.3.1-1]`. +- When a rule has no bracketed identifier, reference the section number, e.g. `MQTT 5.0 section 3.15.2`. +- MQTT-SN v1.2: `MQTT-SN 1.2 section X.Y`. + +Match the version scope of the change. MQTT v5.0 adds packets and fields (AUTH, Reason Codes, Properties) that do not exist in v3.1.1; do not apply a v5-only rule to v3.1.1 code paths or vice versa. Guard v5-only logic with `WOLFMQTT_V5` and v3.1.1-only logic accordingly. + +## Test Oracle Discipline +Do not use the code under test as its own oracle for wire-format behavior. For encoder or decoder tests, use one of: +- A hand-constructed byte sequence that matches the spec wire format, built by reading the relevant section (not captured from encoder output). Comment the byte layout so the fixture is auditable. +- Values from the spec's worked examples or conformance annex. +- A cross-check against an independent implementation (e.g. mosquitto) captured once and committed as a fixed byte array. + +Roundtrip tests (encode then decode, or vice versa) are acceptable for regression and structural coverage, but they cannot be the sole oracle for a wire-format rule — a bug present in both encoder and decoder will still roundtrip. Pair roundtrip coverage with at least one independent fixture per rule. + +Tests must be fully offline and must not fetch vectors from the network at runtime. + ## Dependencies - **wolfSSL** - Required for TLS support diff --git a/scripts/broker.test b/scripts/broker.test index 2b5eb1c07..7053d4089 100755 --- a/scripts/broker.test +++ b/scripts/broker.test @@ -297,6 +297,93 @@ 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=14 +else + LO_BOUND=5 + HI_BOUND=9 +fi +rm -f "${TMP_DIR}/t6b_watcher.ready" "${TMP_DIR}/t6b_client.ready" +t6b_setup_ok=1 +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 + t6b_setup_ok=0 +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 + t6b_setup_ok=0 +fi +if [ $t6b_setup_ok -eq 1 ]; then + kill -STOP $T6B_CLIENT_PID 2>/dev/null + T6B_T_START=$(date +%s) + 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 +fi +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_setup_ok -eq 1 ]; then + 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 +fi # has_will + fi # skip_plain (tests 1-6) # --- Test 7: Username/password auth --- diff --git a/src/mqtt_broker.c b/src/mqtt_broker.c index 61851dd69..ba9bef14d 100644 --- a/src/mqtt_broker.c +++ b/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 diff --git a/src/mqtt_client.c b/src/mqtt_client.c index b404faebb..51387513e 100644 --- a/src/mqtt_client.c +++ b/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) { diff --git a/src/mqtt_packet.c b/src/mqtt_packet.c index f4ff9530e..9f5cf73fe 100644 --- a/src/mqtt_packet.c +++ b/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)) { @@ -2700,28 +2741,29 @@ 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); } } - else { - return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_OUT_OF_BUFFER); - } - } else { return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_MALFORMED_DATA); } diff --git a/tests/fuzz/gen_corpus.py b/tests/fuzz/gen_corpus.py index 4987b6a87..360a584bb 100644 --- a/tests/fuzz/gen_corpus.py +++ b/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", diff --git a/tests/test_mqtt_client.c b/tests/test_mqtt_client.c index 696f421a5..4d20714b9 100644 --- a/tests/test_mqtt_client.c +++ b/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); diff --git a/tests/test_mqtt_packet.c b/tests/test_mqtt_packet.c index 3b72dd1c7..c9f40ff19 100644 --- a/tests/test_mqtt_packet.c +++ b/tests/test_mqtt_packet.c @@ -161,6 +161,29 @@ TEST(encode_vbi_null_buf) ASSERT_EQ(4, MqttEncode_Vbi(NULL, 2097152)); } +/* [MQTT-2.2.3] Values above 268,435,455 must be rejected. Without the guard, + * the encoding loop's rc < 4 terminator would silently truncate the value + * to 4 bytes and produce a valid-looking but incorrect encoding. */ +TEST(encode_vbi_overflow_above_max) +{ + byte buf[4]; + int rc = MqttEncode_Vbi(buf, 268435456); + ASSERT_EQ(MQTT_CODE_ERROR_MALFORMED_DATA, rc); +} + +TEST(encode_vbi_overflow_u32_max) +{ + byte buf[4]; + int rc = MqttEncode_Vbi(buf, 0xFFFFFFFF); + ASSERT_EQ(MQTT_CODE_ERROR_MALFORMED_DATA, rc); +} + +TEST(encode_vbi_overflow_null_buf) +{ + int rc = MqttEncode_Vbi(NULL, 268435456); + ASSERT_EQ(MQTT_CODE_ERROR_MALFORMED_DATA, rc); +} + TEST(decode_vbi_one_byte_zero) { byte buf[1] = { 0x00 }; @@ -344,6 +367,240 @@ TEST(encode_publish_qos1_valid) ASSERT_TRUE(rc > 0); } +/* Verify the fixed-header flag bits (retain/QoS/dup) are actually emitted. + * Covers deletion mutations of the retain / qos / duplicate branches in + * MqttEncode_FixedHeader. */ +TEST(encode_publish_qos1_retain_flags_in_header) +{ + byte tx_buf[256]; + MqttPublish pub; + int rc; + + XMEMSET(&pub, 0, sizeof(pub)); + pub.topic_name = "test/topic"; + pub.qos = MQTT_QOS_1; + pub.retain = 1; + pub.duplicate = 0; + pub.packet_id = 1; + rc = MqttEncode_Publish(tx_buf, (int)sizeof(tx_buf), &pub, 0); + ASSERT_TRUE(rc > 0); + ASSERT_EQ(MQTT_PACKET_TYPE_PUBLISH, + MQTT_PACKET_TYPE_GET(tx_buf[0])); + ASSERT_EQ(MQTT_QOS_1, + (int)MQTT_PACKET_FLAGS_GET_QOS(tx_buf[0])); + ASSERT_TRUE((MQTT_PACKET_FLAGS_GET(tx_buf[0]) & + MQTT_PACKET_FLAG_RETAIN) != 0); + ASSERT_TRUE((MQTT_PACKET_FLAGS_GET(tx_buf[0]) & + MQTT_PACKET_FLAG_DUPLICATE) == 0); +} + +TEST(encode_publish_qos2_duplicate_flags_in_header) +{ + byte tx_buf[256]; + MqttPublish pub; + int rc; + + XMEMSET(&pub, 0, sizeof(pub)); + pub.topic_name = "test/topic"; + pub.qos = MQTT_QOS_2; + pub.retain = 0; + pub.duplicate = 1; + pub.packet_id = 1; + rc = MqttEncode_Publish(tx_buf, (int)sizeof(tx_buf), &pub, 0); + ASSERT_TRUE(rc > 0); + ASSERT_EQ(MQTT_PACKET_TYPE_PUBLISH, + MQTT_PACKET_TYPE_GET(tx_buf[0])); + ASSERT_EQ(MQTT_QOS_2, + (int)MQTT_PACKET_FLAGS_GET_QOS(tx_buf[0])); + ASSERT_TRUE((MQTT_PACKET_FLAGS_GET(tx_buf[0]) & + MQTT_PACKET_FLAG_DUPLICATE) != 0); + ASSERT_TRUE((MQTT_PACKET_FLAGS_GET(tx_buf[0]) & + MQTT_PACKET_FLAG_RETAIN) == 0); +} + +/* QoS 0, no retain, no dup -> flag nibble must be 0. Catches any mutation + * that unconditionally sets flag bits. */ +TEST(encode_publish_qos0_no_flags_in_header) +{ + byte tx_buf[256]; + MqttPublish pub; + int rc; + + XMEMSET(&pub, 0, sizeof(pub)); + pub.topic_name = "test/topic"; + pub.qos = MQTT_QOS_0; + pub.retain = 0; + pub.duplicate = 0; + rc = MqttEncode_Publish(tx_buf, (int)sizeof(tx_buf), &pub, 0); + ASSERT_TRUE(rc > 0); + ASSERT_EQ(MQTT_PACKET_TYPE_PUBLISH, + MQTT_PACKET_TYPE_GET(tx_buf[0])); + ASSERT_EQ(0, (int)MQTT_PACKET_FLAGS_GET(tx_buf[0])); +} + +/* f-2360: topic_name with strlen > 65535 must not produce a "successful" + * encode. MqttEncode_String returns -1 for oversize strings; the encoder + * must surface that as a negative return rather than adding -1 to the + * tx_payload pointer and reporting header_len+remain_len as success. */ +TEST(encode_publish_topic_oversized_rejected) +{ + const int str_len = 0x10000; /* one byte past MQTT UTF-8 limit */ + const int buf_len = str_len + 64; + byte *tx_buf = (byte*)WOLFMQTT_MALLOC(buf_len); + char *topic = (char*)WOLFMQTT_MALLOC(str_len + 1); + MqttPublish pub; + int rc; + + if (tx_buf == NULL || topic == NULL) { + WOLFMQTT_FREE(tx_buf); + WOLFMQTT_FREE(topic); + FAIL("allocation failed"); + } + XMEMSET(topic, 'A', str_len); + topic[str_len] = '\0'; + + XMEMSET(&pub, 0, sizeof(pub)); + pub.topic_name = topic; + pub.qos = MQTT_QOS_0; + rc = MqttEncode_Publish(tx_buf, buf_len, &pub, 0); + + WOLFMQTT_FREE(topic); + WOLFMQTT_FREE(tx_buf); + ASSERT_TRUE(rc < 0); +} + +/* ============================================================================ + * MqttDecode_Publish + * ============================================================================ */ + +TEST(decode_publish_qos0_valid) +{ + /* Fixed header (PUBLISH, QoS 0, remain_len=7), topic "a/b", + * payload "HI". Using nonzero payload bytes catches a + * qos>MQTT_QOS_0 -> qos>=MQTT_QOS_0 mutation that would read + * the first 2 payload bytes as a spurious packet_id. */ + byte buf[] = { 0x30, 7, + 0x00, 0x03, 'a', '/', 'b', + 'H', 'I' }; + MqttPublish pub; + int rc; + + XMEMSET(&pub, 0, sizeof(pub)); + rc = MqttDecode_Publish(buf, (int)sizeof(buf), &pub); + ASSERT_TRUE(rc > 0); + ASSERT_EQ(MQTT_QOS_0, pub.qos); + ASSERT_EQ(0, pub.packet_id); + ASSERT_EQ(3, pub.topic_name_len); + ASSERT_EQ(0, XMEMCMP(pub.topic_name, "a/b", 3)); + ASSERT_EQ(2, (int)pub.total_len); + ASSERT_EQ(2, (int)pub.buffer_len); + ASSERT_EQ('H', pub.buffer[0]); + ASSERT_EQ('I', pub.buffer[1]); +} + +TEST(decode_publish_qos1_valid) +{ + /* Fixed header (PUBLISH | QoS 1 = 0x32, remain_len=7), + * topic "t", packet_id=42, payload "xy". */ + byte buf[] = { 0x32, 7, + 0x00, 0x01, 't', + 0x00, 0x2A, + 'x', 'y' }; + MqttPublish pub; + int rc; + + XMEMSET(&pub, 0, sizeof(pub)); + rc = MqttDecode_Publish(buf, (int)sizeof(buf), &pub); + ASSERT_TRUE(rc > 0); + ASSERT_EQ(MQTT_QOS_1, pub.qos); + ASSERT_EQ(42, pub.packet_id); + ASSERT_EQ(1, pub.topic_name_len); + ASSERT_EQ(0, XMEMCMP(pub.topic_name, "t", 1)); + ASSERT_EQ(2, (int)pub.total_len); + ASSERT_EQ(2, (int)pub.buffer_len); + ASSERT_EQ('x', pub.buffer[0]); + ASSERT_EQ('y', pub.buffer[1]); +} + +/* Zero-payload PUBLISH is valid per spec; catches a + * variable_len>remain_len -> variable_len>=remain_len mutation. */ +TEST(decode_publish_qos0_zero_payload) +{ + byte buf[] = { 0x30, 3, + 0x00, 0x01, 'a' }; + MqttPublish pub; + int rc; + + XMEMSET(&pub, 0, sizeof(pub)); + rc = MqttDecode_Publish(buf, (int)sizeof(buf), &pub); + ASSERT_TRUE(rc > 0); + ASSERT_EQ(MQTT_QOS_0, pub.qos); + ASSERT_EQ(1, pub.topic_name_len); + ASSERT_EQ(0, (int)pub.total_len); + ASSERT_EQ(0, (int)pub.buffer_len); +} + +/* Fixed header claims remain_len=3, but topic declares length=5 + * (consuming 7 bytes of variable header). After decoding the topic, + * variable_len (7) exceeds remain_len (3), which must be rejected. */ +TEST(decode_publish_malformed_variable_exceeds_remain) +{ + byte buf[] = { 0x30, 3, + 0x00, 0x05, 'h', 'e', 'l', 'l', 'o' }; + MqttPublish pub; + int rc; + + XMEMSET(&pub, 0, sizeof(pub)); + rc = MqttDecode_Publish(buf, (int)sizeof(buf), &pub); + ASSERT_EQ(MQTT_CODE_ERROR_OUT_OF_BUFFER, rc); +} + +#ifdef WOLFMQTT_V5 +/* Hand-validated MQTT v5 PUBLISH packet (independent oracle, not produced by + * MqttEncode_Publish) so encode and decode cannot hide a shared bug: + * fixed header: 0x30 (PUBLISH, QoS 0), remain_len = 21 + * topic: "a/b" (2-byte len + 3 bytes) + * props length: 0x0D (13) + * property: 0x03 CONTENT_TYPE, 2-byte len 0x000A, "text/plain" + * payload: "HI" + */ +TEST(decode_publish_v5_content_type_property) +{ + byte buf[] = { + 0x30, 21, + 0x00, 0x03, 'a', '/', 'b', + 0x0D, + 0x03, 0x00, 0x0A, + 't', 'e', 'x', 't', '/', 'p', 'l', 'a', 'i', 'n', + 'H', 'I' + }; + MqttPublish pub; + MqttProp* prop; + int rc; + + XMEMSET(&pub, 0, sizeof(pub)); + pub.protocol_level = MQTT_CONNECT_PROTOCOL_LEVEL_5; + rc = MqttDecode_Publish(buf, (int)sizeof(buf), &pub); + ASSERT_TRUE(rc > 0); + ASSERT_EQ(MQTT_QOS_0, pub.qos); + ASSERT_EQ(0, pub.packet_id); + ASSERT_EQ(3, pub.topic_name_len); + ASSERT_EQ(0, XMEMCMP(pub.topic_name, "a/b", 3)); + ASSERT_EQ(2, (int)pub.total_len); + + for (prop = pub.props; prop != NULL; prop = prop->next) { + if (prop->type == MQTT_PROP_CONTENT_TYPE) + break; + } + ASSERT_TRUE(prop != NULL); + ASSERT_EQ(MQTT_PROP_CONTENT_TYPE, prop->type); + ASSERT_EQ(10, (int)prop->data_str.len); + ASSERT_EQ(0, XMEMCMP(prop->data_str.str, "text/plain", 10)); + + MqttProps_Free(pub.props); +} +#endif /* WOLFMQTT_V5 */ + /* ============================================================================ * MqttDecode_ConnectAck * ============================================================================ */ @@ -392,141 +649,954 @@ TEST(decode_connack_malformed_remain_len_one) } /* ============================================================================ - * MqttEncode_Subscribe + * MqttEncode_Subscribe + * ============================================================================ */ + +TEST(encode_subscribe_packet_id_zero) +{ + byte tx_buf[256]; + MqttSubscribe sub; + MqttTopic topic; + int rc; + + XMEMSET(&sub, 0, sizeof(sub)); + XMEMSET(&topic, 0, sizeof(topic)); + topic.topic_filter = "test/topic"; + sub.topics = &topic; + sub.topic_count = 1; + sub.packet_id = 0; + rc = MqttEncode_Subscribe(tx_buf, (int)sizeof(tx_buf), &sub); + ASSERT_EQ(MQTT_CODE_ERROR_PACKET_ID, rc); +} + +TEST(encode_subscribe_valid) +{ + byte tx_buf[256]; + MqttSubscribe sub; + MqttTopic topic; + int rc; + + XMEMSET(&sub, 0, sizeof(sub)); + XMEMSET(&topic, 0, sizeof(topic)); + topic.topic_filter = "test/topic"; + sub.topics = &topic; + sub.topic_count = 1; + sub.packet_id = 1; + rc = MqttEncode_Subscribe(tx_buf, (int)sizeof(tx_buf), &sub); + ASSERT_TRUE(rc > 0); +} + +/* [MQTT-3.8.1-1] SUBSCRIBE fixed header flags must be 0b0010 (reserved). + * Verifies the QoS branch in MqttEncode_FixedHeader fires even for non- + * PUBLISH packets. */ +TEST(encode_subscribe_fixed_header_flags) +{ + byte tx_buf[256]; + MqttSubscribe sub; + MqttTopic topic; + int rc; + + XMEMSET(&sub, 0, sizeof(sub)); + XMEMSET(&topic, 0, sizeof(topic)); + topic.topic_filter = "test/topic"; + sub.topics = &topic; + sub.topic_count = 1; + sub.packet_id = 1; + rc = MqttEncode_Subscribe(tx_buf, (int)sizeof(tx_buf), &sub); + ASSERT_TRUE(rc > 0); + ASSERT_EQ(MQTT_PACKET_TYPE_SUBSCRIBE, + MQTT_PACKET_TYPE_GET(tx_buf[0])); + ASSERT_EQ(0x2, (int)MQTT_PACKET_FLAGS_GET(tx_buf[0])); +} + +/* [MQTT-3.8.3-1] Payload options byte bits 0-1 carry QoS. Remaining bits + * carry v5-only No Local (bit 2), Retain As Published (bit 3), and Retain + * Handling (bits 4-5); bits 6-7 are reserved. The encoded options byte is + * the last byte of a single-topic SUBSCRIBE packet. */ +TEST(encode_subscribe_options_byte_qos0) +{ + byte tx_buf[256]; + MqttSubscribe sub; + MqttTopic topic; + int rc; + + XMEMSET(&sub, 0, sizeof(sub)); + XMEMSET(&topic, 0, sizeof(topic)); + topic.topic_filter = "a"; + topic.qos = MQTT_QOS_0; + sub.topics = &topic; + sub.topic_count = 1; + sub.packet_id = 1; + rc = MqttEncode_Subscribe(tx_buf, (int)sizeof(tx_buf), &sub); + ASSERT_TRUE(rc > 0); + ASSERT_EQ(0x00, tx_buf[rc - 1]); +} + +TEST(encode_subscribe_options_byte_qos1) +{ + byte tx_buf[256]; + MqttSubscribe sub; + MqttTopic topic; + int rc; + + XMEMSET(&sub, 0, sizeof(sub)); + XMEMSET(&topic, 0, sizeof(topic)); + topic.topic_filter = "a"; + topic.qos = MQTT_QOS_1; + sub.topics = &topic; + sub.topic_count = 1; + sub.packet_id = 1; + rc = MqttEncode_Subscribe(tx_buf, (int)sizeof(tx_buf), &sub); + ASSERT_TRUE(rc > 0); + ASSERT_EQ(0x01, tx_buf[rc - 1]); +} + +TEST(encode_subscribe_options_byte_qos2) +{ + byte tx_buf[256]; + MqttSubscribe sub; + MqttTopic topic; + int rc; + + XMEMSET(&sub, 0, sizeof(sub)); + XMEMSET(&topic, 0, sizeof(topic)); + topic.topic_filter = "a"; + topic.qos = MQTT_QOS_2; + sub.topics = &topic; + sub.topic_count = 1; + sub.packet_id = 1; + rc = MqttEncode_Subscribe(tx_buf, (int)sizeof(tx_buf), &sub); + ASSERT_TRUE(rc > 0); + ASSERT_EQ(0x02, tx_buf[rc - 1]); +} + +/* f-2360: topic_filter with strlen > 65535 must be rejected with a negative + * return. Guards the unchecked tx_payload += MqttEncode_String(...) in the + * SUBSCRIBE payload loop. */ +TEST(encode_subscribe_topic_filter_oversized_rejected) +{ + const int str_len = 0x10000; /* one byte past MQTT UTF-8 limit */ + const int buf_len = str_len + 64; + byte *tx_buf = (byte*)WOLFMQTT_MALLOC(buf_len); + char *filter = (char*)WOLFMQTT_MALLOC(str_len + 1); + MqttSubscribe sub; + MqttTopic topic; + int rc; + + if (tx_buf == NULL || filter == NULL) { + WOLFMQTT_FREE(tx_buf); + WOLFMQTT_FREE(filter); + FAIL("allocation failed"); + } + XMEMSET(filter, 'A', str_len); + filter[str_len] = '\0'; + + XMEMSET(&sub, 0, sizeof(sub)); + XMEMSET(&topic, 0, sizeof(topic)); + topic.topic_filter = filter; + sub.topics = &topic; + sub.topic_count = 1; + sub.packet_id = 1; + rc = MqttEncode_Subscribe(tx_buf, buf_len, &sub); + + WOLFMQTT_FREE(filter); + WOLFMQTT_FREE(tx_buf); + ASSERT_TRUE(rc < 0); +} + +/* when multiple topics are supplied and a later one is oversized, + * the encoder must still reject — the length-validation loop covers every + * entry, not just the first. */ +TEST(encode_subscribe_topic_filter_oversized_second_rejected) +{ + const int str_len = 0x10000; + const int buf_len = str_len + 128; + byte *tx_buf = (byte*)WOLFMQTT_MALLOC(buf_len); + char *filter = (char*)WOLFMQTT_MALLOC(str_len + 1); + MqttSubscribe sub; + MqttTopic topics[2]; + int rc; + + if (tx_buf == NULL || filter == NULL) { + WOLFMQTT_FREE(tx_buf); + WOLFMQTT_FREE(filter); + FAIL("allocation failed"); + } + XMEMSET(filter, 'B', str_len); + filter[str_len] = '\0'; + + XMEMSET(&sub, 0, sizeof(sub)); + XMEMSET(topics, 0, sizeof(topics)); + topics[0].topic_filter = "ok/topic"; + topics[1].topic_filter = filter; + sub.topics = topics; + sub.topic_count = 2; + sub.packet_id = 1; + rc = MqttEncode_Subscribe(tx_buf, buf_len, &sub); + + WOLFMQTT_FREE(filter); + WOLFMQTT_FREE(tx_buf); + ASSERT_TRUE(rc < 0); +} + +/* [MQTT-3.8.1-1] SUBSCRIBE fixed header reserved flags MUST equal 0b0010, + * which encodes QoS 1. Verifies the QoS 1 bit directly via + * MQTT_PACKET_FLAGS_GET_QOS so a mutation of MQTT_QOS_1 to MQTT_QOS_0 in + * MqttEncode_Subscribe's call to MqttEncode_FixedHeader is detected. */ +TEST(encode_subscribe_has_qos1_flag) +{ + byte tx_buf[256]; + MqttSubscribe sub; + MqttTopic topic; + int rc; + + XMEMSET(&sub, 0, sizeof(sub)); + XMEMSET(&topic, 0, sizeof(topic)); + topic.topic_filter = "test/topic"; + sub.topics = &topic; + sub.topic_count = 1; + sub.packet_id = 1; + rc = MqttEncode_Subscribe(tx_buf, (int)sizeof(tx_buf), &sub); + ASSERT_TRUE(rc > 0); + ASSERT_EQ(MQTT_QOS_1, MQTT_PACKET_FLAGS_GET_QOS(tx_buf[0])); +} + +/* ============================================================================ + * MqttEncode_Unsubscribe + * ============================================================================ */ + +TEST(encode_unsubscribe_packet_id_zero) +{ + byte tx_buf[256]; + MqttUnsubscribe unsub; + MqttTopic topic; + int rc; + + XMEMSET(&unsub, 0, sizeof(unsub)); + XMEMSET(&topic, 0, sizeof(topic)); + topic.topic_filter = "test/topic"; + unsub.topics = &topic; + unsub.topic_count = 1; + unsub.packet_id = 0; + rc = MqttEncode_Unsubscribe(tx_buf, (int)sizeof(tx_buf), &unsub); + ASSERT_EQ(MQTT_CODE_ERROR_PACKET_ID, rc); +} + +TEST(encode_unsubscribe_valid) +{ + byte tx_buf[256]; + MqttUnsubscribe unsub; + MqttTopic topic; + int rc; + + XMEMSET(&unsub, 0, sizeof(unsub)); + XMEMSET(&topic, 0, sizeof(topic)); + topic.topic_filter = "test/topic"; + unsub.topics = &topic; + unsub.topic_count = 1; + unsub.packet_id = 1; + rc = MqttEncode_Unsubscribe(tx_buf, (int)sizeof(tx_buf), &unsub); + ASSERT_TRUE(rc > 0); +} + +/* [MQTT-3.10.1-1] UNSUBSCRIBE fixed header flags must be 0b0010. */ +TEST(encode_unsubscribe_fixed_header_flags) +{ + byte tx_buf[256]; + MqttUnsubscribe unsub; + MqttTopic topic; + int rc; + + XMEMSET(&unsub, 0, sizeof(unsub)); + XMEMSET(&topic, 0, sizeof(topic)); + topic.topic_filter = "test/topic"; + unsub.topics = &topic; + unsub.topic_count = 1; + unsub.packet_id = 1; + rc = MqttEncode_Unsubscribe(tx_buf, (int)sizeof(tx_buf), &unsub); + ASSERT_TRUE(rc > 0); + ASSERT_EQ(MQTT_PACKET_TYPE_UNSUBSCRIBE, + MQTT_PACKET_TYPE_GET(tx_buf[0])); + ASSERT_EQ(0x2, (int)MQTT_PACKET_FLAGS_GET(tx_buf[0])); +} + +/* topic_filter with strlen > 65535 must be rejected with a negative + * return. Guards the unchecked tx_payload += MqttEncode_String(...) in the + * UNSUBSCRIBE payload loop. */ +TEST(encode_unsubscribe_topic_filter_oversized_rejected) +{ + const int str_len = 0x10000; /* one byte past MQTT UTF-8 limit */ + const int buf_len = str_len + 64; + byte *tx_buf = (byte*)WOLFMQTT_MALLOC(buf_len); + char *filter = (char*)WOLFMQTT_MALLOC(str_len + 1); + MqttUnsubscribe unsub; + MqttTopic topic; + int rc; + + if (tx_buf == NULL || filter == NULL) { + WOLFMQTT_FREE(tx_buf); + WOLFMQTT_FREE(filter); + FAIL("allocation failed"); + } + XMEMSET(filter, 'A', str_len); + filter[str_len] = '\0'; + + XMEMSET(&unsub, 0, sizeof(unsub)); + XMEMSET(&topic, 0, sizeof(topic)); + topic.topic_filter = filter; + unsub.topics = &topic; + unsub.topic_count = 1; + unsub.packet_id = 1; + rc = MqttEncode_Unsubscribe(tx_buf, buf_len, &unsub); + + WOLFMQTT_FREE(filter); + WOLFMQTT_FREE(tx_buf); + ASSERT_TRUE(rc < 0); +} + +/* when multiple topics are supplied and a later one is oversized, + * the encoder must still reject — the length-validation loop covers every + * entry, not just the first. */ +TEST(encode_unsubscribe_topic_filter_oversized_second_rejected) +{ + const int str_len = 0x10000; + const int buf_len = str_len + 128; + byte *tx_buf = (byte*)WOLFMQTT_MALLOC(buf_len); + char *filter = (char*)WOLFMQTT_MALLOC(str_len + 1); + MqttUnsubscribe unsub; + MqttTopic topics[2]; + int rc; + + if (tx_buf == NULL || filter == NULL) { + WOLFMQTT_FREE(tx_buf); + WOLFMQTT_FREE(filter); + FAIL("allocation failed"); + } + XMEMSET(filter, 'B', str_len); + filter[str_len] = '\0'; + + XMEMSET(&unsub, 0, sizeof(unsub)); + XMEMSET(topics, 0, sizeof(topics)); + topics[0].topic_filter = "ok/topic"; + topics[1].topic_filter = filter; + unsub.topics = topics; + unsub.topic_count = 2; + unsub.packet_id = 1; + rc = MqttEncode_Unsubscribe(tx_buf, buf_len, &unsub); + + WOLFMQTT_FREE(filter); + WOLFMQTT_FREE(tx_buf); + ASSERT_TRUE(rc < 0); +} + +/* [MQTT-3.10.1-1] UNSUBSCRIBE fixed header reserved flags MUST equal 0b0010, + * which encodes QoS 1. Verifies the QoS 1 bit directly via + * MQTT_PACKET_FLAGS_GET_QOS so a mutation of MQTT_QOS_1 to MQTT_QOS_0 in + * MqttEncode_Unsubscribe's call to MqttEncode_FixedHeader is detected. */ +TEST(encode_unsubscribe_has_qos1_flag) +{ + byte tx_buf[256]; + MqttUnsubscribe unsub; + MqttTopic topic; + int rc; + + XMEMSET(&unsub, 0, sizeof(unsub)); + XMEMSET(&topic, 0, sizeof(topic)); + topic.topic_filter = "test/topic"; + unsub.topics = &topic; + unsub.topic_count = 1; + unsub.packet_id = 1; + rc = MqttEncode_Unsubscribe(tx_buf, (int)sizeof(tx_buf), &unsub); + ASSERT_TRUE(rc > 0); + ASSERT_EQ(MQTT_QOS_1, MQTT_PACKET_FLAGS_GET_QOS(tx_buf[0])); +} + +/* ============================================================================ + * MqttEncode_Connect + * ============================================================================ */ + +/* [MQTT-3.1.2-22] Password must not be present without username */ +TEST(encode_connect_password_without_username) +{ + byte tx_buf[256]; + MqttConnect conn; + int rc; + + XMEMSET(&conn, 0, sizeof(conn)); + conn.client_id = "test_client"; + conn.username = NULL; + conn.password = "secret"; + rc = MqttEncode_Connect(tx_buf, (int)sizeof(tx_buf), &conn); + ASSERT_EQ(MQTT_CODE_ERROR_BAD_ARG, rc); +} + +TEST(encode_connect_username_and_password) +{ + byte tx_buf[256]; + MqttConnect conn; + int rc; + + XMEMSET(&conn, 0, sizeof(conn)); + conn.client_id = "test_client"; + conn.username = "user"; + conn.password = "secret"; + rc = MqttEncode_Connect(tx_buf, (int)sizeof(tx_buf), &conn); + ASSERT_TRUE(rc > 0); +} + +TEST(encode_connect_username_only) +{ + byte tx_buf[256]; + MqttConnect conn; + int rc; + + XMEMSET(&conn, 0, sizeof(conn)); + conn.client_id = "test_client"; + conn.username = "user"; + conn.password = NULL; + rc = MqttEncode_Connect(tx_buf, (int)sizeof(tx_buf), &conn); + ASSERT_TRUE(rc > 0); +} + +TEST(encode_connect_no_credentials) +{ + byte tx_buf[256]; + MqttConnect conn; + int rc; + + XMEMSET(&conn, 0, sizeof(conn)); + conn.client_id = "test_client"; + conn.username = NULL; + conn.password = NULL; + rc = MqttEncode_Connect(tx_buf, (int)sizeof(tx_buf), &conn); + ASSERT_TRUE(rc > 0); +} + +/* [MQTT-3.1.1] CONNECT fixed header flags must be all zero. */ +TEST(encode_connect_fixed_header_flags) +{ + byte tx_buf[256]; + MqttConnect conn; + int rc; + + XMEMSET(&conn, 0, sizeof(conn)); + conn.client_id = "test_client"; + rc = MqttEncode_Connect(tx_buf, (int)sizeof(tx_buf), &conn); + ASSERT_TRUE(rc > 0); + ASSERT_EQ(MQTT_PACKET_TYPE_CONNECT, + MQTT_PACKET_TYPE_GET(tx_buf[0])); + ASSERT_EQ(0, (int)MQTT_PACKET_FLAGS_GET(tx_buf[0])); +} + +/* CONNECT variable header layout: 2-byte protocol name length + 4-byte "MQTT" + * + 1-byte protocol level + 1-byte connect flags. Offset computed from the + * decoded VBI instead of assuming a 1-byte remaining-length encoding. */ +static int connect_flags_offset(const byte *tx_buf, int tx_buf_len) +{ + word32 remain_len = 0; + int vbi_len = MqttDecode_Vbi((byte*)&tx_buf[1], &remain_len, + (word32)(tx_buf_len - 1)); + if (vbi_len < 0) { + return vbi_len; + } + return 1 + vbi_len + 2 + 4 + 1; +} + +/* [MQTT-3.1.2] CONNECT variable header flags byte encodes credential and + * clean-session bits. */ +TEST(encode_connect_flags_username_password_clean) +{ + byte tx_buf[256]; + MqttConnect conn; + int rc; + int flags_off; + byte flags; + + XMEMSET(&conn, 0, sizeof(conn)); + conn.client_id = "test_client"; + conn.username = "user"; + conn.password = "pass"; + conn.clean_session = 1; + rc = MqttEncode_Connect(tx_buf, (int)sizeof(tx_buf), &conn); + ASSERT_TRUE(rc > 0); + flags_off = connect_flags_offset(tx_buf, rc); + ASSERT_TRUE(flags_off > 0 && flags_off < rc); + flags = tx_buf[flags_off]; + ASSERT_EQ(MQTT_CONNECT_FLAG_USERNAME, + flags & MQTT_CONNECT_FLAG_USERNAME); + ASSERT_EQ(MQTT_CONNECT_FLAG_PASSWORD, + flags & MQTT_CONNECT_FLAG_PASSWORD); + ASSERT_EQ(MQTT_CONNECT_FLAG_CLEAN_SESSION, + flags & MQTT_CONNECT_FLAG_CLEAN_SESSION); + ASSERT_EQ(0, flags & MQTT_CONNECT_FLAG_WILL_FLAG); + ASSERT_EQ(0, flags & MQTT_CONNECT_FLAG_WILL_RETAIN); + ASSERT_EQ(0, flags & MQTT_CONNECT_FLAG_WILL_QOS_MASK); + ASSERT_EQ(0, flags & MQTT_CONNECT_FLAG_RESERVED); +} + +TEST(encode_connect_flags_none) +{ + byte tx_buf[256]; + MqttConnect conn; + int rc; + int flags_off; + + XMEMSET(&conn, 0, sizeof(conn)); + conn.client_id = "test_client"; + rc = MqttEncode_Connect(tx_buf, (int)sizeof(tx_buf), &conn); + ASSERT_TRUE(rc > 0); + flags_off = connect_flags_offset(tx_buf, rc); + ASSERT_TRUE(flags_off > 0 && flags_off < rc); + ASSERT_EQ(0, (int)tx_buf[flags_off]); +} + +TEST(encode_connect_flags_clean_session_only) +{ + byte tx_buf[256]; + MqttConnect conn; + int rc; + int flags_off; + + XMEMSET(&conn, 0, sizeof(conn)); + conn.client_id = "test_client"; + conn.clean_session = 1; + rc = MqttEncode_Connect(tx_buf, (int)sizeof(tx_buf), &conn); + ASSERT_TRUE(rc > 0); + flags_off = connect_flags_offset(tx_buf, rc); + ASSERT_TRUE(flags_off > 0 && flags_off < rc); + ASSERT_EQ(MQTT_CONNECT_FLAG_CLEAN_SESSION, (int)tx_buf[flags_off]); +} + +TEST(encode_connect_flags_username_only) +{ + byte tx_buf[256]; + MqttConnect conn; + int rc; + int flags_off; + + XMEMSET(&conn, 0, sizeof(conn)); + conn.client_id = "test_client"; + conn.username = "user"; + rc = MqttEncode_Connect(tx_buf, (int)sizeof(tx_buf), &conn); + ASSERT_TRUE(rc > 0); + flags_off = connect_flags_offset(tx_buf, rc); + ASSERT_TRUE(flags_off > 0 && flags_off < rc); + ASSERT_EQ(MQTT_CONNECT_FLAG_USERNAME, + tx_buf[flags_off] & MQTT_CONNECT_FLAG_USERNAME); + ASSERT_EQ(0, tx_buf[flags_off] & MQTT_CONNECT_FLAG_PASSWORD); +} + +TEST(encode_connect_flags_lwt_qos1_retain) +{ + byte tx_buf[256]; + byte lwt_payload[] = {'b', 'y', 'e'}; + MqttConnect conn; + MqttMessage lwt; + int rc; + int flags_off; + byte flags; + + XMEMSET(&conn, 0, sizeof(conn)); + XMEMSET(&lwt, 0, sizeof(lwt)); + lwt.topic_name = "will/topic"; + lwt.buffer = lwt_payload; + lwt.total_len = (word32)sizeof(lwt_payload); + lwt.qos = MQTT_QOS_1; + lwt.retain = 1; + + conn.client_id = "test_client"; + conn.enable_lwt = 1; + conn.lwt_msg = &lwt; + rc = MqttEncode_Connect(tx_buf, (int)sizeof(tx_buf), &conn); + ASSERT_TRUE(rc > 0); + flags_off = connect_flags_offset(tx_buf, rc); + ASSERT_TRUE(flags_off > 0 && flags_off < rc); + flags = tx_buf[flags_off]; + ASSERT_EQ(MQTT_CONNECT_FLAG_WILL_FLAG, + flags & MQTT_CONNECT_FLAG_WILL_FLAG); + ASSERT_EQ(MQTT_CONNECT_FLAG_WILL_RETAIN, + flags & MQTT_CONNECT_FLAG_WILL_RETAIN); + ASSERT_EQ((int)MQTT_QOS_1, + (int)MQTT_CONNECT_FLAG_GET_QOS(flags)); + ASSERT_EQ(0, flags & MQTT_CONNECT_FLAG_USERNAME); + ASSERT_EQ(0, flags & MQTT_CONNECT_FLAG_PASSWORD); + ASSERT_EQ(0, flags & MQTT_CONNECT_FLAG_CLEAN_SESSION); +} + +/* f-2360: client_id with strlen > 65535 must be rejected with a negative + * return. MqttEncode_String returns -1 for such strings; the encoder must + * not report header_len+remain_len as a successful encode while tx_payload + * silently moves backward by one byte. */ +TEST(encode_connect_client_id_oversized_rejected) +{ + const int str_len = 0x10000; /* one byte past MQTT UTF-8 limit */ + const int buf_len = str_len + 64; + byte *tx_buf = (byte*)WOLFMQTT_MALLOC(buf_len); + char *client_id = (char*)WOLFMQTT_MALLOC(str_len + 1); + MqttConnect conn; + int rc; + + if (tx_buf == NULL || client_id == NULL) { + WOLFMQTT_FREE(tx_buf); + WOLFMQTT_FREE(client_id); + FAIL("allocation failed"); + } + XMEMSET(client_id, 'A', str_len); + client_id[str_len] = '\0'; + + XMEMSET(&conn, 0, sizeof(conn)); + conn.client_id = client_id; + rc = MqttEncode_Connect(tx_buf, buf_len, &conn); + + WOLFMQTT_FREE(client_id); + WOLFMQTT_FREE(tx_buf); + ASSERT_TRUE(rc < 0); +} + +/* f-2360: username with strlen > 65535. Password is supplied so the + * USERNAME+PASSWORD branch exercises both credential encodes. */ +TEST(encode_connect_username_oversized_rejected) +{ + const int str_len = 0x10000; + const int buf_len = str_len + 128; + byte *tx_buf = (byte*)WOLFMQTT_MALLOC(buf_len); + char *username = (char*)WOLFMQTT_MALLOC(str_len + 1); + MqttConnect conn; + int rc; + + if (tx_buf == NULL || username == NULL) { + WOLFMQTT_FREE(tx_buf); + WOLFMQTT_FREE(username); + FAIL("allocation failed"); + } + XMEMSET(username, 'U', str_len); + username[str_len] = '\0'; + + XMEMSET(&conn, 0, sizeof(conn)); + conn.client_id = "cid"; + conn.username = username; + conn.password = "pw"; + rc = MqttEncode_Connect(tx_buf, buf_len, &conn); + + WOLFMQTT_FREE(username); + WOLFMQTT_FREE(tx_buf); + ASSERT_TRUE(rc < 0); +} + +/* f-2360: password with strlen > 65535. */ +TEST(encode_connect_password_oversized_rejected) +{ + const int str_len = 0x10000; + const int buf_len = str_len + 128; + byte *tx_buf = (byte*)WOLFMQTT_MALLOC(buf_len); + char *password = (char*)WOLFMQTT_MALLOC(str_len + 1); + MqttConnect conn; + int rc; + + if (tx_buf == NULL || password == NULL) { + WOLFMQTT_FREE(tx_buf); + WOLFMQTT_FREE(password); + FAIL("allocation failed"); + } + XMEMSET(password, 'P', str_len); + password[str_len] = '\0'; + + XMEMSET(&conn, 0, sizeof(conn)); + conn.client_id = "cid"; + conn.username = "user"; + conn.password = password; + rc = MqttEncode_Connect(tx_buf, buf_len, &conn); + + WOLFMQTT_FREE(password); + WOLFMQTT_FREE(tx_buf); + ASSERT_TRUE(rc < 0); +} + +/* f-2360: LWT topic_name with strlen > 65535. */ +TEST(encode_connect_lwt_topic_oversized_rejected) +{ + const int str_len = 0x10000; + const int buf_len = str_len + 128; + byte *tx_buf = (byte*)WOLFMQTT_MALLOC(buf_len); + char *lwt_topic = (char*)WOLFMQTT_MALLOC(str_len + 1); + byte lwt_payload[] = { 'b', 'y', 'e' }; + MqttConnect conn; + MqttMessage lwt; + int rc; + + if (tx_buf == NULL || lwt_topic == NULL) { + WOLFMQTT_FREE(tx_buf); + WOLFMQTT_FREE(lwt_topic); + FAIL("allocation failed"); + } + XMEMSET(lwt_topic, 'T', str_len); + lwt_topic[str_len] = '\0'; + + XMEMSET(&conn, 0, sizeof(conn)); + XMEMSET(&lwt, 0, sizeof(lwt)); + lwt.topic_name = lwt_topic; + lwt.buffer = lwt_payload; + lwt.total_len = (word32)sizeof(lwt_payload); + conn.client_id = "cid"; + conn.enable_lwt = 1; + conn.lwt_msg = &lwt; + rc = MqttEncode_Connect(tx_buf, buf_len, &conn); + + WOLFMQTT_FREE(lwt_topic); + WOLFMQTT_FREE(tx_buf); + ASSERT_TRUE(rc < 0); +} + +/* ============================================================================ + * MqttDecode_Connect (broker-side) * ============================================================================ */ -TEST(encode_subscribe_packet_id_zero) +#ifdef WOLFMQTT_BROKER +/* (a) Valid v3.1.1 CONNECT with username + password: decoder must surface both + * credential strings and the session/keep-alive fields. */ +TEST(decode_connect_v311_username_password) { - byte tx_buf[256]; - MqttSubscribe sub; - MqttTopic topic; - int rc; + byte buf[256]; + MqttConnect enc, dec; + int enc_len, dec_len; - XMEMSET(&sub, 0, sizeof(sub)); - XMEMSET(&topic, 0, sizeof(topic)); - topic.topic_filter = "test/topic"; - sub.topics = &topic; - sub.topic_count = 1; - sub.packet_id = 0; - rc = MqttEncode_Subscribe(tx_buf, (int)sizeof(tx_buf), &sub); - ASSERT_EQ(MQTT_CODE_ERROR_PACKET_ID, rc); + XMEMSET(&enc, 0, sizeof(enc)); + enc.protocol_level = MQTT_CONNECT_PROTOCOL_LEVEL_4; + enc.client_id = "test_client"; + enc.username = "user"; + enc.password = "pass"; + enc.clean_session = 1; + enc.keep_alive_sec = 60; + + enc_len = MqttEncode_Connect(buf, (int)sizeof(buf), &enc); + ASSERT_TRUE(enc_len > 0); + + XMEMSET(&dec, 0, sizeof(dec)); + dec_len = MqttDecode_Connect(buf, enc_len, &dec); + ASSERT_EQ(enc_len, dec_len); + ASSERT_EQ(MQTT_CONNECT_PROTOCOL_LEVEL_4, dec.protocol_level); + ASSERT_EQ(1, dec.clean_session); + ASSERT_EQ(0, dec.enable_lwt); + ASSERT_EQ(60, dec.keep_alive_sec); + ASSERT_NOT_NULL(dec.client_id); + ASSERT_EQ(0, XMEMCMP(dec.client_id, "test_client", + XSTRLEN("test_client"))); + ASSERT_NOT_NULL(dec.username); + ASSERT_EQ(0, XMEMCMP(dec.username, "user", 4)); + ASSERT_NOT_NULL(dec.password); + ASSERT_EQ(0, XMEMCMP(dec.password, "pass", 4)); } -TEST(encode_subscribe_valid) +/* (b) CONNECT with USERNAME/PASSWORD flags clear: decoder must NULL the + * credential fields so the caller never observes uninitialized rx_buf + * bytes as credentials. Pre-poison the struct to make the clearing visible. */ +TEST(decode_connect_v311_no_credentials) { - byte tx_buf[256]; - MqttSubscribe sub; - MqttTopic topic; - int rc; + byte buf[256]; + MqttConnect enc, dec; + int enc_len, dec_len; - XMEMSET(&sub, 0, sizeof(sub)); - XMEMSET(&topic, 0, sizeof(topic)); - topic.topic_filter = "test/topic"; - sub.topics = &topic; - sub.topic_count = 1; - sub.packet_id = 1; - rc = MqttEncode_Subscribe(tx_buf, (int)sizeof(tx_buf), &sub); - ASSERT_TRUE(rc > 0); -} + XMEMSET(&enc, 0, sizeof(enc)); + enc.protocol_level = MQTT_CONNECT_PROTOCOL_LEVEL_4; + enc.client_id = "c1"; + enc.clean_session = 1; -/* ============================================================================ - * MqttEncode_Unsubscribe - * ============================================================================ */ + enc_len = MqttEncode_Connect(buf, (int)sizeof(buf), &enc); + ASSERT_TRUE(enc_len > 0); -TEST(encode_unsubscribe_packet_id_zero) + XMEMSET(&dec, 0, sizeof(dec)); + dec.username = (const char*)0x1; + dec.password = (const char*)0x1; + dec_len = MqttDecode_Connect(buf, enc_len, &dec); + ASSERT_EQ(enc_len, dec_len); + ASSERT_NULL(dec.username); + ASSERT_NULL(dec.password); +} + +/* (c) Wrong protocol name ("MQT_" with correct length=4) must be rejected + * with MQTT_CODE_ERROR_MALFORMED_DATA. Catches an '||' -> '&&' mutation of + * the protocol-name guard, where an acceptance would require both the + * length and name to simultaneously be wrong. */ +TEST(decode_connect_wrong_protocol_name) { - byte tx_buf[256]; - MqttUnsubscribe unsub; - MqttTopic topic; + byte buf[] = { + 0x10, 0x10, /* CONNECT, remain_len = 16 */ + 0x00, 0x04, /* protocol name length = 4 */ + 'M', 'Q', 'T', '_', /* WRONG protocol name */ + 0x04, /* protocol level = v3.1.1 */ + 0x02, /* flags: clean_session */ + 0x00, 0x3C, /* keep alive = 60 */ + 0x00, 0x04, 'c', 'i', 'd', '1' /* client_id "cid1" */ + }; + MqttConnect dec; int rc; - XMEMSET(&unsub, 0, sizeof(unsub)); - XMEMSET(&topic, 0, sizeof(topic)); - topic.topic_filter = "test/topic"; - unsub.topics = &topic; - unsub.topic_count = 1; - unsub.packet_id = 0; - rc = MqttEncode_Unsubscribe(tx_buf, (int)sizeof(tx_buf), &unsub); - ASSERT_EQ(MQTT_CODE_ERROR_PACKET_ID, rc); + XMEMSET(&dec, 0, sizeof(dec)); + rc = MqttDecode_Connect(buf, (int)sizeof(buf), &dec); + ASSERT_EQ(MQTT_CODE_ERROR_MALFORMED_DATA, rc); } -TEST(encode_unsubscribe_valid) +/* (d) Wrong protocol length (3 instead of 4) must be rejected with + * MQTT_CODE_ERROR_MALFORMED_DATA. Paired with (c) this covers both sides + * of the length/name disjunction. */ +TEST(decode_connect_wrong_protocol_length) { - byte tx_buf[256]; - MqttUnsubscribe unsub; - MqttTopic topic; + byte buf[] = { + 0x10, 0x10, + 0x00, 0x03, /* WRONG protocol name length = 3 */ + 'M', 'Q', 'T', 'T', + 0x04, + 0x02, + 0x00, 0x3C, + 0x00, 0x04, 'c', 'i', 'd', '1' + }; + MqttConnect dec; int rc; - XMEMSET(&unsub, 0, sizeof(unsub)); - XMEMSET(&topic, 0, sizeof(topic)); - topic.topic_filter = "test/topic"; - unsub.topics = &topic; - unsub.topic_count = 1; - unsub.packet_id = 1; - rc = MqttEncode_Unsubscribe(tx_buf, (int)sizeof(tx_buf), &unsub); - ASSERT_TRUE(rc > 0); + XMEMSET(&dec, 0, sizeof(dec)); + rc = MqttDecode_Connect(buf, (int)sizeof(buf), &dec); + ASSERT_EQ(MQTT_CODE_ERROR_MALFORMED_DATA, rc); } -/* ============================================================================ - * MqttEncode_Connect - * ============================================================================ */ - -/* [MQTT-3.1.2-22] Password must not be present without username */ -TEST(encode_connect_password_without_username) +/* (e) CONNECT with WILL_FLAG set: decoder must set enable_lwt=1 and populate + * lwt_msg topic/payload/qos/retain from the flags and payload. Catches a + * mutation that flips the enable_lwt boolean, which would drop LWT decoding + * entirely. */ +TEST(decode_connect_v311_with_lwt) { - byte tx_buf[256]; - MqttConnect conn; - int rc; + byte buf[256]; + byte lwt_payload[] = { 'b', 'y', 'e' }; + MqttConnect enc, dec; + MqttMessage enc_lwt, dec_lwt; + int enc_len, dec_len; - XMEMSET(&conn, 0, sizeof(conn)); - conn.client_id = "test_client"; - conn.username = NULL; - conn.password = "secret"; - rc = MqttEncode_Connect(tx_buf, (int)sizeof(tx_buf), &conn); - ASSERT_EQ(MQTT_CODE_ERROR_BAD_ARG, rc); + XMEMSET(&enc, 0, sizeof(enc)); + XMEMSET(&enc_lwt, 0, sizeof(enc_lwt)); + enc.protocol_level = MQTT_CONNECT_PROTOCOL_LEVEL_4; + enc.client_id = "c1"; + enc.enable_lwt = 1; + enc.lwt_msg = &enc_lwt; + enc_lwt.topic_name = "will/topic"; + enc_lwt.buffer = lwt_payload; + enc_lwt.total_len = (word32)sizeof(lwt_payload); + enc_lwt.qos = MQTT_QOS_1; + enc_lwt.retain = 1; + + enc_len = MqttEncode_Connect(buf, (int)sizeof(buf), &enc); + ASSERT_TRUE(enc_len > 0); + + XMEMSET(&dec, 0, sizeof(dec)); + XMEMSET(&dec_lwt, 0, sizeof(dec_lwt)); + dec.lwt_msg = &dec_lwt; + dec_len = MqttDecode_Connect(buf, enc_len, &dec); + ASSERT_EQ(enc_len, dec_len); + ASSERT_EQ(1, dec.enable_lwt); + ASSERT_EQ((int)MQTT_QOS_1, (int)dec_lwt.qos); + ASSERT_EQ(1, dec_lwt.retain); + ASSERT_EQ((int)XSTRLEN("will/topic"), (int)dec_lwt.topic_name_len); + ASSERT_EQ(0, XMEMCMP(dec_lwt.topic_name, "will/topic", + XSTRLEN("will/topic"))); + ASSERT_EQ((word32)sizeof(lwt_payload), dec_lwt.total_len); + ASSERT_EQ((word32)sizeof(lwt_payload), dec_lwt.buffer_len); + ASSERT_NOT_NULL(dec_lwt.buffer); + ASSERT_EQ(0, XMEMCMP(dec_lwt.buffer, lwt_payload, sizeof(lwt_payload))); } +#endif /* WOLFMQTT_BROKER */ -TEST(encode_connect_username_and_password) +/* ============================================================================ + * MqttDecode_Subscribe (broker-side) + * ============================================================================ */ + +#ifdef WOLFMQTT_BROKER +/* Hand-built v3.1.1 SUBSCRIBE wire buffer serves as an independent oracle. + * Wire: type|flags=0x82, remaining=6, packet_id=0x0001, topic_len=0x0001, + * "a", options=0x01 (QoS 1). */ +TEST(decode_subscribe_v311_single_topic) { - byte tx_buf[256]; - MqttConnect conn; + byte rx_buf[] = { + 0x82, 0x06, + 0x00, 0x01, + 0x00, 0x01, + 0x61, + 0x01 + }; + MqttSubscribe sub; + MqttTopic topic_arr[1]; int rc; - XMEMSET(&conn, 0, sizeof(conn)); - conn.client_id = "test_client"; - conn.username = "user"; - conn.password = "secret"; - rc = MqttEncode_Connect(tx_buf, (int)sizeof(tx_buf), &conn); + XMEMSET(&sub, 0, sizeof(sub)); + XMEMSET(topic_arr, 0, sizeof(topic_arr)); + sub.topics = topic_arr; + rc = MqttDecode_Subscribe(rx_buf, (int)sizeof(rx_buf), &sub); ASSERT_TRUE(rc > 0); + ASSERT_EQ(1, sub.packet_id); + ASSERT_EQ(1, sub.topic_count); + ASSERT_EQ(MQTT_QOS_1, topic_arr[0].qos); + ASSERT_NOT_NULL(topic_arr[0].topic_filter); + ASSERT_EQ(0, XMEMCMP(topic_arr[0].topic_filter, "a", 1)); } -TEST(encode_connect_username_only) +/* Options byte QoS bits (0-1) = 0b11 is reserved. The decoder forwards the + * raw value verbatim (options & 0x03 == 3), so the broker's + * BrokerHandle_Subscribe cap at MQTT_QOS_2 is the only thing preventing + * QoS=3 from propagating into BrokerSubs_Add / the SUBACK return code. + * This test pins the precondition: if the decoder ever starts rejecting + * QoS=3, the broker cap becomes dead code and this test will flag it. */ +TEST(decode_subscribe_v311_qos3_reserved) { - byte tx_buf[256]; - MqttConnect conn; + byte rx_buf[] = { + 0x82, 0x06, + 0x00, 0x01, + 0x00, 0x01, + 0x61, + 0x03 + }; + MqttSubscribe sub; + MqttTopic topic_arr[1]; int rc; - XMEMSET(&conn, 0, sizeof(conn)); - conn.client_id = "test_client"; - conn.username = "user"; - conn.password = NULL; - rc = MqttEncode_Connect(tx_buf, (int)sizeof(tx_buf), &conn); + XMEMSET(&sub, 0, sizeof(sub)); + XMEMSET(topic_arr, 0, sizeof(topic_arr)); + sub.topics = topic_arr; + rc = MqttDecode_Subscribe(rx_buf, (int)sizeof(rx_buf), &sub); ASSERT_TRUE(rc > 0); + ASSERT_EQ(1, sub.topic_count); + ASSERT_EQ(MQTT_QOS_3, topic_arr[0].qos); } -TEST(encode_connect_no_credentials) +#ifdef WOLFMQTT_V5 +/* [MQTT-3.8.3] v5 SUBSCRIBE options byte carries QoS (bits 0-1), No Local + * (bit 2), Retain As Published (bit 3), and Retain Handling (bits 4-5). + * Hand-built wire with options = 0x2D (RH=2, RAP=1, NL=1, QoS=1). The + * decoder must accept the packet and surface QoS. */ +TEST(decode_subscribe_v5_options_byte_qos_extracted) { - byte tx_buf[256]; - MqttConnect conn; + /* Wire: type|flags=0x82, remaining=7, packet_id=0x0001, props_len=0x00, + * topic_len=0x0001, "a", options=0x2D. */ + byte rx_buf[] = { + 0x82, 0x07, + 0x00, 0x01, + 0x00, + 0x00, 0x01, + 0x61, + 0x2D + }; + MqttSubscribe sub; + MqttTopic topic_arr[1]; int rc; - XMEMSET(&conn, 0, sizeof(conn)); - conn.client_id = "test_client"; - conn.username = NULL; - conn.password = NULL; - rc = MqttEncode_Connect(tx_buf, (int)sizeof(tx_buf), &conn); + XMEMSET(&sub, 0, sizeof(sub)); + XMEMSET(topic_arr, 0, sizeof(topic_arr)); + sub.topics = topic_arr; + sub.protocol_level = MQTT_CONNECT_PROTOCOL_LEVEL_5; + rc = MqttDecode_Subscribe(rx_buf, (int)sizeof(rx_buf), &sub); ASSERT_TRUE(rc > 0); + ASSERT_EQ(1, sub.packet_id); + ASSERT_EQ(1, sub.topic_count); + ASSERT_EQ(MQTT_QOS_1, topic_arr[0].qos); } +#endif /* WOLFMQTT_V5 */ +#endif /* WOLFMQTT_BROKER */ /* ============================================================================ * QoS 2 next-ack arithmetic (PUBLISH_REC -> REL -> COMP) @@ -557,11 +1627,34 @@ TEST(decode_suback_valid) buf[1] = 3; buf[2] = 0; buf[3] = 1; - buf[4] = 0; + buf[4] = MQTT_QOS_2; XMEMSET(&ack, 0, sizeof(ack)); rc = MqttDecode_SubscribeAck(buf, 5, &ack); ASSERT_TRUE(rc > 0); ASSERT_EQ(1, ack.packet_id); + ASSERT_EQ(MQTT_QOS_2, ack.return_codes[0]); +} + +TEST(decode_suback_multiple_return_codes) +{ + byte buf[7]; + MqttSubscribeAck ack; + int rc; + + buf[0] = MQTT_PACKET_TYPE_SET(MQTT_PACKET_TYPE_SUBSCRIBE_ACK); + buf[1] = 5; + buf[2] = 0; + buf[3] = 1; + buf[4] = MQTT_QOS_1; + buf[5] = MQTT_QOS_2; + buf[6] = MQTT_SUBSCRIBE_ACK_CODE_FAILURE; + XMEMSET(&ack, 0, sizeof(ack)); + rc = MqttDecode_SubscribeAck(buf, 7, &ack); + ASSERT_TRUE(rc > 0); + ASSERT_EQ(1, ack.packet_id); + ASSERT_EQ(MQTT_QOS_1, ack.return_codes[0]); + ASSERT_EQ(MQTT_QOS_2, ack.return_codes[1]); + ASSERT_EQ(MQTT_SUBSCRIBE_ACK_CODE_FAILURE, ack.return_codes[2]); } TEST(decode_suback_malformed_remain_len_zero) @@ -638,6 +1731,86 @@ TEST(decode_publish_resp_malformed_remain_len_one) ASSERT_EQ(MQTT_CODE_ERROR_MALFORMED_DATA, rc); } +/* ============================================================================ + * MqttEncode_PublishResp fixed-header QoS bits + * + * MQTT-3.6.1-1: PUBREL fixed header reserved flags MUST be 0010 (QoS 1 bit). + * All other publish response types (PUBACK/PUBREC/PUBCOMP) use QoS 0 flags. + * ============================================================================ */ + +TEST(encode_publish_rel_has_qos1_flag) +{ + byte buf[8]; + MqttPublishResp resp; + int enc_len; + + XMEMSET(&resp, 0, sizeof(resp)); + resp.packet_id = 1; + + enc_len = MqttEncode_PublishResp(buf, (int)sizeof(buf), + MQTT_PACKET_TYPE_PUBLISH_REL, &resp); + ASSERT_TRUE(enc_len > 0); + + /* Fixed header: packet type PUBREL (6) in upper nibble, QoS 1 in flags */ + ASSERT_EQ(MQTT_PACKET_TYPE_PUBLISH_REL, + MQTT_PACKET_TYPE_GET(buf[0])); + ASSERT_EQ(0x02, buf[0] & MQTT_PACKET_FLAG_QOS_MASK); +} + +TEST(encode_publish_ack_has_qos0_flag) +{ + byte buf[8]; + MqttPublishResp resp; + int enc_len; + + XMEMSET(&resp, 0, sizeof(resp)); + resp.packet_id = 1; + + enc_len = MqttEncode_PublishResp(buf, (int)sizeof(buf), + MQTT_PACKET_TYPE_PUBLISH_ACK, &resp); + ASSERT_TRUE(enc_len > 0); + + ASSERT_EQ(MQTT_PACKET_TYPE_PUBLISH_ACK, + MQTT_PACKET_TYPE_GET(buf[0])); + ASSERT_EQ(0x00, buf[0] & MQTT_PACKET_FLAG_QOS_MASK); +} + +TEST(encode_publish_rec_has_qos0_flag) +{ + byte buf[8]; + MqttPublishResp resp; + int enc_len; + + XMEMSET(&resp, 0, sizeof(resp)); + resp.packet_id = 1; + + enc_len = MqttEncode_PublishResp(buf, (int)sizeof(buf), + MQTT_PACKET_TYPE_PUBLISH_REC, &resp); + ASSERT_TRUE(enc_len > 0); + + ASSERT_EQ(MQTT_PACKET_TYPE_PUBLISH_REC, + MQTT_PACKET_TYPE_GET(buf[0])); + ASSERT_EQ(0x00, buf[0] & MQTT_PACKET_FLAG_QOS_MASK); +} + +TEST(encode_publish_comp_has_qos0_flag) +{ + byte buf[8]; + MqttPublishResp resp; + int enc_len; + + XMEMSET(&resp, 0, sizeof(resp)); + resp.packet_id = 1; + + enc_len = MqttEncode_PublishResp(buf, (int)sizeof(buf), + MQTT_PACKET_TYPE_PUBLISH_COMP, &resp); + ASSERT_TRUE(enc_len > 0); + + ASSERT_EQ(MQTT_PACKET_TYPE_PUBLISH_COMP, + MQTT_PACKET_TYPE_GET(buf[0])); + ASSERT_EQ(0x00, buf[0] & MQTT_PACKET_FLAG_QOS_MASK); +} + /* ============================================================================ * MqttDecode_UnsubscribeAck * ============================================================================ */ @@ -772,6 +1945,125 @@ TEST(publish_resp_v5_success_no_props_roundtrip) ASSERT_TRUE(dec_len > 0); ASSERT_EQ(MQTT_REASON_SUCCESS, dec.reason_code); } + +/* ============================================================================ + * MqttEncode/Decode_Auth roundtrip + * + * Every reason code accepted by MqttEncode_Auth must round-trip through + * MqttDecode_Auth. MQTT 5.0 section 3.15.1 lists SUCCESS (0x00), CONT_AUTH + * (0x18), and REAUTH (0x19) as valid AUTH reason codes. + * ============================================================================ */ + +static void auth_roundtrip_with_method(byte reason_code) +{ + byte buf[256]; + MqttAuth enc, dec; + MqttProp prop; + int enc_len, dec_len; + char auth_method[] = "SCRAM-SHA-256"; + + XMEMSET(&enc, 0, sizeof(enc)); + XMEMSET(&prop, 0, sizeof(prop)); + prop.type = MQTT_PROP_AUTH_METHOD; + prop.data_str.str = auth_method; + prop.data_str.len = (word16)XSTRLEN(auth_method); + prop.next = NULL; + enc.reason_code = reason_code; + enc.props = ∝ + + enc_len = MqttEncode_Auth(buf, (int)sizeof(buf), &enc); + ASSERT_TRUE(enc_len > 0); + + XMEMSET(&dec, 0, sizeof(dec)); + dec_len = MqttDecode_Auth(buf, enc_len, &dec); + ASSERT_TRUE(dec_len > 0); + ASSERT_EQ(enc_len, dec_len); + ASSERT_EQ(reason_code, dec.reason_code); + ASSERT_NOT_NULL(dec.props); + ASSERT_EQ(MQTT_PROP_AUTH_METHOD, dec.props->type); + if (dec.props) { + MqttProps_Free(dec.props); + } +} + +TEST(auth_v5_cont_auth_roundtrip) +{ + auth_roundtrip_with_method(MQTT_REASON_CONT_AUTH); +} + +TEST(auth_v5_reauth_roundtrip) +{ + auth_roundtrip_with_method(MQTT_REASON_REAUTH); +} + +TEST(auth_v5_reauth_decodes_without_error) +{ + /* Decode a hand-built REAUTH packet to guard against encoder changes + * masking a decoder-only regression. */ + byte buf[64]; + MqttAuth enc, dec; + MqttProp prop; + int enc_len, dec_len; + char auth_method[] = "OAUTH"; + + XMEMSET(&enc, 0, sizeof(enc)); + XMEMSET(&prop, 0, sizeof(prop)); + prop.type = MQTT_PROP_AUTH_METHOD; + prop.data_str.str = auth_method; + prop.data_str.len = (word16)XSTRLEN(auth_method); + prop.next = NULL; + enc.reason_code = MQTT_REASON_REAUTH; + enc.props = ∝ + + enc_len = MqttEncode_Auth(buf, (int)sizeof(buf), &enc); + ASSERT_TRUE(enc_len > 0); + ASSERT_EQ(MQTT_REASON_REAUTH, buf[2]); + + XMEMSET(&dec, 0, sizeof(dec)); + dec_len = MqttDecode_Auth(buf, enc_len, &dec); + ASSERT_NE(MQTT_CODE_ERROR_MALFORMED_DATA, dec_len); + ASSERT_TRUE(dec_len > 0); + if (dec.props) { + MqttProps_Free(dec.props); + } +} + +TEST(auth_v5_success_remaining_length_zero) +{ + /* Per MQTT 5.0 3.15.2, a Remaining Length of 0 means SUCCESS with no + * properties. Build that wire form directly since MqttEncode_Auth does + * not emit it. */ + byte buf[2]; + MqttAuth dec; + int dec_len; + + buf[0] = (byte)(MQTT_PACKET_TYPE_AUTH << 4); + buf[1] = 0x00; /* Remaining Length */ + + XMEMSET(&dec, 0, sizeof(dec)); + dec_len = MqttDecode_Auth(buf, (int)sizeof(buf), &dec); + ASSERT_EQ(2, dec_len); + ASSERT_EQ(MQTT_REASON_SUCCESS, dec.reason_code); + ASSERT_NULL(dec.props); +} + +TEST(auth_v5_invalid_reason_code_rejected) +{ + /* A reason code outside {SUCCESS, CONT_AUTH, REAUTH} must not decode + * as a valid AUTH packet. Use 0x7F which is not assigned for AUTH. */ + byte buf[8]; + MqttAuth dec; + int dec_len; + + buf[0] = (byte)(MQTT_PACKET_TYPE_AUTH << 4); + buf[1] = 0x02; /* Remaining Length */ + buf[2] = 0x7F; /* Invalid reason code */ + buf[3] = 0x00; /* Property Length = 0 */ + + XMEMSET(&dec, 0, sizeof(dec)); + dec_len = MqttDecode_Auth(buf, 4, &dec); + ASSERT_EQ(MQTT_CODE_ERROR_MALFORMED_DATA, dec_len); +} #endif /* WOLFMQTT_V5 */ /* ============================================================================ @@ -803,6 +2095,9 @@ void run_mqtt_packet_tests(void) RUN_TEST(encode_vbi_three_bytes); RUN_TEST(encode_vbi_four_bytes); RUN_TEST(encode_vbi_null_buf); + RUN_TEST(encode_vbi_overflow_above_max); + RUN_TEST(encode_vbi_overflow_u32_max); + RUN_TEST(encode_vbi_overflow_null_buf); RUN_TEST(decode_vbi_one_byte_zero); RUN_TEST(decode_vbi_one_byte_max); RUN_TEST(decode_vbi_two_bytes_min); @@ -822,6 +2117,19 @@ void run_mqtt_packet_tests(void) RUN_TEST(encode_publish_qos2_packet_id_zero); RUN_TEST(encode_publish_qos0_packet_id_zero_ok); RUN_TEST(encode_publish_qos1_valid); + RUN_TEST(encode_publish_qos1_retain_flags_in_header); + RUN_TEST(encode_publish_qos2_duplicate_flags_in_header); + RUN_TEST(encode_publish_qos0_no_flags_in_header); + RUN_TEST(encode_publish_topic_oversized_rejected); + + /* MqttDecode_Publish */ + RUN_TEST(decode_publish_qos0_valid); + RUN_TEST(decode_publish_qos1_valid); + RUN_TEST(decode_publish_qos0_zero_payload); + RUN_TEST(decode_publish_malformed_variable_exceeds_remain); +#ifdef WOLFMQTT_V5 + RUN_TEST(decode_publish_v5_content_type_property); +#endif /* MqttDecode_ConnectAck */ RUN_TEST(decode_connack_valid); @@ -831,22 +2139,60 @@ void run_mqtt_packet_tests(void) /* MqttEncode_Subscribe */ RUN_TEST(encode_subscribe_packet_id_zero); RUN_TEST(encode_subscribe_valid); + RUN_TEST(encode_subscribe_fixed_header_flags); + RUN_TEST(encode_subscribe_options_byte_qos0); + RUN_TEST(encode_subscribe_options_byte_qos1); + RUN_TEST(encode_subscribe_options_byte_qos2); + RUN_TEST(encode_subscribe_topic_filter_oversized_rejected); + RUN_TEST(encode_subscribe_topic_filter_oversized_second_rejected); + RUN_TEST(encode_subscribe_has_qos1_flag); /* MqttEncode_Unsubscribe */ RUN_TEST(encode_unsubscribe_packet_id_zero); RUN_TEST(encode_unsubscribe_valid); + RUN_TEST(encode_unsubscribe_fixed_header_flags); + RUN_TEST(encode_unsubscribe_topic_filter_oversized_rejected); + RUN_TEST(encode_unsubscribe_topic_filter_oversized_second_rejected); + RUN_TEST(encode_unsubscribe_has_qos1_flag); /* MqttEncode_Connect */ RUN_TEST(encode_connect_password_without_username); RUN_TEST(encode_connect_username_and_password); RUN_TEST(encode_connect_username_only); RUN_TEST(encode_connect_no_credentials); + RUN_TEST(encode_connect_fixed_header_flags); + RUN_TEST(encode_connect_flags_username_password_clean); + RUN_TEST(encode_connect_flags_none); + RUN_TEST(encode_connect_flags_clean_session_only); + RUN_TEST(encode_connect_flags_username_only); + RUN_TEST(encode_connect_flags_lwt_qos1_retain); + RUN_TEST(encode_connect_client_id_oversized_rejected); + RUN_TEST(encode_connect_username_oversized_rejected); + RUN_TEST(encode_connect_password_oversized_rejected); + RUN_TEST(encode_connect_lwt_topic_oversized_rejected); + +#ifdef WOLFMQTT_BROKER + /* MqttDecode_Connect */ + RUN_TEST(decode_connect_v311_username_password); + RUN_TEST(decode_connect_v311_no_credentials); + RUN_TEST(decode_connect_wrong_protocol_name); + RUN_TEST(decode_connect_wrong_protocol_length); + RUN_TEST(decode_connect_v311_with_lwt); + + /* MqttDecode_Subscribe */ + RUN_TEST(decode_subscribe_v311_single_topic); + RUN_TEST(decode_subscribe_v311_qos3_reserved); +#ifdef WOLFMQTT_V5 + RUN_TEST(decode_subscribe_v5_options_byte_qos_extracted); +#endif +#endif /* QoS 2 ack arithmetic */ RUN_TEST(qos2_ack_arithmetic); /* MqttDecode_SubscribeAck */ RUN_TEST(decode_suback_valid); + RUN_TEST(decode_suback_multiple_return_codes); RUN_TEST(decode_suback_malformed_remain_len_zero); RUN_TEST(decode_suback_malformed_remain_len_one); @@ -855,6 +2201,12 @@ void run_mqtt_packet_tests(void) RUN_TEST(decode_publish_resp_malformed_remain_len_zero); RUN_TEST(decode_publish_resp_malformed_remain_len_one); + /* MqttEncode_PublishResp fixed-header QoS bits */ + RUN_TEST(encode_publish_rel_has_qos1_flag); + RUN_TEST(encode_publish_ack_has_qos0_flag); + RUN_TEST(encode_publish_rec_has_qos0_flag); + RUN_TEST(encode_publish_comp_has_qos0_flag); + /* MqttDecode_UnsubscribeAck */ RUN_TEST(decode_unsuback_valid); RUN_TEST(decode_unsuback_malformed_remain_len_zero); @@ -864,6 +2216,13 @@ void run_mqtt_packet_tests(void) RUN_TEST(publish_resp_v5_success_with_props_roundtrip); RUN_TEST(publish_resp_v5_error_no_props_roundtrip); RUN_TEST(publish_resp_v5_success_no_props_roundtrip); + + /* MqttEncode/Decode_Auth */ + RUN_TEST(auth_v5_cont_auth_roundtrip); + RUN_TEST(auth_v5_reauth_roundtrip); + RUN_TEST(auth_v5_reauth_decodes_without_error); + RUN_TEST(auth_v5_success_remaining_length_zero); + RUN_TEST(auth_v5_invalid_reason_code_rejected); #endif TEST_SUITE_END(); diff --git a/wolfmqtt/mqtt_types.h b/wolfmqtt/mqtt_types.h index a161b2f83..86b5d64d1 100644 --- a/wolfmqtt/mqtt_types.h +++ b/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