From 70d8460fc20dab2e1408b8d27ef5fe7a80ba9dc6 Mon Sep 17 00:00:00 2001 From: Timur Pocheptsov Date: Mon, 27 Mar 2017 15:55:34 +0200 Subject: QNAM: delay SSL initialization It's really unfortunate that even a plain 'http' request results in (Open)SSL initialization; this is apparently done by QSslConfiguration's default constructor and we have several classes including QSslConfiguration as a data-member. There are different problems reported because of this, from crashes (a broken OpenSSL on Windows) to long initialization times, which is not acceptable if no 'https' request was actually executed. This patch-set is replacing data-members of type QSslConfiguration with smart-pointers and delays (Open)SSL initialization. Task-number: QTBUG-59750 Change-Id: Id1d375e689dbd2d134abbb0572a9e804d595110e Reviewed-by: Edward Welbourne Reviewed-by: Timur Pocheptsov --- .../access/qhttpnetworkconnectionchannel.cpp | 23 +++++++++++++++------- .../access/qhttpnetworkconnectionchannel_p.h | 4 +++- src/network/access/qhttpthreaddelegate.cpp | 12 ++++++----- src/network/access/qhttpthreaddelegate_p.h | 4 ++-- src/network/access/qnetworkreplyhttpimpl.cpp | 17 +++++++++++----- src/network/access/qnetworkreplyhttpimpl_p.h | 5 +++-- 6 files changed, 43 insertions(+), 22 deletions(-) (limited to 'src/network') diff --git a/src/network/access/qhttpnetworkconnectionchannel.cpp b/src/network/access/qhttpnetworkconnectionchannel.cpp index c86cc9d8c9..84681561f2 100644 --- a/src/network/access/qhttpnetworkconnectionchannel.cpp +++ b/src/network/access/qhttpnetworkconnectionchannel.cpp @@ -55,7 +55,6 @@ # include # include # include -# include #endif #ifndef QT_NO_BEARERMANAGEMENT @@ -176,8 +175,8 @@ void QHttpNetworkConnectionChannel::init() if (!ignoreSslErrorsList.isEmpty()) sslSocket->ignoreSslErrors(ignoreSslErrorsList); - if (!sslConfiguration.isNull()) - sslSocket->setSslConfiguration(sslConfiguration); + if (sslConfiguration.data() && !sslConfiguration->isNull()) + sslSocket->setSslConfiguration(*sslConfiguration); } else { #endif // !QT_NO_SSL if (connection->connectionType() != QHttpNetworkConnection::ConnectionTypeHTTP2) @@ -656,7 +655,10 @@ void QHttpNetworkConnectionChannel::setSslConfiguration(const QSslConfiguration if (socket) static_cast(socket)->setSslConfiguration(config); - sslConfiguration = config; + if (sslConfiguration.data()) + *sslConfiguration = config; + else + sslConfiguration.reset(new QSslConfiguration(config)); } #endif @@ -1085,8 +1087,15 @@ void QHttpNetworkConnectionChannel::_q_encrypted() Q_FALLTHROUGH(); case QSslConfiguration::NextProtocolNegotiationNone: { protocolHandler.reset(new QHttpProtocolHandler(this)); + if (!sslConfiguration.data()) { + // Our own auto-tests bypass the normal initialization (done by + // QHttpThreadDelegate), this means in the past we'd have here + // the default constructed QSslConfiguration without any protocols + // to negotiate. Let's create it now: + sslConfiguration.reset(new QSslConfiguration); + } - QList protocols = sslConfiguration.allowedNextProtocols(); + QList protocols = sslConfiguration->allowedNextProtocols(); const int nProtocols = protocols.size(); // Clear the protocol that we failed to negotiate, so we do not try // it again on other channels that our connection can create/open. @@ -1096,10 +1105,10 @@ void QHttpNetworkConnectionChannel::_q_encrypted() protocols.removeAll(QSslConfiguration::NextProtocolSpdy3_0); if (nProtocols > protocols.size()) { - sslConfiguration.setAllowedNextProtocols(protocols); + sslConfiguration->setAllowedNextProtocols(protocols); const int channelCount = connection->d_func()->channelCount; for (int i = 0; i < channelCount; ++i) - connection->d_func()->channels[i].setSslConfiguration(sslConfiguration); + connection->d_func()->channels[i].setSslConfiguration(*sslConfiguration); } connection->setConnectionType(QHttpNetworkConnection::ConnectionTypeHTTP); diff --git a/src/network/access/qhttpnetworkconnectionchannel_p.h b/src/network/access/qhttpnetworkconnectionchannel_p.h index 61aea9d35d..584d52ddb7 100644 --- a/src/network/access/qhttpnetworkconnectionchannel_p.h +++ b/src/network/access/qhttpnetworkconnectionchannel_p.h @@ -78,6 +78,8 @@ # include #endif +#include + QT_BEGIN_NAMESPACE class QHttpNetworkRequest; @@ -128,7 +130,7 @@ public: #ifndef QT_NO_SSL bool ignoreAllSslErrors; QList ignoreSslErrorsList; - QSslConfiguration sslConfiguration; + QScopedPointer sslConfiguration; void ignoreSslErrors(); void ignoreSslErrors(const QList &errors); void setSslConfiguration(const QSslConfiguration &config); diff --git a/src/network/access/qhttpthreaddelegate.cpp b/src/network/access/qhttpthreaddelegate.cpp index e9287b233a..603f5dc9ab 100644 --- a/src/network/access/qhttpthreaddelegate.cpp +++ b/src/network/access/qhttpthreaddelegate.cpp @@ -293,19 +293,22 @@ void QHttpThreadDelegate::startRequest() = httpRequest.isHTTP2Allowed() ? QHttpNetworkConnection::ConnectionTypeHTTP2 : QHttpNetworkConnection::ConnectionTypeHTTP; + if (ssl && !incomingSslConfiguration.data()) + incomingSslConfiguration.reset(new QSslConfiguration); + #ifndef QT_NO_SSL if (httpRequest.isHTTP2Allowed() && ssl) { QList protocols; protocols << QSslConfiguration::ALPNProtocolHTTP2 << QSslConfiguration::NextProtocolHttp1_1; - incomingSslConfiguration.setAllowedNextProtocols(protocols); + incomingSslConfiguration->setAllowedNextProtocols(protocols); } else if (httpRequest.isSPDYAllowed() && ssl) { connectionType = QHttpNetworkConnection::ConnectionTypeSPDY; urlCopy.setScheme(QStringLiteral("spdy")); // to differentiate SPDY requests from HTTPS requests QList nextProtocols; nextProtocols << QSslConfiguration::NextProtocolSpdy3_0 << QSslConfiguration::NextProtocolHttp1_1; - incomingSslConfiguration.setAllowedNextProtocols(nextProtocols); + incomingSslConfiguration->setAllowedNextProtocols(nextProtocols); } #endif // QT_NO_SSL @@ -334,9 +337,8 @@ void QHttpThreadDelegate::startRequest() #endif #ifndef QT_NO_SSL // Set the QSslConfiguration from this QNetworkRequest. - if (ssl && incomingSslConfiguration != QSslConfiguration::defaultConfiguration()) { - httpConnection->setSslConfiguration(incomingSslConfiguration); - } + if (ssl) + httpConnection->setSslConfiguration(*incomingSslConfiguration); #endif #ifndef QT_NO_NETWORKPROXY diff --git a/src/network/access/qhttpthreaddelegate_p.h b/src/network/access/qhttpthreaddelegate_p.h index 6d1ea11f29..2f6954aa3b 100644 --- a/src/network/access/qhttpthreaddelegate_p.h +++ b/src/network/access/qhttpthreaddelegate_p.h @@ -63,7 +63,7 @@ #include "qhttpnetworkrequest_p.h" #include "qhttpnetworkconnection_p.h" #include -#include "qsslconfiguration.h" +#include #include "private/qnoncontiguousbytedevice_p.h" #include "qnetworkaccessauthenticationmanager_p.h" @@ -88,7 +88,7 @@ public: // incoming bool ssl; #ifndef QT_NO_SSL - QSslConfiguration incomingSslConfiguration; + QScopedPointer incomingSslConfiguration; #endif QHttpNetworkRequest httpRequest; qint64 downloadBufferMaximumSize; diff --git a/src/network/access/qnetworkreplyhttpimpl.cpp b/src/network/access/qnetworkreplyhttpimpl.cpp index fec3b0a100..84b1ddf5ac 100644 --- a/src/network/access/qnetworkreplyhttpimpl.cpp +++ b/src/network/access/qnetworkreplyhttpimpl.cpp @@ -180,7 +180,8 @@ QNetworkReplyHttpImpl::QNetworkReplyHttpImpl(QNetworkAccessManager* const manage d->outgoingData = outgoingData; d->url = request.url(); #ifndef QT_NO_SSL - d->sslConfiguration = request.sslConfiguration(); + if (request.url().scheme() == QLatin1String("https")) + d->sslConfiguration.reset(new QSslConfiguration(request.sslConfiguration())); #endif // FIXME Later maybe set to Unbuffered, especially if it is zerocopy or from cache? @@ -419,7 +420,10 @@ void QNetworkReplyHttpImpl::setSslConfigurationImplementation(const QSslConfigur void QNetworkReplyHttpImpl::sslConfigurationImplementation(QSslConfiguration &configuration) const { Q_D(const QNetworkReplyHttpImpl); - configuration = d->sslConfiguration; + if (d->sslConfiguration.data()) + configuration = *d->sslConfiguration; + else + configuration = request().sslConfiguration(); } #endif @@ -786,7 +790,7 @@ void QNetworkReplyHttpImplPrivate::postRequest(const QNetworkRequest &newHttpReq delegate->ssl = ssl; #ifndef QT_NO_SSL if (ssl) - delegate->incomingSslConfiguration = newHttpRequest.sslConfiguration(); + delegate->incomingSslConfiguration.reset(new QSslConfiguration(newHttpRequest.sslConfiguration())); #endif // Do we use synchronous HTTP? @@ -1411,10 +1415,13 @@ void QNetworkReplyHttpImplPrivate::replySslErrors( *toBeIgnored = pendingIgnoreSslErrorsList; } -void QNetworkReplyHttpImplPrivate::replySslConfigurationChanged(const QSslConfiguration &sslConfiguration) +void QNetworkReplyHttpImplPrivate::replySslConfigurationChanged(const QSslConfiguration &newSslConfiguration) { // Receiving the used SSL configuration from the HTTP thread - this->sslConfiguration = sslConfiguration; + if (sslConfiguration.data()) + *sslConfiguration = newSslConfiguration; + else + sslConfiguration.reset(new QSslConfiguration(newSslConfiguration)); } void QNetworkReplyHttpImplPrivate::replyPreSharedKeyAuthenticationRequiredSlot(QSslPreSharedKeyAuthenticator *authenticator) diff --git a/src/network/access/qnetworkreplyhttpimpl_p.h b/src/network/access/qnetworkreplyhttpimpl_p.h index 9383149124..26b16e8386 100644 --- a/src/network/access/qnetworkreplyhttpimpl_p.h +++ b/src/network/access/qnetworkreplyhttpimpl_p.h @@ -58,6 +58,7 @@ #include "QtCore/qpointer.h" #include "QtCore/qdatetime.h" #include "QtCore/qsharedpointer.h" +#include "QtCore/qscopedpointer.h" #include "qatomic.h" #include @@ -260,7 +261,7 @@ public: #ifndef QT_NO_SSL - QSslConfiguration sslConfiguration; + QScopedPointer sslConfiguration; bool pendingIgnoreAllSslErrors; QList pendingIgnoreSslErrorsList; #endif @@ -290,7 +291,7 @@ public: #ifndef QT_NO_SSL void replyEncrypted(); void replySslErrors(const QList &, bool *, QList *); - void replySslConfigurationChanged(const QSslConfiguration&); + void replySslConfigurationChanged(const QSslConfiguration &newSslConfiguration); void replyPreSharedKeyAuthenticationRequiredSlot(QSslPreSharedKeyAuthenticator *); #endif #ifndef QT_NO_NETWORKPROXY -- cgit v1.2.3