From f2a6eeb978c15e46199bdddcb57301502d58f02c Mon Sep 17 00:00:00 2001
From: Timur Pocheptsov <timur.pocheptsov@qt.io>
Date: Fri, 2 Dec 2022 11:41:11 +0100
Subject: [PATCH] qsslsocket_shared_mac: add more logging into certificate
 parsing

On macOS we observe strange CA certificates that are coming from
Security framework and which it cannot later parse from the DER
format we feed it in. Add some more debugging in order to understand,
which certificate gives such result.

Task-number: QTBUG-109135
Change-Id: I75cf4591e33c85db6fe80d37d84ede1456c56231
Reviewed-by: Timur Pocheptsov <timur.pocheptsov@qt.io>
(cherry picked from commit 8a18466e38779b63c19e281e92031cebaedbcba5)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
---
 src/plugins/tls/securetransport/qtls_st.cpp   |  2 -
 .../tls/shared/qsslsocket_mac_shared.cpp      | 64 ++++++++++++++++++-
 2 files changed, 62 insertions(+), 4 deletions(-)

diff --git a/src/plugins/tls/securetransport/qtls_st.cpp b/src/plugins/tls/securetransport/qtls_st.cpp
index 5306cad71de..cde5c05a358 100644
--- a/src/plugins/tls/securetransport/qtls_st.cpp
+++ b/src/plugins/tls/securetransport/qtls_st.cpp
@@ -1126,8 +1126,6 @@ bool TlsCryptographSecureTransport::verifyPeerTrust()
         QCFType<CFDataRef> certData = cert.toDer().toCFData();
         if (QCFType<SecCertificateRef> secRef = SecCertificateCreateWithData(nullptr, certData))
             CFArrayAppendValue(certArray, secRef);
-        else
-            qCWarning(lcSecureTransport, "Failed to create SecCertificate from QSslCertificate");
     }
 
     SecTrustSetAnchorCertificates(trust, certArray);
diff --git a/src/plugins/tls/shared/qsslsocket_mac_shared.cpp b/src/plugins/tls/shared/qsslsocket_mac_shared.cpp
index f40d2fb770e..1257240ee25 100644
--- a/src/plugins/tls/shared/qsslsocket_mac_shared.cpp
+++ b/src/plugins/tls/shared/qsslsocket_mac_shared.cpp
@@ -6,6 +6,7 @@
 
 #include <QtNetwork/qsslcertificate.h>
 
+#include <QtCore/qloggingcategory.h>
 #include <QtCore/qglobal.h>
 #include <QtCore/qdebug.h>
 
@@ -21,6 +22,8 @@
 
 QT_BEGIN_NAMESPACE
 
+Q_LOGGING_CATEGORY(lcX509, "qt.mac.shared.x509");
+
 #ifdef Q_OS_MACOS
 namespace {
 
@@ -74,6 +77,52 @@ bool isCaCertificateTrusted(SecCertificateRef cfCert, int domain)
     return false;
 }
 
+bool canDERBeParsed(CFDataRef derData, const QSslCertificate &qtCert)
+{
+    // We are observing certificates, that while accepted when we copy them
+    // from the keychain(s), later give us 'Failed to create SslCertificate
+    // from QSslCertificate'. It's interesting to know at what step the failure
+    // occurred. Let's check it and skip it below if it's not valid.
+
+    auto checkDer = [](CFDataRef derData, const char *source)
+    {
+        Q_ASSERT(source);
+        Q_ASSERT(derData);
+
+        const auto cfLength = CFDataGetLength(derData);
+        if (cfLength <= 0) {
+            qCWarning(lcX509) << source << "returned faulty DER data with invalid length.";
+            return false;
+        }
+
+        QCFType<SecCertificateRef> secRef = SecCertificateCreateWithData(nullptr, derData);
+        if (!secRef) {
+            qCWarning(lcX509) << source << "returned faulty DER data which cannot be parsed back.";
+            return false;
+        }
+        return true;
+    };
+
+    if (!checkDer(derData, "SecCertificateCopyData")) {
+        qCDebug(lcX509) << "Faulty QSslCertificate is:" << qtCert;// Just in case we managed to parse something.
+        return false;
+    }
+
+    // Generic parser failed?
+    if (qtCert.isNull()) {
+        qCWarning(lcX509, "QSslCertificate failed to parse DER");
+        return false;
+    }
+
+    const QCFType<CFDataRef> qtDerData = qtCert.toDer().toCFData();
+    if (!checkDer(qtDerData, "QSslCertificate")) {
+        qCWarning(lcX509) << "Faulty QSslCertificate is:" << qtCert;
+        return false;
+    }
+
+    return true;
+}
+
 } // unnamed namespace
 #endif // Q_OS_MACOS
 
@@ -94,8 +143,19 @@ QList<QSslCertificate> systemCaCertificates()
                 SecCertificateRef cfCert = (SecCertificateRef)CFArrayGetValueAtIndex(cfCerts, i);
                 QCFType<CFDataRef> derData = SecCertificateCopyData(cfCert);
                 if (isCaCertificateTrusted(cfCert, dom)) {
-                    if (derData)
-                        systemCerts << QSslCertificate(QByteArray::fromCFData(derData), QSsl::Der);
+                    if (derData) {
+                        const auto newCert = QSslCertificate(QByteArray::fromCFData(derData), QSsl::Der);
+                        if (!canDERBeParsed(derData, newCert)) {
+                            // Last attempt to get some information about the certificate:
+                            CFShow(cfCert);
+                            continue;
+                        }
+                        systemCerts << newCert;
+                    } else {
+                        // "Returns NULL if the data passed in the certificate parameter
+                        // is not a valid certificate object."
+                        qCWarning(lcX509, "SecCertificateCopyData returned invalid DER data (nullptr).");
+                    }
                 }
             }
         }