[462] Add auth_plugin_deny_special_chars option.

Auth plugins can be configured to disable the check for +# in
usernames/client ids with the auth_plugin_deny_special_chars option.

Thanks to wiebeytec.

Bug: https://github.com/eclipse/mosquitto/issues/462

ChangeLog.txt

@@ -16,6 +16,9 @@ Broker:
 - Fix CONNECT check for reserved=0, as per MQTT v3.1.1 check MQTT-3.1.2-3.
 - When the broker stop, wills for any connected clients are now "sent". Closes
   #477.
+- Auth plugins can be configured to disable the check for +# in
+  usernames/client ids with the auth_plugin_deny_special_chars option.
+  Partially closes #462.
 
 Clients:
 - Don't use / in auto-generated client ids.

man/mosquitto.conf.5.xml

@@ -202,6 +202,31 @@
 					<para>Not currently reloaded on reload signal.</para>
 				</listitem>
 			</varlistentry>
+			<varlistentry>
+				<term><option>auth_plugin_deny_special_chars</option> [ true | false ]</term>
+				<listitem>
+					<para>If <replaceable>true</replaceable> then before an ACL
+						check is made, the username/client id of the client
+						needing the check is searched for the presence of
+						either a '+' or '#' character. If either of these
+						characters is found in either the username or client
+						id, then the ACL check is denied before it is sent to
+						the plugin.</para>
+					<para>This check prevents the case where a malicious user
+						could circumvent an ACL check by using one of these
+						characters as their username or client id. This is the
+						same issue as was reported with mosquitto itself as
+						CVE-2017-7650.</para>
+					<para>If you are entirely sure that the plugin you are
+						using is not vulnerable to this attack (i.e. if you
+						never use usernames or client ids in topics) then you
+						can disable this extra check and hence have all ACL
+						checks delivered to your plugin by setting this option
+						to <replaceable>false</replaceable>.</para>
+					<para>Defaults to <replaceable>true</replaceable>.</para>
+					<para>Not currently reloaded on reload signal.</para>
+				</listitem>
+			</varlistentry>
 			<varlistentry>
 				<term><option>autosave_interval</option> <replaceable>seconds</replaceable></term>
 				<listitem>

mosquitto.conf

@@ -521,6 +521,22 @@
 # "Authentication and topic access plugin options" section below.
 #auth_plugin
 
+# If auth_plugin_deny_special_chars is true, the default, then before an ACL
+# check is made, the username/client id of the client needing the check is
+# searched for the presence of either a '+' or '#' character. If either of
+# these characters is found in either the username or client id, then the ACL
+# check is denied before it is sent to the plugin.o
+#
+# This check prevents the case where a malicious user could circumvent an ACL
+# check by using one of these characters as their username or client id. This
+# is the same issue as was reported with mosquitto itself as CVE-2017-7650.
+#
+# If you are entirely sure that the plugin you are using is not vulnerable to
+# this attack (i.e. if you never use usernames or client ids in topics) then
+# you can disable this extra check and have all ACL checks delivered to your
+# plugin by setting auth_plugin_deny_special_chars to false.
+#auth_plugin_deny_special_chars true
+
 # -----------------------------------------------------------------
 # Default authentication and topic access control
 # -----------------------------------------------------------------

src/conf.c

@@ -204,6 +204,7 @@ void mqtt3_config_init(struct mqtt3_config *config)
 	config->bridge_count = 0;
 #endif
 	config->auth_plugin = NULL;
+	config->auth_plugin_deny_special_chars = true;
 	config->verbose = false;
 	config->message_size_limit = 0;
 }
@@ -669,6 +670,9 @@ int _config_read_file_core(struct mqtt3_config *config, bool reload, const char
 				}else if(!strcmp(token, "auth_plugin")){
 					if(reload) continue; // Auth plugin not currently valid for reloading.
 					if(_conf_parse_string(&token, "auth_plugin", &config->auth_plugin, saveptr)) return MOSQ_ERR_INVAL;
+				}else if(!strcmp(token, "auth_plugin_deny_special_chars")){
+					if(reload) continue; // Auth plugin not currently valid for reloading.
+					if(_conf_parse_bool(&token, "auth_plugin_deny_special_chars", &config->auth_plugin_deny_special_chars, saveptr)) return MOSQ_ERR_INVAL;
 				}else if(!strcmp(token, "auto_id_prefix")){
 					if(_conf_parse_string(&token, "auto_id_prefix", &config->auto_id_prefix, saveptr)) return MOSQ_ERR_INVAL;
 					if(config->auto_id_prefix){

src/mosquitto_broker.h

@@ -106,6 +106,7 @@ struct mqtt3_config {
 	bool allow_anonymous;
 	bool allow_duplicate_messages;
 	bool allow_zero_length_clientid;
+	bool auth_plugin_deny_special_chars;
 	char *auto_id_prefix;
 	int auto_id_prefix_len;
 	int autosave_interval;

src/security.c

@@ -234,19 +234,21 @@ int mosquitto_acl_check(struct mosquitto_db *db, struct mosquitto *context, cons
 			username = context->username;
 		}
 
-		/* Check whether the client id or username contains a +, # or / and if
-		 * so deny access.
-		 *
-		 * Do this check for every message regardless, we have to protect the
-		 * plugins against possible pattern based attacks.
-		 */
-		if(username && strpbrk(username, "+#/")){
-			_mosquitto_log_printf(NULL, MOSQ_LOG_NOTICE, "ACL denying access to client with dangerous username \"%s\"", username);
-			return MOSQ_ERR_ACL_DENIED;
-		}
-		if(context->id && strpbrk(context->id, "+#/")){
-			_mosquitto_log_printf(NULL, MOSQ_LOG_NOTICE, "ACL denying access to client with dangerous client id \"%s\"", context->id);
-			return MOSQ_ERR_ACL_DENIED;
+		if(db->config->auth_plugin_deny_special_chars == true){
+			/* Check whether the client id or username contains a +, # or / and if
+			* so deny access.
+			*
+			* Do this check for every message regardless, we have to protect the
+			* plugins against possible pattern based attacks.
+			*/
+			if(username && strpbrk(username, "+#/")){
+				_mosquitto_log_printf(NULL, MOSQ_LOG_NOTICE, "ACL denying access to client with dangerous username \"%s\"", username);
+				return MOSQ_ERR_ACL_DENIED;
+			}
+			if(context->id && strpbrk(context->id, "+#/")){
+				_mosquitto_log_printf(NULL, MOSQ_LOG_NOTICE, "ACL denying access to client with dangerous client id \"%s\"", context->id);
+				return MOSQ_ERR_ACL_DENIED;
+			}
 		}
 		return db->auth_plugin.acl_check(db->auth_plugin.user_data, context->id, username, topic, access);
 	}