Fix up SSL client certificates #5213 #69

csync/src/csync.h

@@ -44,11 +44,6 @@
 extern "C" {
 #endif
 
-struct csync_client_certs_s {
-  char *certificatePath;
-  char *certificatePasswd;
-};  
-
 enum csync_status_codes_e {
   CSYNC_STATUS_OK         = 0,
 

csync/src/csync_private.h

@@ -103,9 +103,6 @@ struct csync_s {
 
   } callbacks;
   c_strlist_t *excludes;
-
-  // needed for SSL client certificate support
-  struct csync_client_certs_s *clientCerts;
 
   struct {
     char *file;

src/cmd/cmd.cpp

@@ -121,7 +121,7 @@ QString queryPassword(const QString &user)
 class HttpCredentialsText : public HttpCredentials {
 public:
     HttpCredentialsText(const QString& user, const QString& password)
-        : HttpCredentials(user, password, "", ""), // FIXME: not working with client certs yet (qknight)
+        : HttpCredentials(user, password), // FIXME: not working with client certs yet (qknight)
           _sslTrusted(false)
     {}
 

src/gui/CMakeLists.txt

@@ -151,7 +151,6 @@ set(3rdparty_SRC
     ../3rdparty/qtsingleapplication/qtlocalpeer.cpp
     ../3rdparty/qtsingleapplication/qtsingleapplication.cpp
     ../3rdparty/qtsingleapplication/qtsinglecoreapplication.cpp
-    ../3rdparty/certificates/p12topem.cpp
    )
 
 if (APPLE)

src/gui/accountmanager.cpp

@@ -237,7 +237,7 @@ AccountPtr AccountManager::loadAccountHelper(QSettings& settings)
 
     acc->setCredentials(CredentialsFactory::create(authType));
 
-    // now the cert, it is in the general group
+    // now the server cert, it is in the general group
     settings.beginGroup(QLatin1String("General"));
     acc->setApprovedCerts(QSslCertificate::fromData(settings.value(caCertsKeyC).toByteArray()));
     settings.endGroup();

src/gui/addcertificatedialog.ui

@@ -10,7 +10,7 @@
     <x>0</x>
     <y>0</y>
     <width>462</width>
-    <height>186</height>
+    <height>188</height>
    </rect>
   </property>
   <property name="windowTitle">
@@ -32,7 +32,7 @@
      <item row="0" column="0">
       <widget class="QLabel" name="labelCertificateFile">
        <property name="text">
-        <string>Certificate :</string>
+        <string>Certificate &amp; Key (pkcs12) :</string>
        </property>
       </widget>
      </item>

src/gui/creds/httpcredentialsgui.h

@@ -27,7 +27,7 @@ class HttpCredentialsGui : public HttpCredentials {
     Q_OBJECT
 public:
     explicit HttpCredentialsGui() : HttpCredentials() {}
-    HttpCredentialsGui(const QString& user, const QString& password, const QString& certificatePath, const QString& certificatePasswd) : HttpCredentials(user, password, certificatePath, certificatePasswd) {}
+    HttpCredentialsGui(const QString& user, const QString& password, const QSslCertificate& certificate, const QSslKey& key) : HttpCredentials(user, password, certificate, key) {}
     void askFromUser() Q_DECL_OVERRIDE;
     Q_INVOKABLE void askFromUserAsync();
 

src/gui/wizard/owncloudconnectionmethoddialog.cpp

@@ -29,8 +29,11 @@ OwncloudConnectionMethodDialog::OwncloudConnectionMethodDialog(QWidget *parent)
     connect(ui->btnClientSideTLS, SIGNAL(clicked(bool)), this, SLOT(returnClientSideTLS()));
     connect(ui->btnBack, SIGNAL(clicked(bool)), this, SLOT(returnBack()));
 
-    // DM: TLS Client Cert GUI support disabled for now
+
+#if QT_VERSION < QT_VERSION_CHECK(5, 4, 0)
+    // We support only from Qt 5.4.x because of https://doc.qt.io/qt-5/qsslcertificate.html#importPkcs12
     ui->btnClientSideTLS->hide();
+#endif
 }
 
 void OwncloudConnectionMethodDialog::setUrl(const QUrl &url)

src/gui/wizard/owncloudhttpcredspage.cpp

@@ -192,7 +192,7 @@ void OwncloudHttpCredsPage::setErrorString(const QString& err)
 
 AbstractCredentials* OwncloudHttpCredsPage::getCredentials() const
 {
-    return new HttpCredentialsGui(_ui.leUsername->text(), _ui.lePassword->text(), _ocWizard->ownCloudCertificatePath, _ocWizard->ownCloudCertificatePasswd);
+    return new HttpCredentialsGui(_ui.leUsername->text(), _ui.lePassword->text(), _ocWizard->_clientSslCertificate, _ocWizard->_clientSslKey);
 }
 
 

src/gui/wizard/owncloudsetuppage.cpp

@@ -21,13 +21,13 @@
 #include <QMessageBox>
 #include <QSsl>
 #include <QSslCertificate>
+#include <QNetworkAccessManager>
 
 #include "QProgressIndicator.h"
 
 #include "wizard/owncloudwizardcommon.h"
 #include "wizard/owncloudsetuppage.h"
 #include "wizard/owncloudconnectionmethoddialog.h"
-#include "../3rdparty/certificates/p12topem.h"
 #include "theme.h"
 #include "account.h"
 
@@ -71,7 +71,6 @@ OwncloudSetupPage::OwncloudSetupPage(QWidget *parent)
     connect(_ui.leUrl, SIGNAL(editingFinished()), SLOT(slotUrlEditFinished()));
 
     addCertDial = new AddCertificateDialog(this);
-    connect(_ocWizard,SIGNAL(needCertificate()),this,SLOT(slotAskSSLClientCertificate()));
 }
 
 void OwncloudSetupPage::setServerUrl( const QString& newUrl )
@@ -269,7 +268,10 @@ void OwncloudSetupPage::setErrorString( const QString& err, bool retryHTTPonly )
                     }
                     break;
                 case OwncloudConnectionMethodDialog::Client_Side_TLS:
-                    slotAskSSLClientCertificate();
+#if QT_VERSION >= QT_VERSION_CHECK(5, 4, 0)
+                    addCertDial->show();
+                    connect(addCertDial, SIGNAL(accepted()),this,SLOT(slotCertificateAccepted()));
+#endif
                     break;
                 case OwncloudConnectionMethodDialog::Closed:
                 case OwncloudConnectionMethodDialog::Back:
@@ -302,12 +304,6 @@ void OwncloudSetupPage::stopSpinner()
     _progressIndi->stopAnimation();
 }
 
-void OwncloudSetupPage::slotAskSSLClientCertificate()
-{
-    addCertDial->show();
-    connect(addCertDial, SIGNAL(accepted()),this,SLOT(slotCertificateAccepted()));
-}
-
 QString subjectInfoHelper(const QSslCertificate& cert, const QByteArray &qa)
 {
 #if QT_VERSION < QT_VERSION_CHECK(5,0,0)
@@ -320,36 +316,40 @@ QString subjectInfoHelper(const QSslCertificate& cert, const QByteArray &qa)
 //called during the validation of the client certificate.
 void OwncloudSetupPage::slotCertificateAccepted()
 {
-    QSslCertificate sslCertificate;
+#if QT_VERSION >= QT_VERSION_CHECK(5, 4, 0)
+    QList<QSslCertificate> clientCaCertificates;
+    QFile certFile(addCertDial->getCertificatePath());
+    certFile.open(QFile::ReadOnly);
+    if(QSslCertificate::importPkcs12(&certFile,
+                                         &_ocWizard->_clientSslKey, &_ocWizard->_clientSslCertificate,
+                                            &clientCaCertificates,
+                                            addCertDial->getCertificatePasswd().toLocal8Bit())){
+        AccountPtr acc = _ocWizard->account();
 
-    resultP12ToPem certif = p12ToPem(addCertDial->getCertificatePath().toStdString() , addCertDial->getCertificatePasswd().toStdString());
-    if(certif.ReturnCode){
-        QString s = QString::fromStdString(certif.Certificate);
-        QByteArray ba = s.toLocal8Bit();
+        // to re-create the session ticket because we added a key/cert
+        acc->setSslConfiguration(QSslConfiguration());
+        QSslConfiguration sslConfiguration = acc->getOrCreateSslConfig();
 
-        QList<QSslCertificate> sslCertificateList = QSslCertificate::fromData(ba, QSsl::Pem);
-        sslCertificate = sslCertificateList.takeAt(0);
+        // We're stuffing the certificate into the configuration form here. Later the
+        // cert will come via the HttpCredentials
+        sslConfiguration.setLocalCertificate(_ocWizard->_clientSslCertificate);
+        sslConfiguration.setPrivateKey(_ocWizard->_clientSslKey);
+        acc->setSslConfiguration(sslConfiguration);
 
-        _ocWizard->ownCloudCertificate = ba;
-        _ocWizard->ownCloudPrivateKey = certif.PrivateKey.c_str();
-        _ocWizard->ownCloudCertificatePath = addCertDial->getCertificatePath();
-        _ocWizard->ownCloudCertificatePasswd = addCertDial->getCertificatePasswd();
+        // Make sure TCP connections get re-established
+        acc->networkAccessManager()->clearAccessCache();
 
-        AccountPtr acc = _ocWizard->account();
-        acc->setCertificate(_ocWizard->ownCloudCertificate, _ocWizard->ownCloudPrivateKey);
-        addCertDial->reinit();
+        addCertDial->reinit(); // FIXME: Why not just have this only created on use?
         validatePage();
     } else {
-        QString message;
-        message = certif.Comment.c_str();
-        addCertDial->showErrorMessage(message);
+        addCertDial->showErrorMessage("Could not load certificate");
         addCertDial->show();
     }
+#endif
 }
 
 OwncloudSetupPage::~OwncloudSetupPage()
 {
-    delete addCertDial;
 }
 
 } // namespace OCC

src/gui/wizard/owncloudsetuppage.h

@@ -59,7 +59,6 @@ public slots:
   void setErrorString( const QString&, bool retryHTTPonly );
   void startSpinner();
   void stopSpinner();
-  void slotAskSSLClientCertificate();
   void slotCertificateAccepted();
 
 protected slots:

src/gui/wizard/owncloudwizard.cpp

@@ -224,11 +224,4 @@ AbstractCredentials* OwncloudWizard::getCredentials() const
   return 0;
 }
 
-// outputs the signal needed to authenticate a certificate
-void OwncloudWizard::raiseCertificatePopup()
-{
-    emit needCertificate();
-}
-
-
 } // end namespace