QSslSocket::verify: do not alter the default configuration

QSslCertificate::verify() has an undocumented and not very desirable property -
on some platorms it updates the default configuration, which can be surprising.
For example, we deprecated QSslSocket::setDefaultCaCertificates() and recommend
using QSslConfiguration::defaultConfiguration(), QSslConfiguration::setDefaultConfiguration(),
and QSslConfiguration::setCaCertificates(). If an application does this to select
CA roots it trusts explicitly, and then for some reason is calling verify, the
application can have its QSslSockets successfully connecting to a host, whose
root was not trusted by the application. Also, on Windows, defaultCaCertificates()
include system roots already, no need to have them twice.

[ChangeLog][QtCore][QtNetwork] QSslSocket::verify - do not change the default configuration

Pick-to: 5.15
Pick-to: 6.0
Pick-to: 6.0.0
Fixes: QTBUG-88639
Change-Id: I1cd40b259d0a6dcd15c78d1e7c027ff10859595c
Reviewed-by: Mårten Nordheim <marten.nordheim@qt.io>
This commit is contained in:
Timur Pocheptsov 2020-11-20 10:34:15 +01:00
parent 6a1d9f6fc1
commit 1158ff67b4
4 changed files with 108 additions and 8 deletions

View File

@ -591,9 +591,9 @@ QList<QSslCertificate> QSslCertificate::fromData(const QByteArray &data, QSsl::E
the specified host name.
Note that the root (CA) certificate should not be included in the list to be verified,
this will be looked up automatically either using the CA list specified by
QSslSocket::defaultCaCertificates() or, if possible, it will be loaded on demand
on Unix.
this will be looked up automatically using the CA list specified in the
default QSslConfiguration, and, in addition, if possible, CA certificates loaded on
demand on Unix and Windows.
\since 5.0
*/

View File

@ -2319,10 +2319,16 @@ QList<QSslCertificate> QSslSocketBackendPrivate::STACKOFX509_to_QSslCertificates
QList<QSslError> QSslSocketBackendPrivate::verify(const QList<QSslCertificate> &certificateChain,
const QString &hostName)
{
auto roots = QSslConfiguration::defaultConfiguration().caCertificates();
#ifndef Q_OS_WIN
// On Windows, system CA certificates are already set as default ones.
// No need to add them again (and again) and also, if the default configuration
// has its own set of CAs, this probably should not be amended by the ones
// from the 'ROOT' store, since it's not what an application chose to trust.
if (s_loadRootCertsOnDemand)
setDefaultCaCertificates(defaultCaCertificates() + systemCaCertificates());
return verify(QSslConfiguration::defaultConfiguration().caCertificates(), certificateChain, hostName);
roots.append(systemCaCertificates());
#endif // Q_OS_WIN
return verify(roots, certificateChain, hostName);
}
QList<QSslError> QSslSocketBackendPrivate::verify(const QList<QSslCertificate> &caCertificates,

View File

@ -0,0 +1,50 @@
-----BEGIN CERTIFICATE-----
MIIEjjCCBDOgAwIBAgIQCQsKtxCf9ik3vIVQ+PMa5TAKBggqhkjOPQQDAjBKMQsw
CQYDVQQGEwJVUzEZMBcGA1UEChMQQ2xvdWRmbGFyZSwgSW5jLjEgMB4GA1UEAxMX
Q2xvdWRmbGFyZSBJbmMgRUNDIENBLTMwHhcNMjAwODE2MDAwMDAwWhcNMjEwODE2
MTIwMDAwWjBhMQswCQYDVQQGEwJVUzELMAkGA1UECBMCQ0ExFjAUBgNVBAcTDVNh
biBGcmFuY2lzY28xGTAXBgNVBAoTEENsb3VkZmxhcmUsIEluYy4xEjAQBgNVBAMT
CXd3dy5xdC5pbzBZMBMGByqGSM49AgEGCCqGSM49AwEHA0IABP/r0xH22wdU8fLk
RsXhxRj5fmUNUo7rxnUl3lyqYYp53cLvn3agQifXkegpE8Xv4pGmuyWZj85FtoeZ
UZh8iyCjggLiMIIC3jAfBgNVHSMEGDAWgBSlzjfq67B1DpRniLRF+tkkEIeWHzAd
BgNVHQ4EFgQU7qPYGi9VtC4/6MS+54LNEAXApBgwFAYDVR0RBA0wC4IJd3d3LnF0
LmlvMA4GA1UdDwEB/wQEAwIHgDAdBgNVHSUEFjAUBggrBgEFBQcDAQYIKwYBBQUH
AwIwewYDVR0fBHQwcjA3oDWgM4YxaHR0cDovL2NybDMuZGlnaWNlcnQuY29tL0Ns
b3VkZmxhcmVJbmNFQ0NDQS0zLmNybDA3oDWgM4YxaHR0cDovL2NybDQuZGlnaWNl
cnQuY29tL0Nsb3VkZmxhcmVJbmNFQ0NDQS0zLmNybDBMBgNVHSAERTBDMDcGCWCG
SAGG/WwBATAqMCgGCCsGAQUFBwIBFhxodHRwczovL3d3dy5kaWdpY2VydC5jb20v
Q1BTMAgGBmeBDAECAjB2BggrBgEFBQcBAQRqMGgwJAYIKwYBBQUHMAGGGGh0dHA6
Ly9vY3NwLmRpZ2ljZXJ0LmNvbTBABggrBgEFBQcwAoY0aHR0cDovL2NhY2VydHMu
ZGlnaWNlcnQuY29tL0Nsb3VkZmxhcmVJbmNFQ0NDQS0zLmNydDAMBgNVHRMBAf8E
AjAAMIIBBAYKKwYBBAHWeQIEAgSB9QSB8gDwAHYA9lyUL9F3MCIUVBgIMJRWjuNN
Exkzv98MLyALzE7xZOMAAAFz90PlSQAABAMARzBFAiAhrrtxdmuxpCy8HAJJ5Qkg
WNvlo8nZqfe6pqGUcz0dmwIhAOMqDtd5ZhcfRk96GAJxPm8bH4hDnmqDP/zJG2Mq
nFpMAHYAXNxDkv7mq0VEsV6a1FbmEDf71fpH3KFzlLJe5vbHDsoAAAFz90PlewAA
BAMARzBFAiB/EkdY10LDdaRcf6eSc/QxucxU+2PI+3pWjh/21A8ZUAIhAK2Qz9Kw
onlRNyHpV3E6qyVydkXihj3c3q5UclpURYcmMAoGCCqGSM49BAMCA0kAMEYCIQDz
K/lzLb2Rbeg1HErRLLm2HkJUmfOGU2+tbROSTGK8ugIhAKA+MKqaZ8VjPxQ+Ho4v
fuwccvZfkU/fg8tAHTOzX23v
-----END CERTIFICATE-----
-----BEGIN CERTIFICATE-----
MIIDzTCCArWgAwIBAgIQCjeHZF5ftIwiTv0b7RQMPDANBgkqhkiG9w0BAQsFADBa
MQswCQYDVQQGEwJJRTESMBAGA1UEChMJQmFsdGltb3JlMRMwEQYDVQQLEwpDeWJl
clRydXN0MSIwIAYDVQQDExlCYWx0aW1vcmUgQ3liZXJUcnVzdCBSb290MB4XDTIw
MDEyNzEyNDgwOFoXDTI0MTIzMTIzNTk1OVowSjELMAkGA1UEBhMCVVMxGTAXBgNV
BAoTEENsb3VkZmxhcmUsIEluYy4xIDAeBgNVBAMTF0Nsb3VkZmxhcmUgSW5jIEVD
QyBDQS0zMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEua1NZpkUC0bsH4HRKlAe
nQMVLzQSfS2WuIg4m4Vfj7+7Te9hRsTJc9QkT+DuHM5ss1FxL2ruTAUJd9NyYqSb
16OCAWgwggFkMB0GA1UdDgQWBBSlzjfq67B1DpRniLRF+tkkEIeWHzAfBgNVHSME
GDAWgBTlnVkwgkdYzKz6CFQ2hns6tQRN8DAOBgNVHQ8BAf8EBAMCAYYwHQYDVR0l
BBYwFAYIKwYBBQUHAwEGCCsGAQUFBwMCMBIGA1UdEwEB/wQIMAYBAf8CAQAwNAYI
KwYBBQUHAQEEKDAmMCQGCCsGAQUFBzABhhhodHRwOi8vb2NzcC5kaWdpY2VydC5j
b20wOgYDVR0fBDMwMTAvoC2gK4YpaHR0cDovL2NybDMuZGlnaWNlcnQuY29tL09t
bmlyb290MjAyNS5jcmwwbQYDVR0gBGYwZDA3BglghkgBhv1sAQEwKjAoBggrBgEF
BQcCARYcaHR0cHM6Ly93d3cuZGlnaWNlcnQuY29tL0NQUzALBglghkgBhv1sAQIw
CAYGZ4EMAQIBMAgGBmeBDAECAjAIBgZngQwBAgMwDQYJKoZIhvcNAQELBQADggEB
AAUkHd0bsCrrmNaF4zlNXmtXnYJX/OvoMaJXkGUFvhZEOFp3ArnPEELG4ZKk40Un
+ABHLGioVplTVI+tnkDB0A+21w0LOEhsUCxJkAZbZB2LzEgwLt4I4ptJIsCSDBFe
lpKU1fwg3FZs5ZKTv3ocwDfjhUkV+ivhdDkYD7fa86JXWGBPzI6UAPxGezQxPk1H
goE6y/SJXQ7vTQ1unBuCJN0yJV0ReFEQPaA1IwQvZW+cwdFD19Ae8zFnWSfda9J1
CZMRJCQUzym+5iPDuI9yP+kHyCREU3qzuWFloUwOxkgAyXVjBYdwRVKD05WdRerw
6DEdfgkfCv4+3ao8XnTSrLE=
-----END CERTIFICATE-----

View File

@ -31,6 +31,7 @@
#include <QtCore/qthread.h>
#include <QtCore/qelapsedtimer.h>
#include <QtCore/qrandom.h>
#include <QtCore/qscopeguard.h>
#include <QtNetwork/qhostaddress.h>
#include <QtNetwork/qhostinfo.h>
#include <QtNetwork/qnetworkproxy.h>
@ -51,10 +52,12 @@
#include "../../../network-settings.h"
#ifndef QT_NO_SSL
#ifndef QT_NO_OPENSSL
#include "private/qsslsocket_openssl_p.h"
#include "private/qsslsocket_openssl_symbols_p.h"
#endif
#endif // QT_NO_OPENSSL
#include "private/qsslsocket_p.h"
#include "private/qsslconfiguration_p.h"
@ -73,7 +76,8 @@ typedef QSharedPointer<QSslSocket> QSslSocketPtr;
#define FLUKE_CERTIFICATE_ERROR QSslError::SelfSignedCertificate
#else
#define FLUKE_CERTIFICATE_ERROR QSslError::CertificateUntrusted
#endif
#endif // QT_NO_OPENSSL
#endif // QT_NO_OPENSSL
// Detect ALPN (Application-Layer Protocol Negotiation) support
@ -215,6 +219,9 @@ private slots:
void waitForMinusOne();
void verifyMode();
void verifyDepth();
#ifndef QT_NO_OPENSSL
void verifyAndDefaultConfiguration();
#endif // QT_NO_OPENSSL
void disconnectFromHostWhenConnecting();
void disconnectFromHostWhenConnected();
#ifndef QT_NO_OPENSSL
@ -2413,6 +2420,43 @@ void tst_QSslSocket::verifyDepth()
QCOMPARE(socket.peerVerifyDepth(), 1);
}
#ifndef QT_NO_OPENSSL
void tst_QSslSocket::verifyAndDefaultConfiguration()
{
QFETCH_GLOBAL(const bool, setProxy);
if (setProxy)
return;
const auto defaultCACertificates = QSslConfiguration::defaultConfiguration().caCertificates();
const auto chainGuard = qScopeGuard([&defaultCACertificates]{
auto conf = QSslConfiguration::defaultConfiguration();
conf.setCaCertificates(defaultCACertificates);
QSslConfiguration::setDefaultConfiguration(conf);
});
auto chain = QSslCertificate::fromPath(testDataDir + QStringLiteral("certs/qtiochain.crt"), QSsl::Pem);
QCOMPARE(chain.size(), 2);
QVERIFY(!chain.at(0).isNull());
QVERIFY(!chain.at(1).isNull());
auto errors = QSslCertificate::verify(chain);
// At least, test that 'verify' did not alter the default configuration:
QCOMPARE(defaultCACertificates, QSslConfiguration::defaultConfiguration().caCertificates());
if (!errors.isEmpty())
QSKIP("The certificate for qt.io could not be trusted, skipping the rest of the test");
#ifdef Q_OS_WINDOWS
const auto fakeCaChain = QSslCertificate::fromPath(testDataDir + QStringLiteral("certs/fluke.cert"));
QCOMPARE(fakeCaChain.size(), 1);
const auto caCert = fakeCaChain.at(0);
QVERIFY(!caCert.isNull());
auto conf = QSslConfiguration::defaultConfiguration();
conf.setCaCertificates({caCert});
QSslConfiguration::setDefaultConfiguration(conf);
errors = QSslCertificate::verify(chain);
QVERIFY(errors.size() > 0);
QCOMPARE(QSslConfiguration::defaultConfiguration().caCertificates(), QList{caCert});
#endif
}
#endif // QT_NO_OPENSSL
void tst_QSslSocket::disconnectFromHostWhenConnecting()
{
QSslSocketPtr socket = newSocket();