Skip to content

Commit

Permalink
Breaking: allow_anonymous defaults to false.
Browse files Browse the repository at this point in the history
  • Loading branch information
ralight committed Sep 17, 2020
1 parent d7d3087 commit 97bd527
Show file tree
Hide file tree
Showing 55 changed files with 336 additions and 237 deletions.
7 changes: 6 additions & 1 deletion ChangeLog.txt
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,12 @@ Breaking changes:
If the broker is run as `mosquitto -c mosquitto.conf -p 1884`, and a
listener is defined in the configuration file, then the port defined on the
command line will be IGNORED, and no listener configured for it.

- All listeners now default to `allow_anonymous false` unless explicitly set
to true in the configuration file. This means that when configuring a
listener the user must either configure an authentication and access control
method, or set `allow_anonymous true`. When the broker is run without a
configured listener, and so binds to the loopback interface, anonymous
connections are allowed.

Broker:
- When running as root, if dropping privileges to the "mosquitto" user fails,
Expand Down
15 changes: 8 additions & 7 deletions man/mosquitto.conf.5.xml
Original file line number Diff line number Diff line change
Expand Up @@ -171,13 +171,10 @@
connect. If set to <replaceable>false</replaceable>
then another means of connection should be created to
control authenticated client access.</para>
<para>Defaults to <replaceable>true</replaceable> if no
other security options are set. If <option>password_file</option>
or <option>psk_file</option> is set, or if an
authentication plugin is loaded which implements
username/password or TLS-PSK checks, then
<option>allow_anonymous</option> defaults to
<replaceable>false</replaceable>.</para>
<para>Defaults to <replaceable>false</replaceable>,
unless no listeners are defined in the configuration
file, in which case it set to <replaceable>true</replaceable>,
but connections are only allowed from the local machine.</para>

<para>If <option>per_listener_settings</option> is
<replaceable>true</replaceable>, this option applies to
Expand All @@ -186,6 +183,10 @@
<replaceable>false</replaceable>, this option applies
to all listeners.</para>

<important><para>In version 1.6.x and earlier, this option defaulted
to <replaceable>true</replaceable> unless there was another security
option set.</para></important>

<para>Reloaded on reload signal.</para>
</listitem>
</varlistentry>
Expand Down
10 changes: 4 additions & 6 deletions mosquitto.conf
Original file line number Diff line number Diff line change
Expand Up @@ -668,12 +668,10 @@
# false then a password file should be created (see the
# password_file option) to control authenticated client access.
#
# Defaults to true if no other security options are set. If `password_file` or
# `psk_file` is set, or if an authentication plugin is loaded which implements
# username/password or TLS-PSK checks, then `allow_anonymous` defaults to
# false.
#
#allow_anonymous true
# Defaults to false, unless there are no listeners defined in the configuration
# file, in which case it is set to true, but connections are only allowed from
# the local machine.
#allow_anonymous false

# -----------------------------------------------------------------
# Default authentication and topic access control
Expand Down
86 changes: 15 additions & 71 deletions src/conf.c
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ static void config__init_reload(struct mosquitto_db *db, struct mosquitto__confi
config->listeners[i].security_options.auto_id_prefix_len = 0;
}

config->local_only = true;
config->allow_duplicate_messages = false;

mosquitto__free(config->security_options.acl_file);
Expand Down Expand Up @@ -240,12 +241,7 @@ void config__init(struct mosquitto_db *db, struct mosquitto__config *config)

config->daemon = false;
memset(&config->default_listener, 0, sizeof(struct mosquitto__listener));
config->default_listener.max_connections = -1;
config->default_listener.protocol = mp_mqtt;
config->default_listener.security_options.allow_anonymous = -1;
config->default_listener.security_options.allow_zero_length_clientid = true;
config->default_listener.maximum_qos = 2;
config->default_listener.max_topic_alias = 10;
listener__set_defaults(&config->default_listener);
}

void config__cleanup(struct mosquitto__config *config)
Expand Down Expand Up @@ -450,7 +446,6 @@ int config__parse_args(struct mosquitto_db *db, struct mosquitto__config *config
|| config->default_listener.security_options.password_file
|| config->default_listener.security_options.psk_file
|| config->default_listener.security_options.auth_plugin_config_count
|| config->default_listener.security_options.allow_anonymous != -1
|| config->default_listener.security_options.allow_zero_length_clientid != true
){

Expand Down Expand Up @@ -602,8 +597,7 @@ int config__read(struct mosquitto_db *db, struct mosquitto__config *config, bool
int len;
#endif
struct mosquitto__config config_reload;
struct mosquitto__auth_plugin *plugin;
int i, j;
int i;

if(reload){
memset(&config_reload, 0, sizeof(struct mosquitto__config));
Expand Down Expand Up @@ -641,69 +635,20 @@ int config__read(struct mosquitto_db *db, struct mosquitto__config *config, bool
}

/* If auth/access options are set and allow_anonymous not explicitly set, disallow anon. */
if(config->per_listener_settings){
for(i=0; i<config->listener_count; i++){
if(config->listeners[i].security_options.allow_anonymous == -1){
if(config->local_only == true){
config->security_options.allow_anonymous = true;
}else{
if(config->per_listener_settings){
for(i=0; i<config->listener_count; i++){
/* Default option if no security options set */
config->listeners[i].security_options.allow_anonymous = true;

if(config->listeners[i].security_options.password_file
|| config->listeners[i].security_options.psk_file){

/* allow_anonymous not set explicitly, some other security options
* have been set - so disable allow_anonymous
*/
if(config->listeners[i].security_options.allow_anonymous == -1){
config->listeners[i].security_options.allow_anonymous = false;
}

/* Check plugins loaded to see if they have username/password checks enabled */
for(j=0; j<config->listeners[i].security_options.auth_plugin_config_count; j++){
plugin = &config->listeners[i].security_options.auth_plugin_configs[j].plugin;

if(plugin->version == 3 || plugin->version == 2){
/* Version 2 and 3 always have username/password checks */
config->listeners[i].security_options.allow_anonymous = false;
break;
}else{
/* Version 4 has optional unpwd checks. */
if(plugin->unpwd_check_v4 != NULL){
config->listeners[i].security_options.allow_anonymous = false;
break;
}
}
}
}
}
}else{
if(config->security_options.allow_anonymous == -1){
/* Default option if no security options set */
config->security_options.allow_anonymous = true;

if(config->security_options.password_file
|| config->security_options.psk_file){

/* allow_anonymous not set explicitly, some other security options
* have been set - so disable allow_anonymous
*/
}else{
if(config->security_options.allow_anonymous == -1){
config->security_options.allow_anonymous = false;
}

/* Check plugins loaded to see if they have username/password checks enabled */
for(j=0; j<config->security_options.auth_plugin_config_count; j++){
plugin = &config->security_options.auth_plugin_configs[j].plugin;

if(plugin->version == 3 || plugin->version == 2){
/* Version 2 and 3 always have username/password checks */
config->security_options.allow_anonymous = false;
break;
}else{
/* Version 4 has optional unpwd checks. */
if(plugin->unpwd_check_v4 != NULL){
config->security_options.allow_anonymous = false;
break;
}
}
}
}
}
#ifdef WITH_PERSISTENCE
Expand Down Expand Up @@ -949,6 +894,7 @@ int config__read_file_core(struct mosquitto__config *config, bool reload, struct
}else if(!strcmp(token, "autosave_on_changes")){
if(conf__parse_bool(&token, "autosave_on_changes", &config->autosave_on_changes, saveptr)) return MOSQ_ERR_INVAL;
}else if(!strcmp(token, "bind_address")){
config->local_only = false;
if(reload) continue; // Listener not valid for reloading.
if(conf__parse_string(&token, "default listener bind_address", &config->default_listener.host, saveptr)) return MOSQ_ERR_INVAL;
if(conf__attempt_resolve(config->default_listener.host, "bind_address", MOSQ_LOG_ERR, "Error")){
Expand Down Expand Up @@ -1374,6 +1320,7 @@ int config__read_file_core(struct mosquitto__config *config, bool reload, struct
log__printf(NULL, MOSQ_LOG_WARNING, "Warning: TLS support not available.");
#endif
}else if(!strcmp(token, "listener")){
config->local_only = false;
token = strtok_r(NULL, " ", &saveptr);
if(token){
tmp_int = atoi(token);
Expand Down Expand Up @@ -1436,12 +1383,8 @@ int config__read_file_core(struct mosquitto__config *config, bool reload, struct
memset(cur_listener, 0, sizeof(struct mosquitto__listener));
}

cur_listener->security_options.allow_anonymous = -1;
cur_listener->security_options.allow_zero_length_clientid = true;
cur_listener->protocol = mp_mqtt;
listener__set_defaults(cur_listener);
cur_listener->port = tmp_int;
cur_listener->maximum_qos = 2;
cur_listener->max_topic_alias = 10;

mosquitto__free(cur_listener->host);
cur_listener->host = NULL;
Expand Down Expand Up @@ -1813,6 +1756,7 @@ int config__read_file_core(struct mosquitto__config *config, bool reload, struct
if(reload) continue; // pid file not valid for reloading.
if(conf__parse_string(&token, "pid_file", &config->pid_file, saveptr)) return MOSQ_ERR_INVAL;
}else if(!strcmp(token, "port")){
config->local_only = false;
if(reload) continue; // Listener not valid for reloading.
if(config->default_listener.port){
log__printf(NULL, MOSQ_LOG_WARNING, "Warning: Default listener port specified multiple times. Only the latest will be used.");
Expand Down
65 changes: 35 additions & 30 deletions src/handle_connect.c
Original file line number Diff line number Diff line change
Expand Up @@ -771,44 +771,16 @@ int handle__connect(struct mosquitto_db *db, struct mosquitto *context)
#ifdef FINAL_WITH_TLS_PSK
}
#endif /* FINAL_WITH_TLS_PSK */
}else{
}else
#endif /* WITH_TLS */
{
/* 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
}
#endif

if(context->listener->use_username_as_clientid){
if(context->username){
Expand Down Expand Up @@ -862,6 +834,39 @@ int handle__connect(struct mosquitto_db *db, struct mosquitto *context)
}
}
}else{
#ifdef WITH_TLS
if(context->listener->ssl_ctx && (context->listener->use_identity_as_username || context->listener->use_subject_as_username)){
/* Authentication assumed to be cleared */
}else
#endif
{
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;
}
}
return connect__on_authorised(db, context, NULL, 0);
}

Expand Down
18 changes: 13 additions & 5 deletions src/mosquitto.c
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,17 @@ void mosquitto__daemonise(void)
}


void listener__set_defaults(struct mosquitto__listener *listener)
{
listener->security_options.allow_anonymous = -1;
listener->security_options.allow_zero_length_clientid = true;
listener->protocol = mp_mqtt;
listener->max_connections = -1;
listener->maximum_qos = 2;
listener->max_topic_alias = 10;
}


int listeners__start_single_mqtt(struct mosquitto_db *db, mosq_sock_t **listensock, int *listensock_count, int *listensock_index, struct mosquitto__listener *listener)
{
int i;
Expand Down Expand Up @@ -244,12 +255,9 @@ int listeners__add_local(struct mosquitto_db *db, mosq_sock_t **listensock, int
db->config->listeners = listeners;
memset(&listeners[db->config->listener_count-1], 0, sizeof(struct mosquitto__listener));

listeners[db->config->listener_count-1].security_options.allow_anonymous = -1;
listeners[db->config->listener_count-1].security_options.allow_zero_length_clientid = true;
listeners[db->config->listener_count-1].protocol = mp_mqtt;
listener__set_defaults(&listeners[db->config->listener_count-1]);
listeners[db->config->listener_count-1].security_options.allow_anonymous = true;
listeners[db->config->listener_count-1].port = port;
listeners[db->config->listener_count-1].maximum_qos = 2;
listeners[db->config->listener_count-1].max_topic_alias = 10;
listeners[db->config->listener_count-1].host = mosquitto__strdup(host);
if(listeners[db->config->listener_count-1].host == NULL){
return MOSQ_ERR_NOMEM;
Expand Down
6 changes: 6 additions & 0 deletions src/mosquitto_broker_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,7 @@ struct mosquitto__config {
struct mosquitto__listener default_listener;
struct mosquitto__listener *listeners;
int listener_count;
bool local_only;
int log_dest;
int log_facility;
unsigned int log_type;
Expand Down Expand Up @@ -759,6 +760,11 @@ int mux__wait(void);
int mux__handle(struct mosquitto_db *db, mosq_sock_t *listensock, int listensock_count);
int mux__cleanup(struct mosquitto_db *db);

/* ============================================================
* Listener related functions
* ============================================================ */
void listener__set_defaults(struct mosquitto__listener *listener);

/* ============================================================
* Property related functions
* ============================================================ */
Expand Down
1 change: 0 additions & 1 deletion src/security.c
Original file line number Diff line number Diff line change
Expand Up @@ -692,7 +692,6 @@ int mosquitto_unpwd_check(struct mosquitto_db *db, struct mosquitto *context)
opts = &db->config->security_options;
}

rc = MOSQ_ERR_SUCCESS;
for(i=0; i<opts->auth_plugin_config_count; i++){
if(opts->auth_plugin_configs[i].plugin.version == 4
&& opts->auth_plugin_configs[i].plugin.unpwd_check_v4){
Expand Down
Loading

0 comments on commit 97bd527

Please sign in to comment.