MQTT (OASIS MQTT v5.0) Compliance Issues in sol

[Zhaodl1]

Hi, while reviewing sol's MQTT implementation against the OASIS MQTT v5.0 specification, we noticed several places where the current behavior appears to differ from the specification. Each item below cites the relevant specification text alongside the corresponding source code for your reference. We hope this is helpful for improving conformance.

---

1. Data Received After a Rejected CONNECT Continues to Be Processed

Spec Reference: OASIS MQTT v5.0 Section 3.1.4 ([MQTT-3.1.4-6])

"If the Server rejects the CONNECT, it MUST NOT process any data sent by the
Client after the CONNECT packet except AUTH packets [MQTT-3.1.4-6]."

Analysis:
On an authentication failure, the handler queues a CONNACK with the
MQTT_BAD_USERNAME_OR_PASSWORD return code and returns that same value. In the
caller, this return code is grouped together with REPLY, so the connection is
retained on the event loop rather than closed. A subsequent packet (for example a
PUBLISH) is then unpacked and dispatched to its handler, which does not check
whether the client has successfully connected. This differs from the
specification, which states that data after a rejected CONNECT is not to be
processed.

Source Code Evidence (server.c, handlers.c):

// server.c:868-877
    switch (c->rc) {
    case REPLY:
    case MQTT_NOT_AUTHORIZED:
    case MQTT_BAD_USERNAME_OR_PASSWORD:
        enqueue_event_write(c);     // CONNACK queued; connection kept open
        if (io.data.header.bits.type != PUBLISH)
            mqtt_packet_destroy(&io.data);
        break;

// handlers.c:482-487
bad_auth:
    set_connack(cc, MQTT_BAD_USERNAME_OR_PASSWORD, session_present);
    return MQTT_BAD_USERNAME_OR_PASSWORD;   // falls into the kept-open case above

---

2. Second CONNECT Results in Silent Close Rather Than a DISCONNECT Packet

Spec Reference: OASIS MQTT v5.0 Section 3.1 ([MQTT-3.1.0-2])

"A Client can only send the CONNECT packet once over a Network Connection. The
Server MUST process a second CONNECT packet sent from a Client as a Protocol
Error and close the Network Connection [MQTT-3.1.0-2]."

Analysis:
A second CONNECT on an already-connected client is detected (cc->connected == true) and the handler returns -ERRCLIENTDC, after which the caller removes the
file descriptor and deactivates the client. The client is disconnected; however,
no DISCONNECT packet is constructed or sent prior to closing, and no reason code
is communicated. This differs from the v5.0 specification, where the protocol
error is signaled with a DISCONNECT packet carrying a Protocol Error reason code
before the connection is closed.

Source Code Evidence (handlers.c, server.c):

// handlers.c:352-360
    if (cc->connected == true) {
        /*
         * Already connected client, 2 CONNECT packet should be interpreted as
         * a violation of the protocol, causing disconnection of the client
         */
        log_info("Received double CONNECT from %s, disconnecting client",
                 c->payload.client_id);
        goto clientdc;          // returns -ERRCLIENTDC; no DISCONNECT packet sent
    }

// server.c:882-884
    case -ERRCLIENTDC:
        ev_del_fd(ctx, c->conn.fd);
        client_deactivate(io.client);   // TCP closed without a preceding DISCONNECT

---

3. PUBLISH With Reserved QoS Value (3) Processed as a Normal Publish

Spec Reference: OASIS MQTT v5.0 Section 3.3.1 ([MQTT-3.3.1-4])

"A PUBLISH Packet MUST NOT have both QoS bits set to 1 [MQTT-3.3.1-4]. If a
Server or Client receives a PUBLISH packet which has both QoS bits set to 1 it
is a Malformed Packet. Use DISCONNECT with Reason Code 0x81 (Malformed Packet)
as described in section 4.13."

Analysis:
The publish handler reads the QoS field from the fixed header and does not
validate it against the reserved value. The reserved QoS value (3) is not equal
to AT_MOST_ONCE (0) or EXACTLY_ONCE (2), so the EXACTLY_ONCE ? PUBREC : PUBACK expression selects PUBACK, and the packet is published and acknowledged
as though it were a normal QoS 1 message. This differs from the specification,
which classifies this case as a Malformed Packet requiring connection closure.

Source Code Evidence (handlers.c):

// handlers.c:696-706, 758-767
    char topic[p->topiclen + 2];
    unsigned char qos = hdr->bits.qos;   // qos == 3 read without validation
    ...
    if (qos == AT_MOST_ONCE)
        goto exit;

    int ptype = qos == EXACTLY_ONCE ? PUBREC : PUBACK;   // qos 3 -> PUBACK
    mqtt_pack_mono(c->wbuf + c->towrite, ptype, orig_mid);

---

4. CONNECT With Will Flag 0 but Non-Zero Will QoS / Will Retain Is Accepted

Spec Reference: OASIS MQTT v5.0 Section 3.1.2.6 / 3.1.2.7 ([MQTT-3.1.2-11], [MQTT-3.1.2-13])

"If the Will Flag is set to 0, then the Will QoS MUST be set to 0 (0x00)
[MQTT-3.1.2-11]."

"If the Will Flag is set to 0, then Will Retain MUST be set to 0
[MQTT-3.1.2-13]."

Analysis:
The Will Topic and Will Message fields are read only when the Will Flag is set to
1; when the Will Flag is 0, the non-zero Will QoS and Will Retain bits present in
the Connect Flags byte are not inspected. No check appears to reject a CONNECT
whose Will Flag is 0 while Will QoS is non-zero or Will Retain is set, and such a
packet is accepted with a success CONNACK. This differs from the specification,
which requires those combinations to be treated as malformed.

Source Code Evidence (mqtt.c):

// mqtt.c:245-249
    /* Read the will topic and message if will is set on flags */
    if (pkt->connect.bits.will == 1) {
        unpack_string16(&buf, &pkt->connect.payload.will_topic);
        unpack_string16(&buf, &pkt->connect.payload.will_message);
    }
    // when will == 0, will_qos and will_retain bits are not validated

---

5. CONNECT User Name / Password Flag-to-Payload Consistency Not Enforced

Spec Reference: OASIS MQTT v5.0 Section 3.1.2.8 / 3.1.2.9 ([MQTT-3.1.2-16], [MQTT-3.1.2-18])

"If the User Name Flag is set to 0, a User Name MUST NOT be present in the
Payload [MQTT-3.1.2-16]."

"If the Password Flag is set to 0, a Password MUST NOT be present in the
Payload [MQTT-3.1.2-18]."

Analysis:
The User Name and Password fields are each read conditionally on their respective
connect flag bits. There does not appear to be any validation that the flag and
payload agree — for example, that a flag set to 0 implies the corresponding
payload field is absent. A CONNECT where a flag is 0 while the field bytes are
nonetheless present is accepted, with the unconsumed bytes left in the buffer.
This differs from the specification, which requires such combinations to be
treated as malformed.

Source Code Evidence (mqtt.c):

// mqtt.c:251-259
    /* Read the username if username flag is set */
    if (pkt->connect.bits.username == 1)
        unpack_string16(&buf, &pkt->connect.payload.username);

    /* Read the password if password flag is set */
    if (pkt->connect.bits.password == 1)
        unpack_string16(&buf, &pkt->connect.payload.password);

    return MQTT_OK;   // no flag/payload consistency check before returning OK

---

6. PUBREL Reserved Fixed-Header Bits Are Not Validated

Spec Reference: OASIS MQTT v5.0 Section 3.6.1 ([MQTT-3.6.1-1])

"Bits 3,2,1 and 0 of the Fixed Header in the PUBREL packet are reserved and
MUST be set to 0,0,1 and 0 respectively. The Server MUST treat any other value
as malformed and close the Network Connection [MQTT-3.6.1-1]."

Analysis:
The PUBREL handler reads the packet identifier and replies with PUBCOMP without
inspecting the lower four bits of the fixed header. A PUBREL whose reserved bits
differ from 0010 is therefore processed normally and answered, rather than being
treated as malformed. This differs from the specification, which requires such a
packet to result in connection closure.

Source Code Evidence (handlers.c):

// handlers.c:822-836
static int pubrel_handler(struct io_event *e)
{
    struct client *c = e->client;
    unsigned pkt_id  = e->data.ack.pkt_id;
    ...
    mqtt_pack_mono(c->wbuf + c->towrite, PUBCOMP, pkt_id);   // fixed-header bits unchecked
    c->towrite += MQTT_ACK_LEN;
    ...
    return REPLY;
}

---

7. SUBSCRIBE Reserved Subscription-Options Bits Are Not Validated

Spec Reference: OASIS MQTT v5.0 Section 3.8.3.1 ([MQTT-3.8.3-5])

"Bits 6 and 7 of the Subscription Options byte are reserved for future use.
The Server MUST treat a SUBSCRIBE packet as malformed if any of Reserved bits
in the Payload are non-zero [MQTT-3.8.3-5]."

Analysis:
The SUBSCRIBE handler iterates over the topic/options tuples and copies each
options byte (including the reserved bits 6 and 7) directly into the per-topic
return code array, which is then serialized into the SUBACK. There does not
appear to be any check that the reserved bits are zero. This differs from the
specification, which requires a SUBSCRIBE with non-zero reserved bits to be
treated as malformed.

Source Code Evidence (handlers.c):

// handlers.c:545-549, 622-627
    unsigned char rcs[s->tuples_len];
    struct client *c = e->client;
    /* Subscribe packets contains a list of topics and QoS tuples */
    for (unsigned i = 0; i < s->tuples_len; i++) {
        ...
        rcs[i] = s->tuples[i].qos;   // raw options byte, reserved bits not masked/checked
    }

    struct mqtt_packet pkt = {.header = (union mqtt_header){.byte = SUBACK_B}};
    mqtt_suback(&pkt, s->pkt_id, rcs, s->tuples_len);