diff --git a/ChangeLog.txt b/ChangeLog.txt index e6f24e7c07..36b0a9b79f 100644 --- a/ChangeLog.txt +++ b/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. diff --git a/man/mosquitto.conf.5.xml b/man/mosquitto.conf.5.xml index 00216c59da..37614dd4d6 100644 --- a/man/mosquitto.conf.5.xml +++ b/man/mosquitto.conf.5.xml @@ -202,6 +202,31 @@ Not currently reloaded on reload signal. + + [ true | false ] + + If true 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. + 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 hence have all ACL + checks delivered to your plugin by setting this option + to false. + Defaults to true. + Not currently reloaded on reload signal. + + seconds diff --git a/mosquitto.conf b/mosquitto.conf index f0e1402300..99af968cfb 100644 --- a/mosquitto.conf +++ b/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 # ----------------------------------------------------------------- diff --git a/src/conf.c b/src/conf.c index a4bfb01169..a3e233de19 100644 --- a/src/conf.c +++ b/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){ diff --git a/src/mosquitto_broker.h b/src/mosquitto_broker.h index 9ab3ab0841..f33007c875 100644 --- a/src/mosquitto_broker.h +++ b/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; diff --git a/src/security.c b/src/security.c index d0c376bd56..244a3a6a01 100644 --- a/src/security.c +++ b/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); }