From 03b99ae3d29aa9b49e6e1bd538feddb9813d6daf Mon Sep 17 00:00:00 2001 From: Sean Bright Date: Thu, 23 Mar 2017 10:30:18 -0400 Subject: [PATCH 1/3] res_xmpp: Correctly check return value of SSL_connect SSL_connect returns non-zero for both success and some error conditions so simply negating is inadequate. Change-Id: Ifbf882896e598703b6c615407fa456d3199f95b1 --- res/res_xmpp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/res/res_xmpp.c b/res/res_xmpp.c index e422c14a2a..aee2cf5cf5 100644 --- a/res/res_xmpp.c +++ b/res/res_xmpp.c @@ -2664,7 +2664,7 @@ static int xmpp_client_requested_tls(struct ast_xmpp_client *client, struct ast_ goto failure; } - if (!SSL_connect(client->ssl_session)) { + if (SSL_connect(client->ssl_session) <= 0) { goto failure; } From 196626556250ae395026f9b235b3e7c0b98a508c Mon Sep 17 00:00:00 2001 From: Sean Bright Date: Thu, 23 Mar 2017 10:45:35 -0400 Subject: [PATCH 2/3] res_xmpp: Try to provide useful errors messages from OpenSSL If any errors occur during the TLS connection setup, we currently dump a fairly generic error message. So instead we try to pull in something useful from OpenSSL to report instead. ASTERISK-24712 Reported by: Matthias Urlichs Change-Id: I288500991a9681f447d92913b11fedaf426087f4 --- res/res_xmpp.c | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/res/res_xmpp.c b/res/res_xmpp.c index aee2cf5cf5..ebf29741aa 100644 --- a/res/res_xmpp.c +++ b/res/res_xmpp.c @@ -2623,12 +2623,31 @@ static int xmpp_client_request_tls(struct ast_xmpp_client *client, struct ast_xm #endif } +#ifdef HAVE_OPENSSL +static char *openssl_error_string(void) +{ + char *buf = NULL, *ret; + size_t len; + BIO *bio = BIO_new(BIO_s_mem()); + + ERR_print_errors(bio); + len = BIO_get_mem_data(bio, &buf); + ret = ast_calloc(1, len + 1); + if (ret) { + memcpy(ret, buf, len); + } + BIO_free(bio); + return ret; +} +#endif + /*! \brief Internal function called when we receive a response to our TLS initiation request */ static int xmpp_client_requested_tls(struct ast_xmpp_client *client, struct ast_xmpp_client_config *cfg, int type, iks *node) { #ifdef HAVE_OPENSSL int sock; long ssl_opts; + char *err; #endif if (!strcmp(iks_name(node), "success")) { @@ -2684,7 +2703,10 @@ static int xmpp_client_requested_tls(struct ast_xmpp_client *client, struct ast_ return 0; failure: - ast_log(LOG_ERROR, "TLS connection for client '%s' cannot be established. OpenSSL initialization failed.\n", client->name); + err = openssl_error_string(); + ast_log(LOG_ERROR, "TLS connection for client '%s' cannot be established. " + "OpenSSL initialization failed: %s\n", client->name, err); + ast_free(err); return -1; #endif } From 73bb08fd6a37b99aa61c5b6e75587c7a4512ee39 Mon Sep 17 00:00:00 2001 From: Sean Bright Date: Thu, 23 Mar 2017 10:48:40 -0400 Subject: [PATCH 3/3] res_xmpp: Use incremental backoff when a read error occurs If a read error occurs, we immediately attempt a reconnect without any delay. Instead, let's sleep and backoff up to 60 seconds before we try again. ASTERISK-24712 #close Reported by: Matthias Urlichs Change-Id: I6fe10ef4734837727437beab715e336777f13f48 --- res/res_xmpp.c | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/res/res_xmpp.c b/res/res_xmpp.c index ebf29741aa..95d3cc009e 100644 --- a/res/res_xmpp.c +++ b/res/res_xmpp.c @@ -3581,6 +3581,7 @@ int ast_xmpp_client_disconnect(struct ast_xmpp_client *client) { if ((client->thread != AST_PTHREADT_NULL) && !pthread_equal(pthread_self(), client->thread)) { xmpp_client_change_state(client, XMPP_STATE_DISCONNECTING); + pthread_cancel(client->thread); pthread_join(client->thread, NULL); client->thread = AST_PTHREADT_NULL; } @@ -3760,11 +3761,26 @@ static int xmpp_client_receive(struct ast_xmpp_client *client, unsigned int time return IKS_OK; } +static void sleep_with_backoff(unsigned int *sleep_time) +{ + /* We're OK with our thread dying here */ + pthread_setcancelstate(PTHREAD_CANCEL_ENABLE, NULL); + + sleep(*sleep_time); + *sleep_time = MIN(60, *sleep_time * 2); + + pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, NULL); +} + /*! \brief XMPP client connection thread */ static void *xmpp_client_thread(void *data) { struct ast_xmpp_client *client = data; int res = IKS_NET_RWERR; + unsigned int sleep_time = 1; + + /* We only allow cancellation while sleeping */ + pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, NULL); do { if (client->state == XMPP_STATE_DISCONNECTING) { @@ -3775,7 +3791,7 @@ static void *xmpp_client_thread(void *data) if (res == IKS_NET_RWERR || client->timeout == 0) { ast_debug(3, "[%s] Connecting\n", client->name); if ((res = xmpp_client_reconnect(client)) != IKS_OK) { - sleep(4); + sleep_with_backoff(&sleep_time); res = IKS_NET_RWERR; } continue; @@ -3814,6 +3830,8 @@ static void *xmpp_client_thread(void *data) } } else if (res == IKS_NET_RWERR) { ast_log(LOG_WARNING, "[%s] Socket read error\n", client->name); + ast_xmpp_client_disconnect(client); + sleep_with_backoff(&sleep_time); } else if (res == IKS_NET_NOSOCK) { ast_log(LOG_WARNING, "[%s] No socket\n", client->name); } else if (res == IKS_NET_NOCONN) { @@ -3826,6 +3844,8 @@ static void *xmpp_client_thread(void *data) ast_log(LOG_WARNING, "[%s] Dropped?\n", client->name); } else if (res == IKS_NET_UNKNOWN) { ast_debug(5, "[%s] Unknown\n", client->name); + } else if (res == IKS_OK) { + sleep_time = 1; } } while (1);