[asterisk-bugs] [JIRA] (ASTERISK-26606) tcptls: Incorrect OpenSSL function call leads to misleading error report

[Joshua Colp (JIRA)]

     [ https://issues.asterisk.org/jira/browse/ASTERISK-26606?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Joshua Colp updated ASTERISK-26606:
-----------------------------------

    Severity: Minor  (was: Major)

> tcptls: Incorrect OpenSSL function call leads to misleading error report
> ------------------------------------------------------------------------
>
>                 Key: ASTERISK-26606
>                 URL: https://issues.asterisk.org/jira/browse/ASTERISK-26606
>             Project: Asterisk
>          Issue Type: Bug
>      Security Level: None
>          Components: Core/General
>    Affects Versions: 13.8.0, GIT
>            Reporter: Bob Ham
>            Severity: Minor
>
> In the function [handle_tcptls_connection() in tcptls.c|https://github.com/asterisk/asterisk/blob/a6e5bae3ef9fe498927e0b5f9318a64c9ff101a9/main/tcptls.c#L633], a call is made to SSL_connect() or SSL_accept() and the return value is checked with ERR_get_error().  However, [the man page|https://linux.die.net/man/3/ssl_get_error] for both these functions specify that SSL_get_error() should be used.
> While trying to diagnose an error, I was getting the message
> {{Problem setting up ssl connection: error:00000000:lib(0):func(0):reason(0)}}
> Modifying the code to use SSL_get_error() instead of ERR_get_error() gives the correct (although no more helpful) message:
> {{Problem setting up ssl connection: error:00000005:lib(0):func(0):reason(5)}}
> In fact, the return value from SSL_get_error() should be checked explicitly.  The test code that I added was as follows:
> {code}
>                 if ((ret = ssl_setup(tcptls_session->ssl)) <= 0) {
>                         int sslerr = SSL_get_error(tcptls_session->ssl, ret);
>                         const char *msg;
>                         ast_log(LOG_ERROR, "Problem setting up ssl connection: %s\n",
>                                 ERR_error_string(sslerr, err));
>                         switch (sslerr) {
>                         case SSL_ERROR_NONE: msg = "None"; break;
>                         case SSL_ERROR_ZERO_RETURN: msg = "Zero return"; break;
>                         case SSL_ERROR_WANT_READ: msg = "Want read"; break;
>                         case SSL_ERROR_WANT_WRITE: msg = "Want write"; break;
>                         case SSL_ERROR_WANT_CONNECT: msg = "Want connect"; break;
>                         case SSL_ERROR_WANT_ACCEPT: msg = "Want accept"; break;
>                         case SSL_ERROR_WANT_X509_LOOKUP: msg = "Want X509 lookup"; break;
>                         case SSL_ERROR_SYSCALL:
>                                 if (ret == 0) {
>                                         msg = "Syscall EOF";
>                                 } else if (ret == -1) {
>                                         ast_log(LOG_ERROR, "Underlying BIO error: %s\n",
>                                                 strerror(errno));
>                                         msg = "Syscall underlying BIO error (see above)";
>                                 } else {
>                                         msg = "Syscall other";
>                                 }
>                                 break;
>                         case SSL_ERROR_SSL: msg = "SSL"; break;
>                         default: msg = "[Unknown]"; break;
>                         }
>                         ast_log(LOG_ERROR, "Error from ssl_setup: %s\n", msg);
> {code}
> Which produces the following output:
> {code}
> [Nov 15 23:29:49] ERROR[2672]: tcptls.c:611 handle_tcptls_connection: Problem setting up ssl connection: error:00000005:lib(0):func(0):reason(5)
> [Nov 15 23:29:49] ERROR[2672]: tcptls.c:625 handle_tcptls_connection: Underlying BIO error: Connection reset by peer
> [Nov 15 23:29:49] ERROR[2672]: tcptls.c:635 handle_tcptls_connection: Error from ssl_setup: Syscall underlying BIO error (see above)
> {code}
> Obviously my switch statement is a hack but it would be good to have proper checking of the return value of SSL_get_error(), as directed by the man page documentation, in some wrapper function.



--
This message was sent by Atlassian JIRA
(v6.2#6252)