SecureTransport - fix verifyPeerTrust for the renegotiation case

Also remove unneeded and now wrong check (was marked with TODO)
which was a copy and paste from OpenSSL counterpart. There, testing
if peerCertificateChain.isEmpty() makes sense, since there we
potentially call storePeerCertificates() twice during the handshake.

Change-Id: I946e6876adb3f9504e93c06ac90ff36dd44aca4c
Reviewed-by: Mårten Nordheim <marten.nordheim@qt.io>
Reviewed-by: Edward Welbourne <edward.welbourne@qt.io>

1 files changed, 23 insertions, 22 deletions

diff --git a/src/network/ssl/qsslsocket_mac.cpp b/src/network/ssl/qsslsocket_mac.cpp
index 8aa01deee7..bcc65874bd 100644
--- a/src/network/ssl/qsslsocket_mac.cpp
+++ b/src/network/ssl/qsslsocket_mac.cpp
@@ -1239,29 +1239,30 @@ bool QSslSocketBackendPrivate::verifyPeerTrust()
     QList<QSslError> errors;
     // store certificates
     const int certCount = SecTrustGetCertificateCount(trust);
-    // TODO: why this test depends on configuration.peerCertificateChain not being empty????
-    if (configuration.peerCertificateChain.isEmpty()) {
-        // Apple's docs say SetTrustEvaluate must be called before
-        // SecTrustGetCertificateAtIndex, but this results
-        // in 'kSecTrustResultRecoverableTrustFailure', so
-        // here we just ignore 'res' (later we'll use SetAnchor etc.
-        // and evaluate again).
-        SecTrustResultType res = kSecTrustResultInvalid;
-        err = SecTrustEvaluate(trust, &res);
-        if (err != errSecSuccess) {
-            // We can not ignore this, it's not even about trust verification
-            // probably ...
-            setErrorAndEmit(QAbstractSocket::SslHandshakeFailedError,
-                            QStringLiteral("SecTrustEvaluate failed: %1").arg(err));
-            plainSocket->disconnectFromHost();
-            return false;
-        }
 
-        for (int i = 0; i < certCount; ++i) {
-            SecCertificateRef cert = SecTrustGetCertificateAtIndex(trust, i);
-            QCFType<CFDataRef> derData = SecCertificateCopyData(cert);
-            configuration.peerCertificateChain << QSslCertificate(QByteArray::fromCFData(derData), QSsl::Der);
-        }
+    // Apple's docs say SetTrustEvaluate must be called before
+    // SecTrustGetCertificateAtIndex, but this results
+    // in 'kSecTrustResultRecoverableTrustFailure', so
+    // here we just ignore 'res' (later we'll use SetAnchor etc.
+    // and evaluate again).
+    SecTrustResultType res = kSecTrustResultInvalid;
+    err = SecTrustEvaluate(trust, &res);
+    if (err != errSecSuccess) {
+        // We can not ignore this, it's not even about trust verification
+        // probably ...
+        setErrorAndEmit(QAbstractSocket::SslHandshakeFailedError,
+                        QStringLiteral("SecTrustEvaluate failed: %1").arg(err));
+        plainSocket->disconnectFromHost();
+        return false;
+    }
+
+    configuration.peerCertificate.clear();
+    configuration.peerCertificateChain.clear();
+
+    for (int i = 0; i < certCount; ++i) {
+        SecCertificateRef cert = SecTrustGetCertificateAtIndex(trust, i);
+        QCFType<CFDataRef> derData = SecCertificateCopyData(cert);
+        configuration.peerCertificateChain << QSslCertificate(QByteArray::fromCFData(derData), QSsl::Der);
     }
 
     if (certCount > 0) {