4 files changed, 94 insertions, 28 deletions

diff --git a/src/plugins/tls/shared/qasn1element.cpp b/src/plugins/tls/shared/qasn1element.cpp
index cbf038f569..97be46866d 100644
--- a/src/plugins/tls/shared/qasn1element.cpp
+++ b/src/plugins/tls/shared/qasn1element.cpp
@@ -6,13 +6,17 @@
 
 #include <QtCore/qdatastream.h>
 #include <QtCore/qdatetime.h>
+#include <QtCore/qtimezone.h>
 #include <QtCore/qlist.h>
 #include <QDebug>
+#include <private/qtools_p.h>
 
 #include <limits>
 
 QT_BEGIN_NAMESPACE
 
+using namespace QtMiscUtils;
+
 typedef QMap<QByteArray, QByteArray> OidNameMap;
 static OidNameMap createOidMap()
 {
@@ -213,11 +217,6 @@ QDateTime QAsn1Element::toDateTime() const
 
     // QDateTime::fromString is lenient and accepts +- signs in front
     // of the year; but ASN.1 doesn't allow them.
-    const auto isAsciiDigit = [](char c)
-    {
-        return c >= '0' && c <= '9';
-    };
-
     if (!isAsciiDigit(mValue[0]))
         return result;
 
@@ -226,31 +225,28 @@ QDateTime QAsn1Element::toDateTime() const
         return result;
 
     if (mType == UtcTimeType && mValue.size() == 13) {
-        result = QDateTime::fromString(QString::fromLatin1(mValue),
-                                       QStringLiteral("yyMMddHHmmsst"));
-        if (!result.isValid())
-            return result;
-
-        Q_ASSERT(result.timeSpec() == Qt::UTC);
-
-        QDate date = result.date();
-
         // RFC 2459:
         //   Where YY is greater than or equal to 50, the year shall be
         //   interpreted as 19YY; and
         //
         //   Where YY is less than 50, the year shall be interpreted as 20YY.
         //
-        // QDateTime interprets the 'yy' format as 19yy, so we may need to adjust
-        // the year (bring it in the [1950, 2049] range).
-        if (date.year() < 1950)
-            result.setDate(date.addYears(100));
+        // so use 1950 as base year.
+        constexpr int rfc2459CenturyStart = 1950;
+        const QLatin1StringView inputView(mValue);
+        QDate date = QDate::fromString(inputView.first(6), u"yyMMdd", rfc2459CenturyStart);
+        if (!date.isValid())
+            return result;
 
-        Q_ASSERT(result.date().year() >= 1950);
-        Q_ASSERT(result.date().year() <= 2049);
+        Q_ASSERT(date.year() >= rfc2459CenturyStart);
+        Q_ASSERT(date.year() < 100 + rfc2459CenturyStart);
+
+        QTime time = QTime::fromString(inputView.sliced(6, 6), u"HHmmss");
+        if (!time.isValid())
+            return result;
+        result = QDateTime(date, time, QTimeZone::UTC);
     } else if (mType == GeneralizedTimeType && mValue.size() == 15) {
-        result = QDateTime::fromString(QString::fromLatin1(mValue),
-                                       QStringLiteral("yyyyMMddHHmmsst"));
+        result = QDateTime::fromString(QString::fromLatin1(mValue), u"yyyyMMddHHmmsst");
     }
 
     return result;
diff --git a/src/plugins/tls/shared/qsslsocket_mac_shared.cpp b/src/plugins/tls/shared/qsslsocket_mac_shared.cpp
index f40d2fb770..1257240ee2 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).");
+                    }
                 }
             }
         }
diff --git a/src/plugins/tls/shared/qwincrypt_p.h b/src/plugins/tls/shared/qwincrypt_p.h
index 1b1f0f16c0..48ca4247fa 100644
--- a/src/plugins/tls/shared/qwincrypt_p.h
+++ b/src/plugins/tls/shared/qwincrypt_p.h
@@ -40,6 +40,16 @@ struct QHCertStoreDeleter {
 // A simple RAII type used by Schannel code and Window CA fetcher class:
 using QHCertStorePointer = std::unique_ptr<void, QHCertStoreDeleter>;
 
+struct QPCCertContextDeleter {
+    void operator()(PCCERT_CONTEXT context) const
+    {
+        CertFreeCertificateContext(context);
+    }
+};
+
+// A simple RAII type used by Schannel code
+using QPCCertContextPointer = std::unique_ptr<const CERT_CONTEXT, QPCCertContextDeleter>;
+
 QT_END_NAMESPACE
 
 #endif // QWINCRYPT_P_H
diff --git a/src/plugins/tls/shared/qx509_generic.cpp b/src/plugins/tls/shared/qx509_generic.cpp
index cfe2786680..5006db1a72 100644
--- a/src/plugins/tls/shared/qx509_generic.cpp
+++ b/src/plugins/tls/shared/qx509_generic.cpp
@@ -118,7 +118,7 @@ QList<QSslCertificate> X509CertificateGeneric::certificatesFromPem(const QByteAr
 
         QByteArray decoded = QByteArray::fromBase64(
             QByteArray::fromRawData(pem.data() + startPos, endPos - startPos));
-        certificates << certificatesFromDer(decoded, 1);;
+        certificates << certificatesFromDer(decoded, 1);
     }
 
     return certificates;
@@ -188,7 +188,7 @@ bool X509CertificateGeneric::parse(const QByteArray &data)
     if (!elem.read(certStream) || elem.type() != QAsn1Element::SequenceType)
         return false;
 
-    QByteArray issuerDer = data.mid(dataStream.device()->pos() - elem.value().length(), elem.value().length());
+    QByteArray issuerDer = data.mid(dataStream.device()->pos() - elem.value().size(), elem.value().size());
     issuerInfoEntries = elem.toInfo();
 
     // validity period
@@ -215,7 +215,7 @@ bool X509CertificateGeneric::parse(const QByteArray &data)
     if (!elem.read(certStream) || elem.type() != QAsn1Element::SequenceType)
         return false;
 
-    QByteArray subjectDer = data.mid(dataStream.device()->pos() - elem.value().length(), elem.value().length());
+    QByteArray subjectDer = data.mid(dataStream.device()->pos() - elem.value().size(), elem.value().size());
     subjectInfoEntries = elem.toInfo();
     subjectMatchesIssuer = issuerDer == subjectDer;
 
@@ -285,7 +285,7 @@ bool X509CertificateGeneric::parse(const QByteArray &data)
                                 case QAsn1Element::IpAddressType: {
                                     QHostAddress ipAddress;
                                     QByteArray ipAddrValue = nameElem.value();
-                                    switch (ipAddrValue.length()) {
+                                    switch (ipAddrValue.size()) {
                                     case 4: // IPv4
                                         ipAddress = QHostAddress(qFromBigEndian(*reinterpret_cast<quint32 *>(ipAddrValue.data())));
                                         break;