Merge pull request #5856 from icing/errq-improvements

Improvements in OpenSSL Compat ERR Queue handling.

Author: JacobBarthelmeh

configure.ac

@@ -1629,12 +1629,23 @@ fi
 AC_ARG_ENABLE([error-queue-per-thread],
 [AS_HELP_STRING([--enable-error-queue-per-thread],[Enable one error queue per thread. Requires thread local storage. (default: disabled)])],
 [ ENABLED_ERRORQUEUEPERTHREAD=$enableval ],
-[ ENABLED_ERRORQUEUEPERTHREAD=no ]
+[ ENABLED_ERRORQUEUEPERTHREAD=check ]
 )
 
+if test "$ENABLED_ERRORQUEUEPERTHREAD" = "check"
+then
+    AS_IF([test "$thread_ls_on" = "no"],
+        [ENABLED_ERRORQUEUEPERTHREAD=no],
+        [ENABLED_ERRORQUEUEPERTHREAD=yes])
+fi
+
 if test "$ENABLED_ERRORQUEUEPERTHREAD" = "yes"
 then
-  AM_CFLAGS="$AM_CFLAGS -DERROR_QUEUE_PER_THREAD"
+    if test "$thread_ls_on" != "yes"
+    then
+        AC_MSG_ERROR(error-queue-per-thread needs thread-local storage.)
+    fi
+    AM_CFLAGS="$AM_CFLAGS -DERROR_QUEUE_PER_THREAD"
 fi
 
 # High Strength Build
@@ -8831,6 +8842,7 @@ echo "   * NXP SE050:                  $ENABLED_SE050"
 echo "   * Maxim Integrated MAXQ10XX:  $ENABLED_MAXQ10XX"
 echo "   * PSA:                        $ENABLED_PSA"
 echo "   * System CA certs:            $ENABLED_SYS_CA_CERTS"
+echo "   * ERR Queues per Thread:      $ENABLED_ERRORQUEUEPERTHREAD"
 echo ""
 echo "---"
 

src/ssl.c

@@ -17045,51 +17045,12 @@ int wolfSSL_set_compression(WOLFSSL* ssl)
 
     unsigned long wolfSSL_ERR_get_error(void)
     {
-        int ret;
-
         WOLFSSL_ENTER("wolfSSL_ERR_get_error");
-
 #ifdef WOLFSSL_HAVE_ERROR_QUEUE
-        ret = wc_PullErrorNode(NULL, NULL, NULL);
-        if (ret < 0) {
-            if (ret == BAD_STATE_E) {
-                ret = 0; /* no errors in queue */
-            }
-            else {
-                WOLFSSL_MSG("Error with pulling error node!");
-                WOLFSSL_LEAVE("wolfSSL_ERR_get_error", ret);
-                ret = 0 - ret; /* return absolute value of error */
-                /* panic and try to clear out nodes */
-                wc_ClearErrorNodes();
-            }
-        }
-        else {
-            int idx = wc_GetCurrentIdx();
-            if (idx < 0) {
-                WOLFSSL_MSG("Error with getting current index!");
-                ret = BAD_STATE_E;
-                WOLFSSL_LEAVE("wolfSSL_ERR_get_error", ret);
-
-                /* panic and try to clear out nodes and reset queue state */
-                wc_ClearErrorNodes();
-            }
-            else if (idx > 0) {
-                idx -= 1;
-                wc_RemoveErrorNode(idx);
-            }
-            else {
-                /* if current idx is 0 then the queue only had one node */
-                wc_RemoveErrorNode(idx);
-            }
-        }
-
-        return ret;
+        return wc_GetErrorNodeErr();
 #else
-
-        (void)ret;
-
         return (unsigned long)(0 - NOT_COMPILED_IN);
-#endif /* WOLFSSL_HAVE_ERROR_QUEUE */
+#endif
     }
 
 #ifdef WOLFSSL_HAVE_ERROR_QUEUE
@@ -33220,63 +33181,42 @@ int wolfSSL_AsyncPoll(WOLFSSL* ssl, WOLF_EVENT_FLAG flags)
 #endif /* WOLFSSL_ASYNC_CRYPT */
 
 #ifdef OPENSSL_EXTRA
+
+static int peek_ignore_err(int err)
+{
+  switch(err) {
+    case -WANT_READ:
+    case -WANT_WRITE:
+    case -ZERO_RETURN:
+    case -WOLFSSL_ERROR_ZERO_RETURN:
+    case -SOCKET_PEER_CLOSED_E:
+    case -SOCKET_ERROR_E:
+      return 1;
+    default:
+      return 0;
+  }
+}
+
 unsigned long wolfSSL_ERR_peek_error_line_data(const char **file, int *line,
                                                const char **data, int *flags)
 {
-    WOLFSSL_ENTER("wolfSSL_ERR_peek_error_line_data");
-
-    (void)line;
-    (void)file;
-
-    /* No data or flags stored - error display only in Nginx. */
-    if (data != NULL) {
-        *data = "";
-    }
-    if (flags != NULL) {
-        *flags = 0;
-    }
-
-#ifdef WOLFSSL_HAVE_ERROR_QUEUE
-    {
-        int ret = 0;
-        int idx = wc_GetCurrentIdx();
-
-        while (1) {
-            ret = wc_PeekErrorNode(idx, file, NULL, line);
-            if (ret == BAD_MUTEX_E || ret == BAD_FUNC_ARG || ret == BAD_STATE_E) {
-                WOLFSSL_MSG("Issue peeking at error node in queue");
-                return 0;
-            }
-            /* OpenSSL uses positive error codes */
-            if (ret < 0) {
-                ret = -ret;
-            }
+  unsigned long err;
 
-            if (ret == -ASN_NO_PEM_HEADER)
-                return (ERR_LIB_PEM << 24) | PEM_R_NO_START_LINE;
-        #ifdef OPENSSL_ALL
-            /* PARSE_ERROR is returned if an HTTP request is detected. */
-            if (ret == -SSL_R_HTTP_REQUEST)
-                return (ERR_LIB_SSL << 24) | -SSL_R_HTTP_REQUEST;
-        #endif
-        #if defined(OPENSSL_ALL) && defined(WOLFSSL_PYTHON)
-            if (ret == ASN1_R_HEADER_TOO_LONG) {
-                return (ERR_LIB_ASN1 << 24) | ASN1_R_HEADER_TOO_LONG;
-            }
-        #endif
-            if (ret != -WANT_READ && ret != -WANT_WRITE &&
-                    ret != -ZERO_RETURN && ret != -WOLFSSL_ERROR_ZERO_RETURN &&
-                    ret != -SOCKET_PEER_CLOSED_E && ret != -SOCKET_ERROR_E)
-                break;
-
-            wc_RemoveErrorNode(idx);
-        }
+    WOLFSSL_ENTER("wolfSSL_ERR_peek_error_line_data");
+    err = wc_PeekErrorNodeLineData(file, line, data, flags, peek_ignore_err);
 
-        return (unsigned long)ret;
-    }
-#else
-    return (unsigned long)(0 - NOT_COMPILED_IN);
+    if (err == -ASN_NO_PEM_HEADER)
+        return (ERR_LIB_PEM << 24) | PEM_R_NO_START_LINE;
+#ifdef OPENSSL_ALL
+    /* PARSE_ERROR is returned if an HTTP request is detected. */
+    else if (err == -SSL_R_HTTP_REQUEST)
+        return (ERR_LIB_SSL << 24) | -SSL_R_HTTP_REQUEST;
+#endif
+#if defined(OPENSSL_ALL) && defined(WOLFSSL_PYTHON)
+    else if (err == ASN1_R_HEADER_TOO_LONG)
+        return (ERR_LIB_ASN1 << 24) | ASN1_R_HEADER_TOO_LONG;
 #endif
+  return err;
 }
 #endif
 

tests/api.c

@@ -35237,6 +35237,13 @@ static void post_auth_version_cb(WOLFSSL* ssl)
     /* do handshake and then test version error */
     AssertIntEQ(wolfSSL_accept(ssl), WOLFSSL_SUCCESS);
     AssertStrEQ("TLSv1.2", wolfSSL_get_version(ssl));
+}
+
+static void post_auth_version_client_cb(WOLFSSL* ssl)
+{
+    /* do handshake and then test version error */
+    AssertIntEQ(wolfSSL_connect(ssl), WOLFSSL_SUCCESS);
+    AssertStrEQ("TLSv1.2", wolfSSL_get_version(ssl));
     AssertIntEQ(wolfSSL_verify_client_post_handshake(ssl), WOLFSSL_FAILURE);
 #if defined(OPENSSL_ALL) && !defined(NO_ERROR_QUEUE)
     /* check was added to error queue */
@@ -35302,8 +35309,9 @@ static int test_wolfSSL_Tls13_postauth(void)
     XMEMSET(&client_cbf, 0, sizeof(callback_functions));
     server_cbf.method = wolfTLSv1_2_server_method;
     server_cbf.ssl_ready = set_post_auth_cb;
-    client_cbf.ssl_ready = set_post_auth_cb;
     server_cbf.on_result = post_auth_version_cb;
+    client_cbf.ssl_ready = set_post_auth_cb;
+    client_cbf.on_result = post_auth_version_client_cb;
     server_args.callbacks = &server_cbf;
     client_args.callbacks = &client_cbf;
 
@@ -35319,6 +35327,7 @@ static int test_wolfSSL_Tls13_postauth(void)
     server_cbf.ssl_ready = set_post_auth_cb;
     client_cbf.ssl_ready = set_post_auth_cb;
     server_cbf.on_result = post_auth_cb;
+    client_cbf.on_result = NULL;
     server_args.callbacks = &server_cbf;
     client_args.callbacks = &client_cbf;
 
@@ -38734,12 +38743,22 @@ static int test_wolfSSL_PKCS8_d2i(void)
     defined(OPENSSL_EXTRA) && defined(DEBUG_WOLFSSL)
 #define LOGGING_THREADS 5
 #define ERROR_COUNT 10
+/* copied from logging.c since this is not exposed otherwise */
+#ifndef ERROR_QUEUE_MAX
+#ifdef ERROR_QUEUE_PER_THREAD
+    #define ERROR_QUEUE_MAX 16
+#else
+    /* this breaks from compat of unlimited error queue size */
+    #define ERROR_QUEUE_MAX 100
+#endif
+#endif
+
 static volatile int loggingThreadsReady;
 static THREAD_RETURN WOLFSSL_THREAD test_logging(void* args)
 {
     const char* file;
     int line;
-    int err;
+    unsigned long err;
     int errorCount = 0;
     int i;
 
@@ -38756,6 +38775,7 @@ static THREAD_RETURN WOLFSSL_THREAD test_logging(void* args)
     AssertIntEQ(errorCount, ERROR_COUNT);
 
     /* test max queue behavior, trying to add an arbitrary 3 errors over */
+    ERR_clear_error(); /* ERR_get_error_line() does not remove */
     errorCount = 0;
     for (i = 0; i < ERROR_QUEUE_MAX + 3; i++)
         ERR_put_error(ERR_LIB_PEM, SYS_F_ACCEPT, -990 - i, __FILE__, __LINE__);