From 6d63468a285e9593a2969b2d7643b77c6bf19dc0 Mon Sep 17 00:00:00 2001 From: "Roger A. Light" Date: Wed, 31 May 2017 21:45:53 +0100 Subject: [PATCH 01/28] Don't use / in auto-generated client ids. --- ChangeLog.txt | 4 ++++ client/client_shared.c | 4 ++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/ChangeLog.txt b/ChangeLog.txt index 75ca22f0..e505f825 100644 --- a/ChangeLog.txt +++ b/ChangeLog.txt @@ -1,3 +1,7 @@ +Clients: +- Don't use / in auto-generated client ids. + + 1.4.12 - 20170528 ================= diff --git a/client/client_shared.c b/client/client_shared.c index caa33e18..fa3ea188 100644 --- a/client/client_shared.c +++ b/client/client_shared.c @@ -727,14 +727,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'; From 6f9842ae02206a6485ddf417d3c60dd3086d83e8 Mon Sep 17 00:00:00 2001 From: "Roger A. Light" Date: Wed, 31 May 2017 23:24:12 +0100 Subject: [PATCH 02/28] Fix man page links. --- man/Makefile | 8 ++++++++ man/mosquitto.8.xml | 5 +++-- man/mosquitto.conf.5.xml | 4 ++-- man/mosquitto_passwd.1.xml | 4 ++-- man/mosquitto_pub.1.xml | 3 ++- man/mosquitto_sub.1.xml | 4 ++-- 6 files changed, 19 insertions(+), 9 deletions(-) diff --git a/man/Makefile b/man/Makefile index a0496869..502a78c6 100644 --- a/man/Makefile +++ b/man/Makefile @@ -5,6 +5,14 @@ include ../config.mk all : mosquitto.8 mosquitto-tls.7 mosquitto.conf.5 mosquitto_passwd.1 mosquitto_pub.1 mosquitto_sub.1 mqtt.7 libmosquitto.3 clean : + -rm -f libmosquitto.3 + -rm -f mosquitto.8 + -rm -f mosquitto.conf.5 + -rm -f mosquitto_passwd.1 + -rm -f mosquitto_pub.1 + -rm -f mosquitto_sub.1 + -rm -f mosquitto-tls.7 + -rm -f mqtt.7 reallyclean : clean -rm -f *.orig diff --git a/man/mosquitto.8.xml b/man/mosquitto.8.xml index 40242784..4d7dd982 100644 --- a/man/mosquitto.8.xml +++ b/man/mosquitto.8.xml @@ -473,7 +473,8 @@ Bugs - mosquitto bug information can be found at https://bugs.eclipse.org/bugs/describecomponents.cgi?product=Mosquitto + mosquitto bug information can be found at + https://github.com/eclipse/mosquitto/issues @@ -499,7 +500,7 @@ - hosts_access + hosts_access 5 diff --git a/man/mosquitto.conf.5.xml b/man/mosquitto.conf.5.xml index d2b1fc52..1005a9da 100644 --- a/man/mosquitto.conf.5.xml +++ b/man/mosquitto.conf.5.xml @@ -1396,8 +1396,8 @@ topic clients/total in 0 test/mosquitto/org $SYS/broker/ Bugs - mosquitto bug information can be found at https://bugs.eclipse.org/bugs/describecomponents.cgi?product=Mosquitto + mosquitto bug information can be found at + https://github.com/eclipse/mosquitto/issues diff --git a/man/mosquitto_passwd.1.xml b/man/mosquitto_passwd.1.xml index 087ed166..a7bc67d0 100644 --- a/man/mosquitto_passwd.1.xml +++ b/man/mosquitto_passwd.1.xml @@ -123,8 +123,8 @@ Bugs - mosquitto_passwd bug information can be found at - https://bugs.eclipse.org/bugs/describecomponents.cgi?product=Mosquitto + mosquitto bug information can be found at + https://github.com/eclipse/mosquitto/issues diff --git a/man/mosquitto_pub.1.xml b/man/mosquitto_pub.1.xml index 69e12b92..25efdc22 100644 --- a/man/mosquitto_pub.1.xml +++ b/man/mosquitto_pub.1.xml @@ -475,7 +475,8 @@ Bugs - mosquitto_pub bug information can be found at https://bugs.eclipse.org/bugs/describecomponents.cgi?product=Mosquitto + mosquitto bug information can be found at + https://github.com/eclipse/mosquitto/issues diff --git a/man/mosquitto_sub.1.xml b/man/mosquitto_sub.1.xml index 93524b3e..953fd9ac 100644 --- a/man/mosquitto_sub.1.xml +++ b/man/mosquitto_sub.1.xml @@ -508,8 +508,8 @@ Bugs - mosquitto_sub bug information can be found at - https://bugs.eclipse.org/bugs/describecomponents.cgi?product=Mosquitto + mosquitto bug information can be found at + https://github.com/eclipse/mosquitto/issues From fe8fef27ee16f7759ed208d97852b705248dd630 Mon Sep 17 00:00:00 2001 From: "Roger A. Light" Date: Wed, 31 May 2017 23:31:55 +0100 Subject: [PATCH 03/28] Make bug urls clickable. --- man/mosquitto.8.xml | 2 +- man/mosquitto.conf.5.xml | 2 +- man/mosquitto_passwd.1.xml | 2 +- man/mosquitto_pub.1.xml | 2 +- man/mosquitto_sub.1.xml | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/man/mosquitto.8.xml b/man/mosquitto.8.xml index 4d7dd982..ecec6bfb 100644 --- a/man/mosquitto.8.xml +++ b/man/mosquitto.8.xml @@ -474,7 +474,7 @@ Bugs mosquitto bug information can be found at - https://github.com/eclipse/mosquitto/issues + diff --git a/man/mosquitto.conf.5.xml b/man/mosquitto.conf.5.xml index 1005a9da..00216c59 100644 --- a/man/mosquitto.conf.5.xml +++ b/man/mosquitto.conf.5.xml @@ -1397,7 +1397,7 @@ topic clients/total in 0 test/mosquitto/org $SYS/broker/ Bugs mosquitto bug information can be found at - https://github.com/eclipse/mosquitto/issues + diff --git a/man/mosquitto_passwd.1.xml b/man/mosquitto_passwd.1.xml index a7bc67d0..95b3c130 100644 --- a/man/mosquitto_passwd.1.xml +++ b/man/mosquitto_passwd.1.xml @@ -124,7 +124,7 @@ Bugs mosquitto bug information can be found at - https://github.com/eclipse/mosquitto/issues + diff --git a/man/mosquitto_pub.1.xml b/man/mosquitto_pub.1.xml index 25efdc22..7d214861 100644 --- a/man/mosquitto_pub.1.xml +++ b/man/mosquitto_pub.1.xml @@ -476,7 +476,7 @@ Bugs mosquitto bug information can be found at - https://github.com/eclipse/mosquitto/issues + diff --git a/man/mosquitto_sub.1.xml b/man/mosquitto_sub.1.xml index 953fd9ac..876f7dca 100644 --- a/man/mosquitto_sub.1.xml +++ b/man/mosquitto_sub.1.xml @@ -509,7 +509,7 @@ Bugs mosquitto bug information can be found at - https://github.com/eclipse/mosquitto/issues + From 621f18d696c5139edaee232dfc844972b480a758 Mon Sep 17 00:00:00 2001 From: Jan Lukavsky Date: Wed, 31 May 2017 11:02:32 +0200 Subject: [PATCH 04/28] #419 Broker sometimes kills connection to client Signed-off-by: Jan Lukavsky --- lib/net_mosq.c | 3 +++ src/net.c | 1 + 2 files changed, 4 insertions(+) diff --git a/lib/net_mosq.c b/lib/net_mosq.c index 88160abe..063c4a22 100644 --- a/lib/net_mosq.c +++ b/lib/net_mosq.c @@ -464,6 +464,7 @@ int _mosquitto_try_connect(struct mosquitto *mosq, const char *host, uint16_t po int mosquitto__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); @@ -762,6 +763,7 @@ ssize_t _mosquitto_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); @@ -812,6 +814,7 @@ ssize_t _mosquitto_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); diff --git a/src/net.c b/src/net.c index ac8c055b..b8a03dc0 100644 --- a/src/net.c +++ b/src/net.c @@ -165,6 +165,7 @@ int mqtt3_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); From c07ba2a3da7ced1335978ce2623530726f362f39 Mon Sep 17 00:00:00 2001 From: "Roger A. Light" Date: Sun, 18 Jun 2017 12:52:59 +0100 Subject: [PATCH 05/28] Experimental fix for poor websockets performance. --- ChangeLog.txt | 3 +++ lib/mosquitto_internal.h | 1 + src/loop.c | 45 +++++++++++++++++++++++++++++----------- src/websockets.c | 18 ++++++++++++++++ 4 files changed, 55 insertions(+), 12 deletions(-) diff --git a/ChangeLog.txt b/ChangeLog.txt index e505f825..b98c5513 100644 --- a/ChangeLog.txt +++ b/ChangeLog.txt @@ -1,3 +1,6 @@ +Broker: +- Fix for poor websockets performance. + Clients: - Don't use / in auto-generated client ids. diff --git a/lib/mosquitto_internal.h b/lib/mosquitto_internal.h index ac925738..8d3014dd 100644 --- a/lib/mosquitto_internal.h +++ b/lib/mosquitto_internal.h @@ -226,6 +226,7 @@ struct mosquitto { struct libwebsocket *wsi; # endif # endif + bool ws_want_write; #else # ifdef WITH_SOCKS char *socks5_host; diff --git a/src/loop.c b/src/loop.c index 6e9de59f..2c90e249 100644 --- a/src/loop.c +++ b/src/loop.c @@ -108,6 +108,7 @@ int mosquitto_main_loop(struct mosquitto_db *db, mosq_sock_t *listensock, int li struct pollfd *pollfds = NULL; int pollfd_count = 0; int pollfd_index; + int pollfd_max; #ifdef WITH_BRIDGE mosq_sock_t bridge_sock; int rc; @@ -122,6 +123,18 @@ int mosquitto_main_loop(struct mosquitto_db *db, mosq_sock_t *listensock, int li sigaddset(&sigblock, SIGINT); #endif +#ifdef WIN32 + pollfd_max = _getmaxstdio(); +#else + pollfd_max = getdtablesize(); +#endif + + pollfds = _mosquitto_malloc(sizeof(struct pollfd)*pollfd_max); + if(!pollfds){ + _mosquitto_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; } @@ -139,16 +152,8 @@ int mosquitto_main_loop(struct mosquitto_db *db, mosq_sock_t *listensock, int li 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){ - _mosquitto_log_printf(NULL, MOSQ_LOG_ERR, "Error: Out of memory."); - return MOSQ_ERR_NOMEM; - } - } - - memset(pollfds, -1, sizeof(struct pollfd)*pollfd_count); + pollfd_count = listensock_count + context_count; + memset(pollfds, -1, sizeof(struct pollfd)*pollfd_max); pollfd_index = 0; for(i=0; isock; 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++; @@ -436,7 +442,10 @@ 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; + } }else #endif { @@ -482,6 +491,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 || diff --git a/src/websockets.c b/src/websockets.c index b44943d3..84e5089b 100644 --- a/src/websockets.c +++ b/src/websockets.c @@ -218,6 +218,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: @@ -226,6 +228,10 @@ 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->wsi = NULL; do_disconnect(db, mosq); } @@ -412,6 +418,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. */ @@ -583,6 +592,15 @@ static int callback_http(struct libwebsocket_context *context, break; #endif + 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){ + mosq->ws_want_write = true; + } + break; + default: return 0; } From 8f59d5ad283147b2a47b3eb1e3172c9c76a5f77a Mon Sep 17 00:00:00 2001 From: "Roger A. Light" Date: Mon, 19 Jun 2017 16:14:59 +0100 Subject: [PATCH 06/28] Remove unused vars and reset pollfd_index on disconnect. --- src/loop.c | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/src/loop.c b/src/loop.c index 2c90e249..46f15d24 100644 --- a/src/loop.c +++ b/src/loop.c @@ -106,14 +106,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; time_t last_timeout_check = 0; char *id; @@ -147,12 +145,6 @@ 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 - - pollfd_count = listensock_count + context_count; memset(pollfds, -1, sizeof(struct pollfd)*pollfd_max); pollfd_index = 0; @@ -445,6 +437,7 @@ void do_disconnect(struct mosquitto_db *db, struct mosquitto *context) if(context->sock != INVALID_SOCKET){ HASH_DELETE(hh_sock, db->contexts_by_sock, context); context->sock = INVALID_SOCKET; + context->pollfd_index = -1; } }else #endif From 326983d35eb1e0807843671ba7794032bede5164 Mon Sep 17 00:00:00 2001 From: "Roger A. Light" Date: Mon, 19 Jun 2017 17:15:00 +0100 Subject: [PATCH 07/28] [417] Fix lazy bridges not timing out for idle_timeout. Thanks to spinachmedia. Bug: https://github.com/eclipse/mosquitto/issues/417 --- ChangeLog.txt | 1 + lib/util_mosq.c | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/ChangeLog.txt b/ChangeLog.txt index b98c5513..9272af78 100644 --- a/ChangeLog.txt +++ b/ChangeLog.txt @@ -1,5 +1,6 @@ Broker: - Fix for poor websockets performance. +- Fix lazy bridges not timing out for idle_timeout. Closes #417. Clients: - Don't use / in auto-generated client ids. diff --git a/lib/util_mosq.c b/lib/util_mosq.c index 139bea34..a80b0cbc 100644 --- a/lib/util_mosq.c +++ b/lib/util_mosq.c @@ -95,7 +95,7 @@ void _mosquitto_check_keepalive(struct mosquitto *mosq) /* Check if a lazy bridge should be timed out due to idle. */ if(mosq->bridge && mosq->bridge->start_type == bst_lazy && mosq->sock != INVALID_SOCKET - && now - mosq->next_msg_out - mosq->keepalive >= mosq->bridge->idle_timeout){ + && now - mosq->next_msg_out + mosq->keepalive >= mosq->bridge->idle_timeout){ _mosquitto_log_printf(NULL, MOSQ_LOG_NOTICE, "Bridge connection %s has exceeded idle timeout, disconnecting.", mosq->id); _mosquitto_socket_close(db, mosq); From c78678607d3f31ecc5305c37a3f19be40f9c031e Mon Sep 17 00:00:00 2001 From: "Roger A. Light" Date: Mon, 19 Jun 2017 21:40:19 +0100 Subject: [PATCH 08/28] [427] Fix large retained messages over websockets. Thanks to Brian Block. Bug: https://github.com/eclipse/mosquitto/issues/427 --- ChangeLog.txt | 1 + src/websockets.c | 7 ++----- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/ChangeLog.txt b/ChangeLog.txt index 9272af78..3e1691be 100644 --- a/ChangeLog.txt +++ b/ChangeLog.txt @@ -1,6 +1,7 @@ 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. Clients: - Don't use / in auto-generated client ids. diff --git a/src/websockets.c b/src/websockets.c index 84e5089b..9923a287 100644 --- a/src/websockets.c +++ b/src/websockets.c @@ -231,6 +231,7 @@ static int callback_mqtt(struct libwebsocket_context *context, 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); @@ -256,7 +257,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){ @@ -296,10 +297,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); From ab45f86d7421a80b46d8a7c3b5c6e9984fe7347d Mon Sep 17 00:00:00 2001 From: "Roger A. Light" Date: Thu, 22 Jun 2017 09:47:03 +0100 Subject: [PATCH 09/28] Prevent out of bounds array access. --- src/loop.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/loop.c b/src/loop.c index 46f15d24..0828c40c 100644 --- a/src/loop.c +++ b/src/loop.c @@ -539,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; } From 09cb1b61c8f48284d9c42bd911faa7525cc689c7 Mon Sep 17 00:00:00 2001 From: "Roger A. Light" Date: Fri, 23 Jun 2017 14:50:39 +0100 Subject: [PATCH 10/28] [468] Set persistence file to only be readable by owner. Not implemented on Windows. Thanks to Moshe Zioni. Bug: https://github.com/eclipse/mosquitto/issues/468 --- ChangeLog.txt | 2 ++ src/persist.c | 3 +++ 2 files changed, 5 insertions(+) diff --git a/ChangeLog.txt b/ChangeLog.txt index 3e1691be..5540fda0 100644 --- a/ChangeLog.txt +++ b/ChangeLog.txt @@ -2,6 +2,8 @@ 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. Clients: - Don't use / in auto-generated client ids. diff --git a/src/persist.c b/src/persist.c index 024317a2..f5ba0cd8 100644 --- a/src/persist.c +++ b/src/persist.c @@ -402,6 +402,9 @@ int mqtt3_db_backup(struct mosquitto_db *db, bool shutdown) goto error; } } + + /* Set permissions to -rw------- */ + umask(0077); #endif db_fptr = _mosquitto_fopen(outfile, "wb"); From 408972ddc1fe8fa0ac37f40c7bc446df7372633b Mon Sep 17 00:00:00 2001 From: Pierre Fersing Date: Fri, 23 Jun 2017 23:30:57 +0200 Subject: [PATCH 11/28] Fix two issues with Websocket (#472) * Websocket were always marked as "want_write" (even if they only want to read, or worse want nothing). * Websocket FD was read twice in some case (when socket recv queue was larger that size read by libwebsocket) Signed-off-by: Pierre Fersing --- src/loop.c | 6 ++++++ src/websockets.c | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/loop.c b/src/loop.c index 0828c40c..6f863728 100644 --- a/src/loop.c +++ b/src/loop.c @@ -525,6 +525,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 || diff --git a/src/websockets.c b/src/websockets.c index 9923a287..dde0c9cc 100644 --- a/src/websockets.c +++ b/src/websockets.c @@ -593,7 +593,7 @@ static int callback_http(struct libwebsocket_context *context, 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){ + if(mosq && (pollargs->events & POLLOUT)){ mosq->ws_want_write = true; } break; From 6e7d02ba160c15d087c1741a61c3f9f222834d74 Mon Sep 17 00:00:00 2001 From: "Roger A. Light" Date: Mon, 26 Jun 2017 14:53:33 +0100 Subject: [PATCH 12/28] Fix for CVE-2017-9868 for Windows. --- ChangeLog.txt | 9 ++++++ lib/mosquitto.c | 6 ++-- lib/util_mosq.c | 64 +++++++++++++++++++++++++++++++++++++++--- lib/util_mosq.h | 2 +- src/conf.c | 2 +- src/logging.c | 2 +- src/mosquitto.c | 2 +- src/persist.c | 7 ++--- src/security_default.c | 4 +-- 9 files changed, 80 insertions(+), 18 deletions(-) diff --git a/ChangeLog.txt b/ChangeLog.txt index 5540fda0..39bc64ec 100644 --- a/ChangeLog.txt +++ b/ChangeLog.txt @@ -1,3 +1,12 @@ +1.4.13 - 20170xxx +================= + +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. diff --git a/lib/mosquitto.c b/lib/mosquitto.c index afd7e375..be8e62e5 100644 --- a/lib/mosquitto.c +++ b/lib/mosquitto.c @@ -653,7 +653,7 @@ int mosquitto_tls_set(struct mosquitto *mosq, const char *cafile, const char *ca if(!mosq || (!cafile && !capath) || (certfile && !keyfile) || (!certfile && keyfile)) return MOSQ_ERR_INVAL; if(cafile){ - fptr = _mosquitto_fopen(cafile, "rt"); + fptr = _mosquitto_fopen(cafile, "rt", false); if(fptr){ fclose(fptr); }else{ @@ -680,7 +680,7 @@ int mosquitto_tls_set(struct mosquitto *mosq, const char *cafile, const char *ca } if(certfile){ - fptr = _mosquitto_fopen(certfile, "rt"); + fptr = _mosquitto_fopen(certfile, "rt", false); if(fptr){ fclose(fptr); }else{ @@ -704,7 +704,7 @@ int mosquitto_tls_set(struct mosquitto *mosq, const char *cafile, const char *ca } if(keyfile){ - fptr = _mosquitto_fopen(keyfile, "rt"); + fptr = _mosquitto_fopen(keyfile, "rt", false); if(fptr){ fclose(fptr); }else{ diff --git a/lib/util_mosq.c b/lib/util_mosq.c index a80b0cbc..3281a579 100644 --- a/lib/util_mosq.c +++ b/lib/util_mosq.c @@ -18,7 +18,12 @@ Contributors: #include #ifdef WIN32 -#include +# include +# include +# include +# include +#else +# include #endif @@ -95,7 +100,7 @@ void _mosquitto_check_keepalive(struct mosquitto *mosq) /* Check if a lazy bridge should be timed out due to idle. */ if(mosq->bridge && mosq->bridge->start_type == bst_lazy && mosq->sock != INVALID_SOCKET - && now - mosq->next_msg_out + mosq->keepalive >= mosq->bridge->idle_timeout){ + && now - mosq->next_msg_out - mosq->keepalive >= mosq->bridge->idle_timeout){ _mosquitto_log_printf(NULL, MOSQ_LOG_NOTICE, "Bridge connection %s has exceeded idle timeout, disconnecting.", mosq->id); _mosquitto_socket_close(db, mosq); @@ -338,7 +343,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]; @@ -347,9 +352,60 @@ 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 + if (restrict_read) { + umask(0700); + } return fopen(path, mode); #endif } diff --git a/lib/util_mosq.h b/lib/util_mosq.h index 5aa5371f..8a8b7b53 100644 --- a/lib/util_mosq.h +++ b/lib/util_mosq.h @@ -32,7 +32,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); diff --git a/src/conf.c b/src/conf.c index 56ea9ace..a4bfb011 100644 --- a/src/conf.c +++ b/src/conf.c @@ -1762,7 +1762,7 @@ int _config_read_file(struct mqtt3_config *config, bool reload, const char *file int rc; FILE *fptr = NULL; - fptr = _mosquitto_fopen(file, "rt"); + fptr = _mosquitto_fopen(file, "rt", false); if(!fptr){ _mosquitto_log_printf(NULL, MOSQ_LOG_ERR, "Error: Unable to open config file %s\n", file); return 1; diff --git a/src/logging.c b/src/logging.c index f1a58111..18d8d7db 100644 --- a/src/logging.c +++ b/src/logging.c @@ -69,7 +69,7 @@ int mqtt3_log_init(struct mqtt3_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; diff --git a/src/mosquitto.c b/src/mosquitto.c index 9777ad80..c1bd328f 100644 --- a/src/mosquitto.c +++ b/src/mosquitto.c @@ -269,7 +269,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); diff --git a/src/persist.c b/src/persist.c index f5ba0cd8..16323588 100644 --- a/src/persist.c +++ b/src/persist.c @@ -402,12 +402,9 @@ int mqtt3_db_backup(struct mosquitto_db *db, bool shutdown) goto error; } } - - /* Set permissions to -rw------- */ - umask(0077); #endif - db_fptr = _mosquitto_fopen(outfile, "wb"); + db_fptr = _mosquitto_fopen(outfile, "wb", true); if(db_fptr == NULL){ _mosquitto_log_printf(NULL, MOSQ_LOG_INFO, "Error saving in-memory database, unable to open %s for writing.", outfile); goto error; @@ -824,7 +821,7 @@ int mqtt3_db_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){ diff --git a/src/security_default.c b/src/security_default.c index a41c21f4..a0f1b6d0 100644 --- a/src/security_default.c +++ b/src/security_default.c @@ -352,7 +352,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){ _mosquitto_log_printf(NULL, MOSQ_LOG_ERR, "Error: Unable to open acl_file \"%s\".", db->config->acl_file); return 1; @@ -511,7 +511,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){ _mosquitto_log_printf(NULL, MOSQ_LOG_ERR, "Error: Unable to open pwfile \"%s\".", file); return 1; From 94978ac89ba6a2c385b5b231c40d879a635365ab Mon Sep 17 00:00:00 2001 From: "Roger A. Light" Date: Tue, 27 Jun 2017 09:57:47 +0100 Subject: [PATCH 13/28] Restore old umask after creating file. --- lib/util_mosq.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/lib/util_mosq.c b/lib/util_mosq.c index 3281a579..055b310a 100644 --- a/lib/util_mosq.c +++ b/lib/util_mosq.c @@ -404,9 +404,17 @@ FILE *_mosquitto_fopen(const char *path, const char *mode, bool restrict_read) } #else if (restrict_read) { - umask(0700); + FILE *fptr; + mode_t old_mask; + + old_mask = umask(0700); + fptr = fopen(path, mode); + umask(old_mask); + + return fptr; + }else{ + return fopen(path, mode); } - return fopen(path, mode); #endif } From 96db6d6644786fb89ddc16df18c455b0d3954f03 Mon Sep 17 00:00:00 2001 From: "Roger A. Light" Date: Tue, 27 Jun 2017 11:11:43 +0100 Subject: [PATCH 14/28] Fix CONNECT check for reserved=0, as per MQTT v3.1.1 check MQTT-3.1.2-3. --- ChangeLog.txt | 1 + src/read_handle_server.c | 7 +++++ test/broker/01-connect-invalid-reserved.py | 33 ++++++++++++++++++++++ test/broker/Makefile | 1 + test/mosq_test.py | 22 ++++++++++++++- 5 files changed, 63 insertions(+), 1 deletion(-) create mode 100755 test/broker/01-connect-invalid-reserved.py diff --git a/ChangeLog.txt b/ChangeLog.txt index 39bc64ec..1109359f 100644 --- a/ChangeLog.txt +++ b/ChangeLog.txt @@ -13,6 +13,7 @@ Broker: - 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. Clients: - Don't use / in auto-generated client ids. diff --git a/src/read_handle_server.c b/src/read_handle_server.c index 14f72d79..6be6a250 100644 --- a/src/read_handle_server.c +++ b/src/read_handle_server.c @@ -171,6 +171,13 @@ int mqtt3_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; diff --git a/test/broker/01-connect-invalid-reserved.py b/test/broker/01-connect-invalid-reserved.py new file mode 100755 index 00000000..182fcc9d --- /dev/null +++ b/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) + diff --git a/test/broker/Makefile b/test/broker/Makefile index ea6ab074..45f08b2e 100644 --- a/test/broker/Makefile +++ b/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 diff --git a/test/mosq_test.py b/test/mosq_test.py index b62b971a..01404242 100644 --- a/test/mosq_test.py +++ b/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=3): +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=3, 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 From 2d90a1f45ba81d6e340ba56f8ac6697d8b1c7161 Mon Sep 17 00:00:00 2001 From: "Roger A. Light" Date: Tue, 27 Jun 2017 11:21:34 +0100 Subject: [PATCH 15/28] Fix umask value. --- lib/util_mosq.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/util_mosq.c b/lib/util_mosq.c index 055b310a..6f6bb49a 100644 --- a/lib/util_mosq.c +++ b/lib/util_mosq.c @@ -407,7 +407,7 @@ FILE *_mosquitto_fopen(const char *path, const char *mode, bool restrict_read) FILE *fptr; mode_t old_mask; - old_mask = umask(0700); + old_mask = umask(0077); fptr = fopen(path, mode); umask(old_mask); From 5246a76f87708e04721f6977736511ea7d3192dc Mon Sep 17 00:00:00 2001 From: "Roger A. Light" Date: Tue, 27 Jun 2017 14:33:02 +0100 Subject: [PATCH 16/28] [477] Send will messages for connected clients when broker stops. Thanks to mikeS7. Bug: https://github.com/eclipse/mosquitto/issues/477 --- ChangeLog.txt | 2 ++ src/context.c | 18 +++++++++++------- src/mosquitto.c | 12 ++++++------ src/mosquitto_broker.h | 1 + 4 files changed, 20 insertions(+), 13 deletions(-) diff --git a/ChangeLog.txt b/ChangeLog.txt index 1109359f..e6f24e7c 100644 --- a/ChangeLog.txt +++ b/ChangeLog.txt @@ -14,6 +14,8 @@ Broker: - 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. Clients: - Don't use / in auto-generated client ids. diff --git a/src/context.c b/src/context.c index 20e1820f..561ff4aa 100644 --- a/src/context.c +++ b/src/context.c @@ -143,6 +143,8 @@ void mqtt3_context_cleanup(struct mosquitto_db *db, struct mosquitto *context, b context->address = NULL; } + mqtt3_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 mqtt3_context_cleanup(struct mosquitto_db *db, struct mosquitto *context, b context->out_packet = context->out_packet->next; _mosquitto_free(packet); } - if(context->will){ - if(context->will->topic) _mosquitto_free(context->will->topic); - if(context->will->payload) _mosquitto_free(context->will->payload); - _mosquitto_free(context->will); - context->will = NULL; - } if(do_free || context->clean_session){ msg = context->msgs; while(msg){ @@ -185,7 +181,8 @@ void mqtt3_context_cleanup(struct mosquitto_db *db, struct mosquitto *context, b } } -void mqtt3_context_disconnect(struct mosquitto_db *db, struct mosquitto *ctxt) + +void mqtt3_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){ @@ -199,6 +196,13 @@ void mqtt3_context_disconnect(struct mosquitto_db *db, struct mosquitto *ctxt) _mosquitto_free(ctxt->will); ctxt->will = NULL; } +} + + +void mqtt3_context_disconnect(struct mosquitto_db *db, struct mosquitto *ctxt) +{ + mqtt3_context_send_will(db, ctxt); + ctxt->disconnect_t = time(NULL); _mosquitto_socket_close(db, ctxt); } diff --git a/src/mosquitto.c b/src/mosquitto.c index c1bd328f..1fd1e7ff 100644 --- a/src/mosquitto.c +++ b/src/mosquitto.c @@ -387,12 +387,6 @@ int main(int argc, char *argv[]) _mosquitto_log_printf(NULL, MOSQ_LOG_INFO, "mosquitto version %s terminating", VERSION); mqtt3_log_close(&config); -#ifdef WITH_PERSISTENCE - if(config.persistence){ - mqtt3_db_backup(&int_db, true); - } -#endif - #ifdef WITH_WEBSOCKETS for(i=0; ilistener_count; i++){ if(int_db.config->listeners[i].ws_context){ @@ -428,6 +422,12 @@ int main(int argc, char *argv[]) #endif mosquitto__free_disused_contexts(&int_db); +#ifdef WITH_PERSISTENCE + if(config.persistence){ + mqtt3_db_backup(&int_db, true); + } +#endif + mqtt3_db_close(&int_db); if(listensock){ diff --git a/src/mosquitto_broker.h b/src/mosquitto_broker.h index c89489b6..9ab3ab08 100644 --- a/src/mosquitto_broker.h +++ b/src/mosquitto_broker.h @@ -452,6 +452,7 @@ void mqtt3_context_cleanup(struct mosquitto_db *db, struct mosquitto *context, b void mqtt3_context_disconnect(struct mosquitto_db *db, struct mosquitto *context); void mosquitto__add_context_to_disused(struct mosquitto_db *db, struct mosquitto *context); void mosquitto__free_disused_contexts(struct mosquitto_db *db); +void mqtt3_context_send_will(struct mosquitto_db *db, struct mosquitto *context); /* ============================================================ * Logging functions From c3823c0a817ffcad4e062ce3b7a9b787f1833396 Mon Sep 17 00:00:00 2001 From: "Roger A. Light" Date: Tue, 27 Jun 2017 14:59:37 +0100 Subject: [PATCH 17/28] [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 | 3 +++ man/mosquitto.conf.5.xml | 25 +++++++++++++++++++++++++ mosquitto.conf | 16 ++++++++++++++++ src/conf.c | 4 ++++ src/mosquitto_broker.h | 1 + src/security.c | 28 +++++++++++++++------------- 6 files changed, 64 insertions(+), 13 deletions(-) diff --git a/ChangeLog.txt b/ChangeLog.txt index e6f24e7c..36b0a9b7 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 00216c59..37614dd4 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 f0e14023..99af968c 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 a4bfb011..a3e233de 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 9ab3ab08..f33007c8 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 d0c376bd..244a3a6a 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); } From cd17ca45cd313dc00480091505f708858db73ee9 Mon Sep 17 00:00:00 2001 From: "Roger A. Light" Date: Tue, 27 Jun 2017 15:10:43 +0100 Subject: [PATCH 18/28] [462] Relax CVE-2017-7650 checks. Checks for '/' are no longer made, this character is a much lower risk and is widely used in usernames. Bug: https://github.com/eclipse/mosquitto/issues/462 --- ChangeLog.txt | 2 ++ src/security.c | 6 +++--- src/security_default.c | 6 +++--- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/ChangeLog.txt b/ChangeLog.txt index 36b0a9b7..019d3a02 100644 --- a/ChangeLog.txt +++ b/ChangeLog.txt @@ -19,6 +19,8 @@ Broker: - 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. Clients: - Don't use / in auto-generated client ids. diff --git a/src/security.c b/src/security.c index 244a3a6a..67a4f791 100644 --- a/src/security.c +++ b/src/security.c @@ -235,17 +235,17 @@ int mosquitto_acl_check(struct mosquitto_db *db, struct mosquitto *context, cons } if(db->config->auth_plugin_deny_special_chars == true){ - /* Check whether the client id or username contains a +, # or / and if + /* 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, "+#/")){ + 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, "+#/")){ + 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; } diff --git a/src/security_default.c b/src/security_default.c index a0f1b6d0..43cd3f0c 100644 --- a/src/security_default.c +++ b/src/security_default.c @@ -264,18 +264,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, "+#")){ _mosquitto_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, "+#")){ _mosquitto_log_printf(NULL, MOSQ_LOG_NOTICE, "ACL denying access to client with dangerous client id \"%s\"", context->id); return MOSQ_ERR_ACL_DENIED; } From 6b351ce0f1babbe57284618f1b755c6f0851660d Mon Sep 17 00:00:00 2001 From: "Roger A. Light" Date: Tue, 27 Jun 2017 22:14:08 +0100 Subject: [PATCH 19/28] Bump version number. --- CMakeLists.txt | 2 +- ChangeLog.txt | 4 ++-- config.mk | 2 +- installer/mosquitto-cygwin.nsi | 2 +- installer/mosquitto.nsi | 2 +- lib/mosquitto.h | 2 +- set-version.sh | 2 +- 7 files changed, 8 insertions(+), 8 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index f8eac48a..5b9a1cb6 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -11,7 +11,7 @@ project(mosquitto) cmake_minimum_required(VERSION 2.8) # Only for version 3 and up. cmake_policy(SET CMP0042 NEW) -set (VERSION 1.4.12) +set (VERSION 1.4.13) if (WIN32) execute_process(COMMAND cmd /c echo %DATE% %TIME% OUTPUT_VARIABLE TIMESTAMP diff --git a/ChangeLog.txt b/ChangeLog.txt index 019d3a02..e83010eb 100644 --- a/ChangeLog.txt +++ b/ChangeLog.txt @@ -1,4 +1,4 @@ -1.4.13 - 20170xxx +1.4.13 - 20170627 ================= Security: @@ -20,7 +20,7 @@ Broker: 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. + usernames/client ids. Remainder of fix for #462. Clients: - Don't use / in auto-generated client ids. diff --git a/config.mk b/config.mk index 7f7491be..f6ac9869 100644 --- a/config.mk +++ b/config.mk @@ -86,7 +86,7 @@ WITH_SOCKS:=yes # Also bump lib/mosquitto.h, CMakeLists.txt, # installer/mosquitto.nsi, installer/mosquitto-cygwin.nsi -VERSION=1.4.12 +VERSION=1.4.13 TIMESTAMP:=$(shell date "+%F %T%z") # Client library SO version. Bump if incompatible API/ABI changes are made. diff --git a/installer/mosquitto-cygwin.nsi b/installer/mosquitto-cygwin.nsi index 9860230d..1115e07f 100644 --- a/installer/mosquitto-cygwin.nsi +++ b/installer/mosquitto-cygwin.nsi @@ -7,7 +7,7 @@ !define env_hklm 'HKLM "SYSTEM\CurrentControlSet\Control\Session Manager\Environment"' Name "mosquitto" -!define VERSION 1.4.12 +!define VERSION 1.4.13 OutFile "mosquitto-${VERSION}-install-cygwin.exe" InstallDir "$PROGRAMFILES\mosquitto" diff --git a/installer/mosquitto.nsi b/installer/mosquitto.nsi index a599dba6..ff34be76 100644 --- a/installer/mosquitto.nsi +++ b/installer/mosquitto.nsi @@ -9,7 +9,7 @@ !define env_hklm 'HKLM "SYSTEM\CurrentControlSet\Control\Session Manager\Environment"' Name "mosquitto" -!define VERSION 1.4.12 +!define VERSION 1.4.13 OutFile "mosquitto-${VERSION}-install-win32.exe" InstallDir "$PROGRAMFILES\mosquitto" diff --git a/lib/mosquitto.h b/lib/mosquitto.h index c106618d..02e4ff67 100644 --- a/lib/mosquitto.h +++ b/lib/mosquitto.h @@ -45,7 +45,7 @@ extern "C" { #define LIBMOSQUITTO_MAJOR 1 #define LIBMOSQUITTO_MINOR 4 -#define LIBMOSQUITTO_REVISION 12 +#define LIBMOSQUITTO_REVISION 13 /* LIBMOSQUITTO_VERSION_NUMBER looks like 1002001 for e.g. version 1.2.1. */ #define LIBMOSQUITTO_VERSION_NUMBER (LIBMOSQUITTO_MAJOR*1000000+LIBMOSQUITTO_MINOR*1000+LIBMOSQUITTO_REVISION) diff --git a/set-version.sh b/set-version.sh index ee81183d..5284d9b6 100755 --- a/set-version.sh +++ b/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 From 8de5ed446494e20aaeceebcfdde0b1969031f3c0 Mon Sep 17 00:00:00 2001 From: "Roger A. Light" Date: Tue, 27 Jun 2017 22:32:10 +0100 Subject: [PATCH 20/28] Remove "error in poll" messages. --- src/loop.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/loop.c b/src/loop.c index 6f863728..3d068181 100644 --- a/src/loop.c +++ b/src/loop.c @@ -119,6 +119,8 @@ int mosquitto_main_loop(struct mosquitto_db *db, mosq_sock_t *listensock, int li #ifndef WIN32 sigemptyset(&sigblock); sigaddset(&sigblock, SIGINT); + sigaddset(&sigblock, SIGTERM); + sigaddset(&sigblock, SIGHUP); #endif #ifdef WIN32 From 2a50b2e9bd3695b2a021d170b6c60f4104f928de Mon Sep 17 00:00:00 2001 From: "Roger A. Light" Date: Mon, 10 Jul 2017 23:43:42 +0100 Subject: [PATCH 21/28] Fix regression from 1.4.13 where persistence data was not being saved. --- ChangeLog.txt | 7 +++++++ src/mosquitto.c | 16 ++++++++++------ 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/ChangeLog.txt b/ChangeLog.txt index e83010eb..becbb44a 100644 --- a/ChangeLog.txt +++ b/ChangeLog.txt @@ -1,3 +1,10 @@ +1.4.14 - 20170710 +================= + +Broker: +- Fix regression from 1.4.13 where persistence data was not being saved. + + 1.4.13 - 20170627 ================= diff --git a/src/mosquitto.c b/src/mosquitto.c index 1fd1e7ff..b28150ce 100644 --- a/src/mosquitto.c +++ b/src/mosquitto.c @@ -398,6 +398,16 @@ int main(int argc, char *argv[]) } #endif + HASH_ITER(hh_id, int_db.contexts_by_id, ctxt, ctxt_tmp){ + mqtt3_context_send_will(&int_db, ctxt); + } + +#ifdef WITH_PERSISTENCE + if(config.persistence){ + mqtt3_db_backup(&int_db, true); + } +#endif + HASH_ITER(hh_id, int_db.contexts_by_id, ctxt, ctxt_tmp){ #ifdef WITH_WEBSOCKETS if(!ctxt->wsi){ @@ -422,12 +432,6 @@ int main(int argc, char *argv[]) #endif mosquitto__free_disused_contexts(&int_db); -#ifdef WITH_PERSISTENCE - if(config.persistence){ - mqtt3_db_backup(&int_db, true); - } -#endif - mqtt3_db_close(&int_db); if(listensock){ From 1fa4d742783eb1452aee0332150e924c95dfb274 Mon Sep 17 00:00:00 2001 From: "Roger A. Light" Date: Mon, 10 Jul 2017 23:44:16 +0100 Subject: [PATCH 22/28] Bump version number. --- CMakeLists.txt | 2 +- config.mk | 2 +- installer/mosquitto-cygwin.nsi | 2 +- installer/mosquitto.nsi | 2 +- lib/mosquitto.h | 2 +- set-version.sh | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 5b9a1cb6..6d5d7201 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -11,7 +11,7 @@ project(mosquitto) cmake_minimum_required(VERSION 2.8) # Only for version 3 and up. cmake_policy(SET CMP0042 NEW) -set (VERSION 1.4.13) +set (VERSION 1.4.14) if (WIN32) execute_process(COMMAND cmd /c echo %DATE% %TIME% OUTPUT_VARIABLE TIMESTAMP diff --git a/config.mk b/config.mk index f6ac9869..71f17e54 100644 --- a/config.mk +++ b/config.mk @@ -86,7 +86,7 @@ WITH_SOCKS:=yes # Also bump lib/mosquitto.h, CMakeLists.txt, # installer/mosquitto.nsi, installer/mosquitto-cygwin.nsi -VERSION=1.4.13 +VERSION=1.4.14 TIMESTAMP:=$(shell date "+%F %T%z") # Client library SO version. Bump if incompatible API/ABI changes are made. diff --git a/installer/mosquitto-cygwin.nsi b/installer/mosquitto-cygwin.nsi index 1115e07f..354cfcaa 100644 --- a/installer/mosquitto-cygwin.nsi +++ b/installer/mosquitto-cygwin.nsi @@ -7,7 +7,7 @@ !define env_hklm 'HKLM "SYSTEM\CurrentControlSet\Control\Session Manager\Environment"' Name "mosquitto" -!define VERSION 1.4.13 +!define VERSION 1.4.14 OutFile "mosquitto-${VERSION}-install-cygwin.exe" InstallDir "$PROGRAMFILES\mosquitto" diff --git a/installer/mosquitto.nsi b/installer/mosquitto.nsi index ff34be76..7bf3162b 100644 --- a/installer/mosquitto.nsi +++ b/installer/mosquitto.nsi @@ -9,7 +9,7 @@ !define env_hklm 'HKLM "SYSTEM\CurrentControlSet\Control\Session Manager\Environment"' Name "mosquitto" -!define VERSION 1.4.13 +!define VERSION 1.4.14 OutFile "mosquitto-${VERSION}-install-win32.exe" InstallDir "$PROGRAMFILES\mosquitto" diff --git a/lib/mosquitto.h b/lib/mosquitto.h index 02e4ff67..b7cc749f 100644 --- a/lib/mosquitto.h +++ b/lib/mosquitto.h @@ -45,7 +45,7 @@ extern "C" { #define LIBMOSQUITTO_MAJOR 1 #define LIBMOSQUITTO_MINOR 4 -#define LIBMOSQUITTO_REVISION 13 +#define LIBMOSQUITTO_REVISION 14 /* LIBMOSQUITTO_VERSION_NUMBER looks like 1002001 for e.g. version 1.2.1. */ #define LIBMOSQUITTO_VERSION_NUMBER (LIBMOSQUITTO_MAJOR*1000000+LIBMOSQUITTO_MINOR*1000+LIBMOSQUITTO_REVISION) diff --git a/set-version.sh b/set-version.sh index 5284d9b6..31a9a576 100755 --- a/set-version.sh +++ b/set-version.sh @@ -2,7 +2,7 @@ MAJOR=1 MINOR=4 -REVISION=13 +REVISION=14 sed -i "s/^VERSION=.*/VERSION=${MAJOR}.${MINOR}.${REVISION}/" config.mk From d72ec39d79effae08011e13faf5870fa7e80fa54 Mon Sep 17 00:00:00 2001 From: "Roger A. Light" Date: Mon, 10 Jul 2017 23:45:13 +0100 Subject: [PATCH 23/28] Don't clean man pages with 'clean' target. --- man/Makefile | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/man/Makefile b/man/Makefile index 502a78c6..635df3e9 100644 --- a/man/Makefile +++ b/man/Makefile @@ -5,6 +5,9 @@ include ../config.mk all : mosquitto.8 mosquitto-tls.7 mosquitto.conf.5 mosquitto_passwd.1 mosquitto_pub.1 mosquitto_sub.1 mqtt.7 libmosquitto.3 clean : + +reallyclean : clean + -rm -f *.orig -rm -f libmosquitto.3 -rm -f mosquitto.8 -rm -f mosquitto.conf.5 @@ -14,9 +17,6 @@ clean : -rm -f mosquitto-tls.7 -rm -f mqtt.7 -reallyclean : clean - -rm -f *.orig - dist : mosquitto.8 mosquitto-tls.7 mosquitto.conf.5 mosquitto_passwd.1 mosquitto_pub.1 mosquitto_sub.1 mqtt.7 libmosquitto.3 install : From 366194cde40d0cdea31248c8dbd2fe6dd4f81e20 Mon Sep 17 00:00:00 2001 From: Fredrik Fornwall Date: Sun, 16 Jul 2017 17:11:04 +0200 Subject: [PATCH 24/28] Replace getdtablesize() with sysconf(_SC_OPEN_MAX) From http://man7.org/linux/man-pages/man3/getdtablesize.3.html: "It is not specified in POSIX.1; portable applications should employ sysconf(_SC_OPEN_MAX) instead of this call." Specifically this fixes a build failure on Android which does not have getdtablesize(). Signed-off-by: Fredrik Fornwall --- src/loop.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/loop.c b/src/loop.c index 3d068181..bf4876bc 100644 --- a/src/loop.c +++ b/src/loop.c @@ -21,6 +21,7 @@ Contributors: #include #ifndef WIN32 #include +#include #else #include #include @@ -126,7 +127,7 @@ int mosquitto_main_loop(struct mosquitto_db *db, mosq_sock_t *listensock, int li #ifdef WIN32 pollfd_max = _getmaxstdio(); #else - pollfd_max = getdtablesize(); + pollfd_max = sysconf(_SC_OPEN_MAX); #endif pollfds = _mosquitto_malloc(sizeof(struct pollfd)*pollfd_max); From 0ba0bc434eba0c7b0d30110707e5b462be28b464 Mon Sep 17 00:00:00 2001 From: "Roger A. Light" Date: Wed, 5 Jul 2017 22:56:19 +0100 Subject: [PATCH 25/28] Use constant time memcmp for password checks. --- ChangeLog.txt | 3 +++ src/security_default.c | 22 +++++++++++++++++++++- 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/ChangeLog.txt b/ChangeLog.txt index e83010eb..b56a8660 100644 --- a/ChangeLog.txt +++ b/ChangeLog.txt @@ -1,3 +1,6 @@ +Broker: +- Use constant time memcmp for password comparisons. + 1.4.13 - 20170627 ================= diff --git a/src/security_default.c b/src/security_default.c index 43cd3f0c..c4085828 100644 --- a/src/security_default.c +++ b/src/security_default.c @@ -33,6 +33,9 @@ static int _pw_digest(const char *password, const unsigned char *salt, unsigned static int _base64_decode(char *in, unsigned char **decoded, unsigned int *decoded_len); #endif +static int mosquitto__memcmp_const(const void *ptr1, const void *b, size_t len); + + int mosquitto_security_init_default(struct mosquitto_db *db, bool reload) { int rc; @@ -650,6 +653,23 @@ static int _psk_file_parse(struct mosquitto_db *db) return MOSQ_ERR_SUCCESS; } + +static int mosquitto__memcmp_const(const void *a, const void *b, size_t len) +{ + int i; + int rc = 0; + + if(!a || !b) return 1; + + for(i=0; isalt, u->salt_len, hash, &hash_len); if(rc == MOSQ_ERR_SUCCESS){ - if(hash_len == u->password_len && !memcmp(u->password, hash, hash_len)){ + if(hash_len == u->password_len && !mosquitto__memcmp_const(u->password, hash, hash_len)){ return MOSQ_ERR_SUCCESS; }else{ return MOSQ_ERR_AUTH; From 5b73897f9892918f6148ea97bfa6ea998f196927 Mon Sep 17 00:00:00 2001 From: Zard1096 Date: Tue, 11 Jul 2017 13:08:38 +0800 Subject: [PATCH 26/28] Fix iOS crash issues Relate to issues #327 and #63. mosq->sock may be closed before FD_SET(mosq->sock, &writefds) and FD_ISSET(mosq->sock, &writefds) but after judgement in line 947 if(mosq->sock != INVALID_SOCKET). FD_SET(-1, ...) and FD_ISSET(-1, ...) would certainly crash. Signed-off-by: Zard1096 --- lib/mosquitto.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/mosquitto.c b/lib/mosquitto.c index be8e62e5..61ffdd87 100644 --- a/lib/mosquitto.c +++ b/lib/mosquitto.c @@ -971,9 +971,10 @@ int mosquitto_loop(struct mosquitto *mosq, int timeout, int max_packets) /* Fake write possible, to stimulate output write even though * we didn't ask for it, because at that point the publish or * other command wasn't present. */ - FD_SET(mosq->sock, &writefds); + if(mosq->sock != INVALID_SOCKET) + FD_SET(mosq->sock, &writefds); } - if(FD_ISSET(mosq->sock, &writefds)){ + if(mosq->sock != INVALID_SOCKET && FD_ISSET(mosq->sock, &writefds)){ #ifdef WITH_TLS if(mosq->want_connect){ rc = mosquitto__socket_connect_tls(mosq); From d40d7772d37f1de587dfd9bc49d374d8d5b393b7 Mon Sep 17 00:00:00 2001 From: "Aska.Wu" Date: Tue, 18 Jul 2017 16:32:05 +0800 Subject: [PATCH 27/28] Fix the TLS handshake problem if PSK has leading zero Incorrect psk will be provided by psk_server_callback() because leading zero is skipped by BN_bn2bin() and BN_num_bytes(). Signed-off-by: Aska.Wu --- lib/util_mosq.c | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/lib/util_mosq.c b/lib/util_mosq.c index 6f6bb49a..55e65e9e 100644 --- a/lib/util_mosq.c +++ b/lib/util_mosq.c @@ -327,19 +327,33 @@ int _mosquitto_hex2bin(const char *hex, unsigned char *bin, int bin_max_len) { BIGNUM *bn = NULL; int len; + int leading_zero = 0; + int start = 0; + int i = 0; + + /* Count the number of leading zero */ + for(i=0; i bin_max_len){ + if(BN_num_bytes(bn) + leading_zero > bin_max_len){ BN_free(bn); return 0; } - len = BN_bn2bin(bn, bin); + len = BN_bn2bin(bn, bin + leading_zero); BN_free(bn); - return len; + return len + leading_zero; } #endif From 46630e7325a82644be1a6fef00ef30bbd52c7ef6 Mon Sep 17 00:00:00 2001 From: "Roger A. Light" Date: Tue, 18 Jul 2017 21:53:29 +0100 Subject: [PATCH 28/28] Update change log. --- ChangeLog.txt | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/ChangeLog.txt b/ChangeLog.txt index 86f809cf..9f471d92 100644 --- a/ChangeLog.txt +++ b/ChangeLog.txt @@ -1,5 +1,10 @@ Broker: - Use constant time memcmp for password comparisons. +- Fix incorrect PSK key being used if it had leading zeroes. + +Client library: +- Fix incorrect PSK key being used if it had leading zeroes. + 1.4.14 - 20170710 =================