From 628441d33b779b3c7f0529ad862a295ed5049460 Mon Sep 17 00:00:00 2001 From: "Roger A. Light" Date: Wed, 18 Nov 2020 15:36:34 +0000 Subject: [PATCH] Fix bridge sock not being removed from sock hash on error Prior to this, duplicate entries could be added to the sock hash, which caused an infinite loop. Only affects bridges with bad settings on startup, and only when compiled using WITH_ADNS=yes. Closes #1897. Thanks to Rodolfo Ochoa. --- ChangeLog.txt | 1 + lib/net_mosq.c | 59 +++++++++++++++++++++----------------------------- 2 files changed, 26 insertions(+), 34 deletions(-) diff --git a/ChangeLog.txt b/ChangeLog.txt index 0d652c89..cb21fda6 100644 --- a/ChangeLog.txt +++ b/ChangeLog.txt @@ -4,6 +4,7 @@ Broker: file and `per_listener_settings true` is set and the client did not set a username. Closes #1891. - Fix file logging on Windows. Closes #1880. +- Fix bridge sock not being removed from sock hash on error. Closes #1897. Client library: - Fix build on Mac Big Sur. Closes #1905. diff --git a/lib/net_mosq.c b/lib/net_mosq.c index 35650024..9c0fb717 100644 --- a/lib/net_mosq.c +++ b/lib/net_mosq.c @@ -191,6 +191,9 @@ int net__socket_close(struct mosquitto *mosq) #endif { int rc = 0; +#ifdef WITH_BROKER + struct mosquitto *mosq_found; +#endif assert(mosq); #ifdef WITH_TLS @@ -220,7 +223,10 @@ int net__socket_close(struct mosquitto *mosq) { if(mosq->sock != INVALID_SOCKET){ #ifdef WITH_BROKER - HASH_DELETE(hh_sock, db->contexts_by_sock, mosq); + HASH_FIND(hh_sock, db->contexts_by_sock, &mosq->sock, sizeof(mosq->sock), mosq_found); + if(mosq_found){ + HASH_DELETE(hh_sock, db->contexts_by_sock, mosq_found); + } #endif rc = COMPAT_CLOSE(mosq->sock); mosq->sock = INVALID_SOCKET; @@ -617,8 +623,6 @@ static int net__init_ssl_ctx(struct mosquitto *mosq) if(!mosq->ssl_ctx){ log__printf(mosq, MOSQ_LOG_ERR, "Error: Unable to create TLS context."); - COMPAT_CLOSE(mosq->sock); - mosq->sock = INVALID_SOCKET; net__print_ssl_error(mosq); return MOSQ_ERR_TLS; } @@ -641,8 +645,6 @@ static int net__init_ssl_ctx(struct mosquitto *mosq) #endif }else{ log__printf(mosq, MOSQ_LOG_ERR, "Error: Protocol %s not supported.", mosq->tls_version); - COMPAT_CLOSE(mosq->sock); - mosq->sock = INVALID_SOCKET; return MOSQ_ERR_INVAL; } @@ -667,15 +669,11 @@ static int net__init_ssl_ctx(struct mosquitto *mosq) engine = ENGINE_by_id(mosq->tls_engine); if(!engine){ log__printf(mosq, MOSQ_LOG_ERR, "Error loading %s engine\n", mosq->tls_engine); - COMPAT_CLOSE(mosq->sock); - mosq->sock = INVALID_SOCKET; return MOSQ_ERR_TLS; } if(!ENGINE_init(engine)){ log__printf(mosq, MOSQ_LOG_ERR, "Failed engine initialisation\n"); ENGINE_free(engine); - COMPAT_CLOSE(mosq->sock); - mosq->sock = INVALID_SOCKET; return MOSQ_ERR_TLS; } ENGINE_set_default(engine, ENGINE_METHOD_ALL); @@ -690,8 +688,6 @@ static int net__init_ssl_ctx(struct mosquitto *mosq) #if !defined(OPENSSL_NO_ENGINE) ENGINE_FINISH(engine); #endif - COMPAT_CLOSE(mosq->sock); - mosq->sock = INVALID_SOCKET; net__print_ssl_error(mosq); return MOSQ_ERR_TLS; } @@ -702,8 +698,6 @@ static int net__init_ssl_ctx(struct mosquitto *mosq) # if !defined(OPENSSL_NO_ENGINE) ENGINE_FINISH(engine); # endif - COMPAT_CLOSE(mosq->sock); - mosq->sock = INVALID_SOCKET; net__print_ssl_error(mosq); return MOSQ_ERR_TLS; } @@ -729,8 +723,6 @@ static int net__init_ssl_ctx(struct mosquitto *mosq) #if !defined(OPENSSL_NO_ENGINE) ENGINE_FINISH(engine); #endif - COMPAT_CLOSE(mosq->sock); - mosq->sock = INVALID_SOCKET; net__print_ssl_error(mosq); return MOSQ_ERR_TLS; } @@ -743,16 +735,12 @@ static int net__init_ssl_ctx(struct mosquitto *mosq) if(!ENGINE_ctrl_cmd(engine, ENGINE_SECRET_MODE, ENGINE_SECRET_MODE_SHA, NULL, NULL, 0)){ log__printf(mosq, MOSQ_LOG_ERR, "Error: Unable to set engine secret mode sha1"); ENGINE_FINISH(engine); - COMPAT_CLOSE(mosq->sock); - mosq->sock = INVALID_SOCKET; net__print_ssl_error(mosq); return MOSQ_ERR_TLS; } if(!ENGINE_ctrl_cmd(engine, ENGINE_PIN, 0, mosq->tls_engine_kpass_sha1, NULL, 0)){ log__printf(mosq, MOSQ_LOG_ERR, "Error: Unable to set engine pin"); ENGINE_FINISH(engine); - COMPAT_CLOSE(mosq->sock); - mosq->sock = INVALID_SOCKET; net__print_ssl_error(mosq); return MOSQ_ERR_TLS; } @@ -762,16 +750,12 @@ static int net__init_ssl_ctx(struct mosquitto *mosq) if(!pkey){ log__printf(mosq, MOSQ_LOG_ERR, "Error: Unable to load engine private key file \"%s\".", mosq->tls_keyfile); ENGINE_FINISH(engine); - COMPAT_CLOSE(mosq->sock); - mosq->sock = INVALID_SOCKET; net__print_ssl_error(mosq); return MOSQ_ERR_TLS; } if(SSL_CTX_use_PrivateKey(mosq->ssl_ctx, pkey) <= 0){ log__printf(mosq, MOSQ_LOG_ERR, "Error: Unable to use engine private key file \"%s\".", mosq->tls_keyfile); ENGINE_FINISH(engine); - COMPAT_CLOSE(mosq->sock); - mosq->sock = INVALID_SOCKET; net__print_ssl_error(mosq); return MOSQ_ERR_TLS; } @@ -787,8 +771,6 @@ static int net__init_ssl_ctx(struct mosquitto *mosq) #if !defined(OPENSSL_NO_ENGINE) ENGINE_FINISH(engine); #endif - COMPAT_CLOSE(mosq->sock); - mosq->sock = INVALID_SOCKET; net__print_ssl_error(mosq); return MOSQ_ERR_TLS; } @@ -799,8 +781,6 @@ static int net__init_ssl_ctx(struct mosquitto *mosq) #if !defined(OPENSSL_NO_ENGINE) ENGINE_FINISH(engine); #endif - COMPAT_CLOSE(mosq->sock); - mosq->sock = INVALID_SOCKET; net__print_ssl_error(mosq); return MOSQ_ERR_TLS; } @@ -817,13 +797,26 @@ static int net__init_ssl_ctx(struct mosquitto *mosq) #endif +void net__socket_close_compat(struct mosquitto *mosq) +{ +#ifdef WITH_BROKER + struct mosquitto_db *db = mosquitto__get_db(); + net__socket_close(db, mosq); +#else + net__socket_close(mosq); +#endif +} + int net__socket_connect_step3(struct mosquitto *mosq, const char *host) { #ifdef WITH_TLS BIO *bio; int rc = net__init_ssl_ctx(mosq); - if(rc) return rc; + if(rc){ + net__socket_close_compat(mosq); + return rc; + } if(mosq->ssl_ctx){ if(mosq->ssl){ @@ -831,8 +824,7 @@ int net__socket_connect_step3(struct mosquitto *mosq, const char *host) } mosq->ssl = SSL_new(mosq->ssl_ctx); if(!mosq->ssl){ - COMPAT_CLOSE(mosq->sock); - mosq->sock = INVALID_SOCKET; + net__socket_close_compat(mosq); net__print_ssl_error(mosq); return MOSQ_ERR_TLS; } @@ -840,8 +832,7 @@ int net__socket_connect_step3(struct mosquitto *mosq, const char *host) SSL_set_ex_data(mosq->ssl, tls_ex_index_mosq, mosq); bio = BIO_new_socket(mosq->sock, BIO_NOCLOSE); if(!bio){ - COMPAT_CLOSE(mosq->sock); - mosq->sock = INVALID_SOCKET; + net__socket_close_compat(mosq); net__print_ssl_error(mosq); return MOSQ_ERR_TLS; } @@ -851,12 +842,12 @@ int net__socket_connect_step3(struct mosquitto *mosq, const char *host) * required for the SNI resolving */ if(SSL_set_tlsext_host_name(mosq->ssl, host) != 1) { - COMPAT_CLOSE(mosq->sock); - mosq->sock = INVALID_SOCKET; + net__socket_close_compat(mosq); return MOSQ_ERR_TLS; } if(net__socket_connect_tls(mosq)){ + net__socket_close_compat(mosq); return MOSQ_ERR_TLS; }