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.

diff --git a/libs/sofia-sip/libsofia-sip-ua/tport/tport_tls.c b/libs/sofia-sip/libsofia-sip-ua/tport/tport_tls.c
index 976a88dede72340bbbef21a443ec9d11f4c6c841..c872336b0b5e2e9e153453fe3eb69a1186f31f65 100644 (file)
--- a/libs/sofia-sip/libsofia-sip-ua/tport/tport_tls.c
+++ b/libs/sofia-sip/libsofia-sip-ua/tport/tport_tls.c
@@ -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;

diff --git a/libs/sofia-sip/libsofia-sip-ua/tport/tport_tls.h b/libs/sofia-sip/libsofia-sip-ua/tport/tport_tls.h
index 74a8db6a15a01157b6c5d8518f3c700c6a0fe6a6..e8d04a14b79b946345802f412e79663dd939ef1d 100644 (file)
--- a/libs/sofia-sip/libsofia-sip-ua/tport/tport_tls.h
+++ b/libs/sofia-sip/libsofia-sip-ua/tport/tport_tls.h
@@ -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);

diff --git a/libs/sofia-sip/libsofia-sip-ua/tport/tport_type_ws.c b/libs/sofia-sip/libsofia-sip-ua/tport/tport_type_ws.c
index 8857515cb35816c139bbfc5af133cfe463d4c2d2..050b2f8364f5e0c4331fc88c877db8f01463faca 100644 (file)
--- a/libs/sofia-sip/libsofia-sip-ua/tport/tport_type_ws.c
+++ b/libs/sofia-sip/libsofia-sip-ua/tport/tport_type_ws.c
@@ -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);