diff --git a/ChangeLog.txt b/ChangeLog.txt index c6cee2b0..509dbaf8 100644 --- a/ChangeLog.txt +++ b/ChangeLog.txt @@ -23,6 +23,8 @@ Broker: - Disable TLS renegotiation. Client initiated renegotiation is considered to be a potential attack vector against servers. Closes #1257. - Fix incorrect shared subscription topic '$shared'. +- Fix zero length client ids being rejected for MQTT v5 clients with clean + start set to true. Client library: - Fix typo causing build error on Windows when building without TLS support. diff --git a/src/handle_connect.c b/src/handle_connect.c index 1bd35ace..9de98dd9 100644 --- a/src/handle_connect.c +++ b/src/handle_connect.c @@ -540,8 +540,12 @@ int handle__connect(struct mosquitto_db *db, struct mosquitto *context) }else{ allow_zero_length_clientid = db->config->security_options.allow_zero_length_clientid; } - if(clean_start == 0 || allow_zero_length_clientid == false){ - send__connack(db, context, 0, CONNACK_REFUSED_IDENTIFIER_REJECTED, NULL); + if((context->protocol == mosq_p_mqtt311 && clean_start == 0) || allow_zero_length_clientid == false){ + if(context->protocol == mosq_p_mqtt311){ + send__connack(db, context, 0, CONNACK_REFUSED_IDENTIFIER_REJECTED, NULL); + }else{ + send__connack(db, context, 0, MQTT_RC_UNSPECIFIED, NULL); + } rc = MOSQ_ERR_PROTOCOL; goto handle_connect_error; }else{ diff --git a/test/broker/12-prop-assigned-client-identifier.py b/test/broker/12-prop-assigned-client-identifier.py index 376d10ce..28f5037b 100755 --- a/test/broker/12-prop-assigned-client-identifier.py +++ b/test/broker/12-prop-assigned-client-identifier.py @@ -5,43 +5,48 @@ from mosq_test_helper import * -rc = 1 -keepalive = 10 -connect_packet = mosq_test.gen_connect(None, proto_ver=5, keepalive=keepalive) +def do_test(clean_start): + rc = 1 + keepalive = 10 + connect_packet = mosq_test.gen_connect(None, proto_ver=5, keepalive=keepalive, clean_session=clean_start) -props = mqtt5_props.gen_string_prop(mqtt5_props.PROP_ASSIGNED_CLIENT_IDENTIFIER, "auto-00000000-0000-0000-0000-000000000000") -connack_packet = mosq_test.gen_connack(rc=0, proto_ver=5, properties=props) + props = mqtt5_props.gen_string_prop(mqtt5_props.PROP_ASSIGNED_CLIENT_IDENTIFIER, "auto-00000000-0000-0000-0000-000000000000") + connack_packet = mosq_test.gen_connack(rc=0, proto_ver=5, properties=props) -props = mqtt5_props.gen_uint32_prop(mqtt5_props.PROP_SESSION_EXPIRY_INTERVAL, 1) -disconnect_client_packet = mosq_test.gen_disconnect(proto_ver=5, properties=props) + props = mqtt5_props.gen_uint32_prop(mqtt5_props.PROP_SESSION_EXPIRY_INTERVAL, 1) + disconnect_client_packet = mosq_test.gen_disconnect(proto_ver=5, properties=props) -disconnect_server_packet = mosq_test.gen_disconnect(proto_ver=5, reason_code=130) + disconnect_server_packet = mosq_test.gen_disconnect(proto_ver=5, reason_code=130) -port = mosq_test.get_port() -broker = mosq_test.start_broker(filename=os.path.basename(__file__), port=port) + port = mosq_test.get_port() + broker = mosq_test.start_broker(filename=os.path.basename(__file__), port=port) -try: - sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM) - sock.settimeout(10) - sock.connect(("localhost", port)) + try: + sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM) + sock.settimeout(10) + sock.connect(("localhost", port)) - sock.send(connect_packet) - connack_recvd = sock.recv(len(connack_packet)) + sock.send(connect_packet) + connack_recvd = sock.recv(len(connack_packet)) - if connack_recvd[0:12] == connack_packet[0:12]: - # FIXME - this test could be tightened up a lot - rc = 0 + if connack_recvd[0:12] == connack_packet[0:12]: + # FIXME - this test could be tightened up a lot + rc = 0 - sock.close() -finally: - broker.terminate() - broker.wait() - (stdo, stde) = broker.communicate() - if rc: - print(stde.decode('utf-8')) - -exit(rc) + sock.close() + finally: + broker.terminate() + broker.wait() + (stdo, stde) = broker.communicate() + if rc: + print(stde.decode('utf-8')) + exit(rc) + + +do_test(True) +do_test(False) +exit(0)