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.

Fixes: QTBUG-83450
Change-Id: I6326119f4e79605429263045ac20605c30dccca3
Reviewed-by: Mårten Nordheim <marten.nordheim@qt.io>
This commit is contained in:
Timur Pocheptsov 2020-04-13 20:31:34 +02:00
parent 18fa1ed747
commit 8907635da5
5 changed files with 22 additions and 7 deletions

View File

@ -2241,7 +2241,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();

View File

@ -649,10 +649,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;
}
@ -1126,6 +1132,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,
@ -1777,8 +1784,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();
}

View File

@ -373,6 +373,7 @@ DEFINEFUNC3(void, SSL_set_bio, SSL *a, a, BIO *b, b, BIO *c, c, return, DUMMYARG
DEFINEFUNC(void, SSL_set_accept_state, SSL *a, a, return, DUMMYARG)
DEFINEFUNC(void, SSL_set_connect_state, SSL *a, a, return, DUMMYARG)
DEFINEFUNC(int, SSL_shutdown, SSL *a, a, return -1, return)
DEFINEFUNC(int, SSL_in_init, const SSL *a, a, return 0, return)
DEFINEFUNC(int, SSL_get_shutdown, const SSL *ssl, ssl, return 0, return)
DEFINEFUNC2(int, SSL_set_session, SSL* to, to, SSL_SESSION *session, session, return -1, return)
DEFINEFUNC(void, SSL_SESSION_free, SSL_SESSION *ses, ses, return, DUMMYARG)
@ -1065,6 +1066,7 @@ bool q_resolveOpenSslSymbols()
RESOLVEFUNC(SSL_set_bio)
RESOLVEFUNC(SSL_set_connect_state)
RESOLVEFUNC(SSL_shutdown)
RESOLVEFUNC(SSL_in_init)
RESOLVEFUNC(SSL_get_shutdown)
RESOLVEFUNC(SSL_set_session)
RESOLVEFUNC(SSL_SESSION_free)

View File

@ -516,6 +516,7 @@ void q_SSL_set_bio(SSL *a, BIO *b, BIO *c);
void q_SSL_set_accept_state(SSL *a);
void q_SSL_set_connect_state(SSL *a);
int q_SSL_shutdown(SSL *a);
int q_SSL_in_init(const SSL *s);
int q_SSL_get_shutdown(const SSL *ssl);
int q_SSL_set_session(SSL *to, SSL_SESSION *session);
void q_SSL_SESSION_free(SSL_SESSION *ses);

View File

@ -207,6 +207,7 @@ protected:
bool verifyErrorsHaveBeenIgnored();
bool paused;
bool flushTriggered;
bool systemOrSslErrorDetected = false;
QVector<QOcspResponse> ocspResponses;
};