diff options
author | Timur Pocheptsov <timur.pocheptsov@qt.io> | 2020-04-13 20:31:34 +0200 |
---|---|---|
committer | Timur Pocheptsov <timur.pocheptsov@qt.io> | 2020-04-17 12:34:52 +0200 |
commit | 36a8bdbc8417506513207daf4f36533a3d6632f3 (patch) | |
tree | c152f9e65cadf61837784b74e6e87ec6992b6522 | |
parent | 5ddec6ba089dade1c6688e0ff840c6774f29544a (diff) |
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)
-rw-r--r-- | src/network/ssl/qsslsocket.cpp | 2 | ||||
-rw-r--r-- | src/network/ssl/qsslsocket_openssl.cpp | 23 | ||||
-rw-r--r-- | src/network/ssl/qsslsocket_openssl11_symbols_p.h | 7 | ||||
-rw-r--r-- | src/network/ssl/qsslsocket_openssl_symbols.cpp | 8 | ||||
-rw-r--r-- | src/network/ssl/qsslsocket_opensslpre11_symbols_p.h | 2 | ||||
-rw-r--r-- | 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 4f49a71e8a..9f9eaf3d36 100644 --- a/src/network/ssl/qsslsocket.cpp +++ b/src/network/ssl/qsslsocket.cpp @@ -2108,7 +2108,7 @@ void QSslSocketPrivate::init() 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 ec772ddb45..c4abc1eef0 100644 --- a/src/network/ssl/qsslsocket_openssl.cpp +++ b/src/network/ssl/qsslsocket_openssl.cpp @@ -471,10 +471,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; } @@ -909,6 +915,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, @@ -1309,8 +1316,12 @@ void QSslSocketBackendPrivate::_q_caRootLoaded(QSslCertificate cert, QSslCertifi 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 0c32b0a934..c80baa20aa 100644 --- a/src/network/ssl/qsslsocket_openssl11_symbols_p.h +++ b/src/network/ssl/qsslsocket_openssl11_symbols_p.h @@ -186,4 +186,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 62ac228398..60ba3a0958 100644 --- a/src/network/ssl/qsslsocket_openssl_symbols.cpp +++ b/src/network/ssl/qsslsocket_openssl_symbols.cpp @@ -161,6 +161,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) @@ -213,6 +218,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) @@ -988,6 +994,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) @@ -1060,6 +1067,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 48364ceb1d..c139ecb32e 100644 --- a/src/network/ssl/qsslsocket_opensslpre11_symbols_p.h +++ b/src/network/ssl/qsslsocket_opensslpre11_symbols_p.h @@ -132,6 +132,8 @@ SSL_CTX *q_SSL_CTX_new(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 6f34c6c888..e657987769 100644 --- a/src/network/ssl/qsslsocket_p.h +++ b/src/network/ssl/qsslsocket_p.h @@ -220,6 +220,7 @@ protected: bool verifyErrorsHaveBeenIgnored(); bool paused; bool flushTriggered; + bool systemOrSslErrorDetected = false; }; QT_END_NAMESPACE |