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.

diff --git a/src/gui/owncloudsetupwizard.cpp b/src/gui/owncloudsetupwizard.cpp
index 8f97188130ec1e4777ad45824d66e138b99ad083..adc24f25b2e1a6cab0b9f8353bd9982a46049611 100644 (file)
--- 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 75a1f2a21a4cd1b934592939fd7681b68b892a9a..1fbd751cb4aeb681adf09d35d3d665a79a051b49 100644 (file)
--- 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 dc4240ca835fc359d6d4e94c3ed6b9722768db0c..03af4fb13ccb926a051afd84df7600afa658e0df 100644 (file)
--- 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;