diff --git a/BROKER.md b/BROKER.md index a993ef9a0..97711b68b 100644 --- a/BROKER.md +++ b/BROKER.md @@ -201,3 +201,7 @@ The CONNECT-handler unit test (`tests/test_broker_connect`) is part of `make che ## Limitations The wolfMQTT broker targets embedded and edge use cases. It is intentionally smaller in scope than full-featured server brokers such as Mosquitto or EMQX: there is no clustering, no bridging, no plugin/ACL framework, and no dynamic configuration reload. For large-scale or feature-rich deployments use a dedicated server broker; for a small, auditable, optionally-TLS broker that runs without threads or a heap, wolfMQTT is a good fit. + +Fan-out of a PUBLISH to its subscribers is not atomic. If the broker runs out of memory part-way through queueing copies of a QoS 1 or QoS 2 message, subscribers already served keep their copy and the publisher is not told the message was accepted: an MQTT v5 publisher receives a PUBACK or PUBREC with Reason Code 0x80, and an MQTT v3.1.1 publisher is disconnected. For a v3.1.1 QoS 2 publisher with a persistent session, the retransmitted PUBLISH is then treated as a duplicate, so subscribers that missed the original fan-out do not receive it. + +A single MQTT v5 packet may carry at most `MQTT_MAX_PROPS` (default 30) properties. The MQTT specification sets no such limit, but the broker rejects a packet with more properties as it would a malformed one. For a DISCONNECT this means the connection is treated as closed abnormally, so the client's Will Message is published even when the Reason Code is 0x00 (Normal disconnection). A DISCONNECT within the limit whose properties cannot be allocated because the broker is out of memory is still handled according to its Reason Code. diff --git a/src/mqtt_broker.c b/src/mqtt_broker.c index fcea7ee23..0e0d6df05 100644 --- a/src/mqtt_broker.c +++ b/src/mqtt_broker.c @@ -525,6 +525,39 @@ static void BrokerTls_Free(MqttBroker* broker) } #endif /* ENABLE_MQTT_TLS && !WOLFMQTT_BROKER_CUSTOM_NET */ +#if defined(WOLFMQTT_WOLFIP) || !defined(WOLFMQTT_BROKER_CUSTOM_NET) +/* Record err (0 for an accepted or no pending connection) as the latest + * accept result for broker's listener listen_sock. Returns 1 when err is a + * failure not yet logged for that listener, so a persistent failure is + * reported once and an idle listener cannot clear another's entry. */ +static int BrokerAcceptLog_Update(MqttBroker* broker, + BROKER_SOCKET_T listen_sock, int err) +{ + int* last; + + if (broker == NULL) { + return 0; + } + if (listen_sock == broker->listen_sock) { + last = &broker->accept_log_err; + } +#ifdef ENABLE_MQTT_TLS + else if (listen_sock == broker->listen_sock_tls) { + last = &broker->accept_log_err_tls; + } +#endif + else { + /* Not one of this broker's listeners: report every failure. */ + return (err != 0) ? 1 : 0; + } + if (*last == err) { + return 0; + } + *last = err; + return (err != 0) ? 1 : 0; +} +#endif /* WOLFMQTT_WOLFIP || !WOLFMQTT_BROKER_CUSTOM_NET */ + /* -------------------------------------------------------------------------- */ /* wolfIP network backend */ /* -------------------------------------------------------------------------- */ @@ -542,6 +575,9 @@ typedef struct BrokerWolfIP_Ctx { * with one broker instance. For multiple instances, use * WOLFMQTT_BROKER_CUSTOM_NET and provide per-instance context. */ static BrokerWolfIP_Ctx broker_wolfip_ctx; +/* Broker that logs for the wolfIP adapter, set by MqttBroker_Init when it is + * given this adapter's context. */ +static MqttBroker* broker_wolfip_log; static int BrokerWolfIP_Listen(void* ctx, BROKER_SOCKET_T* sock, word16 port, int backlog) @@ -590,13 +626,29 @@ static int BrokerWolfIP_Accept(void* ctx, BROKER_SOCKET_T listen_sock, fd = wolfIP_sock_accept(wctx->stack, listen_sock, NULL, NULL); if (fd == -WOLFIP_EAGAIN) { - /* No pending connection */ + /* No pending connection. */ + (void)BrokerAcceptLog_Update(broker_wolfip_log, listen_sock, 0); return MQTT_CODE_CONTINUE; } - if (fd < 0) { + if (fd == -WOLFIP_EINVAL) { + /* The listening socket itself is invalid. */ return MQTT_CODE_ERROR_NETWORK; } + if (fd < 0) { + /* wolfIP returns -1 for an exhausted socket pool, a connection its + * filter rejected, and a listener no longer in the listening state, + * without telling them apart. Retry on the next step rather than stop + * the broker, which a peer could otherwise force by opening + * connections, but report the condition. */ + if (BrokerAcceptLog_Update(broker_wolfip_log, listen_sock, fd)) { + WBLOG_ERR(broker_wolfip_log, + "broker: accept failed sock=%d rc=%d, retrying", + (int)listen_sock, fd); + } + return MQTT_CODE_CONTINUE; + } + (void)BrokerAcceptLog_Update(broker_wolfip_log, listen_sock, 0); *client_sock = fd; return MQTT_CODE_SUCCESS; } @@ -736,6 +788,7 @@ static int BrokerPosix_Accept(void* ctx, BROKER_SOCKET_T listen_sock, BROKER_SOCKET_T* client_sock) { BROKER_SOCKET_T fd; + int err; #ifdef SO_NOSIGPIPE int on = 1; #endif @@ -743,14 +796,35 @@ static int BrokerPosix_Accept(void* ctx, BROKER_SOCKET_T listen_sock, fd = accept(listen_sock, NULL, NULL); if (fd < 0) { - if (errno == EWOULDBLOCK || errno == EAGAIN) { - return MQTT_CODE_CONTINUE; + err = errno; + /* Only an unusable listening socket is fatal. Per-connection and + * resource errors (ECONNABORTED, EINTR, EMFILE, pending network + * errors) are retried, so a peer cannot stop the broker by + * aborting its own handshake or exhausting descriptors. */ + if (err == EBADF || err == EINVAL || err == ENOTSOCK || + err == EFAULT) { + return MQTT_CODE_ERROR_NETWORK; + } + if (err == EAGAIN || err == EWOULDBLOCK) { + /* No pending connection. */ + (void)BrokerAcceptLog_Update((MqttBroker*)ctx, listen_sock, 0); + } + else if (err != EINTR && + BrokerAcceptLog_Update((MqttBroker*)ctx, listen_sock, err)) { + WBLOG_ERR((MqttBroker*)ctx, + "broker: accept failed sock=%d errno=%d, retrying", + (int)listen_sock, err); } - return MQTT_CODE_ERROR_NETWORK; + return MQTT_CODE_CONTINUE; } + (void)BrokerAcceptLog_Update((MqttBroker*)ctx, listen_sock, 0); if (BrokerPosix_SetNonBlocking(fd) != MQTT_CODE_SUCCESS) { + /* Drop just this connection; the listener is still healthy. */ + WBLOG_ERR((MqttBroker*)ctx, + "broker: accept sock=%d set nonblocking failed errno=%d", + (int)fd, errno); close(fd); - return MQTT_CODE_ERROR_SYSTEM; + return MQTT_CODE_CONTINUE; } #ifdef SO_NOSIGPIPE /* macOS / BSDs: suppress SIGPIPE on writes to a peer-closed socket. @@ -1903,6 +1977,56 @@ static void BrokerOutPub_Free(BrokerOutPub* e) WOLFMQTT_FREE(e); } +/* Returns 1 when the PUBLISH that BrokerClient_DrainOutQueue would encode for + * topic, payload_len, qos and props, sent to a client at protocol_level, fits + * a transmit buffer of tx_len bytes. The encoder clamps an oversized payload + * to the buffer while the fixed header still declares all of it [MQTT 3.1.1 + * section 2.2.3], so such a message can never be delivered. */ +static int BrokerOutPub_FitsTx(const char* topic, word32 payload_len, + MqttQoS qos, byte protocol_level +#ifdef WOLFMQTT_V5 + , const MqttProp* props +#endif + , int tx_len) +{ + word32 remain_len; + int vbi_len; +#ifdef WOLFMQTT_V5 + int props_len; +#endif + + /* Topic Name (MQTT 3.1.1 section 3.3.2.1), Packet Identifier for QoS 1 + * and 2 (section 3.3.2.2), then the payload. */ + remain_len = MQTT_DATA_LEN_SIZE + (word32)XSTRLEN(topic) + payload_len; + if (qos > MQTT_QOS_0) { + remain_len += MQTT_DATA_LEN_SIZE; + } +#ifdef WOLFMQTT_V5 + /* Property Length and Properties (MQTT 5.0 section 3.3.2.3). */ + if (protocol_level >= MQTT_CONNECT_PROTOCOL_LEVEL_5) { + props_len = MqttEncode_Props(MQTT_PACKET_TYPE_PUBLISH, + (MqttProp*)props, NULL); + if (props_len < 0) { + return 0; + } + vbi_len = MqttEncode_Vbi(NULL, (word32)props_len); + if (vbi_len < 0) { + return 0; + } + remain_len += (word32)props_len + (word32)vbi_len; + } +#else + (void)protocol_level; +#endif + vbi_len = MqttEncode_Vbi(NULL, remain_len); + if (vbi_len < 0 || tx_len <= 0) { + return 0; + } + /* Fixed header byte and Remaining Length (MQTT 3.1.1 section 2.2). */ + return (remain_len <= (word32)tx_len && + 1 + (word32)vbi_len + remain_len <= (word32)tx_len) ? 1 : 0; +} + /* Allocate a new entry holding a deep copy of topic + payload (+ props if * src_props is non-NULL). Returns NULL on allocation failure (caller * decides whether that means drop or close). Fields are zero-initialized; @@ -2104,6 +2228,11 @@ static int BrokerClient_DrainOutQueue(BrokerClient* bc) enc_rc = MqttEncode_Publish(bc->tx_buf, BROKER_CLIENT_TX_SZ(bc), &out_pub, 0); + /* A payload clamped to tx_buf leaves Remaining Length declaring bytes + * the frame does not carry [MQTT 3.1.1 section 2.2.3]. */ + if (enc_rc > 0 && out_pub.buffer_pos != cur->payload_len) { + enc_rc = MQTT_TRACE_ERROR(MQTT_CODE_ERROR_OUT_OF_BUFFER); + } if (enc_rc <= 0) { if (bc->out_q_pending_len != 0) { return (enc_rc < 0) ? enc_rc : MQTT_CODE_ERROR_SYSTEM; @@ -2351,6 +2480,14 @@ static void BrokerClient_OnPubAck(BrokerClient* bc, word16 packet_id) (int)bc->sock, (unsigned)packet_id); return; } + /* A PUBACK completes only a QoS 1 exchange. Leave a QoS 2 entry awaiting + * its PUBREC so the identifier is not released early [MQTT-2.3.1-3]. */ + if (e->qos != MQTT_QOS_1) { + WBLOG_DBG(bc->broker, + "broker: PUBACK for QoS %d PUBLISH sock=%d packet_id=%u", + (int)e->qos, (int)bc->sock, (unsigned)packet_id); + return; + } BrokerClient_UnlinkOutPub(bc, prev, e); #ifdef WOLFMQTT_BROKER_PERSIST /* Defense in depth: in normal flow the orphan-reclaim path already @@ -3830,8 +3967,10 @@ static int BrokerOrphan_Reclaim(MqttBroker* broker, BrokerClient* new_bc) /* Enqueue a fan-out target onto an orphan session's queue. Called from * BrokerHandle_Publish when sub->client is NULL but an orphan with the * matching client_id exists. QoS 0 is dropped per spec; only persistent - * messages live in the offline queue. */ -static void BrokerOrphan_Enqueue(MqttBroker* broker, BrokerOrphanSession* o, + * messages live in the offline queue. Returns MQTT_CODE_ERROR_MEMORY when + * the message copy cannot be allocated, MQTT_CODE_ERROR_OUT_OF_BUFFER when it + * is too large to deliver, otherwise MQTT_CODE_SUCCESS. */ +static int BrokerOrphan_Enqueue(MqttBroker* broker, BrokerOrphanSession* o, const char* topic, const byte* payload, word32 payload_len, MqttQoS qos, byte retain #ifdef WOLFMQTT_V5 @@ -3842,7 +3981,21 @@ static void BrokerOrphan_Enqueue(MqttBroker* broker, BrokerOrphanSession* o, BrokerOutPub* e; if (broker == NULL || o == NULL || topic == NULL || qos == MQTT_QOS_0) { - return; + return MQTT_CODE_SUCCESS; + } + /* A copy too large for the transmit buffer of the client that resumes + * this Session could never be delivered. Refuse it before evicting an + * older message to make room. */ + if (!BrokerOutPub_FitsTx(topic, payload_len, qos, o->protocol_level + #ifdef WOLFMQTT_V5 + , src_props + #endif + , BROKER_TX_BUF_SZ)) { + WBLOG_ERR(broker, + "broker: orphan enqueue too large client_id=%s", + BrokerLog_Sanitize( + BROKER_STR_VALID(o->client_id) ? o->client_id : "(null)")); + return MQTT_CODE_ERROR_OUT_OF_BUFFER; } /* Drop-oldest eviction when the per-session offline queue is full. */ while (o->out_q_count >= BROKER_MAX_OFFLINE_MSGS_PER_SUB) { @@ -3883,7 +4036,7 @@ static void BrokerOrphan_Enqueue(MqttBroker* broker, BrokerOrphanSession* o, "broker: orphan enqueue alloc failed client_id=%s", BrokerLog_Sanitize( BROKER_STR_VALID(o->client_id) ? o->client_id : "(null)")); - return; + return MQTT_CODE_ERROR_MEMORY; } e->qos = qos; e->packet_id = BrokerNextPacketIdForQueue(broker, o->out_q_head); @@ -3893,7 +4046,7 @@ static void BrokerOrphan_Enqueue(MqttBroker* broker, BrokerOrphanSession* o, BrokerLog_Sanitize( BROKER_STR_VALID(o->client_id) ? o->client_id : "(null)")); BrokerOutPub_Free(e); - return; + return MQTT_CODE_SUCCESS; } e->retain = retain; e->state = BROKER_OUTQ_QUEUED; @@ -3907,7 +4060,7 @@ static void BrokerOrphan_Enqueue(MqttBroker* broker, BrokerOrphanSession* o, BrokerLog_Sanitize( BROKER_STR_VALID(o->client_id) ? o->client_id : "(null)")); BrokerOutPub_Free(e); - return; + return MQTT_CODE_SUCCESS; } else { e->enqueue_seq = o->out_q_tail->enqueue_seq + 1; @@ -3930,6 +4083,7 @@ static void BrokerOrphan_Enqueue(MqttBroker* broker, BrokerOrphanSession* o, BrokerLog_Sanitize( BROKER_STR_VALID(o->client_id) ? o->client_id : "(null)"), BrokerLog_Sanitize(topic), (int)qos, o->out_q_count); + return MQTT_CODE_SUCCESS; } /* Free every orphan (used by MqttBroker_Free and by wipe paths). */ @@ -6382,16 +6536,13 @@ static int BrokerHandle_Connect(BrokerClient* bc, int rx_len, int ack_len = 0; #endif /* [MQTT-3.2.2-2] Session Present is set when an accepted - * CleanSession=0 connection finds stored session state for the - * client_id. The MQTT spec defines Session state as more than - * subscriptions (in-flight QoS 1/2 PUBLISH, unacknowledged PUBREL, - * outstanding packet identifiers); this broker only persists - * subscriptions across disconnects today (BrokerSubs_OrphanClient - * keeps them, but per-message QoS 2 state in bc->qos2_in_flight is - * dropped). If broker session persistence ever widens to cover QoS - * state, the source of session_present needs to widen too - a - * BrokerSession_HasStoredState() helper is the natural extension - * point. */ + * CleanSession=0 connection resumes stored Session state for the + * client_id. A resumed Session can carry subscriptions, queued and + * in-flight outbound QoS 1/2 messages, and inbound QoS 2 identifiers + * awaiting PUBREL, whether taken over from a still-connected client or + * reclaimed from an orphaned or restored Session. Each takeover and + * reclaim path below sets session_present when it carries any of that + * state over; a new kind of Session state must do the same. */ int session_present = 0; #ifdef WOLFMQTT_V5 int auto_assigned = 0; @@ -6883,6 +7034,217 @@ static int BrokerHandle_Connect(BrokerClient* bc, int rx_len, } #endif + /* Store the Will and build the CONNACK properties that acceptance depends + * on before any existing Session is touched. A refusal from here on must + * leave a connected client with the same ClientId, and every stored + * Session, as they were. */ + BrokerClient_ClearWill(bc); +#ifdef WOLFMQTT_BROKER_WILL + if (mc.enable_lwt && mc.lwt_msg != NULL) { + int will_stored = 1; + if (mc.lwt_msg->topic_name != NULL && + mc.lwt_msg->topic_name_len > 0) { + BROKER_STORE_STR(bc->will_topic, mc.lwt_msg->topic_name, + mc.lwt_msg->topic_name_len, BROKER_MAX_TOPIC_LEN); +#ifndef WOLFMQTT_STATIC_MEMORY + if (bc->will_topic == NULL) { + will_stored = 0; + } +#endif + } + if (will_stored && mc.lwt_msg->total_len > 0 && + mc.lwt_msg->buffer != NULL) { + word16 wp_len; + wp_len = (word16)mc.lwt_msg->total_len; +#ifdef WOLFMQTT_STATIC_MEMORY + XMEMCPY(bc->will_payload, mc.lwt_msg->buffer, wp_len); +#else + bc->will_payload = (byte*)WOLFMQTT_MALLOC(wp_len); + if (bc->will_payload != NULL) { + XMEMCPY(bc->will_payload, mc.lwt_msg->buffer, wp_len); + } + else { + wp_len = 0; + will_stored = 0; + } +#endif + bc->will_payload_len = wp_len; + } + if (!will_stored) { + /* A Will missing its topic or payload would be dropped or + * published altered, so refuse the connection instead. */ + WBLOG_ERR(broker, "broker: LWT alloc failed sock=%d", + (int)bc->sock); + BrokerClient_ClearWill(bc); + #ifdef WOLFMQTT_V5 + if (mc.protocol_level >= MQTT_CONNECT_PROTOCOL_LEVEL_5) { + ack.return_code = MQTT_REASON_SERVER_UNAVAILABLE; + } + else + #endif + { + ack.return_code = MQTT_CONNECT_ACK_CODE_REFUSED_UNAVAIL; + } + goto send_connack; + } + /* Clamp will QoS to this build's Maximum QoS. A v5 Will over the cap + * was already refused above, so this only downgrades v3.1.1, which has + * no Maximum QoS advertisement to reject against. */ + bc->will_qos = (mc.lwt_msg->qos > WOLFMQTT_MAX_QOS) ? + (MqttQoS)WOLFMQTT_MAX_QOS : mc.lwt_msg->qos; + bc->will_retain = mc.lwt_msg->retain; + bc->will_delay_sec = 0; +#ifdef WOLFMQTT_V5 + if (mc.lwt_msg->props != NULL) { + MqttProp* prop = BrokerProps_Find(mc.lwt_msg->props, + MQTT_PROP_WILL_DELAY_INTERVAL); + if (prop != NULL) { + /* [MQTT-3.1.2.11.5] Honor the negotiated value, but clamp to a + * sane maximum so a client advertising a huge delay cannot + * hold a pending-will slot indefinitely. */ + if (prop->data_int > BROKER_MAX_WILL_DELAY_SEC) { + bc->will_delay_sec = BROKER_MAX_WILL_DELAY_SEC; + } + else { + bc->will_delay_sec = prop->data_int; + } + } + } +#endif + bc->has_will = 1; + WBLOG_DBG(broker, "broker: LWT stored sock=%d topic=%s qos=%d retain=%d " + "len=%u delay=%u", (int)bc->sock, BrokerLog_Sanitize(bc->will_topic), + bc->will_qos, bc->will_retain, + (unsigned)bc->will_payload_len, + (unsigned)bc->will_delay_sec); + } +#endif /* WOLFMQTT_BROKER_WILL */ + +#ifdef WOLFMQTT_V5 + /* Release the decoded CONNECT and Will properties before building the + * CONNACK: they share the fixed property pool, and a CONNECT carrying many + * User Properties would otherwise exhaust it and drop the mandatory + * Assigned Client Identifier added below. */ + if (mc.props != NULL) { + (void)MqttProps_Free(mc.props); + mc.props = NULL; + } + if (lwt.props != NULL) { + (void)MqttProps_Free(lwt.props); + lwt.props = NULL; + } + if (bc->protocol_level >= MQTT_CONNECT_PROTOCOL_LEVEL_5) { + MqttProp* prop; + int adv_failed = 0; + + /* [MQTT-3.1.3-6] Echo any server-assigned ClientId to v5 clients. + * Keyed off auto_assigned (set in the auto-id branch above) so this + * stays true to its name even if a future code path populates + * bc->client_id from another source (e.g., a TLS-cert identity). */ + if (auto_assigned) { + prop = MqttProps_Add(&ack.props); + if (prop != NULL) { + prop->type = MQTT_PROP_ASSIGNED_CLIENT_ID; + prop->data_str.str = bc->client_id; + prop->data_str.len = (word16)XSTRLEN(bc->client_id); + } + else { + /* [MQTT-3.1.3-6] An empty-ClientId client must be told its + * assigned id; refuse rather than accept an unusable connection + * when the property pool cannot hold it. */ + ack.return_code = MQTT_REASON_SERVER_UNAVAILABLE; + goto send_connack; + } + } + + /* Advertise feature availability. An absent availability property + * means the feature is supported (MQTT 5.0 sections 3.2.2.3.11, + * 3.2.2.3.12 and 3.2.2.3.15), so failing to add one that reports an + * unsupported feature must refuse the connection. */ + prop = MqttProps_Add(&ack.props); + if (prop != NULL) { + prop->type = MQTT_PROP_RETAIN_AVAIL; + #ifdef WOLFMQTT_BROKER_RETAINED + prop->data_byte = 1; + #else + prop->data_byte = 0; + #endif + } + #ifndef WOLFMQTT_BROKER_RETAINED + else { + adv_failed = 1; + } + #endif + prop = MqttProps_Add(&ack.props); + if (prop != NULL) { + prop->type = MQTT_PROP_WILDCARD_SUB_AVAIL; + #ifdef WOLFMQTT_BROKER_WILDCARDS + prop->data_byte = 1; + #else + prop->data_byte = 0; + #endif + } + #ifndef WOLFMQTT_BROKER_WILDCARDS + else { + adv_failed = 1; + } + #endif + /* Shared Subscriptions are not implemented. */ + prop = MqttProps_Add(&ack.props); + if (prop != NULL) { + prop->type = MQTT_PROP_SHARED_SUBSCRIPTION_AVAIL; + prop->data_byte = 0; + } + else { + adv_failed = 1; + } + /* [MQTT-3.2.2.3.4] Maximum QoS property MUST be 0 or 1. Absence + * of the property signals server supports Maximum QoS 2. Emitting + * Maximum QoS = 2 is a Protocol Error and strict v5 clients (e.g. + * mosquitto) will disconnect on receipt. Emit the property only + * when this build caps below QoS 2 via WOLFMQTT_MAX_QOS, and + * refuse the connection when it cannot be added. */ + #if WOLFMQTT_MAX_QOS < 2 + prop = MqttProps_Add(&ack.props); + if (prop != NULL) { + prop->type = MQTT_PROP_MAX_QOS; + prop->data_byte = (byte)WOLFMQTT_MAX_QOS; + } + else { + adv_failed = 1; + } + #endif + /* [MQTT-3.2.2.3.3] Receive Maximum. Advertise the broker's + * per-client inbound QoS 1/2 cap so well-behaved publishers can + * pace themselves. We accept up to BROKER_MAX_INBOUND_QOS2 + * concurrent QoS 2 PUBLISHes awaiting PUBREL ([MQTT-4.3.3]); use + * the same number as the wire value. Absence would mean 65535, so + * refuse the connection when it cannot be added. The property MUST + * NOT be 0 - we skip the emission entirely in that (unreachable) + * case so a future tunable down to 0 cannot send an illegal value. + * The cap applies in both dynamic and static memory modes. */ + if (BROKER_MAX_INBOUND_QOS2 > 0) { + prop = MqttProps_Add(&ack.props); + if (prop != NULL) { + prop->type = MQTT_PROP_RECEIVE_MAX; + prop->data_short = + (BROKER_MAX_INBOUND_QOS2 > 0xFFFF) ? + (word16)0xFFFF : + (word16)BROKER_MAX_INBOUND_QOS2; + } + else { + adv_failed = 1; + } + } + if (adv_failed) { + WBLOG_ERR(broker, "broker: CONNACK property alloc failed sock=%d", + (int)bc->sock); + ack.return_code = MQTT_REASON_SERVER_UNAVAILABLE; + goto send_connack; + } + } +#endif + #ifdef WOLFMQTT_STATIC_MEMORY /* Reserve bounded Session storage before accepting a persistent * connection. Refusing a new Session preserves every existing Session; @@ -7085,64 +7447,6 @@ static int BrokerHandle_Connect(BrokerClient* bc, int rx_len, #endif } - /* Store Last Will and Testament */ - BrokerClient_ClearWill(bc); -#ifdef WOLFMQTT_BROKER_WILL - if (mc.enable_lwt && mc.lwt_msg != NULL) { - if (mc.lwt_msg->topic_name != NULL && - mc.lwt_msg->topic_name_len > 0) { - BROKER_STORE_STR(bc->will_topic, mc.lwt_msg->topic_name, - mc.lwt_msg->topic_name_len, BROKER_MAX_TOPIC_LEN); - } - if (mc.lwt_msg->total_len > 0 && mc.lwt_msg->buffer != NULL) { - word16 wp_len; - wp_len = (word16)mc.lwt_msg->total_len; -#ifdef WOLFMQTT_STATIC_MEMORY - XMEMCPY(bc->will_payload, mc.lwt_msg->buffer, wp_len); -#else - bc->will_payload = (byte*)WOLFMQTT_MALLOC(wp_len); - if (bc->will_payload != NULL) { - XMEMCPY(bc->will_payload, mc.lwt_msg->buffer, wp_len); - } - else { - wp_len = 0; - } -#endif - bc->will_payload_len = wp_len; - } - /* Clamp will QoS to this build's Maximum QoS. A v5 Will over the cap - * was already refused above, so this only downgrades v3.1.1, which has - * no Maximum QoS advertisement to reject against. */ - bc->will_qos = (mc.lwt_msg->qos > WOLFMQTT_MAX_QOS) ? - (MqttQoS)WOLFMQTT_MAX_QOS : mc.lwt_msg->qos; - bc->will_retain = mc.lwt_msg->retain; - bc->will_delay_sec = 0; -#ifdef WOLFMQTT_V5 - if (mc.lwt_msg->props != NULL) { - MqttProp* prop = BrokerProps_Find(mc.lwt_msg->props, - MQTT_PROP_WILL_DELAY_INTERVAL); - if (prop != NULL) { - /* [MQTT-3.1.2.11.5] Honor the negotiated value, but clamp to a - * sane maximum so a client advertising a huge delay cannot - * hold a pending-will slot indefinitely. */ - if (prop->data_int > BROKER_MAX_WILL_DELAY_SEC) { - bc->will_delay_sec = BROKER_MAX_WILL_DELAY_SEC; - } - else { - bc->will_delay_sec = prop->data_int; - } - } - } -#endif - bc->has_will = 1; - WBLOG_DBG(broker, "broker: LWT stored sock=%d topic=%s qos=%d retain=%d " - "len=%u delay=%u", (int)bc->sock, BrokerLog_Sanitize(bc->will_topic), - bc->will_qos, bc->will_retain, - (unsigned)bc->will_payload_len, - (unsigned)bc->will_delay_sec); - } -#endif /* WOLFMQTT_BROKER_WILL */ - /* Credentials were already validated above, before any session-state * mutation. [MQTT-3.2.2-2]: when the accepted CleanSession=0 connection * finds stored session state, Session Present MUST be 1; otherwise it @@ -7151,104 +7455,6 @@ static int BrokerHandle_Connect(BrokerClient* bc, int rx_len, * Present=0 on a refused CONNACK. */ ack.flags = session_present ? MQTT_CONNECT_ACK_FLAG_SESSION_PRESENT : 0; ack.return_code = MQTT_CONNECT_ACK_CODE_ACCEPTED; -#ifdef WOLFMQTT_V5 - ack.props = NULL; - - /* Release the decoded CONNECT and Will properties before building the - * CONNACK: they share the fixed property pool, and a CONNECT carrying many - * User Properties would otherwise exhaust it and drop the mandatory - * Assigned Client Identifier added below. */ - if (mc.props != NULL) { - (void)MqttProps_Free(mc.props); - mc.props = NULL; - } - if (lwt.props != NULL) { - (void)MqttProps_Free(lwt.props); - lwt.props = NULL; - } -#endif - -#ifdef WOLFMQTT_V5 - if (bc->protocol_level >= MQTT_CONNECT_PROTOCOL_LEVEL_5 && - ack.return_code == MQTT_CONNECT_ACK_CODE_ACCEPTED) { - MqttProp* prop; - - /* [MQTT-3.1.3-6] Echo any server-assigned ClientId to v5 clients. - * Keyed off auto_assigned (set in the auto-id branch above) so this - * stays true to its name even if a future code path populates - * bc->client_id from another source (e.g., a TLS-cert identity). */ - if (auto_assigned) { - prop = MqttProps_Add(&ack.props); - if (prop != NULL) { - prop->type = MQTT_PROP_ASSIGNED_CLIENT_ID; - prop->data_str.str = bc->client_id; - prop->data_str.len = (word16)XSTRLEN(bc->client_id); - } - else { - /* [MQTT-3.1.3-6] An empty-ClientId client must be told its - * assigned id; refuse rather than accept an unusable connection - * when the property pool cannot hold it. Clear the effective - * session expiry first so the refused connection tears its - * tentative session down instead of orphaning an unreachable - * one that could evict a valid session at capacity. */ - bc->session_expiry_sec = 0; - ack.return_code = MQTT_REASON_SERVER_UNAVAILABLE; - goto send_connack; - } - } - - /* Advertise feature availability */ - prop = MqttProps_Add(&ack.props); - if (prop != NULL) { - prop->type = MQTT_PROP_RETAIN_AVAIL; - #ifdef WOLFMQTT_BROKER_RETAINED - prop->data_byte = 1; - #else - prop->data_byte = 0; - #endif - } - prop = MqttProps_Add(&ack.props); - if (prop != NULL) { - prop->type = MQTT_PROP_WILDCARD_SUB_AVAIL; - #ifdef WOLFMQTT_BROKER_WILDCARDS - prop->data_byte = 1; - #else - prop->data_byte = 0; - #endif - } - /* [MQTT-3.2.2.3.4] Maximum QoS property MUST be 0 or 1. Absence - * of the property signals server supports Maximum QoS 2. Emitting - * Maximum QoS = 2 is a Protocol Error and strict v5 clients (e.g. - * mosquitto) will disconnect on receipt. Emit the property only - * when this build caps below QoS 2 via WOLFMQTT_MAX_QOS. */ - #if WOLFMQTT_MAX_QOS < 2 - prop = MqttProps_Add(&ack.props); - if (prop != NULL) { - prop->type = MQTT_PROP_MAX_QOS; - prop->data_byte = (byte)WOLFMQTT_MAX_QOS; - } - #endif - - /* [MQTT-3.2.2.3.3] Receive Maximum. Advertise the broker's - * per-client inbound QoS 1/2 cap so well-behaved publishers can - * pace themselves. We accept up to BROKER_MAX_INBOUND_QOS2 - * concurrent QoS 2 PUBLISHes awaiting PUBREL ([MQTT-4.3.3]); use - * the same number as the wire value. The property MUST NOT be - * 0 - we skip the emission entirely in that (unreachable) case - * so a future tunable down to 0 cannot send an illegal value. - * The cap applies in both dynamic and static memory modes. */ - if (BROKER_MAX_INBOUND_QOS2 > 0) { - prop = MqttProps_Add(&ack.props); - if (prop != NULL) { - prop->type = MQTT_PROP_RECEIVE_MAX; - prop->data_short = - (BROKER_MAX_INBOUND_QOS2 > 0xFFFF) ? - (word16)0xFFFF : - (word16)BROKER_MAX_INBOUND_QOS2; - } - } - } -#endif send_connack: /* [MQTT-3.2.2-4] A refused CONNACK MUST have Session Present = 0. The @@ -7257,6 +7463,19 @@ static int BrokerHandle_Connect(BrokerClient* bc, int rx_len, * return_code to a refusal after that point. */ if (ack.return_code != MQTT_CONNECT_ACK_CODE_ACCEPTED) { ack.flags = 0; + /* [MQTT-3.1.2-8] A Will is stored only for an accepted connection, + * and one stored before a later refusal must not be published by any + * close path. */ + BrokerClient_ClearWill(bc); + #ifdef WOLFMQTT_V5 + /* The CONNACK properties built before a later refusal describe an + * accepted connection, including any Assigned Client Identifier + * (MQTT 5.0 section 3.2.2.3.7), so a refusal carries none. */ + if (ack.props != NULL) { + (void)MqttProps_Free(ack.props); + ack.props = NULL; + } + #endif } rc = MqttEncode_ConnectAck(bc->tx_buf, BROKER_CLIENT_TX_SZ(bc), &ack); if (rc > 0) { @@ -7446,6 +7665,17 @@ static int BrokerHandle_Subscribe(BrokerClient* bc, int rx_len, #endif } if (sub_rc == MQTT_CODE_SUCCESS) + #endif + #ifdef WOLFMQTT_V5 + /* Shared Subscriptions are advertised as unavailable, so refuse + * a '$share/' filter rather than match it literally (MQTT 5.0 + * sections 3.9.3 and 4.8.2). */ + if (bc->protocol_level >= MQTT_CONNECT_PROTOCOL_LEVEL_5 && + flen >= 7 && XMEMCMP(f, "$share/", 7) == 0) { + sub_rc = MQTT_CODE_ERROR_BAD_ARG; + fail_code = MQTT_REASON_SS_NOT_SUPPORTED; + } + if (sub_rc == MQTT_CODE_SUCCESS) #endif { sub_rc = BrokerSubs_Add(broker, bc, f, flen, topic_qos); @@ -7648,6 +7878,7 @@ static int BrokerHandle_Publish(BrokerClient* bc, int rx_len, byte* payload = NULL; char* topic = NULL; MqttQoS eff_qos; + int fanout_failed = 0; #if WOLFMQTT_MAX_QOS >= 2 int qos2_duplicate = 0; #endif @@ -8043,13 +8274,15 @@ static int BrokerHandle_Publish(BrokerClient* bc, int rx_len, if (eff_qos > MQTT_QOS_0) { BrokerOrphanSession* o = BrokerOrphan_Find(broker, sub->client_id); - if (o != NULL) { - BrokerOrphan_Enqueue(broker, o, topic, payload, - pub.total_len, eff_qos, 0 - #ifdef WOLFMQTT_V5 - , pub.props - #endif - ); + if (o != NULL && + BrokerOrphan_Enqueue(broker, o, topic, payload, + pub.total_len, eff_qos, 0 + #ifdef WOLFMQTT_V5 + , pub.props + #endif + ) < 0) { + /* The offline copy was not queued either. */ + fanout_failed = 1; } } } @@ -8059,6 +8292,7 @@ static int BrokerHandle_Publish(BrokerClient* bc, int rx_len, "broker: fan-out list alloc failed after %d destination(s), " "remaining subscribers not served sock=%d topic=%s", targets.count, (int)bc->sock, BrokerLog_Sanitize(topic)); + fanout_failed = 1; } clients_gen_snapshot = broker->clients_gen; @@ -8117,6 +8351,21 @@ static int BrokerHandle_Publish(BrokerClient* bc, int rx_len, } wc->connected = 0; } + else if (!BrokerOutPub_FitsTx(topic, pub.total_len, eff_qos, + wc->protocol_level + #ifdef WOLFMQTT_V5 + , pub.props + #endif + , BROKER_CLIENT_TX_SZ(wc))) { + WBLOG_ERR(broker, + "broker: PUBLISH too large for subscriber " + "sock=%d -> sock=%d topic=%s", + (int)bc->sock, (int)wc->sock, BrokerLog_Sanitize(topic)); + /* QoS 0 delivery may be dropped; QoS 1/2 may not. */ + if (eff_qos >= MQTT_QOS_1) { + fanout_failed = 1; + } + } else { BrokerOutPub* e = BrokerOutPub_Alloc(topic, payload, pub.total_len @@ -8128,6 +8377,10 @@ static int BrokerHandle_Publish(BrokerClient* bc, int rx_len, WBLOG_ERR(broker, "broker: PUBLISH fwd alloc failed sock=%d " "-> sock=%d", (int)bc->sock, (int)wc->sock); + /* QoS 0 delivery may be dropped; QoS 1/2 may not. */ + if (eff_qos >= MQTT_QOS_1) { + fanout_failed = 1; + } } else { e->qos = eff_qos; @@ -8177,6 +8430,34 @@ static int BrokerHandle_Publish(BrokerClient* bc, int rx_len, resp.reason_code = MQTT_REASON_SUCCESS; resp.props = NULL; #endif + /* A subscriber's QoS 1/2 copy could not be queued, so the PUBLISH + * must not be acknowledged as accepted. */ + if (fanout_failed) { + #ifdef WOLFMQTT_V5 + if (bc->protocol_level >= MQTT_CONNECT_PROTOCOL_LEVEL_5) { + resp.reason_code = MQTT_REASON_UNSPECIFIED_ERR; + #if WOLFMQTT_MAX_QOS >= 2 + /* A failure PUBREC ends the QoS 2 exchange (MQTT 5.0 + * section 4.3.3), so no PUBREL will follow. */ + if (pub.qos == MQTT_QOS_2) { + BrokerInboundQos2_Remove(bc, pub.packet_id); + } + #endif + } + else + #endif + { + /* v3.1.1 has no failure acknowledgement; close instead. + * Known limitation: a QoS 2 Packet Identifier recorded + * above stays recorded, so on a resumed Session the + * retransmission is treated as a duplicate and the + * subscribers that missed this fan-out do not receive it. + * Removing it would instead re-deliver to subscribers + * already served, breaking exactly-once for them. */ + rc = MQTT_CODE_ERROR_MEMORY; + goto publish_cleanup; + } + } rc = MqttEncode_PublishResp(bc->tx_buf, BROKER_CLIENT_TX_SZ(bc), (pub.qos == MQTT_QOS_1) ? MQTT_PACKET_TYPE_PUBLISH_ACK : MQTT_PACKET_TYPE_PUBLISH_REC, &resp); @@ -8549,6 +8830,11 @@ static int BrokerClient_Process(MqttBroker* broker, BrokerClient* bc) if (rc != MQTT_CODE_SUCCESS) { return 0; } + if (bc->close_after_direct) { + /* The DISCONNECT that announced this close is fully written. */ + BrokerClient_AbnormalClose(broker, bc); + return 0; + } activity = 1; } @@ -8848,6 +9134,20 @@ static int BrokerClient_Process(MqttBroker* broker, BrokerClient* bc) if (disc.props != NULL) { (void)MqttProps_Free(disc.props); } + /* MQTT 5.0 section 3.14.2.2.2 */ + #ifndef WOLFMQTT_STATIC_MEMORY + if (BrokerSend_Disconnect(bc, + MQTT_REASON_PROTOCOL_ERR) == + MQTT_CODE_CONTINUE) { + /* Finish writing the DISCONNECT on a + * later step before closing. */ + bc->close_after_direct = 1; + return 0; + } + #else + (void)BrokerSend_Disconnect(bc, + MQTT_REASON_PROTOCOL_ERR); + #endif BrokerClient_AbnormalClose(broker, bc); return 0; } @@ -8864,11 +9164,34 @@ static int BrokerClient_Process(MqttBroker* broker, BrokerClient* bc) } #endif } + else if (disc_rc == MQTT_CODE_ERROR_MEMORY) { + /* The packet is well formed but its Properties did + * not fit in the property pool. The Reason Code is + * decoded before them, so honor it [MQTT-3.14.4-3]; + * the Session Expiry Interval keeps its CONNECT + * value. */ + WBLOG_ERR(broker, + "broker: DISCONNECT properties dropped, out of " + "memory sock=%d", (int)bc->sock); #ifdef WOLFMQTT_BROKER_WILL + if (disc.reason_code == + MQTT_REASON_DISCONNECT_W_WILL_MSG) { + BrokerClient_PublishWill(broker, bc); + } + else { + BrokerClient_ClearWill(bc); + } + #endif + } else { - BrokerClient_ClearWill(bc); + /* A Malformed Packet is not a normal disconnect, so + * the Will is published (MQTT 5.0 section 3.1.2.5). */ + if (disc.props != NULL) { + (void)MqttProps_Free(disc.props); + } + BrokerClient_AbnormalClose(broker, bc); + return 0; } - #endif /* Free any decoded v5 DISCONNECT properties. */ if (disc.props != NULL) { (void)MqttProps_Free(disc.props); @@ -9062,6 +9385,11 @@ int MqttBroker_Init(MqttBroker* broker, MqttBrokerNet* net) } #endif +#if defined(WOLFMQTT_WOLFIP) + if (broker->net.ctx == &broker_wolfip_ctx) { + broker_wolfip_log = broker; + } +#endif #if !defined(WOLFMQTT_WOLFIP) && !defined(WOLFMQTT_BROKER_CUSTOM_NET) /* For the default POSIX backend, the net callbacks expect ctx to be a * MqttBroker* for logging via WBLOG_*. If no context was provided, @@ -9074,10 +9402,20 @@ int MqttBroker_Init(MqttBroker* broker, MqttBrokerNet* net) return MQTT_CODE_SUCCESS; } +/* An accept callback returns MQTT_CODE_CONTINUE or MQTT_CODE_ERROR_TIMEOUT + * when no connection is pending; any other negative result means the + * listener itself has failed. */ +static int BrokerAccept_IsFailure(int rc) +{ + return (rc < 0 && rc != MQTT_CODE_CONTINUE && + rc != MQTT_CODE_ERROR_TIMEOUT) ? 1 : 0; +} + int MqttBroker_Step(MqttBroker* broker) { int activity = 0; int rc; + int accept_rc = MQTT_CODE_SUCCESS; if (broker == NULL) { return MQTT_CODE_ERROR_BAD_ARG; @@ -9107,6 +9445,10 @@ int MqttBroker_Step(MqttBroker* broker) BROKER_SOCKET_T new_sock = BROKER_SOCKET_INVALID; rc = broker->net.accept(broker->net.ctx, broker->listen_sock, &new_sock); + if (BrokerAccept_IsFailure(rc)) { + WBLOG_ERR(broker, "broker: accept failed (plain) rc=%d", rc); + accept_rc = rc; + } if (rc == MQTT_CODE_SUCCESS && new_sock != BROKER_SOCKET_INVALID) { #ifdef WOLFMQTT_POSIX_SOCKET /* Reject socket if >= FD_SETSIZE (would overflow fd_set) */ @@ -9138,6 +9480,10 @@ int MqttBroker_Step(MqttBroker* broker) BROKER_SOCKET_T new_sock = BROKER_SOCKET_INVALID; rc = broker->net.accept(broker->net.ctx, broker->listen_sock_tls, &new_sock); + if (BrokerAccept_IsFailure(rc)) { + WBLOG_ERR(broker, "broker: accept failed (TLS) rc=%d", rc); + accept_rc = rc; + } if (rc == MQTT_CODE_SUCCESS && new_sock != BROKER_SOCKET_INVALID) { #ifdef WOLFMQTT_POSIX_SOCKET /* Reject socket if >= FD_SETSIZE (would overflow fd_set) */ @@ -9238,6 +9584,11 @@ int MqttBroker_Step(MqttBroker* broker) activity = 1; } + /* Report a failed listener only after this step has serviced the + * connected clients. */ + if (accept_rc != MQTT_CODE_SUCCESS) { + return accept_rc; + } return activity ? MQTT_CODE_SUCCESS : MQTT_CODE_CONTINUE; } @@ -9423,6 +9774,9 @@ int MqttBroker_Start(MqttBroker* broker) } #endif + /* New listeners start with no accept failure reported. */ + broker->accept_log_err = 0; + broker->accept_log_err_tls = 0; broker->running = 1; return MQTT_CODE_SUCCESS; @@ -9517,6 +9871,11 @@ int MqttBroker_Free(MqttBroker* broker) return MQTT_CODE_ERROR_BAD_ARG; } broker->running = 0; +#if defined(WOLFMQTT_WOLFIP) + if (broker_wolfip_log == broker) { + broker_wolfip_log = NULL; + } +#endif /* Disconnect and free all clients and subscriptions */ #ifdef WOLFMQTT_STATIC_MEMORY diff --git a/src/mqtt_broker_persist.c b/src/mqtt_broker_persist.c index d85053e7f..54f674b1a 100644 --- a/src/mqtt_broker_persist.c +++ b/src/mqtt_broker_persist.c @@ -38,7 +38,10 @@ * first, variable-length strings/payloads last, lengths prefixed * big-endian. Forward compatibility is by schema-version bump + wipe. * wrap_mode is bound as AAD by the AES-GCM path so a tamper that - * flips it (or flips rec_kind) fails the tag check. + * flips it (or flips rec_kind) fails the tag check. The AAD also carries + * the namespace and exact storage key, so a ciphertext copied to another + * key fails too. Stores written before the key was bound fail the META + * tag check and go through the schema-mismatch wipe on the next start. */ #ifdef HAVE_CONFIG_H @@ -265,16 +268,43 @@ static void wmqb_force_zero(void* mem, word32 len) } } -static int wmqb_encrypt_blob(MqttBroker* broker, +/* AES-GCM additional data: the record header followed by where the record + * is stored, encoded unambiguously as namespace(1) || key_len(2, big + * endian) || key. A ciphertext moved to another namespace or storage key + * then fails authentication. Caller frees the returned buffer. */ +static byte* wmqb_build_aad(const byte* hdr, byte ns, const byte* key, + word16 key_len, word32* aad_len) +{ + byte* aad; + + *aad_len = (word32)WMQB_HDR_LEN + 3u + key_len; + aad = (byte*)WOLFMQTT_MALLOC(*aad_len); + if (aad == NULL) { + return NULL; + } + XMEMCPY(aad, hdr, WMQB_HDR_LEN); + aad[WMQB_HDR_LEN] = ns; + wmqb_w_u16(&aad[WMQB_HDR_LEN + 1], key_len); + if (key_len > 0) { + XMEMCPY(&aad[WMQB_HDR_LEN + 3], key, key_len); + } + return aad; +} + +static int wmqb_encrypt_blob(MqttBroker* broker, byte ns, + const byte* key, word16 key_len, const byte* plain, word32 plain_len, byte** ct_out, word32* ct_out_len) { Aes aes; WC_RNG rng; byte* out; + byte* aad; + word32 aad_len; word32 body_len; int rc; if (broker == NULL || plain == NULL || plain_len < WMQB_HDR_LEN || + (key == NULL && key_len > 0) || ct_out == NULL || ct_out_len == NULL) { return MQTT_CODE_ERROR_BAD_ARG; } @@ -289,26 +319,35 @@ static int wmqb_encrypt_blob(MqttBroker* broker, if (out == NULL) { return MQTT_CODE_ERROR_MEMORY; } + aad = wmqb_build_aad(plain, ns, key, key_len, &aad_len); + if (aad == NULL) { + WOLFMQTT_FREE(out); + return MQTT_CODE_ERROR_MEMORY; + } XMEMCPY(out, plain, WMQB_HDR_LEN); if (wc_InitRng(&rng) != 0) { + WOLFMQTT_FREE(aad); WOLFMQTT_FREE(out); return MQTT_CODE_ERROR_SYSTEM; } if (wc_RNG_GenerateBlock(&rng, out + WMQB_HDR_LEN, WMQB_GCM_NONCE_LEN) != 0) { wc_FreeRng(&rng); + WOLFMQTT_FREE(aad); WOLFMQTT_FREE(out); return MQTT_CODE_ERROR_SYSTEM; } wc_FreeRng(&rng); if (wc_AesInit(&aes, NULL, INVALID_DEVID) != 0) { + WOLFMQTT_FREE(aad); WOLFMQTT_FREE(out); return MQTT_CODE_ERROR_SYSTEM; } if (wc_AesGcmSetKey(&aes, broker->persist_key_cache, (word32)sizeof(broker->persist_key_cache)) != 0) { wc_AesFree(&aes); + WOLFMQTT_FREE(aad); WOLFMQTT_FREE(out); return MQTT_CODE_ERROR_SYSTEM; } @@ -318,29 +357,35 @@ static int wmqb_encrypt_blob(MqttBroker* broker, out + WMQB_HDR_LEN, WMQB_GCM_NONCE_LEN, /* nonce */ out + WMQB_HDR_LEN + WMQB_GCM_NONCE_LEN + body_len, /* tag */ WMQB_GCM_TAG_LEN, - plain, WMQB_HDR_LEN) != 0) { /* aad = header */ + aad, aad_len) != 0) { /* aad = header + location */ wc_AesFree(&aes); + WOLFMQTT_FREE(aad); WOLFMQTT_FREE(out); return MQTT_CODE_ERROR_SYSTEM; } wc_AesFree(&aes); + WOLFMQTT_FREE(aad); *ct_out = out; return 0; } /* Reverse of wmqb_encrypt_blob. Caller must free the returned plain. * TODO(perf): see wmqb_encrypt_blob - same per-call key-schedule cost. */ -static int wmqb_decrypt_blob(MqttBroker* broker, +static int wmqb_decrypt_blob(MqttBroker* broker, byte ns, + const byte* key, word16 key_len, const byte* ct, word32 ct_len, byte** plain_out, word32* plain_out_len) { Aes aes; byte* out; + byte* aad; + word32 aad_len; word32 body_len; int rc; if (broker == NULL || ct == NULL || ct_len < (word32)(WMQB_HDR_LEN + WMQB_GCM_NONCE_LEN + WMQB_GCM_TAG_LEN) || + (key == NULL && key_len > 0) || plain_out == NULL || plain_out_len == NULL) { return MQTT_CODE_ERROR_BAD_ARG; } @@ -355,9 +400,15 @@ static int wmqb_decrypt_blob(MqttBroker* broker, if (out == NULL) { return MQTT_CODE_ERROR_MEMORY; } + aad = wmqb_build_aad(ct, ns, key, key_len, &aad_len); + if (aad == NULL) { + WOLFMQTT_FREE(out); + return MQTT_CODE_ERROR_MEMORY; + } XMEMCPY(out, ct, WMQB_HDR_LEN); if (wc_AesInit(&aes, NULL, INVALID_DEVID) != 0) { + WOLFMQTT_FREE(aad); wmqb_force_zero(out, *plain_out_len); WOLFMQTT_FREE(out); return MQTT_CODE_ERROR_SYSTEM; @@ -365,6 +416,7 @@ static int wmqb_decrypt_blob(MqttBroker* broker, if (wc_AesGcmSetKey(&aes, broker->persist_key_cache, (word32)sizeof(broker->persist_key_cache)) != 0) { wc_AesFree(&aes); + WOLFMQTT_FREE(aad); wmqb_force_zero(out, *plain_out_len); WOLFMQTT_FREE(out); return MQTT_CODE_ERROR_SYSTEM; @@ -375,13 +427,15 @@ static int wmqb_decrypt_blob(MqttBroker* broker, ct + WMQB_HDR_LEN, WMQB_GCM_NONCE_LEN, ct + WMQB_HDR_LEN + WMQB_GCM_NONCE_LEN + body_len, WMQB_GCM_TAG_LEN, - ct, WMQB_HDR_LEN) != 0) { + aad, aad_len) != 0) { wc_AesFree(&aes); + WOLFMQTT_FREE(aad); wmqb_force_zero(out, *plain_out_len); WOLFMQTT_FREE(out); return MQTT_CODE_ERROR_MALFORMED_DATA; } wc_AesFree(&aes); + WOLFMQTT_FREE(aad); *plain_out = out; return 0; } @@ -407,7 +461,8 @@ static int wmqb_kv_put_commit(MqttBroker* broker, byte ns, { byte* enc; word32 enc_len; - rc = wmqb_encrypt_blob(broker, blob, blob_len, &enc, &enc_len); + rc = wmqb_encrypt_blob(broker, ns, key, key_len, blob, blob_len, + &enc, &enc_len); if (rc != 0) { return rc; } @@ -417,8 +472,9 @@ static int wmqb_kv_put_commit(MqttBroker* broker, byte ns, #else rc = h->kv_put(h->ctx, ns, key, key_len, blob, blob_len); #endif + /* The write is not committed until the backend has made it durable. */ if (rc == 0 && h->sync != NULL) { - (void)h->sync(h->ctx); + rc = h->sync(h->ctx); } return rc; } @@ -436,8 +492,9 @@ static int wmqb_kv_del_commit(MqttBroker* broker, byte ns, return 0; } rc = h->kv_del(h->ctx, ns, key, key_len); + /* The delete is not committed until the backend has made it durable. */ if (rc == 0 && h->sync != NULL) { - (void)h->sync(h->ctx); + rc = h->sync(h->ctx); } return rc; } @@ -452,6 +509,8 @@ struct wmqb_iter_decrypt_ctx { MqttBroker* broker; MqttBrokerPersist_IterCb inner_cb; void* inner_ctx; + int fatal_rc; + byte ns; }; static int wmqb_iter_decrypt_cb(const byte* key, word16 key_len, @@ -464,7 +523,14 @@ static int wmqb_iter_decrypt_cb(const byte* key, word16 key_len, int rc; int stop; - rc = wmqb_decrypt_blob(d->broker, blob, blob_len, &plain, &plain_len); + rc = wmqb_decrypt_blob(d->broker, d->ns, key, key_len, blob, blob_len, + &plain, &plain_len); + if (rc == MQTT_CODE_ERROR_MEMORY) { + /* Out of memory says nothing about the record; stop rather than + * restore without it. */ + d->fatal_rc = rc; + return 1; + } if (rc != 0) { /* Forward an empty blob; inner cb will read_header-fail and * bump its skipped counter. */ @@ -482,6 +548,8 @@ static int wmqb_iter_decrypt(MqttBroker* broker, byte ns, { struct wmqb_iter_decrypt_ctx wrap; const MqttBrokerPersistHooks* h; + int rc; + if (broker == NULL || cb == NULL) { return MQTT_CODE_ERROR_BAD_ARG; } @@ -492,7 +560,13 @@ static int wmqb_iter_decrypt(MqttBroker* broker, byte ns, wrap.broker = broker; wrap.inner_cb = cb; wrap.inner_ctx = cb_ctx; - return h->kv_iter(h->ctx, ns, wmqb_iter_decrypt_cb, &wrap); + wrap.fatal_rc = 0; + wrap.ns = ns; + rc = h->kv_iter(h->ctx, ns, wmqb_iter_decrypt_cb, &wrap); + if (rc == 0) { + rc = wrap.fatal_rc; + } + return rc; } /* Decrypt-on-get for the META record specifically. The 256-byte stack @@ -530,7 +604,8 @@ static int wmqb_kv_get_decrypt_meta(MqttBroker* broker, byte ns, if (rc != 0) { return rc; } - rc = wmqb_decrypt_blob(broker, enc, cap, &plain, &plain_len); + rc = wmqb_decrypt_blob(broker, ns, key, key_len, enc, cap, &plain, + &plain_len); if (rc != 0) { return rc; } @@ -1104,6 +1179,7 @@ struct wmqb_delq_ctx { const byte* cid; word16 cid_len; struct wmqb_wipe_key* head; + int alloc_failed; /* a matching key could not be collected */ }; static int wmqb_delq_iter_cb(const byte* key, word16 key_len, @@ -1124,11 +1200,13 @@ static int wmqb_delq_iter_cb(const byte* key, word16 key_len, } node = (struct wmqb_wipe_key*)WOLFMQTT_MALLOC(sizeof(*node)); if (node == NULL) { + dq->alloc_failed = 1; return 1; } node->key = (byte*)WOLFMQTT_MALLOC(key_len); if (node->key == NULL) { WOLFMQTT_FREE(node); + dq->alloc_failed = 1; return 1; } XMEMCPY(node->key, key, key_len); @@ -1180,6 +1258,9 @@ int BrokerPersist_DelOutQueue(MqttBroker* broker, const char* client_id) struct wmqb_delq_ctx ctx; struct wmqb_wipe_key* cur; int deleted = 0; + int rc; + int del_rc; + int sync_rc; if (h->kv_iter == NULL || h->kv_del == NULL) { return 0; } @@ -1190,23 +1271,35 @@ int BrokerPersist_DelOutQueue(MqttBroker* broker, const char* client_id) * iterator callback ignores blob bytes - so bypassing the * decrypt wrapper here is safe and avoids unnecessary AES * cycles. */ - (void)h->kv_iter(h->ctx, BROKER_PERSIST_NS_OUTQ, + rc = h->kv_iter(h->ctx, BROKER_PERSIST_NS_OUTQ, wmqb_delq_iter_cb, &ctx); + if (rc == 0 && ctx.alloc_failed) { + rc = MQTT_CODE_ERROR_MEMORY; + } + /* Delete what was collected even after a failure, but report the + * first error: the queue is not fully removed. */ cur = ctx.head; while (cur != NULL) { struct wmqb_wipe_key* next = cur->next; - if (h->kv_del(h->ctx, BROKER_PERSIST_NS_OUTQ, cur->key, - cur->key_len) == 0) { + del_rc = h->kv_del(h->ctx, BROKER_PERSIST_NS_OUTQ, cur->key, + cur->key_len); + if (del_rc == 0) { deleted++; } + else if (rc == 0) { + rc = del_rc; + } WOLFMQTT_FREE(cur->key); WOLFMQTT_FREE(cur); cur = next; } if (h->sync != NULL) { - (void)h->sync(h->ctx); + sync_rc = h->sync(h->ctx); + if (rc == 0) { + rc = sync_rc; + } } - return deleted; + return (rc != 0) ? rc : deleted; } #endif } @@ -1519,6 +1612,20 @@ static BrokerOrphanSession* wmqb_restore_find_orphan(MqttBroker* broker, } #endif /* WOLFMQTT_STATIC_MEMORY */ +/* Account for a record that failed to restore. A malformed or unsupported + * record is skipped; running out of memory is not a property of the record, + * so it stops the iteration and fails the restore. Returns the iterator + * callback's stop flag. */ +static int wmqb_restore_record_failed(struct wmqb_restore_ctx* c, int rc) +{ + c->skipped++; + if (rc == MQTT_CODE_ERROR_MEMORY) { + c->fatal_rc = rc; + return 1; + } + return 0; +} + #ifdef WOLFMQTT_BROKER_RETAINED /* NS_RETAINED body prefix: qos(1) _reserved(1) store_time(8) expiry(4) * topic_len(2). */ @@ -1693,11 +1800,9 @@ static int wmqb_iter_retained_cb(const byte* key, word16 key_len, rc = wmqb_decode_and_insert_retained(c->broker, blob, blob_len); if (rc == 0) { c->loaded++; + return 0; } - else { - c->skipped++; - } - return 0; /* always continue */ + return wmqb_restore_record_failed(c, rc); } @@ -2009,11 +2114,9 @@ static int wmqb_iter_subs_cb(const byte* key, word16 key_len, blob_len); if (rc == 0) { c->loaded++; + return 0; } - else { - c->skipped++; - } - return 0; + return wmqb_restore_record_failed(c, rc); } /* Decode NS_SESSION record and create a matching orphan slot. */ @@ -2064,6 +2167,16 @@ static int wmqb_decode_and_insert_session(MqttBroker* broker, return MQTT_CODE_ERROR_NOT_FOUND; } } +#else + /* An empty ClientId or a store holding more Sessions than the cap (the + * cap is enforced on orphaning, not on every write) are records to skip, + * not failures; only a NULL after these checks means memory ran out. */ + if (cid_len == 0) { + return MQTT_CODE_ERROR_MALFORMED_DATA; + } + if (broker->orphan_session_count >= BROKER_MAX_PERSIST_SESSIONS) { + return MQTT_CODE_ERROR_OUT_OF_BUFFER; + } #endif if (wmqb_restore_create_orphan(broker, &p[16], cid_len, proto_level, session_expiry, orphan_since) == NULL) { @@ -2121,6 +2234,9 @@ static int wmqb_iter_session_cb(const byte* key, word16 key_len, if (rc == 0) { c->loaded++; } + else if (rc == MQTT_CODE_ERROR_MEMORY) { + return wmqb_restore_record_failed(c, rc); + } else { c->skipped++; #ifdef WOLFMQTT_STATIC_MEMORY @@ -2453,11 +2569,9 @@ static int wmqb_iter_outq_cb(const byte* key, word16 key_len, blob_len); if (rc == 0) { c->loaded++; + return 0; } - else { - c->skipped++; - } - return 0; + return wmqb_restore_record_failed(c, rc); } /* Sweep orphans whose v5 Session Expiry has elapsed. Cascades to subs @@ -2555,32 +2669,47 @@ static int wmqb_wipe_iter_cb(const byte* key, word16 key_len, return 0; } +/* Delete every record in one namespace. Returns the number deleted, or the + * first iterator, allocation, deletion or sync failure. */ static int wmqb_wipe_ns(MqttBroker* broker, byte ns) { const MqttBrokerPersistHooks* h = broker->persist; struct wmqb_wipe_ctx wc; struct wmqb_wipe_key* cur; int deleted = 0; + int rc; + int del_rc; + int sync_rc; if (h->kv_iter == NULL || h->kv_del == NULL) { return 0; } XMEMSET(&wc, 0, sizeof(wc)); - (void)h->kv_iter(h->ctx, ns, wmqb_wipe_iter_cb, &wc); + rc = h->kv_iter(h->ctx, ns, wmqb_wipe_iter_cb, &wc); + if (rc == 0 && wc.alloc_failed) { + rc = MQTT_CODE_ERROR_MEMORY; + } cur = wc.head; while (cur != NULL) { struct wmqb_wipe_key* next = cur->next; - if (h->kv_del(h->ctx, ns, cur->key, cur->key_len) == 0) { + del_rc = h->kv_del(h->ctx, ns, cur->key, cur->key_len); + if (del_rc == 0) { deleted++; } + else if (rc == 0) { + rc = del_rc; + } WOLFMQTT_FREE(cur->key); WOLFMQTT_FREE(cur); cur = next; } if (h->sync != NULL) { - (void)h->sync(h->ctx); + sync_rc = h->sync(h->ctx); + if (rc == 0) { + rc = sync_rc; + } } - return deleted; + return (rc != 0) ? rc : deleted; } #endif /* !WOLFMQTT_STATIC_MEMORY */ @@ -2595,15 +2724,30 @@ static int wmqb_wipe_all(MqttBroker* broker) "broker: persist wipe skipped (STATIC_MEMORY mode)"); return 0; #else + /* META goes last: if the wipe stops early, the mismatched META is still + * there and the next restore repeats the wipe instead of treating the + * leftover records as a first run. */ + static const byte wipe_order[] = { + BROKER_PERSIST_NS_SESSION, BROKER_PERSIST_NS_SUBS, + BROKER_PERSIST_NS_RETAINED, BROKER_PERSIST_NS_OUTQ, + BROKER_PERSIST_NS_META + }; int total = 0; + int rc; + size_t i; + if (broker == NULL || broker->persist == NULL) { return 0; } - total += wmqb_wipe_ns(broker, BROKER_PERSIST_NS_META); - total += wmqb_wipe_ns(broker, BROKER_PERSIST_NS_SESSION); - total += wmqb_wipe_ns(broker, BROKER_PERSIST_NS_SUBS); - total += wmqb_wipe_ns(broker, BROKER_PERSIST_NS_RETAINED); - total += wmqb_wipe_ns(broker, BROKER_PERSIST_NS_OUTQ); + for (i = 0; i < sizeof(wipe_order); i++) { + rc = wmqb_wipe_ns(broker, wipe_order[i]); + if (rc < 0) { + WMQB_LOG_ERR(broker, "broker: persist wipe ns=%u failed rc=%d", + (unsigned)wipe_order[i], rc); + return rc; + } + total += rc; + } WMQB_LOG_INFO(broker, "broker: persist wipe deleted=%d", total); return total; #endif @@ -2636,7 +2780,11 @@ int BrokerPersist_Restore(MqttBroker* broker) * failure from wmqb_kv_get_decrypt_meta.) */ WMQB_LOG_ERR(broker, "broker: persist schema mismatch - wiping all records"); - (void)wmqb_wipe_all(broker); + rc = wmqb_wipe_all(broker); + if (rc < 0) { + /* Leave the old META so the next start retries the wipe. */ + return rc; + } rc = wmqb_meta_write(broker); if (rc == 0) { broker->persist_restored = 1; @@ -2676,11 +2824,9 @@ int BrokerPersist_Restore(MqttBroker* broker) if (h->kv_iter != NULL) { rc = wmqb_kv_iter(broker, BROKER_PERSIST_NS_SESSION, wmqb_iter_session_cb, &ctx); -#ifdef WOLFMQTT_STATIC_MEMORY if (rc == 0) { rc = ctx.fatal_rc; } -#endif if (rc != 0) { WMQB_LOG_ERR(broker, "broker: persist restore sessions failed rc=%d", rc); @@ -2709,6 +2855,9 @@ int BrokerPersist_Restore(MqttBroker* broker) if (h->kv_iter != NULL) { rc = wmqb_kv_iter(broker, BROKER_PERSIST_NS_RETAINED, wmqb_iter_retained_cb, &ctx); + if (rc == 0) { + rc = ctx.fatal_rc; + } if (rc != 0) { WMQB_LOG_ERR(broker, "broker: persist restore retained failed rc=%d", rc); @@ -2725,6 +2874,9 @@ int BrokerPersist_Restore(MqttBroker* broker) if (h->kv_iter != NULL) { rc = wmqb_kv_iter(broker, BROKER_PERSIST_NS_SUBS, wmqb_iter_subs_cb, &ctx); + if (rc == 0) { + rc = ctx.fatal_rc; + } if (rc != 0) { WMQB_LOG_ERR(broker, "broker: persist restore subs failed rc=%d", rc); @@ -2741,6 +2893,9 @@ int BrokerPersist_Restore(MqttBroker* broker) if (h->kv_iter != NULL) { rc = wmqb_kv_iter(broker, BROKER_PERSIST_NS_OUTQ, wmqb_iter_outq_cb, &ctx); + if (rc == 0) { + rc = ctx.fatal_rc; + } if (rc != 0) { WMQB_LOG_ERR(broker, "broker: persist restore outq failed rc=%d", rc); diff --git a/src/mqtt_broker_persist_posix.c b/src/mqtt_broker_persist_posix.c index e2e9e3bae..39c2a3b04 100644 --- a/src/mqtt_broker_persist_posix.c +++ b/src/mqtt_broker_persist_posix.c @@ -65,6 +65,26 @@ typedef struct WmqbPosixCtx { #define O_CLOEXEC 0 #endif +/* Overridable so tests can inject durability failures. */ +#ifndef WMQB_POSIX_FSYNC + #define WMQB_POSIX_FSYNC(fd) fsync(fd) +#endif + +/* Flush a namespace directory so a rename or unlink in it survives a crash. + * Retries a bounded number of EINTRs, like the record write loop. */ +static int wmqb_posix_fsync_dir(int dir_fd) +{ + int eintr_count = 0; + + while (WMQB_POSIX_FSYNC(dir_fd) < 0) { + if (errno == EINTR && eintr_count++ < 16) { + continue; + } + return MQTT_CODE_ERROR_SYSTEM; + } + return 0; +} + /* Forward decl of all hook callbacks. */ static int wmqb_posix_put(void* ctx, byte ns, const byte* key, word16 key_len, const byte* blob, word32 blob_len); @@ -399,7 +419,7 @@ static int wmqb_posix_put(void* ctx, byte ns, const byte* key, written += (word32)w; } } - if (fsync(fd) < 0) { + if (WMQB_POSIX_FSYNC(fd) < 0) { (void)close(fd); (void)unlinkat(ns_fd, tmp_name, 0); (void)close(ns_fd); @@ -418,9 +438,9 @@ static int wmqb_posix_put(void* ctx, byte ns, const byte* key, return MQTT_CODE_ERROR_SYSTEM; } /* fsync the namespace dir so rename is durable. */ - (void)fsync(ns_fd); + rc = wmqb_posix_fsync_dir(ns_fd); (void)close(ns_fd); - return 0; + return rc; } static int wmqb_posix_get(void* ctx, byte ns, const byte* key, @@ -521,9 +541,10 @@ static int wmqb_posix_del(void* ctx, byte ns, const byte* key, (void)close(ns_fd); return MQTT_CODE_ERROR_SYSTEM; } - (void)fsync(ns_fd); + /* fsync the namespace dir so the unlink is durable. */ + rc = wmqb_posix_fsync_dir(ns_fd); (void)close(ns_fd); - return 0; + return rc; } static int wmqb_posix_iter(void* ctx, byte ns, MqttBrokerPersist_IterCb cb, @@ -550,7 +571,7 @@ static int wmqb_posix_iter(void* ctx, byte ns, MqttBrokerPersist_IterCb cb, (void)close(ns_fd); return MQTT_CODE_ERROR_SYSTEM; } - while ((ent = readdir(d)) != NULL) { + for (;;) { char key_hex[2 * 256 + 1]; byte key_buf[256]; byte* blob; @@ -564,6 +585,16 @@ static int wmqb_posix_iter(void* ctx, byte ns, MqttBrokerPersist_IterCb cb, const char* dot; int stop; + /* readdir reports both end of directory and failure as NULL; only + * errno tells them apart. */ + errno = 0; + ent = readdir(d); + if (ent == NULL) { + if (errno != 0) { + rc = MQTT_CODE_ERROR_SYSTEM; + } + break; + } if (ent->d_name[0] == '.') { continue; } @@ -584,12 +615,37 @@ static int wmqb_posix_iter(void* ctx, byte ns, MqttBrokerPersist_IterCb cb, if (kn < 0) { continue; } + /* Classify the entry by type and owner before opening it, so + * symlinks, sockets, FIFOs and foreign files are skipped however the + * platform would fail to open them. */ + if (fstatat(dirfd(d), ent->d_name, &st, AT_SYMLINK_NOFOLLOW) < 0) { + if (errno == ENOENT) { + continue; /* removed since readdir */ + } + rc = MQTT_CODE_ERROR_SYSTEM; + break; + } + if (!S_ISREG(st.st_mode) || st.st_uid != geteuid()) { + continue; + } fd = openat(dirfd(d), ent->d_name, O_RDONLY | O_NONBLOCK | O_NOFOLLOW | O_CLOEXEC); if (fd < 0) { - continue; + /* Removed or swapped for a symlink since fstatat; anything else + * on an owned record is an I/O failure that must not be + * reported as a complete pass. */ + if (errno == ENOENT || errno == ELOOP) { + continue; + } + rc = MQTT_CODE_ERROR_SYSTEM; + break; + } + if (fstat(fd, &st) < 0) { + (void)close(fd); + rc = MQTT_CODE_ERROR_SYSTEM; + break; } - if (fstat(fd, &st) < 0 || !S_ISREG(st.st_mode) || + if (!S_ISREG(st.st_mode) || st.st_uid != geteuid() || st.st_size <= 0 || (word64)st.st_size > 16 * 1024 * 1024) { /* Refuse non-regular, foreign-owned, or oversized records. */ @@ -625,10 +681,13 @@ static int wmqb_posix_iter(void* ctx, byte ns, MqttBrokerPersist_IterCb cb, /* Scrub the record before releasing the heap buffer. In * encrypt-at-rest builds the blob is ciphertext, so the wipe is * harmless; in plaintext builds it clears session and payload - * data from a buffer that would otherwise linger in the heap. */ + * data from a buffer that would otherwise linger in the heap. + * A read error or short read leaves the record unloaded, so + * report it rather than skip it. */ wmqb_posix_force_zero(blob, blob_cap); WOLFMQTT_FREE(blob); - continue; + rc = MQTT_CODE_ERROR_SYSTEM; + break; } stop = cb(key_buf, (word16)kn, blob, blob_cap, cb_ctx); wmqb_posix_force_zero(blob, blob_cap); @@ -638,7 +697,7 @@ static int wmqb_posix_iter(void* ctx, byte ns, MqttBrokerPersist_IterCb cb, } } (void)closedir(d); - return 0; + return rc; } static int wmqb_posix_sync(void* ctx) @@ -650,7 +709,7 @@ static int wmqb_posix_sync(void* ctx) if (c == NULL || c->root_fd < 0) { return MQTT_CODE_ERROR_BAD_ARG; } - return (fsync(c->root_fd) == 0) ? 0 : MQTT_CODE_ERROR_SYSTEM; + return (WMQB_POSIX_FSYNC(c->root_fd) == 0) ? 0 : MQTT_CODE_ERROR_SYSTEM; } int MqttBrokerNet_PersistPosix_Init(MqttBrokerPersistHooks* hooks, diff --git a/src/mqtt_client.c b/src/mqtt_client.c index 1ce10bb4f..60983c190 100644 --- a/src/mqtt_client.c +++ b/src/mqtt_client.c @@ -655,10 +655,10 @@ static void MqttClient_Replay_FreeSlot(MqttReplayMsg* slot) XMEMSET(slot, 0, sizeof(*slot)); } -/* Retain a copy of an outbound QoS > 0 PUBLISH. A message that does not fit - * the pool is left unretained rather than refused: it still goes out, it just - * cannot be replayed, which is the same position the client was in before the - * store existed. */ +/* Retain a copy of an outbound QoS > 0 PUBLISH. On a persistent Session the + * caller refuses a publish that would not fit the pool first, see + * MqttClient_Replay_CheckRoomSafe; on a clean Session a message that does not + * fit still goes out unretained, since it is never replayed. */ static void MqttClient_Replay_Add(MqttClient* client, MqttPublish* publish) { MqttReplayMsg* slot; @@ -797,6 +797,53 @@ static void MqttClient_Replay_AddSafe(MqttClient* client, MqttPublish* publish) #endif } +/* [MQTT-4.4.0-1] A persistent Session must be able to re-send every + * unacknowledged QoS > 0 PUBLISH, so refuse one that has no replay slot. */ +static int MqttClient_Replay_CheckRoomSafe(MqttClient* client, + word16 packet_id) +{ + int rc = MQTT_CODE_SUCCESS; + + if ((MqttClient_Flags(client, 0, 0) & + MQTT_CLIENT_FLAG_PERSISTENT_SESSION) == 0) { + return MQTT_CODE_SUCCESS; + } +#ifdef WOLFMQTT_MULTITHREAD + rc = wm_SemLock(&client->lockClient); + if (rc != MQTT_CODE_SUCCESS) { + return rc; + } +#endif + if (MqttClient_Replay_Find(client, packet_id) == NULL && + MqttClient_Replay_Find(client, 0) == NULL) { + rc = MQTT_TRACE_ERROR(MQTT_CODE_ERROR_OUT_OF_BUFFER); + } +#ifdef WOLFMQTT_MULTITHREAD + wm_SemUnlock(&client->lockClient); +#endif + return rc; +} + +/* Whether the Session requested by this CONNECT outlives the Network + * Connection: CleanSession 0 in v3.1.1, a non-zero Session Expiry Interval + * in v5 (MQTT 5.0 section 3.1.2.11.2). */ +static int MqttClient_ConnectIsPersistent(const MqttConnect* mc_connect) +{ +#ifdef WOLFMQTT_V5 + if (mc_connect->protocol_level >= MQTT_CONNECT_PROTOCOL_LEVEL_5) { + const MqttProp* prop; + + for (prop = mc_connect->props; prop != NULL; prop = prop->next) { + if (prop->type == MQTT_PROP_SESSION_EXPIRY_INTERVAL) { + return (prop->data_int != 0) ? 1 : 0; + } + } + return 0; + } +#endif + return (mc_connect->clean_session == 0) ? 1 : 0; +} + static void MqttClient_Replay_RemoveSafe(MqttClient* client, word16 packet_id) { #ifdef WOLFMQTT_MULTITHREAD @@ -1272,6 +1319,21 @@ static int Handle_ConnectAck_Props(MqttClient* client, MqttProp* props) /* MQTT v5 [3.1.2.11.8] */ client->topic_alias_max = prop->data_short; } + #ifndef WOLFMQTT_NO_SESSION_REPLAY + else if (prop->type == MQTT_PROP_SESSION_EXPIRY_INTERVAL) { + /* MQTT 5.0 section 3.2.2.3.2: the server's value replaces the + * one requested in CONNECT, so it decides whether this Session + * outlives the connection and needs replay. */ + if (prop->data_int != 0) { + (void)MqttClient_Flags(client, 0, + MQTT_CLIENT_FLAG_PERSISTENT_SESSION); + } + else { + (void)MqttClient_Flags(client, + MQTT_CLIENT_FLAG_PERSISTENT_SESSION, 0); + } + } + #endif } return rc; } @@ -2533,6 +2595,11 @@ static int MqttClient_WaitType(MqttClient *client, void *packet_obj, #endif if (rc < 0) { MqttWriteStop(client, mms_stat); + /* Drop the unsendable ack so later waits read new packets + * instead of retrying the same encode. */ + mms_stat->ackPacketType = MQTT_PACKET_TYPE_RESERVED; + mms_stat->ack = MQTT_MSG_BEGIN; + mms_stat->read = MQTT_MSG_BEGIN; break; } @@ -2605,8 +2672,13 @@ static int MqttClient_WaitType(MqttClient *client, void *packet_obj, #endif read_cleanup: - if (recvFatal && MqttClient_IsFatalProtoError(rc)) { - /* Scrub decoded peer data before releasing lockRecv. */ + /* Scrub received peer data, which may be a partial v5 AUTH packet, + * before releasing lockRecv whenever the receive has ended. A timeout or + * wake-up keeps a partially read packet for the next call. Only the + * holder of the read may touch rx_buf: after an ack-phase failure the + * read was already released and another thread may be filling it. */ + if (mms_stat->isReadActive && rc < 0 && rc != MQTT_CODE_CONTINUE && + rc != MQTT_CODE_ERROR_TIMEOUT && rc != MQTT_CODE_STDIN_WAKE) { CLIENT_FORCE_ZERO(client->rx_buf, client->rx_buf_len); } MqttReadStop(client, mms_stat); @@ -3373,6 +3445,16 @@ int MqttClient_Connect(MqttClient *client, MqttConnect *mc_connect) * out. Resuming this same write re-enters past MQTT_MSG_BEGIN, so * the once-per-connection guard does not see it. */ (void)MqttClient_Flags(client, 0, MQTT_CLIENT_FLAG_CONNECT_SENT); + #ifndef WOLFMQTT_NO_SESSION_REPLAY + if (MqttClient_ConnectIsPersistent(mc_connect)) { + (void)MqttClient_Flags(client, 0, + MQTT_CLIENT_FLAG_PERSISTENT_SESSION); + } + else { + (void)MqttClient_Flags(client, + MQTT_CLIENT_FLAG_PERSISTENT_SESSION, 0); + } + #endif } #ifdef WOLFMQTT_NONBLOCK if (rc == MQTT_CODE_CONTINUE @@ -3721,6 +3803,11 @@ static int MqttClient_Publish_ReadPayload(MqttClient* client, if (msg_len > 0) { rc = MqttSocket_Read(client, client->rx_buf, msg_len, timeout_ms); + /* Zero means the peer closed mid-payload; retrying the same + * read would never make progress. */ + if (rc == 0) { + rc = MQTT_TRACE_ERROR(MQTT_CODE_ERROR_NETWORK); + } if (rc < 0) { break; } @@ -3943,10 +4030,13 @@ static int MqttClient_Publish_WritePayload(MqttClient *client, } #ifdef WOLFMQTT_V5 -/* Give back the Receive Maximum unit reserved for a QoS>0 v5 PUBLISH when it - * terminates without a matching PUBACK/PUBCOMP (write failure, cancel, - * ack-wait timeout, PUBREC rejection). Clamped to the negotiated ceiling so a - * later stray ack for the same id cannot inflate the quota past it. */ +/* Give back the Receive Maximum unit reserved for a QoS>0 v5 PUBLISH once the + * server can no longer be counting it: the PUBLISH failed before it was sent + * or its write failed, or the exchange ended with PUBACK/PUBCOMP or a + * rejecting PUBREC. Never use it after an ack-wait timeout or a cancel of a + * sent PUBLISH; the server still counts that one [MQTT-4.9]. Clamped to the + * negotiated ceiling so a later stray ack for the same id cannot inflate the + * quota past it. */ static void MqttClient_RestoreRecvQuota(MqttClient* client, MqttPublish* publish) { @@ -4075,6 +4165,20 @@ static int MqttPublishMsg(MqttClient *client, MqttPublish *publish, #endif client->write.len = rc; + #ifndef WOLFMQTT_NO_SESSION_REPLAY + if (publish->qos > MQTT_QOS_0) { + rc = MqttClient_Replay_CheckRoomSafe(client, + publish->packet_id); + if (rc != MQTT_CODE_SUCCESS) { + MqttWriteStop(client, &publish->stat); + #ifdef WOLFMQTT_V5 + MqttClient_RestoreRecvQuota(client, publish); + #endif + return rc; + } + } + #endif + if (publish->qos > MQTT_QOS_0) { /* [MQTT-2.3.1-2] A new QoS>0 PUBLISH must carry a currently * unused Packet Identifier. A re-send of this same PUBLISH is @@ -5422,6 +5526,10 @@ int MqttClient_CancelMessage(MqttClient *client, MqttObject* msg) #ifdef WOLFMQTT_DEBUG_CLIENT PRINTF("Cancel Read Lock"); #endif + /* An abandoned read can leave part of a received packet - possibly + * v5 AUTH data - in rx_buf. Scrub it before MqttReadStop releases + * lockRecv. */ + CLIENT_FORCE_ZERO(client->rx_buf, client->rx_buf_len); MqttReadStop(client, mms_stat); } if (mms_stat->isWriteActive) { @@ -5487,10 +5595,10 @@ int MqttClient_NetConnect(MqttClient *client, const char* host, int MqttClient_NetDisconnect(MqttClient *client) { + int rc; #ifdef WOLFMQTT_MULTITHREAD MqttPendResp *tmpResp; MqttPendResp *nextResp; - int rc; #endif if (client == NULL) { @@ -5540,7 +5648,29 @@ int MqttClient_NetDisconnect(MqttClient *client) * session state across one, so nothing stays in flight [MQTT-2.3.1-3]. */ MqttClient_SendIdsReset(client); - return MqttSocket_Disconnect(client); + rc = MqttSocket_Disconnect(client); + + /* An operation abandoned with the connection, such as a partly written + * nonblocking CONNECT, can leave credentials in tx_buf or peer data in + * rx_buf. Nothing on this connection can resume, so wipe both. With + * multiple threads a buffer whose read or write is still active belongs + * to that operation's owner (lockRecv/lockSend, not lockClient), so it is + * left for MqttClient_CancelMessage, which scrubs it on release. */ +#ifdef WOLFMQTT_MULTITHREAD + if (wm_SemLock(&client->lockClient) == 0) { + if (!client->write.isActive) { + CLIENT_FORCE_ZERO(client->tx_buf, client->tx_buf_len); + } + if (!client->read.isActive) { + CLIENT_FORCE_ZERO(client->rx_buf, client->rx_buf_len); + } + wm_SemUnlock(&client->lockClient); + } +#else + CLIENT_FORCE_ZERO(client->tx_buf, client->tx_buf_len); + CLIENT_FORCE_ZERO(client->rx_buf, client->rx_buf_len); +#endif + return rc; } int MqttClient_GetProtocolVersion(MqttClient *client) diff --git a/src/mqtt_packet.c b/src/mqtt_packet.c index d3edadecf..63dcd052b 100644 --- a/src/mqtt_packet.c +++ b/src/mqtt_packet.c @@ -2844,12 +2844,16 @@ int MqttDecode_PublishResp(byte* rx_buf, int rx_buf_len, byte type, #ifdef WOLFMQTT_V5 publish_resp->props = NULL; if (publish_resp->protocol_level >= MQTT_CONNECT_PROTOCOL_LEVEL_5) { + byte* packet_end; + + /* remain_len is attacker-controlled; the whole packet must be + * present before any field is bounded by its end */ + if (rx_buf_len < header_len + remain_len) { + return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_OUT_OF_BUFFER); + } + packet_end = &rx_buf[header_len + remain_len]; + if (remain_len > MQTT_DATA_LEN_SIZE) { - /* remain_len is attacker-controlled; confirm the reason-code - * byte was received before reading past the buffer */ - if (rx_buf_len < (rx_payload - rx_buf) + 1) { - return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_OUT_OF_BUFFER); - } /* Decode the Reason Code */ publish_resp->reason_code = *rx_payload++; } @@ -2862,30 +2866,32 @@ int MqttDecode_PublishResp(byte* rx_buf, int rx_buf_len, byte type, int props_tmp; /* Decode Length of Properties */ - if (rx_buf_len < (rx_payload - rx_buf)) { - return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_OUT_OF_BUFFER); - } props_tmp = MqttDecode_Vbi(rx_payload, &props_len, - (word32)(rx_buf_len - (rx_payload - rx_buf))); + (word32)(packet_end - rx_payload)); if (props_tmp < 0) return props_tmp; - - if (props_len <= (word32)(rx_buf_len - (rx_payload - rx_buf))) { + rx_payload += props_tmp; + if (props_len > (word32)(packet_end - rx_payload)) { + return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_OUT_OF_BUFFER); + } + if (props_len > 0) { + /* Decode the Properties */ + props_tmp = MqttDecode_Props((MqttPacketType)type, + &publish_resp->props, rx_payload, + (word32)(packet_end - rx_payload), props_len); + if (props_tmp < 0) + return props_tmp; rx_payload += props_tmp; - if (props_len > 0) { - /* Decode the Properties */ - props_tmp = MqttDecode_Props((MqttPacketType)type, - &publish_resp->props, rx_payload, - (word32)(rx_buf_len - (rx_payload - rx_buf)), - props_len); - if (props_tmp < 0) - return props_tmp; - rx_payload += props_tmp; - } } - else { - return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_OUT_OF_BUFFER); + } + /* MQTT 5.0 sections 3.4.2 to 3.7.2: no payload follows the + * Properties. */ + if (rx_payload != packet_end) { + if (publish_resp->props != NULL) { + (void)MqttProps_Free(publish_resp->props); + publish_resp->props = NULL; } + return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_MALFORMED_DATA); } } #endif @@ -4046,41 +4052,46 @@ int MqttDecode_Disconnect(byte *rx_buf, int rx_buf_len, MqttDisconnect *disc) disc->props = NULL; if (remain_len > 0) { - /* remain_len is attacker-controlled; confirm the reason-code byte - * was received before reading past the buffer */ - if (rx_buf_len < (rx_payload - rx_buf) + 1) { + byte* packet_end; + + /* remain_len is attacker-controlled; the whole packet must be + * present before any field is bounded by its end */ + if (rx_buf_len < header_len + remain_len) { return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_OUT_OF_BUFFER); } + packet_end = &rx_buf[header_len + remain_len]; + /* Decode variable header */ disc->reason_code = *rx_payload++; if (remain_len > 1) { int props_tmp; /* Decode Length of Properties */ - if (rx_buf_len < (rx_payload - rx_buf)) { - return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_OUT_OF_BUFFER); - } props_tmp = MqttDecode_Vbi(rx_payload, &props_len, - (word32)(rx_buf_len - (rx_payload - rx_buf))); + (word32)(packet_end - rx_payload)); if (props_tmp < 0) return props_tmp; - - if (props_len <= (word32)(rx_buf_len - (rx_payload - rx_buf))) { + rx_payload += props_tmp; + if (props_len > (word32)(packet_end - rx_payload)) { + return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_OUT_OF_BUFFER); + } + if (props_len > 0) { + /* Decode the Properties */ + props_tmp = MqttDecode_Props(MQTT_PACKET_TYPE_DISCONNECT, + &disc->props, rx_payload, + (word32)(packet_end - rx_payload), props_len); + if (props_tmp < 0) + return props_tmp; rx_payload += props_tmp; - if (props_len > 0) { - /* Decode the Properties */ - props_tmp = MqttDecode_Props(MQTT_PACKET_TYPE_DISCONNECT, - &disc->props, rx_payload, - (word32)(rx_buf_len - (rx_payload - rx_buf)), - props_len); - if (props_tmp < 0) - return props_tmp; - rx_payload += props_tmp; - } } - else { - return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_OUT_OF_BUFFER); + } + /* MQTT 5.0 section 3.14.3: DISCONNECT has no payload. */ + if (rx_payload != packet_end) { + if (disc->props != NULL) { + (void)MqttProps_Free(disc->props); + disc->props = NULL; } + return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_MALFORMED_DATA); } } else { @@ -4218,11 +4229,15 @@ int MqttDecode_Auth(byte *rx_buf, int rx_buf_len, MqttAuth *auth) auth->props = NULL; if (remain_len > 0) { - /* remain_len is attacker-controlled; confirm the reason-code byte - * was received before reading past the buffer */ - if (rx_buf_len < (rx_payload - rx_buf) + 1) { + byte* packet_end; + + /* remain_len is attacker-controlled; the whole packet must be + * present before any field is bounded by its end */ + if (rx_buf_len < header_len + remain_len) { return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_OUT_OF_BUFFER); } + packet_end = &rx_buf[header_len + remain_len]; + /* Decode variable header */ auth->reason_code = *rx_payload++; if ((auth->reason_code == MQTT_REASON_SUCCESS) || @@ -4230,29 +4245,30 @@ int MqttDecode_Auth(byte *rx_buf, int rx_buf_len, MqttAuth *auth) (auth->reason_code == MQTT_REASON_REAUTH)) { /* Decode Length of Properties */ - if (rx_buf_len < (rx_payload - rx_buf)) { - return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_OUT_OF_BUFFER); - } tmp = MqttDecode_Vbi(rx_payload, &props_len, - (word32)(rx_buf_len - (rx_payload - rx_buf))); + (word32)(packet_end - rx_payload)); if (tmp < 0) return tmp; - if (props_len <= (word32)(rx_buf_len - (rx_payload - rx_buf))) { + if (props_len <= (word32)(packet_end - rx_payload - tmp)) { rx_payload += tmp; - if ((rx_payload - rx_buf) > rx_buf_len) { - return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_OUT_OF_BUFFER); - } if (props_len > 0) { /* Decode the Properties */ tmp = MqttDecode_Props(MQTT_PACKET_TYPE_AUTH, &auth->props, rx_payload, - (word32)(rx_buf_len - (rx_payload - rx_buf)), - props_len); + (word32)(packet_end - rx_payload), props_len); if (tmp < 0) return tmp; rx_payload += tmp; } + /* MQTT 5.0 section 3.15.3: AUTH has no payload. */ + if (rx_payload != packet_end) { + if (auth->props != NULL) { + (void)MqttProps_Free(auth->props); + auth->props = NULL; + } + return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_MALFORMED_DATA); + } if (auth->props == NULL) { /* An AUTH that carries a reason code must also carry properties (at minimum the Authentication Method). The diff --git a/src/mqtt_sn_client.c b/src/mqtt_sn_client.c index fee129a6d..4158e4a64 100644 --- a/src/mqtt_sn_client.c +++ b/src/mqtt_sn_client.c @@ -645,6 +645,7 @@ static int SN_Client_WaitType(MqttClient *client, void* packet_obj, int waitMatchFound; byte response_pending; void* use_packet_obj = NULL; + void* user_ctx; if (client == NULL || packet_obj == NULL) { return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_BAD_ARG); @@ -745,6 +746,16 @@ static int SN_Client_WaitType(MqttClient *client, void* packet_obj, /* Clear shared union for next call */ MqttSNClient_PacketReset(packet_type, &client->msgSN); + /* A reused caller object can hold stale union bytes, such as a + * gwAddr pointer, from a previously decoded packet type. Keep + * the caller's publish callback context across the reset. */ + if (wait_type == SN_MSG_TYPE_ANY && packet_obj != &client->msgSN) { + user_ctx = ((SN_Object*)packet_obj)->publish.ctx; + MqttSNClient_PacketReset(packet_type, packet_obj); + if (packet_type == SN_MSG_TYPE_PUBLISH) { + ((SN_Object*)packet_obj)->publish.ctx = user_ctx; + } + } #ifdef WOLFMQTT_DEBUG_CLIENT PRINTF("Read Packet: Len %d, Type %d, ID %d", diff --git a/src/mqtt_sn_packet.c b/src/mqtt_sn_packet.c index 206d5a284..baba295d7 100644 --- a/src/mqtt_sn_packet.c +++ b/src/mqtt_sn_packet.c @@ -384,9 +384,12 @@ int SN_Encode_Connect(byte *tx_buf, int tx_buf_len, SN_Connect *mc_connect) /* Determine packet length */ total_len = 6; /* Len + Message Type + Flags + ProtocolID + Duration(2) */ - /* Client ID size */ + /* Client ID size. MQTT-SN 1.2 section 5.3.1 caps the ClientId at 23 + * characters; truncating would merge distinct identities. */ id_len = XSTRLEN(mc_connect->client_id); - id_len = (id_len <= SN_CLIENTID_MAX_LEN) ? id_len : SN_CLIENTID_MAX_LEN; + if (id_len > SN_CLIENTID_MAX_LEN) { + return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_BAD_ARG); + } total_len += id_len; @@ -1676,10 +1679,11 @@ int SN_Encode_Ping(byte *tx_buf, int tx_buf_len, SN_PingReq *ping, byte type) if ((type == SN_MSG_TYPE_PING_REQ) && (ping != NULL) && (ping->clientId != NULL)) { - clientId_len = (int)XSTRLEN(ping->clientId); - if (clientId_len > SN_CLIENTID_MAX_LEN) { - clientId_len = SN_CLIENTID_MAX_LEN; + /* MQTT-SN 1.2 section 5.3.1: reject rather than truncate. */ + if (XSTRLEN(ping->clientId) > SN_CLIENTID_MAX_LEN) { + return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_BAD_ARG); } + clientId_len = (int)XSTRLEN(ping->clientId); total_len += clientId_len; } diff --git a/src/mqtt_socket.c b/src/mqtt_socket.c index 914a2dd6c..e98f80ba1 100644 --- a/src/mqtt_socket.c +++ b/src/mqtt_socket.c @@ -203,8 +203,8 @@ static int MqttSocket_WriteDo(MqttClient *client, const byte* buf, int buf_len, client->tls.sockRcWrite = 0; /* init value */ rc = wolfSSL_write(client->tls.ssl, (char*)buf, buf_len); - if (rc < 0) { - int error = wolfSSL_get_error(client->tls.ssl, 0); + if (rc <= 0) { + int error = wolfSSL_get_error(client->tls.ssl, rc); client->tls.lastError = error; #ifdef WOLFMQTT_DEBUG_SOCKET if (error != WOLFSSL_ERROR_WANT_WRITE @@ -223,7 +223,14 @@ static int MqttSocket_WriteDo(MqttClient *client, const byte* buf, int buf_len, if (error == WC_PENDING_E) { rc = MQTT_CODE_CONTINUE; } + else #endif + /* A closed or failed session must not look like a retryable + * zero-byte write. */ + if (rc >= 0 && error != WOLFSSL_ERROR_WANT_WRITE && + error != WOLFSSL_ERROR_WANT_READ) { + rc = MQTT_CODE_ERROR_NETWORK; + } } } else @@ -312,8 +319,8 @@ static int MqttSocket_ReadDo(MqttClient *client, byte* buf, int buf_len, client->tls.sockRcRead = 0; /* init value */ rc = wolfSSL_read(client->tls.ssl, (char*)buf, buf_len); - if (rc < 0) { - int error = wolfSSL_get_error(client->tls.ssl, 0); + if (rc <= 0) { + int error = wolfSSL_get_error(client->tls.ssl, rc); client->tls.lastError = error; #ifdef WOLFMQTT_DEBUG_SOCKET if (error != WOLFSSL_ERROR_WANT_READ @@ -338,6 +345,12 @@ static int MqttSocket_ReadDo(MqttClient *client, byte* buf, int buf_len, if (error == WOLFSSL_ERROR_ZERO_RETURN) { rc = MQTT_CODE_ERROR_NETWORK; } + /* A closed or failed session must not look like a retryable + * zero-byte read. */ + else if (rc >= 0 && error != WOLFSSL_ERROR_WANT_READ && + error != WOLFSSL_ERROR_WANT_WRITE) { + rc = MQTT_CODE_ERROR_NETWORK; + } } } else diff --git a/tests/test_broker_alloc.h b/tests/test_broker_alloc.h index 9bbfddabf..0a31efa12 100644 --- a/tests/test_broker_alloc.h +++ b/tests/test_broker_alloc.h @@ -13,8 +13,10 @@ void* wolfmqtt_test_broker_malloc(size_t size); void wolfmqtt_test_broker_free(void* ptr); unsigned long wolfmqtt_test_broker_time_s(void); +int wolfmqtt_test_broker_fsync(int fd); #define WOLFMQTT_MALLOC(size) wolfmqtt_test_broker_malloc((size)) #define WOLFMQTT_FREE(ptr) wolfmqtt_test_broker_free((ptr)) +#define WMQB_POSIX_FSYNC(fd) wolfmqtt_test_broker_fsync((fd)) #endif /* WOLFMQTT_TEST_BROKER_ALLOC_H */ diff --git a/tests/test_broker_connect.c b/tests/test_broker_connect.c index 37557a96f..5844231b9 100644 --- a/tests/test_broker_connect.c +++ b/tests/test_broker_connect.c @@ -42,8 +42,14 @@ #include "wolfmqtt/mqtt_packet.h" #ifdef WOLFMQTT_BROKER_PERSIST + #ifdef WOLFMQTT_BROKER_PERSIST_ENCRYPT + #include + #endif + #include #include + #include #include + #include #include #include #endif @@ -108,6 +114,23 @@ unsigned long wolfmqtt_test_broker_time_s(void) return g_broker_time_s; } +#ifdef WOLFMQTT_BROKER_PERSIST +/* When set, fsync of a directory fails with EIO so a test can exercise the + * POSIX persistence backend's durability error paths. */ +static int g_fsync_dir_fail; + +int wolfmqtt_test_broker_fsync(int fd) +{ + struct stat st; + + if (g_fsync_dir_fail && fstat(fd, &st) == 0 && S_ISDIR(st.st_mode)) { + errno = EIO; + return -1; + } + return fsync(fd); +} +#endif + #ifndef WOLFMQTT_STATIC_MEMORY static void broker_test_fail_alloc_after(int successful_allocations) { @@ -174,6 +197,7 @@ typedef struct MockClient { static MockClient g_clients[MOCK_MAX_CLIENTS]; static int g_clients_active; /* how many clients accept() will hand out */ static int g_accept_count; /* incremented per successful accept() */ +static int g_accept_fail_rc; /* non-zero: accept() returns this */ static int g_listen_count; static int g_listen_close_count; static int g_listen_fail_on_call; @@ -216,6 +240,9 @@ static int mock_accept(void* ctx, BROKER_SOCKET_T listen_sock, BROKER_SOCKET_T* client_sock) { (void)ctx; (void)listen_sock; + if (g_accept_fail_rc != 0) { + return g_accept_fail_rc; + } if (g_accept_count < g_clients_active) { *client_sock = (BROKER_SOCKET_T)(MOCK_CLIENT_SOCK_BASE + g_accept_count); @@ -345,6 +372,7 @@ static void reset_mock_clients(int n_clients) } g_clients_active = n_clients; g_accept_count = 0; + g_accept_fail_rc = 0; #ifndef WOLFMQTT_STATIC_MEMORY g_write_publish_hook = NULL; #endif @@ -678,6 +706,234 @@ TEST(connect_v5_client_id_alloc_failure_refused) #endif #endif +#if !defined(WOLFMQTT_STATIC_MEMORY) && defined(WOLFMQTT_BROKER_WILL) +/* A Will that cannot be stored in full must not be accepted in altered form: + * a missing Will Topic would drop the Will and a missing Will Payload would + * publish an empty one (MQTT 3.1.1 section 3.1.2.5). The CONNECT is refused + * with "Server unavailable" instead. fail_after selects the Will Topic or + * Will Payload copy. */ +static void connect_will_alloc_failure_refused(int fail_after) +{ + MqttBroker broker; + MqttBrokerNet net; + /* CONNECT with Will: flags 0x06 (will_flag | clean_session), ClientId + * "P", Will Topic "lwt", Will Payload "bye". */ + static const byte connect[] = { + 0x10, 0x17, + 0x00, 0x04, 'M', 'Q', 'T', 'T', + 0x04, 0x06, 0x00, 0x3C, + 0x00, 0x01, 'P', + 0x00, 0x03, 'l', 'w', 't', + 0x00, 0x03, 'b', 'y', 'e' + }; + + install_mock_net(&net); + XMEMSET(&broker, 0, sizeof(broker)); + ASSERT_EQ(MQTT_CODE_SUCCESS, MqttBroker_Init(&broker, &net)); + ASSERT_EQ(MQTT_CODE_SUCCESS, MqttBroker_Start(&broker)); + + reset_mock_state(connect, sizeof(connect)); + broker_test_fail_alloc_after(fail_after); + run_broker_one_connect(&broker); + broker_test_disable_alloc_failure(); + + ASSERT_EQ(1, g_alloc_failure_count); + ASSERT_TRUE(g_out_len >= 4); + ASSERT_EQ(0x20, g_out_buf[0]); + ASSERT_EQ(MQTT_CONNECT_ACK_CODE_REFUSED_UNAVAIL, g_out_buf[3]); + ASSERT_TRUE(g_client_closed); + + MqttBroker_Stop(&broker); + MqttBroker_Free(&broker); +} + +TEST(connect_will_topic_alloc_failure_refused) +{ + /* Client object, two I/O buffers and ClientId precede the Will Topic. */ + connect_will_alloc_failure_refused(4); +} + +TEST(connect_will_payload_alloc_failure_refused) +{ + connect_will_alloc_failure_refused(5); +} + +/* CleanSession 0 with no stored Session also creates a new Session. A + * refused CONNECT must not orphan it either, since nothing was resumed. */ +TEST(connect_v311_resume_will_alloc_failure_leaves_no_session) +{ + MqttBroker broker; + MqttBrokerNet net; + /* CONNECT with Will: flags 0x04 (will_flag, CleanSession 0), ClientId + * "P", Will Topic "lwt", Will Payload "bye". */ + static const byte connect[] = { + 0x10, 0x17, + 0x00, 0x04, 'M', 'Q', 'T', 'T', + 0x04, 0x04, 0x00, 0x3C, + 0x00, 0x01, 'P', + 0x00, 0x03, 'l', 'w', 't', + 0x00, 0x03, 'b', 'y', 'e' + }; + int orphans; + + install_mock_net(&net); + XMEMSET(&broker, 0, sizeof(broker)); + ASSERT_EQ(MQTT_CODE_SUCCESS, MqttBroker_Init(&broker, &net)); + ASSERT_EQ(MQTT_CODE_SUCCESS, MqttBroker_Start(&broker)); + + reset_mock_state(connect, sizeof(connect)); + /* Client object, two I/O buffers and ClientId precede the Will Topic. */ + broker_test_fail_alloc_after(4); + run_broker_one_connect(&broker); + broker_test_disable_alloc_failure(); + orphans = broker.orphan_session_count; + + ASSERT_EQ(1, g_alloc_failure_count); + ASSERT_TRUE(g_out_len >= 4); + ASSERT_EQ(0x20, g_out_buf[0]); + ASSERT_EQ(MQTT_CONNECT_ACK_CODE_REFUSED_UNAVAIL, g_out_buf[3]); + ASSERT_TRUE(g_client_closed); + ASSERT_EQ(0, orphans); + + MqttBroker_Stop(&broker); + MqttBroker_Free(&broker); +} + +#ifdef WOLFMQTT_V5 +/* A refused CONNECT must not leave behind a Session it created. With Clean + * Start and a non-zero Session Expiry Interval, the Session is new and empty; + * orphaning it after the refusal would hold (and at capacity evict for) a + * Session nobody can resume [MQTT-3.2.2-4]. */ +TEST(connect_v5_will_alloc_failure_leaves_no_session) +{ + MqttBroker broker; + MqttBrokerNet net; + /* v5 CONNECT: flags 0x06 (will_flag | clean_start), keep alive 60, + * properties: Session Expiry Interval (0x11) = 60, ClientId "P", Will + * Properties length 0, Will Topic "lwt", Will Payload "bye". + * Remaining Length 30. */ + static const byte connect[] = { + 0x10, 0x1E, + 0x00, 0x04, 'M', 'Q', 'T', 'T', + 0x05, 0x06, 0x00, 0x3C, + 0x05, 0x11, 0x00, 0x00, 0x00, 0x3C, + 0x00, 0x01, 'P', + 0x00, + 0x00, 0x03, 'l', 'w', 't', + 0x00, 0x03, 'b', 'y', 'e' + }; + int orphans; + + install_mock_net(&net); + XMEMSET(&broker, 0, sizeof(broker)); + ASSERT_EQ(MQTT_CODE_SUCCESS, MqttBroker_Init(&broker, &net)); + ASSERT_EQ(MQTT_CODE_SUCCESS, MqttBroker_Start(&broker)); + + reset_mock_state(connect, sizeof(connect)); + /* Client object, two I/O buffers and ClientId precede the Will Topic. */ + broker_test_fail_alloc_after(4); + run_broker_one_connect(&broker); + broker_test_disable_alloc_failure(); + orphans = broker.orphan_session_count; + + ASSERT_EQ(1, g_alloc_failure_count); + ASSERT_TRUE(g_out_len >= 4); + ASSERT_EQ(0x20, g_out_buf[0]); + ASSERT_EQ(MQTT_REASON_SERVER_UNAVAILABLE, g_out_buf[3]); + ASSERT_TRUE(g_client_closed); + ASSERT_EQ(0, orphans); + + MqttBroker_Stop(&broker); + MqttBroker_Free(&broker); +} +#endif /* WOLFMQTT_V5 */ + +static int count_packets_of_type(const byte* buf, size_t len, byte type); + +/* A CONNECT refused because its Will cannot be stored must not take over the + * connected client that holds the same ClientId: that client stays connected + * with its Session (here, its subscription) unchanged. */ +TEST(connect_will_alloc_failure_keeps_incumbent_session) +{ + MqttBroker broker; + MqttBrokerNet net; + int i; + int incumbent_closed; + int incumbent_publishes; + /* CONNECT: CleanSession 0, keep alive 60, ClientId "P". */ + static const byte incumbent_connect[] = { + 0x10, 0x0D, + 0x00, 0x04, 'M', 'Q', 'T', 'T', + 0x04, 0x00, 0x00, 0x3C, + 0x00, 0x01, 'P' + }; + /* SUBSCRIBE packet id 1, filter "t", QoS 0. */ + static const byte incumbent_subscribe[] = { + 0x82, 0x06, + 0x00, 0x01, + 0x00, 0x01, 't', + 0x00 + }; + /* CONNECT with Will: flags 0x04 (will_flag, CleanSession 0), ClientId + * "P", Will Topic "lwt", Will Payload "bye". */ + static const byte takeover_connect[] = { + 0x10, 0x17, + 0x00, 0x04, 'M', 'Q', 'T', 'T', + 0x04, 0x04, 0x00, 0x3C, + 0x00, 0x01, 'P', + 0x00, 0x03, 'l', 'w', 't', + 0x00, 0x03, 'b', 'y', 'e' + }; + /* QoS 0 PUBLISH to "t", payload "x". */ + static const byte publish[] = { + 0x30, 0x04, + 0x00, 0x01, 't', + 'x' + }; + + install_mock_net(&net); + XMEMSET(&broker, 0, sizeof(broker)); + ASSERT_EQ(MQTT_CODE_SUCCESS, MqttBroker_Init(&broker, &net)); + ASSERT_EQ(MQTT_CODE_SUCCESS, MqttBroker_Start(&broker)); + + reset_mock_clients(2); + mock_client_input_append(0, incumbent_connect, sizeof(incumbent_connect)); + mock_client_input_append(0, incumbent_subscribe, + sizeof(incumbent_subscribe)); + for (i = 0; i < 16; i++) { + MqttBroker_Step(&broker); + } + + /* Both connections are already accepted; the ClientId copy precedes the + * Will Topic. */ + mock_client_input_append(1, takeover_connect, sizeof(takeover_connect)); + broker_test_fail_alloc_after(1); + for (i = 0; i < 16; i++) { + MqttBroker_Step(&broker); + } + broker_test_disable_alloc_failure(); + + mock_client_input_append(0, publish, sizeof(publish)); + for (i = 0; i < 16; i++) { + MqttBroker_Step(&broker); + } + incumbent_closed = g_clients[0].closed; + incumbent_publishes = count_packets_of_type(g_clients[0].out_buf, + g_clients[0].out_len, MQTT_PACKET_TYPE_PUBLISH); + + MqttBroker_Stop(&broker); + MqttBroker_Free(&broker); + + ASSERT_EQ(1, g_alloc_failure_count); + ASSERT_TRUE(g_clients[1].out_len >= 4); + ASSERT_EQ(0x20, g_clients[1].out_buf[0]); + ASSERT_EQ(MQTT_CONNECT_ACK_CODE_REFUSED_UNAVAIL, g_clients[1].out_buf[3]); + ASSERT_TRUE(g_clients[1].closed); + ASSERT_FALSE(incumbent_closed); + ASSERT_EQ(1, incumbent_publishes); +} +#endif /* !WOLFMQTT_STATIC_MEMORY && WOLFMQTT_BROKER_WILL */ + #if defined(WOLFMQTT_V5) && defined(WOLFMQTT_BROKER_WILL) TEST(connect_v5_oversize_will_payload_emits_connack) { @@ -3733,6 +3989,52 @@ TEST(puback_write_failure_fires_will) #endif #endif /* WOLFMQTT_BROKER_WILL */ +/* A listener whose accept callback reports a hard failure must surface it + * from MqttBroker_Step, so MqttBroker_Run stops instead of spinning forever on + * a dead listener. Idle results (CONTINUE/TIMEOUT) stay silent, and existing + * clients are still serviced in the step that reports the failure. */ +TEST(broker_step_reports_listener_accept_failure) +{ + MqttBroker broker; + MqttBrokerNet net; + int rc; + static const byte connect[] = { + 0x10, 14, + 0x00, 0x04, 'M', 'Q', 'T', 'T', + 0x04, 0x02, 0x00, 0x3C, + 0x00, 0x02, 'i', 'd' + }; + static const byte pingreq[] = { 0xC0, 0x00 }; + size_t out_mark; + + install_mock_net(&net); + XMEMSET(&broker, 0, sizeof(broker)); + ASSERT_EQ(MQTT_CODE_SUCCESS, MqttBroker_Init(&broker, &net)); + ASSERT_EQ(MQTT_CODE_SUCCESS, MqttBroker_Start(&broker)); + + reset_mock_state(connect, sizeof(connect)); + run_broker_one_connect(&broker); + ASSERT_FALSE(g_client_closed); + + /* Idle listener: nothing to report. */ + rc = MqttBroker_Step(&broker); + ASSERT_TRUE(rc >= 0 || rc == MQTT_CODE_CONTINUE); + + out_mark = g_out_len; + mock_client_input_append(0, pingreq, sizeof(pingreq)); + g_accept_fail_rc = MQTT_CODE_ERROR_NETWORK; + rc = MqttBroker_Step(&broker); + g_accept_fail_rc = 0; + + ASSERT_EQ(MQTT_CODE_ERROR_NETWORK, rc); + /* The connected client was still serviced: PINGRESP went out. */ + ASSERT_EQ(1, count_packets_of_type(g_out_buf + out_mark, + g_out_len - out_mark, MQTT_PACKET_TYPE_PING_RESP)); + + MqttBroker_Stop(&broker); + MqttBroker_Free(&broker); +} + TEST(broker_keepalive_expires_at_exact_deadline) { MqttBroker broker; @@ -3776,7 +4078,7 @@ TEST(broker_keepalive_expires_at_exact_deadline) * delivery. v5 has its own decoder that legitimately accepts Reason Code * + Properties, so the broker's remain_len check (and this test) is * v3.1.1-only. */ -#ifndef WOLFMQTT_V5 +#if !defined(WOLFMQTT_V5) && defined(WOLFMQTT_BROKER_WILL) TEST(disconnect_v311_nonzero_remain_len_fires_will) { MqttBroker broker; @@ -3834,7 +4136,7 @@ TEST(disconnect_v311_nonzero_remain_len_fires_will) MqttBroker_Stop(&broker); MqttBroker_Free(&broker); } -#endif /* !WOLFMQTT_V5 */ +#endif /* !WOLFMQTT_V5 && WOLFMQTT_BROKER_WILL */ /* The broker switch's default branch must close the connection on any * unhandled packet type rather than silently no-op'ing. Wire is an @@ -4894,6 +5196,7 @@ TEST(broker_subscribe_packet_id_zero_closes) * handlers, so a malformed DISCONNECT (e.g. 0xE1) takes the abnormal- * close path. Same LWT observable as the nonzero-remain-len test: * abnormal close fires the will, normal close clears it. */ +#ifdef WOLFMQTT_BROKER_WILL TEST(disconnect_invalid_fixed_header_flags_fires_will) { MqttBroker broker; @@ -4947,64 +5250,249 @@ TEST(disconnect_invalid_fixed_header_flags_fires_will) MqttBroker_Stop(&broker); MqttBroker_Free(&broker); } +#endif /* WOLFMQTT_BROKER_WILL */ -/* Parsed details of a single PUBLISH packet on the wire. found = 0 means - * no PUBLISH was present in the buffer; subsequent fields are zero. */ -typedef struct { - int found; - byte first_byte; /* type | DUP | QoS | RETAIN */ - size_t remain_len; /* fixed-header Remaining Length value */ - word16 packet_id; /* 0 if QoS 0 (no packet identifier on wire) */ -} PublishInfo; - -/* Find the first PUBLISH packet (high nibble = 0x3) in `buf` and return - * its first byte plus parsed remaining-length and packet identifier. - * Mirrors count_packets_of_type's VBI walk so any malformed input stops - * cleanly instead of overrunning. */ -static PublishInfo first_publish_info(const byte* buf, size_t len) +#if defined(WOLFMQTT_V5) && defined(WOLFMQTT_BROKER_WILL) +/* A v5 DISCONNECT that fails to decode is a Malformed Packet, not a normal + * disconnect: the Server closes the Network Connection and the Will Message + * is published (MQTT 5.0 sections 3.1.2.5 and 4.13). Only a successfully + * decoded DISCONNECT with Reason Code 0x00 discards the Will [MQTT-3.14.4-3]. */ +TEST(disconnect_v5_malformed_fires_will) { - PublishInfo info; - size_t pos = 0; - XMEMSET(&info, 0, sizeof(info)); - while (pos < len) { - byte type = (byte)((buf[pos] >> 4) & 0x0F); - size_t remain = 0; - size_t mult = 1; - size_t hdr_len = 1; - int vbi_complete = 0; - while (pos + hdr_len < len && hdr_len <= 5) { - byte b = buf[pos + hdr_len]; - remain += (size_t)(b & 0x7F) * mult; - hdr_len++; - if ((b & 0x80) == 0) { - vbi_complete = 1; - break; - } - mult *= 128; - } - if (!vbi_complete) { - break; - } - if (type == MQTT_PACKET_TYPE_PUBLISH) { - byte qos = (byte)((buf[pos] >> 1) & 0x03); - info.found = 1; - info.first_byte = buf[pos]; - info.remain_len = remain; - if (qos >= MQTT_QOS_1) { - /* Skip the 2-byte topic length + topic to reach the - * 2-byte packet identifier that follows for QoS >= 1. */ - size_t var_off = pos + hdr_len; - if (var_off + 2 <= len) { - word16 topic_len = - (word16)((buf[var_off] << 8) | buf[var_off + 1]); - size_t pid_off = var_off + 2 + topic_len; - if (pid_off + 2 <= len) { - info.packet_id = (word16)( - (buf[pid_off] << 8) | buf[pid_off + 1]); - } - } - } - return info; + MqttBroker broker; + MqttBrokerNet net; + int i; + static const byte sub_connect[] = { + 0x10, 0x0D, + 0x00, 0x04, 'M', 'Q', 'T', 'T', + 0x04, 0x02, 0x00, 0x3C, + 0x00, 0x01, 'S' + }; + static const byte sub_subscribe[] = { + 0x82, 0x08, + 0x00, 0x01, + 0x00, 0x03, 'l', 'w', 't', + 0x00 + }; + /* v5 CONNECT with Will: flags 0x06 (will_flag | clean_start), keep + * alive 60, Properties length 0, ClientId "P", Will Properties length 0, + * Will Topic "lwt", Will Payload "bye". Remaining Length 25. */ + static const byte pub_connect[] = { + 0x10, 0x19, + 0x00, 0x04, 'M', 'Q', 'T', 'T', + 0x05, 0x06, 0x00, 0x3C, + 0x00, + 0x00, 0x01, 'P', + 0x00, + 0x00, 0x03, 'l', 'w', 't', + 0x00, 0x03, 'b', 'y', 'e' + }; + /* v5 DISCONNECT, Reason Code 0x00, Property Length 5 with no property + * bytes following: the declared Properties overrun the packet. */ + static const byte disconnect_bad[] = { 0xE0, 0x02, 0x00, 0x05 }; + + install_mock_net(&net); + XMEMSET(&broker, 0, sizeof(broker)); + ASSERT_EQ(MQTT_CODE_SUCCESS, MqttBroker_Init(&broker, &net)); + ASSERT_EQ(MQTT_CODE_SUCCESS, MqttBroker_Start(&broker)); + + reset_mock_clients(2); + mock_client_input_append(0, sub_connect, sizeof(sub_connect)); + mock_client_input_append(0, sub_subscribe, sizeof(sub_subscribe)); + mock_client_input_append(1, pub_connect, sizeof(pub_connect)); + mock_client_input_append(1, disconnect_bad, sizeof(disconnect_bad)); + for (i = 0; i < 16; i++) { + MqttBroker_Step(&broker); + } + + ASSERT_EQ(1, count_packets_of_type(g_clients[0].out_buf, + g_clients[0].out_len, MQTT_PACKET_TYPE_PUBLISH)); + ASSERT_TRUE(g_clients[1].closed); + + MqttBroker_Stop(&broker); + MqttBroker_Free(&broker); +} + +#ifndef WOLFMQTT_DYN_PROP +/* Connect a v5 publisher with a Will and a v3.1.1 subscriber to its Will + * topic, leave the shared property pool one slot short of the publisher's + * DISCONNECT Properties, then send that DISCONNECT with reason_code. Returns + * the number of PUBLISH packets the subscriber received. */ +static int disconnect_v5_props_pool_exhausted_wills(byte reason_code, + int* closed) +{ + MqttBroker broker; + MqttBrokerNet net; + MqttProp* hold = NULL; + int free_slots = 0; + int i; + int publishes; + static const byte sub_connect[] = { + 0x10, 0x0D, + 0x00, 0x04, 'M', 'Q', 'T', 'T', + 0x04, 0x02, 0x00, 0x3C, + 0x00, 0x01, 'S' + }; + static const byte sub_subscribe[] = { + 0x82, 0x08, + 0x00, 0x01, + 0x00, 0x03, 'l', 'w', 't', + 0x00 + }; + /* Same v5 CONNECT with Will as disconnect_v5_malformed_fires_will. */ + static const byte pub_connect[] = { + 0x10, 0x19, + 0x00, 0x04, 'M', 'Q', 'T', 'T', + 0x05, 0x06, 0x00, 0x3C, + 0x00, + 0x00, 0x01, 'P', + 0x00, + 0x00, 0x03, 'l', 'w', 't', + 0x00, 0x03, 'b', 'y', 'e' + }; + /* v5 DISCONNECT (MQTT 5.0 section 3.14): Remaining Length 16, Reason + * Code, Property Length 14, then two User Properties (identifier 0x26, + * UTF-8 String Pair "k" / "v"). */ + byte disconnect[] = { + 0xE0, 0x10, + 0x00, + 0x0E, + 0x26, 0x00, 0x01, 'k', 0x00, 0x01, 'v', + 0x26, 0x00, 0x01, 'k', 0x00, 0x01, 'v' + }; + disconnect[2] = reason_code; + + install_mock_net(&net); + XMEMSET(&broker, 0, sizeof(broker)); + if (MqttBroker_Init(&broker, &net) != MQTT_CODE_SUCCESS || + MqttBroker_Start(&broker) != MQTT_CODE_SUCCESS) { + return -1; + } + + reset_mock_clients(2); + mock_client_input_append(0, sub_connect, sizeof(sub_connect)); + mock_client_input_append(0, sub_subscribe, sizeof(sub_subscribe)); + mock_client_input_append(1, pub_connect, sizeof(pub_connect)); + for (i = 0; i < 16; i++) { + MqttBroker_Step(&broker); + } + + /* Hold a property-stack reference of our own (see + * connect_v5_refused_when_shared_sub_property_unavailable), then leave + * a single free slot: the DISCONNECT is well formed, but its second + * User Property cannot be allocated. */ + publishes = -1; + if (MqttProps_Init() == MQTT_CODE_SUCCESS) { + while (MqttProps_Add(&hold) != NULL) { + free_slots++; + } + (void)MqttProps_Free(hold); + hold = NULL; + for (i = 0; i < free_slots - 1; i++) { + (void)MqttProps_Add(&hold); + } + + mock_client_input_append(1, disconnect, sizeof(disconnect)); + for (i = 0; i < 16; i++) { + MqttBroker_Step(&broker); + } + (void)MqttProps_Free(hold); + (void)MqttProps_ShutDown(); + + if (free_slots >= 2) { + publishes = count_packets_of_type(g_clients[0].out_buf, + g_clients[0].out_len, MQTT_PACKET_TYPE_PUBLISH); + } + } + *closed = g_clients[1].closed; + + MqttBroker_Stop(&broker); + MqttBroker_Free(&broker); + return publishes; +} + +/* A well-formed v5 DISCONNECT whose Properties cannot be allocated is not a + * Malformed Packet. Reason Code 0x00 is a normal disconnection, so the Will + * is discarded [MQTT-3.14.4-3]. */ +TEST(disconnect_v5_props_pool_exhausted_reason_0_discards_will) +{ + int closed = 0; + + ASSERT_EQ(0, disconnect_v5_props_pool_exhausted_wills( + MQTT_REASON_SUCCESS, &closed)); + ASSERT_TRUE(closed); +} + +/* Reason Code 0x04 still publishes the Will when the DISCONNECT Properties + * cannot be allocated [MQTT-3.14.4-3]. */ +TEST(disconnect_v5_props_pool_exhausted_reason_4_publishes_will) +{ + int closed = 0; + + ASSERT_EQ(1, disconnect_v5_props_pool_exhausted_wills( + MQTT_REASON_DISCONNECT_W_WILL_MSG, &closed)); + ASSERT_TRUE(closed); +} +#endif /* !WOLFMQTT_DYN_PROP */ +#endif /* WOLFMQTT_V5 && WOLFMQTT_BROKER_WILL */ + +/* Parsed details of a single PUBLISH packet on the wire. found = 0 means + * no PUBLISH was present in the buffer; subsequent fields are zero. */ +typedef struct { + int found; + byte first_byte; /* type | DUP | QoS | RETAIN */ + size_t remain_len; /* fixed-header Remaining Length value */ + word16 packet_id; /* 0 if QoS 0 (no packet identifier on wire) */ +} PublishInfo; + +/* Find the first PUBLISH packet (high nibble = 0x3) in `buf` and return + * its first byte plus parsed remaining-length and packet identifier. + * Mirrors count_packets_of_type's VBI walk so any malformed input stops + * cleanly instead of overrunning. */ +static PublishInfo first_publish_info(const byte* buf, size_t len) +{ + PublishInfo info; + size_t pos = 0; + XMEMSET(&info, 0, sizeof(info)); + while (pos < len) { + byte type = (byte)((buf[pos] >> 4) & 0x0F); + size_t remain = 0; + size_t mult = 1; + size_t hdr_len = 1; + int vbi_complete = 0; + while (pos + hdr_len < len && hdr_len <= 5) { + byte b = buf[pos + hdr_len]; + remain += (size_t)(b & 0x7F) * mult; + hdr_len++; + if ((b & 0x80) == 0) { + vbi_complete = 1; + break; + } + mult *= 128; + } + if (!vbi_complete) { + break; + } + if (type == MQTT_PACKET_TYPE_PUBLISH) { + byte qos = (byte)((buf[pos] >> 1) & 0x03); + info.found = 1; + info.first_byte = buf[pos]; + info.remain_len = remain; + if (qos >= MQTT_QOS_1) { + /* Skip the 2-byte topic length + topic to reach the + * 2-byte packet identifier that follows for QoS >= 1. */ + size_t var_off = pos + hdr_len; + if (var_off + 2 <= len) { + word16 topic_len = + (word16)((buf[var_off] << 8) | buf[var_off + 1]); + size_t pid_off = var_off + 2 + topic_len; + if (pid_off + 2 <= len) { + info.packet_id = (word16)( + (buf[pid_off] << 8) | buf[pid_off + 1]); + } + } + } + return info; } if (remain > len - pos - hdr_len) { break; @@ -5045,6 +5533,58 @@ static BrokerClient* find_static_broker_client(MqttBroker* broker, return NULL; } +#ifdef WOLFMQTT_V5 +/* A CONNECT refused because the static persistent-Session quota is full gets a + * CONNACK with Reason Code 0x97 and no properties: in particular no Assigned + * Client Identifier the refused client could never use (MQTT 5.0 section + * 3.2.2.3.7 applies to an accepted connection). */ +TEST(static_quota_refusal_connack_has_no_properties) +{ + MqttBroker broker; + MqttBrokerNet net; + int i; + /* v5 CONNECT: Clean Start, keep alive 60, properties: Session Expiry + * Interval (0x11) = 60, empty ClientId. Remaining Length 18. */ + static const byte connect[] = { + 0x10, 0x12, + 0x00, 0x04, 'M', 'Q', 'T', 'T', + 0x05, 0x02, 0x00, 0x3C, + 0x05, 0x11, 0x00, 0x00, 0x00, 0x3C, + 0x00, 0x00 + }; + + install_mock_net(&net); + XMEMSET(&broker, 0, sizeof(broker)); + ASSERT_EQ(MQTT_CODE_SUCCESS, MqttBroker_Init(&broker, &net)); + ASSERT_EQ(MQTT_CODE_SUCCESS, MqttBroker_Start(&broker)); + + /* Every persistent-Session slot holds an unexpired Session. */ + for (i = 0; i < BROKER_MAX_STATIC_ORPHAN_SESSIONS; i++) { + broker.static_orphans[i].in_use = 1; + XSNPRINTF(broker.static_orphans[i].client_id, + sizeof(broker.static_orphans[i].client_id), "q%d", i); + broker.static_orphans[i].protocol_level = + MQTT_CONNECT_PROTOCOL_LEVEL_5; + broker.static_orphans[i].session_expiry_sec = 3600; + broker.static_orphans[i].orphan_since = g_broker_time_s; + } + + reset_mock_state(connect, sizeof(connect)); + run_broker_one_connect(&broker); + + /* CONNACK: Remaining Length 3 = flags, Reason Code, Property Length 0. */ + ASSERT_TRUE(g_out_len >= 5); + ASSERT_EQ(0x20, g_out_buf[0]); + ASSERT_EQ(0x03, g_out_buf[1]); + ASSERT_EQ(MQTT_REASON_QUOTA_EXCEEDED, g_out_buf[3]); + ASSERT_EQ(0x00, g_out_buf[4]); + ASSERT_TRUE(g_client_closed); + + MqttBroker_Stop(&broker); + MqttBroker_Free(&broker); +} +#endif /* WOLFMQTT_V5 */ + /* [MQTT-2.3.1-4] holds a Server sending a QoS > 0 PUBLISH to the same * identifier rule as a Client: the value stays in use until the matching * PUBACK is processed [MQTT-2.3.1-3]. The static-memory fan-out has no @@ -5379,64 +5919,597 @@ TEST(static_fanout_releases_packet_id_on_v5_pubrec_reject) static const byte publish_x[] = { 0x34, 0x07, 0x00, 0x01, 'x', - 0x00, 0x09, + 0x00, 0x09, + 0x00, + 'A' + }; + + sub_bc = static_setup_pub_sub(&broker, &net, + MQTT_CONNECT_PROTOCOL_LEVEL_5, 0x02); + ASSERT_NOT_NULL(sub_bc); + + before = g_clients[1].out_len; + mock_client_input_append(0, publish_x, sizeof(publish_x)); + for (i = 0; i < 8; i++) { + (void)MqttBroker_Step(&broker); + } + info = first_publish_info(g_clients[1].out_buf + before, + g_clients[1].out_len - before); + ASSERT_TRUE(info.found); + id = info.packet_id; + ASSERT_TRUE(id != 0); + ASSERT_TRUE(static_out_id_in_use(sub_bc, id)); + + /* v5 PUBREC with reason 0x87 (Not authorized). */ + pubrec[0] = 0x50; + pubrec[1] = 0x03; + pubrec[2] = (byte)(id >> 8); + pubrec[3] = (byte)(id & 0xFF); + pubrec[4] = 0x87; + mock_client_input_append(1, pubrec, 5); + for (i = 0; i < 8; i++) { + (void)MqttBroker_Step(&broker); + } + + ASSERT_FALSE(static_out_id_in_use(sub_bc, id)); + + MqttBroker_Stop(&broker); + MqttBroker_Free(&broker); +} +#endif /* WOLFMQTT_V5 && WOLFMQTT_MAX_QOS >= 2 */ + +#endif /* WOLFMQTT_STATIC_MEMORY */ + +#ifndef WOLFMQTT_STATIC_MEMORY +/* Find the orphan session carrying a disconnected client's Session state. The + * BrokerClient itself is freed by the close, so anything a test wants to check + * about the preserved queue must be read from here. */ +static BrokerOrphanSession* find_orphan_session(MqttBroker* broker, + const char* id) +{ + BrokerOrphanSession* o = broker->orphan_sessions; + + while (o != NULL) { + if (o->client_id != NULL && XSTRCMP(o->client_id, id) == 0) { + return o; + } + o = o->next; + } + return NULL; +} +#endif /* !WOLFMQTT_STATIC_MEMORY */ + +#ifndef WOLFMQTT_STATIC_MEMORY +/* When the broker cannot queue a QoS 1/2 delivery for a matching subscriber, + * it must not acknowledge the PUBLISH as successful (MQTT 3.1.1 section 4.3.2: + * PUBACK transfers ownership of the message). v3.1.1 has no failure ack, so + * the publisher is disconnected without a PUBACK. */ +TEST(publish_fanout_alloc_failure_v311_not_acked) +{ + MqttBroker broker; + MqttBrokerNet net; + BrokerClient* sub_bc; + size_t pub_mark; + size_t sub_mark; + int i; + static const byte connect_pub[] = { + 0x10, 0x0D, + 0x00, 0x04, 'M', 'Q', 'T', 'T', + 0x04, 0x02, 0x00, 0x3C, + 0x00, 0x01, 'P' + }; + static const byte connect_sub[] = { + 0x10, 0x0D, + 0x00, 0x04, 'M', 'Q', 'T', 'T', + 0x04, 0x02, 0x00, 0x3C, + 0x00, 0x01, 'S' + }; + /* SUBSCRIBE packet_id 1, filter "x", QoS 1. */ + static const byte subscribe_x[] = { + 0x82, 0x06, + 0x00, 0x01, + 0x00, 0x01, 'x', + 0x01 + }; + /* PUBLISH QoS 1, topic "x", packet_id 7, payload "p". */ + static const byte publish_x[] = { + 0x32, 0x06, + 0x00, 0x01, 'x', + 0x00, 0x07, + 'p' + }; + + install_mock_net(&net); + XMEMSET(&broker, 0, sizeof(broker)); + ASSERT_EQ(MQTT_CODE_SUCCESS, MqttBroker_Init(&broker, &net)); + ASSERT_EQ(MQTT_CODE_SUCCESS, MqttBroker_Start(&broker)); + + reset_mock_clients(3); + g_clients_active = 2; + mock_client_input_append(0, connect_pub, sizeof(connect_pub)); + mock_client_input_append(1, connect_sub, sizeof(connect_sub)); + mock_client_input_append(1, subscribe_x, sizeof(subscribe_x)); + for (i = 0; i < 16; i++) { + (void)MqttBroker_Step(&broker); + } + sub_bc = find_broker_client(&broker, "S"); + ASSERT_NOT_NULL(sub_bc); + pub_mark = g_clients[0].out_len; + sub_mark = g_clients[1].out_len; + + /* The topic copy succeeds; the subscriber's queued copy does not. */ + mock_client_input_append(0, publish_x, sizeof(publish_x)); + broker_test_fail_alloc_after(1); + for (i = 0; i < 8; i++) { + (void)MqttBroker_Step(&broker); + } + broker_test_disable_alloc_failure(); + + ASSERT_EQ(1, g_alloc_failure_count); + ASSERT_EQ((int)sub_mark, (int)g_clients[1].out_len); + ASSERT_EQ(0, count_packets_of_type(g_clients[0].out_buf + pub_mark, + g_clients[0].out_len - pub_mark, MQTT_PACKET_TYPE_PUBLISH_ACK)); + ASSERT_TRUE(g_clients[0].closed); + + MqttBroker_Stop(&broker); + MqttBroker_Free(&broker); +} + +/* The same holds when the subscriber is offline: its QoS 1/2 copy is queued + * on the orphaned Session, and failing to allocate that copy must not be + * acknowledged as a successful publish either. */ +TEST(publish_offline_fanout_alloc_failure_v311_not_acked) +{ + MqttBroker broker; + MqttBrokerNet net; + size_t pub_mark; + int i; + /* Subscriber "S" with CleanSession 0 so its Session outlives it. */ + static const byte connect_sub[] = { + 0x10, 0x0D, + 0x00, 0x04, 'M', 'Q', 'T', 'T', + 0x04, 0x00, 0x00, 0x3C, + 0x00, 0x01, 'S' + }; + /* SUBSCRIBE packet_id 1, filter "x", QoS 1. */ + static const byte subscribe_x[] = { + 0x82, 0x06, + 0x00, 0x01, + 0x00, 0x01, 'x', + 0x01 + }; + static const byte disconnect[] = { 0xE0, 0x00 }; + static const byte connect_pub[] = { + 0x10, 0x0D, + 0x00, 0x04, 'M', 'Q', 'T', 'T', + 0x04, 0x02, 0x00, 0x3C, + 0x00, 0x01, 'P' + }; + /* PUBLISH QoS 1, topic "x", packet_id 7, payload "p". */ + static const byte publish_x[] = { + 0x32, 0x06, + 0x00, 0x01, 'x', + 0x00, 0x07, + 'p' + }; + + install_mock_net(&net); + XMEMSET(&broker, 0, sizeof(broker)); + ASSERT_EQ(MQTT_CODE_SUCCESS, MqttBroker_Init(&broker, &net)); + ASSERT_EQ(MQTT_CODE_SUCCESS, MqttBroker_Start(&broker)); + + reset_mock_clients(2); + mock_client_input_append(0, connect_sub, sizeof(connect_sub)); + mock_client_input_append(0, subscribe_x, sizeof(subscribe_x)); + mock_client_input_append(0, disconnect, sizeof(disconnect)); + for (i = 0; i < 16; i++) { + (void)MqttBroker_Step(&broker); + if (g_clients[0].closed) { + break; + } + } + ASSERT_TRUE(g_clients[0].closed); + ASSERT_EQ(1, broker.orphan_session_count); + + mock_client_input_append(1, connect_pub, sizeof(connect_pub)); + for (i = 0; i < 8; i++) { + (void)MqttBroker_Step(&broker); + } + pub_mark = g_clients[1].out_len; + + /* The topic copy succeeds; the offline subscriber's queued copy does + * not. */ + mock_client_input_append(1, publish_x, sizeof(publish_x)); + broker_test_fail_alloc_after(1); + for (i = 0; i < 8; i++) { + (void)MqttBroker_Step(&broker); + } + broker_test_disable_alloc_failure(); + + ASSERT_EQ(1, g_alloc_failure_count); + ASSERT_EQ(0, count_packets_of_type(g_clients[1].out_buf + pub_mark, + g_clients[1].out_len - pub_mark, MQTT_PACKET_TYPE_PUBLISH_ACK)); + ASSERT_TRUE(g_clients[1].closed); + + MqttBroker_Stop(&broker); + MqttBroker_Free(&broker); +} + +#ifdef WOLFMQTT_V5 +/* A v5 publisher is told with a failure Reason Code instead (MQTT 5.0 + * section 3.5.2.1). A PUBREC with a Reason Code of 0x80 or greater ends the + * QoS 2 exchange (section 4.3.3), so the Packet Identifier must not stay + * marked as awaiting PUBREL: a later PUBLISH reusing it is a new message. */ +TEST(publish_fanout_alloc_failure_v5_qos2_pubrec_failure) +{ + MqttBroker broker; + MqttBrokerNet net; + BrokerClient* sub_bc; + size_t pub_mark; + size_t sub_mark; + int i; + /* v5 CONNECT, clean=1, props_len=0, ClientId "P". Remaining Length 14. */ + static const byte connect_pub[] = { + 0x10, 0x0E, + 0x00, 0x04, 'M', 'Q', 'T', 'T', + 0x05, 0x02, 0x00, 0x3C, + 0x00, + 0x00, 0x01, 'P' + }; + static const byte connect_sub[] = { + 0x10, 0x0D, + 0x00, 0x04, 'M', 'Q', 'T', 'T', + 0x04, 0x02, 0x00, 0x3C, + 0x00, 0x01, 'S' + }; + /* SUBSCRIBE packet_id 1, filter "x", QoS 2. */ + static const byte subscribe_x[] = { + 0x82, 0x06, + 0x00, 0x01, + 0x00, 0x01, 'x', + 0x02 + }; + /* v5 PUBLISH QoS 2, topic "x", packet_id 7, props_len 0, payload "p". + * Remaining Length 7. */ + static const byte publish_x[] = { + 0x34, 0x07, + 0x00, 0x01, 'x', + 0x00, 0x07, + 0x00, + 'p' + }; + + install_mock_net(&net); + XMEMSET(&broker, 0, sizeof(broker)); + ASSERT_EQ(MQTT_CODE_SUCCESS, MqttBroker_Init(&broker, &net)); + ASSERT_EQ(MQTT_CODE_SUCCESS, MqttBroker_Start(&broker)); + + reset_mock_clients(3); + g_clients_active = 2; + mock_client_input_append(0, connect_pub, sizeof(connect_pub)); + mock_client_input_append(1, connect_sub, sizeof(connect_sub)); + mock_client_input_append(1, subscribe_x, sizeof(subscribe_x)); + for (i = 0; i < 16; i++) { + (void)MqttBroker_Step(&broker); + } + sub_bc = find_broker_client(&broker, "S"); + ASSERT_NOT_NULL(sub_bc); + pub_mark = g_clients[0].out_len; + sub_mark = g_clients[1].out_len; + + /* The v5 PUBLISH makes one more allocation than v3.1.1 before the + * topic copy; fail the subscriber's queued copy after both. */ + mock_client_input_append(0, publish_x, sizeof(publish_x)); + broker_test_fail_alloc_after(2); + for (i = 0; i < 8; i++) { + (void)MqttBroker_Step(&broker); + } + broker_test_disable_alloc_failure(); + + ASSERT_EQ(1, g_alloc_failure_count); + ASSERT_EQ((int)sub_mark, (int)g_clients[1].out_len); + ASSERT_FALSE(g_clients[0].closed); + /* PUBREC: type 5, Remaining Length >= 3, packet_id 7, Reason 0x80. */ + ASSERT_TRUE(g_clients[0].out_len >= pub_mark + 5); + ASSERT_EQ(0x50, g_clients[0].out_buf[pub_mark]); + ASSERT_EQ(0x07, g_clients[0].out_buf[pub_mark + 3]); + ASSERT_EQ(MQTT_REASON_UNSPECIFIED_ERR, g_clients[0].out_buf[pub_mark + 4]); + + /* Reusing packet_id 7 delivers a new message rather than being treated + * as a retransmission of the refused one. */ + mock_client_input_append(0, publish_x, sizeof(publish_x)); + for (i = 0; i < 8; i++) { + (void)MqttBroker_Step(&broker); + } + ASSERT_EQ(1, count_packets_of_type(g_clients[1].out_buf + sub_mark, + g_clients[1].out_len - sub_mark, MQTT_PACKET_TYPE_PUBLISH)); + + MqttBroker_Stop(&broker); + MqttBroker_Free(&broker); +} +#endif /* WOLFMQTT_V5 */ +#endif /* !WOLFMQTT_STATIC_MEMORY */ + +#ifndef WOLFMQTT_STATIC_MEMORY +/* A PUBACK belongs only to a QoS 1 exchange (MQTT 3.1.1 section 4.3.2). A + * subscriber that answers a QoS 2 delivery with PUBACK must not complete it: + * the entry keeps its packet identifier and inflight slot until the real + * PUBREC/PUBCOMP exchange (section 4.3.3) finishes. */ +TEST(outbound_qos2_puback_does_not_complete_delivery) +{ + MqttBroker broker; + MqttBrokerNet net; + BrokerClient* sub_bc; + BrokerOutPub* e; + word16 pid; + int inflight; + int i; + byte ack[4]; + static const byte connect_pub[] = { + 0x10, 0x0D, + 0x00, 0x04, 'M', 'Q', 'T', 'T', + 0x04, 0x02, 0x00, 0x3C, + 0x00, 0x01, 'P' + }; + static const byte connect_sub[] = { + 0x10, 0x0D, + 0x00, 0x04, 'M', 'Q', 'T', 'T', + 0x04, 0x02, 0x00, 0x3C, + 0x00, 0x01, 'S' + }; + /* SUBSCRIBE packet_id 1, filter "x", QoS 2. */ + static const byte subscribe_x[] = { + 0x82, 0x06, + 0x00, 0x01, + 0x00, 0x01, 'x', + 0x02 + }; + /* PUBLISH QoS 2, topic "x", packet_id 7, payload "p". */ + static const byte publish_x[] = { + 0x34, 0x06, + 0x00, 0x01, 'x', + 0x00, 0x07, + 'p' + }; + + install_mock_net(&net); + XMEMSET(&broker, 0, sizeof(broker)); + ASSERT_EQ(MQTT_CODE_SUCCESS, MqttBroker_Init(&broker, &net)); + ASSERT_EQ(MQTT_CODE_SUCCESS, MqttBroker_Start(&broker)); + + reset_mock_clients(3); + g_clients_active = 2; + mock_client_input_append(0, connect_pub, sizeof(connect_pub)); + mock_client_input_append(1, connect_sub, sizeof(connect_sub)); + mock_client_input_append(1, subscribe_x, sizeof(subscribe_x)); + for (i = 0; i < 16; i++) { + (void)MqttBroker_Step(&broker); + } + sub_bc = find_broker_client(&broker, "S"); + ASSERT_NOT_NULL(sub_bc); + + mock_client_input_append(0, publish_x, sizeof(publish_x)); + for (i = 0; i < 8; i++) { + (void)MqttBroker_Step(&broker); + } + e = sub_bc->out_q_head; + ASSERT_NOT_NULL(e); + ASSERT_EQ(MQTT_QOS_2, e->qos); + ASSERT_EQ(BROKER_OUTQ_PUBLISH_SENT, e->state); + pid = e->packet_id; + inflight = sub_bc->out_q_inflight; + + /* PUBACK: type 4, remain 2, packet_id of the QoS 2 delivery. */ + ack[0] = 0x40; + ack[1] = 0x02; + ack[2] = (byte)(pid >> 8); + ack[3] = (byte)(pid & 0xFF); + mock_client_input_append(1, ack, sizeof(ack)); + for (i = 0; i < 8; i++) { + (void)MqttBroker_Step(&broker); + } + ASSERT_TRUE(sub_bc->out_q_head == e); + ASSERT_EQ(pid, e->packet_id); + ASSERT_EQ(BROKER_OUTQ_PUBLISH_SENT, e->state); + ASSERT_EQ(inflight, sub_bc->out_q_inflight); + + /* The real PUBREC still drives the exchange forward with a PUBREL. */ + ack[0] = 0x50; + mock_client_input_append(1, ack, sizeof(ack)); + for (i = 0; i < 8; i++) { + (void)MqttBroker_Step(&broker); + } + ASSERT_EQ(BROKER_OUTQ_PUBREL_SENT, e->state); + ASSERT_EQ(1, count_packets_of_type(g_clients[1].out_buf, + g_clients[1].out_len, MQTT_PACKET_TYPE_PUBLISH_REL)); + + MqttBroker_Stop(&broker); + MqttBroker_Free(&broker); +} +#endif /* !WOLFMQTT_STATIC_MEMORY */ + +#ifndef WOLFMQTT_STATIC_MEMORY +/* MQTT 3.1.1 section 2.2.3: Remaining Length counts every byte that follows + * in the packet. When a queued delivery does not fit the subscriber's + * transmit buffer, the encoder copies only part of the payload while the + * fixed header still declares all of it. The drain must drop that entry + * rather than send a short frame that desynchronizes the subscriber. */ +TEST(outbound_publish_exceeding_tx_buf_not_sent_truncated) +{ + MqttBroker broker; + MqttBrokerNet net; + BrokerClient* sub_bc; + size_t sub_out_mark; + int i; + static const byte connect_pub[] = { + 0x10, 0x0D, + 0x00, 0x04, 'M', 'Q', 'T', 'T', + 0x04, 0x02, 0x00, 0x3C, + 0x00, 0x01, 'P' + }; + static const byte connect_sub[] = { + 0x10, 0x0D, + 0x00, 0x04, 'M', 'Q', 'T', 'T', + 0x04, 0x02, 0x00, 0x3C, + 0x00, 0x01, 'S' + }; + /* SUBSCRIBE packet_id 1, filter "x", QoS 0. */ + static const byte subscribe_x[] = { + 0x82, 0x06, + 0x00, 0x01, + 0x00, 0x01, 'x', + 0x00 + }; + /* QoS 0 PUBLISH to "x": Remaining Length 23 = 3 topic bytes + 20 + * payload bytes. Forwarded, it encodes to 25 bytes. */ + static const byte publish_x[] = { + 0x30, 0x17, + 0x00, 0x01, 'x', + '0', '1', '2', '3', '4', '5', '6', '7', '8', '9', + '0', '1', '2', '3', '4', '5', '6', '7', '8', '9' + }; + + install_mock_net(&net); + XMEMSET(&broker, 0, sizeof(broker)); + ASSERT_EQ(MQTT_CODE_SUCCESS, MqttBroker_Init(&broker, &net)); + ASSERT_EQ(MQTT_CODE_SUCCESS, MqttBroker_Start(&broker)); + + reset_mock_clients(3); + g_clients_active = 2; + mock_client_input_append(0, connect_pub, sizeof(connect_pub)); + mock_client_input_append(1, connect_sub, sizeof(connect_sub)); + mock_client_input_append(1, subscribe_x, sizeof(subscribe_x)); + for (i = 0; i < 16; i++) { + (void)MqttBroker_Step(&broker); + } + sub_bc = find_broker_client(&broker, "S"); + ASSERT_NOT_NULL(sub_bc); + + /* Subscriber transmit buffer smaller than the forwarded frame, as with a + * build whose receive buffer exceeds its transmit buffer. */ + sub_bc->tx_buf_len = 16; + sub_out_mark = g_clients[1].out_len; + + mock_client_input_append(0, publish_x, sizeof(publish_x)); + for (i = 0; i < 8; i++) { + (void)MqttBroker_Step(&broker); + } + + ASSERT_EQ((int)sub_out_mark, (int)g_clients[1].out_len); + ASSERT_NULL(sub_bc->out_q_head); + ASSERT_EQ(0, sub_bc->out_q_count); + + MqttBroker_Stop(&broker); + MqttBroker_Free(&broker); +} + +#ifdef WOLFMQTT_V5 +/* A QoS 1 or QoS 2 copy that cannot fit a subscriber's transmit buffer is + * never delivered, so the v5 publisher must not be told the PUBLISH was + * accepted: its PUBACK or PUBREC carries Reason Code 0x80 (MQTT 5.0 sections + * 3.4.2.1 and 3.5.2.1). Returns 0 when that holds. */ +static int publish_exceeding_sub_tx_buf_not_acked(MqttQoS qos) +{ + MqttBroker broker; + MqttBrokerNet net; + BrokerClient* sub_bc; + size_t pub_out_mark; + size_t sub_out_mark; + int i; + int result = -1; + /* v5 CONNECT: Clean Start, keep alive 60, Property Length 0, ClientId + * "P". Remaining Length 14. */ + static const byte connect_pub[] = { + 0x10, 0x0E, + 0x00, 0x04, 'M', 'Q', 'T', 'T', + 0x05, 0x02, 0x00, 0x3C, + 0x00, + 0x00, 0x01, 'P' + }; + static const byte connect_sub[] = { + 0x10, 0x0D, + 0x00, 0x04, 'M', 'Q', 'T', 'T', + 0x04, 0x02, 0x00, 0x3C, + 0x00, 0x01, 'S' + }; + /* SUBSCRIBE packet_id 1, filter "x", QoS 2. */ + static const byte subscribe_x[] = { + 0x82, 0x06, + 0x00, 0x01, + 0x00, 0x01, 'x', + 0x02 + }; + /* v5 PUBLISH to "x", packet id 7, Property Length 0, 20 payload bytes: + * Remaining Length 26. The QoS bits are set below. Forwarded at the same + * QoS to a v3.1.1 subscriber, it encodes to 27 bytes. */ + byte publish_x[] = { + 0x30, 0x1A, + 0x00, 0x01, 'x', + 0x00, 0x07, 0x00, - 'A' + '0', '1', '2', '3', '4', '5', '6', '7', '8', '9', + '0', '1', '2', '3', '4', '5', '6', '7', '8', '9' }; + publish_x[0] = (byte)(0x30 | ((byte)qos << 1)); - sub_bc = static_setup_pub_sub(&broker, &net, - MQTT_CONNECT_PROTOCOL_LEVEL_5, 0x02); - ASSERT_NOT_NULL(sub_bc); - - before = g_clients[1].out_len; - mock_client_input_append(0, publish_x, sizeof(publish_x)); - for (i = 0; i < 8; i++) { - (void)MqttBroker_Step(&broker); + install_mock_net(&net); + XMEMSET(&broker, 0, sizeof(broker)); + if (MqttBroker_Init(&broker, &net) != MQTT_CODE_SUCCESS || + MqttBroker_Start(&broker) != MQTT_CODE_SUCCESS) { + return -1; } - info = first_publish_info(g_clients[1].out_buf + before, - g_clients[1].out_len - before); - ASSERT_TRUE(info.found); - id = info.packet_id; - ASSERT_TRUE(id != 0); - ASSERT_TRUE(static_out_id_in_use(sub_bc, id)); - /* v5 PUBREC with reason 0x87 (Not authorized). */ - pubrec[0] = 0x50; - pubrec[1] = 0x03; - pubrec[2] = (byte)(id >> 8); - pubrec[3] = (byte)(id & 0xFF); - pubrec[4] = 0x87; - mock_client_input_append(1, pubrec, 5); - for (i = 0; i < 8; i++) { + reset_mock_clients(3); + g_clients_active = 2; + mock_client_input_append(0, connect_pub, sizeof(connect_pub)); + mock_client_input_append(1, connect_sub, sizeof(connect_sub)); + mock_client_input_append(1, subscribe_x, sizeof(subscribe_x)); + for (i = 0; i < 16; i++) { (void)MqttBroker_Step(&broker); } + sub_bc = find_broker_client(&broker, "S"); + if (sub_bc != NULL) { + /* Subscriber transmit buffer smaller than the forwarded frame, as + * with a build whose receive buffer exceeds its transmit buffer. */ + sub_bc->tx_buf_len = 16; + pub_out_mark = g_clients[0].out_len; + sub_out_mark = g_clients[1].out_len; - ASSERT_FALSE(static_out_id_in_use(sub_bc, id)); + mock_client_input_append(0, publish_x, sizeof(publish_x)); + for (i = 0; i < 8; i++) { + (void)MqttBroker_Step(&broker); + } + + /* The response's first byte is PUBACK (0x40) or PUBREC (0x50), and + * its Reason Code follows the 2-byte Packet Identifier. */ + if (g_clients[0].out_len >= pub_out_mark + 5 && + g_clients[0].out_buf[pub_out_mark] == + ((qos == MQTT_QOS_1) ? 0x40 : 0x50) && + g_clients[0].out_buf[pub_out_mark + 4] == + MQTT_REASON_UNSPECIFIED_ERR && + g_clients[1].out_len == sub_out_mark && + sub_bc->out_q_count == 0) { + result = 0; + } + } MqttBroker_Stop(&broker); MqttBroker_Free(&broker); + return result; } -#endif /* WOLFMQTT_V5 && WOLFMQTT_MAX_QOS >= 2 */ - -#endif /* WOLFMQTT_STATIC_MEMORY */ -#ifndef WOLFMQTT_STATIC_MEMORY -/* Find the orphan session carrying a disconnected client's Session state. The - * BrokerClient itself is freed by the close, so anything a test wants to check - * about the preserved queue must be read from here. */ -static BrokerOrphanSession* find_orphan_session(MqttBroker* broker, - const char* id) +TEST(publish_qos1_exceeding_sub_tx_buf_not_acked) { - BrokerOrphanSession* o = broker->orphan_sessions; + ASSERT_EQ(0, publish_exceeding_sub_tx_buf_not_acked(MQTT_QOS_1)); +} - while (o != NULL) { - if (o->client_id != NULL && XSTRCMP(o->client_id, id) == 0) { - return o; - } - o = o->next; - } - return NULL; +#if WOLFMQTT_MAX_QOS >= 2 +TEST(publish_qos2_exceeding_sub_tx_buf_not_acked) +{ + ASSERT_EQ(0, publish_exceeding_sub_tx_buf_not_acked(MQTT_QOS_2)); } +#endif +#endif /* WOLFMQTT_V5 */ #endif /* !WOLFMQTT_STATIC_MEMORY */ #if defined(WOLFMQTT_NONBLOCK) && !defined(WOLFMQTT_STATIC_MEMORY) @@ -5811,6 +6884,7 @@ TEST(outbound_qos0_partial_behind_unacked_qos1_not_replayed) MqttBroker_Free(&broker); } + /* MQTT 3.1.1 section 4.3.1: a partly transmitted QoS 0 publication does * not follow a surviving Session onto a replacement connection. */ TEST(takeover_drops_partial_qos0_after_unacked_qos1) @@ -6869,29 +7943,330 @@ TEST(connack_session_present_v5_set_on_resumed_session) for (i = 0; i < 16; i++) { MqttBroker_Step(&broker); } - ASSERT_TRUE(g_clients[0].closed); + ASSERT_TRUE(g_clients[0].closed); + + g_clients_active = 2; + for (i = 0; i < 16; i++) { + MqttBroker_Step(&broker); + } + + XMEMSET(&ack, 0, sizeof(ack)); + ack.protocol_level = MQTT_CONNECT_PROTOCOL_LEVEL_5; + rc = MqttDecode_ConnectAck(g_clients[1].out_buf, + (int)g_clients[1].out_len, &ack); + ASSERT_TRUE(rc > 0); + ASSERT_EQ(MQTT_CONNECT_ACK_FLAG_SESSION_PRESENT, + (int)(ack.flags & MQTT_CONNECT_ACK_FLAG_SESSION_PRESENT)); + ASSERT_EQ(MQTT_CONNECT_ACK_CODE_ACCEPTED, ack.return_code); + if (ack.props != NULL) { + (void)MqttProps_Free(ack.props); + } + + MqttBroker_Stop(&broker); + MqttBroker_Free(&broker); +} +#endif /* WOLFMQTT_V5 */ + +#ifdef WOLFMQTT_V5 +/* The broker has no Shared Subscription routing, so it must not let a v5 + * client believe otherwise. MQTT 5.0 section 3.2.2.3.15: an absent Shared + * Subscription Available property means "supported", so CONNACK must carry + * it with value 0. A '$share/' filter (section 4.8.2) is then refused with + * SUBACK Reason Code 0x9E (section 3.9.3) instead of being stored and matched + * literally. */ +TEST(broker_shared_subscription_not_supported_v5) +{ + MqttBroker broker; + MqttBrokerNet net; + MqttConnectAck ack; + MqttProp* prop; + int shared_avail = -1; + int last_byte; + int rc; + int i; + /* v5 CONNECT, clean=1, props_len=0, client_id="A". Remaining Length 14. */ + static const byte connect[] = { + 0x10, 0x0E, + 0x00, 0x04, 'M', 'Q', 'T', 'T', + 0x05, + 0x02, + 0x00, 0x3C, + 0x00, + 0x00, 0x01, 'A' + }; + /* v5 SUBSCRIBE packet_id 7, props_len 0, filter "$share/g/x", options + * QoS 0. Remaining Length = 2 + 1 + 2 + 10 + 1 = 16. */ + static const byte subscribe_shared[] = { + 0x82, 0x10, + 0x00, 0x07, + 0x00, + 0x00, 0x0A, '$', 's', 'h', 'a', 'r', 'e', '/', 'g', '/', 'x', + 0x00 + }; + + install_mock_net(&net); + XMEMSET(&broker, 0, sizeof(broker)); + ASSERT_EQ(MQTT_CODE_SUCCESS, MqttBroker_Init(&broker, &net)); + ASSERT_EQ(MQTT_CODE_SUCCESS, MqttBroker_Start(&broker)); + + reset_mock_clients(1); + mock_client_input_append(0, connect, sizeof(connect)); + mock_client_input_append(0, subscribe_shared, sizeof(subscribe_shared)); + for (i = 0; i < 16; i++) { + MqttBroker_Step(&broker); + } + + XMEMSET(&ack, 0, sizeof(ack)); + ack.protocol_level = MQTT_CONNECT_PROTOCOL_LEVEL_5; + rc = MqttDecode_ConnectAck(g_clients[0].out_buf, + (int)g_clients[0].out_len, &ack); + ASSERT_TRUE(rc > 0); + ASSERT_EQ(MQTT_CONNECT_ACK_CODE_ACCEPTED, ack.return_code); + for (prop = ack.props; prop != NULL; prop = prop->next) { + if (prop->type == MQTT_PROP_SHARED_SUBSCRIPTION_AVAIL) { + shared_avail = prop->data_byte; + } + } + if (ack.props != NULL) { + (void)MqttProps_Free(ack.props); + } + ASSERT_EQ(0, shared_avail); + + ASSERT_EQ(1, count_packets_of_type(g_clients[0].out_buf, + g_clients[0].out_len, MQTT_PACKET_TYPE_SUBSCRIBE_ACK)); + last_byte = g_clients[0].out_buf[g_clients[0].out_len - 1]; + ASSERT_EQ(MQTT_REASON_SS_NOT_SUPPORTED, last_byte); + + MqttBroker_Stop(&broker); + MqttBroker_Free(&broker); +} +#endif /* WOLFMQTT_V5 */ + +#if defined(WOLFMQTT_V5) && !defined(WOLFMQTT_DYN_PROP) +/* An absent Shared Subscription Available property means "supported" (MQTT + * 5.0 section 3.2.2.3.15). If the property cannot be allocated, the CONNECT + * is refused rather than accepted with a CONNACK that advertises a feature + * the broker does not have. */ +TEST(connect_v5_refused_when_shared_sub_property_unavailable) +{ + MqttBroker broker; + MqttBrokerNet net; + MqttProp* hold = NULL; + int free_slots = 0; + int held = 0; + int i; + int last_byte_code; + /* v5 CONNECT, clean=1, props_len=0, client_id="A". Remaining Length 14. */ + static const byte connect[] = { + 0x10, 0x0E, + 0x00, 0x04, 'M', 'Q', 'T', 'T', + 0x05, + 0x02, + 0x00, 0x3C, + 0x00, + 0x00, 0x01, 'A' + }; + + install_mock_net(&net); + XMEMSET(&broker, 0, sizeof(broker)); + ASSERT_EQ(MQTT_CODE_SUCCESS, MqttBroker_Init(&broker, &net)); + ASSERT_EQ(MQTT_CODE_SUCCESS, MqttBroker_Start(&broker)); + + /* Hold a property-stack reference of our own: no client exists yet, + * and in multithread builds the stack lock is only initialized while + * one does. */ + ASSERT_EQ(MQTT_CODE_SUCCESS, MqttProps_Init()); + + /* Count the free property slots, then hold all but two: enough for + * Retain Available and Wildcard Subscription Available, not for Shared + * Subscription Available. */ + while (MqttProps_Add(&hold) != NULL) { + free_slots++; + } + (void)MqttProps_Free(hold); + hold = NULL; + for (i = 0; i < free_slots - 2; i++) { + if (MqttProps_Add(&hold) != NULL) { + held++; + } + } + + reset_mock_state(connect, sizeof(connect)); + run_broker_one_connect(&broker); + (void)MqttProps_Free(hold); + (void)MqttProps_ShutDown(); + + ASSERT_TRUE(free_slots > 2); + ASSERT_EQ(free_slots - 2, held); + ASSERT_TRUE(g_out_len >= 4); + ASSERT_EQ(0x20, g_out_buf[0]); + last_byte_code = g_out_buf[3]; + ASSERT_EQ(MQTT_REASON_SERVER_UNAVAILABLE, last_byte_code); + ASSERT_TRUE(g_client_closed); + + MqttBroker_Stop(&broker); + MqttBroker_Free(&broker); +} + +/* Receive Maximum and, when this build caps QoS below 2, Maximum QoS are + * also capability statements: their absence means 65535 and QoS 2 (MQTT 5.0 + * sections 3.2.2.3.3 and 3.2.2.3.4). With room for only the three + * availability properties, the CONNECT must be refused rather than accepted + * without them. */ +TEST(connect_v5_refused_when_receive_max_property_unavailable) +{ + MqttBroker broker; + MqttBrokerNet net; + MqttProp* hold = NULL; + int free_slots = 0; + int held = 0; + int i; + int last_byte_code; + /* v5 CONNECT, clean=1, props_len=0, client_id="A". Remaining Length 14. */ + static const byte connect[] = { + 0x10, 0x0E, + 0x00, 0x04, 'M', 'Q', 'T', 'T', + 0x05, + 0x02, + 0x00, 0x3C, + 0x00, + 0x00, 0x01, 'A' + }; + + install_mock_net(&net); + XMEMSET(&broker, 0, sizeof(broker)); + ASSERT_EQ(MQTT_CODE_SUCCESS, MqttBroker_Init(&broker, &net)); + ASSERT_EQ(MQTT_CODE_SUCCESS, MqttBroker_Start(&broker)); + + /* See connect_v5_refused_when_shared_sub_property_unavailable for the + * property-stack reference. Leave three free slots: Retain Available, + * Wildcard Subscription Available and Shared Subscription Available. */ + ASSERT_EQ(MQTT_CODE_SUCCESS, MqttProps_Init()); + while (MqttProps_Add(&hold) != NULL) { + free_slots++; + } + (void)MqttProps_Free(hold); + hold = NULL; + for (i = 0; i < free_slots - 3; i++) { + if (MqttProps_Add(&hold) != NULL) { + held++; + } + } + + reset_mock_state(connect, sizeof(connect)); + run_broker_one_connect(&broker); + (void)MqttProps_Free(hold); + (void)MqttProps_ShutDown(); + + ASSERT_TRUE(free_slots > 3); + ASSERT_EQ(free_slots - 3, held); + ASSERT_TRUE(g_out_len >= 4); + ASSERT_EQ(0x20, g_out_buf[0]); + last_byte_code = g_out_buf[3]; + ASSERT_EQ(MQTT_REASON_SERVER_UNAVAILABLE, last_byte_code); + ASSERT_TRUE(g_client_closed); + + MqttBroker_Stop(&broker); + MqttBroker_Free(&broker); +} + +/* A CONNECT refused because a required CONNACK property cannot be allocated + * must not take over the connected client that holds the same ClientId: that + * client stays connected with its Session (here, its subscription) + * unchanged. */ +TEST(connect_v5_connack_property_failure_keeps_incumbent_session) +{ + MqttBroker broker; + MqttBrokerNet net; + MqttProp* hold = NULL; + int free_slots = 0; + int held = 0; + int i; + int incumbent_closed; + int incumbent_publishes; + /* v5 CONNECT: Clean Start 0, keep alive 60, properties: Session Expiry + * Interval (0x11) = 60, ClientId "P". Remaining Length 19. Used by both + * the incumbent and the takeover attempt. */ + static const byte connect[] = { + 0x10, 0x13, + 0x00, 0x04, 'M', 'Q', 'T', 'T', + 0x05, 0x00, 0x00, 0x3C, + 0x05, 0x11, 0x00, 0x00, 0x00, 0x3C, + 0x00, 0x01, 'P' + }; + /* v5 SUBSCRIBE packet id 1, Property Length 0, filter "t", options 0. */ + static const byte subscribe[] = { + 0x82, 0x07, + 0x00, 0x01, + 0x00, + 0x00, 0x01, 't', + 0x00 + }; + /* v5 QoS 0 PUBLISH to "t", Property Length 0, payload "x". */ + static const byte publish[] = { + 0x30, 0x05, + 0x00, 0x01, 't', + 0x00, + 'x' + }; + + install_mock_net(&net); + XMEMSET(&broker, 0, sizeof(broker)); + ASSERT_EQ(MQTT_CODE_SUCCESS, MqttBroker_Init(&broker, &net)); + ASSERT_EQ(MQTT_CODE_SUCCESS, MqttBroker_Start(&broker)); + + reset_mock_clients(2); + mock_client_input_append(0, connect, sizeof(connect)); + mock_client_input_append(0, subscribe, sizeof(subscribe)); + for (i = 0; i < 16; i++) { + MqttBroker_Step(&broker); + } - g_clients_active = 2; + /* Hold a property-stack reference of our own (see + * connect_v5_refused_when_shared_sub_property_unavailable), then leave + * two free slots: enough to decode the CONNECT's Session Expiry Interval + * and later to hold Retain Available and Wildcard Subscription + * Available, not Shared Subscription Available. */ + ASSERT_EQ(MQTT_CODE_SUCCESS, MqttProps_Init()); + while (MqttProps_Add(&hold) != NULL) { + free_slots++; + } + (void)MqttProps_Free(hold); + hold = NULL; + for (i = 0; i < free_slots - 2; i++) { + if (MqttProps_Add(&hold) != NULL) { + held++; + } + } + + mock_client_input_append(1, connect, sizeof(connect)); for (i = 0; i < 16; i++) { MqttBroker_Step(&broker); } + (void)MqttProps_Free(hold); + (void)MqttProps_ShutDown(); - XMEMSET(&ack, 0, sizeof(ack)); - ack.protocol_level = MQTT_CONNECT_PROTOCOL_LEVEL_5; - rc = MqttDecode_ConnectAck(g_clients[1].out_buf, - (int)g_clients[1].out_len, &ack); - ASSERT_TRUE(rc > 0); - ASSERT_EQ(MQTT_CONNECT_ACK_FLAG_SESSION_PRESENT, - (int)(ack.flags & MQTT_CONNECT_ACK_FLAG_SESSION_PRESENT)); - ASSERT_EQ(MQTT_CONNECT_ACK_CODE_ACCEPTED, ack.return_code); - if (ack.props != NULL) { - (void)MqttProps_Free(ack.props); + mock_client_input_append(0, publish, sizeof(publish)); + for (i = 0; i < 16; i++) { + MqttBroker_Step(&broker); } + incumbent_closed = g_clients[0].closed; + incumbent_publishes = count_packets_of_type(g_clients[0].out_buf, + g_clients[0].out_len, MQTT_PACKET_TYPE_PUBLISH); MqttBroker_Stop(&broker); MqttBroker_Free(&broker); + + ASSERT_TRUE(free_slots > 2); + ASSERT_EQ(free_slots - 2, held); + ASSERT_TRUE(g_clients[1].out_len >= 4); + ASSERT_EQ(0x20, g_clients[1].out_buf[0]); + ASSERT_EQ(MQTT_REASON_SERVER_UNAVAILABLE, g_clients[1].out_buf[3]); + ASSERT_TRUE(g_clients[1].closed); + ASSERT_FALSE(incumbent_closed); + ASSERT_EQ(1, incumbent_publishes); } -#endif /* WOLFMQTT_V5 */ +#endif /* WOLFMQTT_V5 && !WOLFMQTT_DYN_PROP */ #ifndef WOLFMQTT_BROKER_WILDCARDS /* [MQTT-3.8.3-2] (v3.1.1 section 3.8.3): when the server does not support @@ -7482,6 +8857,52 @@ TEST(connect_v5_receive_max_zero_protocol_error) MqttBroker_Free(&broker); } +/* MQTT 5.0 section 3.14.2.2.2: when CONNECT set a Session Expiry Interval of + * zero, a DISCONNECT carrying a non-zero interval is a Protocol Error + * [MQTT-3.14.2-1]. The Server does not treat it as a valid DISCONNECT and + * sends DISCONNECT with Reason Code 0x82 before closing (section 4.13). */ +TEST(disconnect_v5_session_expiry_raise_sends_protocol_error) +{ + MqttBroker broker; + MqttBrokerNet net; + int i; + /* v5 CONNECT, CleanStart, keep alive 60, no properties (Session Expiry + * Interval 0), ClientId "A". Remaining Length 14. */ + static const byte connect[] = { + 0x10, 14, + 0x00, 0x04, 'M', 'Q', 'T', 'T', + 0x05, 0x02, 0x00, 0x3C, + 0x00, + 0x00, 0x01, 'A' + }; + /* v5 DISCONNECT, Reason Code 0x00, Properties length 5: Session Expiry + * Interval (0x11) = 60. Remaining Length 7. */ + static const byte disconnect[] = { + 0xE0, 0x07, + 0x00, + 0x05, + 0x11, 0x00, 0x00, 0x00, 0x3C + }; + + install_mock_net(&net); + XMEMSET(&broker, 0, sizeof(broker)); + ASSERT_EQ(MQTT_CODE_SUCCESS, MqttBroker_Init(&broker, &net)); + ASSERT_EQ(MQTT_CODE_SUCCESS, MqttBroker_Start(&broker)); + + reset_mock_state(connect, sizeof(connect)); + mock_client_input_append(0, disconnect, sizeof(disconnect)); + for (i = 0; i < 8; i++) { + (void)MqttBroker_Step(&broker); + } + + ASSERT_EQ(MQTT_REASON_PROTOCOL_ERR, + first_disconnect_reason(g_out_buf, g_out_len)); + ASSERT_TRUE(g_client_closed); + + MqttBroker_Stop(&broker); + MqttBroker_Free(&broker); +} + /* v5 CONNECT with Clean Start=0, no Session Expiry: must default to 0, * not the v3.1.1 0xFFFFFFFF "never" sentinel. */ TEST(connect_v5_clean0_no_se_prop_orphan_expiry_zero) @@ -7727,6 +9148,90 @@ TEST(disconnect_v5_session_expiry_0_to_nonzero_protocol_error) MqttBroker_Stop(&broker); MqttBroker_Free(&broker); } +#if defined(WOLFMQTT_NONBLOCK) && !defined(WOLFMQTT_STATIC_MEMORY) +/* The Protocol Error DISCONNECT (Reason Code 0x82, MQTT 5.0 section + * 3.14.2.2.2) for a 0-to-nonzero Session Expiry change must reach the client + * in full before the Network Connection is closed, even when the transport + * takes it in more than one write. */ +TEST(disconnect_v5_session_expiry_protocol_error_short_write_completes) +{ + MqttBroker broker; + MqttBrokerNet net; + int i; + size_t pub_mark; + int closed_after_first; + static const byte connect_sub[] = { + 0x10, 0x0D, + 0x00, 0x04, 'M', 'Q', 'T', 'T', + 0x04, 0x02, 0x00, 0x3C, + 0x00, 0x01, 'S' + }; + static const byte subscribe_lwt[] = { + 0x82, 0x08, + 0x00, 0x01, + 0x00, 0x03, 'l', 'w', 't', + 0x00 + }; + /* Same publisher CONNECT and DISCONNECT as + * disconnect_v5_session_expiry_0_to_nonzero_protocol_error. */ + static const byte connect_pub[] = { + 0x10, 25, + 0x00, 0x04, 'M', 'Q', 'T', 'T', + 0x05, + 0x04, + 0x00, 0x3C, + 0x00, + 0x00, 0x01, 'P', + 0x00, + 0x00, 0x03, 'l', 'w', 't', + 0x00, 0x03, 'b', 'y', 'e' + }; + static const byte disconnect_se[] = { + 0xE0, 0x07, + 0x00, + 0x05, + 0x11, 0x00, 0x00, 0x00, 0x1E + }; + + install_mock_net(&net); + XMEMSET(&broker, 0, sizeof(broker)); + ASSERT_EQ(MQTT_CODE_SUCCESS, MqttBroker_Init(&broker, &net)); + ASSERT_EQ(MQTT_CODE_SUCCESS, MqttBroker_Start(&broker)); + + reset_mock_clients(2); + mock_client_input_append(0, connect_sub, sizeof(connect_sub)); + mock_client_input_append(0, subscribe_lwt, sizeof(subscribe_lwt)); + mock_client_input_append(1, connect_pub, sizeof(connect_pub)); + for (i = 0; i < 16; i++) { + MqttBroker_Step(&broker); + } + + /* The transport takes only the first byte of the DISCONNECT. */ + pub_mark = g_clients[1].out_len; + g_clients[1].write_limit_once = 1; + mock_client_input_append(1, disconnect_se, sizeof(disconnect_se)); + MqttBroker_Step(&broker); + closed_after_first = g_clients[1].closed; + for (i = 0; i < 8; i++) { + MqttBroker_Step(&broker); + } + + MqttBroker_Stop(&broker); + MqttBroker_Free(&broker); + + ASSERT_FALSE(closed_after_first); + /* DISCONNECT: type 0xE0, Remaining Length, Reason Code 0x82 first. */ + ASSERT_TRUE(g_clients[1].out_len >= pub_mark + 3); + ASSERT_EQ(0xE0, g_clients[1].out_buf[pub_mark]); + ASSERT_EQ(g_clients[1].out_len - pub_mark, + (size_t)2 + g_clients[1].out_buf[pub_mark + 1]); + ASSERT_EQ(MQTT_REASON_PROTOCOL_ERR, g_clients[1].out_buf[pub_mark + 2]); + ASSERT_TRUE(g_clients[1].closed); + /* The close is still abnormal, so the Will is published. */ + ASSERT_EQ(1, count_packets_of_type(g_clients[0].out_buf, + g_clients[0].out_len, MQTT_PACKET_TYPE_PUBLISH)); +} +#endif /* WOLFMQTT_NONBLOCK && !WOLFMQTT_STATIC_MEMORY */ #endif /* WOLFMQTT_BROKER_WILL */ #ifdef WOLFMQTT_BROKER_RETAINED @@ -9730,93 +11235,296 @@ TEST(persist_iter_skips_fifo_without_blocking) root = mkdtemp(root_template); ASSERT_NOT_NULL(root); if (snprintf(ns_path, sizeof(ns_path), "%s/7", root) > 0 && - snprintf(fifo_path, sizeof(fifo_path), "%s/1234.bin", + snprintf(fifo_path, sizeof(fifo_path), "%s/1234.bin", + ns_path) > 0) { + paths_ok = 1; + } + ASSERT_TRUE(paths_ok); + ASSERT_EQ(0, mkdir(ns_path, 0700)); + ASSERT_EQ(0, mkfifo(fifo_path, 0600)); + + init_rc = MqttBrokerNet_PersistPosix_Init(&hooks, root); + ASSERT_EQ(MQTT_CODE_SUCCESS, init_rc); + child = fork(); + ASSERT_TRUE(child >= 0); + if (child == 0) { + int iter_rc; + + (void)alarm(2); + iter_rc = hooks.kv_iter(hooks.ctx, 7, persist_test_iter_cb, &capture); + _exit((iter_rc == MQTT_CODE_SUCCESS && capture.calls == 0) ? 0 : 1); + } + waited = waitpid(child, &status, 0); + + MqttBrokerNet_PersistPosix_Free(&hooks); + (void)unlink(fifo_path); + (void)rmdir(ns_path); + (void)rmdir(root); + + ASSERT_EQ((int)child, (int)waited); + ASSERT_TRUE(WIFEXITED(status)); + ASSERT_EQ(0, WEXITSTATUS(status)); +} + +#ifdef __MACH__ +/* macOS exposes /tmp as a trusted operating-system symlink to /private/tmp. + * A persistence root below that standard alias must initialize without + * weakening the no-follow checks on namespaces or record files. */ +TEST(persist_root_below_macos_tmp_alias_accepted) +{ + char root_template[] = "/tmp/wolfmqtt-persist-XXXXXX"; + MqttBrokerPersistHooks hooks; + char* root; + int rc; + + XMEMSET(&hooks, 0, sizeof(hooks)); + root = mkdtemp(root_template); + ASSERT_NOT_NULL(root); + + rc = MqttBrokerNet_PersistPosix_Init(&hooks, root); + if (rc == MQTT_CODE_SUCCESS) { + MqttBrokerNet_PersistPosix_Free(&hooks); + } + ASSERT_EQ(0, rmdir(root)); + ASSERT_EQ(MQTT_CODE_SUCCESS, rc); +} +#endif + +static int persist_test_init_root_mode(mode_t mode) +{ + char root_template[] = "./wolfmqtt-persist-XXXXXX"; + MqttBrokerPersistHooks hooks; + char* root; + int rc; + + XMEMSET(&hooks, 0, sizeof(hooks)); + root = mkdtemp(root_template); + if (root == NULL) { + return MQTT_CODE_ERROR_SYSTEM; + } + if (chmod(root, mode) != 0) { + (void)rmdir(root); + return MQTT_CODE_ERROR_SYSTEM; + } + rc = MqttBrokerNet_PersistPosix_Init(&hooks, root); + if (rc == MQTT_CODE_SUCCESS) { + MqttBrokerNet_PersistPosix_Free(&hooks); + } + (void)rmdir(root); + return rc; +} + +TEST(persist_root_readable_permissions_accepted) +{ + ASSERT_EQ(MQTT_CODE_SUCCESS, persist_test_init_root_mode(0755)); +} + +TEST(persist_root_writable_permissions_rejected) +{ + ASSERT_EQ(MQTT_CODE_ERROR_SYSTEM, persist_test_init_root_mode(0777)); +} + +/* A rename or unlink is not durable until its namespace directory is + * flushed, so the POSIX backend must report a failed directory fsync rather + * than claim the update was committed. */ +TEST(persist_put_del_report_namespace_fsync_failure) +{ + static const byte key[] = { 0x56, 0x78 }; + static const byte value[] = "value"; + char root_template[] = "./wolfmqtt-persist-XXXXXX"; + char ns_path[640]; + char final_path[680]; + MqttBrokerPersistHooks hooks; + char* root; + int init_rc = MQTT_CODE_ERROR_SYSTEM; + int put_fail_rc = MQTT_CODE_SUCCESS; + int put_ok_rc = MQTT_CODE_ERROR_SYSTEM; + int del_fail_rc = MQTT_CODE_SUCCESS; + int paths_ok = 0; + + XMEMSET(&hooks, 0, sizeof(hooks)); + root = mkdtemp(root_template); + ASSERT_NOT_NULL(root); + if (snprintf(ns_path, sizeof(ns_path), "%s/7", root) > 0 && + snprintf(final_path, sizeof(final_path), "%s/5678.bin", + ns_path) > 0) { + paths_ok = 1; + } + ASSERT_TRUE(paths_ok); + + init_rc = MqttBrokerNet_PersistPosix_Init(&hooks, root); + if (init_rc == MQTT_CODE_SUCCESS) { + g_fsync_dir_fail = 1; + put_fail_rc = hooks.kv_put(hooks.ctx, 7, key, (word16)sizeof(key), + value, (word32)sizeof(value)); + g_fsync_dir_fail = 0; + put_ok_rc = hooks.kv_put(hooks.ctx, 7, key, (word16)sizeof(key), + value, (word32)sizeof(value)); + g_fsync_dir_fail = 1; + del_fail_rc = hooks.kv_del(hooks.ctx, 7, key, (word16)sizeof(key)); + g_fsync_dir_fail = 0; + } + + MqttBrokerNet_PersistPosix_Free(&hooks); + (void)unlink(final_path); + (void)rmdir(ns_path); + (void)rmdir(root); + + ASSERT_EQ(MQTT_CODE_SUCCESS, init_rc); + ASSERT_EQ(MQTT_CODE_ERROR_SYSTEM, put_fail_rc); + ASSERT_EQ(MQTT_CODE_SUCCESS, put_ok_rc); + ASSERT_EQ(MQTT_CODE_ERROR_SYSTEM, del_fail_rc); +} + +/* A record the backend cannot open is an I/O failure, not a record to skip: + * reporting success would let restore or a delete scan finish with durable + * state silently left out. Deliberately non-record entries (wrong suffix, + * non-regular files) are still skipped. */ +TEST(persist_iter_reports_unreadable_record) +{ + static const byte value[] = "value"; + char root_template[] = "./wolfmqtt-persist-XXXXXX"; + char ns_path[640]; + char good_path[680]; + char bad_path[680]; + char other_path[680]; + MqttBrokerPersistHooks hooks; + PersistIterCapture capture; + char* root; + int init_rc = MQTT_CODE_ERROR_SYSTEM; + int clean_iter_rc = MQTT_CODE_ERROR_SYSTEM; + int iter_rc = MQTT_CODE_SUCCESS; + int bad_unreadable = 0; + int fd; + int paths_ok = 0; + + XMEMSET(&hooks, 0, sizeof(hooks)); + XMEMSET(&capture, 0, sizeof(capture)); + root = mkdtemp(root_template); + ASSERT_NOT_NULL(root); + if (snprintf(ns_path, sizeof(ns_path), "%s/7", root) > 0 && + snprintf(good_path, sizeof(good_path), "%s/1111.bin", + ns_path) > 0 && + snprintf(bad_path, sizeof(bad_path), "%s/2222.bin", + ns_path) > 0 && + snprintf(other_path, sizeof(other_path), "%s/notes.txt", ns_path) > 0) { paths_ok = 1; } ASSERT_TRUE(paths_ok); ASSERT_EQ(0, mkdir(ns_path, 0700)); - ASSERT_EQ(0, mkfifo(fifo_path, 0600)); + ASSERT_EQ(0, persist_test_write_file(good_path, value, + (word32)sizeof(value))); + ASSERT_EQ(0, persist_test_write_file(other_path, value, + (word32)sizeof(value))); init_rc = MqttBrokerNet_PersistPosix_Init(&hooks, root); - ASSERT_EQ(MQTT_CODE_SUCCESS, init_rc); - child = fork(); - ASSERT_TRUE(child >= 0); - if (child == 0) { - int iter_rc; - - (void)alarm(2); - iter_rc = hooks.kv_iter(hooks.ctx, 7, persist_test_iter_cb, &capture); - _exit((iter_rc == MQTT_CODE_SUCCESS && capture.calls == 0) ? 0 : 1); + if (init_rc == MQTT_CODE_SUCCESS) { + /* Readable record plus a non-record file: iteration succeeds. */ + clean_iter_rc = hooks.kv_iter(hooks.ctx, 7, persist_test_iter_cb, + &capture); + /* Add a record the backend owns but cannot open. Mode 0 does not + * stop a privileged process (root in container CI), so only check + * the error path when the open really fails. */ + if (persist_test_write_file(bad_path, value, + (word32)sizeof(value)) == 0 && chmod(bad_path, 0) == 0) { + fd = open(bad_path, O_RDONLY); + if (fd >= 0) { + (void)close(fd); + } + else { + bad_unreadable = 1; + iter_rc = hooks.kv_iter(hooks.ctx, 7, persist_test_iter_cb, + &capture); + } + } } - waited = waitpid(child, &status, 0); MqttBrokerNet_PersistPosix_Free(&hooks); - (void)unlink(fifo_path); + (void)unlink(good_path); + (void)unlink(bad_path); + (void)unlink(other_path); (void)rmdir(ns_path); (void)rmdir(root); - ASSERT_EQ((int)child, (int)waited); - ASSERT_TRUE(WIFEXITED(status)); - ASSERT_EQ(0, WEXITSTATUS(status)); -} - -#ifdef __MACH__ -/* macOS exposes /tmp as a trusted operating-system symlink to /private/tmp. - * A persistence root below that standard alias must initialize without - * weakening the no-follow checks on namespaces or record files. */ -TEST(persist_root_below_macos_tmp_alias_accepted) -{ - char root_template[] = "/tmp/wolfmqtt-persist-XXXXXX"; - MqttBrokerPersistHooks hooks; - char* root; - int rc; - - XMEMSET(&hooks, 0, sizeof(hooks)); - root = mkdtemp(root_template); - ASSERT_NOT_NULL(root); - - rc = MqttBrokerNet_PersistPosix_Init(&hooks, root); - if (rc == MQTT_CODE_SUCCESS) { - MqttBrokerNet_PersistPosix_Free(&hooks); + ASSERT_EQ(MQTT_CODE_SUCCESS, init_rc); + ASSERT_EQ(MQTT_CODE_SUCCESS, clean_iter_rc); + if (bad_unreadable) { + ASSERT_EQ(MQTT_CODE_ERROR_SYSTEM, iter_rc); } - ASSERT_EQ(0, rmdir(root)); - ASSERT_EQ(MQTT_CODE_SUCCESS, rc); } -#endif -static int persist_test_init_root_mode(mode_t mode) +/* Entries named like records that are not regular files the broker owns - a + * UNIX socket, a symlink - are skipped by type before any open, so their + * platform-specific open errors (ENXIO, ELOOP/EMLINK) cannot fail restore. */ +TEST(persist_iter_skips_socket_and_symlink_entries) { + static const byte value[] = "value"; char root_template[] = "./wolfmqtt-persist-XXXXXX"; + char ns_path[640]; + char good_path[680]; + char sock_path[680]; + char link_path[680]; MqttBrokerPersistHooks hooks; + PersistIterCapture capture; + struct sockaddr_un sun; char* root; - int rc; + int sock = -1; + int init_rc = MQTT_CODE_ERROR_SYSTEM; + int iter_rc = MQTT_CODE_ERROR_SYSTEM; + int sock_ok = 0; + int paths_ok = 0; XMEMSET(&hooks, 0, sizeof(hooks)); + XMEMSET(&capture, 0, sizeof(capture)); root = mkdtemp(root_template); - if (root == NULL) { - return MQTT_CODE_ERROR_SYSTEM; + ASSERT_NOT_NULL(root); + if (snprintf(ns_path, sizeof(ns_path), "%s/7", root) > 0 && + snprintf(good_path, sizeof(good_path), "%s/1111.bin", + ns_path) > 0 && + snprintf(sock_path, sizeof(sock_path), "%s/3333.bin", + ns_path) > 0 && + snprintf(link_path, sizeof(link_path), "%s/4444.bin", + ns_path) > 0) { + paths_ok = 1; } - if (chmod(root, mode) != 0) { - (void)rmdir(root); - return MQTT_CODE_ERROR_SYSTEM; + ASSERT_TRUE(paths_ok); + ASSERT_EQ(0, mkdir(ns_path, 0700)); + ASSERT_EQ(0, persist_test_write_file(good_path, value, + (word32)sizeof(value))); + ASSERT_EQ(0, symlink("1111.bin", link_path)); + + XMEMSET(&sun, 0, sizeof(sun)); + sun.sun_family = AF_UNIX; + if (strlen(sock_path) < sizeof(sun.sun_path)) { + XMEMCPY(sun.sun_path, sock_path, strlen(sock_path) + 1); + sock = socket(AF_UNIX, SOCK_STREAM, 0); + if (sock >= 0 && bind(sock, (struct sockaddr*)&sun, + (socklen_t)sizeof(sun)) == 0) { + sock_ok = 1; + } } - rc = MqttBrokerNet_PersistPosix_Init(&hooks, root); - if (rc == MQTT_CODE_SUCCESS) { - MqttBrokerNet_PersistPosix_Free(&hooks); + + init_rc = MqttBrokerNet_PersistPosix_Init(&hooks, root); + if (init_rc == MQTT_CODE_SUCCESS) { + iter_rc = hooks.kv_iter(hooks.ctx, 7, persist_test_iter_cb, + &capture); } - (void)rmdir(root); - return rc; -} -TEST(persist_root_readable_permissions_accepted) -{ - ASSERT_EQ(MQTT_CODE_SUCCESS, persist_test_init_root_mode(0755)); -} + MqttBrokerNet_PersistPosix_Free(&hooks); + if (sock >= 0) { + (void)close(sock); + } + (void)unlink(sock_path); + (void)unlink(link_path); + (void)unlink(good_path); + (void)rmdir(ns_path); + (void)rmdir(root); -TEST(persist_root_writable_permissions_rejected) -{ - ASSERT_EQ(MQTT_CODE_ERROR_SYSTEM, persist_test_init_root_mode(0777)); + ASSERT_TRUE(sock_ok); + ASSERT_EQ(MQTT_CODE_SUCCESS, init_rc); + ASSERT_EQ(MQTT_CODE_SUCCESS, iter_rc); + ASSERT_EQ(1, capture.calls); } TEST(persist_put_rejects_existing_temp_symlink) @@ -9960,10 +11668,18 @@ typedef struct KvRec { } KvRec; static KvRec g_kv[KV_MAX_RECS]; +static int g_kv_sync_rc; /* result kv_sync reports */ +static int g_kv_del_rc; /* non-zero: kv_del fails with this, keeping data */ +static int g_kv_del_fail_ns; /* namespace g_kv_del_rc applies to, -1 = all */ +static int g_kv_iter_rc; /* result kv_iter reports after visiting records */ static void kv_reset(void) { XMEMSET(g_kv, 0, sizeof(g_kv)); + g_kv_sync_rc = 0; + g_kv_del_rc = 0; + g_kv_del_fail_ns = -1; + g_kv_iter_rc = 0; } static KvRec* kv_find(byte ns, const byte* key, word16 key_len) @@ -10030,6 +11746,10 @@ static int kv_del(void* ctx, byte ns, const byte* key, word16 key_len) { KvRec* r; (void)ctx; + if (g_kv_del_rc != 0 && + (g_kv_del_fail_ns < 0 || g_kv_del_fail_ns == (int)ns)) { + return g_kv_del_rc; + } r = kv_find(ns, key, key_len); if (r != NULL) { XMEMSET(r, 0, sizeof(*r)); @@ -10058,13 +11778,13 @@ static int kv_iter(void* ctx, byte ns, MqttBrokerPersist_IterCb cb, } } } - return 0; + return g_kv_iter_rc; } static int kv_sync(void* ctx) { (void)ctx; - return 0; + return g_kv_sync_rc; } #ifndef WOLFMQTT_STATIC_MEMORY @@ -10097,6 +11817,400 @@ static int takeover_setup_persist(MqttBroker* broker, return MqttBroker_SetPersistHooks(broker, hooks); } +/* A write or delete is only committed once the backend's sync hook has made + * it durable. A failing sync must reach the caller instead of being reported + * as a successful commit. */ +TEST(persist_commit_reports_sync_failure) +{ + MqttBroker broker; + MqttBrokerNet net; + MqttBrokerPersistHooks hooks; + int put_fail_rc; + int del_fail_rc; + int put_ok_rc; + + install_mock_net(&net); + XMEMSET(&broker, 0, sizeof(broker)); + ASSERT_EQ(MQTT_CODE_SUCCESS, MqttBroker_Init(&broker, &net)); + ASSERT_EQ(MQTT_CODE_SUCCESS, takeover_setup_persist(&broker, &hooks)); + + g_kv_sync_rc = MQTT_CODE_ERROR_SYSTEM; + put_fail_rc = BrokerPersist_PutOrphanSession(&broker, "C", + MQTT_CONNECT_PROTOCOL_LEVEL_4, 60, 0); + del_fail_rc = BrokerPersist_DelSession(&broker, "C"); + g_kv_sync_rc = 0; + put_ok_rc = BrokerPersist_PutOrphanSession(&broker, "C", + MQTT_CONNECT_PROTOCOL_LEVEL_4, 60, 0); + + MqttBroker_Free(&broker); + + ASSERT_EQ(MQTT_CODE_ERROR_SYSTEM, put_fail_rc); + ASSERT_EQ(MQTT_CODE_ERROR_SYSTEM, del_fail_rc); + ASSERT_EQ(MQTT_CODE_SUCCESS, put_ok_rc); +} + +/* Seed two outbound-queue records for ClientId "C". The OUTQ key is the + * ClientId, a 0x00 separator, then the Packet Identifier. Only keys matter + * to BrokerPersist_DelOutQueue. */ +static void delq_seed_two(void) +{ + static const byte key1[] = { 'C', 0x00, 0x00, 0x01 }; + static const byte key2[] = { 'C', 0x00, 0x00, 0x02 }; + static const byte blob[] = { 0x00 }; + + (void)kv_put(NULL, BROKER_PERSIST_NS_OUTQ, key1, (word16)sizeof(key1), + blob, (word32)sizeof(blob)); + (void)kv_put(NULL, BROKER_PERSIST_NS_OUTQ, key2, (word16)sizeof(key2), + blob, (word32)sizeof(blob)); +} + +/* Deleting a client's persisted outbound queue must report every way the + * deletion can be incomplete or not durable, rather than a non-negative + * count that callers take as success. */ +TEST(persist_del_outqueue_reports_failures) +{ + MqttBroker broker; + MqttBrokerNet net; + MqttBrokerPersistHooks hooks; + int del_fail_rc; + int sync_fail_rc; + int iter_fail_rc; + int alloc_fail_rc; + int ok_rc; + int left_after_ok; + + install_mock_net(&net); + XMEMSET(&broker, 0, sizeof(broker)); + ASSERT_EQ(MQTT_CODE_SUCCESS, MqttBroker_Init(&broker, &net)); + ASSERT_EQ(MQTT_CODE_SUCCESS, takeover_setup_persist(&broker, &hooks)); + + delq_seed_two(); + g_kv_del_rc = MQTT_CODE_ERROR_SYSTEM; + del_fail_rc = BrokerPersist_DelOutQueue(&broker, "C"); + g_kv_del_rc = 0; + + g_kv_sync_rc = MQTT_CODE_ERROR_SYSTEM; + sync_fail_rc = BrokerPersist_DelOutQueue(&broker, "C"); + g_kv_sync_rc = 0; + + delq_seed_two(); + g_kv_iter_rc = MQTT_CODE_ERROR_SYSTEM; + iter_fail_rc = BrokerPersist_DelOutQueue(&broker, "C"); + g_kv_iter_rc = 0; + + delq_seed_two(); + broker_test_fail_alloc_after(0); + alloc_fail_rc = BrokerPersist_DelOutQueue(&broker, "C"); + broker_test_disable_alloc_failure(); + + delq_seed_two(); + ok_rc = BrokerPersist_DelOutQueue(&broker, "C"); + left_after_ok = kv_outq_count(); + + MqttBroker_Free(&broker); + + ASSERT_EQ(MQTT_CODE_ERROR_SYSTEM, del_fail_rc); + ASSERT_EQ(MQTT_CODE_ERROR_SYSTEM, sync_fail_rc); + ASSERT_EQ(MQTT_CODE_ERROR_SYSTEM, iter_fail_rc); + ASSERT_EQ(MQTT_CODE_ERROR_MEMORY, alloc_fail_rc); + ASSERT_EQ(2, ok_rc); + ASSERT_EQ(0, left_after_ok); +} + +/* Schema-mismatch recovery wipes every record and then stamps a current + * META. If the wipe is incomplete, stamping META would mark it done and the + * leftover old-schema records would never be wiped. The old META must stay + * in place, and restore must fail, until a later wipe completes. */ +TEST(persist_schema_wipe_failure_keeps_old_meta) +{ + MqttBroker broker; + MqttBrokerNet net; + MqttBrokerPersistHooks hooks; + static const byte meta_key[] = { 0x00 }; + static const byte outq_key[] = { 'C', 0x00, 0x00, 0x01 }; + byte bad_meta[32]; + static const byte blob[] = { 0x00 }; + KvRec* meta; + int fail_rc; + int fail_restored; + int meta_kept; + int ok_rc; + int ok_restored; + int outq_left; + int meta_replaced; + + install_mock_net(&net); + XMEMSET(&broker, 0, sizeof(broker)); + ASSERT_EQ(MQTT_CODE_SUCCESS, MqttBroker_Init(&broker, &net)); + ASSERT_EQ(MQTT_CODE_SUCCESS, takeover_setup_persist(&broker, &hooks)); + + /* A META record that is not a valid header for this build forces the + * schema-mismatch wipe. */ + XMEMSET(bad_meta, 0xEE, sizeof(bad_meta)); + (void)kv_put(NULL, BROKER_PERSIST_NS_META, meta_key, + (word16)sizeof(meta_key), bad_meta, (word32)sizeof(bad_meta)); + (void)kv_put(NULL, BROKER_PERSIST_NS_OUTQ, outq_key, + (word16)sizeof(outq_key), blob, (word32)sizeof(blob)); + + /* Deleting the old outbound-queue record fails. */ + g_kv_del_rc = MQTT_CODE_ERROR_SYSTEM; + g_kv_del_fail_ns = BROKER_PERSIST_NS_OUTQ; + fail_rc = BrokerPersist_Restore(&broker); + fail_restored = broker.persist_restored; + meta = kv_find(BROKER_PERSIST_NS_META, meta_key, + (word16)sizeof(meta_key)); + meta_kept = (meta != NULL && meta->blob_len == sizeof(bad_meta) && + XMEMCMP(meta->blob, bad_meta, sizeof(bad_meta)) == 0); + g_kv_del_rc = 0; + g_kv_del_fail_ns = -1; + + /* With the backend healthy again the wipe completes. */ + ok_rc = BrokerPersist_Restore(&broker); + ok_restored = broker.persist_restored; + outq_left = kv_outq_count(); + meta = kv_find(BROKER_PERSIST_NS_META, meta_key, + (word16)sizeof(meta_key)); + meta_replaced = (meta != NULL && + XMEMCMP(meta->blob, bad_meta, sizeof(bad_meta)) != 0); + + MqttBroker_Free(&broker); + + ASSERT_EQ(MQTT_CODE_ERROR_SYSTEM, fail_rc); + ASSERT_EQ(0, fail_restored); + ASSERT_TRUE(meta_kept); + ASSERT_EQ(MQTT_CODE_SUCCESS, ok_rc); + ASSERT_EQ(1, ok_restored); + ASSERT_EQ(0, outq_left); + ASSERT_TRUE(meta_replaced); +} + +/* Running out of memory while restoring a durable record is not a malformed + * record to skip: restore must fail and roll back rather than start with part + * of the persisted Session state silently missing. */ +TEST(persist_restore_alloc_failure_is_fatal) +{ + MqttBroker broker; + MqttBroker broker2; + MqttBrokerNet net; + MqttBrokerPersistHooks hooks; + int fail_rc; + int fail_restored; + int fail_has_orphan; + int ok_rc; + int ok_has_orphan; + + install_mock_net(&net); + XMEMSET(&broker, 0, sizeof(broker)); + ASSERT_EQ(MQTT_CODE_SUCCESS, MqttBroker_Init(&broker, &net)); + ASSERT_EQ(MQTT_CODE_SUCCESS, takeover_setup_persist(&broker, &hooks)); + /* Stamp META, then persist one orphaned Session for ClientId "C". */ + ASSERT_EQ(MQTT_CODE_SUCCESS, BrokerPersist_Restore(&broker)); + ASSERT_EQ(MQTT_CODE_SUCCESS, BrokerPersist_PutOrphanSession(&broker, "C", + MQTT_CONNECT_PROTOCOL_LEVEL_4, 3600, 0)); + MqttBroker_Free(&broker); + + /* A fresh broker over the same backend restores it. The first allocation + * made for the Session record fails: in encrypt-at-rest builds that is + * the Session's decrypted copy, after META's decrypt has allocated its + * own copy and its AAD buffer; otherwise the orphan Session itself. */ + XMEMSET(&broker2, 0, sizeof(broker2)); + ASSERT_EQ(MQTT_CODE_SUCCESS, MqttBroker_Init(&broker2, &net)); + ASSERT_EQ(MQTT_CODE_SUCCESS, MqttBroker_SetPersistHooks(&broker2, &hooks)); +#ifdef WOLFMQTT_BROKER_PERSIST_ENCRYPT + broker_test_fail_alloc_after(2); +#else + broker_test_fail_alloc_after(0); +#endif + fail_rc = BrokerPersist_Restore(&broker2); + broker_test_disable_alloc_failure(); + fail_restored = broker2.persist_restored; + fail_has_orphan = (broker2.orphan_sessions != NULL); + + ok_rc = BrokerPersist_Restore(&broker2); + ok_has_orphan = (broker2.orphan_sessions != NULL); + MqttBroker_Free(&broker2); + + ASSERT_EQ(1, g_alloc_failure_count); + ASSERT_EQ(MQTT_CODE_ERROR_MEMORY, fail_rc); + ASSERT_EQ(0, fail_restored); + ASSERT_FALSE(fail_has_orphan); + ASSERT_EQ(MQTT_CODE_SUCCESS, ok_rc); + ASSERT_TRUE(ok_has_orphan); +} + +/* More persisted Sessions than BROKER_MAX_PERSIST_SESSIONS is a normal state + * (records are written per CONNECT; the cap is enforced on orphaning), so + * restore skips the excess rather than failing startup. Only a genuine + * allocation failure is fatal. */ +TEST(persist_restore_skips_sessions_over_cap) +{ + MqttBroker broker; + MqttBroker broker2; + MqttBrokerNet net; + MqttBrokerPersistHooks hooks; + char cid[16]; + int i; + int rc; + int restored; + + install_mock_net(&net); + XMEMSET(&broker, 0, sizeof(broker)); + ASSERT_EQ(MQTT_CODE_SUCCESS, MqttBroker_Init(&broker, &net)); + ASSERT_EQ(MQTT_CODE_SUCCESS, takeover_setup_persist(&broker, &hooks)); + ASSERT_EQ(MQTT_CODE_SUCCESS, BrokerPersist_Restore(&broker)); + for (i = 0; i < BROKER_MAX_PERSIST_SESSIONS + 1; i++) { + XSNPRINTF(cid, sizeof(cid), "c%d", i); + ASSERT_EQ(MQTT_CODE_SUCCESS, BrokerPersist_PutOrphanSession(&broker, + cid, MQTT_CONNECT_PROTOCOL_LEVEL_4, 3600, 0)); + } + MqttBroker_Free(&broker); + + XMEMSET(&broker2, 0, sizeof(broker2)); + ASSERT_EQ(MQTT_CODE_SUCCESS, MqttBroker_Init(&broker2, &net)); + ASSERT_EQ(MQTT_CODE_SUCCESS, MqttBroker_SetPersistHooks(&broker2, &hooks)); + rc = BrokerPersist_Restore(&broker2); + restored = broker2.orphan_session_count; + MqttBroker_Free(&broker2); + + ASSERT_EQ(MQTT_CODE_SUCCESS, rc); + ASSERT_EQ(BROKER_MAX_PERSIST_SESSIONS, restored); +} + +#ifdef WOLFMQTT_BROKER_PERSIST_ENCRYPT +/* An encrypted record is bound to the namespace and storage key it was + * written under (AES-GCM additional data). A valid ciphertext moved to a + * different key by someone with write access to the store must fail + * authentication instead of being restored under the new key. */ +TEST(persist_encrypted_record_bound_to_storage_key) +{ + MqttBroker broker; + MqttBroker broker2; + MqttBrokerNet net; + MqttBrokerPersistHooks hooks; + static const byte key_a[] = { 'A' }; + static const byte key_b[] = { 'B' }; + KvRec* rec; + int moved = 0; + int rc; + + install_mock_net(&net); + XMEMSET(&broker, 0, sizeof(broker)); + ASSERT_EQ(MQTT_CODE_SUCCESS, MqttBroker_Init(&broker, &net)); + ASSERT_EQ(MQTT_CODE_SUCCESS, takeover_setup_persist(&broker, &hooks)); + ASSERT_EQ(MQTT_CODE_SUCCESS, BrokerPersist_Restore(&broker)); + ASSERT_EQ(MQTT_CODE_SUCCESS, BrokerPersist_PutOrphanSession(&broker, "A", + MQTT_CONNECT_PROTOCOL_LEVEL_4, 3600, 0)); + MqttBroker_Free(&broker); + + /* Relocate the unchanged ciphertext from key "A" to key "B". */ + rec = kv_find(BROKER_PERSIST_NS_SESSION, key_a, (word16)sizeof(key_a)); + if (rec != NULL) { + XMEMCPY(rec->key, key_b, sizeof(key_b)); + moved = 1; + } + ASSERT_TRUE(moved); + ASSERT_NOT_NULL(kv_find(BROKER_PERSIST_NS_SESSION, key_b, + (word16)sizeof(key_b))); + + XMEMSET(&broker2, 0, sizeof(broker2)); + ASSERT_EQ(MQTT_CODE_SUCCESS, MqttBroker_Init(&broker2, &net)); + ASSERT_EQ(MQTT_CODE_SUCCESS, MqttBroker_SetPersistHooks(&broker2, &hooks)); + rc = BrokerPersist_Restore(&broker2); + moved = (broker2.orphan_sessions != NULL); + MqttBroker_Free(&broker2); + + ASSERT_EQ(MQTT_CODE_SUCCESS, rc); + /* The relocated record was rejected, so no Session was restored. */ + ASSERT_FALSE(moved); +} + +/* Migration: a store encrypted before the storage key was bound has a META + * whose tag covers only the 12-byte header. Built here directly with + * wolfCrypt, it must fail authentication under the new additional data, so + * restore takes the schema-mismatch path: wipe every record, then stamp a + * META the current build can read. */ +TEST(persist_header_only_aad_store_is_wiped) +{ + MqttBroker broker; + MqttBroker broker2; + MqttBrokerNet net; + MqttBrokerPersistHooks hooks; + Aes aes; + byte key[32]; + byte old_meta[12 + 12 + 4 + 16]; + byte body[4]; + static const byte meta_key[] = { 0x00 }; + static const byte outq_key[] = { 'C', 0x00, 0x00, 0x01 }; + static const byte blob[] = { 0x00 }; + KvRec* meta; + int enc_rc; + int rc; + int rc2; + int outq_left; + int meta_changed; + + /* Header: "WMQB", schema version, rec_kind META, wrap AES-GCM, body + * length 4. Body: schema version as a 32-bit big-endian value. */ + XMEMSET(old_meta, 0, sizeof(old_meta)); + old_meta[0] = WOLFMQTT_BROKER_PERSIST_MAGIC0; + old_meta[1] = WOLFMQTT_BROKER_PERSIST_MAGIC1; + old_meta[2] = WOLFMQTT_BROKER_PERSIST_MAGIC2; + old_meta[3] = WOLFMQTT_BROKER_PERSIST_MAGIC3; + old_meta[4] = (byte)(WOLFMQTT_BROKER_PERSIST_SCHEMA_VER >> 8); + old_meta[5] = (byte)(WOLFMQTT_BROKER_PERSIST_SCHEMA_VER & 0xFF); + old_meta[6] = BROKER_PERSIST_NS_META; + old_meta[7] = WOLFMQTT_BROKER_PERSIST_WRAP_AES_GCM; + old_meta[11] = 4; + body[0] = 0; + body[1] = 0; + body[2] = (byte)(WOLFMQTT_BROKER_PERSIST_SCHEMA_VER >> 8); + body[3] = (byte)(WOLFMQTT_BROKER_PERSIST_SCHEMA_VER & 0xFF); + /* Nonce bytes 12..23 stay zero. Key matches persist_order_derive_key. */ + XMEMSET(key, 0x11, sizeof(key)); + enc_rc = wc_AesInit(&aes, NULL, INVALID_DEVID); + if (enc_rc == 0) { + enc_rc = wc_AesGcmSetKey(&aes, key, (word32)sizeof(key)); + if (enc_rc == 0) { + enc_rc = wc_AesGcmEncrypt(&aes, &old_meta[24], body, + (word32)sizeof(body), &old_meta[12], 12, &old_meta[28], 16, + old_meta, 12); + } + wc_AesFree(&aes); + } + ASSERT_EQ(0, enc_rc); + + install_mock_net(&net); + XMEMSET(&broker, 0, sizeof(broker)); + ASSERT_EQ(MQTT_CODE_SUCCESS, MqttBroker_Init(&broker, &net)); + ASSERT_EQ(MQTT_CODE_SUCCESS, takeover_setup_persist(&broker, &hooks)); + (void)kv_put(NULL, BROKER_PERSIST_NS_META, meta_key, + (word16)sizeof(meta_key), old_meta, (word32)sizeof(old_meta)); + (void)kv_put(NULL, BROKER_PERSIST_NS_OUTQ, outq_key, + (word16)sizeof(outq_key), blob, (word32)sizeof(blob)); + + rc = BrokerPersist_Restore(&broker); + outq_left = kv_outq_count(); + meta = kv_find(BROKER_PERSIST_NS_META, meta_key, + (word16)sizeof(meta_key)); + meta_changed = (meta != NULL && (meta->blob_len != sizeof(old_meta) || + XMEMCMP(meta->blob, old_meta, sizeof(old_meta)) != 0)); + MqttBroker_Free(&broker); + + /* The restamped META is readable by the current build: a second start + * restores normally. */ + XMEMSET(&broker2, 0, sizeof(broker2)); + ASSERT_EQ(MQTT_CODE_SUCCESS, MqttBroker_Init(&broker2, &net)); + ASSERT_EQ(MQTT_CODE_SUCCESS, MqttBroker_SetPersistHooks(&broker2, &hooks)); + rc2 = BrokerPersist_Restore(&broker2); + MqttBroker_Free(&broker2); + + ASSERT_EQ(MQTT_CODE_SUCCESS, rc); + ASSERT_EQ(0, outq_left); + ASSERT_TRUE(meta_changed); + ASSERT_EQ(MQTT_CODE_SUCCESS, rc2); +} +#endif /* WOLFMQTT_BROKER_PERSIST_ENCRYPT */ + TEST(persist_takeover_resumes_outbound_qos1_queue) { takeover_resumes_outbound_qos1_queue(1); @@ -10197,6 +12311,18 @@ static void persist_pump(PersistFixture* f, int steps) } } +#ifdef WOLFMQTT_BROKER_PERSIST_ENCRYPT +/* Encrypt-at-rest needs a key before the first record is written. A fixed + * key is enough here: the fixture checks record lifetimes, not secrecy. */ +static int persist_fixture_derive_key(void* ctx, byte* out_key, + word32 key_len) +{ + (void)ctx; + XMEMSET(out_key, 0x22, key_len); + return MQTT_CODE_SUCCESS; +} +#endif + /* Bring a broker up over whatever the backend currently holds. */ static int persist_up(PersistFixture* f) { @@ -10208,6 +12334,9 @@ static int persist_up(PersistFixture* f) f->hooks.kv_del = kv_del; f->hooks.kv_iter = kv_iter; f->hooks.sync = kv_sync; +#ifdef WOLFMQTT_BROKER_PERSIST_ENCRYPT + f->hooks.derive_key = persist_fixture_derive_key; +#endif reset_mock_clients(1); if (MqttBroker_Init(&f->broker, &f->net) != MQTT_CODE_SUCCESS || MqttBroker_SetPersistHooks(&f->broker, &f->hooks) != @@ -10449,6 +12578,15 @@ int main(int argc, char** argv) RUN_TEST(connect_v5_client_id_alloc_failure_refused); #endif #endif +#if !defined(WOLFMQTT_STATIC_MEMORY) && defined(WOLFMQTT_BROKER_WILL) + RUN_TEST(connect_will_topic_alloc_failure_refused); + RUN_TEST(connect_will_payload_alloc_failure_refused); + RUN_TEST(connect_v311_resume_will_alloc_failure_leaves_no_session); +#ifdef WOLFMQTT_V5 + RUN_TEST(connect_v5_will_alloc_failure_leaves_no_session); +#endif + RUN_TEST(connect_will_alloc_failure_keeps_incumbent_session); +#endif #if defined(WOLFMQTT_V5) && defined(WOLFMQTT_BROKER_WILL) RUN_TEST(connect_v5_oversize_will_payload_emits_connack); #ifdef WOLFMQTT_STATIC_MEMORY @@ -10456,6 +12594,9 @@ int main(int argc, char** argv) #endif #endif #ifdef WOLFMQTT_STATIC_MEMORY +#ifdef WOLFMQTT_V5 + RUN_TEST(static_quota_refusal_connack_has_no_properties); +#endif RUN_TEST(static_fanout_does_not_reuse_unacked_packet_id); RUN_TEST(static_retained_releases_packet_id_on_write_failure); RUN_TEST(static_fanout_drops_delivery_when_ids_exhausted); @@ -10516,6 +12657,19 @@ int main(int argc, char** argv) RUN_TEST(online_qos1_flood_disconnects_slow_v311_subscriber); RUN_TEST(online_qos1_at_cap_keeps_subscriber); RUN_TEST(outbound_packet_ids_are_scoped_per_session); + RUN_TEST(outbound_publish_exceeding_tx_buf_not_sent_truncated); +#ifdef WOLFMQTT_V5 + RUN_TEST(publish_qos1_exceeding_sub_tx_buf_not_acked); +#if WOLFMQTT_MAX_QOS >= 2 + RUN_TEST(publish_qos2_exceeding_sub_tx_buf_not_acked); +#endif +#endif + RUN_TEST(outbound_qos2_puback_does_not_complete_delivery); + RUN_TEST(publish_fanout_alloc_failure_v311_not_acked); + RUN_TEST(publish_offline_fanout_alloc_failure_v311_not_acked); +#ifdef WOLFMQTT_V5 + RUN_TEST(publish_fanout_alloc_failure_v5_qos2_pubrec_failure); +#endif #ifdef WOLFMQTT_NONBLOCK RUN_TEST(outbound_queue_short_write_resumes_on_next_step); #ifdef WOLFMQTT_BROKER_RETAINED @@ -10553,10 +12707,20 @@ int main(int argc, char** argv) #endif #endif RUN_TEST(broker_keepalive_expires_at_exact_deadline); -#ifndef WOLFMQTT_V5 + RUN_TEST(broker_step_reports_listener_accept_failure); +#if !defined(WOLFMQTT_V5) && defined(WOLFMQTT_BROKER_WILL) RUN_TEST(disconnect_v311_nonzero_remain_len_fires_will); #endif +#ifdef WOLFMQTT_BROKER_WILL RUN_TEST(disconnect_invalid_fixed_header_flags_fires_will); +#endif +#if defined(WOLFMQTT_V5) && defined(WOLFMQTT_BROKER_WILL) + RUN_TEST(disconnect_v5_malformed_fires_will); +#ifndef WOLFMQTT_DYN_PROP + RUN_TEST(disconnect_v5_props_pool_exhausted_reason_0_discards_will); + RUN_TEST(disconnect_v5_props_pool_exhausted_reason_4_publishes_will); +#endif +#endif #if defined(WOLFMQTT_BROKER_WILL) && !defined(WOLFMQTT_STATIC_MEMORY) RUN_TEST(broker_will_scrub_after_failed_write); #ifdef WOLFMQTT_NONBLOCK @@ -10619,6 +12783,12 @@ int main(int argc, char** argv) RUN_TEST(connack_session_present_clear_on_clean_session_reconnect); #ifdef WOLFMQTT_V5 RUN_TEST(connack_session_present_v5_set_on_resumed_session); + RUN_TEST(broker_shared_subscription_not_supported_v5); +#ifndef WOLFMQTT_DYN_PROP + RUN_TEST(connect_v5_refused_when_shared_sub_property_unavailable); + RUN_TEST(connect_v5_refused_when_receive_max_property_unavailable); + RUN_TEST(connect_v5_connack_property_failure_keeps_incumbent_session); +#endif #endif #ifndef WOLFMQTT_STATIC_MEMORY RUN_TEST(broker_suback_reserved_v311_code_rejected); @@ -10650,6 +12820,7 @@ int main(int argc, char** argv) #ifdef WOLFMQTT_V5 #ifndef WOLFMQTT_STATIC_MEMORY RUN_TEST(connect_v5_receive_max_zero_protocol_error); + RUN_TEST(disconnect_v5_session_expiry_raise_sends_protocol_error); #endif #endif #ifdef WOLFMQTT_V5 @@ -10671,6 +12842,9 @@ int main(int argc, char** argv) #ifndef WOLFMQTT_STATIC_MEMORY #ifdef WOLFMQTT_BROKER_WILL RUN_TEST(disconnect_v5_session_expiry_0_to_nonzero_protocol_error); +#ifdef WOLFMQTT_NONBLOCK + RUN_TEST(disconnect_v5_session_expiry_protocol_error_short_write_completes); +#endif #endif #endif #endif @@ -10735,6 +12909,15 @@ int main(int argc, char** argv) #endif #ifdef WOLFMQTT_BROKER_PERSIST #ifndef WOLFMQTT_STATIC_MEMORY + RUN_TEST(persist_commit_reports_sync_failure); + RUN_TEST(persist_del_outqueue_reports_failures); + RUN_TEST(persist_schema_wipe_failure_keeps_old_meta); + RUN_TEST(persist_restore_alloc_failure_is_fatal); + RUN_TEST(persist_restore_skips_sessions_over_cap); +#ifdef WOLFMQTT_BROKER_PERSIST_ENCRYPT + RUN_TEST(persist_encrypted_record_bound_to_storage_key); + RUN_TEST(persist_header_only_aad_store_is_wiped); +#endif RUN_TEST(persist_takeover_resumes_outbound_qos1_queue); #if WOLFMQTT_MAX_QOS >= 2 RUN_TEST(persist_takeover_resumes_outbound_qos2_pubrel); @@ -10758,6 +12941,9 @@ int main(int argc, char** argv) RUN_TEST(persist_root_readable_permissions_accepted); RUN_TEST(persist_root_writable_permissions_rejected); RUN_TEST(persist_put_rejects_existing_temp_symlink); + RUN_TEST(persist_put_del_report_namespace_fsync_failure); + RUN_TEST(persist_iter_reports_unreadable_record); + RUN_TEST(persist_iter_skips_socket_and_symlink_entries); #ifdef WOLFMQTT_BROKER_PERSIST_ENCRYPT RUN_TEST(persist_failed_key_derivation_scrubs_cache); RUN_TEST(persist_bad_tag_scrubs_plaintext_before_free); diff --git a/tests/test_mqtt_client.c b/tests/test_mqtt_client.c index 1cea25033..bfa74e224 100644 --- a/tests/test_mqtt_client.c +++ b/tests/test_mqtt_client.c @@ -1666,6 +1666,119 @@ TEST(publish_qos1_v5_receive_max_quota_decrements_once_and_replenishes) #endif /* WOLFMQTT_MAX_QOS >= 1 */ #endif /* WOLFMQTT_NONBLOCK */ +#ifdef WOLFMQTT_NONBLOCK +/* Cancelling a wait that is part-way through receiving a packet must scrub + * rx_buf, as cancelling an active write already scrubs tx_buf: the partial + * packet can carry application data or v5 AUTH authentication data. */ +TEST(cancel_message_active_read_scrubs_rx_buf) +{ + int rc; + int i; + /* v3.1.1 QoS 0 PUBLISH declaring Remaining Length 20 (topic "t" and 17 + * payload bytes); only the first 10 bytes arrive before the read would + * block. */ + static const byte partial[] = { + 0x30, 0x14, 0x00, 0x01, 't', 'S', 'E', 'C', 'R', 'E' + }; + + rc = test_init_client(); + ASSERT_EQ(MQTT_CODE_SUCCESS, rc); +#ifdef WOLFMQTT_V5 + test_client.protocol_level = MQTT_CONNECT_PROTOCOL_LEVEL_4; +#endif + test_net.read = mock_net_read_canned; + XMEMCPY(g_canned_buf, partial, sizeof(partial)); + g_canned_len = (int)sizeof(partial); + g_canned_pos = 0; + + rc = MqttClient_WaitMessage(&test_client, TEST_CMD_TIMEOUT_MS); + ASSERT_EQ(MQTT_CODE_CONTINUE, rc); + ASSERT_TRUE(test_client.msg.stat.isReadActive); + + rc = MqttClient_CancelMessage(&test_client, &test_client.msg); + ASSERT_EQ(MQTT_CODE_SUCCESS, rc); + ASSERT_EQ(0, test_client.read.isActive); + for (i = 0; i < TEST_RX_BUF_SIZE; i++) { + ASSERT_EQ(0, test_rx_buf[i]); + } +} +#endif /* WOLFMQTT_NONBLOCK */ + +#ifdef WOLFMQTT_NONBLOCK +/* Accepts the first few bytes of a packet, then reports would-block. */ +static int g_write_partial_calls; +static int mock_net_write_partial_then_block(void *context, const byte* buf, + int buf_len, int timeout_ms) +{ + (void)context; (void)buf; (void)timeout_ms; + if (g_write_partial_calls++ == 0) { + return (buf_len < 5) ? buf_len : 5; + } + return MQTT_CODE_CONTINUE; +} + +/* Abandoning a connection with MqttClient_NetDisconnect while a nonblocking + * CONNECT is only partly written must not leave the serialized credentials + * in tx_buf, nor received peer data in rx_buf, once the transport is gone. + * With multiple threads the still-active write owns tx_buf, so NetDisconnect + * leaves it and releasing the write with MqttClient_CancelMessage scrubs it. */ +TEST(net_disconnect_scrubs_partial_connect_credentials) +{ + int rc; + int i; + int tx_clear; + MqttConnect connect; + + rc = test_init_client(); + ASSERT_EQ(MQTT_CODE_SUCCESS, rc); +#ifdef WOLFMQTT_V5 + test_client.protocol_level = MQTT_CONNECT_PROTOCOL_LEVEL_4; +#endif + (void)MqttClient_Flags(&test_client, 0, MQTT_CLIENT_FLAG_IS_CONNECTED); + test_net.write = mock_net_write_partial_then_block; + test_net.read = mock_net_read_wouldblock; + test_net.disconnect = mock_net_disconnect; + g_write_partial_calls = 0; + XMEMSET(test_rx_buf, 0xAB, sizeof(test_rx_buf)); + + XMEMSET(&connect, 0, sizeof(connect)); + connect.keep_alive_sec = 60; + connect.clean_session = 1; + connect.client_id = "c"; + connect.username = "user"; + connect.password = "hunter2"; + connect.password_len = 7; + + rc = MqttClient_Connect(&test_client, &connect); + ASSERT_EQ(MQTT_CODE_CONTINUE, rc); + + rc = MqttClient_NetDisconnect(&test_client); + ASSERT_EQ(MQTT_CODE_SUCCESS, rc); + for (i = 0; i < TEST_RX_BUF_SIZE; i++) { + ASSERT_EQ(0, test_rx_buf[i]); + } + tx_clear = 1; + for (i = 0; i < TEST_TX_BUF_SIZE; i++) { + if (test_tx_buf[i] != 0) { + tx_clear = 0; + } + } +#ifdef WOLFMQTT_MULTITHREAD + /* The active write's owner still holds tx_buf. */ + ASSERT_FALSE(tx_clear); +#else + ASSERT_TRUE(tx_clear); +#endif + + /* Release the abandoned write as an application would. */ + rc = MqttClient_CancelMessage(&test_client, (MqttObject*)&connect); + ASSERT_EQ(MQTT_CODE_SUCCESS, rc); + for (i = 0; i < TEST_TX_BUF_SIZE; i++) { + ASSERT_EQ(0, test_tx_buf[i]); + } +} +#endif /* WOLFMQTT_NONBLOCK */ + /* These two exercise MqttClient_CancelMessage, which is only a public API when * WOLFMQTT_MULTITHREAD or WOLFMQTT_NONBLOCK is defined (otherwise it is a * file-local static in mqtt_client.c). */ @@ -3525,10 +3638,9 @@ TEST(reconnect_drops_replay_larger_than_tx_buf) } } -/* A retained entry the client cannot rebuild is dropped, so the pool bounds - * how many messages survive a reconnect. Publishing more than - * MQTT_MAX_REPLAY_MSGS leaves the extras unretained; they are still sent, they - * just cannot be replayed. */ +/* The pool bounds how many messages survive a reconnect. On this + * CleanSession 0 connection a publish beyond MQTT_MAX_REPLAY_MSGS is refused + * before it is sent, since it could not be replayed [MQTT-4.4.0-1]. */ TEST(reconnect_replays_at_most_pool_size) { int rc; @@ -3551,7 +3663,12 @@ TEST(reconnect_replays_at_most_pool_size) init_qos_publish(&publish, MQTT_QOS_1, (word16)(0x100 + i), "sensor/temp", payload, (word32)(sizeof(payload) - 1)); rc = run_publish_unacked(&publish); - ASSERT_EQ(MQTT_CODE_ERROR_NETWORK, rc); + if (i < MQTT_MAX_REPLAY_MSGS) { + ASSERT_EQ(MQTT_CODE_ERROR_NETWORK, rc); + } + else { + ASSERT_EQ(MQTT_CODE_ERROR_OUT_OF_BUFFER, rc); + } } ASSERT_EQ(MQTT_CODE_SUCCESS, MqttClient_NetDisconnect(&test_client)); @@ -4071,6 +4188,145 @@ TEST(replay_resume_arms_keep_alive) ASSERT_EQ(60, (int)test_client.keep_alive_sec); } #endif /* WOLFMQTT_NONBLOCK */ +/* Publish MQTT_MAX_REPLAY_MSGS unacknowledged QoS 1 messages, filling the + * replay pool, then attempt one more. Returns the result of the extra + * publish; *frames_out reports whether it reached the wire. */ +static int run_publish_past_replay_pool(int* frames_out) +{ + int rc; + int i; + int frames_before; + MqttPublish publish[MQTT_MAX_REPLAY_MSGS + 1]; + static byte payload[] = "hello"; + + for (i = 0; i < MQTT_MAX_REPLAY_MSGS; i++) { + init_qos_publish(&publish[i], MQTT_QOS_1, (word16)(i + 1), "t", + payload, (word32)(sizeof(payload) - 1)); + rc = run_publish_unacked(&publish[i]); + if (rc != MQTT_CODE_ERROR_NETWORK) { + return rc; + } + } + init_qos_publish(&publish[i], MQTT_QOS_1, (word16)(i + 1), "t", + payload, (word32)(sizeof(payload) - 1)); + frames_before = g_frames_written; + rc = run_publish_unacked(&publish[i]); + *frames_out = g_frames_written - frames_before; + return rc; +} + +/* [MQTT-4.4.0-1] requires every unacknowledged QoS > 0 PUBLISH to be re-sent + * when a persistent Session resumes. With the replay pool full there is no + * copy to re-send, so on a CleanSession 0 connection the publish is refused + * before it reaches the wire instead of being sent unrecoverable. */ +TEST(publish_refused_when_replay_pool_full_on_persistent_session) +{ + int rc; + int frames = -1; + MqttConnect connect; + + rc = test_init_client(); + ASSERT_EQ(MQTT_CODE_SUCCESS, rc); +#ifdef WOLFMQTT_V5 + test_client.protocol_level = MQTT_CONNECT_PROTOCOL_LEVEL_4; +#endif + ASSERT_EQ(MQTT_CODE_SUCCESS, run_initial_connect(&connect)); + + rc = run_publish_past_replay_pool(&frames); + + ASSERT_EQ(MQTT_CODE_ERROR_OUT_OF_BUFFER, rc); + ASSERT_EQ(0, frames); +} + +#ifdef WOLFMQTT_V5 +/* MQTT 5.0 section 3.2.2.3.2: a Session Expiry Interval in CONNACK replaces + * the one requested in CONNECT. When the server negotiates it down to 0 the + * Session ends with the connection, so the replay pool must not cap how many + * messages are in flight. */ +TEST(publish_allowed_past_replay_pool_when_connack_ends_session) +{ + int rc; + int i; + int frames = -1; + MqttConnect connect; + MqttProp* prop; + /* v5 CONNACK: flags 0, reason 0, Properties length 5: Session Expiry + * Interval (0x11) = 0. Remaining Length 8. */ + static const byte connack[] = { + 0x20, 0x08, 0x00, 0x00, 0x05, 0x11, 0x00, 0x00, 0x00, 0x00 + }; + + rc = test_init_client(); + ASSERT_EQ(MQTT_CODE_SUCCESS, rc); + test_client.protocol_level = MQTT_CONNECT_PROTOCOL_LEVEL_5; + test_net.write = mock_net_write_accept; + test_net.read = mock_net_read_canned; + XMEMCPY(g_canned_buf, connack, sizeof(connack)); + g_canned_len = (int)sizeof(connack); + g_canned_pos = 0; + + XMEMSET(&connect, 0, sizeof(connect)); + connect.keep_alive_sec = 60; + connect.clean_session = 0; + connect.client_id = "replay_client"; + connect.protocol_level = MQTT_CONNECT_PROTOCOL_LEVEL_5; + prop = MqttProps_Add(&connect.props); + ASSERT_NOT_NULL(prop); + prop->type = MQTT_PROP_SESSION_EXPIRY_INTERVAL; + prop->data_int = 60; + + rc = MQTT_CODE_CONTINUE; + for (i = 0; i < 20 && rc == MQTT_CODE_CONTINUE; i++) { + rc = MqttClient_Connect(&test_client, &connect); + } + (void)MqttProps_Free(connect.props); + connect.props = NULL; + ASSERT_EQ(MQTT_CODE_SUCCESS, rc); + + rc = run_publish_past_replay_pool(&frames); + + ASSERT_EQ(MQTT_CODE_ERROR_NETWORK, rc); + ASSERT_EQ(1, frames); +} +#endif /* WOLFMQTT_V5 */ + +/* A CleanSession 1 Session ends with the Network Connection, so nothing is + * ever replayed and a full pool must not limit how many messages are in + * flight. */ +TEST(publish_allowed_past_replay_pool_on_clean_session) +{ + int rc; + int i; + int frames = -1; + MqttConnect connect; + static const byte connack[] = { 0x20, 0x02, 0x00, 0x00 }; + + rc = test_init_client(); + ASSERT_EQ(MQTT_CODE_SUCCESS, rc); +#ifdef WOLFMQTT_V5 + test_client.protocol_level = MQTT_CONNECT_PROTOCOL_LEVEL_4; +#endif + test_net.write = mock_net_write_accept; + test_net.read = mock_net_read_canned; + XMEMCPY(g_canned_buf, connack, sizeof(connack)); + g_canned_len = (int)sizeof(connack); + g_canned_pos = 0; + XMEMSET(&connect, 0, sizeof(connect)); + connect.keep_alive_sec = 60; + connect.clean_session = 1; + connect.client_id = "replay_client"; + rc = MQTT_CODE_CONTINUE; + for (i = 0; i < 20 && rc == MQTT_CODE_CONTINUE; i++) { + rc = MqttClient_Connect(&test_client, &connect); + } + ASSERT_EQ(MQTT_CODE_SUCCESS, rc); + + rc = run_publish_past_replay_pool(&frames); + + ASSERT_EQ(MQTT_CODE_ERROR_NETWORK, rc); + ASSERT_EQ(1, frames); +} + #endif /* !WOLFMQTT_NO_SESSION_REPLAY && WOLFMQTT_MAX_QOS >= 1 */ @@ -6529,6 +6785,122 @@ TEST(wait_message_v5_topic_alias_with_topic_rejected) * acknowledge path must be unchanged. Without this, every new test runs with a * NULL callback and only asserts the FALSE direction of g_pubresp_written, so a * regression that suppressed ACKs outright would go unnoticed. */ +#ifdef WOLFMQTT_V5 +/* Rejects the QoS 1 message (packet_id 7) with a Reason Code that is not valid + * for PUBACK, and counts every delivery. */ +static int g_bad_ack_cb_calls; +static int test_bad_puback_reason_cb(MqttClient* client, MqttMessage* msg, + byte msg_new, byte msg_done) +{ + (void)client; (void)msg_new; (void)msg_done; + g_bad_ack_cb_calls++; + if (msg->packet_id == 7) { + /* 0x01 is not in the PUBACK Reason Code table (MQTT 5.0 3.4.2.1). */ + msg->resp.reason_code = 0x01; + } + return MQTT_CODE_SUCCESS; +} + +/* A local failure to encode the automatic PUBACK must not leave the wait object + * parked in the ack state: the next wait has to read new packets instead of + * retrying the same invalid acknowledgement forever. */ +TEST(wait_message_ack_encode_failure_resets_wait_state) +{ + int rc; + int i; + /* v5 QoS 1 PUBLISH: remain=9, topic "a", packet_id 7, props len 0, + * payload "h". Then v5 QoS 0 PUBLISH: remain=5, topic "b", props len 0, + * payload "z". */ + static const byte frames[] = { + 0x32, 0x07, 0x00, 0x01, 'a', 0x00, 0x07, 0x00, 'h', + 0x30, 0x05, 0x00, 0x01, 'b', 0x00, 'z' + }; + + rc = test_init_client(); + ASSERT_EQ(MQTT_CODE_SUCCESS, rc); + test_client.protocol_level = MQTT_CONNECT_PROTOCOL_LEVEL_5; + test_client.msg_cb = test_bad_puback_reason_cb; + g_bad_ack_cb_calls = 0; + + g_pubresp_written = 0; + test_net.write = mock_net_write_accept; + test_net.read = mock_net_read_canned; + XMEMCPY(g_canned_buf, frames, sizeof(frames)); + g_canned_len = (int)sizeof(frames); + g_canned_pos = 0; + + rc = MQTT_CODE_CONTINUE; + for (i = 0; i < 20 && rc == MQTT_CODE_CONTINUE; i++) { + rc = MqttClient_WaitMessage(&test_client, TEST_CMD_TIMEOUT_MS); + } + ASSERT_EQ(MQTT_CODE_ERROR_PROPERTY, rc); + ASSERT_EQ(1, g_bad_ack_cb_calls); + ASSERT_EQ(0, g_pubresp_written); + ASSERT_EQ(MQTT_MSG_BEGIN, test_client.msg.stat.read); + ASSERT_EQ(MQTT_MSG_BEGIN, test_client.msg.stat.ack); + ASSERT_EQ(0, test_client.msg.stat.isWriteActive); + + rc = MQTT_CODE_CONTINUE; + for (i = 0; i < 20 && rc == MQTT_CODE_CONTINUE; i++) { + rc = MqttClient_WaitMessage(&test_client, TEST_CMD_TIMEOUT_MS); + } + ASSERT_EQ(MQTT_CODE_SUCCESS, rc); + ASSERT_EQ(2, g_bad_ack_cb_calls); +} +#endif /* WOLFMQTT_V5 */ + +/* Serves g_canned_buf, then fails the connection. */ +static int mock_net_read_canned_then_error(void *context, byte* buf, + int buf_len, int timeout_ms) +{ + int n = g_canned_len - g_canned_pos; + (void)context; (void)timeout_ms; + if (n <= 0) { + return MQTT_CODE_ERROR_NETWORK; + } + if (n > buf_len) { + n = buf_len; + } + XMEMCPY(buf, g_canned_buf + g_canned_pos, n); + g_canned_pos += n; + return n; +} + +/* A transport failure after part of a packet arrived ends the receive, so + * the partial packet - which can carry v5 AUTH authentication data - must + * be scrubbed from rx_buf before the read is released, as it already is for + * protocol errors. */ +TEST(wait_message_network_error_after_partial_read_scrubs_rx_buf) +{ + int rc; + int i; + /* v3.1.1 QoS 0 PUBLISH declaring Remaining Length 20; only the first 10 + * bytes arrive before the connection fails. */ + static const byte partial[] = { + 0x30, 0x14, 0x00, 0x01, 't', 'S', 'E', 'C', 'R', 'E' + }; + + rc = test_init_client(); + ASSERT_EQ(MQTT_CODE_SUCCESS, rc); +#ifdef WOLFMQTT_V5 + test_client.protocol_level = MQTT_CONNECT_PROTOCOL_LEVEL_4; +#endif + test_net.read = mock_net_read_canned_then_error; + XMEMCPY(g_canned_buf, partial, sizeof(partial)); + g_canned_len = (int)sizeof(partial); + g_canned_pos = 0; + + rc = MQTT_CODE_CONTINUE; + for (i = 0; i < 10 && rc == MQTT_CODE_CONTINUE; i++) { + rc = MqttClient_WaitMessage(&test_client, TEST_CMD_TIMEOUT_MS); + } + ASSERT_EQ(MQTT_CODE_ERROR_NETWORK, rc); + ASSERT_EQ(0, test_client.read.isActive); + for (i = 0; i < TEST_RX_BUF_SIZE; i++) { + ASSERT_EQ(0, test_rx_buf[i]); + } +} + TEST(wait_message_qos1_with_msg_cb_delivers_and_acks) { int rc; @@ -7543,6 +7915,74 @@ TEST(client_flags_set_clear) ASSERT_EQ(0, (int)(flags & MQTT_CLIENT_FLAG_IS_CONNECTED)); } +#ifndef WOLFMQTT_NONBLOCK +/* Serves g_canned_buf, then returns 0 (orderly peer close) until a cap, after + * which it returns a hard error so a client stuck retrying cannot hang the + * test. */ +static int g_zero_reads; +static int mock_net_read_canned_then_close(void *context, byte* buf, + int buf_len, int timeout_ms) +{ + int n = g_canned_len - g_canned_pos; + (void)context; (void)timeout_ms; + if (n <= 0) { + g_zero_reads++; + return (g_zero_reads > 5) ? MQTT_CODE_ERROR_NETWORK : 0; + } + if (n > buf_len) { + n = buf_len; + } + XMEMCPY(buf, g_canned_buf + g_canned_pos, n); + g_canned_pos += n; + return n; +} + +static int mock_msg_cb_ok(MqttClient* client, MqttMessage* msg, + byte msg_new, byte msg_done) +{ + (void)client; (void)msg; (void)msg_new; (void)msg_done; + return MQTT_CODE_SUCCESS; +} + +/* A PUBLISH whose payload exceeds rx_buf is read in chunks. If the peer closes + * the connection mid-payload the continuation read returns 0; that must end + * the wait with a network error instead of retrying the same read forever. + * MQTT 3.1.1 section 2.2.3: Remaining Length counts every remaining byte. */ +TEST(publish_payload_zero_read_returns_network_error) +{ + int rc; + /* Static so teardown's DeInit never sees a dangling rx_buf. */ + static byte small_rx[16]; + /* v3.1.1 QoS 0 PUBLISH, Remaining Length 43 (0x2B): topic length 0x0001, + * topic "t", 40 declared payload bytes. Only 20 payload bytes follow. */ + static const byte frame[] = { + 0x30, 0x2B, 0x00, 0x01, 't', + 'p', 'p', 'p', 'p', 'p', 'p', 'p', 'p', 'p', 'p', + 'p', 'p', 'p', 'p', 'p', 'p', 'p', 'p', 'p', 'p' + }; + + rc = MqttClient_Init(&test_client, &test_net, mock_msg_cb_ok, + test_tx_buf, TEST_TX_BUF_SIZE, small_rx, (int)sizeof(small_rx), + TEST_CMD_TIMEOUT_MS); + ASSERT_EQ(MQTT_CODE_SUCCESS, rc); + test_client_inited = 1; +#ifdef WOLFMQTT_V5 + test_client.protocol_level = MQTT_CONNECT_PROTOCOL_LEVEL_4; +#endif + + test_net.read = mock_net_read_canned_then_close; + XMEMCPY(g_canned_buf, frame, sizeof(frame)); + g_canned_len = (int)sizeof(frame); + g_canned_pos = 0; + g_zero_reads = 0; + + rc = MqttClient_WaitMessage(&test_client, TEST_CMD_TIMEOUT_MS); + + ASSERT_EQ(MQTT_CODE_ERROR_NETWORK, rc); + ASSERT_EQ(1, g_zero_reads); +} +#endif /* !WOLFMQTT_NONBLOCK */ + /* ============================================================================ * Test Suite Runner * ============================================================================ */ @@ -7639,6 +8079,11 @@ void run_mqtt_client_tests(void) RUN_TEST(reconnect_on_reused_connect_object_waits_for_connack); #if !defined(WOLFMQTT_NO_SESSION_REPLAY) && WOLFMQTT_MAX_QOS >= 1 RUN_TEST(reconnect_replays_unacked_qos1_publish); + RUN_TEST(publish_refused_when_replay_pool_full_on_persistent_session); + RUN_TEST(publish_allowed_past_replay_pool_on_clean_session); +#ifdef WOLFMQTT_V5 + RUN_TEST(publish_allowed_past_replay_pool_when_connack_ends_session); +#endif RUN_TEST(reconnect_without_session_present_replays_nothing); RUN_TEST(reconnect_does_not_replay_acked_publish); #if WOLFMQTT_MAX_QOS >= 2 @@ -7741,6 +8186,10 @@ void run_mqtt_client_tests(void) RUN_TEST(cancel_message_retains_recv_quota_on_wire); RUN_TEST(cancel_message_retain_is_idempotent); RUN_TEST(cancel_message_reuse_does_not_bypass_recv_quota); +#endif +#ifdef WOLFMQTT_NONBLOCK + RUN_TEST(cancel_message_active_read_scrubs_rx_buf); + RUN_TEST(net_disconnect_scrubs_partial_connect_credentials); #endif RUN_TEST(publish_qos1_v5_write_failure_restores_recv_quota); #if defined(WOLFMQTT_MULTITHREAD) && defined(WOLFMQTT_NONBLOCK) && \ @@ -7790,6 +8239,10 @@ void run_mqtt_client_tests(void) RUN_TEST(wait_message_v5_topic_alias_with_topic_rejected); #endif RUN_TEST(wait_message_qos1_with_msg_cb_delivers_and_acks); + RUN_TEST(wait_message_network_error_after_partial_read_scrubs_rx_buf); +#ifdef WOLFMQTT_V5 + RUN_TEST(wait_message_ack_encode_failure_resets_wait_state); +#endif #ifdef WOLFMQTT_NONBLOCK RUN_TEST(wait_message_timeout_preserves_partial_vbi); #endif @@ -7853,6 +8306,9 @@ void run_mqtt_client_tests(void) /* MqttClient_Flags tests */ RUN_TEST(client_flags_set_clear); +#ifndef WOLFMQTT_NONBLOCK + RUN_TEST(publish_payload_zero_read_returns_network_error); +#endif TEST_SUITE_END(); } diff --git a/tests/test_mqtt_packet.c b/tests/test_mqtt_packet.c index b27cea9c4..d85fb6cbd 100644 --- a/tests/test_mqtt_packet.c +++ b/tests/test_mqtt_packet.c @@ -2002,6 +2002,125 @@ TEST(decode_connack_v5_rejects_bytes_after_property_block) } #endif /* WOLFMQTT_V5 */ +#ifdef WOLFMQTT_V5 +/* MQTT 5.0 sections 3.4.2 to 3.7.2: a PUBACK, PUBREC, PUBREL or PUBCOMP has + * no payload, so Packet Identifier, Reason Code, Property Length and + * Properties must consume the Remaining Length exactly. */ +TEST(decode_puback_v5_rejects_bytes_after_property_block) +{ + /* Remaining Length 5: packet_id 0x0001, Reason 0x00, Property Length 0, + * then one trailing byte. */ + byte buf[] = { 0x40, 0x05, 0x00, 0x01, MQTT_REASON_SUCCESS, 0x00, 0xFF }; + MqttPublishResp resp; + int rc; + + XMEMSET(&resp, 0, sizeof(resp)); + resp.protocol_level = MQTT_CONNECT_PROTOCOL_LEVEL_5; + + rc = MqttDecode_PublishResp(buf, (int)sizeof(buf), + MQTT_PACKET_TYPE_PUBLISH_ACK, &resp); + + ASSERT_EQ(MQTT_CODE_ERROR_MALFORMED_DATA, rc); + ASSERT_NULL(resp.props); +} + +/* The Property Length belongs to this PUBACK; bytes after its Remaining + * Length cannot satisfy it. The trailing bytes form a valid empty Reason + * String property deliberately. */ +TEST(decode_puback_v5_props_cannot_cross_packet_end) +{ + byte buf[] = { + 0x40, 0x04, 0x00, 0x01, MQTT_REASON_SUCCESS, 0x03, + MQTT_PROP_REASON_STR, 0x00, 0x00 + }; + MqttPublishResp resp; + int rc; + + XMEMSET(&resp, 0, sizeof(resp)); + resp.protocol_level = MQTT_CONNECT_PROTOCOL_LEVEL_5; + + rc = MqttDecode_PublishResp(buf, (int)sizeof(buf), + MQTT_PACKET_TYPE_PUBLISH_ACK, &resp); + + ASSERT_EQ(MQTT_CODE_ERROR_OUT_OF_BUFFER, rc); + ASSERT_NULL(resp.props); +} + +/* MQTT 5.0 section 3.14.3: DISCONNECT has no payload. */ +TEST(decode_disconnect_v5_rejects_bytes_after_property_block) +{ + /* Remaining Length 3: Reason 0x00, Property Length 0, one trailing + * byte. */ + byte buf[] = { 0xE0, 0x03, MQTT_REASON_SUCCESS, 0x00, 0xFF }; + MqttDisconnect disc; + int rc; + + XMEMSET(&disc, 0, sizeof(disc)); + disc.protocol_level = MQTT_CONNECT_PROTOCOL_LEVEL_5; + + rc = MqttDecode_Disconnect(buf, (int)sizeof(buf), &disc); + + ASSERT_EQ(MQTT_CODE_ERROR_MALFORMED_DATA, rc); + ASSERT_NULL(disc.props); +} + +TEST(decode_disconnect_v5_props_cannot_cross_packet_end) +{ + byte buf[] = { + 0xE0, 0x02, MQTT_REASON_SUCCESS, 0x03, + MQTT_PROP_REASON_STR, 0x00, 0x00 + }; + MqttDisconnect disc; + int rc; + + XMEMSET(&disc, 0, sizeof(disc)); + disc.protocol_level = MQTT_CONNECT_PROTOCOL_LEVEL_5; + + rc = MqttDecode_Disconnect(buf, (int)sizeof(buf), &disc); + + ASSERT_EQ(MQTT_CODE_ERROR_OUT_OF_BUFFER, rc); + ASSERT_NULL(disc.props); +} + +/* MQTT 5.0 section 3.15.3: AUTH has no payload. */ +TEST(decode_auth_v5_rejects_bytes_after_property_block) +{ + /* Remaining Length 7: Reason 0x18 (Continue authentication), Property + * Length 4, Authentication Method "x", one trailing byte. */ + byte buf[] = { + 0xF0, 0x07, MQTT_REASON_CONT_AUTH, 0x04, + MQTT_PROP_AUTH_METHOD, 0x00, 0x01, 'x', + 0xFF + }; + MqttAuth auth; + int rc; + + XMEMSET(&auth, 0, sizeof(auth)); + + rc = MqttDecode_Auth(buf, (int)sizeof(buf), &auth); + + ASSERT_EQ(MQTT_CODE_ERROR_MALFORMED_DATA, rc); + ASSERT_NULL(auth.props); +} + +TEST(decode_auth_v5_props_cannot_cross_packet_end) +{ + byte buf[] = { + 0xF0, 0x02, MQTT_REASON_CONT_AUTH, 0x04, + MQTT_PROP_AUTH_METHOD, 0x00, 0x01, 'x' + }; + MqttAuth auth; + int rc; + + XMEMSET(&auth, 0, sizeof(auth)); + + rc = MqttDecode_Auth(buf, (int)sizeof(buf), &auth); + + ASSERT_EQ(MQTT_CODE_ERROR_MALFORMED_DATA, rc); + ASSERT_NULL(auth.props); +} +#endif /* WOLFMQTT_V5 */ + /* [MQTT-3.2.2-1] / [MQTT-3.2.2-4] CONNACK Flags receive-side validation. * * The Connect Acknowledge Flags byte has only bit 0 (Session Present) @@ -7015,6 +7134,14 @@ void run_mqtt_packet_tests(void) RUN_TEST(decode_connack_v5_shape_without_protocol_level_rejected); RUN_TEST(decode_connack_v5_props_cannot_cross_packet_end); RUN_TEST(decode_connack_v5_rejects_bytes_after_property_block); +#endif +#ifdef WOLFMQTT_V5 + RUN_TEST(decode_puback_v5_rejects_bytes_after_property_block); + RUN_TEST(decode_puback_v5_props_cannot_cross_packet_end); + RUN_TEST(decode_disconnect_v5_rejects_bytes_after_property_block); + RUN_TEST(decode_disconnect_v5_props_cannot_cross_packet_end); + RUN_TEST(decode_auth_v5_rejects_bytes_after_property_block); + RUN_TEST(decode_auth_v5_props_cannot_cross_packet_end); #endif RUN_TEST(decode_connack_flags_session_present_accepted); RUN_TEST(decode_connack_flags_no_session_accepted); diff --git a/tests/test_mqtt_sn.c b/tests/test_mqtt_sn.c index 4c80e4876..112e13cc5 100644 --- a/tests/test_mqtt_sn.c +++ b/tests/test_mqtt_sn.c @@ -932,6 +932,87 @@ TEST(sn_typedesc_unknown_type) } #endif /* !WOLFMQTT_NO_ERROR_STRINGS */ +/* ============================================================================ + * MQTT-SN ClientId length + * + * MQTT-SN 1.2 section 5.3.1: the ClientId is a 1 to 23 character string. + * Encoding a longer one by truncating it would let distinct identities that + * share a 23-byte prefix reach the gateway as the same client, so the + * encoders must refuse it. + * ============================================================================ */ + +/* 23 characters: the longest permitted ClientId. */ +#define SN_TEST_ID_23 "abcdefghijklmnopqrstuvw" +/* 24 characters: one past the limit, sharing the 23-byte prefix. */ +#define SN_TEST_ID_24 "abcdefghijklmnopqrstuvwx" + +TEST(sn_encode_connect_client_id_too_long_rejected) +{ + byte buf[64]; + SN_Connect connect; + + XMEMSET(&connect, 0, sizeof(connect)); + connect.protocol_level = SN_PROTOCOL_ID; + connect.keep_alive_sec = 60; + connect.client_id = SN_TEST_ID_24; + + ASSERT_EQ(MQTT_CODE_ERROR_BAD_ARG, + SN_Encode_Connect(buf, (int)sizeof(buf), &connect)); +} + +TEST(sn_encode_connect_client_id_max_len_encoded) +{ + byte buf[64]; + SN_Connect connect; + int rc; + + XMEMSET(&connect, 0, sizeof(connect)); + connect.protocol_level = SN_PROTOCOL_ID; + connect.keep_alive_sec = 60; + connect.client_id = SN_TEST_ID_23; + + rc = SN_Encode_Connect(buf, (int)sizeof(buf), &connect); + + /* MQTT-SN 1.2 section 5.4.4: Length(1) MsgType(1) Flags(1) + * ProtocolId(1) Duration(2) ClientId(23) = 29 bytes. */ + ASSERT_EQ(29, rc); + ASSERT_EQ(29, buf[0]); + ASSERT_EQ(SN_MSG_TYPE_CONNECT, buf[1]); + ASSERT_MEM_EQ(SN_TEST_ID_23, &buf[6], 23); +} + +TEST(sn_encode_pingreq_client_id_too_long_rejected) +{ + byte buf[64]; + SN_PingReq ping; + char id[] = SN_TEST_ID_24; + + XMEMSET(&ping, 0, sizeof(ping)); + ping.clientId = id; + + ASSERT_EQ(MQTT_CODE_ERROR_BAD_ARG, + SN_Encode_Ping(buf, (int)sizeof(buf), &ping, SN_MSG_TYPE_PING_REQ)); +} + +TEST(sn_encode_pingreq_client_id_max_len_encoded) +{ + byte buf[64]; + SN_PingReq ping; + char id[] = SN_TEST_ID_23; + int rc; + + XMEMSET(&ping, 0, sizeof(ping)); + ping.clientId = id; + + rc = SN_Encode_Ping(buf, (int)sizeof(buf), &ping, SN_MSG_TYPE_PING_REQ); + + /* MQTT-SN 1.2 section 5.4.19: Length(1) MsgType(1) ClientId(23). */ + ASSERT_EQ(25, rc); + ASSERT_EQ(25, buf[0]); + ASSERT_EQ(SN_MSG_TYPE_PING_REQ, buf[1]); + ASSERT_MEM_EQ(SN_TEST_ID_23, &buf[2], 23); +} + /* ============================================================================ * SN_Encode_Publish * @@ -1952,6 +2033,10 @@ int main(int argc, char** argv) RUN_TEST(sn_typedesc_pubrec); RUN_TEST(sn_typedesc_unknown_type); #endif + RUN_TEST(sn_encode_connect_client_id_too_long_rejected); + RUN_TEST(sn_encode_connect_client_id_max_len_encoded); + RUN_TEST(sn_encode_pingreq_client_id_too_long_rejected); + RUN_TEST(sn_encode_pingreq_client_id_max_len_encoded); TEST_SUITE_END(); diff --git a/tests/test_mqtt_sn_client.c b/tests/test_mqtt_sn_client.c index de30daf1d..c0cec1809 100644 --- a/tests/test_mqtt_sn_client.c +++ b/tests/test_mqtt_sn_client.c @@ -409,14 +409,11 @@ static const byte SUBACK_REJECT_FRAME[] = { 0x08, SN_MSG_TYPE_SUBACK, 0x00, /* Gateway PINGRESP: total_len=2, type. */ static const byte PINGRESP_FRAME[] = { 0x02, SN_MSG_TYPE_PING_RESP }; -#ifdef WOLFMQTT_NONBLOCK -/* Unsolicited GWINFO broadcast: total_len=5, type, gwId, addr(2). Not a - * match for any pending wait, so SN_Client_WaitType routes it through the - * shared client->msgSN object. */ +/* GWINFO from a gateway: total_len=5, type, gwId, addr(2) + * (MQTT-SN 1.2 section 5.4.4). */ #define SN_TEST_GWINFO_GWID 0x09 static const byte GWINFO_FRAME[] = { 0x05, SN_MSG_TYPE_GWINFO, SN_TEST_GWINFO_GWID, 0xAA, 0xBB }; -#endif /* Scripted publish-response frames for packet_id 1. * PUBACK: total_len=7, type, topicId(2), packet_id(2), return_code. @@ -2918,6 +2915,72 @@ TEST(sn_ping_unsolicited_gwinfo_captured) #endif /* WOLFMQTT_NONBLOCK */ +/* A caller-owned SN_Object reused with SN_Client_WaitMessage_ex can carry + * leftover union bytes from an earlier packet where gwInfo.gwAddr lives. A + * GWINFO decoded into it must use the object's own gwAddrBuf and never write + * through that stale pointer. */ +TEST(sn_wait_message_ex_reused_obj_gwinfo_ignores_stale_addr) +{ + SN_Object obj; + SN_GwAddr stale; + SN_GwAddr stale_expect; + int rc; + const byte expect_addr[2] = { 0xAA, 0xBB }; + + ASSERT_EQ(MQTT_CODE_SUCCESS, sn_client_init(0)); + + XMEMSET(&stale, 0x5A, sizeof(stale)); + XMEMCPY(&stale_expect, &stale, sizeof(stale)); + XMEMSET(&obj, 0, sizeof(obj)); + obj.gwInfo.gwAddr = &stale; + + mock_net_push(&g_mock, GWINFO_FRAME, (int)sizeof(GWINFO_FRAME)); + rc = SN_Client_WaitMessage_ex(&g_client, &obj, 1000); + + ASSERT_EQ(MQTT_CODE_SUCCESS, rc); + ASSERT_MEM_EQ(&stale_expect, &stale, sizeof(stale)); + ASSERT_EQ(SN_TEST_GWINFO_GWID, obj.gwInfo.gwId); + ASSERT_TRUE(obj.gwInfo.gwAddr == &obj.gwInfo.gwAddrBuf); + ASSERT_MEM_EQ(expect_addr, &obj.gwInfo.gwAddrBuf, sizeof(expect_addr)); +} + +/* The context a caller stores in its own SN_Object's publish.ctx is the + * user-supplied callback context; reusing the object for an incoming PUBLISH + * must deliver that context to msg_cb, not a reset NULL. */ +static void* g_sn_seen_ctx; +static int sn_record_ctx_cb(MqttClient* client, MqttMessage* msg, + byte msg_new, byte msg_done) +{ + (void)client; (void)msg_new; (void)msg_done; + g_sn_seen_ctx = ((SN_Publish*)msg)->ctx; + return MQTT_CODE_SUCCESS; +} + +TEST(sn_wait_message_ex_publish_keeps_caller_ctx) +{ + SN_Object obj; + int marker = 0; + int rc; + /* MQTT-SN 1.2 section 5.4.21 PUBLISH, QoS 0: Length 9, MsgType, Flags + * 0x00, TopicId 0x000A, MsgId 0x0000, Data 0x01 0x02. */ + static const byte publish_qos0[] = { + 0x09, SN_MSG_TYPE_PUBLISH, 0x00, 0x00, 0x0A, 0x00, 0x00, 0x01, 0x02 + }; + + ASSERT_EQ(MQTT_CODE_SUCCESS, sn_client_init(0)); + g_client.msg_cb = sn_record_ctx_cb; + g_sn_seen_ctx = NULL; + + XMEMSET(&obj, 0, sizeof(obj)); + obj.publish.ctx = ▮ + + mock_net_push(&g_mock, publish_qos0, (int)sizeof(publish_qos0)); + rc = SN_Client_WaitMessage_ex(&g_client, &obj, 1000); + + ASSERT_EQ(MQTT_CODE_SUCCESS, rc); + ASSERT_TRUE(g_sn_seen_ctx == (void*)&marker); +} + TEST(sn_packet_read_bounds_fixed_header) { static const byte short_frame[] = { 0x02, SN_MSG_TYPE_PING_RESP }; @@ -3222,6 +3285,8 @@ int main(int argc, char** argv) RUN_TEST(sn_ping_null_no_continue); RUN_TEST(sn_ping_null_consecutive_success); RUN_TEST(sn_ping_null_recovers_after_malformed_response); + RUN_TEST(sn_wait_message_ex_reused_obj_gwinfo_ignores_stale_addr); + RUN_TEST(sn_wait_message_ex_publish_keeps_caller_ctx); RUN_TEST(sn_packet_read_bounds_fixed_header); RUN_TEST(sn_packet_read_rejects_header_past_buffer); RUN_TEST(sn_nondtls_reads_full_frames); diff --git a/tests/test_mqtt_tls_host.c b/tests/test_mqtt_tls_host.c index ef2aab71d..526639313 100644 --- a/tests/test_mqtt_tls_host.c +++ b/tests/test_mqtt_tls_host.c @@ -50,6 +50,7 @@ typedef struct TlsTestServer { static const char* g_tls_test_identity; static int g_tls_test_precreate; +static int g_tls_test_peer_close; static int tls_test_connect(void* context, const char* host, word16 port, int timeout_ms) @@ -205,9 +206,40 @@ static void* tls_test_server_run(void* arg) TlsTestServer* server = (TlsTestServer*)arg; server->result = wolfSSL_accept(server->ssl); + if (g_tls_test_peer_close && server->result == WOLFSSL_SUCCESS) { + (void)wolfSSL_shutdown(server->ssl); + } return NULL; } +/* After the peer sends close_notify, wolfSSL_read reports the closure with a + * zero return. That must surface as a terminal network error on the first + * read, not as MQTT_CODE_CONTINUE, and later writes must fail as well. */ +static int tls_test_check_peer_close(MqttClient* client) +{ + int rc; + int attempts = 0; + + rc = MqttClient_WaitMessage(client, 1000); + if (rc != MQTT_CODE_ERROR_NETWORK) { + PRINTF(" TLS peer close: read expected %d, got %d", + MQTT_CODE_ERROR_NETWORK, rc); + return 1; + } + + (void)MqttClient_Flags(client, 0, MQTT_CLIENT_FLAG_CONNECT_SENT); + attempts = 0; + do { + rc = MqttClient_Ping(client); + } while (rc == MQTT_CODE_CONTINUE && ++attempts < 8); + if (rc != MQTT_CODE_ERROR_NETWORK) { + PRINTF(" TLS peer close: write expected %d, got %d", + MQTT_CODE_ERROR_NETWORK, rc); + return 1; + } + return 0; +} + static int tls_test_host_with_cb(const char* host, const char* identity, int precreate, int expected, MqttTlsCb cb) { @@ -303,6 +335,10 @@ static int tls_test_host_with_cb(const char* host, const char* identity, do { connect_rc = MqttClient_NetConnect(&client, host, 8883, 3000, 1, cb); } while (connect_rc == MQTT_CODE_CONTINUE && ++attempts < 8); + if (connect_rc == MQTT_CODE_SUCCESS && g_tls_test_peer_close && + tls_test_check_peer_close(&client) != 0) { + goto cleanup; + } if (connect_rc == MQTT_CODE_SUCCESS) { (void)MqttClient_NetDisconnect(&client); if (MqttClient_Flags(&client, 0, 0) & @@ -330,6 +366,7 @@ static int tls_test_host_with_cb(const char* host, const char* identity, #endif g_tls_test_identity = NULL; g_tls_test_precreate = 0; + g_tls_test_peer_close = 0; (void)tls_test_disconnect(&client_net); if (thread_started) { (void)pthread_join(thread, NULL); @@ -467,6 +504,11 @@ int main(void) rc = tls_test_host_with_cb(NULL, NULL, 0, MQTT_CODE_SUCCESS, tls_test_no_verify); } + if (rc == 0) { + g_tls_test_peer_close = 1; + rc = tls_test_host_with_cb("127.0.0.1", NULL, 0, + MQTT_CODE_SUCCESS, tls_test_no_verify); + } #ifdef HAVE_SNI if (rc == 0) { rc = tls_test_host_with_cb("127.0.0.1", NULL, 0, diff --git a/wolfmqtt/mqtt_broker.h b/wolfmqtt/mqtt_broker.h index d291c1d45..b42a2efdc 100644 --- a/wolfmqtt/mqtt_broker.h +++ b/wolfmqtt/mqtt_broker.h @@ -329,6 +329,10 @@ struct MqttBroker; /* -------------------------------------------------------------------------- */ typedef int (*MqttBrokerNet_ListenCb)(void* ctx, BROKER_SOCKET_T* sock, word16 port, int backlog); +/* Returns MQTT_CODE_SUCCESS with *client_sock set for a new connection, and + * MQTT_CODE_CONTINUE or MQTT_CODE_ERROR_TIMEOUT when none is pending or a + * single connection attempt failed. Any other negative value means the + * listener has failed; MqttBroker_Step returns it and MqttBroker_Run stops. */ typedef int (*MqttBrokerNet_AcceptCb)(void* ctx, BROKER_SOCKET_T listen_sock, BROKER_SOCKET_T* client_sock); typedef int (*MqttBrokerNet_ReadCb)(void* ctx, BROKER_SOCKET_T sock, @@ -372,8 +376,12 @@ typedef int (*MqttBrokerPersist_IterCb)(const byte* key, word16 key_len, #define BROKER_PERSIST_STREAM_APPEND 3 typedef struct MqttBrokerPersistHooks { - /* Key/value blob API. key bytes are opaque to the backend; len is - * always <= 256 in current use (a client_id or topic). */ + /* Key/value blob API. Key bytes are opaque to the backend and + * implementations must honor key_len. Keys are derived from + * peer-supplied client_ids and topics, which dynamic-memory builds do + * not cap. A backend with a maximum key length (the POSIX backend + * accepts up to 256 bytes) must reject a longer key with an error + * rather than truncate it. */ int (*kv_put)(void* ctx, byte ns, const byte* key, word16 key_len, const byte* blob, word32 blob_len); int (*kv_get)(void* ctx, byte ns, const byte* key, word16 key_len, @@ -698,6 +706,10 @@ typedef struct BrokerClient { * Network Connection, so the client is kept only until those bytes are * fully delivered and is then dropped without becoming connected. */ unsigned int connack_refused : 1; + /* A DISCONNECT announcing an abnormal close is still being written in + * direct_pending_len. Once that write finishes or fails, the client is + * closed instead of reading another packet. */ + unsigned int close_after_direct : 1; #endif } BrokerClient; @@ -867,6 +879,11 @@ typedef struct MqttBroker { #ifdef WOLFMQTT_BROKER_PERSIST byte persist_restored; #endif + /* Last accept failure the built-in network adapters logged for + * listen_sock and for the TLS listener (0 = none), so a persistent + * failure is reported once rather than on every step. */ + int accept_log_err; + int accept_log_err_tls; } MqttBroker; /* -------------------------------------------------------------------------- */ diff --git a/wolfmqtt/mqtt_client.h b/wolfmqtt/mqtt_client.h index 562019a0c..f4b04a67f 100644 --- a/wolfmqtt/mqtt_client.h +++ b/wolfmqtt/mqtt_client.h @@ -189,7 +189,15 @@ enum MqttClientFlags { MQTT_CLIENT_FLAG_TLS_DEFAULT_CTX = 0x01 << 8, /* Skip host identity setup when the callback disables authentication. * Does not change wolfSSL's certificate verification policy. */ - MQTT_CLIENT_FLAG_TLS_SKIP_HOST_CHECK = 0x01 << 9 + MQTT_CLIENT_FLAG_TLS_SKIP_HOST_CHECK = 0x01 << 9, + /* The last CONNECT asked for a Session that outlives the Network + * Connection (CleanSession 0, or a non-zero v5 Session Expiry Interval), + * so unacknowledged QoS > 0 messages must be replayable. A v5 CONNACK + * Session Expiry Interval overrides the requested value (MQTT 5.0 + * section 3.2.2.3.2). Managed by the library - set or cleared by + * MqttClient_Connect and updated from the CONNACK. Applications must + * not set or clear it through MqttClient_Flags. */ + MQTT_CLIENT_FLAG_PERSISTENT_SESSION = 0x01 << 10 }; /*! \brief Sets flags in the MqttClient structure. To be used from the application before calling MqttClient_NetConnect. @@ -283,8 +291,10 @@ typedef struct _MqttSk { * MqttPublish is gone, so the client keeps its own copy. That costs memory, * hence a pool sized separately from the (much cheaper) identifier table: * raise MQTT_MAX_REPLAY_MSGS to retain more in-flight messages, or define - * WOLFMQTT_NO_SESSION_REPLAY to compile the store out entirely. A message - * that does not fit is still sent, just not retained for replay. + * WOLFMQTT_NO_SESSION_REPLAY to compile the store out entirely. On a Session + * that outlives the connection, a QoS > 0 publish that finds the pool full is + * refused with MQTT_CODE_ERROR_OUT_OF_BUFFER before it is sent; on a clean + * Session, which is never replayed, it is still sent without a copy. * * A PUBREL awaiting its PUBCOMP is retained regardless of the size limits: * it carries no payload, only the Packet Identifier. */ @@ -941,6 +951,10 @@ WOLFMQTT_API int MqttClient_NetConnect( MqttTlsCb cb); /*! \brief Performs a network disconnect + * \note Scrubs tx_buf and rx_buf once the transport is closed. In + WOLFMQTT_MULTITHREAD builds a buffer whose read or write is + still active is left to its owner; release an abandoned + operation with MqttClient_CancelMessage, which scrubs it. * \param client Pointer to MqttClient structure * \return MQTT_CODE_SUCCESS or MQTT_CODE_ERROR_* (see enum MqttPacketResponseCodes) diff --git a/wolfmqtt/mqtt_sn_client.h b/wolfmqtt/mqtt_sn_client.h index aed7f195a..985d0889b 100644 --- a/wolfmqtt/mqtt_sn_client.h +++ b/wolfmqtt/mqtt_sn_client.h @@ -197,11 +197,17 @@ WOLFMQTT_API int SN_Client_Disconnect( /*! \brief Encodes and sends the MQTT-SN Disconnect packet. Client may send the disconnect with a duration to indicate the client is entering the "asleep" state. - * \note This is a non-blocking function that will try and send using - MqttNet.write + * \note With a NULL disconnect or a zero sleepTmr this only sends the + packet using MqttNet.write. With a nonzero sleepTmr it then + waits up to cmd_timeout_ms for the gateway's DISCONNECT + acknowledgement (MQTT-SN 1.2 section 6.14): blocking builds + wait for it, while WOLFMQTT_NONBLOCK builds may return + MQTT_CODE_CONTINUE and must be called again to complete. * \param client Pointer to MqttClient structure * \param disconnect Pointer to SN_Disconnect structure. NULL is valid. - * \return MQTT_CODE_SUCCESS or MQTT_CODE_ERROR_* + * \return MQTT_CODE_SUCCESS, MQTT_CODE_CONTINUE when the call must be + repeated (another write is in progress, or a nonblocking wait + for the acknowledgement), or MQTT_CODE_ERROR_* (see enum MqttPacketResponseCodes) */ WOLFMQTT_API int SN_Client_Disconnect_ex(