Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 44 additions & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
87 changes: 87 additions & 0 deletions scripts/broker.test
Original file line number Diff line number Diff line change
Expand Up @@ -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
Comment thread
embhorn marked this conversation as resolved.
# 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"
Comment thread
embhorn marked this conversation as resolved.
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 ---
Expand Down
22 changes: 20 additions & 2 deletions src/mqtt_broker.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down Expand Up @@ -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';
Expand Down Expand Up @@ -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

Expand Down
16 changes: 14 additions & 2 deletions src/mqtt_client.c
Original file line number Diff line number Diff line change
Expand Up @@ -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:
*
Expand Down Expand Up @@ -2918,6 +2930,7 @@ int MqttClient_NetDisconnect(MqttClient *client)
{
#ifdef WOLFMQTT_MULTITHREAD
MqttPendResp *tmpResp;
MqttPendResp *nextResp;
int rc;
#endif

Expand All @@ -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) {
Expand Down
92 changes: 67 additions & 25 deletions src/mqtt_packet.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 ||
Expand All @@ -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) {
Expand Down Expand Up @@ -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 */
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -1721,7 +1752,12 @@ int MqttEncode_Subscribe(byte *tx_buf, int tx_buf_len,
for (i = 0; i < subscribe->topic_count; i++) {
Comment thread
embhorn marked this conversation as resolved.
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 {
Expand Down Expand Up @@ -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 */
Expand Down Expand Up @@ -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) ||
Comment thread
embhorn marked this conversation as resolved.
(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)) {
Expand All @@ -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);
}
Expand Down
5 changes: 5 additions & 0 deletions tests/fuzz/gen_corpus.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Loading
Loading