FS-9113 [sofia-sip] Clear out ssl error queue

Sofia will unpredictably close a tls transport during call setup. This
occurs when the epoll event loop wakes up the socket reader and SSL_read
returns an error because there is no packet on the socket. Normally
sofia will read the last error using SSL_get_error and return
SSL_ERROR_WANT_READ. Sofia gracefully handles this error and the
transport stays open. Sometimes, however, the worker thread will call
SSL_shutdown for a different transport, which can write an error to the
internal openssl error queue. If that error is not read off the queue,
the next time that SSL_get_error is called, it will read that unrelated
error.

The documentation for SSL_shutdown explains that there are three
possible results -1, 0 and 1 with, oddly, 1 indicating success. The -1
result code occurs when there is no handshake callback registered on the
connection. It can return 0 when there is still work to be done. The
documentation suggest that it is insufficient to call it just once. This
is why I added the do {} while () construct.

Although just the fix to SSL_shutdown was enough to resolve my issue, I
a also audited other calls to SSL_* functions and found a few other
cases where an error may be generated, but was not handled.
This commit is contained in:
Ethan Atkins 2016-04-27 11:34:58 -07:00 committed by Ethan Atkins
parent b61bf02c6b
commit db0dfe94d0
3 changed files with 43 additions and 12 deletions

View File

@ -137,7 +137,6 @@ enum { tls_buffer_size = 16384 };
* Log the TLS error specified by the error code @a e and all the errors in
* the queue. The error code @a e implies no error, and it is not logged.
*/
static
void tls_log_errors(unsigned level, char const *s, unsigned long e)
{
if (e == 0)
@ -449,12 +448,22 @@ int tls_init_context(tls_t *tls, tls_issues_t const *ti)
void tls_free(tls_t *tls)
{
int ret;
if (!tls)
return;
if (tls->con != NULL) {
SSL_shutdown(tls->con);
SSL_free(tls->con), tls->con = NULL;
do {
ret = SSL_shutdown(tls->con);
if (ret == -1) {
/* The return value -1 means that the connection wasn't actually established */
/* so it should be safe to not call shutdown again. We need to clear the eror */
/* queue for other connections though. */
tls_log_errors(3, "tls_free", 0);
ret = 1;
}
} while (ret != 1);
SSL_free(tls->con), tls->con = NULL;
}
if (tls->ctx != NULL && tls->type != tls_slave) {
@ -498,13 +507,18 @@ tls_t *tls_init_master(tls_issues_t *ti)
RAND_pseudo_bytes(sessionId, sizeof(sessionId));
SSL_CTX_set_session_id_context(tls->ctx,
if (!SSL_CTX_set_session_id_context(tls->ctx,
(void*) sessionId,
sizeof(sessionId));
sizeof(sessionId))) {
tls_log_errors(3, "tls_init_master", 0);
}
if (ti->CAfile != NULL)
if (ti->CAfile != NULL) {
SSL_CTX_set_client_CA_list(tls->ctx,
SSL_load_client_CA_file(ti->CAfile));
if (tls->ctx->client_CA == NULL)
tls_log_errors(3, "tls_init_master", 0);
}
#if 0
if (sock != -1) {
@ -576,6 +590,7 @@ int tls_post_connection_check(tport_t *self, tls_t *tls)
if (!tls) return -1;
if (!(cipher = SSL_get_current_cipher(tls->con))) {
tls_log_errors(3, "tls_post_connection_check", 0);
SU_DEBUG_7(("%s(%p): %s\n", __func__, (void*)self,
"OpenSSL failed to return an SSL_CIPHER object to us."));
return SSL_ERROR_SSL;

View File

@ -83,6 +83,7 @@ tls_t *tls_init_master(tls_issues_t *tls_issues);
tls_t *tls_init_secondary(tls_t *tls_master, int sock, int accept);
void tls_free(tls_t *tls);
int tls_get_socket(tls_t *tls);
void tls_log_errors(unsigned level, char const *s, unsigned long e);
ssize_t tls_read(tls_t *tls);
void *tls_read_buffer(tls_t *tls, size_t N);
int tls_want_read(tls_t *tls, int events);

View File

@ -385,22 +385,37 @@ static int tport_ws_init_primary_secure(tport_primary_t *pri,
SSL_CTX_sess_set_remove_cb(wspri->ssl_ctx, NULL);
wspri->ws_secure = 1;
if ( !wspri->ssl_ctx ) goto done;
if ( !wspri->ssl_ctx ) {
tls_log_errors(3, "tport_ws_init_primary_secure", 0);
goto done;
}
if (chain) {
SSL_CTX_use_certificate_chain_file(wspri->ssl_ctx, chain);
if ( !SSL_CTX_use_certificate_chain_file(wspri->ssl_ctx, chain) ) {
tls_log_errors(3, "tport_ws_init_primary_secure", 0);
}
}
/* set the local certificate from CertFile */
SSL_CTX_use_certificate_file(wspri->ssl_ctx, cert, SSL_FILETYPE_PEM);
if ( !SSL_CTX_use_certificate_file(wspri->ssl_ctx, cert, SSL_FILETYPE_PEM) ) {
tls_log_errors(3, "tport_ws_init_primary_secure", 0);
goto done;
}
/* set the private key from KeyFile */
SSL_CTX_use_PrivateKey_file(wspri->ssl_ctx, key, SSL_FILETYPE_PEM);
if ( !SSL_CTX_use_PrivateKey_file(wspri->ssl_ctx, key, SSL_FILETYPE_PEM) ) {
tls_log_errors(3, "tport_ws_init_primary_secure", 0);
goto done;
}
/* verify private key */
if ( !SSL_CTX_check_private_key(wspri->ssl_ctx) ) {
goto done;
tls_log_errors(3, "tport_ws_init_primary_secure", 0);
goto done;
}
SSL_CTX_set_cipher_list(wspri->ssl_ctx, "!eNULL:!aNULL:!DSS:HIGH:@STRENGTH");
if ( !SSL_CTX_set_cipher_list(wspri->ssl_ctx, "!eNULL:!aNULL:!DSS:HIGH:@STRENGTH") ) {
tls_log_errors(3, "tport_ws_init_primary_secure", 0);
goto done;
}
ret = tport_ws_init_primary(pri, tpn, ai, tags, return_culprit);