0015-Ssl-Copy-the-on-demand-cert-loading-bool-from-defaul.patch 5.5 KB

123456789101112131415161718192021222324252627282930313233343536373839404142434445464748495051525354555657585960616263646566676869707172737475767778798081828384858687888990919293949596979899100101102103104105106107108109110111112113114115116
  1. From 99a9f2eccb5de9843dc956dd380dcf7515cbce27 Mon Sep 17 00:00:00 2001
  2. From: =?UTF-8?q?M=C3=A5rten=20Nordheim?= <marten.nordheim@qt.io>
  3. Date: Thu, 25 May 2023 14:40:29 +0200
  4. Subject: [PATCH] Ssl: Copy the on-demand cert loading bool from default config
  5. Otherwise individual sockets will still load system certificates when
  6. a chain doesn't match against the configured CA certificates.
  7. That's not intended behavior, since specifically setting the CA
  8. certificates means you don't want the system certificates to be used.
  9. Follow-up to/amends ada2c573c1a25f8d96577734968fe317ddfa292a
  10. This is potentially a breaking change because now, if you ever add a
  11. CA to the default config, it will disable loading system certificates
  12. on demand for all sockets. And the only way to re-enable it is to
  13. create a null-QSslConfiguration and set it as the new default.
  14. Pick-to: 6.5 6.2 5.15
  15. Change-Id: Ic3b2ab125c0cdd58ad654af1cb36173960ce2d1e
  16. Reviewed-by: Timur Pocheptsov <timur.pocheptsov@qt.io>
  17. Fixes: https://security-tracker.debian.org/tracker/CVE-2023-34410
  18. Upstream: https://codereview.qt-project.org/gitweb?p=qt%2Fqtbase.git;a=commit;h=57ba6260c0801055b7188fdaa1818b940590f5f1
  19. Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
  20. ---
  21. src/network/ssl/qsslsocket.cpp | 5 ++++
  22. .../tst_manual_ssl_client_auth.cpp | 24 ++++++++++++++++---
  23. 2 files changed, 26 insertions(+), 3 deletions(-)
  24. diff --git a/src/network/ssl/qsslsocket.cpp b/src/network/ssl/qsslsocket.cpp
  25. index 4eefe439293..0563fd06634 100644
  26. --- a/src/network/ssl/qsslsocket.cpp
  27. +++ b/src/network/ssl/qsslsocket.cpp
  28. @@ -1973,6 +1973,10 @@ QSslSocketPrivate::QSslSocketPrivate()
  29. , flushTriggered(false)
  30. {
  31. QSslConfigurationPrivate::deepCopyDefaultConfiguration(&configuration);
  32. + // If the global configuration doesn't allow root certificates to be loaded
  33. + // on demand then we have to disable it for this socket as well.
  34. + if (!configuration.allowRootCertOnDemandLoading)
  35. + allowRootCertOnDemandLoading = false;
  36. const auto *tlsBackend = tlsBackendInUse();
  37. if (!tlsBackend) {
  38. @@ -2281,6 +2285,7 @@ void QSslConfigurationPrivate::deepCopyDefaultConfiguration(QSslConfigurationPri
  39. ptr->sessionProtocol = global->sessionProtocol;
  40. ptr->ciphers = global->ciphers;
  41. ptr->caCertificates = global->caCertificates;
  42. + ptr->allowRootCertOnDemandLoading = global->allowRootCertOnDemandLoading;
  43. ptr->protocol = global->protocol;
  44. ptr->peerVerifyMode = global->peerVerifyMode;
  45. ptr->peerVerifyDepth = global->peerVerifyDepth;
  46. diff --git a/tests/manual/network/ssl/client-auth/tst_manual_ssl_client_auth.cpp b/tests/manual/network/ssl/client-auth/tst_manual_ssl_client_auth.cpp
  47. index 2307cbb1911..4d4aaca7e34 100644
  48. --- a/tests/manual/network/ssl/client-auth/tst_manual_ssl_client_auth.cpp
  49. +++ b/tests/manual/network/ssl/client-auth/tst_manual_ssl_client_auth.cpp
  50. @@ -16,6 +16,9 @@
  51. // but the other side presents a certificate signed by a different CA.
  52. constexpr bool TestServerPresentsIncorrectCa = false;
  53. constexpr bool TestClientPresentsIncorrectCa = true;
  54. +// Decides whether or not to put the root CA into the global ssl configuration
  55. +// or into the socket's specific ssl configuration.
  56. +constexpr bool UseGlobalConfiguration = true;
  57. class ServerThread : public QThread
  58. {
  59. @@ -26,8 +29,10 @@ public:
  60. QSslServer server;
  61. QSslConfiguration config = server.sslConfiguration();
  62. - QList<QSslCertificate> certs = QSslCertificate::fromPath(QStringLiteral(":/rootCA.pem"));
  63. - config.setCaCertificates(certs);
  64. + if (!UseGlobalConfiguration) {
  65. + QList<QSslCertificate> certs = QSslCertificate::fromPath(QStringLiteral(":/rootCA.pem"));
  66. + config.setCaCertificates(certs);
  67. + }
  68. config.setLocalCertificate(QSslCertificate::fromPath(QStringLiteral(":/127.0.0.1.pem"))
  69. .first());
  70. QFile keyFile(QStringLiteral(":/127.0.0.1-key.pem"));
  71. @@ -73,6 +78,12 @@ int main(int argc, char **argv)
  72. if (!QFileInfo(u":/rootCA.pem"_s).exists())
  73. qFatal("rootCA.pem not found. Did you run generate.sh in the certs directory?");
  74. + if (UseGlobalConfiguration) {
  75. + QSslConfiguration config = QSslConfiguration::defaultConfiguration();
  76. + config.setCaCertificates(QSslCertificate::fromPath(u":/rootCA.pem"_s));
  77. + QSslConfiguration::setDefaultConfiguration(config);
  78. + }
  79. +
  80. ServerThread serverThread;
  81. serverThread.start();
  82. @@ -88,12 +99,19 @@ int main(int argc, char **argv)
  83. keyFileName = u":/accepted-client-key.pem"_s;
  84. }
  85. config.setLocalCertificate(QSslCertificate::fromPath(certificatePath).first());
  86. - if (TestServerPresentsIncorrectCa) // true: Verify server using incorrect CA: should fail
  87. + if (!UseGlobalConfiguration && TestServerPresentsIncorrectCa) {
  88. + // Verify server using incorrect CA: should fail
  89. config.setCaCertificates(QSslCertificate::fromPath(u":/rootCA.pem"_s));
  90. + } else if (UseGlobalConfiguration && !TestServerPresentsIncorrectCa) {
  91. + // Verify server using correct CA, we need to explicitly set the
  92. + // system CAs when the global config is overridden.
  93. + config.setCaCertificates(QSslConfiguration::systemCaCertificates());
  94. + }
  95. QFile keyFile(keyFileName);
  96. if (!keyFile.open(QIODevice::ReadOnly))
  97. qFatal("Failed to open key file");
  98. config.setPrivateKey(QSslKey(&keyFile, QSsl::Rsa));
  99. +
  100. socket.setSslConfiguration(config);
  101. QObject::connect(&socket, &QSslSocket::encrypted, []() { qDebug() << "[c] encrypted"; });
  102. --
  103. 2.46.0