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.
eclipse · Nov 18, 2020 · 10ecae6 · 10ecae6
1 parent a65242d
commit 10ecae6
Show file tree

Hide file tree

Showing 2 changed files with 26 additions and 34 deletions.
diff --git a/ChangeLog.txt b/ChangeLog.txt
@@ -82,6 +82,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:
 - Client no longer generates random client ids for v3.1.1 clients, these are

diff --git a/lib/net_mosq.c b/lib/net_mosq.c
@@ -203,6 +203,9 @@ void net__init_tls(void)
 int net__socket_close(struct mosquitto *mosq)
 {
  int rc = 0;
+#ifdef WITH_BROKER
+ struct mosquitto *mosq_found;
+#endif
 
  assert(mosq);
 #ifdef WITH_TLS
@@ -232,7 +235,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;
@@ -680,8 +686,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;
  }
@@ -699,8 +703,6 @@ static int net__init_ssl_ctx(struct mosquitto *mosq)
  SSL_CTX_set_options(mosq->ssl_ctx, SSL_OP_NO_SSLv3 | SSL_OP_NO_TLSv1);
  }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;
  }
 
@@ -725,15 +727,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);
@@ -748,8 +746,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;
  }
@@ -760,8 +756,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;
  }
@@ -787,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;
  }
@@ -801,16 +793,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;
  }
@@ -820,16 +808,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;
  }
@@ -845,8 +829,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;
  }
@@ -857,8 +839,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;
  }
@@ -875,31 +855,42 @@ 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){
  SSL_free(mosq->ssl);
  }
  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;
  }
 
  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;
  }
@@ -909,12 +900,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;
  }