From 1158ff67b492853b72199ed78bfcf24132e1c7ff Mon Sep 17 00:00:00 2001 From: Timur Pocheptsov Date: Fri, 20 Nov 2020 10:34:15 +0100 Subject: QSslSocket::verify: do not alter the default configuration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../network/ssl/qsslsocket/certs/qtiochain.crt | 50 ++++++++++++++++++++++ .../auto/network/ssl/qsslsocket/tst_qsslsocket.cpp | 48 ++++++++++++++++++++- 2 files changed, 96 insertions(+), 2 deletions(-) create mode 100644 tests/auto/network/ssl/qsslsocket/certs/qtiochain.crt (limited to 'tests/auto/network') diff --git a/tests/auto/network/ssl/qsslsocket/certs/qtiochain.crt b/tests/auto/network/ssl/qsslsocket/certs/qtiochain.crt new file mode 100644 index 0000000000..9634bcef68 --- /dev/null +++ b/tests/auto/network/ssl/qsslsocket/certs/qtiochain.crt @@ -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----- diff --git a/tests/auto/network/ssl/qsslsocket/tst_qsslsocket.cpp b/tests/auto/network/ssl/qsslsocket/tst_qsslsocket.cpp index c020f0a2c4..2bfb905620 100644 --- a/tests/auto/network/ssl/qsslsocket/tst_qsslsocket.cpp +++ b/tests/auto/network/ssl/qsslsocket/tst_qsslsocket.cpp @@ -31,6 +31,7 @@ #include #include #include +#include #include #include #include @@ -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 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(); -- cgit v1.2.3