diff --git a/ChangeLog.txt b/ChangeLog.txt index 90e14207..0b5e4c41 100644 --- a/ChangeLog.txt +++ b/ChangeLog.txt @@ -10,6 +10,8 @@ Broker: - Don't send retained messages when a shared subscription is made. - Fix log being truncated in Windows. - Fix client id not showing in log on failed connections, where possible. +- Fix broker sending duplicate CONNACK on failed MQTT v5 reauthentication. + Closes #2339. Client library: - Initialise sockpairR/W to invalid in `mosquitto_reinitialise()` to avoid diff --git a/src/handle_auth.c b/src/handle_auth.c index c2908f84..812f368a 100644 --- a/src/handle_auth.c +++ b/src/handle_auth.c @@ -129,18 +129,22 @@ int handle__auth(struct mosquitto *context) will__clear(context); } if(rc == MOSQ_ERR_AUTH){ - send__connack(context, 0, MQTT_RC_NOT_AUTHORIZED, NULL); if(context->state == mosq_cs_authenticating){ + send__connack(context, 0, MQTT_RC_NOT_AUTHORIZED, NULL); mosquitto__free(context->id); context->id = NULL; + }else{ + send__disconnect(context, MQTT_RC_NOT_AUTHORIZED, NULL); } return MOSQ_ERR_PROTOCOL; }else if(rc == MOSQ_ERR_NOT_SUPPORTED){ /* Client has requested extended authentication, but we don't support it. */ - send__connack(context, 0, MQTT_RC_BAD_AUTHENTICATION_METHOD, NULL); if(context->state == mosq_cs_authenticating){ + send__connack(context, 0, MQTT_RC_BAD_AUTHENTICATION_METHOD, NULL); mosquitto__free(context->id); context->id = NULL; + }else{ + send__disconnect(context, MQTT_RC_BAD_AUTHENTICATION_METHOD, NULL); } return MOSQ_ERR_PROTOCOL; }else{ diff --git a/test/broker/09-extended-auth-reauth.py b/test/broker/09-extended-auth-reauth.py new file mode 100755 index 00000000..1521bf8d --- /dev/null +++ b/test/broker/09-extended-auth-reauth.py @@ -0,0 +1,52 @@ +#!/usr/bin/env python3 + +from mosq_test_helper import * + +def write_config(filename, port): + with open(filename, 'w') as f: + f.write("port %d\n" % (port)) + f.write("auth_plugin c/auth_plugin_extended_reauth.so\n") + +port = mosq_test.get_port() +conf_file = os.path.basename(__file__).replace('.py', '.conf') +write_config(conf_file, port) + +rc = 1 + +# First authentication succeeds +props = mqtt5_props.gen_string_prop(mqtt5_props.PROP_AUTHENTICATION_METHOD, "repeat") +props += mqtt5_props.gen_string_prop(mqtt5_props.PROP_AUTHENTICATION_DATA, "repeat") +connect_packet = mosq_test.gen_connect("client-params-test", keepalive=42, proto_ver=5, properties=props) + +props = mqtt5_props.gen_uint16_prop(mqtt5_props.PROP_TOPIC_ALIAS_MAXIMUM, 10) +props += mqtt5_props.gen_string_prop(mqtt5_props.PROP_AUTHENTICATION_METHOD, "repeat") +props += mqtt5_props.gen_uint16_prop(mqtt5_props.PROP_RECEIVE_MAXIMUM, 20) +connack_packet = mosq_test.gen_connack(rc=0, proto_ver=5, properties=props, property_helper=False) + +# Reauthentication fails +props = mqtt5_props.gen_string_prop(mqtt5_props.PROP_AUTHENTICATION_METHOD, "repeat") +props += mqtt5_props.gen_string_prop(mqtt5_props.PROP_AUTHENTICATION_DATA, "repeat") +auth_packet = mosq_test.gen_auth(reason_code=mqtt5_rc.MQTT_RC_REAUTHENTICATE, properties=props) +disconnect_packet = mosq_test.gen_disconnect(reason_code=mqtt5_rc.MQTT_RC_NOT_AUTHORIZED, proto_ver=5) + +broker = mosq_test.start_broker(filename=os.path.basename(__file__), use_conf=True, port=port) + +try: + sock = mosq_test.do_client_connect(connect_packet, connack_packet, timeout=20, port=port) + mosq_test.do_send_receive(sock, auth_packet, disconnect_packet) + sock.close() + + rc = 0 +except mosq_test.TestError: + pass +finally: + os.remove(conf_file) + broker.terminate() + broker.wait() + (stdo, stde) = broker.communicate() + if rc: + print(stde.decode('utf-8')) + + +exit(rc) + diff --git a/test/broker/Makefile b/test/broker/Makefile index 7fc9752e..d046cf92 100644 --- a/test/broker/Makefile +++ b/test/broker/Makefile @@ -168,6 +168,7 @@ endif ./09-extended-auth-change-username.py ./09-extended-auth-multistep-reauth.py ./09-extended-auth-multistep.py + ./09-extended-auth-reauth.py ./09-extended-auth-single.py ./09-plugin-acl-change.py ./09-plugin-auth-acl-pub.py diff --git a/test/broker/c/Makefile b/test/broker/c/Makefile index babd396d..51fb2b79 100644 --- a/test/broker/c/Makefile +++ b/test/broker/c/Makefile @@ -8,6 +8,7 @@ PLUGIN_SRC = \ auth_plugin_acl_sub_denied.c \ auth_plugin_context_params.c \ auth_plugin_extended_multiple.c \ + auth_plugin_extended_reauth.c \ auth_plugin_extended_single.c \ auth_plugin_extended_single2.c \ auth_plugin_msg_params.c \ diff --git a/test/broker/c/auth_plugin_extended_reauth.c b/test/broker/c/auth_plugin_extended_reauth.c new file mode 100644 index 00000000..16e2842d --- /dev/null +++ b/test/broker/c/auth_plugin_extended_reauth.c @@ -0,0 +1,54 @@ +#include +#include +#include +#include +#include +#include + +static int auth_count = 0; + +int mosquitto_auth_plugin_version(void) +{ + return 4; +} + +int mosquitto_auth_plugin_init(void **user_data, struct mosquitto_opt *auth_opts, int auth_opt_count) +{ + return MOSQ_ERR_SUCCESS; +} + +int mosquitto_auth_plugin_cleanup(void *user_data, struct mosquitto_opt *auth_opts, int auth_opt_count) +{ + return MOSQ_ERR_SUCCESS; +} + +int mosquitto_auth_security_init(void *user_data, struct mosquitto_opt *auth_opts, int auth_opt_count, bool reload) +{ + return MOSQ_ERR_SUCCESS; +} + +int mosquitto_auth_security_cleanup(void *user_data, struct mosquitto_opt *auth_opts, int auth_opt_count, bool reload) +{ + return MOSQ_ERR_SUCCESS; +} + +int mosquitto_auth_acl_check(void *user_data, int access, struct mosquitto *client, const struct mosquitto_acl_msg *msg) +{ + return MOSQ_ERR_PLUGIN_DEFER; +} + + +int mosquitto_auth_start(void *user_data, struct mosquitto *client, const char *method, bool reauth, const void *data, uint16_t data_len, void **data_out, uint16_t *data_out_len) +{ + if(auth_count == 0){ + auth_count++; + return MOSQ_ERR_SUCCESS; + }else{ + return MOSQ_ERR_AUTH; + } +} + +int mosquitto_auth_continue(void *user_data, struct mosquitto *client, const char *method, const void *data, uint16_t data_len, void **data_out, uint16_t *data_out_len) +{ + return MOSQ_ERR_AUTH; +} diff --git a/test/broker/test.py b/test/broker/test.py index 25351e4e..2fbb39e4 100755 --- a/test/broker/test.py +++ b/test/broker/test.py @@ -138,6 +138,7 @@ tests = [ (1, './09-extended-auth-change-username.py'), (1, './09-extended-auth-multistep-reauth.py'), (1, './09-extended-auth-multistep.py'), + (1, './09-extended-auth-reauth.py'), (1, './09-extended-auth-single.py'), (1, './09-plugin-acl-change.py'), (1, './09-plugin-auth-acl-pub.py'),