From 0e2c16e827d6d860dcb92aa7d4af02090ec7d40e Mon Sep 17 00:00:00 2001
From: ckamm <mail@ckamm.de>
Date: Fri, 27 May 2016 12:08:42 +0200
Subject: [PATCH] Certs: Re-ask for different cert after rejection #4898
 (#4911)

Previously rejecting any kind of certificate meant that the user
was never asked again, even if the certificate changed.

Now we keep track of which certificates were rejected and ask again
if the ones mentioned in the ssl errors change.

mitmproxy is excellent for testing this.
---
 src/gui/owncloudsetupwizard.cpp |  2 +-
 src/libsync/account.cpp         | 23 +++++++++++++++++------
 src/libsync/account.h           |  7 +++++--
 3 files changed, 23 insertions(+), 9 deletions(-)

diff --git a/src/gui/owncloudsetupwizard.cpp b/src/gui/owncloudsetupwizard.cpp
index 8f97188130..adc24f25b2 100644
--- a/src/gui/owncloudsetupwizard.cpp
+++ b/src/gui/owncloudsetupwizard.cpp
@@ -173,7 +173,7 @@ void OwncloudSetupWizard::slotNoOwnCloudFoundAuth(QNetworkReply *reply)
 
     // Allow the credentials dialog to pop up again for the same URL.
     // Maybe the user just clicked 'Cancel' by accident or changed his mind.
-    _ocWizard->account()->resetSslCertErrorState();
+    _ocWizard->account()->resetRejectedCertificates();
 }
 
 void OwncloudSetupWizard::slotNoOwnCloudFoundAuthTimeout(const QUrl&url)
diff --git a/src/libsync/account.cpp b/src/libsync/account.cpp
index 75a1f2a21a..1fbd751cb4 100644
--- a/src/libsync/account.cpp
+++ b/src/libsync/account.cpp
@@ -40,7 +40,6 @@ Account::Account(QObject *parent)
     , _capabilities(QVariantMap())
     , _am(0)
     , _credentials(0)
-    , _treatSslErrorsAsFailure(false)
     , _davPath( Theme::instance()->webDavPath() )
     , _wasMigrated(false)
 {
@@ -329,9 +328,9 @@ void Account::addApprovedCerts(const QList<QSslCertificate> certs)
     _approvedCerts += certs;
 }
 
-void Account::resetSslCertErrorState()
+void Account::resetRejectedCertificates()
 {
-    _treatSslErrorsAsFailure = false;
+    _rejectedCertificates.clear();
 }
 
 void Account::setSslErrorHandler(AbstractSslErrorHandler *handler)
@@ -412,8 +411,15 @@ void Account::slotHandleSslErrors(QNetworkReply *reply , QList<QSslError> errors
                      << error.errorString() << "("<< error.error() << ")" << "\n";
     }
 
-    if( _treatSslErrorsAsFailure ) {
-        // User decided once not to trust. Honor this decision.
+    bool allPreviouslyRejected = true;
+    foreach (const QSslError &error, errors) {
+        if (!_rejectedCertificates.contains(error.certificate())) {
+            allPreviouslyRejected = false;
+        }
+    }
+
+    // If all certs have previously been rejected by the user, don't ask again.
+    if( allPreviouslyRejected ) {
         qDebug() << out << "Certs not trusted by user decision, returning.";
         return;
     }
@@ -436,7 +442,12 @@ void Account::slotHandleSslErrors(QNetworkReply *reply , QList<QSslError> errors
         // certificate changes.
         reply->ignoreSslErrors(errors);
     } else {
-        _treatSslErrorsAsFailure = true;
+        // Mark all involved certificates as rejected, so we don't ask the user again.
+        foreach (const QSslError &error, errors) {
+            if (!_rejectedCertificates.contains(error.certificate())) {
+                _rejectedCertificates.append(error.certificate());
+            }
+        }
         // if during normal operation, a new certificate was MITM'ed, and the user does not
         // ACK it, the running request must be aborted and the QNAM must be reset, to not
         // treat the new cert as granted. See bug #3283
diff --git a/src/libsync/account.h b/src/libsync/account.h
index dc4240ca83..03af4fb13c 100644
--- a/src/libsync/account.h
+++ b/src/libsync/account.h
@@ -133,7 +133,7 @@ public:
     // Usually when a user explicitly rejects a certificate we don't
     // ask again. After this call, a dialog will again be shown when
     // the next unknown certificate is encountered.
-    void resetSslCertErrorState();
+    void resetRejectedCertificates();
 
     // pluggable handler
     void setSslErrorHandler(AbstractSslErrorHandler *handler);
@@ -216,7 +216,10 @@ private:
     QuotaInfo *_quotaInfo;
     QNetworkAccessManager *_am;
     AbstractCredentials* _credentials;
-    bool _treatSslErrorsAsFailure;
+
+    /// Certificates that were explicitly rejected by the user
+    QList<QSslCertificate> _rejectedCertificates;
+
     static QString _configFileName;
     QByteArray _pemCertificate; 
     QString _pemPrivateKey;