Don't disconnect clients when a plugin denies SUBSCRIBE.

Thanks to Ibrahim Koujar.

Bug: #1016

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.

src/handle_subscribe.c

@@ -112,16 +112,16 @@ 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:
  qos = 0x80;
  break;
  default:
  mosquitto__free(sub);
- return rc;
+ return rc2;
  }
  }
 

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:https://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)

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

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
 

test/broker/c/auth_plugin_acl_sub_denied.c

@@ -0,0 +1,49 @@
+#include <stdio.h>
+#include <string.h>
+#include <mosquitto.h>
+#include <mosquitto_broker.h>
+#include <mosquitto_plugin.h>
+
+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;
+}

test/broker/ptest.py

@@ -88,6 +88,7 @@
  (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'),