diff --git a/lib/mqtt.c b/lib/mqtt.c index 84fd272e21..d28a25bb50 100644 --- a/lib/mqtt.c +++ b/lib/mqtt.c @@ -892,6 +892,24 @@ static CURLcode mqtt_doing(struct Curl_easy *data, bool *done) break; } mq->npacket = 0; + /* PINGRESP and DISCONNECT must have remaining_length == 0 and + * reserved bits (low nibble) must be zero per MQTT 3.1.1 + * sections 2.2.2, 3.13.1 and 3.14.1. Reject before state + * dispatch to prevent nextstate confusion. */ + { + const unsigned char type = mq->firstbyte & 0xF0; + const unsigned char reserved = mq->firstbyte & 0x0F; + if((type == MQTT_MSG_DISCONNECT || type == MQTT_MSG_PINGRESP) && + (mq->remaining_length || reserved)) { + failf(data, + "Broker sent malformed %s " + "(remaining_length=%zu, header byte=0x%02x)", + type == MQTT_MSG_DISCONNECT ? "DISCONNECT" : "PINGRESP", + mq->remaining_length, mq->firstbyte); + result = CURLE_WEIRD_SERVER_REPLY; + break; + } + } if(mq->remaining_length) { mqstate(data, mqtt->nextstate, MQTT_NOSTATE); break; diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am index 5a517df9f3..706a4c89ed 100644 --- a/tests/data/Makefile.am +++ b/tests/data/Makefile.am @@ -261,7 +261,7 @@ test2080 test2081 test2082 test2083 test2084 test2085 test2086 test2087 \ test2088 test2089 test2090 test2091 \ test2100 test2101 test2102 test2103 test2104 \ \ -test2200 test2201 test2202 test2203 test2204 test2205 \ +test2200 test2201 test2202 test2203 test2204 test2205 test2206 test2207 \ \ test2300 test2301 test2302 test2303 test2304 test2306 test2307 test2308 \ test2309 \ diff --git a/tests/data/test2206 b/tests/data/test2206 new file mode 100644 index 0000000000..31530221f8 --- /dev/null +++ b/tests/data/test2206 @@ -0,0 +1,59 @@ + + + + +MQTT +MQTT SUBSCRIBE + + + +# Server-side + + +hello + + +# Send a PINGRESP (0xD0) with remaining_length=2 in place of the +# expected CONNACK. MQTT 3.1.1 s. 3.13.1 requires PINGRESP to have +# remaining_length == 0. Curl must reject this with +# CURLE_WEIRD_SERVER_REPLY rather than dispatching to the CONNACK +# handler. + +PINGRESP-as-CONNACK TRUE + + + +# Client-side + + +mqtt + + +mqtt + + +MQTT reject PINGRESP with nonzero remaining_length in place of CONNACK + + +mqtt://%HOSTIP:%MQTTPORT/%TESTNUMBER + + + +# Verify data after the test has been "shot" + +# Strip out the random part of the client id from the CONNECT message +# before comparison + +s/^(.* 00044d5154540402003c000c6375726c).*/$1/ + + +client CONNECT 18 00044d5154540402003c000c6375726c +server PINGRESP-as-CONNACK 2 d0020000 + + +# 8 is CURLE_WEIRD_SERVER_REPLY + +8 + + + diff --git a/tests/data/test2207 b/tests/data/test2207 new file mode 100644 index 0000000000..2aa3bd2636 --- /dev/null +++ b/tests/data/test2207 @@ -0,0 +1,59 @@ + + + + +MQTT +MQTT SUBSCRIBE + + + +# Server-side + + +hello + + +# Send a DISCONNECT with remaining_length=2 after the PUBLISH. +# MQTT 3.1.1 s. 3.14.1 requires DISCONNECT to have remaining_length == 0. +# Curl must reject this with CURLE_WEIRD_SERVER_REPLY. + +DISCONNECT-malformed TRUE + + + +# Client-side + + +mqtt + + +mqtt + + +MQTT reject DISCONNECT with nonzero remaining_length + + +mqtt://%HOSTIP:%MQTTPORT/%TESTNUMBER + + + +# Verify data after the test has been "shot" + + +s/^(.* 00044d5154540402003c000c6375726c).*/$1/ + + +client CONNECT 18 00044d5154540402003c000c6375726c +server CONNACK 2 20020000 +client SUBSCRIBE 9 000100043232303700 +server SUBACK 3 9003000100 +server PUBLISH c 300c00043232303768656c6c6f0a +server DISCONNECT-malformed 2 e0020000 + + +# 8 is CURLE_WEIRD_SERVER_REPLY + +8 + + + diff --git a/tests/server/mqttd.c b/tests/server/mqttd.c index 0a3a6ee024..ad4aa3a640 100644 --- a/tests/server/mqttd.c +++ b/tests/server/mqttd.c @@ -40,6 +40,7 @@ /* #define MQTT_MSG_PUBACK 0x40 */ #define MQTT_MSG_SUBSCRIBE 0x82 #define MQTT_MSG_SUBACK 0x90 +#define MQTT_MSG_PINGRESP 0xd0 #define MQTT_MSG_DISCONNECT 0xe0 struct mqttd_configurable { @@ -49,6 +50,8 @@ struct mqttd_configurable { bool publish_before_suback; bool short_publish; bool excessive_remaining; + bool pingresp_as_connack; /* send PINGRESP with payload instead of CONNACK */ + bool disconnect_malformed; /* DISCONNECT with nonzero remlen */ unsigned char error_connack; unsigned char remlen_connack; }; @@ -65,6 +68,8 @@ static void mqttd_resetdefaults(void) m_config.publish_before_suback = FALSE; m_config.short_publish = FALSE; m_config.excessive_remaining = FALSE; + m_config.pingresp_as_connack = FALSE; + m_config.disconnect_malformed = FALSE; m_config.error_connack = 0; m_config.remlen_connack = 0; m_config.testnum = 0; @@ -98,6 +103,14 @@ static void mqttd_getconfig(void) logmsg("short-PUBLISH set"); m_config.short_publish = TRUE; } + else if(!strcmp(key, "PINGRESP-as-CONNACK")) { + logmsg("PINGRESP-as-CONNACK set"); + m_config.pingresp_as_connack = TRUE; + } + else if(!strcmp(key, "DISCONNECT-malformed")) { + logmsg("DISCONNECT-malformed set"); + m_config.disconnect_malformed = TRUE; + } else if(!strcmp(key, "error-CONNACK")) { pval = value; if(!curlx_str_number(&pval, &num, 0xff)) { @@ -166,6 +179,16 @@ static int connack(FILE *dump, curl_socket_t fd) 0x00, 0x00 }; ssize_t rc; + const char *label = "CONNACK"; + + if(m_config.pingresp_as_connack) { + /* Send a PINGRESP (0xD0) with remaining_length=2 and payload + mimicking a successful CONNACK. MQTT 3.1.1 s. 3.13.1 requires + PINGRESP to have remaining_length=0, so this is malformed. */ + packet[0] = MQTT_MSG_PINGRESP; + label = "PINGRESP-as-CONNACK"; + logmsg("Sending malformed PINGRESP in place of CONNACK"); + } if(m_config.remlen_connack) packet[1] = m_config.remlen_connack; @@ -173,10 +196,10 @@ static int connack(FILE *dump, curl_socket_t fd) rc = swrite(fd, packet, sizeof(packet)); if(rc > 0) { - logmsg("WROTE %zd bytes [CONNACK]", rc); + logmsg("WROTE %zd bytes [%s]", rc, label); loghex(packet, rc); - logprotocol(FROM_SERVER, "CONNACK", packet[1], dump, - packet, sizeof(packet)); + logprotocol(FROM_SERVER, label, packet[1], dump, + packet, rc); } if(rc == sizeof(packet)) { return 0; @@ -235,15 +258,30 @@ static int disconnect(FILE *dump, curl_socket_t fd) { unsigned char packet[] = { MQTT_MSG_DISCONNECT, 0x00, + 0x00, 0x00 /* extra bytes for malformed variant */ }; - ssize_t rc = swrite(fd, packet, sizeof(packet)); - if(rc == sizeof(packet)) { - logmsg("WROTE %zd bytes [DISCONNECT]", rc); + size_t pktlen = 2; + const char *label = "DISCONNECT"; + ssize_t rc; + + if(m_config.disconnect_malformed) { + /* Send DISCONNECT with remaining_length=2 (must be 0 per spec) */ + packet[1] = 0x02; + pktlen = 4; + label = "DISCONNECT-malformed"; + logmsg("Sending malformed DISCONNECT with nonzero remaining_length"); + } + + rc = swrite(fd, packet, pktlen); + if(rc > 0) { + logmsg("WROTE %zd bytes [%s]", rc, label); loghex(packet, rc); - logprotocol(FROM_SERVER, "DISCONNECT", 0, dump, packet, rc); + logprotocol(FROM_SERVER, label, packet[1], dump, packet, rc); + } + if(rc == (ssize_t)pktlen) { return 0; } - logmsg("Failed sending [DISCONNECT]"); + logmsg("Failed sending [%s]", label); return 1; }