summaryrefslogtreecommitdiff
blob: 9bbdda61a25ac0fe4e02fd289ab9ce09b6140969 (plain)
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
From 8ddffc6ba4f38bb8dbeb0cf61b6b10ee73505bbb 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
MIME-Version: 1.0
Content-Type: text/plain; charset=utf8
Content-Transfer-Encoding: 8bit

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)
---
 src/network/ssl/qsslsocket.cpp                     |  2 +-
 src/network/ssl/qsslsocket_openssl.cpp             | 23 ++++++++++++++++------
 src/network/ssl/qsslsocket_openssl11_symbols_p.h   |  7 +++++++
 src/network/ssl/qsslsocket_openssl_symbols.cpp     |  8 ++++++++
 .../ssl/qsslsocket_opensslpre11_symbols_p.h        |  2 ++
 src/network/ssl/qsslsocket_p.h                     |  1 +
 6 files changed, 36 insertions(+), 7 deletions(-)

diff --git a/src/network/ssl/qsslsocket.cpp b/src/network/ssl/qsslsocket.cpp
index 4e9e9472631..5c9e589ec39 100644
--- a/src/network/ssl/qsslsocket.cpp
+++ b/src/network/ssl/qsslsocket.cpp
@@ -2166,7 +2166,7 @@ void QSslSocketPrivate::init()
     pendingClose = false;
     flushTriggered = false;
     ocspResponses.clear();
-
+    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 51510f1c60b..855865209bc 100644
--- a/src/network/ssl/qsslsocket_openssl.cpp
+++ b/src/network/ssl/qsslsocket_openssl.cpp
@@ -648,10 +648,16 @@ bool QSslSocketBackendPrivate::initSslContext()
 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;
     }
@@ -1084,6 +1090,7 @@ void QSslSocketBackendPrivate::transmit()
             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,
@@ -1681,8 +1688,12 @@ bool QSslSocketBackendPrivate::checkOcspStatus()
 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 0fe0899d4fd..b7193ad1807 100644
--- a/src/network/ssl/qsslsocket_openssl11_symbols_p.h
+++ b/src/network/ssl/qsslsocket_openssl11_symbols_p.h
@@ -192,4 +192,11 @@ typedef int (*q_SSL_psk_use_session_cb_func_t)(SSL *, const EVP_MD *, const unsi
 }
 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 85029a6ff3f..d1bd84cf25f 100644
--- a/src/network/ssl/qsslsocket_openssl_symbols.cpp
+++ b/src/network/ssl/qsslsocket_openssl_symbols.cpp
@@ -160,6 +160,11 @@ DEFINEFUNC(void, OPENSSL_sk_free, OPENSSL_STACK *a, a, return, DUMMYARG)
 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)
@@ -242,6 +247,7 @@ DEFINEFUNC2(void, BIO_set_shutdown, BIO *a, a, int shut, shut, return, DUMMYARG)
 // 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)
@@ -971,6 +977,7 @@ bool q_resolveOpenSslSymbols()
 #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)
@@ -1066,6 +1073,7 @@ bool q_resolveOpenSslSymbols()
 #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 f5626d5d164..92841017793 100644
--- a/src/network/ssl/qsslsocket_opensslpre11_symbols_p.h
+++ b/src/network/ssl/qsslsocket_opensslpre11_symbols_p.h
@@ -121,6 +121,8 @@ SSL_CTX *q_SSL_CTX_new(const SSL_METHOD *a);
 
 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 daa9be23f4a..350b1f1fc18 100644
--- a/src/network/ssl/qsslsocket_p.h
+++ b/src/network/ssl/qsslsocket_p.h
@@ -208,6 +208,7 @@ protected:
     bool verifyErrorsHaveBeenIgnored();
     bool paused;
     bool flushTriggered;
+    bool systemOrSslErrorDetected = false;
     QVector<QOcspResponse> ocspResponses;
 };
 
-- 
2.16.3