Merge branch 'master' into develop

ChangeLog.txt

@@ -72,6 +72,34 @@ Build:
   build and install static versions of the client libraries.
 
 
+1.4.13 - 20170627
+=================
+
+Security:
+- Fix CVE-2017-9868. The persistence file was readable by all local users,
+  potentially allowing sensitive information to be leaked.
+  This can also be fixed administratively, by restricting access to the
+  directory in which the persistence file is stored.
+
+Broker:
+- Fix for poor websockets performance.
+- Fix lazy bridges not timing out for idle_timeout. Closes #417.
+- Fix problems with large retained messages over websockets. Closes #427.
+- Set persistence file to only be readable by owner, except on Windows. Closes
+  #468.
+- 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.
+- Restrictions for CVE-2017-7650 have been relaxed - '/' is allowed in
+  usernames/client ids. Remainder of fix for #462.
+
+Clients:
+- Don't use / in auto-generated client ids.
+
+
 1.4.12 - 20170528
 =================
 

client/client_shared.c

@@ -939,14 +939,14 @@ int client_id_generate(struct mosq_config *cfg, const char *id_base)
 		hostname[0] = '\0';
 		gethostname(hostname, 256);
 		hostname[255] = '\0';
-		len = strlen(id_base) + strlen("/-") + 6 + strlen(hostname);
+		len = strlen(id_base) + strlen("|-") + 6 + strlen(hostname);
 		cfg->id = malloc(len);
 		if(!cfg->id){
 			if(!cfg->quiet) fprintf(stderr, "Error: Out of memory.\n");
 			mosquitto_lib_cleanup();
 			return 1;
 		}
-		snprintf(cfg->id, len, "%s/%d-%s", id_base, getpid(), hostname);
+		snprintf(cfg->id, len, "%s|%d-%s", id_base, getpid(), hostname);
 		if(strlen(cfg->id) > MOSQ_MQTT_ID_MAX_LENGTH){
 			/* Enforce maximum client id length of 23 characters */
 			cfg->id[MOSQ_MQTT_ID_MAX_LENGTH] = '\0';

lib/mosquitto.c

@@ -645,7 +645,7 @@ int mosquitto_tls_set(struct mosquitto *mosq, const char *cafile, const char *ca
 	mosquitto__free(mosq->tls_cafile);
 	mosq->tls_cafile = NULL;
 	if(cafile){
-		fptr = mosquitto__fopen(cafile, "rt");
+		fptr = mosquitto__fopen(cafile, "rt", false);
 		if(fptr){
 			fclose(fptr);
 		}else{
@@ -670,7 +670,7 @@ int mosquitto_tls_set(struct mosquitto *mosq, const char *cafile, const char *ca
 	mosquitto__free(mosq->tls_certfile);
 	mosq->tls_certfile = NULL;
 	if(certfile){
-		fptr = mosquitto__fopen(certfile, "rt");
+		fptr = mosquitto__fopen(certfile, "rt", false);
 		if(fptr){
 			fclose(fptr);
 		}else{
@@ -690,7 +690,7 @@ int mosquitto_tls_set(struct mosquitto *mosq, const char *cafile, const char *ca
 	mosquitto__free(mosq->tls_keyfile);
 	mosq->tls_keyfile = NULL;
 	if(keyfile){
-		fptr = mosquitto__fopen(keyfile, "rt");
+		fptr = mosquitto__fopen(keyfile, "rt", false);
 		if(fptr){
 			fclose(fptr);
 		}else{

lib/mosquitto_internal.h

@@ -230,6 +230,7 @@ struct mosquitto {
 	struct libwebsocket *wsi;
 #    endif
 #  endif
+	bool ws_want_write;
 #else
 #  ifdef WITH_SOCKS
 	char *socks5_host;

lib/net_mosq.c

@@ -409,6 +409,7 @@ int net__socket_connect_tls(struct mosquitto *mosq)
 {
 	int ret, err;
 
+	ERR_clear_error();
 	ret = SSL_connect(mosq->ssl);
 	if(ret != 1) {
 		err = SSL_get_error(mosq->ssl, ret);
@@ -626,6 +627,7 @@ ssize_t net__read(struct mosquitto *mosq, void *buf, size_t count)
 	errno = 0;
 #ifdef WITH_TLS
 	if(mosq->ssl){
+		ERR_clear_error();
 		ret = SSL_read(mosq->ssl, buf, count);
 		if(ret <= 0){
 			err = SSL_get_error(mosq->ssl, ret);
@@ -670,6 +672,7 @@ ssize_t net__write(struct mosquitto *mosq, void *buf, size_t count)
 #ifdef WITH_TLS
 	if(mosq->ssl){
 		mosq->want_write = false;
+		ERR_clear_error();
 		ret = SSL_write(mosq->ssl, buf, count);
 		if(ret < 0){
 			err = SSL_get_error(mosq->ssl, ret);

lib/util_mosq.c

@@ -18,7 +18,12 @@ Contributors:
 #include <string.h>
 
 #ifdef WIN32
-#include <winsock2.h>
+#  include <winsock2.h>
+#  include <aclapi.h>
+#  include <io.h>
+#  include <lmcons.h>
+#else
+#  include <sys/stat.h>
 #endif
 
 
@@ -299,7 +304,7 @@ int mosquitto__hex2bin(const char *hex, unsigned char *bin, int bin_max_len)
 }
 #endif
 
-FILE *mosquitto__fopen(const char *path, const char *mode)
+FILE *mosquitto__fopen(const char *path, const char *mode, bool restrict_read)
 {
 #ifdef WIN32
 	char buf[4096];
@@ -308,10 +313,69 @@ FILE *mosquitto__fopen(const char *path, const char *mode)
 	if(rc == 0 || rc > 4096){
 		return NULL;
 	}else{
-		return fopen(buf, mode);
+		if (restrict_read) {
+			HANDLE hfile;
+			SECURITY_ATTRIBUTES sec;
+			EXPLICIT_ACCESS ea;
+			PACL pacl = NULL;
+			char username[UNLEN + 1];
+			int ulen = UNLEN;
+			SECURITY_DESCRIPTOR sd;
+
+			GetUserName(username, &ulen);
+			if (!InitializeSecurityDescriptor(&sd, SECURITY_DESCRIPTOR_REVISION)) {
+				return NULL;
+			}
+			BuildExplicitAccessWithName(&ea, username, GENERIC_ALL, SET_ACCESS, NO_INHERITANCE);
+			if (SetEntriesInAcl(1, &ea, NULL, &pacl) != ERROR_SUCCESS) {
+				return NULL;
+			}
+			if (!SetSecurityDescriptorDacl(&sd, TRUE, pacl, FALSE)) {
+				LocalFree(pacl);
+				return NULL;
+			}
+
+			sec.nLength = sizeof(SECURITY_ATTRIBUTES);
+			sec.bInheritHandle = FALSE;
+			sec.lpSecurityDescriptor = &sd;
+
+			hfile = CreateFile(buf, GENERIC_READ | GENERIC_WRITE, 0,
+				&sec,
+				CREATE_NEW,
+				FILE_ATTRIBUTE_NORMAL,
+				NULL);
+
+			LocalFree(pacl);
+
+			int fd = _open_osfhandle((intptr_t)hfile, 0);
+			if (fd < 0) {
+				return NULL;
+			}
+
+			FILE *fptr = _fdopen(fd, mode);
+			if (!fptr) {
+				_close(fd);
+				return NULL;
+			}
+			return fptr;
+
+		}else {
+			return fopen(buf, mode);
+		}
 	}
 #else
-	return fopen(path, mode);
+	if (restrict_read) {
+		FILE *fptr;
+		mode_t old_mask;
+
+		old_mask = umask(0077);
+		fptr = fopen(path, mode);
+		umask(old_mask);
+
+		return fptr;
+	}else{
+		return fopen(path, mode);
+	}
 #endif
 }
 

lib/util_mosq.h

@@ -31,7 +31,7 @@ void mosquitto__check_keepalive(struct mosquitto_db *db, struct mosquitto *mosq)
 void mosquitto__check_keepalive(struct mosquitto *mosq);
 #endif
 uint16_t mosquitto__mid_generate(struct mosquitto *mosq);
-FILE *mosquitto__fopen(const char *path, const char *mode);
+FILE *mosquitto__fopen(const char *path, const char *mode, bool restrict_read);
 
 #ifdef REAL_WITH_TLS_PSK
 int mosquitto__hex2bin(const char *hex, unsigned char *bin, int bin_max_len);

man/mosquitto.8.xml

@@ -483,7 +483,7 @@
 	<refsect1>
 		<title>Bugs</title>
 		<para><command>mosquitto</command> bug information can be found at
-			<uri type="webpage">https://github.com/eclipse/mosquitto/issues</uri></para>
+			<ulink url="https://github.com/eclipse/mosquitto/issues"/></para>
 	</refsect1>
 
 	<refsect1>

man/mosquitto.conf.5.xml

@@ -212,6 +212,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>
@@ -1453,7 +1478,7 @@ topic clients/total in 0 test/mosquitto/org $SYS/broker/
 	<refsect1>
 		<title>Bugs</title>
 		<para><command>mosquitto</command> bug information can be found at
-			<uri type="webpage">https://github.com/eclipse/mosquitto/issues</uri></para>
+			<ulink url="https://github.com/eclipse/mosquitto/issues"/></para>
 	</refsect1>
 
 	<refsect1>

man/mosquitto_passwd.1.xml

@@ -124,7 +124,7 @@
 	<refsect1>
 		<title>Bugs</title>
 		<para><command>mosquitto</command> bug information can be found at
-			<uri type="webpage">https://github.com/eclipse/mosquitto/issues</uri></para>
+			<ulink url="https://github.com/eclipse/mosquitto/issues"/></para>
 	</refsect1>
 
 	<refsect1>

man/mosquitto_pub.1.xml

@@ -497,7 +497,7 @@
 	<refsect1>
 		<title>Bugs</title>
 		<para><command>mosquitto</command> bug information can be found at
-			<uri type="webpage">https://github.com/eclipse/mosquitto/issues</uri></para>
+			<ulink url="https://github.com/eclipse/mosquitto/issues"/></para>
 	</refsect1>
 
 	<refsect1>

man/mosquitto_sub.1.xml

@@ -717,7 +717,7 @@
 	<refsect1>
 		<title>Bugs</title>
 		<para><command>mosquitto</command> bug information can be found at
-			<uri type="webpage">https://github.com/eclipse/mosquitto/issues</uri></para>
+			<ulink url="https://github.com/eclipse/mosquitto/issues"/></para>
 	</refsect1>
 
 	<refsect1>

set-version.sh

@@ -2,7 +2,7 @@
 
 MAJOR=1
 MINOR=4
-REVISION=12
+REVISION=13
 
 sed -i "s/^VERSION=.*/VERSION=${MAJOR}.${MINOR}.${REVISION}/" config.mk
 

src/conf.c

@@ -706,8 +706,16 @@ int config__read_file_core(struct mosquitto__config *config, bool reload, const
 					cur_auth_plugin->path = NULL;
 					cur_auth_plugin->options = NULL;
 					cur_auth_plugin->option_count = 0;
+					cur_auth_plugin->deny_special_chars = true;
 					config->auth_plugin_count++;
 					if(conf__parse_string(&token, "auth_plugin", &cur_auth_plugin->path, 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(!cur_auth_plugin){
+						log__printf(NULL, MOSQ_LOG_ERR, "Error: An auth_plugin_deny_special_chars option exists in the config file without an auth_plugin.");
+						return MOSQ_ERR_INVAL;
+					}
+					if(conf__parse_bool(&token, "auth_plugin_deny_special_chars", &cur_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){
@@ -1825,7 +1833,7 @@ int config__read_file(struct mosquitto__config *config, bool reload, const char
 	int rc;
 	FILE *fptr = NULL;
 
-	fptr = mosquitto__fopen(file, "rt");
+	fptr = mosquitto__fopen(file, "rt", false);
 	if(!fptr){
 		log__printf(NULL, MOSQ_LOG_ERR, "Error: Unable to open config file %s\n", file);
 		return 1;

src/context.c

@@ -143,6 +143,8 @@ void context__cleanup(struct mosquitto_db *db, struct mosquitto *context, bool d
 	mosquitto__free(context->address);
 	context->address = NULL;
 
+	context__send_will(db, context);
+
 	if(context->id){
 		assert(db); /* db can only be NULL here if the client hasn't sent a
 					   CONNECT and hence wouldn't have an id. */
@@ -163,12 +165,6 @@ void context__cleanup(struct mosquitto_db *db, struct mosquitto *context, bool d
 		context->out_packet = context->out_packet->next;
 		mosquitto__free(packet);
 	}
-	if(context->will){
-		mosquitto__free(context->will->topic);
-		mosquitto__free(context->will->payload);
-		mosquitto__free(context->will);
-		context->will = NULL;
-	}
 	if(do_free || context->clean_session){
 		msg = context->inflight_msgs;
 		while(msg){
@@ -194,7 +190,8 @@ void context__cleanup(struct mosquitto_db *db, struct mosquitto *context, bool d
 	}
 }
 
-void context__disconnect(struct mosquitto_db *db, struct mosquitto *ctxt)
+
+void context__send_will(struct mosquitto_db *db, struct mosquitto *ctxt)
 {
 	if(ctxt->state != mosq_cs_disconnecting && ctxt->will){
 		if(mosquitto_acl_check(db, ctxt, ctxt->will->topic, MOSQ_ACL_WRITE) == MOSQ_ERR_SUCCESS){
@@ -208,6 +205,13 @@ void context__disconnect(struct mosquitto_db *db, struct mosquitto *ctxt)
 		mosquitto__free(ctxt->will);
 		ctxt->will = NULL;
 	}
+}
+
+
+void context__disconnect(struct mosquitto_db *db, struct mosquitto *ctxt)
+{
+	context__send_will(db, ctxt);
+
 	ctxt->disconnect_t = time(NULL);
 	net__socket_close(db, ctxt);
 }

src/handle_connect.c

@@ -196,6 +196,13 @@ int handle__connect(struct mosquitto_db *db, struct mosquitto *context)
 		rc = 1;
 		goto handle_connect_error;
 	}
+	if(context->protocol == mosq_p_mqtt311){
+		if((connect_flags & 0x01) != 0x00){
+			rc = MOSQ_ERR_PROTOCOL;
+			goto handle_connect_error;
+		}
+	}
+
 	clean_session = (connect_flags & 0x02) >> 1;
 	will = connect_flags & 0x04;
 	will_qos = (connect_flags & 0x18) >> 3;

src/logging.c

@@ -69,7 +69,7 @@ int log__init(struct mosquitto__config *config)
 		if(drop_privileges(config, true)){
 			return 1;
 		}
-		config->log_fptr = mosquitto__fopen(config->log_file, "at");
+		config->log_fptr = mosquitto__fopen(config->log_file, "at", true);
 		if(!config->log_fptr){
 			log_destinations = MQTT3_LOG_STDERR;
 			log_priorities = MOSQ_LOG_ERR;

src/loop.c

@@ -105,13 +105,12 @@ int mosquitto_main_loop(struct mosquitto_db *db, mosq_sock_t *listensock, int li
 #endif
 	int i;
 	struct pollfd *pollfds = NULL;
-	int pollfd_count = 0;
 	int pollfd_index;
+	int pollfd_max;
 #ifdef WITH_BRIDGE
 	mosq_sock_t bridge_sock;
 	int rc;
 #endif
-	int context_count;
 	time_t expiration_check_time = 0;
 	char *id;
 
@@ -121,8 +120,21 @@ int mosquitto_main_loop(struct mosquitto_db *db, mosq_sock_t *listensock, int li
 	sigaddset(&sigblock, SIGTERM);
 	sigaddset(&sigblock, SIGUSR1);
 	sigaddset(&sigblock, SIGUSR2);
+	sigaddset(&sigblock, SIGHUP);
 #endif
 
+#ifdef WIN32
+	pollfd_max = _getmaxstdio();
+#else
+	pollfd_max = getdtablesize();
+#endif
+
+	pollfds = mosquitto__malloc(sizeof(struct pollfd)*pollfd_max);
+	if(!pollfds){
+		log__printf(NULL, MOSQ_LOG_ERR, "Error: Out of memory.");
+		return MOSQ_ERR_NOMEM;
+	}
+
 	if(db->config->persistent_client_expiration > 0){
 		expiration_check_time = time(NULL) + 3600;
 	}
@@ -135,21 +147,7 @@ int mosquitto_main_loop(struct mosquitto_db *db, mosq_sock_t *listensock, int li
 		}
 #endif
 
-		context_count = HASH_CNT(hh_sock, db->contexts_by_sock);
-#ifdef WITH_BRIDGE
-		context_count += db->bridge_count;
-#endif
-
-		if(listensock_count + context_count > pollfd_count || !pollfds){
-			pollfd_count = listensock_count + context_count;
-			pollfds = mosquitto__realloc(pollfds, sizeof(struct pollfd)*pollfd_count);
-			if(!pollfds){
-				log__printf(NULL, MOSQ_LOG_ERR, "Error: Out of memory.");
-				return MOSQ_ERR_NOMEM;
-			}
-		}
-
-		memset(pollfds, -1, sizeof(struct pollfd)*pollfd_count);
+		memset(pollfds, -1, sizeof(struct pollfd)*pollfd_max);
 
 		pollfd_index = 0;
 		for(i=0; i<listensock_count; i++){
@@ -197,8 +195,9 @@ int mosquitto_main_loop(struct mosquitto_db *db, mosq_sock_t *listensock, int li
 						pollfds[pollfd_index].fd = context->sock;
 						pollfds[pollfd_index].events = POLLIN;
 						pollfds[pollfd_index].revents = 0;
-						if(context->current_out_packet || context->state == mosq_cs_connect_pending){
+						if(context->current_out_packet || context->state == mosq_cs_connect_pending || context->ws_want_write){
 							pollfds[pollfd_index].events |= POLLOUT;
+							context->ws_want_write = false;
 						}
 						context->pollfd_index = pollfd_index;
 						pollfd_index++;
@@ -276,7 +275,7 @@ int mosquitto_main_loop(struct mosquitto_db *db, mosq_sock_t *listensock, int li
 								if(context->adns->ar_result){
 									freeaddrinfo(context->adns->ar_result);
 								}
-								_mosquitto_free(context->adns);
+								mosquitto__free(context->adns);
 								context->adns = NULL;
 							}
 						}else{
@@ -429,7 +428,11 @@ void do_disconnect(struct mosquitto_db *db, struct mosquitto *context)
 		if(context->wsi){
 			libwebsocket_callback_on_writable(context->ws_context, context->wsi);
 		}
-		context->sock = INVALID_SOCKET;
+		if(context->sock != INVALID_SOCKET){
+			HASH_DELETE(hh_sock, db->contexts_by_sock, context);
+			context->sock = INVALID_SOCKET;
+			context->pollfd_index = -1;
+		}
 	}else
 #endif
 	{
@@ -475,6 +478,18 @@ static void loop_handle_reads_writes(struct mosquitto_db *db, struct pollfd *pol
 		}
 
 		assert(pollfds[context->pollfd_index].fd == context->sock);
+
+#ifdef WITH_WEBSOCKETS
+		if(context->wsi){
+			struct lws_pollfd wspoll;
+			wspoll.fd = pollfds[context->pollfd_index].fd;
+			wspoll.events = pollfds[context->pollfd_index].events;
+			wspoll.revents = pollfds[context->pollfd_index].revents;
+			lws_service_fd(lws_get_context(context->wsi), &wspoll);
+			continue;
+		}
+#endif
+
 #ifdef WITH_TLS
 		if(pollfds[context->pollfd_index].revents & POLLOUT ||
 				context->want_write ||
@@ -504,6 +519,12 @@ static void loop_handle_reads_writes(struct mosquitto_db *db, struct pollfd *pol
 		if(context->pollfd_index < 0){
 			continue;
 		}
+#ifdef WITH_WEBSOCKETS
+		if(context->wsi){
+			// Websocket are already handled above
+			continue;
+		}
+#endif
 
 #ifdef WITH_TLS
 		if(pollfds[context->pollfd_index].revents & POLLIN ||
@@ -518,7 +539,7 @@ static void loop_handle_reads_writes(struct mosquitto_db *db, struct pollfd *pol
 				}
 			}while(SSL_DATA_PENDING(context));
 		}
-		if(pollfds[context->pollfd_index].revents & (POLLERR | POLLNVAL | POLLHUP)){
+		if(context->pollfd_index >= 0 && pollfds[context->pollfd_index].revents & (POLLERR | POLLNVAL | POLLHUP)){
 			do_disconnect(db, context);
 			continue;
 		}

src/mosquitto.c

@@ -246,7 +246,7 @@ int main(int argc, char *argv[])
 	}
 
 	if(config.daemon && config.pid_file){
-		pid = mosquitto__fopen(config.pid_file, "wt");
+		pid = mosquitto__fopen(config.pid_file, "wt", false);
 		if(pid){
 			fprintf(pid, "%d", getpid());
 			fclose(pid);
@@ -365,12 +365,6 @@ int main(int argc, char *argv[])
 	log__printf(NULL, MOSQ_LOG_INFO, "mosquitto version %s terminating", VERSION);
 	log__close(&config);
 
-#ifdef WITH_PERSISTENCE
-	if(config.persistence){
-		persist__backup(&int_db, true);
-	}
-#endif
-
 #ifdef WITH_WEBSOCKETS
 	for(i=0; i<int_db.config->listener_count; i++){
 		if(int_db.config->listeners[i].ws_context){
@@ -402,6 +396,12 @@ int main(int argc, char *argv[])
 #endif
 	context__free_disused(&int_db);
 
+#ifdef WITH_PERSISTENCE
+	if(config.persistence){
+		persist__backup(&int_db, true);
+	}
+#endif
+
 	db__close(&int_db);
 
 	if(listensock){

src/mosquitto_broker.h

@@ -37,6 +37,7 @@ enum mosquitto_protocol {
  *
  * ========================================================================= */
 
+
 /*
  * Function: mosquitto_log_printf
  *

src/mosquitto_broker_internal.h

@@ -174,6 +174,7 @@ struct mosquitto__auth_plugin_config
 	char *path;
 	struct mosquitto_opt *options;
 	int option_count;
+	bool deny_special_chars;
 };
 
 struct mosquitto__config {
@@ -557,6 +558,7 @@ void context__cleanup(struct mosquitto_db *db, struct mosquitto *context, bool d
 void context__disconnect(struct mosquitto_db *db, struct mosquitto *context);
 void context__add_to_disused(struct mosquitto_db *db, struct mosquitto *context);
 void context__free_disused(struct mosquitto_db *db);
+void context__send_will(struct mosquitto_db *db, struct mosquitto *context);
 
 /* ============================================================
  * Logging functions

src/net.c

@@ -161,6 +161,7 @@ int net__socket_accept(struct mosquitto_db *db, mosq_sock_t listensock)
 					new_context->want_write = true;
 					bio = BIO_new_socket(new_sock, BIO_NOCLOSE);
 					SSL_set_bio(new_context->ssl, bio, bio);
+					ERR_clear_error();
 					rc = SSL_accept(new_context->ssl);
 					if(rc != 1){
 						rc = SSL_get_error(new_context->ssl, rc);

src/persist.c

@@ -401,7 +401,7 @@ int persist__backup(struct mosquitto_db *db, bool shutdown)
 	}
 #endif
 
-	db_fptr = mosquitto__fopen(outfile, "wb");
+	db_fptr = mosquitto__fopen(outfile, "wb", true);
 	if(db_fptr == NULL){
 		log__printf(NULL, MOSQ_LOG_INFO, "Error saving in-memory database, unable to open %s for writing.", outfile);
 		goto error;
@@ -833,7 +833,7 @@ int persist__restore(struct mosquitto_db *db)
 
 	db->msg_store_load = NULL;
 
-	fptr = mosquitto__fopen(db->config->persistence_filepath, "rb");
+	fptr = mosquitto__fopen(db->config->persistence_filepath, "rb", false);
 	if(fptr == NULL) return MOSQ_ERR_SUCCESS;
 	rlen = fread(&header, 1, 15, fptr);
 	if(rlen == 0){

src/security.c

@@ -372,19 +372,21 @@ int mosquitto_acl_check(struct mosquitto_db *db, struct mosquitto *context, cons
 		msg.topic = topic;
 
 		username = mosquitto_client_username(context);
-		/* 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, "+#/")){
-			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, "+#/")){
-			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_plugins[i].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, "+#")){
+				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, "+#")){
+				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->auth_plugins[i].version == 3){

src/security_default.c

@@ -266,18 +266,18 @@ int mosquitto_acl_check_default(struct mosquitto_db *db, struct mosquitto *conte
 
 	if(acl_root){
 		/* We are using pattern based acls. Check whether the username or
-		 * client id contains a +, # or / and if so deny access.
+		 * client id contains a + or # and if so deny access.
 		 *
 		 * Without this, a malicious client may configure its username/client
 		 * id to bypass ACL checks (or have a username/client id that cannot
 		 * publish or receive messages to its own place in the hierarchy).
 		 */
-		if(context->username && strpbrk(context->username, "+#/")){
+		if(context->username && strpbrk(context->username, "+#")){
 			log__printf(NULL, MOSQ_LOG_NOTICE, "ACL denying access to client with dangerous username \"%s\"", context->username);
 			return MOSQ_ERR_ACL_DENIED;
 		}
 
-		if(context->id && strpbrk(context->id, "+#/")){
+		if(context->id && strpbrk(context->id, "+#")){
 			log__printf(NULL, MOSQ_LOG_NOTICE, "ACL denying access to client with dangerous client id \"%s\"", context->id);
 			return MOSQ_ERR_ACL_DENIED;
 		}
@@ -354,7 +354,7 @@ static int aclfile__parse(struct mosquitto_db *db)
 	if(!db || !db->config) return MOSQ_ERR_INVAL;
 	if(!db->config->acl_file) return MOSQ_ERR_SUCCESS;
 
-	aclfile = mosquitto__fopen(db->config->acl_file, "rt");
+	aclfile = mosquitto__fopen(db->config->acl_file, "rt", false);
 	if(!aclfile){
 		log__printf(NULL, MOSQ_LOG_ERR, "Error: Unable to open acl_file \"%s\".", db->config->acl_file);
 		return 1;
@@ -509,7 +509,7 @@ static int pwfile__parse(const char *file, struct mosquitto__unpwd **root)
 	int len;
 	char *saveptr = NULL;
 
-	pwfile = mosquitto__fopen(file, "rt");
+	pwfile = mosquitto__fopen(file, "rt", false);
 	if(!pwfile){
 		log__printf(NULL, MOSQ_LOG_ERR, "Error: Unable to open pwfile \"%s\".", file);
 		return 1;

src/websockets.c

@@ -200,6 +200,8 @@ static int callback_mqtt(struct libwebsocket_context *context,
 				u->mosq = NULL;
 				return -1;
 			}
+			mosq->sock = libwebsocket_get_socket_fd(wsi);
+			HASH_ADD(hh_sock, db->contexts_by_sock, sock, sizeof(mosq->sock), mosq);
 			break;
 
 		case LWS_CALLBACK_CLOSED:
@@ -208,6 +210,11 @@ static int callback_mqtt(struct libwebsocket_context *context,
 			}
 			mosq = u->mosq;
 			if(mosq){
+				if(mosq->sock > 0){
+					HASH_DELETE(hh_sock, db->contexts_by_sock, mosq);
+					mosq->sock = INVALID_SOCKET;
+					mosq->pollfd_index = -1;
+				}
 				mosq->wsi = NULL;
 				do_disconnect(db, mosq);
 			}
@@ -232,7 +239,7 @@ static int callback_mqtt(struct libwebsocket_context *context,
 				}
 			}
 
-			while(mosq->current_out_packet && !lws_send_pipe_choked(mosq->wsi)){
+			if(mosq->current_out_packet && !lws_send_pipe_choked(mosq->wsi)){
 				packet = mosq->current_out_packet;
 
 				if(packet->pos == 0 && packet->to_process == packet->packet_length){
@@ -268,10 +275,6 @@ static int callback_mqtt(struct libwebsocket_context *context,
 				mosquitto__free(packet);
 
 				mosq->next_msg_out = mosquitto_time() + mosq->keepalive;
-
-				if(mosq->current_out_packet){
-					libwebsocket_callback_on_writable(mosq->ws_context, mosq->wsi);
-				}
 			}
 			if(mosq->current_out_packet){
 				libwebsocket_callback_on_writable(mosq->ws_context, mosq->wsi);
@@ -387,6 +390,9 @@ static int callback_http(struct libwebsocket_context *context,
 	char *filename, *filename_canonical;
 	unsigned char buf[4096];
 	struct stat filestat;
+	struct mosquitto_db *db = &int_db;
+	struct mosquitto *mosq;
+	struct lws_pollargs *pollargs = (struct lws_pollargs *)in;
 
 	/* FIXME - ssl cert verification is done here. */
 
@@ -554,6 +560,15 @@ static int callback_http(struct libwebsocket_context *context,
 			}
 			break;
 
+		case LWS_CALLBACK_ADD_POLL_FD:
+		case LWS_CALLBACK_DEL_POLL_FD:
+		case LWS_CALLBACK_CHANGE_MODE_POLL_FD:
+			HASH_FIND(hh_sock, db->contexts_by_sock, &pollargs->fd, sizeof(pollargs->fd), mosq);
+			if(mosq && (pollargs->events & POLLOUT)){
+				mosq->ws_want_write = true;
+			}
+			break;
+
 		default:
 			return 0;
 	}

test/broker/01-connect-invalid-reserved.py

@@ -0,0 +1,33 @@
+#!/usr/bin/env python
+
+# Test whether a CONNECT with reserved set to 1 results in a disconnect. MQTT-3.1.2-3
+
+import inspect, os, sys
+# From http://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
+
+rc = 1
+keepalive = 10
+connect_packet = mosq_test.gen_connect("connect-invalid-test", keepalive=keepalive, connect_reserved=True, proto_ver=4)
+
+cmd = ['../../src/mosquitto', '-p', '1888']
+broker = mosq_test.start_broker(filename=os.path.basename(__file__), cmd=cmd)
+
+try:
+    sock = mosq_test.do_client_connect(connect_packet, "")
+    sock.close()
+    rc = 0
+
+finally:
+    broker.terminate()
+    broker.wait()
+    if rc:
+        (stdo, stde) = broker.communicate()
+        print(stde)
+
+exit(rc)
+

test/broker/Makefile

@@ -20,6 +20,7 @@ test : test-compile 01 02 03 04 05 06 07 08 09 10
 	./01-connect-invalid-id-0.py
 	./01-connect-invalid-id-0-311.py
 	./01-connect-invalid-id-missing.py
+	./01-connect-invalid-reserved.py
 	./01-connect-anon-denied.py
 	./01-connect-uname-no-password-denied.py
 	./01-connect-uname-password-denied.py

test/mosq_test.py

@@ -91,6 +91,19 @@ def remaining_length(packet):
     return (packet, rl)
 
 
+def to_hex_string(packet):
+    if len(packet) == 0:
+        return ""
+
+    s = ""
+    while len(packet) > 0:
+        packet0 = struct.unpack("!B", packet[0])
+        s = s+hex(packet0[0]) + " "
+        packet = packet[1:]
+
+    return s
+
+
 def to_string(packet):
     if len(packet) == 0:
         return ""
@@ -150,6 +163,9 @@ def to_string(packet):
             (password, packet) = struct.unpack(pack_format, packet)
             s = s+", password="+password
 
+        if flags&1:
+            s = s+", reserved=1"
+
         return s
     elif cmd == 0x20:
         # CONNACK
@@ -250,7 +266,7 @@ def to_string(packet):
         # Reserved
         return "0xF0"
 
-def gen_connect(client_id, clean_session=True, keepalive=60, username=None, password=None, will_topic=None, will_qos=0, will_retain=False, will_payload="", proto_ver=4):
+def gen_connect(client_id, clean_session=True, keepalive=60, username=None, password=None, will_topic=None, will_qos=0, will_retain=False, will_payload="", proto_ver=4, connect_reserved=False):
     if (proto_ver&0x7F) == 3 or proto_ver == 0:
         remaining_length = 12
     elif (proto_ver&0x7F) == 4:
@@ -262,6 +278,10 @@ def gen_connect(client_id, clean_session=True, keepalive=60, username=None, pass
         remaining_length = remaining_length + 2+len(client_id)
 
     connect_flags = 0
+
+    if connect_reserved:
+        connect_flags = connect_flags | 0x01
+
     if clean_session:
         connect_flags = connect_flags | 0x02