[Asterisk-code-review] tcptls: Improve error messages for TLS connections. (asterisk[13])

[Joshua Colp]

Joshua Colp has uploaded a new change for review. ( https://gerrit.asterisk.org/5614 )

Change subject: tcptls: Improve error messages for TLS connections.
......................................................................

tcptls: Improve error messages for TLS connections.

This change uses the functions provided by OpenSSL to query
and better construct error messages for situations where
the connection encounters a problem.

ASTERISK-26606

Change-Id: I7ae40ce88c0dc4e185c4df1ceb3a6ccc198f075b
---
M main/tcptls.c
1 file changed, 59 insertions(+), 8 deletions(-)


  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/14/5614/1

diff --git a/main/tcptls.c b/main/tcptls.c
index 3fd3c53..7e09e66 100644
--- a/main/tcptls.c
+++ b/main/tcptls.c
@@ -83,6 +83,39 @@
 	int exclusive_input;
 };
 
+#if defined(DO_SSL)
+AST_THREADSTORAGE(err2str_threadbuf);
+#define ERR2STR_BUFSIZE   128
+
+static const char *ssl_error_to_string(int sslerr, int ret)
+{
+	switch (sslerr) {
+	case SSL_ERROR_SSL:
+		return "Internal SSL error";
+	case SSL_ERROR_SYSCALL:
+		if (!ret) {
+			return "System call EOF";
+		} else if (ret == -1) {
+			char *buf;
+
+			buf = ast_threadstorage_get(&err2str_threadbuf, ERR2STR_BUFSIZE);
+			if (!buf) {
+				return "Unknown";
+			}
+
+			snprintf(buf, ERR2STR_BUFSIZE, "Underlying BIO error: %s", strerror(errno));
+			return buf;
+		} else {
+			return "System call other";
+		}
+	default:
+		break;
+	}
+
+	return "Unknown";
+}
+#endif
+
 void ast_tcptls_stream_set_timeout_disable(struct ast_tcptls_stream *stream)
 {
 	ast_assert(stream != NULL);
@@ -151,12 +184,17 @@
 #if defined(DO_SSL)
 	if (stream->ssl) {
 		for (;;) {
+			int sslerr;
+			char err[256];
+
 			res = SSL_read(stream->ssl, buf, size);
 			if (0 < res) {
 				/* We read some payload data. */
 				return res;
 			}
-			switch (SSL_get_error(stream->ssl, res)) {
+
+			sslerr = SSL_get_error(stream->ssl, res);
+			switch (sslerr) {
 			case SSL_ERROR_ZERO_RETURN:
 				/* Report EOF for a shutdown */
 				ast_debug(1, "TLS clean shutdown alert reading data\n");
@@ -204,7 +242,8 @@
 				break;
 			default:
 				/* Report EOF for an undecoded SSL or transport error. */
-				ast_debug(1, "TLS transport or SSL error reading data\n");
+				ast_debug(1, "TLS transport or SSL error reading data: %s, %s\n", ERR_error_string(sslerr, err),
+					ssl_error_to_string(sslerr, res));
 				return 0;
 			}
 			if (!ms) {
@@ -279,6 +318,9 @@
 		written = 0;
 		remaining = size;
 		for (;;) {
+			int sslerr;
+			char err[256];
+
 			res = SSL_write(stream->ssl, buf + written, remaining);
 			if (res == remaining) {
 				/* Everything was written. */
@@ -290,7 +332,8 @@
 				remaining -= res;
 				continue;
 			}
-			switch (SSL_get_error(stream->ssl, res)) {
+			sslerr = SSL_get_error(stream->ssl, res);
+			switch (sslerr) {
 			case SSL_ERROR_ZERO_RETURN:
 				ast_debug(1, "TLS clean shutdown alert writing data\n");
 				if (written) {
@@ -319,7 +362,8 @@
 				break;
 			default:
 				/* Undecoded SSL or transport error. */
-				ast_debug(1, "TLS transport or SSL error writing data\n");
+				ast_debug(1, "TLS transport or SSL error writing data: %s, %s\n", ERR_error_string(sslerr, err),
+					ssl_error_to_string(sslerr, res));
 				if (written) {
 					/* Report partial write. */
 					return written;
@@ -396,8 +440,11 @@
 			 */
 			res = SSL_shutdown(stream->ssl);
 			if (res < 0) {
-				ast_log(LOG_ERROR, "SSL_shutdown() failed: %d\n",
-					SSL_get_error(stream->ssl, res));
+				int sslerr = SSL_get_error(stream->ssl, res);
+				char err[256];
+
+				ast_log(LOG_ERROR, "SSL_shutdown() failed: %s, %s\n",
+					ERR_error_string(sslerr, err), ssl_error_to_string(sslerr, res));
 			}
 
 #if defined(OPENSSL_VERSION_NUMBER) && OPENSSL_VERSION_NUMBER >= 0x10100000L
@@ -589,6 +636,7 @@
 
 	return ret;
 }
+
 #endif
 
 /*! \brief
@@ -604,7 +652,6 @@
 #ifdef DO_SSL
 	int (*ssl_setup)(SSL *) = (tcptls_session->client) ? SSL_connect : SSL_accept;
 	int ret;
-	char err[256];
 #endif
 
 	/* TCP/TLS connections are associated with external protocols, and
@@ -642,7 +689,11 @@
 	else if ( (tcptls_session->ssl = SSL_new(tcptls_session->parent->tls_cfg->ssl_ctx)) ) {
 		SSL_set_fd(tcptls_session->ssl, tcptls_session->fd);
 		if ((ret = ssl_setup(tcptls_session->ssl)) <= 0) {
-			ast_log(LOG_ERROR, "Problem setting up ssl connection: %s\n", ERR_error_string(ERR_get_error(), err));
+			char err[256];
+			int sslerr = SSL_get_error(tcptls_session->ssl, ret);
+
+			ast_log(LOG_ERROR, "Problem setting up ssl connection: %s, %s\n", ERR_error_string(sslerr, err),
+				ssl_error_to_string(sslerr, ret));
 		} else if ((tcptls_session->f = tcptls_stream_fopen(tcptls_session->stream_cookie,
 			tcptls_session->ssl, tcptls_session->fd, -1))) {
 			if ((tcptls_session->client && !ast_test_flag(&tcptls_session->parent->tls_cfg->flags, AST_SSL_DONT_VERIFY_SERVER))

-- 
To view, visit https://gerrit.asterisk.org/5614
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I7ae40ce88c0dc4e185c4df1ceb3a6ccc198f075b
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: Joshua Colp <jcolp at digium.com>