From 73cc271d373ab43fd587fb1c0951d15289718500 Mon Sep 17 00:00:00 2001 From: "Roger A. Light" Date: Thu, 28 May 2020 00:23:49 +0100 Subject: [PATCH] Allow auth plugin to see all logins, unless accepted by password file. --- src/handle_connect.c | 72 +++++++++++++++------------------ src/mosquitto_broker_internal.h | 2 +- src/security.c | 34 +++++++++------- src/security_default.c | 2 +- 4 files changed, 54 insertions(+), 56 deletions(-) diff --git a/src/handle_connect.c b/src/handle_connect.c index fbd93af84f..1d7219962b 100644 --- a/src/handle_connect.c +++ b/src/handle_connect.c @@ -636,6 +636,12 @@ int handle__connect(struct mosquitto_db *db, struct mosquitto *context) goto handle_connect_error; } + /* Once context->id is set, if we return from this function with an error + * we must make sure that context->id is freed and set to NULL, so that the + * client isn't erroneously removed from the by_id hash table. */ + context->id = client_id; + client_id = NULL; + #ifdef WITH_TLS if(context->listener->ssl_ctx && (context->listener->use_identity_as_username || context->listener->use_subject_as_username)){ /* Don't need the username or password if provided */ @@ -762,49 +768,38 @@ int handle__connect(struct mosquitto_db *db, struct mosquitto *context) #endif /* FINAL_WITH_TLS_PSK */ }else{ #endif /* WITH_TLS */ - if(username_flag || password_flag){ - /* FIXME - these ensure the mosquitto_client_id() and - * mosquitto_client_username() functions work, but is hacky */ - context->id = client_id; - context->username = username; - rc = mosquitto_unpwd_check(db, context, username, password); - context->username = NULL; - context->id = NULL; - switch(rc){ - case MOSQ_ERR_SUCCESS: - break; - case MOSQ_ERR_AUTH: - if(context->protocol == mosq_p_mqtt5){ - send__connack(db, context, 0, MQTT_RC_NOT_AUTHORIZED, NULL); - }else{ - send__connack(db, context, 0, CONNACK_REFUSED_NOT_AUTHORIZED, NULL); - } - context__disconnect(db, context); - rc = 1; - goto handle_connect_error; - break; - default: - context__disconnect(db, context); - rc = 1; - goto handle_connect_error; - break; - } - context->username = username; - context->password = password; - username = NULL; /* Avoid free() in error: below. */ - password = NULL; - }else{ - if((db->config->per_listener_settings && context->listener->security_options.allow_anonymous == false) - || (!db->config->per_listener_settings && db->config->security_options.allow_anonymous == false)){ + /* FIXME - these ensure the mosquitto_client_id() and + * mosquitto_client_username() functions work, but is hacky */ + context->username = username; + context->password = password; + username = NULL; /* Avoid free() in error: below. */ + password = NULL; + rc = mosquitto_unpwd_check(db, context); + if(rc != MOSQ_ERR_SUCCESS){ + /* We must have context->id == NULL here so we don't later try and + * remove the client from the by_id hash table */ + mosquitto__free(context->id); + context->id = NULL; + } + switch(rc){ + case MOSQ_ERR_SUCCESS: + break; + case MOSQ_ERR_AUTH: if(context->protocol == mosq_p_mqtt5){ send__connack(db, context, 0, MQTT_RC_NOT_AUTHORIZED, NULL); }else{ send__connack(db, context, 0, CONNACK_REFUSED_NOT_AUTHORIZED, NULL); } + context__disconnect(db, context); rc = 1; goto handle_connect_error; - } + break; + default: + context__disconnect(db, context); + rc = 1; + goto handle_connect_error; + break; } #ifdef WITH_TLS } @@ -812,9 +807,9 @@ int handle__connect(struct mosquitto_db *db, struct mosquitto *context) if(context->listener->use_username_as_clientid){ if(context->username){ - mosquitto__free(client_id); - client_id = mosquitto__strdup(context->username); - if(!client_id){ + mosquitto__free(context->id); + context->id = mosquitto__strdup(context->username); + if(!context->id){ rc = MOSQ_ERR_NOMEM; goto handle_connect_error; } @@ -829,7 +824,6 @@ int handle__connect(struct mosquitto_db *db, struct mosquitto *context) } } context->clean_start = clean_start; - context->id = client_id; context->will = will_struct; if(context->auth_method){ diff --git a/src/mosquitto_broker_internal.h b/src/mosquitto_broker_internal.h index 70153be373..bf14f93c34 100644 --- a/src/mosquitto_broker_internal.h +++ b/src/mosquitto_broker_internal.h @@ -749,7 +749,7 @@ int mosquitto_security_init(struct mosquitto_db *db, bool reload); int mosquitto_security_apply(struct mosquitto_db *db); int mosquitto_security_cleanup(struct mosquitto_db *db, bool reload); int mosquitto_acl_check(struct mosquitto_db *db, struct mosquitto *context, const char *topic, long payloadlen, void* payload, int qos, bool retain, int access); -int mosquitto_unpwd_check(struct mosquitto_db *db, struct mosquitto *context, const char *username, const char *password); +int mosquitto_unpwd_check(struct mosquitto_db *db, struct mosquitto *context); int mosquitto_psk_key_get(struct mosquitto_db *db, struct mosquitto *context, const char *hint, const char *identity, char *key, int max_key_len); int mosquitto_security_init_default(struct mosquitto_db *db, bool reload); diff --git a/src/security.c b/src/security.c index eb37c88d5a..abf5820d20 100644 --- a/src/security.c +++ b/src/security.c @@ -673,13 +673,13 @@ int mosquitto_acl_check(struct mosquitto_db *db, struct mosquitto *context, cons return rc; } -int mosquitto_unpwd_check(struct mosquitto_db *db, struct mosquitto *context, const char *username, const char *password) +int mosquitto_unpwd_check(struct mosquitto_db *db, struct mosquitto *context) { int rc; int i; struct mosquitto__security_options *opts; - rc = mosquitto_unpwd_check_default(db, context, username, password); + rc = mosquitto_unpwd_check_default(db, context, context->username, context->password); if(rc != MOSQ_ERR_PLUGIN_DEFER){ return rc; } @@ -700,33 +700,37 @@ int mosquitto_unpwd_check(struct mosquitto_db *db, struct mosquitto *context, co rc = opts->auth_plugin_configs[i].plugin.unpwd_check_v4( opts->auth_plugin_configs[i].plugin.user_data, context, - username, - password); + context->username, + context->password); }else if(opts->auth_plugin_configs[i].plugin.version == 3){ rc = opts->auth_plugin_configs[i].plugin.unpwd_check_v3( opts->auth_plugin_configs[i].plugin.user_data, context, - username, - password); + context->username, + context->password); }else if(opts->auth_plugin_configs[i].plugin.version == 2){ rc = opts->auth_plugin_configs[i].plugin.unpwd_check_v2( opts->auth_plugin_configs[i].plugin.user_data, - username, - password); - }else{ - rc = MOSQ_ERR_INVAL; - } - if(rc != MOSQ_ERR_PLUGIN_DEFER){ - return rc; + context->username, + context->password); } } /* If all plugins deferred, this is a denial. If rc == MOSQ_ERR_SUCCESS - * here, then no plugins were configured. */ + * here, then no plugins were configured. Unless we have all deferred, and + * anonymous logins are allowed. */ if(rc == MOSQ_ERR_PLUGIN_DEFER){ - rc = MOSQ_ERR_AUTH; + if(context->username == NULL && + ((db->config->per_listener_settings && context->listener->security_options.allow_anonymous == true) + || (!db->config->per_listener_settings && db->config->security_options.allow_anonymous == true))){ + + return MOSQ_ERR_SUCCESS; + }else{ + return MOSQ_ERR_AUTH; + } } + return rc; } diff --git a/src/security_default.c b/src/security_default.c index 7cc10614d1..031eb2439e 100644 --- a/src/security_default.c +++ b/src/security_default.c @@ -1145,7 +1145,7 @@ int mosquitto_security_apply_default(struct mosquitto_db *db) #endif { /* Username/password check only if the identity/subject check not used */ - if(mosquitto_unpwd_check(db, context, context->username, context->password) != MOSQ_ERR_SUCCESS){ + if(mosquitto_unpwd_check(db, context) != MOSQ_ERR_SUCCESS){ mosquitto__set_state(context, mosq_cs_disconnecting); do_disconnect(db, context, MOSQ_ERR_AUTH); continue;