diff --git a/ChangeLog.txt b/ChangeLog.txt index 1dfe7edf..bc3f0991 100644 --- a/ChangeLog.txt +++ b/ChangeLog.txt @@ -33,6 +33,8 @@ Broker: removed. Closes #645. - Fix Windows version not starting if include_dir did not contain any files. Closes #566. +- When an authentication plugin denied access to a SUBSCRIBE, the client would + be disconnected incorrectly. This has been fixed. Closes #1016. Build: - Various fixes to ease building. diff --git a/src/handle_subscribe.c b/src/handle_subscribe.c index 3b2e2591..8f594351 100644 --- a/src/handle_subscribe.c +++ b/src/handle_subscribe.c @@ -112,8 +112,8 @@ int handle__subscribe(struct mosquitto_db *db, struct mosquitto *context) log__printf(NULL, MOSQ_LOG_DEBUG, "\t%s (QoS %d)", sub, qos); if(context->protocol == mosq_p_mqtt311){ - rc = mosquitto_acl_check(db, context, sub, 0, NULL, qos, false, MOSQ_ACL_SUBSCRIBE); - switch(rc){ + rc2 = mosquitto_acl_check(db, context, sub, 0, NULL, qos, false, MOSQ_ACL_SUBSCRIBE); + switch(rc2){ case MOSQ_ERR_SUCCESS: break; case MOSQ_ERR_ACL_DENIED: @@ -121,7 +121,7 @@ int handle__subscribe(struct mosquitto_db *db, struct mosquitto *context) break; default: mosquitto__free(sub); - return rc; + return rc2; } } diff --git a/test/broker/09-plugin-auth-acl-sub-denied.py b/test/broker/09-plugin-auth-acl-sub-denied.py new file mode 100755 index 00000000..83726653 --- /dev/null +++ b/test/broker/09-plugin-auth-acl-sub-denied.py @@ -0,0 +1,58 @@ +#!/usr/bin/env python + +# Test topic subscription. All SUBSCRIBE requests are denied. Check this +# produces the correct response, and check the client isn't disconnected (ref: +# issue #1016). + +import inspect, os, sys +# From http://stackoverflow.com/questions/279237/python-import-a-module-from-a-folder +cmd_subfolder = os.path.realpath(os.path.abspath(os.path.join(os.path.split(inspect.getfile( inspect.currentframe() ))[0],".."))) +if cmd_subfolder not in sys.path: + sys.path.insert(0, cmd_subfolder) + +import mosq_test + +def write_config(filename, port): + with open(filename, 'w') as f: + f.write("port %d\n" % (port)) + f.write("auth_plugin c/auth_plugin_acl_sub_denied.so\n") + f.write("allow_anonymous false\n") + +port = mosq_test.get_port() +conf_file = os.path.basename(__file__).replace('.py', '.conf') +write_config(conf_file, port) + +rc = 1 +keepalive = 10 +connect_packet = mosq_test.gen_connect("sub-denied-test", keepalive=keepalive, username="denied") +connack_packet = mosq_test.gen_connack(rc=0) + +mid = 53 +subscribe_packet = mosq_test.gen_subscribe(mid, "qos0/test", 0) +suback_packet = mosq_test.gen_suback(mid, 128) + +mid_pub = 54 +publish_packet = mosq_test.gen_publish("topic", qos=1, payload="test", mid=mid_pub) +puback_packet = mosq_test.gen_puback(mid_pub) + +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, subscribe_packet, suback_packet, "suback") + + mosq_test.do_send_receive(sock, publish_packet, puback_packet, "puback") + + rc = 0 + + sock.close() +finally: + os.remove(conf_file) + broker.terminate() + broker.wait() + (stdo, stde) = broker.communicate() + if rc: + print(stde) + + +exit(rc) diff --git a/test/broker/Makefile b/test/broker/Makefile index 0b4fee08..c70f1f7d 100644 --- a/test/broker/Makefile +++ b/test/broker/Makefile @@ -116,6 +116,7 @@ endif ./09-plugin-auth-unpwd-success.py ./09-plugin-auth-unpwd-fail.py ./09-plugin-auth-acl-sub.py + ./09-plugin-auth-acl-sub-denied.py ./09-plugin-auth-v2-unpwd-success.py ./09-plugin-auth-v2-unpwd-fail.py ./09-plugin-auth-defer-unpwd-success.py diff --git a/test/broker/c/Makefile b/test/broker/c/Makefile index f60f9d5f..0ce17768 100644 --- a/test/broker/c/Makefile +++ b/test/broker/c/Makefile @@ -2,7 +2,17 @@ CFLAGS=-I../../../lib -I../../../src -Wall -Werror -all : auth_plugin.so auth_plugin_pwd.so auth_plugin_acl.so auth_plugin_v2.so auth_plugin_msg_params.so auth_plugin_context_params.so 08 +PLUGINS= \ + auth_plugin.so \ + auth_plugin_pwd.so \ + auth_plugin_acl.so \ + auth_plugin_v2.so \ + auth_plugin_msg_params.so \ + auth_plugin_context_params.so \ + auth_plugin_acl_sub_denied.so + + +all : ${PLUGINS} 08 08 : 08-tls-psk-pub.test 08-tls-psk-bridge.test @@ -24,6 +34,9 @@ auth_plugin_context_params.so : auth_plugin_context_params.c auth_plugin_msg_params.so : auth_plugin_msg_params.c $(CC) ${CFLAGS} -fPIC -shared $^ -o $@ +auth_plugin_acl_sub_denied.so : auth_plugin_acl_sub_denied.c + $(CC) ${CFLAGS} -fPIC -shared $^ -o $@ + 08-tls-psk-pub.test : 08-tls-psk-pub.c $(CC) ${CFLAGS} $^ -o $@ ../../../lib/libmosquitto.so.1 diff --git a/test/broker/c/auth_plugin_acl_sub_denied.c b/test/broker/c/auth_plugin_acl_sub_denied.c new file mode 100644 index 00000000..4c5a26fa --- /dev/null +++ b/test/broker/c/auth_plugin_acl_sub_denied.c @@ -0,0 +1,49 @@ +#include +#include +#include +#include +#include + +int mosquitto_auth_plugin_version(void) +{ + return MOSQ_AUTH_PLUGIN_VERSION; +} + +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, const struct mosquitto *client, const struct mosquitto_acl_msg *msg) +{ + if(access == MOSQ_ACL_SUBSCRIBE){ + return MOSQ_ERR_ACL_DENIED; + }else{ + return MOSQ_ERR_SUCCESS; + } +} + +int mosquitto_auth_unpwd_check(void *user_data, const struct mosquitto *client, const char *username, const char *password) +{ + return MOSQ_ERR_SUCCESS; +} + +int mosquitto_auth_psk_key_get(void *user_data, const struct mosquitto *client, const char *hint, const char *identity, char *key, int max_key_len) +{ + return MOSQ_ERR_AUTH; +} diff --git a/test/broker/ptest.py b/test/broker/ptest.py index 27b0df6d..f39049e1 100755 --- a/test/broker/ptest.py +++ b/test/broker/ptest.py @@ -88,6 +88,7 @@ tests = [ (1, './09-plugin-auth-unpwd-success.py'), (1, './09-plugin-auth-unpwd-fail.py'), (1, './09-plugin-auth-acl-sub.py'), + (1, './09-plugin-auth-acl-sub-denied.py'), (1, './09-plugin-auth-v2-unpwd-success.py'), (1, './09-plugin-auth-v2-unpwd-fail.py'), (1, './09-plugin-auth-defer-unpwd-success.py'),