From 36a8bdbc8417506513207daf4f36533a3d6632f3 Mon Sep 17 00:00:00 2001

From: Timur Pocheptsov <timur.pocheptsov@qt.io>

Date: Mon, 13 Apr 2020 20:31:34 +0200

Subject: [PATCH] OpenSSL: handle SSL_shutdown's errors properly

Do not call SSL_shutdown on a session that is in handshake state (SSL_in_init(s)

returns 1). Also, do not call SSL_shutdown if a session encountered a fatal

error (SSL_ERROR_SYSCALL or SSL_ERROR_SSL was found before). If SSL_shutdown

was unsuccessful (returned code != 1), we have to clear the error(s) it queued.

Unfortunately, SSL_in_init was a macro in OpenSSL 1.0.x. We have to

resolve SSL_state to implement SSL_in_init.

Fixes: QTBUG-83450

Change-Id: I6326119f4e79605429263045ac20605c30dccca3

Reviewed-by: Mårten Nordheim <marten.nordheim@qt.io>

(cherry picked from commit 8907635da59c2ae0e8db01f27b24a841b830e655)

(cherry picked from commit 8ddffc6ba4f38bb8dbeb0cf61b6b10ee73505bbb)

---

diff --git a/src/network/ssl/qsslsocket.cpp b/src/network/ssl/qsslsocket.cpp

index 4f49a71..9f9eaf3 100644

--- a/src/network/ssl/qsslsocket.cpp

+++ b/src/network/ssl/qsslsocket.cpp

@@ -2108,7 +2108,7 @@

     shutdown = false;

     pendingClose = false;

     flushTriggered = false;

-

+    systemOrSslErrorDetected = false;

     // we don't want to clear the ignoreErrorsList, so

     // that it is possible setting it before connecting

 //    ignoreErrorsList.clear();

diff --git a/src/network/ssl/qsslsocket_openssl.cpp b/src/network/ssl/qsslsocket_openssl.cpp

index ec772dd..c4abc1e 100644

--- a/src/network/ssl/qsslsocket_openssl.cpp

+++ b/src/network/ssl/qsslsocket_openssl.cpp

@@ -471,10 +471,16 @@

 void QSslSocketBackendPrivate::destroySslContext()

 {

     if (ssl) {

-        // We do not send a shutdown alert here. Just mark the session as

-        // resumable for qhttpnetworkconnection's "optimization", otherwise

-        // OpenSSL won't start a session resumption.

-        q_SSL_shutdown(ssl);

+        if (!q_SSL_in_init(ssl) && !systemOrSslErrorDetected) {

+            // We do not send a shutdown alert here. Just mark the session as

+            // resumable for qhttpnetworkconnection's "optimization", otherwise

+            // OpenSSL won't start a session resumption.

+            if (q_SSL_shutdown(ssl) != 1) {

+                // Some error may be queued, clear it.

+                const auto errors = getErrorsFromOpenSsl();

+                Q_UNUSED(errors);

+            }

+        }

         q_SSL_free(ssl);

         ssl = nullptr;

     }

@@ -909,6 +915,7 @@

             case SSL_ERROR_SSL: // error in the SSL library

                 // we do not know exactly what the error is, nor whether we can recover from it,

                 // so just return to prevent an endless loop in the outer "while" statement

+                systemOrSslErrorDetected = true;

                 {

                     const ScopedBool bg(inSetAndEmitError, true);

                     setErrorAndEmit(QAbstractSocket::SslInternalError,

@@ -1309,8 +1316,12 @@

 void QSslSocketBackendPrivate::disconnectFromHost()

 {

     if (ssl) {

-        if (!shutdown) {

-            q_SSL_shutdown(ssl);

+        if (!shutdown && !q_SSL_in_init(ssl) && !systemOrSslErrorDetected) {

+            if (q_SSL_shutdown(ssl) != 1) {

+                // Some error may be queued, clear it.

+                const auto errors = getErrorsFromOpenSsl();

+                Q_UNUSED(errors);

+            }

             shutdown = true;

             transmit();

         }

diff --git a/src/network/ssl/qsslsocket_openssl11_symbols_p.h b/src/network/ssl/qsslsocket_openssl11_symbols_p.h

index 0c32b0a..c80baa2 100644

--- a/src/network/ssl/qsslsocket_openssl11_symbols_p.h

+++ b/src/network/ssl/qsslsocket_openssl11_symbols_p.h

@@ -186,4 +186,11 @@

 }

 void q_SSL_set_psk_use_session_callback(SSL *s, q_SSL_psk_use_session_cb_func_t);

+#if OPENSSL_VERSION_NUMBER < 0x10101000L

+// What a mess!

+int q_SSL_in_init(SSL *s);

+#else

+int q_SSL_in_init(const SSL *s);

+#endif // 1.1.1 or 1.1.0

+

 #endif

diff --git a/src/network/ssl/qsslsocket_openssl_symbols.cpp b/src/network/ssl/qsslsocket_openssl_symbols.cpp

index 62ac228..60ba3a0 100644

--- a/src/network/ssl/qsslsocket_openssl_symbols.cpp

+++ b/src/network/ssl/qsslsocket_openssl_symbols.cpp

@@ -161,6 +161,11 @@

 DEFINEFUNC2(void *, OPENSSL_sk_value, OPENSSL_STACK *a, a, int b, b, return nullptr, return)

 DEFINEFUNC(int, SSL_session_reused, SSL *a, a, return 0, return)

 DEFINEFUNC2(unsigned long, SSL_CTX_set_options, SSL_CTX *ctx, ctx, unsigned long op, op, return 0, return)

+#if OPENSSL_VERSION_NUMBER < 0x10101000L

+DEFINEFUNC(int, SSL_in_init, SSL *a, a, return 0, return)

+#else

+DEFINEFUNC(int, SSL_in_init, const SSL *a, a, return 0, return)

+#endif

 #ifdef TLS1_3_VERSION

 DEFINEFUNC2(int, SSL_CTX_set_ciphersuites, SSL_CTX *ctx, ctx, const char *str, str, return 0, return)

 DEFINEFUNC2(void, SSL_set_psk_use_session_callback, SSL *ssl, ssl, q_SSL_psk_use_session_cb_func_t callback, callback, return, DUMMYARG)

@@ -213,6 +218,7 @@

 // Functions below are either deprecated or removed in OpenSSL >= 1.1:

 DEFINEFUNC(unsigned char *, ASN1_STRING_data, ASN1_STRING *a, a, return nullptr, return)

+DEFINEFUNC(int, SSL_state, const SSL *a, a, return 0, return)

 #ifdef SSLEAY_MACROS

 DEFINEFUNC3(void *, ASN1_dup, i2d_of_void *a, a, d2i_of_void *b, b, char *c, c, return nullptr, return)

@@ -988,6 +994,7 @@

 #if QT_CONFIG(opensslv11)

     RESOLVEFUNC(OPENSSL_init_ssl)

+    RESOLVEFUNC(SSL_in_init)

     RESOLVEFUNC(OPENSSL_init_crypto)

     RESOLVEFUNC(ASN1_STRING_get0_data)

     RESOLVEFUNC(EVP_CIPHER_CTX_reset)

@@ -1060,6 +1067,7 @@

 #else // !opensslv11

     RESOLVEFUNC(ASN1_STRING_data)

+    RESOLVEFUNC(SSL_state)

 #ifdef SSLEAY_MACROS

     RESOLVEFUNC(ASN1_dup)

diff --git a/src/network/ssl/qsslsocket_opensslpre11_symbols_p.h b/src/network/ssl/qsslsocket_opensslpre11_symbols_p.h

index 48364ce..c139ecb 100644

--- a/src/network/ssl/qsslsocket_opensslpre11_symbols_p.h

+++ b/src/network/ssl/qsslsocket_opensslpre11_symbols_p.h

@@ -132,6 +132,8 @@

 int q_SSL_library_init();

 void q_SSL_load_error_strings();

+int q_SSL_state(const SSL *a);

+#define q_SSL_in_init(a) (q_SSL_state(a) & SSL_ST_INIT)

 #if OPENSSL_VERSION_NUMBER >= 0x10001000L

 int q_SSL_get_ex_new_index(long argl, void *argp, CRYPTO_EX_new *new_func, CRYPTO_EX_dup *dup_func, CRYPTO_EX_free *free_func);

diff --git a/src/network/ssl/qsslsocket_p.h b/src/network/ssl/qsslsocket_p.h

index 6f34c6c..e657987 100644

--- a/src/network/ssl/qsslsocket_p.h

+++ b/src/network/ssl/qsslsocket_p.h

@@ -220,6 +220,7 @@

     bool verifyErrorsHaveBeenIgnored();

     bool paused;

     bool flushTriggered;

+    bool systemOrSslErrorDetected = false;

 };

 QT_END_NAMESPACE